diff --git a/bin/gstack-decision-log b/bin/gstack-decision-log index 17708980b..6ee838955 100755 --- a/bin/gstack-decision-log +++ b/bin/gstack-decision-log @@ -51,39 +51,64 @@ if (args.includes("--compact")) { process.exit(0); } +// The payload is identified by its leading `{`, not by "first non-flag arg" — a +// `--supersede '{...}'` call would otherwise mistake the target id for the payload. +const jsonArg = args.find((a) => a.trimStart().startsWith("{")); + +/** Parse + validate a decision payload. Exits 1 (nothing persisted) when it's bad. */ +function validPayload(raw: string): DecisionEvent { + let obj: Partial; + try { + obj = JSON.parse(raw); + } catch { + process.stderr.write("gstack-decision-log: invalid JSON\n"); + process.exit(1); + } + if (obj.scope === "branch" && !obj.branch) obj.branch = gitBranch(); + const res = validateDecide(obj); + if (!res.ok) { + process.stderr.write(`gstack-decision-log: ${res.error}\n`); + process.exit(1); + } + return res.event; +} + const supersedeId = flagValue(args, "--supersede"); const redactId = flagValue(args, "--redact"); if (supersedeId || redactId) { const kind = supersedeId ? "supersede" : "redact"; const targetId = (supersedeId || redactId) as string; + if (targetId.trimStart().startsWith("{")) { + process.stderr.write(`gstack-decision-log: --${kind} needs the target decision id before the replacement JSON\n`); + process.exit(1); + } + if (kind === "redact" && jsonArg) { + process.stderr.write( + "gstack-decision-log: --redact expunges and takes no replacement; log the replacement in its own call so it isn't dropped\n", + ); + process.exit(1); + } + // Validate the replacement BEFORE anything is written, then append it FIRST and + // retire the old one SECOND. Appends are individually atomic, so the only visible + // interleaving is "both active" (recoverable); the reverse order could retire the + // old decision and lose the replacement the user was recording. + const replacement = jsonArg ? { ...validPayload(jsonArg), supersedes: targetId } : undefined; + if (replacement) appendEvent(paths, replacement); appendEvent(paths, makeRefEvent(kind, targetId, { source: "agent" })); rebuildSnapshot(paths); enqueue(); - console.log(`${kind}: ${targetId}`); + console.log(replacement ? `${kind}: ${targetId} -> ${replacement.id}` : `${kind}: ${targetId}`); process.exit(0); } -const jsonArg = args.find((a) => !a.startsWith("--")); if (!jsonArg) { process.stderr.write( "gstack-decision-log: provide a JSON decision, or --supersede/--redact , or --compact\n", ); process.exit(1); } -let obj: Partial; -try { - obj = JSON.parse(jsonArg); -} catch { - process.stderr.write("gstack-decision-log: invalid JSON\n"); - process.exit(1); -} -if (obj.scope === "branch" && !obj.branch) obj.branch = gitBranch(); -const res = validateDecide(obj); -if (!res.ok) { - process.stderr.write(`gstack-decision-log: ${res.error}\n`); - process.exit(1); -} -appendEvent(paths, res.event); +const event = validPayload(jsonArg); +appendEvent(paths, event); rebuildSnapshot(paths); enqueue(); -console.log(res.event.id); +console.log(event.id); diff --git a/test/gstack-decision-bins.test.ts b/test/gstack-decision-bins.test.ts index 219dbe9b2..5ae6155cd 100644 --- a/test/gstack-decision-bins.test.ts +++ b/test/gstack-decision-bins.test.ts @@ -63,6 +63,39 @@ describe("gstack-decision-log", () => { const r = log("not json", true); expect(r.code).toBe(1); }); + test("--supersede with a replacement body records the replacement, linked to the old id", () => { + const id = log('{"decision":"old-call","scope":"repo","source":"user"}').out; + const out = logFlag( + `--supersede ${id} '{"decision":"new-call","rationale":"better","scope":"repo","source":"user"}'`, + ); + expect(out).toContain(id); + expect(search()).toContain("new-call"); // the replacement is NOT silently dropped + expect(search()).not.toContain("old-call"); + const arr = JSON.parse(search("--json")); + expect(arr.find((d: any) => d.decision === "new-call")?.supersedes).toBe(id); + }); + test("--supersede with an INVALID replacement persists nothing (old stays active)", () => { + const id = log('{"decision":"keep-me","scope":"repo","source":"user"}').out; + let code = 0; + try { + logFlag(`--supersede ${id} '{"decision":""}'`); + } catch (e: any) { + code = e.status || 1; + } + expect(code).toBe(1); + expect(search()).toContain("keep-me"); // not retired by a failed replacement + }); + test("--redact refuses a replacement body instead of dropping it", () => { + const id = log('{"decision":"redact-target","scope":"repo","source":"user"}').out; + let code = 0; + try { + logFlag(`--redact ${id} '{"decision":"would-be-lost","scope":"repo","source":"user"}'`); + } catch (e: any) { + code = e.status || 1; + } + expect(code).toBe(1); + expect(search()).toContain("redact-target"); // nothing happened at all + }); }); describe("gstack-decision-search", () => {