diff --git a/packages/queue/README.md b/packages/queue/README.md index daf59eb08..d6ae4032d 100644 --- a/packages/queue/README.md +++ b/packages/queue/README.md @@ -59,6 +59,14 @@ create that shape, since a row can't collect its own owner's required approval. Replay doesn't check it, so a log written before the rule still loads. +`add` takes `--gate-owner SEAT`. The privileged actors, `jason` and `sage`, +must pass it: the write path refuses a privileged add with no gate owner, and its +message names the flag, before anything is written. There is no default for +them, since a Jason gate nobody meant is a step in front of Jason that he +never agreed to. A non-privileged add can't set a gate owner, and its row gets +`jason`. The check is in the write path, not replay, so existing log entries +replay unchanged and the log format is unchanged. + Exit codes: 0 ok; 1 the operation failed; 2 invalid data or refused; 3 uncertain (visible or durable, not acknowledged; for a review request, not known to be posted); 4 usage. diff --git a/packages/queue/src/cli.mjs b/packages/queue/src/cli.mjs index 5eb5343c5..b37d61e8e 100644 --- a/packages/queue/src/cli.mjs +++ b/packages/queue/src/cli.mjs @@ -4,6 +4,7 @@ // Reads: list | show ID | next [SEAT] // Changes: add --piece TEXT --gate TEXT --brief PATH#ANCHOR [--issue N]... [--note TEXT] // [--owner SEAT] [--gate-owner SEAT] [--after ID[:settled]]... [--reviewer SEAT]... [--required] +// (jason and sage must pass --gate-owner; see the usage text) // move ID STATE [--reason TEXT] [--candidate COMMIT|MANIFEST] [--issue N] [--evidence TEXT] // release ID | assign ID SEAT | note ID TEXT | set ID FIELD VALUE [--reason TEXT] // genesis --root PATH --branch NAME --map PATH @@ -28,6 +29,7 @@ import { list, mutate, next, renderView, show, snapshot, sync, unlock, verify, v const USAGE = [ "usage: queue list | show ID | next [SEAT]", " queue add --op ID --piece TEXT --gate TEXT --brief PATH#ANCHOR [--issue N]... [--note TEXT] [--owner SEAT] [--gate-owner SEAT] [--after ID[:settled]]... [--reviewer SEAT]... [--required]", + " jason and sage must pass --gate-owner: a privileged add has no default gate owner. Other seats can't set it; their rows get jason", " queue move ID STATE --op ID [--reason TEXT] [--candidate COMMIT|MANIFEST] [--issue N] [--evidence TEXT]", " queue release ID --op ID | assign ID SEAT --op ID | note ID TEXT --op ID", ` queue set ID FIELD VALUE --op ID [--reason TEXT] (fields: ${SET_FIELDS.join(", ")})`, diff --git a/packages/queue/src/store.mjs b/packages/queue/src/store.mjs index 237d35258..a28a60e87 100644 --- a/packages/queue/src/store.mjs +++ b/packages/queue/src/store.mjs @@ -527,6 +527,13 @@ function mutateAs(ctx, { verb, op, args, by, yes = false, derived = false }) { const view = classifyView(viewRead.text, cur.doc.log); if (view.state === "unknown") throw refuse(unknownMessage(view)); if (view.state === "stale" && verb !== "accept-history") throw refuse(staleMessage(view, cur.doc.log)); + // A privileged add names its gate owner. There is no default for it: the + // Jason default put a gate nobody meant on row 100. This check lives in the + // write path, not in applyEntry, so replay of old entries is unchanged and an + // older reader still loads the log. + if (verb === "add" && PRIVILEGED.has(actor) && cargs.gateOwner === null) { + throw refuse(`${actor} must pass --gate-owner SEAT on add: a privileged add has no default gate owner, so a row never gets a gate nobody meant (row 100 got jason by default)`); + } // Step 5. const resolved = resolveFor(ctx, loc, cur, verb, cargs, cmp); const rev = cur.doc.revision + 1; diff --git a/packages/queue/tests/gate-owner.test.mjs b/packages/queue/tests/gate-owner.test.mjs new file mode 100644 index 000000000..b3407dbc6 --- /dev/null +++ b/packages/queue/tests/gate-owner.test.mjs @@ -0,0 +1,115 @@ +// Row 101 (#1595): a privileged add names its gate owner. The refusal is a +// write-time check in store.mjs, so it is exercised through the CLI in a +// scratch repository. Replay is exercised through loadDoc, which runs the real +// replay path over a log that holds an old privileged add with no gate owner. +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { test } from "node:test"; +import { + applyEntry, buildDoc, canonArgs, genesisReceipt, genesisRows, loadDoc, parseMigrationMap, render, rowsArray, serialize, sha256, +} from "../src/queue.mjs"; +import { QueueError } from "../src/errors.mjs"; +import { MAP_ROWS, cli, genesisCommitted, mapText, scratchRepo } from "./helpers.mjs"; + +const ROOT = "/srv/repo"; +const BLOB = "a".repeat(40); +const BLOB2 = "b".repeat(40); +let clock = 0; +const at = () => new Date(Date.UTC(2026, 9, 13, 12, 0, clock++)).toISOString(); + +function brief() { + return { path: "docs/plans/brief-b.md", anchor: "Queue", blob: BLOB }; +} + +function addArgs(extra = {}) { + return { piece: "Gate row", gate: "g", brief: "docs/plans/brief-b.md#Queue", ...extra }; +} + +// A genesis document built the way store.mjs builds one. +function genesisDoc() { + const map = parseMigrationMap(mapText(MAP_ROWS)); + const blobs = new Map(map.rows.filter((r) => r.brief).map((r) => [r.id, BLOB])); + const t = at(); + const grows = genesisRows(map, blobs, "genesis-gate-0001", t, "sage"); + const entry = { + rev: 0, op: "genesis-gate-0001", verb: "genesis", args: { root: ROOT, branch: "refactor", map: "agents/sage/work/queue-migration-map.md" }, + by: "sage", at: t, semantics: 1, + result: { mapBlob: BLOB2, highWater: map.highWater, retired: map.retired, rows: grows, legacyView: "legacy\n", receipt: genesisReceipt("genesis-gate-0001", grows.length) }, + viewSha: sha256(render(grows, 0)), + }; + return { version: 1, canonicalRoot: ROOT, revision: 0, rows: grows, log: [entry] }; +} + +// Appends one add at the given semantics, the way store.mjs would have written +// it before this row: applyEntry only, no write-time check. +function addUnder(doc, by, args, semantics) { + const { state } = loadDoc(Buffer.from(serialize(doc))); + const rev = doc.revision + 1; + const entry = { rev, op: `add-gate-${rev}`, verb: "add", args: canonArgs("add", args), by, at: at(), semantics, result: null, viewSha: null }; + const out = applyEntry(state, entry, { brief: brief() }); + entry.result = out.result; + entry.viewSha = sha256(render(rowsArray(out.state), rev)); + return buildDoc(doc.canonicalRoot, out.state, [...doc.log, entry]); +} + +test("an old privileged add with no gate owner still replays through loadDoc, and gets jason", () => { + for (const semantics of [1, 2]) { + const d = addUnder(genesisDoc(), "sage", addArgs(), semantics); + const { doc, state } = loadDoc(Buffer.from(serialize(d))); + assert.equal(doc.rows.at(-1).gateOwner, "jason"); + assert.equal(state.rows.get(doc.rows.at(-1).id).gateOwner, "jason"); + } +}); + +test("a non-privileged add gets jason through the replay path and can't name a gate owner", () => { + const d = addUnder(genesisDoc(), "dewey", addArgs(), 2); + assert.equal(loadDoc(Buffer.from(serialize(d))).doc.rows.at(-1).gateOwner, "jason"); + assert.throws( + () => addUnder(genesisDoc(), "dewey", addArgs({ gateOwner: "filbert" }), 2), + (err) => err instanceof QueueError && err.code === 2 && /only a privileged actor may set gateOwner on add/.test(err.message), + ); +}); + +test("the CLI refuses a privileged add without --gate-owner, for jason and sage, and writes nothing", (t) => { + const repo = scratchRepo(t); + genesisCommitted(repo); + const queueBefore = readFileSync(repo.queuePath); + const viewBefore = readFileSync(repo.viewPath); + const revBefore = JSON.parse(queueBefore.toString("utf8")).revision; + const args = ["add", "--op", "add-gate-nogate-1", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template"]; + for (const by of ["sage", "jason"]) { + const r = cli(repo, args, { by }); + assert.equal(r.code, 2, r.err); + assert.match(r.err, /--gate-owner/); + assert.match(r.err, /no default gate owner/); + assert.deepEqual(readFileSync(repo.queuePath), queueBefore); + assert.deepEqual(readFileSync(repo.viewPath), viewBefore); + assert.equal(JSON.parse(readFileSync(repo.queuePath, "utf8")).revision, revBefore); + } +}); + +test("the CLI add with --gate-owner carries the named owner and records a log entry", (t) => { + const repo = scratchRepo(t); + genesisCommitted(repo); + const r = cli(repo, ["add", "--op", "add-gate-named-1", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--gate-owner", "filbert"], { by: "sage" }); + assert.equal(r.code, 0, r.err); + const d = JSON.parse(readFileSync(repo.queuePath, "utf8")); + assert.equal(d.rows.at(-1).gateOwner, "filbert"); + assert.equal(d.log.at(-1).op, "add-gate-named-1"); + assert.equal(d.log.at(-1).by, "sage"); +}); + +test("the CLI add by a non-privileged seat gets jason and can't name a gate owner", (t) => { + const repo = scratchRepo(t); + genesisCommitted(repo); + const base = ["--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template"]; + const ok = cli(repo, ["add", "--op", "add-gate-dewey-1", ...base], { by: "dewey" }); + assert.equal(ok.code, 0, ok.err); + assert.equal(JSON.parse(readFileSync(repo.queuePath, "utf8")).rows.at(-1).gateOwner, "jason"); + + const before = readFileSync(repo.queuePath); + const refusedRun = cli(repo, ["add", "--op", "add-gate-dewey-2", ...base, "--gate-owner", "filbert"], { by: "dewey" }); + assert.equal(refusedRun.code, 2, refusedRun.err); + assert.match(refusedRun.err, /only a privileged actor may set gateOwner on add/); + assert.deepEqual(readFileSync(repo.queuePath), before); +}); diff --git a/packages/queue/tests/store.test.mjs b/packages/queue/tests/store.test.mjs index c55f96647..336bce960 100644 --- a/packages/queue/tests/store.test.mjs +++ b/packages/queue/tests/store.test.mjs @@ -176,7 +176,7 @@ test("op ids: missing, too long, reserved; a retry answers; another payload refu test("a retried add returns the id it first allocated, after reassignment and after done", (t) => { const repo = ready(t); - const add = ["add", "--op", "add-new-row-01", "--piece", "New thing", "--gate", "Jason says", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--issue", "#1508"]; + const add = ["add", "--op", "add-new-row-01", "--piece", "New thing", "--gate", "Jason says", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--gate-owner", "filbert", "--issue", "#1508"]; const first = ok(cli(repo, add, { by: "sage" }), /row 12 none→queued$/m).out.trim(); ok(cli(repo, ["assign", "12", "dewey", "--op", "assign-12-dewey"], { by: "sage" }), /owner: rocko→dewey/); ok(cli(repo, add, { by: "sage" }), new RegExp(`^${first} \\(already recorded at rev 1\\)$`, "m")); @@ -226,7 +226,7 @@ test("claims and add defaults through the CLI; candidates are manifests or reach ok(cli(repo, ["add", "--op", "add-by-dewey-1", "--piece", "Mine", "--gate", "tests", "--brief", "docs/plans/brief-b.md#Template"], { by: "dewey" })); const r = row(repo, 12); assert.deepEqual([r.owner, r.gateOwner, r.state, r.claim, r.brief.blob], ["dewey", "jason", "queued", null, repo.g("rev-parse", "HEAD:docs/plans/brief-b.md").trim()]); - no(cli(repo, ["add", "--op", "add-by-dewey-2", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--reviewer", "ghost"], { by: "sage" }), 2, /reviewer ghost is not jason or a seat/); + no(cli(repo, ["add", "--op", "add-by-dewey-2", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--gate-owner", "filbert", "--reviewer", "ghost"], { by: "sage" }), 2, /reviewer ghost is not jason or a seat/); no(cli(repo, ["add", "--op", "add-by-dewey-3", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md"], { by: "dewey" }), 2, /PATH#ANCHOR/); no(cli(repo, ["add", "--op", "add-by-dewey-4", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Queue", "--issue", "x"], { by: "dewey" }), 4); ok(cli(repo, ["move", "11", "in-progress", "--op", "start-11-0001"], { by: "dewey" })); @@ -245,7 +245,7 @@ test("claims and add defaults through the CLI; candidates are manifests or reach test("add, set reviewers and assign refuse the row's owner as a reviewer", (t) => { const repo = ready(t); const before = doc(repo); - no(cli(repo, ["add", "--op", "add-owner-reviewer", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--reviewer", "rocko"], { by: "sage" }), 2, /row 12 owner rocko can't be a listed reviewer/); + no(cli(repo, ["add", "--op", "add-owner-reviewer", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--gate-owner", "filbert", "--reviewer", "rocko"], { by: "sage" }), 2, /row 12 owner rocko can't be a listed reviewer/); assert.deepEqual(doc(repo), before); no(cli(repo, ["set", "9", "reviewers", "filbert,darkwing", "--op", "set-9-owner-reviewer"], { by: "sage" }), 2, /row 9 owner darkwing can't be a listed reviewer/); assert.deepEqual(doc(repo), before); @@ -394,7 +394,7 @@ test("usage errors exit 4", (t) => { no(cli(repo, ["move", "9", "done", "--op", "x-00000001", "--op", "y-00000001"], { by: "sage" }), 4, /--op given twice/); no(cli(repo, ["set", "9", "owner", "x", "--op", "set-9-000001"], { by: "sage" }), 4, /set fields/); no(cli(repo, ["set", "9", "required", "yes", "--op", "set-9-000001"], { by: "sage" }), 4, /true or false/); - no(cli(repo, ["add", "--op", "add-0000001", "--piece", "x"], { by: "sage" }), 4, /add needs --gate/); + no(cli(repo, ["add", "--op", "add-0000001", "--piece", "x", "--gate-owner", "filbert"], { by: "sage" }), 4, /add needs --gate/); no(cli(repo, ["list", "--check"]), 4, /does not apply here/); ok(cli(repo, ["--help"]), /^usage: queue list/); });