# Module deepenings — five internal refactors + web data path

audience: AI coding agents first.

- Status: Draft
- Source: architecture-review-20260813-102418.html candidates C8, C19, C9, C17, C13, C16, C10, C11, C15
- Scope: lib/fleet/, modules/gptbridge/, modules/systray/, controller/src/, collector/src/actions.ts, apps/web/src/lib/ + apps/web/src/components/incidents/
- Delivery scope: ends after reviewed design + implementation plan documents are written. MUST NOT launch, MUST NOT implement (owner instruction 2026-08-13: "do not continue to execution yet")
- Related: 2026-08-13-arch-shared-contracts-design.md (the web data path section consumes it); 2026-08-13-arch-domain-glossary-design.md
- Last updated: 2026-08-13

## 1. Outcome

Five self-contained deepenings inside five modules, plus one web data-path cleanup that depends on the shared-contracts spec. Each deepening is independently implementable and testable; none changes behavior visible to users.

1. fleet: one digest module, one canonicalize, transport polymorphism replaces `transport.name === "ssh"` branches, delegated provisioning errors propagate.
2. gptbridge: one conversation model with two roles (runtime registry, durable log); no `cls.__new__` seat construction.
3. systray: command router split into router + per-command handler modules with a registry table; indicator split into view + service modules.
4. controller: store split into cohesive modules; `TransitionVerb` moves to transitions.ts (locality inversion fixed).
5. collector: actions gateway becomes a handler registry table.
6. web: incident data path has exactly one fetch adapter and pure mappers, all typed by the contract.

## 2. Decisions

