diff --git a/devlog/_plan/261111_child_disconnect_followups/000_plan.md b/devlog/_plan/261111_child_disconnect_followups/000_plan.md new file mode 100644 index 00000000000..8f4a547747d --- /dev/null +++ b/devlog/_plan/261111_child_disconnect_followups/000_plan.md @@ -0,0 +1,14 @@ +# Child disconnect follow-ups (O7, O8, O9, O10) + +Stacked on #8529 (`fix/child-restart-desktop-owned`). Field reports: six Remote Link Children moved to new SSH aliases on 2026-11-11, OpenCodex 2.100.2. + +| Unit | Doc | Kind | Outcome wanted | +|---|---|---|---| +| O8 | 010 | defect, test-first | a refused disconnect never leaves state that stops the runtime from starting; a retry finishes or rolls back from `prepared`, including a receipt already stuck on 2.100.2 | +| O9 | 020 | defect, test-first | disconnect succeeds when Codex edited config.toml after injection and no OpenCodex line remains; never overwrites Codex or user lines; keeps the journal while an OpenCodex line remains | +| O7 | 030 | message, test-first | each `desktop_lifecycle_unsafe` refusal names the path or condition and the fix, code at the end | +| O10 | 040 | research + proposal only | what a safe "restart Codex clients" is and where it belongs | + +Order: O8 and O9 share `disconnectClient` and land together (O9 makes the O8 retry succeed); O7 next; O10 is a written proposal in the PR body. + +Verification floor: each new test fails on the base branch first; `bun run typecheck`, `bun run structure:check`, `bun run privacy:scan`, the layout guards, and the files that import the changed modules. diff --git a/devlog/_plan/261111_child_disconnect_followups/010_o8_prepared_receipt_rollback.md b/devlog/_plan/261111_child_disconnect_followups/010_o8_prepared_receipt_rollback.md new file mode 100644 index 00000000000..4e757a1ccca --- /dev/null +++ b/devlog/_plan/261111_child_disconnect_followups/010_o8_prepared_receipt_rollback.md @@ -0,0 +1,18 @@ +# O8 — a refused disconnect rolls back its `prepared` receipt + +## Cause + +`disconnectClient` (src/client/connect.ts) writes `desktop-remote/disconnect.json` with phase `prepared`, then runs `restoreConnectedCodex` outside the lifecycle lock. A throw there leaves the receipt. Every non-`complete` receipt makes `observeClientRotationRecovery` (src/client/state.ts) answer `client_disconnect_pending`, and client startup refuses (src/cli/index.ts). A retry repeats the same Codex restore and fails the same way. + +## Decision + +`desktop_restored` is the commit point. Before it, the receipt records intent only: the Claude Desktop store, the catalog, the service token and the connection are untouched. So a failure before `desktop_restored` aborts: the receipt is removed (compare-and-delete against the receipt this run read), and the `desktop-remote` folder is removed too when the receipt was the only thing in it. At or after `desktop_restored` a retry must finish, as today. + +What a partial Codex journal restore leaves behind: the journal (recovery evidence), config.toml as it was, possibly the generated profile already reverted. The connection stays `connected`; the client runtime's next sync re-injects routing and regenerates the profile. + +The startup refusal names its fix: run `ocx disconnect` again (same `--keep-catalog` choice) to finish or roll back. + +## Tests (fail on base first) + +- refused disconnect: receipt gone, folder gone, connection connected, rotation gate `clean` (startup would pass), journal kept. +- upgrade path: a `prepared` receipt written as 2.100.2 left it; `ocx disconnect` on the new build either completes (Codex side now restorable) or rolls back (still refused), never needs a folder moved. diff --git a/devlog/_plan/261111_child_disconnect_followups/020_o9_codex_edited_config.md b/devlog/_plan/261111_child_disconnect_followups/020_o9_codex_edited_config.md new file mode 100644 index 00000000000..20ba35a3eea --- /dev/null +++ b/devlog/_plan/261111_child_disconnect_followups/020_o9_codex_edited_config.md @@ -0,0 +1,21 @@ +# O9 — disconnect after Codex edited config.toml + +## Cause + +`compareJournalState` (src/codex/journal.ts) accepts config.toml only when it equals the original snapshot or the first injected bytes (hash). Codex appends `[projects."…"] trust_level` blocks and notice flags; the restore is then partial forever. + +## Rule + +Inside one held Codex config write lock, after `restoreJournalState` reports incomplete: + +- proceed only if the journal is verified, the profile is restored, there is no lock or ownership refusal, and config.toml is what changed; +- and the owned-field strip that `ocx restore` uses (`stripOpencodexConfigResult` with the journaled values) would change nothing — no marked or journaled base URL / realtime URL, no `opencodex` provider or table, no managed web_search or subagent defaults, no profile section. + +Then config.toml is left byte-for-byte and the journal is removed: its only remaining use, a whole-file replay of the pre-injection snapshot, would delete the lines Codex wrote, so it is never safe to apply again. If any OpenCodex line remains, the journal is kept and the refusal names the file and the remaining OpenCodex settings, quoting only an allowlist of keys OpenCodex writes and fixed table labels (never values, continuation lines, or quoted table names), with the fix: remove each named key with its whole value and each named table with every line under it, then run `ocx disconnect` again. `ocx restore` is not named: on a connected Child it also stores Codex integration as off, and nothing here proves a later link turns it back on. + +Not done here: stripping the remaining OpenCodex lines inside disconnect. `ocx restore`'s strip does that with history-provider handling, and folding it into disconnect is a follow-up decision. + +## Tests + +- positive: Codex-appended lines, no routing → disconnect completes, config.toml unchanged, journal gone. +- negative: Codex-appended lines plus a remaining OpenCodex line (journaled `openai_base_url` without its marker) → refused, config.toml unchanged, journal kept, message names `openai_base_url`, config.toml and the fix. diff --git a/devlog/_plan/261111_child_disconnect_followups/030_o7_named_desktop_refusals.md b/devlog/_plan/261111_child_disconnect_followups/030_o7_named_desktop_refusals.md new file mode 100644 index 00000000000..7e4d2964c5b --- /dev/null +++ b/devlog/_plan/261111_child_disconnect_followups/030_o7_named_desktop_refusals.md @@ -0,0 +1,7 @@ +# O7 (rest) — name every desktop_lifecycle_unsafe cause + +`inspectRemoteDesktopStore` (src/claude/desktop-remote-store.ts) folds its failure sites into `conflict` / `unsafe`. Thread a reason and path out of each site class and map each to a message with the condition, the path and the fix, ending `(code desktop_lifecycle_unsafe)`. + +Classes (to be confirmed against source): store owned by another connection; OpenCodex profile removed or renamed in Claude Desktop; OpenCodex profile edited in Claude Desktop; an unreadable or unsafe store / library file; the folder with no state or receipt (#8529). + +Tests: one per class through `disconnectClient`, asserting path, fix and trailing code. diff --git a/devlog/_plan/261111_child_disconnect_followups/040_o10_restart_codex_clients.md b/devlog/_plan/261111_child_disconnect_followups/040_o10_restart_codex_clients.md new file mode 100644 index 00000000000..a601b16a374 --- /dev/null +++ b/devlog/_plan/261111_child_disconnect_followups/040_o10_restart_codex_clients.md @@ -0,0 +1,19 @@ +# O10 — restarting running Codex clients after sync (proposal, not built) + +## What happens today (source-checked) + +- `ocx sync` on a Child takes the connected path (`src/cli/sync-command.ts` → `syncConnectedClient`). After it, `handleConnectedSyncCatalogWrite` runs the restart scope only when that sync wrote the catalog or the models cache (`if (!result.catalogWritten && !result.cacheSynced) return`). +- Without a restart flag, the same write path calls `afterCatalogWriteHandleAppServers({ restart: false })` (`src/codex/app-server-processes.ts`), which prints "WARNING: N Codex app-server process(es) still running … Re-run with `ocx sync --restart-codex`" for app-servers started before the catalog's mtime. `ocx status` reports `clients=pending-restart(pid …)` from the same stale-client observation (`src/codex/autostart-health.ts`). +- Why the advice did not apply: the re-run writes nothing (the catalog is already current), so the restart scope never runs. The registry already says so for `ocx system codex-restart`: "`sync --restart-codex` is not a substitute: it restarts only as a side effect after a catalog, cache, or changed configuration write". +- `ocx system codex-restart --yes` restarts on request, but it posts to `/api/system/codex-restart` on the running runtime's management API. A Child's client runtime serves only the machine listener (`src/client/machine-listener.ts`: `/healthz`, the bound `POST /api/system/restart`); nothing under `src/client/` serves `codex-restart`, so on a Child this command has no endpoint to reach (to confirm with a test before relying on it). +- The Home's `ocx link restart-codex` and `ocx link sync --restart-*` send a second pinned `ocx sync --expect-link --json --restart-codex` to the Child (`src/link/ssh-argv.ts` `remotePinnedSyncArgv`). That goes through the same write-gated path, so a second sync that writes nothing restarts nothing. That is likely the same gap and needs a test to confirm. The Home withholds the restart while a relayed model request for that Child is in flight (`src/link/sync-then-restart.ts`, `restart-skipped:link-active`). +- Idleness: OpenCodex sees a turn only as a request through its own proxy (`getActiveTurnCount()` in `src/server/lifecycle.ts`, used by `src/codex/catalog-self-heal.ts`; the link's in-flight count on the Home). It cannot see a Codex app-server's own state: a turn waiting on an approval prompt, or a composer draft, has no request in flight. + +## Proposal + +1. **Restart what is actually stale, on request.** When `ocx sync --restart-codex` / `--restart-app-server-only` writes nothing, still restart the app-servers that `computeCodexAppServerCatalogStatus` classifies as stale (started at or before the current catalog's mtime). That makes the command the warning prints do what it says, and makes the Home's second pinned sync effective. Processes are the ones `listCodexAppServerProcesses` already selects (same user, Codex app-server / code-mode-host argv), narrowed to the stale set. A fresh process is never touched. The desktop app is quit only for the full `--restart-codex` scope, as today. +2. **Idle gate.** Before restarting, the Child checks its own client runtime for active turns, which needs a small `activeTurns` field on the client `/healthz`. If a turn is active it withholds the restart and reports `restart-skipped:active`, matching the Home's `link-active`. Because app-server-internal state stays invisible, the explicit flag (and `--yes` for the desktop scope) remains the consent. "Idle" here means no proxied request in flight, not "safe to discard UI state". +3. **Where it belongs.** Child-local: `ocx sync --restart-*` (the code path above), so a person on the Child, the Home's pinned sync, and `ocx link restart-codex` all share one implementation. `ocx system codex-restart` on a Child should either run the same local module or refuse with a message naming `ocx sync --restart-app-server-only`, instead of reaching for a management API the client runtime does not serve. +4. **Hints.** `clients=pending-restart` in `ocx status` and the sync warning on a Child should name the command that works there. + +Not built in this PR: (1) changes Home fan-out restart behaviour and needs its own tests across the three entry points. (2) adds a field to the client health contract. diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 3e8c1d719a7..297b9715b6e 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -111,6 +111,10 @@ To disconnect a Child-initiated link, run `ocx disconnect` on the Child. It disc On a full OpenCodex Child, `ocx disconnect` also restarts a client runtime that is still running, so the Child is serving standalone OpenCodex when the command returns. If that restart does not complete, the command exits non-zero and prints advice for the state it observed: `ocx restart` on the Child when the same runtime still serves the old connection; `ocx service restart` when a service that cannot be shown to relaunch the runtime refused the restart; and inspection with `ocx status` and `ocx connect status` when a new connection is in use, the client state cannot be read, or a different or unknown runtime answers. A client runtime whose connection was removed accepts `ocx restart` when its owner can relaunch it: the OpenCodex desktop app, or a background service that is enabled and running, starts the standalone replacement, so the same command works over SSH with nobody at the Child's screen. A service that cannot be shown to relaunch it (for example a unit started by hand while disabled) keeps the runtime and the command says to run `ocx service restart`. A runtime that carries only `OCX_SERVICE=1` (a service installed before 2.49, or a proxy started by `ocx claude`, `ocx codex` or `ocx opencode`) is not changed by this: `ocx disconnect` does not restart it, and `ocx restart` refuses while its service is viable, as before. The same holds for a client runtime started by an OpenCodex version before this change: `ocx disconnect` restarts a runtime only after it has been restarted once on the new version (an update's own restart, the desktop app restarting it, `ocx service restart`, or `ocx restart`). +If `ocx disconnect` is refused before it has changed anything outside Codex, for example because the Codex configuration cannot be restored, it rolls itself back: the Child stays connected, its runtime still starts, and the message says the disconnect was rolled back. A Child left with a half-finished disconnect by an older release refuses to start its client runtime with `client startup refused: an ocx disconnect on this machine stopped part way`; run `ocx disconnect` again (with `--keep-catalog` if the first run used it) to finish it or roll it back. If an earlier run had already begun restoring Claude Desktop, the receipt is kept and the retry finishes the disconnect. Only one `ocx disconnect` runs at a time on a machine; a second one refuses with `client_disconnect_busy`. + +Codex adds its own lines to `~/.codex/config.toml`, such as trusted projects and notice flags. When it has done so after OpenCodex wrote its routing and no OpenCodex line remains, `ocx disconnect` leaves the file as it is and removes the Codex journal, whose snapshot would otherwise overwrite those lines. The one exception is your own root `web_search` line, if OpenCodex set it aside while routing: disconnect puts that line back. If an OpenCodex setting remains, disconnect keeps the journal and names it; remove each named key with its whole value and each named table with every line under it, then run `ocx disconnect` again. A refusal about the Claude Desktop remote profile store (`desktop_lifecycle_unsafe`) names the file or folder involved and the step that clears it. + ## Troubleshooting When a step fails, the dashboard shows the reason and, when SSH reported one, a short sanitized hint from its last non-empty error line under the message. Remote-shell errors can appear there even when the remote shell emits non-UTF-8 text; OpenCodex removes terminal controls, link keys and URL queries and limits the hint's length. diff --git a/scripts/test-layout/layout-clients.json b/scripts/test-layout/layout-clients.json index 7df15a75789..d825bd96e25 100644 --- a/scripts/test-layout/layout-clients.json +++ b/scripts/test-layout/layout-clients.json @@ -162,6 +162,8 @@ "client-link-state.test.ts": "clients", "client-link-status.test.ts": "clients", "client-link-teardown.test.ts": "clients", + "client-disconnect-recovery.test.ts": "clients", + "client-disconnect-desktop-refusals.test.ts": "clients", "client-link-tunnel.test.ts": "clients", "link-ports.test.ts": "clients", "link-supervisor.test.ts": "clients", diff --git a/src/claude/desktop-remote-store.ts b/src/claude/desktop-remote-store.ts index e59c4db44d9..a61539d9880 100644 --- a/src/claude/desktop-remote-store.ts +++ b/src/claude/desktop-remote-store.ts @@ -1,8 +1,8 @@ -import { existsSync } from "node:fs"; +import { existsSync, rmdirSync } from "node:fs"; import { join } from "node:path"; import { readConfigDiagnostics, withConfigMutationLockSync } from "../config"; import { assertClientLifecycleHeld, type ClientLifecycleHeld } from "../client/lifecycle-lock"; -import { readServiceApiTokenState, serviceApiTokenFingerprint } from "../lib/service-secrets"; +import { readServiceApiTokenState, serviceApiTokenBackupPath, serviceApiTokenFingerprint } from "../lib/service-secrets"; import { assertDesktop3pModelsValid } from "./desktop-3p-guard"; import { profilePath } from "./desktop-3p-library"; import type { Desktop3pConfigMode, Desktop3pModelEntry } from "./desktop-3p"; @@ -50,35 +50,62 @@ function recordedGenerations(io: StoreIO, owner: DesktopRemoteOwner): string[] { ...(bundle.state?.pending ? [bundle.state.pending.tokenFingerprint] : [])]; } +/** + * Why an inspection answered conflict or unsafe, and the file or folder it was looking at, so a + * refusal can name both. The step that failed decides the reason; nothing here changes the verdict. + */ +export type DesktopStoreInspectReason = + | "root_without_state" | "store_unreadable" | "store_other_connection" | "library_unreadable" + | "profile_missing" | "profile_edited" | "profile_ambiguous" | "token_unreadable" | "connection_changed" | "unknown"; + export function inspectRemoteDesktopStore(owner: DesktopRemoteOwner): { kind: "absent" | "active" | "pending" | "restored" | "legacy_current_connection" | "conflict" | "unsafe"; - /** Set for the one refusal a person can clear: the store folder exists with nothing in it OpenCodex reads. */ - reason?: "root_without_state"; + /** Set with conflict and unsafe. */ + reason?: DesktopStoreInspectReason; path?: string; } { + let site: { reason: DesktopStoreInspectReason; path?: string } = { reason: "unknown" }; try { parseOwner(owner); - const io = new StoreIO(), bundle = loadBundle(io); + site = { reason: "store_unreadable" }; + const io = new StoreIO(); + site.path = storePaths().root; + const bundle = loadBundle(io); if (bundle.state?.phase === "cleaned") { const terminal = receipt(io); if (terminal?.phase === "complete" && sameOwner(bundle.state.owner, terminal.owner) && !sameOwner(owner, terminal.owner)) return { kind: "absent" }; } + site = { reason: "store_other_connection", path: bundle.paths.state }; requireOwner(bundle, owner); if (bundle.state) { if (bundle.state.phase === "prepared" || bundle.state.pending) return { kind: "pending" }; if (bundle.state.phase === "cleaned") return { kind: "restored" }; + site = { reason: "library_unreadable", path: bundle.paths.library }; const meta = metadata(io, bundle.paths.library); selectedFile(io, bundle.paths.library, meta.value); - if (!meta.value.entries.some(e => e.id === bundle.state!.targetId && e.name === "opencodex")) return { kind: "conflict" }; - const file = io.read(profilePath(bundle.paths.library, bundle.state.targetId)); - if (projectionHash(file?.value ?? null) !== bundle.state.lastProjectionHash) return { kind: "conflict" }; + if (!meta.value.entries.some(e => e.id === bundle.state!.targetId && e.name === "opencodex")) { + return { kind: "conflict", reason: "profile_missing", path: bundle.paths.library }; + } + const target = profilePath(bundle.paths.library, bundle.state.targetId); + site.path = target; + const file = io.read(target); + if (projectionHash(file?.value ?? null) !== bundle.state.lastProjectionHash) return { kind: "conflict", reason: "profile_edited", path: target }; return { kind: bundle.state.phase === "restored" ? "restored" : "active" }; } if (existsSync(bundle.paths.root) && !receipt(io)) return { kind: "unsafe", reason: "root_without_state", path: bundle.paths.root }; - const legacy = locateLegacy(io, owner, serviceGenerations()); + site = { reason: "token_unreadable", path: serviceApiTokenBackupPath() }; + const generations = serviceGenerations(); + site = { reason: "profile_ambiguous", path: bundle.paths.library }; + const legacy = locateLegacy(io, owner, generations); + site = { reason: "connection_changed" }; if (legacy) currentConnection(owner); return { kind: legacy ? "legacy_current_connection" : "absent" }; - } catch (error) { return { kind: error instanceof DesktopStoreError && error.reason === "conflict" ? "conflict" : "unsafe" }; } + } catch (error) { + const conflict = error instanceof DesktopStoreError && error.reason === "conflict"; + // Only an ambiguous match is a conflict in the legacy scan; an unreadable list is not. + const reason = site.reason === "profile_ambiguous" && !conflict ? "library_unreadable" : site.reason; + return { kind: conflict ? "conflict" : "unsafe", reason, ...(site.path ? { path: site.path } : {}) }; + } } export function applyRemoteDesktopStore(held: ClientLifecycleHeld, options: { @@ -230,6 +257,29 @@ export function writeDesktopDisconnectReceipt(held: ClientLifecycleHeld, expecte } } +/** + * Remove a disconnect receipt that never passed `prepared`, and its folder when the receipt was all + * it held. Compare-and-delete: a receipt another run advanced or replaced is left alone. + */ +export function removeDesktopDisconnectReceipt(held: ClientLifecycleHeld, expected: DesktopDisconnectReceipt): void { + assertClientLifecycleHeld(held); + try { + withConfigMutationLockSync(() => { + if (parseDisconnect(expected).phase !== "prepared") throw new DesktopStoreError("conflict"); + const io = new StoreIO(), paths = storePaths(); + const file = io.read(paths.disconnect, 64 * 1024, true); + if (!file || canonical(parseDisconnect(file.value)) !== canonical(expected)) throw new DesktopStoreError("conflict"); + io.remove(paths.disconnect, file); + // Not recursive: a folder that still holds Claude Desktop state stays, and that state + // explains itself to the next inspection. An empty one would read as a leftover. + try { rmdirSync(paths.root); } catch { /* not empty, or already gone */ } + }); + } catch (error) { + throw new Error(error instanceof DesktopStoreError && error.reason === "conflict" + ? "desktop_disconnect_receipt_conflict" : "desktop_disconnect_receipt_unsafe"); + } +} + export function inspectRemoteDesktopCleanup(): | { kind: "absent" } | { kind: "active" | "restored" | "pending"; owner: DesktopRemoteOwner } | { kind: "unsafe" } { try { diff --git a/src/cli/index.ts b/src/cli/index.ts index 870000f41dd..3220973afed 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -50,7 +50,7 @@ import { runCodexHistoryJob, } from "../codex/history-job"; import { findCrossHomeOwner, markCrossHomeSibling, markLiveHomeSibling } from "./cross-home-owner"; -import { inspectClientRotationRecoveryGate, readClientConnectionState } from "../client/state"; +import { clientStartupRefusalMessage, inspectClientRotationRecoveryGate, readClientConnectionState } from "../client/state"; import { codexAutoStartEnabled, getConfigDir, @@ -487,7 +487,7 @@ async function handleStart(options: { block?: boolean } = {}) { const rotationGate = inspectClientRotationRecoveryGate(clientState); if (clientState.kind === "connected") { if (rotationGate.kind === "recovery-required" || rotationGate.kind === "unsafe") { - throw new Error(`client startup refused: ${rotationGate.reason}`); + throw new Error(clientStartupRefusalMessage(rotationGate.reason)); } const { startClientRuntime } = await import("../client/runtime"); // Same lease-held owner recheck as the server path below: this branch binds and publishes too. diff --git a/src/client/connect.ts b/src/client/connect.ts index 19c5c2dab6e..8619d265831 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -11,15 +11,17 @@ import { getConfigDir, withConfigDirHardeningSettled } from "../config/paths"; import { claudeDesktopIntegrationEnabledNow } from "../codex/desired-state"; import { siblingOfLivePort, siblingSkipMessage } from "../codex/sibling-start"; import { - inspectRemoteDesktopStore, readDesktopDisconnectReceipt, writeDesktopDisconnectReceipt, + inspectRemoteDesktopStore, readDesktopDisconnectReceipt, writeDesktopDisconnectReceipt, removeDesktopDisconnectReceipt, replaceRemoteDesktopCredential, restoreRemoteDesktopStore, finishRemoteDesktopCleanup, type DesktopDisconnectReceipt, type DesktopRemoteOwner, type DesktopStoreResult, } from "../claude/desktop-remote-store"; import { - withClientLifecycle, withClientLifecycleSync, + withClientDisconnectAttempt, withClientLifecycle, withClientLifecycleSync, type ClientLifecycleHeld, type ClientLifecycleLockDeps, } from "./lifecycle-lock"; import { invalidateCodexModelsCache, invalidateCodexModelsCacheWithPermit } from "../codex/catalog/sync"; +import { restoreCodexForDisconnect } from "../codex/inject/disconnect-restore"; +import { desktopLifecycleRefusal } from "./desktop-refusal"; import { withCatalogWriteSerialization } from "../codex/catalog-write-serialization"; import { injectCodexConfig, @@ -1116,13 +1118,59 @@ function restoreConnectedCodex(connection: OcxClientConnectionConfig): void { throw new Error("disconnect refused: Codex journal ownership conflicts with the connected key"); } if (owner !== null) { - if (!restoreJournalState().complete) throw new Error("disconnect refused: Codex journal restore was partial"); + const restored = restoreCodexForDisconnect(); + if (!restored.ok) throw new Error(`disconnect refused: Codex journal restore was partial: ${restored.reason}`); } else if (isCodexRoutingInjected()) { throw new Error("disconnect refused: Codex routing is injected but no journal records the original state"); } } +/** + * Abort a disconnect that failed before its commit point. + * + * Until `desktop_restored` the receipt records intent only, so removing it returns the Child to a plain + * connected state; left in place it made every client start refuse with client_disconnect_pending + * until someone moved the folder aside by hand. A receipt this attempt wrote is removed outright: + * D excludes other disconnects and every Desktop writer refuses while it exists. A receipt an + * earlier run left (including one stuck by an older release) is removed only while the Desktop + * store shows no restore has started; that run may have stopped inside the Desktop restore before + * it could record desktop_restored, and then the receipt is the gate a retry needs. + */ +function abortPreparedDisconnect( + prepared: { receipt: DesktopDisconnectReceipt; created: boolean }, + error: unknown, + deps: Pick, +): Error { + const base = error instanceof Error ? error.message : String(error); + let outcome: "removed" | "desktop_started"; + try { + outcome = withClientLifecycleSync(held => withConfigMutationLockSync(() => { + if (!prepared.created && !UNTOUCHED_DESKTOP.has(inspectRemoteDesktopStore(prepared.receipt.owner).kind)) return "desktop_started" as const; + removeDesktopDisconnectReceipt(held, prepared.receipt); + return "removed" as const; + }), deps.lifecycleLockDeps); + } catch { + return new Error(`${base}. The disconnect receipt could not be rolled back; run ocx disconnect again to finish it or roll it back.`, { cause: error }); + } + return new Error(outcome === "removed" + ? `${base}. The disconnect was rolled back: this Child is still connected and its runtime can start.` + : `${base}. A Claude Desktop restore had already started under this disconnect, so its receipt was kept; once the cause above is fixed, run ocx disconnect again to finish it.`, { cause: error }); +} + +/** Desktop store states an interrupted disconnect cannot have produced: no restore has begun. */ +const UNTOUCHED_DESKTOP: ReadonlySet = new Set(["absent", "active", "legacy_current_connection"]); + export async function disconnectClient( + options: Parameters[0] = {}, + deps: Pick = {}, +): ReturnType { + // The attempt lock is a file under the config directory: wait for parent ACL admission first, + // as every other phase does, so nothing is created under a directory still being hardened. + await withConfigDirHardeningSettled(getConfigDir(), () => undefined); + return withClientDisconnectAttempt(() => disconnectClientAttempt(options, deps), deps.lifecycleLockDeps); +} + +async function disconnectClientAttempt( options: { keepCatalog?: boolean; expectedOwner?: DesktopRemoteOwner } = {}, deps: Pick = {}, ): Promise<{ @@ -1145,6 +1193,7 @@ export async function disconnectClient( throw new Error("client_disconnect_expected_owner_changed"); } let receipt = previous; + let created = false; let connection: OcxClientConnectionConfig | null = null; if (state.kind === "connected") { connection = state.value; @@ -1161,25 +1210,24 @@ export async function disconnectClient( } if (!receipt) { const desktop = inspectRemoteDesktopStore(desktopOwner(connection)); - if (desktop.reason === "root_without_state") { - throw new Error(`desktop_lifecycle_unsafe: ${desktop.path} exists but holds no Claude Desktop remote state or disconnect receipt (a leftover or renamed file), so disconnect cannot tell what it would restore. Move that folder aside, then run ocx disconnect again.`); - } - if (desktop.kind === "unsafe" || desktop.kind === "conflict") throw new Error(`desktop_lifecycle_unsafe: the Claude Desktop remote profile store could not be verified (${desktop.kind}); nothing was changed.`); + if (desktop.kind === "unsafe" || desktop.kind === "conflict") throw new Error(desktopLifecycleRefusal(desktop)); preflightDisconnectCatalog(connection, keepCatalog); receipt = { version: 1, owner: desktopOwner(connection), tokenFingerprint: connection.tokenFingerprint, keepCatalog, phase: "prepared" }; writeDesktopDisconnectReceipt(held, previous, receipt); + created = true; } } else if (state.kind !== "disconnected" || !receipt || !disconnectAtLeast(receipt, "clearing_connection")) { throw new Error("disconnect refused: no recoverable connected state; if this is not a Hub and no admission secret or installed service uses the token, inspect it and use ocx disconnect --orphaned-token --yes"); } if (!receipt || receipt.keepCatalog !== keepCatalog) throw new Error("client_disconnect_options_changed"); - return { receipt, connection }; + return { receipt, connection, created }; }), deps.lifecycleLockDeps)); // The prepared receipt blocks even a sync queued for its actual N/C injection commit. // Codex-only restore MUST stay outside L; it never invokes Desktop cleanup. if (prepared.connection && !disconnectAtLeast(prepared.receipt, "desktop_restored")) { - restoreConnectedCodex(prepared.connection); + try { restoreConnectedCodex(prepared.connection); } + catch (error) { throw abortPreparedDisconnect(prepared, error, deps); } } return withConfigDirHardeningSettled(getConfigDir(), () => withClientLifecycleSync(held => withConfigMutationLockSync(() => { diff --git a/src/client/desktop-refusal.ts b/src/client/desktop-refusal.ts new file mode 100644 index 00000000000..09b24110014 --- /dev/null +++ b/src/client/desktop-refusal.ts @@ -0,0 +1,38 @@ +import type { DesktopStoreInspectReason } from "../claude/desktop-remote-store"; + +const FOLDER = "the desktop-remote folder in the OpenCodex home"; + +/** + * The `ocx disconnect` refusal for a Claude Desktop remote store it cannot verify: what was found, + * where, and the step that clears it, with the machine-readable code last. Each fix leaves the next + * inspection able to decide; none is run for the person, because each gives up or rewrites + * something in Claude Desktop that only they can judge. + */ +export function desktopLifecycleRefusal(desktop: { kind: string; reason?: DesktopStoreInspectReason; path?: string }): string { + const at = desktop.path; + const body = (() => { + switch (desktop.reason) { + case "root_without_state": + return `${at} exists but holds no Claude Desktop remote state or disconnect receipt (a leftover or renamed file), so disconnect cannot tell what it would restore. Nothing was changed. Move that folder aside, then run ocx disconnect again.`; + case "store_unreadable": + return `OpenCodex cannot read safely the Claude Desktop remote state in ${at ?? FOLDER}: state.json or baseline.json is not an owner-only regular file, is not valid, or was written for another OpenCodex home or Claude Desktop library. Nothing was changed. If either file is not owner-only, make it so (chmod 600) and retry. Otherwise move that folder aside and keep it (its baseline.json holds the Claude Desktop profile OpenCodex replaced); disconnect then gives Claude Desktop's OpenCodex profile its standard settings. In either case, then run ocx disconnect again.`; + case "store_other_connection": + return `${at} records a Claude Desktop profile applied for a different connection (another Home or an earlier link), so this disconnect cannot restore it. Nothing was changed. In Claude Desktop, remove that connection's OpenCodex profile; then move ${FOLDER} aside, then run ocx disconnect again.`; + case "library_unreadable": + return `the Claude Desktop profile list in ${at} (_meta.json or the selected profile file) could not be read safely. Nothing was changed. Restore that file to valid JSON from Claude Desktop or a backup, then run ocx disconnect again.`; + case "profile_missing": + return `the OpenCodex profile in the Claude Desktop library ${at} was removed or renamed outside OpenCodex, so disconnect cannot restore the profile it replaced. Nothing was changed. Rename it back to "opencodex" in Claude Desktop; if it was deleted, move ${FOLDER} aside instead; then run ocx disconnect again.`; + case "profile_edited": + return `the OpenCodex profile ${at} was edited outside OpenCodex after it was applied. Nothing was changed. Disconnect restores the profile OpenCodex replaced, which discards those edits: copy anything you want to keep from that file, undo the edits, then run ocx disconnect again.`; + case "profile_ambiguous": + return `a profile in the Claude Desktop library ${at} reaches this Home, but disconnect cannot identify it as OpenCodex's current profile (it has another name, an older key, or is a second copy). Nothing was changed. In Claude Desktop, remove every profile that reaches this Home except one named "opencodex" that uses this connection's current key; if the only one left uses an older key, remove that one too, then run ocx disconnect again.`; + case "token_unreadable": + return `the service token or its rotation backup (${at}) is not a safe regular file, so disconnect cannot tell which keys belong to this connection. Nothing was changed. Make both files owner-only regular files (move a directory or link at that path aside), then run ocx disconnect again.`; + case "connection_changed": + return `the saved connection changed while disconnect was checking Claude Desktop. Nothing was changed. Check ocx connect status, then run ocx disconnect again.`; + default: + return `the Claude Desktop remote profile store could not be verified (${desktop.kind}). Nothing was changed. Check ${FOLDER} and the Claude Desktop profile library, then run ocx disconnect again.`; + } + })(); + return `Disconnect refused: ${body} (code desktop_lifecycle_unsafe)`; +} diff --git a/src/client/lifecycle-lock.ts b/src/client/lifecycle-lock.ts index a6f025e2afc..4b525a448a2 100644 --- a/src/client/lifecycle-lock.ts +++ b/src/client/lifecycle-lock.ts @@ -1,4 +1,7 @@ -/** Client/Desktop lifecycle exclusion. Lock order: N -> L -> C; never acquire N inside L. */ +/** + * Client/Desktop lifecycle exclusion. Lock order: D -> N -> L -> C; never acquire N inside L. + * D (one disconnect attempt) is outermost and taken by nothing else. + */ import { Database } from "bun:sqlite"; import { chmodSync, closeSync, lstatSync, mkdirSync, openSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; @@ -54,14 +57,14 @@ function preparePath(lockPath: string): void { assertPath(lockPath, false); } -function acquire(deps: ClientLifecycleLockDeps): Database { +function acquire(deps: ClientLifecycleLockDeps, name = "client-desktop-lifecycle.sqlite", suffix = ""): Database { let database: Database | undefined; try { // The production namespace belongs to the effective OS user, never HOME, // OPENCODEX_HOME, Desktop library overrides or an environment test switch. const path = deps.lockPath === undefined - ? join(resolveEffectiveUserRuntimeRoot(resolveEffectiveUserIdentity()), "client-desktop-lifecycle.sqlite") - : resolve(deps.lockPath); + ? join(resolveEffectiveUserRuntimeRoot(resolveEffectiveUserIdentity()), name) + : resolve(deps.lockPath) + suffix; preparePath(path); database = new Database(path, { create: true }); database.exec("PRAGMA locking_mode = NORMAL; PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); @@ -127,3 +130,25 @@ export function withClientLifecycleSync( finally { activeLeases.delete(held); } return finish(database, outcome); } + +/** + * D: one ocx disconnect attempt at a time, held across the whole attempt. + * + * A disconnect restores Codex outside L, so two attempts used to share one prepared receipt + * there; rolling back one attempt's receipt then removed the other's while it was still + * restoring. D is a separate lock so taking it never orders against N, L or C. + */ +export async function withClientDisconnectAttempt(work: () => Promise, deps: ClientLifecycleLockDeps = {}): Promise { + let database: Database; + try { database = acquire(deps, "client-disconnect-attempt.sqlite", ".disconnect-attempt"); } + catch (error) { + if (errorCode(error) === "client_lifecycle_busy") { + throw new ClientLifecycleError("client_disconnect_busy: another ocx disconnect is running on this machine; let it finish, then run ocx disconnect again"); + } + throw error; + } + let outcome: Outcome; + try { outcome = { ok: true, value: await work() }; } + catch (error) { outcome = { ok: false, error }; } + return finish(database, outcome); +} diff --git a/src/client/state.ts b/src/client/state.ts index 132475860f3..766d418908b 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -162,6 +162,20 @@ export function assertNoClientDisconnectPending(): void { } } +/** + * The message client startup refuses with. A pending disconnect names the command that clears it: + * under the desktop app or a service the supervisor only logs this line, and on 2.100.2 a person + * had to move desktop-remote/ aside by hand because nothing said a retry would finish or roll back. + */ +export function clientStartupRefusalMessage(reason: string): string { + if (reason !== "client_disconnect_pending") return `client startup refused: ${reason}`; + const receipt = readDesktopDisconnectReceipt(); + const valid = receipt.kind === "valid" ? receipt.value : null; + const command = valid?.keepCatalog ? "ocx disconnect --keep-catalog" : "ocx disconnect"; + return `client startup refused: an ocx disconnect on this machine stopped part way${valid ? ` at phase ${valid.phase}` : ""}. ` + + `Run ${command} again to finish it or roll it back. (code client_disconnect_pending)`; +} + /** Full snapshot CAS for work returning from an await, including selection and rotation state. */ export function assertClientConnectionUnchanged(expected: OcxClientConnectionConfig): void { assertNoClientDisconnectPending(); diff --git a/src/codex/inject/disconnect-restore.ts b/src/codex/inject/disconnect-restore.ts new file mode 100644 index 00000000000..3bdba579124 --- /dev/null +++ b/src/codex/inject/disconnect-restore.ts @@ -0,0 +1,65 @@ +/** + * The Codex half of `ocx disconnect`: restore what the client injection changed, or prove there + * is nothing left to restore. + * + * The journal replays its pre-injection snapshot only while config.toml is byte-for-byte the + * original or the first injected bytes. Codex itself appends to config.toml (trusted projects, + * notice flags), and after that the replay would delete those lines, so the journal restore + * stays partial forever. When the only difference is lines OpenCodex does not own, there is + * nothing to undo: config.toml is left as it is and the journal, whose one remaining use is that + * unsafe replay, is retired. While any OpenCodex line remains the journal is kept. + * + * Everything runs under one held Codex config write lock, so the file judged is the file kept. + */ +import { existsSync, readFileSync } from "node:fs"; +import { configWriteLockFailureMessage, withConfigWriteLockHeld } from "../config-write-lock"; +import { JOURNAL_PATH, removeJournal, restoreJournalState } from "../journal"; +import { CodexHomeOwnerRefusal } from "../codex-home-owner"; +import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "../paths"; +import { beginCodexWriteSection, publishCodexArtifact } from "./config-write-section"; +import { remainingOpencodexConfigKeys, withReplacedWebSearchRestored } from "./remove"; +import { atomicWriteFile } from "../../config"; + +export type DisconnectCodexRestore = + | { ok: true; journalRetired: boolean } + | { ok: false; reason: string }; + +export function restoreCodexForDisconnect(): DisconnectCodexRestore { + const locked = withConfigWriteLockHeld(CODEX_CONFIG_PATH, undefined, (held): DisconnectCodexRestore => { + beginCodexWriteSection(held); + const restored = restoreJournalState({ heldConfigWriteLock: held }); + if (restored.complete) return { ok: true, journalRetired: false }; + if (restored.lockBusy) return { ok: false, reason: "another process is writing the Codex configuration; retry shortly" }; + if (restored.lockUnsafe) return { ok: false, reason: restored.lockUnsafe }; + if (restored.ownershipRefusal) return { ok: false, reason: new CodexHomeOwnerRefusal(restored.ownershipRefusal).message }; + if (restored.unverified) { + return { ok: false, reason: `the journal ${JOURNAL_PATH} does not record what OpenCodex wrote, so a changed Codex file cannot be told apart from your own edits; files and journal were kept` }; + } + if (restored.profileRestoreFailed || !restored.profileRestored) { + return { ok: false, reason: `${CODEX_PROFILE_PATH} changed after OpenCodex wrote it or could not be restored; files and journal were kept` }; + } + if (!restored.configChanged) return { ok: false, reason: `the journal ${JOURNAL_PATH} could not be removed` }; + const config = existsSync(CODEX_CONFIG_PATH) ? readFileSync(CODEX_CONFIG_PATH, "utf-8") : ""; + const remaining = remainingOpencodexConfigKeys(config); + if (remaining.length > 0) { + // The one difference that is the person's, not OpenCodex's: their own web_search line, set + // aside while routed. Put back exactly that line when nothing OpenCodex owns is left after it. + const restoredLine = withReplacedWebSearchRestored(config); + if (restoredLine !== null && remainingOpencodexConfigKeys(restoredLine).length === 0) { + publishCodexArtifact(CODEX_CONFIG_PATH, held, (path, hooks) => atomicWriteFile(path, restoredLine, undefined, hooks)); + publishCodexArtifact(JOURNAL_PATH, held, (_path, hooks) => removeJournal(hooks)); + return { ok: true, journalRetired: true }; + } + return { + ok: false, + reason: `${CODEX_CONFIG_PATH} changed after OpenCodex wrote it and still has OpenCodex settings (${remaining.join(", ")}), so the journal was kept. ` + + "Remove them from that file: each named key with its whole value (a multiline value ends at its closing quotes or bracket), " + + "and each named table with every line under it. Keep every other line, then run ocx disconnect again", + }; + } + publishCodexArtifact(JOURNAL_PATH, held, (_path, hooks) => removeJournal(hooks)); + return { ok: true, journalRetired: true }; + }); + if (!locked.ok) return { ok: false, reason: configWriteLockFailureMessage(locked) }; + return locked.value; +} diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index 916e96ed222..98edef45490 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -1,6 +1,6 @@ import { beginCodexWriteSection, publishCodexArtifact, removeCodexProfileEntry } from "./config-write-section"; import { existsSync, lstatSync, readFileSync } from "node:fs"; -import { normalizeStructuralWhitespace, rootAssignmentKey, rootSourceLines, sourceAssignment, sourceText } from "../toml-source-lines"; +import { normalizeStructuralWhitespace, rootAssignmentKey, rootSourceLines, sourceAssignment, sourceText, splitSourceLines } from "../toml-source-lines"; import { atomicWriteFile } from "../../config"; import { ConfigWriteLockRefusal, configWriteLockFailureMessage, withConfigWriteLockHeld, withConfigWriteRecovery } from "../config-write-lock"; import type { LockHandle } from "../config-write-lock"; @@ -146,6 +146,92 @@ export function stripOpencodexConfig(content: string): string { return stripOpencodexConfigResult(content).content; } +/** + * What OpenCodex still owns in a Codex config, judged by the same strip `ocx restore` applies and + * the journaled injection: the key or table header of each line it would remove. Empty means + * nothing OpenCodex wrote is left, so lines Codex or the person added are all that changed. + * Only parsed key names and table headers are returned, never values or continuation lines of a + * multiline value, so a refusal can quote them without leaking a credential. + */ +export function remainingOpencodexConfigKeys(rawContent: string): string[] { + const content = applyEol(rawContent, "\n"); + const stripped = stripOpencodexConfigResult( + content, + journaledInjectedOpenaiBaseUrl(), + journaledInjectedRealtimeWsBaseUrl(), + journaledInjectedRootWebSearch(), + journaledReplacedRootWebSearch(), + journaledInjectedSubagentDefaults(), + ); + if (stripped.managedDefaultsError) return ["[agents] managed subagent defaults"]; + // The strip also normalizes blank lines; that alone is not an OpenCodex line. + if (stripped.content === normalizeStructuralWhitespace(content)) return []; + const kept = new Map(); + for (const line of stripped.content.split("\n")) kept.set(line.trim(), (kept.get(line.trim()) ?? 0) + 1); + const keys: string[] = []; + const add = (label: string) => { if (!keys.includes(label)) keys.push(label); }; + // Lines under a removed table are covered by the table's label. + let underRemovedTable = false; + for (const line of splitSourceLines(content)) { + const text = line.text.trim(); + if (!text) continue; + const header = line.structural && text.startsWith("["); + const left = kept.get(text) ?? 0; + if (left > 0) { kept.set(text, left - 1); if (header) underRemovedTable = false; continue; } + // A line inside a multiline string or array is a value, never a name. + if (!line.structural) continue; + if (header) { underRemovedTable = true; add(ownedTableLabel(text)); continue; } + if (underRemovedTable) continue; + if (text.startsWith("#")) { add("an OpenCodex marker comment"); continue; } + const key = /^([A-Za-z0-9_]{1,64})\s*=/.exec(text)?.[1]; + add(key && OWNED_CONFIG_KEYS.has(key) ? key : "an OpenCodex line"); + } + + // The strip changed something without removing a line: it would put back a line it replaced. + return keys.length > 0 ? keys : ["web_search"]; +} + +/** + * The config with the person's own root `web_search` line put back, when OpenCodex set it aside while + * routed (the journal records the exact line) and the file now has no root `web_search` at all. + * Only that line is inserted, at the end of the root section; every other byte is kept. Null when + * there is nothing to put back. The caller must re-check the result for OpenCodex settings. + */ +export function withReplacedWebSearchRestored(rawContent: string): string | null { + const line = journaledReplacedRootWebSearch()?.replace(/\r?\n$/, ""); + if (!line || rootAssignmentKey(line) !== "web_search") return null; + // Work on the file's own lines and endings: a mixed CRLF/LF file keeps every existing byte. + const { bom, lines, rootEnd } = rootSourceLines(rawContent); + if (lines.slice(0, rootEnd).some(entry => entry.structural && rootAssignmentKey(entry.text) === "web_search")) return null; + // Insert after the last non-blank root line, so a blank separator before the first table stays put. + let at = rootEnd; + while (at > 0 && lines[at - 1]!.structural && lines[at - 1]!.text.trim() === "") at -= 1; + const before = at > 0 ? lines[at - 1]! : null; + // The new line takes the ending of the line it follows; only a final line with no ending gains one. + const eol = before?.eol || (dominantEol(rawContent) === "\r\n" ? "\r\n" as const : "\n" as const); + const next = [...lines]; + if (before && before.eol === "") next[at - 1] = { ...before, eol }; + next.splice(at, 0, { text: line, eol, structural: true }); + return bom + sourceText(next); +} + + +/** The only names a refusal may quote: keys OpenCodex writes. Anything else gets a fixed label. */ +const OWNED_CONFIG_KEYS: ReadonlySet = new Set([ + "openai_base_url", REALTIME_WS_BASE_URL_KEY, "model_provider", "model", "model_catalog_json", "web_search", "profile", + "default_subagent_model", "default_subagent_reasoning_effort", +]); + +/** A fixed label per table OpenCodex writes; a table name can carry arbitrary quoted text. */ +function ownedTableLabel(header: string): string { + const name = header.replace(/^\[+|\]+$/g, "").replace(/[\s"']/g, ""); + for (const label of ["model_providers.opencodex", "profiles.opencodex", "agents"]) { + if (name === label || name.startsWith(label + ".")) return "[" + label + "]"; + } + return "an OpenCodex table"; +} + + function hasOpencodexRouting(content: string): boolean { return ( hasOcxProviderTable(content) || diff --git a/structure/remote-link.md b/structure/remote-link.md index 2b98850c7de..29831353d09 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -205,7 +205,7 @@ Coverage: `tests/server/link-management-relay.test.ts` (the real hub-link listen `ocx status --json` reports `appliedCodexSettingsDigest` on every device (a Child's record, or a Home's own projection, which is what it serves), and on a Child `lastApplyAt` and `runtimeStartedAt` as epoch ms (`src/cli/status-codex-settings.ts`). The client runtime's `/healthz` reports `startedAt`. After `ocx sync` records an apply, `src/cli/link-apply-restart.ts` restarts the client runtime through `handleProxyRestart`, the `ocx restart` path, only when the live client runtime started before `appliedAt` or reports no start; its start callback starts nothing unless an accepted restart left no runtime, which takes the ordinary recovery start. An explicit Desktop or service supervised refusal defers that restart without failing the applied sync; the notice directs Desktop users to Quit from the tray menu and reopen OpenCodex, or service users to `ocx service restart`. The next sync retries until the runtime starts after the apply. Other restart failures remain failures, and any earlier sync error is retained. The Child's JSON shape stays unchanged so existing Home fan-out parsers accept the successful result. -The client listener serves that bound restart (`src/client/machine-listener.ts`, `src/client/runtime-restart.ts`). `/healthz` answers the attestation challenge with the secret the runtime publishes in `runtime-port.json` and advertises `restartCapability: "v1"`; `POST /api/system/restart` admits only the HMAC capability bound to this pid and port, never a dashboard session, admin token or link key. If the relaunch command cannot be identified, `src/client/runtime.ts` omits it from the runtime record; update admission treats the identity as unproven, while client startup and shutdown continue. Under the desktop app or a viable service it answers `409 restart_supervised` naming the supervisor and keeps serving, because handing off to a supervisor means exiting before the replacement is proven; the CLI reports "not performed, runtime kept". That refusal applies only while the durable connection is still the one the runtime serves: after `ocx disconnect` or a newer connect there is nothing to keep, so the restart is accepted and hands off to the standalone recycle (desktop exit 75, a viable service exit 1, otherwise a spawned replacement carrying no supervisor marker). "Ended" needs evidence (`readClientConnectionStateWithEvidence`, one snapshot of config.json): a saved file that validates and records no client, or another connection; missing, unreadable or invalid is invalid. Ownership and relaunch are separate (`classifyClientRestartSupervisor`): manager-launched evidence is the current markers in the environment only (`isMarkedSupervisedServiceChild`: the managed marker or the Windows wrapper marker); a legacy Windows registration recognised only from its parent's command line counts as a bare OCX_SERVICE=1; relaunch needs the service diagnostic to be viable and running. Manager-launched without that is `service-unproven`. `service-unproven` keeps the runtime and is refused at admission even when the connection ended; on this PR's recycle path only a process whose owner is proven to be none detaches, carrying no supervisor marker. A bare OCX_SERVICE=1 is outside that path: with a viable service it is refused, and otherwise it takes origin/dev's replacement restart, which spawns a detached replacement even while a legacy manager still owns the process (#8545). A marker proves only that the process was started with it, so no supervisor marker may reach a process that supervisor did not launch: `src/lib/supervisor-markers.ts` holds the one list, and every detached runtime launch (companions, recycle, replacement, detached and post-update starts) and every shell a runtime hands to a tool clears it. The ended-connection recycle needs positive ownership evidence (`restartOwnerProven`: the desktop app, a manager's marker, or no OCX_SERVICE at all). A bare OCX_SERVICE=1 (a service registered before v2.49, or the proxy of ocx claude, ocx codex or ocx opencode) keeps origin/dev's behaviour unchanged: `ocx restart` refuses when the service is viable and otherwise restarts with a replacement (rollback exits 1 without spawning), its runtime record states no owner, and `ocx disconnect` restarts only a runtime whose record states one it handles (`restartOwner`: desktop, service-manager or none). A bare-flag runtime, one started before this field existed, or an unknown value gets origin/dev's disconnect: no restart request, exit 0. An inconclusive liveness probe or an unreadable record still exits 1 before any request, the one deliberate difference from origin/dev (no runtime is stopped, and the Home's leave stops before its own `ocx restart`). Owner evidence for those runtimes is a separate design (follow-up issue). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live, bound to the runtime generation it observed: it captures that runtime's record (pid, port, per-process attestation secret) and the bound restart attests and derives its capability from the captured record, so another process at a reused pid is refused. It reads the client state once after its observation; when no runtime or a non-client runtime answered, that read decides alone (exit 0 only if disconnected). When it restarted a client runtime, it reads the state again after the restart: exit 0 needs that read to be disconnected and a live non-client runtime answering, and every exit-1 outcome takes its advice from one function (`recoveryAdvice`) given the state read at that point and whether the captured runtime, a different one or nothing answers: a plain restart is advised only for the captured runtime under a disconnected state (or a start when nothing answers), and a connected or unreadable state gets `ocx status` inspection only. Every other outcome exits 1, so a Home-driven leave stops before its own `ocx restart`. No read fences a process that runs after the command has exited. Right after its connection commits, `connectClient` (and each connected `ocx sync` injection) takes over a Codex journal another owner wrote (`adoptJournalForClient` in `src/codex/journal.ts`), keeping the original snapshot; before the commit the journal stays its live writer's, so a runtime starting mid-connect leaves it alone, and after it a supervisor relaunch does not read the standalone owner as a crashed session and restore native Codex. Only a disconnected state or a different committed connection counts as ended for a supervised restart; unreadable or mismatched state keeps the refusal, and the controller rechecks after its supervisor lookup, immediately before recycling, handing the recycle the same supervisor verdict (a service only when viable). Otherwise one budget (`CLIENT_RESTART_BUDGET_MS`, 90 s from acceptance) covers a 20 s listener drain, a replacement `ocx start` that must answer by 70 s (`spawnReplacementStart` with `notReady: "terminate"`, which stops an unready replacement and returns only after its exit is observed), and a rollback that binds the port again with a 10 s wait, builds a fresh tunnel supervisor (a stopped one cannot restart) and republishes the same attestation secret and start time. Before binding, and again under the client lifecycle lock right before it publishes, it checks that the durable connection is still the one it served; a disconnect, a new connection or a held lock closes the new listener unpublished and hands off to the standalone recycle, which starts what the current state calls for. `requestBoundSystemRestart` sends the request to a client target only while `CLIENT_RESTART_REQUEST_MIN_REMAINING_MS` remains, so a rollback always ends before the CLI's replacement observation and its recovery start. Coverage: `tests/clients/client-runtime-restart.test.ts`, `tests/server/restart-replacement-not-ready.test.ts`, `tests/cli/link-apply-restart.test.ts`. +The client listener serves that bound restart (`src/client/machine-listener.ts`, `src/client/runtime-restart.ts`). `/healthz` answers the attestation challenge with the secret the runtime publishes in `runtime-port.json` and advertises `restartCapability: "v1"`; `POST /api/system/restart` admits only the HMAC capability bound to this pid and port, never a dashboard session, admin token or link key. If the relaunch command cannot be identified, `src/client/runtime.ts` omits it from the runtime record; update admission treats the identity as unproven, while client startup and shutdown continue. Under the desktop app or a viable service it answers `409 restart_supervised` naming the supervisor and keeps serving, because handing off to a supervisor means exiting before the replacement is proven; the CLI reports "not performed, runtime kept". That refusal applies only while the durable connection is still the one the runtime serves: after `ocx disconnect` or a newer connect there is nothing to keep, so the restart is accepted and hands off to the standalone recycle (desktop exit 75, a viable service exit 1, otherwise a spawned replacement carrying no supervisor marker). "Ended" needs evidence (`readClientConnectionStateWithEvidence`, one snapshot of config.json): a saved file that validates and records no client, or another connection; missing, unreadable or invalid is invalid. Ownership and relaunch are separate (`classifyClientRestartSupervisor`): manager-launched evidence is the current markers in the environment only (`isMarkedSupervisedServiceChild`: the managed marker or the Windows wrapper marker); a legacy Windows registration recognised only from its parent's command line counts as a bare OCX_SERVICE=1; relaunch needs the service diagnostic to be viable and running. Manager-launched without that is `service-unproven`. `service-unproven` keeps the runtime and is refused at admission even when the connection ended; on this PR's recycle path only a process whose owner is proven to be none detaches, carrying no supervisor marker. A bare OCX_SERVICE=1 is outside that path: with a viable service it is refused, and otherwise it takes origin/dev's replacement restart, which spawns a detached replacement even while a legacy manager still owns the process (#8545). A marker proves only that the process was started with it, so no supervisor marker may reach a process that supervisor did not launch: `src/lib/supervisor-markers.ts` holds the one list, and every detached runtime launch (companions, recycle, replacement, detached and post-update starts) and every shell a runtime hands to a tool clears it. The ended-connection recycle needs positive ownership evidence (`restartOwnerProven`: the desktop app, a manager's marker, or no OCX_SERVICE at all). A bare OCX_SERVICE=1 (a service registered before v2.49, or the proxy of ocx claude, ocx codex or ocx opencode) keeps origin/dev's behaviour unchanged: `ocx restart` refuses when the service is viable and otherwise restarts with a replacement (rollback exits 1 without spawning), its runtime record states no owner, and `ocx disconnect` restarts only a runtime whose record states one it handles (`restartOwner`: desktop, service-manager or none). A bare-flag runtime, one started before this field existed, or an unknown value gets origin/dev's disconnect: no restart request, exit 0. An inconclusive liveness probe or an unreadable record still exits 1 before any request, the one deliberate difference from origin/dev (no runtime is stopped, and the Home's leave stops before its own `ocx restart`). Owner evidence for those runtimes is a separate design (follow-up issue). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live, bound to the runtime generation it observed: it captures that runtime's record (pid, port, per-process attestation secret) and the bound restart attests and derives its capability from the captured record, so another process at a reused pid is refused. It reads the client state once after its observation; when no runtime or a non-client runtime answered, that read decides alone (exit 0 only if disconnected). When it restarted a client runtime, it reads the state again after the restart: exit 0 needs that read to be disconnected and a live non-client runtime answering, and every exit-1 outcome takes its advice from one function (`recoveryAdvice`) given the state read at that point and whether the captured runtime, a different one or nothing answers: a plain restart is advised only for the captured runtime under a disconnected state (or a start when nothing answers), and a connected or unreadable state gets `ocx status` inspection only. Every other outcome exits 1, so a Home-driven leave stops before its own `ocx restart`. No read fences a process that runs after the command has exited. Each `ocx disconnect` attempt holds its own lock D (`withClientDisconnectAttempt` in `src/client/lifecycle-lock.ts`, outermost and taken by nothing else), so a second attempt refuses with `client_disconnect_busy` instead of sharing the first one's receipt while Codex is restored outside L. A disconnect refused before its receipt reaches `desktop_restored` removes that `prepared` receipt (and an otherwise empty `desktop-remote/` folder) when this attempt wrote it, or when an earlier run's receipt finds the Desktop store with no restore started (`absent`, `active` or `legacy_current_connection`); otherwise the receipt is kept, because that run may have stopped inside the Desktop restore. A pending receipt makes client startup refuse with `client_disconnect_pending`; that startup refusal names `ocx disconnect` as the way to finish or roll back (`src/client/connect.ts`, `src/client/state.ts`). The Codex half (`src/codex/inject/disconnect-restore.ts`) accepts a config.toml Codex changed after injection only when the owned-field strip `ocx restore` uses would remove nothing, and then retires the journal instead of replaying its snapshot; the one line it may add is the person's own root `web_search` that OpenCodex set aside while routed (`withReplacedWebSearchRestored`), and only when nothing OpenCodex owns remains after it. While any OpenCodex line remains the journal is kept, and the refusal quotes only an allowlist of keys and fixed table labels. `inspectRemoteDesktopStore` reports which step failed and the path it read, and `src/client/desktop-refusal.ts` turns that into the refusal. Coverage: `tests/clients/client-disconnect-recovery.test.ts`, `tests/clients/client-disconnect-desktop-refusals.test.ts`. Right after its connection commits, `connectClient` (and each connected `ocx sync` injection) takes over a Codex journal another owner wrote (`adoptJournalForClient` in `src/codex/journal.ts`), keeping the original snapshot; before the commit the journal stays its live writer's, so a runtime starting mid-connect leaves it alone, and after it a supervisor relaunch does not read the standalone owner as a crashed session and restore native Codex. Only a disconnected state or a different committed connection counts as ended for a supervised restart; unreadable or mismatched state keeps the refusal, and the controller rechecks after its supervisor lookup, immediately before recycling, handing the recycle the same supervisor verdict (a service only when viable). Otherwise one budget (`CLIENT_RESTART_BUDGET_MS`, 90 s from acceptance) covers a 20 s listener drain, a replacement `ocx start` that must answer by 70 s (`spawnReplacementStart` with `notReady: "terminate"`, which stops an unready replacement and returns only after its exit is observed), and a rollback that binds the port again with a 10 s wait, builds a fresh tunnel supervisor (a stopped one cannot restart) and republishes the same attestation secret and start time. Before binding, and again under the client lifecycle lock right before it publishes, it checks that the durable connection is still the one it served; a disconnect, a new connection or a held lock closes the new listener unpublished and hands off to the standalone recycle, which starts what the current state calls for. `requestBoundSystemRestart` sends the request to a client target only while `CLIENT_RESTART_REQUEST_MIN_REMAINING_MS` remains, so a rollback always ends before the CLI's replacement observation and its recovery start. Coverage: `tests/clients/client-runtime-restart.test.ts`, `tests/server/restart-replacement-not-ready.test.ts`, `tests/cli/link-apply-restart.test.ts`. The Home learns a Child's apply from the Child (`src/link/child-parity.ts`, `src/client/link-parity-reporter.ts`). The linked client runtime POSTs `{linkId, lastApplyAt, runtimeStartedAt, appliedCodexSettingsDigest, sequence}` to `/v1/link-management/child-parity` through the connection-bound relay, shortly after start with backoff and then once a minute; a 404 stops it. On the Home the hub-link listener admits exactly that POST beside the read table, and `src/server/index/link-management.ts` handles it before the passive-read dispatch: the admitting key must own the named link, the body is capped at 1 KiB and 5 s, the parse is strict, and the only effect is an in-memory entry pruned to current links. Entries are ordered by `runtimeStartedAt`, then by `sequence` (process-wide, so a rollback keeps counting), and an older report is ignored, so a late POST from a replaced runtime or an earlier send never moves the row backward. `GET /api/link/status` and `ocx link status --json` carry `lastApplyAt`, `runtimeStartedAt`, `appliedCodexSettingsDigest` and `reportedAt` (epoch ms) on every link row, null without a report; the CLI accepts their absence from an older runtime. Coverage: `tests/server/link-child-parity.test.ts`, `tests/clients/link-parity-reporter.test.ts`. diff --git a/tests/clients/client-connect.test.ts b/tests/clients/client-connect.test.ts index 1ebd7dd6ecf..973b138292e 100644 --- a/tests/clients/client-connect.test.ts +++ b/tests/clients/client-connect.test.ts @@ -1211,11 +1211,13 @@ function runDesktopLifecycleScenario(mode: string) { owner: { kind: "client", apiKeyId: owner.apiKeyId }, })); const actualRestore = journal.restoreJournalState; - journalSpy = spyOn(journal, "restoreJournalState").mockImplementation(() => { + // Forward the arguments: disconnect passes its held config write lock, and dropping it + // makes the real restore see its own lock as another writer. + journalSpy = spyOn(journal, "restoreJournalState").mockImplementation((...args) => { const r = store.readDesktopDisconnectReceipt(); codexSawPrepared = r.kind === "valid" && r.value.phase === "prepared"; codexOutsideL = locks.withClientLifecycleSync(() => true, lockDeps); - return actualRestore(); + return actualRestore(...args); }); } if (mode === "disconnect-foreign") { diff --git a/tests/clients/client-disconnect-desktop-refusals.test.ts b/tests/clients/client-disconnect-desktop-refusals.test.ts new file mode 100644 index 00000000000..140b1fdc2f3 --- /dev/null +++ b/tests/clients/client-disconnect-desktop-refusals.test.ts @@ -0,0 +1,221 @@ +import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { repoRoot } from "../helpers/repo-root"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; + +/** + * O7: every desktop_lifecycle_unsafe refusal from ocx disconnect names the path or condition and + * the fix, and keeps the code at the end. Before, all of these printed a bare + * "could not be verified (conflict|unsafe)". Where a refusal names a fix, the retry case applies + * exactly that fix and proves the next disconnect gets through. Each case runs in its own homes. + */ + +type Case = + | "store-invalid" | "owner-mismatch" | "profile-missing" | "profile-edited" | "library-unreadable" + | "token-backup-unsafe" | "profile-foreign" | "profile-old-key" | "desktop-restore-started"; +type Outcome = { + error: string | null; retryError: string | null; receipt: string; + root: string; library: string; profile: string; backup: string; +}; + +function run(kind: Case, retry = false): Outcome { + const root = mkdtempSync(join(tmpdir(), "ocx-disconnect-desktop-refusal-")); + try { + const script = ` + const fs = require("node:fs"); + const path = require("node:path"); + const crypto = require("node:crypto"); + const configApi = require("./src/config"); + const connectApi = require("./src/client/connect"); + const store = require("./src/claude/desktop-remote-store"); + const locks = require("./src/client/lifecycle-lock"); + const { DEFAULT_CATALOG_PATH } = require("./src/codex/paths"); + const kind = ${JSON.stringify(kind)}; + const retry = ${JSON.stringify(retry)}; + const home = process.env.OPENCODEX_HOME, desktop = process.env.OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR; + for (const dir of [home, desktop, process.env.CODEX_HOME]) fs.mkdirSync(dir, { recursive: true }); + const hash = value => crypto.createHash("sha256").update(value).digest("hex"); + const key = "ocx_data_" + "1".repeat(40); + const owner = { serverUrl: "https://hub.example.test", apiKeyId: "fixture-key", connectedAt: "2026-09-06T00:00:00.000Z" }; + const catalog = '{"models":[]}'; + const withCodex = kind === "desktop-restore-started"; + configApi.saveConfig({ port: 10100, providers: {}, defaultProvider: "openai", runtimeRole: "client", client: { + ...owner, managementUrl: owner.serverUrl, managementTransport: "direct", selectedClients: withCodex ? ["claude", "codex"] : ["claude"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", tokenFingerprint: hash(key), protocolVersion: 1, + catalogFingerprint: crypto.createHash("sha256").update(catalog).digest("base64url"), + } }); + fs.writeFileSync(path.join(home, "service-api-token"), key, { mode: 0o600 }); + fs.writeFileSync(DEFAULT_CATALOG_PATH, catalog); + const profile = path.join(desktop, "fixture.json"); + const meta = path.join(desktop, "_meta.json"); + const remote = path.join(home, "desktop-remote"); + const lockDeps = { lockPath: path.join(home, "lifecycle.sqlite") }; + const writeProfile = (name, value) => { + fs.writeFileSync(meta, JSON.stringify({ appliedId: "fixture", entries: [{ id: "fixture", name }] })); + fs.writeFileSync(profile, JSON.stringify(value)); + }; + const removeProfile = () => { fs.unlinkSync(profile); fs.writeFileSync(meta, JSON.stringify({ entries: [] })); }; + let fix = null; + if (kind === "library-unreadable") { + fs.writeFileSync(meta, "{ not json"); + } else if (kind === "store-invalid") { + fs.mkdirSync(remote, { recursive: true, mode: 0o700 }); + fs.writeFileSync(path.join(remote, "state.json"), "{ not json", { mode: 0o600 }); + // Owner-only and still corrupt: the fix is to set the folder aside and keep it. + fix = () => fs.renameSync(remote, remote + ".aside"); + } else if (kind === "token-backup-unsafe") { + fs.mkdirSync(path.join(home, "service-api-token.prev")); + } else if (kind === "profile-foreign") { + // One profile reaches this Home with the current key but is not named opencodex. + writeProfile("mine", { inferenceProvider: "gateway", inferenceCredentialKind: "static", + inferenceGatewayBaseUrl: owner.serverUrl, inferenceGatewayApiKey: key, modelDiscoveryEnabled: true }); + fix = removeProfile; + } else if (kind === "profile-old-key") { + // The only profile for this Home is named opencodex but carries a key this connection no longer has. + writeProfile("opencodex", { inferenceProvider: "gateway", inferenceCredentialKind: "static", + inferenceGatewayBaseUrl: owner.serverUrl, inferenceGatewayApiKey: "ocx_data_" + "9".repeat(40), modelDiscoveryEnabled: true }); + fix = removeProfile; + } else { + writeProfile("opencodex", { inferenceProvider: "gateway", inferenceCredentialKind: "static", + inferenceGatewayBaseUrl: "http://127.0.0.1:10100", inferenceGatewayApiKey: "fixture-local-key", + modelDiscoveryEnabled: false, inferenceModels: [] }); + const applied = locks.withClientLifecycleSync(held => store.applyRemoteDesktopStore(held, { + owner, expectedTokenFingerprint: hash(key), baseUrl: owner.serverUrl, apiKey: key, mode: "static", + models: [{ name: "claude-opus-4-8-20260101", labelOverride: "Fixture", anthropicFamilyTier: "opus" }], + }), lockDeps); + if (!applied.ok) throw new Error("fixture_desktop_apply_failed " + JSON.stringify(applied)); + if (kind === "profile-edited") { + const value = JSON.parse(fs.readFileSync(profile, "utf8")); + value.inferenceModels = []; + fs.writeFileSync(profile, JSON.stringify(value)); + } + if (kind === "profile-missing") { + fs.writeFileSync(meta, JSON.stringify({ entries: [{ id: "fixture", name: "renamed by hand" }] })); + fix = () => fs.writeFileSync(meta, JSON.stringify({ appliedId: "fixture", entries: [{ id: "fixture", name: "opencodex" }] })); + } + if (kind === "owner-mismatch") { + // The store was written for an earlier connection to the same Home. + const config = configApi.loadConfig(); + config.client.connectedAt = "2026-10-01T00:00:00.000Z"; + configApi.saveConfig(config); + // The fix the refusal names: remove that connection's profile, then move the folder aside. + fix = () => { removeProfile(); fs.renameSync(remote, remote + ".aside"); }; + } + if (kind === "desktop-restore-started") { + // An earlier run restored Claude Desktop and stopped before recording desktop_restored; + // this run's Codex restore is refused (an OpenCodex line Codex kept without its marker). + const original = 'model_provider = "openai"\\n'; + const url = "http://127.0.0.1:10100/v1"; + const injected = original + '# Auto-injected by opencodex (undo: ocx restore)\\nopenai_base_url = "' + url + '"\\n'; + fs.writeFileSync(path.join(process.env.CODEX_HOME, "config.toml"), original + 'openai_base_url = "' + url + '"\\n[notice]\\nhide = true\\n'); + fs.writeFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), JSON.stringify({ + version: 1, originalConfig: Buffer.from(original).toString("base64"), originalProfile: null, + injectedConfigHash: hash(injected), injectedProfileHash: null, injectedOpenaiBaseUrl: url, + owner: { kind: "client", apiKeyId: owner.apiKeyId }, pid: 999999, timestamp: "2026-09-06T00:00:00.000Z" })); + locks.withClientLifecycleSync(held => { + store.writeDesktopDisconnectReceipt(held, null, { version: 1, owner, tokenFingerprint: hash(key), keepCatalog: false, phase: "prepared" }); + const restored = store.restoreRemoteDesktopStore(held, { owner, knownTokenFingerprints: [hash(key)] }); + if (!restored.ok) throw new Error("fixture_restore_failed"); + }, lockDeps); + } + } + (async () => { + const attempt = async () => { + try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); return null; } + catch (cause) { return cause instanceof Error ? cause.message : String(cause); } + }; + const error = await attempt(); + let retryError = null; + if (retry) { if (!fix) throw new Error("no fix for " + kind); fix(); retryError = await attempt(); } + const receipt = store.readDesktopDisconnectReceipt(); + const real = p => fs.realpathSync.native(p); + console.log(JSON.stringify({ + error, retryError, receipt: receipt.kind === "valid" ? receipt.value.phase : receipt.kind, + root: real(home) + "/desktop-remote", library: real(desktop), profile: real(desktop) + "/fixture.json", + backup: path.join(home, "service-api-token.prev"), + })); + })(); + `; + const child = spawnSync(process.execPath, ["-e", script], { + cwd: repoRoot(), encoding: "utf8", timeout: SPAWN_BUDGET_MS - 1_000, + env: { ...process.env, OPENCODEX_HOME: join(root, "home"), CODEX_HOME: join(root, "codex"), + OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR: join(root, "desktop") }, + }); + const line = child.stdout.trim().split("\n").at(-1) ?? ""; + if (child.status !== 0 || !line.startsWith("{")) throw new Error("scenario failed: " + child.status + "\n" + child.stdout + "\n" + child.stderr); + return JSON.parse(line) as Outcome; + } finally { + removeTreeWithRetry(root); + } +} + +function expectNamedRefusal(out: Outcome, ...parts: string[]): void { + expect(out.error).not.toBeNull(); + for (const part of parts) expect(out.error).toContain(part); + expect(out.error).toContain("then run ocx disconnect again"); + expect(out.error!.endsWith("(code desktop_lifecycle_unsafe)")).toBe(true); +} + +describe("desktop_lifecycle_unsafe names its cause and fix (O7)", () => { + test("a corrupt owner-only store file; setting the folder aside (kept) clears it", () => { + const out = run("store-invalid", true); + expectNamedRefusal(out, out.root, "cannot read safely", "keep it"); + expect(out.retryError).toBeNull(); + }, SPAWN_BUDGET_MS); + + test("a store written for another connection; removing that profile and moving the folder aside clears it", () => { + const out = run("owner-mismatch", true); + expectNamedRefusal(out, out.root + "/state.json", "a different connection", "remove that connection's OpenCodex profile"); + expect(out.retryError).toBeNull(); + }, SPAWN_BUDGET_MS); + + test("the OpenCodex profile renamed in Claude Desktop; renaming it back clears it", () => { + const out = run("profile-missing", true); + expectNamedRefusal(out, out.library, "removed or renamed"); + expect(out.retryError).toBeNull(); + }, SPAWN_BUDGET_MS); + + test("the OpenCodex profile edited outside OpenCodex: no fix that would overwrite the edits silently", () => { + const out = run("profile-edited"); + expectNamedRefusal(out, out.profile, "edited outside OpenCodex", "copy anything you want to keep"); + expect(out.error).not.toContain("aside"); + }, SPAWN_BUDGET_MS); + + test("an unreadable Claude Desktop profile list", () => { + const out = run("library-unreadable"); + expectNamedRefusal(out, out.library, "profile list"); + }, SPAWN_BUDGET_MS); + + test("an unsafe service token backup is named as such, not as the profile list", () => { + const out = run("token-backup-unsafe"); + expectNamedRefusal(out, out.backup, "rotation backup"); + expect(out.error).not.toContain("profile list"); + }, SPAWN_BUDGET_MS); + + test("a single profile for this Home under another name; removing it clears it", () => { + const out = run("profile-foreign", true); + expectNamedRefusal(out, out.library, "cannot identify it as OpenCodex's"); + expect(out.error).not.toContain("extra"); + expect(out.retryError).toBeNull(); + }, SPAWN_BUDGET_MS); + + test("the only profile for this Home uses an older key; removing it, as named, clears it", () => { + const out = run("profile-old-key", true); + expectNamedRefusal(out, out.library, "older key", "remove that one too"); + expect(out.retryError).toBeNull(); + }, SPAWN_BUDGET_MS); +}); + +describe("a disconnect is rolled back only before Claude Desktop is touched (O8)", () => { + test("a receipt whose Desktop restore already ran is kept when the Codex restore is refused", () => { + const out = run("desktop-restore-started"); + expect(out.error).toContain("Codex journal restore was partial"); + expect(out.error).toContain("Claude Desktop restore had already started"); + expect(out.error).not.toContain("was rolled back"); + expect(out.receipt).toBe("prepared"); + }, SPAWN_BUDGET_MS); +}); diff --git a/tests/clients/client-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts new file mode 100644 index 00000000000..32c0720174a --- /dev/null +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -0,0 +1,296 @@ +import { describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { repoRoot } from "../helpers/repo-root"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; +import { remainingOpencodexConfigKeys } from "../../src/codex/inject/remove"; + +/** + * O8 and O9: a Child whose ocx disconnect was refused by the Codex journal restore. + * + * Seen on 2.100.2 under the desktop app: the refusal left desktop-remote/disconnect.json at + * phase "prepared", the client runtime then refused to start until the folder was moved + * aside, and the refusal itself came from Codex appending lines to config.toml after the + * injection. Each scenario runs in its own process because the Codex paths are fixed at import. + */ + +const ORIGINAL = 'model_provider = "openai"\n'; +const BASE_URL = "http://127.0.0.1:10100/v1"; +const INJECTED = ORIGINAL + "# Auto-injected by opencodex (undo: ocx restore)\nopenai_base_url = \"" + BASE_URL + "\"\n"; +// What Codex wrote on the field machine after injection: a trusted project and notice flags. +const CODEX_LINES = '\n[notice]\nhide_full_access_warning = true\n\n[projects."/srv/work/project"]\ntrust_level = "trusted"\n'; +const ROUTING_GONE = ORIGINAL + CODEX_LINES; +// Codex dropped our marker comment but kept the value: still OpenCodex routing. +const ROUTING_KEPT = ORIGINAL + "openai_base_url = \"" + BASE_URL + "\"\n" + CODEX_LINES; + +type Scenario = { config: string; stuckReceipt?: boolean; overlapping?: boolean; retryConfig?: string; replacedWebSearch?: string }; +type Outcome = { + error: string | null; stateKind: string; gateBefore: string; gateAfter: string; + receipt: string; desktopRoot: boolean; journal: boolean; config: string; startupRefusal: string; + overlappingError: string | null; retryError: string | null; +}; + +const sha = (value: string) => createHash("sha256").update(value).digest("hex"); + +function run(scenario: Scenario): Outcome { + const opencodexHome = mkdtempSync(join(tmpdir(), "ocx-disconnect-recovery-home-")); + const codexHome = mkdtempSync(join(tmpdir(), "ocx-disconnect-recovery-codex-")); + try { + const token = "ocx_data_" + "e".repeat(40); + const catalog = '{"models":[]}'; + writeFileSync(join(opencodexHome, "config.json"), JSON.stringify({ + port: 10100, providers: {}, defaultProvider: "openai", runtimeRole: "client", + client: { + serverUrl: "https://hub.example.test", managementUrl: "https://hub.example.test", managementTransport: "direct", + selectedClients: ["codex"], tokenEnv: "OPENCODEX_API_AUTH_TOKEN", apiKeyId: "client-key-1", + tokenFingerprint: sha(token), protocolVersion: 1, connectedAt: "2026-08-28T00:00:00.000Z", + catalogFingerprint: createHash("sha256").update(catalog).digest("base64url"), catalogSyncedAt: "2026-08-28T00:00:00.000Z", + }, + })); + writeFileSync(join(opencodexHome, "service-api-token"), token + "\n", { mode: 0o600 }); + writeFileSync(join(codexHome, "opencodex-catalog.json"), catalog); + writeFileSync(join(codexHome, "config.toml"), scenario.config); + writeFileSync(join(codexHome, "opencodex-journal.json"), JSON.stringify({ + version: 1, originalConfig: Buffer.from(ORIGINAL).toString("base64"), originalProfile: null, + injectedConfigHash: sha(INJECTED), injectedProfileHash: null, injectedOpenaiBaseUrl: BASE_URL, + injectedRealtimeWsBaseUrl: null, injectedCatalogPath: null, + ...(scenario.replacedWebSearch ? { replacedRootWebSearch: scenario.replacedWebSearch } : {}), + owner: { kind: "client", apiKeyId: "client-key-1" }, pid: 999_999, timestamp: "2026-08-28T00:00:00.000Z", + })); + const script = ` + const fs = require("node:fs"); + const path = require("node:path"); + const connectApi = require("./src/client/connect"); + const stateApi = require("./src/client/state"); + const store = require("./src/claude/desktop-remote-store"); + const locks = require("./src/client/lifecycle-lock"); + const lockDeps = { lockPath: path.join(process.env.OPENCODEX_HOME, "lifecycle.sqlite") }; + (async () => { + if (${JSON.stringify(scenario.stuckReceipt === true)}) { + // The receipt 2.100.2 left behind after its refused disconnect. + const state = stateApi.readClientConnectionState(); + const owner = { serverUrl: state.value.serverUrl, apiKeyId: state.value.apiKeyId, connectedAt: state.value.connectedAt }; + locks.withClientLifecycleSync(held => store.writeDesktopDisconnectReceipt(held, null, + { version: 1, owner, tokenFingerprint: state.value.tokenFingerprint, keepCatalog: false, phase: "prepared" }), lockDeps); + } + const gateBefore = stateApi.inspectClientRotationRecoveryGate(undefined, lockDeps).kind; + const startupRefusal = stateApi.clientStartupRefusalMessage("client_disconnect_pending"); + // A second ocx disconnect started while the first is restoring Codex. + let overlapping = null; + if (${JSON.stringify(scenario.overlapping === true)}) { + const journal = require("./src/codex/journal"); + const actual = journal.restoreJournalState; + const { spyOn } = require("bun:test"); + spyOn(journal, "restoreJournalState").mockImplementation((...args) => { + // A second process, as a second ocx disconnect would be, while this one holds its attempt lock. + if (!overlapping) { + const child = require("node:child_process").spawnSync(process.execPath, ["-e", 'require("./src/client/connect").disconnectClient({}, { lifecycleLockDeps: ' + JSON.stringify(lockDeps) + ' }).then(() => console.log("OK"), e => console.log(String(e && e.message)))'], { encoding: "utf8", env: process.env }); + const said = child.stdout.trim().split(String.fromCharCode(10)).at(-1) ?? ""; + overlapping = Promise.resolve(said === "OK" ? null : said || child.stderr); + } + return actual(...args); + }); + } + let error = null; + try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); } + catch (cause) { error = cause instanceof Error ? cause.message : String(cause); } + const overlappingError = overlapping ? await overlapping : null; + // The edit the refusal asked for, then the retry it promised would get through. + let retryError = null; + const retryConfig = ${JSON.stringify(scenario.retryConfig ?? null)}; + if (retryConfig !== null) { + fs.writeFileSync(path.join(process.env.CODEX_HOME, "config.toml"), retryConfig); + try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); } + catch (cause) { retryError = cause instanceof Error ? cause.message : String(cause); } + } + const receipt = store.readDesktopDisconnectReceipt(); + console.log(JSON.stringify({ + error, + overlappingError, + retryError, + stateKind: stateApi.readClientConnectionState().kind, + gateBefore, + startupRefusal, + gateAfter: stateApi.inspectClientRotationRecoveryGate(undefined, lockDeps).kind, + receipt: receipt.kind === "valid" ? receipt.value.phase : receipt.kind, + desktopRoot: fs.existsSync(path.join(process.env.OPENCODEX_HOME, "desktop-remote")), + journal: fs.existsSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json")), + config: fs.readFileSync(path.join(process.env.CODEX_HOME, "config.toml"), "utf8"), + })); + })(); + `; + const child = spawnSync(process.execPath, ["-e", script], { + cwd: repoRoot(), encoding: "utf8", timeout: SPAWN_BUDGET_MS - 1_000, + env: { ...process.env, OPENCODEX_HOME: opencodexHome, CODEX_HOME: codexHome, + OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR: join(opencodexHome, "desktop") }, + }); + const line = child.stdout.trim().split("\n").at(-1) ?? ""; + if (child.status !== 0 || !line.startsWith("{")) throw new Error("scenario failed: " + child.status + "\n" + child.stdout + "\n" + child.stderr); + return JSON.parse(line) as Outcome; + } finally { + removeTreeWithRetry(opencodexHome); + removeTreeWithRetry(codexHome); + } +} + +describe("disconnect after Codex edited config.toml (O9)", () => { + // Byte-for-byte boundaries of putting the person's own web_search line back. + test.each([ + ["a BOM and a final line without a newline", "\uFEFFmodel_provider = \"openai\"", 'web_search = "live"', + "\uFEFFmodel_provider = \"openai\"\nweb_search = \"live\"\n"], + ["a file with no root lines", CODEX_LINES.slice(1), 'web_search = "live"', + 'web_search = "live"\n' + CODEX_LINES.slice(1)], + ["a root array before the insertion point", 'model_provider = "openai"\nnotify = [\n "a",\n "b",\n]\n' + CODEX_LINES, 'web_search = "live"', + 'model_provider = "openai"\nnotify = [\n "a",\n "b",\n]\nweb_search = "live"\n' + CODEX_LINES], + ["a journaled line with its own spacing and comment", ROUTING_GONE, 'web_search = "live" # mine', + 'model_provider = "openai"\nweb_search = "live" # mine\n' + CODEX_LINES], + ])("putting the web_search line back keeps every other byte: %s", (_name, config, replaced, expected) => { + const out = run({ config, replacedWebSearch: replaced }); + expect(out.error).toBeNull(); + expect(out.journal).toBe(false); + expect(out.config).toBe(expected); + }, SPAWN_BUDGET_MS); + + test("with another OpenCodex setting still present, nothing is put back and the journal is kept", () => { + const out = run({ config: ROUTING_KEPT, replacedWebSearch: 'web_search = "live"' }); + expect(out.error).toContain("openai_base_url"); + expect(out.config).toBe(ROUTING_KEPT); + expect(out.journal).toBe(true); + }, SPAWN_BUDGET_MS); + + test("putting the web_search line back keeps every other byte of a mixed CRLF/LF file", () => { + const mixed = 'model_provider = "openai"\r\n[notice]\nhide_full_access_warning = true\r\n[projects."/srv/work/project"]\ntrust_level = "trusted"\n'; + const out = run({ config: mixed, replacedWebSearch: 'web_search = "live"' }); + expect(out.error).toBeNull(); + expect(out.config).toBe('model_provider = "openai"\r\nweb_search = "live"\r\n[notice]\nhide_full_access_warning = true\r\n[projects."/srv/work/project"]\ntrust_level = "trusted"\n'); + }, SPAWN_BUDGET_MS); + + test("putting the web_search line back lands after a root multiline value, not inside it", () => { + const config = 'model_provider = "openai"\ndeveloper_instructions = """\nfirst\n\nsecond\n"""\n' + CODEX_LINES; + const out = run({ config, replacedWebSearch: 'web_search = "live"' }); + expect(out.error).toBeNull(); + expect(out.config).toBe('model_provider = "openai"\ndeveloper_instructions = """\nfirst\n\nsecond\n"""\nweb_search = "live"\n' + CODEX_LINES); + }, SPAWN_BUDGET_MS); + + test("a web_search line OpenCodex set aside is put back, alone, and the disconnect completes", () => { + // While routed, OpenCodex removed the person's own root web_search line and journaled it. + const out = run({ config: ROUTING_GONE, replacedWebSearch: 'web_search = "live"' }); + expect(out.error).toBeNull(); + expect(out.stateKind).toBe("disconnected"); + expect(out.journal).toBe(false); + expect(out.config).toBe('model_provider = "openai"\nweb_search = "live"\n' + CODEX_LINES); + }, SPAWN_BUDGET_MS); + + test("no OpenCodex line left: disconnect completes, keeps every Codex line, retires the journal", () => { + const out = run({ config: ROUTING_GONE }); + expect(out.error).toBeNull(); + expect(out.stateKind).toBe("disconnected"); + expect(out.config).toBe(ROUTING_GONE); + expect(out.journal).toBe(false); + }, SPAWN_BUDGET_MS); + + test("an OpenCodex line still present: refused, journal kept, config untouched, the line and its fix named", () => { + const out = run({ config: ROUTING_KEPT }); + expect(out.error).toContain("openai_base_url"); + expect(out.error).toContain("config.toml"); + expect(out.error).toContain("then run ocx disconnect again"); + expect(out.stateKind).toBe("connected"); + expect(out.config).toBe(ROUTING_KEPT); + expect(out.journal).toBe(true); + }, SPAWN_BUDGET_MS); + + test("the named edit (remove the OpenCodex key) lets the retry finish", () => { + const out = run({ config: ROUTING_KEPT, retryConfig: ROUTING_GONE }); + expect(out.error).toContain("openai_base_url"); + expect(out.retryError).toBeNull(); + expect(out.stateKind).toBe("disconnected"); + }, SPAWN_BUDGET_MS); + + test("an OpenCodex table with a multiline value is named as a table, and removing it whole lets the retry finish", () => { + const table = '[model_providers.opencodex]\nname = "OpenCodex"\nexperimental_bearer_token = """\nvalue-line\n"""\n'; + const out = run({ config: ROUTING_GONE + table, retryConfig: ROUTING_GONE }); + expect(out.error).toContain("[model_providers.opencodex]"); + expect(out.error).toContain("whole value"); + expect(out.error).toContain("every line under it"); + expect(out.retryError).toBeNull(); + expect(out.stateKind).toBe("disconnected"); + }, SPAWN_BUDGET_MS); +}); + +describe("a refused disconnect does not block the next start (O8)", () => { + test("the prepared receipt and its folder are rolled back, so startup's recovery gate is clean", () => { + const out = run({ config: ROUTING_KEPT }); + expect(out.error).not.toBeNull(); + expect(out.error).toContain("still connected"); + expect(out.receipt).toBe("absent"); + expect(out.desktopRoot).toBe(false); + expect(out.gateAfter).toBe("clean"); + }, SPAWN_BUDGET_MS); + + test("upgrade path: a receipt stuck at prepared by 2.100.2 finishes on the next disconnect", () => { + const out = run({ config: ROUTING_GONE, stuckReceipt: true }); + expect(out.gateBefore).toBe("recovery-required"); + // What the desktop supervisor logged five times on the field machine, now with its fix. + expect(out.startupRefusal).toContain("stopped part way at phase prepared"); + expect(out.startupRefusal).toContain("Run ocx disconnect again to finish it or roll it back"); + expect(out.startupRefusal.endsWith("(code client_disconnect_pending)")).toBe(true); + expect(out.error).toBeNull(); + expect(out.stateKind).toBe("disconnected"); + expect(out.receipt).toBe("complete"); + expect(out.config).toBe(ROUTING_GONE); + }, SPAWN_BUDGET_MS); + + test("upgrade path: a stuck receipt whose disconnect is still refused rolls back instead of staying stuck", () => { + const out = run({ config: ROUTING_KEPT, stuckReceipt: true }); + expect(out.gateBefore).toBe("recovery-required"); + expect(out.error).not.toBeNull(); + expect(out.stateKind).toBe("connected"); + expect(out.receipt).toBe("absent"); + expect(out.desktopRoot).toBe(false); + expect(out.gateAfter).toBe("clean"); + expect(out.journal).toBe(true); + }, SPAWN_BUDGET_MS); + + test("a second disconnect started during the first one's Codex restore is refused and cannot clear its receipt", () => { + const out = run({ config: ROUTING_KEPT, overlapping: true }); + expect(out.overlappingError).toContain("another ocx disconnect is running"); + // The first attempt still owns its receipt, so its own rollback is the one that removes it. + expect(out.error).toContain("was rolled back"); + expect(out.receipt).toBe("absent"); + expect(out.gateAfter).toBe("clean"); + }, SPAWN_BUDGET_MS); +}); + +describe("the O9 refusal quotes key names only", () => { + test("a multiline value inside an OpenCodex table never reaches the message", () => { + const keys = remainingOpencodexConfigKeys([ + 'model_provider = "openai"', + "[model_providers.opencodex]", + 'base_url = "http://127.0.0.1:10100/v1"', + 'experimental_bearer_token = """', + "continuation-line-value-must-not-leak", + '"""', + "", + ].join("\n")); + expect(keys.length).toBeGreaterThan(0); + expect(keys.join(" ")).not.toContain("must-not-leak"); + expect(keys.join(" ")).not.toContain('"""'); + }); + + test("a quoted table name or an unknown key never reaches the message", () => { + const keys = remainingOpencodexConfigKeys([ + 'model_provider = "openai"', + '[model_providers.opencodex."QUOTED_NAME_MUST_NOT_LEAK"]', + 'base_url = "http://127.0.0.1:10100/v1"', + "[model_providers.opencodex]", + "BARE_KEY_MUST_NOT_LEAK = 1", + "", + ].join("\n")).join(" "); + expect(keys).not.toContain("MUST_NOT_LEAK"); + expect(keys).toContain("[model_providers.opencodex]"); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index f092ad216e2..156f7fe2819 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1599,7 +1599,7 @@ "grok-47-fast-model-wire.test.ts": "providers/xai", "client-link-connect.test.ts": "clients", "client-link-relay.test.ts": "clients", "client-link-runtime.test.ts": "clients", "client-link-stop.test.ts": "clients", "link-routes.test.ts": "clients", "client-link-state.test.ts": "clients", - "client-link-status.test.ts": "clients", "client-link-teardown.test.ts": "clients", + "client-link-status.test.ts": "clients", "client-link-teardown.test.ts": "clients", "client-disconnect-recovery.test.ts": "clients", "client-disconnect-desktop-refusals.test.ts": "clients", "client-link-tunnel.test.ts": "clients", "link-ports.test.ts": "clients", "injection-link-websocket.test.ts": "codex-integration", "link-supervisor.test.ts": "clients", "link-status-projection.test.ts": "clients", "link-admission-wait.test.ts": "clients",