Repository navigation
Ship trustworthy daily sweep ingestion and clearer cards - #104
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
/review claude Review this release for P0/P1 correctness, security, RLS and migration safety, idempotency, ingestion trust, and production readiness. Do not merge. |
|
Warning Review limit reached
Next review available in: 40 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. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds authenticated official URL intake and destination-policy administration, claim-aware durable work processing, ingestion readiness and stale-run recovery, provenance observation persistence, fail-closed redirect policy checks, and related UI, database migrations, and tests. ChangesOfficial ingestion controls
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
/review claude Fresh review requested now that all automated gates are green. Review P0/P1 correctness, security, RLS and migration safety, idempotency, ingestion trust, and production readiness. Do not merge. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb9e398bc7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (19)
lib/db/__tests__/discovery-work-claims.test.ts (1)
41-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the empty-items and invalid-count branches of
enqueueOfficialUrlIntakeWork.Current tests cover the happy path and the idempotency-conflict mapping, but not the
items.length === 0short-circuit or the "invalid inserted count" guard (datanot a safe integer / negative /> items.length) described inlib/db/discovery-work.ts. Both guard an idempotency-sensitive write 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 `@lib/db/__tests__/discovery-work-claims.test.ts` around lines 41 - 61, Extend the tests for enqueueOfficialUrlIntakeWork to cover the empty-items short-circuit, asserting it returns zero without calling mocks.rpc, and the invalid inserted-count guard for unsafe, negative, and greater-than-input counts, asserting each rejects with the expected error. Reuse the existing items and RPC mock setup while preserving the current happy-path and conflict-mapping coverage.components/official-url-intake-status.tsx (1)
14-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
formatWhenduplicates the identical helper inapp/admin/sources/page.tsx(lines 43-51).Consider a shared
lib/format-datetime.tsso admin timestamp formatting stays consistent as options change.🤖 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 `@components/official-url-intake-status.tsx` around lines 14 - 21, Extract the duplicated formatWhen helper into a shared lib/format-datetime.ts utility, then update both formatWhen usages in official-url-intake-status.tsx and app/admin/sources/page.tsx to reuse it. Preserve the existing en-US date formatting options and output.app/admin/sources/page.tsx (2)
175-181: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winReturn a real 404/redirect instead of
nullfor unauthorized visitors.Rendering
nullyields a 200 with an empty page: no feedback for a legitimately signed-out admin, and no distinct status for probes.notFound()(orredirect("/sign-in")when unauthenticated) matches App Router conventions and keeps the surface indistinguishable from a non-existent route.♻️ Suggested change
+import { notFound } from "next/navigation";const authUser = await ensureCurrentAppUser(); if ( !authUser || (!authUser.appUser.is_admin && !authUser.appUser.is_owner) ) { - return null; + notFound(); }🤖 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 `@app/admin/sources/page.tsx` around lines 175 - 181, Update the authorization branch in the page’s ensureCurrentAppUser flow so unauthenticated visitors are redirected to “/sign-in” and authenticated users lacking admin or owner privileges invoke notFound() instead of returning null. Preserve the existing access checks for authorized users.
183-200: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRoute these readers through a shared admin/owner-authorized entry point.
These functions read operational tables with
createServiceRoleClient()(RLS-bypassing), andrequireOperator()/ensureCurrentAppUser()are not enforced before the calls. Add a shared guard for this page/API route pattern so future callers cannot expose these reads by calling the helpers directly.🤖 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 `@app/admin/sources/page.tsx` around lines 183 - 200, Add a shared admin/owner authorization entry point for the page’s operational readers, enforcing requireOperator() and ensureCurrentAppUser() before getSourceHealth(), getOfficialUrlIntakeBacklogStatus(), and listCurrentOfficialDestinationPolicies() can access service-role data. Route the current page/API calls through this guarded entry point and make the helpers inaccessible for unguarded direct use, preserving the existing fail-closed handling for unreadable statuses and policies.Source: Coding guidelines
app/admin/sources/__tests__/page.test.tsx (1)
71-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider also covering the destination-policy read failure branch.
page.tsxhas a second independenttry/catch(officialDestinationPoliciesReadable). A sibling test asserting the "Policy data unreadable" rendering would lock in that fail-closed branch too.🤖 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 `@app/admin/sources/__tests__/page.test.tsx` around lines 71 - 85, Add a sibling test in the admin sources page test suite for the independent officialDestinationPoliciesReadable failure path: mock the destination-policy read to reject, render AdminSourcesPage, and assert the output contains “Policy data unreadable.” Keep the existing backlog failure test unchanged and verify the relevant policy-read mock is invoked.components/official-destination-policy-console.tsx (1)
94-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale success/error text persists while the operator edits the next decision.
resultis only reset at submit time, so the previous "Decision appended…" message stays next to the button while fields are being changed for a different scope. Clearing it on input change (or keying it to the current signature) avoids a misleading confirmation.🤖 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 `@components/official-destination-policy-console.tsx` around lines 94 - 129, Clear the existing result whenever the operator edits any decision input, rather than only at submission. Update the input-change handlers or shared form state logic in the component containing the startTransition submission flow so prior success or error messages cannot remain visible for a new decision, while preserving the current submit behavior.package.json (1)
65-69: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExact-version override keys are brittle for advisory remediation.
brace-expansion@5.0.6/@5.0.7only patch those two exact resolutions; if a transitive dep later resolves 5.0.9 (or a still-vulnerable 5.0.x), the override silently stops applying. A range key expresses the intent durably.♻️ Suggested change
- "brace-expansion@5.0.6": "5.0.8", - "brace-expansion@5.0.7": "5.0.8", + "brace-expansion@<5.0.8": "5.0.8",🤖 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 `@package.json` around lines 65 - 69, Update the brace-expansion entries in the package.json overrides configuration to use a single appropriate 5.0.x range key instead of separate exact-version keys for 5.0.6 and 5.0.7, while preserving the patched 5.0.8 resolution for all matching vulnerable versions.app/api/health/route.ts (1)
18-23: 🚀 Performance & Scalability | 🔵 TrivialHealth checks now perform live DB round-trips on every probe.
getIngestionReadiness()callsgetSourceHealth()andgetOfficialDestinationPolicyReadiness(), both DB-backed, on every/api/healthrequest. If this endpoint is polled frequently by an uptime monitor or load balancer, this adds sustained DB load and latency to a path that should ideally stay cheap and fast, especially since none of it appears cached.Consider caching the ingestion readiness result for a short TTL (e.g., 10–30s) so frequent health probes don't each trigger fresh DB queries.
🤖 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 `@app/api/health/route.ts` around lines 18 - 23, Cache the result of getIngestionReadiness in the health-check flow for a short TTL, such as 10–30 seconds, so repeated /api/health probes reuse recent readiness data instead of issuing DB queries on every request. Preserve the existing ok calculation and refresh the cached result after the TTL expires.lib/__tests__/ingestion-gate.test.ts (1)
118-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
killSwitch: truecase for theofficial_directexception.The bypass in
lib/ingestion/gate.ts(Lines 120-129) reduces descriptor-level protection for this capability to exactly one condition:descriptor.killSwitch. Nothing in this suite asserts that condition, so a regression that drops it (e.g.ineligible = destinationScopedOfficialCapability ? null : ...) would still pass.♻️ Suggested addition
it("does not extend the exception to a fixed-host or discovery descriptor", () => { + // pinned separately below: the code-level kill switch is the ONLY + // descriptor condition still guarding this capability.it("still honors the code-level kill switch for the capability descriptor", () => { const official = SOURCE_REGISTRY.find((source) => source.id === "official_direct")!; const decision = evaluateSourceGate({ descriptor: { ...official, killSwitch: true }, record: record({ id: "official_direct", complianceState: "approved_for_production" }), ingestionEnabled: "true", }); expect(decision).toMatchObject({ allowed: false, reason: "kill_switch" }); });🤖 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 `@lib/__tests__/ingestion-gate.test.ts` around lines 118 - 167, Add a test in the `official_direct capability uses per-destination authority` suite that clones the registry descriptor with `killSwitch: true`, evaluates it with an approved production record and ingestion enabled, and asserts the decision is denied with reason `kill_switch`.lib/ingestion/http.ts (1)
1032-1058: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFold the triplicated policy-refresh failure mapping into one helper.
The same try/catch →
policy_unavailableblock now exists infollowChain,followAssetChain, andguard, differing only in the message. A single helper keeps the three paths from drifting.♻️ Suggested refactor
async function reachDenial(url: string, context: string): Promise<SourceFailureResult | null> { try { return (await isWithinReach(url)) ? null : policyFailure(url, "blocked_by_policy", context); } catch (error) { return policyFailure( url, "policy_unavailable", `destination authority could not be refreshed for ${url}: ${ error instanceof Error ? error.message : String(error) }`, ); } }
guardand both redirect walks then branch on a single returned failure instead of repeating the catch.Also applies to: 1120-1145
🤖 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 `@lib/ingestion/http.ts` around lines 1032 - 1058, Extract the repeated isWithinReach try/catch and policyFailure mapping into a shared reachDenial(url, context) helper near the existing policy utilities. Update followChain, followAssetChain, and guard to call it and return the failure when non-null, preserving each caller’s existing context message and behavior for reachable targets.lib/__tests__/ingestion-official-destination-policy.test.ts (1)
325-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining production-approval branches of the schema.
officialDestinationPolicyEventSchema'ssuperRefinehas six independent gates (confirmation flag, ToS posture, robots posture, terms URL, robots URL, expiry window); only the expiry bound is exercised here. A table-driven case per omitted field would pin the fail-closed contract, plus one happy-path parse assertingsuccess: trueso the whole refine can't silently start rejecting valid approvals.♻️ Suggested addition
const approved = { complianceState: "approved_for_production", robotsPosture: "permissive", tosPosture: "permits_use", termsUrl: "https://promotions.example.com/terms", robotsUrl: "https://promotions.example.com/robots.txt", reviewExpiresAt: new Date(Date.now() + 30 * 86_400_000).toISOString(), productionApprovalConfirmed: true, }; it("accepts a fully evidenced production approval", () => { expect(officialDestinationPolicyEventSchema.safeParse(decision(approved)).success).toBe(true); }); it.each([ { productionApprovalConfirmed: false }, { tosPosture: "requires_agreement" }, { robotsPosture: "restricted" }, { termsUrl: null }, { robotsUrl: null }, { reviewExpiresAt: null }, ])("refuses production approval missing %o", (missing) => { expect( officialDestinationPolicyEventSchema.safeParse(decision({ ...approved, ...missing })).success, ).toBe(false); });As per coding guidelines, "Add tests for non-trivial logic."
🤖 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 `@lib/__tests__/ingestion-official-destination-policy.test.ts` around lines 325 - 374, Extend the “official destination decision input” tests with a shared fully approved production case and assert that it parses successfully. Add table-driven coverage for each remaining superRefine gate—confirmation disabled, non-permissive ToS or robots posture, missing terms or robots URL, and missing expiry—asserting each variant is rejected while preserving the existing expiry-window test.Source: Coding guidelines
lib/db/ingestion.ts (1)
162-193: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse codepoint key ordering (and guard non-finite numbers) for a hash used as a durable identity.
localeCompareis ICU/locale-dependent, so key order — and therefore the sha256 identity persisted inlisting_ingestion_observation.provenance_identity— is not guaranteed byte-stable across runtimes or ICU versions. A plain codepoint comparison is the canonical-JSON convention. Separately,JSON.stringify(NaN)/Infinityyields"null", so a non-finiteextractionConfidencesilently hashes identically tonull.♻️ Proposed fix
const entries = Object.entries(value as Record<string, unknown>) .filter(([, item]) => item !== undefined) - .sort(([left], [right]) => left.localeCompare(right)); + .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)); @@ if ( typeof value === "string" || - typeof value === "number" || + (typeof value === "number" && Number.isFinite(value)) || typeof value === "boolean" ) {🤖 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 `@lib/db/ingestion.ts` around lines 162 - 193, Update canonicalJson to sort object keys with a deterministic codepoint comparison instead of localeCompare, ensuring provenanceIdentity is byte-stable across runtimes. In the number serialization branch, reject non-finite values before JSON.stringify so NaN and Infinity cannot hash as null; preserve existing handling for valid JSON values and unsupported types.supabase/tests/database/source_discovery_work_claims.test.sql (1)
3-91: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe claim CAS guarantee itself is untested here.
The file is named for claims, but nothing exercises
claim_source_discovery_work→complete/defer/dead_letter_source_discovery_work. The migration's headline property — a stale token cannot acknowledge a refreshed generation — is exactly the case worth pinning in pgTAP: claim an item, refresh its payload viaenqueue_source_discovery_work(non-official source), then assertcomplete_source_discovery_work(..., old_token)returnsfalse. Want me to draft those assertions?🤖 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 `@supabase/tests/database/source_discovery_work_claims.test.sql` around lines 3 - 91, Extend the pgTAP plan and add coverage for the claim CAS flow using the existing non-official work item: claim it with claim_source_discovery_work, save the returned token, refresh the item through enqueue_source_discovery_work, then assert complete_source_discovery_work with the old token returns false. Include the corresponding setup and cleanup assertions while preserving the existing privilege and official-intake checks.lib/ingestion/orchestrator.ts (4)
199-214: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePer-hop policy refresh means one DB round-trip per request, redirect, and image fetch.
urlPolicycallsloadOfficialDestinationPolicies({ refresh: true }), so every hop of every lead re-reads the full policy table (plus the per-lead refresh at Line 285). Correct, but it multiplies control-plane reads by ~3-4x per lead. Consider a short TTL (e.g. a few seconds) on the cache so revocation latency stays bounded while collapsing the redundant reads within a single request chain.🤖 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 `@lib/ingestion/orchestrator.ts` around lines 199 - 214, The urlPolicy callback in the officialHttp creation flow refreshes the full policy table for every request, redirect, and asset fetch. Add a short, shared TTL-based cache for loadOfficialDestinationPolicies results so hops within the same request chain reuse policies while revocations are still observed within the bounded TTL; preserve evaluateOfficialDestinationPolicy and the existing per-lead refresh behavior.
216-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant cast in
readOfficialStats.
officialHttpis already typedReturnType<typeof createSourceHttpClient> | null; the local alias andasadd nothing.♻️ Simplify
- const readOfficialStats = () => { - const client = - officialHttp as ReturnType<typeof createSourceHttpClient> | null; - return client?.stats() ?? { + const readOfficialStats = () => { + return officialHttp?.stats() ?? { requests: 0, budget: official.requestBudgetPerRun, notModified: 0, failures: 0, }; };🤖 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 `@lib/ingestion/orchestrator.ts` around lines 216 - 225, Remove the redundant local alias and type assertion in readOfficialStats, and call stats() directly on officialHttp with the existing null-safe fallback unchanged.
284-294: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDefer failure inside the catch handler discards the original policy error.
If
deferLead()rejects, its error replaces theofficial destination policy unavailable: …diagnostic that the run notes rely on. Wrap the defer so the control-plane cause survives.🛡️ Proposed fix
await loadOfficialDestinationPolicies({ refresh: true }).catch( async (error: unknown) => { - await deferLead(); + await deferLead().catch(() => undefined); throw new Error(🤖 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 `@lib/ingestion/orchestrator.ts` around lines 284 - 294, Update the catch handler around loadOfficialDestinationPolicies so deferLead() failures cannot replace the original “official destination policy unavailable” error. Preserve the policy-loading error as the thrown diagnostic while attempting deferLead(), including any defer failure only as secondary context if supported by the existing error-handling conventions.
284-306: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA permanently denied destination is deferred forever with no terminal state.
policy_missing/policy_not_production_approvedare stable answers for an unreviewed host, yet the item is only deferred. Withdefer_source_discovery_workincrementing attempts and capping backoff at 1440 minutes and no attempt ceiling, such items stay open indefinitely, keep inflating theretryingbacklog surfaced bygetOfficialUrlIntakeBacklogStatus, and re-consume the per-runtakelimit ahead of fresh work. Consider dead-lettering after a bounded attempt count (the reason string is already available), keeping only transient reasons (policy_unavailable) on infinite retry.🤖 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 `@lib/ingestion/orchestrator.ts` around lines 284 - 306, Update the denied-destination handling in the orchestration flow around evaluateOfficialDestinationPolicy: permanently denied reasons such as policy_missing and policy_not_production_approved must transition to a terminal/dead-letter state after a bounded attempt count instead of always calling deferLead, while transient policy_unavailable results continue retrying indefinitely. Reuse the existing attempt tracking, dead-letter mechanism, and destinationDecision.reason, and preserve the current skip/count behavior.lib/__tests__/ingestion-work-queue.test.ts (1)
37-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
deadLetterclaim-CAS test alongside the new stale-completetest.
deadLetterhas the identical claim-token compare-and-swap logic ascomplete/deferinlib/ingestion/work-queue.ts, but onlycompletegets a stale-claim regression test here.deadLetter's CAS behavior against the real memory-queue implementation isn't exercised anywhere in the provided context (the otherdeadLetterreference uses a fully mocked queue).✅ Suggested addition
+ it("rejects a stale dead-letter after changed payload invalidates the claim", async () => { + const queue = createMemoryDiscoveryWorkQueue(); + await queue.enqueue([{ key: "post-1", payload: { title: "Original" } }]); + const [stale] = await queue.take(1); + + await queue.enqueue([{ key: "post-1", payload: { title: "Corrected" } }]); + + await expect( + queue.deadLetter(stale.key, stale.claimToken, "bad payload"), + ).rejects.toThrow('discovery work claim lost for "post-1"'); + });As per coding guidelines,
**/*.{test,spec}.{ts,tsx}: "Add tests for non-trivial logic."🤖 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 `@lib/__tests__/ingestion-work-queue.test.ts` around lines 37 - 55, Add a regression test alongside the stale-completion test that verifies memory-queue deadLetter rejects a stale claim token after the same key is re-enqueued with changed payload. Use createMemoryDiscoveryWorkQueue, enqueue and take the original item, enqueue the corrected payload, assert deadLetter(stale.key, stale.claimToken) rejects with the claim-lost error, then take the corrected item and verify its payload and refreshed claimToken.Source: Coding guidelines
lib/db/source-health.ts (1)
171-182: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider issuing the three health probes in parallel.
source_registry,ingestion_run, and the new queue probe are fully independent and each already has isolated error handling, but they run as three sequential round trips on a request path (/api/healthand the admin console).Promise.allover three.then-style wrappers would cut latency roughly threefold without changing the per-read fail-closed semantics.🤖 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 `@lib/db/source-health.ts` around lines 171 - 182, Update the health-check flow containing the source_registry, ingestion_run, and source_discovery_work_item probes to execute all three independent Supabase reads concurrently, using Promise.all with each probe’s existing isolated error handling. Preserve the current fail-closed behavior and queueReadable/source health assignments for each individual probe.
🤖 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 `@app/api/admin/ingestion/destinations/route.ts`:
- Around line 51-89: Update the unexpected-error fallback in the destination
decision handler after the explicit 40001, 23505, and 42501 branches to report
the caught error through the route’s Sentry integration and return an HTTP 500
response instead of 422. Preserve the existing mapped responses and user-facing
message for the explicitly handled codes, matching the unexpected-error behavior
used by the sibling official-URLs route.
In `@components/official-destination-policy-console.tsx`:
- Around line 76-78: Update the reviewExpiresAt conversion in the official
destination policy form so the selected date is interpreted in the operator’s
local timezone rather than hard-coded to UTC; alternatively, explicitly label
the field as UTC and preserve that contract. Ensure the resulting instant
represents the selected calendar date’s local end-of-day.
In `@components/official-url-intake-console.tsx`:
- Around line 41-70: Extract the URL validation and normalization logic from
normalizeOfficialUrl into a shared normalizer, then reuse it in both
normalizeOfficialUrl and the officialHttpsUrlSchema transform. Remove the
duplicated rules from the console component while preserving HTTPS, credential,
port, hostname, length, and hash normalization behavior so client and server
produce identical URLs.
- Around line 125-141: The official URL intake flow needs an admin force
re-intake path that bypasses stale idempotency keys after retryable work
completes or dead-letters. Update the submit logic using
deriveOfficialUrlIdempotencyKey so forced admin retries generate and use fresh
key metadata, while preserving the stable-key behavior for normal submissions.
In `@lib/db/official-url-intake-status.ts`:
- Around line 64-107: Remove the consistency assertions in
getOfficialUrlIntakeBacklogStatus that throw when openCount and oldestPendingAt
disagree, since the parallel queries may legitimately observe different queue
states. Treat the query results as concurrent-read skew and derive the reported
age from the available reportedOldestAt value while preserving the existing
error handling for oldestResult failures.
In `@lib/ingestion/gate.ts`:
- Around line 120-129: Update the destinationScopedOfficialCapability branch in
the gate eligibility calculation to preserve registry containment failures: when
the official_direct descriptor’s SourceComplianceState is in CONTAINMENT_STATES,
return that ineligibility before allowing the branch to fall back to null or
kill_switch. Keep the existing official_direct registry entry’s complianceState
as "reviewed" and retain the current behavior for non-containment states.
In `@lib/official-url-intake-schema.ts`:
- Around line 80-92: Extend the batch-level superRefine validation to track
normalized officialUrl values in addition to idempotencyKey values. Add a custom
issue at ["entries", index, "officialUrl"] when a normalized URL repeats within
the batch, while preserving the existing idempotency-key uniqueness validation.
---
Nitpick comments:
In `@app/admin/sources/__tests__/page.test.tsx`:
- Around line 71-85: Add a sibling test in the admin sources page test suite for
the independent officialDestinationPoliciesReadable failure path: mock the
destination-policy read to reject, render AdminSourcesPage, and assert the
output contains “Policy data unreadable.” Keep the existing backlog failure test
unchanged and verify the relevant policy-read mock is invoked.
In `@app/admin/sources/page.tsx`:
- Around line 175-181: Update the authorization branch in the page’s
ensureCurrentAppUser flow so unauthenticated visitors are redirected to
“/sign-in” and authenticated users lacking admin or owner privileges invoke
notFound() instead of returning null. Preserve the existing access checks for
authorized users.
- Around line 183-200: Add a shared admin/owner authorization entry point for
the page’s operational readers, enforcing requireOperator() and
ensureCurrentAppUser() before getSourceHealth(),
getOfficialUrlIntakeBacklogStatus(), and
listCurrentOfficialDestinationPolicies() can access service-role data. Route the
current page/API calls through this guarded entry point and make the helpers
inaccessible for unguarded direct use, preserving the existing fail-closed
handling for unreadable statuses and policies.
In `@app/api/health/route.ts`:
- Around line 18-23: Cache the result of getIngestionReadiness in the
health-check flow for a short TTL, such as 10–30 seconds, so repeated
/api/health probes reuse recent readiness data instead of issuing DB queries on
every request. Preserve the existing ok calculation and refresh the cached
result after the TTL expires.
In `@components/official-destination-policy-console.tsx`:
- Around line 94-129: Clear the existing result whenever the operator edits any
decision input, rather than only at submission. Update the input-change handlers
or shared form state logic in the component containing the startTransition
submission flow so prior success or error messages cannot remain visible for a
new decision, while preserving the current submit behavior.
In `@components/official-url-intake-status.tsx`:
- Around line 14-21: Extract the duplicated formatWhen helper into a shared
lib/format-datetime.ts utility, then update both formatWhen usages in
official-url-intake-status.tsx and app/admin/sources/page.tsx to reuse it.
Preserve the existing en-US date formatting options and output.
In `@lib/__tests__/ingestion-gate.test.ts`:
- Around line 118-167: Add a test in the `official_direct capability uses
per-destination authority` suite that clones the registry descriptor with
`killSwitch: true`, evaluates it with an approved production record and
ingestion enabled, and asserts the decision is denied with reason `kill_switch`.
In `@lib/__tests__/ingestion-official-destination-policy.test.ts`:
- Around line 325-374: Extend the “official destination decision input” tests
with a shared fully approved production case and assert that it parses
successfully. Add table-driven coverage for each remaining superRefine
gate—confirmation disabled, non-permissive ToS or robots posture, missing terms
or robots URL, and missing expiry—asserting each variant is rejected while
preserving the existing expiry-window test.
In `@lib/__tests__/ingestion-work-queue.test.ts`:
- Around line 37-55: Add a regression test alongside the stale-completion test
that verifies memory-queue deadLetter rejects a stale claim token after the same
key is re-enqueued with changed payload. Use createMemoryDiscoveryWorkQueue,
enqueue and take the original item, enqueue the corrected payload, assert
deadLetter(stale.key, stale.claimToken) rejects with the claim-lost error, then
take the corrected item and verify its payload and refreshed claimToken.
In `@lib/db/__tests__/discovery-work-claims.test.ts`:
- Around line 41-61: Extend the tests for enqueueOfficialUrlIntakeWork to cover
the empty-items short-circuit, asserting it returns zero without calling
mocks.rpc, and the invalid inserted-count guard for unsafe, negative, and
greater-than-input counts, asserting each rejects with the expected error. Reuse
the existing items and RPC mock setup while preserving the current happy-path
and conflict-mapping coverage.
In `@lib/db/ingestion.ts`:
- Around line 162-193: Update canonicalJson to sort object keys with a
deterministic codepoint comparison instead of localeCompare, ensuring
provenanceIdentity is byte-stable across runtimes. In the number serialization
branch, reject non-finite values before JSON.stringify so NaN and Infinity
cannot hash as null; preserve existing handling for valid JSON values and
unsupported types.
In `@lib/db/source-health.ts`:
- Around line 171-182: Update the health-check flow containing the
source_registry, ingestion_run, and source_discovery_work_item probes to execute
all three independent Supabase reads concurrently, using Promise.all with each
probe’s existing isolated error handling. Preserve the current fail-closed
behavior and queueReadable/source health assignments for each individual probe.
In `@lib/ingestion/http.ts`:
- Around line 1032-1058: Extract the repeated isWithinReach try/catch and
policyFailure mapping into a shared reachDenial(url, context) helper near the
existing policy utilities. Update followChain, followAssetChain, and guard to
call it and return the failure when non-null, preserving each caller’s existing
context message and behavior for reachable targets.
In `@lib/ingestion/orchestrator.ts`:
- Around line 199-214: The urlPolicy callback in the officialHttp creation flow
refreshes the full policy table for every request, redirect, and asset fetch.
Add a short, shared TTL-based cache for loadOfficialDestinationPolicies results
so hops within the same request chain reuse policies while revocations are still
observed within the bounded TTL; preserve evaluateOfficialDestinationPolicy and
the existing per-lead refresh behavior.
- Around line 216-225: Remove the redundant local alias and type assertion in
readOfficialStats, and call stats() directly on officialHttp with the existing
null-safe fallback unchanged.
- Around line 284-294: Update the catch handler around
loadOfficialDestinationPolicies so deferLead() failures cannot replace the
original “official destination policy unavailable” error. Preserve the
policy-loading error as the thrown diagnostic while attempting deferLead(),
including any defer failure only as secondary context if supported by the
existing error-handling conventions.
- Around line 284-306: Update the denied-destination handling in the
orchestration flow around evaluateOfficialDestinationPolicy: permanently denied
reasons such as policy_missing and policy_not_production_approved must
transition to a terminal/dead-letter state after a bounded attempt count instead
of always calling deferLead, while transient policy_unavailable results continue
retrying indefinitely. Reuse the existing attempt tracking, dead-letter
mechanism, and destinationDecision.reason, and preserve the current skip/count
behavior.
In `@package.json`:
- Around line 65-69: Update the brace-expansion entries in the package.json
overrides configuration to use a single appropriate 5.0.x range key instead of
separate exact-version keys for 5.0.6 and 5.0.7, while preserving the patched
5.0.8 resolution for all matching vulnerable versions.
In `@supabase/tests/database/source_discovery_work_claims.test.sql`:
- Around line 3-91: Extend the pgTAP plan and add coverage for the claim CAS
flow using the existing non-official work item: claim it with
claim_source_discovery_work, save the returned token, refresh the item through
enqueue_source_discovery_work, then assert complete_source_discovery_work with
the old token returns false. Include the corresponding setup and cleanup
assertions while preserving the existing privilege and official-intake checks.
🪄 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 Plus
Run ID: 3fab7561-98d8-4843-b54a-99234ecdfb90
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (62)
app/admin/sources/__tests__/page.test.tsxapp/admin/sources/page.tsxapp/api/admin/ingestion/destinations/__tests__/route.test.tsapp/api/admin/ingestion/destinations/route.tsapp/api/admin/ingestion/official-urls/__tests__/route.test.tsapp/api/admin/ingestion/official-urls/route.tsapp/api/cron/ingest/__tests__/route.test.tsapp/api/cron/ingest/route.tsapp/api/health/__tests__/route.test.tsapp/api/health/route.tscomponents/listing-card.tsxcomponents/official-destination-policy-console.tsxcomponents/official-url-intake-console.tsxcomponents/official-url-intake-status.tsxlib/__tests__/ingestion-gate.test.tslib/__tests__/ingestion-http.test.tslib/__tests__/ingestion-lifecycle.test.tslib/__tests__/ingestion-official-destination-policy.test.tslib/__tests__/ingestion-official-url-intake.test.tslib/__tests__/ingestion-orchestrator.test.tslib/__tests__/ingestion-sweeps-advantage.test.tslib/__tests__/ingestion-sweepstakes-today.test.tslib/__tests__/ingestion-work-queue.test.tslib/__tests__/listing-media-component.test.tsxlib/__tests__/listing-presentation-contract.test.tslib/__tests__/official-url-intake-console.test.tsxlib/__tests__/official-url-intake-status-component.test.tsxlib/db/__tests__/discovery-work-claims.test.tslib/db/__tests__/ingestion-provenance-observation.test.tslib/db/__tests__/ingestion-run-recovery.test.tslib/db/__tests__/listing-review-permissions.test.tslib/db/__tests__/official-destination-policy.test.tslib/db/__tests__/official-url-intake-status.test.tslib/db/__tests__/official-url-intake.test.tslib/db/__tests__/source-health.test.tslib/db/discovery-work.tslib/db/ingestion.tslib/db/listing-review.tslib/db/official-destination-policy.tslib/db/official-url-intake-status.tslib/db/official-url-intake.tslib/db/source-health.tslib/ingestion/adapters/freebie-guy.tslib/ingestion/adapters/sweeps-advantage.tslib/ingestion/adapters/sweepstakes-today.tslib/ingestion/fixtures/http.tslib/ingestion/gate.tslib/ingestion/http.tslib/ingestion/lifecycle.tslib/ingestion/official-destination-policy.tslib/ingestion/official-url-intake.tslib/ingestion/orchestrator.tslib/ingestion/source.tslib/ingestion/work-queue.tslib/official-destination-policy-event-schema.tslib/official-url-intake-schema.tslib/public-hostname.tspackage.jsonsupabase/migrations/20260729170000_official_destination_policy.sqlsupabase/migrations/20260729180221_harden_source_discovery_work_claims.sqlsupabase/migrations/20260729181206_listing_ingestion_observations.sqlsupabase/tests/database/source_discovery_work_claims.test.sql
|
Release verification checkpoint — exact SHA 486e579. Green evidence:
Independent review:
No production migration, provider/environment activation, cron enablement, or merge was performed. |
AutomatedEmpires
left a comment
AutomatedEmpires
left a comment
There was a problem hiding this comment.
Independent review — Fable 5 (verdict: approve for merge)
Reviewed as the independent reviewer required by AGENTS.md (builder: Codex). Six dimensions, evidence-based against the branch head 486e579:
- Bot-thread triage — all 8 actionable points from CodeRabbit + Codex verified fixed in the head, including the two threads left unresolved on GitHub (gate containment allowlist at
lib/ingestion/gate.ts:78-90; single-snapshot backlog RPC in20260729183000). - Migrations — all five are additive and uniformly hardened: RLS on every new table, service-role-only SECURITY DEFINER RPCs, revoked sequences, append-only enforced by trigger, production-approval completeness CHECK. The only removals (
defer_source_discovery_work(text,text)drop, service-role UPDATE revoke) close unversioned mutation paths that no live code uses while ingestion is dark. - Security — new admin APIs enforce Clerk auth + DB
is_admin/is_ownerserver-side and attribute the actor; URL normalization is HTTPS-only, credential/port-free, registrable-public-hostname-only (tldts); official_direct destination authority is default-deny, re-checked on every redirect hop and asset fetch, and fails closed (policy_unavailable) when the policy read errors. - Concurrency — claim-token CAS on complete/defer/dead-letter, bounded lease (60–3600s) with expiry recovery, 5-attempt terminal dead-letter with poison sweep, payload refresh invalidates in-flight claims. Verified in SQL, not just in TS.
- UI/product — card change is minimal (line-clamp 2→4, dense facts grid removed; Official Rules + date sheet retained); tokens only; design-guardrails green.
- Ops — no vercel.json/README/workflow changes (cron-table test contract intact); /api/health degrades gracefully if tables are missing;
next^15.5.21 andpostcss8.5.18 bumps supersede dependabot #97/#100.
Database: the five migrations were applied to production ahead of this merge (additive, dark — no live code path references them until this PR deploys). Ledger recorded with exact repo versions; count now 46/46; security advisors unchanged (3 pre-existing INFO items only).
Ingestion remains dark after merge: INGESTION_ENABLED unset, no source at production floor, no destination policies seeded. Activation stays founder-gated per ops/sweepza/SWEEPZA_LAUNCH_GATE_PACKET.md.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
supabase/migrations/20260729180221_harden_source_discovery_work_claims.sql (1)
41-48: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd the check constraints as
NOT VALID, then validate them.Both new check constraints trigger a full table scan under
ACCESS EXCLUSIVE, which blocks writes tosource_discovery_work_item. The constrained columns are new, so every existing row isNULLand already satisfies the checks. Add each constraint asNOT VALIDand runVALIDATE CONSTRAINTin a separate statement, which takes a weaker lock.♻️ Proposed change
add constraint source_discovery_work_last_failure_reason_bounded check ( last_failure_reason is null or ( nullif(btrim(last_failure_reason), '') is not null and char_length(last_failure_reason) <= 1000 ) - ); + ) not valid; + +alter table public.source_discovery_work_item + validate constraint source_discovery_work_last_failure_reason_bounded;Apply the same pattern to the other constraint added in this statement.
🤖 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 `@supabase/migrations/20260729180221_harden_source_discovery_work_claims.sql` around lines 41 - 48, Define both new check constraints on source_discovery_work_item with NOT VALID, including source_discovery_work_last_failure_reason_bounded and the other constraint added in the same migration. Add separate ALTER TABLE ... VALIDATE CONSTRAINT statements afterward for each constraint so validation occurs under the weaker lock.Source: Linters/SAST tools
supabase/tests/database/official_url_intake_backlog_status.test.sql (1)
23-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the count assertions independent of pre-existing rows.
Each assertion expects exactly
1::bigint. Any otherofficial_directrow insource_discovery_work_item, from seed data or another fixture, breaks these assertions even when the function is correct. Clear the queue rows inside the transaction before seeding. The surroundingrollbackkeeps the change local to the test.♻️ Proposed change
+delete from public.source_discovery_work_item + where source_id = 'official_direct'; + insert into public.source_discovery_work_item (🤖 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 `@supabase/tests/database/official_url_intake_backlog_status.test.sql` around lines 23 - 98, Clear existing source_discovery_work_item rows within the test transaction before inserting the pgtap-status fixtures, targeting the official_direct records used by get_official_url_intake_backlog_status. Keep the existing fixture data, assertions, and surrounding rollback unchanged.supabase/tests/database/source_discovery_work_claims.test.sql (1)
32-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExtend the anonymous-denial check to the other two claim mutations.
The suite proves that
anoncannot executedefer_source_discovery_work. It does not make the same assertion forcomplete_source_discovery_workordead_letter_source_discovery_work. Those two functions also close out claimed work, so a permissive grant on either would let an anonymous caller drop queued ingestion work.Add the two matching assertions and raise
plan()to 28.🛡️ Proposed additional assertions
select ok( not has_function_privilege( 'anon', 'public.defer_source_discovery_work(text,text,uuid,text)', 'EXECUTE' ), 'anonymous callers cannot mutate retry state' ); + +select ok( + not has_function_privilege( + 'anon', + 'public.complete_source_discovery_work(text,text,uuid)', + 'EXECUTE' + ), + 'anonymous callers cannot complete claimed work' +); + +select ok( + not has_function_privilege( + 'anon', + 'public.dead_letter_source_discovery_work(text,text,uuid,text)', + 'EXECUTE' + ), + 'anonymous callers cannot quarantine claimed work' +);🤖 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 `@supabase/tests/database/source_discovery_work_claims.test.sql` around lines 32 - 39, Extend the anonymous privilege checks in the source discovery work claims test to assert that anon cannot EXECUTE complete_source_discovery_work and dead_letter_source_discovery_work, using their existing text,text,uuid,text signatures and matching denial messaging. Update plan() from 26 to 28 to account for both new assertions.lib/ingestion/gate.ts (1)
70-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the registry-floor exception next to the descriptor contract.
OFFICIAL_CAPABILITY_REVIEWED_STATESacceptsreviewed,approved_for_fixtures, andapproved_for_manual_check. TheSourceDescriptor.complianceStatedocumentation inlib/ingestion/source.tsstates that the database record may sit at or below the registry value but never above it. Forofficial_direct, anapproved_for_productionrecord now sits above areviewedregistry floor. The behavior is intentional and the record-sideisProductionExecutablecheck still gates execution, but the two documents now disagree.Add the
official_directexception to thecomplianceStatefield documentation so a later reader does not treat the registry floor as a hard ceiling and revert this helper.🤖 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 `@lib/ingestion/gate.ts` around lines 70 - 90, Add documentation to the SourceDescriptor.complianceState field in source.ts describing the intentional official_direct exception: its registry floor may be reviewed while the database record can be approved_for_production, with isProductionExecutable still gating execution. Keep the existing general registry constraint documented for other capabilities.lib/ingestion/orchestrator.ts (1)
350-356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the root-cause message when the queue mutation fails on these throw paths.
Both paths call
deferLeadand then throw the real cause.deferLeadcan reject, becauseworkQueue.deferthrows when the compare-and-set claim is lost. If it rejects, its error replaces the lease denial or the policy-unavailable message, andfinishIngestionRunrecords "claim lost …" instead of the actual outage.The policy-load path at lines 312-331 already handles this. It wraps the defer in
try/catchand appendsqueue defer failed: …to the original message. Apply the same pattern here so the two control-plane failures stay distinguishable in the run notes.Also applies to: 385-392
🤖 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 `@lib/ingestion/orchestrator.ts` around lines 350 - 356, Update the failure paths around ensureOfficialClient and the corresponding policy-unavailable branch to wrap deferLead in try/catch, preserving the original lease-denial or policy-unavailable error as the thrown cause while appending any queue-defer failure details. Match the established handling used by the policy-load path and keep the existing deferLead reason values unchanged.lib/ingestion/official-url-intake.ts (1)
44-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName both mismatch causes in the dead-letter diagnostic.
The guard fails when
refresh.requestItemKeyorrefresh.generationdisagrees withitem.key. The stored reason names only the generation. An operator who reads the dead-letter row cannot tell which field is wrong.Report the expected and actual key instead. Update the substring assertion in
lib/__tests__/ingestion-official-url-intake.test.tsif you apply this.♻️ Proposed wording change
+ const expectedRefreshKey = parsed.data.refresh + ? `${parsed.data.refresh.requestItemKey}:refresh:${parsed.data.refresh.generation}` + : null; if ( parsed.data.refresh && - item.key !== - `${parsed.data.refresh.requestItemKey}:refresh:${parsed.data.refresh.generation}` + item.key !== expectedRefreshKey ) { await queue.deadLetter( item.key, item.claimToken, - "invalid_official_url_intake_payload: refresh generation does not match the claimed queue key", + `invalid_official_url_intake_payload: refresh metadata expects queue key "${expectedRefreshKey}" but the claimed key is "${item.key}"`.slice( + 0, + 1000, + ), ); continue; }🤖 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 `@lib/ingestion/official-url-intake.ts` around lines 44 - 55, The refresh-key mismatch diagnostic in the official URL intake guard should identify both possible causes by reporting the expected composed key and the actual item.key. Update the deadLetter reason in the refresh validation block, and adjust the corresponding substring assertion in the official URL intake test to match the new diagnostic.
🤖 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 `@components/official-destination-policy-console.tsx`:
- Line 94: Validate the operator-supplied expiry date before constructing the
payload in the submission flow around reviewExpiresAt. If an expiry value is
present but toLocalEndOfDayIso returns null, block submission and report the
invalid date; only send null when no date was supplied, and preserve the
existing ISO value for valid dates.
In `@components/official-url-intake-console.tsx`:
- Around line 264-277: Update the success-message construction around
formatOfficialUrlRevalidationSuccess and formatOfficialUrlIntakeSuccess so
missing revalidated or accepted counts produce an unconfirmed outcome rather
than falling back to prepared.entries.length. Preserve the confirmed
server-provided counts and existing pending/replayed handling when those numeric
fields are present.
In `@lib/__tests__/official-destination-policy-console.test.tsx`:
- Around line 13-29: The test’s runtime timezone mutation is unreliable in
Vitest workers. Update toLocalEndOfDayIso and its callers to accept an explicit
timezone, or configure America/Los_Angeles before workers start via Vitest
setup/config; remove the in-test TZ mutation while preserving the expected
end-of-day and invalid-date assertions.
In `@lib/ingestion/orchestrator.ts`:
- Around line 513-517: Wrap the enqueueDueOfficialUrlRevalidations call in the
officialDecision.allowed branch with failure capture so errors do not escape
runIngestion or prevent source discovery. Store the caught error as
revalidationFailure, then include that value in the notes array construction
alongside the existing run notes, preserving normal execution when enqueueing
succeeds.
---
Nitpick comments:
In `@lib/ingestion/gate.ts`:
- Around line 70-90: Add documentation to the SourceDescriptor.complianceState
field in source.ts describing the intentional official_direct exception: its
registry floor may be reviewed while the database record can be
approved_for_production, with isProductionExecutable still gating execution.
Keep the existing general registry constraint documented for other capabilities.
In `@lib/ingestion/official-url-intake.ts`:
- Around line 44-55: The refresh-key mismatch diagnostic in the official URL
intake guard should identify both possible causes by reporting the expected
composed key and the actual item.key. Update the deadLetter reason in the
refresh validation block, and adjust the corresponding substring assertion in
the official URL intake test to match the new diagnostic.
In `@lib/ingestion/orchestrator.ts`:
- Around line 350-356: Update the failure paths around ensureOfficialClient and
the corresponding policy-unavailable branch to wrap deferLead in try/catch,
preserving the original lease-denial or policy-unavailable error as the thrown
cause while appending any queue-defer failure details. Match the established
handling used by the policy-load path and keep the existing deferLead reason
values unchanged.
In `@supabase/migrations/20260729180221_harden_source_discovery_work_claims.sql`:
- Around line 41-48: Define both new check constraints on
source_discovery_work_item with NOT VALID, including
source_discovery_work_last_failure_reason_bounded and the other constraint added
in the same migration. Add separate ALTER TABLE ... VALIDATE CONSTRAINT
statements afterward for each constraint so validation occurs under the weaker
lock.
In `@supabase/tests/database/official_url_intake_backlog_status.test.sql`:
- Around line 23-98: Clear existing source_discovery_work_item rows within the
test transaction before inserting the pgtap-status fixtures, targeting the
official_direct records used by get_official_url_intake_backlog_status. Keep the
existing fixture data, assertions, and surrounding rollback unchanged.
In `@supabase/tests/database/source_discovery_work_claims.test.sql`:
- Around line 32-39: Extend the anonymous privilege checks in the source
discovery work claims test to assert that anon cannot EXECUTE
complete_source_discovery_work and dead_letter_source_discovery_work, using
their existing text,text,uuid,text signatures and matching denial messaging.
Update plan() from 26 to 28 to account for both new assertions.
🪄 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 Plus
Run ID: 6d6ba16f-ce01-4a54-9992-90faa4cf7f96
📒 Files selected for processing (39)
app/api/admin/ingestion/destinations/__tests__/route.test.tsapp/api/admin/ingestion/destinations/route.tsapp/api/admin/ingestion/official-urls/__tests__/route.test.tsapp/api/admin/ingestion/official-urls/route.tscomponents/official-destination-policy-console.tsxcomponents/official-url-intake-console.tsxlib/__tests__/ingestion-fingerprint.test.tslib/__tests__/ingestion-gate.test.tslib/__tests__/ingestion-official-url-intake.test.tslib/__tests__/ingestion-orchestrator.test.tslib/__tests__/ingestion-work-queue.test.tslib/__tests__/official-destination-policy-console.test.tsxlib/__tests__/official-url-intake-console.test.tsxlib/__tests__/official-url-intake-schema.test.tslib/db/__tests__/discovery-work-claims.test.tslib/db/__tests__/ingestion-provenance-observation.test.tslib/db/__tests__/official-url-intake-status.test.tslib/db/__tests__/official-url-intake.test.tslib/db/discovery-work.tslib/db/ingestion.tslib/db/official-url-intake-status.tslib/db/official-url-intake.tslib/ingestion/adapters/freebie-guy.tslib/ingestion/adapters/sweeps-advantage.tslib/ingestion/adapters/sweepstakes-today.tslib/ingestion/fingerprint.tslib/ingestion/gate.tslib/ingestion/official-url-intake.tslib/ingestion/orchestrator.tslib/ingestion/source.tslib/ingestion/work-queue.tslib/official-url-intake-schema.tslib/official-url-normalization.tssupabase/migrations/20260729180221_harden_source_discovery_work_claims.sqlsupabase/migrations/20260729183000_official_url_intake_backlog_status.sqlsupabase/migrations/20260729190132_official_url_intake_revalidation.sqlsupabase/tests/database/official_url_intake_backlog_status.test.sqlsupabase/tests/database/official_url_intake_revalidation.test.sqlsupabase/tests/database/source_discovery_work_claims.test.sql
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/ingestion/adapters/sweeps-advantage.ts
- lib/ingestion/adapters/sweepstakes-today.ts
- lib/ingestion/adapters/freebie-guy.ts
- lib/ingestion/source.ts
- lib/tests/official-url-intake-console.test.tsx
- lib/db/ingestion.ts
- lib/db/discovery-work.ts
- Guard the official revalidation enqueue so one maintenance failure cannot abort the whole ingestion invocation; the failure lands in run notes when intake work exists and as an explicit error summary (Sentry via the cron route) when it does not. - Block destination-policy submission when the operator-supplied expiry date is unparsable instead of silently requesting a no-expiry decision. - Report an unconfirmed outcome when the intake API responds 2xx without counts instead of claiming the full batch was queued. The fourth re-review point (in-test TZ mutation) is deliberately not changed: modern Node intercepts process.env.TZ assignment and the test is deterministic in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Delivers the mobile listing-card refinement and a governed, twice-daily official-source ingestion pipeline. This is an operator-directed release slice with no linked issue.
Lane: C / H / I
Acceptance criteria
Production impact
Canon alignment
listingobject; no parallel listing model.Security & quality
Release boundary
This PR does not itself authorize production launch, source crawling, AI extraction, email, payments, or media-provider activation. Merge remains subject to independent review, green hosted checks, and the repository launch gate.
Summary by CodeRabbit