# Architecture Deepening — Design Spec

- **Slug:** `arch-deepening`
- **Date:** 2026-07-02
- **Executor:** `node ~/.claude/workflows/run-plan-codex.js --slug arch-deepening`
- **Source audit:** `docs/architecture-gap-audit-2026-07-02.md` (Architectural Deepening Gaps 15–25)
- **Scope decision:** full deepening 15–25, including the `Indicator` God-object breakup cluster.

## Purpose

Turn shallow, God-object-bound modules into deep modules with stable seams, so the tray/account subsystem is testable through interfaces (not private methods) and AI-navigable. Fix two correctness bugs surfaced by the same audit (fail-open routing, divergent cache freshness) as part of the relevant deepenings.

## Preconditions (HARD — must hold before `run-plan-codex` starts)

1. **origin/main ↔ local reconcile is DONE.** The audited code (830-line `indicator.py` working tree; 384-line `account_registry.py` carrying shared-state at ~185–319) lives on `save-local-main-before-remote-dashboard-20260701`, **not** on `main`. `origin/main` has diverged (29 ahead / 4 behind; a different 994-line `indicator.py` lineage). The refactor **cannot** run until these two lineages are merged into a single coherent branch.
2. **`meta.base_branch` points at the reconciled branch** that contains the merged, committed code. It is stamped with the **fail-closed sentinel `RECONCILE-SET-BASE-BRANCH`** — git cannot resolve it, so `run-plan-codex` setup halts loudly until you replace it with the real reconciled ref. Do **not** leave it as a resolvable branch like `main`: `main` carries the divergent 994-line lineage where the W4 target does not exist, so a forgotten edit would silently build the whole refactor on the wrong tree. `run-plan-codex` branches `plan/arch-deepening` from `base_branch` and never sees uncommitted work.
3. **Working tree is clean** on that base (the currently-uncommitted edits to `indicator.py`, `cdx.py`, `tests/test_cdx.py`, `tests/test_indicator.py` must be committed into the base first).
4. **Re-verify mapping after reconcile.** Tasks below are pinned to *behavioral contracts*, not line numbers — but if the merge changes a module's responsibilities materially (esp. `indicator.py`, whose two lineages differ), re-check the affected task before running.

## Verification model (why Wave 1 is first)

`run-plan-codex` gate0 uses `task.gate0_cmd || meta.gate0_cmd`, falling back to a per-file derivation from `files`. This plan **stamps `meta.gate0_cmd = "python3 -m pytest -q"`** (full suite) for every task, for two reasons: (a) gate0 runs in a raw `bash -c` subprocess outside the RTK-hooked tool — bare `pytest` on PATH is unverified there, while `python3 -m pytest` is the form confirmed to pass all 87 tests; (b) per-file gate0 misses cross-module regressions (a `account_registry.py` change breaking `test_cdx.py`), and there is no final full-suite gate before ship. Full suite costs ~0.45s and gives a cumulative regression net — each wave's worktree includes prior waves' committed tests. Therefore:

- Every task still lists its own `tests/test_*.py` in `files` (documents the test surface; also feeds any future per-file mode).
- Refactors that break private-method-coupled tests **must update those tests to target the new seam in the same task** — never weaken assertions to pass gate0.
- Each new module ships with its own test file — the new seam *is* the test surface.