- D1: Each deepening keeps its module's existing conventions: bun for TS (collector, controller), pytest for Python (gptbridge, systray), bun test in modules/fleet/test/ for fleet (engine.test.mjs, loader.test.mjs, expand.test.mjs, harden.test.mjs, transport-local.test.mjs, transport-ssh.test.mjs + *.sh harness tests).
- D2: No deepening may change user-visible behavior: responses, messages, exit codes, CLI output, file paths. Deletion tests prove it.
- D3: The web data path section (3.6) MUST NOT be implemented before the shared-contracts package exists (2026-08-13-arch-shared-contracts-design.md D1-D3), and the controller section (3.4) depends on shared-contracts D6 (the offload-verb import that `TransitionVerb` extends); all other sections are independent and may land in any order or in parallel.
- D4: Doc structure (owner decision 2026-08-13): grouped as module-deepenings; the two other specs are shared-contracts and domain-glossary.
- D5: Nothing in this spec touches modules/harness/** or packaging/**. The mandatory harness pytest suite is out of scope.

## 3. Architecture

### 3.1 fleet — one digest, one canonicalize, transport capabilities (C8, C19)

Current state (origin/main):

- `sha256Hex` exists in 5 files: lib/fleet/engine.mjs:59, expand.mjs, harden.mjs, transport-local.mjs, transport-ssh.mjs.
- `canonicalize` exists in 2 files: engine.mjs:31 and loader.mjs:92.
- `auditDelegated` (engine.mjs:243) and `convergeDelegated` (engine.mjs:477) branch on `transport.name === "ssh"` and call `auditHostSeat`/`provisionHost` synchronously (spawnSync-based) inside async paths — the ssh capability is embedded in the engine instead of the transport. The ssh branch is 7 lines in auditDelegated (engine.mjs:249-255) and ~14 in convergeDelegated (:499-513).
- engine.mjs: 34 functions, ~753 lines: canonicalize :31, computeItemsRevision :45, sha256Hex :59, isConnectionFailure :63, isUnreachableError :69, expandTilde :77, resolveRepoRoot :83, resolveDeployRoot :87, resolveSourceAbs :91, canonicalMode :103, expectedMode :121, expectedLinkTarget :136, expectedUnitState :147, formatUnitState :151, splitItems :158, buildSeatCfg :171, auditFleetItem :185, auditDelegated :243, readSourceBytes :264, execCommand :276, runTransportCommand :290, applySymlink :314, applyGitCheckout :350, applyFileMode :397, applyUnit :423, convergeFleetItem :454, convergeDelegated :477, convergeNode :591, formatAuditReport :514, printAuditReport :550, auditNodes :667, convergeNodes :700, aggregateExitCode :740.

Decisions:

- New module lib/fleet/digest.mjs: `sha256Hex`, `canonicalize`, `readSourceBytes` (readSourceBytes currently lives in engine.mjs:264). All five sha256Hex sites and both canonicalize sites import it; engine.mjs, expand.mjs, harden.mjs, transport-local.mjs, transport-ssh.mjs MUST NOT declare their own.
- transport-ssh.mjs gains capabilities `delegatedAudit(nodeName, items, options)` and `delegatedConverge(nodeName, items, options)`; transport-local.mjs gains no-op capabilities of the same shape. The engine MUST NOT test `transport.name` — capability dispatch only (`transport.delegatedAudit ? ... : localLoop(...)`), and the no-op shape is mandatory so no transport can be identity-checked.
- `buildSeatCfg` (engine.mjs:171) moves to transport-ssh.mjs — it is the only consumer; the engine passes the node's items and options through, the transport builds its own seat cfg.
- The ssh branch's sync provisioning MUST become awaited with failure propagation: a failed remote audit fails the node's audit, it MUST NOT resolve `ok: true` on exception.
- Behavior: audit report format (formatAuditReport/printAuditReport, engine.mjs:514/550) unchanged.

### 3.2 gptbridge — one conversation model, explicit seat construction (C9, C17)

Current state (origin/main):

- Three conversation representations: registry.py:25 `Conversation` (in-memory routing registry), translate.py:26 `Conversation` + :74 `Registry` (translator's parallel model), convlog.py:143 `ConversationLog` (durable log with `turns()`).
- solwebd.py:223 `SeatPool.wrapping(cls, seat: BrowserSeat)` constructs seats via the `cls.__new__` alternative-constructor trick (bypassing `__init__`); cap detection is already single-source via `chat.detect_cap` (solwebd.py:177, :187, :493).

Decisions:

- registry.py owns the canonical in-memory `Conversation` shape; translate.py consumes registry entries via its adapter (its own Conversation/Registry classes collapse to adapters or vanish); convlog.py persists what the registry produces — one model, two roles, three call sites.
- `SeatPool.wrapping` is replaced by an explicit factory: `SeatPool.borrow(seat: BrowserSeat)` (or plain constructor — implementer picks, spec pins the outcome: no `cls.__new__` construction anywhere in solwebd.py).
- Cap detection stays at `chat.detect_cap`; no new detection site.

### 3.3 systray — router split and indicator split (C13, C16)

Current state (origin/main):

- command_router.py: 1410 lines; `credential()` appears at :649, :717, :758; the router mixes argument mapping, permission checks, and handler bodies.
- indicator.py: 2065 lines, ~120 methods; `remote_dispatch.py` and `account_registry.py` already exist as the service-layer precedent.

Decisions:

- New modules under modules/systray/commands/ — one file per command (credential, account, dispatch, ...), each exporting `(argsSchema, handler)`; command_router.py keeps routing + argument mapping only; a `COMMANDS` registry table maps command name → handler module (same table shape as the collector actions gateway in 3.5).
- indicator.py keeps view construction + event loop; state and service logic moves to modules/systray/services/ (extending the remote_dispatch/account_registry precedent). Menu-building and dialog code stays in the indicator.
- Both splits MUST keep CLI entry points and command names identical.

### 3.4 controller — store split, verb locality fix (C10)

Current state (origin/main):

- controller/src/store.ts: 2724 lines, ~119 methods; `TransitionVerb` union (8+1 verbs) is DEFINED at store.ts:75-84 but transitions.ts:11 IMPORTS it from store — the verb lives in the wrong module. transitions.ts:28 also defines its own `TRANSITION_VERBS` const (8 verbs) with `VerbSchema = z.enum(TRANSITION_VERBS)` at :39 — a second verb list. `TransitionVerb` is consumed at store.ts:232/:2314/:2335 and transitions.ts:124/:274/:642; events.ts:4 imports it too.
- FallbackLease interface at store.ts:86; admission-loop, k3s-watcher, scheduler already exist as sibling modules (with tests: store.test.ts, transitions.test.ts, admission-loop.test.ts, scheduler.test.ts, k3s-watcher.test.ts).

Decisions:

- Extract cohesive modules from store.ts: leases (FallbackLease + lease logic), admission reconcile glue, cluster snapshot, delivery-feature reconcile. Each extraction is a move, not a rewrite: same state, same errors, same tests passing.
- `TransitionVerb` moves to transitions.ts — which then owns ONE verb source: `TRANSITION_VERBS` const (the 8 offload verbs, imported from `@overdeck/incident-contract` per shared-contracts D6) + `TransitionVerb = OffloadActionVerb | "delivery-feature-reconcile"` + `VerbSchema = z.enum(TRANSITION_VERBS)`. store.ts (:232/:2314/:2335) and events.ts:4 update their import to transitions.ts. Transitions.ts MUST NOT import anything from store.ts — no store↔transitions cycle; the moved modules carry their own types.
- store.ts keeps the store class, state, and orchestration; it MUST NOT grow in this delivery.

### 3.5 collector — actions gateway table (C11)

Current state (origin/main):

- collector/src/actions.ts: 1549 lines; `ALLOWED_ACTION_VERBS` (30 verbs) at :19-50; `OFFLOAD_ACTION_VERBS` at :53 (moves to the contract per shared-contracts D6); ~21 zod-schema handlers each following the same parse → authorize → journal → dispatch skeleton; `ActionGateway.handle` at :1475, preceded by the `isAllowedVerb` gate at :1476 and the unknown-verb `default` at :1543.

Decisions:

- New `ACTION_HANDLERS` registry table: `Partial<Record<AllowedActionVerb, { argsSchema: z.ZodType; handle(args, ctx): Promise<Response>; journal?(args, ctx): void }>>` with an explicit fallback — a verb absent from the table (or rejected by the `isAllowedVerb` gate) returns the same unknown-verb response as today. The dispatch loop in `handle` is the single parser/autorizer/journaler; journaling MUST NOT run for verbs handled by their own `journal` override twice.
- Handlers keep their zod schemas verbatim (e.g. ReapArgsSchema, CiRerunArgsSchema, TrainActionArgsSchema at actions.ts:105-137); the table is a move, not a rewrite.
- `IncidentsProviderLike` slice (actions.ts:99-101) stays — deliberate locality.
- Response bodies, statuses, and journal entries MUST be byte-identical (collector/src/actions.test.ts is the proof).

### 3.6 web — one fetch adapter, pure mappers (C15)

Current state (origin/main):

- apps/web/src/lib/: collector-client.ts (the primary fetch adapter), collector-queries.ts (react-query hooks), collector-types.ts (separate protocol mirror — NOT absorbed, per shared-contracts §7), page-mappers.ts, overview-mappers.ts, panel-data.ts, collector-state.ts, panel-freshness.ts.
- Two components bypass the adapter with raw `fetch(`: components/plans/AttemptDetailDrawer.tsx:97 (`void fetch(href)` artifact download) and components/factory/FactoryTraceTables.tsx:75 (`await fetch(artifactHref(...))`). All other `fetch(`-matches in components are react-query `refetch(` calls.
- components/incidents/: IncidentsContent.tsx, IncidentDetailDrawer.tsx, FileIncidentForm.tsx, IncidentsApp.tsx, incident-view.ts. Tests: collector-client.test.ts, collector-queries.test.ts, overview-mappers.test.ts, panel-freshness.test.ts.

Decisions (depends on shared-contracts D1-D3):

- After the contract lands: collector-client.ts (or a named new function in it, e.g. an artifact-download fetch) is the ONLY place that calls fetch against the collector; components and mappers consume via collector-queries hooks or pure mapper functions typed by the contract.
- The two raw-fetch sites (AttemptDetailDrawer.tsx:97, FactoryTraceTables.tsx:75) move into collector-client.ts + a collector-queries hook; the components call the hook.
- mappers stay pure (no network, no hooks); panel-freshness stays as-is.
- The deletion test below is the acceptance proof.

## 4. Behavior

- Section 3.1: audit output (formatAuditReport/printAuditReport) identical; failure behavior strictly stronger (delegated failures propagate).
- Section 3.2: conversation routing and persistence identical; seat borrowing identical from the caller's view.
- Section 3.3: every systray command name, argument, and permission behavior identical.
- Section 3.4: store behavior identical; only module boundaries move.
- Section 3.5: every action verb's request handling, response, and journal identical.
- Section 3.6: every page renders identically (honest data rule: no fabricated values, per od-ui-dev).

## 5. Error handling

- 3.1: remote seat audit failure MUST fail the node's audit with the remote error surfaced; no silent `ok: true`.
- 3.2: seat borrow on full pool raises the same "no free seats" error the `cls.__new__` path raised.
- 3.3-3.6: error classes, exit codes, and response codes unchanged; each section's existing tests prove it.

## 6. Testing

- 3.1: `bun test modules/fleet/test/` — engine.test.mjs, transport-local.test.mjs, transport-ssh.test.mjs, expand.test.mjs, harden.test.mjs, loader.test.mjs, plus the *.sh harness tests. The delegation path is currently UNTESTED (engine.test.mjs:575's delegated case passes a local transport), so this delivery MUST add a test: a mocked ssh transport (or injected deps.auditHostSeat that throws) asserting the node audit FAILS with the remote error propagated — the old sync branch's silent-success behavior must be impossible. Deletion tests: `grep -rc "function sha256Hex" lib/fleet/` = 1 and `grep -c "transport.name" lib/fleet/engine.mjs` = 0.
- 3.2: `python3 -m pytest modules/gptbridge/tests/ -q`.
- 3.3: `python3 -m pytest modules/systray/tests/ -q` (run_gui_tests.sh / run_remote_tests.sh are the interactive tiers — not required by this delivery).
- 3.4: `bun test` in controller/ + `tsc --noEmit` — store.test.ts, transitions.test.ts stay green.
- 3.5: `bun test` in collector/ + `tsc --noEmit` — actions.test.ts stays green.
- 3.6: `pnpm --filter web build`, `pnpm --filter web typecheck` — collector-client.test.ts, collector-queries.test.ts, overview-mappers.test.ts, panel-freshness.test.ts stay green; deletion test: `grep -rn "fetch(" apps/web/src/components/ | grep -vE "collector-client|useQuery|useMutation|useInfiniteQuery|refetch"` = empty.
- Modules/harness/** untouched: the mandatory factory pytest suite MUST NOT be affected; if an implementer touches it anyway, run `python3 -m pytest modules/harness/factory/tests/ -q` and keep it green.

## 7. Out of scope

- The shared-contracts package itself (its own spec).
- The five already-landed candidates (C1, C2, C6, C14, C20) — document, never re-specify.
- Any UI restyling, new components, or new primitives (od-ui-dev governs any edit under apps/web/src/** and packages/deck-ui/src/**; no new primitive without owner approval).
- Factory/harness/packaging behavior.
- gptbridge model or prompt changes; systray appearance changes.

## 8. Architecture Decisions

- ADR-0004: fleet transports own their delegation capabilities (3.1) — engine orchestrates, transports execute.
- ADR-0005: one conversation model, two roles (3.2).
- ADR-0006: handler registry tables over per-handler skeletons (3.3 commands, 3.5 actions).
- ADR-0007: store splits by cohesion, verb union lives with its consumer (3.4).
- Not an ADR: 3.6 is a consequence of shared-contracts D1-D3 plus the existing collector-client seam.