fix(audit-sprint-7a): db:test:verify cleanup leak + attestation landing surface - #175
Merged
Merged
Conversation
…ng surface
Two unrelated follow-ups bundled because each is too small for its own PR.
CI probe leak in check-db-test-verify.mjs
- cleanupProbe() called delete().eq(...) without checking the
returned error — every failed delete left a "FormaOS DB Verify
<ts>" row in organizations. Sprint 5b counted 95 such orphans.
- Wrap every cleanup delete in deleteAndLog() that surfaces the
error message to console.warn. Future failures get diagnosed
instead of accumulating silently.
- Add sweepOldProbes(admin) that runs at the START of every
invocation: removes any 'FormaOS DB Verify%' org older than 1
hour. Bounded by the LIKE pattern + age cutoff so a parallel
invocation isn't disturbed and real orgs are never touched.
First invocation cleans up the historical 95-row backlog;
subsequent invocations stay at ~0 unless a future bug fires
and the new error logging surfaces it.
Compliance landing → attestations surface
- Sprint 6c shipped /app/compliance/attestations but it had no
entry point from the rest of the UI. Per-industry sidebar navs
(lib/navigation/industry-sidebar.ts) are 9 separate arrays —
not worth churning for a single sub-link.
- Add a simple linked card on the /app/compliance landing page
pointing at the attestations workflow. The landing is the
natural compliance hub; users browsing the area now see the
affordance.
Validation
- tsc -p tsconfig.typecheck.json: clean
- eslint: 0 errors, 18 warnings (baseline)
- jest: not run (no test-relevant changes)
- scripts/check-db-test-verify.mjs not exercised end-to-end here
(needs Supabase env); shape changes verified via dry read
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a compliance page navigation link to manual attestations and enhances database test cleanup with improved observability. The compliance page now displays a styled call-to-action link, while the database verification script adds delete logging, probe cleanup enhancements, and a stale probe pre-check sweep. ChangesCompliance Attestations Navigation
Database Probe Cleanup Observability
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: e59fd913f1
ℹ️ 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".
| * sub-link; the landing page is the natural hub. | ||
| */} | ||
| <Link | ||
| href="/app/compliance/attestations" |
There was a problem hiding this comment.
The new card links to /app/compliance/attestations, but there is no matching App Router page in this repo (under app/app/compliance only page.tsx, soc2, cross-map, and frameworks routes exist), so clicking this CTA currently lands users on a 404. Because this is now the primary UI entry point for manual attestations, the broken href makes the surfaced workflow unusable.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/check-db-test-verify.mjs (1)
370-377: ⚡ Quick winPaginate stale-probe sweep beyond the first 500 rows.
Line 377 caps cleanup candidates to 500 in a single pass. If backlog exceeds that, stale probe orgs remain after a run. Iterate in batches until fewer than batch size are returned.
♻️ Proposed fix
async function sweepOldProbes(admin) { const cutoff = new Date(Date.now() - 60 * 60 * 1000).toISOString(); - const { data: orphans, error: listError } = await admin - .from('organizations') - .select('id, name, created_at') - .like('name', 'FormaOS DB Verify%') - .lt('created_at', cutoff) - .limit(500); - - if (listError) { - console.warn(`[sweepOldProbes] list failed: ${listError.message}`); - return; - } - - const rows = orphans ?? []; - if (rows.length === 0) return; - - console.log(`[sweepOldProbes] removing ${rows.length} stale probe org(s)`); - const ids = rows.map((r) => r.id); - - // Order matters — FK-respecting teardown. - await deleteAndLog( - admin, - 'org_form_submissions', - admin.from('org_form_submissions').delete().in('org_id', ids), - ); - await deleteAndLog( - admin, - 'org_forms', - admin.from('org_forms').delete().in('org_id', ids), - ); - await deleteAndLog( - admin, - 'org_members', - admin.from('org_members').delete().in('organization_id', ids), - ); - await deleteAndLog( - admin, - 'organizations', - admin.from('organizations').delete().in('id', ids), - ); - await deleteAndLog( - admin, - 'orgs', - admin.from('orgs').delete().in('id', ids), - ); + const batchSize = 500; + let total = 0; + + while (true) { + const { data: orphans, error: listError } = await admin + .from('organizations') + .select('id, name, created_at') + .like('name', 'FormaOS DB Verify%') + .lt('created_at', cutoff) + .limit(batchSize); + + if (listError) { + console.warn(`[sweepOldProbes] list failed: ${listError.message}`); + return; + } + + const rows = orphans ?? []; + if (rows.length === 0) break; + + total += rows.length; + const ids = rows.map((r) => r.id); + + // Order matters — FK-respecting teardown. + await deleteAndLog( + admin, + 'org_form_submissions', + admin.from('org_form_submissions').delete().in('org_id', ids), + ); + await deleteAndLog( + admin, + 'org_forms', + admin.from('org_forms').delete().in('org_id', ids), + ); + await deleteAndLog( + admin, + 'org_members', + admin.from('org_members').delete().in('organization_id', ids), + ); + await deleteAndLog( + admin, + 'organizations', + admin.from('organizations').delete().in('id', ids), + ); + await deleteAndLog( + admin, + 'orgs', + admin.from('orgs').delete().in('id', ids), + ); + + if (rows.length < batchSize) break; + } + + if (total > 0) { + console.log(`[sweepOldProbes] removing ${total} stale probe org(s)`); + } }🤖 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 `@scripts/check-db-test-verify.mjs` around lines 370 - 377, The sweepOldProbes function currently limits candidates to 500 once, leaving leftovers; change it to paginate and repeat deletes until a fetched batch is smaller than the batch size: extract the current cutoff and batchSize (e.g., 500) and wrap the query+deletion logic in a loop that calls admin.from('organizations').select(...).like(...).lt('created_at', cutoff).limit(batchSize) each iteration, processes/deletes the returned orphans (the variable orphans) and breaks when orphans.length < batchSize or when no rows are returned; ensure you still handle listError and deletion errors inside the loop and avoid infinite loops by preserving the same cutoff and reducing results via deletion each pass.
🤖 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 `@scripts/check-db-test-verify.mjs`:
- Around line 370-377: The sweepOldProbes function currently limits candidates
to 500 once, leaving leftovers; change it to paginate and repeat deletes until a
fetched batch is smaller than the batch size: extract the current cutoff and
batchSize (e.g., 500) and wrap the query+deletion logic in a loop that calls
admin.from('organizations').select(...).like(...).lt('created_at',
cutoff).limit(batchSize) each iteration, processes/deletes the returned orphans
(the variable orphans) and breaks when orphans.length < batchSize or when no
rows are returned; ensure you still handle listError and deletion errors inside
the loop and avoid infinite loops by preserving the same cutoff and reducing
results via deletion each pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c89aa8f-5e26-45f5-b816-7835cd99e939
📒 Files selected for processing (2)
app/app/compliance/page.tsxscripts/check-db-test-verify.mjs
There was a problem hiding this comment.
Pull request overview
This PR bundles two follow-ups: (1) harden db:test:verify cleanup so probe orgs don’t leak into production, and (2) add a discoverable UI entry point to the compliance attestation workflow from the compliance landing page.
Changes:
- Add
deleteAndLog()and apply it to probe cleanup deletions to surface Supabase delete errors in logs. - Add
sweepOldProbes(admin)at the start ofdb:test:verifyto remove stale “FormaOS DB Verify%” orgs older than 1 hour. - Add a linked “Manual attestations” card on
/app/compliancepointing to/app/compliance/attestations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/check-db-test-verify.mjs | Adds error-logging wrapper for cleanup deletes and a startup sweep for stale probe organizations. |
| app/app/compliance/page.tsx | Adds a card link on the compliance landing page to the manual attestation workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+308
to
+316
| async function deleteAndLog(admin, table, query) { | ||
| const { error } = await query; | ||
| if (error) { | ||
| // Audit Sprint 7a (2026-05-24): previously cleanup errors were | ||
| // ignored. Live DB had 95 leftover "FormaOS DB Verify" orgs | ||
| // because a failing delete left rows behind silently. Log so the | ||
| // operator can investigate without grepping prod for orphans. | ||
| console.warn(`[cleanupProbe] ${table} delete failed: ${error.message}`); | ||
| } |
Comment on lines
+308
to
+310
| async function deleteAndLog(admin, table, query) { | ||
| const { error } = await query; | ||
| if (error) { |
Comment on lines
+372
to
+415
| const { data: orphans, error: listError } = await admin | ||
| .from('organizations') | ||
| .select('id, name, created_at') | ||
| .like('name', 'FormaOS DB Verify%') | ||
| .lt('created_at', cutoff) | ||
| .limit(500); | ||
|
|
||
| if (listError) { | ||
| console.warn(`[sweepOldProbes] list failed: ${listError.message}`); | ||
| return; | ||
| } | ||
|
|
||
| const rows = orphans ?? []; | ||
| if (rows.length === 0) return; | ||
|
|
||
| console.log(`[sweepOldProbes] removing ${rows.length} stale probe org(s)`); | ||
| const ids = rows.map((r) => r.id); | ||
|
|
||
| // Order matters — FK-respecting teardown. | ||
| await deleteAndLog( | ||
| admin, | ||
| 'org_form_submissions', | ||
| admin.from('org_form_submissions').delete().in('org_id', ids), | ||
| ); | ||
| await deleteAndLog( | ||
| admin, | ||
| 'org_forms', | ||
| admin.from('org_forms').delete().in('org_id', ids), | ||
| ); | ||
| await deleteAndLog( | ||
| admin, | ||
| 'org_members', | ||
| admin.from('org_members').delete().in('organization_id', ids), | ||
| ); | ||
| await deleteAndLog( | ||
| admin, | ||
| 'organizations', | ||
| admin.from('organizations').delete().in('id', ids), | ||
| ); | ||
| await deleteAndLog( | ||
| admin, | ||
| 'orgs', | ||
| admin.from('orgs').delete().in('id', ids), | ||
| ); |
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
This was referenced May 23, 2026
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
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
Two small follow-ups bundled together.
1. db:test:verify probe-org cleanup leak
Symptom: Sprint 5b discovery — 95 orphaned
FormaOS DB Verify <ts>rows in productionorganizations. Cleanup was silently failing.Root cause: scripts/check-db-test-verify.mjs:308
cleanupProbe()called.delete().eq(...)without checking the returnederror— failed deletes left rows behind with no log line.Fix:
deleteAndLog()that surfaceserror.messagetoconsole.warnsweepOldProbes(admin)at the START of every invocation: removes anyFormaOS DB Verify%org older than 1 hour. Bounded by the LIKE pattern + age cutoff so parallel invocations aren't disturbed and real orgs are never touched.2. Compliance attestation landing surface
Symptom: PR #173 shipped
/app/compliance/attestationsbut no UI entry point. Discoverable only by URL.Why not the sidebar: Per-industry sidebar navs live in 9 separate arrays in lib/navigation/industry-sidebar.ts. Adding a sub-link to all 9 is churn for one route.
Fix: Add a simple linked card on the /app/compliance landing above the obligations table. The landing is the natural compliance hub.
Validation
npm run type-checkcleannpm run lint0 errors, 18 warnings (baseline)[sweepOldProbes] removing 95 stale probe org(s)once, then 0 on subsequent runs/app/complianceshows the new attestations card above the obligations table🤖 Generated with Claude Code
Summary by CodeRabbit