Skip to content

fix: wrap insertEvent with withRetry for concurrent PostToolUse hooks#243

Open
james-gough-op wants to merge 1 commit intomksglu:mainfrom
james-gough-op:fix/posttooluse-concurrent-sqlite-contention
Open

fix: wrap insertEvent with withRetry for concurrent PostToolUse hooks#243
james-gough-op wants to merge 1 commit intomksglu:mainfrom
james-gough-op:fix/posttooluse-concurrent-sqlite-contention

Conversation

@james-gough-op
Copy link
Copy Markdown

What / Why / How

What: Wrap SessionDB.insertEvent() with withRetry() and propagate busy_timeout pragma to Bun's SQLite adapter.

Why: When many tool calls complete in parallel (e.g., 16 batch mcp__linear__get_issue calls during a standup summary), Claude Code spawns multiple PostToolUse hooks simultaneously. These all open the same per-project SessionDB and compete for the SQLite write lock. While better-sqlite3's busy_timeout (30s) handles most contention, edge cases during transaction lock escalation can surface SQLITE_BUSY, causing Claude Code to report PostToolUse:mcp__linear__get_issue hook error to users.

Additionally, the BunSQLiteAdapter factory was silently dropping the timeout option passed from SQLiteBase, meaning Bun runtime had no busy_timeout at all — concurrent writes would fail immediately with SQLITE_BUSY.

How:

  1. src/session/db.ts — Changed transaction() to this.withRetry(() => transaction()) in insertEvent, leveraging the existing retry infrastructure in SQLiteBase
  2. src/db-base.ts — Added busy_timeout pragma in BunDatabaseFactory when opts.timeout is provided, matching better-sqlite3's automatic behavior
  3. Added a concurrent insert test verifying multiple DB instances can write to the same file without data loss

Affected platforms

  • Claude Code
  • All platforms

(The Bun busy_timeout fix applies to any platform using Bun runtime for the MCP server. The withRetry fix applies to all platforms since PostToolUse hooks run with Node.js.)

Test plan

  • Added Concurrent Insert Resilience test in tests/session/session-db.test.ts — opens 5 SessionDB instances against the same DB file and inserts events from each, verifying all 5 events are stored without errors
  • All 26 session-db tests pass
  • npm run typecheck passes clean

Checklist

  • Tests added/updated (TDD: red → green)
  • npm test passes (6 pre-existing failures unrelated to this change — security, cursor-hooks, vscode-hooks, session-hooks-smoke, hooks/integration)
  • npm run typecheck passes
  • Docs updated if needed (README, platform-support.md) — no docs changes needed
  • No Windows path regressions (forward slashes only)
  • Targets next branch (unless hotfix)

When many tool calls complete in parallel (e.g., batch Linear get_issue),
Claude Code spawns multiple PostToolUse hooks simultaneously. These all
open the same per-project SessionDB and compete for the SQLite write lock.

While better-sqlite3's busy_timeout (30s) handles most contention, edge
cases during transaction lock escalation can still surface SQLITE_BUSY.
This causes hooks to fail, and Claude Code reports "hook error" to users.

Changes:
- Wrap SessionDB.insertEvent() transaction with withRetry() for
  defense-in-depth against SQLITE_BUSY under concurrent hook access
- Set busy_timeout pragma in BunSQLiteAdapter to match better-sqlite3's
  timeout option (previously ignored, causing immediate SQLITE_BUSY
  failures under Bun runtime)
- Add concurrent insert test verifying multiple DB instances can write
  to the same file without data loss
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.

1 participant