Skip to content

Add missing storage-bucket creation to the migration chain (drift fix)#710

Closed
BigSimmo wants to merge 9 commits into
mainfrom
claude/storage-bucket-migration
Closed

Add missing storage-bucket creation to the migration chain (drift fix)#710
BigSimmo wants to merge 9 commits into
mainfrom
claude/storage-bucket-migration

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

The last open P2 from the audit — a schema/migration drift.

The clinical-documents and clinical-images storage buckets were declared only in supabase/schema.sql (lines 17–40). The migration chain created the storage.objects RLS policies that reference them (20260527000000_bulk_ingestion.sql) but never the buckets. So a database built purely by replaying migrations had RLS policies for buckets that don't exist, and uploads failed until the buckets were created out-of-band (dashboard/schema.sql).

This adds supabase/migrations/20260717130000_create_storage_buckets.sql, which mirrors schema.sql's bucket inserts exactly — idempotent (on conflict (id) do update set public = false), so it's a no-op re-affirmation on the existing production DB and creates the buckets on a fresh migrated DB. schema.sql and the committed drift manifest already declare the buckets, so no reconciliation edit is needed.

Verification

  • ✅ Change-scope classifies this db_changed, so CI's db-reset-verify (supabase db reset) migration-replay gate runs — that is the authoritative validation.
  • ✅ The migration's insert statements mirror schema.sql byte-for-byte
  • ✅ 79 offline drift/schema/storage unit tests pass (drift-detection, supabase-schema, storage-cleanup-safety, supabase-project)
  • ✅ prettier skips .sql (no format-gate impact)

⚠️ Not runnable locally here: supabase db reset needs the Supabase CLI + Docker, which aren't available in this environment (provider boundary). The replay is validated by the CI db-reset-verify job. Because this touches the storage layer, please give it a careful DB review before merge rather than relying on auto-merge alone.

Clinical Governance Preflight

Touches Supabase schema (storage buckets).

  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — unchanged; this only affects the migration chain
  • Service-role keys and private document access remain server-only — unchanged; buckets stay public = false
  • No patient-identifiable workflow introduced — this fixes bucket provisioning for fresh environments
  • No change to ingestion, answer generation, retrieval, source governance, or clinical output
  • Idempotent + public = false preserved, so no privacy/access regression on the existing DB

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added secure storage buckets for clinical documents and images.
    • Files remain private by default, with supported file types and size limits enforced.
  • Chores

    • Updated pull request policy validation to use the latest base-branch policy configuration, helping ensure checks remain current.

The clinical-documents and clinical-images buckets were declared only in
supabase/schema.sql. The migration chain created the storage.objects RLS
policies that reference them (20260527000000_bulk_ingestion.sql) but never the
buckets, so a database built by replaying migrations had policies for buckets
that did not exist and uploads failed until the buckets were created
out-of-band.

Add a migration mirroring schema.sql's bucket inserts (idempotent, on conflict
do update set public = false), so a migrated-from-scratch DB and the CI
`supabase db reset` replay have the buckets. schema.sql and the drift manifest
already declare them, so no reconciliation edit is needed.

Verified offline: change-scope classifies it db_changed (so the CI migration
replay runs), the inserts mirror schema.sql exactly, prettier skips .sql, and 79
drift/schema/storage unit tests pass. The replay itself (supabase db reset) runs
in CI's db-reset-verify job; it is not runnable locally here (no Supabase Docker
/ provider boundary).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019UQUcNriJUVSGRhWz794RQ
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

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

Next review available in: 1 minute

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 75fa5621-c72f-4b77-bb4b-40715f0c9dfd

📥 Commits

Reviewing files that changed from the base of the PR and between c562540 and 7f6fb8b.

📒 Files selected for processing (1)
  • supabase/migrations/20260717139000_create_storage_buckets.sql
📝 Walkthrough

Walkthrough

Updates the PR policy workflow to checkout its trusted script from the current base branch tip and adds an idempotent migration for two private storage buckets with size and MIME-type constraints.

Changes

Storage bucket setup

Layer / File(s) Summary
Create and configure storage buckets
supabase/migrations/20260717139000_create_storage_buckets.sql
Adds idempotent inserts for the private clinical-documents and clinical-images buckets, including file-size limits, allowed MIME types, and conflict handling that reaffirms private access.

Trusted policy checkout

Layer / File(s) Summary
Checkout the current base branch policy
.github/workflows/pr-policy.yml
Changes the trusted policy checkout from the pull request base SHA to github.base_ref and updates the related comments while preserving trusted-policy-only execution.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the main change: adding storage-bucket creation to the migration chain to fix drift.
Description check ✅ Passed The description closely follows the template with summary, verification, governance, and notes; it only omits the risk/rollback section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/storage-bucket-migration

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Jul 17, 2026

