Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions devlog/_plan/260917_l2_safe_teardown/000_master_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# 000 — Safe teardown and honest settings application

- Unit: `260917_l2_safe_teardown`
- Opened 2026-09-17
- Base: `origin/dev` = `f1dfda8e48`
- Issues: #4812 (parent), #4809 (child)
- Class C4 — writes `$CODEX_HOME/config.toml`, decides teardown outcome, and
touches the conversation-history safety boundary.

## Objective

Close the question "can a user turn OpenCodex off and get their original
environment back?" Today they cannot, in two different ways.

#4812 is a deadlock. The Codex history preflight refuses the whole config
restore whenever the Codex state store has a `history_mode` column, which
every current Codex build has. `ocx restore`, `ocx stop`, and `ocx uninstall`
all funnel through that refusal, so the proxy can be removed while
`~/.codex/config.toml` still routes at `127.0.0.1:10100`. The guard exists to
keep `opencodex`-tagged threads resolvable, but once the proxy is gone those
threads fail at request time anyway — and now every native Codex invocation
fails too. The protection protects nothing and costs everything.

#4809 is a configuration lie. `--desktop-authless` and `--client-compaction`
persist to `config.json` and report success, but the injected
`~/.codex/config.toml` does not change until a separate `ocx sync`. On a
non-loopback bind the authless flag is silently dropped altogether while the
API still reads back `true`. Nothing tells the user that the flag also moves
the auth source — whether the Codex app presents `~/.codex/auth.json`.

Both are the same underlying defect: **the stored value, the effective value,
and the work still owed are collapsed into one answer.** This unit separates
them.

## Delivery shape

Two stacked pull requests, in order:

| PR | Issue | Branch | Base |
|---|---|---|---|
| parent | #4812 | `codex/restore-routing-without-history` | `dev` |
| child | #4809 | `codex/settings-apply-and-effective-state` | the parent's head branch |

`enforce-target` admits a stacked child whose base is an open parent's head
branch. The child is retargeted to `dev` after the parent squash-merges; the
host directs that step, not this lane.

## Constraints

- **No local verification of any kind.** `bun test` (in any form),
`bun run test`, `bun run test:changed`, `bun run typecheck`, `bun x tsc`,
`bun install`, `bun run build:gui`, and running `ocx` are all forbidden for
this unit. A local suite has previously deleted a real `~/.opencodex`.
Verification is static reasoning plus hosted CI at the exact head.
- Pushes use `git push --no-verify`; the pre-push hook runs the local suite.
- This lane never merges, never pushes to `dev`, and never rebases unasked.
The lane ends at "PR open with exact-head CI evidence".
- No flake management. No widened timeouts, added retries, platform skips, or
masking. The Windows job is dispatch-only; a Windows-affecting change is
reported to the host rather than dispatched here.
- **Paginated rollout bytes and thread rows stay untouched.** The native
writer remains the only writer of that shape. Nothing in this unit relaxes
`history_paginated_requires_native_writer` as a guard on *history*.
- Repository artifacts — commits, PR bodies, issues, reviews, these docs — are
English. Security analysis that is not already public goes to `.tmp/`, never
here.

## Work-phase map

| wp | Doc | Output |
|---|---|---|
| wp0 | this file | objective, topology, completion criteria |
| wp1 | `010_upstream_resolution_facts.md` | what codex-rs actually does with a provider id, and what that forces |
| wp2 | `020_issue_4812_contract.md` | the degraded-restore contract and its seam |
| wp3 | `030_issue_4809_contract.md` | stored / effective / pending separation for the two switches |
| wp4 | `040_verification.md` | static-proof obligations and hosted-CI evidence plan |

## Completion criteria

Shared across both pull requests:

1. Every surface that reports one of these settings distinguishes three
things: the **stored** value, the **effective** value actually in force,
and whether **further action** is required to reconcile them.
2. Repeating `restore`, `stop`, and `uninstall` in any order never damages
user-owned configuration and never mutates Codex conversation history.
3. A partial outcome is never reported as full success, and no path ends with
a failed restore that leaves the client pointed at a dead address.

