fix(worker): re-check document after the legacy claim_ingestion_jobs row lock by BigSimmo · Pull Request #3636 · BigSimmo/PsychSift

BigSimmo · 2026-10-18T06:30:05Z

Summary

Follow-up to fix(worker): re-check document and allowlist scope after claim_ingestion_jobs row lock #3633. The legacy three-argument claim_ingestion_jobs(text, integer, integer) picks candidates from a snapshot, takes the picked document's authority lock and sibling-lease locks, then locks the job row with FOR UPDATE SKIP LOCKED, but never checked that the locked row still belonged to the picked document. A job re-pointed at another document in that window was leased under the old document's authority and sibling-lease check.
New migration supabase/migrations/20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sql replaces the three-argument function in place. After the job row lock it skips the job unless v_job.document_id still equals the picked document. A skipped job stays pending with no lease and no consumed attempt, the same as every other skip in the loop. Signature, defaults, grants, return shape and every other branch are unchanged. The five-argument function (fix(worker): re-check document and allowlist scope after claim_ingestion_jobs row lock #3633) is not touched.
New tests/sql/ingestion-legacy-claim-recheck-after-lock.sql, modelled on tests/sql/ingestion-claim-recheck-after-lock.sql (run by the Migration replay job with every tests/sql/*.sql). It wraps lock_ingestion_document_authority inside its rolled-back transaction so the job is re-pointed between the pick and the job row lock. It covers a re-point to a same-type document and to a different file type, plus a control and a follow-up claim that leases the job under its new document.
Companion files: supabase/schema.sql mirrors the migration (same pattern as fix(worker): re-check document and allowlist scope after claim_ingestion_jobs row lock #3633), supabase/applied-migration-hashes.json via npm run migrations:seal (append-only, no existing entry changed). supabase/drift-manifest.json will be taken byte-for-byte from this PR's Migration replay artifact (npm run drift:manifest needs Docker, which isn't available locally). database.types.ts is unchanged because the signature is unchanged.

Areas touched: Data platform and access

Verification

Checks run:
- Offline harness (PGlite with a minimal stub of the touched tables and authority lock): against the 20261013075944 function the new test fails (legacy claim: a job whose document changed after the pick must not be leased under the old document's authority, and the file-type case); the control passes. With the new migration applied the whole file passes.
- node scripts/check-migration-immutability.mjs: Migration immutability guard passed: 321 shipped migrations unchanged.
- check:migration-role: passed. check:function-grants: OK — all 130 SECURITY DEFINER public function(s) are revoked from PUBLIC.
- Vitest supabase-schema, migration-sql, migration-immutability, migration-history-guards, hosted-migration-role-guard, worker-row-contract, drift-detection: 300/301 passed; the one failure is the expected stale drift manifest before the CI artifact lands.
- The first push used the documented scoped SKIP_DRIFT_GUARD=1 override, as in fix(worker): re-check document and allowlist scope after claim_ingestion_jobs row lock #3633, because the manifest can only be generated in CI. No other guard was skipped.
- Hosted proof: the Migration replay job runs the full chain and the new SQL test against the real lock_ingestion_document_authority.

Risk and rollout

Risk: Low. One extra predicate on the locked row inside an existing SECURITY DEFINER function. It can only make the claim skip a job it previously leased by mistake.
Rollback: a later migration restoring the 20261013075944 body.
Merge gate: this PR adds a migration under supabase/migrations/**; merging applies it to the live database. The owner approved this PR including auto-merge (relayed 14:26 AWST 2026-10-18).
Provider or production effects: merging applies this migration to the live database immediately. No provider settings or other live data were changed from this branch.

…row lock Follow-up to #3633 for the three-argument claim_ingestion_jobs(text, integer, integer). It picked candidates from a snapshot, locked the picked document's authority and sibling leases, then locked the job row without checking that the row still belonged to the picked document, so a job re-pointed in that window was leased under the old document's authority. New migration 20261018062736 replaces the three-argument function in place (same signature, defaults, grants and behaviour) and, after the job row lock, skips the job unless it still belongs to the picked document. A skipped job stays pending with no lease and no consumed attempt. tests/sql/ingestion-legacy-claim-recheck-after-lock.sql wraps the authority lock to re-point the job between pick and lock. It fails against the 20261013075944 function and passes with this one. schema.sql mirrors the migration; the hash is sealed with npm run migrations:seal (append-only). The drift manifest follows from the Migration replay artifact.

greptile-apps

BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

codeant-ai · 2026-10-18T06:30:15Z

🤖 CodeAnt AI — Review Status

Status	Commit	Started (UTC)	Finished (UTC)
✅ Reviewed your PR	`be0b278`	Oct 18, 2026 · 06:30	06:35

codeant-ai · 2026-10-18T06:30:16Z

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

coderabbitai · 2026-10-18T06:30:17Z

Warning

Review limit reached

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 121 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: BigSimmo/PsychSift/.coderabbit.yaml
Review profile: CHILL
Plan: Team
Run ID: 2b50ebad-a8a5-40f5-9f10-4115122a2d54

📥 Commits

Reviewing files that changed from the base of the PR and between b12f3d4 and 2d1906b.

📒 Files selected for processing: 5

supabase/applied-migration-hashes.json
supabase/drift-manifest.json
supabase/migrations/20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sql
supabase/schema.sql
tests/sql/ingestion-legacy-claim-recheck-after-lock.sql

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

supabase · 2026-10-18T06:30:19Z

Updates to Preview Branch (fix/legacy-claim-recheck-after-lock) ↗︎

Deployments	Status	Updated
Database	✅	Sun, 18 Oct 2026 06:46:30 UTC
Services	✅	Sun, 18 Oct 2026 06:46:30 UTC
APIs	✅	Sun, 18 Oct 2026 06:46:30 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	✅	Sun, 18 Oct 2026 06:46:31 UTC
Migrations	✅	Sun, 18 Oct 2026 06:46:33 UTC
Seeding	✅	Sun, 18 Oct 2026 06:46:35 UTC
Edge Functions	✅	Sun, 18 Oct 2026 06:46:39 UTC

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

claude · 2026-10-18T06:30:26Z

Claude encountered an error after 2s —— View job

I'll analyze this and get back to you.

qodo-code-review · 2026-10-18T06:30:50Z

PR Summary by Qodo

Prevent legacy ingestion claims from leasing re-pointed jobs

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Re-check a job’s document after locking its row, preventing leases under stale document authority.
• Keep re-pointed jobs pending and eligible for a later claim without consuming an attempt.
• Add deterministic SQL regression coverage and seal the append-only migration.

Diagram

graph TD
  C["Candidate pick"] --> A["Authority lock"] --> S["Sibling lease check"] --> R["Job row lock"] --> D{"Same document?"} -->|Yes| L["Issue lease"]
  D -->|No| K["Keep pending"]

High-Level Assessment

The post-lock check is the least disruptive fix: it verifies that the existing authority and sibling-lease checks apply to the row being leased. Consolidating the legacy and five-argument functions could reduce duplication, but would broaden the migration and regression risk for this targeted race fix.

Files changed (4) +205 / -2

Bug fix (2) +111 / -0

20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sqlGuard legacy claims against a changed document +55/-0

Guard legacy claims against a changed document
• Replaces the three-argument claim function in place. After locking the job row, it skips the job if its document differs from the authority-locked candidate, preserving the signature, defaults, return shape and grants.
supabase/migrations/20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sql

schema.sqlMirror the corrected legacy claim function +56/-0

Mirror the corrected legacy claim function
• Appends the migration’s three-argument function definition and grants to the checked-in schema.
supabase/schema.sql

Tests (1) +91 / -0

ingestion-legacy-claim-recheck-after-lock.sqlExercise document changes between candidate pick and row lock +91/-0

Exercise document changes between candidate pick and row lock
• Uses a transaction-scoped authority-lock wrapper to re-point a job during a legacy claim. Tests same-type and different-type targets, an unchanged-job control, and a later successful claim under the new document.
tests/sql/ingestion-legacy-claim-recheck-after-lock.sql

Other (1) +3 / -2

applied-migration-hashes.jsonSeal the new migration +3/-2

Seal the new migration
• Appends the migration’s SHA-256 entry and updates the seal timestamp without changing existing migration hashes.
supabase/applied-migration-hashes.json

codeant-ai · 2026-10-18T06:31:12Z

🏁 CodeAnt Quality Gate Results

Commit: 2d1906bf
Scan Time: 2026-10-18 06:47:56 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate	Status	Details
Secrets	✅ PASSED	0 secrets found, 4 false positive secrets suppressed
Duplicate Code	✅ PASSED	23.4% duplicated
SAST	✅ PASSED	No security issues
Bugs	✅ PASSED	Rating S: No bugs
IAC	✅ PASSED	Rating S: No issues

View Full Results

qodo-code-review · 2026-10-18T06:31:34Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

Resolved findings

1. ~~Migration replay rejects stale manifest~~ ✗ Dismissed

Description

The new claim_ingestion_jobs definition changes supabase/schema.sql, but the committed
supabase/drift-manifest.json still contains the previous schema hash and function snapshot. When
the offline drift test hashes the current schema and migration replay regenerates the manifest, both
freshness checks fail against the stale artifact.

Code

supabase/schema.sql[R34880-34881]

+create or replace function public.claim_ingestion_jobs(p_worker_id text, p_claim_limit integer default 1, p_stale_after_minutes integer default 45)
+returns setof jsonb language plpgsql security definer set search_path = '' as $$

Relevance

●●● Strong
Schema changes require regenerated drift metadata; prior migration replay found stale manifests as
accepted findings.
PR-#3633
PR-#3332

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added schema definition changes the bytes hashed by the offline drift test, while the committed
manifest retains the earlier hash that the test requires to match. The migration-replay freshness
verifier also compares the committed manifest with a regenerated one, ignoring only volatile fields,
so its comparison catches the stale function snapshot as well.
supabase/schema.sql[34880-34906]
supabase/drift-manifest.json[1-6]
tests/drift-detection.test.ts[17-24]
supabase/schema.sql[34868-34923]
supabase/drift-manifest.json[1-5]
tests/drift-detection.test.ts[17-22]
scripts/check-drift-manifest-freshness.ts[68-76]
.github/workflows/ci.yml[1551-1558]
PR-#3633
Skill: drift

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The schema definition changed, but the committed drift manifest still describes the previous schema, causing drift freshness checks to fail.

## Fix Focus Areas
- supabase/schema.sql[34868-34923]
- supabase/drift-manifest.json[1-6]

## Recommended Fix
Regenerate the drift manifest from this PR's migration replay using `npm run drift:manifest`, commit the resulting `supabase/drift-manifest.json`, and rerun the offline drift freshness check.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

❌ Tickets: failed to load — Ticket collection could not establish complete evidence

✅ Compliance rules (platform): 9 rules

✅ Skills: 9 invoked

migrate

ingest

supabase-postgres-best-practices

testing-review

security-review

supabase

review

gates

clinical

✅ Cross-repo context — repo relationships

Review mode: Auto: ⚖️ Balanced: Concurrency-sensitive database claim logic warrants a careful single-pass review.

Tip of the day

💡 Did you know, you can move or hide resolved findings to focus on what still needs action

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 2d1906b ⚖️ Balanced

Results up to commit be0b278 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

Resolved findings

1. ~~Migration replay rejects stale manifest~~ ✗ Dismissed

Description

The new claim_ingestion_jobs definition changes supabase/schema.sql, but the committed
supabase/drift-manifest.json still contains the previous schema hash and function snapshot. When
the offline drift test hashes the current schema and migration replay regenerates the manifest, both
freshness checks fail against the stale artifact.

Code

supabase/schema.sql[R34880-34881]

+create or replace function public.claim_ingestion_jobs(p_worker_id text, p_claim_limit integer default 1, p_stale_after_minutes integer default 45)
+returns setof jsonb language plpgsql security definer set search_path = '' as $$

Relevance

●●● Strong
Schema changes require regenerated drift metadata; prior migration replay found stale manifests as
accepted findings.
PR-#3633
PR-#3332

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added schema definition changes the bytes hashed by the offline drift test, while the committed
manifest retains the earlier hash that the test requires to match. The migration-replay freshness
verifier also compares the committed manifest with a regenerated one, ignoring only volatile fields,
so its comparison catches the stale function snapshot as well.
supabase/schema.sql[34880-34906]
supabase/drift-manifest.json[1-6]
tests/drift-detection.test.ts[17-24]
supabase/schema.sql[34868-34923]
supabase/drift-manifest.json[1-5]
tests/drift-detection.test.ts[17-22]
scripts/check-drift-manifest-freshness.ts[68-76]
.github/workflows/ci.yml[1551-1558]
PR-#3633
Skill: drift

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The schema definition changed, but the committed drift manifest still describes the previous schema, causing drift freshness checks to fail.

## Fix Focus Areas
- supabase/schema.sql[34868-34923]
- supabase/drift-manifest.json[1-6]

## Recommended Fix
Regenerate the drift manifest from this PR's migration replay using `npm run drift:manifest`, commit the resulting `supabase/drift-manifest.json`, and rerun the offline drift freshness check.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

chatgpt-codex-connector · 2026-10-18T06:31:59Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-18T06:31:57.904376Z	`be0b278`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

qodo-code-review · 2026-10-18T06:32:26Z

Qodo Fixer

✅ Merged (0) · ☑ Fixed (0)

Process

⏭ Skipped (1)

codeant-ai · 2026-10-18T06:35:35Z

CodeAnt PR Risk: Low Risk

The PR appears safe to merge; the legacy claim now skips jobs re-pointed after selection instead of leasing them under the wrong document's authority.
The SQL test covers both re-pointing to another file type and later claiming the job under its current document.

Assessed commit: be0b2788133f

cubic-dev-ai · 2026-10-18T06:43:07Z

Auto-fix isn't included in your plan ↗

Upgrade to Team

…recheck Byte-identical to the drift-manifest-39104250354 artifact that scripts/generate-drift-manifest.ts (npm run drift:manifest) produced in the Migration replay job of run 39104250354 for be0b278. Only claim_ingestion_jobs(text,integer,integer) def_hash and the schema sha change; no hand edits.

gitar-bot

Gitar has auto-approved this PR (configure)

gitar-bot · 2026-10-18T06:46:33Z

Code Review ✅ Approved 1 closed / 1 findings

🔴 High risk · The SECURITY DEFINER claim now checks document identity before leasing; a flaw could let ingestion run under the wrong document’s authority.

Re-checks the legacy three-argument claim_ingestion_jobs after the job row lock so a job re-pointed to a different document is skipped instead of leased under the old document's authority. No open issues found.

✅ 1 closed

Closed: Quality: PR plans auto-merge on a migrations PR, which repo policy forbids

📄 supabase/migrations/20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sql:1-15
The PR body says "The owner approved this PR including auto-merge". This PR adds a file under supabase/migrations/**, and the repo's safety rules say auto-merge must never be enabled on such a PR, owner approval or not. Merging to main applies the migration to the live clinical database in about 34 s. With auto-merge on, the merge, and so the production deploy, happens whenever checks go green instead of inside an approved window. The fix is to leave auto-merge off, merge by hand only inside the approved window, and remove the auto-merge approval line from the PR body.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Approved

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

Important

Your trial ends in 3 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}

qodo-code-review · 2026-10-18T06:47:11Z

Code review by qodo was updated up to the latest commit 2d1906b

greptile-apps Bot reviewed Oct 18, 2026

View reviewed changes

codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Oct 18, 2026

qodo-code-review Bot added the 🕐 20-40 Minutes label Oct 18, 2026

gitar-bot Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread supabase/migrations/20261018062736_recheck_legacy_ingestion_claim_document_after_job_lock.sql Resolved

qodo-code-review Bot added the 🟠 Medium blast radius label Oct 18, 2026

qodo-code-review Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread supabase/schema.sql Resolved

gitar-bot Bot enabled auto-merge (squash) October 18, 2026 06:46

gitar-bot Bot approved these changes Oct 18, 2026

View reviewed changes

gitar-bot Bot merged commit 57b2cd0 into main Oct 18, 2026
42 checks passed

gitar-bot Bot deleted the fix/legacy-claim-recheck-after-lock branch October 18, 2026 06:54

This was referenced Oct 18, 2026

Fix confirmed merged-PR review findings and stage atomic milestone repair #3638

Closed

Fix confirmed merged-PR review findings and stage atomic milestone repair (rebuild of #3638) #3652

Merged

Conversation

BigSimmo commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Risk and rollout

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codeant-ai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 CodeAnt AI — Review Status

Uh oh!

codeant-ai Bot commented Oct 18, 2026

Thanks for using CodeAnt! 🎉

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

supabase Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

PR Summary by Qodo

Uh oh!

codeant-ai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🏁 CodeAnt Quality Gate Results

✅ Overall Status: PASSED

Quality Gate Details

Uh oh!

Uh oh!

qodo-code-review Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Resolved findings

Resolved findings

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 18, 2026

Codex Review Summary

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

Qodo Fixer

Uh oh!

codeant-ai Bot commented Oct 18, 2026

CodeAnt PR Risk: Low Risk

Uh oh!

cubic-dev-ai Bot commented Oct 18, 2026

Uh oh!

gitar-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Oct 18, 2026

Uh oh!

qodo-code-review Bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

BigSimmo commented Oct 18, 2026 •

edited

Loading

codeant-ai Bot commented Oct 18, 2026 •

edited

Loading

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

supabase Bot commented Oct 18, 2026 •

edited

Loading

claude Bot commented Oct 18, 2026 •

edited

Loading

codeant-ai Bot commented Oct 18, 2026 •

edited

Loading

qodo-code-review Bot commented Oct 18, 2026 •

edited

Loading