Add Railway, CI-failure, and Supabase ingestion webhooks#968
Merged
BigSimmo merged 15 commits intoJul 20, 2026
Conversation
Implements three webhook integrations: 1. POST /api/webhooks/railway — receives Railway deploy status webhooks (secret via ?token=, constant-time) and forwards notable statuses to Slack/Discord. 2. .github/workflows/notify-ci-failure.yml — pings chat when a workflow_run on main/release fails, via curl (no external actions). 3. POST /api/webhooks/supabase/document-change — Supabase Database Webhook that enqueues a reindex job on new/flagged documents. Idempotent (relies on the one-open-job-per-document unique index) and loop-safe (UPDATE only acts on metadata.reindex_requested, then clears the flag). Shared helpers: src/lib/webhooks/secret-auth.ts (constant-time secret gate), src/lib/webhooks/chat-notify.ts (Slack/Discord forwarder), and src/lib/ingestion-enqueue.ts (reindex enqueue mirroring the reindex route's concurrency guards). Adds env vars, .env.example, docs/webhooks.md, codebase index + site-map entries, and unit/route tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHMgBiXbH4Q5tUC7WxoyaX
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds authenticated Railway and Supabase webhook receivers, shared Slack/Discord delivery, CI failure notifications, ingestion job enqueueing, route tests, environment configuration, and webhook documentation. ChangesWebhook integrations
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Supabase
participant DocumentChangeRoute
participant MutationSafety
participant SupabaseAdmin
participant IngestionQueue
Supabase->>DocumentChangeRoute: POST document change event
DocumentChangeRoute->>MutationSafety: Check mutation safety
MutationSafety-->>DocumentChangeRoute: Safety result
DocumentChangeRoute->>SupabaseAdmin: Reload owner-scoped document
DocumentChangeRoute->>IngestionQueue: Enqueue reindex job
IngestionQueue->>SupabaseAdmin: Update document and insert ingestion job
DocumentChangeRoute->>SupabaseAdmin: Clear reindex_requested
DocumentChangeRoute-->>Supabase: Return enqueue or skip response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
BigSimmo
marked this pull request as ready for review
July 20, 2026 08:28
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BigSimmo
enabled auto-merge (squash)
July 20, 2026 08:41
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/ingestion-enqueue.ts (1)
36-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct unit tests for the concurrency-sensitive rollback logic.
enqueueDocumentReindexJobhandles two race conditions (FK-deleted document, competing-job rollback via the rollback fence) but is only exercised via the Supabase route test with this function fully mocked out. A dedicated test suite exercising the23503/23505branches directly against a mocked Supabase client would catch regressions in this concurrency-critical path.🤖 Prompt for 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. In `@src/lib/ingestion-enqueue.ts` around lines 36 - 121, Add dedicated unit tests for enqueueDocumentReindexJob covering the 23503 document-deleted outcome and the 23505 competing-job path, including rollback only when no open competing job remains and respecting the rollback fence. Mock the Supabase client while verifying queue updates, job insertion, competing-job lookup, and rollback behavior directly.
🤖 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/notify-ci-failure.yml:
- Around line 42-45: Update the workflow_run condition guarding notifications to
also require github.event.workflow_run.event == 'push', while preserving the
existing failure and main/release branch checks. Keep webhook-secret
notifications limited to trusted push runs.
In `@docs/webhooks.md`:
- Around line 7-11: Renumber the integration rows in the webhook documentation
table sequentially from 1 through 3, updating the Supabase document-change row
currently labeled 5 to 3 while leaving the integration details unchanged.
In `@src/app/api/webhooks/supabase/document-change/route.ts`:
- Around line 106-121: Update clearReindexFlagIfRequested to return whether the
requested RPC clear succeeded, and propagate RPC failures instead of only
logging them. In both route call sites around checkIngestionMutationSafety,
inspect that result and return a non-2xx response when reindexRequested is true
but the clear fails, while preserving the existing skip or success responses
after a successful clear or when no clear was requested.
---
Nitpick comments:
In `@src/lib/ingestion-enqueue.ts`:
- Around line 36-121: Add dedicated unit tests for enqueueDocumentReindexJob
covering the 23503 document-deleted outcome and the 23505 competing-job path,
including rollback only when no open competing job remains and respecting the
rollback fence. Mock the Supabase client while verifying queue updates, job
insertion, competing-job lookup, and rollback behavior directly.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f273e36d-ea6c-4d5f-b4f1-3fb29781c79a
📒 Files selected for processing (16)
.env.example.github/workflows/notify-ci-failure.ymldocs/codebase-index.mddocs/site-map.mddocs/webhooks.mdscripts/generate-site-map.tssrc/app/api/webhooks/railway/route.tssrc/app/api/webhooks/supabase/document-change/route.tssrc/lib/env.tssrc/lib/ingestion-enqueue.tssrc/lib/webhooks/chat-notify.tssrc/lib/webhooks/secret-auth.tstests/webhooks-chat-notify.test.tstests/webhooks-railway-route.test.tstests/webhooks-secret-auth.test.tstests/webhooks-supabase-document-change-route.test.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
… trigger - Supabase document-change webhook: clearReindexFlagIfRequested now reports success/failure; when a requested clear fails the route responds 500 so Supabase retries delivery until the flag is cleared (the enqueue is idempotent, so a retry cannot double-queue). Prevents a failed clear from silently defeating the documented loop-safety guarantee. - notify-ci-failure.yml: gate on head_repository.full_name == github.repository so only runs originating in this repo can post with repo secrets (blocks fork PR forgery/spam). Preferred over an `event == 'push'` filter, which would drop the scheduled production monitors (schedule runs, not push). - docs/webhooks.md: renumber the integration table 1/2/3 and note the clear-failure retry behavior. - Add tests/ingestion-enqueue.test.ts covering the 23503/23505 concurrency branches directly, plus a route test for the clear-failure 500 path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHMgBiXbH4Q5tUC7WxoyaX
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/ingestion-enqueue.test.ts (3)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isCompetingSelectis set but never read.The
stateobject and itsisCompetingSelectflag are dead code — the flag is set inselect("id")but never consulted anywhere in the mock. Remove the unused state to reduce confusion.♻️ Remove dead state
function builder(table: string) { - const state = { isCompetingSelect: false }; const b: Record<string, unknown> = {}; b.update = () => { if (table === "documents") calls.documentUpdates += 1; return b; }; b.insert = () => b; b.select = (cols?: string) => { - if (cols === "id") state.isCompetingSelect = true; return b; };Also applies to: 32-32
🤖 Prompt for 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. In `@tests/ingestion-enqueue.test.ts` at line 24, Remove the unused state object and all assignments to its isCompetingSelect flag in the mock, including the related select("id") handling. Preserve the mock’s existing behavior while eliminating this dead state.
59-103: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing test for the initial document update error path.
The implementation throws when the initial
documents.updatefails (if (updateError) throw new Error(updateError.message)), but no test covers this branch. The mock'sthenalways resolves with{ error: null }, making the path untestable without modification. Add a configurable update-error path to the mock and a test case asserting the throw.♻️ Add update-error config and test
type ClientConfig = { jobInsert?: { data: unknown; error: { code?: string; message: string } | null }; competing?: { data: unknown[]; error: null }; + updateError?: { message: string }; }; // In makeClient, update the then handler: - b.then = (resolve: (v: unknown) => void, reject?: (e: unknown) => void) => - Promise.resolve({ error: null }).then(resolve, reject); + b.then = (resolve: (v: unknown) => void, reject?: (e: unknown) => void) => + Promise.resolve({ error: cfg.updateError ?? null }).then(resolve, reject);it("throws when the initial document update fails", async () => { const { enqueueDocumentReindexJob, client } = await load({ updateError: { message: "update failed" }, }); await expect(enqueueDocumentReindexJob({ supabase: client as never, document: doc })) .rejects.toThrow("update failed"); });🤖 Prompt for 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. In `@tests/ingestion-enqueue.test.ts` around lines 59 - 103, Extend the test mock’s configurable setup, including load, so the initial documents.update operation can return an injected updateError instead of always resolving with no error. Add a test in the enqueueDocumentReindexJob suite that passes updateError and asserts the call rejects with the configured message, covering the initial updateError branch.
21-48: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMock doesn't verify
owner_idscoping on document writes.The PR emphasizes owner-scoped server-side document access, but the mock's
.eq()unconditionally returnsbwithout recording arguments. This means the tests cannot confirm that.eq("owner_id", ownerId)is actually called on both the initial queue update and the rollback update. Consider tracking.eq()arguments so tests can assert owner scoping is applied.♻️ Track eq calls to verify owner scoping
function makeClient(cfg: ClientConfig) { - const calls = { documentUpdates: 0, competingSelects: 0 }; + const calls = { documentUpdates: 0, competingSelects: 0, eqCalls: [] as string[] }; function builder(table: string) { const b: Record<string, unknown> = {}; b.update = () => { if (table === "documents") calls.documentUpdates += 1; return b; }; b.insert = () => b; b.select = (cols?: string) => { return b; }; - b.eq = () => b; + b.eq = (col: string) => { calls.eqCalls.push(col); return b; }; b.in = () => b; // ... } return { client: { from: (t: string) => builder(t) }, calls }; }Then in tests, assert that
"owner_id"appears incalls.eqCallsfor the initial update and rollback paths.🤖 Prompt for 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. In `@tests/ingestion-enqueue.test.ts` around lines 21 - 48, Update the makeClient mock’s builder and calls tracking to record .eq() arguments, especially the column names, instead of returning b without observation. Expose the recorded eqCalls and add assertions in the initial queue-update and rollback test paths that owner_id scoping is applied to both document writes.
🤖 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.
Nitpick comments:
In `@tests/ingestion-enqueue.test.ts`:
- Line 24: Remove the unused state object and all assignments to its
isCompetingSelect flag in the mock, including the related select("id") handling.
Preserve the mock’s existing behavior while eliminating this dead state.
- Around line 59-103: Extend the test mock’s configurable setup, including load,
so the initial documents.update operation can return an injected updateError
instead of always resolving with no error. Add a test in the
enqueueDocumentReindexJob suite that passes updateError and asserts the call
rejects with the configured message, covering the initial updateError branch.
- Around line 21-48: Update the makeClient mock’s builder and calls tracking to
record .eq() arguments, especially the column names, instead of returning b
without observation. Expose the recorded eqCalls and add assertions in the
initial queue-update and rollback test paths that owner_id scoping is applied to
both document writes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fde4e48-916d-42e5-a8c7-8deaa2f50ef2
📒 Files selected for processing (6)
.github/workflows/notify-ci-failure.ymldocs/webhooks.mdsrc/app/api/webhooks/supabase/document-change/route.tssrc/lib/env.tstests/ingestion-enqueue.test.tstests/webhooks-supabase-document-change-route.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/env.ts
- .github/workflows/notify-ci-failure.yml
- src/app/api/webhooks/supabase/document-change/route.ts
- tests/webhooks-supabase-document-change-route.test.ts
- docs/webhooks.md
- Remove dead isCompetingSelect state from the mock. - Cover the initial documents.update error branch (throws). - Record .eq() columns and assert owner_id scoping on both the queue-state update and the rollback update. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHMgBiXbH4Q5tUC7WxoyaX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds three webhook integrations recommended for this repo (all optional / fail-closed when their secret is unset):
POST /api/webhooks/railway. Receives Railway deploy status webhooks (shared secret via?token=, since Railway only lets you set a URL; compared constant-time) and forwards notable statuses (SUCCESS/FAILED/CRASHED/REMOVED) for the app + worker services to Slack/Discord. This surfaces the deploy outcome GitHub can't see..github/workflows/notify-ci-failure.yml. Fires onworkflow_run: completedfor the key workflows and pings chat when a run onmain/release/*fails. Usescurlonly (no external actions, so the pinned-action allowlist is untouched); readsSLACK_WEBHOOK_URL/DISCORD_WEBHOOK_URLrepo secrets.POST /api/webhooks/supabase/document-change. A Supabase Database Webhook onpublic.documentsthat enqueues one reindex job the worker then claims, turning the polling autopilot into an event-driven path.Shared building blocks:
src/lib/webhooks/secret-auth.ts— constant-time, byte-length-safe secret gate (mirrorsdeep-probe-auth.ts).src/lib/webhooks/chat-notify.ts— Slack/Discord forwarder that never throws (a failed post still lets the receiver 2xx its provider).src/lib/ingestion-enqueue.ts— reindex enqueue that mirrors the reindex route's rollback-fence + one-open-job-per-document concurrency guards.Idempotency + loop-safety (Supabase route): INSERT of a not-yet-
indexeddocument enqueues (duplicate app-upload inserts are absorbed by theingestion_jobsunique index); UPDATE only acts onmetadata.reindex_requested === trueand then clears that flag, so the worker's own completion writes can't retrigger a loop. Everydocumentswrite is owner-scoped.Also adds env vars,
.env.exampleentries,docs/webhooks.md, and codebase-index + site-map entries.Verification
npm run typecheck— cleannpm run linton the new/changed files — cleannpm run testunit suite — 2992 passing; newtests/webhooks-*.test.ts(23) includedcheck:github-actions,check:owner-scope,docs:check-index,check:docs-links,check:ci-scope,check:knip,check:maintainability-budgets,sitemap:checkall passnpm run verify:pr-localwrapper andnpm run verify:release/ live evals — no code path here touches retrieval/ranking/answer generation, and the release/eval gates require live provider (OpenAI/Supabase) keys, which is outside the provider-confirmation boundary. One pre-existing unit failure (tests/pdf-extraction-budget.test.ts) reproduces on cleanorigin/mainin this VM (Python OCR timing) and is unrelated to this change.Risk and rollout
503when its secret is unset and401on a bad secret, and the Supabase enqueue reuses the existing reindex concurrency guards (idempotent + loop-safe)..github/workflows/notify-ci-failure.yml. No migrations, no schema changes, no new RPCs (the Supabase route calls the existingapply_document_metadata_patch).docs/webhooks.md.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes: this change is webhook/transport infrastructure only — it does not alter retrieval, ranking, answer generation, source rendering, or any clinical decision-support behavior, so the SaMD classification is unchanged. The receivers use the service-role admin client server-side and scope every
documentswrite byowner_id; the Supabase route no-ops in demo mode.🤖 Generated with Claude Code
Summary by CodeRabbit
.env.examplewith webhook secrets/destinations (commented by default).