fix(auditlog): cast audit thread key to text for legacy uuid schemas - #615
Conversation
PostgreSQL databases created before the unified schema (#586) keep their original uuid audit_logs.id column - CREATE TABLE IF NOT EXISTS never retypes it. The session thread key added in #602 coalesces id with the text session_id column, so every GET /admin/audit/sessions call on such a database failed with SQLSTATE 42804 (COALESCE types text and uuid cannot be matched), leaving the Audit Logs page empty in its default group-by-session view. Cast the id to text inside the thread key; the cast is valid on both backends and a no-op on SQLite. Covered by a regression test that recreates the exact pre-unification PostgreSQL DDL and runs GetSessions against it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe SQL reader casts legacy UUID audit-log IDs to text when deriving thread keys. A PostgreSQL regression test creates legacy tables, seeds entries, and verifies grouped sessions, latest IDs, session IDs, and counts. ChangesLegacy audit session compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/auditlog/session_id_test.go`:
- Around line 213-218: Update the session summary assertions in the test around
result.Sessions to use table-driven expected summaries, including each session’s
key, count, and Latest.ID. Ensure sess-a explicitly expects the newer UUID
ending in 0002, while preserving the existing expectation for the other session.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 36089eae-fa29-4f80-b0e4-7e9f52ad9205
📒 Files selected for processing (2)
internal/auditlog/reader_sessions_sql.gointernal/auditlog/session_id_test.go
Table-drive the session summaries and assert Latest.ID for the grouped thread too, so a regression in the per-thread recency ranking on the uuid id column cannot slip past the count check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Confidence Score: 5/5The change is safe to merge with no actionable defects identified. The cast is valid for both supported SQL dialects, preserves existing grouping semantics, fixes the legacy PostgreSQL type mismatch, and is covered by fresh-schema and legacy-schema tests.
What T-Rex did
Reviews (1): Last reviewed commit: "fix(auditlog): cast audit thread key to ..." | Re-trigger Greptile |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Since v0.1.63 the Audit Logs page comes up empty on PostgreSQL deployments upgraded from older releases, and the gateway logs:
Root cause: databases created before the unified schema (#586) have
audit_logs.idasuuid(the old standalone PostgreSQL store's DDL);CREATE TABLE IF NOT EXISTSnever retypes it. The session thread key added in #602 —COALESCE(NULLIF(session_id, ''), id)— then mixestextwithuuid, so everyGET /admin/audit/sessionscall fails, and since group-by-session is the dashboard's default view, the page shows no entries. Fresh databases and SQLite are unaffected, which is why the suite stayed green.Fix: cast the id inside the thread key:
COALESCE(NULLIF(session_id, ''), CAST(id AS TEXT)).CAST(… AS TEXT)is valid on both backends and a no-op on SQLite, where the column is already TEXT.User-visible impact
The Audit Logs page (session threads view) works again on PostgreSQL databases upgraded from pre-unification releases. No behavior change for fresh databases or SQLite.
Testing
TestSQLReader_GetSessionsOnLegacyUUIDSchemarecreates the exact pre-unification PostgreSQL DDL (uuidid, uuidaudit_log_idFK, nosession_idcolumn — the migration adds it) and runsGetSessions. Against the unfixed query it reproduces the reported SQLSTATE 42804 error verbatim; with the fix it passes.internal/auditlogsuite green on SQLite and PostgreSQL (GOMODEL_TEST_POSTGRES_URLset), build and vet clean.🤖 Generated with Claude Code
Summary by CodeRabbit