**Provisional after reconcile:** wave boundaries and the seam decomposition are the durable value; the exact `files` lists will churn when the two `indicator.py` lineages merge (precondition #4). Treat `files` as provisional and re-verify per task post-reconcile.

## Methodology

Contract-first. Behavior-preserving extraction: each wave moves behavior behind a new seam and re-points callers + tests, with no user-visible behavior change except the two explicit bug fixes. New modules are non-GTK and unit-testable without the fake-GTK harness. `Indicator` shrinks to a GTK adapter over the extracted modules.

---

## Modules & seams

Each entry: file(s), responsibility, the seam callers must match, behavior delta, test surface. No implementation bodies — the implementer derives them from the repo.

### W1 — Verification foundation (audit #26/#27/#31/#32/#33)

**T1 · Move embedded tests + make suite trustworthy**
- Files: `pytest.ini`, `device_auth.py`, `device_auth_dialog.py`, `tests/test_device_auth.py` (new), `tests/test_device_auth_dialog.py`.
- Responsibility: production modules contain no `test_*` functions; all tests live under `tests/` and are collected by a plain `pytest`.
- Delta: relocate `test_*` from `device_auth.py` (~260+) and `device_auth_dialog.py` (~389+) into `tests/`; production modules keep only production code.
- Test surface: `pytest -q` collects the full suite; relocated tests still pass.

**T2 · Project vocabulary + ADR log**
- Files: `CONTEXT.md` (new), `docs/adr/0001-record-architecture-decisions.md` (new).
- Responsibility: anchor module/seam names to stable domain vocabulary; give rejected/accepted deepenings a home so reviews don't re-litigate.
- Contract: `CONTEXT.md` defines — account registry, tool account, selected default, routing rules, health snapshot, health snapshot store, shared Codex state, tray model, device-auth operation, command router, subprocess protocol adapter.
- Test surface: none (docs); disjoint from T1 → same wave, parallel.

### W2 — Health snapshot store + routing correctness

**T1 · Health snapshot store (audit #16 + freshness bug)**
- Files: `health_store.py` (new), `health_client.py`, `cdx.py`, `cld.py`, `indicator.py`, `tests/test_health_store.py` (new), `tests/test_health_client.py`, `tests/test_cdx.py`, `tests/test_indicator.py`.
- Responsibility: single owner of health-cache read/write/freshness/atomicity. Absorbs the three divergent freshness rules (none / 60-min stale / file-mtime) into **one** freshness policy exposed on the interface.
- Seam:
  ```
  HealthSnapshotStore(path: Path, stale_after_s: int)
    write(snapshot: AccountSnapshot) -> None      # atomic (tempfile+fsync+replace)
    read() -> AccountSnapshot | None              # parse + schema-tolerant
    read_fresh() -> AccountSnapshot | None         # None when older than stale_after_s
    age_seconds() -> float | None
  ```
- Delta: `health_client.write_cache`/`read_cache`, `cdx.load_health` (+ its `HEALTH_STALE_SECONDS` gate), and `indicator._load_health_cache`/`_write_health_cache` all become thin callers of the store. Callers get freshness from the store, never re-infer it. `cld` uses the store for `claude_health_cache.json` (path-parameterized).
- Test surface: `tests/test_health_store.py` covers atomicity, staleness boundary, schema tolerance — the new deep seam.

**T2 · Fail-open routing fix + registry-checked seed (audit #23 + #10)**
- Files: `routing_resolver.py`, `systray_codex_switcher.py`, `tests/test_routing_resolver.py`, `tests/test_systray_codex_switcher.py`.
- Responsibility: routing never silently resolves to an account absent from the registry.
- Delta:
  - `RoutingResolver._is_available` currently returns `True` when a slug has **no** health snapshot (`routing_resolver.py:67-68`) → a phantom slug is treated as healthy and exec'd against a credential-less home. Change so an **unknown-to-registry** slug is not selectable; missing *health* for a *known* account keeps its current "assume available" semantics (health cache may legitimately be empty). This requires the resolver to know the registry's account set — pass it in as an **optional** constructor param (`known_slugs: set[str] | None = None`; `None` = current "assume available" behavior, so existing callers keep compiling unchanged). Do not widen behavior silently.
  - `seed_routing_rules` (`systray_codex_switcher.py:55`) must validate seeded slugs against the registry: derive from registry, or emit an explicit setup-required state — never write rules naming `avi`/`rafa`/`roy` blindly.
- Seam delta:
  ```
  RoutingResolver(rules, health, known_slugs: set[str] | None = None)
    resolve(project) -> ResolvedRoute        # raises NoHealthyAccountError incl. "unknown-account" reason
  ```
- Test surface: add cases for unknown-slug rejection and seed-validation.
- **Disjointness:** the optional param keeps this task file-local to `routing_resolver.py` + `systray_codex_switcher.py` (+ their tests). The `cdx.py`/`cld.py` callers are **not** touched here — they keep the default (`None`) until **W5**, where `CommandRouter` rebuilds them and passes the real registry set. This makes T2 genuinely disjoint from W2/T1 (which owns `cdx.py`/`cld.py`) → same wave, parallel is safe.

### W3 — Tool-qualified account identity (audit #19 + #8)

**T1**
- Files: `account_registry.py`, `indicator.py`, `cdx.py`, `cld.py`, `tests/test_account_registry.py`, `tests/test_indicator.py`.
- Responsibility: an account is identified by (tool, slug), and its home directory is named neutrally — Claude accounts stop masquerading through the `codex_home` field.
- Delta: introduce a tool-qualified account reference and rename the home field to a neutral `account_home` (keep `codex_home` as a compat alias only if a caller cannot be updated in-wave). Tray keys (`_menu_items`, snapshot lookup) become tool-qualified so a shared slug across Codex/Claude no longer collides (audit #8).
- Seam:
  ```
  AccountRef(tool: ToolKind, slug: str)         # hashable identity key
  Account(ref: AccountRef, alias, account_home: Path, email, plan, account_id)
  ```
- Test surface: registry lists both tools without collision; tray menu keys are tool-qualified.

### W4 — Extract SharedCodexState (audit #18)

**T1**
- Files: `shared_codex_state.py` (new), `account_registry.py`, `tests/test_shared_codex_state.py` (new), `tests/test_account_registry.py`.
- Responsibility: the Codex-specific merge/symlink engine (currently `account_registry.py:185-319`) moves out of the generic registry.
- Seam:
  ```
  SharedCodexState(legacy_codex_home: Path)
    sync_links(account_home: Path) -> None
    sync_all(account_homes: list[Path]) -> None
    # raises on conflicting Codex state
  ```
- Delta: `AccountRegistry` calls `SharedCodexState` for CODEX accounts; registry no longer contains merge/link/jsonl helpers. Credential vs sqlite-state classification moves with it.
- Test surface: `tests/test_shared_codex_state.py` covers merge/append/jsonl/symlink + conflict; registry tests shrink to metadata only.
- **Depends on reconcile** — target code exists only on the local lineage (precondition #1).

### W5 — CommandRouter seam (audit #17)

**T1**
- Files: `command_router.py` (new), `cdx.py`, `cld.py`, `tests/test_command_router.py` (new), `tests/test_cdx.py`, `tests/test_cld.py`.
- Responsibility: one router owns the resolve→exec flow; per-tool differences live in adapters. Kills the `str(exc).replace("cdx:", "cld:")` leak.
- Seam:
  ```
  CommandRouter(adapter: ToolAdapter)
    run(argv) -> NoReturn                     # parse override -> detect project -> load rules
                                              # -> load health (via store) -> resolve -> exec
  ToolAdapter (protocol): name, home_env_var, health_store_path, build_home(slug), exec(argv, home)
  CodexAdapter / ClaudeAdapter
  ```
- Delta: `cdx.main`/`cld.main` become thin `CommandRouter(CodexAdapter()).run` / `ClaudeAdapter`. Error text is tool-neutral from the router; adapters supply the tool name. Codex-only extras (multi-dir candidates, `selected_default`, shared-link sync) live in `CodexAdapter`.
- Test surface: router tested once against a fake adapter; adapter tests cover tool-specific bits.

### W6 — Subprocess protocol adapters (audit #22)

**T1**
- Files: `codex_appserver.py` (new), `device_auth_protocol.py` (new), `health_client.py`, `device_auth.py`, `tests/test_codex_appserver.py` (new), `tests/test_device_auth.py`.
- Responsibility: the two hand-rolled subprocess protocols (Codex app-server JSON-RPC in `health_client.py`; device-auth stream parsing in `device_auth.py`) become explicit modules with typed request/response contracts. **Justification is locality/encapsulation, not adapter-swapping** (one consumer each) — hide JSON-RPC line constants, regexes, and non-blocking read logic behind a small interface; do not over-build a pluggable framework.
- Seam:
  ```
  CodexAppServer.fetch_rate_limits(codex_home: Path) -> RateLimits
  DeviceAuthProtocol.parse_chunk(text: str) -> DeviceAuthEvent | None   # url | code | done
  ```
- Test surface: protocol parsing tested on captured fixtures without spawning real subprocesses.

### W7–W10 — Indicator God-object breakup (audit #15/#20/#24/#25)

Strictly sequential — all touch `indicator.py`. Each wave extracts one concern and re-points `indicator.py` + its tests at the new seam.

**W7/T1 · Scheduler seam (audit #25)**
- Files: `scheduler.py` (new), `indicator.py`, `tests/test_scheduler.py` (new), `tests/test_indicator.py`.
- Responsibility: concurrency policy (daemon thread spawn, GLib idle dispatch, refresh-generation/pending-refresh bookkeeping) leaves `Indicator`.
- Seam:
  ```
  Scheduler(idle_add=..., thread_runner=...)
    run_background(fn) -> None
    on_main(fn) -> None
    next_generation() -> int ; is_current(gen) -> bool
  ```
- Delta: the already-injectable `_idle_add`/`_thread_runner` hooks plus `_refresh_generation`/`_pending_refreshes` state move into `Scheduler`; `Indicator` holds a `Scheduler` and no longer tangles generation state into menu rendering.
- Test surface: `Scheduler` tested deterministically with fake idle/thread; refresh-generation logic testable without GTK.

**W8/T1 · Tray view-model (audit #24)**
- Files: `tray_model.py` (new), `indicator.py`, `tests/test_tray_model.py` (new), `tests/test_indicator.py`.
- Responsibility: derive the tray's title + menu-item state as a pure data model from (accounts, snapshots, selection); GTK only renders it.
- Seam:
  ```
  build_tray_model(accounts, snapshots, selected: AccountRef, popup_open: bool) -> TrayModel
  TrayModel: title: str ; items: list[TrayItem(ref, label, checked, status)]
  ```
- Delta: `_update_title`/`_account_label`/`_rebuild_menu`/`_update_account_menu_item` consume `TrayModel`; label/title formatting moves into the pure builder.
- Test surface: `tests/test_tray_model.py` asserts title/label/checked/status from inputs — no fake GTK.

**W9/T1 · Device-auth operation module (audit #20)**
- Files: `device_auth_operation.py` (new), `indicator.py`, `device_auth.py`, `tests/test_device_auth_operation.py` (new), `tests/test_indicator.py`.
- Responsibility: the repair/add orchestration (process start, prompt read, commit/rollback, collision handling, snapshots) becomes a non-GTK operation emitting flow events; the tray only shows dialogs and renders events.
- Seam:
  ```
  DeviceAuthOperation(flow, registry, shared_state)
    repair(ref) -> Iterator[FlowEvent]        # PROMPT | SUCCESS | FAILURE(reason)
    add(slug) -> Iterator[FlowEvent]          # PROMPT | COLLISION | SUCCESS | FAILURE
  FlowEvent variants carry prompt text / error / new AccountRef
  ```
- Delta: `Indicator._run_repair_flow`/`_run_add_account_flow` shrink to: drive the operation, show a dialog on `PROMPT`, react to terminal events via the `Scheduler`. Commit/rollback/collision logic leaves the tray.
- Test surface: the most failure-prone behavior tested through `FlowEvent`s, no fake GTK and no private tray methods.

**W10/T1 · Indicator becomes an adapter (audit #15)**
- Files: `indicator.py`, `tests/test_indicator.py`.
- Responsibility: `Indicator` is a thin GTK adapter wiring `Scheduler` + `TrayModel` + `DeviceAuthOperation` + `HealthSnapshotStore`; it holds GTK widget lifecycle only.
- Delta: remove now-migrated logic; `tests/test_indicator.py` shrinks to adapter wiring, its private-method-heavy assertions replaced by the extracted modules' interface tests.
- Test surface: full `pytest -q` green; `test_indicator.py` materially smaller and public-surface oriented.

---

## Architecture Decisions

- **#22 (protocol adapters) kept despite single consumer each** — accepted on *locality* grounds (hide subprocess noise), explicitly **not** as a pluggable-adapter seam. Do not generalize beyond the two current protocols. (Single-adapter test: would normally collapse; kept for encapsulation only.)
- **#25 scheduler** — the injectable `_idle_add`/`_thread_runner` hooks already exist; the deepening is real only because refresh-generation state is tangled into rendering. The extraction targets that tangle, not the hooks alone.
- **Deletion test — all extracted modules pass:** deleting `HealthSnapshotStore`, `SharedCodexState`, `CommandRouter`, `DeviceAuthOperation`, `TrayModel`, or `Scheduler` scatters real (freshness/merge/routing/rollback/formatting/concurrency) complexity back across multiple callers.
- **Two-adapter seams are real:** `CommandRouter` (Codex+Claude) and tool-qualified identity both have two concrete tools today.
- **Reconcile-first** (chosen over base-on-current) — recorded so a future review does not re-suggest running the refactor against the divergent tree.

## Risks

- **Highest risk: W7–W10 on `indicator.py`** with a private-method-coupled `test_indicator.py` (1909 lines). Each wave must move assertions to the new seam, not weaken them. Codex fix-loops that "make gate0 green" by deleting/relaxing tests are the failure mode — the review leg must reject that.
- **Reconcile may shift structure** — W4 target (shared state) exists only on the local lineage; confirm it survives the merge before running W4.
- **Sequential depth** — 10 waves, ~13 tasks, heavy `indicator.py`/`cdx.py`/`cld.py` overlap force near-sequential execution; wall-clock is long.