stack/agents/filbert/work/queue-126/r1/body.md

Filbert review, row 126 round 1: approve

Candidate manifest d18314f70c44917ae4fe870891f2f2e4bcef12d3bca8f3815e99ee67349a8e65 (39 paths), build.patch 5e7ab310196eb7a4732af9b11b8f95273672f39c0a6d35509e6816d63a3be9be. I applied it with --index to 81f26551b. All 39 manifest lines matched, git diff --cached --check is clean, and the diff is +1334 -54 over 39 files. I read it against the brief anchor "Daily quota for the credential the stack uses today" (blob 6b473b93), BUILD.md from "Round 4", MIGRATION.md, and the addendum in comment 29064.

The 3h to 3i step (where to look 1)

• Text. The CREATE TABLE usage_records text in 3h-3i.sql:9-32 is byte-equal to schema.sql's (cmp). The three triggers at :38-40 are byte-equal to both schema.sql's and 3g-3h.sql:26-28. The only schema.sql change is the account line at :347. Nothing else in schema.sql names usage_records (no view, index or foreign key), so the rename touches nothing outside the table.
• Order. Triggers drop first (:5-7), then the rename (:8). So the rename has no trigger text to rewrite, and the new table's text stays unquoted. schemaDigest (store.mjs:77-84) sorts by type and name, so the rebuilt rows' new place in sqlite_master doesn't matter.
• Sequence. The rename moves the sqlite_sequence row to usage_records_3h. The copy (:33-34) makes a fresh row for usage_records at max(seq). :35-36 deletes that row and inserts the old value. DROP TABLE usage_records_3h (:37) then removes the old row. An empty 3h table has no sequence row, so the insert adds none, which is what a never-used AUTOINCREMENT table looks like.
• Transaction. #migrateSchema (store.mjs:197-221) runs the step SQL, the meta rewrite and the toDigest check inside one transaction() (:251, BEGIN IMMEDIATE, ROLLBACK on throw). That is condition 1 in code.
• Plan parse. stack-release.sh plan_steps (:607) greps only from:/to: lines, so the new step's comment line doesn't break it.

Test 1 (account-migration.test.mjs:75) sets the sequence to 50 with three rows, so a recomputed sequence would give 3 and the next seq 4. The test checks both: :81 compares every table's rows, sqlite_sequence included, and :96 checks the next seq is 51. It compares all of sqlite_master (type, name, tbl_name, sql) at :87 with the one substitution, after :86 checks that only usage_records held the old line.

The forced failure (where to look 2)

Test 2 (:109) adds INSERT INTO usage_records SELECT * FROM usage_records_3h LIMIT 1 before the DROP TABLE (:114-117, and :116 asserts that the anchor occurs once). On the import-time replay the legacy store is empty, so the extra insert copies nothing and the engine imports. await engine(...) sits outside assert.throws, so an import failure would fail the test some other way, not pass it. On the 3h store the duplicate seq fails with UNIQUE constraint failed: usage_records.seq, after the drops, the rename and the copy. The triggers are gone at that point, so it is the primary key that refuses, which is the message :119 expects. Afterwards the test checks the version (3h), sqlite_master and every row, and the real step then migrates the same store. So the failure is at run time, after the copy, as asked.

The account-is-credential guard (where to look 3)

launcher.mjs:249-258. For a claude-code launch with an account, it refuses account-is-credential when the name equals the value of any variable in SESSION_CREDENTIALS (seat/src/session.mjs:98, ANTHROPIC_BASE_URL included). This runs before account-credential-missing and account-credential-ambiguous, and both of those put account ${account} in their message. So the order is what keeps a pasted value out of the log, and the test (launch-account.test.mjs:83) checks the log for the value with one and with two matching variables.

My reading is that equality is the right shape for condition 3, on top of the shape checks:

• The business read already holds the name to ID_PATTERN (vocabulary.mjs:127, lowercase, at most 64 characters, through checkAccount). The broker holds it to identifier() (broker.mjs:127).
• Equality closes the remaining case: a lowercase credential that passes the shape check, pasted whole.
• A containment check would also catch a name that is a piece of a credential value, but it would refuse short names that happen to occur in a token. I wouldn't widen it.

Limit, as BUILD.md says: it compares against the launching host's credential variables only. Not blocking.

Round 3's subject change (where to look 4)

The plugin contract already said subject is {instance, provider, model, account} at base (plugins/README.md:150 on 81f26551b). The candidate makes the code match:

• the broker's mustStop subject gains account: s.account ?? null;
• runtime's mayLaunch passes account = null through.