Copy link
Copy Markdown

Updates to Preview Branch (claude/storage-bucket-migration) ↗︎

Deployments Status Updated
Database Fri, 17 Jul 2026 18:32:18 UTC
Services Fri, 17 Jul 2026 18:32:18 UTC
APIs Fri, 17 Jul 2026 18:32:18 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Fri, 17 Jul 2026 18:32:22 UTC
Migrations Fri, 17 Jul 2026 18:32:25 UTC
Seeding ⏸️ Fri, 17 Jul 2026 18:32:10 UTC
Edge Functions ⏸️ Fri, 17 Jul 2026 18:32:10 UTC

❌ Branch Error • Fri, 17 Jul 2026 18:32:25 UTC

ERROR: Unsafe supabase_admin default privileges; migration blocked. (SQLSTATE 42501)
{"safe": false, "entries": ["function:PUBLIC:execute", "function:anon:execute", "function:authenticated:execute", "function:postgres:execute", "function:service_role:execute", "function:supabase_admin:execute", "sequence:anon:select", "sequence:anon:update", "sequence:anon:usage", "sequence:authenticated:select", "sequence:authenticated:update", "sequence:authenticated:usage", "sequence:postgres:select", "sequence:postgres:update", "sequence:postgres:usage", "sequence:service_role:select", "sequence:service_role:update", "sequence:service_role:usage", "sequence:supabase_admin:usage", "table:anon:delete", "table:anon:insert", "table:anon:maintain", "table:anon:references", "table:anon:select", "table:anon:trigger", "table:anon:truncate", "table:anon:update", "table:authenticated:delete", "table:authenticated:insert", "table:authenticated:maintain", "table:authenticated:references", "table:authenticated:select", "table:authenticated:trigger", "table:authenticated:truncate", "table:authenticated:update", "table:postgres:delete", "table:postgres:insert", "table:postgres:maintain", "table:postgres:references", "table:postgres:select", "table:postgres:trigger", "table:postgres:truncate", "table:postgres:update", "table:service_role:delete", "table:service_role:insert", "table:service_role:maintain", "table:service_role:references", "table:service_role:select", "table:service_role:trigger", "table:service_role:truncate", "table:service_role:update", "table:supabase_admin:delete", "table:supabase_admin:insert", "table:supabase_admin:maintain", "table:supabase_admin:references", "table:supabase_admin:select", "table:supabase_admin:trigger", "table:supabase_admin:truncate", "table:supabase_admin:update"], "role_exists": true, "schema_exists": true}
At statement: 3
do $$
declare
  v_status jsonb;
begin
  if not exists (select 1 from pg_catalog.pg_roles where rolname = 'supabase_admin') then
    raise notice 'role supabase_admin does not exist; default-privilege assertion is not applicable';
    return;
  end if;

  begin
    -- Local/Superuser-capable environments can assume the target role even
    -- when the migration role cannot use ALTER DEFAULT PRIVILEGES FOR ROLE
    -- directly. Hosted environments that cannot assume it fall through to the
    -- catalog assertion and block with operator instructions.
    execute 'set local role supabase_admin';
    -- Revokes must be global: per-schema ACLs cannot subtract privileges from
    -- built-in or previously granted global defaults.
    alter default privileges for role supabase_admin
      revoke all privileges on tables from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin in schema public
      revoke all privileges on tables from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin
      revoke all privileges on sequences from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin in schema public
      revoke all privileges on sequences from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin
      revoke execute on functions from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin in schema public
      revoke execute on functions from public, anon, authenticated, service_role;
    alter default privileges for role supabase_admin in schema public
      grant select, insert, update, delete on tables to service_role;
    alter default privileges for role supabase_admin in schema public
      grant usage, select on sequences to service_role;
    alter default privileges for role supabase_admin in schema public
      grant execute on functions to service_role;
    execute 'reset role';
  exception when insufficient_privilege then
    begin execute 'reset role'; exception when others then null; end;
    raise notice 'current role % cannot remediate supabase_admin default privileges; asserting the catalog postcondition', current_user;
  end;

  v_status := public.default_privileges_status('supabase_admin', 'public');
  if not coalesce((v_status->>'safe')::boolean, false) then
    raise exception using
      errcode = '42501',
      message = 'Unsafe supabase_admin default privileges; migration blocked.',
      detail = v_status::text,
      hint = E'Run these six statements as supabase_admin, then retry the migration:\n'
        'DO $remediate$ BEGIN ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin REVOKE ALL PRIVILEGES ON TABLES FROM PUBLIC, anon, authenticated, service_role; ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public REVOKE ALL PRIVILEGES ON TABLES FROM PUBLIC, anon, authenticated, service_role; END $remediate$;\n'
        'DO $remediate$ BEGIN ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin REVOKE ALL PRIVILEGES ON SEQUENCES FROM PUBLIC, anon, authenticated, service_role; ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public REVOKE ALL PRIVILEGES ON SEQUENCES FROM PUBLIC, anon, authenticated, service_role; END $remediate$;\n'
        'DO $remediate$ BEGIN ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC, anon, authenticated, service_role; ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC, anon, authenticated, service_role; END $remediate$;\n'
        'ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO service_role;\n'
        'ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public GRANT USAGE, SELECT ON SEQUENCES TO service_role;\n'
        'ALTER DEFAULT PRIVILEGES FOR ROLE supabase_admin IN SCHEMA public GRANT EXECUTE ON FUNCTIONS TO service_role;';
  end if;
end;
$$

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Note

Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

1 similar comment
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Note

Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Note

Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

@github-actions

Copy link
Copy Markdown

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

  • Migration replayneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #2787 (success).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

✅ Created PR with unit tests: #724

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

✅ Created PR with unit tests: #725

…heckout

When a PR is opened before scripts/pr-policy.mjs existed on main, the
pull_request_target workflow checked out the exact base.sha (fd79bc8) which
predated the script, causing ERR_MODULE_NOT_FOUND.

Fix: use github.base_ref (the branch name, e.g. "main") instead of
github.event.pull_request.base.sha. This always checks out the current tip of
the base branch, ensuring the policy script is always available. The
pull_request_target security model is preserved: only base-branch code is
checked out, never the PR head.
auto-merge was automatically disabled July 17, 2026 16:26

Head branch was pushed to by a user without write access

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

✅ Created PR with unit tests: #726

BigSimmo pushed a commit that referenced this pull request Jul 17, 2026
…eak-OR footgun

Follow-through on the memory/task review. Cross-checked every tracked
ongoing/recommended task against the current tree and updated the ledgers to
match reality:

- ingestion-concurrency-fix-workorder: mark deep-memory delete-scoping RESOLVED
  (producer-scoped model — migration 20260713030000 + #569 + deep-memory.ts
  cleanupStagedArtifacts). This doc now has no open repository items.
- operator-backlog: close the drift-codify forward-migration row (step 1h) — the
  forward-codify work-order is complete (applied 2026-07-13). Add a new "apply
  repo-ahead migrations to live" verify row for the three committed-but-unapplied
  migrations (20260713201542, 20260714110000, 20260717120000).
- audit-remediation-plan-2026-07-14: add a dated reconciliation block recording
  which findings are now DONE in code, which remain open (OWNER:CODE), which are
  in flight (PRs #708/#710), and which are operator-pending.

Also fix a config footgun the review surfaced (audit finding F5): .env.example
advertised RAG_TEXT_WEAK_OR_RELAXATION=true, but src/lib/env.ts defaults it to
false because it is a known-regressive opt-in experiment (buries some correct
docs after re-ranking). Align the example with the safe code default.

Docs + one config default only; no runtime code or schema changed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zbpyexer9cLgxhrB61RCU

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@supabase/migrations/20260717139000_create_storage_buckets.sql`:
- Line 26: Update the conflict handlers for the managed storage buckets so they
copy name, public, file_size_limit, and allowed_mime_types from excluded. Apply
this change to both clinical-documents at
supabase/migrations/20260717139000_create_storage_buckets.sql lines 26-26 and
clinical-images at lines 36-36.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dad604fa-4159-4007-9f12-44d1af0078af

📥 Commits

Reviewing files that changed from the base of the PR and between acf6b63 and c562540.

📒 Files selected for processing (1)
  • supabase/migrations/20260717139000_create_storage_buckets.sql

Comment thread supabase/migrations/20260717139000_create_storage_buckets.sql Outdated
@BigSimmo
BigSimmo enabled auto-merge July 17, 2026 17:34
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

  • supabase/migrations/20260717139000_create_storage_buckets.sql

Commit: 38eb01ed135ca98ec4edf7f16839c188f8b964da

The changes have been pushed to the claude/storage-bucket-migration branch.

Time taken: 3m 27s

coderabbitai Bot and others added 3 commits July 17, 2026 17:38
Fixed 1 file(s) based on 1 unresolved review comment.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
@BigSimmo BigSimmo closed this Jul 17, 2026
auto-merge was automatically disabled July 17, 2026 19:06

Pull request was closed

@BigSimmo
BigSimmo deleted the claude/storage-bucket-migration branch July 18, 2026 18:27
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.

3 participants