fix(capture): make session-event INSERTs idempotent to stop duplicate rows (C1)#324
Merged
Conversation
…rows Session events were written with a plain `INSERT ... VALUES` and the sessions table has no UNIQUE constraint on `id`. When the API retry loop re-sends an insert that already committed but returned a transient 5xx (502/503), a duplicate row is created — the ~17% duplication observed in production during the 2026-07-10 gateway-degradation window. Route every capture INSERT through a shared `buildDirectSessionInsertSql` helper that emits `INSERT ... SELECT ... WHERE NOT EXISTS (id = ...)`, making a re-send a no-op. Verified lag-safe against the real backend: a rapid re-send inside the ~5s read-your-writes window yields exactly one row. Applied across all runtimes — claude/codex/cursor/hermes capture, codex stop, and openclaw — via one helper, removing the duplicated inline SQL at the same time. Tests: real e2e against the backend, source + bundle-guard tests, and updates to the three existing tests that scraped the now-centralized column list.
Codex review of the previous commit flagged that the pi extension still wrote
session rows with a bare `INSERT ... VALUES` in writeSessionRow. pi has no
active 5xx-retry path today (dlQuery does not retry; writeSessionRow only
retries on table-not-visible), so it has no live duplicate vector — but the
bare insert is inconsistent with the fix and a latent bug if retry behavior
changes.
pi deliberately imports nothing from src/ ("raw .ts, zero deps"), so inline the
same `INSERT ... SELECT ... WHERE NOT EXISTS (id = ...)` shape rather than the
shared helper, kept in lockstep. Adds a pi-source regression guard.
…empotent # Conflicts: # harnesses/openclaw/src/index.ts
Codex review found the batched session-queue insert path — used by the Cowork MCP ingest (src/mcp/cowork-ingest.ts) — still emitted a bare multi-row `INSERT ... VALUES`. That path builds a DeeplakeApi whose query() retries on 5xx (and re-sends again on table-missing), so it carried the same duplicate vector as the per-event capture path. Switch buildSessionInsertSql to the anti-join form: INSERT ... SELECT v.* FROM (VALUES ...) AS v(cols) WHERE NOT EXISTS (SELECT 1 FROM <table> t WHERE t.id = v.id) so a re-sent batch skips rows already present. Verified against the real backend: a 2-row batch re-sent inside and beyond the ~5s read-your-writes window stays at 2 rows, and a partially-overlapping batch inserts only the new id. Adds a source-level guard for the shape.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughChangesSession insert idempotency
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 7 files changed
Generated for commit 14cea7b. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harnesses/pi/extension-source/hivemind.ts`:
- Around line 972-975: Update the SQL construction around the session INSERT to
pass the agent value through sqlStr() before interpolating it, matching the
escaping used for other string parameters and preserving valid handling of
quotes and injection characters.
In `@src/hooks/capture.ts`:
- Around line 171-184: Use the JSON envelope ID as the database row ID instead
of generating a new UUID in the capture insert builders: update
src/hooks/capture.ts lines 171-184, src/hooks/codex/capture.ts lines 140-153,
and harnesses/openclaw/src/index.ts lines 1542-1555 to pass entry.id with the
appropriate typing. Apply the same change to the capture flows in
cursor/capture.ts, hermes/capture.ts, and codex/stop.ts, preserving each flow’s
existing insert behavior.
In `@src/hooks/cursor/capture.ts`:
- Around line 166-179: The capture hooks currently generate database row IDs
separately from the IDs in their JSON payloads. In src/hooks/cursor/capture.ts
lines 166-179 and src/hooks/hermes/capture.ts lines 149-162, update the id
passed to buildDirectSessionInsertSql to reuse entry!.id as a string; in
src/hooks/codex/stop.ts lines 134-147, reuse entry.id instead of generating a
new UUID.
In `@tests/claude-code/plugin-version-resolution.test.ts`:
- Around line 78-83: Update the sessionsInsert regex assertion to match the
complete expected column list from buildDirectSessionInsertSql, replacing the
generic [^`]*? wildcard with explicit column names and ordering while preserving
the plugin_version check.
In `@tests/claude-code/session-queue.test.ts`:
- Around line 112-123: Update the test "builds an idempotent batch insert
(anti-join, not a bare VALUES insert)" to replace broad regex assertions
containing .* with specific string-inclusion assertions for the expected SQL
structure. Keep validating the VALUES alias, the NOT EXISTS anti-join, and the
absence of the direct duplicate-prone VALUES form without relying on generic
substring patterns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cbeca6d2-00a3-4d4a-8e3b-fd22f9f8e21f
📒 Files selected for processing (15)
harnesses/openclaw/src/index.tsharnesses/pi/extension-source/hivemind.tssrc/hooks/capture.tssrc/hooks/codex/capture.tssrc/hooks/codex/stop.tssrc/hooks/cursor/capture.tssrc/hooks/hermes/capture.tssrc/hooks/session-queue.tssrc/hooks/shared/session-insert-sql.tstests/claude-code/plugin-version-resolution.test.tstests/claude-code/session-insert-sql.test.tstests/claude-code/session-queue.test.tstests/openclaw/openclaw-embed-bundle.test.tstests/pi/pi-extension-source.test.tstests/shared/agent-sessions-insert-schema.test.ts
- Align the row id with the event id: every capture path now passes the message JSON's own `entry.id` as the row PK instead of a fresh random UUID, so the `id` column matches the id embedded in the `message` payload and stays the dedup key for the logical event. Applies to claude/codex/cursor/hermes capture, codex stop, openclaw, and pi (with a uuid fallback if a caller ever omits entry.id). - pi: escape the `agent` value via sqlStr() in the inline INSERT, matching the shared helper and the other columns. - tests: replace wildcard regexes with exact column-list assertions in plugin-version-resolution and session-queue tests.
Collaborator
Author
|
Thanks @coderabbitai — all 5 addressed in c556c2c:
Note: the redaction gaps flagged elsewhere (codex/stop, pi, cowork-ingest) are out of scope for this PR — they belong to the separate C2 secret-masking workstream already on main. tsc + full suite (5577) green. |
|
Tip For best results, initiate chat on the files or code changes.
🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Fixes C1 from the deeplake.ai ETL issue log: the
sessionstable was ~17% duplicate rows (249,294 rows / 206,292 distinct ids).Root cause: session events were written with a plain
INSERT ... VALUES, and thesessionstable has noUNIQUEconstraint onid. When the API retry loop re-sends an insert that actually committed but returned a transient 5xx (the 502/503 gateway-degradation window on 2026-07-10), a duplicate row is created. Timeouts already fail-fast, so the culprit is retryable 5xx re-sends.Fix
Make every session-event INSERT idempotent so a re-send is a no-op. No dependency on a server-side
UNIQUEconstraint (which can't be applied to the existing table — it already contains duplicates — and risksCREATE INDEXoid errors on fresh tables).buildDirectSessionInsertSql(src/hooks/shared/session-insert-sql.ts) emitsINSERT ... SELECT ... WHERE NOT EXISTS (id = ...). Wired into all direct-insert runtimes, removing the duplicated inline SQL: claude / codex / cursor / hermescapture.ts,codex/stop.ts,openclaw/index.ts, andpi(inlined — pi imports nothing fromsrc/).buildSessionInsertSql(src/hooks/session-queue.ts, used by the Cowork MCP ingest) switched to the multi-row anti-join:INSERT ... SELECT v.* FROM (VALUES ...) v WHERE NOT EXISTS (t.id = v.id).Composes with the
redactSecrets()masking already onmain(C2): masking runs online, which feeds the idempotent insert.Verification (real backend, not just mocks)
Probed the real Deeplake backend on a sandbox table:
UNIQUE INDEXis accepted+enforced now (so the "no server-side UNIQUE" note ingraph/deeplake-push.tsis stale), but theWHERE NOT EXISTSguard is preferred — it works on the existing duplicated table with zero DDL risk.Plus: real e2e driving the production SQL builder (5/5), source + bundle-guard tests, and updates to the existing tests that scraped the now-centralized column list.
tscclean,npm run buildclean, full suite green (5577 tests).Scope
C1 only. The redaction gaps codex flagged in
codex/stop.ts,pi, andcowork-ingest.tsare C2 (secret masking) — a separate workstream;main's masking commit simply didn't cover those paths. Not addressed here.Follow-up (not in this PR)
sessions(dedup byid).Summary by CodeRabbit
Bug Fixes
Tests