Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/cli/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -902,6 +907,22 @@ export async function runDoctor(args: string[] = []): Promise<void> {
// 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)");
Comment thread
Yuxin-Qiao marked this conversation as resolved.
} 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");
Expand Down
18 changes: 15 additions & 3 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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`
Expand All @@ -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.");
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion src/codex/catalog-write-serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
CodexUserIdentityRefusal,
resolveCodexCatalogSerializationDatabasePath,
resolveEffectiveUserIdentity,
samePathIdentity,
} from "./user-identity";

/**
Expand Down Expand Up @@ -180,7 +181,7 @@ export function withCatalogWriteSerialization<T>(
}
const opened = lstatSync(databasePath);
if (opened.isSymbolicLink() || !opened.isFile()
|| realpathSync.native(databasePath) !== databasePath) {
|| !samePathIdentity(realpathSync.native(databasePath), databasePath)) {
return { kind: "unavailable", reason: "unsafe-path" };
}

Expand Down
82 changes: 81 additions & 1 deletion src/codex/history-job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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'.";
Comment thread
Yuxin-Qiao marked this conversation as resolved.
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'.`;
Comment thread
Yuxin-Qiao marked this conversation as resolved.
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("~");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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 } : {}),
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/codex/history-lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
CodexUserIdentityRefusal,
resolveCodexHistorySerializationDatabasePath,
resolveEffectiveUserIdentity,
samePathIdentity,
} from "./user-identity";

/**
Expand Down Expand Up @@ -183,7 +184,7 @@ export function withHistoryWriteSerialization<T>(
}
const opened = lstatSync(databasePath);
if (opened.isSymbolicLink() || !opened.isFile()
|| realpathSync.native(databasePath) !== databasePath) {
|| !samePathIdentity(realpathSync.native(databasePath), databasePath)) {
return { kind: "unavailable", reason: "unsafe-path" };
}

Expand Down
7 changes: 4 additions & 3 deletions src/codex/history-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}

/**
Expand Down
68 changes: 56 additions & 12 deletions src/codex/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1250,7 +1248,7 @@ export interface CodexNativeRestoreResult {
};
}

function failedHistoryRestore(reason?: CodexHistoryFailureReason): CodexRestoreHistoryResult {
function failedHistoryRestore(reason?: CodexHistoryFailureReason, detail?: string): CodexRestoreHistoryResult {
return {
state: "failed",
changed: false,
Expand All @@ -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<CodexHistoryJobOutcome, { kind: "blocked" | "failed" }>,
): 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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
3 changes: 2 additions & 1 deletion src/codex/transition-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
CodexUserIdentityRefusal,
resolveCodexCoordinatorDatabasePath,
resolveEffectiveUserIdentity,
samePathIdentity,
} from "./user-identity";

const COORDINATOR_SCHEMA_VERSION = 1;
Expand Down Expand Up @@ -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.");
}
};
Expand Down
Loading
Loading