Skip to content

fix(codex): honor split SQLite state homes - #1425

Merged
Wibias merged 4 commits into
devfrom
agent/fix-codex-sqlite-home
Aug 11, 2026
Merged

fix(codex): honor split SQLite state homes#1425
Wibias merged 4 commits into
devfrom
agent/fix-codex-sqlite-home

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • resolve Codex SQLite state with Codex-compatible precedence: root sqlite_home in the effective config.toml, then CODEX_SQLITE_HOME, then the effective CODEX_HOME
  • make history provider defaults, history jobs, write admission, and native-residue checks address the same call-time state_5.sqlite
  • fail closed when the authoritative config is unreadable, malformed, or contains a present but blank/non-string sqlite_home; only an absent file or absent key may use the environment/home fallback
  • preserve the total read-only native-residue contract by converting an indeterminate SQLite authority into a config indeterminate result instead of throwing or selecting another database
  • preserve an explicit SQLite home in systemd, launchd, Task Scheduler, and WinSW service artifacts, including Windows drive/UNC paths rendered from non-Windows repair tooling
  • document the split-home boundary, backup identity, storage-cleanup non-authority, and decision rationale

Root cause

OpenCodex rebuilt CODEX_HOME/state_5.sqlite independently in several paths. Codex can place SQLite thread state in another root, so a Windows Desktop + WSL installation could make the history writer, admission snapshot, and residue check reason about a database different from the one Codex actually opened.

The initial resolver also used the tolerant root-string reader. That made a present invalid sqlite_home indistinguishable from an absent key and allowed an unsafe fallback. The strict resolver now distinguishes absent, valid, and invalid states without changing tolerant config readers used by unrelated injection/catalog paths.

The fix centralizes one call-time resolver and passes the selected database identity through the existing history job/worker boundary. It deliberately does not give storage cleanup authority over an external SQLite root.

User impact

History migration and provider synchronization now follow the active Codex SQLite database in split-home installations. Background services retain the explicit install-time SQLite destination instead of silently falling back to CODEX_HOME. Invalid or unreadable SQLite authority stops the operation rather than risking inspection or mutation of a stale database.

Verification

  • exact-head focused SQLite/history/admission/residue/service coverage: 198 passed, 0 failed
  • bun run typecheck: passed
  • bun run privacy:scan: passed
  • docs-site bun install --frozen-lockfile and production build: 221 pages
  • CPU-limited full-suite attempt before the residue-boundary follow-up: 10,929 passed, 11 skipped, 4 failed; the three failures caused by the new resolver were fixed and rerun green in the 198-test exact-head set, while the remaining codex-shim environment-token failure reproduces on clean dev
  • git diff --check: passed
  • refreshed GitHub CI on exact head ea2895046: pending

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Closes #1401

Summary by CodeRabbit

  • New Features

    • Added support for resolving Codex history data from a configured SQLite location.
    • Added precedence for configuration, environment settings, and the Codex home directory.
    • Preserved configured SQLite locations across macOS, Windows, and Linux service installations.
    • Added support for relative and Windows-specific paths.
  • Bug Fixes

    • Invalid or unreadable SQLite configuration now fails safely without selecting an unintended database.
  • Documentation

    • Documented SQLite history location resolution, fallback behavior, and retention boundaries.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8dac4345-7ee8-4686-b79b-52aec26bec85

📥 Commits

Reviewing files that changed from the base of the PR and between 9f545f2 and ea28950.

📒 Files selected for processing (13)
  • docs-site/src/content/docs/guides/codex-integration.md
  • src/codex/admission.ts
  • src/codex/history-job.ts
  • src/codex/history-provider.ts
  • src/codex/native-residue.ts
  • src/codex/paths.ts
  • src/lib/winsw.ts
  • src/service.ts
  • structure/02_config-and-codex-home.md
  • tests/codex-native-residue.test.ts
  • tests/codex-sqlite-home.test.ts
  • tests/service.test.ts
  • tests/winsw.test.ts

📝 Walkthrough

Walkthrough

