Repository navigation
ci: wire ephemeral Postgres into go-tests so RLS suites actually run - #336
Conversation
Closes the blind spot that hid the Repository.Transition 42P18 bug (PR #333): HIVE_TEST_DB_URL-gated suites (rag, artifacts, agenttask, egress) existed but had no database in CI, so they always skipped silently and nothing ever caught it. Adds a pgvector/pgvector:pg17 service to the go-tests job (pinned to Postgres 17, the closest available reference to Supabase's current default managed major -- no in-repo config.toml or other pin exists; pgvector image chosen over plain postgres since 20260625_01_enable_pgvector.sql needs the vector extension pre-built). A new step, gated to the control-plane and edge-api matrix legs only, bootstraps the Supabase-managed objects those migrations assume exist on a real project (hive_app/auditor_ro/authenticated/ supabase_auth_admin roles, the auth schema, minimal auth.uid()/ auth.jwt() stand-ins) via a new CI-only .github/ci/test-db-bootstrap.sql, then applies every supabase/migrations/*.sql file in order, then runs the four target packages with HIVE_TEST_DB_URL set and -v so the job log shows individual PASS lines, not just a package-level ok. The existing go test ./... step is untouched (no HIVE_TEST_DB_URL there), so nothing already green changes behavior. While replaying every migration from scratch for the first time ever, found a genuine pre-existing schema defect unrelated to this PR: 20260516_04_phase19_audit_log.sql and 20260625_06_audit_cold_archive_ manifest.sql both create a table named public.audit_cold_archive_ manifest with different column shapes; the later file's CREATE TABLE IF NOT EXISTS silently no-ops against the earlier (stale) shape, so a later ALTER in that same file fails against columns that don't exist. Per scope (CI wiring only, no product-code changes), this is worked around with a CI-only DROP TABLE immediately before that one migration file, clearly commented, applied only to this run's throwaway database -- never a real environment. Tracked separately for a real fix; flagging in the PR description that this may indicate the same defect is live wherever this migration history was ever applied fresh. Verified locally end to end against a scratch container with the exact workflow commands before pushing: bootstrap + all 61 migrations apply cleanly (given the one documented workaround), and apps/control-plane/internal/{agenttask,egress,rag} plus apps/edge-api/internal/artifacts all run and pass for real, not skip. Also discovered (out of scope, not fixed, flagged separately): apps/control-plane/internal/marketplace's marketplace_entries table has no GRANT to hive_app at all in 20260716_01_marketplace_catalog.sql; apps/control-plane/tests/compliance's TestAuditRetention_ColdArchiveManifestExists checks for a partition_name column that hasn't existed since the schema evolved; and apps/control-plane/internal/tenants has two failures whose audit log write assertions don't match what's actually persisted against a real database. None of these are touched by this PR (this PR's new live-DB step only runs the four named packages); each is a real, separate, pre-existing bug this same blind spot was hiding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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. 📝 WalkthroughWalkthroughAdds an ephemeral PostgreSQL service to selected CI matrix legs, bootstraps Supabase-compatible auth objects and migrations, exports ChangesRLS CI database testing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GoTests
participant Postgres
participant BootstrapSQL
participant SupabaseMigrations
participant RLSGoTests
GoTests->>Postgres: start pgvector/pg17 service
GoTests->>BootstrapSQL: apply CI auth objects
GoTests->>SupabaseMigrations: apply migration chain
GoTests->>RLSGoTests: run with HIVE_TEST_DB_URL
RLSGoTests->>Postgres: execute RLS-focused tests
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/ci.yml:
- Around line 71-75: Update the Postgres health check command in the service
options to include the host argument -h 127.0.0.1 with pg_isready, ensuring
readiness is validated over TCP rather than the local Unix socket.
- Around line 151-155: Update the non-control-plane branch of the CI test matrix
to run the edge-api RLS tests under internal/rag in addition to the existing
internal/artifacts tests. Preserve the control-plane command unchanged and
include the repository_rls_test.go coverage through the appropriate
./internal/rag/... package pattern.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8850dcc-9b8c-478a-939d-c63528d4f2bb
📒 Files selected for processing (2)
.github/ci/test-db-bootstrap.sql.github/workflows/ci.yml
CodeRabbit: pg_isready without -h checks the Unix socket, which the Postgres entrypoint's temporary init-phase instance answers before restarting for real on TCP -- a false-ready health check that can race CI steps against connection-refused. Add -h 127.0.0.1. Also: edge-api's non-control-plane branch only ran internal/artifacts, silently skipping apps/edge-api/internal/rag's own RLS suite (repository_rls_test.go, separate from control-plane's rag package). Added it. Verified both fixes locally: reproduced the exact race by omitting -h (confirms the finding), then confirmed clean with it; edge-api's rag RLS tests (incl. TestRepo_RLS_SearchChunksCannotReadOtherTenantChunks) now pass for real alongside artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Both fixed in f60ee21:
|
1 participant
Summary
Closes the blind spot that hid the
Repository.Transition42P18 bug found in PR #333:HIVE_TEST_DB_URL-gated RLS suites (rag,artifacts,agenttask,egress) existed but had no database in CI, so they always skipped silently and nothing ever caught real bugs in that code path.pgvector/pgvector:pg17service to thego-testsjob. Version pin: nosupabase/config.tomlor other in-repo Postgres version pin exists, so this tracks Supabase's current default managed major (17) as the closest available reference — bump the tag if that default changes.pgvector/pgvector(not the plainpostgresimage) because20260625_01_enable_pgvector.sqlneeds thevectorextension pre-built; otherwise it's a drop-in postgres image.control-planeandedge-apimatrix legs only: bootstraps the Supabase-managed objects those migrations assume already exist on a real project (hive_app/auditor_ro/authenticated/supabase_auth_adminroles, theauthschema, minimalauth.uid()/auth.jwt()stand-ins) via a new CI-only.github/ci/test-db-bootstrap.sql, then applies everysupabase/migrations/*.sqlfile in order, then runs the four target packages withHIVE_TEST_DB_URLset and-vso the job log shows individualPASSlines, not just a package-levelok.go test ./...step is untouched (noHIVE_TEST_DB_URLthere), so nothing already green changes behavior — this PR only adds new coverage, it doesn't touch the existing required-check baseline.Job log evidence (this exact reproduction, verified locally before pushing with the workflow's literal commands against a scratch
pgvector/pgvector:pg17container)All previously-
--- SKIP'd RLS tests in these four packages now show--- PASSindividually — this PR's own CI run will show the same live.Pre-existing bugs found while replaying every migration from scratch for the first time ever (none fixed here — CI wiring only, no product-code changes per scope)
20260516_04_phase19_audit_log.sqland20260625_06_audit_cold_archive_manifest.sqlboth create a table namedpublic.audit_cold_archive_manifestwith different column shapes. The later file'sCREATE TABLE IF NOT EXISTSsilently no-ops against the earlier (stale) shape on any from-scratch replay, so a laterALTERin that same file fails against columns that don't exist. Worked around with a CI-onlyDROP TABLEimmediately before that one migration file — applies only to this run's throwaway database, never a real environment. This may indicate the same defect is live wherever this migration history was ever applied fresh (a new environment, a disaster-recovery rebuild) — flagging for a real fix and for someone with prod access to check\d public.audit_cold_archive_manifestthere.apps/control-plane/internal/marketplace'smarketplace_entriestable has noGRANTtohive_appat all in20260716_01_marketplace_catalog.sql(permission denied for table marketplace_entries, SQLSTATE 42501).apps/control-plane/tests/compliance'sTestAuditRetention_ColdArchiveManifestExistschecks for apartition_namecolumn that hasn't existed since the schema evolved to the richer shape.apps/control-plane/internal/tenantshas two failures (TestSwitch_Allowed_UpdatesMetadataAndAudits,TestSwitch_NonMember_403CrossTenant) whose audit-log-write assertions don't match what's actually persisted against a real database.Test plan
apps/control-plane/internal/agenttask,egress,rag, andapps/edge-api/internal/artifactsall run and PASS for real (not skipped) — verified locally with the exact workflow commandspython3 -c "import yaml; yaml.safe_load(...)")🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores