Skip to content

fix(ai-vault): handle live OpenCode database contention - #15671

Closed
brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/sta-4078-session-db-locked
Closed

brennanb2025 wants to merge 3 commits into
mainfrom
brennanb2025/sta-4078-session-db-locked

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​318 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​317
Prod 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​160 $\color{#cf222e}{\Huge{\mathbf{−}}}$​44 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​116

ELI5

OpenCode normally lets Orca read session history while OpenCode is writing, but a brief exclusive database lock could make Orca fail immediately and show database is locked. Agent Session History now waits for that short lock in its background scanner. If a lock outlives the five-second bound, Orca shows a temporary-unavailability notice and retries on the next scan instead of reporting a skipped transcript or risking an inconsistent file copy.

What Changed

  • Added a shared read-only SQLite opener with a five-second busy timeout, query_only, a first-read probe, and deterministic handle cleanup.
  • OpenCode session discovery and parsing use the opener inside the background session scanner.
  • Remaining SQLite lock failures become an existing kind: 'notice' scan issue with no raw SQLite lock text.
  • Desktop and mobile Agent Session History classify notices separately from skipped transcripts.
  • Kept OpenCode usage and Codex state-database reads unchanged because they are outside the reported session-history path; in particular, no synchronous five-second wait was added to the Electron main process.
  • Removed the filesystem snapshot fallback. Copying a live database and sidecars is not an atomic SQLite snapshot across journal modes and could expose inconsistent or uncommitted data.

Why

OpenCode uses WAL mode, where ordinary readers and writers coexist. A bounded busy timeout handles exceptional short lock transitions without blocking the UI because these reads run in the session-scanner worker. A genuinely persistent whole-file lock cannot be read safely through an ad hoc filesystem copy, so the accurate fallback is a retryable notice rather than stale, corrupt, or uncommitted session data.

This revision adopts the bounded read-only design also proposed in #14134 while retaining this PR's retryable notice behavior and desktop/mobile classification. #14134 can be closed once this lands.

Linked Issue

Fixes #14133
STA-4078

Visual Proof

N/A — the behavioral change is background SQLite contention handling plus scanner-authored banner classification. The Windows reporter box remains unavailable, so real-Windows reproduction is still pending.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Actual local platform: macOS, Node 26.5.0.

  • Acceptance test with the busy timeout neutered to zero: session-scanner-opencode-sqlite-lock.test.ts failed 2/2. Listing returned the temporary-unavailability notice, and parsing threw database is locked.
  • Restored implementation: the same acceptance file passed 2/2.
  • Focused affected suites: 6 files, 50/50 tests passed (shared opener, OpenCode lock/list/parse, scan-issue mapping, usage regression coverage, shared banner state).
  • pnpm typecheck — passed.
  • node config/scripts/check-changed-code-quality.mjs — passed with 0 findings.
  • node config/scripts/check-react-doctor-changed.mjs — passed with 0 issues.

AI Disclosure

Codex implemented and verified the revision after independent plan reviews.

Review

  • Security/data integrity: provider databases remain read-only and query_only; Orca never checkpoints, copies, deletes, or mutates the user's database or sidecars.
  • Cross-platform: uses Node SQLite and path APIs already used by Orca. Deterministic worker-thread tests cover exclusive writer contention and cleanup.
  • Remote SSH: no RPC or wire-format change. The execution host that owns the provider database still performs the read.
  • Mobile: existing notice issues render as notices rather than increasing the skipped-transcript count.
  • Backward compatibility: no new issue kind or stream opcode. Existing clients already understand kind: 'notice'.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

  • This PR is small and focused
  • I explained what changed and why (including ELI5)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Targeted tests, changed-code quality, React Doctor, and typecheck pass; full test/build remain for CI.

Author

  • X / Twitter: @BrennanKB5

Agent Session History opened the user's opencode.db in place, so a live
writer made the scan fail immediately as "database is locked" and skip
the transcript. Snapshot the db+WAL instead of waiting, and surface a
retryable unavailable notice when even the snapshot cannot be read.

Supersedes #14134. Fixes #14133. STA-4078.

Co-authored-by: Bing.Z <zzb@gxsmjx.com>
@coderabbitai

coderabbitai Bot commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds a shared live SQLite read-only opener with lock detection, snapshot fallback, cleanup, and error classification. OpenCode, Hermes, Codex, and usage scanners now use this opener. SQLite availability failures become standardized notice issues. Session parsing uses centralized issue conversion. Shared scan-state helpers use a local view type. Mobile history displays notices and skipped-transcript details through a dedicated banner component. Tests cover locking, recovery, cleanup, classification, and presentation state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 16 files. (1 skipped: 1 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes prevent database-lock errors and skipped transcripts by using snapshots and retryable notices, directly addressing issue #14133.
Out of Scope Changes check ✅ Passed The provider readers, issue classification, desktop UI, mobile UI, and tests support the shared SQLite lock-handling objective.
Title check ✅ Passed The title clearly identifies the main change: handling live OpenCode SQLite database contention in AI Vault.
Description check ✅ Passed The description follows the required template and documents the change, rationale, issue, testing, platform scope, review notes, and checklist.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e6067501-f825-4861-b3a0-4d21ff6d7c0e

📥 Commits

Reviewing files that changed from the base of the PR and between 012e9f4 and 39d3209.

📒 Files selected for processing (17)
  • config/tsconfig.cli.json
  • mobile/src/agent-history/MobileAgentSessionHistoryPanel.tsx
  • mobile/src/agent-history/mobile-agent-history-scan-banners.tsx
  • src/main/ai-vault/session-scan-issues.test.ts
  • src/main/ai-vault/session-scan-issues.ts
  • src/main/ai-vault/session-scanner-opencode-sqlite-list.ts
  • src/main/ai-vault/session-scanner-opencode-sqlite-lock.test.ts
  • src/main/ai-vault/session-scanner-opencode-sqlite.ts
  • src/main/ai-vault/session-scanner.ts
  • src/main/automations/hermes-cron-output.ts
  • src/main/codex/codex-state-db.ts
  • src/main/opencode-usage/scanner.ts
  • src/main/sqlite/live-sqlite-readonly.test.ts
  • src/main/sqlite/live-sqlite-readonly.ts
  • src/renderer/src/components/right-sidebar/AiVaultScanIssueBanners.tsx
  • src/shared/ai-vault-scan-issue-state.test.ts
  • src/shared/ai-vault-scan-issue-state.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Comment thread src/main/opencode-usage/scanner.ts Resolved
Comment thread src/main/sqlite/live-sqlite-readonly.ts Resolved
A named node:fs import binds at module evaluation, so every suite that
partially mocks fs failed the moment this module entered its graph - 33 CI
shards that never touch sqlite. A namespace import defers resolution to call
time.

Also reverts the Hermes cron reader to its previous direct open: that path is
a sibling of this fix, not part of it, and routing it through the snapshot
broke its test because the suite mocks sync-database rather than creating a
real state.db.
@brennanb2025 brennanb2025 changed the title fix(ai-vault): read live provider sqlite without waiting on locks fix(ai-vault): handle live OpenCode database contention Aug 23, 2026
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Revised this from scratch around the safe SQLite concurrency boundary.

What changed:

  • Replaced the live-file copy/snapshot cache with a small read-only opener: five-second busy timeout, query_only, first-read probe, and failure-safe cleanup.
  • Scoped it to OpenCode session list/parse in the background session scanner. Reverted the Codex state and main-thread OpenCode usage integrations; the latter must not synchronously stall Electron, and its previous empty-success fallback could cache an outage.
  • Kept the retryable kind: 'notice' mapping and desktop/mobile notice-vs-skipped-transcript classification because those are the right product semantics when a lock outlives the bound.
  • Removed the generic filesystem snapshot because db/sidecar copies are not atomic across SQLite journal modes and can expose inconsistent or uncommitted data.

Acceptance non-vacuity was re-proven by setting the production timeout to zero:

src/main/ai-vault/session-scanner-opencode-sqlite-lock.test.ts
2 tests | 2 failed

listing: expected [] issues, received the temporary-unavailability notice
parsing: Error: database is locked

After restoring the implementation, that file passed 2/2. Final verification: 6 affected test files passed 50/50 tests; pnpm typecheck, changed-code quality, and React Doctor all passed with zero new findings/issues.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

OpenCode history reads now wait briefly for database contention, close safely, and report a retryable source notice, with further bounds in #25292. This earlier implementation can be closed.

AmoabaKelvin added a commit that referenced this pull request Nov 5, 2026
…ripts (#38626)

* Show source scan notices accurately in mobile Agent History

Reuse the existing desktop presentation policy in shared code, keep host failures and source notices separate from skipped transcript files, and retain their scanner-authored messages.

Adapted from the remaining mobile presentation in original PR #15671. Rebased onto main: the classifier now lives in src/shared/ai-vault/scanning/, desktop keeps its translated skip reasons, and mobile reuses its existing skipped-transcript notice.

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>

* refactor(ai-vault): share the scan issue row key between desktop and mobile

---------

Co-authored-by: Neil <neil@stably.ai>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Agent Session History show the database is locked

1 participant