Files
stack/agents/darkwing/work/queue-d/build.md
T
jason.woltjeandClaude Opus 5.5 f539466fcb feat(queue): Piece D, reviews as issue comments, raw per-seat token helper (row 12, #1508)
queue move ID in-review posts the review request as a Gitea comment and
review record reads verdicts back, so reviews stop being files in
docs/plans/reviews/. On a comment round, in-review to waiting-on-jason
now needs every listed reviewer's approval for the current round, the
same as in-review to done (Filbert r1 C1). scripts/gitea-api.sh reads
the raw per-seat token files (lead decisions 37 to 39): config built and
checked before curl starts, export attribute cleared, fixed base URL.
test-queue.sh skips its live checks outside the canonical root.

Darkwing authored. Filbert approved D r2 (cf1d3fd0) after r1 (a2dc2302)
and corrected the plan (293747cd). Rocko reviewed the helper (e896192f,
2096b0a3), and Sage's lead check passed under decision 38. Manifest
b402fb38, 19 files.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-09-27 10:07:29 -05:00

14 KiB

Queue D (#1508, row 12), candidate for review, round 2

Darkwing, 2026-09-27. Piece D is review requests, per plan section 8.9 (agents/filbert/work/queue-as-data-plan-2026-09-26.md). Moving a row with reviewers to in-review posts one request comment on its issue as the acting seat, and the log records what happened. The row closes on the reviewers' recorded verdicts. The candidate also carries the DEFERRED item Sage attached to rows 12 and 13: test-queue.sh skips its live checks outside the canonical root.

Base is 8ffbd73b. The patch applies unchanged to HEAD cdcedb27, which since then has changed only lead-decisions.md, QUEUE.md and queue.json. Filbert reviews D. The helper patch is separate (helper.md); Sage approved it in lead decision 39, and it ships in the D commit. Sage commits. Nothing is committed, staged or pushed.

Round 2 fixes Filbert's C1 (agents/filbert/work/queue-d-review-r1-2026-09-27.md): on a request round, in-review→waiting-on-jason now makes the same checks as in-review→done. See "Waiting-on-jason on a request round" below.

Files

build.patch (sha256 28d0790e797e1f24f295f70b2e0162bb1b4b0a154b220c0527f7d6624f0aa1c8) changes 7 files and adds 3. build-manifest.sha256 (sha256 af57ead2697a88d74a9214783dfa35ba3bd171aa32a594584f018181e261e956) pins all 10 after the patch. In a fresh clone at cdcedb27 the patch applies and the result matches the manifest 10/10.

  • packages/queue/src/review.mjs (new): the credential check, the request body, the helper call under the deadline, and the reading of each answer.
  • packages/queue/src/queue.mjs: SEMANTICS 2, the five review verbs, the round, attempt and receipt shapes, and the rules for moves and done.
  • packages/queue/src/store.mjs: the request step after the move is logged, the resolve check, review verify-commit, and the retry and settle hints.
  • packages/queue/src/cli.mjs: the review subcommands.
  • packages/queue/README.md: a "Review requests" section, verbs, exit 3, known limits, tests.
  • scripts/test-queue.sh: the live checks skip outside the canonical root.
  • Tests: review.test.mjs (new, 20 tests), fixtures/fake-gitea.mjs (new), fixtures/kill-at.mjs (a step can be NAME#N, the Nth time it is reached), store.test.mjs (two A2 tests set row 9's reviewers to none first, so their moves post nothing; a note there moves from filbert to sage, since filbert is no longer a reviewer). 122 tests at A2, 142 now.

Outside the patch:

  • helper.patch (sha256 48edd46b93c54908e9d59737aab78a33e007ddfafa1e6ad4d34333015bb75430): raw per-seat token files in scripts/gitea-api.sh, lead decision 37. Rocko reviewed two rounds, and Sage approved the result (decisions 38 and 39); notes in helper.md. D's tests don't need it, but the live round does.
  • tools-md.patch (sha256 d30d65b842bd56077ccd9164eccd946f2aa9f53290d02eba61f284960d230e14): docs/TOOLS.md, the review commands and the raw token file. It is Sage's file, so it's a proposal.

How a request goes

  1. Intent. move ID in-review --candidate C --op OP on a row with reviewers logs the round and its first attempt, requesting, under the lock, like any other op. If that write doesn't finish, nothing is sent.
  2. Pre-send. Without the lock: the credential file check (lstat only), then GET user, which must return the acting seat's login, or jarvis for sage. A failure here is failed, and the POST never runs.
  3. POST one comment on the round's issue through scripts/gitea-api.sh, under timeout -s KILL 30. The body carries two markers: <!-- mosaic-queue-op: OP --> and <!-- mosaic-queue-round: row=N round=R candidate=DIGEST -->.
  4. Outcome. A second entry, OP.outcome, under the lock. It holds the HTTP status, the comment id and a fixed detail string. Nothing from the response body is logged or printed.

Exit 0 is posted, 1 is failed (400, 401, 403, 404, 422, or pre-send), and 3 is uncertain (anything else). A same-op retry prints the logged state and sends nothing. An uncertain or requesting attempt prints where to look and the two commands that settle it.

Choices I made

  • The CLI shape of resolve. The plan has review resolve ID --attempt REQOP --posted <id>. I made it review resolve ID REQOP --comment N, and abandon takes the attempt the same way. The attempt is always required, and --comment is the flag name record uses for a comment id too.
  • Resolve checks the author. The plan lists issue, marker, round and candidate. I added the comment's author: it must be the requester's login, fetched with the resolver's own token. Otherwise anyone could copy the markers into a comment of their own.
  • Done on a request round needs an approval recorded by every listed reviewer in the current round, and refuses --evidence. If the reviewers were removed after the round opened, done refuses until a privileged actor sets them. A round from before D, or on a row with no reviewers, closes with --evidence as in A2.
  • Waiting-on-jason on a request round (round 2, C1). The plan's table let the owner move in-review→waiting-on-jason with no other check, so a Jason-gated row could reach Jason, and then close, with no reviewer's verdict. The move now refuses while a request is unresolved, and on a request round it needs an approval recorded by every listed reviewer. requireApprovals in queue.mjs holds the approval checks that done and this move share. A round with no request is unchanged, so v1 replay is too. Filbert approved round 2 and asked for one more check (n1): an approval from an earlier round doesn't count in the current one. The Jason-gated test now has rocko approve round 2 first, and the move refuses until filbert approves round 2 as well. Jason gets no exemption, because the owner makes this move. In-review→in-progress, the changes path, is unchanged.
  • The owner records no verdict, even when listed as a reviewer.
  • A late outcome on a done row. review-outcome is the one entry a done row accepts, so a POST that answers after the row closed is still recorded. It can only set conflict; nothing reopens the row, and review resolve on it refuses without fetching the comment.
  • A late uncertain after a resolve keeps posted: the resolve saw the comment. A late failed after a resolve is conflict, because the server said it refused a comment someone found.
  • Semantics per entry. Every log entry already records semantics. Entries at 1 replay under A2's rules, so the live log (rev 13, all semantics 1) loads unchanged. Review verbs need 2, checked in the entry shape and again in apply.
  • Body limit. A body over 60,000 bytes is a pre-send failed, and nothing is sent. A 700-line manifest is enough to reach it.
  • verify-commit on a commit candidate compares every path the candidate changed against its first parent, by blob and mode, and requires the paths it deleted to be absent in REF. A diff-tree line it can't parse refuses with exit 1 instead of being skipped.
  • The claim refusal message. ownerOrPriv read "darkwing cannot resolve a request on". It now names the owner and the privileged actors in every case, with the claim first when there is one. No A2 test matched the old text; the resolve test pins the new one.
  • test-queue.sh. After genesis it reads canonicalRoot from HEAD:docs/plans/queue.json and compares it with the real path of the toplevel. If they differ, it prints skip queue verify and render --check: this checkout (TOP) is not the queue's canonical root (CANON) and passes. An empty canonicalRoot is a failure.

Tests

review.test.mjs runs each case in a scratch repository with genesis committed. scripts/gitea-api.sh there is a stub that runs fixtures/fake-gitea.mjs: same argv and output as the helper, calls logged to a file, rules from a scenario file, and posted comments served back by id. The token files are dummies under the scratch directory. No test reads a real token or opens ~/.mosaic or ~/.t3.

Against the test list in 8.9:

8.9 asks for Test
a kill before the POST, after it, at the outcome write, while retaking the lock "a same-op retry after a kill sends nothing, even with a stale view" (steps pre-send, posted, outcome, locked#2), and "a held lock at the outcome exits 3"
posted; each listed 4xx; 5xx; request failed; timeout; 201 without an id "each transport answer maps to posted, failed or uncertain" (deadline 300 ms)
a new op while requesting or uncertain refuses "an unresolved request blocks a new request, a new round, waiting-on-jason and done", which also reaches waiting-on-jason with a late POST still out, and Jason's close refuses
(round 2, C1) a Jason-gated row skips its reviewers "a Jason-gated row reaches waiting-on-jason only on every reviewer's approval": Filbert's steps refuse, then pass after both reviewers approve a later round; reviewers removed refuse too
a same-op retry after a stale view sends nothing the kill test
a late POST after abandon; a late outcome after resolve, same id and another "late outcomes": rounds 1 to 3, plus a late 500 (stays posted) and a late 422 (conflict); "a late POST on a closed row" (abandoned, approved and closed while the POST was out)
resolve with the wrong issue, marker, round or candidate "resolve checks the comment", which adds author, id, 404, 500, a transport failure and the wrong credential
GET user mismatch and timeout "the pre-send checks"
the lead posts as jarvis; sage as a login refuses "the pre-send checks" and "the lead's request refuses a token for login sage"
the credential checks: mode, another seat's path, the default file "the credential file" (also unset, relative, missing, symlink, a linked directory, and jarvis's path for sage)
request, changes, new candidate, approval, pinned "request, changes, a new candidate, approval", which also checks no file lands under docs/plans/reviews/ or agents/*/work/

Every test that retries, kills or answers late counts the POSTs the fake saw. None sees a second POST for one attempt.

Mutations

61 hand-written mutants: 29 in queue.mjs, 16 in store.mjs, 16 in review.mjs. Each runs alone against the whole queue test directory. Against the round 2 tests, 59 are killed, the same result as round 1.

Round 2 adds six in queue.mjs for C1, run against the round 2 tests. All six are killed:

  • the round 1 branch restored (waiting-on-jason checks only the actor);
  • the approval check dropped from waiting-on-jason;
  • the unresolved check dropped from waiting-on-jason;
  • the unresolved check dropped from waiting-on-jason→done;
  • the no-reviewers check dropped from requireApprovals;
  • the approval check applied to every round, not only request rounds.

Seven survived earlier runs. Five of them now have a test that kills them: a second outcome for one attempt, the owner recording a verdict, done on a row whose reviewers were removed, a same-op resolve retry that fetched the comment again, and a resolve on a done row that fetched the comment before the log refused it. The last one is the new test "a late POST on a closed row".

Two survive, and each is equivalent while the other stays:

  • the semantics check in applyEntry dropped;
  • the semantics check in the entry shape dropped.

Every entry passes the shape check before it is applied, so either check alone refuses a review verb at semantics 1. With both dropped, the semantics test fails.

Suites

In the scratch clone with the patch and the helper patch applied: config 24/0, task 90/0, foundation 44/0, conductor 17/0, release 14/0, auth 15/0, discord 64/0, extension-package 18/0, queue 27/0. node --test packages/queue/tests/ passes 142, and node --test packages/ledger/tests/ passes 58.

The scratch clone isn't the canonical root, so the queue suite printed its skip line for the two live checks. In the canonical tree they run.

In a fresh clone at cdcedb27 with build.patch applied: the manifest matches 10/10, the queue tests pass 142, the queue suite 27/0. With helper.patch applied on top, the ledger tests pass 58.

Known limits

  • Scope of darkwing's and dewey's tokens. They hold write:repository. If that doesn't cover an issue comment, the live POST gets 403, which is failed with nothing posted (8.9 says so). The scope question then goes to Sage.
  • Post to outcome. A kill between the POST and the outcome entry leaves the attempt requesting with a comment on the issue. The queue never posts again on its own; a person resolves it.
  • Verdict comments aren't fetched. review record takes the reviewer's comment id on trust, as the queue takes --by.
  • Commit candidates stay checkable only while a ref keeps the commit.
  • The helper's own limits are in helper.md.

After approval (Sage)

  1. Apply build.patch to the canonical tree and check it against build-manifest.sha256. Run scripts/test-queue.sh and node --test packages/queue/tests/. Commit the 10 files by path.
  2. Apply helper.patch for the same commit (decision 39). The live round needs it: without it the helper refuses a raw token file.
  3. tools-md.patch if you want it.
  4. Row 12 lists no reviewers, so moving it to in-review now would open a round that posts nothing. Set them first: scripts/mosaic queue set 12 reviewers filbert --op OP --by sage, and add rocko if the helper counts as part of this round.

The live round, which is mine:

  1. MOSAIC_GITEA_CREDENTIAL_FILE=~/.mosaic/fleet/agents/darkwing/secrets/gitea-mosaicstack-darkwing.token scripts/mosaic queue move 12 in-review --candidate <D's commit> --op OP --by darkwing. The queue lstats the file, the helper reads it, and GET user must answer darkwing. The request goes on #1508.
  2. Exit 0 means posted. Exit 1 with HTTP 403 is the scope question above. Exit 3 means I look on #1508 for the marker and resolve, or ask you to abandon.
  3. Filbert posts a verdict on #1508 and runs review record 12. Then move 12 done, and queue-commit.sh for the queue ops.