From 70bcd62931ebb29b9bd6cef755f9d540dc8b45ea Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:04:29 +0900 Subject: [PATCH 1/7] fix(codex): stop the Codex shim hiding a failed autostart, and never block Codex The shim ran "ocx ensure" with both streams discarded and its exit status ignored, then launched Codex regardless. A proxy that failed to come up was therefore completely silent, and Codex started against injected routing pointing at a port nothing was listening on -- the #5261 shape, with nothing on screen naming opencodex. It now checks the exit status and prints one line on stderr when the start failed, naming "ocx doctor" and "ocx restore". Ensure's own streams stay discarded: it prints progress and warnings on exit-zero runs too, and a wrapper that leaked those would put noise in front of every ordinary launch, which is how a diagnostic gets ignored. The exit status is the signal, and the one line is the whole message. PowerShell had the opposite defect in the same place. An "ensure" that threw escaped the try/finally, which has no catch, so Codex never launched at all -- the autostart helper creating the exact lockout it exists to prevent. That path now catches, reports, and hands over to the real launcher, with Codex's exit status still authoritative. The Unix revision marker moves to 3 so installed Unix shims are detected as obsolete and regenerated. Windows shims have no revision marker and are excluded from obsolete-shim refresh, so existing Windows wrappers keep the old text until reinstalled; that gap is recorded in the lane document rather than papered over here. The new behaviour is covered by running the generated script against a stand-in ensure, in a sibling file because codex-shim.test.ts is close to its line cap and the cap only moves down. The test constants it kept as private literal copies now come from the source module. --- scripts/test-layout/layout.json | 1 + src/codex/shim-templates.ts | 32 ++++- .../codex-shim-ensure-failure.test.ts | 117 ++++++++++++++++++ tests/codex-integration/codex-shim.test.ts | 13 +- tests/fixtures/test-layout-expected.json | 1 + 5 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 tests/codex-integration/codex-shim-ensure-failure.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 74b27f8859c..983186163dc 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -560,6 +560,7 @@ "codex-service-manager-probe-hardening.test.ts": "codex-integration", "codex-service-manager-probe.test.ts": "codex-integration", "codex-shim-autorestore.test.ts": "codex-integration", + "codex-shim-ensure-failure.test.ts": "codex-integration", "codex-shim-readiness.test.ts": "codex-integration", "codex-shim.test.ts": "codex-integration", "codex-signin-lockout.test.ts": "codex-integration", diff --git a/src/codex/shim-templates.ts b/src/codex/shim-templates.ts index 5ed2cb6b4a1..0f5af00d022 100644 --- a/src/codex/shim-templates.ts +++ b/src/codex/shim-templates.ts @@ -4,11 +4,29 @@ import { serviceApiTokenFilePath } from "../lib/service-secrets"; import { windowsEnvIndirectBatchValue } from "../lib/win-paths"; const SHIM_MARKER = "opencodex codex autostart shim"; -const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; +const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 3"; const CODEX_SHIM_REENTRY_EXIT_CODE = 126; const CODEX_SHIM_REENTRY_DIAGNOSTIC = "opencodex: saved Codex launcher resolved back to the autostart shim; run ocx codex-shim uninstall and reinstall Codex before enabling codexAutoStart."; +/** + * Said once, on stderr, when `ocx ensure` could not bring the proxy up (#5261). + * + * The shim used to discard both of ensure's streams and ignore its exit status, so a failed + * autostart was completely silent: Codex launched against injected routing pointing at a port + * nothing was listening on, and every request — sign-in included — failed with no mention of + * opencodex anywhere. + * + * Ensure's own streams stay discarded rather than being let through. Ensure prints progress and + * warnings on exit-zero runs too, and a wrapper that leaked those would put noise in front of + * every ordinary Codex launch, which is how a diagnostic gets ignored. The exit status is the + * signal; this line is the whole message. + * + * It names `ocx restore` because bringing the proxy back is only half the choice. A user who + * cannot sign in needs the way out that does not require the proxy at all. + */ +export const CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC = "opencodex: proxy autostart failed; launching Codex anyway. Run 'ocx doctor' for details, or 'ocx restore' to hand Codex back to its own account."; + const CODEX_INTERNAL_COMMANDS = [ "app-server", "archive", @@ -136,7 +154,9 @@ case "$ocx_subcommand" in ;; *) if [ -z "$OCX_SHIM_BYPASS" ]; then - ${BUN_RUNTIME_SOURCE_ENV}=${shQuote(bunRuntimeSource)} ${BUN_RUNTIME_PATH_ENV}=${shQuote(bunPath)} ${shQuote(bunPath)} ${shQuote(cliPath)} ensure >/dev/null 2>&1 || true + if ! ${BUN_RUNTIME_SOURCE_ENV}=${shQuote(bunRuntimeSource)} ${BUN_RUNTIME_PATH_ENV}=${shQuote(bunPath)} ${shQuote(bunPath)} ${shQuote(cliPath)} ensure >/dev/null 2>&1; then + printf '%s\\n' ${shQuote(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC)} >&2 + fi fi ;; esac @@ -197,6 +217,7 @@ setlocal\r ${windowsBatchSet(BUN_RUNTIME_SOURCE_ENV, bunRuntimeSource)}\r ${windowsBatchSet(BUN_RUNTIME_PATH_ENV, bunPath)}\r "%OCX_BUN%" "%OCX_CLI%" ensure >nul 2>nul\r +if errorlevel 1 echo ${CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC} 1>&2\r endlocal\r :run_codex\r "%OCX_REAL_CODEX%" %*\r @@ -239,13 +260,18 @@ if (-not $skipEnsure) { $priorRuntimePath = $env:${BUN_RUNTIME_PATH_ENV} $env:${BUN_RUNTIME_SOURCE_ENV} = ${psString(bunRuntimeSource)} $env:${BUN_RUNTIME_PATH_ENV} = ${psString(bunPath)} - try { & ${psString(bunPath)} ${psString(cliPath)} ensure *> $null } + $ocxEnsureFailed = $false + # Caught, not propagated: a throwing ensure used to escape this wrapper and Codex never + # launched at all, which is a lockout produced by the autostart helper itself (#5261). + try { & ${psString(bunPath)} ${psString(cliPath)} ensure *> $null; if ($LASTEXITCODE -ne 0) { $ocxEnsureFailed = $true } } + catch { $ocxEnsureFailed = $true } finally { if ($null -eq $priorRuntimeSource) { Remove-Item Env:\\${BUN_RUNTIME_SOURCE_ENV} -ErrorAction SilentlyContinue } else { $env:${BUN_RUNTIME_SOURCE_ENV} = $priorRuntimeSource } if ($null -eq $priorRuntimePath) { Remove-Item Env:\\${BUN_RUNTIME_PATH_ENV} -ErrorAction SilentlyContinue } else { $env:${BUN_RUNTIME_PATH_ENV} = $priorRuntimePath } } + if ($ocxEnsureFailed) { [Console]::Error.WriteLine(${psString(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC)}) } } & ${psString(realCodexPath)} @args $codexExitCode = $LASTEXITCODE diff --git a/tests/codex-integration/codex-shim-ensure-failure.test.ts b/tests/codex-integration/codex-shim-ensure-failure.test.ts new file mode 100644 index 00000000000..ab652d64305 --- /dev/null +++ b/tests/codex-integration/codex-shim-ensure-failure.test.ts @@ -0,0 +1,117 @@ +/** + * #5261: the Codex autostart shim must never hide a failed autostart, and must never be the + * reason Codex does not launch. + * + * It used to do both. Both of `ocx ensure`'s streams were discarded and its exit status ignored + * with `|| true`, so a proxy that failed to come up was completely silent while Codex launched + * against injected routing pointing at a dead port. These run the real generated script against + * a stand-in `ensure` rather than asserting on template text, because the property under test is + * what the shell does with the exit status, not what the file says. + * + * Lives beside `codex-shim.test.ts` rather than inside it: that file sits close to its line cap, + * and the cap only moves down. + */ +import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildUnixCodexShim } from "../../src/codex/shim"; +import { CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC } from "../../src/codex/shim-templates"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "../helpers/test-budget"; + +/** The shim refuses to re-enter itself, so a nested test run must not inherit its guard state. */ +function cleanShimEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...process.env, OCX_SHIM_BYPASS: "" }; + delete env.OCX_SHIM_ACTIVE_PID; + delete env.OCX_SHIM_ACTIVE_DEPTH; + delete env.OCX_SHIM_PROBE_ACTIVE; + return env; +} + +/** + * A shim whose `ensure` is `script`. The real Codex prints a marker and exits 7, so every case + * can tell whether Codex ran and whether its exit status survived the wrapper. + */ +function shimWithEnsure(dir: string, script: string): string { + const realCodex = join(dir, "codex-real"); + writeFileSync(realCodex, "#!/bin/sh\necho codex-ran\nexit 7\n"); + chmodSync(realCodex, 0o755); + const fakeBun = join(dir, "fake-bun"); + writeFileSync(fakeBun, script); + chmodSync(fakeBun, 0o755); + const wrapper = join(dir, "codex"); + writeFileSync(wrapper, buildUnixCodexShim(realCodex, fakeBun, join(dir, "cli.ts"), "process", join(dir, "absent-token"))); + chmodSync(wrapper, 0o755); + return wrapper; +} + +function runShim(wrapper: string) { + return spawnSync("/bin/sh", [wrapper, "exec", "hello"], { + encoding: "utf8", + env: cleanShimEnv(), + timeout: INTERNAL_DEADLINE_MS, + }); +} + +describe.skipIf(process.platform === "win32")("Codex shim autostart failure (#5261)", () => { + test("a failed ensure is reported once on stderr and Codex still launches", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-shim-ensure-fail-")); + try { + // Writes to both streams so the assertions below also prove ensure's own chatter + // stays discarded: leaking it would put noise in front of every Codex launch. + const wrapper = shimWithEnsure(dir, "#!/bin/sh\necho ensure-stdout\necho ensure-stderr >&2\nexit 1\n"); + const run = runShim(wrapper); + + expect(run.error).toBeUndefined(); + expect(run.stdout).toContain("codex-ran"); + expect(run.status).toBe(7); + expect(run.stderr).toContain(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC); + expect(run.stderr).not.toContain("ensure-stderr"); + expect(run.stdout).not.toContain("ensure-stdout"); + + // Once, not once per stream or per retry. + const occurrences = run.stderr.split(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC).length - 1; + expect(occurrences).toBe(1); + } finally { + removeTreeWithRetry(dir); + } + }, SPAWN_BUDGET_MS); + + test("a successful ensure says nothing at all", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-shim-ensure-ok-")); + try { + const wrapper = shimWithEnsure(dir, "#!/bin/sh\necho ensure-stdout\nexit 0\n"); + const run = runShim(wrapper); + + expect(run.stdout).toContain("codex-ran"); + expect(run.status).toBe(7); + expect(run.stderr).not.toContain(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC); + expect(run.stdout).not.toContain("ensure-stdout"); + } finally { + removeTreeWithRetry(dir); + } + }, SPAWN_BUDGET_MS); + + test("an ensure that cannot be executed at all is still not a lockout", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-shim-ensure-missing-")); + try { + // The launcher path does not exist. This is the headless/broken-install shape, and the + // wrapper must treat it exactly like a failed start rather than dying with it. + const realCodex = join(dir, "codex-real"); + writeFileSync(realCodex, "#!/bin/sh\necho codex-ran\nexit 7\n"); + chmodSync(realCodex, 0o755); + const wrapper = join(dir, "codex"); + writeFileSync(wrapper, buildUnixCodexShim(realCodex, join(dir, "no-such-bun"), join(dir, "cli.ts"), "process", join(dir, "absent-token"))); + chmodSync(wrapper, 0o755); + + const run = runShim(wrapper); + expect(run.stdout).toContain("codex-ran"); + expect(run.status).toBe(7); + expect(run.stderr).toContain(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC); + } finally { + removeTreeWithRetry(dir); + } + }, SPAWN_BUDGET_MS); +}); diff --git a/tests/codex-integration/codex-shim.test.ts b/tests/codex-integration/codex-shim.test.ts index f684fa8de91..09241d8b58d 100644 --- a/tests/codex-integration/codex-shim.test.ts +++ b/tests/codex-integration/codex-shim.test.ts @@ -8,9 +8,8 @@ import { prependPath, withInstalledShim } from "../helpers/codex-shim-install-fi import { removeTreeWithRetry } from "../helpers/remove-tree"; import { repoPath, repoRoot } from "../helpers/repo-root"; import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "../helpers/test-budget"; +import { CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC, SHIM_MARKER, UNIX_SHIM_REVISION_MARKER } from "../../src/codex/shim-templates"; -const SHIM_MARKER = "opencodex codex autostart shim"; -const UNIX_SHIM_REVISION_MARKER = "opencodex unix codex shim revision 2"; /** * A child environment with the shim's recursion-guard state stripped. @@ -1333,9 +1332,14 @@ printf '%s\\n' child-codex }); expect(result.error).toBeUndefined(); expect(result.status, result.stderr).toBe(0); + // An ensure failure no longer stops Codex from launching (#5261). The wrapper + // reports it on stderr and hands over to the real launcher, so the error that + // reaches the caller is always Codex's own, in both phases. + const surfaced = "fixture Codex failure"; expect(result.stdout.trim().split(/\r?\n/)).toEqual([ - `error:fixture ${failurePhase} failure`, `after:${callerToken ?? ""}`, "presence-preserved:True", + `error:${surfaced}`, `after:${callerToken ?? ""}`, "presence-preserved:True", ]); + expect(result.stderr.includes(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC)).toBe(failurePhase === "ensure"); // A failed process must complete, rather than satisfy the check through a timeout. writeFileSync(driverPath, `\uFEFF$ErrorActionPreference = 'Stop'\n& '${wrapperPath.replace(/'/g, "''")}' exec\n`); @@ -1346,7 +1350,8 @@ printf '%s\\n' child-codex expect(uncaught.signal).toBeNull(); expect(typeof uncaught.status, uncaught.stderr).toBe("number"); expect(uncaught.status, uncaught.stderr).not.toBe(0); - expect(uncaught.stderr).toContain(`fixture ${failurePhase} failure`); + expect(uncaught.stderr).toContain(surfaced); + expect(uncaught.stderr.includes(CODEX_SHIM_ENSURE_FAILED_DIAGNOSTIC)).toBe(failurePhase === "ensure"); } finally { if (oldHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = oldHome; diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 646d7e75263..9256eebe2fb 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -391,6 +391,7 @@ "codex-service-manager-probe-hardening.test.ts": "codex-integration", "codex-service-manager-probe.test.ts": "codex-integration", "codex-shim-autorestore.test.ts": "codex-integration", + "codex-shim-ensure-failure.test.ts": "codex-integration", "codex-shim-readiness.test.ts": "codex-integration", "codex-shim.test.ts": "codex-integration", "codex-signin-lockout.test.ts": "codex-integration", From f90d94c559ed012bbf459214ce4818c1c560ef3f Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:06:41 +0900 Subject: [PATCH 2/7] fix(cli): report a Codex catalog pointer whose file is gone A model_catalog_json naming a file that no longer exists does not degrade Codex, it stops Codex loading its configuration at all. That presents as the same blank wall as the dead routing in #5261 while having a different cause and a different fix, and it is the state the reporter's machine was left in after the catalog file was deleted by hand. Injection already repairs this: the chooser refuses a missing owned path and the caller strips the stale line. My earlier note that the pointer survived injection was wrong, and the end-to-end coverage for it already exists. What was missing is that the repair only reaches someone who runs opencodex again, and the whole difficulty of this state is that Codex is the thing that stopped working, so nothing prompts them to. "ocx status" now says it out loud, names the file, and offers both outcomes: regenerate the catalog with "ocx start", or take opencodex out of Codex with "ocx restore". Which one they want is their choice. A catalog the user named is left alone whether or not it exists, exactly as during injection. Claiming it would put opencodex's recovery advice in front of a problem that is not opencodex's to explain. An unreadable or absent config reports nothing rather than inventing a finding. --- src/cli/index.ts | 7 ++- src/cli/status.ts | 35 ++++++++++++ src/codex/inject/config-toml.ts | 23 ++++++++ .../codex-signin-lockout.test.ts | 56 ++++++++++++++++++- 4 files changed, 118 insertions(+), 3 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index e689bd26f36..4495c66541a 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -63,7 +63,7 @@ import { pendingTeardownsAreExactly, quarantinePendingTeardown, } from "../config/pending-teardown"; -import { collectStatus, deadProxyRoutingAdviceLines, hubStatusLines, remoteHubBannerLine, remoteHubStatusLines, unusedProxyWarningLines } from "./status"; +import { collectStatus, deadProxyRoutingAdviceLines, detectMissingCodexCatalogPath, hubStatusLines, missingCodexCatalogLines, remoteHubBannerLine, remoteHubStatusLines, unusedProxyWarningLines } from "./status"; import { endpointsToProve, everyEndpointProvenDown, sharedTeardownAuthorized, type UninstallObservation } from "./uninstall-plan"; import { takeFlag } from "./runtime-api"; import { parseStartOptions, StartArgsError } from "./start-args"; @@ -1614,6 +1614,11 @@ async function handleStatus() { console.log(` Codex autostart: ${status.json.codexAutostart ? "enabled" : "disabled"}${local}`); console.log(` Restart safety: ${startupHealthSummary(status.json.startup)}${local}`); console.log(` ${formatStartupRoutingDetail(status.json.startup)}${local}`); + // Independent of whether the proxy is up: a catalog pointer whose file is gone stops Codex + // loading its config at all, and presents as the same blank wall as dead routing (#5261). + for (const line of missingCodexCatalogLines(detectMissingCodexCatalogPath())) { + console.log(` ${line}`); + } if (status.json.startup.routingKind === "native") { let retainedProviderTable = false; try { diff --git a/src/cli/status.ts b/src/cli/status.ts index 22d1e25871c..9079a2b5f90 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -1,4 +1,5 @@ import { durableBunRuntime } from "../lib/bun-runtime"; +import { existsSync, readFileSync } from "node:fs"; import { codexAutoStartEnabled, getConfigPath, readConfigDiagnostics } from "../config"; import { getPidPath, readPid, readRuntimePort, type RuntimePortState } from "../config/process-state"; import { diagnoseCodexBundledPlugins, type CodexPluginsDiagnostic } from "../codex/plugins-doctor"; @@ -7,6 +8,8 @@ import type { OcxConfig } from "../types"; import { diagnoseService, serviceLogPath } from "../service"; import { collectStartupHealth, type StartupHealth } from "../codex/autostart-health"; import { getCodexRoutingKind } from "../codex/inject"; +import { missingOwnedCatalogPath } from "../codex/inject/config-toml"; +import { CODEX_CONFIG_PATH } from "../codex/paths"; import { diagnoseCodexShim } from "../codex/shim"; import { displayCodexRuntimePath, effortClampAppliesToRuntime, liveRemovedEfforts, loadLastEffortClamp, resolveCodexRuntime } from "../codex/runtime"; import { packageVersion } from "./help"; @@ -529,6 +532,38 @@ export function deadProxyRoutingAdviceLines(input: { ]; } +/** + * Read the live Codex config and report an opencodex catalog pointer whose file is gone. + * + * Unreadable or absent config is reported as no finding rather than as a problem: this is a + * diagnostic line, and inventing one from missing evidence is worse than staying quiet. + */ +export function detectMissingCodexCatalogPath(): string | null { + try { + if (!existsSync(CODEX_CONFIG_PATH)) return null; + return missingOwnedCatalogPath(readFileSync(CODEX_CONFIG_PATH, "utf8")); + } catch { + return null; + } +} + +/** + * The one state in this report where Codex is broken independently of the proxy (#5261). + * + * A `model_catalog_json` naming a file that is gone stops Codex loading its configuration at + * all, so it presents as the same blank wall as dead routing while having a different cause and + * a different fix. Both are named, because restarting the proxy rewrites the catalog and + * restoring removes the pointer, and which one the user wants is their choice, not ours. + */ +export function missingCodexCatalogLines(missingCatalogPath: string | null): string[] { + if (!missingCatalogPath) return []; + return [ + "⚠️ Codex is pointed at a model catalog that is no longer on disk, so Codex cannot load its config:", + ` ${missingCatalogPath}`, + " Regenerate it with 'ocx start', or remove opencodex from Codex with 'ocx restore'.", + ]; +} + export async function collectStatus(): Promise { const configDiagnostics = readConfigDiagnostics(); const config = configDiagnostics.config; diff --git a/src/codex/inject/config-toml.ts b/src/codex/inject/config-toml.ts index 7577e2c6813..3ee7068ef42 100644 --- a/src/codex/inject/config-toml.ts +++ b/src/codex/inject/config-toml.ts @@ -609,3 +609,26 @@ export function chooseCatalogPathForInjection( return existsSync(DEFAULT_CATALOG_PATH) ? DEFAULT_CATALOG_PATH : null; } + +/** + * The effective `model_catalog_json` is one of ours and the file is gone. + * + * Codex does not degrade on this: a catalog path it cannot read stops it loading its + * configuration at all, which looks exactly like the routing lockout in #5261 and is what the + * reporter's machine was left in after the catalog file was deleted by hand. + * + * Injection already repairs it — the chooser refuses a missing owned path and the caller strips + * the stale line. That only helps someone who runs opencodex again, and the whole difficulty of + * this state is that Codex is the thing that stopped working, so nothing prompts them to. This + * predicate exists so the CLI can say it out loud. + * + * A user-owned catalog assignment wins here exactly as it does during injection: if they named + * the file, its absence is theirs to explain, and we do not claim it. + */ +export function missingOwnedCatalogPath(content: string): string | null { + const existing = readRootModelCatalogPath(content); + if (!existing) return null; + const resolved = resolveCodexConfigPath(existing); + if (!isOpencodexCatalogPath(resolved)) return null; + return existsSync(resolved) ? null : existing; +} diff --git a/tests/codex-integration/codex-signin-lockout.test.ts b/tests/codex-integration/codex-signin-lockout.test.ts index 14452235647..9e256369ee4 100644 --- a/tests/codex-integration/codex-signin-lockout.test.ts +++ b/tests/codex-integration/codex-signin-lockout.test.ts @@ -11,16 +11,20 @@ * test that needed one running would be testing the wrong state. */ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { OCX_ROUTING_MARKER_LINE, OCX_SECTION_MARKER, hasInjectedCodexRouting, hasInjectedOpenaiBaseUrl, } from "../../src/codex/injected-marker"; -import { setRootOpenaiBaseUrl, setRootRealtimeWsBaseUrl } from "../../src/codex/inject/config-toml"; +import { missingOwnedCatalogPath, setRootOpenaiBaseUrl, setRootRealtimeWsBaseUrl } from "../../src/codex/inject/config-toml"; import { stripOpencodexConfig } from "../../src/codex/inject/remove"; -import { deadProxyRoutingAdviceLines } from "../../src/cli/status"; +import { deadProxyRoutingAdviceLines, missingCodexCatalogLines } from "../../src/cli/status"; import { findCommand } from "../../src/cli/registry"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; /** A Windows catalog path as TOML stores it: a basic string doubles the separators (#1798). */ const WINDOWS_CATALOG = JSON.stringify(String.raw`C:\Users\example\.codex\opencodex-catalog.json`); @@ -129,3 +133,51 @@ describe("Codex sign-in lockout behind a stopped proxy (#5261)", () => { .toContain(`ocx ${named}`); }); }); + +describe("a Codex catalog pointer whose file is gone (#5261)", () => { + /** Builds a root config naming `catalogPath`, in the escaped form TOML actually stores. */ + const configNaming = (catalogPath: string) => + `model = "gpt-5.5"\nmodel_catalog_json = ${JSON.stringify(catalogPath)}\n`; + + test("an owned catalog that is missing is reported, and one that exists is not", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-catalog-pointer-")); + try { + const present = join(dir, "opencodex-catalog.json"); + writeFileSync(present, "{}"); + const absent = join(dir, "gone", "opencodex-catalog.json"); + + expect(missingOwnedCatalogPath(configNaming(absent))).toBe(absent); + expect(missingOwnedCatalogPath(configNaming(present))).toBeNull(); + } finally { + removeTreeWithRetry(dir); + } + }); + + test("a catalog the user named is theirs, present or not", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-catalog-user-")); + try { + // Same missing file, a name we never write. Claiming it would put opencodex's recovery + // advice in front of a problem that is not opencodex's to explain. + const userOwned = join(dir, "gone", "my-catalog.json"); + expect(missingOwnedCatalogPath(configNaming(userOwned))).toBeNull(); + expect(missingOwnedCatalogPath('model = "gpt-5.5"\n')).toBeNull(); + } finally { + removeTreeWithRetry(dir); + } + }); + + test("the report names the file and both ways out", () => { + const lines = missingCodexCatalogLines("/somewhere/opencodex-catalog.json"); + expect(lines.length).toBeGreaterThan(0); + const joined = lines.join(" "); + expect(joined).toContain("/somewhere/opencodex-catalog.json"); + // Regenerating and removing are different outcomes; the user picks, so both are offered. + expect(joined).toContain("ocx start"); + expect(joined).toContain("ocx restore"); + expect(missingCodexCatalogLines(null)).toEqual([]); + }); + + test("the commands it names are real", () => { + for (const name of ["start", "restore"]) expect(findCommand(name)).toBeDefined(); + }); +}); From 006118e30bb7c77c167679801d5257c3e287485f Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:08:04 +0900 Subject: [PATCH 3/7] fix(cli): say at setup time that Codex routing outlives the proxy Applying the Codex integration writes routing that survives a restart, and then setup ends on "Setup complete". It does not install a background service -- that is a separate command -- so "routing written, nothing listening" is an ordinary state after the next reboot rather than a corruption. Nobody says so, which is half of why #5261 read as a Codex fault rather than an opencodex one. Setup now ends by reusing the existing restart-health model: when the install is restart-unsafe it prints what status and doctor already say about it, and names "ocx restore" as the way out that does not need the proxy back. It runs after the autostart choice, because that choice is what decides whether the warning applies, and a diagnostic that cannot be computed does not fail a completed setup. Deliberately not a Windows boot trigger. The scheduled task runs as the interactive user, so before logon there is no session for it to run in; adding a BootTrigger would read like a fix and change nothing. Making it genuinely pre-logon means a different principal and a different service backend, which is a larger change than this lane, and is recorded as such rather than half-done here. --- src/cli/init.ts | 8 +++++++ src/codex/autostart-health.ts | 26 +++++++++++++++++++++++ tests/service/autostart-health.test.ts | 29 +++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/cli/init.ts b/src/cli/init.ts index f310b7f7966..e53b203415a 100644 --- a/src/cli/init.ts +++ b/src/cli/init.ts @@ -253,6 +253,14 @@ export async function runInit(): Promise { } console.log(`\n🚀 Setup complete! Run 'ocx start' to start the proxy.`); + // Said after the autostart choice, because the choice is what decides whether it applies. + // Setup otherwise ends on a success line while leaving a restart dependency unmentioned. + try { + const { collectStartupHealth, injectedRoutingRestartWarningLines } = await import("../codex/autostart-health"); + for (const line of injectedRoutingRestartWarningLines(collectStartupHealth(config))) console.log(line); + } catch { + // A diagnostic that cannot be computed must not fail a completed setup. + } for (const line of modelSelectionGuidance(providerName)) console.log(line); } catch (error) { if (error instanceof InitCancelledError) { diff --git a/src/codex/autostart-health.ts b/src/codex/autostart-health.ts index 5fc990a5f6c..7486d7b30fb 100644 --- a/src/codex/autostart-health.ts +++ b/src/codex/autostart-health.ts @@ -154,6 +154,32 @@ export function startupHealthSummary(health: StartupHealth): string { return action ? `${summary}; ${action}` : summary; } +/** + * What to say at the end of setup, once routing is on disk and the autostart choice is made. + * + * #5261: applying the Codex integration does not install a background service, and on Windows + * the scheduled task that a separate install would create is logon-triggered rather than + * boot-triggered. So "routing written, nothing listening" is an ordinary state after a restart + * rather than a corruption — and the user is never told, because setup ends on a success line. + * + * A boot trigger is not the missing piece and would be a false reassurance. The task runs as + * the interactive user, so before logon there is no session for it to run in; making it truly + * pre-logon means a different principal and a different service backend, not another trigger. + * What is actually missing is that nobody says the dependency exists, which is cheap to fix and + * true on every platform. + * + * Reuses the existing health model rather than re-deriving the condition, so this cannot drift + * from what `ocx status` and `ocx doctor` report about the same install. + */ +export function injectedRoutingRestartWarningLines(health: StartupHealth): string[] { + if (health.status !== "at-risk") return []; + return [ + "⚠️ Codex routing is written to disk and survives a restart, but nothing here will restart the proxy.", + ` ${startupHealthSummary(health)}`, + " While the proxy is down Codex cannot sign in or reach a model. 'ocx restore' undoes the routing without needing it.", + ]; +} + function classifyStartupHealthSummary(health: StartupHealth): string { if (health.status === "native") return health.routingKind === "custom-remote" ? "custom remote Codex routing (no local restart dependency)" diff --git a/tests/service/autostart-health.test.ts b/tests/service/autostart-health.test.ts index 293f21f4cec..b9f2ceabf08 100644 --- a/tests/service/autostart-health.test.ts +++ b/tests/service/autostart-health.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { collectStartupHealth, deriveStartupHealth, formatStartupRoutingDetail, startupHealthSummary } from "../../src/codex/autostart-health"; +import { collectStartupHealth, deriveStartupHealth, formatStartupRoutingDetail, injectedRoutingRestartWarningLines, startupHealthSummary } from "../../src/codex/autostart-health"; import { unusedProxyWarningLines } from "../../src/cli/status"; import { classifyCodexRouting, hasInjectedCodexRouting } from "../../src/codex/inject"; import { isCodexClientProcess, listCodexClientProcesses } from "../../src/codex/native-profile-processes"; @@ -396,6 +396,33 @@ describe("routing visibility (#2411)", () => { expect(unusedProxyWarningLines({ proxyUp: true, routingKind: "custom-local" })).toEqual([]); expect(unusedProxyWarningLines({ proxyUp: true, routingKind: "unknown" })).toEqual([]); }); + + // #5261: setup writes routing that outlives the session and then ends on a success line. + // The warning reuses the health model rather than re-deriving the condition, so it cannot + // disagree with what status and doctor say about the same install. + test("injectedRoutingRestartWarningLines speaks exactly when the install is restart-unsafe", () => { + const atRisk = deriveStartupHealth(base); + expect(atRisk.status).toBe("at-risk"); + const lines = injectedRoutingRestartWarningLines(atRisk); + expect(lines.length).toBeGreaterThan(0); + const joined = lines.join(" "); + expect(joined).toContain("survives a restart"); + expect(joined).toContain(startupHealthSummary(atRisk)); + // The way out that does not require the proxy to come back first. + expect(joined).toContain("ocx restore"); + + // A CLI-only shim still leaves Codex Desktop uncovered, which is the reported shape. + const shimmed = deriveStartupHealth({ ...base, shimInstalled: true, shimHealthy: true }); + expect(shimmed.status).toBe("at-risk"); + expect(injectedRoutingRestartWarningLines(shimmed).length).toBeGreaterThan(0); + + // Native routing has no opencodex restart dependency, so there is nothing to warn about. + expect(injectedRoutingRestartWarningLines(deriveStartupHealth({ ...base, routingKind: "native" }))).toEqual([]); + // Neither does a viable service, which is the state the warning is steering toward. + const served = deriveStartupHealth({ ...base, serviceInstalled: true, serviceViable: true, serviceEnabled: true, serviceRunning: true }); + expect(served.status).toBe("protected"); + expect(injectedRoutingRestartWarningLines(served)).toEqual([]); + }); }); // #4550: configured routing is not adopted routing. A Codex client that started From f0b84abf6b8a47941935bfe22d36558a48fbb921 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:11:13 +0900 Subject: [PATCH 4/7] fix(oauth): report a browser launch that never happened The URL launcher swallowed its own failure and returned nothing, so the Codex login route answered identically whether a browser opened, failed to open, or was deliberately skipped. The CLI then printed the URL and started polling, and a user whose machine could not launch a browser sat watching something that looked like it was working. That is the account-pool half of #5261: not an error, a silence. openUrl now resolves a result instead of returning void. It still never rejects and never throws, because a browser that will not open is an inconvenience rather than a login failure -- the URL remains a valid thing to open by hand and the flow stays live. Callers that genuinely do not care say so with void. The Codex login response carries browserLaunch, and the CLI prints a recovery line only when the launch failed. It names the fixed callback port, because that is the part a user cannot work out alone: ChatGPT supplies the redirect URI, so the flow cannot move to a free port, and --device is the way around it. Existing tests mocked openUrl as returning void, which the awaiting caller would have read as a failed launch; all eight mock sites now resolve a result. The new test does not exercise the started case: the launcher command is fixed per platform, so proving it would mean opening a real browser on whatever machine runs the suite. --- scripts/test-layout/layout.json | 1 + src/cli/account-auth.ts | 15 ++++++ src/cli/dispatch.ts | 2 +- src/codex/auth-api/login-flow.ts | 7 ++- src/lib/open-url.ts | 38 +++++++++++--- src/oauth/login-cli.ts | 4 +- src/server/management/oauth-account-routes.ts | 2 +- .../codex-integration/codex-auth-api.test.ts | 12 ++--- tests/fixtures/test-layout-expected.json | 1 + tests/oauth/oauth-login-open-browser.test.ts | 2 +- tests/oauth/oauth-open-browser-choice.test.ts | 2 +- tests/oauth/oauth-open-url-result.test.ts | 50 +++++++++++++++++++ 12 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 tests/oauth/oauth-open-url-result.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 983186163dc..860db7a1879 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1019,6 +1019,7 @@ "oauth-login-summary.test.ts": "oauth", "oauth-manual-code.test.ts": "oauth", "oauth-open-browser-choice.test.ts": "oauth", + "oauth-open-url-result.test.ts": "oauth", "oauth-provider-reconcile.test.ts": "oauth", "oauth-public-surface.test.ts": "oauth", "oauth-reauth-bind.test.ts": "oauth", diff --git a/src/cli/account-auth.ts b/src/cli/account-auth.ts index 73838f9da8e..de6364eb05f 100644 --- a/src/cli/account-auth.ts +++ b/src/cli/account-auth.ts @@ -69,8 +69,22 @@ interface LoginStart { flowId?: string; instructions?: string; deviceCode?: string; + /** Whether the host actually opened a browser. Absent from older proxies. */ + browserLaunch?: "started" | "failed" | "skipped"; } +/** + * Said only when the host could not open a browser (#5261). + * + * Without it, a failed launch is indistinguishable from a successful one: the URL is printed + * either way, so the user waits at a terminal that looks like it is working. Names the fixed + * callback port because that is the part people cannot guess — ChatGPT supplies the redirect + * URI, so the flow cannot move to a free port, and `--device` is the way around it. + */ +export const BROWSER_LAUNCH_FAILED_HINT = + "⚠️ No browser could be opened here — open the URL above yourself." + + "\n If nothing on this machine can reach http://localhost:1455, rerun with --device instead."; + /** `-` means "read it from stdin", the documented way to pass a code silently. */ const STDIN_SENTINEL = "-"; @@ -145,6 +159,7 @@ async function login(argv: string[], deps: RuntimeApiDeps): Promise { start.url ? `Open this URL to sign in:\n${start.url}` : "", start.deviceCode ? `Device code: ${start.deviceCode}` : "", start.instructions ?? "", + start.browserLaunch === "failed" ? BROWSER_LAUNCH_FAILED_HINT : "", start.flowId ? `Flow: ${start.flowId}` : "", ].filter(line => line !== "").join("\n"); if (block) writeStdoutFully(`${block}\n`); diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 029336f66c9..9b27cbf999d 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -615,7 +615,7 @@ const commandRunners: Record = { const guiUrl = selectDefaultGuiUrl(config, live, deps.probeHostname); console.log(`Opening ${guiUrl}`); const { openUrl } = await import("../lib/open-url"); - openUrl(guiUrl); + void openUrl(guiUrl); return 0; }, }); diff --git a/src/codex/auth-api/login-flow.ts b/src/codex/auth-api/login-flow.ts index fa6d1c6fbf7..3a9111e2a4d 100644 --- a/src/codex/auth-api/login-flow.ts +++ b/src/codex/auth-api/login-flow.ts @@ -226,9 +226,12 @@ export async function handleCodexAuthLoginStart(req: Request, config: OcxConfig, // machine. Opening it on the hub host is useless at best, and on a // headless host it fails. `deviceCode` is the same signal the generic // OAuth login route uses to make this decision. + // Reported to the caller rather than discarded (#5261): a login whose browser never opened + // is indistinguishable from one that did, so it reads as success while nothing happens. + let browserLaunch: "started" | "failed" | "skipped" = "skipped"; if (result.url && !result.deviceCode && shouldOpenBrowserForLogin(body.openBrowser, runtimeConfig)) { const { openUrl } = await import("../../lib/open-url"); - openUrl(result.url); + browserLaunch = (await openUrl(result.url)).status === "started" ? "started" : "failed"; } (async () => { @@ -488,6 +491,8 @@ export async function handleCodexAuthLoginStart(req: Request, config: OcxConfig, flowId, url: result.url, instructions: result.instructions, + // Never fatal: the URL is still a valid thing to open by hand, and the flow stays live. + browserLaunch, // Dropped before #3366: every device-code surface renders this field, // so withholding it left the GUI and CLI with no code to show. ...(result.deviceCode ? { deviceCode: result.deviceCode } : {}), diff --git a/src/lib/open-url.ts b/src/lib/open-url.ts index ae6cb900362..62f3947e45f 100644 --- a/src/lib/open-url.ts +++ b/src/lib/open-url.ts @@ -8,8 +8,22 @@ function windowsRundll32(): string { return existsSync(candidate) ? candidate : "rundll32"; } -export function openUrl(url: string): void { - if (!/^https?:\/\//i.test(url)) return; +/** + * Whether the OS launcher actually started. `started` does not prove a browser rendered the + * page — nothing observable from here can — but it does separate "we handed the URL off" from + * "there was nothing to hand it to", which is the distinction a caller needs (#5261). + */ +export type OpenUrlResult = + | { status: "started" } + | { status: "failed"; reason: "invalid-url" | "spawn-error" }; + +/** + * Never rejects. A browser that would not open is an inconvenience, not a login failure: the + * URL is still a valid thing to open by hand, so the caller decides what to say about it. + * Callers that genuinely do not care use `void openUrl(...)`. + */ +export function openUrl(url: string): Promise { + if (!/^https?:\/\//i.test(url)) return Promise.resolve({ status: "failed", reason: "invalid-url" }); const cmd = process.platform === "darwin" ? "open" : process.platform === "win32" ? windowsRundll32() @@ -17,9 +31,19 @@ export function openUrl(url: string): void { const args = process.platform === "win32" ? ["url.dll,FileProtocolHandler", url] : [url]; - const child = spawn(cmd, args, { detached: true, stdio: "ignore", shell: false }); - // Headless hosts (no xdg-open) emit ENOENT as an async 'error' event; without a - // listener that is an uncaught exception that kills the whole proxy/login flow. - child.on("error", () => {}); - child.unref(); + return new Promise(resolve => { + let settled = false; + const settle = (result: OpenUrlResult) => { + if (settled) return; + settled = true; + resolve(result); + }; + const child = spawn(cmd, args, { detached: true, stdio: "ignore", shell: false }); + // Headless hosts (no xdg-open) emit ENOENT as an async 'error' event; without a + // listener that is an uncaught exception that kills the whole proxy/login flow. + // It is also the signal itself: on Windows this is how a missing rundll32 arrives. + child.on("error", () => settle({ status: "failed", reason: "spawn-error" })); + child.on("spawn", () => settle({ status: "started" })); + child.unref(); + }); } diff --git a/src/oauth/login-cli.ts b/src/oauth/login-cli.ts index 45db1af0ecf..19ad8f833cd 100644 --- a/src/oauth/login-cli.ts +++ b/src/oauth/login-cli.ts @@ -106,7 +106,7 @@ async function handleOAuthLogin(name: string): Promise { onAuth: ({ url, instructions }) => { console.log(`\n🔐 Opening browser for ${name} login...\n${url}\n`); if (instructions) console.log(instructions); - openUrl(url); + void openUrl(url); }, onProgress: (m) => console.log(` ${m}`), onManualCodeInput: () => @@ -201,7 +201,7 @@ async function handleKeyLogin(name: string): Promise { process.exit(1); } console.log(`\n🔑 ${def.label} — opening ${def.dashboardUrl} so you can create/copy an API key...`); - openUrl(def.dashboardUrl); + void openUrl(def.dashboardUrl); const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); const key = (await new Promise((res) => rl.question(`Paste your ${def.label} API key: `, res))).trim(); // Template URL with placeholders needs resolution before saving. diff --git a/src/server/management/oauth-account-routes.ts b/src/server/management/oauth-account-routes.ts index 0cb29ff8acd..0c03a96d3da 100644 --- a/src/server/management/oauth-account-routes.ts +++ b/src/server/management/oauth-account-routes.ts @@ -210,7 +210,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const { shouldOpenBrowserForLogin } = await import("../../oauth/open-browser-choice"); if (authUrl && !deviceCode && shouldOpenBrowserForLogin(body.openBrowser, config)) { const { openUrl } = await import("../../lib/open-url"); - openUrl(authUrl); + void openUrl(authUrl); } return jsonResponse({ url: authUrl, instructions, deviceCode }); } catch (err) { diff --git a/tests/codex-integration/codex-auth-api.test.ts b/tests/codex-integration/codex-auth-api.test.ts index ab750aa06d0..c17088cb15e 100644 --- a/tests/codex-integration/codex-auth-api.test.ts +++ b/tests/codex-integration/codex-auth-api.test.ts @@ -170,7 +170,7 @@ async function completeMockCodexOAuth(options: { done: true, loggedIn: true, } as ReturnType); - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async () => ({ status: "started" as const })); // Mirrors the login-status poll delay in login-flow.ts; other timers are intentionally dropped. const CODEX_OAUTH_LOGIN_POLL_INTERVAL_MS = 2_000; const timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation((( @@ -4593,7 +4593,7 @@ describe("codex-auth API", () => { instructions: "Enter code: ABCD-EFGH", deviceCode: "ABCD-EFGH", })); - const openSpy = spyOn(openUrlModule, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlModule, "openUrl").mockImplementation(async () => ({ status: "started" as const })); try { const req = new Request("http://localhost/api/codex-auth/login", { method: "POST", @@ -4699,7 +4699,7 @@ describe("codex-auth API", () => { controller.onAuth({ url: "https://example.test/oauth" }); throw new Error("late failure at /home/alice/.opencodex/auth.json.ocx-tmp sk-secret-provider-key"); }; - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async () => ({ status: "started" as const })); const timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation((( callback: (...args: unknown[]) => void, delay?: number, @@ -4747,7 +4747,7 @@ describe("codex-auth API", () => { const { OAuthMutationBusyError } = await import("../../src/oauth/store"); const openUrlMod = await import("../../src/lib/open-url"); const originalLogin = oauth.OAUTH_PROVIDERS.chatgpt.login; - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async () => ({ status: "started" as const })); const timeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation((( callback: (...args: unknown[]) => void, delay?: number, @@ -4834,7 +4834,7 @@ describe("codex-auth API", () => { done: false, loggedIn: false, } as ReturnType); - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async () => ({ status: "started" as const })); const req = new Request("http://localhost/api/codex-auth/login", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -5161,7 +5161,7 @@ describe("codex-auth API", () => { loggedIn: false, error: "test-stop", } as ReturnType); - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(() => {}); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async () => ({ status: "started" as const })); try { const req = new Request("http://localhost/api/codex-auth/login", { diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 9256eebe2fb..90591082831 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -845,6 +845,7 @@ "oauth-login-summary.test.ts": "oauth", "oauth-manual-code.test.ts": "oauth", "oauth-open-browser-choice.test.ts": "oauth", + "oauth-open-url-result.test.ts": "oauth", "oauth-provider-reconcile.test.ts": "oauth", "oauth-public-surface.test.ts": "oauth", "oauth-reauth-bind.test.ts": "oauth", diff --git a/tests/oauth/oauth-login-open-browser.test.ts b/tests/oauth/oauth-login-open-browser.test.ts index 0b7928d2195..676641a305e 100644 --- a/tests/oauth/oauth-login-open-browser.test.ts +++ b/tests/oauth/oauth-login-open-browser.test.ts @@ -33,7 +33,7 @@ async function startLogin(flow: { url: string; instructions?: string; deviceCode const openUrlMod = await import("../../src/lib/open-url"); const opened: string[] = []; const startSpy = spyOn(oauth, "startLoginFlow").mockResolvedValue(flow); - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation((url: string) => { opened.push(url); }); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async (url: string) => { opened.push(url); return { status: "started" as const }; }); try { const req = loginRequest("kimi"); const response = await handleOauthAccountRoutes({ diff --git a/tests/oauth/oauth-open-browser-choice.test.ts b/tests/oauth/oauth-open-browser-choice.test.ts index 48c7f542788..81c133bb028 100644 --- a/tests/oauth/oauth-open-browser-choice.test.ts +++ b/tests/oauth/oauth-open-browser-choice.test.ts @@ -49,7 +49,7 @@ async function startLogin( const openUrlMod = await import("../../src/lib/open-url"); const opened: string[] = []; const startSpy = spyOn(oauth, "startLoginFlow").mockResolvedValue({ url: "https://accounts.x.ai/oauth/authorize?code_challenge=x" }); - const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation((url: string) => { opened.push(url); }); + const openSpy = spyOn(openUrlMod, "openUrl").mockImplementation(async (url: string) => { opened.push(url); return { status: "started" as const }; }); try { const req = new Request("http://127.0.0.1/api/oauth/login", { method: "POST", diff --git a/tests/oauth/oauth-open-url-result.test.ts b/tests/oauth/oauth-open-url-result.test.ts new file mode 100644 index 00000000000..e131dacb128 --- /dev/null +++ b/tests/oauth/oauth-open-url-result.test.ts @@ -0,0 +1,50 @@ +/** + * #5261: a browser that never opened must not look like a login that is working. + * + * The launcher used to swallow its own failure and return nothing, so the Codex login route + * reported the same success whether a browser opened, failed to open, or was deliberately + * skipped. The user then waited at a terminal showing a URL nobody had opened. + * + * Nothing here opens a real browser. The started case is deliberately untested rather than + * faked: the launcher command is fixed per platform, so proving it would mean actually + * launching one on the machine running the suite. + */ +import { describe, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { openUrl } from "../../src/lib/open-url"; +import { BROWSER_LAUNCH_FAILED_HINT } from "../../src/cli/account-auth"; + +describe("browser launch is reported, not swallowed (#5261)", () => { + test("a URL we will not hand to a launcher is reported as such", async () => { + // Refusing a non-http scheme is the one failure that never reaches spawn, so it is also + // the one that must not silently look like a launch. + for (const url of ["", "not-a-url", "file:///etc/passwd", "javascript:alert(1)"]) { + expect(await openUrl(url)).toEqual({ status: "failed", reason: "invalid-url" }); + } + }); + + test.skipIf(process.platform !== "linux")("a launcher that cannot be resolved reports a failure", async () => { + // Linux only, and on purpose: this empties PATH so the launcher cannot be found, and on a + // developer machine with a real browser any weaker setup risks actually opening one. + const priorPath = process.env.PATH; + try { + process.env.PATH = mkdtempSync(`${tmpdir()}/ocx-empty-path-`); + expect(await openUrl("http://127.0.0.1:1455/auth/callback")).toEqual({ + status: "failed", + reason: "spawn-error", + }); + } finally { + if (priorPath === undefined) delete process.env.PATH; + else process.env.PATH = priorPath; + } + }); + + test("the CLI hint names the manual route and the reason the port cannot move", () => { + // ChatGPT supplies the redirect URI, so the callback cannot move to a free port. That is + // the part a user cannot work out alone, which is why the hint names it and --device. + expect(BROWSER_LAUNCH_FAILED_HINT).toContain("1455"); + expect(BROWSER_LAUNCH_FAILED_HINT).toContain("--device"); + expect(BROWSER_LAUNCH_FAILED_HINT.toLowerCase()).toContain("open the url"); + }); +}); From b6fc33b4f089424d3b726d53d9d2f4fd7a22d0f7 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:11:58 +0900 Subject: [PATCH 5/7] docs(devlog): record lane H2, the remaining lockout dead ends Explains why the four paths belong in one lane: none locks anyone out alone, and the incident is what happens when every signal is missing at once. Records two corrections rather than burying them -- lane H's claim that the catalog pointer survives injection was wrong, and a Windows BootTrigger is a fake fix given the task's interactive principal -- plus the three residuals this lane deliberately leaves open. --- .../030_lane_h2_lockout_dead_ends.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 devlog/_plan/260920_meaning_preservation_batch/030_lane_h2_lockout_dead_ends.md diff --git a/devlog/_plan/260920_meaning_preservation_batch/030_lane_h2_lockout_dead_ends.md b/devlog/_plan/260920_meaning_preservation_batch/030_lane_h2_lockout_dead_ends.md new file mode 100644 index 00000000000..d1c6f7c1d94 --- /dev/null +++ b/devlog/_plan/260920_meaning_preservation_batch/030_lane_h2_lockout_dead_ends.md @@ -0,0 +1,90 @@ +# Lane H2 — the remaining Codex lockout dead ends (#5261) + +Status: OPEN. Base is `origin/dev` at `043aa435ff`, after lane H landed as `9880c3cad2`. +One branch, ordered commits, one pull request to `dev`. + +Lane H removed the dead end a locked-out user hits: injected routing now names its own undo, +a dead proxy is told to say so, and there is a troubleshooting page. This lane takes the four +paths INTO that state which lane H listed as not closed. + +## Why these four belong together + +None of them locks anyone out alone. Each one removes a signal, and the incident is what +happens when all of them are missing at once: a proxy that stopped, an autostart that failed +silently, a reboot that restarted nothing, a catalog pointer left naming a deleted file, and an +account flow that looked like it was working. Every individual step had an explanation; the +user had no way to reach any of them. + +## 1. The shim hid autostart failure, and could prevent Codex launching + +The wrapper ran `ocx ensure` with both streams discarded and `|| true`, then launched Codex +regardless (`src/codex/shim-templates.ts:139`). A failed start was invisible. + +Ensure's own streams stay discarded rather than being let through. It prints progress and +warnings on exit-zero runs too, so a wrapper that leaked them would put noise in front of every +ordinary launch, which is how a diagnostic gets ignored. The exit status is the signal. + +PowerShell had the opposite defect in the same place: a throwing `ensure` escaped a `try/finally` +with no `catch`, so Codex never launched at all. The autostart helper was producing the exact +lockout it exists to prevent, and the previous test asserted that propagation as correct +behaviour. It now catches, reports, and hands over. + +Not closed: the Unix revision marker moved to 3 so installed Unix shims regenerate, but Windows +shims carry no revision marker and are excluded from obsolete-shim refresh +(`src/codex/shim.ts:892`). Existing Windows wrappers keep the old text until reinstalled. Giving +Windows a refresh path is its own change. + +## 2. The catalog pointer — my lane H note was wrong + +Lane H recorded that nothing revalidates `model_catalog_json` after injection. That is not true. +The chooser refuses a missing owned path and the caller strips the stale line +(`src/codex/inject.ts:327`), with end-to-end coverage already in place. + +The real gap is narrower and worse: that repair only reaches someone who runs opencodex again, +and the difficulty of this state is that Codex is the thing that stopped working, so nothing +prompts them to. A `model_catalog_json` naming a file that is gone does not degrade Codex, it +stops Codex loading its configuration at all — the same blank wall as dead routing, from a +different cause. So this lane adds detection, not repair, and `ocx status` now names the file +and both ways out. + +## 3. Reboot: stated, not faked + +Applying the integration does not install a service, and the Windows scheduled task a separate +install would create is logon-triggered (`src/service/windows-taskxml.ts:225`). Setup ended on a +success line without ever saying routing outlives the proxy. + +A BootTrigger is deliberately NOT the fix. The task runs as the interactive user +(`LogonType: InteractiveToken`), so before logon there is no session for it to run in; the +trigger would read like a fix and change nothing. Genuine pre-logon start means a different +principal and a different service backend, which is larger than this lane and is left open +rather than half-done. What was cheap and true on every platform is saying the dependency +exists, reusing the health model `ocx status` and `ocx doctor` already report. + +## 4. Account pool: a silence, not an error + +The URL launcher swallowed its own failure and returned nothing, so the Codex login route +answered identically whether a browser opened, failed, or was skipped. The CLI printed the URL +and polled, and a user whose machine could not launch a browser watched something that looked +like it was working. + +Launch failure is now reported and never fatal — the URL is still worth opening by hand and the +flow stays live. The CLI names the fixed callback port 1455, because ChatGPT supplies the +redirect URI and the flow cannot move to a free port, so `--device` is the way around it. + +Not closed: the dashboard keeps last-good rows after a failed refresh +(`gui/src/hooks/useCodexAccountPool.ts:325`), which is why a newly added account can be absent +while older rows still show. That is a GUI change, and the pull-request gate requires a +screenshot of a UI change, which cannot be produced under this lane's no-build constraint. It +is left for a lane that can build the GUI. + +## Verification + +Static source review and exact-head hosted CI only. NOT RUN, by lane constraint: local suite, +individual tests, typecheck, build, install, `ocx` execution, service start or restart, and any +change to credentials or configuration on the machine. The incident being fixed is a +configuration change that locked a user out. + +Checked before push: the file-size ratchet (no tracked file over cap; `codex-shim.test.ts` +sits close to its cap, so the new shim tests went to a sibling file registered in both +layout maps), and the restated-constant class that broke two lanes in this batch — the shim +test's private literal copies of the marker constants now come from the source module. From 6982340b61626642e9c26d6227fb353d6c018cc9 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:19:58 +0900 Subject: [PATCH 6/7] test(ci): record the new shim test's cold-spawn disposition Hosted CI failed on the guard that every test file bounding a spawned child with the internal deadline must declare whether it warms that child's module graph. The new shim test spawns children and did not. Recorded as unwarmed, for the same reason as the file it sits beside: its children are throwaway shell scripts standing in for ensure and for the real Codex launcher, so the cold cost is shell and process startup rather than a repository module graph, and an import scan has nothing to warm. The generated shim never loads a repository module in the child -- the point of the file is what the shell does with an exit status. --- tests/ci-workflows/cold-spawn-warmup.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/ci-workflows/cold-spawn-warmup.test.ts b/tests/ci-workflows/cold-spawn-warmup.test.ts index 7e969d8cf2a..29e86fdc944 100644 --- a/tests/ci-workflows/cold-spawn-warmup.test.ts +++ b/tests/ci-workflows/cold-spawn-warmup.test.ts @@ -78,6 +78,14 @@ const DISPOSITIONS: Readonly> = { warmed: true, why: "the first resetAt iteration loads src/codex/quota.ts and src/codex/main-account-cache.ts", }, + "tests/codex-integration/codex-shim-ensure-failure.test.ts": { + warmed: false, + why: + "Its children are two throwaway /bin/sh scripts standing in for ensure and for the real Codex " + + "launcher, so the cold cost is shell and process startup rather than a repository module " + + "graph, and an import scan has nothing to warm. The generated shim never loads a repository " + + "module in the child: the point of the file is what the shell does with an exit status.", + }, "tests/codex-integration/codex-shim.test.ts": { warmed: false, why: From 61a938fca6bfa4487332d95f6fd19773c1654328 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:26:51 +0900 Subject: [PATCH 7/7] fix: close the gaps an adversarial review found in this lane Blocker: the PowerShell shim test's Codex-failure fixture also exited 19 from ensure, which the wrapper now correctly reports as a failed autostart, so that phase no longer isolated a Codex failure and both PowerShell variants would have failed on Windows. Its ensure now succeeds, which is what the phase always meant. The setup warning said "nothing here will restart the proxy" for every restart-unsafe install. A healthy launcher shim is also restart-unsafe, because it covers CLI launches only, but it does restart the proxy for those -- and the summary line printed directly beneath said so. The warning now states the dependency without the false absolute, and a test pins that the shim case does not claim otherwise. The URL launcher resolved "started" on the spawn event, which only proves a process began. A launcher with no handler spawns happily and exits nonzero a moment later without opening anything, so a login could still report a launch that never happened. It now watches briefly for an immediate nonzero exit, which is how those failures arrive. "ocx gui" awaits the launch and says so when it did not happen. It still exits 0: the proxy is serving and the URL it printed is reachable, only the launch failed. The catalog finding is tagged (local) on its header like every other local-state line, so a connected client cannot read a finding about its own Codex home as something the hub reported. Catalog ownership stays decided by basename, now stated as a choice rather than left to look like an oversight: it is the same test injection applies, and a detector drawing the line elsewhere would report a state injection would then treat differently. Also removes the temporary directory the launcher test created, and corrects that file's header to describe what it actually covers. --- src/cli/dispatch.ts | 6 ++++- src/cli/index.ts | 7 +++--- src/codex/autostart-health.ts | 4 +++- src/codex/inject/config-toml.ts | 6 +++++ src/lib/open-url.ts | 24 +++++++++++++++++-- tests/codex-integration/codex-shim.test.ts | 5 +++- .../codex-signin-lockout.test.ts | 2 ++ tests/oauth/oauth-open-url-result.test.ts | 13 ++++++---- tests/service/autostart-health.test.ts | 4 ++++ 9 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 9b27cbf999d..b9b09420c87 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -615,7 +615,11 @@ const commandRunners: Record = { const guiUrl = selectDefaultGuiUrl(config, live, deps.probeHostname); console.log(`Opening ${guiUrl}`); const { openUrl } = await import("../lib/open-url"); - void openUrl(guiUrl); + // Awaited so a launcher that never opened anything is said out loud (#5261). Still exit + // 0: the proxy is serving and the URL above is reachable, only the launch did not happen. + if ((await openUrl(guiUrl)).status === "failed") { + console.error("⚠️ No browser could be opened here; open the URL above yourself."); + } return 0; }, }); diff --git a/src/cli/index.ts b/src/cli/index.ts index 4495c66541a..c34ad2cf9b2 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1616,9 +1616,10 @@ async function handleStatus() { console.log(` ${formatStartupRoutingDetail(status.json.startup)}${local}`); // Independent of whether the proxy is up: a catalog pointer whose file is gone stops Codex // loading its config at all, and presents as the same blank wall as dead routing (#5261). - for (const line of missingCodexCatalogLines(detectMissingCodexCatalogPath())) { - console.log(` ${line}`); - } + // Tagged `(local)` on its header like every other local-state line, so a connected client + // cannot read a finding about its own Codex home as something the hub reported. + missingCodexCatalogLines(detectMissingCodexCatalogPath()) + .forEach((line, index) => console.log(` ${line}${index === 0 ? local : ""}`)); if (status.json.startup.routingKind === "native") { let retainedProviderTable = false; try { diff --git a/src/codex/autostart-health.ts b/src/codex/autostart-health.ts index 7486d7b30fb..83b330ca3db 100644 --- a/src/codex/autostart-health.ts +++ b/src/codex/autostart-health.ts @@ -174,7 +174,9 @@ export function startupHealthSummary(health: StartupHealth): string { export function injectedRoutingRestartWarningLines(health: StartupHealth): string[] { if (health.status !== "at-risk") return []; return [ - "⚠️ Codex routing is written to disk and survives a restart, but nothing here will restart the proxy.", + // Deliberately not "nothing will restart the proxy": a healthy launcher shim does restart it, + // for CLI launches only, and is still at-risk. The summary line below says which case this is. + "⚠️ Codex routing is written to disk and survives a restart; keeping the proxy running is a separate matter.", ` ${startupHealthSummary(health)}`, " While the proxy is down Codex cannot sign in or reach a model. 'ocx restore' undoes the routing without needing it.", ]; diff --git a/src/codex/inject/config-toml.ts b/src/codex/inject/config-toml.ts index 3ee7068ef42..1e552dbbef3 100644 --- a/src/codex/inject/config-toml.ts +++ b/src/codex/inject/config-toml.ts @@ -624,6 +624,12 @@ export function chooseCatalogPathForInjection( * * A user-owned catalog assignment wins here exactly as it does during injection: if they named * the file, its absence is theirs to explain, and we do not claim it. + * + * Ownership is decided by basename, which is a weak test — a file the user happens to name + * `opencodex-catalog.json` is read as ours wherever it sits. That is deliberate rather than + * overlooked: it is the same test injection already applies, and a detector that drew the line + * somewhere else would report a state injection would then treat differently. Tightening it is a + * change to injection, not to this. */ export function missingOwnedCatalogPath(content: string): string | null { const existing = readRootModelCatalogPath(content); diff --git a/src/lib/open-url.ts b/src/lib/open-url.ts index 62f3947e45f..a13287ccf6e 100644 --- a/src/lib/open-url.ts +++ b/src/lib/open-url.ts @@ -15,7 +15,18 @@ function windowsRundll32(): string { */ export type OpenUrlResult = | { status: "started" } - | { status: "failed"; reason: "invalid-url" | "spawn-error" }; + | { status: "failed"; reason: "invalid-url" | "spawn-error" | "launcher-exit" }; + +/** + * How long to watch a launcher that did spawn before calling it started. + * + * `spawn` only proves a process began. `xdg-open` with no desktop handler, and rundll32 given a + * broken association, both spawn happily and exit nonzero a moment later without opening + * anything — so resolving on `spawn` alone would report a launch that did not happen. Those + * failures are immediate, and the only caller that awaits this is a login start, so a short + * window buys a true answer cheaply. A launcher still running when it elapses has started. + */ +const LAUNCHER_SETTLE_MS = 400; /** * Never rejects. A browser that would not open is an inconvenience, not a login failure: the @@ -33,9 +44,11 @@ export function openUrl(url: string): Promise { : [url]; return new Promise(resolve => { let settled = false; + let timer: ReturnType | undefined; const settle = (result: OpenUrlResult) => { if (settled) return; settled = true; + if (timer) clearTimeout(timer); resolve(result); }; const child = spawn(cmd, args, { detached: true, stdio: "ignore", shell: false }); @@ -43,7 +56,14 @@ export function openUrl(url: string): Promise { // listener that is an uncaught exception that kills the whole proxy/login flow. // It is also the signal itself: on Windows this is how a missing rundll32 arrives. child.on("error", () => settle({ status: "failed", reason: "spawn-error" })); - child.on("spawn", () => settle({ status: "started" })); + child.on("spawn", () => { + // Unref'd either way: this never keeps the process alive, it only decides what to report. + timer = setTimeout(() => settle({ status: "started" }), LAUNCHER_SETTLE_MS); + timer.unref?.(); + }); + child.on("exit", code => settle(code === 0 || code === null + ? { status: "started" } + : { status: "failed", reason: "launcher-exit" })); child.unref(); }); } diff --git a/tests/codex-integration/codex-shim.test.ts b/tests/codex-integration/codex-shim.test.ts index 09241d8b58d..efc971030b4 100644 --- a/tests/codex-integration/codex-shim.test.ts +++ b/tests/codex-integration/codex-shim.test.ts @@ -1320,7 +1320,10 @@ printf '%s\\n' child-codex const driverPath = join(dir, "driver.ps1"); const realPath = join(dir, "codex-real.ps1"); writeFileSync(join(dir, "service-api-token"), "file-token\n"); - writeFileSync(ensurePath, failurePhase === "ensure" ? "throw 'fixture ensure failure'\n" : "exit 19\n"); + // The Codex phase must isolate a Codex failure, so its ensure has to SUCCEED. It used + // to exit 19, which the wrapper now correctly reports as a failed autostart (#5261), + // making both phases indistinguishable. + writeFileSync(ensurePath, failurePhase === "ensure" ? "throw 'fixture ensure failure'\n" : "exit 0\n"); writeFileSync(realPath, "throw 'fixture Codex failure'\n"); writeFileSync(wrapperPath, `\uFEFF${buildWindowsPowerShellCodexShim(realPath, ensurePath, "unused.ts", "process")}`); const emptyToken = callerToken === "" ? "$env:OPENCODEX_API_AUTH_TOKEN = ''\n" : ""; diff --git a/tests/codex-integration/codex-signin-lockout.test.ts b/tests/codex-integration/codex-signin-lockout.test.ts index 9e256369ee4..a11f6ec5789 100644 --- a/tests/codex-integration/codex-signin-lockout.test.ts +++ b/tests/codex-integration/codex-signin-lockout.test.ts @@ -142,6 +142,8 @@ describe("a Codex catalog pointer whose file is gone (#5261)", () => { test("an owned catalog that is missing is reported, and one that exists is not", () => { const dir = mkdtempSync(join(tmpdir(), "ocx-catalog-pointer-")); try { + // Ownership is by basename, which is what injection already uses. Pinned here so the + // detector cannot start disagreeing with the code that writes and strips the same line. const present = join(dir, "opencodex-catalog.json"); writeFileSync(present, "{}"); const absent = join(dir, "gone", "opencodex-catalog.json"); diff --git a/tests/oauth/oauth-open-url-result.test.ts b/tests/oauth/oauth-open-url-result.test.ts index e131dacb128..17ca2d97fdb 100644 --- a/tests/oauth/oauth-open-url-result.test.ts +++ b/tests/oauth/oauth-open-url-result.test.ts @@ -1,9 +1,10 @@ /** * #5261: a browser that never opened must not look like a login that is working. * - * The launcher used to swallow its own failure and return nothing, so the Codex login route - * reported the same success whether a browser opened, failed to open, or was deliberately - * skipped. The user then waited at a terminal showing a URL nobody had opened. + * The launcher used to swallow its own failure and return nothing, which is what left the Codex + * login route unable to tell a browser that opened from one that never did. These cover the + * launcher itself and the recovery line it feeds; the route and the CLI block that consume the + * result are covered where those live. * * Nothing here opens a real browser. The started case is deliberately untested rather than * faked: the launcher command is fixed per platform, so proving it would mean actually @@ -11,6 +12,7 @@ */ import { describe, expect, test } from "bun:test"; import { mkdtempSync } from "node:fs"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; import { tmpdir } from "node:os"; import { openUrl } from "../../src/lib/open-url"; import { BROWSER_LAUNCH_FAILED_HINT } from "../../src/cli/account-auth"; @@ -28,8 +30,10 @@ describe("browser launch is reported, not swallowed (#5261)", () => { // Linux only, and on purpose: this empties PATH so the launcher cannot be found, and on a // developer machine with a real browser any weaker setup risks actually opening one. const priorPath = process.env.PATH; + let emptyDir: string | undefined; try { - process.env.PATH = mkdtempSync(`${tmpdir()}/ocx-empty-path-`); + emptyDir = mkdtempSync(`${tmpdir()}/ocx-empty-path-`); + process.env.PATH = emptyDir; expect(await openUrl("http://127.0.0.1:1455/auth/callback")).toEqual({ status: "failed", reason: "spawn-error", @@ -37,6 +41,7 @@ describe("browser launch is reported, not swallowed (#5261)", () => { } finally { if (priorPath === undefined) delete process.env.PATH; else process.env.PATH = priorPath; + if (emptyDir) removeTreeWithRetry(emptyDir); } }); diff --git a/tests/service/autostart-health.test.ts b/tests/service/autostart-health.test.ts index b9f2ceabf08..b681fa7a775 100644 --- a/tests/service/autostart-health.test.ts +++ b/tests/service/autostart-health.test.ts @@ -415,6 +415,10 @@ describe("routing visibility (#2411)", () => { const shimmed = deriveStartupHealth({ ...base, shimInstalled: true, shimHealthy: true }); expect(shimmed.status).toBe("at-risk"); expect(injectedRoutingRestartWarningLines(shimmed).length).toBeGreaterThan(0); + // ...and the warning must stay true in that case: a healthy shim DOES restart the proxy, for + // CLI launches. Claiming nothing will would contradict the summary line printed beneath it. + expect(injectedRoutingRestartWarningLines(shimmed).join(" ")).not.toContain("nothing here will restart"); + expect(injectedRoutingRestartWarningLines(shimmed).join(" ")).toContain(startupHealthSummary(shimmed)); // Native routing has no opencodex restart dependency, so there is nothing to warn about. expect(injectedRoutingRestartWarningLines(deriveStartupHealth({ ...base, routingKind: "native" }))).toEqual([]);