Skip to content

fix(server): bound retention work under rowid reuse#65

Closed
khaliqgant wants to merge 2 commits into
mainfrom
codex/relayauth-rowid-reuse-bound
Closed

fix(server): bound retention work under rowid reuse#65
khaliqgant wants to merge 2 commits into
mainfrom
codex/relayauth-rowid-reuse-bound

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

  • bound cursor-window candidate counts by the preceding scan's observed scannedCount
  • bound cursor-window deletes with a LIMIT scannedCount rowid subquery
  • remove the false assumption that ordinary SQLite rowids are monotonic across concurrent max-row deletion and reinsertion
  • preserve all existing exported APIs and leave package versions untouched for the release workflow

Why

tokens and audit_logs use ordinary hidden rowids, so SQLite may reuse a deleted maximum rowid. The prior numeric-range-only COUNT and DELETE could therefore expand after a scan: a two-row window {1, 2000} could become 2,000 candidates/deletions before the next statement. The new semantic limit makes both properties structural:

  • expiredCount <= scannedCount <= 1000
  • deletedCount <= scannedCount <= 1000

Rows inserted or expiring behind the cursor remain next-sweep work. COUNT and DELETE are independent statement snapshots, so their counts are not required to match under concurrency.

Red-first proof

The first commit (e1b9e139) reproduces the exact race on real SQLite for both tables: scan rowids 1,2000, delete rowid 2000, insert 1,999 expired rows without explicit rowids so SQLite reuses 2..2000. Against the old source both tests failed with scannedCount=2, expiredCount=2000, deletedCount=2000. The fix makes both pass at 2/2/2.

D1 proof

Before implementation, the approved query shapes were checked with EXPLAIN QUERY PLAN against the DEV D1 database. Token and audit COUNT/DELETE used intrinsic rowid range searches; DELETE used a list subquery; the outer COUNT scanned only the limited subquery; no plan contained a table scan or USE TEMP B-TREE. These were EXPLAIN-only reads (changed_db=false, changes=0, rows_written=0). The final statements use 4 token binds and 7 audit binds, below D1's 100-bind cap.

Validation

Observed runtime: Node v22.22.2, npm 10.9.7 (matching upstream CI).

  • focused rowid-reuse tests: 2/2 PASS
  • full retention suite: 13/13 PASS
  • @relayauth/server suite: 409/409 PASS
  • exact CI build: 8/8 tasks PASS
  • exact CI typecheck: 11/11 tasks PASS
  • exact CI forced tests: 15/15 tasks PASS
  • SDK contract check: PASS (existing Python parity advisories only)
  • server pack dry run includes dist/engine/retention-gc.{js,d.ts}; built JS contains all bounded COUNT/DELETE statements
  • git diff --check: PASS
  • no package manifest or lockfile change

Release sequencing

Do not hand-bump the manifest in this PR. After merge, the owner-gated release workflow should publish @relayauth/server@0.2.22; Cloud PR #2805 then pins the real registry artifact and is re-reviewed at its new head.

Review in cubic

@cursor

cursor Bot commented Jul 22, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@khaliqgant, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ba4fe73-337a-4d2b-aafe-d59327eb20d1

📥 Commits

Reviewing files that changed from the base of the PR and between 9225108 and c512c54.

📒 Files selected for processing (2)
  • packages/server/src/__tests__/retention-gc.test.ts
  • packages/server/src/engine/retention-gc.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/relayauth-rowid-reuse-bound

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.

@khaliqgant

Copy link
Copy Markdown
Member Author

Superseded by #66 after independent review showed that this LIMIT capped output rows but not rows examined. Leaving #65 and its review history intact as the durable rejected design artifact; #66 starts fresh from main with a valid backend-agnostic red-first commit and the stable-ID evidence design.

@khaliqgant khaliqgant closed this Jul 22, 2026
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