feat(queue): refuse a row's owner as its reviewer at add, set reviewers and assign (row 31, #1508)

Built by rocko (Gate G run), approved by filbert in round 1 (#1508 comment
26629). Manifest 35199daf. Suites green on an index export.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

packages/queue/tests/data.test.mjs

@@ -120,6 +120,8 @@ test("add: defaults for an ordinary seat, privileged extras, refusals", () => {
   refused(() => step(d, "add", { ...add, owner: "rocko" }, "dewey", { brief: brief() }), /only a privileged actor may set owner/);
   refused(() => step(d, "add", { ...add, reviewers: ["rocko"] }, "dewey", { brief: brief() }), /reviewers/);
   refused(() => step(d, "add", { ...add, required: true }, "dewey", { brief: brief() }), /required/);
+  refused(() => step(d, "add", { ...add, owner: "rocko", reviewers: ["rocko"] }, "sage", { brief: brief() }), /row 12 owner rocko can't be a listed reviewer/);
+  refused(() => step(d, "add", { ...add, reviewers: ["sage"] }, "sage", { brief: brief() }), /row 12 owner sage can't be a listed reviewer/);
   const d2 = step(d, "add", { ...add, owner: "rocko", gateOwner: "filbert", after: [{ id: 9, when: "done" }], reviewers: ["dewey"], required: true }, "sage", { brief: brief() });
   const r2 = row(d2, 12);
   assert.deepEqual([r2.owner, r2.gateOwner, r2.required, r2.after], ["rocko", "filbert", true, [{ id: 9, when: "done" }]]);
@@ -390,6 +392,9 @@ test("field edits: who may change what", () => {
   const set = (id, field, value, reason = null) => ({ id, field, value, reason });
   refused(() => step(d, "set", set(11, "piece", "x"), "dewey"), /privileged/);
   assert.match(step(d, "set", set(11, "piece", "Renamed"), "sage").log.at(-1).result.receipt, /row 11 piece: Brief template→Renamed$/);
+  refused(() => step(d, "set", set(9, "reviewers", ["darkwing"]), "sage"), /row 9 owner darkwing can't be a listed reviewer/);
+  refused(() => step(d, "set", set(9, "reviewers", ["filbert", "darkwing"]), "sage"), /row 9 owner darkwing can't be a listed reviewer/);
+  assert.deepEqual(row(step(d, "set", set(9, "reviewers", ["dewey"]), "sage"), 9).reviewers, ["dewey"]);
   refused(() => step(d, "set", set(9, "after", []), "sage"), /only jason may change after on a required row/);
   step(d, "set", set(9, "after", []), "jason");
   step(d, "set", set(11, "after", [{ id: 9, when: "done" }]), "sage");
@@ -544,6 +549,7 @@ test("note: owner, listed reviewer or privileged; empty clears", () => {
 test("assign moves the claim with the owner; done clears it", () => {
   let d = row9Started();
   refused(() => step(d, "assign", { id: 9, seat: "dewey" }, "darkwing"), /privileged/);
+  refused(() => step(d, "assign", { id: 9, seat: "filbert" }, "sage"), /row 9 owner filbert can't be a listed reviewer/);
   d = step(d, "assign", { id: 9, seat: "dewey" }, "sage", {}, "assign-row-9-dewey");
   assert.deepEqual(row(d, 9).claim, { seat: "dewey", op: "assign-row-9-dewey" });
   assert.match(d.log.at(-1).result.receipt, /owner: darkwing→dewey$/);

packages/queue/tests/review.test.mjs

@@ -591,12 +591,38 @@ test("semantics: v1 entries replay as before; review entries need v2", async (t)
   assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 200, comment: 4, detail: "x" }), /posted exactly when HTTP 201/);
 });
 
+test("set reviewers refuses the row's owner (2026-09-28)", async (t) => {
+  const s = await ready(t);
+  no(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"), 2, /row 9 owner darkwing can't be a listed reviewer/);
+  assert.deepEqual(s.row(9).reviewers, ["filbert"]);
+});
+
+// `set reviewers` now refuses a list that carries the owner (2026-09-28), so
+// this can no longer happen through the CLI. review-record's own refusal
+// (queue.mjs, applyReview) is defense in depth for a row that reached this
+// shape another way (a pre-fix log, a hand edit); this drives applyEntry
+// directly against a state built with the owner spliced into reviewers.
 test("the owner records no verdict, even as a listed reviewer", async (t) => {
   const s = await ready(t);
   ok(s.run(move9(), "darkwing"));
-  ok(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"));
-  no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "1000", "--candidate", s.head(), "--op", "record-9-self-01"], "darkwing"), 2, /only a listed reviewer other than the owner may record a verdict on row 9/);
-  ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "1000", "--candidate", s.head(), "--op", "record-9-filbert"], "filbert"));
+  const { queue } = s.m;
+  const d = s.doc();
+  const state = queue.replay(d.log);
+  const rows = new Map(state.rows);
+  const r9 = rows.get(9);
+  rows.set(9, { ...r9, reviewers: [...r9.reviewers, r9.owner].sort() });
+  const legacy = { ...state, rows };
+  const record = (by, op, comment) => ({
+    rev: d.revision + 1, op, verb: "review-record",
+    args: queue.canonArgs("review-record", { id: 9, verdict: "approve", comment, candidate: s.head() }),
+    by, at: "2026-09-28T00:00:00.000Z", semantics: 2, result: null, viewSha: null,
+  });
+  assert.throws(
+    () => queue.applyEntry(legacy, record("darkwing", "record-9-self-01", 1000), {}),
+    /only a listed reviewer other than the owner may record a verdict on row 9/,
+  );
+  const recorded = queue.applyEntry(legacy, record("filbert", "record-9-filbert", 1000), {});
+  assert.equal(recorded.result.verdict, "approve");
 });
 
 test("a request comment over the length limit is not sent", async (t) => {

packages/queue/tests/store.test.mjs

@@ -234,6 +234,22 @@ test("claims and add defaults through the CLI; candidates are manifests or reach
   assert.equal(row(repo, 6).claim, null);
 });
 
+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/);
+  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);
+  no(cli(repo, ["assign", "9", "filbert", "--op", "assign-9-filbert"], { by: "sage" }), 2, /row 9 owner filbert can't be a listed reviewer/);
+  assert.deepEqual(doc(repo), before);
+  // The unchanged accept paths: a reviewer list or a new owner that does not collide with the other.
+  ok(cli(repo, ["set", "9", "reviewers", "filbert,dewey", "--op", "set-9-ok"], { by: "sage" }));
+  assert.deepEqual(row(repo, 9).reviewers, ["dewey", "filbert"]);
+  ok(cli(repo, ["assign", "11", "rocko", "--op", "assign-11-rocko"], { by: "sage" }));
+  assert.equal(row(repo, 11).owner, "rocko");
+});
+
 test("the working-brief check: a changed working copy refuses the start and flags next", (t) => {
   const repo = ready(t);
   writeFileSync(join(repo.root, "docs/plans/brief-b.md"), "# Briefs B\n\n## Queue\n\nEdited.\n\n## Template\n\nEdited.\n");