feat(codex): a write substrate that survives being interrupted

devlog/_plan/260803_codex_desktop_toggle/000_plan.md

@@ -1,81 +1,130 @@
-# Codex and Claude Desktop toggles
+# Client integration switches: Codex, Claude Desktop, and the memory they need
 
-Split out of `260803_integrations_toggle_all` after its fourth audit
-(`../260803_integrations_toggle_all/007_audit_synthesis_r4.md`). That unit keeps
-Claude Code and Grok, whose toggles need no durable operation state. These two
-need one, and it is the shared dependency four audit rounds kept circling.
+Split out of `260803_integrations_toggle_all` after its fourth audit. That unit
+shipped Claude Code and Grok. This one was scoped around a durable
+operation-state engine — and a research cycle against the real code says that
+engine solves the wrong problem.
 
-## Why these two are together and separate
+> **Re-planned 260803 after four research passes.** The rollback engine
+> (`010_operation_state.md`, never written) is **dropped**. Evidence:
+> `001`-`004`. The replacement is smaller, and it also fixes a defect in the
+> toggle we shipped hours ago.
 
-| | Codex | Claude Desktop |
-|---|---|---|
-| Artifacts | `config.toml`, `opencodex.config.toml`, model catalog, resume history (SQLite) | `<id>.json`, `.bak`, `_meta.json`, four fields in our config |
-| Owner | us, mostly | **Claude Desktop** — we edit another app's registry |
-| Prior state recoverable? | from its own journal, when hashes still match | only from `_meta.json` bytes; `appliedId` was never recorded |
-| Removal code exists? | yes, `restoreNativeCodex` | **no** |
+## The requirement in the owner's words
 
-Neither can be undone by re-running its enable path, which is what makes Claude
-Code and Grok cheap. Codex's enable is not the inverse of its disable — the
-journal fallback means a disable may strip fragments rather than restore bytes,
-and a later enable writes today's catalog, not yesterday's arrangement. Desktop
-cannot re-derive which profile the user had selected at all.
+> "스위치를 꺼도 프록시는 살아있어야 돼. 코덱스 말고 다른 것만 켜고 싶을 수도 있잖아."
 
-So both need an operation record that outlives the request, and that record is
-the first phase.
+Turning a client off must leave the proxy running and serving every other
+client. That is two obligations: the mutation must not stop the proxy, and the
+OFF must survive a restart.
+
+## What the research changed
+
+| Prior claim | What the code says |
+|---|---|
+| Codex needs a durable operation-state engine | `ocx restore` already restores native Codex **without stopping the proxy** (`src/cli/help.ts:18`), and `ocx restore back` is the enable direction. Both exist. (`001`) |
+| Desktop removal is impossible to do safely | Anthropic documents that a selected config without a valid `inferenceProvider` launches **standard mode**. We aim at that instead of guessing. (`002`) |
+| The missing piece is crash recovery | The missing piece is **desired state**. Only Claude Code has one; Grok's shipped toggle is silently re-enabled by the next `ocx start`. (`003`) |
+
+The fourth research doc (`004`) is an unrelated live defect found while looking:
+one out-of-enum modality value makes a client reject its entire config. It joins
+this unit as an independent work-phase (LOOP-UNIT-CHAIN-01).
 
 ## Read first
 
-From the parent unit, all still authoritative:
+In this unit, all four written this cycle:
+
+- `001_native_restore_thesis.md` — Codex restore, its asymmetries, the history lock
+- `002_desktop_standard_mode.md` — the official standard-mode contract, and why `default` is not the restore verb
+- `003_durable_desired_state.md` — desired vs observed, the Grok regression, the schema
+- `004_export_modality_poisoning.md` — the gjc/Pi enum defect
 