Parent (#4812):

- `history_paginated_requires_native_writer` no longer refuses the config
half of a restore. It selects a **degraded restore**: OpenCodex-owned root
routing comes out, `[model_providers.opencodex]` stays, history is skipped
rather than attempted.
- Every other preflight reason keeps its hard refusal and its compensating
rollback, unchanged.
- `ocx restore --remove-codex-provider-table` performs the full removal for a
user who accepts that `opencodex`-tagged threads stop opening. It is never
the default, and it states the consequence before acting.
- `ocx uninstall` on a paginated home completes with native Codex working,
names the retained table and the exact lines, and exits 0. It no longer
records the config restore as a failure that blocks local-state cleanup.
- `ocx status` reports retained-table residue instead of leaving it invisible.

Child (#4809):

- Flipping either switch through the settings API or the CLI applies the
injected `config.toml` inline when the proxy is live and the integration is
enabled; the response says whether it applied and, if not, exactly why.
- The response reports the **effective** `codexDesktopAuthless`, not only the
configured one, with the reason when the two differ.
- Both surfaces state the auth-source consequence — whether the Codex app will
present `~/.codex/auth.json` — at the moment of the change.
- The stale comment at `src/server/management/config-routes.ts:600-601`, which
asserts the opposite of what the code does, is corrected.

## Terminal outcomes

- **DONE** — both PRs open, exact-head CI recorded, criteria above hold.
- **BLOCKED** — recorded here with the blocking evidence; the lane does not
work around a gate by weakening it.

## Prior art in this repository

`devlog/_plan/260914_codex_history_preflight_scope/` narrowed the same guard
on the apply direction and explicitly left this open:

> "The uninstall deadlock on an already-paginated home remains open follow-up;
> a later fix needs a keep-the-table seam on the restore path."

That is what `020` specifies.
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# 010 — What codex-rs actually does with a provider id

The parent design rests on one upstream claim: keeping
`[model_providers.opencodex]` on disk while removing the root routing keys
leaves native Codex working *and* leaves `opencodex`-tagged threads loadable.
That claim is checkable, and checking it also rules out the obvious
alternative orderings. Evidence below is from the Codex upstream corpus at
`/Users/jun/Developer/codex/_raw/repos/121_openai-codex/codex-rs`.

## Provider resolution is a whole-config concern, not a per-request one

`Config::load` builds the provider map and then resolves exactly one id:

```rust
let model_providers =
merge_configured_model_providers(built_in_model_providers(openai_base_url), cfg.model_providers)
.map_err(...)?;

let model_provider_id = model_provider
.or(cfg.model_provider)
.unwrap_or_else(|| "openai".to_string());
let model_provider = model_providers
.get(&model_provider_id)
.ok_or_else(|| {
...
format!("Model provider \`{model_provider_id}\` not found")
std::io::Error::new(std::io::ErrorKind::NotFound, message)
})?
.clone();
```

`core/src/config/mod.rs:3732-3749`

Three consequences follow directly, and they decide the whole contract.

**A missing provider id is fatal to config load, not to one request.** The
`?` propagates a `NotFound` out of `Config::load`. So removing the provider
table while root `model_provider = "opencodex"` survives does not degrade
anything — it breaks every single `codex` invocation with
`Model provider \`opencodex\` not found`, which is strictly worse than the
connection error #4812 reports. **The two removals can never be split in that
direction.** The degraded write must therefore be a single atomic
transformation, never a strip followed by a re-add.

**Root `openai_base_url` rewrites the built-in provider.**
`built_in_model_providers(openai_base_url)` constructs the `openai` provider
from that value (`model-provider-info/src/lib.rs:512-527`), so an injected
`openai_base_url` pointing at a dead proxy breaks native Codex even when no
OpenCodex provider table exists at all. Removing it is not optional; it is the
single most load-bearing part of the degraded restore.

**The default is `openai` when no root selector is present.** Dropping root
`model_provider` is sufficient to return the home to native operation. No
positive rewrite is needed.

## A resumed thread supplies its own provider override

```rust
typesafe_overrides.model_provider = Some(persisted_metadata.model_provider.clone());
```

`app-server/src/request_processors/thread_processor.rs:234`

That override is the `model_provider` argument in the resolution above, so a
thread row tagged `opencodex` needs a map entry named exactly `opencodex`.
With the table retained the resume succeeds and only that thread's requests
fail, against a dead port, with an ordinary connection error. With the table
removed the resume fails at config load.

This matches what `src/codex/inject.ts:490-495` already asserts on the apply
side — "Rows this home may have tagged `opencodex` resolve only through a
provider table" — and it is why the injector re-appends an existing table
before building its write witness (`src/codex/inject.ts:496-502`). The restore
direction is getting the same seam, for the same reason.

## `requires_openai_auth` is the auth source, visibly

```rust
fn should_show_login_screen(login_status: LoginStatus, requires_openai_auth: bool) -> bool {
...
if !requires_openai_auth {
```

`tui/src/lib.rs:2070-2073`, reached from `tui/src/lib.rs:1214-1233`

The flag OpenCodex emits at `src/codex/inject/config-toml.ts:95` decides
whether Codex asks the user to sign in and whether it presents
`~/.codex/auth.json`. That is a user-visible identity change, which is why
#4809's requirement that the switch announce its auth-source consequence is a
correctness requirement rather than a cosmetic one.

## Bounds of this evidence

The corpus is a vendored snapshot, not the running binary on any particular
user's machine. What it establishes is the *shape* of resolution — override
beats root key beats `openai`, and a miss is fatal at load. The degraded
contract in `020` depends only on that shape, and it is conservative in the
one direction that matters: it never produces a config where a referenced
provider id is absent.
161 changes: 161 additions & 0 deletions devlog/_plan/260917_l2_safe_teardown/020_issue_4812_contract.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
# 020 — Degraded restore: separate routing recovery from history

Issue #4812. Parent PR, branch `codex/restore-routing-without-history`.

## The defect, stated as a decision error

`preflightCodexHistoryInjection` answers one question — "may I rewrite Codex
conversation history?" — and four call sites use that answer to decide a
different question: "may I take OpenCodex routing out of `config.toml`?"

```ts
const historyError = preflightCodexHistoryInjection(false, false);
if (historyError) return { state: "failed", ... };
```

`src/codex/inject/restore.ts:260-261`, `:275-276`, `:371-372`, `:519-520`, and
`src/codex/inject/remove.ts:145-146`

Because `assertLegacyHistoryStore` refuses on the mere *presence* of a
`history_mode` column (`src/codex/history-provider.ts:397-400`), and every
current Codex build has that column, the answer is permanently no. The config
half is therefore permanently unreachable, and `ocx uninstall` removes the
proxy while leaving the routing that points at it.

The fix is not to weaken the guard. The guard is right about history. It is
being asked the wrong question.

## Contract

### Two classes of owned state

**Routing state** — makes *every* `codex` invocation go through the proxy:
marker-owned root `openai_base_url` and `experimental_realtime_ws_base_url`,
root `model_provider = "opencodex"`, a routed root `model`, an OpenCodex
`model_catalog_json`, `[profiles.opencodex]` and the generated profile file,
and the managed subagent defaults.

**Thread-resolution state** — `[model_providers.opencodex]` and its
sub-tables. It affects nothing unless a thread row names that provider id.

Routing state is what strands the user. Thread-resolution state is what the
history guard is protecting. They are separable, and `010` establishes that
separating them in this direction is safe while the opposite direction is
catastrophic.

### The rule

When `preflightCodexHistoryInjection` returns
`history_paginated_requires_native_writer` **and nothing else**, restore takes
the degraded path:

- All routing state is removed, through the existing journal-or-strip logic,
unchanged.
- `[model_providers.opencodex]` is retained verbatim, including its ownership
marker.
- The history relabel is **skipped, not attempted**. No rollout byte, thread
row, or manifest entry is touched. The native writer stays the only writer.
- The outcome is reported as degraded, never as a plain success.

Every other refusal reason — `history_injection_preflight_unavailable`,
`history_state_database_missing`, and the rollout-integrity codes — keeps the
existing hard refusal and its compensating rollback, byte for byte. This is
the exact asymmetry the apply direction already encodes as
`HISTORY_RELABEL_STANDS_DOWN` (`src/codex/inject.ts:472-484`); restore is
being brought into line with it, not given something new.

### Atomicity

`010` shows that a config containing root `model_provider = "opencodex"`
without a matching table fails `Config::load` outright. The degraded write is
therefore **one** `atomicWriteFile` of fully-computed content. The
implementation must not strip and then re-add as two writes, and must not
leave that combination reachable through an error path.

The seam is a verbatim capture, taken before the transform and re-appended
into the same output buffer:

- `extractOcxProviderTableBlock(content): string | null` — new pure function
in `src/codex/inject/remove.ts`, the exact inverse of the existing
`removeOcxSection` scan (`:50-80`), sharing `isOcxProviderHeaderLine` so the
two cannot drift on what counts as our table.
- `removeCodexConfig({ retainProviderTable })` re-appends the captured block
after the strip, before the single write.
- `restoreCodexConfigInlineImpl` captures the block from the on-disk config
**before** the journal restore, because an exact journal restore replays the
original pre-injection bytes and deletes the journal
(`src/codex/journal.ts:258-293`). After a successful journal restore the
captured block is re-appended inside the same lock and the same preimage
window.

Verbatim capture, rather than rebuilding the table from the live routing
target, is deliberate. Rebuilding needs a port and a config that `uninstall`
is in the middle of removing, and it would silently change the retained
definition. Capture cannot.

### Reported outcome

`CodexRestoreArtifactState` gains `"partial"`. `CodexRestoreConfigResult.action`
gains `"routing-restored-provider-retained"`. The envelope gains:

```ts
retainedCodexProviderTable?: {
reason: "history_paginated_requires_native_writer";
/** Exact config.toml lines left on disk. */
lines: string[];
/** What to run to remove them, and what breaks if you do. */
followUp: string;
};
```

`success` stays `true`: the routing restore genuinely succeeded and the
dead-address trap is gone. The residue is a deliberate, named outcome rather
than a hidden failure, which is what completion criterion 1 asks for —
stored, effective, and still-owed are three separate fields, not one boolean.
A degraded restore that *fails* is still a failure and keeps today's handling.

`historyPreflightRefusal` keeps its current meaning — "nothing was attempted
at all" — and must therefore **not** be set on the degraded path, because
`src/cli/index.ts:813-818` reads it together with three `skipped` artifacts to
decide that a stop obligation is still owed. A degraded restore discharged
the config obligation, so the receipt must be released, not preserved.

### Caller obligations

Report B found that seven of eight callers reduce the result to `.success`.
They keep working unchanged, which is the point of keeping `success: true`.
Three need real changes:

- `ocx restore` (`src/cli/dispatch.ts:194-240`) prints the retained lines and
the follow-up command; `--json` carries the new field.
- `ocx stop` (`src/cli/index.ts:796-825`) must classify degraded as neither
`historyDeferred` nor `other`. The obligation was performed; exit stays `0`
and the receipt is discharged.
- `ocx uninstall` (`src/cli/index.ts:1357-1360`) no longer records a failed
step, so `failures` stays empty and `~/.opencodex` is removed
(`:1393-1410`). It prints the retained lines. This is the concrete end of
the trap: uninstall completes, native Codex works, and the user is told
exactly what is left and why.

### Full removal, on request

`ocx restore --remove-codex-provider-table` strips the table too. It states
before acting that `opencodex`-tagged threads will stop opening, and it is
never implied, never defaulted, and never selected by `stop` or `uninstall`.

### `ocx status`

A config with `[model_providers.opencodex]` but no OpenCodex root routing is
retained residue, and status says so, with the removal command. Report B
confirms status has no such line today
(`src/codex/inject/routing-classify.ts:55-107` classifies endpoint ownership
only), so residue is currently invisible.

## What this does not do

- It does not make `opencodex`-tagged threads work after teardown. They point
at a proxy that is gone. They open, and their requests fail with an ordinary
connection error instead of a config-load error.
- It does not touch conversation history on a paginated home, ever.
- It does not add a `--force` that bypasses the history guard. There is no
such flag, because there is no safe version of it.
Loading
Loading