From 3b2017df9756689ac81aea0d052c9a8f64674341 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 01/15] fix(link): ocx restart recycles a client runtime whose connection was removed, under any owner After ocx disconnect (or a newer connect) a desktop- or service-owned client runtime refused ocx restart with restart_supervised and kept answering role client, so a Home-driven leave over SSH could not finish. A changed connection has nothing to keep: accept the restart and hand off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). ocx disconnect now restarts a still-live client runtime itself and exits 1 naming ocx restart when that does not complete. --- scripts/test-layout/layout.json | 2 +- src/cli/disconnect-runtime.ts | 34 +++++++++++++ src/cli/dispatch.ts | 9 +++- src/client/runtime-restart.ts | 16 ++++++ src/client/runtime.ts | 2 + structure/runtime.md | 2 +- tests/cli/disconnect-runtime.test.ts | 51 ++++++++++++++++++++ tests/clients/client-runtime-restart.test.ts | 24 +++++++++ tests/fixtures/test-layout-expected.json | 2 +- 9 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 src/cli/disconnect-runtime.ts create mode 100644 tests/cli/disconnect-runtime.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index de0c4fa181a..9642bce4793 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1987,7 +1987,7 @@ "zz-ci-storage-policy-isolation.test.ts": "ci-workflows", "zz-pr-coderabbit-readiness-revalidation.test.ts": "ci-workflows", "injection-link-websocket.test.ts": "codex-integration", - "cli-link.test.ts": "cli", "cli-link-enrollment.test.ts": "cli", "link-apply-restart.test.ts": "cli", "link-child-parity.test.ts": "server", "restart-replacement-not-ready.test.ts": "server", + "cli-link.test.ts": "cli", "cli-link-enrollment.test.ts": "cli", "link-apply-restart.test.ts": "cli", "disconnect-runtime.test.ts": "cli", "link-child-parity.test.ts": "server", "restart-replacement-not-ready.test.ts": "server", "link-management-routes.test.ts": "server", "link-enrollment.test.ts": "server", "web-search-run-turn-loop.test.ts": "web-search", "responses-run-turn-web-search.test.ts": "responses", diff --git a/src/cli/disconnect-runtime.ts b/src/cli/disconnect-runtime.ts new file mode 100644 index 00000000000..e6bc4013b85 --- /dev/null +++ b/src/cli/disconnect-runtime.ts @@ -0,0 +1,34 @@ +/** + * `ocx disconnect` ends the connection in the durable state; the runtime that served it keeps + * running until something restarts it. Left alone it still answered as a client, so the machine + * was neither a working Child nor standalone, and a Home could not link it again. Disconnect + * therefore restarts a live client runtime itself (the same restart as `ocx restart`, which a + * disconnected client runtime hands to whichever owner holds it), and fails with the remaining + * step when that restart does not complete. + */ +export interface DisconnectRuntimeDeps { + findLive: () => Promise<{ pid: number | null; role?: string } | null>; + /** `ocx restart`; resolves true once a replacement is verified. */ + restart: () => Promise; + json: boolean; + info?: (line: string) => void; + error?: (line: string) => void; +} + +export async function restartRuntimeAfterDisconnect(deps: DisconnectRuntimeDeps): Promise { + const info = deps.info ?? (line => console.log(line)); + const error = deps.error ?? (line => console.error(line)); + const live = await deps.findLive(); + if (live?.role !== "client") return 0; + // A --json caller reads one document from stdout; restart progress goes to stderr. + const originalLog = console.log; + if (deps.json) console.log = (...args: unknown[]) => console.error(...args); + let ok = false; + try { ok = await deps.restart(); } finally { console.log = originalLog; } + if (ok) { + (deps.json ? error : info)("The client runtime was restarted; this machine now runs standalone OpenCodex."); + return 0; + } + error(`Disconnected, but the client runtime${live.pid ? ` (pid ${live.pid})` : ""} still serves the old connection. Run on this machine: ocx restart`); + return 1; +} diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 6cbe047f2a1..e992dac9186 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -423,7 +423,14 @@ const commandRunners: Record = { }, disconnect: async deps => { const { handleDisconnectCommand } = await import("./connect"); - return await handleDisconnectCommand(deps.args.slice(1)); + const code = await handleDisconnectCommand(deps.args.slice(1)); + if (code !== 0) return code; + const { restartRuntimeAfterDisconnect } = await import("./disconnect-runtime"); + return await restartRuntimeAfterDisconnect({ + findLive: () => deps.findLiveProxy(), + restart: () => deps.handleProxyRestart(deps.handleRestartStartWhenStopped), + json: deps.args.includes("--json"), + }); }, catalog: async deps => { const { handleCatalogCommand } = await import("./catalog"); diff --git a/src/client/runtime-restart.ts b/src/client/runtime-restart.ts index 899fb165112..e7848854d40 100644 --- a/src/client/runtime-restart.ts +++ b/src/client/runtime-restart.ts @@ -8,6 +8,11 @@ * - Under the desktop app or a viable service supervisor the restart is refused (409 * `restart_supervised`) and the runtime keeps serving. The supervisor owns this process and its * port; handing off to it means exiting before anything proves the replacement can start. + * - Unless the connection it serves is no longer the durable one (`ocx disconnect`, or a newer + * connect). Then there is nothing to keep, and under every owner the restart hands off to the + * standalone recycle, which exits the way that owner relaunches (desktop 75, service 1) or spawns + * its own replacement. Without this a Home-driven leave over SSH could not finish under the + * desktop app or a service: `ocx restart` refused and the stale client runtime kept serving. * - Otherwise the listener drains, a replacement `ocx start` is spawned and must answer within the * budget. Success exits this process. A replacement that exited, or never answered and was then * stopped, leaves this process to bind the port again, start a fresh tunnel supervisor and @@ -38,6 +43,10 @@ export interface ClientRestartIo { stopping?: () => boolean; /** Which supervisor owns this process, or null when it starts its own replacement. */ supervisor(): Promise; + /** The durable connection is no longer the one this runtime serves. */ + connectionChanged?: () => boolean; + /** Hand off to what the durable state now calls for; exits the process for its owner to relaunch. */ + recycle?: () => Promise; /** Stop the tunnel supervisor and the listener, giving in-flight requests up to `drainMs`. */ stop(drainMs: number): Promise; /** Start the replacement; resolves once it answers, rejects when none survives. */ @@ -97,6 +106,13 @@ export function createClientRestartController(io: ClientRestartIo): ClientRestar hasHandoff: () => flight !== null, async request() { if (io.stopping?.()) return { kind: "stopping" }; + if (io.recycle && io.connectionChanged?.()) { + if (flight) return { kind: "accepted", alreadyRestarting: true }; + const recycle = io.recycle; + // Let the 202 reach the CLI before the listener closes. + flight = sleep(200).then(recycle).finally(() => { flight = null; }); + return { kind: "accepted", alreadyRestarting: false }; + } const supervisor = await io.supervisor(); if (io.stopping?.()) return { kind: "stopping" }; if (supervisor) return { kind: "supervised", supervisor }; diff --git a/src/client/runtime.ts b/src/client/runtime.ts index 75e3462f147..dd5c44512c2 100644 --- a/src/client/runtime.ts +++ b/src/client/runtime.ts @@ -433,6 +433,8 @@ export async function startClientRuntime( const restartController: ClientRestartController = createClientRestartController({ stopping: () => shuttingDown, supervisor: io.restartSupervisor ?? clientRestartSupervisor, + connectionChanged: () => clientRestartRollbackAction(state.value, readClientConnectionState()) === "recycle", + recycle: () => recycleStandalone(state.value.tokenFingerprint), stop: stopServing, spawnReplacement: async deadlineAt => { const { replacementStartEnvironment } = await import("../server/management/system-restart"); diff --git a/structure/runtime.md b/structure/runtime.md index 55a1d0f1ec7..a7ee4934c8c 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -180,7 +180,7 @@ until shutdown. Standalone shutdown restores native Codex; connected hub clients uninstall still restore. Native client signal cleanup follows the [served-connection and supervisor boundaries](remote-link.md); client update stops use full parent restoration because client start cannot discharge standalone handover receipts. `src/service/cli.ts` removes the service token on uninstall only when persisted client state is disconnected and no pending connect marker owns the newly issued key. `src/client/connect.ts` publishes that fingerprint marker before the key, then clears it with the connection commit or rollback under the client lifecycle and config mutation locks. Connected, invalid, or mismatched client state retains an existing token. A valid pending marker retains only its matching fingerprint; an older marker does not own a replacement service key. An absent token is reported as absent; unsafe, malformed, or unreadable markers and lock, state-read, or deletion failures leave cleanup unverified. `src/cli/tray-proxy.ts` coordinates `ocx restart` and tray restart. A second start requires proof the prior attempt never launched or its child exited, plus confirmed absence under a fresh bounded discovery window; a missing health response alone is insufficient. After an accepted restart, polling continues through uncertain observations until its deadline and requires a new runtime PID on the same port. If the old proxy then disappears, explicit recovery starts it regardless of the Codex autostart preference; a skipped recovery fails rather than reporting success. The Windows tray startup-health probe in `src/tray/windows-tray.ps1` requests termination at its 30-second deadline without waiting on the UI thread. It retains the process through later ticks until actual exit and both redirected readers settle; replacements cannot start while that reference is held. Launch setup failures follow the same cleanup. Tray shutdown alone waits at most 500 ms before disposal. -`ocx restart` also restarts a connected client runtime, which serves the same bound `POST /api/system/restart` from its client listener; the CLI sends that request only while the client's 90-second restart budget still fits before replacement observation ends (`CLIENT_RESTART_REQUEST_MIN_REMAINING_MS`), and reports a supervised client's `409 restart_supervised` as "not performed, runtime kept". See [Remote Link](remote-link.md) for the rollback. The Remote Link Child listener checks its startup owner and link against durable client state once per `/readyz`, after readiness probing, and for authenticated machine/link status only on those routes. If they differ, `/readyz` and those status routes answer 503 with `client_state_changed`; the dashboard shows that reason with its recovery guidance. This reports divergence; it does not disconnect the relay or reconstruct a connection. `tests/clients/client-machine-listener.test.ts` covers missing and replaced records. Explicit `ocx disconnect --orphaned-token --yes` in `src/client/orphan-token-recovery.ts` preserves unowned token files only after the start lease, lifecycle lock, liveness proof and config-lock revalidation; Hub, admission-token use and installed or unverified services refuse. Windows closes source handles before rename while private hard links pin identity; a refusal removes a pin only while another name still holds the token, and an extra hard link left by an interrupted run is refused with a message naming the `.identity` pin to remove from the `orphaned-token-*` folder. Missing ownership is never reconstructed. `ocx config import` refuses to replace a connected client record under the config lock. `tests/clients/client-link-connect.test.ts` covers these refusals and local liveness. +`ocx restart` also restarts a connected client runtime, which serves the same bound `POST /api/system/restart` from its client listener; the CLI sends that request only while the client's 90-second restart budget still fits before replacement observation ends (`CLIENT_RESTART_REQUEST_MIN_REMAINING_MS`), and reports a supervised client's `409 restart_supervised` as "not performed, runtime kept"; a client whose connection was removed is never refused and recycles to standalone. See [Remote Link](remote-link.md) for the rollback. The Remote Link Child listener checks its startup owner and link against durable client state once per `/readyz`, after readiness probing, and for authenticated machine/link status only on those routes. If they differ, `/readyz` and those status routes answer 503 with `client_state_changed`; the dashboard shows that reason with its recovery guidance. This reports divergence; it does not disconnect the relay or reconstruct a connection. `tests/clients/client-machine-listener.test.ts` covers missing and replaced records. Explicit `ocx disconnect --orphaned-token --yes` in `src/client/orphan-token-recovery.ts` preserves unowned token files only after the start lease, lifecycle lock, liveness proof and config-lock revalidation; Hub, admission-token use and installed or unverified services refuse. Windows closes source handles before rename while private hard links pin identity; a refusal removes a pin only while another name still holds the token, and an extra hard link left by an interrupted run is refused with a message naming the `.identity` pin to remove from the `orphaned-token-*` folder. Missing ownership is never reconstructed. `ocx config import` refuses to replace a connected client record under the config lock. `tests/clients/client-link-connect.test.ts` covers these refusals and local liveness. The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Numeric loopback classification covers the IPv4 127/8 range, equivalent IPv6 loopback spellings and IPv4-mapped loopback literals; it probes the configured address itself plus the other family, retaining the original bind scope and the responding probe address separately. This recovery classification does not change listener admission or bind policy. `probeEndpointLiveness` and `probePortOwner` retain their existing host selection: other specific loopback literals are checked at the requested address without adopting the opposite family. Only definitive absence permits PID deletion and journal reconciliation: no alive or indeterminate PID from either the PID file or runtime record unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. One recovery-only exception covers a specific non-loopback literal IP address (a Tailscale or LAN IP, never a hostname, loopback or wildcard), where a connect to this host's own unbound address can time out instead of being refused: when its probe ends "unknown", `canBindAddress` tries an exclusive listen on that raw socket address (IPv6 unbracketed) and port, closes it at once, and only a successful bind turns that endpoint "dead". The proof shares the discovery deadline: it is skipped once the budget is spent, its timeout is capped by the remaining budget, and a proof finishing past the deadline is discarded. Hostnames remain unknown after an inconclusive probe because name resolution may select different addresses for the probe and bind. Any bind error or timeout leaves the endpoint unknown, so startup still refuses; early (non-observer) discovery never binds. `tests/server/proxy-liveness-nonloopback-bind-proof.test.ts` covers hostnames, bound and unbindable IPs, IPv6 and deadline cases. An orphan runtime PID is checked separately from listener ownership: only a definite missing process or a fresh readable foreign command line permits recovery, while an indeterminate process check, unreadable/empty command line, or OpenCodex invocation remains unknown. The runtime PID is never supplied to destructive callers merely because it appears in the record; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Grok start and ensure/restart synchronization also recheck current desired-state admission after catalog discovery, immediately before the synchronous writer in `src/grok/sync.ts`; start additionally rejects work after shutdown begins or cleanup runs. This closes the same-process await gap without turning the desired-state and Grok files into a cross-process transaction. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery. A busy preferred port is never resolved by starting somewhere else. Both questions a start asks diff --git a/tests/cli/disconnect-runtime.test.ts b/tests/cli/disconnect-runtime.test.ts new file mode 100644 index 00000000000..896ad8cac7f --- /dev/null +++ b/tests/cli/disconnect-runtime.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; +import { restartRuntimeAfterDisconnect } from "../../src/cli/disconnect-runtime"; + +function run(live: { pid: number | null; role?: string } | null, restartOk: boolean, json = false) { + const out: string[] = [], err: string[] = []; + let restarts = 0; + const done = restartRuntimeAfterDisconnect({ + findLive: async () => live, + restart: async () => { restarts += 1; return restartOk; }, + json, + info: line => out.push(line), + error: line => err.push(line), + }); + return done.then(code => ({ code, out, err, restarts })); +} + +describe("ocx disconnect restarts the client runtime it left behind", () => { + test("a live client runtime is restarted and the machine reported standalone", async () => { + const result = await run({ pid: 4321, role: "client" }, true); + expect(result).toMatchObject({ code: 0, restarts: 1, err: [] }); + expect(result.out.join("\n")).toContain("now runs standalone"); + }); + + test("a restart that does not complete fails and names the remaining command", async () => { + const result = await run({ pid: 4321, role: "client" }, false); + expect(result.code).toBe(1); + expect(result.err.join("\n")).toContain("client runtime (pid 4321) still serves the old connection. Run on this machine: ocx restart"); + }); + + test("a standalone runtime or none is left alone", async () => { + for (const live of [{ pid: 4321 }, { pid: 4321, role: "standalone" }, null]) { + expect(await run(live, true)).toMatchObject({ code: 0, restarts: 0, out: [], err: [] }); + } + }); + + test("--json keeps stdout to the disconnect document", async () => { + const logged: unknown[][] = []; + const original = console.log; + console.log = (...args: unknown[]) => { logged.push(args); }; + try { + const result = await restartRuntimeAfterDisconnect({ + findLive: async () => ({ pid: 1, role: "client" }), + restart: async () => { console.log("restart progress"); return true; }, + json: true, + error: () => {}, + }); + expect(result).toBe(0); + } finally { console.log = original; } + expect(logged).toEqual([]); + }); +}); diff --git a/tests/clients/client-runtime-restart.test.ts b/tests/clients/client-runtime-restart.test.ts index 06cd7feb460..67e99fc1f41 100644 --- a/tests/clients/client-runtime-restart.test.ts +++ b/tests/clients/client-runtime-restart.test.ts @@ -253,6 +253,30 @@ describe("client restart controller", () => { expect(calls).toEqual([]); }); + // After `ocx disconnect` (or a newer connect) the served connection is gone, so there is no + // old runtime worth keeping: the restart hands off to the recycle, which every owner relaunches. + for (const supervisor of ["desktop", "service", null] as const) { + test(`a runtime whose connection changed recycles under ${supervisor ?? "no"} supervisor`, async () => { + const { io, calls } = fakeIo({ + supervisor: async () => supervisor, + connectionChanged: () => true, + recycle: async () => { calls.push("recycle"); }, + }); + const controller = createClientRestartController(io); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: false }); + expect(controller.hasHandoff()).toBe(true); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: true }); + await controller.settled(); + expect(calls).toEqual(["recycle"]); + }); + } + + test("an unchanged connection under a supervisor is still refused", async () => { + const { io, calls } = fakeIo({ supervisor: async () => "service", connectionChanged: () => false, recycle: async () => { calls.push("recycle"); } }); + expect(await createClientRestartController(io).request()).toEqual({ kind: "supervised", supervisor: "service" }); + expect(calls).toEqual([]); + }); + test("a second request while one is in flight joins it", async () => { let finish!: () => void; let spawns = 0; diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d03860e91ff..8e9314c92d6 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1625,7 +1625,7 @@ "codex-set-prompt.test.ts": "gui", "codex-prompt-writes.test.ts": "codex-integration", "codex-prompt-scalar-scope.test.ts": "codex-integration", "advisor-settings.test.ts": "advisor", "advisor-context.test.ts": "advisor", "advisor-state.test.ts": "advisor", "advisor-guard.test.ts": "advisor", "advisor-consult.test.ts": "advisor", "advisor-plan.test.ts": "advisor", "advisor-responses-wiring.test.ts": "advisor", "advisor-routes.test.ts": "server", "advisor-internal-authority.test.ts": "advisor", "advisor-core-boundary.test.ts": "advisor", "advisor-continuation-limit.test.ts": "advisor", "advisor-authority-transport.test.ts": "advisor", "advisor-evidence.test.ts": "advisor", "advisor-budget.test.ts": "advisor", "advisor-adaptive.test.ts": "advisor", "advisor-scope-transport.test.ts": "advisor" ,"link-codex-settings-digest.test.ts": "clients", "link-apply-record.test.ts": "clients", "client-runtime-restart.test.ts": "clients", "link-parity-reporter.test.ts": "clients", - "link-apply-restart.test.ts": "cli", "link-child-parity.test.ts": "server", "restart-replacement-not-ready.test.ts": "server", + "link-apply-restart.test.ts": "cli", "disconnect-runtime.test.ts": "cli", "link-child-parity.test.ts": "server", "restart-replacement-not-ready.test.ts": "server", "hermes-profiles.test.ts": "clients", "hermes-profile-routes.test.ts": "clients" ,"cli-sync-expect-link.test.ts": "cli" ,"local-credentials.test.ts": "cli", "local-credential-generation.test.ts": "config", "link-connection-generation.test.ts": "clients" From 3bca971a718fdb2ae4b49ac2c90803625d4922f1 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 02/15] fix(link): name the failed condition and its fix in the Child restart preflight refusal child_restart_unsupported now carries a hint for each cause (still linked, Hub, stale client runtime, no restart capability, port mismatch, nothing answering) with the Child command that fixes it, and the CLI appends the error code. --- src/cli/link-enrollment.ts | 4 ++- src/server/management/link-child-preflight.ts | 31 +++++++++++++++++++ src/server/management/link-enrollment.ts | 9 +++--- tests/cli/cli-link-enrollment.test.ts | 13 ++++++++ tests/server/link-enrollment.test.ts | 17 ++++++++++ 5 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/cli/link-enrollment.ts b/src/cli/link-enrollment.ts index f459c68ba47..f7af5a9b6a9 100644 --- a/src/cli/link-enrollment.ts +++ b/src/cli/link-enrollment.ts @@ -212,7 +212,9 @@ export async function runLinkEnrollment(command: "probe" | "add", args: string[] const hint = record(error.body.error) && typeof error.body.error.hint === "string" ? error.body.error.hint : ""; const message = phase ? "Remote Link failed during " + phase + (hint ? ": " + hint : "") : error.message; const recovery = ephemeralFlow ? RECOVERY_EPHEMERAL : RECOVERY_FULL; - if (phase || error.body.residual === true) throw new Error(message + (error.body.residual === true ? ". " + recovery : "")); + // The machine-readable code rides along so a script can branch without parsing prose. + const codeSuffix = phase && typeof code === "string" ? " (code " + code + ")" : ""; + if (phase || error.body.residual === true) throw new Error(message + (error.body.residual === true ? ". " + recovery : "") + codeSuffix); } throw error; } finally { diff --git a/src/server/management/link-child-preflight.ts b/src/server/management/link-child-preflight.ts index d3332f03dee..8c9d9a2f37e 100644 --- a/src/server/management/link-child-preflight.ts +++ b/src/server/management/link-child-preflight.ts @@ -30,6 +30,37 @@ export function childConfigFailureHint(result: { code: number; stderr: string }) return "ocx config show --json on the Child did not print a configuration with a valid port"; } + +/** + * Why a full Child cannot be enrolled as it stands, and the Child command that fixes it. Each of + * these used to share one phase name ("Child restart preflight"), which left the Home operator to + * reproduce the probe over SSH. Kept under the 160 code point hint cap. + */ +export function childRoleRefusalHint(runtimeRole: unknown): string { + return runtimeRole === "client" + ? "The Child is still linked to a Home (config runtimeRole client). On the Child run: ocx disconnect, then ocx restart." + : `The Child is a Hub (config runtimeRole ${runtimeRole === "hub" ? "hub" : "unknown"}). On the Child run: ocx hub disable, then retry.`; +} + +export function childHealthRefusalHint(health: unknown, port: number): string { + const body = health && typeof health === "object" && !Array.isArray(health) ? health as Record : null; + if (!body || body.service !== "opencodex") { + return `No OpenCodex answered /healthz on the Child's port ${port}. On the Child run: ocx start (or ocx service start).`; + } + if (body.port !== port) { + const seen = typeof body.port === "number" ? String(body.port) : "another port"; + return `The Child's runtime answers as port ${seen} but its config port is ${port}. On the Child run: ocx restart.`; + } + if (body.role === "client") { + return `The Child still serves a client runtime (/healthz role client on port ${port}). On the Child run: ocx restart.`; + } + if (body.role !== undefined && body.role !== "standalone") { + return `The Child serves a Hub runtime (/healthz role hub on port ${port}). On the Child run: ocx hub disable, then ocx restart.`; + } + return `The Child's runtime advertises no restart capability v1 on port ${port}. Update OpenCodex on the Child, then run: ocx restart.`; +} + + /** `ocx config validate --json` prints one small object; this bounds a misbehaving Child. */ export const CHILD_CONFIG_VALIDATE_MAX_OUTPUT_BYTES = 16 * 1024; diff --git a/src/server/management/link-enrollment.ts b/src/server/management/link-enrollment.ts index 5afa7eabb96..f45970187e4 100644 --- a/src/server/management/link-enrollment.ts +++ b/src/server/management/link-enrollment.ts @@ -14,7 +14,7 @@ import { withConfigMutationLockSync } from "../../config/mutation-lock"; import type { ManagementContext } from "./context"; import { readManagementJsonBodyOr } from "./body"; import { fail, exactBody, port, isRecord, runnerFor, readStoreFor, writeStoreFor, knownHostsFile, putKnownHost, issueKey, compensateNewLink, type LinkRouteState } from "./link-route-state"; -import { CHILD_CONFIG_TOO_LARGE_HINT, CHILD_CONFIG_TOO_LARGE_MESSAGE, CHILD_CONFIG_VALIDATE_MAX_OUTPUT_BYTES, CHILD_PREFLIGHT_MAX_OUTPUT_BYTES, childConfigFailureReason } from "./link-child-preflight"; +import { CHILD_CONFIG_TOO_LARGE_HINT, CHILD_CONFIG_TOO_LARGE_MESSAGE, CHILD_CONFIG_VALIDATE_MAX_OUTPUT_BYTES, CHILD_PREFLIGHT_MAX_OUTPUT_BYTES, childConfigFailureReason, childHealthRefusalHint, childRoleRefusalHint } from "./link-child-preflight"; const PROBE_TTL_MS = 5 * 60_000; const MIN_REMOTE_OCX = [2, 66, 0] as const; @@ -293,7 +293,7 @@ export async function apply(ctx: ManagementContext, state: LinkRouteState): Prom const hint = await childConfigFailureReason(configured, () => run(ocx(["config", "validate", "--json"]), budget, CHILD_CONFIG_VALIDATE_MAX_OUTPUT_BYTES)); return await failed(fail("child_preflight_failed", "The Child effective configuration could not be read.", 502, hint)); } - if ((childConfig.runtimeRole ?? "standalone") !== "standalone") return await failed(fail("child_restart_unsupported", "The Child must run standalone OpenCodex; an existing Hub or Child cannot be converted by Add Child.", 409)); + if ((childConfig.runtimeRole ?? "standalone") !== "standalone") return await failed(fail("child_restart_unsupported", "The Child must run standalone OpenCodex; an existing Hub or Child cannot be converted by Add Child.", 409, childRoleRefusalHint(childConfig.runtimeRole))); const childPort = childConfig.port; if (remotePlatform === "posix" && (await run(REMOTE_CURL_PREFLIGHT)).code !== 0) return await failed(fail("child_curl_missing", "Install curl on the Child before connecting a Remote Link.", 409)); const before = await run(remoteChildHttpArgv(childPort, "healthz", LINK_RELAY_VERIFY_MS, remotePlatform)); @@ -302,8 +302,9 @@ export async function apply(ctx: ManagementContext, state: LinkRouteState): Prom if (before.code === 255) return await failed(fail("child_preflight_failed", "The Child loopback port could not be inspected.", 502, sshFailureHint(before.stderr))); if (before.code === 0) return await failed(fail("child_port_busy", "The Child's target port is already in use; no Child files were modified.", 409)); } else { - const beforeHealth = before.code === 0 ? parseChildHttp(before.stdout) : null; - if (!isChildHealth(beforeHealth, "standalone", childPort)) return await failed(fail("child_restart_unsupported", "The Child must run a standalone OpenCodex with bound restart support; update or disconnect it before linking.", 409)); + let beforeHealth: unknown = null; + try { if (before.code === 0) beforeHealth = parseChildHttp(before.stdout); } catch { /* reported as nothing answering */ } + if (!isChildHealth(beforeHealth, "standalone", childPort)) return await failed(fail("child_restart_unsupported", "The Child must run a standalone OpenCodex with bound restart support; update or disconnect it before linking.", 409, childHealthRefusalHint(beforeHealth, childPort))); } state.confirmedHosts?.delete(alias); phase = "home_setup"; diff --git a/tests/cli/cli-link-enrollment.test.ts b/tests/cli/cli-link-enrollment.test.ts index 13dd81b9832..4a34e0078e7 100644 --- a/tests/cli/cli-link-enrollment.test.ts +++ b/tests/cli/cli-link-enrollment.test.ts @@ -180,6 +180,19 @@ describe("CLI Remote Link enrollment", () => { } finally { h.restore(); } }); + test("a Child restart preflight refusal names the condition, the fix and the code", async () => { + let step = 0; + const h = harness(); + try { + const hint = "The Child still serves a client runtime (/healthz role client on port 10100). On the Child run: ocx restart."; + h.deps.fetchImpl = async () => ++step < 3 ? Response.json(step === 1 ? probe : confirm) : Response.json({ error: { code: "child_restart_unsupported", message: "x", hint } }, { status: 409 }); + expect(await runLinkCommand(["add", "child-a", "--expect-fingerprint", fingerprint], h.deps)).not.toBe(0); + const message = h.err.join("\n"); + expect(message).toContain("Remote Link failed during Child restart preflight: " + hint); + expect(message).toContain("(code child_restart_unsupported)"); + } finally { h.restore(); } + }); + test("only the bounded, redacted hint survives from an error body (#8357)", async () => { let step = 0; const h = harness(); diff --git a/tests/server/link-enrollment.test.ts b/tests/server/link-enrollment.test.ts index c7e1365d016..3209102d7f7 100644 --- a/tests/server/link-enrollment.test.ts +++ b/tests/server/link-enrollment.test.ts @@ -303,6 +303,23 @@ describe("Home enrollment of a standalone Child", () => { expect(h.state.confirmedHosts?.has("child")).toBe(true); }); + // The refusal names the failed condition and the command that fixes it; a phase name alone + // left a Home operator guessing whether the Child was still linked, stale, old or misbound. + for (const [name, step, output, hint] of [ + ["config still linked", "config", result('{"port":10100,"runtimeRole":"client"}'), "still linked to a Home (config runtimeRole client). On the Child run: ocx disconnect, then ocx restart"], + ["config Hub", "config", result('{"port":10100,"runtimeRole":"hub"}'), "is a Hub (config runtimeRole hub). On the Child run: ocx hub disable"], + ["stale client runtime", "before", health("client"), "still serves a client runtime (/healthz role client on port 10100). On the Child run: ocx restart"], + ["Hub runtime", "before", health("hub"), "serves a Hub runtime (/healthz role hub on port 10100). On the Child run: ocx hub disable, then ocx restart"], + ["no restart capability", "before", health("standalone", null), "advertises no restart capability v1 on port 10100. Update OpenCodex on the Child, then run: ocx restart"], + ["port mismatch", "before", http({ service: "opencodex", pid: 4321, port: 10101, role: "standalone", restartCapability: "v1" }), "answers as port 10101 but its config port is 10100. On the Child run: ocx restart"], + ["nothing answering", "before", result("", 7), "No OpenCodex answered /healthz on the Child's port 10100. On the Child run: ocx start (or ocx service start)"], + ] as const) test(`${name} refusal names the condition and the fix`, async () => { + const h = harness(); h.confirmed(); h.overrides[step] = () => output; + const payload = await denied(await h.call("apply", { alias: "child" }), "child_restart_unsupported", 409); + expect(payload.error.hint).toContain(hint); + expect(Array.from(payload.error.hint as string).length).toBeLessThanOrEqual(160); + }); + test("measured 3400ms SSH latency allows 28600ms commands and separate restart/relay budgets", async () => { const h = harness(); h.overrides.probe = call => { From 69867afd3a5936f273462756cc2cc1f6b925b650 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 03/15] fix(link): a client injection takes over the Codex journal so a supervised relaunch keeps routing ocx connect --link re-injected routing over a standalone proxy's injection but left the journal owned by that process. After the supervisor relaunch (desktop or service), startup recovery saw a dead process owner and restored native Codex and the old catalog under the new link. --- src/codex/journal.ts | 21 ++++++++++-- tests/codex-integration/codex-journal.test.ts | 34 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/codex/journal.ts b/src/codex/journal.ts index fb8311a551e..dab13667b96 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -192,7 +192,7 @@ export function writeJournal(options: WriteJournalOptions = {}, hooks: AtomicWri // Ownership is decided HERE, from the bytes about to be journaled — never taken // on the caller's word. A caller that says "native" about injected content would // otherwise make opencodex's own routing the user's permanent "original". - if (hasInjectedCodexRouting(config)) return; + if (hasInjectedCodexRouting(config)) { adoptJournalForClient(options.owner, hooks); return; } // The caller's verdict only authorizes REPLACEMENT. It is weaker evidence than // the check above (it may describe bytes read a moment earlier), so an // unclassified call creates a first snapshot but never overwrites one. @@ -201,7 +201,7 @@ export function writeJournal(options: WriteJournalOptions = {}, hooks: AtomicWri throw new CodexHomeOwnerRefusal("owner-unknown"); } const previous = inspection.kind === "read" ? inspection.journal as unknown as Journal : null; - if (previous && options.currentStateIsNative !== true) return; + if (previous && options.currentStateIsNative !== true) { adoptJournalForClient(options.owner, hooks); return; } const profile = existsSync(CODEX_PROFILE_PATH) ? readFileSync(CODEX_PROFILE_PATH, "utf-8") : null; @@ -219,6 +219,23 @@ export function writeJournal(options: WriteJournalOptions = {}, hooks: AtomicWri atomicWriteFile(JOURNAL_PATH, JSON.stringify(journal), undefined, hooks); } +/** + * A committing client injection takes over a journal another owner wrote, keeping its original + * snapshot. Routing is the client's from here on; left with a standalone process as owner, the + * first start after that process exited (a supervisor relaunch after `ocx connect --link` and + * `ocx restart`) read it as a crashed session and restored native Codex under the new link. + */ +function adoptJournalForClient(owner: WriteJournalOptions["owner"], hooks: AtomicWriteHooks): void { + if (owner?.kind !== "client") return; + const inspection = readCodexHomeJournal(JOURNAL_PATH); + if (inspection.kind === "unknown") throw new CodexHomeOwnerRefusal("owner-unknown"); + if (inspection.kind !== "read") return; + const previous = inspection.journal as unknown as Journal; + if (previous.owner?.kind === "client" && previous.owner.apiKeyId === owner.apiKeyId) return; + const adopted: Journal = { ...previous, owner: { kind: "client", apiKeyId: owner.apiKeyId } }; + atomicWriteFile(JOURNAL_PATH, JSON.stringify(adopted), undefined, hooks); +} + export interface InjectedJournalOwnership { injectedOpenaiBaseUrl: string | null; injectedRealtimeWsBaseUrl: string | null; diff --git a/tests/codex-integration/codex-journal.test.ts b/tests/codex-integration/codex-journal.test.ts index 5cd189eb3d5..5392fed23d1 100644 --- a/tests/codex-integration/codex-journal.test.ts +++ b/tests/codex-integration/codex-journal.test.ts @@ -387,6 +387,40 @@ describe("codex-journal", () => { expect(existsSync(journalPath)).toBe(false); }); + // A Remote Link Child linked while a standalone proxy had routing injected: `ocx connect --link` + // re-injects for the client, but the journal stayed the standalone process's. Once that process + // exited into its supervisor (desktop exit 75, service exit 1), the supervisor's fresh start saw a + // dead process owner and restored native Codex and the old catalog under the new link. + test("a client injection over a standalone injection takes the journal, so a supervised restart keeps it", () => { + const original = '# original config\nmodel_provider = "openai"\n'; + writeFileSync(join(testDir, "config.toml"), original, "utf8"); + const linked = runScript(testDir, ` + const { injectCodexConfig } = require("./src/codex/inject"); + (async () => { + const config = { port: 10100, providers: {}, defaultProvider: "openai" }; + const standalone = await injectCodexConfig(10100, config, { catalogPath: null }); + const client = await injectCodexConfig(10100, config, { catalogPath: null, journalOwner: { kind: "client", apiKeyId: "link-key" } }); + console.log(JSON.stringify({ standalone: standalone.success, client: client.success })); + })(); + `); + expect(linked.status).toBe(0); + expect(JSON.parse(linked.stdout)).toEqual({ standalone: true, client: true }); + const journalPath = join(testDir, "opencodex-journal.json"); + const journal = JSON.parse(readFileSync(journalPath, "utf8")); + expect(journal.owner).toEqual({ kind: "client", apiKeyId: "link-key" }); + expect(Buffer.from(journal.originalConfig, "base64").toString("utf8")).toBe(original); + const routed = readFileSync(join(testDir, "config.toml"), "utf8"); + + // The supervisor's start: the injecting process is gone, the link is the committed client. + const restarted = runScript(testDir, ` + const { reconcileJournal } = require("./src/codex/journal"); + console.log(JSON.stringify({ restored: reconcileJournal({ activeClientApiKeyId: "link-key" }) })); + `); + expect(restarted.status).toBe(0); + expect(JSON.parse(restarted.stdout).restored).toBe(false); + expect(readFileSync(join(testDir, "config.toml"), "utf8")).toBe(routed); + }); + test("removeJournal refuses unknown journal evidence", () => { const journalPath = join(testDir, "opencodex-journal.json"); writeFileSync(journalPath, "{}", "utf8"); From d79b051f079d95b67f0eb9df57c2104c7bc2a4ee Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 04/15] fix(cli): point refusal and status hints at commands that work beside a live or desktop-owned proxy --- src/cli/desktop-runtime-guidance.ts | 2 +- src/cli/restart-failure.ts | 2 +- src/cli/status.ts | 4 +++- tests/cli/cli-restart-health.test.ts | 2 +- tests/cli/link-apply-restart.test.ts | 2 +- tests/service/autostart-health.test.ts | 7 ++++--- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/cli/desktop-runtime-guidance.ts b/src/cli/desktop-runtime-guidance.ts index 241c6d1ad64..ece73a7b47e 100644 --- a/src/cli/desktop-runtime-guidance.ts +++ b/src/cli/desktop-runtime-guidance.ts @@ -19,7 +19,7 @@ export function duplicateRuntimeMessage( if (pid != null && latch.observe(evidence)) { const supervisor = evidence.kind === "desktop" ? `, ${evidence.app}` : ""; return `OpenCodex Desktop supervises the running proxy (pid ${pid}${supervisor}, port ${port}). ` - + "Use the running proxy, or quit OpenCodex before starting it from this CLI."; + + "Use the running proxy (restart it with 'ocx restart', re-point Codex with 'ocx sync'), or quit OpenCodex before starting it from this CLI."; } return `⚠️ Proxy already running (PID ${pid ?? "unknown"}, port ${port}). Use 'ocx stop' first.`; } diff --git a/src/cli/restart-failure.ts b/src/cli/restart-failure.ts index 5352cdf19c1..a086a753199 100644 --- a/src/cli/restart-failure.ts +++ b/src/cli/restart-failure.ts @@ -36,7 +36,7 @@ export function reportRestartFailure(result: Extract { test("duplicate start guidance names the verified Desktop target", () => { const probes: Array = []; expect(duplicateRuntimeMessage(4242, 19999, deps => { probes.push(deps?.targetPid); return DESKTOP; })) - .toBe(`OpenCodex Desktop supervises the running proxy (pid 4242, ${DESKTOP.app}, port 19999). Use the running proxy, or quit OpenCodex before starting it from this CLI.`); + .toBe(`OpenCodex Desktop supervises the running proxy (pid 4242, ${DESKTOP.app}, port 19999). Use the running proxy (restart it with 'ocx restart', re-point Codex with 'ocx sync'), or quit OpenCodex before starting it from this CLI.`); expect(probes).toEqual([4242]); for (const evidence of [...NON_DESKTOP, { ...DESKTOP, runtimePid: 9999 }]) { expect(duplicateRuntimeMessage(4242, 19999, () => evidence)) diff --git a/tests/cli/link-apply-restart.test.ts b/tests/cli/link-apply-restart.test.ts index 4a4f867c626..fb384be7781 100644 --- a/tests/cli/link-apply-restart.test.ts +++ b/tests/cli/link-apply-restart.test.ts @@ -198,7 +198,7 @@ describe("ocx restart against a client runtime", () => { } expect(lines).toEqual([ "Restart not performed; the running client runtime was kept. Its background service owns it: run `ocx service restart`.", - "Restart not performed; the running client runtime was kept. The OpenCodex desktop app owns it: restart it from the app.", + "Restart not performed; the running client runtime was kept. The OpenCodex desktop app owns it while its connection is current: use Quit in the OpenCodex tray menu, then reopen it. After `ocx disconnect`, `ocx restart` hands it to the app.", ]); }); }); diff --git a/tests/service/autostart-health.test.ts b/tests/service/autostart-health.test.ts index d7a1ea51212..60aa28da140 100644 --- a/tests/service/autostart-health.test.ts +++ b/tests/service/autostart-health.test.ts @@ -452,8 +452,9 @@ describe("routing visibility (#2411)", () => { // Routing that is already ours needs no re-point. expect(unusedProxyWarningLines({ proxyUp: true, routingKind: "opencodex-local", connectedClient: link })).toEqual([]); expect(unusedProxyWarningLines({ proxyUp: false, routingKind: "opencodex-local", connectedClient: link })).toEqual([]); - // Standalone wording is unchanged. - expect(unusedProxyWarningLines({ proxyUp: true, routingKind: "native" }).join("\n")).toContain("Re-point with: ocx start"); + // Standalone: 'ocx start' refuses beside a live proxy (and under the desktop app), so the + // re-point is the command that writes routing to the running one. + expect(unusedProxyWarningLines({ proxyUp: true, routingKind: "native" }).join("\n")).toContain("Re-point with: ocx sync"); }); test("the dead-proxy restore advice names the way back only on a connected client", () => { @@ -473,7 +474,7 @@ describe("routing visibility (#2411)", () => { expect(statusUnusedProxyLines(view(true, "link", "native", true)).join("\n")).toContain("not the Home. Re-point with: ocx sync --restart-codex"); expect(statusUnusedProxyLines(view(true, undefined, "native", false)).join("\n")).toContain("not the hub"); expect(statusUnusedProxyLines(view(true, "link", "native", false)).join("\n")).toContain("'ocx service start'"); - expect(statusUnusedProxyLines(view(false, undefined, "native", true)).join("\n")).toContain("Re-point with: ocx start"); + expect(statusUnusedProxyLines(view(false, undefined, "native", true)).join("\n")).toContain("Re-point with: ocx sync"); expect(statusDeadProxyLines(view(true, "link", "opencodex-local", false)).join("\n")).toContain("ocx sync --restart-codex"); expect(statusDeadProxyLines(view(false, undefined, "opencodex-local", false)).join("\n")).not.toContain("ocx sync"); const routedDown = statusDeadProxyLines(view(true, "link", "opencodex-local", false)).join("\n"); From 73464c7c6b351d69321d629b2a9d5d4274edb935 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 05/15] fix(cli): explain client_catalog_ownership_changed on disconnect and name --keep-catalog --- src/cli/connect.ts | 8 +++- tests/clients/client-link-teardown.test.ts | 54 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/cli/connect.ts b/src/cli/connect.ts index 695f3f752cc..74cbab8deab 100644 --- a/src/cli/connect.ts +++ b/src/cli/connect.ts @@ -599,7 +599,13 @@ export async function handleDisconnectCommand( knownHostsFile: deps.linkTeardownDeps?.knownHostsFile ?? linkKnownHostsPath(), timeoutMs: deps.linkTeardownDeps?.timeoutMs, }); - const result = await disconnectClient({ keepCatalog }, deps); + const result = await disconnectClient({ keepCatalog }, deps).catch((error: unknown) => { + // The bare code left a Home-driven leave with nothing to act on; the remedy already exists. + if (error instanceof Error && error.message === "client_catalog_ownership_changed") { + throw new Error("client_catalog_ownership_changed: the Codex model catalog (opencodex-catalog.json in the Codex home) matches neither the catalog this connection wrote nor the one recorded before it connected, so something changed it after connecting and disconnect will not overwrite it. To disconnect and leave that file as it is, run: ocx disconnect --keep-catalog"); + } + throw error; + }); const payload = { ...result, homeRevoke: teardown.homeRevoke, diff --git a/tests/clients/client-link-teardown.test.ts b/tests/clients/client-link-teardown.test.ts index 540bc7cf348..42e9309b197 100644 --- a/tests/clients/client-link-teardown.test.ts +++ b/tests/clients/client-link-teardown.test.ts @@ -6,6 +6,7 @@ import { connectClient } from "../../src/client/connect"; import { teardownClientLink } from "../../src/client/link-teardown"; import { clientLinkStatePath, writeClientLinkState } from "../../src/client/link-state"; import { quoteRemote, remoteOcxArgv } from "../../src/link/ssh-argv"; +import { DEFAULT_CATALOG_PATH } from "../../src/codex/paths"; import { createTempHome } from "../helpers/temp-home"; const linkId = "lnk_0123456789abcdef"; @@ -151,3 +152,56 @@ test("an unreadable sidecar skips the revoke but still reports the manual revoke expect(standalone).toEqual({ linkId: null, homeRevoke: "not_applicable", tunnel: { tunnel: "absent" } }); expect(calls).toBe(0); }); + +test("a catalog changed after connect names what was found and --keep-catalog", async () => { + const home = createTempHome("ocx-client-link-catalog-"); + try { + const config = getDefaultConfig(); + config.port = 10100; + saveConfig(config); + mkdirSync(home.codexHome, { recursive: true }); + writeFileSync(home.path(".codex", "config.toml"), "model_provider = \"openai\"\n"); + await connectClient({ + serverUrl: "http://127.0.0.1:34567", + managementUrl: "http://127.0.0.1:34567", + managementTransport: "direct", + transport: "link", + link: { tunnelPort: 34567, linkId }, + credential: { kind: "link", apiKeyId: "key-1", key }, + selectedClients: ["codex"], + noSync: true, + }, { + fetchImpl: async input => String(input).endsWith("/readyz") + ? Response.json({ status: "ready", protocol: 1, minimumClientProtocol: 1, managementUrl: "http://127.0.0.1:34567" }) + : Response.json({ models: [] }), + catalogCompatibility: { supportedEfforts: () => new Set() }, + lifecycleLockDeps: { lockPath: home.path("lifecycle.sqlite") }, + }); + // Something else rewrote the catalog after connect: neither ours nor the recorded prior one. + writeFileSync(DEFAULT_CATALOG_PATH, JSON.stringify({ models: [{ slug: "rewritten-elsewhere" }] })); + const logs = spyOn(console, "log").mockImplementation(() => {}); + const errors = spyOn(console, "error").mockImplementation(() => {}); + let exit = -1; + let output = ""; + try { + exit = await handleDisconnectCommand([], { + lifecycleLockDeps: { lockPath: home.path("lifecycle.sqlite") }, + linkTeardownDeps: { + reapOrphanTunnel: async () => ({ tunnel: "absent" }), + runner: { run: async () => ({ code: 1, stdout: "", stderr: "" }) }, + knownHostsFile, + }, + }); + output = errors.mock.calls.flat().join("\n"); + } finally { + logs.mockRestore(); + errors.mockRestore(); + } + expect(exit).not.toBe(0); + expect(output).toContain("client_catalog_ownership_changed"); + expect(output).toContain("matches neither the catalog this connection wrote nor the one recorded before it connected"); + expect(output).toContain("ocx disconnect --keep-catalog"); + } finally { + home.remove(); + } +}); From 9c20cdecba503ec32835fc0652c3944b8bd8696a Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:33:50 +0900 Subject: [PATCH 06/15] docs(link): Child leave and re-link behavior under the desktop app or a service --- docs-site/src/content/docs/guides/remote-link.md | 6 +++++- structure/remote-link.md | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 79a367d50ca..5115609da29 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -109,6 +109,8 @@ ocx disconnect # or ~/.cache/opencodex/bin/ocx-child detach on a lightweight C To disconnect a Child-initiated link, run `ocx disconnect` on the Child. It disconnects the client tunnel and revokes the link on Home over SSH. If Home revocation fails, it prints: `Home revoke failed; run ocx link revoke --link-id on the home.` +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 the remaining step: `ocx restart` on the Child. A client runtime whose connection was removed accepts `ocx restart` whoever owns it: the OpenCodex desktop app or a background service starts the standalone replacement, so the same command works over SSH with nobody at the Child's screen. + ## 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. @@ -252,6 +254,8 @@ A Windows x64 Child needs OpenSSH Server, Windows PowerShell and either standalo Run `ocx link cleanup` on Home to inspect recorded Child platforms without changing them. Current Home releases disconnect Windows Children through PowerShell. If a remote disconnect fails, resolve the reported shell or installation error and retry; **Remove here only** remains an explicit recovery action that can leave the Child needing a local disconnect. -Restart waits for running turns and may take several minutes. Success requires the restarted Child to serve the client role and relay a fresh complete catalog to this Home. SSH setup has a bounded allowance derived from measured latency; relay verification starts after restart with its own 15-second window. Errors name the failing phase. If an error reports residual Child configuration, run `ocx disconnect` and `ocx restart` on that Child before retrying. Home cleanup alone does not restore Child configuration. A CLI timeout reports an uncertain outcome; inspect Home `ocx link status` and Child `ocx connect status` first. +Restart waits for running turns and may take several minutes. Success requires the restarted Child to serve the client role and relay a fresh complete catalog to this Home. SSH setup has a bounded allowance derived from measured latency; relay verification starts after restart with its own 15-second window. Errors name the failing phase and end with the error code. If an error reports residual Child configuration, run `ocx disconnect` and `ocx restart` on that Child before retrying. Home cleanup alone does not restore Child configuration. A CLI timeout reports an uncertain outcome; inspect Home `ocx link status` and Child `ocx connect status` first. + +A `Child restart preflight` failure (`child_restart_unsupported`) names what the Child is and the Child command that fixes it. Typical causes are a Child still linked to a Home (run `ocx disconnect`, then `ocx restart`), a Child still serving a client runtime (`ocx restart`), a Hub (`ocx hub disable`), a runtime without restart support (update OpenCodex), and a runtime answering on a port other than its configured one (`ocx restart`). On a Home-initiated Child, `ocx link status` reports role `child`, link identity and tunnel port from its saved connection even though its Home-side link store is empty. Locally unobserved tunnel state is `idle`. `ocx status` calls the peer a Remote Link Home. When Home runs in Hub mode, status reads its supported projection through the local Child relay. A standalone Home reports that it does not publish Hub state. Child-initiated tunnel identity comes from its valid sidecar; unreadable evidence does not claim a direction. diff --git a/structure/remote-link.md b/structure/remote-link.md index bfd7574e84e..9e0d738eb3b 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -190,7 +190,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". 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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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`. From 95b4a51e8d3d2cd1aa1307acb6ad9f40783973f6 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 16:51:50 +0900 Subject: [PATCH 07/15] fix(cli): name the desktop-remote folder that blocks disconnect and how to clear it A desktop-remote folder holding no state or receipt (seen with a renamed receipt) refused disconnect with only desktop_lifecycle_unsafe. The inspector now returns that reason and path, and disconnect names the folder and says to move it aside. --- src/claude/desktop-remote-store.ts | 5 +- src/client/connect.ts | 5 +- tests/clients/client-link-teardown.test.ts | 57 +++++++++++++++++++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/claude/desktop-remote-store.ts b/src/claude/desktop-remote-store.ts index 3e18307c980..e59c4db44d9 100644 --- a/src/claude/desktop-remote-store.ts +++ b/src/claude/desktop-remote-store.ts @@ -52,6 +52,9 @@ function recordedGenerations(io: StoreIO, owner: DesktopRemoteOwner): string[] { 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"; + path?: string; } { try { parseOwner(owner); @@ -71,7 +74,7 @@ export function inspectRemoteDesktopStore(owner: DesktopRemoteOwner): { if (projectionHash(file?.value ?? null) !== bundle.state.lastProjectionHash) return { kind: "conflict" }; return { kind: bundle.state.phase === "restored" ? "restored" : "active" }; } - if (existsSync(bundle.paths.root) && !receipt(io)) throw new DesktopStoreError("unsafe"); + if (existsSync(bundle.paths.root) && !receipt(io)) return { kind: "unsafe", reason: "root_without_state", path: bundle.paths.root }; const legacy = locateLegacy(io, owner, serviceGenerations()); if (legacy) currentConnection(owner); return { kind: legacy ? "legacy_current_connection" : "absent" }; diff --git a/src/client/connect.ts b/src/client/connect.ts index af6bf711eaf..31fd038a592 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -1118,7 +1118,10 @@ export async function disconnectClient( } if (!receipt) { const desktop = inspectRemoteDesktopStore(desktopOwner(connection)); - if (desktop.kind === "unsafe" || desktop.kind === "conflict") throw new Error("desktop_lifecycle_unsafe"); + 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.`); preflightDisconnectCatalog(connection, keepCatalog); receipt = { version: 1, owner: desktopOwner(connection), tokenFingerprint: connection.tokenFingerprint, keepCatalog, phase: "prepared" }; writeDesktopDisconnectReceipt(held, previous, receipt); diff --git a/tests/clients/client-link-teardown.test.ts b/tests/clients/client-link-teardown.test.ts index 42e9309b197..3c9a49ec014 100644 --- a/tests/clients/client-link-teardown.test.ts +++ b/tests/clients/client-link-teardown.test.ts @@ -1,5 +1,5 @@ import { expect, spyOn, test } from "bun:test"; -import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, realpathSync, writeFileSync } from "node:fs"; import { getDefaultConfig, saveConfig } from "../../src/config"; import { handleDisconnectCommand } from "../../src/cli/connect"; import { connectClient } from "../../src/client/connect"; @@ -205,3 +205,58 @@ test("a catalog changed after connect names what was found and --keep-catalog", home.remove(); } }); + +test("a desktop-remote folder with no state or receipt is named, with the way to clear it", async () => { + const home = createTempHome("ocx-client-link-desktop-store-"); + try { + const config = getDefaultConfig(); + config.port = 10100; + saveConfig(config); + mkdirSync(home.codexHome, { recursive: true }); + writeFileSync(home.path(".codex", "config.toml"), "model_provider = \"openai\"\n"); + await connectClient({ + serverUrl: "http://127.0.0.1:34567", + managementUrl: "http://127.0.0.1:34567", + managementTransport: "direct", + transport: "link", + link: { tunnelPort: 34567, linkId }, + credential: { kind: "link", apiKeyId: "key-1", key }, + selectedClients: ["codex"], + noSync: true, + }, { + fetchImpl: async input => String(input).endsWith("/readyz") + ? Response.json({ status: "ready", protocol: 1, minimumClientProtocol: 1, managementUrl: "http://127.0.0.1:34567" }) + : Response.json({ models: [] }), + catalogCompatibility: { supportedEfforts: () => new Set() }, + lifecycleLockDeps: { lockPath: home.path("lifecycle.sqlite") }, + }); + // Seen on a Child: someone renamed the receipt, leaving the folder with nothing OpenCodex reads. + mkdirSync(home.path("desktop-remote"), { recursive: true }); + writeFileSync(home.path("desktop-remote", "disconnect.json.stale-20261108"), "{}"); + const logs = spyOn(console, "log").mockImplementation(() => {}); + const errors = spyOn(console, "error").mockImplementation(() => {}); + let exit = -1; + let output = ""; + try { + exit = await handleDisconnectCommand([], { + lifecycleLockDeps: { lockPath: home.path("lifecycle.sqlite") }, + linkTeardownDeps: { + reapOrphanTunnel: async () => ({ tunnel: "absent" }), + runner: { run: async () => ({ code: 1, stdout: "", stderr: "" }) }, + knownHostsFile, + }, + }); + output = errors.mock.calls.flat().join("\n"); + } finally { + logs.mockRestore(); + errors.mockRestore(); + } + expect(exit).not.toBe(0); + expect(output).toContain("desktop_lifecycle_unsafe"); + expect(output).toContain(realpathSync.native(home.path("desktop-remote"))); + expect(output).toContain("holds no Claude Desktop remote state or disconnect receipt"); + expect(output).toContain("Move that folder aside, then run ocx disconnect again"); + } finally { + home.remove(); + } +}); From 37f295d2b12f60b0e2987031a5029cb4da7310fb Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 17:34:42 +0900 Subject: [PATCH 08/15] fix(link): a refused disconnect rolls back, Codex-edited config.toml no longer blocks it, and desktop refusals name their cause O8: a disconnect that fails before desktop_restored removes its prepared receipt (and an otherwise empty desktop-remote folder), so the client runtime still starts; a receipt left stuck by an older release finishes or rolls back on the next ocx disconnect, and the startup refusal names that command. O9: when Codex appended lines to config.toml after injection and no OpenCodex line remains, disconnect leaves the file as it is and retires the journal; while an OpenCodex line remains it keeps the journal and names the line. O7: inspectRemoteDesktopStore reports which step failed and the path it read; each desktop_lifecycle_unsafe refusal names the condition, path and fix, code last. --- scripts/test-layout/layout-clients.json | 2 + src/claude/desktop-remote-store.ts | 63 ++++++- src/cli/index.ts | 4 +- src/client/connect.ts | 38 +++- src/client/desktop-refusal.ts | 34 ++++ src/client/state.ts | 14 ++ src/codex/inject/disconnect-restore.ts | 55 ++++++ src/codex/inject/remove.ts | 34 ++++ tests/clients/client-connect.test.ts | 6 +- ...client-disconnect-desktop-refusals.test.ts | 132 ++++++++++++++ .../client-disconnect-recovery.test.ts | 163 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 2 +- 12 files changed, 527 insertions(+), 20 deletions(-) create mode 100644 src/client/desktop-refusal.ts create mode 100644 src/codex/inject/disconnect-restore.ts create mode 100644 tests/clients/client-disconnect-desktop-refusals.test.ts create mode 100644 tests/clients/client-disconnect-recovery.test.ts diff --git a/scripts/test-layout/layout-clients.json b/scripts/test-layout/layout-clients.json index c392db24cae..68a4d790817 100644 --- a/scripts/test-layout/layout-clients.json +++ b/scripts/test-layout/layout-clients.json @@ -160,6 +160,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..916e0fcddef 100644 --- a/src/claude/desktop-remote-store.ts +++ b/src/claude/desktop-remote-store.ts @@ -1,4 +1,4 @@ -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"; @@ -50,35 +50,59 @@ 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" | "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 }; + site = { reason: "profile_ambiguous", path: bundle.paths.library }; const legacy = locateLegacy(io, owner, serviceGenerations()); 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 +254,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 10e429277bc..f0825978f8a 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 31fd038a592..f12f0fae6b1 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -10,7 +10,7 @@ import { atomicWriteFile, loadConfig, withConfigMutationLockSync } from "../conf 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"; @@ -19,6 +19,8 @@ import { 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, @@ -1076,12 +1078,36 @@ 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: the Claude Desktop store, the catalog, + * the service token and the connection are untouched, 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. This also clears a + * receipt an older release left stuck at `prepared`. + */ +function abortPreparedDisconnect( + receipt: DesktopDisconnectReceipt, + error: unknown, + deps: Pick, +): Error { + const base = error instanceof Error ? error.message : String(error); + try { + withClientLifecycleSync(held => withConfigMutationLockSync(() => removeDesktopDisconnectReceipt(held, receipt)), deps.lifecycleLockDeps); + return new Error(`${base}. The disconnect was rolled back: this Child is still connected and its runtime can start.`, { cause: error }); + } 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 }); + } +} + export async function disconnectClient( options: { keepCatalog?: boolean; expectedOwner?: DesktopRemoteOwner } = {}, deps: Pick = {}, @@ -1118,10 +1144,7 @@ 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); @@ -1136,7 +1159,8 @@ export async function disconnectClient( // 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.receipt, error, deps); } } return withClientLifecycleSync(held => withConfigMutationLockSync(() => { diff --git a/src/client/desktop-refusal.ts b/src/client/desktop-refusal.ts new file mode 100644 index 00000000000..80a0481d194 --- /dev/null +++ b/src/client/desktop-refusal.ts @@ -0,0 +1,34 @@ +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. Make both files owner-only (chmod 600) if they are not; if the folder was copied from elsewhere, move it aside instead; 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. Move ${FOLDER} aside, then run ocx disconnect again; that profile stays in Claude Desktop until you remove it there.`; + 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, and restoring would discard those edits. Nothing was changed. Undo the edits, or move ${FOLDER} aside to give up restoring the profile it replaced, then run ocx disconnect again.`; + case "profile_ambiguous": + return `the Claude Desktop library ${at} has more than one profile using this Home's address, or one not named "opencodex", so disconnect cannot tell which is OpenCodex's. Nothing was changed. Remove the extra profiles in Claude Desktop, 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/state.ts b/src/client/state.ts index deaaffe1c2f..fdaadc64370 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -132,6 +132,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..641f0793c84 --- /dev/null +++ b/src/codex/inject/disconnect-restore.ts @@ -0,0 +1,55 @@ +/** + * 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 } from "./remove"; + +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) { + return { + ok: false, + reason: `${CODEX_CONFIG_PATH} changed after OpenCodex wrote it and still has OpenCodex lines (${remaining.join(", ")}), so the journal was kept. ` + + "Delete only those lines from that file, 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..79e79d69b38 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -146,6 +146,40 @@ 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 keys are returned, never values, so a refusal can quote them. + */ +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[] = []; + for (const line of content.split("\n")) { + const text = line.trim(); + if (!text) continue; + const left = kept.get(text) ?? 0; + if (left > 0) { kept.set(text, left - 1); continue; } + const key = text.startsWith("[") ? text : text.startsWith("#") ? "an OpenCodex marker comment" : text.split("=")[0]!.trim(); + if (!keys.includes(key)) keys.push(key); + } + // The strip changed something without removing a line: it would put back a line it replaced. + return keys.length > 0 ? keys : ["web_search"]; +} + function hasOpencodexRouting(content: string): boolean { return ( hasOcxProviderTable(content) || 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..995914a047a --- /dev/null +++ b/tests/clients/client-disconnect-desktop-refusals.test.ts @@ -0,0 +1,132 @@ +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)". Each case runs in its own homes and process. + */ + +type Case = "store-invalid" | "owner-mismatch" | "profile-missing" | "profile-edited" | "library-unreadable"; +type Outcome = { error: string | null; root: string; library: string; profile: string }; + +function run(kind: Case): 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 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":[]}'; + configApi.saveConfig({ port: 10100, providers: {}, defaultProvider: "openai", runtimeRole: "client", client: { + ...owner, managementUrl: owner.serverUrl, managementTransport: "direct", selectedClients: ["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 lockDeps = { lockPath: path.join(home, "lifecycle.sqlite") }; + if (kind === "library-unreadable") { + fs.writeFileSync(path.join(desktop, "_meta.json"), "{ not json"); + } else if (kind === "store-invalid") { + fs.mkdirSync(path.join(home, "desktop-remote"), { recursive: true, mode: 0o700 }); + fs.writeFileSync(path.join(home, "desktop-remote", "state.json"), "{ not json", { mode: 0o600 }); + } else { + fs.writeFileSync(path.join(desktop, "_meta.json"), JSON.stringify({ appliedId: "fixture", entries: [{ id: "fixture", name: "opencodex" }] })); + fs.writeFileSync(profile, JSON.stringify({ 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(path.join(desktop, "_meta.json"), JSON.stringify({ entries: [{ id: "fixture", name: "renamed by hand" }] })); + } + 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); + } + } + (async () => { + let error = null; + try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); } + catch (cause) { error = cause instanceof Error ? cause.message : String(cause); } + console.log(JSON.stringify({ + error, root: fs.realpathSync.native(home) + "/desktop-remote", + library: fs.realpathSync.native(desktop), profile: fs.realpathSync.native(desktop) + "/fixture.json", + })); + })(); + `; + 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("an unreadable store file", () => { + const out = run("store-invalid"); + expectNamedRefusal(out, out.root, "cannot read safely"); + }, SPAWN_BUDGET_MS); + + test("a store written for another connection", () => { + const out = run("owner-mismatch"); + expectNamedRefusal(out, out.root + "/state.json", "a different connection"); + }, SPAWN_BUDGET_MS); + + test("the OpenCodex profile removed or renamed in Claude Desktop", () => { + const out = run("profile-missing"); + expectNamedRefusal(out, out.library, "removed or renamed"); + }, SPAWN_BUDGET_MS); + + test("the OpenCodex profile edited outside OpenCodex", () => { + const out = run("profile-edited"); + expectNamedRefusal(out, out.profile, "edited outside OpenCodex"); + }, SPAWN_BUDGET_MS); + + test("an unreadable Claude Desktop profile list", () => { + const out = run("library-unreadable"); + expectNamedRefusal(out, out.library, "profile list"); + }, 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..4cc93687dd6 --- /dev/null +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -0,0 +1,163 @@ +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"; + +/** + * 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."/Users/someone/work"]\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 }; +type Outcome = { + error: string | null; stateKind: string; gateBefore: string; gateAfter: string; + receipt: string; desktopRoot: boolean; journal: boolean; config: string; startupRefusal: string; +}; + +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, + 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"); + let error = null; + try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); } + catch (cause) { error = cause instanceof Error ? cause.message : String(cause); } + const receipt = store.readDesktopDisconnectReceipt(); + console.log(JSON.stringify({ + error, + 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)", () => { + 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); +}); + +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); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 8e9314c92d6..693045fc2cd 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1592,7 +1592,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", From 216dda7f7c4184b2353f884a4ad9b972f7f04fcc Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 17:34:42 +0900 Subject: [PATCH 09/15] docs(link): disconnect rollback, Codex-edited config and named desktop refusals; plan unit for O7-O10 --- .../000_plan.md | 14 +++++++++++++ .../010_o8_prepared_receipt_rollback.md | 18 ++++++++++++++++ .../020_o9_codex_edited_config.md | 21 +++++++++++++++++++ .../030_o7_named_desktop_refusals.md | 7 +++++++ .../040_o10_restart_codex_clients.md | 19 +++++++++++++++++ .../src/content/docs/guides/remote-link.md | 4 ++++ structure/remote-link.md | 2 +- 7 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 devlog/_plan/261111_child_disconnect_followups/000_plan.md create mode 100644 devlog/_plan/261111_child_disconnect_followups/010_o8_prepared_receipt_rollback.md create mode 100644 devlog/_plan/261111_child_disconnect_followups/020_o9_codex_edited_config.md create mode 100644 devlog/_plan/261111_child_disconnect_followups/030_o7_named_desktop_refusals.md create mode 100644 devlog/_plan/261111_child_disconnect_followups/040_o10_restart_codex_clients.md 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..63aabd75c1a --- /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 keys (keys only, never values), with the fix: delete only those lines, 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 5115609da29..7e18f4233f9 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 the remaining step: `ocx restart` on the Child. A client runtime whose connection was removed accepts `ocx restart` whoever owns it: the OpenCodex desktop app or a background service starts the standalone replacement, so the same command works over SSH with nobody at the Child's screen. +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. + +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. If an OpenCodex line remains, disconnect keeps the journal and names the line; delete only that line, 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/structure/remote-link.md b/structure/remote-link.md index 9e0d738eb3b..12e0d28c617 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -190,7 +190,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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. A disconnect refused before its receipt reaches `desktop_restored` removes that `prepared` receipt (and an otherwise empty `desktop-remote/` folder), because nothing outside Codex has changed yet and 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; while any OpenCodex line remains the journal is kept. `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`. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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`. From 0fd13d7412d3066db6554be246db3dbaeccf3149 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 17:46:44 +0900 Subject: [PATCH 10/15] fix(link): review round 1 - one disconnect at a time, keep a receipt whose Desktop restore began, quote key names only, safe desktop refusal fixes - withClientDisconnectAttempt (lock D) serializes ocx disconnect so a second attempt cannot roll back the first one's prepared receipt while it restores Codex outside L. - A prepared receipt an earlier run left is rolled back only while the Desktop store shows no restore started; otherwise it is kept so the retry can finish. - remainingOpencodexConfigKeys reports only structural key names and table headers, never a continuation line of a multiline value. - Desktop refusals: token backup and connection checks get their own reasons; the edited-profile, other-connection and foreign-profile fixes no longer lead to a lost edit or a second refusal, and each named retry is tested. --- .../src/content/docs/guides/remote-link.md | 2 +- src/claude/desktop-remote-store.ts | 9 +- src/client/connect.ts | 43 ++++-- src/client/desktop-refusal.ts | 10 +- src/client/lifecycle-lock.ts | 33 ++++- src/codex/inject/remove.ts | 16 ++- structure/remote-link.md | 2 +- ...client-disconnect-desktop-refusals.test.ts | 123 ++++++++++++++---- .../client-disconnect-recovery.test.ts | 46 ++++++- 9 files changed, 230 insertions(+), 54 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 7e18f4233f9..7e3ffc7ec34 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -111,7 +111,7 @@ 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 the remaining step: `ocx restart` on the Child. A client runtime whose connection was removed accepts `ocx restart` whoever owns it: the OpenCodex desktop app or a background service starts the standalone replacement, so the same command works over SSH with nobody at the Child's screen. -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 `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. If an OpenCodex line remains, disconnect keeps the journal and names the line; delete only that line, 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. diff --git a/src/claude/desktop-remote-store.ts b/src/claude/desktop-remote-store.ts index 916e0fcddef..a61539d9880 100644 --- a/src/claude/desktop-remote-store.ts +++ b/src/claude/desktop-remote-store.ts @@ -2,7 +2,7 @@ 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"; @@ -56,7 +56,7 @@ function recordedGenerations(io: StoreIO, owner: DesktopRemoteOwner): string[] { */ export type DesktopStoreInspectReason = | "root_without_state" | "store_unreadable" | "store_other_connection" | "library_unreadable" - | "profile_missing" | "profile_edited" | "profile_ambiguous" | "unknown"; + | "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"; @@ -93,8 +93,11 @@ export function inspectRemoteDesktopStore(owner: DesktopRemoteOwner): { 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 }; + site = { reason: "token_unreadable", path: serviceApiTokenBackupPath() }; + const generations = serviceGenerations(); site = { reason: "profile_ambiguous", path: bundle.paths.library }; - const legacy = locateLegacy(io, owner, serviceGenerations()); + const legacy = locateLegacy(io, owner, generations); + site = { reason: "connection_changed" }; if (legacy) currentConnection(owner); return { kind: legacy ? "legacy_current_connection" : "absent" }; } catch (error) { diff --git a/src/client/connect.ts b/src/client/connect.ts index f12f0fae6b1..513b111495f 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -15,7 +15,7 @@ import { 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"; @@ -1088,27 +1088,46 @@ function restoreConnectedCodex(connection: OcxClientConnectionConfig): void { /** * Abort a disconnect that failed before its commit point. * - * Until `desktop_restored` the receipt records intent only: the Claude Desktop store, the catalog, - * the service token and the connection are untouched, 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. This also clears a - * receipt an older release left stuck at `prepared`. + * 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( - receipt: DesktopDisconnectReceipt, + 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 { - withClientLifecycleSync(held => withConfigMutationLockSync(() => removeDesktopDisconnectReceipt(held, receipt)), deps.lifecycleLockDeps); - return new Error(`${base}. The disconnect was rolled back: this Child is still connected and its runtime can start.`, { cause: error }); + 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 { + return withClientDisconnectAttempt(() => disconnectClientAttempt(options, deps), deps.lifecycleLockDeps); +} + +async function disconnectClientAttempt( options: { keepCatalog?: boolean; expectedOwner?: DesktopRemoteOwner } = {}, deps: Pick = {}, ): Promise<{ @@ -1128,6 +1147,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; @@ -1148,19 +1168,20 @@ export async function disconnectClient( 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")) { try { restoreConnectedCodex(prepared.connection); } - catch (error) { throw abortPreparedDisconnect(prepared.receipt, error, deps); } + catch (error) { throw abortPreparedDisconnect(prepared, error, deps); } } return withClientLifecycleSync(held => withConfigMutationLockSync(() => { diff --git a/src/client/desktop-refusal.ts b/src/client/desktop-refusal.ts index 80a0481d194..4cca8346f49 100644 --- a/src/client/desktop-refusal.ts +++ b/src/client/desktop-refusal.ts @@ -17,15 +17,19 @@ export function desktopLifecycleRefusal(desktop: { kind: string; reason?: Deskto 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. Make both files owner-only (chmod 600) if they are not; if the folder was copied from elsewhere, move it aside instead; 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. Move ${FOLDER} aside, then run ocx disconnect again; that profile stays in Claude Desktop until you remove it there.`; + 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, and restoring would discard those edits. Nothing was changed. Undo the edits, or move ${FOLDER} aside to give up restoring the profile it replaced, then run ocx disconnect again.`; + 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 `the Claude Desktop library ${at} has more than one profile using this Home's address, or one not named "opencodex", so disconnect cannot tell which is OpenCodex's. Nothing was changed. Remove the extra profiles in Claude Desktop, then run ocx disconnect again.`; + 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 the profile that reaches this Home and is not OpenCodex's current one, 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.`; } 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/codex/inject/remove.ts b/src/codex/inject/remove.ts index 79e79d69b38..f410a85c4a6 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"; @@ -150,7 +150,8 @@ export function stripOpencodexConfig(content: string): string { * 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 keys are returned, never values, so a refusal can quote them. + * 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"); @@ -168,12 +169,17 @@ export function remainingOpencodexConfigKeys(rawContent: string): string[] { 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[] = []; - for (const line of content.split("\n")) { - const text = line.trim(); + for (const line of splitSourceLines(content)) { + const text = line.text.trim(); if (!text) continue; const left = kept.get(text) ?? 0; if (left > 0) { kept.set(text, left - 1); continue; } - const key = text.startsWith("[") ? text : text.startsWith("#") ? "an OpenCodex marker comment" : text.split("=")[0]!.trim(); + // A line inside a multiline string or array is a value, never a name. + if (!line.structural) continue; + const header = /^\[\[?\s*([A-Za-z0-9_.\-"' ]{1,128})\s*\]\]?$/.exec(text); + const key = header ? `[${header[1]!.trim()}]` + : text.startsWith("#") ? "an OpenCodex marker comment" + : /^([A-Za-z0-9_.\-]{1,64})\s*=/.exec(text)?.[1] ?? "an OpenCodex line"; if (!keys.includes(key)) keys.push(key); } // The strip changed something without removing a line: it would put back a line it replaced. diff --git a/structure/remote-link.md b/structure/remote-link.md index 12e0d28c617..5c6cc6cd9c8 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -190,7 +190,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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. A disconnect refused before its receipt reaches `desktop_restored` removes that `prepared` receipt (and an otherwise empty `desktop-remote/` folder), because nothing outside Codex has changed yet and 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; while any OpenCodex line remains the journal is kept. `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`. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. 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; while any OpenCodex line remains the journal is kept. `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`. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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-disconnect-desktop-refusals.test.ts b/tests/clients/client-disconnect-desktop-refusals.test.ts index 995914a047a..384f33aef03 100644 --- a/tests/clients/client-disconnect-desktop-refusals.test.ts +++ b/tests/clients/client-disconnect-desktop-refusals.test.ts @@ -10,13 +10,19 @@ 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)". Each case runs in its own homes and process. + * "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"; -type Outcome = { error: string | null; root: string; library: string; profile: string }; +type Case = + | "store-invalid" | "owner-mismatch" | "profile-missing" | "profile-edited" | "library-unreadable" + | "token-backup-unsafe" | "profile-foreign" | "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): Outcome { +function run(kind: Case, retry = false): Outcome { const root = mkdtempSync(join(tmpdir(), "ocx-disconnect-desktop-refusal-")); try { const script = ` @@ -29,31 +35,47 @@ function run(kind: Case): Outcome { 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: ["claude"], + ...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(path.join(desktop, "_meta.json"), "{ not json"); + fs.writeFileSync(meta, "{ not json"); } else if (kind === "store-invalid") { - fs.mkdirSync(path.join(home, "desktop-remote"), { recursive: true, mode: 0o700 }); - fs.writeFileSync(path.join(home, "desktop-remote", "state.json"), "{ not json", { mode: 0o600 }); + fs.mkdirSync(remote, { recursive: true, mode: 0o700 }); + fs.writeFileSync(path.join(remote, "state.json"), "{ not json", { mode: 0o600 }); + } 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 { - fs.writeFileSync(path.join(desktop, "_meta.json"), JSON.stringify({ appliedId: "fixture", entries: [{ id: "fixture", name: "opencodex" }] })); - fs.writeFileSync(profile, JSON.stringify({ inferenceProvider: "gateway", inferenceCredentialKind: "static", + writeProfile("opencodex", { inferenceProvider: "gateway", inferenceCredentialKind: "static", inferenceGatewayBaseUrl: "http://127.0.0.1:10100", inferenceGatewayApiKey: "fixture-local-key", - modelDiscoveryEnabled: false, inferenceModels: [] })); + 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" }], @@ -65,22 +87,49 @@ function run(kind: Case): Outcome { fs.writeFileSync(profile, JSON.stringify(value)); } if (kind === "profile-missing") { - fs.writeFileSync(path.join(desktop, "_meta.json"), JSON.stringify({ entries: [{ id: "fixture", name: "renamed by hand" }] })); + 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 () => { - let error = null; - try { await connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }); } - catch (cause) { error = cause instanceof Error ? cause.message : String(cause); } + 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, root: fs.realpathSync.native(home) + "/desktop-remote", - library: fs.realpathSync.native(desktop), profile: fs.realpathSync.native(desktop) + "/fixture.json", + 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"), })); })(); `; @@ -110,23 +159,49 @@ describe("desktop_lifecycle_unsafe names its cause and fix (O7)", () => { expectNamedRefusal(out, out.root, "cannot read safely"); }, SPAWN_BUDGET_MS); - test("a store written for another connection", () => { - const out = run("owner-mismatch"); - expectNamedRefusal(out, out.root + "/state.json", "a different connection"); + 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 removed or renamed in Claude Desktop", () => { - const out = run("profile-missing"); + 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", () => { + 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"); + 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); +}); + +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 index 4cc93687dd6..e912523ea2f 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -7,6 +7,7 @@ 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. @@ -21,15 +22,16 @@ 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."/Users/someone/work"]\ntrust_level = "trusted"\n'; +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 }; +type Scenario = { config: string; stuckReceipt?: boolean; overlapping?: boolean }; type Outcome = { error: string | null; stateKind: string; gateBefore: string; gateAfter: string; receipt: string; desktopRoot: boolean; journal: boolean; config: string; startupRefusal: string; + overlappingError: string | null; }; const sha = (value: string) => createHash("sha256").update(value).digest("hex"); @@ -76,12 +78,26 @@ function run(scenario: Scenario): Outcome { } 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) => { + if (!overlapping) overlapping = connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }) + .then(() => null, cause => cause instanceof Error ? cause.message : String(cause)); + 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; const receipt = store.readDesktopDisconnectReceipt(); console.log(JSON.stringify({ error, + overlappingError, stateKind: stateApi.readClientConnectionState().kind, gateBefore, startupRefusal, @@ -160,4 +176,30 @@ describe("a refused disconnect does not block the next start (O8)", () => { 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('"""'); + }); }); From 6bd12c39dd283b3b8881715020ded524541bd757 Mon Sep 17 00:00:00 2001 From: cpa Date: Wed, 11 Nov 2026 17:52:25 +0900 Subject: [PATCH 11/15] fix(link): final review - quote only allowlisted keys and fixed table labels, name whole-value removal, and give tested fixes for older-key profiles and corrupt store files - remainingOpencodexConfigKeys reports keys OpenCodex writes and fixed labels for its tables; a quoted table name or any other key gets a fixed label, so nothing from the file's text leaks. - The O9 fix says to remove each named key with its whole value and each named table with every line under it; a test applies that edit and the retry completes. - profile_ambiguous covers the single older-key opencodex profile; store_unreadable gives a path for corrupt owner-only files that keeps the baseline evidence; each is retry-tested. --- .../020_o9_codex_edited_config.md | 2 +- .../src/content/docs/guides/remote-link.md | 2 +- src/client/desktop-refusal.ts | 4 +- src/codex/inject/disconnect-restore.ts | 5 ++- src/codex/inject/remove.ts | 33 +++++++++++--- ...client-disconnect-desktop-refusals.test.ts | 22 ++++++++-- .../client-disconnect-recovery.test.ts | 43 ++++++++++++++++++- 7 files changed, 93 insertions(+), 18 deletions(-) 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 index 63aabd75c1a..20ba35a3eea 100644 --- 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 @@ -11,7 +11,7 @@ Inside one held Codex config write lock, after `restoreJournalState` reports inc - 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 keys (keys only, never values), with the fix: delete only those lines, 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. +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. diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 7e3ffc7ec34..cc539a70a94 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -113,7 +113,7 @@ On a full OpenCodex Child, `ocx disconnect` also restarts a client runtime that 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. If an OpenCodex line remains, disconnect keeps the journal and names the line; delete only that line, 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. +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. 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 diff --git a/src/client/desktop-refusal.ts b/src/client/desktop-refusal.ts index 4cca8346f49..09b24110014 100644 --- a/src/client/desktop-refusal.ts +++ b/src/client/desktop-refusal.ts @@ -15,7 +15,7 @@ export function desktopLifecycleRefusal(desktop: { kind: string; reason?: Deskto 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. Make both files owner-only (chmod 600) if they are not; if the folder was copied from elsewhere, move it aside instead; then run ocx disconnect again.`; + 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": @@ -25,7 +25,7 @@ export function desktopLifecycleRefusal(desktop: { kind: string; reason?: Deskto 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 the profile that reaches this Home and is not OpenCodex's current one, then run ocx disconnect again.`; + 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": diff --git a/src/codex/inject/disconnect-restore.ts b/src/codex/inject/disconnect-restore.ts index 641f0793c84..897ea29348b 100644 --- a/src/codex/inject/disconnect-restore.ts +++ b/src/codex/inject/disconnect-restore.ts @@ -43,8 +43,9 @@ export function restoreCodexForDisconnect(): DisconnectCodexRestore { if (remaining.length > 0) { return { ok: false, - reason: `${CODEX_CONFIG_PATH} changed after OpenCodex wrote it and still has OpenCodex lines (${remaining.join(", ")}), so the journal was kept. ` - + "Delete only those lines from that file, keep every other line, then run ocx disconnect again", + 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)); diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index f410a85c4a6..b9e30c0ea73 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -169,23 +169,44 @@ export function remainingOpencodexConfigKeys(rawContent: string): string[] { 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); continue; } + 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; - const header = /^\[\[?\s*([A-Za-z0-9_.\-"' ]{1,128})\s*\]\]?$/.exec(text); - const key = header ? `[${header[1]!.trim()}]` - : text.startsWith("#") ? "an OpenCodex marker comment" - : /^([A-Za-z0-9_.\-]{1,64})\s*=/.exec(text)?.[1] ?? "an OpenCodex line"; - if (!keys.includes(key)) keys.push(key); + 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 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/tests/clients/client-disconnect-desktop-refusals.test.ts b/tests/clients/client-disconnect-desktop-refusals.test.ts index 384f33aef03..140b1fdc2f3 100644 --- a/tests/clients/client-disconnect-desktop-refusals.test.ts +++ b/tests/clients/client-disconnect-desktop-refusals.test.ts @@ -16,7 +16,7 @@ import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; type Case = | "store-invalid" | "owner-mismatch" | "profile-missing" | "profile-edited" | "library-unreadable" - | "token-backup-unsafe" | "profile-foreign" | "desktop-restore-started"; + | "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; @@ -65,6 +65,8 @@ function run(kind: Case, retry = false): Outcome { } 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") { @@ -72,6 +74,11 @@ function run(kind: Case, retry = false): Outcome { 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", @@ -154,9 +161,10 @@ function expectNamedRefusal(out: Outcome, ...parts: string[]): void { } describe("desktop_lifecycle_unsafe names its cause and fix (O7)", () => { - test("an unreadable store file", () => { - const out = run("store-invalid"); - expectNamedRefusal(out, out.root, "cannot read safely"); + 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", () => { @@ -194,6 +202,12 @@ describe("desktop_lifecycle_unsafe names its cause and fix (O7)", () => { 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)", () => { diff --git a/tests/clients/client-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts index e912523ea2f..9e858197311 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -27,11 +27,11 @@ 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 }; +type Scenario = { config: string; stuckReceipt?: boolean; overlapping?: boolean; retryConfig?: 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; + overlappingError: string | null; retryError: string | null; }; const sha = (value: string) => createHash("sha256").update(value).digest("hex"); @@ -94,10 +94,19 @@ function run(scenario: Scenario): Outcome { 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, @@ -141,6 +150,23 @@ describe("disconnect after Codex edited config.toml (O9)", () => { 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)", () => { @@ -202,4 +228,17 @@ describe("the O9 refusal quotes key names only", () => { 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]"); + }); }); From ae7068940599bf8b561f1118cf016ba0688bf4f6 Mon Sep 17 00:00:00 2001 From: lidge-jun <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:55:54 +0900 Subject: [PATCH 12/15] fix(link): put back the person's own web_search line OpenCodex set aside, instead of refusing forever When the journal recorded a user root web_search line that OpenCodex replaced and config.toml no longer has one, the owned-field strip would add it back, so the refusal named web_search and every retry repeated it. Disconnect now inserts exactly that journaled line at the end of the root section and retires the journal, but only when nothing OpenCodex owns remains after it. --- .../src/content/docs/guides/remote-link.md | 2 +- src/codex/inject/disconnect-restore.ts | 11 +++++++++- src/codex/inject/remove.ts | 22 +++++++++++++++++++ structure/remote-link.md | 2 +- .../client-disconnect-recovery.test.ts | 12 +++++++++- 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index cc539a70a94..b510e5be7af 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -113,7 +113,7 @@ On a full OpenCodex Child, `ocx disconnect` also restarts a client runtime that 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. 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. +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 diff --git a/src/codex/inject/disconnect-restore.ts b/src/codex/inject/disconnect-restore.ts index 897ea29348b..3bdba579124 100644 --- a/src/codex/inject/disconnect-restore.ts +++ b/src/codex/inject/disconnect-restore.ts @@ -17,7 +17,8 @@ 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 } from "./remove"; +import { remainingOpencodexConfigKeys, withReplacedWebSearchRestored } from "./remove"; +import { atomicWriteFile } from "../../config"; export type DisconnectCodexRestore = | { ok: true; journalRetired: boolean } @@ -41,6 +42,14 @@ export function restoreCodexForDisconnect(): DisconnectCodexRestore { 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. ` diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index b9e30c0ea73..12dc1bb428c 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -191,6 +191,28 @@ export function remainingOpencodexConfigKeys(rawContent: string): string[] { 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(); + if (!line || rootAssignmentKey(line) !== "web_search") return null; + const eol = dominantEol(rawContent); + const { bom, lines, rootEnd } = rootSourceLines(applyEol(rawContent, "\n")); + 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 inserted = { text: line.trim(), eol: "\n" as const, structural: true }; + if (at > 0 && lines[at - 1]!.eol === "") lines[at - 1] = { ...lines[at - 1]!, eol: "\n" }; + const next = [...lines.slice(0, at), inserted, ...lines.slice(at)]; + return applyEol(bom + sourceText(next), eol); +} + + /** 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", diff --git a/structure/remote-link.md b/structure/remote-link.md index 5c6cc6cd9c8..432b4b0aa3f 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -190,7 +190,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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. 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; while any OpenCodex line remains the journal is kept. `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`. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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 under every owner and hands off to the standalone recycle (desktop exit 75, service exit 1, otherwise a spawned replacement). `ocx disconnect` (`src/cli/disconnect-runtime.ts`) runs that restart itself when a client runtime is still live and exits non-zero naming `ocx restart` when it does not complete. 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`. A committing client injection also takes over a Codex journal another owner wrote (`src/codex/journal.ts`), so a supervisor relaunch after link enrollment does not read the standalone owner as a crashed session and restore native Codex. 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-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts index 9e858197311..c9f7e59f68f 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -27,7 +27,7 @@ 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 }; +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; @@ -58,6 +58,7 @@ function run(scenario: Scenario): Outcome { 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 = ` @@ -133,6 +134,15 @@ function run(scenario: Scenario): Outcome { } describe("disconnect after Codex edited config.toml (O9)", () => { + 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(); From 6255df6c72051f2fa799d842dda8ab9a1db13f88 Mon Sep 17 00:00:00 2001 From: lidge-jun <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:58:58 +0900 Subject: [PATCH 13/15] fix(link): keep each line's own ending when putting the web_search line back withReplacedWebSearchRestored converted the whole file to LF and back to the dominant ending, so a mixed CRLF/LF config had unrelated lines rewritten before the journal was retired. It now inserts into the file's own source lines, gives the new line the ending of the line it follows, and keeps the journaled line's text as written. --- src/codex/inject/remove.ts | 17 ++++++++++------- .../clients/client-disconnect-recovery.test.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/codex/inject/remove.ts b/src/codex/inject/remove.ts index 12dc1bb428c..98edef45490 100644 --- a/src/codex/inject/remove.ts +++ b/src/codex/inject/remove.ts @@ -198,18 +198,21 @@ export function remainingOpencodexConfigKeys(rawContent: string): string[] { * 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(); + const line = journaledReplacedRootWebSearch()?.replace(/\r?\n$/, ""); if (!line || rootAssignmentKey(line) !== "web_search") return null; - const eol = dominantEol(rawContent); - const { bom, lines, rootEnd } = rootSourceLines(applyEol(rawContent, "\n")); + // 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 inserted = { text: line.trim(), eol: "\n" as const, structural: true }; - if (at > 0 && lines[at - 1]!.eol === "") lines[at - 1] = { ...lines[at - 1]!, eol: "\n" }; - const next = [...lines.slice(0, at), inserted, ...lines.slice(at)]; - return applyEol(bom + sourceText(next), eol); + 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); } diff --git a/tests/clients/client-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts index c9f7e59f68f..8c3a1645ffc 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -134,6 +134,20 @@ function run(scenario: Scenario): Outcome { } describe("disconnect after Codex edited config.toml (O9)", () => { + 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"' }); From 688125678c3fa2d6d32dfc9c4270eb165c0b1a70 Mon Sep 17 00:00:00 2001 From: lidge-jun <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:00:11 +0900 Subject: [PATCH 14/15] test(link): byte-for-byte boundaries and ownership for putting the web_search line back BOM with a final line lacking a newline, no root lines, a root array, a journaled line with its own spacing and comment, and a remaining OpenCodex setting (config and journal untouched). --- .../client-disconnect-recovery.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/clients/client-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts index 8c3a1645ffc..4f471c13c0c 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -134,6 +134,30 @@ function run(scenario: Scenario): Outcome { } 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"' }); From 90cad14e56d0658e0aba9e2df41aea3d1934ebf4 Mon Sep 17 00:00:00 2001 From: lidge-jun <243035832+lidge-jun@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:58:52 +0900 Subject: [PATCH 15/15] fix(link): wait for config-directory ACL admission before taking the disconnect attempt lock The attempt lock is a file under the config directory; taking it before the parent ACL harden settled broke dev's contract (tests/windows/windows-config-root-integration-writers.test.ts) that nothing is created there first. The overlapping-disconnect test now runs the second disconnect in its own process, as a second ocx disconnect would. --- src/client/connect.ts | 3 +++ tests/clients/client-disconnect-recovery.test.ts | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/client/connect.ts b/src/client/connect.ts index ca857a7226c..8619d265831 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -1164,6 +1164,9 @@ 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); } diff --git a/tests/clients/client-disconnect-recovery.test.ts b/tests/clients/client-disconnect-recovery.test.ts index 4f471c13c0c..32c0720174a 100644 --- a/tests/clients/client-disconnect-recovery.test.ts +++ b/tests/clients/client-disconnect-recovery.test.ts @@ -86,8 +86,12 @@ function run(scenario: Scenario): Outcome { const actual = journal.restoreJournalState; const { spyOn } = require("bun:test"); spyOn(journal, "restoreJournalState").mockImplementation((...args) => { - if (!overlapping) overlapping = connectApi.disconnectClient({}, { lifecycleLockDeps: lockDeps }) - .then(() => null, cause => cause instanceof Error ? cause.message : String(cause)); + // 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); }); }