mirror of https://github.com/garrytan/gstack.git
fix(decision-log): --supersede silently discarded the replacement decision
The supersede/redact branch appended the retirement event and exited before the JSON argument was ever read — a user recording a reversal WITH its replacement lost the replacement, and the payload finder's first-non-flag-arg predicate would have mistaken the target id for JSON anyway. Payloads are now identified by their leading brace, validated BEFORE any write, and appended FIRST (retirement second), so the only visible interleaving under a crash is both-active — recoverable, never lost. The replacement carries supersedes:<old-id> provenance. Bare --supersede <id> (the documented reversal-without-replacement) stays legal; --redact with a payload now refuses instead of dropping it. Ported from time-attack/gstack (GStack 2), tests included. Co-authored-by: Sina Matian <sina@time-attack.dev> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
d2e6925638
commit
225e6e4ccd
|
|
@ -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 <id> '{...}'` 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<DecisionEvent>;
|
||||
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 <id>, or --compact\n",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
let obj: Partial<DecisionEvent>;
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue