[standards][tracking] Split worst large files into testable modules

[serge-ivo]

Problem

VCQA reports standards D largely because several production files are still very large. This is valid PAGS technical debt, but it is not a single ready-to-pick implementation story unless it is treated as a tracking issue and split into smaller refactors.

Current line counts verified on 2026-08-07:

• packages/sdk/src/voice/use-voice.ts — 1532 lines
• workers/api/src/routes/coding.ts — 1738 lines
• workers/api/src/routes/instances.ts — 1695 lines
• workers/mcp/src/instance-tools/base.ts — 1778 lines
• store/console/src/pages/InstanceDetail.tsx — 1230 lines
• store/console/src/tabs/SettingsTab.tsx — 1148 lines
• agents/coder/web/src/CodingTab.tsx — 1216 lines

Related narrower tickets already exist:

• The #138 refactor was undone within hours — add a size ratchet so decomposition holds #302: add a size ratchet so large-file refactors do not regress
• De-duplicate routes/coding.ts — one clone-URL parser, one owner helper, one build-run lookup #304: de-duplicate workers/api/src/routes/coding.ts

Fix direction

• Treat this as a tracking issue unless a dev explicitly picks one file/module boundary.
• Split by behavior boundary, not arbitrary line count.
• Add or preserve tests before moving behavior.
• Prefer small leaf modules for parsing, state derivation, route handlers, controller actions, and UI subpanels.
• Avoid broad refactors that mix route behavior, UI styling, and public API changes in one PR.

Ready child work candidates

• Extract voice audio/session/state-machine helpers from packages/sdk/src/voice/use-voice.ts.
• Split workers/api/src/routes/instances.ts into route registration plus small handler/service modules.
• Split workers/mcp/src/instance-tools/base.ts into tool groups with shared helpers.
• Extract focused subpanels/hooks from store/console/src/pages/InstanceDetail.tsx.
• Extract settings form sections from store/console/src/tabs/SettingsTab.tsx.
• Extract Coder tab terminal/repo/session subpanels from agents/coder/web/src/CodingTab.tsx.

Acceptance

• Each child PR names the boundary it is extracting and keeps behavior unchanged unless separately tested.
• Typecheck and relevant tests pass for each child PR.
• VCQA large-file/context findings improve over several focused PRs.
• A ratchet or CI check prevents the same files from growing back past the agreed threshold.

[serge-ivo]

Critical reassessment: valid PAGS technical-debt issue. This was split out of #299 because line-count/module-cohesion work should not block or blur the connector cycle fix. Treat this as refactoring work with behavior-preserving tests, not a scanner-only cleanup.

[serge-ivo]

workers/mcp/src/instance-tools/base.ts — done (00ea379). Issue stays open: six of the seven files are untouched.

1871 lines, 67 of the 86 instance tools. Split along the registration boundaries it already had, following the precedent stats.ts set at #312 (a new file rather than four more entries in base.ts):

module	tools	what it is
base.ts	7	lifecycle: the connector-tool gate, subscribe/cancel, chat
runtime.ts	7	the pags up runtime + its task queue
knowledge.ts	10	documents, files, vectors, memory
observability.ts	6	messages, activity, errors, trace, pipeline runs
board.ts	8	the board, its columns, the per-ticket thread (#150)
settings.ts	11	settings, name, instructions, model, translation, state
triggers.ts	5	webhook / cron / connector-sync
composition.ts	8	supervision (#183), connections (#182), loops
account.ts	5	billing, usage, keys, email, profile

Largest is 298 lines, so base.ts leaves the #302 ratchet rather than being pinned lower.

What is now assertable that was not

Moving lines between files asserts nothing, so the split is only half of it. The other half is instance-tools/contract.test.ts, which holds all 86 instance tools to a table of group / scope / confirmation string / dry-run behaviour / input fields — and every value in it is derived by driving the registered handler, not declared:

• scope — call the tool holding only read, then holding everything but read, and read the required scope out of whichever refusal comes back. A tool with no gate denies neither and records as none.
• confirm — call it with full scopes and no confirm, capture the string the refusal demands.
• dryRun — call it with dry_run: true and assert the network was never touched.

Arguments are generated from each tool's own zod shape, so a new tool cannot forget to bring a probe: it appears in the derived table immediately and fails until someone writes down what it is allowed to do. This is the failure mode the surface is actually prone to — a tool that keeps working while quietly losing its gate — and nothing checked it before.

Three things the table found, recorded rather than fixed, because this commit moves code:

• delete_supervision takes destructive scope but asks for no confirmation and offers no dry run, alone among the destructive tools.
• Seven mutating tools declare no dry_run at all. The list is now pinned, so it moves only when someone means it to.
• workers/mcp/CLAUDE.md documented the order requirePermission → requireConfirmation → dry_run. Every tool actually does permission → dry-run → confirmation, and asking what a destructive call would do should not require the confirmation token. Doc corrected; the test now asserts the real order.

Deliberately left whole

The per-tool request bodies and query strings. Extracting set_translation_config's patch builder or list_errors' querystring into leaf modules would move lines without adding an assertion — the harness in instance-tools.test.ts already drives them through the handler, which is the honest path. Nothing became testable, so nothing moved.

Proof behaviour is unchanged

• The contract table was generated against the pre-split base.ts and is byte-identical against the nine post-split modules — 86 tools × scope, confirmation, dry-run, sorted input fields.
• All 67 registration blocks were moved by script and compare byte-for-byte against HEAD:base.ts.
• pnpm docs:drift still reports 130 registered == 130 documented == the constant, and now verifies 16 per-file counts instead of 8.
• security-invariants.test.ts walks workers/mcp/src recursively rather than by filename, so the new modules are in its scan; Turn the security invariants into guard tests that fail when a new route, tool, or connector forgets them #306's connector guard keys on workers/api paths and is untouched.

CI status — stated plainly

No CI run has verified this. GitHub Actions is in a declared major outage. Verified locally only: pnpm typecheck, vitest --project unit (4269 tests, 276 files), npx biome check (3 warnings, all pre-existing in files I did not touch), pnpm docs:drift, node scripts/check-file-size.mjs, check-test-isolation, check-agents-allowlist, check-qa-config.

Remaining on this ticket: workers/api/src/routes/coding.ts, workers/api/src/routes/instances.ts, packages/sdk/src/voice/use-voice.ts, store/console/src/pages/InstanceDetail.tsx, store/console/src/tabs/SettingsTab.tsx, agents/coder/web/src/CodingTab.tsx.

[serge-ivo]

workers/api/src/routes/instances.ts — 1696 → 853 (820d239, CI green)

Third child slice, following base.ts (00ea379). Same shape: half the change is a
contract test, and every value in it is derived by driving the handler.

The seams are the route groups the registrations already had, following the five
sibling instances-*.ts modules this file already delegated to:

module	lines	what it is
instances-tasks.ts	608	the board, the tickets, the runner task list — all seventeen routes share one substrate nothing else on this surface touches: instance_runtime_tasks, the durable D1 mirror
instances-chat.ts	201	the AgentDO message log — turn, loop orchestrator, system-message, history
instances-knowledge.ts	133	the subscriber's own KB — six routes with no business logic, which is the point

All three are under the 800-line LIMIT, so no ratchet entry; instances.ts's pin is
lowered to 853 in the same commit.

What is assertable now that was not. routes/instances.contract.test.ts holds all 75
routes to a table of module / anonymous status / non-owner status. It probes each route
twice — no bearer, then a valid session for a user who owns nothing against a D1 that
resolves no ownership row for anybody — and records what comes back. 71 of the 75 answer
404. A route that loses its WHERE user_id = ? stops refusing, which moves it in the
table. A separate probe asserts no route writes for a stranger, which the status alone
cannot see. Module attribution is derived by mounting each register*Routes on a bare Hono
and reading .routes, so a route drifting between modules fails too.

How the route set was proved unchanged. The table was generated against the pre-split
file and is byte-identical after — including registration ORDER, which is why the three new
register*Routes calls sit at the exact positions their blocks occupied rather than being
tidied in with the other five (Hono matches in order). Separately, each moved block was
verified byte-for-byte against the region it came out of, modulo one level of indentation
and instanceRoutes. → router.. pnpm docs:drift still counts 286/286.

Two things the table found, recorded rather than fixed — that commit moves code:

1. PUT /:instanceId/name has no tenant gate. No requireOwnedInstance, and
   readInstanceConfig answers {} for a row that is not yours rather than throwing — so a
   caller who owns nothing gets 200 {"name":"…"}. It is not a cross-tenant write:
   patchInstanceConfig/removeInstanceConfigKey both bind user_id, so the UPDATE matches
   zero rows (now asserted). But it reports success for work it did not do, and the console
   shows the new name until it reloads. The vestige was visible all along — it computes
   readInstanceConfig into a variable biome has been reporting as unused. Pinned in the
   table; worth its own ticket.
2. POST /:instanceId/chat validates before it checks ownership, so {} from a stranger
   is a 400 rather than a 404. Harmless, but it means a probe has to send a plausible body or
   it asserts the wrong thing; the bodies are listed with that reason.

Left whole, deliberately. The runtime-registration and node-binding routes are contiguous
and would extract cleanly, but their helpers already live in instances-runtime.ts (849,
pinned) — a third file would spread one concept over three, and nothing becomes assertable
that the contract test does not already assert. Same for the voice block, which /trace sits
inside: extracting it would reorder a route to buy no assertion.

---

Still open on this ticket — the other five files are untouched by this slice:

• packages/sdk/src/voice/use-voice.ts (1550)
• workers/api/src/routes/coding.ts (1774) — see also De-duplicate routes/coding.ts — one clone-URL parser, one owner helper, one build-run lookup #304
• store/console/src/pages/InstanceDetail.tsx (1252)
• store/console/src/tabs/SettingsTab.tsx (1155)
• agents/coder/web/src/CodingTab.tsx (1225)
• workers/mcp/src/instance-tools/base.ts — 00ea379
• workers/api/src/routes/instances.ts — 820d239