From 009415e163357ce57143ac18ddfd1738c6ee4d5f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 22 Sep 2026 11:18:02 +0000 Subject: [PATCH] fix(harvest): validate identity maps before writing identity.json Load and reject duplicate handles before mkdir/copy so a bad map writes nothing. Declare warnings before pushing empty-handle notices. Emit harvest receipts through createReceipt with entries after the contract fields. --- src/commands/harvest.mjs | 84 ++++++++++++++++++++-------- tests/gates/baseline/audit-tests.txt | 2 - tests/identity.test.mjs | 38 +++++++++++++ 3 files changed, 98 insertions(+), 26 deletions(-) diff --git a/src/commands/harvest.mjs b/src/commands/harvest.mjs index bdc6fffb..2367fd65 100644 --- a/src/commands/harvest.mjs +++ b/src/commands/harvest.mjs @@ -14,6 +14,7 @@ import { copyFileSync, existsSync, mkdirSync, readFileSync, readdirSync, statSyn import { basename, dirname, extname, join, relative, resolve } from "node:path"; import { register } from "./index.mjs"; +import { createReceipt } from "../cli/receipt.mjs"; import { KnowledgeStore, sha256Hex } from "../knowledge/store.mjs"; import { loadLedger, recordDocument, saveLedger, ledgerStats } from "../knowledge/ledger.mjs"; import { IDENTITY_FILE, applyIdentity, loadIdentity } from "../knowledge/identity.mjs"; @@ -147,48 +148,85 @@ register("harvest", { const parsed = parseHarvestArgs(argv); if (parsed.error) { return { - receipt: { command: "harvest", person: null, ok: false, inputs: [], outputs: [], anchors: { total: 0, cited: 0 }, warnings: [], unavailable: [], error: { code: "harvest/usage", message: parsed.error, remedy: "distilly harvest --help" } }, + receipt: createReceipt("harvest", { + ok: false, + error: { code: "harvest/usage", message: parsed.error, remedy: "distilly harvest --help" }, + }), exitCode: 2, }; } const { options, paths } = parsed; if (!options.person) { return { - receipt: { command: "harvest", person: null, ok: false, inputs: [], outputs: [], anchors: { total: 0, cited: 0 }, warnings: [], unavailable: [], error: { code: "harvest/usage", message: "--person is required", remedy: "distilly harvest --help" } }, + receipt: createReceipt("harvest", { + ok: false, + error: { code: "harvest/usage", message: "--person is required", remedy: "distilly harvest --help" }, + }), exitCode: 2, }; } const personDir = join(resolve(options.baseDir), "skills", "colleague", options.person); - const store = new KnowledgeStore(personDir); - const ledger = loadLedger(store); - // The map travels with the Skill: `harvest --identity ` installs a copy as - // `identity.json` so later imports (and the derivation) see the same mapping. - let identityPath = null; + const warnings = []; + let identity = { path: null, map: new Map(), people: [], warnings: [] }; + // Validate the map before mkdir/copy so a duplicate handle writes nothing. if (options.identity) { const source = resolve(options.identity); if (!existsSync(source)) { return { - receipt: { command: "harvest", person: options.person, ok: false, inputs: [], outputs: [], anchors: { total: 0, cited: 0 }, warnings: [], unavailable: [], error: { code: "harvest/identity", message: `--identity file not found: ${options.identity}`, remedy: "pass a JSON map with a \"people\" array" } }, + receipt: createReceipt("harvest", { + person: options.person, + ok: false, + error: { + code: "harvest/identity", + message: `--identity file not found: ${options.identity}`, + remedy: 'pass a JSON map with a "people" array', + }, + }), + exitCode: 2, + }; + } + try { + identity = loadIdentity(personDir, { path: source }); + } catch (error) { + return { + receipt: createReceipt("harvest", { + person: options.person, + ok: false, + error: { + code: "harvest/identity", + message: error.message, + remedy: "fix identity.json; a handle may only belong to one person", + }, + }), exitCode: 2, }; } mkdirSync(personDir, { recursive: true }); copyFileSync(source, join(personDir, IDENTITY_FILE)); - identityPath = join(personDir, IDENTITY_FILE); - } - let identity = { path: null, map: new Map(), people: [], warnings: [] }; - try { - identity = loadIdentity(personDir, identityPath ? { path: identityPath } : {}); - } catch (error) { - return { - receipt: { command: "harvest", person: options.person, ok: false, inputs: [], outputs: [], anchors: { total: 0, cited: 0 }, warnings: [], unavailable: [], error: { code: "harvest/identity", message: error.message, remedy: "fix identity.json; a handle may only belong to one person" } }, - exitCode: 2, - }; + identity.path = join(personDir, IDENTITY_FILE); + } else { + try { + identity = loadIdentity(personDir); + } catch (error) { + return { + receipt: createReceipt("harvest", { + person: options.person, + ok: false, + error: { + code: "harvest/identity", + message: error.message, + remedy: "fix identity.json; a handle may only belong to one person", + }, + }), + exitCode: 2, + }; + } } for (const warning of identity.warnings ?? []) warnings.push(warning); - const warnings = []; + const store = new KnowledgeStore(personDir); + const ledger = loadLedger(store); const inputs = []; const entries = []; const files = []; @@ -231,17 +269,15 @@ register("harvest", { { path: relative(process.cwd(), store.ledgerPath), sha256: sha256Hex(readFileSync(store.ledgerPath)), bytes: statSync(store.ledgerPath).size }, ]; - const receipt = { - command: "harvest", + const receipt = createReceipt("harvest", { person: options.person, ok: warnings.filter((warning) => warning.startsWith("unsupported")).length === 0, inputs, outputs, anchors: { total: stats.anchors ?? stats.anchorCount ?? 0, cited: 0 }, - entries, warnings, - unavailable: [], - }; + }); + receipt.entries = entries; if (!json) { reporter.line(`harvest: ${entries.filter((entry) => entry.appended).length}/${files.length} recorded, ${ledger.length} ledger entries`); for (const entry of entries) reporter.line(` ${entry.id} ${entry.source}${entry.appended ? "" : " (identical bytes already recorded)"}`); diff --git a/tests/gates/baseline/audit-tests.txt b/tests/gates/baseline/audit-tests.txt index 6aead1cb..05bdb68b 100644 --- a/tests/gates/baseline/audit-tests.txt +++ b/tests/gates/baseline/audit-tests.txt @@ -10,8 +10,6 @@ R08 `collect feishu --mode browser` (SKILL.md 迁移对照表) reaches the brows R08 `collect feishu --mode mcp` (SKILL.md 迁移对照表: feishu_mcp_client.py → collect feishu) reaches the MCP route R08 a contract channel that is not shipped is reported as planned (PR \#185 正文: collect/planned-channel), never as an unknown channel R56 `blind-test prepare` works with the relative --out the handoff (§8) and the blind-test README use -R21 identity.json: a handle claimed twice exits 2 with the exact message and writes nothing -R21 identity.json with a person that lists no handles is a warning, not a crash R28 a document's labels (根因：/ 时间线：) are not counted as participants in a multi-source person R24 Step 1.5: `doctor --require-shape` run after Collect and before Derive fails the C-SPAN floor corpus R49 doctor fails when views/.view.json cites an anchor the ledger does not declare diff --git a/tests/identity.test.mjs b/tests/identity.test.mjs index 1b789bca..b841585f 100644 --- a/tests/identity.test.mjs +++ b/tests/identity.test.mjs @@ -129,6 +129,44 @@ test("a bad map fails loudly and changes nothing", () => { } }); +test("a duplicate handle exits 2 and writes no files", () => { + const box = sandbox(); + try { + const dup = join(box.root, "dup.json"); + writeFileSync( + dup, + `${JSON.stringify({ people: [{ name: "A", handles: ["x"] }, { name: "B", handles: ["x"] }] })}\n`, + "utf8", + ); + const clash = runCli(["harvest", CORPUS, "--person", "lin-gong", "--base-dir", box.root, "--identity", dup, "--json"]); + assert.equal(clash.status, 2, clash.stderr); + const refused = parseReceipt(clash.stdout); + assert.equal(refused.error.code, "harvest/identity"); + assert.equal(refused.error.message, 'identity.json: handle "x" is claimed by both "A" and "B"'); + assert.equal(existsSync(box.personDir), false, "a duplicate handle must not create the Skill directory"); + } finally { + rmSync(box.root, { recursive: true, force: true }); + } +}); + +test("a person with no handles is a warning and harvest still records", () => { + const box = sandbox(); + try { + const map = join(box.root, "no-handles.json"); + writeFileSync(map, `${JSON.stringify({ people: [{ name: "林工" }] })}\n`, "utf8"); + const harvested = runCli(["harvest", CORPUS, "--person", "lin-gong", "--base-dir", box.root, "--identity", map, "--json"]); + assert.equal(harvested.status, 0, harvested.stderr); + const body = parseReceipt(harvested.stdout); + assert.equal(body.ok, true); + assert.ok( + body.warnings.includes('"林工" lists no handles; only its own name is canonicalised'), + `warnings=${JSON.stringify(body.warnings)}`, + ); + } finally { + rmSync(box.root, { recursive: true, force: true }); + } +}); + test("the helpers are pure and the map keeps a record of its own decisions", () => { const identity = { map: new Map([["ou_lin", "林工"]]) }; assert.equal(canonicalSpeaker("ou_lin", identity), "林工");