The three tests, land.test.mjs:538, limit-stop.test.mjs:61 and runner.test.mjs:634, each change only by account: null on the subject. So these are the right fix, and none of them widens what the test accepts.

The land-work gap (where to look 5)

Confirmed: scripts/land-work/read.mjs:20 imports SCHEMA_VERSION from the checkout's store.mjs, and :65 refuses bus-schema on a meta mismatch. Once 3i is on refactor, land-work from the canonical checkout refuses against a live 3h store until the window runs. MIGRATION.md leaves the choice to Sage (land-work from a 3h tree, or land just before the window). I agree it's Sage's call, and it doesn't change the candidate.

The gate's non-UTC day boundary

The main test is daily-quota.test.mjs:64: 05:00 America/Chicago is 10:00Z. 09:59:59.999Z counts in yesterday's day, and 10:00Z starts a new one. I traced D01 and D02 against it:

• D01 (no step back) gives 0 for 11 at :71;
• D02 (window in UTC) counts the 09:59Z record at :68 and gives 110 for 100.

The DST tests at :75, :84, :93 and :104 cover the 23-hour day, the 25-hour day, the skipped 02:30 and the repeated 01:30. I checked the instants in their comments against Chicago's 2026 changes. The zone comes from the leaf (packages/plugins/limit-check/daily-quota/index.mjs:23-34, Intl.DateTimeFormat with timeZone), and the plugin reads no process TZ. So these tests hold under any host time zone.

One precision on the addendum: daily-quota-runtime.test.mjs:41 and :53 configure America/Chicago with a 05:00 reset, but with limit 0 and every record at the same moment. They show the zone and account reach the shipped check through the real loader. They don't exercise the boundary. The boundary evidence is the plugin tests above, which I think satisfies the gate item. Sage judges the gate.

Mos's six conditions

I checked each mapping in BUILD.md against the code and tests:

1. Test 2 and transaction(), above.
2. Test 1's whole-sqlite_master compare, above.
3. The three layers, above.
4. MIGRATION.md "The way back" and test 3 (:129), which builds a 3h engine with the step cut out and asserts the cut (:141). Both opens refuse schema-mismatch and the store stays at 3i, unchanged.
5. DESIGN.md: I didn't check this one beyond BUILD.md's pointer.
6. The cached public/ diff is byte-equal to public.diff (1964ec2c...a56c), and its +/- digest is 03e6d9daed6b74878f80f8b05341b89b7defc6611d260542fbf452692653ba78.

Other checks

• Bus README counts. grep -c on schema.sql gives 16 tables, 71 triggers and 5 views, as the new paragraph says.
• Hash pins. No 64-hex pin in packages/*/tests or scripts/test-*.sh names a non-test manifest path. The old and new schema sha appear only in store.mjs, the bus README, the new test's fixture pin and queue.json. The '3h' literals in stack-release.test.mjs are synthetic store.mjs copies (packages/cli/tests/stack-release.test.mjs:234), not the real one.
• Base drift. From 81f26551b to HEAD ee3f1fb3f, no manifest path changed. build.patch passes git apply --cached --check on HEAD in a temporary index.
• The commit message. The packet has no commit-msg.txt, so there was nothing to check for an attribution line. Whoever writes it at landing should leave one out (mos-0822).

Notes (not blocking)

1. The brief's scope line. The brief's acceptance says the diff outside daily-quota/ touches only the account naming, the two DEPTH tables, checkTokenLimits and the fixed line. The 3h to 3i step and the account-is-credential guard go past that list. BUILD.md records the lead's choice (sage-0970) and Mos's go (mos-0835) for both, so I don't treat it as a candidate finding. Whether the brief text should record the change is Sage's call.
2. The row 167 rebase. If row 167 lands first, I'll do the delta check on the new round: the bus.js sha moves and the public/ digest should stay 03e6d9da...ba78.

Ran by me

Nothing that needed the slot. I built the worktree, ran sha256sum -c (39/39), --check, the public diff cmp and digest, and the cmp checks on the table and trigger text. I read the step, store.mjs, the launcher guard, the plugin and the tests named above. I ran the greps, the README counts and the HEAD apply check. I ran no test and no mutant. The D01 and D02 kills are my reading of the code.

Not mine

Dewey's slot run 3 (00:50Z to 00:59:04Z), as BUILD.md records it:

• 19 touched files singly: 157/157;
• 162 other singles: pass 1384, fail 0, skipped 1 (the esbuild 0.28.2 pin);
• test-land-work.sh: 86 selftests;
• mutants: 30 of 30 killed, D27 to D30 included.

The logs are under ~/dewey-scratch, which I don't read. The full gate is Sage's at the landing target.