From 3e9b7483ed8cb9564c49542bcfef86405e676101 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:39:25 +0800 Subject: [PATCH 1/5] fix(codex): stop reporting every history failure as a Codex DB lock The history Worker collapsed every non-converged outcome - a genuine SQLite busy, a permission denial, an unsafe coordinator path, an unavailable coordinator database, or a dead/timed-out Worker - into one message blaming the Codex app/IDE for holding state_5.sqlite. On Windows that reads as a false lock when nothing holds the file (issue #1191), and it made every other failure undiagnosable. - The H/K/N lock modules now compare the requested lock path against its realpath case-insensitively on Windows (samePathIdentity), matching the case-insensitive path identity Windows actually has, instead of refusing legitimate spellings as unsafe-path. - The Windows runtime root is canonicalized via realpath after creation, so differently-cased or junctioned LocalAppData spellings land on one lock namespace. - restoreLegacyOpenaiHistory now reports its failure reason (busy or permission) instead of a bare failed flag, so recover-history gets the same honest classification. - Apply, restore, and recover-history messages now distinguish genuine lock/busy from unsafe-path, unavailable coordinator DB, permission denial, worker error (with the real message), worker death, and worker timeout, each pointing at 'ocx doctor' when the app is not the cause. --- src/cli/index.ts | 12 +++- src/codex/catalog-write-serialization.ts | 3 +- src/codex/history-job.ts | 54 +++++++++++++++ src/codex/history-lock.ts | 3 +- src/codex/history-provider.ts | 7 +- src/codex/inject.ts | 86 +++++++++++++++++++++--- src/codex/transition-state.ts | 3 +- src/codex/user-identity.ts | 30 ++++++++- tests/codex-history-job.test.ts | 54 +++++++++++++++ tests/codex-history-provider.test.ts | 16 +++++ tests/codex-history-worker.test.ts | 26 ++++++- tests/codex-user-identity.test.ts | 10 +++ 12 files changed, 284 insertions(+), 20 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index 5a995987dd..b2d7631f1f 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -2,7 +2,12 @@ import { spawn } from "node:child_process"; import { currentExternalCodexModelProvider, restoreNativeCodex, restoreNativeCodexAsync, shouldInjectApiAuthHeader } from "../codex/inject"; import { stripGrokConfig } from "../grok/inject"; -import { resolveCodexHistoryJobTarget, runCodexHistoryJob } from "../codex/history-job"; +import { + describeHistoryJobFailure, + resolveCodexHistoryJobTarget, + runCodexHistoryJob, + type CodexHistoryJobOutcome, +} from "../codex/history-job"; import { reconcileJournal } from "../codex/journal"; import { codexAutoStartEnabled, @@ -899,7 +904,10 @@ async function handleRecoverHistory() { : { rows: 0, files: 0, failed: true as const }; if (r.failed) { console.error( - "⚠️ Recovery SKIPPED: the Codex history DB is locked (Codex app/IDE open?). Close it and rerun this command.", + `⚠️ Recovery SKIPPED: ${describeHistoryJobFailure( + outcome as Extract, + "recover-legacy", + )}`, ); process.exit(1); } diff --git a/src/codex/catalog-write-serialization.ts b/src/codex/catalog-write-serialization.ts index fef541e03f..92f8e7f949 100644 --- a/src/codex/catalog-write-serialization.ts +++ b/src/codex/catalog-write-serialization.ts @@ -32,6 +32,7 @@ import { CodexUserIdentityRefusal, resolveCodexCatalogSerializationDatabasePath, resolveEffectiveUserIdentity, + samePathIdentity, } from "./user-identity"; /** @@ -180,7 +181,7 @@ export function withCatalogWriteSerialization( } const opened = lstatSync(databasePath); if (opened.isSymbolicLink() || !opened.isFile() - || realpathSync.native(databasePath) !== databasePath) { + || !samePathIdentity(realpathSync.native(databasePath), databasePath)) { return { kind: "unavailable", reason: "unsafe-path" }; } diff --git a/src/codex/history-job.ts b/src/codex/history-job.ts index fe0c159629..ac20dcda7f 100644 --- a/src/codex/history-job.ts +++ b/src/codex/history-job.ts @@ -135,6 +135,60 @@ export function deriveCodexHistoryOperation(intent: { return intent.legacyMode ? "apply-opencodex" : "migrate-openai"; } +/** + * The honest failure clause for one history job outcome. + * + * The caller adds its own framing ("sync SKIPPED", "could NOT be restored"). + * The point of the surface argument is that a genuine lock keeps today's + * actionable wording, while every other reason stops blaming the Codex app: + * an unsafe-path refusal, an unavailable coordinator database, a permission + * denial, or a dead worker is a different problem with a different remedy. + */ +export function describeHistoryJobFailure( + outcome: Extract, + surface: "apply" | "restore" | "recover-legacy", + legacyMode = false, +): string { + if (outcome.kind === "blocked") { + switch (outcome.reason) { + case "busy": + if (surface === "apply") { + return legacyMode + ? "the history DB is locked (Codex app/IDE open?). Close it and rerun 'ocx start'." + : "the history DB is locked (Codex app/IDE open?). It is retried automatically (while the proxy runs and on every 'ocx start'); to force it now, close the Codex app and run 'ocx sync'."; + } + if (surface === "recover-legacy") { + return "the Codex history DB is locked (Codex app/IDE open?). Close it and rerun this command."; + } + return "the Codex app appears to be holding the history database. Close Codex and run `ocx restore` again."; + case "unsafe-path": + return "opencodex refused its history lock path (unsafe coordinator namespace); this is not a Codex app lock. Run 'ocx doctor' and check the opencodex runtime directory."; + case "database": + return "the history coordinator database is unavailable; this is not a Codex app lock. Run 'ocx doctor'."; + case "desired_disabled": + return "Codex integration is disabled, so the history operation was skipped."; + case "desired_enabled": + return "Codex integration is enabled, so the history operation was skipped."; + } + } + if (outcome.historyFailureReason === "busy") { + return surface === "restore" + ? "the Codex app appears to be holding the history database. Close Codex and run `ocx restore` again." + : "the history DB is locked (Codex app/IDE open?)."; + } + if (outcome.historyFailureReason === "permission") { + return "permission was denied while writing Codex history; this is not a Codex app lock. Run 'ocx doctor'."; + } + switch (outcome.reason) { + case "worker-error": + return `the history worker failed (${outcome.message}). Run 'ocx doctor'.`; + case "worker-died": + return "the history worker exited unexpectedly; this is not a Codex app lock. Run 'ocx doctor'."; + case "timeout": + return "the history worker timed out; this is not a Codex app lock. Run 'ocx doctor'."; + } +} + function classifyWorkerResult(result: HistoryWorkerResult): CodexHistoryJobOutcome { if (result.type === "blocked") return { kind: "blocked", reason: result.reason }; if (result.type === "error") { diff --git a/src/codex/history-lock.ts b/src/codex/history-lock.ts index 8418a878b6..ffbb5e8ef8 100644 --- a/src/codex/history-lock.ts +++ b/src/codex/history-lock.ts @@ -37,6 +37,7 @@ import { CodexUserIdentityRefusal, resolveCodexHistorySerializationDatabasePath, resolveEffectiveUserIdentity, + samePathIdentity, } from "./user-identity"; /** @@ -183,7 +184,7 @@ export function withHistoryWriteSerialization( } const opened = lstatSync(databasePath); if (opened.isSymbolicLink() || !opened.isFile() - || realpathSync.native(databasePath) !== databasePath) { + || !samePathIdentity(realpathSync.native(databasePath), databasePath)) { return { kind: "unavailable", reason: "unsafe-path" }; } diff --git a/src/codex/history-provider.ts b/src/codex/history-provider.ts index 46a21e9807..9a2ced85fe 100644 --- a/src/codex/history-provider.ts +++ b/src/codex/history-provider.ts @@ -722,16 +722,17 @@ function restoreCodexHistoryProvider(stateDbPath: string, backupPath: string): C } } -export function restoreLegacyOpenaiHistory(stateDbPath = STATE_DB_PATH): { rows: number; files: number; failed?: true } { +export function restoreLegacyOpenaiHistory(stateDbPath = STATE_DB_PATH): CodexHistorySyncResult { if (!existsSync(stateDbPath)) return { rows: 0, files: 0 }; - return withHistoryRetry(() => { + const retried = withHistoryRetryResult(() => { const db = openStateDb(stateDbPath); try { return ejectRemainingOpencodexHistory(db); } finally { db.close(); } - }) ?? { rows: 0, files: 0, failed: true }; + }); + return retried.ok ? retried.value : { rows: 0, files: 0, failed: true, failureReason: retried.reason }; } /** diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 20d5708707..0930e592ce 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -38,9 +38,11 @@ import { withCatalogWriteSerialization } from "./catalog-write-serialization"; import { restoreCodexCatalogWithPermit } from "./catalog/sync"; import { syncCodexHistoryProvider, type CodexHistoryFailureReason } from "./history-provider"; import { + describeHistoryJobFailure, deriveCodexHistoryOperation, resolveCodexHistoryJobTarget, runCodexHistoryJob, + type CodexHistoryJobOutcome, } from "./history-job"; import { OCX_SECTION_MARKER, @@ -1034,11 +1036,7 @@ export async function injectCodexConfig( config?.syncResumeHistory === false ? ` Codex resume history: left unchanged (syncResumeHistory=false).\n` : history.failed - ? legacyMode - ? ` ⚠️ Codex resume history sync SKIPPED: the history DB is locked (Codex app/IDE open?). Close it and rerun 'ocx start'.\n` - : // Honest in every caller context: the daemon retries in the background while it runs, - // and this inject path re-runs the migration on every future start/sync anyway. - ` ⚠️ Codex resume history migration deferred: the history DB is locked (Codex app/IDE open?). It is retried automatically (while the proxy runs and on every 'ocx start'); to force it now, close the Codex app and run 'ocx sync'.\n` + ? formatApplyHistoryFailure(historyOutcome, legacyMode) : legacyMode ? ` Codex resume history: ${history.rows} thread(s) made visible for opencodex; originals backed up for restore.\n` : migratedRows > 0 @@ -1250,7 +1248,7 @@ export interface CodexNativeRestoreResult { }; } -function failedHistoryRestore(reason?: CodexHistoryFailureReason): CodexRestoreHistoryResult { +function failedHistoryRestore(reason?: CodexHistoryFailureReason, detail?: string): CodexRestoreHistoryResult { return { state: "failed", changed: false, @@ -1260,10 +1258,62 @@ function failedHistoryRestore(reason?: CodexHistoryFailureReason): CodexRestoreH ejectedRows: 0, message: reason === "permission" ? "Codex resume history could NOT be restored because permission was denied." - : "Codex resume history could NOT be restored — the Codex app appears to be holding the history database.", + : reason === "busy" + ? "Codex resume history could NOT be restored — the Codex app appears to be holding the history database." + : detail + ? `Codex resume history could NOT be restored: ${detail}` + : "Codex resume history could NOT be restored — the Codex app appears to be holding the history database.", }; } +/** + * Restore failure wording for a Worker outcome. + * + * Only a genuine busy result blames the Codex app. An unsafe-path refusal, an + * unavailable coordinator database, a permission denial, or a dead/timed-out + * worker is a different problem; the old collapse made every one of those read + * as "the Codex app is holding the database" (issue #1191). + */ +function failedHistoryRestoreFromOutcome( + outcome: Extract, +): CodexRestoreHistoryResult { + if (outcome.kind === "blocked") { + switch (outcome.reason) { + case "busy": + return failedHistoryRestore("busy"); + case "unsafe-path": + return failedHistoryRestore( + undefined, + "opencodex refused its history lock path (unsafe coordinator namespace); this is not a Codex app lock. Run 'ocx doctor' and check the opencodex runtime directory.", + ); + case "database": + return failedHistoryRestore( + undefined, + "the history coordinator database is unavailable; this is not a Codex app lock. Run 'ocx doctor'.", + ); + case "desired_disabled": + case "desired_enabled": + return failedHistoryRestore(); + } + } + if (outcome.historyFailureReason === "busy") return failedHistoryRestore("busy"); + if (outcome.historyFailureReason === "permission") return failedHistoryRestore("permission"); + switch (outcome.reason) { + case "worker-error": + return failedHistoryRestore(undefined, `the history worker failed (${outcome.message}). Run 'ocx doctor'.`); + case "worker-died": + return failedHistoryRestore( + undefined, + "the history worker exited unexpectedly; this is not a Codex app lock. Run 'ocx doctor'.", + ); + case "timeout": + return failedHistoryRestore( + undefined, + "the history worker timed out; this is not a Codex app lock. Run 'ocx doctor'.", + ); + } +} + function externalProviderRestoreResult(activeProvider: string): CodexNativeRestoreResult { const message = `External Codex provider ${tomlString(activeProvider)} preserved; no native restore was needed.`; return { @@ -1499,10 +1549,10 @@ export async function restoreNativeCodexAsync( ? "Codex integration was disabled; history restoration was skipped." : "Codex integration was enabled; history restoration was skipped.", } - : outcome.kind === "blocked" && outcome.reason === "busy" - ? failedHistoryRestore("busy") + : outcome.kind === "blocked" + ? failedHistoryRestoreFromOutcome(outcome) : outcome.kind === "failed" - ? failedHistoryRestore(outcome.historyFailureReason) + ? failedHistoryRestoreFromOutcome(outcome) : failedHistoryRestore(); const base = catalog.removed > 0 ? `${config.message} Catalog restored to ${catalog.kept} native model(s) (dropped ${catalog.removed} proxy-routed).` @@ -1572,3 +1622,19 @@ export function restoreNativeCodex(options: { skipHistory?: boolean; revalidateD export function getCodexConfigPath(): string { return CODEX_CONFIG_PATH; } + +/** + * Frame one failed apply history job honestly. + * + * A genuine lock keeps the established deferred/SKIPPED wording; any other + * reason names itself instead of blaming the Codex app/IDE. + */ +function formatApplyHistoryFailure(outcome: CodexHistoryJobOutcome, legacyMode: boolean): string { + const failure = outcome as Extract; + const headline = legacyMode + ? "Codex resume history sync SKIPPED" + : outcome.kind === "blocked" && outcome.reason === "busy" + ? "Codex resume history migration deferred" + : "Codex resume history NOT changed"; + return ` ⚠️ ${headline}: ${describeHistoryJobFailure(failure, "apply", legacyMode)}\n`; +} diff --git a/src/codex/transition-state.ts b/src/codex/transition-state.ts index c12115a152..27ce605530 100644 --- a/src/codex/transition-state.ts +++ b/src/codex/transition-state.ts @@ -34,6 +34,7 @@ import { CodexUserIdentityRefusal, resolveCodexCoordinatorDatabasePath, resolveEffectiveUserIdentity, + samePathIdentity, } from "./user-identity"; const COORDINATOR_SCHEMA_VERSION = 1; @@ -440,7 +441,7 @@ export function openCodexCoordinatorTransaction(finalDatabasePath: string): Code const entry = lstatSync(finalDatabasePath); if (entry.isSymbolicLink() || !entry.isFile() || `${entry.dev}:${entry.ino}` !== initialIdentity - || realpathSync.native(finalDatabasePath) !== finalDatabasePath) { + || !samePathIdentity(realpathSync.native(finalDatabasePath), finalDatabasePath)) { throw new CodexUserIdentityRefusal("The coordinator database path was substituted."); } }; diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index dade7ef06b..7ef859a327 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -160,7 +160,35 @@ function resolveWindowsRuntimeRoot(identity: Extract { + const busy = { kind: "blocked", reason: "busy" } as const; + expect(describeHistoryJobFailure(busy, "apply", false)).toContain("history DB is locked"); + expect(describeHistoryJobFailure(busy, "apply", true)).toContain("Close it and rerun 'ocx start'"); + expect(describeHistoryJobFailure(busy, "restore")).toContain("holding the history database"); + expect(describeHistoryJobFailure(busy, "recover-legacy")).toContain("Close it and rerun this command"); + + const unsafe = { kind: "blocked", reason: "unsafe-path" } as const; + const unsafeText = describeHistoryJobFailure(unsafe, "apply"); + expect(unsafeText).toContain("not a Codex app lock"); + expect(unsafeText).toContain("'ocx doctor'"); + + const database = { kind: "blocked", reason: "database" } as const; + expect(describeHistoryJobFailure(database, "restore")).toContain("coordinator database is unavailable"); + + const permission = { + kind: "failed", + reason: "worker-error", + message: "history_transition_failed", + historyFailureReason: "permission", + } as const; + expect(describeHistoryJobFailure(permission, "apply")).toContain("permission was denied"); + expect(describeHistoryJobFailure(permission, "apply")).toContain("'ocx doctor'"); + + const workerError = { kind: "failed", reason: "worker-error", message: "unable to open database file" } as const; + expect(describeHistoryJobFailure(workerError, "apply")).toContain("unable to open database file"); + expect(describeHistoryJobFailure(workerError, "apply")).toContain("'ocx doctor'"); + + const died = { kind: "failed", reason: "worker-died", message: "history_worker_closed_early" } as const; + expect(describeHistoryJobFailure(died, "apply")).toContain("exited unexpectedly"); + + const timeout = { kind: "failed", reason: "timeout", message: "history_worker_timeout" } as const; + expect(describeHistoryJobFailure(timeout, "restore")).toContain("timed out"); +}); + test("skip resolves without spawning a thread and writes nothing", async () => { const fixture = makeFixture("ocx-history-job-skip-"); @@ -134,6 +170,24 @@ test("an overrun Worker returns a typed timeout rather than hanging", async () = expect(Date.now() - started).toBeLessThan(20_000); }, 30_000); +/** + * The false-lock regression (issue #1191) hid every non-busy failure behind + * "the Codex app is holding the DB". A hard error must reach the caller with + * its real message so the diagnosis is possible at all. + */ +test("a hard history error reaches the caller with its real message", async () => { + const fixture = makeFixture("ocx-history-job-hard-error-"); + // A directory at the state-DB path cannot be opened as SQLite. + rmSync(fixture.canonicalStateDbPath, { force: true }); + mkdirSync(fixture.canonicalStateDbPath); + + const outcome = await runCodexHistoryJob({ ...fixture, operation: "recover-legacy-openai" }); + expect(outcome.kind).toBe("failed"); + if (outcome.kind === "failed") { + expect(outcome.message).toMatch(/unable to open|not a database|cannot open/i); + } +}, 30_000); + /** * The async restore wrapper owns history; the synchronous body must not also do * it when told to stand down, or every restore would run the transition twice — diff --git a/tests/codex-history-provider.test.ts b/tests/codex-history-provider.test.ts index 856a68da68..ab6679f9ce 100644 --- a/tests/codex-history-provider.test.ts +++ b/tests/codex-history-provider.test.ts @@ -334,6 +334,22 @@ describe("history lock retry", () => { expect(calls).toBe(2); }); + test("syncCodexHistoryProvider reports why the retry budget died", () => { + // A pending opencodex row makes the eject path actually write; with no rows + // the restore transaction never starts and nothing contends. + const fixture = makeFixture({ includeLegacy: true }); + const holder = new Database(fixture.dbPath); + holder.exec("BEGIN IMMEDIATE"); + try { + const result = syncCodexHistoryProvider("openai", fixture.dbPath, fixture.backupPath); + expect(result.failed).toBe(true); + expect(result.failureReason).toBe("busy"); + } finally { + holder.exec("ROLLBACK"); + holder.close(); + } + }); + test("withHistoryRetry rethrows hard errors immediately", () => { let calls = 0; expect(() => diff --git a/tests/codex-history-worker.test.ts b/tests/codex-history-worker.test.ts index eb76fa1cd5..283275dff0 100644 --- a/tests/codex-history-worker.test.ts +++ b/tests/codex-history-worker.test.ts @@ -1,16 +1,22 @@ -import { afterEach, expect, test } from "bun:test"; +import { afterEach, expect, setDefaultTimeout, test } from "bun:test"; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { Database } from "bun:sqlite"; +import { setHistoryDbBusyTimeoutForTests } from "../src/codex/history-provider"; import { isHistoryWorkerRunMessage, runHistoryUnitUnderLock, type HistoryWorkerRunMessage, } from "../src/codex/history-worker"; +// A held write lock otherwise costs the full production 5s busy timeout per +// attempt, tripping bun's 5s default per-test timeout. +setHistoryDbBusyTimeoutForTests(250); +setDefaultTimeout(30_000); + const repoRoot = resolve(import.meta.dir, ".."); const sandboxes: string[] = []; @@ -191,3 +197,21 @@ test("a second holder of H makes the unit report blocked rather than wait", asyn expect(await holder.exited).toBe(0); } }, 30_000); + +/** + * The reason the parent can tell a false "app holds the DB" from a real one: + * a transition that survives retries reports WHY it failed, not a fixed code + * that reads as "locked" everywhere. + */ +test("a failed transition reports the failure reason, not a fixed lock claim", () => { + const fixture = makeFixture("ocx-history-worker-error-"); + const holder = new Database(fixture.stateDb); + holder.exec("BEGIN IMMEDIATE"); + try { + const result = runHistoryUnitUnderLock(runMessage(fixture, { operation: "recover-legacy-openai" })); + expect(result).toMatchObject({ type: "error", reason: "busy" }); + } finally { + holder.exec("ROLLBACK"); + holder.close(); + } +}); diff --git a/tests/codex-user-identity.test.ts b/tests/codex-user-identity.test.ts index fb7f100313..ba24ed435d 100644 --- a/tests/codex-user-identity.test.ts +++ b/tests/codex-user-identity.test.ts @@ -9,6 +9,7 @@ import { resolveCodexCatalogSerializationDatabasePath, resolveCodexHistorySerializationDatabasePath, resolveEffectiveUserIdentity, + samePathIdentity, } from "../src/codex/user-identity"; let codexHome = ""; @@ -67,6 +68,15 @@ beforeEach(() => { codexHome = mkdtempSync(join(tmpdir(), "ocx-user-identity-codex-home-")); }); +test("samePathIdentity is case-insensitive on Windows and exact elsewhere", () => { + const winPath = "C:\\Users\\Alice\\AppData\\Local\\OpenCodex\\Runtime\\v1\\S-1-5-21\\history-write-locks\\abc.sqlite"; + expect(samePathIdentity(winPath, winPath.toLowerCase(), "win32")).toBe(true); + expect(samePathIdentity(winPath, winPath.toLowerCase(), "linux")).toBe(false); + expect(samePathIdentity(winPath, "D:\\Users\\Alice\\AppData\\Local\\OpenCodex\\Runtime\\v1\\S-1-5-21\\history-write-locks\\abc.sqlite", "win32")).toBe(false); + expect(samePathIdentity("/tmp/a/b.sqlite", "/tmp/a/b.sqlite", "linux")).toBe(true); + expect(samePathIdentity("/tmp/a/b.sqlite", "/tmp/A/b.sqlite", "linux")).toBe(false); +}); + afterEach(() => { if (previousHome === undefined) delete process.env.HOME; else process.env.HOME = previousHome; From d2c4c25229f25353e8e113414e5a5f1c3bfbfd70 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:26:37 +0800 Subject: [PATCH 2/5] fix(codex): close the undefined and false-lock fallback gaps in failure wording --- src/cli/index.ts | 6 +----- src/codex/history-job.ts | 11 ++++++++++- src/codex/inject.ts | 14 +++++++++++--- tests/codex-history-job.test.ts | 7 +++++++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index b2d7631f1f..1eb93afbc4 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -6,7 +6,6 @@ import { describeHistoryJobFailure, resolveCodexHistoryJobTarget, runCodexHistoryJob, - type CodexHistoryJobOutcome, } from "../codex/history-job"; import { reconcileJournal } from "../codex/journal"; import { @@ -904,10 +903,7 @@ async function handleRecoverHistory() { : { rows: 0, files: 0, failed: true as const }; if (r.failed) { console.error( - `⚠️ Recovery SKIPPED: ${describeHistoryJobFailure( - outcome as Extract, - "recover-legacy", - )}`, + `⚠️ Recovery SKIPPED: ${describeHistoryJobFailure(outcome, "recover-legacy")}`, ); process.exit(1); } diff --git a/src/codex/history-job.ts b/src/codex/history-job.ts index ac20dcda7f..7b09f05a53 100644 --- a/src/codex/history-job.ts +++ b/src/codex/history-job.ts @@ -145,10 +145,19 @@ export function deriveCodexHistoryOperation(intent: { * denial, or a dead worker is a different problem with a different remedy. */ export function describeHistoryJobFailure( - outcome: Extract, + outcome: CodexHistoryJobOutcome, surface: "apply" | "restore" | "recover-legacy", legacyMode = false, ): string { + // Callers only invoke this after observing a failure flag, but that flag is + // derived from "not converged", which also covers "skipped". Naming those + // two kinds keeps a widened or miscast call site from printing `undefined`. + if (outcome.kind === "skipped") { + return "the history operation was skipped; no failure was recorded."; + } + if (outcome.kind === "converged") { + return "the history job reported no failure; run 'ocx doctor' if this is unexpected."; + } if (outcome.kind === "blocked") { switch (outcome.reason) { case "busy": diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 0930e592ce..ee0394220b 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -1293,7 +1293,16 @@ function failedHistoryRestoreFromOutcome( ); case "desired_disabled": case "desired_enabled": - return failedHistoryRestore(); + // Unreachable today: the async restore caller intercepts both desired- + // state reasons before this mapper runs. If that ever changes, name + // the actual cause instead of falling back to the lock wording this + // function exists to get rid of (issue #1191). + return failedHistoryRestore( + undefined, + outcome.reason === "desired_disabled" + ? "Codex integration is disabled, so the history operation was skipped." + : "Codex integration is enabled, so the history operation was skipped.", + ); } } if (outcome.historyFailureReason === "busy") return failedHistoryRestore("busy"); @@ -1630,11 +1639,10 @@ export function getCodexConfigPath(): string { * reason names itself instead of blaming the Codex app/IDE. */ function formatApplyHistoryFailure(outcome: CodexHistoryJobOutcome, legacyMode: boolean): string { - const failure = outcome as Extract; const headline = legacyMode ? "Codex resume history sync SKIPPED" : outcome.kind === "blocked" && outcome.reason === "busy" ? "Codex resume history migration deferred" : "Codex resume history NOT changed"; - return ` ⚠️ ${headline}: ${describeHistoryJobFailure(failure, "apply", legacyMode)}\n`; + return ` ⚠️ ${headline}: ${describeHistoryJobFailure(outcome, "apply", legacyMode)}\n`; } diff --git a/tests/codex-history-job.test.ts b/tests/codex-history-job.test.ts index d0c45162c0..060fd86c46 100644 --- a/tests/codex-history-job.test.ts +++ b/tests/codex-history-job.test.ts @@ -114,6 +114,13 @@ test("the failure wording names the real reason instead of always blaming the Co const timeout = { kind: "failed", reason: "timeout", message: "history_worker_timeout" } as const; expect(describeHistoryJobFailure(timeout, "restore")).toContain("timed out"); + + // Callers flag failure as "not converged", which also covers these two + // kinds; the wording must name them rather than returning undefined. + const skipped = { kind: "skipped" } as const; + expect(describeHistoryJobFailure(skipped, "recover-legacy")).toContain("skipped"); + const converged = { kind: "converged", rows: 0, files: 0 } as const; + expect(describeHistoryJobFailure(converged, "apply")).toContain("no failure"); }); test("skip resolves without spawning a thread and writes nothing", async () => { From 7dda9028ad38ec11b90ecfc433298115a32b829d Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:21:45 +0800 Subject: [PATCH 3/5] fix(codex): address review findings on failure wording, path redaction, junctions --- src/cli/doctor.ts | 18 ++++++++++++++ src/codex/history-job.ts | 42 ++++++++++++++++++++------------- src/codex/inject.ts | 9 ++++++- src/codex/user-identity.ts | 12 +++++++++- tests/codex-history-job.test.ts | 14 +++++++++++ 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 29b8012a71..1840dbc003 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -25,6 +25,12 @@ import { NativeProfileError } from "../codex/native-profile-types"; import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHomeDirImpl, isWslRuntime, listWslWindowsCodexHomes, wslAutomountRoot, type CodexHomeDeps } from "../codex/home"; import { findCodexOnPath, isWindowsInteropDir } from "../codex/shim"; import { countPendingOpencodexHistory } from "../codex/history-provider"; +import { getCodexHome } from "../codex/paths"; +import { + CodexUserIdentityRefusal, + resolveCodexHistorySerializationDatabasePath, + resolveEffectiveUserIdentity, +} from "../codex/user-identity"; import { collectProjectCodexConfigWarnings, formatProjectCodexConfigWarningsForDoctor } from "../codex/project-config-warnings"; import { collectStartupHealth, startupHealthSummary } from "../codex/autostart-health"; import { @@ -902,6 +908,18 @@ export async function runDoctor(args: string[] = []): Promise { // Codex app until the one-time migration lands. Read-only probe (readonly sqlite, 100ms // busy timeout) — reports state, never mutates. console.log("\nCodex history migration"); + // The history failure messages point here; make the visit worthwhile by + // probing the coordinator namespace the locks live in. Resolution exercises + // identity, runtime-root, and permission checks without taking any lock. + try { + const identity = resolveEffectiveUserIdentity(); + const codexHome = getCodexHome(); + resolveCodexHistorySerializationDatabasePath(identity, codexHome, join(codexHome, "state_5.sqlite")); + console.log(" ok history coordinator namespace resolves"); + } catch (cause) { + const reason = cause instanceof CodexUserIdentityRefusal ? cause.message : String(cause); + console.log(` -- history coordinator namespace refused: ${reason}`); + } const pending = countPendingOpencodexHistory(); if (pending.failed) { console.log(" -- state DB locked or unreadable (Codex app open?) — migration state unknown"); diff --git a/src/codex/history-job.ts b/src/codex/history-job.ts index 7b09f05a53..3f707b97a5 100644 --- a/src/codex/history-job.ts +++ b/src/codex/history-job.ts @@ -17,6 +17,7 @@ * Design record: devlog/_fin/260804_codex_write_substrate/020_history_isolation.md. */ import { randomUUID } from "node:crypto"; +import { homedir } from "node:os"; import { join } from "node:path"; import type { @@ -158,18 +159,20 @@ export function describeHistoryJobFailure( if (outcome.kind === "converged") { return "the history job reported no failure; run 'ocx doctor' if this is unexpected."; } + // A busy database reaches here two ways: the lock itself was contended + // (blocked/busy), or the lock was acquired and the worker then found SQLite + // busy (failed with historyFailureReason "busy"). Both are the same user + // situation and deserve the same surface-specific guidance. + const busyText = surface === "apply" + ? legacyMode + ? "the history DB is locked (Codex app/IDE open?). Close it and rerun 'ocx start'." + : "the history DB is locked (Codex app/IDE open?). It is retried automatically (while the proxy runs and on every 'ocx start'); to force it now, close the Codex app and run 'ocx sync'." + : surface === "recover-legacy" + ? "the Codex history DB is locked (Codex app/IDE open?). Close it and rerun this command." + : "the Codex app appears to be holding the history database. Close Codex and run `ocx restore` again."; if (outcome.kind === "blocked") { + if (outcome.reason === "busy") return busyText; switch (outcome.reason) { - case "busy": - if (surface === "apply") { - return legacyMode - ? "the history DB is locked (Codex app/IDE open?). Close it and rerun 'ocx start'." - : "the history DB is locked (Codex app/IDE open?). It is retried automatically (while the proxy runs and on every 'ocx start'); to force it now, close the Codex app and run 'ocx sync'."; - } - if (surface === "recover-legacy") { - return "the Codex history DB is locked (Codex app/IDE open?). Close it and rerun this command."; - } - return "the Codex app appears to be holding the history database. Close Codex and run `ocx restore` again."; case "unsafe-path": return "opencodex refused its history lock path (unsafe coordinator namespace); this is not a Codex app lock. Run 'ocx doctor' and check the opencodex runtime directory."; case "database": @@ -180,11 +183,7 @@ export function describeHistoryJobFailure( return "Codex integration is enabled, so the history operation was skipped."; } } - if (outcome.historyFailureReason === "busy") { - return surface === "restore" - ? "the Codex app appears to be holding the history database. Close Codex and run `ocx restore` again." - : "the history DB is locked (Codex app/IDE open?)."; - } + if (outcome.historyFailureReason === "busy") return busyText; if (outcome.historyFailureReason === "permission") { return "permission was denied while writing Codex history; this is not a Codex app lock. Run 'ocx doctor'."; } @@ -198,13 +197,24 @@ export function describeHistoryJobFailure( } } +/** + * Worker exceptions travel into user-facing CLI output, and a raw filesystem + * error carries absolute paths — on every platform that includes the account + * name (`/Users/x`, `/home/x`, `C:\Users\x`). Folding the home directory to + * `~` keeps the diagnostic value and drops the identifier. + */ +function redactWorkerMessage(message: string): string { + const home = homedir(); + return home.length > 1 ? message.split(home).join("~") : message; +} + function classifyWorkerResult(result: HistoryWorkerResult): CodexHistoryJobOutcome { if (result.type === "blocked") return { kind: "blocked", reason: result.reason }; if (result.type === "error") { return { kind: "failed", reason: "worker-error", - message: result.message, + message: redactWorkerMessage(result.message), ...(result.reason ? { historyFailureReason: result.reason } : {}), }; } diff --git a/src/codex/inject.ts b/src/codex/inject.ts index ee0394220b..7c0b8c95f1 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -1639,9 +1639,16 @@ export function getCodexConfigPath(): string { * reason names itself instead of blaming the Codex app/IDE. */ function formatApplyHistoryFailure(outcome: CodexHistoryJobOutcome, legacyMode: boolean): string { + // A busy database is a deferral no matter which half observed it: the lock + // contended (blocked/busy), or the worker acquired the lock and then found + // SQLite busy (failed with a busy history reason). Only those keep the + // deferred headline; every other failure is a real "NOT changed". + const busy = + (outcome.kind === "blocked" && outcome.reason === "busy") || + (outcome.kind === "failed" && outcome.historyFailureReason === "busy"); const headline = legacyMode ? "Codex resume history sync SKIPPED" - : outcome.kind === "blocked" && outcome.reason === "busy" + : busy ? "Codex resume history migration deferred" : "Codex resume history NOT changed"; return ` ⚠️ ${headline}: ${describeHistoryJobFailure(outcome, "apply", legacyMode)}\n`; diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index 7ef859a327..aeaaad41b6 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -165,9 +165,19 @@ function resolveWindowsRuntimeRoot(identity: Extract Date: Sat, 8 Aug 2026 12:19:20 +0800 Subject: [PATCH 4/5] fix(codex): keep doctor probe read-only and dedupe failure wording --- src/cli/doctor.ts | 17 ++--- src/codex/history-job.ts | 9 ++- src/codex/inject.ts | 65 +++++-------------- src/codex/user-identity.ts | 72 +++++++++++++++++++++- tests/codex-inject-history-wording.test.ts | 64 +++++++++++++++++++ tests/codex-user-identity.test.ts | 13 +++- 6 files changed, 179 insertions(+), 61 deletions(-) create mode 100644 tests/codex-inject-history-wording.test.ts diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 1840dbc003..2e12745b4c 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -25,10 +25,9 @@ import { NativeProfileError } from "../codex/native-profile-types"; import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHomeDirImpl, isWslRuntime, listWslWindowsCodexHomes, wslAutomountRoot, type CodexHomeDeps } from "../codex/home"; import { findCodexOnPath, isWindowsInteropDir } from "../codex/shim"; import { countPendingOpencodexHistory } from "../codex/history-provider"; -import { getCodexHome } from "../codex/paths"; import { CodexUserIdentityRefusal, - resolveCodexHistorySerializationDatabasePath, + probeCodexCoordinatorNamespace, resolveEffectiveUserIdentity, } from "../codex/user-identity"; import { collectProjectCodexConfigWarnings, formatProjectCodexConfigWarningsForDoctor } from "../codex/project-config-warnings"; @@ -909,13 +908,17 @@ export async function runDoctor(args: string[] = []): Promise { // busy timeout) — reports state, never mutates. console.log("\nCodex history migration"); // The history failure messages point here; make the visit worthwhile by - // probing the coordinator namespace the locks live in. Resolution exercises - // identity, runtime-root, and permission checks without taking any lock. + // probing the coordinator namespace the locks live in. The probe exercises + // identity, runtime-root, and permission checks without taking any lock or + // creating anything (a doctor run must observe, not initialize). try { const identity = resolveEffectiveUserIdentity(); - const codexHome = getCodexHome(); - resolveCodexHistorySerializationDatabasePath(identity, codexHome, join(codexHome, "state_5.sqlite")); - console.log(" ok history coordinator namespace resolves"); + const probe = probeCodexCoordinatorNamespace(identity); + if (probe.status === "missing") { + console.log(" ok history coordinator namespace not created yet (no history operation has run)"); + } else { + console.log(" ok history coordinator namespace resolves"); + } } catch (cause) { const reason = cause instanceof CodexUserIdentityRefusal ? cause.message : String(cause); console.log(` -- history coordinator namespace refused: ${reason}`); diff --git a/src/codex/history-job.ts b/src/codex/history-job.ts index 3f707b97a5..2d35498daf 100644 --- a/src/codex/history-job.ts +++ b/src/codex/history-job.ts @@ -205,7 +205,14 @@ export function describeHistoryJobFailure( */ function redactWorkerMessage(message: string): string { const home = homedir(); - return home.length > 1 ? message.split(home).join("~") : message; + if (home.length <= 1) return message; + // Windows spellings vary in case and separator; an exact match would leave + // the account name in the message. + if (process.platform === "win32") { + const escaped = home.replace(/[.*+?^${}()|[\]\\]/g, "\\$&").replace(/\\\\/g, "[\\\\/]"); + return message.replace(new RegExp(escaped, "gi"), "~"); + } + return message.split(home).join("~"); } function classifyWorkerResult(result: HistoryWorkerResult): CodexHistoryJobOutcome { diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 7c0b8c95f1..6ab07183e3 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -1262,7 +1262,7 @@ function failedHistoryRestore(reason?: CodexHistoryFailureReason, detail?: strin ? "Codex resume history could NOT be restored — the Codex app appears to be holding the history database." : detail ? `Codex resume history could NOT be restored: ${detail}` - : "Codex resume history could NOT be restored — the Codex app appears to be holding the history database.", + : "Codex resume history could NOT be restored; the reason was not recorded. Run 'ocx doctor'.", }; } @@ -1272,55 +1272,20 @@ function failedHistoryRestore(reason?: CodexHistoryFailureReason, detail?: strin * Only a genuine busy result blames the Codex app. An unsafe-path refusal, an * unavailable coordinator database, a permission denial, or a dead/timed-out * worker is a different problem; the old collapse made every one of those read - * as "the Codex app is holding the database" (issue #1191). + * as "the Codex app is holding the database" (issue #1191). `busy` and + * `permission` keep the restore-specific sentence built by + * `failedHistoryRestore`; every other reason reuses the single formatter so + * the two modules cannot drift apart. */ -function failedHistoryRestoreFromOutcome( +export function failedHistoryRestoreFromOutcome( outcome: Extract, ): CodexRestoreHistoryResult { - if (outcome.kind === "blocked") { - switch (outcome.reason) { - case "busy": - return failedHistoryRestore("busy"); - case "unsafe-path": - return failedHistoryRestore( - undefined, - "opencodex refused its history lock path (unsafe coordinator namespace); this is not a Codex app lock. Run 'ocx doctor' and check the opencodex runtime directory.", - ); - case "database": - return failedHistoryRestore( - undefined, - "the history coordinator database is unavailable; this is not a Codex app lock. Run 'ocx doctor'.", - ); - case "desired_disabled": - case "desired_enabled": - // Unreachable today: the async restore caller intercepts both desired- - // state reasons before this mapper runs. If that ever changes, name - // the actual cause instead of falling back to the lock wording this - // function exists to get rid of (issue #1191). - return failedHistoryRestore( - undefined, - outcome.reason === "desired_disabled" - ? "Codex integration is disabled, so the history operation was skipped." - : "Codex integration is enabled, so the history operation was skipped.", - ); - } - } - if (outcome.historyFailureReason === "busy") return failedHistoryRestore("busy"); - if (outcome.historyFailureReason === "permission") return failedHistoryRestore("permission"); - switch (outcome.reason) { - case "worker-error": - return failedHistoryRestore(undefined, `the history worker failed (${outcome.message}). Run 'ocx doctor'.`); - case "worker-died": - return failedHistoryRestore( - undefined, - "the history worker exited unexpectedly; this is not a Codex app lock. Run 'ocx doctor'.", - ); - case "timeout": - return failedHistoryRestore( - undefined, - "the history worker timed out; this is not a Codex app lock. Run 'ocx doctor'.", - ); + if (outcome.kind === "blocked" && outcome.reason === "busy") return failedHistoryRestore("busy"); + if (outcome.kind === "failed" && outcome.historyFailureReason === "busy") return failedHistoryRestore("busy"); + if (outcome.kind === "failed" && outcome.historyFailureReason === "permission") { + return failedHistoryRestore("permission"); } + return failedHistoryRestore(undefined, describeHistoryJobFailure(outcome, "restore")); } function externalProviderRestoreResult(activeProvider: string): CodexNativeRestoreResult { @@ -1558,11 +1523,9 @@ export async function restoreNativeCodexAsync( ? "Codex integration was disabled; history restoration was skipped." : "Codex integration was enabled; history restoration was skipped.", } - : outcome.kind === "blocked" + : outcome.kind === "blocked" || outcome.kind === "failed" ? failedHistoryRestoreFromOutcome(outcome) - : outcome.kind === "failed" - ? failedHistoryRestoreFromOutcome(outcome) - : failedHistoryRestore(); + : failedHistoryRestore(); const base = catalog.removed > 0 ? `${config.message} Catalog restored to ${catalog.kept} native model(s) (dropped ${catalog.removed} proxy-routed).` : config.message; @@ -1638,7 +1601,7 @@ export function getCodexConfigPath(): string { * A genuine lock keeps the established deferred/SKIPPED wording; any other * reason names itself instead of blaming the Codex app/IDE. */ -function formatApplyHistoryFailure(outcome: CodexHistoryJobOutcome, legacyMode: boolean): string { +export function formatApplyHistoryFailure(outcome: CodexHistoryJobOutcome, legacyMode: boolean): string { // A busy database is a deferral no matter which half observed it: the lock // contended (blocked/busy), or the worker acquired the lock and then found // SQLite busy (failed with a busy history reason). Only those keep the diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index aeaaad41b6..072593a566 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -123,7 +123,7 @@ function ensurePrivatePosixDirectory(path: string, uid: number): void { assertPrivatePosixDirectory(path, uid); } -function resolvePosixRuntimeRoot(uid: number): string { +function resolveTrustedPosixTmp(): string { let realTmp: string; try { realTmp = realpathSync.native(POSIX_TMP_PATH); @@ -138,12 +138,82 @@ function resolvePosixRuntimeRoot(uid: number): string { if (cause instanceof CodexUserIdentityRefusal) throw cause; refuse("The system temporary directory cannot be trusted.", cause); } + return realTmp; +} +function resolvePosixRuntimeRoot(uid: number): string { + const realTmp = resolveTrustedPosixTmp(); const root = join(realTmp, `opencodex-runtime-v1-${uid}`); ensurePrivatePosixDirectory(root, uid); return root; } +export type CoordinatorNamespaceProbe = + | { readonly status: "ok"; readonly root: string } + | { readonly status: "missing" }; + +/** + * Read-only namespace probe for diagnostics (`ocx doctor`). + * + * Unlike the runtime resolvers, this never creates the root or the lock + * directories: a doctor run must observe the namespace, not initialize it. + * A missing namespace is reported as `missing` instead of refused, so a fresh + * machine does not read as a broken one; an existing but unsafe namespace is + * refused exactly like the creating path would refuse it. + */ +export function probeCodexCoordinatorNamespace(identity: UserIdentity): CoordinatorNamespaceProbe { + if (identity.platform === "posix") { + const root = join(resolveTrustedPosixTmp(), `opencodex-runtime-v1-${identity.uid}`); + let entry; + try { + entry = lstatSync(root); + } catch (cause) { + const code = cause && typeof cause === "object" && "code" in cause + ? String((cause as { code?: unknown }).code) + : ""; + if (code === "ENOENT") return { status: "missing" }; + refuse("The Codex coordinator namespace cannot be inspected.", cause); + } + if (entry.isSymbolicLink() || !entry.isDirectory()) { + refuse("The Codex coordinator namespace is not a real directory."); + } + if (entry.uid !== identity.uid || (entry.mode & 0o777) !== POSIX_PRIVATE_MODE) { + refuse("The Codex coordinator namespace has unsafe ownership or permissions."); + } + return { status: "ok", root }; + } + + if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); + const localAppData = powershellValue( + "[Environment]::GetFolderPath([Environment+SpecialFolder]::LocalApplicationData)", + ); + if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); + const root = resolve(localAppData, "OpenCodex", "Runtime", "v1", identity.sid.toUpperCase()); + let entry; + try { + entry = lstatSync(root); + } catch (cause) { + const code = cause && typeof cause === "object" && "code" in cause + ? String((cause as { code?: unknown }).code) + : ""; + if (code === "ENOENT") return { status: "missing" }; + refuse("The Windows coordinator namespace cannot be inspected.", cause); + } + if (entry.isSymbolicLink() || !entry.isDirectory()) { + refuse("The Windows coordinator namespace is not a real directory."); + } + try { + const real = realpathSync.native(root); + if (!samePathIdentity(real, root, "win32")) { + refuse("The Windows coordinator namespace is redirected by a junction or reparse point."); + } + return { status: "ok", root: real }; + } catch (cause) { + if (cause instanceof CodexUserIdentityRefusal) throw cause; + refuse("The Windows coordinator namespace cannot be resolved.", cause); + } +} + function resolveWindowsRuntimeRoot(identity: Extract): string { if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); const localAppData = powershellValue( diff --git a/tests/codex-inject-history-wording.test.ts b/tests/codex-inject-history-wording.test.ts new file mode 100644 index 0000000000..a42e8e308d --- /dev/null +++ b/tests/codex-inject-history-wording.test.ts @@ -0,0 +1,64 @@ +import { expect, test } from "bun:test"; + +import { + failedHistoryRestoreFromOutcome, + formatApplyHistoryFailure, +} from "../src/codex/inject"; + +test("apply keeps the deferred headline for busy outcomes from either half", () => { + const blockedBusy = { kind: "blocked", reason: "busy" } as const; + expect(formatApplyHistoryFailure(blockedBusy, false)).toContain("migration deferred"); + expect(formatApplyHistoryFailure(blockedBusy, false)).toContain("history DB is locked"); + + const workerBusy = { + kind: "failed", + reason: "worker-error", + message: "database is locked", + historyFailureReason: "busy", + } as const; + expect(formatApplyHistoryFailure(workerBusy, false)).toContain("migration deferred"); + expect(formatApplyHistoryFailure(workerBusy, false)).toContain("retried automatically"); +}); + +test("apply says NOT changed for non-busy failures", () => { + const unsafe = { kind: "blocked", reason: "unsafe-path" } as const; + expect(formatApplyHistoryFailure(unsafe, false)).toContain("NOT changed"); + expect(formatApplyHistoryFailure(unsafe, false)).toContain("not a Codex app lock"); + + const workerError = { kind: "failed", reason: "worker-error", message: "unable to open database file" } as const; + expect(formatApplyHistoryFailure(workerError, false)).toContain("NOT changed"); + expect(formatApplyHistoryFailure(workerError, false)).toContain("unable to open database file"); +}); + +test("restore blames the Codex app only for genuine busy reasons", () => { + const blockedBusy = { kind: "blocked", reason: "busy" } as const; + expect(failedHistoryRestoreFromOutcome(blockedBusy).message).toContain("holding the history database"); + + const workerBusy = { + kind: "failed", + reason: "worker-error", + message: "database is locked", + historyFailureReason: "busy", + } as const; + expect(failedHistoryRestoreFromOutcome(workerBusy).message).toContain("holding the history database"); +}); + +test("restore names other reasons instead of a lock", () => { + const unsafe = { kind: "blocked", reason: "unsafe-path" } as const; + const unsafeMessage = failedHistoryRestoreFromOutcome(unsafe).message; + expect(unsafeMessage).toContain("unsafe coordinator namespace"); + expect(unsafeMessage).not.toContain("holding the history database"); + + const permission = { + kind: "failed", + reason: "worker-error", + message: "history_transition_failed", + historyFailureReason: "permission", + } as const; + expect(failedHistoryRestoreFromOutcome(permission).message).toContain("permission was denied"); + + const workerError = { kind: "failed", reason: "worker-error", message: "unable to open database file" } as const; + const workerMessage = failedHistoryRestoreFromOutcome(workerError).message; + expect(workerMessage).toContain("unable to open database file"); + expect(workerMessage).not.toContain("holding the history database"); +}); diff --git a/tests/codex-user-identity.test.ts b/tests/codex-user-identity.test.ts index ba24ed435d..0d4f6ce59e 100644 --- a/tests/codex-user-identity.test.ts +++ b/tests/codex-user-identity.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync } from "node:fs"; import { join, parse } from "node:path"; import { tmpdir } from "node:os"; import { pathToFileURL } from "node:url"; @@ -9,6 +9,7 @@ import { resolveCodexCatalogSerializationDatabasePath, resolveCodexHistorySerializationDatabasePath, resolveEffectiveUserIdentity, + probeCodexCoordinatorNamespace, samePathIdentity, } from "../src/codex/user-identity"; @@ -77,6 +78,16 @@ test("samePathIdentity is case-insensitive on Windows and exact elsewhere", () = expect(samePathIdentity("/tmp/a/b.sqlite", "/tmp/A/b.sqlite", "linux")).toBe(false); }); +test("the coordinator namespace probe is read-only", () => { + if (process.platform === "win32") return; + // No real user has this uid, so the namespace cannot exist before or after. + const uid = 2_147_483_647; + const probe = probeCodexCoordinatorNamespace({ platform: "posix", uid }); + expect(probe.status).toBe("missing"); + const root = join(realpathSync.native("/tmp"), `opencodex-runtime-v1-${uid}`); + expect(existsSync(root)).toBe(false); +}); + afterEach(() => { if (previousHome === undefined) delete process.env.HOME; else process.env.HOME = previousHome; From 7883173adb219ddc9a121118666ff3b2d87d5c92 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Sat, 8 Aug 2026 16:11:13 +0800 Subject: [PATCH 5/5] fix(cli): skip shared teardown when a foreign proxy refuses stop --- src/cli/index.ts | 10 +++++++++- src/lib/process-control.ts | 5 ++++- tests/grok-lifecycle.test.ts | 9 ++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index 1eb93afbc4..a66f0a4e30 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -40,7 +40,7 @@ import { findAvailablePort, isAddrInUse, PortUnavailableError, shouldPersistSele import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-liveness"; import { createReadinessGate } from "../server/readiness"; import { parseReadyArgs, runReady, type ReadyArgs } from "./ready"; -import { stopProxy } from "../lib/process-control"; +import { ProxyOwnershipRefusedError, stopProxy } from "../lib/process-control"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; import { diagnoseService, isServiceOwnershipError, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalled, uninstallServiceIfInstalled } from "../service"; import { startupHealthSummary } from "../codex/autostart-health"; @@ -679,6 +679,10 @@ async function handleStop() { // exact teardown the refusal exists to prevent. const detail = err instanceof Error ? err.message : String(err); if (detail) console.error(` ${detail}`); + if (err instanceof ProxyOwnershipRefusedError) { + ownershipBlocked = true; + console.error(" Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running."); + } } } else { // Snapshot the stale on-disk state BEFORE the async probe: a concurrent `ocx start` @@ -697,6 +701,10 @@ async function handleStop() { console.error(`❌ Failed to stop proxy (PID ${live.pid}).`); const detail = err instanceof Error ? err.message : String(err); if (detail) console.error(` ${detail}`); + if (err instanceof ProxyOwnershipRefusedError) { + ownershipBlocked = true; + console.error(" Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running."); + } } } else if (!stoppedService) { console.log("No running proxy found."); diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index f550284602..8f60e05e67 100644 --- a/src/lib/process-control.ts +++ b/src/lib/process-control.ts @@ -52,6 +52,9 @@ export function gracefulStopHost(hostname: string | undefined): string { */ export type GracefulStopResult = boolean | "refused"; +/** A proxy declined shutdown because a service under another home owns it (HTTP 409). */ +export class ProxyOwnershipRefusedError extends Error {} + /** * Ask a running proxy to stop itself via the management API (`POST /api/stop`), which * drains in-flight turns, restores native Codex, and cleans its pid/runtime files. @@ -110,7 +113,7 @@ export async function stopProxy(pid: number): Promise { if (graceful === "refused") { // The proxy refused on purpose (foreign service owns it). Forcing would strip shared // config while that service keeps the proxy alive. - throw new Error( + throw new ProxyOwnershipRefusedError( "The running proxy refused to stop: a service installed under a different " + "CODEX_HOME/OPENCODEX_HOME owns it. Run the stop from that home.", ); diff --git a/tests/grok-lifecycle.test.ts b/tests/grok-lifecycle.test.ts index 9bf07ebb7b..b7fdc351f0 100644 --- a/tests/grok-lifecycle.test.ts +++ b/tests/grok-lifecycle.test.ts @@ -76,6 +76,13 @@ describe("Grok fence lifecycle wiring", () => { const detailEchoes = stopFn.match(/const detail = err instanceof Error \? err\.message : String\(err\);/g); expect(detailEchoes).toHaveLength(2); expect(stopFn.match(/if \(detail\) console\.error\(` \$\{detail\}`\);/g)).toHaveLength(2); + + // A proxy ownership refusal means a foreign service still owns the running proxy, so the + // shared teardown must be skipped at both call sites, exactly like the service-manager path. + const ownershipRefusals = stopFn.match(/err instanceof ProxyOwnershipRefusedError[\s\S]{0,200}?ownershipBlocked = true;/g); + expect(ownershipRefusals).toHaveLength(2); + expect(stopFn.match(/Skipping shared teardown \(native Codex restore, Grok config\): the foreign proxy is still running\./g)).toHaveLength(2); + expect(PROCESS_CONTROL_SOURCE).toContain("throw new ProxyOwnershipRefusedError("); }); test("handleStop returns its outcome while both restart surfaces share the in-place lifecycle", () => { @@ -177,6 +184,6 @@ describe("POST /api/stop teardown", () => { const killAt = stopProxyFn.indexOf("killProxy(pid)"); expect(refusedAt).toBeGreaterThan(-1); expect(refusedAt).toBeLessThan(killAt); - expect(stopProxyFn).toContain("throw new Error("); + expect(stopProxyFn).toContain("throw new ProxyOwnershipRefusedError("); }); });