Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Add cleanup indexes for ASB retention paths#66

Merged
haasonsaas merged 1 commit intomainfrom
codex/asb-retention-indexes
Apr 15, 2026
Merged

Add cleanup indexes for ASB retention paths#66
haasonsaas merged 1 commit intomainfrom
codex/asb-retention-indexes

Conversation

@haasonsaas
Copy link
Copy Markdown
Collaborator

Summary

  • add the missing Postgres indexes that match ASB's cleanup query shapes and audit time-range scans
  • add a dedicated grants.session_id index for session revocation lookups
  • bound and order ListGrantsBySession so the session-grant lookup is no longer unbounded

Testing

  • go test ./internal/store/postgres ./internal/migrate -count=1
  • go test ./... -count=1
  • GOTOOLCHAIN=go1.26.0 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.3 run ./...
  • git diff --check

Part of #15

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 15, 2026

PR Summary

Medium Risk
Primarily performance-oriented, but the new hard limit on ListGrantsBySession could truncate results for unusually large sessions and affect revoke/cleanup flows if more than 10k grants exist.

Overview
Adds a new migration (0003_cleanup_indexes.sql) that introduces several Postgres indexes to match cleanup/retention query shapes (state+expiry scans) and audit time-range scans, including a dedicated grants(session_id) index.

Updates Repository.ListGrantsBySession to return grants in a stable order (created_at, id) and cap results via a hard limit (maxGrantsBySessionLookup), with a new unit test asserting the query is ordered and bounded.

Reviewed by Cursor Bugbot for commit 9805c67. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.

Reviewed by Cursor Bugbot for commit 9805c67. Configure here.

`, sessionID)
ORDER BY created_at ASC, id ASC
LIMIT $2
`, sessionID, maxGrantsBySessionLookup)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Silent truncation may leave grants unrevoked on session cleanup

Low Severity

ListGrantsBySession now silently caps results at 10,000, but both callers (expireSession and RevokeSession) assume they receive all grants for the session. In expireSession, the session is marked expired even when not all grants were processed, and since the session is no longer active, subsequent cleanup runs won't re-process it — leaving orphaned active grants. In RevokeSession, there's no retry mechanism at all. No error or warning is returned when the limit is reached.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9805c67. Configure here.

@haasonsaas haasonsaas merged commit 17734b4 into main Apr 15, 2026
8 checks passed
@haasonsaas haasonsaas deleted the codex/asb-retention-indexes branch April 15, 2026 23:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant