Both testing blind spots are closed — what remains is a fetch-deadline floor in safeFetch, then two ratchets (deadline + responsive-label)

[serge-ivo]

Architecture scan: the bug rate is not a size or coverage problem — it is two specific blind spots, and the codebase already invented the fix for one of them

Prompted by "we're getting lots of bugs lately". I used the 14 issues filed in one session (#420-#433) as the sample, then looked for shared structural causes rather than surveying file sizes.

What is NOT the problem (checked, so it can be ruled out)

• File size. scripts/check-file-size.mjs (from The #138 refactor was undone within hours — add a size ratchet so decomposition holds #302) passes: "14 pinned files at 15961/15990 lines; 593 source files scanned, none new over 800." The ratchet is working. Margin is thin (29 lines), but nothing is over budget.
• Test volume. workers/api/src: 273 test files / 296 sources. Routes: 44 / 59. agent-do alone has 5 test files. This is a well-tested codebase by any normal measure.
• Documentation. Consistently good — the architecture docs were correct even where I misread them ([docs] "Can I use tmux for coding?" has no correct answer in the docs — the architecture split is documented, the user-facing choice and its unmetered cost are not #433).

So "write more tests" and "split big files" are both the wrong prescription. The bugs are getting through a suite that is large and a codebase that is well-decomposed.

Blind spot 1 — no SQL in the test suite is ever executed

test files mocking D1 by string-matching SQL : 59
@cloudflare/vitest-pool-workers / miniflare  : not configured

Every D1 test is of the shape if (sql.includes("FROM error_log")) return { results: … }. The SQL string is matched, never parsed, never run. So the suite cannot detect a syntax error, a wrong column, or a platform limit.

This is precisely how #423 shipped. activeInstancesForDay builds 6 SELECTs joined by 5 UNIONs — which D1 rejects outright. It failed on every cron tick for 29.6 hours, produced 1780 identical error rows (97% of the entire error log), and the stats feature it powers has never written a row in production. It passed CI green, because stats-rollup.test.ts covers buildSeries / completedDay / enumerateDays / trendCards — the pure helpers — and never executes the query.

A test that runs SQL against real D1 would have caught it the moment it was written.

Blind spot 2 — the contracts between layers are unverified

tests asserting the query string a route passes to its DO : 0

This is how #428 shipped. "Load older messages" is broken in three places at once:

1. the client sends a UUID as an ordering cursor (InstanceDetail.tsx:475),
2. the route rebuilds the query string with only limit, silently dropping before (chat.ts:160),
3. the DO never implemented before at all (agent-do.ts:768).

Each layer is individually tested. Each test passes. Nothing tests the hop. Verified against production: page 2 is byte-identical to page 1.

#432 is the same shape one layer down — terminal snapshots are persisted, fetched in full, and then all but the last are discarded in the client.

The pattern behind the rest: a fix applied to one sibling and not the other

Issue	Correct code exists at	The sibling that missed it
#431	CodingTab.tsx:807 — hidden sm:inline on the 2-button toggle	:887 — the 4-button row, 80 lines below, same file
#430	lengthRule computed at agent-think.ts:645 for every agent	emitted only inside if (plainSpeech) — 1 of 4 branches
#427	—	the same non-streaming-under-a-deadline defect as #421, in a different subsystem
#426	—	mobile overflow, same class as #431

Two of these are literally the same bug in two places (#421/#427, #426/#431). Nothing propagates a fix to analogous code, so a defect class gets fixed once and survives everywhere else.

The codebase already invented the right tool — and applies it inconsistently

This is the important finding. There is an existing, sophisticated pattern for exactly this failure mode:

mute-invariant.test.ts · mute-touch-invariant.test.ts · security-invariants.test.ts
prompt-claims.ts · agent-claims-lint.ts · usage-claims.test.ts · seed-drift.test.ts
+ 6 scripts/check-*.mjs CI guards

mute-invariant.test.ts is the model: it does not pin today's implementation, it asserts a reachability property across every phase, and it includes a structural scan of use-voice.ts because "no pure test can see it". Its header explains why a locally-correct one-line fix would have silently deleted a feature — the exact failure mode above.

That tool exists for mute, security, prompt claims, usage claims and seed drift. It does not exist for:

• "every styleGuidance branch carries a length rule" ([bug] Coding agents get NO length rule in the prompt — lengthRule is computed for all and emitted only inside plain-speech, and the Behaviour tab shows "balanced" for a field that is unset #430)
• "every external call has a deadline" ([bug] A stalled transcription hangs on "Transcribing…" forever and locks the composer — no timeout, no retry, no dismiss, reload is the only escape #421, [bug] Chat times out at 25s on the tool loop's second round — a 4096-token cap under a 25s non-streaming ceiling is an impossible combination #427)
• "every mobile control row hides labels below sm" ([bug][mobile] Copy and Delete sit on top of the message timestamp — 42px of 110px covered in WebKit, and removing the year only recovers 30px #426, [bug][mobile] Terminal/Issues/Pulls/Actions tabs overflow — the icon-only pattern exists 80 lines above in the same file and was not applied #431)
• "every cursor parameter survives client → route → DO" ([bug] "Load older messages" re-returns the newest page — the before cursor is a UUID, the route drops it, and the DO never implemented it #428, [bug] Terminal has no scrollback — every snapshot is fetched then discarded except the last, and none are written while the engine is busy #432)

Recommendation — three changes, in order of value

1. Execute SQL in tests. Adopt @cloudflare/vitest-pool-workers for the query-bearing modules so D1 statements actually run against real D1/miniflare. Highest value per unit of effort in this list: it converts an entire invisible defect class (#423) into a CI failure. It does not require rewriting the 59 existing mocks — start with the modules that build dynamic SQL, which is where the risk concentrates.

2. Contract tests at the seams. For each parameter crossing client → route → DO, one test asserting it survives the hop. Cheap, mechanical, and would have caught #428 and #432. The natural home is beside the existing route tests.

3. Extend the invariant-test pattern to the four properties above. This is not new methodology — it is applying a proven in-house pattern to the properties that are currently unguarded. Each is a small, pure, scanning test in the shape mute-invariant.test.ts already established.

What I deliberately do NOT recommend

• A large refactor or re-architecture. The evidence does not support it. Modules are well-separated, purity is used heavily and deliberately, and the docs are accurate. The defects are at joins and in unexecuted branches, which a re-architecture would move rather than remove.
• Splitting more files. The ratchet already governs size and passes.
• More unit tests. The ratio is already 273/296. More of the same kind would not have caught a single one of the 14.
• Adopting an outside testing framework. The #138 refactor was undone within hours — add a size ratchet so decomposition holds #302's history is instructive: a decomposition was undone within hours and needed a ratchet to hold. A pattern already used in this repo will stick; an imported one is likely to be abandoned.

Acceptance criteria — three of five are MET; only the last two remain (audit 2026-08-23)

Both blind spots the title names are CLOSED. Do not rebuild them.

• A deliberately malformed D1 query fails a test rather than production. — workers/api/src/lib/d1-sqlite.ts (298 lines, a real SQLite engine for the suite), lib/sql-execution.test.ts, lib/sql-schema.test.ts. Commit 77e17c1c. The sweep runs 502 statements across 117 modules against the schema the migrations build.
• Removing a parameter from a route's DO request fails a test. — workers/api/src/lib/do-seam.ts + lib/do-seam.test.ts (commit 597e2377, the dispatch table that can be asked which query parameters a path takes) and workers/api/src/routes/do-seam.contract.test.ts (435 lines, commit 99174a9c). Proven red on the exact [bug] "Load older messages" re-returns the newest page — the before cursor is a UUID, the route drops it, and the DO never implemented it #428 hunk.
• Adding a styleGuidance branch without a length rule fails a test — satisfied by the TYPE SYSTEM, which is stronger than the scan this asked for. lib/agent-style-prompt.ts:196-217 makes StyleBranch.lengthDefault a required field, so a fifth branch omitting it fails tsc --noEmit, which CI runs as a named gate. Behaviour also pinned at lib/agent-style-prompt.test.ts:59 describe("every styleGuidance branch carries a length rule (#430)").

What remains — two guards, and one of them needs a floor built first

• Adding an external call with no deadline fails a test.

  Do not write the scan first — it would go red on 62 of 65 call sites and be suppressed. Measured on main, workers/api/src, excluding tests: 65 fetch( sites, 3 files constructing an AbortController (lib/user-ai.ts:191,421, lib/steps.ts:332, lib/mcp-credentials.ts:520), and 0 uses of AbortSignal.timeout.

  Two steps, in order:

  1. The floor — give safeFetch (workers/api/src/lib/ssrf.ts) a default AbortSignal.timeout. It is already the chokepoint every connector, fetch_url and ingest path goes through, so this converts most of the 62 in one edit without touching a call site. This is a finding in its own right and is the higher-value half.
  2. The ratchet — only then the scan, in the sql-schema.test.ts shape: extract fetch( sites, assert each carries a signal or routes through safeFetch. Pin it EXACTLY (like control-shapes.test.ts), not as a <= ceiling.

• Adding a mobile control row without responsive labels fails a test.

  No responsive-label scan exists. Its natural home is the existing harness, not a new file: store/console/src/lib/control-shapes.test.ts (the exact-pinned ratchet over hand-authored button/card shapes), alongside store/console/src/lib/control-classes.test.ts:160-169 which already holds the console / agents/coder/web / store/admin copies equal.

  Its first assertion should be the live defect the harness already records — control-shapes.test.ts:150: // className="hidden sm:flex" on a <Button> does not hide it — BUTTON_BASE wins, silently. That is a real class of silently-dead responsive utility, not a hypothetical.

Files this work touches: workers/api/src/lib/ssrf.ts (the floor) · a new fetch-deadline scan beside lib/sql-schema.test.ts · store/console/src/lib/control-shapes.test.ts. Collides with nothing else currently open — the two halves are independent of each other and of every other issue in the delivered-work audit.

Sequencing note

#423 should be fixed before (1) lands, not after — 97% of the error log is that one bug, and the log is the instrument for judging whether any of this works. #424's audit and its alerting gap (errors24h is computed, thresholded by nobody) are the other half: these changes are only verifiable if someone finds out when they fail.

Related: #423 (the SQL that no test ran), #428 / #432 (the seam), #421 / #427 and #426 / #431 (one defect class, two sites each), #430 (computed everywhere, used in one branch), #302 (the ratchet that proves guards hold where good intentions did not), #424 (nobody is watching the log).

[serge-ivo]

Second live instance of blind spot 1, filed today as #434: lib/instance-work.ts builds one UNION branch per subordinate at runtime, so subordinate_status breaks at 6+ subordinates — the same D1 ceiling as #423, in a different feature. The live Coder Lead has exactly 5 today.

It sharpens your point in two ways:

• The static guard [bug] The stats rollup has failed on every cron tick since it shipped — 1780 identical D1 compound-SELECT errors in 29h, and it is 97% of the error log #423 shipped names this file as its blind spot (lib/sql.ts:264-270: "a union assembled at RUNTIME … a static scan cannot decide it; it needs a cap at the point of construction"). So even after the fix, the class is only covered where the SQL is a literal.
• The test that hides it is asserting the defect: instance-work.test.ts:28-37 — "issues EXACTLY ONE statement for 12 subordinates" — against a stub that records SQL strings and never parses them. Real-D1 execution would have failed it on the day it was written.

[serge-ivo]

Blind spot 1 is closed. Blind spot 2 is not — leaving this open for it. Shipped as 0f274d2 + 77e17c1, CI green (incl. Playwright chromium + webkit), API Worker deployed.

What landed

One validator, two guards. assertD1Preparable (workers/api/src/lib/d1-sqlite.ts) applies D1's five-term compound-SELECT ceiling and then prepares the statement against a real SQLite carrying all 112 migrations. The ceiling is applied by us on purpose: local SQLite's SQLITE_MAX_COMPOUND_SELECT is its compile-time default of 500, which is exactly why #423 was invisible in every environment anyone could run. The migrations are the only definition of the schema in this repo, so building it from them asks the question production asks.

1. sql-schema.test.ts — the whole-Worker sweep. findLiteralSqlStatements (added to sql.ts) extracts every complete SQL literal in workers/api/src — 502 statements across the 117 modules that issue SQL — and prepares each. Nothing is called.
2. sql-execution.test.ts — the executing harness. realSchemaD1() is a D1Database over that schema that really runs the statement, which is the only way to reach SQL assembled at runtime. Foreign keys stay ON, so a fixture that violates the schema fails in the test.

Proven red on the code as it shipped, not on a paraphrase

Defect	Restore	Result
#423	git show 7852232^:…/stats-rollup.ts	sweep fails at lib/stats-rollup.ts:75 — too many terms in compound SELECT: 6 > 5. Plain SQLite parses that query happily; only the ceiling catches it.
#434	git show 3ef2285^:…/instance-work.ts	5 of the supervision tests fail with 12 > 5, at 12 subordinates — the product limit.

It found a live bug on its first run

external-usage.ts issued SELECT id, roles, github_login, email FROM users. users has no email column and never has — identity is a GitHub/Google login, and 0016's profile fields went to user_profile. D1 answers no such column: email at PARSE time, the surrounding catch {} swallowed it, and so the roles-and-allowlist branch — the whole point of operatorUserIds — has never executed once: ADMIN_ALLOWLIST did nothing, and every admin other than the caller counted as external. That is the "identified NOBODY" symptom this file's own comment attributes to the token-only case, which means the number the #68 focus bet is measured against was read through a query that always threw. Fixed in 0f274d2; isAdmin already matched on id/github_login only, so this brings the two readers back into agreement. It was the only finding across all 502 statements.

What these guards cannot see — stated in the files, not left to be assumed

• A statement that PARSES can still be wrong. [bug] "Executions" is structurally zero on every agent — one legacy route writes agent_executions, and no modern path does #451's WHERE agent_id = ?1 names a real column that is NULL for every row in production: valid SQL, zero rows, forever. Parsing has no opinion on data. Neither guard would have caught [bug] "Executions" is structurally zero on every agent — one legacy route writes agent_executions, and no modern path does #451 and neither claims to.
• The sweep sees nothing with a ${…} in it — ~39 statements, and they are the risky shapes (interpolated column lists, WHEREs built from arrays, IN (${…}), per-call table names). [bug] A supervisor with 6+ subordinates is told its team is idle — instance-work builds one UNION branch per subordinate and D1 parses five #434 is exactly that shape. Those are reachable only by driving the code through realSchemaD1(), which is done for the two modules with a defect on record. Every other runtime-assembling module is uncovered until someone writes it a test — that is a real gap, and the harness exists so the cost of closing one is a fixture rather than a framework.
• D1's other ceilings are not modelled. The documented ~100 bound parameters and the statement-length cap have not been measured against production the way D1_MAX_COMPOUND_TERMS was ([bug] The stats rollup has failed on every cron tick since it shipped — 1780 identical D1 compound-SELECT errors in 29h, and it is 97% of the error log #423), and a guard asserting a number nobody checked is how a false red teaches people to suppress it. D1's argument type-checking is not modelled either.
• Query plans are invisible: an index that stops being used is a production incident this cannot see.

Two extraction rules were learned from real false readings and are documented where they live: adjacent "a" + "b" fragments are joined (stats-store.ts's SELECT_CONFIGS otherwise reads as a truncated query), and a statement must begin with a SQL verb (prompt prose in apply-loop.ts contains both a verb and a clause keyword and is not SQL; it also contains the blast radius of the one file this scanner mis-lexes, mcp-tool-catalog.ts, whose replace(/"/g, "'") inside an interpolation leaves it a quote out of step).

What I did NOT do, and why

Blind spot 2 — a parameter crossing client → route → DO — is not addressed. I could not design a guard I could prove goes red on #428, and the reason is in #428 itself: all three layers were broken simultaneously. The client minted a cursor the ordering could never seek with, both routes dropped before, and the DO never implemented it. No pairwise static relationship existed to violate, so any "the route must forward what the DO reads" guard is green on the pre-fix tree — the DO read nothing to forward.

The implementable form is narrower and worth its own ticket: for every query parameter a DO handler reads, every route forwarding to that DO path must pass it. That catches the #421/#427, #426/#431 class applied to seams — fixed in one sibling, missed in the other, which is precisely how chat.ts and instances-chat.ts both had to be repaired in #428 — and it catches a future re-drop of before. It does not catch a three-layer simultaneous omission, and only an executed round-trip does (which #428 delivered via message-page.ts + console.spec.ts). I would rather say that than ship a guard whose claim is wider than its reach.

Also not done: the remaining acceptance criteria (styleGuidance length rule, external-call deadline, mobile responsive labels) — three separate mute-invariant.test.ts-shaped scans, independent of this work.

Verification

pnpm typecheck · biome --error-on-warnings · all 7 scripts/check-* gates · pnpm docs:drift · --project unit 397 files / 6715 tests green, which contains everything this touches. CI on the pushed SHA is green including pnpm test:e2e. I did not run Playwright locally — this changes no store/ or agents/coder/web file. Local full-suite runs flaked in packages/browser-runner integration files (wall-clock timeouts, and one tmux capture-pane: no server running from a concurrent agent's suite) at load average 60–85 with 19 vitest processes on the box; each passes in isolation and on an origin/main throwaway worktree, and CI runs alone.

Not verified in production: GET /v1/admin/usage/external is admin-gated (confirmed 401 unauthenticated, 404 on the wrong path — the route is live) and I hold no admin session, so I could not observe the corrected operator classification. Worth one check by someone who can: with an admin session, GET /v1/admin/usage/external?days=30 should now identify operators from users.roles + ADMIN_ALLOWLIST instead of the caller alone.

[serge-ivo]

Blind spot 2 is now guarded. Two commits on main, CI green, API Worker deployed.

SHA	Subject
597e237	the AgentDO's dispatch table can be asked which query parameters a path takes
99174a9	a query parameter the Durable Object reads is asserted to reach it from every route that forwards there

New: workers/api/src/lib/do-seam.ts (+ test) and workers/api/src/routes/do-seam.contract.test.ts. No route, DO or config file was modified — this is a guard, not a repair.

It extends the in-house pattern rather than importing one

The route half is driven, exactly as instances.contract.test.ts drives its surface: mount one router on a bare Hono, drive each GET, record what reaches a recording AGENT stub. The DO half is scanned, in the shape security-invariants.test.ts and mute-invariant.test.ts use when no pure test can see the property. The DO half has to be a scan — agent-do.ts extends DurableObject from cloudflare:workers, which does not resolve under vitest; relay-do.test.ts records the same constraint and re-implements the object rather than import it, and a re-implementation would be a paraphrase asserting itself.

The fixture is the deliberate mirror of instances.contract.test.ts: that one resolves no ownership because a refusing tenant gate is its subject; this one owns everything, because a gate that refuses stops the request before the seam.

Red-test proof, both directions

Positive — delete a forwarded parameter from a live route. Removed the before forward from routes/chat.ts (the exact #428 hunk):

 ❯ do-seam.contract.test.ts (6 tests | 3 failed)
   × forwards exactly what the table records
     -      "/messages?before,limit",   +      "/messages?limit",
   × every route forwarding to a DO path passes every parameter that path takes
     + { "chat.ts GET /:id/messages → /messages":
     +   "?before sent as msg:2026-01-01T00:00:00.000Z:probe, arrived as (absent)" }

Negative — the pre-#428 tree. git checkout 807787a^ -- workers/api/src/{agent-do.ts,routes/chat.ts,routes/instances-chat.ts}:

 ✓ a parameter the object reads survives the hop > every route forwarding to a DO path
   passes every parameter that path takes

The invariant passes on the pre-fix tree — your closing comment's prediction, now measured rather than reasoned. The pinned tables fail there, but only because they were written after the repair; had they existed first they would have pinned /messages?limit and been green too. Pins catch the re-drop, not the original omission. Both halves are stated in the file header rather than implied.

One finding from driving it

Three routes reach GET /messages, not the two #428 names — storage.ts's instanceStorageRoutes GET /:id/messages proxies the whole query string. It was correct by accident of being a pass-through, and nothing said so until now.

The probe vocabulary is derived from the object, so a parameter the DO starts reading is probed from that moment without anyone widening a list.

What this guard cannot see — stated in the files, not left to be assumed

• It sees a parameter a handler reads, not one it honours. A handler reading before and ignoring it is indistinguishable here from one that seeks with it — precisely what the DO did before [bug] "Load older messages" re-returns the newest page — the before cursor is a UUID, the route drops it, and the DO never implemented it #428. Only an executed round trip separates those; lib/message-page.ts + e2e/console.spec.ts are where [bug] "Load older messages" re-returns the newest page — the before cursor is a UUID, the route drops it, and the DO never implemented it #428 put that, and this is deliberately not a second copy.
• Query parameters only. Body fields crossing route → DO (/chat, /state, /knowledge POSTs) and the DO callers in workflows/, lib/steps.ts, lib/triggers.ts, routes/drive.ts are untouched.
• The scan follows one hop. A parameter read by something the arm's handler calls is unattributed. Anything unreadable throws rather than returning [], so it degrades to a red build, not a quiet zero.

Acceptance criteria

• Removing a parameter from a route's DO request fails a test.
• A deliberately malformed D1 query fails a test rather than production (closed earlier by 0f274d2 + 77e17c1).
• Adding a styleGuidance branch without a length rule fails a test ([bug] Coding agents get NO length rule in the prompt — lengthRule is computed for all and emitted only inside plain-speech, and the Behaviour tab shows "balanced" for a field that is unset #430)
• Adding an external call with no deadline fails a test ([bug] A stalled transcription hangs on "Transcribing…" forever and locks the composer — no timeout, no retry, no dismiss, reload is the only escape #421/[bug] Chat times out at 25s on the tool loop's second round — a 4096-token cap under a 25s non-streaming ceiling is an impossible combination #427)
• Adding a mobile control row without responsive labels fails a test ([bug][mobile] Copy and Delete sit on top of the message timestamp — 42px of 110px covered in WebKit, and removing the year only recovers 30px #426/[bug][mobile] Terminal/Issues/Pulls/Actions tabs overflow — the icon-only pattern exists 80 lines above in the same file and was not applied #431)

The last three are separate mute-invariant.test.ts-shaped scans, independent of this work as your closing comment says. Not attempted. Leaving this open for them.

Verification

pnpm typecheck 11/11 · pnpm vitest run 7381 passed / 431 files · biome --error-on-warnings · all 7 check-* gates · pnpm docs:drift — all green, re-run after rebasing onto c77d9f3. pnpm test:e2e skipped and named: no store/, agents/coder/web or e2e-exercised file is touched — CI ran it anyway and it passed.