fix(codex): close the silent paths into a Codex lockout

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.

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",
@@ -1018,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",

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<void> {
         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`);

src/cli/dispatch.ts

@@ -615,7 +615,11 @@ const commandRunners: Record<string, CommandRunner> = {
         const guiUrl = selectDefaultGuiUrl(config, live, deps.probeHostname);
         console.log(`Opening ${guiUrl}`);
         const { openUrl } = await import("../lib/open-url");
-        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;
       },
     });

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,12 @@ 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).
+  // 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 {

src/cli/init.ts

@@ -253,6 +253,14 @@ export async function runInit(): Promise<void> {
     }
 
     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) {

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<CliStatusView> {
   const configDiagnostics = readConfigDiagnostics();
   const config = configDiagnostics.config;

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 } : {}),

src/codex/autostart-health.ts

@@ -154,6 +154,34 @@ 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 [
+    // 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.",
+  ];
+}
+
 function classifyStartupHealthSummary(health: StartupHealth): string {
   if (health.status === "native") return health.routingKind === "custom-remote"
     ? "custom remote Codex routing (no local restart dependency)"

src/codex/inject/config-toml.ts

@@ -609,3 +609,32 @@ 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.
+ *
+ * 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);
+  if (!existing) return null;
+  const resolved = resolveCodexConfigPath(existing);
+  if (!isOpencodexCatalogPath(resolved)) return null;
+  return existsSync(resolved) ? null : existing;
+}