The change adds Codex-compatible SQLite directory resolution, updates history and residue consumers to use it at call time, preserves CODEX_SQLITE_HOME in service launchers, and adds tests and documentation for precedence and fail-closed behavior.

Changes

Codex SQLite resolution

Layer / File(s) Summary
SQLite home resolver
src/codex/paths.ts
Adds resolveCodexSqliteHome and resolveCodexStateDbPath. Resolution uses sqlite_home, then CODEX_SQLITE_HOME, then CODEX_HOME. Relative paths use the working directory. Invalid authoritative configuration fails closed.
History consumer integration
src/codex/admission.ts, src/codex/history-job.ts, src/codex/history-provider.ts, src/codex/native-residue.ts
History discovery, synchronization, migration, restoration, pending-history inspection, and residue checks use the shared resolver. History defaults resolve at call time.
Service environment propagation
src/service.ts, src/lib/winsw.ts
launchd, Windows services, systemd, and WinSW preserve normalized CODEX_SQLITE_HOME values.
Resolution coverage and documentation
tests/codex-sqlite-home.test.ts, tests/codex-native-residue.test.ts, tests/service.test.ts, tests/winsw.test.ts, docs-site/src/content/docs/guides/codex-integration.md, structure/02_config-and-codex-home.md
Tests cover precedence, parsing failures, relative paths, dynamic history paths, residue classification, and service output. Documentation records the resolution rules and cleanup boundary.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ServiceLauncher
  participant CodexConfig
  participant resolveCodexStateDbPath
  participant HistoryProvider
  participant SQLiteDatabase
  ServiceLauncher->>ServiceLauncher: preserve CODEX_SQLITE_HOME
  HistoryProvider->>resolveCodexStateDbPath: resolve state database at call time
  resolveCodexStateDbPath->>CodexConfig: read sqlite_home
  resolveCodexStateDbPath-->>HistoryProvider: return state_5.sqlite path
  HistoryProvider->>SQLiteDatabase: read or mutate history
Loading

Possibly related PRs

  • lidge-jun/opencodex#1106: Overlaps with history handling in history-job.ts and history-provider.ts, but addresses failure-result behavior rather than SQLite path resolution.

Suggested labels: review-ready

Suggested reviewers: lidge-jun, wibias

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/fix-codex-sqlite-home

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Wibias Wibias left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Request changes based on a full review of the current head.

Merge blocker:

resolveCodexSqliteHome() treats every config.toml read/parse failure as if no authoritative sqlite_home existed, then falls through to CODEX_SQLITE_HOME or CODEX_HOME. That is unsafe for the exact data-integrity problem this PR is fixing. ENOENT means the config is absent and fallback is valid; EACCES, EIO, ENOTDIR, or another read failure means OpenCodex does not know which database Codex selected. Falling back in that state can make history/admission/native-residue operations inspect or mutate a stale or unrelated state_5.sqlite.

Please fail closed for unreadable/indeterminate authoritative config while allowing the genuine-missing (ENOENT) case to fall through. Add regressions for at least:

  • missing config -> environment fallback is allowed;
  • unreadable config (for example EACCES) -> SQLite-home resolution fails closed and no fallback DB is selected.

The overall resolver centralization and service propagation look sound; I did not find a conventional credential or privilege-escalation issue. After the fix, refresh onto current dev and rerun exact-head CI.

@Ingwannu
Ingwannu force-pushed the agent/fix-codex-sqlite-home branch from 151d27d to 3abca61 Compare August 11, 2026 04:48
@Ingwannu

Copy link
Copy Markdown
Owner Author

Addressed the fail-closed blocker on the current head 3abca61fc and rebased the branch onto dev 7779c0574.

  • only a genuine missing config.toml (ENOENT) may fall through to CODEX_SQLITE_HOME / CODEX_HOME;
  • unreadable or indeterminate authoritative config now stops SQLite-home resolution instead of selecting a potentially unrelated database;
  • focused regressions cover both the missing-config fallback and EACCES fail-closed path;
  • the architecture and user integration docs now state this boundary.

Verification on the rebased head:

  • bun test tests/codex-sqlite-home.test.ts tests/service.test.ts tests/winsw.test.ts — 130 passed, 0 failed
  • bun run typecheck — passed
  • bun run privacy:scan — passed
  • docs-site production build — 221 pages
  • git diff --check — passed

