Files

5.8 KiB

Filbert review, row 162 (D1) round 1: approve

Candidate manifest 091693c51d137e4cd31e75ee22aef4f22c307e15abce6f91f6c518e3ff78fdd6 (5 paths). I applied r1/build.patch (f5d28a75...) with --index to cfde62f9f. All 5 manifest lines matched, and git diff --check was clean.

This was a static review, per sage-0873. I read all of scripts/stack-release.sh, the stack-copy.sh diff, the brief section (blob cfc468c9df88), bus-service.sh install_guard, the hard-look tests and both doc diffs. I ran no tests, mutants or probes.

Hard-look items, checked

  • PM journal check (ruling 2, T10b). pm_launched (stack-release.sh:387) reads journalctl --user -u BUS --since @START_EPOCH -o cat. That is an epoch, so the CDT reading of a bare time can't hit it. It needs exactly one ^launched line, parses the run id, and then needs that run in the copy's launches list --json with this business, live: true and startedAt >= START_MS.
    • The pattern fits what the bus prints, packages/cli/src/cli.mjs:503: launched ${pm.instance}: run ${pm.run}, ${pm.harness} ${pm.model}. The fixture line (test :63) has the same shape.
    • T10b (:475) covers all eight failure reasons, one subtest each, plus a pass case that checks the --since argument is @<digits>.
  • The store read (T22, T23). STORE_JS (:223) opens a stopped store with ?immutable=1 and readOnly, but only after store_unclean (:232) finds no -wal or -shm. While the bus holds the store, it uses a plain readOnly open. That is the sage-0739 rule and the probe result in BUILD.md.
    • T22 (:716): a -wal or -shm left by the stop refuses store-unclean, and the bus directory at the restart matches the stop snapshot. On a clean store, only bus.sqlite is there.
    • T23 (:626): --plan on a stopped store and on a held store leaves the data tree, the units, the wrapper and the store sha unchanged, writes no log or lock, and leaves only bus.sqlite when stopped.
  • Rollback after a --restore swap (T14b, T14c). after_fail (:462) goes to rollback_failed with not-attempted once RESTORED is set, and to item 6 otherwise.
    • T14b (:552): exit 2, both units inactive, a single start pair, and the message names the rollback set, the backup and the post-attempt store. The post-attempt store keeps the migrated sha, the store equals the backup sum, and the pointer is byte-identical.
    • T14c (:566): without --restore, item 6 runs, the result is gate-failed with rollback ok, the old copy is active, and the fingerprint is unchanged.
  • migration steps: (plan_steps, T27). The walk is bounded by the map size, and an odd count or a pair that doesn't match prints cannot be read from <file>. T27 (:609) covers two steps, a chain that doesn't reach the target, and a short chain.
    • The real packages/bus/src/store.mjs at the base has SCHEMA_VERSION = '3h' (:17) and a full chain from 3b to 3h, including 3f/3g (:47-48) and 3g/3h (:51-52). So the 3f to 3g, 3g to 3h line in Darkwing's scratch-plan-8e7ea89a1.txt matches the source.

I also checked:

  • migrate-schema.mjs takes exactly one argument, an absolute dataRoot, and the script's call matches.
  • The scratch plans at 606fafd43 and 8e7ea89a1 both say 3f to 3h with --migrate and the Jason line, and list 112 and 92 commits. Both are scratch roots with a hand-written pointer, as their headers say.

Non-blocking notes

  1. drop_in_override (:503) is laxer than install_guard. It uses the same three directory patterns, but skips unreadable directories and files ([ -d ], grep 2>/dev/null), where install_guard dies.
    • Effect: a drop-in that is there but can't be read gets past the precheck. install_guard then refuses after the stop, so the run goes through item 6. That fails closed, but the live stack stops for nothing.
    • A follow-up could refuse an unreadable path in the precheck.
  2. A race between is-active and the read in plan_store (:577) and the rollback count (:716). If the bus stops between the check and the readOnly open, the open can leave -wal or -shm beside a stopped store (the probe result). The next run then refuses store-unclean. That fails closed and the window is small.
  3. The copy-check gate step (:374) runs stack-copy.sh check --business, which proves bus, Console and CLI agree on one copy, but doesn't compare that copy with the target. Earlier gate steps and the successful install make a mismatch unlikely. Passing the expected id would close it.
  4. read_units (:157) splits Environment= on spaces, so a MOSAIC_CLAUDE_BIN path with a space would be read wrong. Today's pin has none.
  5. Brief item 8 (:180-185) says the plan reads the schema "with no store read". sage-0745 and mos-0741 replaced that with the read, and the code follows the ruling. The brief text can be re-pinned later.

docs/TOOLS.md "Pinned stack update" and the packages/cli/README.md paragraph match the script: the exit codes, the log results, step 4 for the drop-in (the refusal text in test :401 cites it), the gate list and the restore and events rule. Two packet notes:

  • BUILD.md :312 and :610 place the launched line at cli.mjs:476 and :479. At cfde62f9f it is at :503. The packet isn't in the manifest, so this doesn't block.
  • BUILD.md Notes already records that the drop-in removal is step 4 here and step 3 in the brief.

Ran by me

Nothing. I used no slot this round.

Not mine

Darkwing's round 1 runs (BUILD.md "Candidate round 1 checks", 19:25:10Z to 19:46:14Z):

  • stack-release.test.mjs 58/58 and stack-copy.test.mjs 12/12, each run singly;
  • 47 mutants killed, then M41, M45b, M46 and M47 to M49 killed;
  • the two scratch-root --plan runs.

I read the tests and the mutant table, but did not rerun them. The full gate is Sage's at landing, and the live --plan is Mos's window.