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
29 changes: 25 additions & 4 deletions src/codex/history-state-open.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,23 @@ export function isStateDbCantOpenError(error: unknown): boolean {
return code === "SQLITE_CANTOPEN" || message.includes("unable to open database file");
}

/** Which step of the primary attempt a test wants to fail. */
export type StateDbPreflightOpenPhase = "open" | "first-read";

/**
* Test-only knob: force the primary read-only open to fail with a supplied error.
* Test-only knob: force the primary attempt to fail with a supplied error.
*
* The fallback below turns on a condition this repository cannot reproduce deterministically
* from a test: whether a plain read-only open of a cleanly-closed WAL store fails or quietly
* creates the `-shm` depends on the platform VFS and on the directory the store sits in.
* Pinning the NARROWING — sidecars absent admits the immutable read, either sidecar present
* still refuses — therefore needs the failure supplied rather than provoked, or the test would
* assert the host's SQLite build instead of this decision (#4943).
*
* The phase exists because the platforms disagree about WHEN the condition is raised, not only
* about whether it is: see the first-read note on `openCodexStateForPreflight`.
*/
let openFailureForTests: ((path: string) => unknown) | undefined;
let openFailureForTests: ((path: string, phase: StateDbPreflightOpenPhase) => unknown) | undefined;
export function setStateDbPreflightOpenFailureForTests(hook: typeof openFailureForTests): void {
openFailureForTests = hook;
}
Expand Down Expand Up @@ -68,13 +74,28 @@ export function setStateDbPreflightOpenFailureForTests(hook: typeof openFailureF
* error and the refusal that follows from it — a `-wal` holds content this connection would
* not read, and a `-shm` means a writer is attached, and neither is a store this preflight
* may inspect from a snapshot.
*
* The first read belongs INSIDE this attempt. `sqlite3_open_v2` does not touch page 1, so a
* store whose header says WAL is not inspected until the first prepare — which is where the
* missing shared memory is discovered on macOS, one caller frame above this function. Opening
* here and reading there put the classification and the failure in different scopes: the
* fallback was never reached, and the operator got the catch-all refusal the fix was supposed
* to remove. Linux hides this because its SQLite materializes the sidecars on that first read
* and never fails at all (#4943, macOS CI).
*/
export function openCodexStateForPreflight(resolvedPath: string): Database {
let db: Database | undefined;
try {
const forced = openFailureForTests?.(resolvedPath);
const forced = openFailureForTests?.(resolvedPath, "open");
if (forced) throw forced;
return new Database(resolvedPath, { readonly: true });
db = new Database(resolvedPath, { readonly: true });
const forcedRead = openFailureForTests?.(resolvedPath, "first-read");
if (forcedRead) throw forcedRead;
// Page 1, read while the failure is still this function's to classify.
db.query<{ tables: number }, []>("SELECT count(*) AS tables FROM sqlite_master").get();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update every structure document mapped to src/codex

This changes behavior in src/codex/, but the commit updates only structure/codex-home.md; structure/INDEX.md also maps this area to runtime.md, config.md, catalog.md, subagents.md, providers/openai-tiers.md, gui-and-management-api.md, and ops/docs-and-release.md. Review and update each mapped document in this change so their duplicated history-writer contract summaries remain synchronized, as required for changes to an owned source area.

AGENTS.md reference: src/AGENTS.md:L11-L11

Useful? React with 👍 / 👎.

return db;
} catch (error) {
db?.close();
if (!isStateDbCantOpenError(error)) throw error;
if (existsSync(`${resolvedPath}-wal`) || existsSync(`${resolvedPath}-shm`)) throw error;
// pathToFileURL percent-encodes the reserved characters a naive `file:${path}` would
Expand Down
2 changes: 1 addition & 1 deletion structure/codex-home.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ management read degrades instead of returning an error page.

Injection preflights affected history using the normalized config candidate before writing config/profile/journal, then checks again after the complete artifact write. Native restore also rechecks after successful journal restoration or fallback removal, while exact config/profile/journal preimages and any coordinated remove transaction remain available for compensation.

The preflight opens the state store read-write-free and in that order deliberately. `{ readonly: true }` is the primary open and the only one that joins a live writer's WAL shared memory, so a thread another process just migrated to paginated history is visible and refuses here. A WAL store whose last writer closed cleanly has no `-shm` to join and a read-only connection may not create one, so that open fails `SQLITE_CANTOPEN` on a perfectly healthy store and the catch-all turned it into `history_injection_preflight_unavailable` on every attempt (#4943). The immutable fallback (`immutable=1` over a `file:` URI, the same idiom as the storage scanner and the log-guard inspector) is admitted only when neither `-wal` nor `-shm` is on disk, because that is the state in which the main database is the whole store and an immutable read is exact rather than stale. Either sidecar present, or any other open failure, keeps the original error and the refusal that follows: an immutable read is a snapshot, and a refusal this preflight fails to observe is a config transition over history Codex owns.
The preflight opens the state store read-write-free and in that order deliberately. `{ readonly: true }` is the primary open and the only one that joins a live writer's WAL shared memory, so a thread another process just migrated to paginated history is visible and refuses here. A WAL store whose last writer closed cleanly has no `-shm` to join and a read-only connection may not create one, so that open fails `SQLITE_CANTOPEN` on a perfectly healthy store and the catch-all turned it into `history_injection_preflight_unavailable` on every attempt (#4943). The immutable fallback (`immutable=1` over a `file:` URI, the same idiom as the storage scanner and the log-guard inspector) is admitted only when neither `-wal` nor `-shm` is on disk, because that is the state in which the main database is the whole store and an immutable read is exact rather than stale. Either sidecar present, or any other open failure, keeps the original error and the refusal that follows: an immutable read is a snapshot, and a refusal this preflight fails to observe is a config transition over history Codex owns. The guard covers the whole primary attempt, not only the constructor. `sqlite3_open_v2` never reads page 1, so a WAL header is not inspected until the first prepare, and on macOS that is where the absent `-shm` is raised; the first read therefore happens inside the attempt, where the error can still be classified. Bun's bundled SQLite on Linux materializes both sidecars on that same read and never fails, which is why Linux and Windows evidence could not see this gap.

What a detected migration does depends on which refusal it is, and on direction. The reason that stands down is one exported constant, `HISTORY_RELABEL_STANDS_DOWN` in `src/codex/history-provider.ts`, because apply and restore have to agree on it exactly and once did not.

Expand Down
19 changes: 19 additions & 0 deletions tests/codex-integration/codex-history-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1719,4 +1719,23 @@ describe("Codex history injection preflight on a WAL store with no live writer",
setStateDbPreflightOpenFailureForTests(() => Object.assign(new Error("access to the database file is denied"), { code: "SQLITE_PERM" }));
expect(preflightCodexHistoryInjection(true, false, fixture.dbPath)).toBe("history_injection_preflight_unavailable");
});

test("covers the first read, which is where macOS raises the missing shared memory", () => {
// `sqlite3_open_v2` never reads page 1, so a WAL header is not inspected until the first
// prepare. On macOS that is where the absent `-shm` is discovered; the guarded attempt
// had already returned, the failure landed in the caller, and a healthy store got the
// catch-all refusal #4943 was supposed to remove. Linux cannot show this: its SQLite
// materializes both sidecars on that same read and never fails.
const fixture = checkpointedWalFixture();
const phases: string[] = [];
setStateDbPreflightOpenFailureForTests((_path, phase) => {
phases.push(String(phase));
return phase === "first-read" ? missingSharedMemory() : undefined;
});
// A failure raised at the first read has to reach the same verdict as one raised at open,
expect(preflightCodexHistoryInjection(true, false, fixture.dbPath)).toBeNull();
// and the attempt has to actually offer that phase: before this fix only "open" existed,
// which is exactly why the platform that fails later was not covered.
expect(phases).toEqual(["open", "first-read"]);
});
});
Loading