-- `001_removal_path_inventory.md` — what each client's disable actually costs
-- `003`, `004`, `005`, `007` — the four audit syntheses; `007` is why this unit
-  exists
-- `002_consequence_dialog_ux.md` — dialog direction; the Codex and Desktop copy
-  lives there
+From the parent unit, still authoritative: `001_removal_path_inventory.md`
+(what each disable costs) and `002_consequence_dialog_ux.md` (dialog direction).
+Its `007_audit_synthesis_r4.md` is the reason this unit exists, but its central
+conclusion is now **superseded** by `001`-`003` here.
 
 ## Phases
 
-| Phase | Doc | Deliverable |
-|---|---|---|
-| WP1 | `010_operation_state.md` | The durable operation record: a versioned discriminated journal entry, prepare/commit, restart reconciliation, a field-scoped config writer |
-| WP2 | `020_codex_toggle.md` | Codex disable/enable on top of it |
-| WP3 | `030_desktop_toggle.md` | Desktop removal + rollback, `appliedProfileId` schema work |
-
-WP2 and WP3 are **parallel siblings**, not a sequence: Desktop does not depend on
-Codex (audit r4 #11). Both depend on WP1 and nothing else.
-
-## What WP1 must deliver
-
-Named by audit round 4, findings #1, #2, #5, #3:
-
-1. **A versioned discriminated journal entry.** Today's `JournalEntry`
-   (`src/integrations/journal.ts:34-51`) is file-shaped: one `configPath`, one
-   `SnapshotRef`, one `resultFingerprint`. A routing description and three
-   library members do not fit it. Needs `file-v1 | native-state-v1 |
-   desktop-hybrid-v1` with validated serialized state, per-member fingerprints,
-   and retention behavior.
-2. **Prepare/commit with restart reconciliation.** A crash between the mutation
-   and the append leaves no undo state at all. Idempotent re-apply does not fix
-   this — it helps once a state exists, and none was recorded. The prepared
-   record must be durable before the mutation and resolved after.
-3. **A field-scoped config writer.** `saveConfigPreservingClaudeCode` persists
-   the whole live object and its own docstring says a `providers` hand edit is
-   clobbered (`src/config.ts:2132-2135`); worse, when disk and caller both
-   changed `claudeCode`, the caller's stale subtree wins, so one toggle field can
-   clobber a concurrent Desktop-profile edit. Desktop's four fields need a write
-   that reloads from disk and touches only named paths.
-
-## Carried-forward findings
-
-Everything in `005` §Carried forward plus round 4's, in particular: Codex's
-pre-state must capture the effective injection mode and history policy, not just
-a routing kind (r4 #4); `injectCodexConfig` takes a concrete `catalogPath`, so
-"selector" needs a resolver (r4 #4); Desktop's ordering is dangling-pointer-safe
-but not transactionally crash-safe and must say so (r4 #6); auto-apply must be
-suppressed while an operation is prepared (r4 #6).
-
-## Status
-
-Not started. `020` and `030` carry their pre-split content and are **stale**
-against the operation-state design WP1 has not written yet; their next P
-re-verifies them against it before any build.
+**Re-sliced after audit round 2** (`006_audit_synthesis_r2.md`). The first map
+sliced along a schema — one ten-key `clientIntegrations` map — which forced every
+phase to touch every client's write path. Two audit rounds widened rather than
+narrowed. This map slices along **ownership boundaries**: one client family per
+phase, each independently auditable.
+
+| Phase | Doc | Deliverable | Audit state |
+|---|---|---|---|
+| WP2 | `010_modality_boundary.md` | The client-dialect modality filter | **clean through two rounds** |
+| WP3 | `020_api_keys_row.md` | API keys out of the card grid into their own row | never blocking |
+| WP4 | `030_desired_state.md` → re-scoped | Desired state for **Codex only**, plus its gates, single-flight, ownership preflight and CLI semantics | re-scope pending |
+| WP5 | `040_codex_toggle.md` | The Codex switch on WP4's flag | rewrite pending |
+| WP6 | *(new doc)* | Grok's desired state, reusing the shape WP4 proved | not started |
+| WP7 | `050_desktop_toggle.md` | The Desktop switch via documented standard mode | deferred behind WP5 |
+
+The two pending docs were renamed so the decade order matches the phase order:
+reading the unit lexicographically now gives the build order, which is the whole
+point of the numbering convention.
+
+WP1 was the research cycle plus this roadmap, twice audited.
+
+WP2 and WP3 depend on nothing here and on each other not at all — they ship
+first, alone, because each changes one thing at one boundary. That property is
+the only thing that survived both audit rounds intact.
+
+Three deliberate exclusions, each an accepted audit finding rather than a
+convenience:
+
+- **Claude Code's ingress gates are not touched at all.** Round 1 #1 established
+  that `claudeCode.enabled` is the documented kill switch for `/v1/messages`;
+  the honest conclusion is not to route it through a new helper in this unit.
+- **The six file clients get no desired-state flag here.** Round 2 #4 (existing
+  explicit OFF choices are not migrated) and #5 (a mutating GET) both belong to a
+  phase that does not exist yet, and inventing it under audit pressure is what
+  produced round 2's new findings.
+- **Desktop is behind Codex, not beside it.** Round 2 #3 found no coherent rule
+  when a foreign profile is selected. The goal explicitly permits an evidenced
+  deferral; this is one.
+
+## Scope boundary
+
+IN: `src/clients/config-export.ts`, `src/types.ts`, `src/config.ts`,
+`src/codex/sync.ts`, `src/grok/sync.ts`, `src/cli/index.ts`,
+`src/cli/opencode.ts`, `src/claude/desktop-3p.ts`,
+`src/server/management/native-integration-routes.ts`,
+`src/server/management/agent-settings-routes.ts`, `src/server/management-api.ts`,
+`gui/src/pages/integrations/*`, `gui/src/styles-integrations.css`,
+`gui/src/i18n/*`, `tests/`, `gui/tests/`.
+
+OUT: releases, publishing, deploys, tags; starring the repository; rewriting the
+six-client file machinery in `src/integrations/`; `docs-site` restructuring;
+recording the previous `appliedId` (deferred, `002` §Residual).
+
+## Criteria
+
+- C1 — gjc loads our emitted config with no schema error, proven from the real
+  file; Pi's identical exposure is closed in the same change.
+- C2 — Codex stays disabled across a proxy restart, an `ocx ensure`, and a
+  `POST /api/sync`.
+- C3 — an upgrading user with no `clientIntegrations` key sees no behavior change.
+- C4 — disabling Codex never stops the proxy and never closes `/v1/responses`.
+- C5 — Codex toggles both directions from the overview with the proxy running.
+- C6 — a Codex disable blocked by the held history DB is an explained refusal
+  naming the cause, never a raw 500 and never a false green.
+- C7 — startup convergence never touches state owned by a service running from a
+  different `OPENCODEX_HOME` (round 2 #2, blocking).
+- C8 — API keys render as a row above the grid, observed rendered.
+- C9 — typecheck, full test, gui lint, gui test, privacy scan all green.
+- C10 — Desktop ships with a proven restore path OR is deferred with recorded
+  evidence. Both outcomes close this criterion.
+
+## Risk register
+
+| Risk | Mitigation |
+|---|---|
+| A gate silently unplugs a working client on upgrade | Absent key means ON, everywhere, with a test for the absent-config case (`003`) |
+| Gating a safety path | Explicit do-not-gate list: journal repair, ownership checks, owned teardown, shared transports (`003`) |
+| Desktop pointed at a missing file | Never delete `appliedId`, never leave it dangling, never pick an entry by the name `Default` — this machine's `Default` is already dangling (`002`) |
+| The modality fix erases valid internal metadata | Filter at the client-dialect boundary only; management and CLI keep carrying `audio` verbatim (`004`) |
+| A green suite hides the real failure | 91 tests pass today beside a config gjc refuses to load. Every criterion names a live artifact, not a unit test |
+| A repair introduces a worse defect than the one it fixes | Round 2 produced exactly this. One client family per phase, re-audited before the next starts — never a cross-client rewrite under audit pressure |
+| Convergence tears down a foreign home's state | Every native remover runs `assertNativeTeardownOwned` before removal, and reconciliation runs only after service-ownership resolution (round 2 #2) |

devlog/_plan/260803_codex_desktop_toggle/001_native_restore_thesis.md

@@ -0,0 +1,144 @@
+# The native-restore thesis, tested against the code
+
+Research doc. No diffs here — the implementation designs live in the decade
+docs. This one records what the code actually does, because the previous
+unit's plan was built on a claim the code does not support.
+
+## The claim under test
+
+`000_plan.md` asserts that Codex and Claude Desktop both need a durable
+operation-state engine — a versioned discriminated journal entry, prepare/commit
+with restart reconciliation, and a field-scoped config writer — before either
+can get a toggle. The owner's counter-thesis: the proxy keeps RUNNING while a
+client is switched back to its native path, so each client only needs to be
+returned to a path that already exists, not replayed from a recorded snapshot.
+
+For Codex the counter-thesis is correct, and the evidence is not subtle.
+
+## Codex: the toggle pair already ships as a CLI
+
+`ocx restore` is documented in `src/cli/help.ts:18-20` as:
+
+> Restore native Codex config without stopping the proxy; `restore back`
+> re-points codex at the running proxy.
+
+That is the toggle, both directions, with the proxy up. `src/cli/index.ts:770`
+calls `restoreNativeCodex()` with no lifecycle operation anywhere near it, and
+`src/cli/index.ts:756` implements the enable direction as `syncModelsToCodex(live.port)`
+against a proxy it first proves is live via `findLiveProxy()` (`:751`).
+
+Stronger still: `POST /api/stop` (`src/server/management-api.ts:181`) calls
+`restoreNativeCodex()` FIRST and only then schedules the drain and exit. The
+restore therefore already executes while the listener is serving. Whether the
+proxy later stops is irrelevant to the restore itself.
+
+The service-stop path does verify the listener is gone before restoring
+(`src/service.ts:2571`), but that check enforces the requested outcome "service
+stopped" so success is not claimed while a supervisor respawns the process. It
+is not a precondition inside `restoreNativeCodex()`.
+
+**Conclusion:** no durable operation-state engine is required to make Codex
+switchable. The disable path exists, the enable path exists, and both are
+proxy-agnostic.
+
+## What restore actually touches, and where it is not symmetric
+
+Four state groups, when no external `model_provider` owns the config:
+
+| Group | Restore behavior | Evidence |
+|---|---|---|
+| `config.toml` + `opencodex.config.toml` | Byte-exact from the journal when the injected hash still matches; otherwise strip owned fragments | `src/codex/journal.ts:109`, `src/codex/inject.ts:770` |
+| Injection journal | Deleted on a complete restore, retained on a partial one | `src/codex/journal.ts:133` |
+| Model catalog | Pristine backup + post-sync native additions, or drop slash-qualified routed rows keeping native ones | `src/codex/catalog/sync.ts:572-590` |
+| Resume history | May update `state_5.sqlite`, patch/append rollout JSONL, consume the backup manifest | `src/codex/history-provider.ts:413,656,691` |
+
+When an external provider such as `custom` owns `model_provider`, restore removes
+only the stale journal and deliberately leaves everything else alone
+(`src/codex/inject.ts:765`). That is a pre-existing courtesy to a user who moved
+off us by hand, and the toggle must preserve it rather than "fixing" it.
+
+Three asymmetries matter for the toggle's honesty:
+
+1. **Enable is `syncModelsToCodex()`, not `injectCodexConfig()`.** Injection
+   selects and writes a catalog path but does not build the routed rows;
+   `src/codex/sync.ts:83-110` refreshes the catalog and then injects, and that
+   is what `ocx restore back` uses. A toggle wired to bare injection would
+   re-point Codex at a catalog that no longer lists the routed models.
+2. **A post-injection root model selection is destroyed, not restored.** If the
+   user edited config after we injected, the hash no longer matches, the
+   fallback strip runs, and a root `model = "provider/slug"` line is removed
+   (`src/codex/inject.ts:315,700`). Re-enabling has no record of that selection.
+   The dialog copy must not promise to put it back.
+3. **Resume history is reversible but not byte-identical.** Restore patches line
+   one when safe and appends a `session_meta`
+   (`src/codex/history-provider.ts:81,444`); re-enabling appends another
+   provider change rather than deleting that history.
+
+## The history lock, and why the current return shape is not enough
+
+The write path mirrors Codex's five-second SQLite busy timeout and retries twice
+with a 500 ms delay (`src/codex/history-provider.ts:25,526`). A Codex app or IDE
+holding the WAL writer lock is exactly what makes it fail.
+
+At the low-level boundary the failure is structured:
+
+```ts
+return withHistoryRetry(...) ?? { rows: 0, files: 0, failed: true };
+```
+
+Recoverable busy/lock/permission failures return `failed: true`; corruption and
+programming errors throw (`src/codex/history-provider.ts:511,536,577`).
+
+But `restoreNativeCodex()` discards that structure. `history.failed` only edits
+the message string, and `success` stays `cfg.success`
+(`src/codex/inject.ts:787,794`). A config restore that succeeded while the
+history stayed locked returns roughly:
+
+```
+{ success: true, message: "... history could NOT be restored ..." }
+```
+
+So a GUI that trusts `success` reports a clean disable while routed threads stay
+tagged `opencodex` and remain invisible in the native app. And `failed: true`
+itself conflates lock contention with `EPERM`/`EACCES`.
+
+**Design consequence for `020`:** the toggle route must not consume
+`restoreNativeCodex()`'s boolean. It needs the structured per-artifact outcome —
+config, catalog, history — with the history failure carrying a classified reason,
+so the card can render "disabled, but your routed threads are still hidden
+because the Codex app is holding the history database" instead of a green check
+or a raw 500. Parsing the message string is not acceptable.
+
+## The real hazard is not the socket
+
+The sibling explorer's central finding, and the one that reshapes this unit:
+**there is no persisted per-client desired state.** After a disable, the next
+proxy startup, `ocx ensure`, `ocx sync`, a provider/model mutation, or
+`POST /api/sync` can silently re-inject Codex (`src/cli/index.ts:318,365`,
+`src/server/management/config-routes.ts:261`).
+
+That is the actual engineering work this unit needs — and it is a much smaller
+thing than the operation-state engine `000_plan.md` specified. A durable
+desired-state flag per client, defaulting ON so no existing setup dies on
+upgrade, consulted by every automatic apply path. It is a switch's memory, not a
+rollback engine.
+
+`030` and the durable-state design pick this up; `010` is retired as specified
+because the crash-recovery machinery it described is not what the evidence asks
+for.
+
+## Observable consequences of a Codex disable
+
+What the consequence dialog must actually name:
+
+- plain `codex` returns to its native provider path (`src/codex/inject.ts:688`)
+- routed `provider/model` catalog rows disappear; native rows survive
+  (`src/codex/catalog/sync.ts:578,590`)
+- the generated `opencodex` profile is removed or replaced, and managed native
+  subagent defaults are restored or stripped (`src/codex/inject.ts:703`)
+- previously routed threads are retagged to their native provider — or stay
+  hidden if the history DB was locked (`src/codex/history-provider.ts:677`)
+- the proxy keeps serving Claude, Grok, the exported file clients, and direct API
+  callers, because restore contains no lifecycle operation at all
+
+That last line is the whole point of the feature.