Skip to content

fix(tracking): record stats for hook-rewritten commands (#1082)#1223

Open
ousamabenyounes wants to merge 1 commit intortk-ai:developfrom
ousamabenyounes:fix/issue-1082
Open

fix(tracking): record stats for hook-rewritten commands (#1082)#1223
ousamabenyounes wants to merge 1 commit intortk-ai:developfrom
ousamabenyounes:fix/issue-1082

Conversation

@ousamabenyounes
Copy link
Copy Markdown
Contributor

Summary

Fixes #1082rtk gain showed no stats for commands rewritten by the Claude Code PreToolUse hook (e.g. git statusrtk git status).

Root cause (two interacting bugs in src/core/tracking.rs):

  1. execute_batch() stops on first error. Tracker::new() bundled PRAGMA journal_mode=WAL and PRAGMA busy_timeout=5000 in a single execute_batch() call. When the Claude Code hook runs, it first invokes rtk rewrite, which spawns a background telemetry thread that briefly holds a DB connection. If that process exits before the thread finishes, SQLite's WAL shared-memory file (.db-shm) can be briefly inconsistent, causing the WAL pragma to return a transient error. Because execute_batch() halts on first error, busy_timeout silently stayed at its default of 0 ms — meaning every subsequent lock-contention attempt in Tracker::new() failed immediately.

  2. Silent failure. TimedExecution::track() / track_passthrough() used if let Ok(tracker) = Tracker::new() { let _ = tracker.record(...) } — both error paths were silently swallowed, so the stat record was dropped with no indication.

Fix:

  • Run PRAGMA busy_timeout=5000 in its own execute_batch() call so it is always applied, independently of the WAL pragma.
  • Add a single retry with a 10 ms sleep in track() and track_passthrough() to outlast the transient .db-shm window before giving up.

New regression tests:

  • Verifies two concurrent Tracker connections can both write successfully (busy_timeout is in effect).
  • Verifies TimedExecution::track() records the stat even when another reader holds the DB open simultaneously.

Test plan

  • cargo test --all — 1381 passed, 0 failed
  • cargo fmt --all && cargo clippy --all-targets — clean
  • Two new regression tests added for the concurrent-access scenario

🤖 Generated with Claude Code

When Claude Code rewrites `git status` → `rtk git status` via its
PreToolUse hook, the hook first runs `rtk rewrite` which spawns a
background telemetry thread holding a brief DB connection. If that
process exits before the thread finishes, SQLite's WAL shared-memory
file (.db-shm) may be inconsistent for a few milliseconds.

Because `execute_batch()` stops on first error, bundling
`PRAGMA journal_mode=WAL` and `PRAGMA busy_timeout=5000` in one call
meant that a transient WAL-mode error silently left `busy_timeout=0`.
With no wait budget, any lock-contention attempt in `Tracker::new()`
failed immediately, silently dropping the tracking record.

Fix two things:
1. Run `PRAGMA busy_timeout=5000` in its own `execute_batch()` call so
   it is always applied, regardless of whether the WAL pragma succeeds.
2. Add a single retry (10 ms sleep) in `TimedExecution::track()` and
   `track_passthrough()` to outlast the transient .db-shm window.

Add regression tests for concurrent-open correctness and for the full
hook-rewrite tracking path.

Fixes rtk-ai#1082

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.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.

1 participant