@Wibias please re-review the exact head after CI completes.

@Ingwannu
Ingwannu requested a review from Wibias August 11, 2026 04:49

@Wibias Wibias left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Request changes after re-review of the current head 3abca61fc34059d83fb9d7861c2cee3a16bffd48.

The previous unreadable-config blocker is fixed: genuine ENOENT may fall through, while read failures such as EACCES now fail closed. There is still one remaining data-integrity edge in the same contract.

resolveCodexSqliteHome() uses readRootTomlString() to discover sqlite_home. That helper only recognises a quoted string assignment and returns null otherwise. As a result, an existing authoritative config such as:

sqlite_home = 123

or a malformed sqlite_home assignment is indistinguishable from the key being absent. resolveCodexSqliteHome() then falls through to CODEX_SQLITE_HOME or CODEX_HOME, potentially selecting a different state_5.sqlite even though the config exists but its authoritative SQLite setting cannot be established.

For history mutation/admission/residue safety this should remain fail-closed. Please distinguish these states:

  • sqlite_home absent -> environment/home fallback allowed;
  • valid string -> use it;
  • sqlite_home present but malformed/wrong type, or otherwise indeterminate -> fail closed and do not select a fallback database.

Add a regression proving at minimum that sqlite_home = 123 with CODEX_SQLITE_HOME set throws instead of returning the environment database. Malformed-string coverage would also be useful.

After that fix, refresh onto current dev and rerun complete exact-head CI.

@Ingwannu
Ingwannu force-pushed the agent/fix-codex-sqlite-home branch from 3abca61 to 5c15c37 Compare August 11, 2026 05:44
@Ingwannu

Copy link
Copy Markdown
Owner Author

Addressed the remaining SQLite-authority blocker and the full-suite residue regression on exact head ea2895046 (rebased on current dev 9f545f204).

  • SQLite authority now uses a strict TOML parse scoped only to resolveCodexSqliteHome(): absent file/key may fall back, a valid non-empty string is used, and malformed/wrong-typed/blank values fail closed.
  • Regressions cover numeric, array, blank, malformed, and valid quoted-key forms with an environment fallback present.
  • The tolerant root-string reader used by injection/catalog paths is unchanged.
  • The full suite exposed that native-residue classification let the new fail-closed exception escape. That read-only boundary now converts the condition to config indeterminate, so it still refuses coordination without throwing or selecting a fallback database. A dedicated regression covers invalid sqlite_home there.

Verification on the final code:

  • bun test tests/codex-native-residue.test.ts tests/codex-sqlite-home.test.ts tests/service.test.ts tests/winsw.test.ts — 198 passed, 0 failed
  • bun run typecheck — passed
  • bun run privacy:scan — passed
  • docs production build — 221 pages
  • git diff --check — passed

The CPU-limited full-suite attempt reached 10,929 passed / 11 skipped. Its three resolver-related failures are the residue cases fixed and rerun green above; the one remaining codex-shim token-inheritance failure is the same host-environment failure reproduced on clean dev. Fresh exact-head GitHub CI is now running.

@Wibias please re-review this exact head after CI completes.

@Wibias
Wibias marked this pull request as ready for review August 11, 2026 06:27

@Wibias Wibias left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. Follow-ups on this head fail closed for unreadable/invalid sqlite_home, keep Codex-compatible SQLite home precedence, and preserve the read-only native-residue indeterminate boundary instead of selecting another database.

@Wibias

Wibias commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Ingwannu — this was useful because split-home Codex installs (especially Windows Desktop + WSL) were letting history, admission, and residue checks talk to a different state_5.sqlite than Codex itself. Centralizing one call-time resolver with fail-closed invalid/unreadable authority, and pinning that identity through services and history jobs, stops silent fallback to the wrong database.

Merging now.

@Wibias
Wibias merged commit e63de8d into dev Aug 11, 2026
45 of 49 checks passed
@Wibias
Wibias deleted the agent/fix-codex-sqlite-home branch August 11, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants