diff --git a/bin/ocx.mjs b/bin/ocx.mjs index dbff457b940..13e5de45e07 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -576,6 +576,8 @@ async function runPackageManagerSelfUpdate(manager, child) { } const env = mutationChildEnvironment(); delete env.OCX_SERVICE; + // Not launched by a supervisor: none of its markers (src/lib/supervisor-markers.ts) may ride along. + for (const name of ["OCX_DESKTOP_SUPERVISED", "OCX_SERVICE_MANAGED", "OCX_WINDOWS_WRAPPER_PROTOCOL"]) delete env[name]; // The restarted proxy is an ordinary owner; only a sibling's own replacement carries this. delete env.OCX_SIBLING_OF_PORT; delete env.OCX_SIBLING_HANDOFF_NONCE; diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index d3548773b63..521174150a3 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 advice for the state it observed: `ocx restart` on the Child when the same runtime still serves the old connection; `ocx service restart` when a service that cannot be shown to relaunch the runtime refused the restart; and inspection with `ocx status` and `ocx connect status` when a new connection is in use, the client state cannot be read, or a different or unknown runtime answers. A client runtime whose connection was removed accepts `ocx restart` when its owner can relaunch it: the OpenCodex desktop app, or a background service that is enabled and running, starts the standalone replacement, so the same command works over SSH with nobody at the Child's screen. A service that cannot be shown to relaunch it (for example a unit started by hand while disabled) keeps the runtime and the command says to run `ocx service restart`. A runtime that carries only `OCX_SERVICE=1` (a service installed before 2.49, or a proxy started by `ocx claude`, `ocx codex` or `ocx opencode`) is not changed by this: `ocx disconnect` does not restart it, and `ocx restart` refuses while its service is viable, as before. The same holds for a client runtime started by an OpenCodex version before this change: `ocx disconnect` restarts a runtime only after it has been restarted once on the new version (an update's own restart, the desktop app restarting it, `ocx service restart`, or `ocx restart`). + ## 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. @@ -276,6 +278,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/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index c2ad2913d08..63ba4826ae8 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1820,7 +1820,7 @@ "terminal-continuation-owner-rotation.test.ts": "adapters", "terminal-guard-server.test.ts": "server", "terminal-guard.test.ts": "server", - "test-home-guard.test.ts": "ci-workflows", "test-home-guard-unpreloaded.test.ts": "ci-workflows", + "test-home-guard.test.ts": "ci-workflows", "codex-home-sandbox.test.ts": "ci-workflows", "test-home-guard-unpreloaded.test.ts": "ci-workflows", "test-run-lock-generation.test.ts": "ci-workflows", "test-runner.test.ts": "ci-workflows", "test-runner-bun.test.ts": "ci-workflows", "test-runner-spawnsync-finalizer.test.ts": "ci-workflows", "test-sandbox-cleanup.test.ts": "ci-workflows", "test-temp-quarantine.test.ts": "ci-workflows", "thought-signature-credential-scope.test.ts": "responses", "token-estimate.test.ts": "lib", @@ -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/adapters/cursor/native-exec-desktop.ts b/src/adapters/cursor/native-exec-desktop.ts index 3a7b207ee89..d72608455a5 100644 --- a/src/adapters/cursor/native-exec-desktop.ts +++ b/src/adapters/cursor/native-exec-desktop.ts @@ -1,4 +1,5 @@ import { spawn } from "node:child_process"; +import { withoutSupervisorMarkers } from "../../lib/supervisor-markers"; import { shellInvocation } from "../../lib/win-exec"; import { create } from "@bufbuild/protobuf"; import { @@ -120,7 +121,7 @@ function runExternalJson(command: string, payload: unknown, config: DesktopExecu const inv = shellInvocation(command); const child = spawn(inv.file, inv.args, { cwd: config.cwd, - env: config.env ? { ...process.env, ...config.env } : process.env, + env: withoutSupervisorMarkers(config.env ? { ...process.env, ...config.env } : process.env), stdio: ["pipe", "pipe", "pipe"], ...inv.options, }); diff --git a/src/adapters/cursor/native-exec-shell.ts b/src/adapters/cursor/native-exec-shell.ts index 6dd9f64c897..9a1faf81a86 100644 --- a/src/adapters/cursor/native-exec-shell.ts +++ b/src/adapters/cursor/native-exec-shell.ts @@ -1,4 +1,5 @@ import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { withoutSupervisorMarkers } from "../../lib/supervisor-markers"; import { resolve } from "node:path"; import { foregroundShellUnavailableMessage, runForegroundShell, type CursorForegroundShellOwner } from "./native-foreground-shell"; import { create } from "@bufbuild/protobuf"; @@ -430,7 +431,7 @@ export function backgroundShellSpawnExec(execMsg: ExecServerMessage, sessionId: if (!admissionLease) return backgroundShellSpawnError(execMsg, args.command, cwd, "background shell limit reached"); let child: ChildProcessWithoutNullStreams; try { - child = backgroundShellRuntime.spawn(args.command, { cwd, shell: true }); + child = backgroundShellRuntime.spawn(args.command, { cwd, shell: true, env: withoutSupervisorMarkers(process.env) }); } catch (err) { admissionLease.release(); return backgroundShellSpawnError(execMsg, args.command, cwd, errorText(err)); 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/cli/claude.ts b/src/cli/claude.ts index 56de313bc04..3c26a55e4fc 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -37,6 +37,17 @@ import { desktopProfileWireAlias, legacyDesktopProfileWireAlias } from "../claud import { inspectDesktopFirstParty } from "../claude/desktop-first-party"; import { isClaudeInterceptProxyUrl, type ClaudeInterceptSettingsState } from "../claude/intercept/settings"; import { withoutSiblingMarker } from "../codex/sibling-start"; +import { withoutSupervisorMarkers } from "../lib/supervisor-markers"; + +/** + * The environment of the detached `ocx start` that `ocx claude` launches (the spawn adds the runtime + * provenance stamp): an ordinary owner. It + * carries OCX_SERVICE=1 but no service manager's marker, which is how a restart tells it apart from + * a manager-launched process (src/client/runtime-restart.ts classifyClientRestartSupervisor). + */ +export function claudeProxyStartEnv(base: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + return withoutSupervisorMarkers(withoutSiblingMarker({ ...base, OCX_SERVICE: "1" })); +} export interface ClaudeLaunchEnv { [key: string]: string | undefined; @@ -532,7 +543,7 @@ export async function ensureProxyForClaude(deps: ClaudeProxyEnsureDeps = {}): Pr stdio: "ignore", windowsHide: true, // An ordinary owner: a stray sibling marker would otherwise mark it before any probe. - env: withProcessRuntimeProvenance(withoutSiblingMarker({ ...process.env, OCX_SERVICE: "1" })), + env: withProcessRuntimeProvenance(claudeProxyStartEnv()), }); child.unref(); const deadline = Date.now() + 8_000; diff --git a/src/cli/codex-launcher.ts b/src/cli/codex-launcher.ts index 7b328602308..617e30edf84 100644 --- a/src/cli/codex-launcher.ts +++ b/src/cli/codex-launcher.ts @@ -38,6 +38,7 @@ import { attestCodexRuntime, codexCredentialDestinationIsTrusted, resolveCodexLa import { codexCommandCanBypass, CODEX_UNVERIFIED_SHIM } from "./codex-launch-command"; import { codexTokenArgsAreSafe, codexTokenEnvironmentIsSafe } from "./codex-launch-policy"; import { codexTokenWithheldDiagnostic, type CodexTokenRefusal, type CodexTokenRefusalReporter } from "./codex-launch-diagnostics"; +import { withoutSupervisorMarkers } from "../lib/supervisor-markers"; export interface CodexLaunchEnv { [key: string]: string | undefined; @@ -115,7 +116,8 @@ export function codexProxyStartEnv(base: CodexLaunchEnv = process.env): CodexLau const withTokenFile = stripped.OPENCODEX_API_AUTH_TOKEN?.trim() ? stripped : { ...stripped, OCX_API_TOKEN_FILE: serviceApiTokenFilePath() }; - return { ...withTokenFile, OCX_SERVICE: "1" } as CodexLaunchEnv; + // A shell a supervised runtime handed to a tool carries its supervisor's markers; this proxy is not its. + return withoutSupervisorMarkers({ ...withTokenFile, OCX_SERVICE: "1" }) as CodexLaunchEnv; } export type CodexProxyEnsureDeps = { 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/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/disconnect-runtime.ts b/src/cli/disconnect-runtime.ts new file mode 100644 index 00000000000..1fa46905f95 --- /dev/null +++ b/src/cli/disconnect-runtime.ts @@ -0,0 +1,168 @@ +/** + * `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 that runtime itself (the same restart as `ocx restart`, which a disconnected + * client runtime hands to whichever owner holds it), bound to the runtime generation it observed, + * and exits 0 only when no client runtime remained or a live standalone runtime answers afterwards. + * Every other outcome exits 1, so a Home-driven leave stops before its own `ocx restart`. + */ +import type { ProxyRestartIo, ProxyRestartLive, ProxyRestartRequestOutcome } from "./tray-proxy"; +import { RESTART_OWNERS, type RestartOwner } from "../client/runtime-restart"; + +function isHandledRestartOwner(value: string | undefined): value is RestartOwner { + return value !== undefined && (RESTART_OWNERS as readonly string[]).includes(value); +} + +/** + * One runtime process: its runtime record. The attestation secret is created per process + * (src/lib/local-management-attestation.ts createLocalAttestationSecret), so a new process at a + * reused pid has a different one. + */ +export interface RuntimeGeneration { + pid: number; + port: number; + hostname?: string; + attestationSecret: string; + /** The owner the runtime classified from positive evidence; absent for a bare flag or an older runtime. */ + restartOwner?: string; +} + +export type DisconnectClientState = + | { kind: "disconnected" } + | { kind: "connected" } + | { kind: "invalid" | "mismatched"; reason: string }; + +export interface DisconnectRuntimeDeps { + /** Liveness that separates a confirmed absence from a probe that could not decide. */ + observe: () => Promise< + | { kind: "live"; live: { pid: number | null; port: number; role?: string } } + | { kind: "absent" | "unknown"; live: null } + >; + /** The runtime record of the observed runtime, only when it names that pid and port. */ + capture: (live: { pid: number | null; port: number }) => RuntimeGeneration | null; + /** The durable client state, read right before the restart. */ + connectionState: () => DisconnectClientState; + /** + * `ocx restart` bound to this generation (see {@link bindRestartToRuntime}). `refusedBy` names a + * supervisor that refused it: an owning service manager whose relaunch could not be proven. + */ + restart: (target: RuntimeGeneration) => Promise<{ ok: boolean; refusedBy?: "service-unproven" }>; + json: boolean; + info?: (line: string) => void; + error?: (line: string) => void; +} + +/** + * Bind a restart to one runtime generation. The restart discovers its target again, and a connect + * that committed in between can have started a new client runtime there, even at a reused pid. + * The request goes out only for the captured pid and port, and the runtime record it attests and + * derives the restart capability from is the captured one: a different process cannot prove that + * secret, so it is refused before the restart request is sent, and its listener would reject the + * capability anyway. A runtime that is gone gets no generic start. + */ +export function bindRestartToRuntime( + io: ProxyRestartIo, + target: RuntimeGeneration, + request: (previous: ProxyRestartLive, readRuntime: (pid: number) => RuntimeGeneration | null) => ProxyRestartRequestOutcome | Promise, +): ProxyRestartIo { + return { + ...io, + startWhenStopped: () => ({ status: "skipped" }), + requestInPlaceRestart: previous => (previous.pid !== target.pid || previous.port !== target.port + ? { accepted: false, uncertain: false, error: new Error("restart_target_changed") } + : request(previous, pid => (pid === target.pid ? target : null))), + }; +} + +/** What is known about the runtime answering when the advice is given. */ +export type AnsweringRuntime = + | { kind: "captured"; pid: number | null; refusedBy?: "service-unproven" } + | { kind: "different"; pid: number | null } + | { kind: "unknown" } + | { kind: "none" }; + +/** + * The only source of recovery advice for every exit-1 outcome of this step. A plain + * `ocx restart` is advised only while the state read now is disconnected and either the captured + * runtime is still the one answering, or nothing answers at all (a start, nothing to interrupt). + * Any other state gets what was seen and `ocx status` inspection, never a restart. + */ +export function recoveryAdvice(state: DisconnectClientState, answering: AnsweringRuntime): string { + if (state.kind === "connected") { + return "A new connection is in use on this machine; inspect it with ocx status and ocx connect status and do not restart it. If this machine should leave its Home, run ocx disconnect again."; + } + if (state.kind !== "disconnected") { + return `The client state is ${state.kind} (${state.reason}). Inspect it with ocx connect status and ocx status, fix the client state, then run ocx disconnect again.`; + } + switch (answering.kind) { + case "captured": return answering.refusedBy + ? `The client runtime${answering.pid ? ` (pid ${answering.pid})` : ""} still serves the old connection, and its service manager did not allow a restart from here. Restart it through the manager: ocx service restart (check ocx service status).` + : `The client runtime${answering.pid ? ` (pid ${answering.pid})` : ""} still serves the old connection. Run on this machine: ocx restart`; + case "different": return `A different client runtime${answering.pid ? ` (pid ${answering.pid})` : ""} answers now, so it was not restarted; inspect it with ocx status and ocx connect status before restarting anything.`; + case "unknown": return "Which runtime answers could not be determined; inspect with ocx status and ocx connect status, and restart a client runtime only while ocx connect status reports disconnected."; + case "none": return "No runtime answers; start one with ocx restart (or the desktop app or ocx service start, whichever owns it)."; + } +} + +/** + * Exit 0 only when no client runtime remains and the state is disconnected: read once after the + * observation when no runtime or a non-client runtime answered, and again after a restart. Every + * exit-1 outcome is a headline plus {@link recoveryAdvice} computed from the state read at that point. + */ +export async function restartRuntimeAfterDisconnect(deps: DisconnectRuntimeDeps): Promise { + const info = deps.info ?? (line => console.log(line)); + const error = deps.error ?? (line => console.error(line)); + const fail = (headline: string, answering: AnsweringRuntime): number => { + error(`${headline} ${recoveryAdvice(deps.connectionState(), answering)}`); + return 1; + }; + const observed = await deps.observe(); + if (observed.kind === "unknown") { + return fail("Disconnected, but this machine could not confirm whether the client runtime stopped: a recorded OpenCodex process did not answer.", { kind: "unknown" }); + } + const client = observed.kind === "live" && observed.live.role === "client" ? observed.live : null; + // Captured before the state is read: a runtime a later connect starts is a later generation. + const target = client ? deps.capture(client) : null; + if (client && !target) { + return fail(`Disconnected, but this machine could not identify the running client runtime${client.pid ? ` (pid ${client.pid})` : ""}: its runtime record is missing or names another process, so it was not restarted.`, { kind: "unknown" }); + } + // Only a record that states an owner this step handles (written by a runtime with this change: + // the desktop app, a service manager with a current marker, or no supervisor) is restarted. Any + // other record, a bare OCX_SERVICE=1 runtime or one started before this change, is left exactly + // as origin/dev's `ocx disconnect` left it: no restart request, the disconnect's own exit 0. + if (target && !isHandledRestartOwner(target.restartOwner)) return 0; + const before = deps.connectionState(); + if (before.kind !== "disconnected") { + error(`Disconnect finished, but the client runtime was not restarted. ${recoveryAdvice(before, { kind: "unknown" })}`); + return 1; + } + if (!client || !target) 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; + let refusedBy: "service-unproven" | undefined; + try { ({ ok, refusedBy } = await deps.restart(target)); } finally { console.log = originalLog; } + const after = await deps.observe(); + if (after.kind === "live" && after.live.role !== "client") { + const late = deps.connectionState(); + if (late.kind !== "disconnected") { + error(`The client runtime was restarted, but the client state changed meanwhile. ${recoveryAdvice(late, { kind: "unknown" })}`); + return 1; + } + (deps.json ? error : info)("The client runtime was restarted; this machine now runs standalone OpenCodex."); + return 0; + } + if (after.kind === "live") { + const answering = deps.capture(after.live); + const who: AnsweringRuntime = !answering ? { kind: "unknown" } + : answering.attestationSecret === target.attestationSecret ? { kind: "captured", pid: after.live.pid, ...(refusedBy ? { refusedBy } : {}) } + : { kind: "different", pid: after.live.pid }; + if (ok && who.kind === "captured") { + return fail("The restart completed, but the runtime still answers as a client.", { kind: "unknown" }); + } + return fail(ok ? "The restart completed, but a client runtime answers." : "Disconnected, but the restart did not complete.", who); + } + return fail("Disconnected, but this machine could not confirm a standalone runtime afterwards.", after.kind === "absent" ? { kind: "none" } : { kind: "unknown" }); +} diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 6cbe047f2a1..b9150d3b7e0 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -1,4 +1,4 @@ -import type { ProxyRestartStartOutcome } from "./tray-proxy"; +import type { ProxyRestartIo, ProxyRestartStartOutcome } from "./tray-proxy"; import type { RestartSupervisor } from "./restart-failure"; /** * Registry-driven command dispatch (Phase 3 of the CLI deepening). @@ -60,7 +60,8 @@ export interface CliDispatchDeps { handleTrayProxyStart: (existingIsSuccess?: boolean) => Promise; handleTrayProxyRestart: () => Promise; handleRestartStartWhenStopped: (recoveringLiveRestart?: boolean) => Promise; - handleProxyRestart: (startWhenStopped: (recoveringLiveRestart: boolean) => Promise, onSupervised?: (supervisor: RestartSupervisor) => void) => Promise; + /** `bindIo` lets a caller bind the restart to one target (ocx disconnect: src/cli/disconnect-runtime.ts). */ + handleProxyRestart: (startWhenStopped: (recoveringLiveRestart: boolean) => Promise, onSupervised?: (supervisor: RestartSupervisor) => void, bindIo?: (io: ProxyRestartIo, deadlineAt: number) => ProxyRestartIo) => Promise; handleUninstall: () => Promise; handleStatus: () => Promise; handleRecoverHistory: () => Promise; @@ -423,7 +424,35 @@ 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)); + // Token recovery refuses any live client runtime and leaves nothing to restart. + if (code !== 0 || deps.args.includes("--orphaned-token")) return code; + const { bindRestartToRuntime, restartRuntimeAfterDisconnect } = await import("./disconnect-runtime"); + const { observeProxyLiveness } = await import("../server/proxy-liveness"); + const { readClientConnectionStateWithEvidence } = await import("../client/state"); + const { readRuntimePort } = await import("../config/process-state"); + const { requestBoundSystemRestart } = await import("./system-restart-client"); + return await restartRuntimeAfterDisconnect({ + observe: () => observeProxyLiveness({ attempts: 2 }), + capture: live => { + if (live.pid === null) return null; + const record = readRuntimePort(live.pid); + return record?.attestationSecret && record.pid === live.pid && record.port === live.port + ? { ...record, attestationSecret: record.attestationSecret } : null; + }, + connectionState: () => { + const state = readClientConnectionStateWithEvidence(); + return state.kind === "invalid" || state.kind === "mismatched" ? { kind: state.kind, reason: state.reason } : { kind: state.kind }; + }, + restart: async target => { + let refusedBy: "service-unproven" | undefined; + const ok = await deps.handleProxyRestart(deps.handleRestartStartWhenStopped, supervisor => { if (supervisor === "service-unproven") refusedBy = supervisor; }, (io, deadlineAt) => + bindRestartToRuntime(io, target, (previous, readRuntime) => + requestBoundSystemRestart(previous as Parameters[0], deadlineAt, { readRuntime: readRuntime as typeof readRuntimePort }))); + return { ok, ...(refusedBy ? { refusedBy } : {}) }; + }, + json: deps.args.includes("--json"), + }); }, catalog: async deps => { const { handleCatalogCommand } = await import("./catalog"); diff --git a/src/cli/index.ts b/src/cli/index.ts index 10e429277bc..870000f41dd 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1,7 +1,7 @@ #!/usr/bin/env bun -import { serviceStayOutExitCode, WINDOWS_WRAPPER_PROTOCOL_ENV } from "../service/windows-wrapper-exit"; +import { serviceStayOutExitCode } from "../service/windows-wrapper-exit"; import { isSupervisedServiceChild, serviceChildOwnershipDecisionForClassifiedChild } from "../service/service-child-ownership"; -import { SERVICE_MANAGED_ENV } from "../service/state"; +import { withoutSupervisorMarkers } from "../lib/supervisor-markers"; import { raiseWindowsProxyPriority } from "../service/windows-process-priority"; import { spawn } from "node:child_process"; import { homedir } from "node:os"; @@ -89,7 +89,7 @@ import { restartStartOutcome, waitForProxyReplacement, runTrayProxyStart, - type ProxyRestartLive, + type ProxyRestartIo, type ProxyRestartLive, type ProxyRestartStartOutcome, } from "./tray-proxy"; import { duplicateRuntimeMessage, runDesktopAwareProxyRestart } from "./desktop-runtime-guidance"; @@ -806,15 +806,13 @@ async function handleStart(options: { block?: boolean } = {}) { } function detachedStartEnvironment(): NodeJS.ProcessEnv { - const env: NodeJS.ProcessEnv = withoutSiblingMarker(process.env); + const env: NodeJS.ProcessEnv = withoutSupervisorMarkers(withoutSiblingMarker(process.env)); // Only a real service wrapper may claim supervision. A detached ensure/tray child is an // ordinary owner, never a sibling: while live it maintains routing, and on exit restores it. // The supervisor markers ride along whenever this runs inside a service child's // environment, so they must leave with OCX_SERVICE — otherwise the child would // answer the ownership gate as a managed job it is not. delete env.OCX_SERVICE; - delete env[SERVICE_MANAGED_ENV]; - delete env[WINDOWS_WRAPPER_PROTOCOL_ENV]; return withProcessRuntimeProvenance(env); } @@ -957,10 +955,10 @@ async function handleTrayProxyStart(existingIsSuccess = true): Promise const PROXY_RESTART_OBSERVE_MS = MEMORY_DRAIN_RESTART_MS + REPLACEMENT_READY_TIMEOUT_MS + 15_000; /** Reserve confirmation time within the shared restart deadline. */ async function handleProxyRestart( - startWhenStopped: (recoveringLiveRestart: boolean) => Promise, onSupervised?: (supervisor: RestartSupervisor) => void, + startWhenStopped: (recoveringLiveRestart: boolean) => Promise, onSupervised?: (supervisor: RestartSupervisor) => void, bindIo: (io: ProxyRestartIo, deadlineAt: number) => ProxyRestartIo = io => io, ): Promise { const deadlineAt = Date.now() + PROXY_RESTART_OBSERVE_MS; - const result = await runDesktopAwareProxyRestart({ + const result = await runDesktopAwareProxyRestart({ ...bindIo({ findLive: () => discoverStableProxyForRestart({ findLive: () => findLiveProxy({ deadlineAt, attempts: 2, acceptPackageTreeFenced: true }), expired: () => Date.now() >= deadlineAt, @@ -977,7 +975,7 @@ async function handleProxyRestart( recheckAfterFailedStart: () => recheckRestartFailedStart(end => discoverStableProxyForRestart({ findLive: () => findLiveProxy({ deadlineAt: end, attempts: 2, acceptPackageTreeFenced: true }), expired: () => Date.now() >= end, })), - }); + }, deadlineAt) }); if (!result.ok && result.phase === "request" && result.error instanceof UpdateRestartRequired) { const candidate = result.error.candidate(); console.log(`🔄 Running proxy ${candidate.target.version} is older than this CLI (${candidate.cliVersion}); restarting it from the current installation...`); @@ -1979,7 +1977,7 @@ process.exit(await dispatchCommand(head, { detached: true, stdio: "ignore", windowsHide: true, - env: withProcessRuntimeProvenance(withoutSiblingMarker(process.env)), + env: withProcessRuntimeProvenance(withoutSupervisorMarkers(withoutSiblingMarker(process.env))), }); child.unref(); }, 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/cli/opencode.ts b/src/cli/opencode.ts index ab66e8dd253..39989946744 100644 --- a/src/cli/opencode.ts +++ b/src/cli/opencode.ts @@ -54,6 +54,7 @@ import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; import { selfLaunchArgv } from "../lib/self-launch-argv"; import { withoutSiblingMarker } from "../codex/sibling-start"; import { parseJsonc } from "../lib/jsonc"; +import { withoutSupervisorMarkers } from "../lib/supervisor-markers"; export { parseJsonc }; /** @@ -575,7 +576,8 @@ export function opencodeProxyStartEnv(base: OpencodeLaunchEnv = process.env): Op const withTokenFile = base.OPENCODEX_API_AUTH_TOKEN?.trim() ? base : serviceTokenLookupEnv(base); - return { ...withTokenFile, OCX_SERVICE: "1" }; + // A shell a supervised runtime handed to a tool carries its supervisor's markers; this proxy is not its. + return withoutSupervisorMarkers({ ...withTokenFile, OCX_SERVICE: "1" }); } /** diff --git a/src/cli/restart-failure.ts b/src/cli/restart-failure.ts index 5352cdf19c1..89fb5526077 100644 --- a/src/cli/restart-failure.ts +++ b/src/cli/restart-failure.ts @@ -1,12 +1,13 @@ import type { ProxyRestartResult } from "./tray-proxy"; -export type RestartSupervisor = "desktop" | "service"; +export type RestartSupervisor = "desktop" | "service" | "service-unproven"; /** Only the bound client's explicit refusal establishes a supervisor for sync. */ export function supervisedRestartRefusal(result: ProxyRestartResult): RestartSupervisor | null { if (result.ok || result.phase !== "request" || !(result.error instanceof Error)) return null; if (result.error.message === "restart_supervised_desktop") return "desktop"; if (result.error.message === "restart_supervised_service") return "service"; + if (result.error.message === "restart_supervised_service-unproven") return "service-unproven"; return null; } @@ -35,8 +36,10 @@ export function reportRestartFailure(result: Extract { console.error("The Home's Codex settings were applied, but the apply record could not be written; the client runtime was not restarted. Run 'ocx sync' again."); process.exitCode = 1; } + if (result.journalHandoverFailed) { + console.error("Codex routing was applied, but the Codex journal could not be handed to this connection (the Codex config was busy or changed, or the journal belongs to an earlier connection), so the client runtime was not restarted: the next OpenCodex restart could restore native routing. Run 'ocx sync' again; if it repeats, run 'ocx sync --restart-codex' after that restart."); + process.exitCode = 1; + } // OCX-6: a link Child's runtime must have started after the apply it serves under. - if (link && result.appliedRecord) { + // Not after a failed handover: that restart is the one that would undo this routing. + if (link && result.appliedRecord && !result.journalHandoverFailed) { const { applyRestartMessage, restartRuntimeAfterApply } = await import("./link-apply-restart"); const { sameClientConnectionOwner } = await import("../client/state"); const applied = result.connection; diff --git a/src/cli/system-restart-client.ts b/src/cli/system-restart-client.ts index 4d5b5b53502..9e3f5542b6b 100644 --- a/src/cli/system-restart-client.ts +++ b/src/cli/system-restart-client.ts @@ -226,13 +226,14 @@ export async function requestBoundSystemRestart( * A client runtime under the desktop app or a service answers 409 `restart_supervised` and keeps * serving (OCX-6). Read that small body and return the validated supervisor, or null. */ -async function supervisedRefusal(response: Response): Promise<"service" | "desktop" | null> { +async function supervisedRefusal(response: Response): Promise<"service" | "desktop" | "service-unproven" | null> { try { const read = await readBoundedResponseBytes(response, { maxBytes: 1024, signal: AbortSignal.timeout(SYSTEM_RESTART_REQUEST_TIMEOUT_MS) }); if (read.oversized) return null; const body = JSON.parse(new TextDecoder().decode(read.bytes)) as { error?: { code?: unknown; supervisor?: unknown } } | null; if (body?.error?.code !== RESTART_SUPERVISED_ERROR) return null; - return body.error.supervisor === "service" || body.error.supervisor === "desktop" ? body.error.supervisor : null; + const supervisor = body.error.supervisor; + return supervisor === "service" || supervisor === "desktop" || supervisor === "service-unproven" ? supervisor : null; } catch { return null; } diff --git a/src/client/connect.ts b/src/client/connect.ts index e9b7b79a37a..19c5c2dab6e 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -29,6 +29,8 @@ import { type CodexRoutingTarget, } from "../codex/inject"; import { + adoptJournalForClient, + journalHandoverIncomplete, journalOwner, restoreJournalState, } from "../codex/journal"; @@ -790,6 +792,24 @@ async function connectClientUnlocked( commitClientConnection(connection); committed = true; }), deps.lifecycleLockDeps); + // Only now does the routing belong to this connection: take over a journal another owner wrote + // (src/codex/journal.ts adoptJournalForClient). A failure keeps the connection; sync retries. + if (injectionCommitted) { + try { + const adoption = adoptJournalForClient(apiKeyId, () => { + // Under the Codex lock: still this committed connection (same owner and link) and its + // token, or write nothing. The stored record is normalized, so identity is compared. + assertNoClientDisconnectPending(); + const current = readClientConnectionState(); + if (current.kind !== "connected" || !sameClientConnectionOwner(current.value, connection) + || current.value.link?.linkId !== connection.link?.linkId) throw new Error("client_connection_changed"); + const token = readServiceApiTokenState(); + if (token.kind !== "present" || token.fingerprint !== connection.tokenFingerprint) throw new Error("client_token_changed"); + }); + if (journalHandoverIncomplete(adoption)) console.warn(FOREIGN_JOURNAL_WARNING); + } + catch { console.warn("Warning: the Codex journal could not be handed to this connection; run 'ocx sync' so a supervised restart keeps Codex routing."); } + } // Recorded only when the injection that applied it ran (not with --no-sync). The connection is // already committed, so a failed record write must not reach the rollback below: it is a // warning, and `ocx status` reports no recorded apply until `ocx sync` writes one. @@ -855,6 +875,8 @@ export interface ConnectedSyncResult { appliedRecord?: HomeCodexSettingsRecord; /** The Home's settings were injected but their record could not be written (OCX-6). */ applyRecordFailed?: true; + /** Routing was injected but the Codex journal stayed with its previous owner; sync again. */ + journalHandoverFailed?: true; /** * The connection this sync ran under, read inside its lock and compare-and-swapped on every * write. Callers decide link-vs-hub wording and restart ownership from this, never from a read @@ -964,6 +986,7 @@ async function syncConnectedClientUnlocked( let homeSettings: HomeCodexSettingsSource | undefined; let appliedRecord: HomeCodexSettingsRecord | undefined; let applyRecordFailed = false; + let journalHandoverFailed = false; if (next.selectedClients.includes("codex")) { let config = loadConfig(); // #6957: a link Child applies the Home's OpenCodex-owned Codex settings with the catalog. @@ -984,6 +1007,13 @@ async function syncConnectedClientUnlocked( }); if (!result.success || result.status === "skipped") throw new Error(result.message); injected = true; + // A committed connection re-injected: own the journal too (retries a takeover connect missed), + // under the same connection guard as the injection, so a stale sync claims nothing. + try { + const adoption = adoptJournalForClient(next.apiKeyId, beforeClientWrite); + // No journal or another client's means the routing is not this connection's to keep. + if (journalHandoverIncomplete(adoption)) journalHandoverFailed = true; + } catch { journalHandoverFailed = true; } if (homeSettings && next.link) { const linkId = next.link.linkId; const settings = homeSettings; @@ -1012,6 +1042,7 @@ async function syncConnectedClientUnlocked( ...(homeSettings ? { homeSettings } : {}), ...(appliedRecord ? { appliedRecord } : {}), ...(applyRecordFailed ? { applyRecordFailed } : {}), + ...(journalHandoverFailed ? { journalHandoverFailed: true as const } : {}), connection: next, }; } @@ -1053,6 +1084,8 @@ function disconnectAtLeast(receipt: DesktopDisconnectReceipt, phase: DesktopDisc return DISCONNECT_PHASES.indexOf(receipt.phase) >= DISCONNECT_PHASES.indexOf(phase); } +const FOREIGN_JOURNAL_WARNING = "Warning: the Codex journal belongs to an earlier connection, so the next OpenCodex restart restores native Codex routing. After that restart, run 'ocx sync --restart-codex' to point Codex at this connection again."; + function catalogIsRecordedPrior(connection: OcxClientConnectionConfig, snapshot: CatalogSnapshot): boolean { return snapshot.kind === "file" && !!connection.priorCatalog && snapshot.fingerprint === sha256(Buffer.from(connection.priorCatalog, "base64").toString("utf8")); @@ -1128,7 +1161,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/src/client/machine-api.ts b/src/client/machine-api.ts index e573050dbb4..5020b3bf985 100644 --- a/src/client/machine-api.ts +++ b/src/client/machine-api.ts @@ -115,6 +115,10 @@ export async function handleMachineApi( input.restartCodex === undefined ? {} : { restartCodex: input.restartCodex }, ); deps.setHubReachability?.("online"); + // A sync whose journal handover failed is not complete: a supervised restart could undo it. + if (result.journalHandoverFailed) { + return Response.json({ success: false, ...result, error: "Codex routing was applied, but the Codex journal could not be handed to this connection; sync again, and if it repeats, sync once more after the next OpenCodex restart." }, { status: 409 }); + } return Response.json({ success: true, ...result }); } catch (error) { const message = error instanceof Error ? error.message : "client sync failed"; diff --git a/src/client/runtime-restart.ts b/src/client/runtime-restart.ts index 899fb165112..0688fc03c04 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 @@ -22,11 +27,22 @@ import { isDesktopSupervised, } from "../lib/system-restart-contract"; import type { OcxClientConnectionConfig } from "../types"; +import { isMarkedSupervisedServiceChild } from "../service/service-child-ownership"; import { sameClientConnectionOwner, type readClientConnectionState } from "./state"; type ClientConnectionState = ReturnType; -export type ClientRestartSupervisor = "service" | "desktop"; +/** + * `service-unproven`: a service manager's marker shows it launched and owns this process, but its + * relaunch cannot be proven (for example a systemd unit started by hand while disabled). Neither + * exiting for it nor detaching from it is safe, so the restart is refused. + */ +export type ClientRestartSupervisor = "service" | "desktop" | "service-unproven"; + +/** Verdicts that keep the runtime: an owner exists, but exiting for it or detaching is not proven safe. */ +export function restartRefusedByOwner(supervisor: ClientRestartSupervisor | null): supervisor is "service-unproven" { + return supervisor === "service-unproven"; +} export type ClientRestartAdmission = | { kind: "accepted"; alreadyRestarting: boolean } @@ -38,6 +54,18 @@ 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; + /** + * Positive evidence of who owns this process ({@link restartOwnerProven}). Without it the + * ended-connection recycle is never reached and the restart is origin/dev's. + */ + ownerProven?: () => boolean; + /** + * Hand off to what the durable state now calls for; exits the process for its owner to relaunch. + * Receives the same supervisor verdict as the refusal path (a service only when it is viable). + */ + recycle?: (supervisor: ClientRestartSupervisor | null) => 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 +125,26 @@ export function createClientRestartController(io: ClientRestartIo): ClientRestar hasHandoff: () => flight !== null, async request() { if (io.stopping?.()) return { kind: "stopping" }; + // Only positive ownership evidence opens the ended-connection recycle; without it (a bare + // OCX_SERVICE=1) the request takes origin/dev's path below. + if (io.recycle && (io.ownerProven?.() ?? true) && io.connectionChanged?.()) { + if (flight) return { kind: "accepted", alreadyRestarting: true }; + // An owner that cannot be proven to relaunch this process keeps it, connection ended or not. + const owner = await io.supervisor(); + if (io.stopping?.()) return { kind: "stopping" }; + if (restartRefusedByOwner(owner)) return { kind: "supervised", supervisor: owner }; + if (flight) return { kind: "accepted", alreadyRestarting: true }; + const recycle = io.recycle, changed = io.connectionChanged; + // Let the 202 reach the CLI before the listener closes, then confirm the connection is + // still ended: a connect that committed meanwhile keeps this runtime (the CLI reports kept). + // The recheck is the last step before the recycle, after the (asynchronous) supervisor lookup. + flight = sleep(200).then(async () => { + const supervisor = await io.supervisor(); + if (restartRefusedByOwner(supervisor)) { warn("[client] restart: this runtime's owner could not be proven to relaunch it; it keeps serving"); return; } + if (changed()) await recycle(supervisor); + }).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 }; @@ -112,11 +160,59 @@ export function createClientRestartController(io: ClientRestartIo): ClientRestar /** Production supervision check: the desktop app, then a service that will really relaunch us. */ export async function clientRestartSupervisor(): Promise { - if (isDesktopSupervised()) return "desktop"; - if (process.env.OCX_SERVICE !== "1") return null; - // Same viability test as the standalone restart: stale service assets would not relaunch. + const { diagnoseService } = await import("../service/diagnostics"); const { isServiceViable } = await import("../service/orchestration"); - return isServiceViable() ? "service" : null; + return classifyClientRestartSupervisor({ desktop: isDesktopSupervised(), env: process.env, diagnose: diagnoseService, legacyServiceViable: isServiceViable }); +} + +/** + * Positive evidence of who owns this process: the desktop app, a service manager's marker + * (OCX_SERVICE_MANAGED or the Windows wrapper's protocol marker, read from the environment only), + * or no OCX_SERVICE at all (every service OpenCodex registers sets it). No inference: a legacy + * Windows service recognised only from its parent's command line counts as a bare OCX_SERVICE=1. + * A bare OCX_SERVICE=1 is none of these: a service registered before the manager marker existed + * and the proxies of ocx claude, ocx codex and ocx opencode all carry it. + */ +export function restartOwnerProven(input: { desktop: boolean; env: NodeJS.ProcessEnv; managed?: (env: NodeJS.ProcessEnv) => boolean }): boolean { + return restartOwnerEvidence(input) !== null; +} + +/** The owners this change handles, as a client runtime states them in its record (`restartOwner`). */ +export type RestartOwner = "desktop" | "service-manager" | "none"; +export const RESTART_OWNERS: readonly RestartOwner[] = ["desktop", "service-manager", "none"]; + +/** The classified owner from positive evidence, or null for a bare OCX_SERVICE=1 ({@link restartOwnerProven}). */ +export function restartOwnerEvidence(input: { desktop: boolean; env: NodeJS.ProcessEnv; managed?: (env: NodeJS.ProcessEnv) => boolean }): RestartOwner | null { + if (input.desktop) return "desktop"; + if ((input.managed ?? isMarkedSupervisedServiceChild)(input.env)) return "service-manager"; + return input.env.OCX_SERVICE !== "1" ? "none" : null; +} + +/** + * Ownership and relaunch are separate facts. With a manager's marker, only a service diagnostic + * that is viable and running lets this process exit for the manager to relaunch it; otherwise it is + * `service-unproven` and keeps the runtime. A bare OCX_SERVICE=1 keeps origin/dev's decision + * unchanged (b1148220574917c8 src/client/runtime-restart.ts:114-119): a viable service, else none. + */ +export function classifyClientRestartSupervisor(input: { + desktop: boolean; + env: NodeJS.ProcessEnv; + diagnose: () => { viable: boolean; running: boolean }; + /** origin/dev's check for a bare OCX_SERVICE=1: src/service/orchestration.ts isServiceViable. */ + legacyServiceViable: () => boolean; + /** Manager-launched evidence; defaults to the current markers only (isMarkedSupervisedServiceChild). */ + managed?: (env: NodeJS.ProcessEnv) => boolean; +}): ClientRestartSupervisor | null { + if (input.desktop) return "desktop"; + // The managed marker (launchd, systemd, WinSW) or the Windows wrapper's protocol marker. + if (!(input.managed ?? isMarkedSupervisedServiceChild)(input.env)) { + if (input.env.OCX_SERVICE !== "1") return null; + return input.legacyServiceViable() ? "service" : null; + } + // Relaunch needs positive evidence: viable AND running (launchd's viable also covers an + // unknown load state for its other callers; running there means loaded). + const diagnostic = input.diagnose(); + return diagnostic.viable && diagnostic.running ? "service" : "service-unproven"; } /** @@ -133,3 +229,15 @@ export function clientRestartRollbackAction( ? "resume" : "recycle"; } + +/** + * Whether the served connection has provably ended: the durable state is disconnected, or names a + * different connection. Unreadable or mismatched state is not proof, so a supervised restart keeps + * refusing on it instead of exiting a runtime whose link may still work. + */ +export function clientConnectionEnded(served: OcxClientConnectionConfig, current: ClientConnectionState): boolean { + // Callers pass readClientConnectionStateWithEvidence(): "disconnected" then means a saved config + // with no client, never a missing file. + if (current.kind === "disconnected") return true; + return current.kind === "connected" && clientRestartRollbackAction(served, current) === "recycle"; +} diff --git a/src/client/runtime.ts b/src/client/runtime.ts index 75e3462f147..4df1fc535c8 100644 --- a/src/client/runtime.ts +++ b/src/client/runtime.ts @@ -28,11 +28,12 @@ import { clientInitiatedLinkReadiness, createHomeInitiatedLinkReadiness } from " import { createClientLinkSupervisor, type ClientLinkSupervisor } from "./link-tunnel"; import { clientLinkStatePath, isChildInitiatedLink, recordChildInitiatedLink } from "./link-state"; import { startMachineListener, type MachineListenerDeps } from "./machine-listener"; -import { isLinkConnection, readClientConnectionState } from "./state"; +import { isLinkConnection, readClientConnectionState, readClientConnectionStateWithEvidence } from "./state"; import { withClientLifecycleSync } from "./lifecycle-lock"; -import { clientRestartRollbackAction, clientRestartSupervisor, createClientRestartController, type ClientRestartController } from "./runtime-restart"; +import { clientConnectionEnded, clientRestartRollbackAction, clientRestartSupervisor, createClientRestartController, restartOwnerEvidence, type ClientRestartController, type RestartOwner } from "./runtime-restart"; import { createLinkParityReporter, type LinkParityReporter } from "./link-parity-reporter"; import { restoreLinkClientRouting } from "./stop-routing"; +import { SUPERVISOR_MARKER_ENVS } from "../lib/supervisor-markers"; import { markClientRuntimeStopping, markClientRuntimeStoppingWithRetry } from "./sync-runtime"; let activeServer: Server | null = null; @@ -83,6 +84,8 @@ export interface StandaloneRecycleIo { exitProcess?: (code: number) => void; configuredPort?: () => number | undefined; isDesktopSupervised?: () => boolean; + /** A service that will really relaunch this process; defaults to the OCX_SERVICE marker. */ + serviceSupervised?: () => boolean; } function cleanup(): void { @@ -99,19 +102,30 @@ export function clientRuntimeRecord( pid: number, port: number, attestationSecret: string = createLocalAttestationSecret(), + owner: RestartOwner | null = classifiedOwner(), ): RuntimePortState { let command: string[] | undefined; try { command = currentServingCommand(); } catch { /* no reliable relaunch identity; update admission fails closed */ } - return { pid, port, hostname: "127.0.0.1", attestationSecret, ...(command ? { command } : {}), ...siblingRuntimeField(), ...(clientStopping ? { clientStopping: true as const } : {}) }; + return { pid, port, hostname: "127.0.0.1", attestationSecret, ...(command ? { command } : {}), ...siblingRuntimeField(), ...(clientStopping ? { clientStopping: true as const } : {}), ...(owner ? { restartOwner: owner } : {}) }; } +/** Evaluated once: the launch environment and parent of this process do not change. */ +let ownerMemo: { value: RestartOwner | null } | undefined; +function classifiedOwner(): RestartOwner | null { + ownerMemo ??= { value: restartOwnerEvidence({ desktop: isDesktopSupervised(), env: process.env }) }; + return ownerMemo.value; +} +const ownerProven = (): boolean => classifiedOwner() !== null; + export function standaloneRecycleEnv( env: NodeJS.ProcessEnv, disconnectedTokenFingerprint: string, ): NodeJS.ProcessEnv { const childEnv = { ...env }; - // A detached replacement is not the desktop app's child. - delete childEnv[DESKTOP_SUPERVISED_ENV]; + // A detached replacement is nobody's child: not the desktop app's, not a service's, not a Windows + // wrapper's. It must carry no supervisor marker it would otherwise misread as its own. + for (const name of SUPERVISOR_MARKER_ENVS) delete childEnv[name]; + delete childEnv.OCX_SERVICE; const admissionToken = childEnv.OPENCODEX_API_AUTH_TOKEN?.trim(); if (admissionToken && serviceApiTokenFingerprint(admissionToken) !== disconnectedTokenFingerprint) { // A surviving env token shadows OCX_API_TOKEN_FILE entirely, so nothing below can @@ -164,7 +178,8 @@ export async function recycleStandalone( const port = activePort ?? (io.configuredPort ?? configuredTcpPort)(); const exit = io.exitProcess ?? ((code: number) => { process.exit(code); }); // Capture the one-use sibling handoff before this process removes its runtime record. - const replacementEnv = port && process.env.OCX_SERVICE !== "1" + const serviceSupervised = (io.serviceSupervised ?? (() => process.env.OCX_SERVICE === "1"))(); + const replacementEnv = port && !serviceSupervised ? withSiblingMarker(standaloneRecycleEnv(process.env, disconnectedTokenFingerprint), issueSiblingHandoff) : null; try { @@ -201,7 +216,7 @@ export async function recycleStandalone( // // launchd's KeepAlive is failure-only too (`SuccessfulExit` false), so exit 1 is what // relaunches it as well; an exit 0 would leave the job stopped. - if (process.env.OCX_SERVICE === "1") { + if (serviceSupervised) { exit(1); return; } @@ -433,6 +448,12 @@ export async function startClientRuntime( const restartController: ClientRestartController = createClientRestartController({ stopping: () => shuttingDown, supervisor: io.restartSupervisor ?? clientRestartSupervisor, + connectionChanged: () => clientConnectionEnded(state.value, readClientConnectionStateWithEvidence()), + ownerProven, + recycle: supervisor => recycleStandalone(state.value.tokenFingerprint, { + isDesktopSupervised: () => supervisor === "desktop", + serviceSupervised: () => supervisor === "service", + }), stop: stopServing, spawnReplacement: async deadlineAt => { const { replacementStartEnvironment } = await import("../server/management/system-restart"); diff --git a/src/client/state.ts b/src/client/state.ts index deaaffe1c2f..132475860f3 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -11,6 +11,7 @@ import { withConfigMutationLockSync, } from "../config"; import { atomicWriteFileNoFollowUnclaimed } from "../config/atomic-write"; +import { readConfigFileSnapshot } from "../config/diagnostics"; import type { OcxClientConnectionConfig } from "../types"; import { inspectRemoteDesktopStore, readDesktopDisconnectReceipt } from "../claude/desktop-remote-store"; import { withClientLifecycleSync, type ClientLifecycleLockDeps } from "./lifecycle-lock"; @@ -82,6 +83,27 @@ function rawTopLevelConfig(): Record | null { } } +/** + * The client state with evidence required for "disconnected", decided from ONE read of the saved + * file (readConfigFileSnapshot): the bytes and their validation belong together, so a connect that + * commits meanwhile cannot pair old bytes with a validation of new ones. "disconnected" means a + * saved config.json that validates and records no client; a missing, unreadable or invalid file is + * invalid with its reason. Used wherever "disconnected" authorizes a restart, a recycle or exit 0. + */ +export function readClientConnectionStateWithEvidence(): ClientConnectionState { + const snapshot = readConfigFileSnapshot({ passive: true }); + if (snapshot.raw === undefined) { + return { kind: "invalid", reason: snapshot.diagnostics.source === "default" ? "config.json is missing" : "config.json is unreadable" }; + } + if (snapshot.diagnostics.source !== "file") { + return { kind: "invalid", reason: `config.json failed validation (${snapshot.diagnostics.error ?? "invalid"})` }; + } + let parsed: unknown; + try { parsed = JSON.parse(snapshot.raw.replace(/^\uFEFF/, "")); } catch { return { kind: "invalid", reason: "config.json is unreadable" }; } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return { kind: "invalid", reason: "config.json is not an object" }; + return classifyClientConnection(parsed as Record, snapshot.diagnostics); +} + export function readClientConnectionState(): ClientConnectionState { const raw = rawTopLevelConfig(); const diagnostics = readConfigDiagnostics({ passive: true }); @@ -90,6 +112,14 @@ export function readClientConnectionState(): ClientConnectionState { ? { kind: "disconnected" } : { kind: "invalid", reason: "config.json is missing or unreadable" }; } + return classifyClientConnection(raw, diagnostics); +} + +/** The client verdict for one saved top-level object and the diagnostics of the same bytes. */ +function classifyClientConnection( + raw: Record, + diagnostics: Pick, "config" | "warnings">, +): ClientConnectionState { const hasClient = Object.hasOwn(raw, "client") && raw.client !== undefined; const role = raw.runtimeRole; if (role !== undefined && role !== "standalone" && role !== "hub" && role !== "client") { diff --git a/src/codex/journal.ts b/src/codex/journal.ts index fb8311a551e..70fd3dcb17e 100644 --- a/src/codex/journal.ts +++ b/src/codex/journal.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto"; import type { AtomicWriteHooks } from "../config/atomic-write"; import { existsSync, lstatSync, readFileSync, unlinkSync } from "node:fs"; import { join } from "node:path"; -import { atomicWriteFile } from "../config"; +import { atomicWriteFile, withConfigMutationLockSync } from "../config"; import { assertCodexHomeOwner, codexHomeIsAbsent, CODEX_HOME_JOURNAL_FILE, CodexHomeOwnerRefusal, opencodexHomeForInjection, readCodexHomeJournal, type CodexHomeOwnerRefusalReason } from "./codex-home-owner"; import { hasInjectedCodexRouting } from "./injected-marker"; import { configWriteLockFailureMessage, withConfigWriteLockHeld } from "./config-write-lock"; @@ -494,6 +494,54 @@ export interface ReconcileJournalOptions { adoptUpdateHandover?: boolean; } +/** + * Called by `connectClient` right after its connection commits, when its injection ran: the + * client takes over a journal another owner wrote (a standalone proxy's injection it re-injected + * over), keeping the original snapshot. Before the commit the journal stays its writer's, so a + * runtime starting mid-connect sees a live process owner and leaves it alone. Without the takeover + * 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. + * Only a process-written journal is taken: another client's journal belongs to a later connection + * (a stale sync must never claim it), and `beforeWrite` re-runs the caller's connection guard under + * the Codex config lock and the config mutation lock together with the write, so a connection that + * changed since its injection writes nothing and none can change before the write lands. + * Outcomes: `adopted`, `owned` (already this client's), `missing` (no journal), `foreign` (another + * client's journal, left alone; see {@link journalHandoverIncomplete}). + */ +export type JournalAdoption = "adopted" | "owned" | "missing" | "foreign"; + +/** + * Whether routing is still at risk after a handover. No journal (external providers leave none) + * gives startup recovery nothing to restore; only another client's journal is restored over this + * connection on the next start. + */ +export function journalHandoverIncomplete(adoption: JournalAdoption): boolean { + return adoption === "foreign"; +} + +export function adoptJournalForClient(apiKeyId: string, beforeWrite: () => void = () => {}): JournalAdoption { + if (codexHomeIsAbsent(CODEX_HOME)) return "missing"; + const locked = withConfigWriteLockHeld(CODEX_CONFIG_PATH, undefined, (handle): JournalAdoption => { + assertCodexHomeOwner(CODEX_HOME); + beginCodexWriteSection(handle); + // Only a confirmed absence is safe; an unreadable journal may still hold a restorable owner. + const inspection = readCodexHomeJournal(JOURNAL_PATH); + if (inspection.kind === "unknown") throw new CodexHomeOwnerRefusal("owner-unknown"); + if (inspection.kind === "missing") return "missing"; + const journal = inspection.journal as unknown as Journal; + if (journal.owner?.kind === "client") return journal.owner.apiKeyId === apiKeyId ? "owned" : "foreign"; + // Codex lock, then the mutation lock (the injection's order): guard and write as one step. + return withConfigMutationLockSync(() => { + beforeWrite(); + const adopted = JSON.stringify({ ...journal, owner: { kind: "client", apiKeyId } }); + publishCodexArtifact(JOURNAL_PATH, handle, (path, hooks) => atomicWriteFile(path, adopted, undefined, hooks)); + return "adopted" as const; + }); + }); + if (!locked.ok) throw new Error(locked.error === "unsafe" ? configWriteLockFailureMessage(locked) : "the Codex config lock is busy"); + return locked.value; +} + export function reconcileJournal(options: ReconcileJournalOptions = {}): boolean { if (codexHomeIsAbsent(CODEX_HOME)) return false; const locked = withConfigWriteLockHeld(CODEX_CONFIG_PATH, undefined, handle => { diff --git a/src/config/process-state.ts b/src/config/process-state.ts index a80bdefa035..3f4b8db8e58 100644 --- a/src/config/process-state.ts +++ b/src/config/process-state.ts @@ -158,6 +158,13 @@ export type RuntimePortState = { attestationSecret?: string; /** A Link runtime is draining; new and in-flight sync commits must stand down. */ clientStopping?: true; + /** + * The owner a client runtime classified from positive evidence (src/client/runtime-restart.ts + * restartOwnerEvidence: desktop, service-manager or none). Absent for a bare OCX_SERVICE=1 and for + * runtimes older than this field; `ocx disconnect` restarts only a runtime whose record states a + * value it recognises. Any string is accepted here so a later value never invalidates the record. + */ + restartOwner?: string; /** * The live proxy's port when this runtime is a sibling instance started beside it * (`src/codex/sibling-start.ts`). `ocx stop` reads it to leave shared client routing alone. @@ -188,6 +195,7 @@ export function isValidRuntimePortState(value: unknown): value is RuntimePortSta && hostnameOk && attestationOk && (state.clientStopping === undefined || state.clientStopping === true) + && (state.restartOwner === undefined || (typeof state.restartOwner === "string" && state.restartOwner.length <= 64)) && siblingOk; } diff --git a/src/lib/supervisor-markers.ts b/src/lib/supervisor-markers.ts new file mode 100644 index 00000000000..00d074ebd61 --- /dev/null +++ b/src/lib/supervisor-markers.ts @@ -0,0 +1,18 @@ +/** + * The environment markers a supervisor writes for the one process it launches: the desktop app + * (OCX_DESKTOP_SUPERVISED), a service manager (OCX_SERVICE_MANAGED: launchd, systemd, WinSW) and the + * Windows Task Scheduler wrapper (OCX_WINDOWS_WRAPPER_PROTOCOL). A marker proves only that the + * process was started with it, so it must never reach a process that supervisor did not launch: a + * detached proxy, a replacement, or a shell handed to a tool. Every such launch removes them with + * {@link withoutSupervisorMarkers}. The values match DESKTOP_SUPERVISED_ENV + * (src/lib/system-restart-contract.ts), SERVICE_MANAGED_ENV (src/service/state.ts) and + * WINDOWS_WRAPPER_PROTOCOL_ENV (src/service/windows-wrapper-exit.ts); a test pins that. + */ +export const SUPERVISOR_MARKER_ENVS = ["OCX_DESKTOP_SUPERVISED", "OCX_SERVICE_MANAGED", "OCX_WINDOWS_WRAPPER_PROTOCOL"] as const; + +/** A copy of `env` without any supervisor marker; `env` itself is not changed. */ +export function withoutSupervisorMarkers>(env: T): T { + const copy = { ...env }; + for (const name of SUPERVISOR_MARKER_ENVS) delete copy[name]; + return copy; +} 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/src/server/management/system-restart.ts b/src/server/management/system-restart.ts index 35e3cefd6e5..d05a0861529 100644 --- a/src/server/management/system-restart.ts +++ b/src/server/management/system-restart.ts @@ -54,6 +54,7 @@ import { withSiblingMarker } from "../../codex/sibling-start"; import { issueSiblingHandoff } from "../../codex/sibling-handoff"; import { readRuntimePort } from "../../config/process-state"; import { spendLedgerRestartEnvironment } from "../../lib/spend-ledger-owner"; +import { SUPERVISOR_MARKER_ENVS } from "../../lib/supervisor-markers"; import { DESKTOP_RESTART_EXIT_CODE, DESKTOP_SUPERVISED_ENV, @@ -275,8 +276,9 @@ export function replacementStartEnvironment( // A sibling's replacement stays a sibling even if the owner is down while it probes. const sourceEnv: NodeJS.ProcessEnv = withSiblingMarker(process.env, issueSiblingHandoff); delete sourceEnv.OCX_SERVICE; - // A detached replacement is not the desktop app's child; it must never exit to an app that is not waiting on it. - delete sourceEnv[DESKTOP_SUPERVISED_ENV]; + // A detached replacement is not the desktop app's child, nor a service manager's or the Windows + // wrapper's; it must never exit to a supervisor that is not waiting on it. + for (const name of SUPERVISOR_MARKER_ENVS) delete sourceEnv[name]; return spendLedgerRestartEnvironment( sourceEnv, waitForHealthBeforeParentExit ? undefined : parentPid, diff --git a/src/service/service-child-ownership.ts b/src/service/service-child-ownership.ts index 0b319dbc3d0..c5cba4184c5 100644 --- a/src/service/service-child-ownership.ts +++ b/src/service/service-child-ownership.ts @@ -31,8 +31,12 @@ export type ServiceChildOwnershipDeps = { serviceHostPaths?: () => readonly string[]; }; -/** The env markers a real supervisor writes that an `ocx claude`/`ocx opencode` child does not. */ -function isMarkedSupervisedServiceChild(env: NodeJS.ProcessEnv): boolean { +/** + * The env markers a current supervisor writes that an `ocx claude`/`ocx opencode` child does not. + * Exported for the client restart (src/client/runtime-restart.ts), which takes only these markers as + * manager evidence; the parent-command-line inference below stays with the start-time ownership check. + */ +export function isMarkedSupervisedServiceChild(env: NodeJS.ProcessEnv): boolean { return env[SERVICE_MANAGED_ENV] === "1" || env[WINDOWS_WRAPPER_PROTOCOL_ENV] === "1"; } diff --git a/src/update/index.ts b/src/update/index.ts index ad49810be9b..41fa4274d4c 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -693,6 +693,8 @@ async function runUpdateWithSession(child: Extract { for (const handle of handles.splice(0)) handle.remove(); }); + +function runUnpreloaded(script: string, env: Record) { + const handle = createTempHome("ocx-codex-sandbox-child-"); + handles.push(handle); + const customHome = handle.path("custom-codex-home"); + mkdirSync(customHome, { recursive: true }); + writeFileSync(join(customHome, "opencodex-catalog.json"), "{\"sentinel\":true}"); + const result = Bun.spawnSync([process.execPath, "--eval", script], { + cwd: handle.root, + env: { PATH: process.env.PATH ?? "", HOME: handle.root, CODEX_HOME: customHome, ...env }, + stdout: "pipe", stderr: "pipe", + }); + return { customHome, status: result.exitCode, stdout: result.stdout.toString().trim(), stderr: result.stderr.toString() }; +} + +const helperUrl = pathToFileURL(repoPath("tests", "helpers", "codex-home-sandbox.ts")).href; +const pathsUrl = pathToFileURL(repoPath("src", "codex", "paths.ts")).href; +const helperThenPaths = ` + const helper = await import(${JSON.stringify(helperUrl)}); + const paths = await import(${JSON.stringify(pathsUrl)}); + let owned = "yes"; + try { helper.assertCodexHomeOwned(paths.CODEX_HOME); } catch { owned = "no"; } + console.log(JSON.stringify({ catalog: paths.DEFAULT_CATALOG_PATH, created: helper.createdCodexHomeForTests(), owned })); +`; + +describe("codex-home-sandbox without the preload", () => { + test.each([ + ["an inherited guard flag", { OCX_TEST_HOME_GUARD: "1" }], + ["a custom CODEX_HOME alone", {}], + ] as const)("%s still resolves into a directory this process created", (_name, env) => { + const run = runUnpreloaded(helperThenPaths, env); + expect(run.status, run.stderr).toBe(0); + const out = JSON.parse(run.stdout) as { catalog: string; created: string | null; owned: string }; + expect(out.created).not.toBeNull(); + expect(out.catalog).toBe(join(out.created!, "opencodex-catalog.json")); + expect(out.catalog.startsWith(run.customHome)).toBe(false); + expect(out.owned).toBe("yes"); + // The custom home is untouched. + expect(readdirSync(run.customHome)).toEqual(["opencodex-catalog.json"]); + }); + + test("paths loaded before the helper are refused", () => { + const run = runUnpreloaded(` + const paths = await import(${JSON.stringify(pathsUrl)}); + const helper = await import(${JSON.stringify(helperUrl)}); + let refused = false; + try { helper.assertCodexHomeOwned(paths.CODEX_HOME); } catch { refused = true; } + console.log(JSON.stringify({ refused, home: paths.CODEX_HOME })); + `, { OCX_TEST_HOME_GUARD: "1" }); + expect(run.status, run.stderr).toBe(0); + const out = JSON.parse(run.stdout) as { refused: boolean; home: string }; + expect(out.refused).toBe(true); + expect(out.home).toBe(run.customHome); + }); +}); + +describe("codex-home-sandbox in this process", () => { + // Valid with or without the preload: whichever branch is in effect is asserted, never skipped. + test("this file's own Codex home is proven owned on the branch in effect", () => { + const created = createdCodexHomeForTests(); + if (created !== null) { + // No preload: the helper ran before src/codex/paths and owns the home it resolved to. + expect(realpathSync.native(CODEX_HOME)).toBe(created); + } else { + // The preload loaded src/codex/paths first; its test root's owner marker names this process. + expect(existsSync(join(dirname(realpathSync.native(CODEX_HOME)), ".opencodex-test-owner.json"))).toBe(true); + } + expect(() => assertCodexHomeOwned(CODEX_HOME)).not.toThrow(); + }); +}); diff --git a/tests/cli/cli-dispatch.test.ts b/tests/cli/cli-dispatch.test.ts index 3a36a246f2d..2b2c3a90427 100644 --- a/tests/cli/cli-dispatch.test.ts +++ b/tests/cli/cli-dispatch.test.ts @@ -733,16 +733,17 @@ describe("a sibling start leaves shared client routing to the live owner", () => expect(recovery.indexOf("markCrossHomeSibling()")).toBeGreaterThan(-1); expect(recovery.indexOf("markCrossHomeSibling()")).toBeLessThan(recovery.indexOf("reconcileStartupJournal()")); const detached = slice("function detachedStartEnvironment(", "async function handleEnsure("); - expect(detached).toContain("const env: NodeJS.ProcessEnv = withoutSiblingMarker(process.env);"); + expect(detached).toContain("const env: NodeJS.ProcessEnv = withoutSupervisorMarkers(withoutSiblingMarker(process.env));"); expect(detached).toContain("delete env.OCX_SERVICE;"); - expect(detached).toContain("delete env[SERVICE_MANAGED_ENV];"); - expect(detached).toContain("delete env[WINDOWS_WRAPPER_PROTOCOL_ENV];"); - expect(cliSource).toContain("env: withProcessRuntimeProvenance(withoutSiblingMarker(process.env)),"); + expect(cliSource).toContain("env: withProcessRuntimeProvenance(withoutSupervisorMarkers(withoutSiblingMarker(process.env))),"); // Every other detached `ocx start` is an ordinary owner too: the client auto-starts and the // updater's restart. A stray marker would mark them before any probe. const opencodeStartEnv = "env: withProcessRuntimeProvenance(opencodeProxyStartEnv(withoutSiblingMarker(process.env)) as NodeJS.ProcessEnv),"; for (const [path, env] of [ - ["src/cli/claude.ts", 'env: withProcessRuntimeProvenance(withoutSiblingMarker({ ...process.env, OCX_SERVICE: "1" })),'], + // ocx claude builds its proxy's environment in claudeProxyStartEnv (also read by the restart's + // owner classification) and spawns with it. + ["src/cli/claude.ts", 'return withoutSupervisorMarkers(withoutSiblingMarker({ ...base, OCX_SERVICE: "1" }));'], + ["src/cli/claude.ts", "env: withProcessRuntimeProvenance(claudeProxyStartEnv()),"], ["src/cli/opencode.ts", opencodeStartEnv], ["src/cli/minimax.ts", opencodeStartEnv], ["src/update/job-pinned-start.ts", "const env: NodeJS.ProcessEnv = withoutSiblingMarker(process.env);"], 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/cli/cli-restart-health.test.ts b/tests/cli/cli-restart-health.test.ts index e51405ca4ae..22802140559 100644 --- a/tests/cli/cli-restart-health.test.ts +++ b/tests/cli/cli-restart-health.test.ts @@ -424,7 +424,7 @@ describe("Desktop CLI start and restart guidance", () => { 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/cli-sync-expect-link.test.ts b/tests/cli/cli-sync-expect-link.test.ts index 26edbc95497..d5d5213e7b4 100644 --- a/tests/cli/cli-sync-expect-link.test.ts +++ b/tests/cli/cli-sync-expect-link.test.ts @@ -5,6 +5,9 @@ * providers. Every case is in-process and deterministic: config fixtures are written directly, the * Home catalog is a stubbed fetch, and both client locks live in the per-test directory. */ +// First: CODEX_HOME must be a temp directory before src/codex/paths resolves DEFAULT_CATALOG_PATH, +// which the shared setup below writes with raw fs calls (tests/helpers/codex-home-sandbox.ts). +import { assertCodexHomeOwned } from "../helpers/codex-home-sandbox"; import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -36,6 +39,8 @@ const TUNNEL = 23461; const SENTINEL_CATALOG = JSON.stringify({ models: [{ slug: "sentinel-local-model" }] }); const HOME_CATALOG = JSON.stringify({ models: [] }); const realSyncConnectedClient = connectApi.syncConnectedClient; +// Refuse to load unless the catalog resolves into a Codex home this process owns (proven by path). +assertCodexHomeOwned(dirname(DEFAULT_CATALOG_PATH)); let root = ""; let previousHome: string | undefined; @@ -227,6 +232,60 @@ describe("ocx sync --expect-link / --expect-hub-key", () => { restart.mockRestore(); connected.mockRestore(); shim.mockRestore(); log.mockRestore(); error.mockRestore(); } }); + // Review round 2: a sync whose Codex journal handover failed must not report a clean success, + // because a later supervised restart would restore native routing again. + test("a sync whose journal handover failed exits 1 and says to sync again", async () => { + writeClient({ transport: "hub" }); + const current = readClientConnectionState(); + if (current.kind !== "connected") throw new Error("fixture must be connected"); + const connected = spyOn(connectApi, "syncConnectedClient").mockResolvedValue({ catalogWritten: false, cacheSynced: false, injected: true, stale: false, connection: current.value, journalHandoverFailed: true }); + const shim = spyOn(shimAutoRestore, "runCodexShimAutoRestoreAsync").mockResolvedValue(undefined); + const errors: string[] = []; + const log = spyOn(console, "log").mockImplementation(() => {}); + const error = spyOn(console, "error").mockImplementation((...parts) => { errors.push(parts.map(String).join(" ")); }); + try { + process.exitCode = 0; + const args = ["sync"]; + const code = await dispatchCommand({ kind: "command", command: "sync", args }, { args } as unknown as CliDispatchDeps); + expect(code).toBe(1); + expect(errors.join("\n")).toContain("Codex journal could not be handed to this connection"); + expect(errors.join("\n")).toContain("Run 'ocx sync' again"); + } finally { + process.exitCode = 0; + connected.mockRestore(); shim.mockRestore(); log.mockRestore(); error.mockRestore(); + } + }); + + // Final review: a failed handover must not be followed by the automatic post-apply restart, + // which on a Child with an earlier connection's journal would undo the routing just applied. + test("a failed journal handover skips the post-apply runtime restart", async () => { + writeClient({ transport: "link" }); + const current = readClientConnectionState(); + if (current.kind !== "connected") throw new Error("fixture must be connected"); + const appliedAt = "2026-10-21T01:00:00.000Z"; + const record = { linkId: LINK_ID, fetchedAt: appliedAt, appliedAt, document: { + version: 1 as const, home: { name: "fixture", version: "2.97.0" }, + settings: { fastMode: true, multiAgentMode: "v1" as const, webSearchSidecarEnabled: false, subagentDefaults: null }, + } }; + const restart = spyOn(applyRestart, "restartRuntimeAfterApply").mockResolvedValue({ kind: "restarted" }); + const connected = spyOn(connectApi, "syncConnectedClient").mockResolvedValue({ catalogWritten: false, cacheSynced: false, injected: true, stale: false, connection: current.value, appliedRecord: record, journalHandoverFailed: true }); + const shim = spyOn(shimAutoRestore, "runCodexShimAutoRestoreAsync").mockResolvedValue(undefined); + const errors: string[] = []; + const log = spyOn(console, "log").mockImplementation(() => {}); + const error = spyOn(console, "error").mockImplementation((...parts) => { errors.push(parts.map(String).join(" ")); }); + try { + process.exitCode = 0; + const args = ["sync"]; + const code = await dispatchCommand({ kind: "command", command: "sync", args }, { args } as unknown as CliDispatchDeps); + expect(code).toBe(1); + expect(restart).not.toHaveBeenCalled(); + expect(errors.join("\n")).toContain("the client runtime was not restarted"); + } finally { + process.exitCode = 0; + restart.mockRestore(); connected.mockRestore(); shim.mockRestore(); log.mockRestore(); error.mockRestore(); + } + }); + async function runSync(args: string[]) { let discoveries = 0; const errors: string[] = []; @@ -502,3 +561,20 @@ test("the machine sync route does not return the saved connection", async () => ); expect(await response?.json()).toEqual({ success: true, catalogWritten: true, cacheSynced: true, injected: false, stale: false }); }); + +// Review round 3: the machine API must not call a sync complete when its journal handover failed. +test("the machine sync route reports a failed journal handover as not complete", async () => { + const connection = clientBlock({ transport: "hub" }) as unknown as OcxClientConnectionConfig; + const response = await handleMachineApi( + new Request("http://127.0.0.1/api/machine/sync", { method: "POST", body: "{}", headers: { "content-type": "application/json" } }), + new URL("http://127.0.0.1/api/machine/sync"), + connection, + { + sync: async () => ({ catalogWritten: true, cacheSynced: true, injected: true, stale: false, connection, journalHandoverFailed: true }), + disconnect: async () => { throw new Error("unused"); }, + scheduleStandaloneRecycle: () => {}, + }, + ); + expect(response?.status).toBe(409); + expect(await response?.json()).toMatchObject({ success: false, journalHandoverFailed: true }); +}); diff --git a/tests/cli/disconnect-runtime.test.ts b/tests/cli/disconnect-runtime.test.ts new file mode 100644 index 00000000000..fa8b93131fc --- /dev/null +++ b/tests/cli/disconnect-runtime.test.ts @@ -0,0 +1,367 @@ +import { describe, expect, test } from "bun:test"; +import { bindRestartToRuntime, restartRuntimeAfterDisconnect, type DisconnectRuntimeDeps, type RuntimeGeneration } from "../../src/cli/disconnect-runtime"; +import { claudeProxyStartEnv } from "../../src/cli/claude"; +import { codexProxyStartEnv } from "../../src/cli/codex-launcher"; +import { opencodeProxyStartEnv } from "../../src/cli/opencode"; +import { restartOwnerEvidence } from "../../src/client/runtime-restart"; +import { clientRuntimeRecord } from "../../src/client/runtime"; +import { runtimeOwnerScope } from "../../src/config/process-state"; +import { observeProxyLiveness } from "../../src/server/proxy-liveness"; +import { requestBoundSystemRestart } from "../../src/cli/system-restart-client"; +import { runProxyRestart, type ProxyRestartIo } from "../../src/cli/tray-proxy"; +import { createLocalAttestationProof, createLocalAttestationSecret, LOCAL_ATTESTATION_CHALLENGE_HEADER, LOCAL_ATTESTATION_PROOF_HEADER } from "../../src/lib/local-management-attestation"; +import { SYSTEM_RESTART_PATH } from "../../src/lib/system-restart-contract"; + +type Observation = Awaited>; +type State = ReturnType; +const live = (role?: string, pid = 4321): Observation => ({ kind: "live", live: { pid, port: 10100, ...(role ? { role } : {}) } }); +// A record written by a runtime with this change and a proven owner (here: no supervisor). +const generation: RuntimeGeneration = { pid: 4321, port: 10100, hostname: "127.0.0.1", attestationSecret: createLocalAttestationSecret(), restartOwner: "none" }; + +function run(observation: Observation, restartOk: boolean, json = false, extra: { state?: State; states?: State[]; after?: Observation; capture?: RuntimeGeneration | null; captureAfter?: RuntimeGeneration | null; refusedBy?: "service-unproven" } = {}) { + const out: string[] = [], err: string[] = []; + let restarts = 0; + const bound: RuntimeGeneration[] = []; + const done = restartRuntimeAfterDisconnect({ + observe: async () => (restarts > 0 ? extra.after ?? live("standalone", 9999) : observation), + capture: () => (restarts > 0 && extra.captureAfter !== undefined ? extra.captureAfter : extra.capture === undefined ? generation : extra.capture), + connectionState: () => extra.states?.shift() ?? extra.state ?? { kind: "disconnected" }, + restart: async target => { restarts += 1; bound.push(target); return { ok: restartOk, ...(extra.refusedBy ? { refusedBy: extra.refusedBy } : {}) }; }, + json, + info: line => out.push(line), + error: line => err.push(line), + }); + return done.then(code => ({ code, out, err, restarts, bound })); +} + +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(live("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(live("client"), false, false, { after: live("client") }); + 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 confirmed absence, is left alone", async () => { + for (const observation of [live(), live("standalone"), { kind: "absent", live: null } as const]) { + expect(await run(observation, true)).toMatchObject({ code: 0, restarts: 0, out: [], err: [] }); + } + }); + + test("an inconclusive liveness probe fails with what to check", async () => { + const result = await run({ kind: "unknown", live: null }, true); + expect(result).toMatchObject({ code: 1, restarts: 0 }); + expect(result.err.join("\n")).toContain("could not confirm whether the client runtime stopped"); + }); + + test("the restart is bound to the generation captured from the observed runtime", async () => { + expect((await run(live("client"), true)).bound).toEqual([generation]); + }); + + test("a client runtime whose generation cannot be captured is not restarted", async () => { + const result = await run(live("client"), true, false, { capture: null }); + expect(result).toMatchObject({ code: 1, restarts: 0 }); + expect(result.err.join("\n")).toContain("could not identify the running client runtime"); + }); + + // Coordinator review round 3: any state but disconnected stops the leave before the Home's own + // \`ocx restart\`, which would interrupt a connection that just landed or act on unreadable state. + test.each([ + ["connected", { kind: "connected" } as State, "A new connection is in use on this machine"], + ["invalid", { kind: "invalid", reason: "config.json is missing or unreadable" } as State, "client state is invalid (config.json is missing or unreadable)"], + ["mismatched", { kind: "mismatched", reason: "runtimeRole=client is present without config.json.client" } as State, "client state is mismatched (runtimeRole=client is present without config.json.client)"], + ])("a %s state after disconnect exits 1 and restarts nothing", async (_name, state, message) => { + const result = await run(live("client"), true, false, { state }); + expect(result).toMatchObject({ code: 1, restarts: 0, out: [] }); + expect(result.err.join("\n")).toContain(message); + }); + + // The rule holds on every branch: exit 0 needs a disconnected state, read again before exit 0. + test.each([ + ["no runtime", { kind: "absent", live: null } as Observation], + ["a standalone runtime", live("standalone")], + ])("%s with a connected state exits 1", async (_name, observation) => { + const result = await run(observation, true, false, { state: { kind: "connected" } }); + expect(result).toMatchObject({ code: 1, restarts: 0, out: [] }); + expect(result.err.join("\n")).toContain("A new connection is in use on this machine"); + }); + + test("a connection that lands during the restart turns the standalone result into exit 1", async () => { + const result = await run(live("client"), true, false, { states: [{ kind: "disconnected" }, { kind: "connected" }] }); + expect(result).toMatchObject({ code: 1, restarts: 1, out: [] }); + expect(result.err.join("\n")).toContain("A new connection is in use on this machine"); + }); + + // Coordinator review round 4: when a client still answers after a refused or failed restart, + // say what is true; recommend a plain ocx restart only when the captured runtime still answers. + const newer: RuntimeGeneration = { pid: 5555, port: 10100, attestationSecret: createLocalAttestationSecret() }; + test("a new connection in use after a refused restart is reported, with no restart advice", async () => { + const result = await run(live("client"), false, false, { after: live("client", 5555), captureAfter: newer, states: [{ kind: "disconnected" }, { kind: "connected" }] }); + expect(result.code).toBe(1); + const text = result.err.join("\n"); + expect(text).toContain("A new connection is in use on this machine"); + expect(text).toContain("ocx status"); + expect(text).not.toContain("Run on this machine: ocx restart"); + }); + + test("a different client runtime with the state still disconnected is reported for inspection", async () => { + const result = await run(live("client"), false, false, { after: live("client", 5555), captureAfter: newer }); + expect(result.code).toBe(1); + const text = result.err.join("\n"); + expect(text).toContain("A different client runtime (pid 5555) answers now"); + expect(text).not.toContain("Run on this machine: ocx restart"); + }); + + test("an unreadable state after a refused restart says what to check", async () => { + const result = await run(live("client"), false, false, { after: live("client", 5555), captureAfter: newer, states: [{ kind: "disconnected" }, { kind: "invalid", reason: "config.json is missing or unreadable" }] }); + expect(result.code).toBe(1); + expect(result.err.join("\n")).toContain("client state is invalid (config.json is missing or unreadable)"); + expect(result.err.join("\n")).not.toContain("Run on this machine: ocx restart"); + }); + + test("nothing answering after the restart while a new connection exists reports that connection", async () => { + const result = await run(live("client"), true, false, { after: { kind: "absent", live: null }, states: [{ kind: "disconnected" }, { kind: "connected" }] }); + expect(result.code).toBe(1); + expect(result.err.join("\n")).toContain("A new connection is in use on this machine"); + expect(result.err.join("\n")).not.toMatch(/ocx restart/); + }); + + test.each([ + ["absent", { kind: "absent", live: null } as const], + ["inconclusive", { kind: "unknown", live: null } as const], + ])("an %s observation after the restart fails with what to run", async (_name, after) => { + const result = await run(live("client"), true, false, { after }); + expect(result.code).toBe(1); + expect(result.out).toEqual([]); + expect(result.err.join("\n")).toContain("could not confirm a standalone runtime"); + }); + + // Coordinator review round 5: the early exits (inconclusive liveness, uncapturable record) read the + // state too, and a connection that landed gets inspection advice, never a restart. + test.each([ + ["inconclusive liveness", { kind: "unknown", live: null } as Observation, undefined], + ["an uncapturable client runtime", live("client"), null], + ] as const)("%s with a connection that landed advises inspection, not a restart", async (_name, observation, capture) => { + const result = await run(observation, true, false, { state: { kind: "connected" }, ...(capture === null ? { capture: null } : {}) }); + expect(result).toMatchObject({ code: 1, restarts: 0 }); + const text = result.err.join("\n"); + expect(text).toContain("A new connection is in use on this machine"); + expect(text).toContain("ocx status"); + expect(text).not.toMatch(/ocx restart/); + }); + + // One rule for every exit-1 branch: a plain restart is advised only while the state is + // disconnected and the captured runtime still answers (or nothing answers at all). + test("no exit-1 branch advises a restart while a connection is in use or the state is unreadable", async () => { + const states: State[] = [{ kind: "connected" }, { kind: "invalid", reason: "unreadable" }, { kind: "mismatched", reason: "mixed" }]; + const newer: RuntimeGeneration = { pid: 5555, port: 10100, attestationSecret: createLocalAttestationSecret() }; + const branches: Array> = []; + for (const state of states) { + branches.push([{ kind: "unknown", live: null }, true, false, { state }]); + branches.push([live("client"), true, false, { state, capture: null }]); + branches.push([live("client"), true, false, { state }]); + branches.push([{ kind: "absent", live: null }, true, false, { state }]); + branches.push([live("standalone"), true, false, { state }]); + for (const after of [live("client"), live("client", 5555), { kind: "absent", live: null } as Observation, { kind: "unknown", live: null } as Observation, live("standalone", 9999)]) { + for (const ok of [true, false]) branches.push([live("client"), ok, false, { states: [{ kind: "disconnected" }, state], after, captureAfter: newer }]); + } + } + for (const args of branches) { + const result = await run(...args); + expect(result.code).toBe(1); + expect(result.err.join("\n")).not.toMatch(/ocx restart/); + } + }); + + // Final review: a restart its service manager refused (relaunch unproven) is redone through that + // manager; a plain ocx restart would only be refused again. + test("a restart refused by an unproven service manager advises the manager", async () => { + const result = await run(live("client"), false, false, { after: live("client"), refusedBy: "service-unproven" }); + expect(result.code).toBe(1); + const text = result.err.join("\n"); + expect(text).toContain("ocx service restart"); + expect(text).not.toContain("Run on this machine: ocx restart"); + }); + + // Final review rounds 4 and 5: origin/dev's `ocx disconnect` restarted nothing (b1148220574917c8 + // src/cli/dispatch.ts disconnect: handleDisconnectCommand's code only). The restart step acts only + // on a record that states a proven owner (restartOwner, written by a runtime with this change). + // A bare OCX_SERVICE=1 runtime states none; built here by the production record builder and + // ownership check, it gets the disconnect's own 0 with nothing restarted or read further. + test.each([ + ["a legacy service child", () => ({ OCX_SERVICE: "1", PATH: "/usr/bin" }) as NodeJS.ProcessEnv], + ["ocx claude's proxy", () => claudeProxyStartEnv({ PATH: "/usr/bin" })], + ["ocx codex's proxy", () => codexProxyStartEnv({ PATH: "/usr/bin" }) as NodeJS.ProcessEnv], + ["ocx opencode's proxy", () => opencodeProxyStartEnv({ PATH: "/usr/bin" })], + ] as const)("%s is left as origin/dev's disconnect left it: exit 0, nothing restarted", async (_name, env) => { + const record = clientRuntimeRecord(4321, 10100, generation.attestationSecret, restartOwnerEvidence({ desktop: false, env: env() })); + expect(record.restartOwner).toBeUndefined(); + for (const state of [{ kind: "disconnected" }, { kind: "connected" }, { kind: "invalid", reason: "x" }] as State[]) { + const result = await run(live("client"), true, false, { capture: { ...record, attestationSecret: generation.attestationSecret }, state }); + expect(result).toMatchObject({ code: 0, restarts: 0, out: [], err: [] }); + } + }); + + // A runtime started before this change writes no restartOwner. Whatever its old handler would + // answer (refused because the versions are known to differ, or accepted by an old handler that + // would start its own replacement), no restart request is sent: origin/dev's disconnect, exit 0. + test.each([ + ["versions known to differ (the request would be refused)", false], + ["an old handler that would accept the request", true], + ] as const)("a record without restartOwner, %s, gets origin/dev's disconnect", async (_name, wouldAccept) => { + const { restartOwner: _dropped, ...oldRecord } = generation; + for (const capture of [oldRecord, { ...oldRecord, restartOwner: "some-later-owner" }]) { + for (const state of [{ kind: "disconnected" }, { kind: "connected" }] as State[]) { + const result = await run(live("client"), wouldAccept, false, { capture, state }); + expect(result).toMatchObject({ code: 0, restarts: 0, out: [], err: [] }); + } + } + }); + + test("a record that states a proven owner is restarted, bound to that record", async () => { + for (const [env, desktop, owner] of [ + [{}, false, "none"], + [{ OCX_SERVICE: "1", OCX_SERVICE_MANAGED: "1" }, false, "service-manager"], + [{ OCX_SERVICE: "1" }, true, "desktop"], + ] as const) { + const record = clientRuntimeRecord(4321, 10100, generation.attestationSecret, restartOwnerEvidence({ desktop, env })); + expect(record.restartOwner).toBe(owner); + const capture = { ...record, attestationSecret: generation.attestationSecret }; + const result = await run(live("client"), true, false, { capture }); + expect(result).toMatchObject({ code: 0, restarts: 1 }); + expect(result.bound).toEqual([capture]); + } + }); + + // P3: after a completed restart, an unreadable state must not also claim nothing was restarted. + test("a completed restart followed by an unreadable state does not say nothing was restarted", async () => { + const result = await run(live("client"), true, false, { states: [{ kind: "disconnected" }, { kind: "invalid", reason: "unreadable" }] }); + expect(result.code).toBe(1); + expect(result.err.join("\n")).toContain("The client runtime was restarted"); + expect(result.err.join("\n")).not.toContain("nothing was restarted"); + }); + + test("--json keeps stdout to the disconnect document", async () => { + const logged: unknown[][] = []; + const original = console.log; + console.log = (...args: unknown[]) => { logged.push(args); }; + try { + let restarted = false; + const result = await restartRuntimeAfterDisconnect({ + observe: async () => live(restarted ? "standalone" : "client", restarted ? 9999 : 4321), + capture: () => generation, + connectionState: () => ({ kind: "disconnected" }), + restart: async () => { console.log("restart progress"); restarted = true; return { ok: true }; }, + json: true, + error: () => {}, + }); + expect(result).toBe(0); + } finally { console.log = original; } + expect(logged).toEqual([]); + }); +}); + +// Coordinator review rounds 2-3: the restart must reach only the runtime generation disconnect +// observed. A pid can be reused, so the binding is the captured runtime record (pid, port and the +// per-process attestation secret), and admission is driven through the real requestBoundSystemRestart. +describe("a disconnect restart bound to the captured runtime generation", () => { + function world(current: { pid: number; secret: string } | null) { + const posts: string[] = []; + const fetchImpl = (async (input: RequestInfo | URL, init: RequestInit = {}) => { + const url = String(input); + if (url.endsWith(SYSTEM_RESTART_PATH)) { posts.push(url); return Response.json({ success: true }, { status: 202 }); } + if (!current) throw new Error("connection refused"); + const challenge = new Headers(init.headers).get(LOCAL_ATTESTATION_CHALLENGE_HEADER) ?? ""; + const proof = createLocalAttestationProof(current.secret, challenge, current.pid, 10100) ?? ""; + return Response.json({ service: "opencodex", pid: current.pid, port: 10100, role: "client", restartCapability: "v1" }, { headers: { [LOCAL_ATTESTATION_PROOF_HEADER]: proof } }); + }) as typeof fetch; + const calls: string[] = []; + const io: ProxyRestartIo = { + findLive: async () => current + ? { status: "live", live: { pid: current.pid, port: 10100, hostname: "127.0.0.1", source: "runtime" } } + : { status: "absent" }, + startWhenStopped: async () => { calls.push("generic-start"); return { status: "started" }; }, + requestInPlaceRestart: async () => { calls.push("unbound-request"); return { accepted: true }; }, + waitForReplacement: async () => ({ pid: 9999, port: 10100, source: "runtime" }), + waitBetweenAttempts: async () => {}, + }; + const request = (previous: Parameters[0], readRuntime: (pid: number) => RuntimeGeneration | null) => + requestBoundSystemRestart({ ...previous, version: "0.0.0" } as never, Date.now() + 600_000, { fetchImpl, readRuntime: readRuntime as never, readUpdateHome: () => null, cliVersion: "0.0.0", + findLive: (async () => current ? { pid: current.pid, port: 10100, hostname: "127.0.0.1", source: "runtime" } : null) as never }); + return { posts, calls, io, request }; + } + + test("same pid, different generation: refused before any restart request", async () => { + const w = world({ pid: 4321, secret: createLocalAttestationSecret() }); + const result = await runProxyRestart(bindRestartToRuntime(w.io, generation, w.request)); + expect(w.posts).toEqual([]); + expect(w.calls).toEqual([]); + expect(result.ok).toBe(false); + }); + + test("a different pid at the restart's own discovery is refused", async () => { + const w = world({ pid: 5555, secret: generation.attestationSecret }); + const result = await runProxyRestart(bindRestartToRuntime(w.io, generation, w.request)); + expect(w.posts).toEqual([]); + expect(result).toMatchObject({ ok: false, phase: "request" }); + }); + + test("the captured generation is restarted", async () => { + const w = world({ pid: 4321, secret: generation.attestationSecret }); + const result = await runProxyRestart(bindRestartToRuntime(w.io, generation, w.request)); + expect(w.posts.length, JSON.stringify(result, (_k, v) => v instanceof Error ? v.message : v)).toBe(1); + expect(w.calls).toEqual([]); + }); + + test("a runtime that vanished gets no generic start", async () => { + const w = world(null); + await runProxyRestart(bindRestartToRuntime(w.io, generation, w.request)); + expect(w.calls).toEqual([]); + expect(w.posts).toEqual([]); + }); +}); + +// The merge with dev #8562 (#8552): observeProxyLiveness skips a runtime record that names this +// process's own PID from an earlier lifetime. Such a stale record can still carry a restartOwner. +// The disconnect step decides from the liveness observation first, so a stale record never reaches +// the capture or a restart. Everything is injected: no network, no real port, no real home. +describe("a stale runtime record from an earlier lifetime of this PID never drives the disconnect restart", () => { + for (const published of [false, true]) { + test("this process " + (published ? "has" : "has not") + " published its own records", async () => { + const ownScope = runtimeOwnerScope(); + const stale = { pid: process.pid, port: 54321, hostname: "127.0.0.1", attestationSecret: createLocalAttestationSecret(), restartOwner: "service-manager", ownerScope: ownScope }; + const refused = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED" }); + const observe = () => observeProxyLiveness({ + attempts: 1, + fetchFn: (async () => { throw refused; }) as unknown as typeof fetch, + readPidFn: () => null, + readRuntimeFn: () => stale, + configFn: () => ({ port: 54321, hostname: "127.0.0.1" }), + bindProofFn: async () => true, + readCommandLineFn: () => "bun ocx start", + ownProcessFn: () => ({ pid: process.pid, publishedOwnership: published, scope: ownScope }), + sleepFn: async () => {}, + }); + const seen = await observe(); + expect(seen.kind).toBe(published ? "unknown" : "absent"); + let captures = 0, restarts = 0; + const code = await restartRuntimeAfterDisconnect({ + observe, + capture: () => { captures += 1; return stale; }, + connectionState: () => ({ kind: "disconnected" }), + restart: async () => { restarts += 1; return { ok: true }; }, + json: false, info: () => {}, error: () => {}, + }); + expect({ captures, restarts }).toEqual({ captures: 0, restarts: 0 }); + // An earlier lifetime proven stale reads as nothing running (origin/dev's exit 0); one this + // process cannot rule out stays inconclusive and fails closed (exit 1), still without a request. + expect(code).toBe(published ? 1 : 0); + }); + } +}); + diff --git a/tests/cli/link-apply-restart.test.ts b/tests/cli/link-apply-restart.test.ts index 4a4f867c626..c47ec965f93 100644 --- a/tests/cli/link-apply-restart.test.ts +++ b/tests/cli/link-apply-restart.test.ts @@ -12,6 +12,7 @@ import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { applyRestartMessage, restartRuntimeAfterApply, type ApplyRestartDeps } from "../../src/cli/link-apply-restart"; +import { classifyClientRestartSupervisor, createClientRestartController, restartOwnerProven } from "../../src/client/runtime-restart"; import { requestBoundSystemRestart } from "../../src/cli/system-restart-client"; import { deferSupervisedRestart, reportRestartFailure, supervisedRestartRefusal } from "../../src/cli/restart-failure"; import { collectAppliedCodexSettingsStatus } from "../../src/cli/status-codex-settings"; @@ -151,7 +152,7 @@ function clientHealthDeps(postResponse: Response) { describe("ocx restart against a client runtime", () => { test("only explicit request-phase supervision refusals qualify for deferral", () => { const notices: string[] = []; - for (const supervisor of ["desktop", "service"] as const) { + for (const supervisor of ["desktop", "service", "service-unproven"] as const) { const refused = { ok: false as const, phase: "request" as const, error: new Error(`restart_supervised_${supervisor}`) }; expect(supervisedRestartRefusal(refused)).toBe(supervisor); expect(deferSupervisedRestart(refused, value => notices.push(value))).toBe(true); @@ -160,7 +161,7 @@ describe("ocx restart against a client runtime", () => { const uncertain = { ok: false as const, phase: "request" as const, error: new Error("restart_request_outcome_unknown") }; expect(supervisedRestartRefusal(uncertain)).toBeNull(); expect(deferSupervisedRestart(uncertain, value => notices.push(value))).toBe(false); - expect(notices).toEqual(["desktop", "service"]); + expect(notices).toEqual(["desktop", "service", "service-unproven"]); expect(supervisedRestartRefusal({ ok: false, phase: "request", error: new Error("restart_supervised_other") })).toBeNull(); expect(supervisedRestartRefusal({ ok: true, mode: "skipped" })).toBeNull(); }); @@ -193,16 +194,53 @@ describe("ocx restart against a client runtime", () => { try { reportRestartFailure({ ok: false, phase: "request", error: new Error("restart_supervised_service") }); reportRestartFailure({ ok: false, phase: "request", error: new Error("restart_supervised_desktop") }); + reportRestartFailure({ ok: false, phase: "request", error: new Error("restart_supervised_service-unproven") }); } finally { console.error = original; } 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.", + "Restart not performed; the running client runtime was kept. A service manager started it (systemd opencodex-proxy.service, launchd com.opencodex.proxy or the Windows OpenCodex service), but it is not enabled or its relaunch could not be verified: restart it through the manager with 'ocx service restart', or check 'ocx service status'.", ]); }); }); +// Final review round 4 (scope split): `ocx sync` on a Child whose runtime carries only a bare +// OCX_SERVICE=1 ends exactly as on origin/dev. Expected values from the merge base +// b1148220574917c8: src/client/runtime-restart.ts:114-119 and :98-106 (the runtime's admission), +// src/cli/link-apply-restart.ts:63-64 and :74-76 (the outcome and its message), and +// src/cli/sync-command.ts (exit 1 only for failed or connection-changed). +describe("ocx sync with a bare OCX_SERVICE=1 runtime keeps origin/dev's outcome", () => { + for (const viable of [true, false]) { + test(`service viable ${viable}`, async () => { + const env = { OCX_SERVICE: "1", PATH: "/usr/bin" } as NodeJS.ProcessEnv; + const supervisor = classifyClientRestartSupervisor({ desktop: false, env, diagnose: () => ({ viable: true, running: true }), legacyServiceViable: () => viable }); + const controller = createClientRestartController({ + supervisor: async () => supervisor, + ownerProven: () => restartOwnerProven({ desktop: false, env }), + // A connected sync: the served connection is current. + connectionChanged: () => false, + recycle: async () => { throw new Error("the recycle is not reachable"); }, + stop: async () => {}, spawnReplacement: async () => {}, resume: async () => {}, exitProcess: () => {}, sleep: async () => {}, + }); + const { value } = deps(0, { + restart: async (_start, onSupervised) => { + const admission = await controller.request(); + await controller.settled(); + if (admission.kind === "supervised") { onSupervised(admission.supervisor); return false; } + return admission.kind === "accepted"; + }, + }); + const outcome = await restartRuntimeAfterApply(record, value); + expect(outcome).toEqual(viable ? { kind: "deferred-supervisor", supervisor: "service" } : { kind: "restarted" }); + expect(applyRestartMessage(outcome)).toBe(viable + ? "Client runtime restart deferred; the settings take effect when the background service restarts it. Run `ocx service restart`." + : "Client runtime restarted after applying the Home's settings."); + }); + } +}); + describe("ocx status --json OCX-6 fields (shape snapshot)", () => { const homeConfig = { fastMode: true, multiAgentMode: "v1", webSearchSidecar: { enabled: false } } as unknown as OcxConfig; const childConfig = { diff --git a/tests/clients/client-link-teardown.test.ts b/tests/clients/client-link-teardown.test.ts index 540bc7cf348..11d6370bb3e 100644 --- a/tests/clients/client-link-teardown.test.ts +++ b/tests/clients/client-link-teardown.test.ts @@ -1,5 +1,9 @@ +// First: in-process connectClient writes the catalog through src/codex/paths, fixed at import. +import { assertCodexHomeOwned } from "../helpers/codex-home-sandbox"; import { expect, spyOn, test } from "bun:test"; -import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { CODEX_HOME } from "../../src/codex/paths"; +import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; import { getDefaultConfig, saveConfig } from "../../src/config"; import { handleDisconnectCommand } from "../../src/cli/connect"; import { connectClient } from "../../src/client/connect"; @@ -7,6 +11,12 @@ import { teardownClientLink } from "../../src/client/link-teardown"; import { clientLinkStatePath, writeClientLinkState } from "../../src/client/link-state"; import { quoteRemote, remoteOcxArgv } from "../../src/link/ssh-argv"; import { createTempHome } from "../helpers/temp-home"; +import { fixturePath, repoPath } from "../helpers/repo-root"; +import { randomUUID } from "node:crypto"; + +const FIXTURE_TOKEN_FILE = ".ocx-fixture-token"; + +assertCodexHomeOwned(CODEX_HOME); const linkId = "lnk_0123456789abcdef"; const key = `ocx_data_${"a".repeat(40)}`; @@ -151,3 +161,141 @@ 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 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(); + } +}); + +/** + * Scenarios that write the Codex files directly. Those paths are fixed when src/codex/paths loads, + * so they run in tests/fixtures/client-link-journal-runner.ts, a child whose CODEX_HOME and + * OPENCODEX_HOME point into this test's own temp home before anything is imported; this process + * never touches a Codex path, and only that owned home is removed. + */ +async function runJournalScenario(scenario: "catalog-changed" | "standalone-handover" | "foreign-journal") { + const home = createTempHome("ocx-client-link-journal-"); + try { + const codex = home.path("codex"); + mkdirSync(codex, { recursive: true }); + mkdirSync(home.path("ocx"), { recursive: true }); + // The runner writes only into a home that carries the one-time token this process hands it. + const token = randomUUID(); + writeFileSync(home.path(FIXTURE_TOKEN_FILE), token); + const child = Bun.spawn([process.execPath, fixturePath("client-link-journal-runner.ts")], { + cwd: repoPath(), stdin: new Blob([scenario]), stdout: "pipe", stderr: "pipe", + env: { ...process.env, HOME: home.root, USERPROFILE: home.root, OPENCODEX_HOME: home.path("ocx"), CODEX_HOME: codex, OCX_SERVICE: "", OCX_SERVICE_MANAGED: "0", OCX_FIXTURE_TOKEN: token }, + }); + const [exit, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]); + expect(exit, stderr).toBe(0); + return JSON.parse(stdout.trim().split("\n").at(-1)!) as { exit?: number; injected?: boolean; before?: unknown; after?: unknown; warnings: string[]; errors: string[] }; + } finally { + home.remove(); + } +} + +test("a catalog changed after connect names what was found and --keep-catalog", async () => { + const result = await runJournalScenario("catalog-changed"); + expect(result.exit).not.toBe(0); + const output = result.errors.join("\n"); + 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"); +}); + +test("connect --link over a standalone injection hands the Codex journal to the committed client", async () => { + const result = await runJournalScenario("standalone-handover"); + expect(result.injected).toBe(true); + expect((result.before as { kind: string }).kind).toBe("process"); + expect(result.after).toEqual({ kind: "client", apiKeyId: "key-1" }); +}); + +// Review round 4: a connect over a journal an earlier connection owns must say so, not report a +// clean handover; the routing is restored on the next restart and a sync after it re-applies it. +test("connect over an earlier connection's Codex journal warns with the recovery step", async () => { + const result = await runJournalScenario("foreign-journal"); + expect(result.after).toEqual({ kind: "client", apiKeyId: "earlier-key" }); + expect(result.warnings.join("\n")).toContain("belongs to an earlier connection"); + expect(result.warnings.join("\n")).toContain("ocx sync --restart-codex"); +}); + + +// Coordinator review round 4: run by hand, with HOME pointing at a real home, the runner must +// refuse before any write. A temp folder stands in for that home and is asserted untouched. +test("the journal runner refuses a home its parent did not create, and writes nothing", async () => { + const home = createTempHome("ocx-client-link-runner-direct-"); + try { + const realLike = home.path("real-home"); + mkdirSync(join(realLike, ".codex"), { recursive: true }); + mkdirSync(join(realLike, ".opencodex"), { recursive: true }); + writeFileSync(join(realLike, ".codex", "config.toml"), "model_provider = \"real\"\n"); + writeFileSync(join(realLike, ".opencodex", "config.json"), "{\"real\":true}"); + for (const token of [undefined, "forged-token"]) { + const child = Bun.spawn([process.execPath, fixturePath("client-link-journal-runner.ts")], { + cwd: repoPath(), stdin: new Blob(["standalone-handover"]), stdout: "pipe", stderr: "pipe", + env: { PATH: process.env.PATH ?? "", HOME: realLike, OPENCODEX_HOME: join(realLike, ".opencodex"), CODEX_HOME: join(realLike, ".codex"), ...(token ? { OCX_FIXTURE_TOKEN: token } : {}) }, + }); + const [exit, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()]); + expect(exit).not.toBe(0); + expect(stdout).toBe(""); + expect(stderr.trim().split("\n")).toEqual(["client-link-journal-runner.ts is a test fixture; run it through tests/clients/client-link-teardown.test.ts."]); + } + expect(readdirSync(join(realLike, ".codex"))).toEqual(["config.toml"]); + expect(readFileSync(join(realLike, ".codex", "config.toml"), "utf8")).toBe("model_provider = \"real\"\n"); + expect(readdirSync(join(realLike, ".opencodex"))).toEqual(["config.json"]); + expect(readFileSync(join(realLike, ".opencodex", "config.json"), "utf8")).toBe("{\"real\":true}"); + } finally { + home.remove(); + } +}); diff --git a/tests/clients/client-runtime-restart.test.ts b/tests/clients/client-runtime-restart.test.ts index 06cd7feb460..f1d3f1ea2ae 100644 --- a/tests/clients/client-runtime-restart.test.ts +++ b/tests/clients/client-runtime-restart.test.ts @@ -6,12 +6,29 @@ * replacement, and a replacement that does not come up leaves the old runtime serving. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import * as fs from "node:fs"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { Server } from "bun"; import { startMachineListener } from "../../src/client/machine-listener"; -import { clientRestartRollbackAction, createClientRestartController, type ClientRestartAdmission, type ClientRestartIo } from "../../src/client/runtime-restart"; +import { classifyClientRestartSupervisor, clientConnectionEnded, clientRestartRollbackAction, createClientRestartController, restartOwnerEvidence, restartOwnerProven, type ClientRestartAdmission, type ClientRestartIo } from "../../src/client/runtime-restart"; +import { readClientConnectionState, readClientConnectionStateWithEvidence } from "../../src/client/state"; +import { claudeProxyStartEnv } from "../../src/cli/claude"; +import { opencodeProxyStartEnv } from "../../src/cli/opencode"; +import { codexProxyStartEnv } from "../../src/cli/codex-launcher"; +import { spyOn } from "bun:test"; +import { deriveLaunchdServiceDiagnostic, deriveSystemdServiceDiagnostic, deriveWindowsServiceDiagnostic } from "../../src/service/diagnostics"; +import * as serviceChildOwnership from "../../src/service/service-child-ownership"; +import { clientRuntimeRecord, standaloneRecycleEnv } from "../../src/client/runtime"; +import { replacementStartEnvironment } from "../../src/server/management/system-restart"; +import { SUPERVISOR_MARKER_ENVS } from "../../src/lib/supervisor-markers"; +import { DESKTOP_SUPERVISED_ENV } from "../../src/lib/system-restart-contract"; +import { SERVICE_MANAGED_ENV } from "../../src/service/state"; +import { WINDOWS_WRAPPER_PROTOCOL_ENV } from "../../src/service/windows-wrapper-exit"; +import { repoPath } from "../helpers/repo-root"; +import { restartRuntimeAfterDisconnect } from "../../src/cli/disconnect-runtime"; +import { restartRuntimeAfterApply } from "../../src/cli/link-apply-restart"; import { createLocalAttestationChallenge, createLocalAttestationSecret, @@ -253,6 +270,111 @@ 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"]); + }); + } + + // Final review: the recycle hands off to the supervisor only when supervision is viable, so it + // receives the same verdict (isServiceViable included) the refusal path uses. + // Coordinator review round 2: the connection recheck must be the last step before the recycle, + // after the asynchronous supervisor lookup. The hook holds that lookup open while the connection + // becomes current again; the recycle must not run. + test("a connection current again during the supervisor lookup keeps this runtime", async () => { + let ended = true; + let releaseLookup!: (value: "desktop") => void; + let lookupEntered!: () => void; + let lookups = 0; + const entered = new Promise(resolve => { lookupEntered = resolve; }); + const recycled: unknown[] = []; + const { io } = fakeIo({ + connectionChanged: () => ended, + supervisor: () => { lookups += 1; if (lookups === 1) return Promise.resolve("desktop" as const); lookupEntered(); return new Promise(resolve => { releaseLookup = resolve; }); }, + recycle: async verdict => { recycled.push(verdict); }, + }); + const controller = createClientRestartController(io); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: false }); + await entered; + ended = false; + releaseLookup("desktop"); + await controller.settled(); + expect(recycled).toEqual([]); + }); + + // Final review: a manager that owns this process but cannot be proven to relaunch it must keep + // the runtime; refused at admission, nothing recycled or detached. + // The restart decision table (PR body): every supervisor verdict, with the connection ended + // (saved, valid, disconnected or another connection) or not (served connection current, or + // the saved state is invalid, missing or unreadable). + test.each([ + ["desktop", true, "accepted", ["recycle:desktop"]], + ["desktop", false, "supervised", []], + ["service", true, "accepted", ["recycle:service"]], + ["service", false, "supervised", []], + ["service-unproven", true, "supervised", []], + ["service-unproven", false, "supervised", []], + [null, true, "accepted", ["recycle:none"]], + [null, false, "accepted", ["stop:20000", "spawn:71000", "exit:0"]], + ] as const)("decision table: supervisor %s, connection ended %s", async (supervisor, ended, kind, expectedCalls) => { + const calls: string[] = []; + const { io, calls: ioCalls } = fakeIo({ supervisor: async () => supervisor, connectionChanged: () => ended, recycle: async verdict => { calls.push(`recycle:${verdict ?? "none"}`); } }); + const controller = createClientRestartController(io); + const admission = await controller.request(); + expect(admission.kind).toBe(kind); + if (kind === "supervised") expect(admission).toEqual({ kind: "supervised", supervisor }); + await controller.settled(); + expect([...calls, ...ioCalls]).toEqual([...expectedCalls]); + }); + + test("an owned but unproven service refuses even an ended connection, and recycles nothing", async () => { + const recycled: unknown[] = []; + const { io } = fakeIo({ supervisor: async () => "service-unproven", connectionChanged: () => true, recycle: async verdict => { recycled.push(verdict); } }); + const controller = createClientRestartController(io); + expect(await controller.request()).toEqual({ kind: "supervised", supervisor: "service-unproven" }); + await controller.settled(); + expect(recycled).toEqual([]); + }); + + test("the recycle receives the supervisor verdict", async () => { + for (const supervisor of ["desktop", "service", null] as const) { + const seen: unknown[] = []; + const { io } = fakeIo({ supervisor: async () => supervisor, connectionChanged: () => true, recycle: async verdict => { seen.push(verdict); } }); + const controller = createClientRestartController(io); + await controller.request(); + await controller.settled(); + expect(seen).toEqual([supervisor]); + } + }); + + 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 connection that is current again by the time the recycle would run keeps this runtime", async () => { + const verdicts = [true, false]; + const { io, calls } = fakeIo({ supervisor: async () => "desktop", connectionChanged: () => verdicts.shift() ?? false, recycle: async () => { calls.push("recycle"); } }); + const controller = createClientRestartController(io); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: false }); + await controller.settled(); + expect(calls).toEqual([]); + expect(controller.hasHandoff()).toBe(false); + }); + test("a second request while one is in flight joins it", async () => { let finish!: () => void; let spawns = 0; @@ -265,10 +387,232 @@ describe("client restart controller", () => { await controller.settled(); expect(spawns).toBe(1); }); + + test("a supervisor verdict that changes between the admission and the recycle lookups is honoured", async () => { + // Admitted with no supervisor, then unproven at the second lookup: kept, no recycle. + const turned = [null, "service-unproven"] as const; + let i = 0; + const late = fakeIo({ supervisor: async () => turned[Math.min(i++, 1)] ?? null, connectionChanged: () => true, recycle: async () => { late.calls.push("recycle"); } }); + const controller = createClientRestartController(late.io); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: false }); + await controller.settled(); + expect(late.calls.filter(call => call === "recycle")).toEqual([]); + const early = fakeIo({ supervisor: async () => "service-unproven", connectionChanged: () => true, recycle: async () => { early.calls.push("recycle"); } }); + expect(await createClientRestartController(early.io).request()).toEqual({ kind: "supervised", supervisor: "service-unproven" }); + expect(early.calls).toEqual([]); + }); +}); + +describe("who supervises a client restart (real service diagnostics)", () => { + const owned = { OCX_SERVICE: "1", OCX_SERVICE_MANAGED: "1" } as NodeJS.ProcessEnv; + const systemd = (enabled: boolean, running: boolean) => deriveSystemdServiceDiagnostic({ installed: true, enabled, running, stale: false, diagnostics: "test" }); + test.each([ + ["systemd running but disabled (started by hand)", systemd(false, true), "service-unproven"], + ["systemd enabled and running", systemd(true, true), "service"], + ["launchd job installed but not loaded", deriveLaunchdServiceDiagnostic({ installed: true, stale: false, load: { state: "not-loaded" } as never, diagnostics: "test" }), "service-unproven"], + ["launchd job loaded from the current plist", deriveLaunchdServiceDiagnostic({ installed: true, stale: false, load: { state: "loaded-current" }, diagnostics: "test" }), "service"], + ["Windows native service stopped", deriveWindowsServiceDiagnostic({ schedulerXml: "", schedulerExpectedUserId: null, schedulerAssetsPresent: false, nativeStatus: "stopped", recordedBackend: "native", staleBakedPaths: false, nativeRepairAssetsOnly: false, diagnostics: "test" }), "service-unproven"], + ] as const)("%s", (_name, diagnostic, expected) => { + expect(classifyClientRestartSupervisor({ desktop: false, env: owned, diagnose: () => diagnostic })).toBe(expected); + }); + + // Final review round 2: launchd's unknown load state stays viable for the diagnostic's other + // callers, but a restart needs positive evidence that the job is loaded. + test("launchd job whose load state is unknown is not proof of relaunch", () => { + const unknown = deriveLaunchdServiceDiagnostic({ installed: true, stale: false, load: { state: "unknown", detail: "launchctl could not be asked" }, diagnostics: "test" }); + expect(unknown.viable).toBe(true); + expect(classifyClientRestartSupervisor({ desktop: false, env: { OCX_SERVICE_MANAGED: "1" }, diagnose: () => unknown })).toBe("service-unproven"); + }); + + test("the Windows wrapper's marker counts as manager-launched", () => { + expect(classifyClientRestartSupervisor({ desktop: false, env: { OCX_SERVICE: "1", OCX_WINDOWS_WRAPPER_PROTOCOL: "1" }, diagnose: () => systemd(true, true) })).toBe("service"); + }); + + test("OCX_SERVICE_MANAGED alone marks the process as manager-owned", () => { + expect(classifyClientRestartSupervisor({ desktop: false, env: { OCX_SERVICE_MANAGED: "1" }, diagnose: () => systemd(false, true) })).toBe("service-unproven"); + }); + + test("no manager marker means no supervisor, whatever the service diagnostic says", () => { + expect(classifyClientRestartSupervisor({ desktop: false, env: {}, diagnose: () => systemd(true, true) })).toBeNull(); + }); + + test("the desktop app wins", () => { + expect(classifyClientRestartSupervisor({ desktop: true, env: owned, diagnose: () => systemd(false, true) })).toBe("desktop"); + }); +}); + +// Final review round 4 (scope split): a runtime whose only supervisor evidence is a bare +// OCX_SERVICE=1 (a legacy service child, or the proxy of ocx claude, ocx codex or ocx opencode) +// keeps origin/dev's `ocx restart` decision exactly. The oracle is written from the merge base +// b1148220574917c8: src/client/runtime-restart.ts:114-119 (desktop, else no OCX_SERVICE -> none, +// else isServiceViable() ? "service" : none) and :98-106 (a supervisor refuses, none restarts with +// a replacement; no ended-connection branch). +describe("a bare OCX_SERVICE=1 keeps origin/dev's ocx restart decision", () => { + const devSupervisor = (env: NodeJS.ProcessEnv, viable: boolean) => (env.OCX_SERVICE !== "1" ? null : viable ? "service" : null); + const bareEnvs = [ + ["a legacy service child", () => ({ OCX_SERVICE: "1", PATH: "/usr/bin" }) as NodeJS.ProcessEnv], + ["ocx claude's proxy", () => claudeProxyStartEnv({ PATH: "/usr/bin" })], + ["ocx codex's proxy", () => codexProxyStartEnv({ PATH: "/usr/bin" }) as NodeJS.ProcessEnv], + ["ocx opencode's proxy", () => opencodeProxyStartEnv({ PATH: "/usr/bin" })], + ] as const; + const anyDiagnostic = [ + deriveSystemdServiceDiagnostic({ installed: false, enabled: false, running: false, stale: false, diagnostics: "test" }), + deriveSystemdServiceDiagnostic({ installed: true, enabled: false, running: false, stale: false, diagnostics: "test" }), + deriveSystemdServiceDiagnostic({ installed: true, enabled: true, running: true, stale: false, diagnostics: "test" }), + ]; + for (const [name, env] of bareEnvs) { + for (const viable of [true, false]) { + test(`${name}, service viable ${viable}, connection ended or not: the same admission as origin/dev`, async () => { + expect(env().OCX_SERVICE).toBe("1"); + expect(restartOwnerProven({ desktop: false, env: env() })).toBe(false); + for (const diagnostic of anyDiagnostic) { + const supervisor = classifyClientRestartSupervisor({ desktop: false, env: env(), diagnose: () => diagnostic, legacyServiceViable: () => viable }); + expect(supervisor).toBe(devSupervisor(env(), viable)); + for (const ended of [true, false]) { + const recycled: unknown[] = []; + const { io, calls } = fakeIo({ + supervisor: async () => supervisor, + ownerProven: () => restartOwnerProven({ desktop: false, env: env() }), + connectionChanged: () => ended, + recycle: async verdict => { recycled.push(verdict); }, + }); + const controller = createClientRestartController(io); + const admission = await controller.request(); + await controller.settled(); + // origin/dev: a supervisor refuses; none runs the replacement restart. Never the recycle. + expect(admission).toEqual(viable ? { kind: "supervised", supervisor: "service" } : { kind: "accepted", alreadyRestarting: false }); + expect(calls).toEqual(viable ? [] : ["stop:20000", "spawn:71000", "exit:0"]); + expect(recycled).toEqual([]); + } + } + }); + } + } + + test("positive evidence opens the ended-connection recycle: desktop, a manager's marker, or no OCX_SERVICE", () => { + expect(restartOwnerProven({ desktop: true, env: { OCX_SERVICE: "1" } })).toBe(true); + expect(restartOwnerProven({ desktop: false, env: { OCX_SERVICE: "1", OCX_SERVICE_MANAGED: "1" } })).toBe(true); + expect(restartOwnerProven({ desktop: false, env: { OCX_SERVICE: "1", OCX_WINDOWS_WRAPPER_PROTOCOL: "1" } })).toBe(true); + expect(restartOwnerProven({ desktop: false, env: {} })).toBe(true); + }); + + // Final review round 6: a markerless legacy Windows registration (only OCX_SERVICE=1; its parent is + // this install's Task Scheduler wrapper) is recognised by the real parent-command-line derivation, + // but that is inference, not a current manager marker. It stays on origin/dev's path for restart, + // disconnect and sync. The production default is routed through the real Windows derivation with + // faked process data, so the branch is reached on any host. + test("a markerless legacy Windows service child stays on origin/dev's path: restart, disconnect, sync", async () => { + const wrapper = "C:\\Users\\u\\.opencodex\\service\\opencodex-service.cmd"; + const windowsParent = { platform: "win32" as const, parentPid: () => 4242, processCommandLine: () => `cmd.exe /d /c "${wrapper}"`, serviceHostPaths: () => [wrapper] }; + const env = { OCX_SERVICE: "1", PATH: "C:\\Windows" } as NodeJS.ProcessEnv; + const real = serviceChildOwnership.isSupervisedServiceChild; + expect(real(env, windowsParent)).toBe(true); + const spy = spyOn(serviceChildOwnership, "isSupervisedServiceChild").mockImplementation(e => real(e, windowsParent)); + try { + // Recorded with no owner, and classified as origin/dev classifies a bare flag (task disabled: not viable). + const owner = restartOwnerEvidence({ desktop: false, env }); + expect(owner).toBeNull(); + const supervisor = classifyClientRestartSupervisor({ desktop: false, env, diagnose: () => ({ viable: false, running: true }), legacyServiceViable: () => false }); + expect(supervisor).toBeNull(); + // ocx restart: origin/dev's replacement restart, never the recycle. + const { io, calls } = fakeIo({ supervisor: async () => supervisor, ownerProven: () => restartOwnerProven({ desktop: false, env }), connectionChanged: () => true, recycle: async () => { calls.push("recycle"); } }); + const controller = createClientRestartController(io); + expect(await controller.request()).toEqual({ kind: "accepted", alreadyRestarting: false }); + await controller.settled(); + expect(calls).toEqual(["stop:20000", "spawn:71000", "exit:0"]); + // ocx disconnect: the record states no owner, so no restart request and exit 0. + const record = clientRuntimeRecord(4321, 10100, createLocalAttestationSecret(), owner); + expect(record.restartOwner).toBeUndefined(); + let restarts = 0; + const code = await restartRuntimeAfterDisconnect({ + observe: async () => ({ kind: "live", live: { pid: 4321, port: 10100, role: "client" } }), + capture: () => ({ ...record, attestationSecret: record.attestationSecret! }), + connectionState: () => ({ kind: "disconnected" }), + restart: async () => { restarts += 1; return { ok: true }; }, + json: false, info: () => {}, error: () => {}, + }); + expect({ code, restarts }).toEqual({ code: 0, restarts: 0 }); + // ocx sync: restarted as on origin/dev, not reported as a service deferral. + const syncController = createClientRestartController({ ...fakeIo({ supervisor: async () => supervisor, ownerProven: () => restartOwnerProven({ desktop: false, env }), connectionChanged: () => false }).io }); + const outcome = await restartRuntimeAfterApply({ linkId: "lnk_x", fetchedAt: "2026-11-11T00:00:00.000Z", appliedAt: "2026-11-11T00:00:00.000Z", document: {} } as never, { + findLive: async () => ({ pid: 4321, port: 10100, source: "runtime", role: "client" }) as never, + readStartedAt: async () => 0, + restart: async (_start, onSupervised) => { const admission = await syncController.request(); await syncController.settled(); if (admission.kind === "supervised") { onSupervised(admission.supervisor); return false; } return admission.kind === "accepted"; }, + recoveryStart: async () => ({ status: "skipped" }), + connectionUnchanged: () => true, + }); + expect(outcome).toEqual({ kind: "restarted" }); + } finally { + spy.mockRestore(); + } + }); +}); + +// Final review: a missing config.json reads as the default configuration, which is not evidence that +// the served connection ended. The real reader is used here, with no injected state. +describe("connection-ended evidence from the real state reader", () => { + const link = { ...connection, transport: "link" as const, link: { linkId: `lnk_${"b".repeat(16)}`, tunnelPort: 23457 } }; + test("a missing config.json is not proof the connection ended", () => { + const empty = mkdtempSync(join(tmpdir(), "ocx-client-restart-missing-config-")); + const previous = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = empty; + try { + const state = readClientConnectionStateWithEvidence(); + expect(state).toEqual({ kind: "invalid", reason: "config.json is missing" }); + expect(clientConnectionEnded(link, state)).toBe(false); + } finally { + process.env.OPENCODEX_HOME = previous; + removeTreeWithRetry(empty); + } + }); + + // Final review round 2: a saved file that fails validation is not evidence either. + test("a saved config that fails validation is not proof", () => { + writeFileSync(join(root, "config.json"), JSON.stringify({ port: "bad" })); + const state = readClientConnectionStateWithEvidence(); + expect(state.kind).toBe("invalid"); + expect(clientConnectionEnded(link, state)).toBe(false); + }); + + // ...and the verdict comes from one read of config.json, so a connect that commits meanwhile cannot + // pair old bytes with a validation of new ones. Counted at the file system: the older two-read + // reader shows the count detects a second read. + test("the verdict comes from exactly one read of config.json", () => { + const path = join(root, "config.json"); + const reads = spyOn(fs, "readFileSync"); + const count = (read: () => unknown) => { + reads.mockClear(); + read(); + return reads.mock.calls.filter(([target]) => String(target) === path).length; + }; + try { + expect(count(readClientConnectionState)).toBe(2); + expect(count(readClientConnectionStateWithEvidence)).toBe(1); + } finally { + reads.mockRestore(); + } + }); + + test("a saved config with no client is proof", () => { + // beforeEach wrote a standalone config.json into this test's home. + const state = readClientConnectionStateWithEvidence(); + expect(state).toEqual({ kind: "disconnected" }); + expect(clientConnectionEnded(link, state)).toBe(true); + }); }); describe("rollback after a failed replacement", () => { const link = { ...connection, transport: "link" as const, link: { linkId: `lnk_${"b".repeat(16)}`, tunnelPort: 23457 } }; + // Only a proven end of the served connection may recycle a supervised runtime; an unreadable or + // mismatched config is not one, and recycling on it could take down a link that still works. + test("a supervised restart treats only a disconnect or a different connection as ended", () => { + expect(clientConnectionEnded(link, { kind: "disconnected" } as never)).toBe(true); + expect(clientConnectionEnded(link, { kind: "connected", value: { ...link, connectedAt: "2026-10-21T00:00:00.000Z" } })).toBe(true); + expect(clientConnectionEnded(link, { kind: "connected", value: { ...link, link: { ...link.link, linkId: `lnk_${"c".repeat(16)}` } } })).toBe(true); + expect(clientConnectionEnded(link, { kind: "connected", value: { ...link, catalogSyncedAt: "2026-10-21T00:00:00.000Z" } })).toBe(false); + expect(clientConnectionEnded(link, { kind: "invalid", reason: "config.json is missing or unreadable" } as never)).toBe(false); + expect(clientConnectionEnded(link, { kind: "mismatched", reason: "x" } as never)).toBe(false); + }); test("serves the same connection again only while it is still the durable one", () => { expect(clientRestartRollbackAction(link, { kind: "connected", value: { ...link, catalogSyncedAt: "2026-10-21T00:00:00.000Z" } })).toBe("resume"); expect(clientRestartRollbackAction(link, { kind: "disconnected" } as never)).toBe("recycle"); @@ -276,3 +620,69 @@ describe("rollback after a failed replacement", () => { expect(clientRestartRollbackAction(link, { kind: "connected", value: { ...link, link: { ...link.link, linkId: `lnk_${"c".repeat(16)}` } } })).toBe("recycle"); }); }); + +// Final review round 7: a marker proves only that a process was started with it. Shells a +// manager-owned runtime hands to tools inherit its environment, so a companion started from one +// would record a manager that does not own it. Every detached launch strips the markers. +describe("supervisor markers never reach a process their supervisor did not launch", () => { + test("one list, matching the constants that define each marker", () => { + expect([...SUPERVISOR_MARKER_ENVS].sort()).toEqual([DESKTOP_SUPERVISED_ENV, SERVICE_MANAGED_ENV, WINDOWS_WRAPPER_PROTOCOL_ENV].sort()); + }); + + const companions = [ + ["ocx claude", (base: NodeJS.ProcessEnv) => claudeProxyStartEnv(base)], + ["ocx codex", (base: NodeJS.ProcessEnv) => codexProxyStartEnv(base) as NodeJS.ProcessEnv], + ["ocx opencode", (base: NodeJS.ProcessEnv) => opencodeProxyStartEnv(base) as NodeJS.ProcessEnv], + ] as const; + for (const [name, launch] of companions) { + for (const marker of SUPERVISOR_MARKER_ENVS) { + test(name + " started from a shell carrying " + marker + ": no marker, no owner, disconnect leaves it", async () => { + const env = launch({ PATH: "/usr/bin", OCX_SERVICE: "1", [marker]: "1" }); + for (const other of SUPERVISOR_MARKER_ENVS) expect(env[other]).toBeUndefined(); + const owner = restartOwnerEvidence({ desktop: false, env }); + expect(owner).toBeNull(); + const record = clientRuntimeRecord(4321, 10100, createLocalAttestationSecret(), owner); + expect(record.restartOwner).toBeUndefined(); + let restarts = 0; + const code = await restartRuntimeAfterDisconnect({ + observe: async () => ({ kind: "live", live: { pid: 4321, port: 10100, role: "client" } }), + capture: () => ({ ...record, attestationSecret: record.attestationSecret! }), + connectionState: () => ({ kind: "disconnected" }), + restart: async () => { restarts += 1; return { ok: true }; }, + json: false, info: () => {}, error: () => {}, + }); + expect({ code, restarts }).toEqual({ code: 0, restarts: 0 }); + }); + } + } + + test("the standalone recycle and the restart replacement strip every marker", () => { + const saved = Object.fromEntries(SUPERVISOR_MARKER_ENVS.map(name => [name, process.env[name]])); + for (const name of SUPERVISOR_MARKER_ENVS) process.env[name] = "1"; + try { + const recycle = standaloneRecycleEnv(process.env, "fp"); + const replacement = replacementStartEnvironment(true); + for (const name of SUPERVISOR_MARKER_ENVS) { + expect(recycle[name]).toBeUndefined(); + expect(replacement[name]).toBeUndefined(); + } + } finally { + for (const name of SUPERVISOR_MARKER_ENVS) { if (saved[name] === undefined) delete process.env[name]; else process.env[name] = saved[name]; } + } + }); + + // The other launch sites build their environment inline; each must pass it through the helper. + test.each([ + ["src/cli/index.ts", "function detachedStartEnvironment(): NodeJS.ProcessEnv {\n const env: NodeJS.ProcessEnv = withoutSupervisorMarkers(withoutSiblingMarker(process.env));"], + ["src/cli/index.ts", "env: withProcessRuntimeProvenance(withoutSupervisorMarkers(withoutSiblingMarker(process.env))),"], + // The update bodies run under their tests' dependency injection, so they delete the list inline. + ["src/update/index.ts", `for (const name of ${JSON.stringify(SUPERVISOR_MARKER_ENVS).replaceAll(",", ", ")}) delete env[name];`], + ["src/adapters/cursor/native-exec-shell.ts", "backgroundShellRuntime.spawn(args.command, { cwd, shell: true, env: withoutSupervisorMarkers(process.env) });"], + ["src/adapters/cursor/native-exec-desktop.ts", "env: withoutSupervisorMarkers(config.env ? { ...process.env, ...config.env } : process.env),"], + ["src/update/job-pinned-start.ts", `for (const name of ${JSON.stringify(SUPERVISOR_MARKER_ENVS).replaceAll(",", ", ")}) delete env[name];`], + ["bin/ocx.mjs", `for (const name of ${JSON.stringify(SUPERVISOR_MARKER_ENVS).replaceAll(",", ", ")}) delete env[name];`], + ] as const)("%s strips the markers where it launches", (path, line) => { + expect(readFileSync(repoPath(path), "utf8")).toContain(line); + }); +}); + diff --git a/tests/clients/client-runtime.test.ts b/tests/clients/client-runtime.test.ts index 45da00f7d8a..44126a5104e 100644 --- a/tests/clients/client-runtime.test.ts +++ b/tests/clients/client-runtime.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -82,6 +82,21 @@ describe("standalone recycle environment", () => { }); describe("standalone recycle", () => { + // recycleStandalone removes this process's pid and runtime records and flushes link diagnostics + // in the OpenCodex home; every case runs against a temp home this process created. + let recycleHome = ""; + let previousRecycleHome: string | undefined; + beforeEach(() => { + previousRecycleHome = process.env.OPENCODEX_HOME; + recycleHome = mkdtempSync(join(tmpdir(), "ocx-recycle-home-")); + process.env.OPENCODEX_HOME = recycleHome; + }); + afterEach(() => { + if (previousRecycleHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousRecycleHome; + removeTreeWithRetry(recycleHome); + }); + async function withServiceMarker(value: string | undefined, run: () => Promise): Promise { const previous = process.env.OCX_SERVICE; if (value === undefined) delete process.env.OCX_SERVICE; @@ -161,6 +176,46 @@ describe("standalone recycle", () => { } }); + // Final review: a process launched with OCX_SERVICE=1 can outlive its service registration. A + // restart-driven recycle told the service is not viable must start a verified replacement (without + // the stale service marker) instead of exiting 1 into nothing. + test("a recycle told the service is not viable starts a replacement despite OCX_SERVICE=1", async () => { + await withServiceMarker("1", async () => { + const calls: string[] = []; + await recycleStandalone(serviceApiTokenFingerprint("disconnected-hub-token"), { + configuredPort: () => 10456, + isDesktopSupervised: () => false, + serviceSupervised: () => false, + spawnReplacement: async request => { calls.push(`spawn:${request.env?.OCX_SERVICE ?? "unset"}`); }, + exitProcess: code => { calls.push(`exit:${code}`); }, + }); + expect(calls).toEqual(["spawn:unset", "exit:0"]); + }); + }); + + // Final review: an authorized detached replacement carries no supervisor marker at all. + test("a detached replacement's environment carries no supervisor marker", async () => { + await withServiceMarker("1", async () => { + const markers = ["OCX_SERVICE", "OCX_SERVICE_MANAGED", "OCX_DESKTOP_SUPERVISED", "OCX_WINDOWS_WRAPPER_PROTOCOL"]; + const previous = Object.fromEntries(markers.map(name => [name, process.env[name]])); + for (const name of markers) process.env[name] = "1"; + let env: NodeJS.ProcessEnv | undefined; + try { + await recycleStandalone(serviceApiTokenFingerprint("disconnected-hub-token"), { + configuredPort: () => 10456, + isDesktopSupervised: () => false, + serviceSupervised: () => false, + spawnReplacement: async request => { env = request.env; }, + exitProcess: () => {}, + }); + } finally { + for (const name of markers) { if (previous[name] === undefined) delete process.env[name]; else process.env[name] = previous[name]; } + } + expect(env).toBeDefined(); + for (const name of markers) expect(env?.[name]).toBeUndefined(); + }); + }); + test("a supervised client still exits 1 without spawning", async () => { await withServiceMarker("1", async () => { const calls: string[] = []; diff --git a/tests/codex-integration/codex-journal.test.ts b/tests/codex-integration/codex-journal.test.ts index 5cd189eb3d5..56ebd5c0fbe 100644 --- a/tests/codex-integration/codex-journal.test.ts +++ b/tests/codex-integration/codex-journal.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test, beforeEach, afterEach, setDefaultTimeout } from "bun:test"; import { createHash } from "node:crypto"; -import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, lstatSync, symlinkSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, lstatSync, symlinkSync } from "node:fs"; import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -17,9 +17,11 @@ const repoRoot = dirname(fileURLToPath(new URL("../../package.json", import.meta setDefaultTimeout(SPAWN_BUDGET_MS); function runScript(codexHome: string, script: string): { stdout: string; stderr: string; status: number } { + mkdirSync(join(codexHome, "ocx-home"), { recursive: true }); const result = spawnSync(process.execPath, ["--eval", script], { cwd: repoRoot, - env: { ...process.env, CODEX_HOME: codexHome }, + // OPENCODEX_HOME too: adoption and injection take the config mutation lock in that home. + env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: join(codexHome, "ocx-home") }, encoding: "utf8", timeout: SPAWN_BUDGET_MS - 5_000, }); @@ -387,6 +389,157 @@ 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. + // A Remote Link Child linked while a standalone proxy had routing injected: `ocx connect --link` + // re-injects for the client. The journal must stay the live standalone's until the connection + // commits (a runtime starting mid-connect then skips it), and become the client's right after, or + // the supervisor's relaunch reads a dead process owner and restores native Codex under the link. + test("a client injection leaves the journal to its owner until connect commits and adopts 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"); + const { adoptJournalForClient } = require("./src/codex/journal"); + const fs = require("fs"), path = require("path"); + const owner = () => JSON.parse(fs.readFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8")).owner; + (async () => { + const config = { port: 10100, providers: {}, defaultProvider: "openai" }; + await injectCodexConfig(10100, config, { catalogPath: null }); + await injectCodexConfig(10100, config, { catalogPath: null, journalOwner: { kind: "client", apiKeyId: "link-key" } }); + const beforeCommit = owner(); + const adopted = adoptJournalForClient("link-key"); + const again = adoptJournalForClient("link-key"); + console.log(JSON.stringify({ beforeCommit: beforeCommit.kind, adopted, again, pid: process.pid })); + })(); + `); + expect(linked.status).toBe(0); + expect(JSON.parse(linked.stdout)).toMatchObject({ beforeCommit: "process", adopted: "adopted", again: "owned" }); + const journal = JSON.parse(readFileSync(join(testDir, "opencodex-journal.json"), "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); + }); + + // Review round 2: a stale sync must never hand a later connection's journal to its own key, so + // adoption never takes another client's journal and re-runs the caller's connection guard + // under the Codex config lock. + test("adoption never takes another client's journal and honors the caller's guard", () => { + writeFileSync(join(testDir, "config.toml"), '# original config\nmodel_provider = "openai"\n', "utf8"); + const r = runScript(testDir, ` + const { injectCodexConfig } = require("./src/codex/inject"); + const { adoptJournalForClient } = require("./src/codex/journal"); + const fs = require("fs"), path = require("path"); + const owner = () => JSON.parse(fs.readFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8")).owner; + (async () => { + const config = { port: 10100, providers: {}, defaultProvider: "openai" }; + await injectCodexConfig(10100, config, { catalogPath: null, journalOwner: { kind: "client", apiKeyId: "newer-key" } }); + const foreign = adoptJournalForClient("stale-key"); + const afterForeign = owner(); + let refused = null; + await injectCodexConfig(10100, config, { catalogPath: null }); + fs.writeFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), JSON.stringify({ ...JSON.parse(fs.readFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8")), owner: { kind: "process", pid: process.pid } })); + try { adoptJournalForClient("link-key", () => { throw new Error("client_connection_changed"); }); } catch (error) { refused = error.message; } + console.log(JSON.stringify({ foreign, afterForeign, refused, afterGuard: owner() })); + })(); + `); + expect(r.status).toBe(0); + const out = JSON.parse(r.stdout); + expect(out.foreign).toBe("foreign"); + expect(out.afterForeign).toEqual({ kind: "client", apiKeyId: "newer-key" }); + expect(out.refused).toBe("client_connection_changed"); + expect(out.afterGuard.kind).toBe("process"); + }); + + // Review round 3: the connection guard and the journal write share the config mutation lock + // (taken inside the Codex config lock), so no connection or runtime change can land between them; + // and a missing journal is reported as such, never as a completed handover. + test("adoption holds the config mutation lock through its guard and reports a missing journal", () => { + writeFileSync(join(testDir, "config.toml"), '# original config\nmodel_provider = "openai"\n', "utf8"); + const ocxHome = join(testDir, "ocx-home"); + require("node:fs").mkdirSync(ocxHome, { recursive: true }); + const r = spawnSync(process.execPath, ["--eval", ` + const { injectCodexConfig } = require("./src/codex/inject"); + const { adoptJournalForClient } = require("./src/codex/journal"); + const { prepareConfigMutationDatabasePathForWrite } = require("./src/config/mutation-lock"); + const { Database } = require("bun:sqlite"); + const fs = require("fs"), path = require("path"); + (async () => { + const missing = adoptJournalForClient("link-key"); + await injectCodexConfig(10100, { port: 10100, providers: {}, defaultProvider: "openai" }, { catalogPath: null }); + let competingWriter = "not-run"; + const mutationDb = prepareConfigMutationDatabasePathForWrite(); + const adopted = adoptJournalForClient("link-key", () => { + let db; + try { db = new Database(mutationDb); db.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); competingWriter = "admitted"; db.exec("ROLLBACK"); } + catch (error) { competingWriter = /locked|busy/i.test(String(error)) ? "refused" : String(error); } + finally { db?.close(); } + }); + console.log(JSON.stringify({ missing, adopted, competingWriter })); + })(); + `], { cwd: repoRoot, env: { ...process.env, CODEX_HOME: testDir, OPENCODEX_HOME: ocxHome }, encoding: "utf8", timeout: SPAWN_BUDGET_MS - 5_000 }); + expect(r.status).toBe(0); + expect(JSON.parse(r.stdout.trim())).toEqual({ missing: "missing", adopted: "adopted", competingWriter: "refused" }); + }); + + // Review round 4: external Codex providers (approved or not) leave no standard journal, and no + // journal means startup recovery has nothing to restore. Only another client's journal leaves the + // routing at risk, so only that counts as an incomplete handover. + test("an external provider leaves no journal to hand over, and only a foreign journal is incomplete", () => { + writeFileSync(join(testDir, "config.toml"), 'model_provider = "acme"\n\n[model_providers.acme]\nname = "Acme"\nbase_url = "https://acme.example.test/v1"\n', "utf8"); + const r = runScript(testDir, ` + const { injectCodexConfig } = require("./src/codex/inject"); + const { adoptJournalForClient, journalHandoverIncomplete } = require("./src/codex/journal"); + (async () => { + const injected = await injectCodexConfig(10100, { port: 10100, providers: {}, defaultProvider: "openai" }, { catalogPath: null, journalOwner: { kind: "client", apiKeyId: "link-key" } }); + const adoption = adoptJournalForClient("link-key"); + const verdicts = ["adopted", "owned", "missing", "foreign"].map(journalHandoverIncomplete); + console.log(JSON.stringify({ success: injected.success, adoption, verdicts })); + })(); + `); + expect(r.status).toBe(0); + expect(JSON.parse(r.stdout)).toEqual({ success: true, adoption: "missing", verdicts: [false, false, false, true] }); + }); + + // Review round 5: only a confirmed absence is safe. A journal that passes the ownership check but + // cannot be read on the handover's own read must fail the handover, never read as "missing". + test("an unreadable journal on the handover read fails instead of reading as missing", () => { + writeFileSync(join(testDir, "config.toml"), '# original config\nmodel_provider = "openai"\n', "utf8"); + const r = runScript(testDir, ` + const { spyOn } = require("bun:test"); + const { injectCodexConfig } = require("./src/codex/inject"); + const owner = require("./src/codex/codex-home-owner"); + const { adoptJournalForClient } = require("./src/codex/journal"); + (async () => { + await injectCodexConfig(10100, { port: 10100, providers: {}, defaultProvider: "openai" }, { catalogPath: null }); + const real = owner.readCodexHomeJournal; + let calls = 0; + spyOn(owner, "readCodexHomeJournal").mockImplementation(path => (++calls === 2 ? { kind: "unknown" } : real(path))); + let outcome; + try { outcome = adoptJournalForClient("link-key"); } catch (error) { outcome = "threw"; } + const fs = require("fs"), path = require("path"); + const kind = JSON.parse(fs.readFileSync(path.join(process.env.CODEX_HOME, "opencodex-journal.json"), "utf8")).owner.kind; + console.log(JSON.stringify({ outcome, calls, kind })); + })(); + `); + expect(r.status).toBe(0); + const out = JSON.parse(r.stdout); + expect(out.calls).toBeGreaterThanOrEqual(2); + expect(out.outcome).toBe("threw"); + expect(out.kind).toBe("process"); + }); + test("removeJournal refuses unknown journal evidence", () => { const journalPath = join(testDir, "opencodex-journal.json"); writeFileSync(journalPath, "{}", "utf8"); diff --git a/tests/fixtures/client-link-journal-runner.ts b/tests/fixtures/client-link-journal-runner.ts new file mode 100644 index 00000000000..0db18510113 --- /dev/null +++ b/tests/fixtures/client-link-journal-runner.ts @@ -0,0 +1,99 @@ +/** + * Remote Link Codex-journal scenarios for tests/clients/client-link-teardown.test.ts. + * + * The Codex paths (config.toml, the journal, the catalog) are fixed when src/codex/paths loads, so + * a scenario that writes them directly runs here, in a child whose CODEX_HOME and OPENCODEX_HOME the + * parent test set to its own temp directory before anything was imported. Prints one JSON result. + */ +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +// First, before any source module loads or anything is written: the parent test creates the temp +// home, writes a one-time token into it and hands the same token over. A direct run (a developer's +// real HOME, no token, or a token the home does not carry) stops here. +{ + const token = process.env.OCX_FIXTURE_TOKEN; + const marker = process.env.HOME ? join(process.env.HOME, ".ocx-fixture-token") : ""; + let carried: string | null = null; + try { carried = marker && existsSync(marker) ? readFileSync(marker, "utf8") : null; } catch { carried = null; } + if (!token || carried !== token) { + console.error("client-link-journal-runner.ts is a test fixture; run it through tests/clients/client-link-teardown.test.ts."); + process.exit(2); + } +} + +const scenario = readFileSync(0, "utf8").trim() as "catalog-changed" | "standalone-handover" | "foreign-journal"; +const home = process.env.OPENCODEX_HOME!; +const { getDefaultConfig, saveConfig } = await import("../../src/config"); +const { connectClient } = await import("../../src/client/connect"); +const { injectCodexConfig } = await import("../../src/codex/inject"); +const { handleDisconnectCommand } = await import("../../src/cli/connect"); +const { CODEX_CONFIG_PATH, DEFAULT_CATALOG_PATH } = await import("../../src/codex/paths"); +const { JOURNAL_PATH } = await import("../../src/codex/journal"); + +// Prove by path that every Codex and OpenCodex path this child writes is inside the temp home the +// parent created and passed as HOME; refuse before the first write otherwise. +const { realpathSync } = await import("node:fs"); +const { dirname: parentOf, relative, isAbsolute } = await import("node:path"); +const ownedRoot = realpathSync.native(process.env.HOME!); +const inside = (path: string) => { const rel = relative(ownedRoot, realpathSync.native(path)); return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); }; +for (const path of [parentOf(CODEX_CONFIG_PATH), parentOf(DEFAULT_CATALOG_PATH), parentOf(JOURNAL_PATH), home]) { + if (!inside(path)) throw new Error(`refusing to run: ${path} is not inside the owned test home ${ownedRoot}`); +} + +const linkId = "lnk_0123456789abcdef"; +const key = `ocx_data_${"a".repeat(40)}`; +const lifecycleLockDeps = { lockPath: join(home, "lifecycle.sqlite") }; +const config = getDefaultConfig(); +config.port = 10100; +saveConfig(config); +writeFileSync(CODEX_CONFIG_PATH, "model_provider = \"openai\"\n"); +const owner = () => JSON.parse(readFileSync(JOURNAL_PATH, "utf8")).owner as unknown; +const connect = (noSync: boolean) => 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 ? { noSync: true } : {}), +}, { + fetchImpl: (async (input: RequestInfo | URL) => String(input).endsWith("/readyz") + ? Response.json({ status: "ready", protocol: 1, minimumClientProtocol: 1, managementUrl: "http://127.0.0.1:34567" }) + : Response.json({ models: [] })) as typeof fetch, + catalogCompatibility: { supportedEfforts: () => new Set() }, + lifecycleLockDeps, +}); +const warnings: string[] = [], errors: string[] = []; +console.warn = (...parts: unknown[]) => { warnings.push(parts.map(String).join(" ")); }; +const log = console.log; +console.log = () => {}; +const result: Record = {}; + +if (scenario === "catalog-changed") { + await connect(true); + // Something else rewrote the catalog after connect: neither ours nor the recorded prior one. + writeFileSync(DEFAULT_CATALOG_PATH, JSON.stringify({ models: [{ slug: "rewritten-elsewhere" }] })); + console.error = (...parts: unknown[]) => { errors.push(parts.map(String).join(" ")); }; + result.exit = await handleDisconnectCommand([], { + lifecycleLockDeps, + linkTeardownDeps: { + reapOrphanTunnel: async () => ({ tunnel: "absent" as const }), + runner: { run: async () => ({ code: 1, stdout: "", stderr: "" }) }, + knownHostsFile: join(home, "known_hosts"), + }, + }); +} else { + // The standalone proxy's own injection (journaled to this live process), or an earlier client's. + const earlier = await injectCodexConfig(10100, { ...config, syncResumeHistory: false }, { + catalogPath: null, + ...(scenario === "foreign-journal" ? { journalOwner: { kind: "client" as const, apiKeyId: "earlier-key" } } : {}), + }); + result.injected = earlier.success; + result.before = owner(); + await connect(false); + result.after = owner(); +} +console.log = log; +console.log(JSON.stringify({ ...result, warnings, errors })); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 96a958e6a5b..e85ffe756ea 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1458,7 +1458,7 @@ "system-routes.test.ts": "server", "systemd-install-cleanup-hardening.test.ts": "service", "tencent-siliconflow-providers.test.ts": "gui", "terminal-continuation-owner-rotation.test.ts": "adapters", "terminal-guard-server.test.ts": "server", "terminal-guard.test.ts": "server", - "test-home-guard.test.ts": "ci-workflows", "test-home-guard-unpreloaded.test.ts": "ci-workflows", + "test-home-guard.test.ts": "ci-workflows", "codex-home-sandbox.test.ts": "ci-workflows", "test-home-guard-unpreloaded.test.ts": "ci-workflows", "test-run-lock-generation.test.ts": "ci-workflows", "test-runner.test.ts": "ci-workflows", "test-runner-bun.test.ts": "ci-workflows", "test-runner-spawnsync-finalizer.test.ts": "ci-workflows", "test-sandbox-cleanup.test.ts": "ci-workflows", "test-temp-quarantine.test.ts": "ci-workflows", "thought-signature-credential-scope.test.ts": "responses", @@ -1630,7 +1630,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" diff --git a/tests/helpers/codex-home-sandbox.ts b/tests/helpers/codex-home-sandbox.ts new file mode 100644 index 00000000000..9f05e8af4ef --- /dev/null +++ b/tests/helpers/codex-home-sandbox.ts @@ -0,0 +1,59 @@ +/** + * Import FIRST in a test file whose code writes Codex paths (raw fs calls, or source that writes + * the import-time paths of src/codex/paths). Module evaluation follows import order. + * + * Unconditional, and decided by paths, never by an inherited flag: + * - If src/codex/paths has not been evaluated yet (no preload ran), CODEX_HOME is pointed at a temp + * directory this module creates in this process; only that directory is removed at exit. + * - If it has (this repository's preload loads it), its CODEX_HOME was fixed by the preload. + * Either way the test file then calls {@link assertCodexHomeOwned} with the Codex home its paths + * actually resolved to, and refuses to load unless that directory is the one created here, or the + * `.codex` of a test root whose owner marker (scripts/test-temp.ts) names this very process. + * Imports nothing from src/ on purpose. + */ +import { existsSync, lstatSync, mkdtempSync, readFileSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, dirname, join } from "node:path"; + +const OWNER_FILE = ".opencodex-test-owner.json"; + +function pathsModuleLoaded(): boolean { + return Object.keys(require.cache).some(key => key.replaceAll("\\", "/").endsWith("/src/codex/paths.ts")); +} + +let created: string | null = null; +if (!pathsModuleLoaded()) { + created = realpathSync.native(mkdtempSync(join(tmpdir(), "ocx-test-codex-home-"))); + process.env.CODEX_HOME = created; + const owned = created; + process.on("exit", () => { rmSync(owned, { recursive: true, force: true }); }); +} + +/** Whether `dir` is the `.codex` of a test root whose owner marker names this process. */ +function ownedTestRootCodexHome(dir: string): boolean { + if (basename(dir) !== ".codex") return false; + const root = dirname(dir); + const marker = join(root, OWNER_FILE); + try { + if (!existsSync(marker) || !lstatSync(marker).isFile() || lstatSync(marker).isSymbolicLink()) return false; + const owner = JSON.parse(readFileSync(marker, "utf8")) as { kind?: unknown; root?: unknown; pid?: unknown }; + return owner.kind === "opencodex-test-root" && owner.pid === process.pid + && typeof owner.root === "string" && realpathSync.native(owner.root) === root; + } catch { + return false; + } +} + +/** Throw unless `codexHome` is a Codex home this process owns (see the module comment). */ +export function assertCodexHomeOwned(codexHome: string): void { + let resolved: string; + try { resolved = realpathSync.native(codexHome); } catch { resolved = codexHome; } + if (created !== null && resolved === created) return; + if (ownedTestRootCodexHome(resolved)) return; + throw new Error(`refusing to run: the Codex home ${resolved} is not a temp directory this test process owns. Import tests/helpers/codex-home-sandbox.ts before anything that loads src/codex/paths.`); +} + +/** The directory this module created, or null when the preload had already fixed the paths. */ +export function createdCodexHomeForTests(): string | null { + return created; +} 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 => { 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");