Skip to content

Commit dc92a17

Browse files
committed
fix(import-markdown): address review comments from PR #589
Addressing rwmjhb's CHANGES_REQUESTED review: - MR1: catch 改為僅處理 EISDIR,其他 I/O 錯誤(權限、損壞)重新拋出 - F2: foundFiles++ 移至 stat + isFile() 檢查之後,確保只計入實際檔案 - MR2: import-markdown CI entry 移至 cli-smoke 之前,確保 cli-smoke 失敗時仍會執行 - F1: 修復 vacuously-true 斷言 assert.ok(skipped >= 0) → assert.strictEqual(skipped, 1)
1 parent dd7f70b commit dc92a17

File tree

4 files changed

+14
-15
lines changed

4 files changed

+14
-15
lines changed

cli.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,6 @@ export async function runImportMarkdown(
658658

659659
// Parse each file for memory entries (lines starting with "- ")
660660
for (const { filePath, scope: discoveredScope } of mdFiles) {
661-
foundFiles++;
662661
let content: string;
663662
try {
664663
const stats = await fsPromises.stat(filePath);
@@ -668,10 +667,19 @@ export async function runImportMarkdown(
668667
skipped++;
669668
continue;
670669
}
670+
foundFiles++; // count only actual files, not directories
671671
content = await fsPromises.readFile(filePath, "utf-8");
672672
} catch (err) {
673-
console.warn(` [skip] unreadable: ${filePath}${err}`);
674-
skipped++;
673+
const e = err as NodeJS.ErrnoException;
674+
if (e.code === "EISDIR") {
675+
// .md directory — already handled above by isFile() check, but catch
676+
// this explicitly so genuine I/O errors (permissions, corruption)
677+
// are not silently swallowed
678+
console.warn(` [skip] not a file: ${filePath}`);
679+
skipped++;
680+
} else {
681+
throw err; // re-throw genuine I/O errors
682+
}
675683
continue;
676684
}
677685
(fix(import-markdown): CI測試登記 + .md目錄skip保護)

scripts/ci-test-manifest.mjs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ export const CI_TEST_MANIFEST = [
1818
{ group: "core-regression", runner: "node", file: "test/recall-text-cleanup.test.mjs", args: ["--test"] },
1919
{ group: "storage-and-schema", runner: "node", file: "test/update-consistency-lancedb.test.mjs" },
2020
{ group: "core-regression", runner: "node", file: "test/strip-envelope-metadata.test.mjs", args: ["--test"] },
21+
{ group: "cli-smoke", runner: "node", file: "test/import-markdown/import-markdown.test.mjs", args: ["--test"] },
2122
{ group: "cli-smoke", runner: "node", file: "test/cli-smoke.mjs" },
2223
{ group: "cli-smoke", runner: "node", file: "test/functional-e2e.mjs" },
23-
{ group: "cli-smoke", runner: "node", file: "test/import-markdown/import-markdown.test.mjs", args: ["--test"] },
2424
{ group: "core-regression", runner: "node", file: "test/retriever-rerank-regression.mjs" },
2525
{ group: "core-regression", runner: "node", file: "test/smart-memory-lifecycle.mjs" },
2626
{ group: "core-regression", runner: "node", file: "test/smart-extractor-branches.mjs" },
27-
(fix(import-markdown): CI測試登記 + .md目錄skip保護)
2827
{ group: "packaging-and-workflow", runner: "node", file: "test/plugin-manifest-regression.mjs" },
2928
{ group: "core-regression", runner: "node", file: "test/session-summary-before-reset.test.mjs", args: ["--test"] },
3029
{ group: "packaging-and-workflow", runner: "node", file: "test/sync-plugin-version.test.mjs", args: ["--test"] },
@@ -42,16 +41,12 @@ export const CI_TEST_MANIFEST = [
4241
{ group: "storage-and-schema", runner: "node", file: "test/cross-process-lock.test.mjs", args: ["--test"] },
4342
{ group: "core-regression", runner: "node", file: "test/preference-slots.test.mjs", args: ["--test"] },
4443
{ group: "core-regression", runner: "node", file: "test/is-latest-auto-supersede.test.mjs" },
45-
<<<<<<< HEAD
4644
{ group: "core-regression", runner: "node", file: "test/hook-dedup-phase1.test.mjs", args: ["--test"] },
4745
{ group: "core-regression", runner: "node", file: "test/temporal-awareness.test.mjs", args: ["--test"] },
4846
// Issue #598 regression tests
4947
{ group: "core-regression", runner: "node", file: "test/store-serialization.test.mjs" },
5048
{ group: "core-regression", runner: "node", file: "test/access-tracker-retry.test.mjs" },
5149
{ group: "core-regression", runner: "node", file: "test/embedder-cache.test.mjs" },
52-
=======
53-
{ group: "core-regression", runner: "node", file: "test/temporal-awareness.test.mjs", args: ["--test"] },
54-
>>>>>>> 1a83d72 (fix(import-markdown): CI測試登記 + .md目錄skip保護)
5550
];
5651

5752
export function getEntriesForGroup(group) {

scripts/verify-ci-test-manifest.mjs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ const EXPECTED_BASELINE = [
1919
{ group: "core-regression", runner: "node", file: "test/recall-text-cleanup.test.mjs", args: ["--test"] },
2020
{ group: "storage-and-schema", runner: "node", file: "test/update-consistency-lancedb.test.mjs" },
2121
{ group: "core-regression", runner: "node", file: "test/strip-envelope-metadata.test.mjs", args: ["--test"] },
22+
{ group: "cli-smoke", runner: "node", file: "test/import-markdown/import-markdown.test.mjs", args: ["--test"] },
2223
{ group: "cli-smoke", runner: "node", file: "test/cli-smoke.mjs" },
2324
{ group: "cli-smoke", runner: "node", file: "test/functional-e2e.mjs" },
24-
{ group: "cli-smoke", runner: "node", file: "test/import-markdown/import-markdown.test.mjs", args: ["--test"] },
2525
{ group: "core-regression", runner: "node", file: "test/retriever-rerank-regression.mjs" },
2626
{ group: "core-regression", runner: "node", file: "test/smart-memory-lifecycle.mjs" },
2727
{ group: "core-regression", runner: "node", file: "test/smart-extractor-branches.mjs" },
28-
(fix(import-markdown): CI測試登記 + .md目錄skip保護)
2928
{ group: "packaging-and-workflow", runner: "node", file: "test/plugin-manifest-regression.mjs" },
3029
{ group: "core-regression", runner: "node", file: "test/session-summary-before-reset.test.mjs", args: ["--test"] },
3130
{ group: "packaging-and-workflow", runner: "node", file: "test/sync-plugin-version.test.mjs", args: ["--test"] },
@@ -44,13 +43,10 @@ const EXPECTED_BASELINE = [
4443
{ group: "core-regression", runner: "node", file: "test/preference-slots.test.mjs", args: ["--test"] },
4544
{ group: "core-regression", runner: "node", file: "test/is-latest-auto-supersede.test.mjs" },
4645
{ group: "core-regression", runner: "node", file: "test/temporal-awareness.test.mjs", args: ["--test"] },
47-
<<<<<<< HEAD
4846
// Issue #598 regression tests
4947
{ group: "core-regression", runner: "node", file: "test/store-serialization.test.mjs" },
5048
{ group: "core-regression", runner: "node", file: "test/access-tracker-retry.test.mjs" },
5149
{ group: "core-regression", runner: "node", file: "test/embedder-cache.test.mjs" },
52-
=======
53-
>>>>>>> 1a83d72 (fix(import-markdown): CI測試登記 + .md目錄skip保護)
5450
];
5551

5652
function fail(message) {

test/import-markdown/import-markdown.test.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ describe("import-markdown CLI", () => {
440440
});
441441
// Should have imported the real file (1 entry from "- Real file entry")
442442
assert.strictEqual(imported, 1, "should import the real .md file");
443-
assert.ok(skipped >= 0);
443+
assert.strictEqual(skipped, 1, "should skip exactly 1 (.md directory entry)");
444444
} catch (err) {
445445
threw = true;
446446
throw new Error(`Import aborted on .md directory: ${err}`);

0 commit comments

Comments
 (0)