fix(queue): genesis owner-not-reviewer check, assign wording, queue-commit HEAD-moved message, calendar dates (row 33, #1508)
Built by filbert, approved by darkwing in round 1 (#1508 comment 26651). Manifest fe7da3ef, 10 paths. Suites green on an index export. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,132 @@
|
||||
# Queue row 33 build notes (#1508)
|
||||
|
||||
Filbert, 2026-10-04. Brief: `docs/plans/2026-10-04_queue-follow-ups.md`
|
||||
(blob 1dc08bf0…). Reviewer: Darkwing. Base: HEAD 3e39a26a, queue rev 45.
|
||||
Candidate: `candidate-manifest.sha256` in this directory, uncommitted.
|
||||
Sage commits it after approval.
|
||||
|
||||
## What changed
|
||||
|
||||
1. **Genesis and the owner-not-reviewer rule.** `parseMigrationMap` refuses
|
||||
a map row whose owner is among its reviewers: "migration map row N owner
|
||||
SEAT can't be a listed reviewer", exit 2. Genesis parses the map before
|
||||
it writes anything, so no file, witness or view changes.
|
||||
2. **Assign wording.** "can't assign row N to SEAT: SEAT is one of its
|
||||
reviewers". The two tests that matched the old text now match this one.
|
||||
3. **HEAD moved.** `queue-commit.sh` gains `head_moved`, called in
|
||||
`guard_check` before the canary and again when the clean run fails. A
|
||||
moved HEAD exits 2 with "refused (STEP): HEAD moved since H was
|
||||
recorded (another commit landed); run queue-commit.sh again". A real
|
||||
guard failure, with HEAD unmoved, keeps the old message.
|
||||
4. **Calendar dates.** `checkTime` refuses any ISO time or date that
|
||||
doesn't format back to itself through `Date.parse` and `toISOString`:
|
||||
"WHAT is not a calendar time|date: VALUE". A shape error keeps its old
|
||||
message. Every time the validator reads goes through `checkTime`: row
|
||||
times, review times and log entry times.
|
||||
5. **Cold start.** See "Cold runs" below.
|
||||
|
||||
README: the commit steps, the owner rule and the time format. DEFERRED:
|
||||
four items move to Done, and the cold-start item too, if no failure shows
|
||||
up.
|
||||
|
||||
## Choices
|
||||
|
||||
- **C1. The genesis check sits in the map parser, not in replay.** The
|
||||
review-record refusal is kept as defense in depth for a log written
|
||||
before the rule (`review.test.mjs`). A check in `checkGenesis` would
|
||||
make such a log unreadable. A new data test pins this: the parser
|
||||
refuses the map, and a genesis document built past the parser still
|
||||
loads. Mutant M10 (the rule added to replay) is killed by it.
|
||||
- **C2. "another commit landed", not "another queue commit landed".** The
|
||||
brief quotes the second wording. The check before the canary fires for
|
||||
any commit, including one that touches no queue file, so "queue" would
|
||||
be wrong there. The rest of the message is the brief's.
|
||||
- **C3. HEAD is checked twice.** The brief asks for a check before the
|
||||
canary. A commit can still land between that check and the hook run, and
|
||||
then the old misdiagnosis comes back. A second check, only when the
|
||||
clean run fails, closes that gap. It adds one `rev-parse` on a failing
|
||||
path only.
|
||||
- **C4. Two existing F1 tests changed.**
|
||||
- "HEAD moving after commit-tree and before update-ref" moved its
|
||||
trigger to `before update-ref`. A move after commit-tree is now caught
|
||||
at the step-7 check (exit 2). The test still proves update-ref's
|
||||
expected-old-value check, now at the only point where it's the last
|
||||
line.
|
||||
- "H is recorded before the canary…": a commit outside the queue files
|
||||
during the step-1 canary used to reach update-ref (exit 1). It's now
|
||||
refused at the step-7 check (exit 2), before that check's canary, and
|
||||
update-ref never runs. The test asserts that.
|
||||
- **C5. 24:00 is refused.** V8 parses `T24:00:00.000Z` as the next day's
|
||||
midnight, so it doesn't round-trip. Minute 60 and second 60 already
|
||||
parse as NaN.
|
||||
|
||||
## Tests
|
||||
|
||||
- `data.test.mjs`:
|
||||
- a new genesis test (map refusal; replay tolerance);
|
||||
- a new calendar test: month 13, month 0, day 32, 2026-02-30,
|
||||
2027-02-29 and 2026-04-31 as dates in `requiredSince` and `createdAt`;
|
||||
the same days plus 24:00, minute 60 and second 60 as ISO times in
|
||||
`updatedAt` and `requiredSince`; 2028-02-29 valid in both forms; the
|
||||
shape message kept; a log entry `at` refused on replay;
|
||||
- the assign wording.
|
||||
- `store.test.mjs`: genesis from a map with row 9's owner among its
|
||||
reviewers exits 2. For that repository and the two refusals before it,
|
||||
there's no queue.json, no witness and an unchanged QUEUE.md. Also the
|
||||
assign wording.
|
||||
- `commit.test.mjs`, two new tests, both with a nested
|
||||
`queue-commit.sh` run landing a real queue commit:
|
||||
- after `symbolic-ref` (H recorded, before step 1's check): refused at
|
||||
step 1 with the HEAD-moved message, no canary run, and a rerun commits
|
||||
the next revision;
|
||||
- before the first `git hook run`: the clean run fails on the moved
|
||||
HEAD, and the recheck reports HEAD moved, not the guard.
|
||||
The existing "the canary refuses a hook that git would not run" test
|
||||
still covers the guard message.
|
||||
|
||||
## Mutants
|
||||
|
||||
Eleven, run on an export of the candidate. Ten are killed:
|
||||
|
||||
| | Mutant | Killed by |
|
||||
|---|---|---|
|
||||
| M1 | no HEAD check before the canary | step-1 and step-7 HEAD tests |
|
||||
| M2 | no recheck after a failed clean run | the between-check-and-canary test |
|
||||
| M3 | `head_moved` exits 1 | all three HEAD tests |
|
||||
| M4 | no round trip | calendar test |
|
||||
| M5 | NaN check only | calendar test |
|
||||
| M6 | round trip on ISO times only | calendar test |
|
||||
| M7 | round trip on dates only | calendar test |
|
||||
| M9 | genesis rule removed | data and store genesis tests |
|
||||
| M10 | genesis rule also in replay | data genesis test |
|
||||
| M11 | old assign wording | data and store assign tests |
|
||||
|
||||
One survives:
|
||||
|
||||
- **M8** compares only the date part of an ISO time. It's equivalent
|
||||
under V8. The only out-of-range time V8 parses is 24:00, and that moves
|
||||
the date, so the date part catches it.
|
||||
|
||||
## Suites
|
||||
|
||||
On the final candidate bytes, HEAD 3e39a26a:
|
||||
- canonical checkout: `node --test packages/queue/tests/` passes 148/148
|
||||
and `bash scripts/test-queue.sh` 29/0;
|
||||
- an export of HEAD with the candidate files: `bash scripts/test-queue.sh`
|
||||
27/0 (the live checks skip, as designed);
|
||||
- `queue verify --current`: ok at rev 45. The live log replays under the
|
||||
calendar check.
|
||||
|
||||
## Cold runs
|
||||
|
||||
Not reproduced in 20 cold runs. `cold.sh` (here) built each tree fresh
|
||||
from HEAD 3e39a26a plus the candidate files: odd runs as a
|
||||
`git clone --shared` with push set to DISABLED, even runs as a
|
||||
`git archive` export. It ran `node --test packages/queue/tests/` once in
|
||||
each, one run at a time with nothing else running, then deleted the tree.
|
||||
Every run passed 148/148, 2960 of 2960 in total, taking 39 to 61 s each.
|
||||
The counts are in `cold-runs.txt`. The 300 ms deadline test is unchanged.
|
||||
|
||||
Run 1 started with a `queue-commit.sh` that differed from the candidate in
|
||||
one comment line, the step-1 comment, which I reworded during run 1. Runs
|
||||
2 to 20 used the candidate bytes.
|
||||
@@ -0,0 +1,21 @@
|
||||
run 1 clone exit=0 pass=148 fail=0 secs=41
|
||||
run 2 export exit=0 pass=148 fail=0 secs=39
|
||||
run 3 clone exit=0 pass=148 fail=0 secs=39
|
||||
run 4 export exit=0 pass=148 fail=0 secs=48
|
||||
run 5 clone exit=0 pass=148 fail=0 secs=59
|
||||
run 6 export exit=0 pass=148 fail=0 secs=47
|
||||
run 7 clone exit=0 pass=148 fail=0 secs=42
|
||||
run 8 export exit=0 pass=148 fail=0 secs=46
|
||||
run 9 clone exit=0 pass=148 fail=0 secs=47
|
||||
run 10 export exit=0 pass=148 fail=0 secs=42
|
||||
run 11 clone exit=0 pass=148 fail=0 secs=45
|
||||
run 12 export exit=0 pass=148 fail=0 secs=47
|
||||
run 13 clone exit=0 pass=148 fail=0 secs=43
|
||||
run 14 export exit=0 pass=148 fail=0 secs=40
|
||||
run 15 clone exit=0 pass=148 fail=0 secs=40
|
||||
run 16 export exit=0 pass=148 fail=0 secs=42
|
||||
run 17 clone exit=0 pass=148 fail=0 secs=39
|
||||
run 18 export exit=0 pass=148 fail=0 secs=61
|
||||
run 19 clone exit=0 pass=148 fail=0 secs=57
|
||||
run 20 export exit=0 pass=148 fail=0 secs=52
|
||||
done
|
||||
@@ -0,0 +1,31 @@
|
||||
#!/bin/bash
|
||||
# 20 cold runs of node --test packages/queue/tests/ for row 33 item 5.
|
||||
set -u
|
||||
SRC=/mnt/storage/src/mosaic-stack
|
||||
W=/tmp/fq33
|
||||
HEADC=$(cat $W/base.head)
|
||||
FILES="packages/queue/README.md packages/queue/src/queue.mjs packages/queue/tests/commit.test.mjs packages/queue/tests/data.test.mjs packages/queue/tests/store.test.mjs scripts/queue-commit.sh"
|
||||
: > $W/cold-results.txt
|
||||
for i in $(seq 1 20); do
|
||||
d=$W/cold-$i
|
||||
rm -rf $d
|
||||
if [ $((i % 2)) = 1 ]; then
|
||||
kind=clone
|
||||
git clone -q --shared --no-checkout $SRC $d && git -C $d remote set-url --push origin DISABLED && git -C $d checkout -q --detach $HEADC
|
||||
else
|
||||
kind=export
|
||||
mkdir -p $d && git -C $SRC archive $HEADC | tar -x -C $d
|
||||
fi
|
||||
(cd $W/base && tar -c $FILES) | tar -x -C $d
|
||||
start=$(date +%s)
|
||||
(cd $d && env -u NODE_TEST_CONTEXT node --test --test-reporter=tap packages/queue/tests/ > $W/cold-$i.tap 2>&1)
|
||||
rc=$?
|
||||
end=$(date +%s)
|
||||
pass=$(grep -E '^# pass' $W/cold-$i.tap | awk '{print $3}')
|
||||
fail=$(grep -E '^# fail' $W/cold-$i.tap | awk '{print $3}')
|
||||
failed=$(grep -E '^not ok' $W/cold-$i.tap | tr '\n' ';')
|
||||
echo "run $i $kind exit=$rc pass=$pass fail=$fail secs=$((end-start)) $failed" >> $W/cold-results.txt
|
||||
[ "$fail" = 0 ] && rm -f $W/cold-$i.tap
|
||||
rm -rf $d
|
||||
done
|
||||
echo done >> $W/cold-results.txt
|
||||
+18
-30
@@ -138,42 +138,12 @@ at every gate. Started 2026-09-12 during the control board MVP.
|
||||
Sage asked the SetSpark lead to use a one-word role. The rule stays as
|
||||
it is. (2026-09-26, #1506)
|
||||
|
||||
- **Queue: `genesis` from a map doesn't check the owner-not-reviewer
|
||||
rule.** Row 31 added the refusal to `add`, `set reviewers` and `assign`,
|
||||
but a genesis map can still create a row whose owner is a reviewer.
|
||||
No current row has that shape. Filbert, row 31 r1 (#1508 comment 26629).
|
||||
(2026-10-04, #1508)
|
||||
- **Queue: `add --issue N` also sets closes to N.** Follow-up rows under
|
||||
an umbrella issue, like rows 31 and 33 under #1508, would close the issue
|
||||
when they finish. Sage narrowed both by hand (revs 36 and 43). Either
|
||||
`add` takes `--closes` or the lead checks closes on every add. Not
|
||||
urgent while Sage adds the rows. (2026-10-04, #1508)
|
||||
- **Queue: the `assign` refusal calls the seat "owner" too early.** The
|
||||
message calls the seat the row's owner before the assign makes it one.
|
||||
Wording only. Filbert, row 31 r1. (2026-10-04, #1508)
|
||||
- **Queue tests: one unexplained failure on a cold start.** Darkwing's
|
||||
first run of the D candidate in a fresh clone gave 141 passed, 1 failed.
|
||||
The failing test's name wasn't captured. 15 reruns passed 142/0,
|
||||
sequential and parallel. Darkwing suspects a timing bound, most likely
|
||||
the 300 ms deadline case in the transport test, which allows 4000 ms.
|
||||
That isn't confirmed. Next time: capture the name, then fix it as its
|
||||
own patch. (2026-09-27, #1508)
|
||||
|
||||
- **queue-commit.sh blames the guard when HEAD moves.** Step 7's canary
|
||||
reads H into its index (`scripts/queue-commit.sh:94`). The hook diffs
|
||||
against the live HEAD. If another queue commit moves the branch after H
|
||||
is recorded, the clean canary sees a difference and refuses with "git is
|
||||
not running the queue guard as installed". The fix it prints does
|
||||
nothing. The refusal is correct, the message is wrong. Fix: guard_check
|
||||
compares HEAD with H before the canary and says "HEAD moved since H;
|
||||
start again". Filbert found this during the row 12 live round.
|
||||
(2026-09-27, #1508)
|
||||
|
||||
- **Queue validator checks date shape, not the calendar.** `ISO_RE` and
|
||||
`DATE_RE` accept month 13, and V8 turns 2026-02-30 into March 2 without
|
||||
an error. The CLI can't write such a value today; a hand edit or a
|
||||
future verb could. Fix: round-trip the parsed date. Filbert and
|
||||
Darkwing, Piece E round 2. (2026-09-27, #1508)
|
||||
- **Ledger metric call can orphan curl.** The metric call in
|
||||
`packages/ledger/src/ledger.mjs` still relies on execFileSync's
|
||||
timeout, which kills the helper but can leave curl running. E's queue
|
||||
@@ -209,3 +179,21 @@ Moved to `docs/plans/QUEUE.md` on 2026-09-13. This file holds only gaps.
|
||||
now refuse the owner as a reviewer with exit 2 and write nothing. Rocko
|
||||
built it in the Gate G run, and Filbert approved round 1 (#1508 comment
|
||||
26629, 8 of 8 mutants caught).
|
||||
- Queue follow-ups from rows 12, 13 and 31 (2026-10-04, #1508): closed by
|
||||
row 33. Filbert built it, and Darkwing reviews it.
|
||||
- `genesis` refuses a map row whose owner is one of its reviewers (exit
|
||||
2, nothing written). The check is on the map, not in replay, so a log
|
||||
from before the rule still loads.
|
||||
- The `assign` refusal now reads "can't assign row N to SEAT: SEAT is
|
||||
one of its reviewers".
|
||||
- `queue-commit.sh` compares HEAD with H before each canary and again if
|
||||
the clean run fails. A moved HEAD exits 2 with "HEAD moved since H was
|
||||
recorded"; the guard message is kept for a real guard failure.
|
||||
- Every time and date the validator accepts must format back to itself,
|
||||
so month 13, day 32, 2026-02-30, 2027-02-29 and 24:00 are refused.
|
||||
The live log at rev 45 still replays.
|
||||
- The cold-start failure (2026-09-27): not reproduced in 20 cold runs.
|
||||
Each ran `node --test packages/queue/tests/` in a fresh tree, 10
|
||||
shared clones and 10 exports, one after another. All 20 passed
|
||||
148/148 (2960 of 2960). The 300 ms deadline test is unchanged. Counts:
|
||||
`agents/filbert/work/queue-33/cold-runs.txt`.
|
||||
|
||||
@@ -53,9 +53,11 @@ becomes the new list. If it was narrowed (`set ID closes` with `--reason`,
|
||||
or genesis from the map), it keeps the part still among the new issues, and
|
||||
the receipt ends in `(kept narrowed)`.
|
||||
|
||||
A row's owner can't also be one of its reviewers: `add`, `set ID reviewers`
|
||||
and `assign ID SEAT` each refuse rather than create that shape, since a row
|
||||
can't collect its own owner's required approval.
|
||||
A row's owner can't also be one of its reviewers: `add`, `set ID reviewers`,
|
||||
`assign ID SEAT` and `genesis` (for a map row) each refuse rather than
|
||||
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.
|
||||
|
||||
Exit codes: 0 ok; 1 the operation failed; 2 invalid data or refused;
|
||||
3 uncertain (visible or durable, not acknowledged; for a review request,
|
||||
@@ -145,7 +147,10 @@ semantics 2.
|
||||
- `docs/plans/queue.json`: `{version, canonicalRoot, revision, rows, log}`,
|
||||
serialized deterministically. A file that doesn't re-serialize byte for
|
||||
byte, or doesn't replay from genesis to its `rows`, is refused by every
|
||||
verb.
|
||||
verb. Times are ISO times in UTC to the millisecond; `requiredSince` and
|
||||
`createdAt` may also be a bare date. Each must be a real calendar day and
|
||||
time: a value that doesn't format back to itself, such as month 13 or
|
||||
2026-02-30, is refused.
|
||||
- `docs/plans/QUEUE.md`: hand-written header, generated body between the
|
||||
markers. Each log entry records the SHA-256 of the body it rendered, so a
|
||||
body is `current`, `stale` (an earlier render) or `unknown` (no render).
|
||||
@@ -212,8 +217,12 @@ is swept in:
|
||||
tests and `verify --snapshot` against the base.
|
||||
5. to 7. Blobs, a tree from a temporary index, `commit-tree`, the guard
|
||||
checks again, then `update-ref` with H as the expected old value. If the
|
||||
branch moved at any point after step 1, this fails and nothing is
|
||||
published.
|
||||
branch moved at any point after H was recorded, nothing is published.
|
||||
Each guard check compares HEAD with H before its canary and again if
|
||||
the clean run fails. A move found there exits 2 with `HEAD moved since
|
||||
H was recorded`, since the canary's index holds H's tree and the hook
|
||||
diffs it with the live HEAD. A move after the step-7 check fails
|
||||
`update-ref` (exit 1).
|
||||
8. Reconcile the shared index's two queue entries with `git reset -q --
|
||||
docs/plans/queue.json docs/plans/QUEUE.md`, only if HEAD is the new
|
||||
commit and those entries are still H's. Otherwise exit 3 and print the
|
||||
|
||||
@@ -141,7 +141,15 @@ function checkAfter(v, what = "after") {
|
||||
|
||||
function checkTime(v, what, { unknown = false, date = false } = {}) {
|
||||
if (unknown && v === "unknown") return v;
|
||||
if (typeof v === "string" && (ISO_RE.test(v) || (date && DATE_RE.test(v)))) return v;
|
||||
const iso = typeof v === "string" && ISO_RE.test(v);
|
||||
if (iso || (date && typeof v === "string" && DATE_RE.test(v))) {
|
||||
// The patterns check the shape only. A value passes if it formats back to
|
||||
// itself, which refuses month 13, day 32 and 2026-02-30 (V8 reads that
|
||||
// as March 2).
|
||||
const t = Date.parse(iso ? v : `${v}T00:00:00.000Z`);
|
||||
if (Number.isFinite(t) && new Date(t).toISOString().slice(0, v.length) === v) return v;
|
||||
throw refuse(`${what} is not a calendar ${iso ? "time" : "date"}: ${JSON.stringify(v)}`);
|
||||
}
|
||||
throw refuse(`${what} must be an ISO time${date ? " or date" : ""}${unknown ? " or \"unknown\"" : ""}: ${JSON.stringify(v)}`);
|
||||
}
|
||||
|
||||
@@ -949,7 +957,7 @@ export function applyEntry(state, entry, resolved) {
|
||||
requirePriv(by, "assign a row");
|
||||
if (row.state === "done") throw refuse(`row ${row.id} is done; done rows never change`);
|
||||
if (row.owner === a.seat) throw refuse(`row ${row.id} is already owned by ${a.seat}`);
|
||||
if (row.reviewers.includes(a.seat)) throw refuse(`row ${row.id} owner ${a.seat} can't be a listed reviewer`);
|
||||
if (row.reviewers.includes(a.seat)) throw refuse(`can't assign row ${row.id} to ${a.seat}: ${a.seat} is one of its reviewers`);
|
||||
const claim = row.claim ? { seat: a.seat, op: entry.op } : null;
|
||||
rows.set(row.id, touch({ ...row, owner: a.seat, claim }, entry));
|
||||
result = { row: row.id, field: "owner", from: row.owner, to: a.seat, receipt: receipt(entry, rev, `row ${row.id} owner: ${row.owner}→${a.seat}`) };
|
||||
@@ -1023,6 +1031,9 @@ export function parseMigrationMap(text) {
|
||||
map.rows.forEach((r) => {
|
||||
keysExactly(r, MAP_ROW_KEYS, `migration map row ${r?.id}`);
|
||||
if (r.brief !== null) keysExactly(r.brief, ["path", "anchor"], `migration map row ${r.id} brief`);
|
||||
// The rule add, set and assign apply. Here, not in checkGenesis, so a
|
||||
// log written before the rule still replays.
|
||||
if (Array.isArray(r.reviewers) && r.reviewers.includes(r.owner)) throw refuse(`migration map row ${r.id} owner ${r.owner} can't be a listed reviewer`);
|
||||
});
|
||||
if (!Array.isArray(map.retired)) throw refuse("migration map retired must be a list");
|
||||
map.retired.forEach((id) => checkId(id, "retired id"));
|
||||
|
||||
@@ -223,11 +223,11 @@ test("F1: step 8 with index.lock held exits 3, and ordinary commits stay refused
|
||||
assert.equal(r.g("rev-parse", "HEAD^").trim(), c);
|
||||
});
|
||||
|
||||
test("F1: HEAD moving after commit-tree and before update-ref: refused, nothing published", (t) => {
|
||||
test("F1: HEAD moving after the step-7 guard check and before update-ref: refused, nothing published", (t) => {
|
||||
const r = ready(t);
|
||||
note(r);
|
||||
const h = r.head();
|
||||
r.on("git", `if [ "$phase $sub" = "after commit-tree" ] && once move; then
|
||||
r.on("git", `if [ "$phase $sub" = "before update-ref" ] && once move; then
|
||||
echo other > "$R/other.txt"; git -C "$R" add other.txt; git -C "$R" commit -q -m other
|
||||
fi`);
|
||||
const res = r.qc(["-m", "queue rev 1"]);
|
||||
@@ -238,7 +238,7 @@ fi`);
|
||||
assert.equal(r.g("log", "-1", "--format=%s").trim(), "other");
|
||||
});
|
||||
|
||||
test("F1: H is recorded before the canary, so HEAD moving during the canary makes update-ref fail", (t) => {
|
||||
test("F1: H is recorded before the canary, so HEAD moving during the step-1 canary is refused at step 7", (t) => {
|
||||
const r = ready(t);
|
||||
note(r);
|
||||
const h = r.head();
|
||||
@@ -246,13 +246,58 @@ test("F1: H is recorded before the canary, so HEAD moving during the canary make
|
||||
echo other > "$R/other.txt"; git -C "$R" add other.txt; git -C "$R" commit -q -m other
|
||||
fi`);
|
||||
const res = r.qc(["-m", "queue rev 1"]);
|
||||
assert.equal(res.code, 1, res.err);
|
||||
assert.match(res.err, new RegExp(`moved since ${h}`));
|
||||
assert.equal(res.code, 2, res.err);
|
||||
assert.match(res.err, new RegExp(`refused \\(step7\\): HEAD moved since ${h} was recorded \\(another commit landed\\); run queue-commit.sh again`));
|
||||
assert.equal(r.g("log", "-1", "--format=%s").trim(), "other");
|
||||
assert.equal(r.g("rev-parse", "HEAD^").trim(), h);
|
||||
assert.equal(r.revAt("HEAD"), 0);
|
||||
// Both guard checks ran in full (two canary runs each), so only update-ref caught the move.
|
||||
assert.equal(r.calls("git").filter((c) => c === "before hook").length, 4);
|
||||
// A commit outside the queue files passes the step-1 canary. The step-7
|
||||
// check refuses before its canary, so update-ref never runs.
|
||||
assert.equal(r.calls("git").filter((c) => c === "before hook").length, 2);
|
||||
assert.ok(!r.calls("git").includes("before update-ref"));
|
||||
});
|
||||
|
||||
// Another queue commit landing after H is recorded. The canary's index
|
||||
// holds H's tree and the hook diffs it with the live HEAD, so before
|
||||
// 2026-10-04 the clean run failed and the script blamed the guard.
|
||||
const NESTED = 'bash "$R/scripts/queue-commit.sh" -m nested >> "$CTL/nested.log" 2>&1; echo $? > "$CTL/nested-exit"';
|
||||
|
||||
test("F1: a queue commit landing after H is recorded: step 1 says HEAD moved, not the guard", (t) => {
|
||||
const r = ready(t);
|
||||
note(r);
|
||||
const h = r.head();
|
||||
r.on("git", `if [ "$phase $sub" = "after symbolic-ref" ] && once move; then ${NESTED}; fi`);
|
||||
const res = r.qc(["-m", "queue rev 1"]);
|
||||
assert.equal(r.ctlFile("nested-exit"), "0", r.ctlFile("nested.log"));
|
||||
assert.equal(res.code, 2, res.err);
|
||||
assert.match(res.err, new RegExp(`refused \\(step1\\): HEAD moved since ${h} was recorded \\(another commit landed\\); run queue-commit.sh again`));
|
||||
assert.doesNotMatch(res.err, /not running the queue guard/);
|
||||
assert.equal(r.calls("git").filter((c) => c === "before hook").length, 0);
|
||||
assert.equal(r.g("log", "-1", "--format=%s").trim(), "nested");
|
||||
assert.equal(r.revAt("HEAD"), 1);
|
||||
// Running it again, as the message says, finds the queue already committed.
|
||||
r.off("git");
|
||||
note(r);
|
||||
const again = r.qc(["-m", "queue rev 2"]);
|
||||
assert.equal(again.code, 0, again.err);
|
||||
assert.equal(r.revAt("HEAD"), 2);
|
||||
});
|
||||
|
||||
test("F1: a queue commit landing between the HEAD check and the canary: the failed clean run is reported as HEAD moved", (t) => {
|
||||
const r = ready(t);
|
||||
note(r);
|
||||
const h = r.head();
|
||||
r.on("git", `if [ "$phase $sub" = "before hook" ] && once move; then ${NESTED}; fi`);
|
||||
const res = r.qc(["-m", "queue rev 1"]);
|
||||
assert.equal(r.ctlFile("nested-exit"), "0", r.ctlFile("nested.log"));
|
||||
assert.equal(res.code, 2, res.err);
|
||||
assert.match(res.err, new RegExp(`refused \\(step1\\): HEAD moved since ${h} was recorded \\(another commit landed\\); run queue-commit.sh again`));
|
||||
assert.doesNotMatch(res.err, /not running the queue guard/);
|
||||
// The clean run did run, and failed on the moved HEAD.
|
||||
assert.equal(r.calls("git").filter((c) => c === "before hook").length, 1);
|
||||
assert.ok(r.calls("git").includes("after hook"));
|
||||
assert.equal(r.g("log", "-1", "--format=%s").trim(), "nested");
|
||||
assert.equal(r.revAt("HEAD"), 1);
|
||||
});
|
||||
|
||||
test("F1: a shared-index change during the procedure is not committed", (t) => {
|
||||
|
||||
@@ -20,8 +20,7 @@ function brief(path = "docs/plans/brief-b.md", anchor = "Queue", blob = BLOB) {
|
||||
}
|
||||
|
||||
// A genesis document built the way store.mjs builds one.
|
||||
function genesisDoc(rows = MAP_ROWS, op = "genesis-op-0001") {
|
||||
const map = parseMigrationMap(mapText(rows));
|
||||
function genesisDoc(rows = MAP_ROWS, op = "genesis-op-0001", map = parseMigrationMap(mapText(rows))) {
|
||||
const blobs = new Map(map.rows.filter((r) => r.brief).map((r) => [r.id, BLOB]));
|
||||
const t = at();
|
||||
const grows = genesisRows(map, blobs, op, t, "sage");
|
||||
@@ -507,6 +506,44 @@ test("every accepted text renders to nine cells on every row (N8)", () => {
|
||||
for (const t of ["a\\| done | x", "x<y", "x\\y"]) assert.ok(!accepted.has(t), t);
|
||||
});
|
||||
|
||||
test("genesis: the map refuses an owner among its row's reviewers; replay doesn't (2026-10-04)", () => {
|
||||
const self = MAP_ROWS.map((m) => (m.id === 9 ? { ...m, reviewers: ["darkwing", "filbert"] } : m));
|
||||
refused(() => parseMigrationMap(mapText(self)), /migration map row 9 owner darkwing can't be a listed reviewer/);
|
||||
// A log from before the rule still loads: the check is on the map, not in replay.
|
||||
const old = genesisDoc(self, "genesis-op-0001", { rows: self, retired: [7], highWater: 11 });
|
||||
assert.deepEqual(row(loadDoc(Buffer.from(serialize(old))).doc, 9).reviewers, ["darkwing", "filbert"]);
|
||||
});
|
||||
|
||||
test("times and dates must be calendar values, not just the shape (2026-10-04)", () => {
|
||||
// Row 9 is required, so requiredSince is checked; it allows a date, as createdAt does.
|
||||
const r = row(genesisDoc(), 9);
|
||||
for (const v of ["2028-02-29", "2026-12-31", "2026-04-30"]) {
|
||||
validateRow({ ...r, requiredSince: v });
|
||||
validateRow({ ...r, createdAt: v });
|
||||
}
|
||||
for (const v of ["2026-13-01", "2026-00-10", "2026-01-32", "2026-02-30", "2027-02-29", "2026-04-31"]) {
|
||||
refused(() => validateRow({ ...r, requiredSince: v }), new RegExp(`row 9 requiredSince is not a calendar date: "${v}"`));
|
||||
refused(() => validateRow({ ...r, createdAt: v }), new RegExp(`row 9 createdAt is not a calendar date: "${v}"`));
|
||||
}
|
||||
// ISO times: the same days, and times past the clock. V8 reads 2026-02-30
|
||||
// as March 2 and 24:00 as the next day; only the round trip catches those.
|
||||
for (const v of ["2028-02-29T00:00:00.000Z", "2026-12-31T23:59:59.999Z"]) {
|
||||
validateRow({ ...r, updatedAt: v });
|
||||
validateRow({ ...r, requiredSince: v });
|
||||
}
|
||||
const T = "T12:00:00.000Z";
|
||||
for (const v of [`2026-13-01${T}`, `2026-01-32${T}`, `2026-02-30${T}`, `2027-02-29${T}`, "2026-01-01T24:00:00.000Z", "2026-01-01T23:60:00.000Z", "2026-01-01T23:59:60.000Z"]) {
|
||||
refused(() => validateRow({ ...r, updatedAt: v }), new RegExp(`row 9 updatedAt is not a calendar time: "${v}"`));
|
||||
refused(() => validateRow({ ...r, requiredSince: v }), new RegExp(`row 9 requiredSince is not a calendar time: "${v}"`));
|
||||
}
|
||||
// A shape error keeps its own message.
|
||||
refused(() => validateRow({ ...r, updatedAt: "2026-01-01" }), /row 9 updatedAt must be an ISO time: "2026-01-01"/);
|
||||
// A log entry's time goes through the same check on replay.
|
||||
const d = step(genesisDoc(), "note", { id: 9, text: "x" }, "darkwing");
|
||||
d.log[1].at = `2026-02-30${T}`;
|
||||
refused(() => loadDoc(Buffer.from(serialize(d))), /log entry 1 at is not a calendar time: "2026-02-30T12:00:00.000Z"/);
|
||||
});
|
||||
|
||||
test("replay holds every op id to the caller's rule (N11)", () => {
|
||||
const load = (d) => loadDoc(Buffer.from(serialize(d)));
|
||||
const d = genesisDoc();
|
||||
@@ -549,7 +586,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/);
|
||||
refused(() => step(d, "assign", { id: 9, seat: "filbert" }, "sage"), /can't assign row 9 to filbert: filbert is one of its reviewers/);
|
||||
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$/);
|
||||
|
||||
@@ -63,6 +63,14 @@ test("genesis: the map must be committed, well formed, with committed briefs and
|
||||
no(cli(ghost, genesisArgs(ghost), { by: "sage" }), 2, /row 11 owner ghost is not jason, coordinator, unassigned or a seat/);
|
||||
const noBrief = scratchRepo(t, { map: mapText(MAP_ROWS.map((r) => (r.id === 11 ? { ...r, brief: null } : r))) });
|
||||
no(cli(noBrief, genesisArgs(noBrief), { by: "sage" }), 2, /only a done row may lack one/);
|
||||
// The owner-not-reviewer rule of add, set and assign (2026-10-04).
|
||||
const selfReview = scratchRepo(t, { map: mapText(MAP_ROWS.map((r) => (r.id === 9 ? { ...r, reviewers: ["darkwing", "filbert"] } : r))) });
|
||||
no(cli(selfReview, genesisArgs(selfReview), { by: "sage" }), 2, /migration map row 9 owner darkwing can't be a listed reviewer/);
|
||||
for (const repo of [ghost, noBrief, selfReview]) {
|
||||
assert.throws(() => readFileSync(repo.queuePath));
|
||||
assert.throws(() => readFileSync(join(repo.gitDir, "mosaic-queue.head")));
|
||||
assert.equal(readFileSync(repo.viewPath, "utf8"), queueMd());
|
||||
}
|
||||
});
|
||||
|
||||
test("genesis: markers, a stray witness, once only; a retry returns the receipt", (t) => {
|
||||
@@ -241,7 +249,7 @@ test("add, set reviewers and assign refuse the row's owner as a reviewer", (t) =
|
||||
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/);
|
||||
no(cli(repo, ["assign", "9", "filbert", "--op", "assign-9-filbert"], { by: "sage" }), 2, /can't assign row 9 to filbert: filbert is one of its reviewers/);
|
||||
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" }));
|
||||
|
||||
+16
-3
@@ -74,6 +74,15 @@ HOOK="$ROOT/.git/hooks/pre-commit"
|
||||
|
||||
TMPD=$(mktemp -d "${TMPDIR:-/tmp}/queue-commit.XXXXXX") || die 1 "mktemp failed"
|
||||
|
||||
# The canary's index holds $1's tree, and the hook diffs it with the live
|
||||
# HEAD. Once another commit lands, the clean run can fail with the guard
|
||||
# working as installed, so a moved HEAD is reported as that, not as the guard.
|
||||
head_moved() {
|
||||
local now
|
||||
now=$(g rev-parse --verify -q HEAD) || die 2 "refused ($2): HEAD has no commit"
|
||||
[ "$now" = "$1" ] || die 2 "refused ($2): HEAD moved since $1 was recorded (another commit landed); run queue-commit.sh again"
|
||||
}
|
||||
|
||||
# --- the queue guard, active and not just present (8.12, G2). $1 is the
|
||||
# commit whose hook blob and tree the checks use. ---
|
||||
guard_check() {
|
||||
@@ -88,11 +97,15 @@ guard_check() {
|
||||
|| die 2 "refused ($when): .git/hooks/pre-commit differs from $HOOK_REL at $h"
|
||||
out=$(g config --show-scope --get-all core.hooksPath 2>/dev/null)
|
||||
[ -z "$out" ] || die 2 "refused ($when): core.hooksPath is set ($(printf '%s' "$out" | tr '\n\t' '; ')), so git would not run the queue guard"
|
||||
# The canary: git itself runs the hook it would run for a commit.
|
||||
# The canary: git itself runs the hook it would run for a commit. HEAD is
|
||||
# checked before it and again if the clean run fails, since a commit can
|
||||
# land between the two.
|
||||
head_moved "$h" "$when"
|
||||
idx="$TMPD/canary-$when/index"
|
||||
mkdir -p "$(dirname "$idx")"
|
||||
GIT_INDEX_FILE=$idx g read-tree "$h" || die 1 "canary ($when): read-tree failed"
|
||||
if ! out=$(cd "$ROOT" && GIT_INDEX_FILE=$idx git hook run pre-commit 2>&1); then
|
||||
head_moved "$h" "$when"
|
||||
die 2 "refused ($when): the canary's clean run failed, so git is not running the queue guard as installed: $out"
|
||||
fi
|
||||
GIT_INDEX_FILE=$idx g update-index --add --cacheinfo "100644,$(g rev-parse "$h:$HOOK_REL"),$QMD" \
|
||||
@@ -133,8 +146,8 @@ if [ "$MODE" = install ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# --- 1. guard. H is recorded first, before the canary, so a branch that moves
|
||||
# at any later point makes update-ref in step 7 fail. ---
|
||||
# --- 1. guard. H is recorded first, before the canary. A branch that moves
|
||||
# later is refused by a guard check or, after step 7's, by update-ref. ---
|
||||
H=$(g rev-parse --verify -q HEAD) || die 2 "refused: HEAD has no commit"
|
||||
BRANCH=$(g symbolic-ref -q --short HEAD) || die 2 "refused: HEAD is detached"
|
||||
guard_check "$H" step1
|
||||
|
||||
Reference in New Issue
Block a user