fix(import-markdown): P0 missing return + register test in CI

[jlin53882]

Summary

Fixes two critical issues identified in PR #426 review:

1. P0: Missing return in runImportMarkdown — cli.ts non-empty path was missing return { imported, skipped, foundFiles }, causing caller to receive undefined
2. P1: Test not registered in CI — test/import-markdown/import-markdown.test.mjs was not included in scripts.test

Additional fixes discovered during implementation

• Jest → Node built-in test runner: Test file used @jest/globals but project uses node --test; all jest.fn() replaced with node:test's mock.fn()
• Jiti TypeScript compilation: Direct import("../../cli.ts") fails with ERR_MODULE_NOT_FOUND; replaced with jiti factory pattern consistent with other tests in the repo

Changes

File	Change
cli.ts	Add return { imported, skipped, foundFiles } in non-empty import path
package.json	Register test in scripts.test
test/import-markdown/import-markdown.test.mjs	Jest→Node test runner + jiti import fix
package-lock.json	Updated dependencies

Testing

All 9 tests pass:

• node --test test/import-markdown/import-markdown.test.mjs → pass 9 / fail 0

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1bc08c3c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Apply workspace-glob when scanning nested agent workspaces

The nested workspace/agents/<id> scan is gated behind if (!workspaceGlob), so passing a workspace-glob skips agent workspaces entirely. Because the earlier top-level loop only filters direct children of workspace/ (e.g., agents), a command like import-markdown <agent-id> cannot find workspace/agents/<agent-id>/MEMORY.md or memory/*.md, causing targeted imports to silently return no files in common agent-based layouts.

Useful? React with 👍 / 👎.

[jlin53882]

已開 Issue 追蹤：#484

[jlin53882]

✅ 已修復。問題出在 if (!workspaceGlob) 把整個 nested agent scan gate 住了，導致 import-markdown <agent-id> 時 workspaceGlob 有值就直接略過 agents 目錄。

修改：將 nested scan 邏輯重構，現在當有明確 workspaceGlob 時只掃描 workspace/agents/<workspaceGlob>/；無指定時掃描全部 agents。

Commit: 212e96a

[rwmjhb]

Review: fix(import-markdown): P0 missing return + register test in CI

Verdict: request-changes | Confidence: 0.95 | Value: 45%

The missing return fix and test registration are both needed. A few issues to address:

Must Fix

1. Build failure — rebase needed

Build is red (stale_base=true). Rebase onto current main to confirm the PR's changes don't interact with the failure.

2. Import-markdown test not confirmed running

Verification only ran cli-smoke.mjs. The new test/import-markdown/import-markdown.test.mjs was not executed — can't confirm the 9 test cases pass in the full pipeline.

Should Fix

3. dedupEnabled reads wrong option in action handler

// current (action handler summary)
const dedupEnabled = !!options.dryRun;  // ← should be options.dedup

The actual dedup logic inside runImportMarkdown correctly uses options.dedup, so stored data is fine — but the CLI summary line lies to the user about whether dedup was active.

4. Dedup only checks top-1 BM25 hit — can miss exact duplicates

--dedup does bm25Search(text, 1, [scope]) and checks if the top result matches exactly. BM25 is a ranked lexical search, not exact-text lookup. If a similar-but-different entry ranks above the actual duplicate, the duplicate is missed and a second copy gets imported.

Suggestion: either increase the limit (e.g. bm25Search(text, 5, ...) and check all results), or use a hash-based dedup.

5. Scope inference misses flat root-memory files

agentsList.find(a => path.normalize(a.workspace) === workspaceDir) compares the root workspace dir against per-agent paths like .openclaw/workspace/agents/theia. This never matches, so flat workspace/memory/*.md files fall back to scope: "shared" instead of the intended workspace scope.

6. CLI help text says --scope defaults to global, but code uses discoveredScope

--scope <scope> description says "default: global", but the implementation falls through to the discovered agent/workspace scope. Users omitting --scope get unexpected scope assignment.

Nit

• Duplicate summary logging (both runImportMarkdown and the action handler print a summary)
• tsx added as devDependency but test uses jiti — unused dep?
• Inconsistent indentation in runImportMarkdown body

---

Auto-reviewed by auto-pr-review-orchestrator | 7 rounds | Claude + Codex adversarial

[jlin53882]

Review 回覆：Fix 3/4/5/6 全部已處理

Fix 3 ✅ — dedupEnabled 讀錯選項

!!options.dryRun → !!options.dedup（cli.ts:655）

Fix 4 ✅ — Dedup top-1 → top-5

bm25Search(text, 1, ...) → bm25Search(text, 5, ...)（cli.ts:682）

Fix 5 ✅ — Scope fallback 從 "shared" → "global"

方向確認：

查看 src/scopes.ts 的正式定義：

• "global" — "Shared knowledge across all agents"，系統預設 scope
• "shared" — 不存在，不是正式 scope

flat root-memory (workspace/memory/*.md) 在 workspace root 層級，不在任何特定 agent 目錄下。語意上應該是「所有 agent 都能用」，對應到 "global"。

原本用 "shared" 應該是開發者直覺命名，但 "shared" 在 scope 系統沒有實際意義（也沒有任何 agent 可以讀取 scope: "shared" 的資料）。

如果你認為應該 fallback 到某個特定 agent scope（例如只有一個 agent 的情況），請讓我知道，我可以調整方向。

Fix 6 ✅ — --scope help text 修正

"default: global" → "default: auto-discovered from workspace"

[jlin53882]

Must Fix 回覆

1. Build failure — rebase needed ✅ 已處理

Branch fix/import-markdown-rebase 已 rebase 到 current origin/master，conflict（package.json）已解決，master 和 PR 的測試檔都保留。

2. Import-markdown test 未確認在 CI 執行 ✅ 已處理

測試結果：

• node --test test/import-markdown/import-markdown.test.mjs → 9/9 PASS
• 完整 npm run test 有 2 個預先存在的失敗（strip-envelope-metadata.test.mjs），與 import-markdown 無關

---

Should Fix 回覆（3/4/5/6）也已在另一個 comment 說明

以上，請確認。

[AliceLJY]

Review: PR #482 — fix(import-markdown): P0 missing return + register test in CI

Fixes two real issues from PR #426 review feedback. The extracted runImportMarkdown function is testable and the test suite covers the important edge cases.

Codex P2 assessment — "Apply workspace-glob when scanning nested agent workspaces"

Valid concern, and already addressed in the current code. The Codex comment noted that workspace-glob would skip agent workspaces because the nested scan was gated behind if (!workspaceGlob). The fix (lines ~137-153 in the diff) correctly handles both cases:

• When workspaceGlob is set: scans only the matching agent via agentEntries.find(e => e.isDirectory() && e.name === workspaceGlob)
• When not set: scans all agent workspaces

This was also filed as Issue #484, which this PR resolves.

Code review findings

1. P0 fix verified — return { imported, skipped, foundFiles } is now present at the end of the non-empty path (line 257 in diff). Without this, callers received undefined.

2. Test registration — The import-markdown.test.mjs is appended to the scripts.test chain in package.json. Correct.

3. Jest to Node test runner migration — Clean conversion from @jest/globals to node:test built-ins (describe, it, mock.fn()). Consistent with the rest of the repo.

4. Indentation inconsistency — The function body of runImportMarkdown starts with extra indentation (the first const openclawHome line at line 30 has ~8 spaces of leading whitespace, while the function signature is at the normal level). This looks like the function body was copy-pasted from a nested context (the original inline action handler). Not a blocker but worth a cosmetic cleanup.

5. Test calls runImportMarkdown(ctx, options) with 2 args but function signature takes 3 — The function signature is runImportMarkdown(ctx, workspaceGlob, options), but the tests call it as runImportMarkdown(ctx, { openclawHome, workspaceGlob, ... }), passing options as the second argument. This means workspaceGlob receives the entire options object (which is truthy), and options is undefined. The tests pass because the function internally reads workspaceGlob from the options object too (the .action handler passes it separately). Verify that the tests are actually exercising the exported function correctly — the 2-arg call signature suggests the tests may be testing a different code path than the CLI invocation.

6. process.exit(1) in a library function — runImportMarkdown calls process.exit(1) when the embedder is missing or the workspace directory is unreadable. For a function that's now exported and testable, throwing an error would be more appropriate than killing the process. The CLI action handler can catch and exit. Minor, follow-up material.

7. Missing trailing newline — package.json lost its trailing newline (\ No newline at end of file). Trivial but worth fixing.

Overall: the P0 fix and test registration are correct. Item 5 (call signature mismatch) is the main concern — please verify the tests call the function with the right arity.

LGTM with the above note.

[jlin53882]

PR #482 — P5 Codex Review Clarification + P1/P2/P3 Fixes Committed

P5 — Test arity mismatch (Codex concern): NOT a real bug

Codex noted that tests call runImportMarkdown(ctx, {options}) with 2 args but the production function takes 3 args. This is a false positive.

The test file has an intentional adapter layer at module level:

// Bottom of test file — adapter that normalizes call signature
async function runImportMarkdown(context, options = {}) {
  return importMarkdown(
    context,
    options.workspaceGlob ?? null,  // ← correctly passes as 2nd argument
    { dryRun: !!options.dryRun, scope: options.scope, ... }
  );
}

The outer test helper accepts an options-object style call and internally converts it to the correct 3-argument production signature. The adapter is specifically designed to bridge the simpler test API and the full production API. Tests are exercising the real production code path correctly. No change needed for this concern.

---

P1/P2/P3 fixes — now committed in c1127ed

The following code quality fixes have been pushed to the PR branch:

Severity	Issue	Fix
P1	process.exit(1) in runImportMarkdown	→ throw Error instead; CLI handler catches
P1	No error boundary in CLI handler	→ added try/catch wrapper
P2	bm25Search catch was silently swallowed	→ added console.warn logging
P3	Function body indentation inconsistent	→ normalized to 2-space
P3	CLI handler duplicated summary output	→ removed duplicate
P3	package.json missing trailing newline	→ added \n

Commit: c1127ed (fix/import-markdown-rebase branch)