diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 29b8012a71..2e12745b4c 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -25,6 +25,11 @@ 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 { + CodexUserIdentityRefusal, + probeCodexCoordinatorNamespace, + resolveEffectiveUserIdentity, +} from "../codex/user-identity"; import { collectProjectCodexConfigWarnings, formatProjectCodexConfigWarningsForDoctor } from "../codex/project-config-warnings"; import { collectStartupHealth, startupHealthSummary } from "../codex/autostart-health"; import { @@ -902,6 +907,22 @@ 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. 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 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}`); + } const pending = countPendingOpencodexHistory(); if (pending.failed) { console.log(" -- state DB locked or unreadable (Codex app open?) — migration state unknown"); diff --git a/src/cli/index.ts b/src/cli/index.ts index 5a995987dd..a66f0a4e30 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -2,7 +2,11 @@ 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, +} from "../codex/history-job"; import { reconcileJournal } from "../codex/journal"; import { codexAutoStartEnabled, @@ -36,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"; @@ -675,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` @@ -693,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."); @@ -899,7 +911,7 @@ 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, "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..2d35498daf 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 { @@ -135,13 +136,92 @@ 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: 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."; + } + // 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 "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 busyText; + 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'."; + } +} + +/** + * 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(); + 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 { 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/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..6ab07183e3 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,36 @@ 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 reason was not recorded. Run 'ocx doctor'.", }; } +/** + * 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). `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. + */ +export function failedHistoryRestoreFromOutcome( + outcome: Extract, +): CodexRestoreHistoryResult { + 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 { const message = `External Codex provider ${tomlString(activeProvider)} preserved; no native restore was needed.`; return { @@ -1499,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.reason === "busy" - ? failedHistoryRestore("busy") - : outcome.kind === "failed" - ? failedHistoryRestore(outcome.historyFailureReason) - : failedHistoryRestore(); + : outcome.kind === "blocked" || outcome.kind === "failed" + ? failedHistoryRestoreFromOutcome(outcome) + : failedHistoryRestore(); const base = catalog.removed > 0 ? `${config.message} Catalog restored to ${catalog.kept} native model(s) (dropped ${catalog.removed} proxy-routed).` : config.message; @@ -1572,3 +1594,25 @@ 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. + */ +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 + // 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" + : busy + ? "Codex resume history migration deferred" + : "Codex resume history NOT changed"; + return ` ⚠️ ${headline}: ${describeHistoryJobFailure(outcome, "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..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( @@ -160,7 +230,45 @@ function resolveWindowsRuntimeRoot(identity: Extract { 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/codex-history-job.test.ts b/tests/codex-history-job.test.ts index 6cae854c62..de4a8b2c32 100644 --- a/tests/codex-history-job.test.ts +++ b/tests/codex-history-job.test.ts @@ -7,6 +7,7 @@ import { join } from "node:path"; import { Database } from "bun:sqlite"; import { + describeHistoryJobFailure, deriveCodexHistoryOperation, runCodexHistoryJob, } from "../src/codex/history-job"; @@ -80,6 +81,62 @@ test("the operation is derived from admitted intent, not chosen by a caller", () .toBe("restore-openai"); }); +test("the failure wording names the real reason instead of always blaming the Codex app", () => { + 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"); + + // A busy SQLite database can also surface as a worker failure once the lock + // was already acquired; it must keep the same lock guidance, not degrade to + // a generic worker error. + const workerBusy = { + kind: "failed", + reason: "worker-error", + message: "database is locked", + historyFailureReason: "busy", + } as const; + expect(describeHistoryJobFailure(workerBusy, "apply", false)).toContain("history DB is locked"); + expect(describeHistoryJobFailure(workerBusy, "apply", false)).toContain("retried automatically"); + expect(describeHistoryJobFailure(workerBusy, "restore")).toContain("holding the history database"); + expect(describeHistoryJobFailure(workerBusy, "recover-legacy")).toContain("locked"); + + 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"); + + // 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 () => { const fixture = makeFixture("ocx-history-job-skip-"); @@ -134,6 +191,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-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 fb7f100313..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,8 @@ import { resolveCodexCatalogSerializationDatabasePath, resolveCodexHistorySerializationDatabasePath, resolveEffectiveUserIdentity, + probeCodexCoordinatorNamespace, + samePathIdentity, } from "../src/codex/user-identity"; let codexHome = ""; @@ -67,6 +69,25 @@ 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); +}); + +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; 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("); }); });