diff --git a/scripts/test-layout/layout-start-ownership.json b/scripts/test-layout/layout-start-ownership.json index 97b289f6539..b112773799a 100644 --- a/scripts/test-layout/layout-start-ownership.json +++ b/scripts/test-layout/layout-start-ownership.json @@ -2,5 +2,7 @@ "cli-start-owner-absence.test.ts": "cli", "cli-start-owner-evidence.test.ts": "cli", "proxy-liveness-owner-evidence.test.ts": "server", - "proxy-liveness-recovery.test.ts": "server" + "proxy-liveness-recovery.test.ts": "server", + "proxy-liveness-bind-scope.test.ts": "server", + "system-restart-ownership.test.ts": "server" } diff --git a/src/cli/ensure-desired-integrations.ts b/src/cli/ensure-desired-integrations.ts index 6897c4dcc16..d3804992fe4 100644 --- a/src/cli/ensure-desired-integrations.ts +++ b/src/cli/ensure-desired-integrations.ts @@ -48,7 +48,7 @@ export interface EnsureDesiredIntegrationsDeps { syncGrokConfig: ( port: number, config: OcxConfig, - opts?: { hostname?: string }, + opts?: { hostname?: string; probeHostname?: string }, ) => Promise; removeDesktop3pStandardPivot: typeof removeDesktop3pStandardPivot; removeDesktopFirstParty?: typeof removeDesktopFirstParty; @@ -67,7 +67,7 @@ export interface EnsureDesiredIntegrationsDeps { async function defaultSyncGrokConfig( port: number, config: OcxConfig, - opts: { hostname?: string } = {}, + opts: { hostname?: string; probeHostname?: string } = {}, ): Promise { const { syncGrokConfig } = await import("../grok/sync"); return syncGrokConfig(port, config, opts); @@ -100,7 +100,7 @@ function io(deps: EnsureDesiredIntegrationsDeps): { */ export async function ensureGrokFenceMatchesDesired( port: number, - opts: { hostname?: string } = {}, + opts: { hostname?: string; probeHostname?: string } = {}, deps: EnsureDesiredIntegrationsDeps = productionDeps, ): Promise { const config = deps.loadConfig(); @@ -132,7 +132,8 @@ export async function ensureGrokFenceMatchesDesired( const g = await deps.syncGrokConfig( port, config, - hostname !== undefined ? { hostname } : {}, + { ...(hostname !== undefined ? { hostname } : {}), + ...(opts.probeHostname !== undefined ? { probeHostname: opts.probeHostname } : {}) }, ); if (g.changed) log(" + Grok Build config updated (~/.grok/config.toml)"); else if (!g.ok) error(`⚠️ ${g.message}`); @@ -227,12 +228,12 @@ export async function ensureClaudeDesktopMatchesDesired( } export type EnsureDesiredIntegrationsBranch = - | { kind: "live"; hostname?: string } + | { kind: "live"; hostname?: string; probeHostname?: string } | { kind: "spawned" }; /** * Reconcile the two external integration files after either ensure race window. - * Only the live proxy's observed bind host crosses this boundary; persisted + * The live proxy's bind scope and responding address cross this boundary; persisted * config is deliberately loaded inside each mutation helper. */ export async function reconcileEnsureDesiredIntegrations( @@ -243,7 +244,8 @@ export async function reconcileEnsureDesiredIntegrations( const liveHost = branch.kind === "live" ? branch.hostname : undefined; await ensureGrokFenceMatchesDesired( port, - liveHost ? { hostname: liveHost } : {}, + { ...(liveHost ? { hostname: liveHost } : {}), + ...(branch.kind === "live" && branch.probeHostname !== undefined ? { probeHostname: branch.probeHostname } : {}) }, deps, ); await ensureClaudeDesktopMatchesDesired(deps); diff --git a/src/cli/index.ts b/src/cli/index.ts index 72174355fba..d1439754154 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -854,13 +854,13 @@ async function handleEnsure(options: { existingIsSuccess?: boolean; forceStart?: const systemEnv = await injectSystemEnv(live.port, config).catch(() => ({ injected: false })); if (siblingOfLivePort() === null) reportShellHookFailure(reconcileShellHook(systemEnv.injected)); if (!systemEnv.injected) await syncClaudeAgentDefsAtProxyStartup(config, live.port); - // Refresh the Grok Build fence too (same contract as start). live.hostname is the - // hostname the running proxy actually bound — config.hostname may have drifted. - // The reconciler re-reads immediately before each client-file mutation; only - // the live proxy's observed bind host is safe to carry across this boundary. + // Refresh the Grok Build fence too (same contract as start). Keep the observed + // bind scope for admission and its responding address for the destination. + // The reconciler re-reads immediately before each client-file mutation; + // config.hostname may have drifted from the running proxy's bind scope. if (siblingOfLivePort() === null) await reconcileEnsureDesiredIntegrations( live.port, - { kind: "live", hostname: live.hostname }, + { kind: "live", hostname: live.hostname, probeHostname: live.probeHostname }, ); console.log(`✅ Proxy running on port ${live.port}`); return true; diff --git a/src/grok/inject.ts b/src/grok/inject.ts index 082997c0732..ab683eefbae 100644 --- a/src/grok/inject.ts +++ b/src/grok/inject.ts @@ -1119,6 +1119,7 @@ export function injectGrokConfig( opts: { grokHome?: string; hostname?: string; + probeHostname?: string; refreshOnly?: { admit: () => boolean }; excluded?: ReadonlySet; /** Unfiltered known ids used only to distinguish hidden current models from retired ones. */ @@ -1215,7 +1216,7 @@ export function injectGrokConfig( const region = orphans.length > 0 ? findManagedRegion(content) : originalRegion; const buildCandidate = (reservedAliases: ReadonlySet): string => { - const block = buildGrokManagedBlock(port, models, opts.hostname, reservedAliases, opts.excluded); + const block = buildGrokManagedBlock(port, models, opts.probeHostname ?? opts.hostname, reservedAliases, opts.excluded); let candidate: string; if (region) { candidate = content.slice(0, region.start) + block + content.slice(region.end); diff --git a/src/grok/sync.ts b/src/grok/sync.ts index 491c1f6cf03..46571b030d4 100644 --- a/src/grok/sync.ts +++ b/src/grok/sync.ts @@ -26,12 +26,13 @@ async function defaultFetchAllModels(config: OcxConfig): Promise * Build the model list and inject the fenced block. `hostname` should be the hostname the * RUNNING proxy actually bound (live.hostname from proxy-liveness for ensure's live branch; * config.hostname for a freshly spawned start) — a stale config.hostname could otherwise - * name a host the process never bound. + * name a host the process never bound. `probeHostname` is only the reachable destination; + * it never replaces bind scope for admission policy or an explicit secondary listener. */ export async function syncGrokConfig( port: number, config: OcxConfig, - opts: { hostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean } } = {}, + opts: { hostname?: string; probeHostname?: string; grokHome?: string; refreshOnly?: { admit: () => boolean } } = {}, deps: GrokSyncDeps = { fetchAllModels: defaultFetchAllModels, injectGrokConfig }, ): Promise { let projection: ReturnType; @@ -57,6 +58,8 @@ export async function syncGrokConfig( hostname: target.requiresAdmissionToken ? (opts.hostname ?? config.hostname) : targetUrl.hostname, + ...(opts.probeHostname !== undefined && !config.unauthenticatedLoopbackListener?.enabled + ? { probeHostname: opts.probeHostname } : {}), ...(opts.grokHome !== undefined ? { grokHome: opts.grokHome } : {}), ...opts.refreshOnly ? { refreshOnly: opts.refreshOnly } : {}, excluded: new Set(config.grokExcludedModels ?? []), diff --git a/src/server/management/system-restart.ts b/src/server/management/system-restart.ts index 0f233f99a3c..35e3cefd6e5 100644 --- a/src/server/management/system-restart.ts +++ b/src/server/management/system-restart.ts @@ -61,6 +61,7 @@ import { isDesktopSupervised, } from "../../lib/system-restart-contract"; import { spawnReplacementStart } from "../restart-replacement"; +import { retireRestartParentOwnership } from "../restart-parent-ownership"; export { MEMORY_DRAIN_RESTART_MS, REPLACEMENT_READY_TIMEOUT_MS } from "../../lib/system-restart-contract"; export { waitForReplacementReady, type ReplacementReadinessIo } from "../restart-replacement"; @@ -289,6 +290,9 @@ function spawnDetachedStart( let env: NodeJS.ProcessEnv; try { env = replacementStartEnvironment(waitForHealthBeforeParentExit); + // Prepare the sibling handoff while its runtime record still exists. Only completed + // drain + listener stop may release parent ownership before waiting for child health. + if (waitForHealthBeforeParentExit) retireRestartParentOwnership(port); } catch (err) { return Promise.reject(err); } diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index ee4669011a9..78ec8c74e9c 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -157,8 +157,10 @@ type LivenessFetch = ( export interface LiveProxy { pid: number | null; port: number; - /** Raw bind hostname the probe succeeded against; compose URLs via `probeHostname`. */ + /** Candidate bind scope, with the default loopback made explicit; never a probe alias. */ hostname?: string; + /** Successful recovery probe address, when discovery checked both loopback families. */ + probeHostname?: string; /** Whether the successful probe used runtime-port metadata or the configured listen port. */ source: "runtime" | "config"; /** @@ -202,7 +204,7 @@ export type HomeOwnershipProof = "proven" | "refuted" | "indeterminate"; export async function proveLiveProxyOwnedByHome(live: LiveProxy, io: LivenessIo = {}): Promise { if (live.pid === null) return "indeterminate"; return attestFencedIdentity( - `http://${probeHostname(live.hostname)}:${live.port}/healthz`, + `http://${probeHostname(live.probeHostname ?? live.hostname)}:${live.port}/healthz`, live.port, live.pid, io, @@ -519,10 +521,13 @@ async function discoverLiveProxy(io: LivenessIo, observer?: { deadlineAt !== undefined && nowFn() >= deadlineAt; const expired = (): null => { observer?.endpoint("unknown"); return null; }; const probe = async (port: number, opts: { hostname?: string; expectedPid?: number }) => { + // Missing/blank means an actual loopback bind, not a later config's bind scope. + const bindHostname = opts.hostname?.trim() || "127.0.0.1"; const hostname = normalizeBindHostname(opts.hostname); if (!observer || (!isLoopbackHostname(hostname) && !isWildcardHostname(hostname))) { const identity = await proxyIdentityAtObserved(port, opts, probeIo, observer?.endpoint); - return identity ? { ...identity, hostname: opts.hostname } : null; + return identity ? { ...identity, hostname: bindHostname, + ...(observer ? { probeHostname: probeHostname(opts.hostname) } : {}) } : null; } // Parallel families share the existing per-probe attempts/timeout and absolute // deadline. A slow family cannot multiply the serial discovery budget. @@ -530,7 +535,7 @@ async function discoverLiveProxy(io: LivenessIo, observer?: { (io.loopbackAvailabilityFn ?? loopbackFamilyAvailability)(host) !== "unavailable"); const identities = await Promise.all(hosts.map(async hostname => { const identity = await proxyIdentityAtObserved(port, { ...opts, hostname }, probeIo, observer.endpoint); - return identity ? { ...identity, hostname } : null; + return identity ? { ...identity, hostname: bindHostname, probeHostname: hostname } : null; })); return identities.find(identity => identity !== null) ?? null; }; @@ -568,6 +573,7 @@ async function discoverLiveProxy(io: LivenessIo, observer?: { pid: trusted, port: runtime.port, hostname: identity.hostname, + ...(identity.probeHostname === undefined ? {} : { probeHostname: identity.probeHostname }), source: "runtime", ...(identity.version === undefined ? {} : { version: identity.version }), ...(identity.role === undefined ? {} : { role: identity.role }), @@ -593,6 +599,7 @@ async function discoverLiveProxy(io: LivenessIo, observer?: { pid: verifiedReportedPid(identity.pid), port: record.port, hostname: identity.hostname, + ...(identity.probeHostname === undefined ? {} : { probeHostname: identity.probeHostname }), source: "runtime", ...(identity.version === undefined ? {} : { version: identity.version }), ...(identity.role === undefined ? {} : { role: identity.role }), @@ -612,6 +619,7 @@ async function discoverLiveProxy(io: LivenessIo, observer?: { pid: verifiedReportedPid(identity.pid) ?? killablePid(pid), port, hostname: identity.hostname, + ...(identity.probeHostname === undefined ? {} : { probeHostname: identity.probeHostname }), source: "config", ...(identity.version === undefined ? {} : { version: identity.version }), ...(identity.role === undefined ? {} : { role: identity.role }), diff --git a/src/server/restart-parent-ownership.ts b/src/server/restart-parent-ownership.ts new file mode 100644 index 00000000000..db2cfc31525 --- /dev/null +++ b/src/server/restart-parent-ownership.ts @@ -0,0 +1,49 @@ +import { lstatSync } from "node:fs"; +import { + getPidPath, getRuntimePortPath, readPidFileValue, readRuntimePort, removePid, removeRuntimePort, +} from "../config/process-state"; +import { acquireOwnershipMutationLease } from "../service/ownership-mutation-lease.mjs"; +import { serviceStatePaths } from "../service/state"; + +function refusal(): Error { + return Object.assign(new Error("Restart parent ownership could not be retired safely"), { + code: "RESTART_PARENT_OWNERSHIP_UNVERIFIED", + }); +} + +/** Only ENOENT proves absence; an unreadable, malformed or linked record is not ours to retire. */ +function recordExists(path: string): boolean { + try { + if (!lstatSync(path).isFile()) throw refusal(); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw refusal(); + } +} + +/** + * Called only after completed drain and successful listener stop, before waiting for child health. + * The parent is still alive, but no longer owns a listener or startup recovery. Retire only this + * process's records under the same lease that serializes child recovery and publication. Ordinary + * live/unknown owners retain the startup fence; no child marker grants an exemption from it. + */ +export function retireRestartParentOwnership(port?: number): void { + const lease = acquireOwnershipMutationLease(serviceStatePaths()); + try { + const pidPath = getPidPath(), runtimePath = getRuntimePortPath(); + const hasPid = recordExists(pidPath), hasRuntime = recordExists(runtimePath); + const runtime = readRuntimePort(); + if ((hasPid && readPidFileValue() !== process.pid) + || (hasRuntime && (!runtime || runtime.pid !== process.pid + || (port !== undefined && runtime.port !== port)))) throw refusal(); + // Keep the alive-PID fence if runtime retirement fails. The helpers also recheck the PID + // immediately before unlink, so a successor's record is never unconditional cleanup. + if (hasRuntime) removeRuntimePort(process.pid); + if (recordExists(runtimePath)) throw refusal(); + if (hasPid) removePid(process.pid); + if (recordExists(pidPath)) throw refusal(); + } finally { + lease.release(); + } +} diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index 9b41f9d7f7e..20c6cefe2c6 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -540,32 +540,32 @@ API opt-in; dashboard, CLI, tray, join and automatic restarts keep their default turn may have an unknown upstream outcome and is not automatically replayed. `tests/server/system-restart-admission.test.ts` covers this contract and active-turn preservation. -A dashboard drain-and-restart (`src/server/management/system-restart.ts`, which is also the restart -after a join into a Child) and the client runtime's standalone recycle (`src/client/runtime.ts`) -replace their process through `src/server/restart-replacement.ts`. Every replacement `ocx start` -carries `OCX_RESTART_PARENT_PID`. `handleStart` consumes the marker before its first probe and -honors it only when it names the process's real parent. When the live owner that probe finds is -exactly that pid, `decideStartWithLiveOwner` answers `await-parent`, and `src/cli/restart-handoff.ts` -waits up to 30 seconds for the parent to exit or stop answering (re-probing once a second) before it -probes again; a parent that outlives the wait is refused like any live proxy. An ordinary start -carries no marker and probes once. - -Only a handoff that waits for health (the drain completed) respawns a replacement that exits before -it answers, at most twice, inside the one 70-second readiness budget. A spawn error is not retried. -A parent-exit handoff (drain deadline, failed or rejected drain, listener-stop fallback) resolves as -soon as the child spawned: the parent must exit to release what the replacement waits for, so it -cannot watch for an early exit. The replacement's `await-parent` wait and the link-mode port reclaim -cover the known transient causes there. Any other early exit on that path is a residual: no proxy -serves the port until something runs `ocx start` again, and Codex keeps pointing at that port. - -The replacement's stdout and stderr go to `/restart-handoff.log`: mode 0600, opened -without following a symlink, recorded as an owned config path, and bounded at 256 KiB on both sides. -A handoff empties the file before it opens it. The replacement keeps writing to it for its whole -life, so the parent hands it `OCX_RESTART_HANDOFF_LOG=1` along with the file; `handleStart` consumes -the flag and arms one unref'd 60-second timer (`armRestartHandoffLogCap`) that lstats the file and -empties it at the cap through a fresh descriptor. A start without the flag arms nothing. The parent -writes only timestamps, pids, ports, attempt counts, exit codes and errno labels, never an -environment value. +A dashboard drain-and-restart (`src/server/management/system-restart.ts`, including a join into a Child) and the +client runtime's standalone recycle (`src/client/runtime.ts`) use `src/server/restart-replacement.ts`. Every child +carries `OCX_RESTART_PARENT_PID`; `handleStart` consumes it before probing and honors only its real parent PID. +If that parent still answers, `decideStartWithLiveOwner` returns `await-parent`: `src/cli/restart-handoff.ts` waits +up to 30 seconds, re-probing each second, then refuses a surviving owner. An ordinary start probes once without a marker. + +After completed drain and successful listener stop, the unsupervised dashboard parent prepares its sibling handoff, +then `src/server/restart-parent-ownership.ts` retires only its own PID/runtime records under the ownership mutation +lease before spawning and waiting for child health. Foreign PIDs, mismatched runtime ports, malformed, linked or +unreadable records and incomplete removal refuse the handoff. Only ENOENT proves a missing record. The lease is +released before spawning, so child recovery and publication can acquire it. The normal fence still refuses live and +unknown owners; no child marker bypasses it. Exact-PID exit cleanup preserves the successor's records. +`tests/server/system-restart-ownership.test.ts` covers refusal controls and real processes, drain, alive-parent endpoint +refusal, replacement health and parent exit; its fixture does not start providers or exercise dashboard HTTP authentication. + +A completed-drain health wait retries a replacement that exits before answering at most twice within one 70-second +readiness budget; spawn errors are not retried. A parent-exit handoff (drain deadline, failed or rejected drain, +listener-stop fallback) resolves as soon as the child spawns, because the parent must exit to release what the child +waits for. It cannot watch for early exit. The child's `await-parent` wait and link-mode port reclaim cover the known +transient causes. Other early exits leave the port unserved until another `ocx start`, while Codex keeps pointing there. + +Replacement output goes to `/restart-handoff.log`: private mode 0600, opened without following symlinks, +recorded as an owned path and bounded at 256 KiB. Handoff empties an already-full file. `OCX_RESTART_HANDOFF_LOG=1` +tells `handleStart` to arm `armRestartHandoffLogCap`, an unref'd 60-second timer which lstats and empties a full log +through a fresh descriptor. An ordinary start arms no timer. Parent log lines carry only timestamps, PIDs, ports, +attempt counts, exit codes and errno labels, never environment values. When every attempt fails while client state is `connected` (a join has committed), the parent marks recycling before `exit(1)`, so its exit cleanup keeps the Codex routing `connectClient` wrote diff --git a/structure/ops/test-sandbox-cleanup.md b/structure/ops/test-sandbox-cleanup.md index ccc50a7057a..1438659d308 100644 --- a/structure/ops/test-sandbox-cleanup.md +++ b/structure/ops/test-sandbox-cleanup.md @@ -135,3 +135,18 @@ are complete allowlists. Catalog tick children drain their config ACL flights/reaps before normal exit. Child work deadlines remain separate from outer test budgets, which reserve time for termination and teardown. The shared warm-up regressions exercise a live interval after work completes and a descendant retaining stdout after its parent exits. + +The completed-drain fixture in `tests/server/system-restart-ownership.test.ts` uses +`tests/helpers/restart-fixture-processes.ts` before loading the runner. On Windows each entry +joins a private Job before spawning; descendants inherit membership, including detached +replacements. Teardown kills through the held Job handle and requires zero active members. +The PID journal is diagnostic only. On POSIX a private stop file requests self-termination; +recorded creation identities are used only to observe exit, never to signal journaled PIDs. +The spawned parent's handle and captured output are joined before home removal; cleanup +failure retains the home. A controlled foreign process injected at a stale journal PID survives. +`tests/fixtures/restart-parent-ownership-runner.ts` journals process phases and captures the +fence probe's output continuously. Failure assertions include that evidence and the replacement +handoff log, including when the parent is terminated before its probe returns. +The fixture bundles the product graph once into the recognized source-launch entry before +spawning. Cold module resolution and transpilation count as setup, not handoff work. The +18-second handoff watchdog starts after spawn; bundling counts against the outer test budget. diff --git a/structure/runtime.md b/structure/runtime.md index 51c378caadb..bb7cd1b8dd3 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -181,7 +181,7 @@ uninstall still restore. `src/service/cli.ts` removes the service token on unins `src/cli/tray-proxy.ts` coordinates `ocx restart` and tray restart. A second start requires proof the prior attempt never launched or its child exited, plus confirmed absence under a fresh bounded discovery window; a missing health response alone is insufficient. After an accepted restart, polling continues through uncertain observations until its deadline and requires a new runtime PID on the same port. If the old proxy then disappears, explicit recovery starts it regardless of the Codex autostart preference; a skipped recovery fails rather than reporting success. The Windows tray startup-health probe in `src/tray/windows-tray.ps1` requests termination at its 30-second deadline without waiting on the UI thread. It retains the process through later ticks until actual exit and both redirected readers settle; replacements cannot start while that reference is held. Launch setup failures follow the same cleanup. Tray shutdown alone waits at most 500 ms before disposal. `ocx restart` also restarts a connected client runtime, which serves the same bound `POST /api/system/restart` from its client listener; the CLI sends that request only while the client's 90-second restart budget still fits before replacement observation ends (`CLIENT_RESTART_REQUEST_MIN_REMAINING_MS`), and reports a supervised client's `409 restart_supervised` as "not performed, runtime kept". See [Remote Link](remote-link.md) for the rollback. -The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and carries the concrete responding hostname; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery. +The package-tree integrity fence for live package replacement follows the [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence). After client journal reconciliation, `ocx ensure` in `src/cli/dispatch.ts` treats validated connected-client state as a successful no-op without local startup or a remote health probe; invalid or mismatched state still fails, and disconnected state keeps local ensure. `tests/cli/cli-ensure-client-state.test.ts` pins this dispatch contract so the Codex shim does not warn merely because its proxy is delegated to a hub. Startup journal reconciliation, including replacement after restart/drain, follows the [Codex-home recovery contract](codex-home.md#codex-home): already-original recovery is silent, incomplete recovery retains its journal and warns without claiming restoration, and only complete recovery that rewrites or removes an artifact reports a restore; restart admission, drain deadlines and replacement ownership are unchanged. `src/cli/start-owner-fence.ts` holds the ownership mutation lease through recovery for ordinary and supervised starts. After an early negative probe, `start` and `ensure` recheck with `START_OWNERSHIP_LIVENESS` under that lease through `observeProxyLiveness` in `src/server/proxy-liveness.ts`, probing both available loopback families concurrently for every loopback or wildcard spelling classified after the same startup normalization (including `localhost.` and case variants) within the existing per-family retry ceilings and shared deadline. Only definitive absence permits PID deletion and journal reconciliation: no alive recorded PID unless fresh readable command-line evidence confirms it is foreign, no answering runtime owner, and only unanimous aggregate connection refusals or clean foreign identities at probed endpoints. Port zero names no existing listener. Timeouts, resets, unreadable/non-200 responses, PID mismatches and exhausted deadlines remain unknown; startup refuses with a fixed message while preserving PID, journal and shared config. Unreadable/empty PID command lines and any OpenCodex invocation remain unknown; a confirmed foreign PID is never signalled. A discovered live owner preserves both records and its runtime/config bind scope in `LiveProxy.hostname`; omitted or blank hostnames become explicit `127.0.0.1` in both early and recovery discovery, so a later config change cannot replace a default loopback bind. Explicit wildcard scope remains intact, and the concrete responding address is separate `probeHostname` metadata. Integration policy uses the bind scope, so loopback discovery of a wildcard listener does not authorize tokenless Grok registration. Ensure carries `probeHostname` separately through `src/cli/ensure-desired-integrations.ts` and `src/grok/sync.ts` to `src/grok/inject.ts` for the written destination; an explicit secondary loopback listener keeps its own address and port; after lease release, start applies `decideStartWithLiveOwner` and ensure follows its existing live-owner path. Cross-home sibling marking still precedes journal recovery. The bind/publication fence remains separate, and supervised recovery checks durable service ownership before liveness or mutation. `tests/cli/cli-start-journal-order.test.ts` covers an early miss; `tests/cli/cli-start-owner-absence.test.ts` covers consecutive misses and dead-owner recovery; `tests/server/proxy-liveness-recovery.test.ts` binds classification and existing budgets; `tests/server/proxy-liveness-bind-scope.test.ts` follows a missed early probe through desired-state reconciliation and the real Grok writer for runtime, orphan and configured-port discovery; `tests/server/proxy-liveness-owner-evidence.test.ts` and `tests/cli/cli-start-owner-evidence.test.ts` cover aggregate children, both listener families, shared deadlines and foreign-PID recovery. A busy preferred port is never resolved by starting somewhere else. Both questions a start asks about an existing proxy — the pre-bind owner check and the port-is-busy check in `src/cli/index.ts` diff --git a/tests/cli/cli-start-owner-evidence.test.ts b/tests/cli/cli-start-owner-evidence.test.ts index 235f8a80452..b0a0ceff6fe 100644 --- a/tests/cli/cli-start-owner-evidence.test.ts +++ b/tests/cli/cli-start-owner-evidence.test.ts @@ -147,7 +147,8 @@ for (const identity of scenario.startsWith("F3") || scenario.startsWith("F5") ? } else { expect(result.refusal).toBeUndefined(); expect(result.live?.source).toBe("runtime"); - expect(result.live?.hostname).toBe(scenario === "F3 ipv6-owner" ? "[::1]" : "127.0.0.1"); + expect(result.live?.hostname).toBe(scenario.startsWith("F5") ? "localhost." : "localhost"); + expect(result.live?.probeHostname).toBe(scenario === "F3 ipv6-owner" ? "[::1]" : "127.0.0.1"); expect(result.ownerStillLive).toEqual({ service: "opencodex", pid: result.evidence.evidencePid }); expect(result.recoveryReads, diagnostics).toBeGreaterThan(0); if (identity === "verified") { diff --git a/tests/fixtures/restart-parent-ownership-runner.ts b/tests/fixtures/restart-parent-ownership-runner.ts new file mode 100644 index 00000000000..7f59af8fa43 --- /dev/null +++ b/tests/fixtures/restart-parent-ownership-runner.ts @@ -0,0 +1,137 @@ +/** Real processes, drain, discovery, recovery fence and replacement readiness; no provider startup. */ +import { appendFileSync, existsSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { recoverStartJournalAfterOwnerRecheck } from "../../src/cli/start-owner-fence"; +import { + readPidFileValue, readRuntimePort, removePid, removeRuntimePort, writePid, writeRuntimePort, +} from "../../src/config/process-state"; +import { acquireOwnershipMutationLease } from "../../src/service/ownership-mutation-lease.mjs"; +import { serviceStatePaths } from "../../src/service/state"; +import { acceptSystemRestart } from "../../src/server/management/system-restart"; +import { drainAndShutdown, setServerRef } from "../../src/server/lifecycle"; +import { observeProxyLiveness } from "../../src/server/proxy-liveness"; + +const home = process.env.OPENCODEX_HOME; +if (!home || !process.env.CODEX_HOME || !process.env.OCX_OWNER_REGISTRY_DIR) throw new Error("isolated homes required"); +delete process.env.OCX_SERVICE; +delete process.env.OCX_DESKTOP_SUPERVISED; +const output = (name: string, value: unknown) => writeFileSync(join(home, name + ".json"), JSON.stringify(value)); +const parent = process.argv.includes("parent"), probe = process.argv.includes("probe"); +const phase = (name: string) => { + const line = `[restart-fixture ${process.pid} ${Date.now()}] ${name}`; + appendFileSync(join(home, "fixture-phases.log"), line + "\n"); + console.error(line); +}; +phase(parent ? "parent loaded" : probe ? "probe loaded" : "replacement loaded"); +// A failed test must never leave a detached fixture running indefinitely. +setTimeout(() => process.exit(97), 20_000).unref(); + +async function capture(stream: ReadableStream, name: string): Promise { + let text = ""; + const decoder = new TextDecoder(); + for await (const chunk of stream) { + const part = decoder.decode(chunk, { stream: true }); + appendFileSync(join(home!, name), part); + text += part; + } + const tail = decoder.decode(); + appendFileSync(join(home!, name), tail); + return text + tail; +} + +async function recover(): Promise { + phase("acquiring recovery lease"); + const lease = acquireOwnershipMutationLease(serviceStatePaths()); + phase("rechecking owner"); + try { await recoverStartJournalAfterOwnerRecheck({ live: null, pidSnapshot: readPidFileValue() }); } + finally { lease.release(); } +} + +if (probe) { + const before = readPidFileValue(); + const runtime = readRuntimePort()!; + const observed = await observeProxyLiveness(); + const endpointsRefused = await Promise.all(["127.0.0.1", "[::1]"].map(async host => { + try { await fetch(`http://${host}:${runtime.port}/healthz`, { signal: AbortSignal.timeout(1_000) }); return false; } + catch (error) { return ["ECONNREFUSED", "ConnectionRefused"].includes((error as NodeJS.ErrnoException).code ?? ""); } + })); + let refused = false; + try { await recover(); } + catch (error) { refused = (error as Error).message.includes("ownership is unverified"); } + console.log(JSON.stringify({ refused, pidUnchanged: before === readPidFileValue(), + observed: observed.kind, endpointsRefused })); + process.exit(refused ? 0 : 2); +} + +if (parent) { + phase("binding parent"); + const listener = Bun.serve({ hostname: "127.0.0.1", port: 0, + fetch: () => Response.json({ service: "opencodex", pid: process.pid }) }); + setServerRef(listener); + const port = listener.port!; + writeFileSync(join(home, "config.json"), JSON.stringify({ port, hostname: "127.0.0.1", defaultProvider: "openai", providers: {} })); + writePid(process.pid); writeRuntimePort({ pid: process.pid, port, hostname: "127.0.0.1" }); + let scheduled!: () => void | Promise; + let beforeHandoff: unknown; + process.on("exit", code => { + const replacement = readRuntimePort(); + // The real CLI exit cleanup uses these same PID guards after readiness. + removePid(process.pid); removeRuntimePort(process.pid); + output("parent-result", { code, pid: process.pid, beforeHandoff, replacement, + successorPid: readPidFileValue(), successorRuntime: readRuntimePort() }); + }); + acceptSystemRestart({ + isDraining: () => false, getActiveTurnCount: () => 0, + isDesktopSupervised: () => false, isSupervisedServiceChild: () => false, + schedule: fn => { scheduled = fn; }, + drainAndShutdown: async (server, timeout) => { + phase("draining parent"); + const completed = await drainAndShutdown(server, timeout); + phase(`drain completed=${completed}; spawning fence probe`); + const guard = Bun.spawn([process.execPath, process.argv[1]!, "start", "probe"], { + env: process.env, stdout: "pipe", stderr: "pipe", + }); + const [code, stdout, stderr] = await Promise.all([ + guard.exited, capture(guard.stdout, "probe.stdout.log"), capture(guard.stderr, "probe.stderr.log"), + ]); + phase(`probe exited ${code}: ${stderr}`); + if (code !== 0) throw new Error("pre-handoff fence did not refuse: " + stderr); + beforeHandoff = JSON.parse(stdout.trim()); + return completed; + }, + }); + phase("running scheduled restart"); + await scheduled(); +} else { + const parentPid = process.ppid; + process.kill(parentPid, 0); + const parentAliveBeforeRecovery = true; + phase("observing retired parent"); + const observed = await observeProxyLiveness(); + phase(`observation=${observed.kind}`); + await recover(); + phase("recovered; binding replacement"); + const port = Number(process.argv[process.argv.indexOf("--port") + 1]); + const lease = acquireOwnershipMutationLease(serviceStatePaths()); + let listener: ReturnType; + try { + listener = Bun.serve({ hostname: "127.0.0.1", port, + fetch: () => Response.json({ service: "opencodex", pid: process.pid }) }); + writePid(process.pid); writeRuntimePort({ pid: process.pid, port, hostname: "127.0.0.1" }); + } finally { lease.release(); } + phase("replacement bound; waiting for parent exit"); + let parentExited = false; + for (let attempt = 0; attempt < 200; attempt++) { + try { process.kill(parentPid, 0); } + catch (error) { if ((error as NodeJS.ErrnoException).code === "ESRCH") { parentExited = true; break; } } + await Bun.sleep(50); + } + const parentResultPath = join(home, "parent-result.json"); + const result = { pid: process.pid, parentPid, parentAliveBeforeRecovery, + observed: observed.kind, parentExited, port, + parentResult: existsSync(parentResultPath) ? JSON.parse(readFileSync(parentResultPath, "utf8")) : null }; + await listener!.stop(true); + removePid(process.pid); removeRuntimePort(process.pid); + output("child-result", result); + phase("replacement finished"); +} diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 6946a347fa8..7016b614457 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1135,6 +1135,8 @@ "proxy-env.test.ts": "server", "proxy-liveness-package-tree-fence.test.ts": "server", "proxy-liveness-recovery.test.ts": "server", "proxy-liveness-owner-evidence.test.ts": "server", + "proxy-liveness-bind-scope.test.ts": "server", + "system-restart-ownership.test.ts": "server", "proxy-liveness.test.ts": "server", "readyz-features.test.ts": "server", "served-by-header.test.ts": "server", "qoder-adapter.test.ts": "providers", "qoder-live-models.test.ts": "providers", "qoder-mcp-server.test.ts": "providers", "qoder-scaffold-guard.test.ts": "providers", "quota-401-recovery-runtime.test.ts": "usage", "quota-401-recovery.test.ts": "usage", diff --git a/tests/helpers/restart-fixture-processes.ts b/tests/helpers/restart-fixture-processes.ts new file mode 100644 index 00000000000..e8825380426 --- /dev/null +++ b/tests/helpers/restart-fixture-processes.ts @@ -0,0 +1,116 @@ +/** Process ownership for the completed-drain fixture, independent of its diagnostic PID journal. */ +import { randomUUID } from "node:crypto"; +import { appendFileSync, existsSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +async function kernel32() { + const { dlopen } = await import("bun:ffi"); + const { resolveTrustedWindowsSystemDirectory } = await import("../../src/lib/windows-elevation"); + return dlopen(join(resolveTrustedWindowsSystemDirectory(), "kernel32.dll"), { + GetLastError: { args: [], returns: "u32" }, + SetLastError: { args: ["u32"], returns: "void" }, + CreateJobObjectW: { args: ["ptr", "ptr"], returns: "u64" }, + OpenJobObjectW: { args: ["u32", "i32", "ptr"], returns: "u64" }, + SetInformationJobObject: { args: ["u64", "i32", "ptr", "u32"], returns: "i32" }, + QueryInformationJobObject: { args: ["u64", "i32", "ptr", "u32", "ptr"], returns: "i32" }, + AssignProcessToJobObject: { args: ["u64", "u64"], returns: "i32" }, + IsProcessInJob: { args: ["u64", "u64", "ptr"], returns: "i32" }, + TerminateJobObject: { args: ["u64", "u32"], returns: "i32" }, + OpenProcess: { args: ["u32", "i32", "u32"], returns: "u64" }, + CloseHandle: { args: ["u64"], returns: "i32" }, + }); +} + +const wide = (value: string) => Buffer.from(value + "\0", "utf16le"); +function requireNative(ok: number | bigint, operation: string): void { + if (!ok) throw new Error(`restart fixture: ${operation} failed`); +} + +/** Each entry joins before any product code can spawn a descendant. No PID lookup admits members. */ +export async function enterRestartFixture(pidsPath: string, stopPath: string): Promise { + if (existsSync(stopPath)) process.exit(98); + if (process.platform === "win32") { + const native = await kernel32(), k = native.symbols; + const name = process.env.OCX_RESTART_FIXTURE_JOB; + if (!name) throw new Error("restart fixture Job required"); + const job = BigInt(k.OpenJobObjectW(5, 0, wide(name))); // JOB_OBJECT_ASSIGN_PROCESS | QUERY + requireNative(job, "OpenJobObjectW"); + let self = 0n; + try { + self = BigInt(k.OpenProcess(0x1101, 0, process.pid)); // QUERY_LIMITED_INFORMATION | SET_QUOTA | TERMINATE, always self + requireNative(self, "OpenProcess(self)"); + const member = new Uint32Array(1); + requireNative(k.IsProcessInJob(self, job, member), "IsProcessInJob"); + if (!member[0]) requireNative(k.AssignProcessToJobObject(job, self), "AssignProcessToJobObject"); + } finally { + if (self) k.CloseHandle(self); + k.CloseHandle(job); + native.close(); + } + appendFileSync(pidsPath, process.pid + "\n"); // Diagnostics only, never termination authority. + } else { + const { readTestRunProcessStartBatch } = await import("../../scripts/test-run-lock"); + const start = readTestRunProcessStartBatch([process.pid]).get(process.pid); + if (!start) throw new Error("restart fixture process start identity unavailable"); + appendFileSync(pidsPath, JSON.stringify({ pid: process.pid, start }) + "\n"); + // POSIX roles terminate themselves; journal identities are used only to observe exit. + setInterval(() => { if (existsSync(stopPath)) process.exit(98); }, 25).unref(); + } + if (existsSync(stopPath)) process.exit(98); +} + +export async function ownRestartFixture(pidsPath: string, stopPath: string) { + if (process.platform !== "win32") return { + env: {}, + async stop() { + writeFileSync(stopPath, "stop"); + const { readTestRunProcessStartBatch } = await import("../../scripts/test-run-lock"); + const { isProcessAlive } = await import("../../src/lib/process-control"); + const deadline = Date.now() + 5_000; + for (;;) { + const records = (existsSync(pidsPath) ? readFileSync(pidsPath, "utf8") : "") + .trim().split("\n").filter(line => line.startsWith("{")).map(line => JSON.parse(line) as { pid: number; start: string }); + const current = readTestRunProcessStartBatch(records.map(record => record.pid)); + if (records.every(record => !isProcessAlive(record.pid) + || (current.get(record.pid) != null && current.get(record.pid) !== record.start))) return; + if (Date.now() >= deadline) throw new Error("restart fixture roles did not exit"); + await Bun.sleep(25); + } + }, + }; + const native = await kernel32(), k = native.symbols; + const name = `Local\\ocx-restart-fixture-${randomUUID()}`; + k.SetLastError(0); + const job = BigInt(k.CreateJobObjectW(null, wide(name))); + const existing = k.GetLastError() === 183; + try { + requireNative(job, "CreateJobObjectW"); + if (existing) throw new Error("restart fixture refuses a pre-existing Job"); + const limits = Buffer.alloc(144); // JOBOBJECT_EXTENDED_LIMIT_INFORMATION, Windows 64-bit + limits.writeUInt32LE(0x2000, 16); // JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + requireNative(k.SetInformationJobObject(job, 9, limits, limits.length), "SetInformationJobObject"); + } catch (error) { + if (job) k.CloseHandle(job); + native.close(); throw error; + } + return { + env: { OCX_RESTART_FIXTURE_JOB: name }, + async stop() { + writeFileSync(stopPath, "stop"); + try { + requireNative(k.TerminateJobObject(job, 98), "TerminateJobObject"); + const accounting = Buffer.alloc(48); // JOBOBJECT_BASIC_ACCOUNTING_INFORMATION + const deadline = Date.now() + 5_000; + for (;;) { + requireNative(k.QueryInformationJobObject(job, 1, accounting, accounting.length, null), "QueryInformationJobObject"); + if (accounting.readUInt32LE(40) === 0) return; + if (Date.now() >= deadline) throw new Error("restart fixture Job did not empty"); + await Bun.sleep(25); + } + } finally { + k.CloseHandle(job); + native.close(); + } + }, + }; +} diff --git a/tests/providers/xai/grok-lifecycle.test.ts b/tests/providers/xai/grok-lifecycle.test.ts index c7b5feaf50d..b3e5634357a 100644 --- a/tests/providers/xai/grok-lifecycle.test.ts +++ b/tests/providers/xai/grok-lifecycle.test.ts @@ -48,13 +48,15 @@ describe("Grok fence lifecycle wiring", () => { expect(grokSyncAt).toBeGreaterThan(afterStartupAt); }); - test("ensure passes only the observed live bind host across the mutation boundary", () => { + test("ensure passes only the observed live bind scope and probe address across the mutation boundary", () => { const ensureFn = sliceFn(CLI_SOURCE, "async function handleEnsure(", "async function handleTrayProxyStart("); const liveBranch = ensureFn.slice(0, ensureFn.indexOf("const pinPort")); const spawnBranch = ensureFn.slice(ensureFn.indexOf("const pinPort")); - // live.hostname is what the proxy ACTUALLY bound; config.hostname may have drifted. - expect(liveBranch).toContain('{ kind: "live", hostname: live.hostname }'); + // live.hostname is the scope the proxy ACTUALLY bound and live.probeHostname the address + // that answered; config.hostname may have drifted and must not cross this boundary. + expect(liveBranch).toContain('{ kind: "live", hostname: live.hostname, probeHostname: live.probeHostname }'); + expect(liveBranch).not.toContain("hostname: config.hostname"); // Spawn passes no config-derived input; the reconciler loads the current hostname. expect(spawnBranch).toContain('{ kind: "spawned" }'); expect(spawnBranch).not.toContain("current.hostname"); diff --git a/tests/providers/xai/grok-sync.test.ts b/tests/providers/xai/grok-sync.test.ts index 6ce02a973fb..a045aa515cb 100644 --- a/tests/providers/xai/grok-sync.test.ts +++ b/tests/providers/xai/grok-sync.test.ts @@ -260,6 +260,7 @@ describe("syncGrokConfig", () => { const result = await syncGrokConfig(10100, config, { grokHome, hostname: "100.64.0.10", + probeHostname: "::1", }, { fetchAllModels: async () => [], injectGrokConfig, @@ -288,6 +289,7 @@ describe("syncGrokConfig", () => { const result = await syncGrokConfig(10100, config, { grokHome, hostname: "100.64.0.10", + probeHostname: "::1", }, { fetchAllModels: async () => [], injectGrokConfig, diff --git a/tests/server/proxy-liveness-bind-scope.test.ts b/tests/server/proxy-liveness-bind-scope.test.ts new file mode 100644 index 00000000000..1ffa904645d --- /dev/null +++ b/tests/server/proxy-liveness-bind-scope.test.ts @@ -0,0 +1,114 @@ +import { expect, test } from "bun:test"; +import { existsSync, mkdirSync, readFileSync } from "node:fs"; +import { createTempHome } from "../helpers/temp-home"; +import { findLiveProxy, observeProxyLiveness, type LivenessIo } from "../../src/server/proxy-liveness"; +import { standaloneCodexRoutingTarget } from "../../src/codex/inject/routing-target"; +import { reconcileEnsureDesiredIntegrations } from "../../src/cli/ensure-desired-integrations"; +import { injectGrokConfig, type GrokInjectResult } from "../../src/grok/inject"; +import { syncGrokConfig } from "../../src/grok/sync"; +import type { OcxConfig } from "../../src/types"; + +const binds = [ + { label: "omitted", hostname: undefined, expected: "127.0.0.1", loopback: true }, + { label: "empty", hostname: "", expected: "127.0.0.1", loopback: true }, + { label: "blank", hostname: " \t ", expected: "127.0.0.1", loopback: true }, + { label: "IPv4 wildcard", hostname: "0.0.0.0", expected: "0.0.0.0", loopback: false }, + { label: "IPv6 wildcard", hostname: "::", expected: "::", loopback: false }, + { label: "bracketed wildcard", hostname: "[::]", expected: "[::]", loopback: false }, + { label: "asterisk wildcard", hostname: "*", expected: "*", loopback: false }, + { label: "explicit loopback", hostname: "127.0.0.1", expected: "127.0.0.1", loopback: true }, +]; + +for (const { label, hostname, expected, loopback } of binds) { +for (const source of ["runtime", "orphan", "config"] as const) { +for (const route of ["early", "late"] as const) { + test(`${route} ${source} discovery preserves ${label} admission through ensure and the actual Grok writer`, async () => { + const home = createTempHome("ocx-live-bind-scope-"); + try { + const grokHome = home.path("grok"); mkdirSync(grokHome); + const config: OcxConfig = { port: 10123, hostname, defaultProvider: "openai", providers: {}, + clientIntegrations: { grok: { enabled: true }, "claude-desktop": { enabled: true } }, + claudeCode: { desktopMode: "gateway" } }; + let answers = route === "early"; + const io: LivenessIo = { + readPidFn: () => source === "runtime" ? 4242 : null, + readRuntimeFn: () => source === "config" ? null : { pid: 4242, port: config.port!, hostname }, + verifyPidFn: pid => pid, configFn: () => config, loopbackAvailabilityFn: () => "available", + fetchFn: (async () => { + if (!answers) throw Object.assign(new Error("refused"), { code: "ECONNREFUSED" }); + return Response.json({ service: "opencodex", pid: 4242 }); + }) as typeof fetch, + }; + if (route === "late") { + expect(await findLiveProxy(io)).toBeNull(); // The early probe misses the owner. + answers = true; + } + const live = route === "early" ? await findLiveProxy(io) : (await observeProxyLiveness(io)).live; + expect(live).not.toBeNull(); + let written: GrokInjectResult | undefined; + await reconcileEnsureDesiredIntegrations(live!.port, { kind: "live", ...live! }, { + // A later config with the opposite scope must not replace the live bind, even + // when the runtime/config candidate omitted the default hostname or left it blank. + loadConfig: () => ({ ...config, hostname: loopback ? "0.0.0.0" : "127.0.0.1" }), + syncGrokConfig: async (port, current, opts) => written = await syncGrokConfig(port, current, + { ...opts, grokHome }, { fetchAllModels: async () => [], injectGrokConfig }), + stripGrokConfig: () => { throw new Error("unexpected strip"); }, + observeClaudeDesktopMode: () => ({}), + removeDesktop3pStandardPivot: () => { throw new Error("unused desktop seam"); }, + log() {}, error: message => { throw new Error(message); }, + }); + if (loopback) { + expect(written).toMatchObject({ ok: true, changed: true }); + expect(readFileSync(home.path("grok", "config.toml"), "utf8")).toContain('api_key = "opencodex-loopback"'); + } else { + expect(written).toMatchObject({ ok: true, changed: false, skippedReason: "non-loopback" }); + expect(existsSync(home.path("grok", "config.toml"))).toBe(false); + } + expect(live!.hostname).toBe(expected); + expect(live!.probeHostname).toBe(route === "late" ? "127.0.0.1" : undefined); + expect(standaloneCodexRoutingTarget(live!.port, { hostname: live!.hostname }) + .requiresAdmissionToken).toBe(!loopback); + } finally { home.remove(); } + }); +} +} +} + +for (const source of ["runtime", "orphan", "config"] as const) { + test(`late ${source} recovery writes the responding IPv4 address for an IPv6 bind candidate`, async () => { + const home = createTempHome("ocx-live-probe-destination-"); + const listener = Bun.serve({ hostname: "127.0.0.1", port: 0, + fetch: () => Response.json({ service: "opencodex", pid: process.pid }) }); + try { + const grokHome = home.path("grok"); mkdirSync(grokHome); + const config: OcxConfig = { port: listener.port!, hostname: "::1", defaultProvider: "openai", providers: {}, + clientIntegrations: { grok: { enabled: true }, "claude-desktop": { enabled: true } }, + claudeCode: { desktopMode: "gateway" } }; + const io: LivenessIo = { + readPidFn: () => source === "runtime" ? process.pid : null, + readRuntimeFn: () => source === "config" ? null : { pid: process.pid, port: listener.port!, hostname: "::1" }, + verifyPidFn: pid => pid, configFn: () => config, attempts: 1, + loopbackAvailabilityFn: () => "available", + }; + expect(await findLiveProxy(io)).toBeNull(); + const observed = await observeProxyLiveness(io); + expect(observed.kind).toBe("live"); + const live = observed.live!; + expect(live).toMatchObject({ hostname: "::1", probeHostname: "127.0.0.1" }); + await reconcileEnsureDesiredIntegrations(live.port, { kind: "live", ...live }, { + loadConfig: () => config, + syncGrokConfig: (port, current, opts) => syncGrokConfig(port, current, { ...opts, grokHome }, + { fetchAllModels: async () => [], injectGrokConfig }), + stripGrokConfig: () => { throw new Error("unexpected strip"); }, + observeClaudeDesktopMode: () => ({}), + removeDesktop3pStandardPivot: () => { throw new Error("unused desktop seam"); }, + log() {}, error: message => { throw new Error(message); }, + }); + const written = Bun.TOML.parse(readFileSync(home.path("grok", "config.toml"), "utf8")) as { + model_providers: { opencodex: { base_url: string } }; + }; + expect(written.model_providers.opencodex.base_url).toBe(`http://127.0.0.1:${listener.port}/v1`); + expect((await fetch(written.model_providers.opencodex.base_url)).ok).toBe(true); + } finally { await listener.stop(true); home.remove(); } + }); +} diff --git a/tests/server/proxy-liveness-owner-evidence.test.ts b/tests/server/proxy-liveness-owner-evidence.test.ts index 926810124f7..9456527d616 100644 --- a/tests/server/proxy-liveness-owner-evidence.test.ts +++ b/tests/server/proxy-liveness-owner-evidence.test.ts @@ -106,7 +106,8 @@ for (const ownerHost of ["127.0.0.1", "::1"]) { expect(observation.kind).toBe("live"); expect(observation.live?.pid).toBe(process.pid); expect(observation.live?.port).toBe(owner.port!); - expect(observation.live?.hostname).toBe(ownerHost === "::1" ? "[::1]" : "127.0.0.1"); + expect(observation.live?.hostname).toBe(hostname); + expect(observation.live?.probeHostname).toBe(ownerHost === "::1" ? "[::1]" : "127.0.0.1"); } finally { await foreign?.stop(true); await owner.stop(true); @@ -175,7 +176,7 @@ test("F3 family probes run concurrently and stop retrying at the shared deadline expect(calls).toBe(2); }); -test("F3 a live family wins over an uncertain peer and preserves its concrete hostname", async () => { +test("F3 a live family retains its probe address separately from bind scope", async () => { const result = await observeProxyLiveness({ ...base, configFn: () => ({ port: 10100, hostname: "localhost" }), loopbackAvailabilityFn: () => "available", verifyPidFn: pid => pid, fetchFn: (async (url: string) => { @@ -183,7 +184,9 @@ test("F3 a live family wins over an uncertain peer and preserves its concrete ho throw Object.assign(new Error("timeout"), { code: "ETIMEDOUT" }); }) as typeof fetch, }); - expect(result).toEqual({ kind: "live", live: { pid: 4242, port: 10100, hostname: "[::1]", source: "config" } }); + expect(result).toEqual({ kind: "live", live: { + pid: 4242, port: 10100, hostname: "localhost", probeHostname: "[::1]", source: "config", + } }); }); test("F4 confirmed foreign PID does not erase endpoint uncertainty or an elapsed identity deadline", async () => { diff --git a/tests/server/proxy-liveness-recovery.test.ts b/tests/server/proxy-liveness-recovery.test.ts index 6d5d52dcaf7..7aa3c065c49 100644 --- a/tests/server/proxy-liveness-recovery.test.ts +++ b/tests/server/proxy-liveness-recovery.test.ts @@ -65,7 +65,8 @@ test("a live owner wins over an earlier runtime timeout", async () => { const probeIo = { ...io, readRuntimeFn: () => ({ pid: 4242, port: 20200 }), fetchFn }; const observation = await observeProxyLiveness(probeIo); expect(observation.kind).toBe("live"); - expect(observation.live).toEqual(await findLiveProxy(probeIo)); + expect(observation.live).toMatchObject((await findLiveProxy(probeIo))!); + expect(observation.live?.probeHostname).toBe("127.0.0.1"); expect(observation.live?.role).toBe("client"); }); diff --git a/tests/server/proxy-liveness.test.ts b/tests/server/proxy-liveness.test.ts index 5e6f9ae453f..295409c67b5 100644 --- a/tests/server/proxy-liveness.test.ts +++ b/tests/server/proxy-liveness.test.ts @@ -359,7 +359,7 @@ describe("findLiveProxy", () => { }) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 58195, source: "runtime", version: "2.6.17" }); + expect(live).toEqual({ pid: 4242, port: 58195, hostname: "127.0.0.1", source: "runtime", version: "2.6.17" }); expect(urls).toEqual(["http://127.0.0.1:58195/healthz"]); }); @@ -372,7 +372,7 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(OURS)) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 10100, source: "config", version: "2.6.17" }); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: "127.0.0.1", source: "config", version: "2.6.17" }); }); test("a foreign listener on the configured port is not treated as our proxy", async () => { @@ -415,7 +415,7 @@ describe("findLiveProxy", () => { // The record's pid 1111 may be dead/reused — synthesizing it would let `ocx stop` // kill an unrelated process via the taskkill/kill fallback. - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime", version: "2.6.16" }); + expect(live).toEqual({ pid: null, port: 58195, hostname: "127.0.0.1", source: "runtime", version: "2.6.16" }); }); test("an orphaned record whose healthz pid mismatches is rejected (config fallback still runs)", async () => { @@ -440,7 +440,7 @@ describe("findLiveProxy", () => { }); // healthz-reported pids must pass identity verification before they become kill targets. - expect(live).toEqual({ pid: null, port: 58195, source: "config", version: "2.6.17" }); + expect(live).toEqual({ pid: null, port: 58195, hostname: "127.0.0.1", source: "config", version: "2.6.17" }); }); test("a pidless legacy healthz never promotes an unverified cheap pid to a kill target", async () => { @@ -453,7 +453,7 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(legacyBody)) as typeof fetch, }); - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime", version: "2.6.16" }); + expect(live).toEqual({ pid: null, port: 58195, hostname: "127.0.0.1", source: "runtime", version: "2.6.16" }); }); test("a pidless legacy healthz returns the cheap pid once full identity verification echoes it", async () => { @@ -471,7 +471,7 @@ describe("findLiveProxy", () => { }); expect(verified).toEqual([1111]); - expect(live).toEqual({ pid: 1111, port: 58195, hostname: undefined, source: "runtime", version: "2.6.16" }); + expect(live).toEqual({ pid: 1111, port: 58195, hostname: "127.0.0.1", source: "runtime", version: "2.6.16" }); }); test("a verifier answering with a DIFFERENT pid than the candidate is rejected (TOCTOU guard)", async () => { @@ -484,7 +484,7 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz(legacyBody)) as typeof fetch, }); - expect(live).toEqual({ pid: null, port: 58195, hostname: undefined, source: "runtime", version: "2.6.16" }); + expect(live).toEqual({ pid: null, port: 58195, hostname: "127.0.0.1", source: "runtime", version: "2.6.16" }); }); }); @@ -617,7 +617,7 @@ describe("findLiveProxy single-deadline candidate gating", () => { }); expect(urls).toEqual(["http://127.0.0.1:58195/healthz", "http://127.0.0.1:10100/healthz"]); expect(urls).toHaveLength(2); - expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined, source: "config", version: "2.6.17" }); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: "127.0.0.1", source: "config", version: "2.6.17" }); }); }); @@ -1138,7 +1138,7 @@ describe("client-role discrimination (#4662)", () => { configFn: () => ({ port: 10100 }), fetchFn: (async () => healthz(CLIENT)) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 10100, source: "runtime", version: "2.6.17", role: "client" }); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: "127.0.0.1", source: "runtime", version: "2.6.17", role: "client" }); }); test("findLiveProxy reports the role on the configured-port path", async () => { @@ -1149,6 +1149,6 @@ describe("client-role discrimination (#4662)", () => { verifyPidFn: candidate => candidate, fetchFn: (async () => healthz(CLIENT)) as typeof fetch, }); - expect(live).toEqual({ pid: 4242, port: 10100, hostname: undefined, source: "config", version: "2.6.17", role: "client" }); + expect(live).toEqual({ pid: 4242, port: 10100, hostname: "127.0.0.1", source: "config", version: "2.6.17", role: "client" }); }); }); diff --git a/tests/server/system-restart-ownership.test.ts b/tests/server/system-restart-ownership.test.ts new file mode 100644 index 00000000000..cc40e6a5da3 --- /dev/null +++ b/tests/server/system-restart-ownership.test.ts @@ -0,0 +1,203 @@ +import { afterEach, expect, test } from "bun:test"; +import { appendFileSync, existsSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { createTempHome, type TempHome } from "../helpers/temp-home"; +import { fixturePath, helperPath, repoRoot } from "../helpers/repo-root"; +import { ownRestartFixture } from "../helpers/restart-fixture-processes"; +import { + getPidPath, getRuntimePortPath, readPidFileValue, readRuntimePort, writePid, writeRuntimePort, +} from "../../src/config/process-state"; +import { inspectOwnershipMutationLease } from "../../src/service/ownership-mutation-lease.mjs"; +import { serviceStatePaths } from "../../src/service/state"; +import { retireRestartParentOwnership } from "../../src/server/restart-parent-ownership"; +import { observeProxyLiveness } from "../../src/server/proxy-liveness"; +import { isProcessAlive } from "../../src/lib/process-control"; + +let home: TempHome | undefined; +let settleProcesses: (() => Promise) | undefined; +afterEach(async () => { + const settle = settleProcesses, owned = home; + settleProcesses = undefined; + home = undefined; + await settle?.(); + owned?.remove(); +}); +function fixture(): TempHome { return home = createTempHome("ocx-restart-parent-ownership-"); } + +test("retirement removes only the current parent's records and releases the ownership lease", () => { + fixture(); + writePid(process.pid); writeRuntimePort({ pid: process.pid, port: 10123, hostname: "127.0.0.1" }); + retireRestartParentOwnership(10123); + expect(existsSync(getPidPath())).toBe(false); + expect(existsSync(getRuntimePortPath())).toBe(false); + expect(inspectOwnershipMutationLease(serviceStatePaths())).toBeNull(); + expect(() => retireRestartParentOwnership(10123)).not.toThrow(); +}); + +for (const mismatch of ["foreign-pid", "foreign-runtime", "different-port", "malformed-pid", "malformed-runtime"]) { + test(`retirement refuses ${mismatch} without removing either ownership record`, () => { + fixture(); + writePid(mismatch === "foreign-pid" ? process.pid + 1 : process.pid); + writeRuntimePort({ pid: mismatch === "foreign-runtime" ? process.pid + 1 : process.pid, port: 10123 }); + if (mismatch === "malformed-pid") writeFileSync(getPidPath(), "not a pid"); + if (mismatch === "malformed-runtime") writeFileSync(getRuntimePortPath(), "{}"); + const before = [readFileSync(getPidPath(), "utf8"), readFileSync(getRuntimePortPath(), "utf8")]; + expect(() => retireRestartParentOwnership(mismatch === "different-port" ? 10124 : 10123)) + .toThrow("ownership could not be retired safely"); + expect([readFileSync(getPidPath(), "utf8"), readFileSync(getRuntimePortPath(), "utf8")]).toEqual(before); + expect(inspectOwnershipMutationLease(serviceStatePaths())).toBeNull(); + }); +} + +test("a non-file runtime cannot retire the parent's alive-PID fence", () => { + fixture(); writePid(process.pid); mkdirSync(getRuntimePortPath()); + expect(() => retireRestartParentOwnership()).toThrow(); + expect(readPidFileValue()).toBe(process.pid); +}); + +test.skipIf(process.platform === "win32")("a linked record is preserved, including its external target", () => { + const owned = fixture(), target = owned.path("pid-target"); + writeFileSync(target, String(process.pid)); symlinkSync(target, getPidPath()); + expect(() => retireRestartParentOwnership()).toThrow(); + expect(readFileSync(target, "utf8")).toBe(String(process.pid)); + expect(existsSync(getPidPath())).toBe(true); +}); + +test("a genuinely exited parent remains an ordinary absent-owner control", async () => { + fixture(); + const dead = Bun.spawn([process.execPath, "--version"], { stdout: "ignore", stderr: "ignore" }); + expect(await dead.exited).toBe(0); + writePid(dead.pid); + const listener = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: () => new Response("unused") }); + const port = listener.port!; await listener.stop(true); + writeRuntimePort({ pid: dead.pid, port, hostname: "127.0.0.1" }); + const observed = await observeProxyLiveness({ configFn: () => ({ port, hostname: "127.0.0.1" }), attempts: 1 }); + expect(observed).toEqual({ kind: "absent", live: null }); + expect(readRuntimePort()?.pid).toBe(dead.pid); // Observation never retires records itself. +}); + +test("completed drain hands off to a healthy child while the real parent is still alive", () => completedDrain(), 25_000); +test("cleanup refuses an exited fixture PID reused by a foreign live process", () => completedDrain(true), 25_000); + +test("cleanup joins an admitted detached descendant after its parent exits", async () => { + const owned = fixture(), pidsPath = owned.path("fixture-pids"), stopPath = owned.path("fixture-stop"); + const ready = owned.path("descendant-ready"), entry = owned.path("owned-role.ts"); + writeFileSync(entry, `import { spawn } from "node:child_process"; +import { existsSync, writeFileSync } from "node:fs"; +import { enterRestartFixture } from ${JSON.stringify(helperPath("restart-fixture-processes.ts"))}; +await enterRestartFixture(${JSON.stringify(pidsPath)}, ${JSON.stringify(stopPath)}); +setTimeout(() => process.exit(97), 20_000).unref(); +if (process.argv.includes("descendant")) { + writeFileSync(${JSON.stringify(ready)}, String(process.pid)); + setInterval(() => {}, 1000); +} else { + spawn(process.execPath, [process.argv[1], "descendant"], { detached: true, stdio: "ignore", env: process.env }).unref(); + for (let i = 0; i < 100 && !existsSync(${JSON.stringify(ready)}); i++) await Bun.sleep(25); + process.exit(existsSync(${JSON.stringify(ready)}) ? 0 : 2); +} +`); + const processes = await ownRestartFixture(pidsPath, stopPath); + let child: ReturnType | undefined, cleanup: Promise | undefined; + settleProcesses = () => cleanup ??= (async () => { + if (child?.exitCode === null) child.kill(); + await processes.stop(); + await child?.exited; + })(); + child = Bun.spawn([process.execPath, entry], { env: { ...process.env, ...processes.env }, stdout: "ignore", stderr: "ignore" }); + try { + expect(await child.exited).toBe(0); + const descendantPid = Number(readFileSync(ready, "utf8")); + expect(isProcessAlive(descendantPid)).toBe(true); + await settleProcesses(); + expect(isProcessAlive(descendantPid)).toBe(false); + } finally { await settleProcesses(); } +}, 25_000); + +async function completedDrain(injectReusedPid = false): Promise { + const owned = fixture(); + mkdirSync(owned.codexHome); + const entry = owned.path("src", "cli", "index.ts"); + mkdirSync(owned.path("src", "cli"), { recursive: true }); + // The real process identity verifier recognizes this source-launch path. + const pidsPath = owned.path("fixture-pids"); + const stopPath = owned.path("fixture-stop"); + writeFileSync(entry, `import { enterRestartFixture } from ${JSON.stringify(helperPath("restart-fixture-processes.ts"))}; +await enterRestartFixture(${JSON.stringify(pidsPath)}, ${JSON.stringify(stopPath)}); +console.error("[restart-fixture " + process.pid + "] loading runner"); +await import(${JSON.stringify(fixturePath("restart-parent-ownership-runner.ts"))});\n`); + // Resolve/transpile this same product graph once. Three cold source imports on Windows + // otherwise spend the handoff watchdog on fixture loading before exercising ownership. + // Keep the recognized source-launch path and all real process/discovery/recovery calls. + const built = await Bun.build({ entrypoints: [entry], target: "bun" }); + if (!built.success) throw new AggregateError(built.logs, "restart fixture bundle failed"); + await Bun.write(entry, built.outputs[0]!); + const processes = await ownRestartFixture(pidsPath, stopPath); + let cleanup: Promise | undefined; + // Install cleanup before spawn so even a failed CreateProcess releases the Job. + let child: ReturnType | undefined; + let output: Promise<[number, string, string]> | undefined; + settleProcesses = () => cleanup ??= (async () => { + // The retained parent handle covers failure before its entry joins the Job. + // Descendants can only be spawned after admission and inherit Job membership. + if (child?.exitCode === null) child.kill(); + await processes.stop(); + await output; + })(); + const parent = child = Bun.spawn([process.execPath, entry, "start", "parent"], { + cwd: repoRoot(), env: { ...process.env, ...processes.env, HOME: owned.root, USERPROFILE: owned.root, + OPENCODEX_HOME: owned.configDir, CODEX_HOME: owned.codexHome, + OCX_OWNER_REGISTRY_DIR: owned.path("registry"), XDG_RUNTIME_DIR: owned.root }, + stdout: "pipe", stderr: "pipe", + }); + output = Promise.all([ + parent.exited, new Response(parent.stdout).text(), new Response(parent.stderr).text(), + ]); + const watchdog = setTimeout(() => { + void settleProcesses!().catch(error => console.error("restart fixture cleanup failed", error)); + }, 18_000); + let captured: unknown; + try { + const [code, stdout, stderr] = await output; + captured = { code, stdout, stderr }; + const logPath = owned.path("restart-handoff.log"); + const handoffLog = existsSync(logPath) ? readFileSync(logPath, "utf8") : "(not created)"; + const phasesPath = owned.path("fixture-phases.log"); + const phases = existsSync(phasesPath) ? readFileSync(phasesPath, "utf8") : "(not loaded)"; + const probeOutput = ["probe.stdout.log", "probe.stderr.log"].map(name => + existsSync(owned.path(name)) ? readFileSync(owned.path(name), "utf8") : "(not created)"); + expect({ code, stdout, stderr, handoffLog, phases, probeOutput }).toMatchObject({ code: 0 }); + const resultPath = owned.path("child-result.json"); + for (let i = 0; i < 100 && !existsSync(resultPath); i++) await Bun.sleep(50); + expect(existsSync(resultPath)).toBe(true); + const result = JSON.parse(readFileSync(resultPath, "utf8")); + expect(result).toMatchObject({ parentPid: parent.pid, parentAliveBeforeRecovery: true, + observed: "absent", parentExited: true }); + expect(result.parentResult).toMatchObject({ code: 0, pid: parent.pid, + beforeHandoff: { refused: true, pidUnchanged: true, observed: "unknown", endpointsRefused: [true, true] }, + successorPid: result.pid, successorRuntime: { pid: result.pid, port: result.port } }); + expect(readFileSync(owned.path("restart-handoff.log"), "utf8")).toContain("is serving port"); + } catch (error) { + const logs = Object.fromEntries(["fixture-phases.log", "probe.stdout.log", "probe.stderr.log", "restart-handoff.log"] + .map(name => [name, existsSync(owned.path(name)) ? readFileSync(owned.path(name), "utf8") : "(not created)"])); + console.error("restart fixture failed", { captured, logs }); + throw error; + } finally { + clearTimeout(watchdog); + if (injectReusedPid) { + // This child belongs to the regression, outside the restart fixture tree. Model an + // exited role's journal PID now naming a live foreign process; never rely on OS reuse. + const foreign = Bun.spawn([process.execPath, "-e", "setInterval(() => {}, 1000)"], { + stdout: "ignore", stderr: "ignore", + }); + try { + appendFileSync(pidsPath, (process.platform === "win32" + ? String(foreign.pid) + : JSON.stringify({ pid: foreign.pid, start: "stale-journal-identity" })) + "\n"); + await settleProcesses(); + expect(isProcessAlive(foreign.pid)).toBe(true); + } finally { + foreign.kill(); + await foreign.exited; + } + } else await settleProcesses(); + } +}