From 6d5f0cf2cf4e9f1bad91c0d57ed36b7ae56dea4a Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Fri, 21 Aug 2026 13:33:24 +0000 Subject: [PATCH 1/4] fix(codex): recover zero-byte coordinator remnants --- .../content/docs/guides/codex-integration.md | 25 ++ .../content/docs/reference/cli/lifecycle.md | 17 + src/cli/dispatch.ts | 4 +- src/cli/doctor.ts | 89 +++++ src/cli/help.ts | 2 + src/cli/registry.ts | 4 + src/codex/coordinator-doctor.ts | 332 ++++++++++++++++++ src/codex/inject-coordination.ts | 45 ++- src/codex/transition-state.ts | 24 +- structure/02_config-and-codex-home.md | 21 ++ tests/codex-coordinator-doctor.test.ts | 207 +++++++++++ tests/codex-inject-write-lock.test.ts | 43 ++- 12 files changed, 792 insertions(+), 21 deletions(-) create mode 100644 src/codex/coordinator-doctor.ts create mode 100644 tests/codex-coordinator-doctor.test.ts diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 1485d49b2c..80e1c152dd 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -203,6 +203,31 @@ Routed catalog entries also get their GPT-5 identity rewritten to the real upstr Reasoning controls come from provider/model metadata across Codex's `low | medium | high | xhigh | max | ultra` ladder; unsupported values are mapped or clamped before the upstream request. +### Coordinator diagnosis and recovery + +Native config/history writes use a per-user SQLite coordinator keyed by the canonical `CODEX_HOME`. +If a process terminates in SQLite's initial creation window, a zero-byte coordinator can remain even +though it contains no authoritative transition row. `ocx doctor` reports the exact coordinator path +and distinguishes zero-byte, unversioned, rowless, valid, unsafe, and unreadable states without +creating SQLite sidecars. Automatic sync tolerates only an identity-stable zero-byte file that has +settled for at least one second and whose immutable SQLite snapshot has version zero with no tables; +a newly created zero-byte file remains on the locked coordinator path. + +For a state that doctor proves is a zero-byte creation remnant, stop the OpenCodex proxy/service +and run: + +```bash +ocx doctor --recover-zero-byte-coordinator --yes +ocx sync +``` + +Recovery moves the still-identical zero-byte file to a same-directory `.zero-byte-backup-*` path; +it does not delete the evidence or adopt legacy routed state. It refuses a running proxy, lock +contention, symlinks/reparse points, foreign ownership, changed files, every non-empty database, +and any coordinator that already has an authoritative row. Desktop renderer filtering is a +separate layer: a correct catalog and coordinator do not by themselves bypass the Codex App model +allowlist. + ### Routed local tools Non-native routed catalog rows use `tool_mode: "code_mode_only"`. This lets Codex expose its official diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 486d10321e..c56bfaa5cc 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -162,6 +162,23 @@ unreachable; and 64 for invalid arguments. ### `ocx doctor` +The default report includes the native-write coordinator state and exact path using immutable +read-only SQLite inspection. Zero-byte, empty-unversioned, and rowless states are shown separately +from catalog/app-server health, so a successful catalog refresh is not mistaken for successful +Codex config injection. + +After stopping the OpenCodex proxy/service, explicitly preserve and move a proven non-authoritative +coordinator, then retry sync: + +```bash +ocx doctor --recover-zero-byte-coordinator --yes +ocx sync +``` + +The recovery accepts only a proven zero-byte remnant. It refuses every non-empty, valid, unknown, +changed, unsafe, or busy database and creates a same-directory `.zero-byte-backup-*` file instead +of deleting anything. + Run read-only environment and connectivity diagnostics: state paths and filesystem type, WSL dual installs, proxy environment/config, ChatGPT reachability, Codex plugin and project-config warnings, and pending history migration. The Codex app-home targeting section also detects the narrow Windows diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index b70de46548..217e2d8967 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -172,9 +172,9 @@ const commandRunners: Record = { }, doctor: async deps => { const doctorArgs = deps.args.slice(1); - const { runDoctor } = await import("./doctor"); + const { RECOVER_ZERO_BYTE_COORDINATOR_FLAG, runDoctor } = await import("./doctor"); await runDoctor(doctorArgs); - if (!doctorArgs.includes("--fix-codex-runtime")) { + if (!doctorArgs.includes("--fix-codex-runtime") && !doctorArgs.includes(RECOVER_ZERO_BYTE_COORDINATOR_FLAG)) { console.log(""); const { printCodexLogGuardDoctor } = await import("./codex-log-guard-doctor"); printCodexLogGuardDoctor(); diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 8af24a2693..d40ba14f2e 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -25,6 +25,11 @@ import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHome import { scanCodexAgentRolesWithTomlModelFallback } from "../codex/subagent-model-fallback"; import { findCodexOnPath, isWindowsInteropDir } from "../codex/shim"; import { countPendingOpencodexHistory } from "../codex/history-provider"; +import { + inspectCodexCoordinator, + recoverZeroByteCodexCoordinator, + type CodexCoordinatorDiagnostic, +} from "../codex/coordinator-doctor"; import { inspectAbandonedResponseStateTemps, reclaimAbandonedResponseStateTemps, @@ -684,6 +689,7 @@ export async function fetchServiceMemory( const mb = (bytes: number): string => `${Math.round(bytes / (1024 * 1024))}MB`; export const RECLAIM_RESPONSE_TEMPS_FLAG = "--reclaim-response-temps"; +export const RECOVER_ZERO_BYTE_COORDINATOR_FLAG = "--recover-zero-byte-coordinator"; /** Matches the dry run's entry bound so report and reclaim agree on a large backlog. */ const RESPONSE_TEMP_RECLAIM_MAX_CLEANUPS = 4_096; /** Names the subsystem: other components mint temps with the same shape and are not covered. */ @@ -734,6 +740,60 @@ export function formatResponseTempLines( return lines; } +export function formatCoordinatorDoctorLines(diagnostic: CodexCoordinatorDiagnostic): string[] { + const pathLine = diagnostic.path ? [` path: ${diagnostic.path}`] : []; + const evidenceLines = "evidence" in diagnostic && diagnostic.evidence + ? [ + ` size: ${diagnostic.evidence.sizeBytes} bytes; user_version: ${diagnostic.evidence.schemaVersion}`, + ` tables: ${diagnostic.evidence.tables.length === 0 ? "none" : diagnostic.evidence.tables.join(", ")}`, + ` transition rows: ${diagnostic.evidence.transitionRows ?? "not inspected"}; singleton=1 rows: ${diagnostic.evidence.singletonRows ?? "not inspected"}`, + ] + : []; + switch (diagnostic.kind) { + case "absent": + return [" ok native-write coordinator not created yet", ...pathLine]; + case "ready": + return [" ok native-write coordinator has an authoritative transition row", ...pathLine, ...evidenceLines]; + case "zero-byte": + return [ + " !! native-write coordinator is a zero-byte remnant and has no authority", + ...pathLine, + ...evidenceLines, + ` Action: stop the OpenCodex proxy/service, then run ocx doctor ${RECOVER_ZERO_BYTE_COORDINATOR_FLAG} --yes`, + ]; + case "unversioned-empty": + return [ + " !! native-write coordinator is a non-empty unversioned database; automatic recovery is refused", + ...pathLine, + ...evidenceLines, + ]; + case "rowless": + return [ + " !! native-write coordinator has schema version 1 but no authoritative row; automatic recovery is refused", + ...pathLine, + ...evidenceLines, + ]; + case "unversioned-nonempty": + return [ + " !! native-write coordinator is unversioned and contains unknown tables; automatic recovery is refused", + ...pathLine, + ...evidenceLines, + ]; + case "unsupported": + return [ + ` !! native-write coordinator schema version ${diagnostic.version} is unsupported; automatic recovery is refused`, + ...pathLine, + ...evidenceLines, + ]; + case "changed": + return [" -- native-write coordinator changed during diagnosis; re-run ocx doctor", ...pathLine]; + case "unsafe": + return [` !! native-write coordinator path is unsafe: ${diagnostic.reason}`, ...pathLine]; + case "unreadable": + return [` !! native-write coordinator is unreadable: ${diagnostic.reason}`, ...pathLine, ...evidenceLines]; + } +} + /** Render the doctor "Memory / runtime" section lines (testable without console capture). */ export function formatServiceMemoryLines(report: ServiceMemoryReport): string[] { const lines: string[] = []; @@ -846,6 +906,33 @@ export async function runDoctor(args: string[] = []): Promise { return; } + if (args.includes(RECOVER_ZERO_BYTE_COORDINATOR_FLAG)) { + if (!args.includes("--yes")) { + console.log(`Recovery is explicit and creates a same-directory backup. Re-run: ocx doctor ${RECOVER_ZERO_BYTE_COORDINATOR_FLAG} --yes`); + process.exitCode = 1; + return; + } + const diagnostics = readConfigDiagnostics().config; + const live = await findLiveProxy({ + configFn: () => ({ port: diagnostics.port, hostname: diagnostics.hostname }), + }); + if (live) { + console.log(`Recovery refused: OpenCodex proxy pid ${live.pid} is still running. Stop the proxy/service and retry.`); + process.exitCode = 1; + return; + } + const recovered = recoverZeroByteCodexCoordinator(); + if (!recovered.ok) { + console.log(`Recovery refused: ${recovered.reason}.`); + process.exitCode = 1; + return; + } + console.log(`Moved the non-authoritative coordinator to ${recovered.backupPath}`); + console.log("Run `ocx sync` to retry Codex config injection. The backup was preserved and no Codex config/catalog file was changed by recovery."); + process.exitCode = 0; + return; + } + console.log("opencodex doctor\n"); // Ordering note: the memory/runtime section renders after "Running proxy @@ -1005,6 +1092,8 @@ export async function runDoctor(args: string[] = []): Promise { const reason = cause instanceof CodexUserIdentityRefusal ? cause.message : String(cause); console.log(` -- history coordinator namespace refused: ${reason}`); } + console.log("\nCodex native-write coordinator"); + for (const line of formatCoordinatorDoctorLines(inspectCodexCoordinator())) console.log(line); const pending = countPendingOpencodexHistory(); if (pending.failed) { console.log(" -- state DB locked or unreadable (Codex app open?) — migration state unknown"); diff --git a/src/cli/help.ts b/src/cli/help.ts index ca1efe8c01..89e2a4edb2 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -38,6 +38,8 @@ Usage: ocx doctor Diagnose environment/network issues (WSL, proxy, ChatGPT reachability) ocx doctor --reclaim-response-temps Reclaim abandoned response-state temp files (works without a running proxy) + ocx doctor --recover-zero-byte-coordinator --yes + Back up a proven zero-byte Codex coordinator after stopping the proxy ocx debug provider/usage/injection/claude on|off|status|reset ocx login OAuth or API-key provider login ocx logout Remove a stored OAuth login diff --git a/src/cli/registry.ts b/src/cli/registry.ts index c8c786b54e..844a644b86 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -108,6 +108,10 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ name: "doctor", usage: "ocx doctor", summary: "Diagnose environment/network issues (paths, WSL /mnt, proxy env, ChatGPT reachability).", + details: [ + "Default mode is observe-only and reports the native-write coordinator state and exact path.", + "After stopping the proxy/service, `--recover-zero-byte-coordinator --yes` moves only a proven zero-byte coordinator to a same-directory backup.", + ], }, { name: "debug", diff --git a/src/codex/coordinator-doctor.ts b/src/codex/coordinator-doctor.ts new file mode 100644 index 0000000000..1c238bd922 --- /dev/null +++ b/src/codex/coordinator-doctor.ts @@ -0,0 +1,332 @@ +/** + * Observe and explicitly quarantine non-authoritative native-write coordinators. + * + * Default doctor runs use immutable SQLite reads so diagnostics cannot create + * WAL/SHM sidecars. Recovery is deliberately opt-in and moves, never deletes, + * only a file that is still the same private regular file observed beforehand. + */ +import { createHash } from "node:crypto"; +import { + existsSync, + lstatSync, + realpathSync, + renameSync, + type Stats, +} from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +import { Database, constants as sqliteConstants } from "bun:sqlite"; + +import { resolveCodexHomeDir } from "./home"; +import { + CodexUserIdentityRefusal, + probeCodexCoordinatorNamespace, + resolveEffectiveUserIdentity, + samePathIdentity, +} from "./user-identity"; +import { + CODEX_COORDINATOR_SCHEMA_VERSION, + readCodexCoordinatorState, +} from "./transition-state"; + +const IMMUTABLE_READONLY_FLAGS = + sqliteConstants.SQLITE_OPEN_READONLY | sqliteConstants.SQLITE_OPEN_URI; + +export type FileIdentity = Pick; + +export interface CodexCoordinatorDiagnosticEvidence { + sizeBytes: number; + schemaVersion: number; + tables: readonly string[]; + transitionRows: number | null; + singletonRows: number | null; +} + +export type CodexCoordinatorDiagnostic = + | { kind: "absent"; path: string | null } + | { kind: "zero-byte"; path: string; identity: FileIdentity; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "unversioned-empty"; path: string; identity: FileIdentity; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "unversioned-nonempty"; path: string; identity: FileIdentity; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "rowless"; path: string; identity: FileIdentity; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "ready"; path: string; identity: FileIdentity; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "unsupported"; path: string; identity: FileIdentity; version: number; evidence: CodexCoordinatorDiagnosticEvidence } + | { kind: "changed"; path: string } + | { kind: "unsafe"; path: string | null; reason: string } + | { kind: "unreadable"; path: string; reason: string; evidence?: CodexCoordinatorDiagnosticEvidence }; + +export type CodexCoordinatorRecoveryResult = + | { ok: true; backupPath: string } + | { ok: false; reason: string }; + +function errorCode(error: unknown): string { + return error && typeof error === "object" && "code" in error + ? String((error as { code?: unknown }).code) + : ""; +} + +function sameIdentity(left: FileIdentity, right: FileIdentity): boolean { + return left.dev === right.dev + && left.ino === right.ino + && left.size === right.size + && left.mtimeMs === right.mtimeMs + && left.ctimeMs === right.ctimeMs; +} + +function sameNodeAndSize(left: FileIdentity, right: FileIdentity): boolean { + return left.dev === right.dev && left.ino === right.ino && left.size === right.size; +} + +function coordinatorPathWithoutCreation(): { kind: "absent"; path: string | null } | { kind: "path"; path: string } { + const identity = resolveEffectiveUserIdentity(); + const canonicalCodexHome = realpathSync.native(resolveCodexHomeDir()); + const namespace = probeCodexCoordinatorNamespace(identity); + if (namespace.status === "missing") return { kind: "absent", path: null }; + + const locks = join(namespace.root, "native-write-locks"); + let locksEntry: Stats; + try { + locksEntry = lstatSync(locks); + } catch (cause) { + if (errorCode(cause) === "ENOENT") { + const digest = createHash("sha256").update(canonicalCodexHome).digest("hex"); + return { kind: "absent", path: join(locks, `${digest}.sqlite`) }; + } + throw new CodexUserIdentityRefusal("The coordinator lock directory cannot be inspected.", { cause }); + } + if (locksEntry.isSymbolicLink() || !locksEntry.isDirectory()) { + throw new CodexUserIdentityRefusal("The coordinator lock namespace is not a real directory."); + } + if (identity.platform === "posix") { + if (locksEntry.uid !== identity.uid || (locksEntry.mode & 0o777) !== 0o700) { + throw new CodexUserIdentityRefusal("The coordinator lock namespace has unsafe ownership or permissions."); + } + } else if (!samePathIdentity(realpathSync.native(locks), locks, "win32")) { + throw new CodexUserIdentityRefusal("The coordinator lock namespace is redirected by a junction or reparse point."); + } + + const digest = createHash("sha256").update(canonicalCodexHome).digest("hex"); + return { kind: "path", path: join(locks, `${digest}.sqlite`) }; +} + +function inspectTarget( + path: string, + options: { allowSqliteSidecars?: boolean } = {}, +): { kind: "absent" } | { kind: "file"; identity: FileIdentity } | { kind: "unsafe"; reason: string } { + let entry: Stats; + try { + entry = lstatSync(path); + } catch (cause) { + if (errorCode(cause) === "ENOENT") return { kind: "absent" }; + return { kind: "unsafe", reason: "the coordinator file cannot be inspected" }; + } + if (entry.isSymbolicLink() || !entry.isFile()) { + return { kind: "unsafe", reason: "the coordinator path is not a real file" }; + } + try { + if (!samePathIdentity(realpathSync.native(path), path)) { + return { kind: "unsafe", reason: "the coordinator path is redirected" }; + } + } catch { + return { kind: "unsafe", reason: "the coordinator path cannot be resolved" }; + } + if (process.platform !== "win32") { + const uid = process.getuid?.(); + if (uid === undefined || entry.uid !== uid || (entry.mode & 0o777) !== 0o600) { + return { kind: "unsafe", reason: "the coordinator file has unsafe ownership or permissions" }; + } + } + if (!options.allowSqliteSidecars) { + for (const suffix of ["-journal", "-wal", "-shm"]) { + if (existsSync(`${path}${suffix}`)) { + return { kind: "unsafe", reason: `the coordinator has an active SQLite ${suffix.slice(1)} sidecar` }; + } + } + } + return { kind: "file", identity: entry }; +} + +function classifyOpenedDatabase( + database: Database, + path: string, + identity: FileIdentity, +): CodexCoordinatorDiagnostic { + const version = database.query<{ user_version: number }, []>("PRAGMA user_version").get()?.user_version ?? 0; + const tables = database.query<{ name: string }, []>( + "SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name", + ).all().map(row => row.name); + const baseEvidence = { + sizeBytes: identity.size, + schemaVersion: version, + tables, + transitionRows: null, + singletonRows: null, + } satisfies CodexCoordinatorDiagnosticEvidence; + if (version === 0) { + const evidence = tables.length === 0 + ? { ...baseEvidence, transitionRows: 0, singletonRows: 0 } + : baseEvidence; + return tables.length === 0 + ? { kind: "unversioned-empty", path, identity, evidence } + : { kind: "unversioned-nonempty", path, identity, evidence }; + } + if (version !== CODEX_COORDINATOR_SCHEMA_VERSION) { + return { kind: "unsupported", path, identity, version, evidence: baseEvidence }; + } + if (tables.length !== 1 || tables[0] !== "codex_transition_state") { + return tables.length === 0 + ? { kind: "rowless", path, identity, evidence: baseEvidence } + : { kind: "unreadable", path, reason: "the coordinator contains unexpected tables", evidence: baseEvidence }; + } + let rowCounts: { total: number; singleton: number } | null; + try { + rowCounts = database.query<{ total: number; singleton: number }, []>( + "SELECT count(*) AS total, sum(CASE WHEN singleton = 1 THEN 1 ELSE 0 END) AS singleton FROM codex_transition_state", + ).get() ?? null; + } catch { + return { + kind: "unreadable", + path, + reason: "the transition table schema is not recognized", + evidence: baseEvidence, + }; + } + const evidence = { + ...baseEvidence, + transitionRows: rowCounts?.total ?? null, + singletonRows: rowCounts?.singleton ?? null, + }; + if (!rowCounts || rowCounts.total === 0) return { kind: "rowless", path, identity, evidence }; + if (rowCounts.total !== 1 || rowCounts.singleton !== 1) { + return { + kind: "unreadable", + path, + reason: "the coordinator does not contain exactly one singleton row", + evidence, + }; + } + try { + readCodexCoordinatorState(database); + } catch { + return { + kind: "unreadable", + path, + reason: "the authoritative transition row is malformed", + evidence, + }; + } + return { kind: "ready", path, identity, evidence }; +} + +export function inspectCodexCoordinator(): CodexCoordinatorDiagnostic { + let resolved: ReturnType; + try { + resolved = coordinatorPathWithoutCreation(); + } catch (cause) { + return { + kind: "unsafe", + path: null, + reason: cause instanceof Error ? cause.message : String(cause), + }; + } + if (resolved.kind === "absent") return resolved; + return inspectCodexCoordinatorPath(resolved.path); +} + +/** Inspect one already-resolved coordinator path without creating SQLite state. */ +export function inspectCodexCoordinatorPath(path: string): CodexCoordinatorDiagnostic { + const target = inspectTarget(path); + if (target.kind === "absent") return { kind: "absent", path }; + if (target.kind === "unsafe") return { kind: "unsafe", path, reason: target.reason }; + + let database: Database | undefined; + try { + const uri = `${pathToFileURL(path).href}?immutable=1`; + database = new Database(uri, IMMUTABLE_READONLY_FLAGS); + const result = classifyOpenedDatabase(database, path, target.identity); + const after = inspectTarget(path); + if (after.kind !== "file" || !sameIdentity(target.identity, after.identity)) { + return { kind: "changed", path }; + } + // Size alone is not evidence that this is a non-authoritative remnant. + // Query the immutable snapshot too, so the recovery label means all three + // facts were observed together: zero bytes, schema version zero, no tables. + if (target.identity.size === 0 && result.kind === "unversioned-empty") { + return { kind: "zero-byte", path, identity: target.identity, evidence: result.evidence }; + } + return result; + } catch (cause) { + return { kind: "unreadable", path, reason: cause instanceof Error ? cause.message : String(cause) }; + } finally { + try { database?.close(); } catch { /* diagnostics already completed */ } + } +} + +function recoverable(diagnostic: CodexCoordinatorDiagnostic): diagnostic is Extract< + CodexCoordinatorDiagnostic, + { kind: "zero-byte" } +> { + return diagnostic.kind === "zero-byte"; +} + +function backupTimestamp(now: Date): string { + return now.toISOString().replace(/[-:.]/g, ""); +} + +export function recoverZeroByteCodexCoordinator(now = new Date()): CodexCoordinatorRecoveryResult { + const observed = inspectCodexCoordinator(); + if (!recoverable(observed)) { + if (observed.kind === "unsafe" || observed.kind === "unreadable") { + return { ok: false, reason: `coordinator state is ${observed.kind}: ${observed.reason}` }; + } + return { ok: false, reason: `coordinator state is ${observed.kind}, not a recoverable zero-byte remnant` }; + } + + let database: Database | undefined; + let transactionOpen = false; + try { + database = new Database(observed.path, { readwrite: true, create: false }); + database.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); + transactionOpen = true; + const lockedEntry = inspectTarget(observed.path, { allowSqliteSidecars: true }); + // SQLite may update file timestamps merely by opening a zero-byte database + // for BEGIN IMMEDIATE. Device/inode/size are the stable identity here; the + // transaction excludes content writers while we reclassify the database. + if (lockedEntry.kind !== "file" || !sameNodeAndSize(observed.identity, lockedEntry.identity)) { + return { ok: false, reason: "the coordinator changed before recovery acquired its SQLite lock" }; + } + if (lockedEntry.identity.size !== 0) { + return { ok: false, reason: "the coordinator stopped being zero-byte before recovery" }; + } + database.exec("ROLLBACK"); + transactionOpen = false; + database.close(); + database = undefined; + + const finalEntry = inspectTarget(observed.path); + if (finalEntry.kind !== "file" || !sameIdentity(lockedEntry.identity, finalEntry.identity)) { + return { ok: false, reason: "the coordinator changed before the backup move" }; + } + const backupPath = `${observed.path}.zero-byte-backup-${backupTimestamp(now)}`; + if (existsSync(backupPath)) return { ok: false, reason: "the same-directory backup path already exists" }; + renameSync(observed.path, backupPath); + const backupEntry = inspectTarget(backupPath); + // The rename itself can advance ctime, so post-move verification uses the + // stable filesystem object and byte size. The full timestamp identity was + // already revalidated immediately before rename while the source existed. + if (backupEntry.kind !== "file" || !sameNodeAndSize(finalEntry.identity, backupEntry.identity) || existsSync(observed.path)) { + return { ok: false, reason: "the coordinator backup move could not be verified" }; + } + return { ok: true, backupPath }; + } catch (cause) { + const message = cause instanceof Error ? cause.message : String(cause); + const busy = errorCode(cause) === "SQLITE_BUSY" || errorCode(cause) === "SQLITE_LOCKED" + || /database (?:is|table is) locked/i.test(message); + return { ok: false, reason: busy ? "the coordinator is busy; stop active sync/service writers and retry" : message }; + } finally { + if (transactionOpen) { + try { database?.exec("ROLLBACK"); } catch { /* close releases the lock */ } + } + try { database?.close(); } catch { /* recovery already completed */ } + } +} diff --git a/src/codex/inject-coordination.ts b/src/codex/inject-coordination.ts index 91f9374bc6..a8b1c28858 100644 --- a/src/codex/inject-coordination.ts +++ b/src/codex/inject-coordination.ts @@ -5,10 +5,11 @@ * sequence it is, rather than doubling in length around the lock. */ import { createHash } from "node:crypto"; -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, lstatSync, readFileSync } from "node:fs"; import { atomicWriteFile } from "../config"; import type { CodexWriteLockResult } from "./codex-write-lock"; +import { inspectCodexCoordinatorPath } from "./coordinator-doctor"; import { JOURNAL_PATH } from "./journal"; import { CODEX_CONFIG_PATH, CODEX_PROFILE_PATH } from "./paths"; import { @@ -43,21 +44,51 @@ export type CodexWriteCoordinationEligibility = | { kind: "legacy-uncoordinated"; reason: string } | { kind: "refused"; reason: string }; +/** + * A live SQLite creator exposes a zero-byte pathname before BEGIN IMMEDIATE. + * Requiring a settled filesystem age makes that scheduling window remain on + * the coordinated path while old crash remnants can use the legacy boundary. + */ +export const STABLE_ZERO_BYTE_COORDINATOR_AGE_MS = 1_000; + export function codexWriteCoordinationEligibility(deps: { coordinatorPath: () => string; residue: () => { kind: string }; integrationRecord: () => { kind: string }; + nowMs?: () => number; }): CodexWriteCoordinationEligibility { let coordinatorExists: boolean; + let coordinatorIsStableZeroByte = false; try { - coordinatorExists = existsSync(deps.coordinatorPath()); + const path = deps.coordinatorPath(); + coordinatorExists = existsSync(path); + if (coordinatorExists) { + const entry = lstatSync(path); + if (entry.isFile() && !entry.isSymbolicLink() && entry.size === 0) { + const diagnostic = inspectCodexCoordinatorPath(path); + if (diagnostic.kind === "zero-byte") { + const lastIdentityChange = Math.max(diagnostic.identity.mtimeMs, diagnostic.identity.ctimeMs); + coordinatorIsStableZeroByte = (deps.nowMs?.() ?? Date.now()) - lastIdentityChange + >= STABLE_ZERO_BYTE_COORDINATOR_AGE_MS; + } + } + } } catch (error) { return { kind: "refused", reason: `the coordinator path could not be resolved: ${String(error)}` }; } - // An existing coordinator is authoritative, and the lock owns validating it — - // including the unversioned and rowless cases it must refuse rather than adopt. - if (coordinatorExists) return { kind: "coordinated" }; + // Every existing coordinator remains authoritative unless it is proven to be + // an old, immutable SQLite-empty remnant. The age gate is part of that proof: + // a live creator exposes the same zero-byte pathname briefly before taking N, + // and sending that fresh file down the legacy path would bypass its lock. + // Non-empty, fresh, unsafe, changed, unversioned, and rowless files therefore + // stay coordinated and are validated/refused by the transaction owner. + // + // We do NOT initialize or adopt it here. Clean homes still enter the + // coordinated path, whose SQLite transaction safely initializes it. Routed + // or indeterminate legacy homes keep the same uncoordinated compatibility + // boundary they would have had if the remnant pathname were absent. + if (coordinatorExists && !coordinatorIsStableZeroByte) return { kind: "coordinated" }; const record = deps.integrationRecord(); if (record.kind === "invalid") { @@ -83,7 +114,9 @@ export function codexWriteCoordinationEligibility(deps: { */ return { kind: "legacy-uncoordinated", - reason: residue.kind === "residue" + reason: coordinatorIsStableZeroByte + ? "the coordinator is a zero-byte non-authoritative remnant and this routed home has not been adopted yet" + : residue.kind === "residue" ? "this home was routed before write coordination existed and has not been adopted yet" : "the existing native Codex state could not be classified, so it cannot seed a coordinator row", }; diff --git a/src/codex/transition-state.ts b/src/codex/transition-state.ts index 27ce605530..ed00fca09f 100644 --- a/src/codex/transition-state.ts +++ b/src/codex/transition-state.ts @@ -37,7 +37,7 @@ import { samePathIdentity, } from "./user-identity"; -const COORDINATOR_SCHEMA_VERSION = 1; +export const CODEX_COORDINATOR_SCHEMA_VERSION = 1; const DURABLE_HISTORY_STATUSES = new Set(["converged", "pending", "running", "blocked", "unknown"]); const DURABLE_HISTORY_REASONS = new Set([ "db-busy", @@ -241,7 +241,7 @@ function rowToState(row: TransitionRow | null): CodexTransitionState { }; } -function readState(database: Database): CodexTransitionState { +export function readCodexCoordinatorState(database: Database): CodexTransitionState { const row = database.query(SELECT_TRANSITION_ROW).get(); return rowToState(row); } @@ -282,7 +282,7 @@ function assertInitialStateCanBeCreated(): void { function initialize(database: Database, databaseWasAbsent: boolean): void { const version = database.query<{ user_version: number }, []>("PRAGMA user_version").get()?.user_version; - if (version !== 0 && version !== COORDINATOR_SCHEMA_VERSION) { + if (version !== 0 && version !== CODEX_COORDINATOR_SCHEMA_VERSION) { throw new CodexCoordinatorTransactionError("The coordinator database schema version is unsupported."); } if (!databaseWasAbsent && version === 0) { @@ -301,8 +301,8 @@ function initialize(database: Database, databaseWasAbsent: boolean): void { assertInitialStateCanBeCreated(); database.query(INITIALIZE_TRANSITION_ROW).run(new Date().toISOString()); } - if (version === 0) database.exec(`PRAGMA user_version = ${COORDINATOR_SCHEMA_VERSION}`); - readState(database); + if (version === 0) database.exec(`PRAGMA user_version = ${CODEX_COORDINATOR_SCHEMA_VERSION}`); + readCodexCoordinatorState(database); } function createCapability( @@ -336,7 +336,7 @@ function createCapability( expected.nativeGeneration, expected.currentTxId, ); - const state = readState(database); + const state = readCodexCoordinatorState(database); const update: TransitionStateUpdate = result.changes === 1 ? { kind: "updated", state } : { kind: "conflict", current: state }; @@ -451,7 +451,7 @@ export function openCodexCoordinatorTransaction(finalDatabasePath: string): Code capability, expectation() { requireOpen(); - const state = readState(db); + const state = readCodexCoordinatorState(db); return { nativeBefore: state.nativeGeneration, nativeAfter: state.nativeGeneration + 1, @@ -460,7 +460,7 @@ export function openCodexCoordinatorTransaction(finalDatabasePath: string): Code }, version() { requireOpen(); - const state = readState(db); + const state = readCodexCoordinatorState(db); return { nativeGeneration: state.nativeGeneration, currentTxId: state.currentTxId }; }, assertPublished(expectation) { @@ -468,7 +468,7 @@ export function openCodexCoordinatorTransaction(finalDatabasePath: string): Code if (lastResult?.kind !== "updated") { throw new CodexCoordinatorTransactionError("The coordinator transition was not published."); } - const state = readState(db); + const state = readCodexCoordinatorState(db); if (state.nativeGeneration !== expectation.nativeAfter || state.currentTxId !== expectation.txId) { throw new CodexCoordinatorTransactionError("The coordinator published a different transition."); } @@ -540,7 +540,7 @@ function readCommittedState(): TransitionStateRead { try { database = new Database(path, { readonly: true }); database.exec("PRAGMA busy_timeout = 0"); - return { kind: "ready", state: readState(database) }; + return { kind: "ready", state: readCodexCoordinatorState(database) }; } catch (error) { return mapUnavailable(error); } finally { @@ -577,7 +577,7 @@ export const updateCodexHistoryTransition: UpdateCodexHistoryTransition = (expec database = new Database(currentCoordinatorDatabasePath(), { readwrite: true, create: false }); database.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); transactionOpen = true; - const current = readState(database); + const current = readCodexCoordinatorState(database); if (current.nativeGeneration > 0 && current.historySchedule === null) { throw new CodexCoordinatorTransactionError("A positive transition cannot lose its direction."); } @@ -594,7 +594,7 @@ export const updateCodexHistoryTransition: UpdateCodexHistoryTransition = (expec expected.currentTxId, expected.currentTxId, ); - const state = readState(database); + const state = readCodexCoordinatorState(database); database.exec("COMMIT"); transactionOpen = false; return result.changes === 1 diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 6054c211d4..26a279f592 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -80,6 +80,27 @@ on proven absence, never on an unreadable path. - 다른 대안 대신 이 방식을 선택한 이유: Physical credential ownership remains cross-process safe, while an inert optional subsystem can no longer create the reported lock/recovery catch-22. - 장점, 단점 및 영향: Fresh installs avoid the SQLite profile lock; any present or uncertain stage state retains the existing locked fail-closed cleanup and recovery behavior. +The native-write coordinator is keyed by the canonical `CODEX_HOME` in the effective-user runtime +namespace. A pathname alone is not authority: SQLite can expose a zero-byte file before its first +schema write, and a terminated process can leave that remnant behind. Eligibility treats the file +as non-authoritative only after an immutable SQLite read proves version zero with no tables, the +filesystem identity remains unchanged, and the file has been settled for at least one second; a +fresh zero-byte creator stays on the coordinated path so its lock cannot be bypassed. `ocx doctor` inspects the +coordinator with immutable read-only SQLite flags so diagnosis never creates WAL/SHM sidecars. It +distinguishes absent, zero-byte, unversioned, rowless, valid, unsupported, changed, unsafe, and +unreadable states and prints the exact path. Explicit recovery is available only after the proxy is +stopped and only for a proven zero-byte state. The command revalidates the same private +regular-file identity under a non-blocking SQLite write lock and moves it to a same-directory +backup; it never deletes or auto-adopts legacy routed residue. + +[Decision Log] +- 목적과 의도: Recover a crashed zero-byte coordinator without mistaking SQLite's normal creation window for stale authority. +- 기존 구현 및 제약 조건: Eligibility treated every existing pathname as coordinated, while initialization correctly refused a missing row over routed residue; catalog sync could therefore succeed before config injection failed permanently. +- 검토한 주요 대안: Delete zero-byte files automatically, initialize a new row over residue, require a manual filesystem command, or add observe-only classification plus explicit guarded quarantine. +- 선택한 방식: Treat only a settled, identity-stable, immutably verified zero-byte database like the existing legacy-uncoordinated boundary; keep fresh creators coordinated, diagnose all other database states immutably, and expose an opt-in zero-byte-only same-directory backup move with identity, ownership, sidecar, liveness, and SQLite-lock checks. +- 다른 대안 대신 이 방식을 선택한 이유: Automatic deletion or adoption can race a live creator or erase transition evidence; a guarded backup preserves evidence and makes the operator action reproducible. +- 장점, 단점 및 영향: A stale zero-byte file no longer wedges sync, valid/unrecognized databases remain fail-closed, and recovery requires the proxy to be stopped before `ocx sync` retries injection. + OpenCodex never overrides an explicit `CODEX_HOME`. On Windows, `ocx doctor` and `ocx status` nevertheless diagnose the high-confidence Orca dual-home case: both `CODEX_HOME` and `ORCA_CODEX_HOME` select Orca's `orca/codex-runtime-home/home`, while the ChatGPT/Codex app uses the diff --git a/tests/codex-coordinator-doctor.test.ts b/tests/codex-coordinator-doctor.test.ts new file mode 100644 index 0000000000..49e9be23ce --- /dev/null +++ b/tests/codex-coordinator-doctor.test.ts @@ -0,0 +1,207 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { Database } from "bun:sqlite"; + +import { + inspectCodexCoordinator, + recoverZeroByteCodexCoordinator, +} from "../src/codex/coordinator-doctor"; +import { + codexWriteCoordinationEligibility, + STABLE_ZERO_BYTE_COORDINATOR_AGE_MS, +} from "../src/codex/inject-coordination"; +import { + openCodexCoordinatorTransaction, +} from "../src/codex/transition-state"; +import { + resolveCodexCoordinatorDatabasePath, + resolveEffectiveUserIdentity, +} from "../src/codex/user-identity"; +import { formatCoordinatorDoctorLines } from "../src/cli/doctor"; + +let codexHome = ""; +let opencodexHome = ""; +let coordinatorPath = ""; +let previousCodexHome: string | undefined; +let previousOpencodexHome: string | undefined; + +beforeEach(() => { + previousCodexHome = process.env.CODEX_HOME; + previousOpencodexHome = process.env.OPENCODEX_HOME; + codexHome = mkdtempSync(join(tmpdir(), "ocx-coordinator-doctor-codex-")); + opencodexHome = mkdtempSync(join(tmpdir(), "ocx-coordinator-doctor-ocx-")); + process.env.CODEX_HOME = codexHome; + process.env.OPENCODEX_HOME = opencodexHome; + coordinatorPath = resolveCodexCoordinatorDatabasePath( + resolveEffectiveUserIdentity(), + realpathSync.native(codexHome), + ); +}); + +afterEach(() => { + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + for (const suffix of ["", "-journal", "-wal", "-shm"]) { + rmSync(`${coordinatorPath}${suffix}`, { force: true }); + } + rmSync(codexHome, { recursive: true, force: true }); + rmSync(opencodexHome, { recursive: true, force: true }); +}); + +function privateFile(path: string, bytes = ""): void { + writeFileSync(path, bytes); + if (process.platform !== "win32") chmodSync(path, 0o600); +} + +test("doctor classifies and explicitly backs up a stable zero-byte coordinator", () => { + privateFile(coordinatorPath); + const diagnostic = inspectCodexCoordinator(); + expect(diagnostic.kind).toBe("zero-byte"); + if (diagnostic.kind !== "zero-byte") return; + expect(formatCoordinatorDoctorLines(diagnostic).join("\n")).toContain( + "ocx doctor --recover-zero-byte-coordinator --yes", + ); + expect(formatCoordinatorDoctorLines(diagnostic).join("\n")).toContain( + "size: 0 bytes; user_version: 0", + ); + + const recovered = recoverZeroByteCodexCoordinator(new Date("2026-08-21T12:00:00.000Z")); + expect(recovered.ok).toBe(true); + if (!recovered.ok) return; + expect(recovered.backupPath).toEndWith(".zero-byte-backup-20260821T120000000Z"); + expect(existsSync(coordinatorPath)).toBe(false); + expect(existsSync(recovered.backupPath)).toBe(true); + rmSync(recovered.backupPath, { force: true }); +}); + +test("doctor distinguishes unversioned, rowless, and authoritative coordinators", () => { + let database = new Database(coordinatorPath, { create: true }); + database.exec("CREATE TABLE temporary_probe (id INTEGER); DROP TABLE temporary_probe"); + database.close(); + if (process.platform !== "win32") chmodSync(coordinatorPath, 0o600); + expect(inspectCodexCoordinator().kind).toBe("unversioned-empty"); + expect(recoverZeroByteCodexCoordinator()).toEqual({ + ok: false, + reason: "coordinator state is unversioned-empty, not a recoverable zero-byte remnant", + }); + + database = new Database(coordinatorPath, { readwrite: true, create: false }); + database.exec("PRAGMA user_version = 1; CREATE TABLE codex_transition_state (singleton INTEGER PRIMARY KEY)"); + database.close(); + expect(inspectCodexCoordinator().kind).toBe("rowless"); + expect(recoverZeroByteCodexCoordinator()).toEqual({ + ok: false, + reason: "coordinator state is rowless, not a recoverable zero-byte remnant", + }); + + database = new Database(coordinatorPath, { readwrite: true, create: false }); + database.exec("INSERT INTO codex_transition_state (singleton) VALUES (1)"); + database.close(); + const malformed = inspectCodexCoordinator(); + expect(malformed.kind).toBe("unreadable"); + expect(formatCoordinatorDoctorLines(malformed).join("\n")).toContain("user_version: 1"); + expect(formatCoordinatorDoctorLines(malformed).join("\n")).toContain("transition rows: 1"); + + rmSync(coordinatorPath, { force: true }); + const transaction = openCodexCoordinatorTransaction(coordinatorPath); + transaction.commit(); + transaction.close(); + expect(inspectCodexCoordinator().kind).toBe("ready"); + expect(recoverZeroByteCodexCoordinator()).toEqual({ + ok: false, + reason: "coordinator state is ready, not a recoverable zero-byte remnant", + }); +}); + +test("doctor inspection is immutable and refuses sidecars, unsafe modes, and symlinks", () => { + privateFile(coordinatorPath); + expect(inspectCodexCoordinator().kind).toBe("zero-byte"); + for (const suffix of ["-journal", "-wal", "-shm"]) { + expect(existsSync(`${coordinatorPath}${suffix}`)).toBe(false); + } + + privateFile(`${coordinatorPath}-wal`, "active"); + expect(inspectCodexCoordinator()).toMatchObject({ kind: "unsafe" }); + rmSync(`${coordinatorPath}-wal`, { force: true }); + + if (process.platform !== "win32") { + chmodSync(coordinatorPath, 0o644); + expect(inspectCodexCoordinator()).toMatchObject({ kind: "unsafe" }); + chmodSync(coordinatorPath, 0o600); + + const target = `${coordinatorPath}.target`; + privateFile(target); + rmSync(coordinatorPath, { force: true }); + symlinkSync(target, coordinatorPath); + expect(inspectCodexCoordinator()).toMatchObject({ kind: "unsafe" }); + rmSync(coordinatorPath, { force: true }); + rmSync(target, { force: true }); + } +}); + +test("recovery refuses a zero-byte coordinator with an active SQLite writer sidecar", () => { + privateFile(coordinatorPath); + const holder = new Database(coordinatorPath, { readwrite: true, create: false }); + holder.exec("PRAGMA journal_mode = OFF; PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); + try { + expect(recoverZeroByteCodexCoordinator()).toMatchObject({ + ok: false, + reason: expect.stringContaining("active SQLite journal sidecar"), + }); + expect(existsSync(coordinatorPath)).toBe(true); + } finally { + holder.exec("ROLLBACK"); + holder.close(); + } +}); + +test("zero-byte residue uses the legacy boundary while clean homes still initialize", () => { + privateFile(coordinatorPath); + const afterStableAge = () => Date.now() + STABLE_ZERO_BYTE_COORDINATOR_AGE_MS + 1; + expect(codexWriteCoordinationEligibility({ + coordinatorPath: () => coordinatorPath, + residue: () => ({ kind: "residue" }), + integrationRecord: () => ({ kind: "missing" }), + nowMs: afterStableAge, + })).toEqual({ + kind: "legacy-uncoordinated", + reason: "the coordinator is a zero-byte non-authoritative remnant and this routed home has not been adopted yet", + }); + expect(codexWriteCoordinationEligibility({ + coordinatorPath: () => coordinatorPath, + residue: () => ({ kind: "clean" }), + integrationRecord: () => ({ kind: "missing" }), + nowMs: afterStableAge, + })).toEqual({ kind: "coordinated" }); + + privateFile(coordinatorPath, "not-empty"); + expect(codexWriteCoordinationEligibility({ + coordinatorPath: () => coordinatorPath, + residue: () => ({ kind: "residue" }), + integrationRecord: () => ({ kind: "missing" }), + })).toEqual({ kind: "coordinated" }); +}); + +test("a fresh zero-byte coordinator stays on the locked path until it is stable", () => { + privateFile(coordinatorPath); + const fresh = codexWriteCoordinationEligibility({ + coordinatorPath: () => coordinatorPath, + residue: () => ({ kind: "residue" }), + integrationRecord: () => ({ kind: "missing" }), + nowMs: () => Date.now(), + }); + expect(fresh).toEqual({ kind: "coordinated" }); + + const settled = codexWriteCoordinationEligibility({ + coordinatorPath: () => coordinatorPath, + residue: () => ({ kind: "residue" }), + integrationRecord: () => ({ kind: "missing" }), + nowMs: () => Date.now() + STABLE_ZERO_BYTE_COORDINATOR_AGE_MS + 1, + }); + expect(settled.kind).toBe("legacy-uncoordinated"); +}); diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 5603137bd2..9054d5bacf 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -8,9 +8,14 @@ */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { + resolveCodexCoordinatorDatabasePath, + resolveEffectiveUserIdentity, +} from "../src/codex/user-identity"; +import { STABLE_ZERO_BYTE_COORDINATOR_AGE_MS } from "../src/codex/inject-coordination"; const repoRoot = join(import.meta.dir, ".."); const CHILD = join(repoRoot, "tests", "helpers", "codex-inject-race-child.ts"); @@ -20,6 +25,7 @@ let root = ""; let codexHome = ""; let opencodexHome = ""; const cleanup: string[] = []; +const coordinatorCleanup: string[] = []; function seedNative(): void { writeFileSync(join(codexHome, "config.toml"), 'model = "gpt-5"\n'); @@ -50,6 +56,12 @@ beforeEach(() => { }); afterEach(() => { + while (coordinatorCleanup.length) { + const path = coordinatorCleanup.pop()!; + for (const suffix of ["", "-journal", "-wal", "-shm"]) { + rmSync(`${path}${suffix}`, { force: true }); + } + } while (cleanup.length) { const dir = cleanup.pop()!; // `force` covers a missing path, not a locked one: a child that is still exiting @@ -184,6 +196,35 @@ describe("homes the coordinator cannot adopt keep working", () => { expect(result.success).toBeTrue(); expect(readFileSync(join(codexHome, "config.toml"), "utf-8")).toContain("openai_base_url"); }); + + test("a zero-byte coordinator remnant does not wedge a pre-substrate routed home", () => { + writeFileSync(join(codexHome, "config.toml"), [ + 'model_provider = "opencodex"', + 'model = "gpt-5.5"', + "", + "[model_providers.opencodex]", + 'name = "OpenCodex Proxy"', + 'base_url = "http://127.0.0.1:10100/v1"', + 'wire_api = "responses"', + "", + ].join("\n")); + const coordinatorPath = resolveCodexCoordinatorDatabasePath( + resolveEffectiveUserIdentity(), + realpathSync.native(codexHome), + ); + coordinatorCleanup.push(coordinatorPath); + writeFileSync(coordinatorPath, ""); + if (process.platform !== "win32") chmodSync(coordinatorPath, 0o600); + // Fresh zero-byte files remain on the coordinated path because they may + // belong to a live SQLite creator. This fixture represents an old remnant. + Bun.sleepSync(STABLE_ZERO_BYTE_COORDINATOR_AGE_MS + 100); + + const result = runInject(10100); + + expect(result.success).toBeTrue(); + expect(readFileSync(join(codexHome, "config.toml"), "utf-8")).toContain("openai_base_url"); + expect(readFileSync(coordinatorPath)).toHaveLength(0); + }); }); describe("the transition is resolved, not left pending", () => { From 7317dde30d53bc0a84f1acc27cbfbb176cbf3641 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 22 Aug 2026 00:05:09 +0900 Subject: [PATCH 2/4] =?UTF-8?q?devlog:=20bug=20merge-train=20roadmap=20(26?= =?UTF-8?q?0821)=20=E2=80=94=20triage,=20dependency=20analysis,=20audited?= =?UTF-8?q?=20disposition=20order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../000_triage_matrix.md | 27 +++++++++++++ .../001_dependency_analysis.md | 40 +++++++++++++++++++ .../002_audit_synthesis.md | 20 ++++++++++ .../010_fix_dev_macos_ci.md | 7 ++++ .../260821_bug_merge_train/020_merge_2295.md | 4 ++ .../260821_bug_merge_train/030_merge_2294.md | 4 ++ .../260821_bug_merge_train/040_merge_2296.md | 5 +++ .../260821_bug_merge_train/050_merge_2289.md | 4 ++ .../260821_bug_merge_train/060_merge_2270.md | 4 ++ .../260821_bug_merge_train/065_merge_2281.md | 4 ++ .../260821_bug_merge_train/070_final_gate.md | 8 ++++ 11 files changed, 127 insertions(+) create mode 100644 devlog/_plan/260821_bug_merge_train/000_triage_matrix.md create mode 100644 devlog/_plan/260821_bug_merge_train/001_dependency_analysis.md create mode 100644 devlog/_plan/260821_bug_merge_train/002_audit_synthesis.md create mode 100644 devlog/_plan/260821_bug_merge_train/010_fix_dev_macos_ci.md create mode 100644 devlog/_plan/260821_bug_merge_train/020_merge_2295.md create mode 100644 devlog/_plan/260821_bug_merge_train/030_merge_2294.md create mode 100644 devlog/_plan/260821_bug_merge_train/040_merge_2296.md create mode 100644 devlog/_plan/260821_bug_merge_train/050_merge_2289.md create mode 100644 devlog/_plan/260821_bug_merge_train/060_merge_2270.md create mode 100644 devlog/_plan/260821_bug_merge_train/065_merge_2281.md create mode 100644 devlog/_plan/260821_bug_merge_train/070_final_gate.md diff --git a/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md b/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md new file mode 100644 index 0000000000..728e503a51 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md @@ -0,0 +1,27 @@ +# 000 — Bug merge-train triage matrix (2026-08-21) + +Session: 01a024bb-1acb-7633-908b-29e4fe4d96c5 (worktree a6a7, detached at c0cbe494e). +Objective: drive the six open bug-labeled PRs to merged on `dev` with strict review, +adversarial xai/grok-4.6 subagent verdicts, and a final green dev CI gate. + +## In-scope PRs (state as of 2026-08-21T14:30Z) + +| PR | Title | Head | Behind dev | Draft | CI on head | Existing review state | +|----|-------|------|-----------:|-------|------------|----------------------| +| #2294 | fix(release): reject credential-bearing SSH remotes | 71598fa45 (ingw/fix-release-ssh-credential-boundary, moved from 86ed0a46a — re-fetch before review) | 3 | yes | green (test 1-4/4 pass on prior head; re-verify) | none; security-review boundary (scripts/release.ts) — Draft on purpose | +| #2289 | fix(service): restart existing installs w/o re-register | 240fc9364 (fix/2287-service-restart) | 9 | yes | green incl. Service lifecycle | none; Closes #2287 | +| #2295 | fix(codex): recover zero-byte coordinator remnants | 6d5f0cf2c (ingw/fix-zero-byte-coordinator-2291) | 0 | yes | green | none; addresses #2291 | +| #2270 | fix(responses): apply_patch on routed Responses | 398b7ade4 (fix/apply-patch-routed-lowering) | 48 | yes | Ingwannu: two CHANGES_REQUESTED resolved on this head; third review says no remaining technical blocker | Linux shards green | +| #2281 | fix: call_id thought-signature replay for Claude Code | b31f3dbed (fix/claude-code-thought-signature-replay) | 50 | no (review-ready + hygiene-blocked label) | BLOCKED state | CodeRabbit minor: normalize promptCacheKey via anthropicSessionKeyFromParts before storing as clientThreadId (core.ts ~1888-1896); lidge-jun review priority 63/80 confirms repro | +| #2296 | fix(codex): bind Desktop reconnects to one pool account | 574cadc86 (ingw/fix-app-pool-affinity-2046) | 0 | yes | green (test shards pass; one cancelled enforce-target) | none; addresses #2046 reconnect rotation only | + +## Baseline dev CI status (pre-train blocker) + +Run 32486877508 on dev head c0cbe494e: attempt 1 **failed** on +`(fail) multiAgentGuidanceText > the v2 default catalog path uses the request collector, not the synchronous one (#1852)` (macos job). Rerun of failed jobs (attempt 2) is **green** (conclusion: success), and the test passes locally at c0cbe494e (52/52). Cycle 1 exits as recorded flake per 010; no direct dev push needed. Watch for recurrence during the train. + +## Hygiene notes + +- #2281 carries `intake: hygiene-blocked` (missing_regression_test) despite having test files — the label state needs re-check after any new commit. +- #2281 is a first-time contributor PR; gate binds completion to exact head. New commits reset the checklist; since we (maintainer) will merge manually, that is acceptable. +- User authorized: stash/merge/cherry-pick/close/extra commits, push with --no-verify, suite on ssh lidge if needed, final CI green on dev is the exit gate. diff --git a/devlog/_plan/260821_bug_merge_train/001_dependency_analysis.md b/devlog/_plan/260821_bug_merge_train/001_dependency_analysis.md new file mode 100644 index 0000000000..1ba285b840 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/001_dependency_analysis.md @@ -0,0 +1,40 @@ +# 001 — Dependency and conflict analysis (r2, post-audit) + +Audit r1 (grok-4.6 "Avicenna") failed the initial order; accepted findings are folded in below. +Rejected findings and why: none rejected outright; the "2270 has no file overlap" observation was +accepted and 2270 moved before 2281 (it still sits after 2289 because its 48-behind rebase wants a +stable dev, and nothing else touches its files so waiting costs only one rebase, which it owes anyway). + +## File overlap between PR heads + +- **src/server/responses/core.ts**: #2281 (+12) and #2296 (+6/-9). Semantic neighborhood: _reasoningReplayScope creation (2281) vs pool-affinity key derivation (2296) both hang off handleResponsesInner request-context setup. +- **src/cli/registry.ts** + **docs .../reference/cli/lifecycle.md**: #2289 and #2295. Disjoint commands (service vs doctor); textual conflict likely trivial. +- **Runtime semantic risk without file overlap**: #2270's routed custom-tool lowering executes in the same request path as 2281's replay scope and 2296's affinity key. Post-merge full-suite runs after each of these three is the guard, plus a targeted cross-check at 2281/2296 time that replay-scope and lowering still compose (tests in tests/responses-custom-tool-repair.test.ts + tests/claude-code-thought-signature-scope.test.ts both green on the merged tree). +- All other files disjoint. + +## Disposition order (r2 — least-rebase, lock-current-first) + +1. **CI fix**: restore dev green (multiAgentGuidanceText #1852 macos failure; rerun already green — confirm and root-cause flakiness). +2. **#2295** (0 behind, green head CI, no rebase owed; lands registry.ts/lifecycle.md first so #2289 absorbs the conflict in the rebase it already owes). +3. **#2294** (3 behind, tiny, no overlap; NAMED SECURITY REVIEW GATE — see below). +4. **#2296** (0 behind; lock core.ts while its base is current; C4 auth — NAMED SECURITY REVIEW GATE; cancelled enforce-target check must be re-run green on the pre-merge head). +5. **#2289** (9 behind; rebase absorbs 2295's registry/lifecycle hunks; Service lifecycle CI green required). +6. **#2270** (48 behind; no file overlap with anything above; single rebase onto stable dev; full suite on the rebased head BEFORE merge). +7. **#2281** (50 behind; takes the core.ts conflict on rebase as the last mover; pre-merge blockers below). + +## Named gates (merge-blocking, not notes) + +- **Security review gate (#2294, #2296)**: per MAINTAINERS.md/AGENTS.md these surfaces (release automation; auth/account binding) require explicit security review. The maintainer (this session, acting for the owner account) performs and RECORDS a written security review in the cycle doc: threat cases checked, rejection matrix, log-boundary check (no token/secret in output), before merge. The grok-4.6 adversarial verdict is additive, not the security review itself. +- **Pre-merge CI-on-head gate (all)**: merge only from a head whose CI (or local full suite for shared-surface PRs: #2270, #2281, #2296) is green ON THE REBASED HEAD, not a stale ancestor. Cancelled/skipped required checks are re-run, not ignored. +- **#2281 pre-merge blockers**: (a) stacked commit normalizing promptCacheKey via anthropicSessionKeyFromParts (CodeRabbit finding) + test rows; (b) hygiene label missing_regression_test resolved — the PR does carry tests, so re-trigger the deterministic check after the stacked commit and confirm the label drops, or record the maintainer override rationale; (c) rebase onto final-form dev; (d) full suite green on that head. +- **Post-merge dev CI check after EVERY merge** before starting the next cycle (train stops on red). + +## Merge mechanics per PR + +fetch pr/N -> read full diff (AGENTS.md review rules) -> rebase onto current dev if behind -> focused tests + typecheck -> FULL SUITE (bun run test) pre-merge for every non-trivial PR (AGENTS.md bar; ssh lidge if local env-limited) -> grok-4.6 adversarial verdict -> security review doc where gated -> stack fix commits if needed. Head remotes: #2294/#2295/#2296/#2289 are in-repo branches (push origin); #2270 head is olddonkey/opencodex, #2281 head is Hsia97/opencodex, both maintainerCanModify=true -> push https://github.com//opencodex.git HEAD: (--no-verify is a local-hook flag). Then merge to dev (merge commit convention) -> push --no-verify -> dev CI green -> next. #2270 extra: dismiss/refresh the stale CHANGES_REQUESTED review so reviewDecision matches the converged head. + +## Issue closure map + +- #2287 -> close after #2289 lands (manual, base is dev). +- #2291 -> close after #2295 lands. +- #2046 -> #2296 fixes reconnect-rotation only; comment with landing commit; keep open unless the remaining Desktop-UI half is split into its own issue at wp6 D. diff --git a/devlog/_plan/260821_bug_merge_train/002_audit_synthesis.md b/devlog/_plan/260821_bug_merge_train/002_audit_synthesis.md new file mode 100644 index 0000000000..ca457a0a11 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/002_audit_synthesis.md @@ -0,0 +1,20 @@ +# 002 — Audit synthesis (round 1 -> round 2) + +Reviewers: Avicenna (grok-4.6, plan-shape audit) FAIL; Hegel (grok-4.6, deep repo audit) FAIL. + +## Accepted (folded into r3 docs) +1. Order rework (Avicenna): CI -> 2295 -> 2294 -> 2296 -> 2289 -> 2270 -> 2281. Adopted in 001 r2 and decade docs 020-065. +2. Fork mechanics (Hegel): #2270 head lives on olddonkey/opencodex, #2281 on Hsia97/opencodex, both maintainerCanModify=true — verified via gh. Stacked commits to those heads push to the FORK remote (https://github.com//opencodex.git :), enabled by maintainerCanModify; --no-verify applies locally. 001 mechanics corrected. +3. Full-suite bar (both): bun run typecheck + bun run test required before approving ANY non-trivial PR (AGENTS.md:178 area); full suite explicitly pre-merge for #2281/#2289/#2295 too, not only 2270/2296. Decade docs updated. +4. #2294 gates (Hegel): add bun run prepush (scripts/AGENTS.md), and record the non-author maintainer review — author is Ingwannu; the merging maintainer account (lidge-jun) supplies the non-author security APPROVE, satisfying MAINTAINERS.md no-self-approval. +5. #2270 stale CHANGES_REQUESTED (Hegel): reviewDecision still CHANGES_REQUESTED although the same reviewer's later comment on exact head 398b7ade4 says no remaining technical blocker. Pre-merge step: dismiss the stale review with rationale (or fresh APPROVE) so the recorded decision matches the converged state. +6. #2294 head drift (Hegel): head moved 86ed0a46a -> 71598fa45; re-fetch and re-review at the new head. 000 corrected. +7. CI cycle-1 (Hegel): rerun attempt 2 green + local 52/52 pass -> exit as flake (010 rewritten); no direct dev push. +8. Docs-sync (Hegel): after both 2295 (en-only doctor docs) and 2289 (8-locale lifecycle) land, verify locales do not contradict the English lifecycle page; added to 070. +9. CODEOWNERS/owner review for core.ts PRs (Hegel): lidge-jun review recorded at 040/065 merge time. + +## Rejected (with evidence) +1. "#2270 already collides with intervening dev on src/providers/registry.ts" (Hegel): git merge-tree merge-base(origin/dev, pr/2270) shows 0 conflict markers; same for pr/2281. Rebase risk is semantic, not textual; covered by full suite on rebased head. +2. "#2281 hygiene failure is unsponsored_surface" (Hegel): latest pr-hygiene comment on #2281 says missing_regression_test (fetched via gh api). Treated per 065: re-trigger after stacked commit; drop or record maintainer override. +3. "#2296 cancelled enforce-target ignored" (Avicenna): not ignored — 040 requires it re-run green pre-merge. Kept. + diff --git a/devlog/_plan/260821_bug_merge_train/010_fix_dev_macos_ci.md b/devlog/_plan/260821_bug_merge_train/010_fix_dev_macos_ci.md new file mode 100644 index 0000000000..3ac7d089f7 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/010_fix_dev_macos_ci.md @@ -0,0 +1,7 @@ +# 010 — Cycle 1: dev CI status (resolved as flake) + +Evidence: +- Run 32486877508 (dev c0cbe494e) attempt 1: platform-macos failed on multiAgentGuidanceText #1852 test; attempt 2 (rerun --failed): conclusion success. +- Local repro at exact c0cbe494e: bun test tests/multi-agent-compat.test.ts -> 52 pass / 0 fail; paired with server-combo-failover-e2e -> 120 pass. +Exit: flake recorded; dev is green at c0cbe494e. No dev push. If the same test fails again during the train, escalate to root-cause mode (test reads catalog collector timing — suspect CI-runner timing sensitivity). + diff --git a/devlog/_plan/260821_bug_merge_train/020_merge_2295.md b/devlog/_plan/260821_bug_merge_train/020_merge_2295.md new file mode 100644 index 0000000000..4fbea69b55 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/020_merge_2295.md @@ -0,0 +1,4 @@ +# 020 — Cycle 2: PR #2295 (zero-byte coordinator, #2291) + +0 behind dev; lands first among PRs. Review: coordinator-doctor state machine (8 classifications), fail-closed defaults, doctor --recover-zero-byte-coordinator gating (proxy stopped + BEGIN IMMEDIATE + identity revalidation + backup-not-delete), no SQLite sidecar creation on diagnosis path, age-gate race reasoning. +Verify: bun test tests/codex-coordinator-doctor.test.ts tests/codex-inject-write-lock.test.ts tests/codex-transition-state*.test.ts tests/cli-doctor.test.ts tests/cli-dispatch.test.ts, bun run typecheck, bun run privacy:scan, FULL SUITE (bun run test) pre-merge. grok verdict. Merge, push --no-verify, dev CI green. Close #2291 with landing commit. diff --git a/devlog/_plan/260821_bug_merge_train/030_merge_2294.md b/devlog/_plan/260821_bug_merge_train/030_merge_2294.md new file mode 100644 index 0000000000..adc7e0b189 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/030_merge_2294.md @@ -0,0 +1,4 @@ +# 030 — Cycle 3: PR #2294 (release SSH credential boundary) + +NAMED SECURITY REVIEW GATE (scripts/release.ts). Written review in this doc before merge: userinfo rejection matrix (ssh:// password, encoded ':', scp-like user:pass@), control-char/query/fragment rejection, GIT_SSH_COMMAND single-literal '-i' proof, log-boundary check (accepted value printed pre-push — verify nothing secret-bearing can pass validation). +Head moved to 71598fa45 — re-fetch and review the live head. Verify: bun test tests/release-helper.test.ts, bun run typecheck, bun run privacy:scan, bun run prepush (scripts/AGENTS.md bar for release tooling). Non-author security review: author is Ingwannu; merging maintainer (lidge-jun) records the security APPROVE (no self-approval). grok verdict. Merge, push --no-verify, dev CI green. diff --git a/devlog/_plan/260821_bug_merge_train/040_merge_2296.md b/devlog/_plan/260821_bug_merge_train/040_merge_2296.md new file mode 100644 index 0000000000..22e88a61f4 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/040_merge_2296.md @@ -0,0 +1,5 @@ +# 040 — Cycle 4: PR #2296 (Desktop reconnect pool affinity, #2046) + +C4 auth surface — NAMED SECURITY REVIEW GATE: HMAC fallback key non-persistence + non-correlatability across restarts, no raw session/thread-id storage or logging (privacy:scan + manual grep), account-qualified selector exclusion from automatic affinity, failover/terminal accounting carries the same key. +Cancelled enforce-target check on head must re-run green pre-merge. Verify: bun test tests/codex-auth-context.test.ts, typecheck, privacy:scan, FULL SUITE on head (shared server surface). grok verdict. Merge, push --no-verify, dev CI green. Comment on #2046 (rotation half fixed; UI-denial half remains). + diff --git a/devlog/_plan/260821_bug_merge_train/050_merge_2289.md b/devlog/_plan/260821_bug_merge_train/050_merge_2289.md new file mode 100644 index 0000000000..d754e7c3db --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/050_merge_2289.md @@ -0,0 +1,4 @@ +# 050 — Cycle 5: PR #2289 (service restart, closes #2287) + +Rebase (9 behind) absorbs #2295's registry.ts/lifecycle.md hunks. Review: bare 'ocx service' idempotency, repair/restart alias routing (src/service.ts, src/cli/registry.ts), Windows WINDOWS_SCHTASKS_CREATE_ACCESS_DENIED path, 8-locale docs consistency. +Verify: bun test tests/cli-help.test.ts tests/service.test.ts tests/winsw.test.ts, bun run typecheck, FULL SUITE (bun run test) pre-merge; Service lifecycle CI green on head. grok verdict. Merge, push --no-verify, dev CI green. Close #2287 with landing commit. diff --git a/devlog/_plan/260821_bug_merge_train/060_merge_2270.md b/devlog/_plan/260821_bug_merge_train/060_merge_2270.md new file mode 100644 index 0000000000..26fa79e3db --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/060_merge_2270.md @@ -0,0 +1,4 @@ +# 060 — Cycle 6: PR #2270 (apply_patch routed lowering) + +48 behind; single rebase onto now-stable dev. Preserve the !isCanonicalOpenAiForwardProvider boundary (already on head 398b7ade4; maintainer review r3 found no remaining technical blocker). Review: supportsResponsesCustomTools capability plumbing (registry/derive/types), compaction-body-last reorder invariant, byte-identical non-compaction pin test. +Fork head (olddonkey/opencodex, maintainerCanModify=true): stacked commits push to the fork remote. Pre-merge: dismiss stale CHANGES_REQUESTED (converged per reviewer's own head-398b7ade4 comment) or record fresh APPROVE. Verify on REBASED head BEFORE merge: bun test tests/custom-tool-compat.test.ts tests/namespace-tool-compat.test.ts tests/openai-responses-passthrough.test.ts tests/responses-custom-tool-repair.test.ts, bun run typecheck, FULL SUITE (shared routing/adapter surface; ssh lidge if local env-limited). grok verdict. Merge, push --no-verify, dev CI green. diff --git a/devlog/_plan/260821_bug_merge_train/065_merge_2281.md b/devlog/_plan/260821_bug_merge_train/065_merge_2281.md new file mode 100644 index 0000000000..67374542f1 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/065_merge_2281.md @@ -0,0 +1,4 @@ +# 065 — Cycle 7: PR #2281 (thought-signature replay, last mover) + +Takes the core.ts rebase conflict deliberately. Pre-merge blockers (ALL merge-blocking): (a) stacked commit: normalize promptCacheKey via anthropicSessionKeyFromParts before assigning clientThreadId (src/server/responses/core.ts ~1888-1896; helper at src/oauth/anthropic-routing.ts:573-594) + trimmed/overlong-key test rows; (b) missing_regression_test hygiene label re-checked after stacked commit — drop or record maintainer override; (c) rebase onto final dev, resolve core.ts against #2296's affinity changes with a semantic re-check (replay scope + affinity key compose; both test files green on merged tree); (d) FULL SUITE green on that head. +Fork head (Hsia97/opencodex, maintainerCanModify=true): stacked commits push to the fork remote. Also: reviewDecision is CHANGES_REQUESTED (lidge-jun priority-63 review) — the stacked fixes must answer that review, then refresh/dismiss it. Verify: bun test tests/claude-code-thought-signature-scope.test.ts tests/google-signature-history-roundtrip.test.ts, bun run typecheck, FULL SUITE. Owner (CODEOWNERS core.ts) review recorded at merge. grok verdict. Merge, push --no-verify, dev CI green. diff --git a/devlog/_plan/260821_bug_merge_train/070_final_gate.md b/devlog/_plan/260821_bug_merge_train/070_final_gate.md new file mode 100644 index 0000000000..595c30f123 --- /dev/null +++ b/devlog/_plan/260821_bug_merge_train/070_final_gate.md @@ -0,0 +1,8 @@ +# 070 — Cycle 7: final gate + +1. Confirm final dev head CI fully green (gh run list --branch dev; the ci aggregate job). +1b. Docs-sync check: after 2295 (en-only doctor docs) + 2289 (8-locale lifecycle) both land, confirm locale lifecycle pages do not contradict the English page (AGENTS.md docs-sync rule). +2. If macos/windows shard flakes, rerun; if real regression from the train, fix forward on dev. +3. Close remaining linked issues with landing-commit comments (#2287, #2291, #2046 decision). +4. Move devlog unit to _fin with terminal outcomes recorded per PR. +5. Goalplan criteria capturedEvidence filled; cxc loop validate green; update_goal complete. From 7f00202d429d96a7e7ecaf13a19e716fa282c4d9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 22 Aug 2026 07:23:10 +0900 Subject: [PATCH 3/4] =?UTF-8?q?devlog:=202295=20cycle=20=E2=80=94=20full-s?= =?UTF-8?q?uite=20rerun=20green=20after=20gui=20deps=20fix=20(14175=20pass?= =?UTF-8?q?=20/=200=20fail,=20lidge)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../260821_bug_merge_train/020_merge_2295.md | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/devlog/_plan/260821_bug_merge_train/020_merge_2295.md b/devlog/_plan/260821_bug_merge_train/020_merge_2295.md index 4fbea69b55..68e5f87cb5 100644 --- a/devlog/_plan/260821_bug_merge_train/020_merge_2295.md +++ b/devlog/_plan/260821_bug_merge_train/020_merge_2295.md @@ -2,3 +2,45 @@ 0 behind dev; lands first among PRs. Review: coordinator-doctor state machine (8 classifications), fail-closed defaults, doctor --recover-zero-byte-coordinator gating (proxy stopped + BEGIN IMMEDIATE + identity revalidation + backup-not-delete), no SQLite sidecar creation on diagnosis path, age-gate race reasoning. Verify: bun test tests/codex-coordinator-doctor.test.ts tests/codex-inject-write-lock.test.ts tests/codex-transition-state*.test.ts tests/cli-doctor.test.ts tests/cli-dispatch.test.ts, bun run typecheck, bun run privacy:scan, FULL SUITE (bun run test) pre-merge. grok verdict. Merge, push --no-verify, dev CI green. Close #2291 with landing commit. + +## Review round 1 (Volta, grok-4.6) — FAIL — synthesis + +Finding 1 (age gate bypasses lock): ACCEPTED AS RESIDUAL RISK, REBUTTED AS BLOCKER. +RCA: a creator stalled >1s between file creation and BEGIN IMMEDIATE is classified stable-zero-byte. +But the consequence is exactly the ENOENT behavior: clean homes still enter the coordinated path +(inject-coordination.ts:96-99 comment + code — the SQLite transaction safely initializes the same file, +still serialized by the lock); ONLY residue/indeterminate legacy homes take legacy-uncoordinated, which +is the identical compatibility boundary those homes used for years pre-coordination and would use today +if the remnant pathname were absent. The trade fixes #2291 (zero-byte blocks sync forever, fail-closed +with no operator exit). Residual: legacy-residue home + creator stalled >1s + concurrent write — +accepted; the alternative is the unfixable wedge this PR exists to remove. + +Finding 2 (recovery rename TOCTOU): REBUTTED AS BLOCKER. +RCA: window between final sameIdentity check (coordinator-doctor.ts:306) and renameSync (:312) allows a +same-uid attacker to swap a file that then gets MOVED (not deleted) to a same-directory backup. +The namespace is 0o700/owner-checked and the file 0o600/owner-checked (inspectTarget); only the same +user can race it. Per AGENTS.md's own boundary statement, a same-user local process is outside the +enforceable threat model (it can already rename these files itself). Recovery is opt-in (--yes), +proxy-stopped, and evidence-preserving. Non-blocking. + +Finding 3 (fail-open vs dev): REBUTTED. +RCA: on dev, an existing zero-byte coordinator stayed "coordinated" and then wedged sync (issue #2291's +literal symptom). The PR routes only proven (zero bytes + user_version 0 + no tables via immutable read ++ 1s settled identity) remnants to the absent-file boundary. unversioned-nonempty / rowless / +unsupported / changed / unsafe all remain fail-closed. This is the intended fix, not an accident. + +Disposition: proceed to merge; findings 1-2 recorded as accepted residual risks in this doc. +Focused tests 55/55, typecheck pass, privacy:scan pass, full suite pending (bg session). + +## Verification close-out (train head 728ca1e8b) + +Full suite re-run on lidge after completing the temporary worktree's gui +dependency install: the first run's 7 failures were all "Unhandled error +between tests: Cannot find package 'react'" (gui/src/i18n/shared.ts and +friends) — an incomplete `gui/node_modules` environment artifact, not test +logic. After `bun install --cwd gui` on the same commit: **14175 pass / +16 skip / 0 fail across 890 files (464.61s), exit 0** +(/tmp/ocx-train-suite-r2.log on lidge). Locally, the same four representative +files that hit the missing-package path pass 55/55 after the identical fix. +Merge-blocker verdict stands; accepted residuals unchanged. Train branch is +ready to land on dev. From 584a3e3e592eda8fffb7984c239c44f50c723904 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 22 Aug 2026 07:24:14 +0900 Subject: [PATCH 4/4] =?UTF-8?q?devlog:=20triage=20matrix=20=E2=80=94=20mar?= =?UTF-8?q?k=20#2295=20merged=20on=20the=20train?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- devlog/_plan/260821_bug_merge_train/000_triage_matrix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md b/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md index 728e503a51..4154e2854a 100644 --- a/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md +++ b/devlog/_plan/260821_bug_merge_train/000_triage_matrix.md @@ -10,7 +10,7 @@ adversarial xai/grok-4.6 subagent verdicts, and a final green dev CI gate. |----|-------|------|-----------:|-------|------------|----------------------| | #2294 | fix(release): reject credential-bearing SSH remotes | 71598fa45 (ingw/fix-release-ssh-credential-boundary, moved from 86ed0a46a — re-fetch before review) | 3 | yes | green (test 1-4/4 pass on prior head; re-verify) | none; security-review boundary (scripts/release.ts) — Draft on purpose | | #2289 | fix(service): restart existing installs w/o re-register | 240fc9364 (fix/2287-service-restart) | 9 | yes | green incl. Service lifecycle | none; Closes #2287 | -| #2295 | fix(codex): recover zero-byte coordinator remnants | 6d5f0cf2c (ingw/fix-zero-byte-coordinator-2291) | 0 | yes | green | none; addresses #2291 | +| #2295 | fix(codex): recover zero-byte coordinator remnants | 6d5f0cf2c (ingw/fix-zero-byte-coordinator-2291) | 0 | yes | green | MERGED to train 728ca1e8b; suite green; landing on dev | | #2270 | fix(responses): apply_patch on routed Responses | 398b7ade4 (fix/apply-patch-routed-lowering) | 48 | yes | Ingwannu: two CHANGES_REQUESTED resolved on this head; third review says no remaining technical blocker | Linux shards green | | #2281 | fix: call_id thought-signature replay for Claude Code | b31f3dbed (fix/claude-code-thought-signature-replay) | 50 | no (review-ready + hygiene-blocked label) | BLOCKED state | CodeRabbit minor: normalize promptCacheKey via anthropicSessionKeyFromParts before storing as clientThreadId (core.ts ~1888-1896); lidge-jun review priority 63/80 confirms repro | | #2296 | fix(codex): bind Desktop reconnects to one pool account | 574cadc86 (ingw/fix-app-pool-affinity-2046) | 0 | yes | green (test shards pass; one cancelled enforce-target) | none; addresses #2046 reconnect rotation only |