feat(queue): queue as data A2, migration, render and dispatch (#1508)
Filbert approved round 1 (f167b85e). Manifest 782bcb62, 21 files, plus the QUEUE.md markers and the TOOLS.md section. Lead decision 35. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -0,0 +1,139 @@
|
||||
# Queue A2 review, round 1 (#1508, row 9)
|
||||
|
||||
Filbert, 2026-09-27. A1 round 1 review: `queue-a1-review-r1-2026-09-26.md`
|
||||
(sha256 e464be6c).
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approved.** The review covers `build.patch` sha256
|
||||
dc0be7ce74aaaaaf43deebe68af439d1b2b80c77aad6538420e929de35e2f21f on base
|
||||
c9539baa, with `build-manifest.sha256` 782bcb62… and `build.md` ef911685….
|
||||
I found no blocking finding. The notes below don't block, and none of them
|
||||
changes the candidate.
|
||||
|
||||
The two patches outside the candidate, `queue-md.patch` 2ca8f689… and
|
||||
`tools-md.patch` c53aea1f…, are also fine to apply. The migration map
|
||||
(`queue-migration-map.md` 012ddce9…) matches QUEUE.md. I think one of
|
||||
its eight choices needs a change; see n1.
|
||||
|
||||
I re-hashed every input after the review. All match the hashes Darkwing
|
||||
sent. The canonical working tree matches the manifest 21/21, HEAD is still
|
||||
c9539baa, `.git` has no `mosaic-queue*` file and no `pre-commit` hook, and
|
||||
`core.hooksPath` is unset.
|
||||
|
||||
## What I checked
|
||||
|
||||
All of this ran in a scratch clone, `/tmp/fqa2`, with push disabled.
|
||||
|
||||
- **Manifest and suites.** The manifest checks 21/21 after
|
||||
`git apply build.patch`. `node --test` passes 122/122.
|
||||
`scripts/test-queue.sh` passes 24/24 before genesis (live checks
|
||||
skipped) and 26/26 after it.
|
||||
- **The source diff**, file by file: `io.mjs`, `lock.mjs`, `queue.mjs`,
|
||||
`store.mjs`, `cli.mjs`, `scripts/mosaic`, the README and the tests.
|
||||
- **Carried-forward items.** N5, N7, N8, N10, N11, N12, P1, P2 and P3 are
|
||||
implemented as ruled. N7 is a README line; the rest have tests.
|
||||
- N5: tmpfs is allowed only when `allowTmpfs === true`, and `realIo`
|
||||
has no such key.
|
||||
- N8: `\` and `<` in a cell are refused, and `|` is escaped. The GFM
|
||||
cell-count test catches a missing escape.
|
||||
- N10: a narrowed `closes` stays narrowed when `issues` changes, and the
|
||||
receipt says so.
|
||||
- N12: an actor mismatch warns on success and on refusal.
|
||||
- P1: every log entry's `op` goes through `checkCallerOpId`.
|
||||
- P2: `review` is `{rounds}` only, and the review issue is the last
|
||||
round's.
|
||||
- P3: `releaseOrWarn` is used on both gate paths and in `withLock`.
|
||||
- **Dispatch.** `dispatch.test.mjs` runs HEAD's `scripts/mosaic`, kept as
|
||||
the `mosaic-pre-a2.sh` fixture, next to the new one. It compares argv,
|
||||
cwd, environment and stdin for every non-queue verb, so the only change
|
||||
is the `queue` branch.
|
||||
- **Mutations.** I wrote 12 mutants of my own, separate from Darkwing's,
|
||||
and the suite kills all 12. They cover:
|
||||
- the cell check on `blockedReason` and on brief anchors;
|
||||
- a cell with no `|` escape, and a cell helper that ignores its options;
|
||||
- `closes` intersected with the old issues instead of the new ones;
|
||||
- `allowTmpfs` present on `realIo`;
|
||||
- `unlock` dropping its warning, and a silent `releaseOrWarn`;
|
||||
- the actor warning firing when `--by` is absent;
|
||||
- the genesis entry's `op` skipping `checkCallerOpId`;
|
||||
- the review issue taken from the first round, not the last;
|
||||
- unquoted arguments in the `queue` dispatch.
|
||||
|
||||
I didn't mutate the genesis claims or the `legacyView` slice. The dry
|
||||
run below checks both directly.
|
||||
- **The map against QUEUE.md.** I checked all 30 rows against the blob
|
||||
c8e3d34e with my own script, not `map-check.mjs`:
|
||||
- piece and gate are verbatim, and the required flags match;
|
||||
- every brief anchor occurs exactly once in HEAD;
|
||||
- `closes` ⊆ `issues` on every row;
|
||||
- highWater is 30, and nothing is retired.
|
||||
- **The genesis dry run.** I reran Darkwing's steps 0–8 myself on the
|
||||
candidate plus both patches, committed in the scratch clone.
|
||||
- `map-check.mjs` passes. The hook installs from HEAD (blob abdf14e7,
|
||||
mode 0755), and its canary passes.
|
||||
- `genesis` reports `rev 0 genesis 30 rows`. `queue-commit.sh --genesis`
|
||||
commits it.
|
||||
- `verify --current` and `render --check` pass.
|
||||
- `legacyView` equals the text between the two marker lines of the
|
||||
patched QUEUE.md, byte for byte. `mapBlob` is the map's blob in that
|
||||
commit.
|
||||
- Claims go to rows 5, 7, 9, 11 and 16, exactly the rows in
|
||||
in-progress, in-review or waiting-on-jason.
|
||||
- `next` resumes row 9 for darkwing, 7 for sage and 5 for dewey, and
|
||||
gives filbert, rocko and jason nothing.
|
||||
- `assign 10 sage` commits as rev 1.
|
||||
- A hand edit of `queue.json` staged for commit is refused by the hook,
|
||||
with the reset command in the message.
|
||||
|
||||
## Non-blocking
|
||||
|
||||
- **n1. Row 13 can close #1508 before Gate G passes (map choices 5 and
|
||||
7).** Row 13 is the only row that closes #1508. Its gate owner is sage,
|
||||
and it has no `after`. Under J5, Sage can move it in-review→done without
|
||||
Jason. Piece E's rule is that a done row's issue is closed. So #1508
|
||||
would close while row 9, Jason's Gate G, is still in progress.
|
||||
- Row 13's own work has to finish first, so this isn't close. But the queue is meant to hold this order, not a seat's memory.
|
||||
- Fix it with either of these at genesis or just after:
|
||||
- `after: [9, 10, 11, 12]` on row 13, if `after` is meant to gate
|
||||
done as well as start; or
|
||||
- gate owner jason on row 13, or a gate text that says done needs
|
||||
rows 9–12 done.
|
||||
- Row 13's gate still reads "first run Monday 2026-09-21", a date that
|
||||
has passed. It's verbatim from QUEUE.md, and the note corrects it
|
||||
(Q7). Worth a `set gate` with a reason once genesis is in.
|
||||
- **n2. The priority paragraph leaves QUEUE.md.** `queue-md.patch` puts
|
||||
the "Lead: … / Current priority" paragraphs inside the markers. They
|
||||
become part of `legacyView` and drop out of the file. That fits Q8.
|
||||
AGENTS.md's cadence still says "unless Jason has named an explicit
|
||||
current priority in that queue", though, and the ratified goal order now
|
||||
lives in `docs/plans/2026-09-27_goals-review.md`. I suggest the header
|
||||
above the markers point to the goals review. Row 10's AGENTS.md pass
|
||||
should reword that cadence line.
|
||||
- **n3. `queue-commit.sh` still calls `node packages/queue/src/cli.mjs`
|
||||
directly.** A1 said that would change "until A2". Keeping it is right,
|
||||
since it keeps condition 4 simple. The README or the A2 build note
|
||||
should say it stays, so no one reads it as unfinished.
|
||||
- **n4. Row 9's note says "Filbert approved both".** It is true once this
|
||||
verdict lands, and genesis runs only after that. No change needed.
|
||||
- **n5. `SEMANTICS` and `VERSION` are unchanged.** That's correct: no log
|
||||
exists yet, so there is nothing to replay under older rules.
|
||||
- **n6. "(kept narrowed)" can appear when the kept `closes` now equals the
|
||||
new `issues`.** The receipt is still true. Cosmetic.
|
||||
|
||||
I agree with the other map choices:
|
||||
- row 7 stays (decision 27);
|
||||
- row 10's owner is coordinator, with `assign 10 sage` right after
|
||||
genesis (tested in the dry run);
|
||||
- rows 10 and 12 have no `after`;
|
||||
- row 11 is waiting-on-jason, which is accurate: the `queue add` brief
|
||||
refusal (`briefCheck`) is already in A1;
|
||||
- row 16 stays open;
|
||||
- rows 20–22 are done per decision 29.
|
||||
|
||||
## For Sage
|
||||
|
||||
The candidate is ready to commit. Then apply the two patches, install the
|
||||
hook, run genesis, commit with `queue-commit.sh --genesis`, and run
|
||||
`assign 10 sage`, in the order of the dry run. n1 is the one I'd settle
|
||||
at genesis time.
|
||||
Reference in New Issue
Block a user