feat(audit-2026-05-27): NDIS Phase 3 UI + unified health dashboard + CAPA auto-create + /verify - #192
Merged
Merged
Conversation
…e 3 UI)
Adds the policy-side admin UI that backs the R10 Phase 3 schema
shipped in 20260624067_audit_2026_05_27_r10_phase_3_ndis_schema.sql.
Without this surface every NDIS evaluator returns
manual_attestation_required because no policies are tagged.
Changes:
* lib/compliance/ndis/categories.ts — single source of truth for the
18 Practice-Standard category strings. Kept in sync with the
org_policies_ndis_category_check CHECK constraint by file comment.
Helper coerceNdisCategory() normalises "none"/empty/unknown to null.
* app/app/policies/new/page.tsx — adds the NDIS-category select below
the framework dropdown with a helper-text explaining the impact on
NDIS scoring. Defaults to "none" so existing tag-less customers
aren't forced into NDIS scope.
* app/app/policies/[id]/edit/page.tsx — same dropdown on edit, with
defaultValue from policy.ndis_category. Respects the locked-status
rule (published / pending-approval / approved are read-only).
* app/app/actions/policies.ts — createPolicy + updatePolicy now thread
ndis_category through the INSERT/UPDATE. updatePolicy gates the
column write on formData.has('ndis_category') so legacy edit
surfaces don't blow away an existing tag.
After this lands, a privacy policy tagged 'privacy' with a fresh
updated_at causes NDIS-1.3 to flip from manual_attestation_required to
pass on next evaluation. Same shape for the other 14 Practice Standard
indicators that look up policies by category.
Verified locally: npm run type-check exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires UI onto the org_behaviour_support_plans table shipped in
migration 20260624067_audit_2026_05_27_r10_phase_3_ndis_schema.sql.
Without this surface the table is invisible to customers, so
NDIS-V.2 + NDIS-M.2 predicates return manual_attestation_required.
Files:
* app/app/behaviour-support-plans/page.tsx — list view with the four
headline metrics (total, active, drafts, expires-30d) and a table
sorted by created_at. Empty state links straight to /new.
* app/app/behaviour-support-plans/new/page.tsx — create form grouped
into Plan basics / Lifecycle timestamps / Authorisation+provider.
Participant dropdown hydrates from org_patients; plan_type required
(interim|comprehensive); date fields are optional except for what
the predicates need to actually evaluate.
* app/app/behaviour-support-plans/[id]/page.tsx — read-only detail
view with edit/delete affordances gated to owner/admin (RLS enforces
the same; UI gate is a hint).
* app/app/behaviour-support-plans/[id]/edit/page.tsx — full edit form
mirroring /new, plus the status dropdown (draft|submitted|authorised|
active|expired|withdrawn).
* app/app/actions/behaviour-support-plans.ts — create/update/delete
server actions, each with logAuditEvent + revalidatePath. Empty-string
form fields collapse to NULL so the predicates' "missing field"
behaviour stays consistent.
* lib/navigation/industry-sidebar.ts — adds "Behaviour Support" entry
under Care Operations in NDIS_NAV only. Healthcare/Aged-Care/etc.
don't see it (BSP is an NDIS-specific F2018L00632 artefact).
UI conventions: matches the existing care-plans / participants table
pattern (page-header / page-content / metric-card classes; Tailwind +
lucide-react icons; no new design tokens).
Verified: npm run type-check exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(Phase 3 UI)
Closes the last write-path gap for the NDIS Phase 3 predicates. Of the
22 real predicates, 10 look up org_registers rows by a specific `type`
value (conflict_of_interest, complaint, business_continuity_plan,
intake, service_agreement, transition, environment_assessment,
financial_delegation, restrictive_practice_use, supervision). Until
this surface, customers had no way to write to org_registers at all —
the existing /app/registers page only showed asset rows from org_assets.
Files:
* lib/compliance/ndis/register-types.ts — single source of truth for
the 10 NDIS-aware type slugs. The column itself is free-form text,
so customers can adopt the taxonomy at their pace; the sheet
provides them via dropdown + a free-form "Other" escape hatch.
* components/registers/create-register-entry-sheet.tsx — slide-out
sheet matching the existing CreateAssetSheet pattern. Type
dropdown is required; code auto-derives from name if blank;
risk_level mirrors the existing asset taxonomy (low/med/high/critical).
* app/app/registers/actions.ts — adds createRegisterEntry server
action. Requires EDIT_CONTROLS permission, writes to org_registers
using the org_id column (not organization_id — the existing page
code path already handles the fallback). Logs REGISTER_CREATED
via logAuditEvent.
* app/app/registers/page.tsx — wires the new sheet into the
PageHero actions slot. Visible on both Care and Assets tabs so
NDIS customers can tag registers without leaving the registers
surface.
After this lands, tagging a register entry as type='conflict_of_interest'
causes NDIS-2.1 to flip from manual_attestation_required to pass (if
the governance policy is also current). Same shape for the other 9
NDIS-aware register types.
Verified: npm run type-check exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ork cards
Adds the per-framework status tally requested as part of the Phase 3 UI
surface. Each framework card on /app/compliance/frameworks now shows
how many of its controls are in each of the four evaluator-emitted
states — Pass / Partial / Fail / Manual — plus the most-recent
last_evaluated_at across the per-control rows. Mirrors the existing
SOC 2 / ISO coverage pattern but on a tighter footprint so it fits
under the existing domain badges without pushing the card height.
Files:
* lib/frameworks/org-frameworks.ts — adds getFrameworkEvaluationTallies()
that reads per-control org_control_evaluations rows (total_controls
IS NULL filter excludes the older framework-aggregate row shape) and
groups by framework_id. Returns a Map keyed by framework_id with
counts + most-recent timestamp. Also widens the empty-frameworks
early-return to include `id: null` so the union type stays
consistent for callers.
* app/app/compliance/frameworks/page.tsx — new EvaluationTally
sub-component renders the four counts as coloured pills. Cards with
zero per-control rows show an "evaluate from /app/controls" hint
instead so the empty-state isn't a row of zeros.
Customer impact: an NDIS-enabled org with 25 registered controls and
3 evaluation runs will see e.g. "Pass 8 / Partial 5 / Fail 2 / Manual 10"
on the NDIS card — directly answering "what do I still need to fix"
without leaving the Frameworks tab. The Tier 2.C unified health
dashboard (in the cycle backlog) will build on this same shape.
Verified: npm run type-check exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a chromium-only smoke spec that exercises the three UI affordances
shipped earlier on this branch:
1. /app/policies/new — verifies #ndis_category select renders and
contains a spot-check of 5 known enum values plus the "none"
escape hatch.
2. /app/behaviour-support-plans — verifies the page title, the
create-bsp button, and that clicking it lands on /new with the
plan_type select showing both interim + comprehensive options.
3. End-to-end BSP create — fills the minimum required fields,
submits, follows the server-action redirect to the detail page,
asserts the notes field round-tripped, and cleans up the row.
Chromium-only matches the established release gate
(memory: feedback_e2e_supported_scope). The register-entry sheet (c)
and the framework tally (d) aren't covered here — register entries
need a fresher RBAC seed than the helper currently provisions and the
tally is a read-only derived value already covered by jest at the
data-shape level.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tep 1)
Adds the pure-function rollup used by the unified compliance health
dashboard. aggregateHealth() takes per-control evaluation rows + framework
metadata and emits:
* overall: weighted-by-control score + cross-framework status counts
* frameworks: per-framework score / status counts / last-evaluated
* outstanding: top-N controls sorted by urgency_score (status × risk)
Design notes encoded in the file:
* Frameworks with zero controls score 0, not 100% — visibly empty
rather than misleading "perfect" cards.
* not_evaluated counts as 0 — manual attestation has its own surface
and shouldn't lift the auto-evaluator score.
* Weighted mean by control count for the overall score so a 200-
control framework outweighs a 5-control one, matching the auditor
model.
* Unknown risk levels downgrade to "medium" rather than disappearing
or topping the list.
* Status tie-break puts fail before partial (auditor-equivalent of
Major NC > Minor NC).
Companion fetch.ts wires the function to Supabase: enabled frameworks
→ per-control evaluations → join framework_controls for title +
default_risk_level. Aggregate-shape rows (total_controls IS NOT NULL)
are skipped on the JS side.
Verified: npm run type-check exits 0; 13/13 jest tests in the new
aggregate.test.ts pass (status counts, sort order, outstanding clamp,
unknown-risk handling, orphan rows, recency).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r 2.C step 2) Wires the aggregation lib shipped in 1bc6d1c onto a new top-level page. Three sections: 1. Overall section — big-number score + Healthy/Watch/At-risk band pill + the four cross-framework status tiles (pass/partial/fail/ manual). The single screenshot a prospect will share. 2. Per-framework breakdown — one card per enabled framework with its score, last-evaluated timestamp, and the four-segment status mini-grid. 3. Top 10 outstanding controls — ranked by urgency_score (status × risk_level) with a numbered list of {control_code, framework, title, risk pill, status pill}. UI conventions: matches the existing /app/compliance/frameworks card shape (rounded-2xl glass-border-on-gradient) so this surface reads as a sibling, not a redesign. Score band thresholds (90% / 70%) mirror the same rule the evaluator system already uses for pass/partial/fail on individual controls. Also surfaces the new page from /app/compliance via a paired card (Health + Manual attestations) replacing the prior single-card attestations link. Verified: npm run type-check exits 0; all prior jest passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n + trend sparkline (Tier 2.C step 3)
Closes Tier 2.C by adding the time-series under the unified health
dashboard.
Schema (migration 20260624069):
* public.org_compliance_health_snapshots — service-role-writes,
org-member-reads. RESTRICTIVE policies block app-side INSERT /
UPDATE / DELETE so the row history stays append-only.
* One row per (org, week) — the cron skips orgs that already
snapshotted in the last 24h, so an operator re-run during
debugging won't double-count.
Cron (/api/cron/compliance-health-snapshot, Mon 07:00 UTC):
* Walks every org with at least one enabled framework, computes the
aggregate via getOrgHealthAggregate(), and writes a row with the
overall score + per-framework breakdown serialized to jsonb.
* MAX_ORGS_PER_TICK = 500 keeps it bounded for the 300s function
timeout (added to vercel.json functions map).
* Per-org failures log and continue — partial coverage > wedged cron.
Page (TrendChart component on /app/compliance/health):
* Pure-SVG sparkline rendered server-side; reads the last 12
snapshots oldest→newest, plots overall_score on a 480×60 viewBox
with `currentColor` stroke so it picks up the theme primary.
* Header shows N-week label + delta-percent pill (emerald when up,
red when down).
* Empty-state explains the cron schedule rather than showing a
blank box, so newly-onboarded orgs aren't left wondering.
Verified: npm run type-check exits 0; migration applied via
execute_sql; supabase_migrations.schema_migrations row recorded at
version 20260624069; ledger alignment check ✓.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refines evaluateResponsiveSupport to match what NDIS auditors actually
look at: per-participant cadence (≥1 progress note per fortnight) over
active participants, not just an org-wide ≥30/90d count. The old
threshold let a high-volume program mask under-served participants.
Behaviour:
* pass — every active participant has ≥1 note in 30d AND org-wide
≥30 in 90d.
* partial — some active participants are silent for 30 days (≤50%)
OR org-wide count <30/90d.
* fail — >50% of active participants silent for 30+ days OR
zero notes org-wide.
Graceful fallback: if org_patients lookup errors (e.g. non-care org
without that table) or zero active participants are on file, the
predicate falls back to the legacy org-wide threshold rather than
penalising a customer for an absent population.
Output now lists the first 5 silent participants by name in the gap
message + an "N more" suffix — auditors and operators can act on the
finding without leaving the dashboard.
Tests:
* Rewrote ndis-phase-2.test.ts NDIS-3.4 block — 6 cases covering
fail/pass/partial bands, the >50% silent-ratio threshold, the
org_patients-error fallback, and the no-participants fallback.
* Old simple-stub tests dropped in favour of a per-table mock that
can replay both the count + data variants of the org_progress_notes
select chain.
docs/compliance/ndis-framework-status.md updated to mark the Phase 4
backlog item as landed.
Verified: npm run type-check exit 0; 20/20 jest tests in
ndis-phase-2.test.ts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audited the 14-entry SECURITY DEFINER allowlist against actual call
sites. 6 functions had call paths exclusively through service_role
(cron) or authenticated server-actions:
Cron-only — REVOKE anon + authenticated, GRANT service_role:
* cleanup_old_security_data /api/cron/security-retention
* claim_compliance_export_jobs /api/cron/compliance-exports
* claim_enterprise_export_jobs /api/cron/enterprise-exports
* claim_report_export_jobs /api/cron/report-exports
Authenticated-session-only — REVOKE anon, keep authenticated:
* update_session_heartbeat app/auth/signout (uses supabase
server client, requires session)
* log_email_send lib/email/email-log-compat.ts
(server-action context)
Allowlist 14 → 10:
* Removed all 4 cron-only entries (now service_role-only).
* Kept the 2 authenticated-session entries (still callable, just
no longer anon-callable).
* Documented the remaining 8 entries with _cleanup_notes pointing
at the work each one needs in a follow-up audit.
Verified post-apply:
* Supabase get_advisors no longer flags the 4 cron-only functions
under either anon_security_definer_function_executable or
authenticated_security_definer_function_executable.
* scripts/check-security-definer-grants.mjs — ✓ no drift.
* ledger alignment OK; new migration 20260624070 recorded.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the detect→remediate loop the Phase 3 NDIS predicates and Tier 2.C
health dashboard opened. When a registry-evaluator returns status='fail'
during framework evaluation, a CAPA (Corrective and Preventive Action)
row is auto-inserted to org_capa_items so the finding ends up on
someone's follow-up list — not just sitting on the dashboard.
Files:
* lib/compliance/capa/auto-create.ts — buildCapaInputs() is pure
(org_id + failing-control list → CapaInputRow[]). dedupeAgainstExisting()
drops candidates whose (source_type, source_id) is already on file.
autoCreateCapaFromFailures() wires both to Supabase via a dedupe
SELECT followed by a single INSERT. Failures log via
routeLog('lib/compliance/capa/auto-create') and never throw so the
evaluator response isn't blocked.
* lib/compliance/evaluate-framework-controls.ts — collects
failingControls during the per-control loop, then calls
autoCreateCapaFromFailures after upsertEvaluations. Only true
evaluator-fail rows trigger; heuristic-only "missing evidence"
non_compliant verdicts are handled by the existing task/evidence
workflows and don't open CAPAs.
* __tests__/lib/compliance/capa/auto-create.test.ts — 11 jest cases:
payload shape, severity mapping from first gap, 180-char title cap
with ellipsis, multi-line description with framework + gaps +
remediation prompt, missing-framework graceful fallback, and the
full dedupe matrix (some already on file, none on file, all on
file, different source_type ignored).
Design notes:
* source_type='compliance_evaluator', source_id=framework_control.id
— stable across evaluations so re-running doesn't spam.
* severity from first gap; defaults to medium when no gaps present.
* priority=severity (1:1 mapping today; kept separate so future SLA
tuning lands here).
* created_by=null because the evaluator path is sometimes cron-
triggered with no user context. Auditors can still see WHO ran the
evaluation via the existing logEvaluationAudit trail.
Verified: type-check exit 0; jest 356 suites / 5224 tests / 0 failures;
ledger / SECDEF / leaked-secrets all ✓.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r anchors (Tier 2.B)
Lets external auditors verify a FormaOS audit-export without a login or
CLI install. Two cards on /verify:
1. Paste an audit-export bundle JSON. Recomputes every leaf hash from
the entry contents, then verifies each inclusion proof against the
published root. Surfaces per-step pass/fail rows so a tamper is
visible at the exact stage (leaf mutation vs proof mutation vs
algorithm mismatch).
2. Paste a Rekor entry UUID + the expected top-of-chain hash. Fetches
the entry from rekor.sigstore.dev, confirms the recorded hash
matches, then verifies the ECDSA P-256 signature over the hash
using the embedded public key.
All verification happens client-side in SubtleCrypto. The only network
call is the Rekor lookup (public transparency log) — no FormaOS server
roundtrip, so customers can share exports with outside auditors and
point them at this page knowing FormaOS sees nothing.
Files:
* lib/audit/verify-export-merkle-client.ts — pure browser port of
scripts/verify-export-merkle.mjs. Same domain separation (RFC 6962:
0x00 leaf, 0x01 node) and the same canonicalize() shape including
formatCreatedAtV2 for cross-script consistency.
* lib/audit/verify-rekor-anchor-client.ts — pure browser port of
scripts/verify-rekor-anchor.mjs. Includes a small inline DER ECDSA
parser so SubtleCrypto (which wants r||s raw) can verify Rekor's
DER-encoded signatures.
* app/(marketing)/verify/page.tsx + VerifyClient.tsx — public marketing
route. force-static, robots noindex (linked to from /status). Per-
step status badges + summary dl.
Tests (jest):
* __tests__/lib/audit/verify-export-merkle-client.test.ts (7) — builds
a 4-leaf bundle in-line then drives happy path + three tamper
variants (entry mutated, proof mutated, tree_size mismatch).
* __tests__/lib/audit/verify-rekor-anchor-client.test.ts (7) — real
ECDSA P-256 round-trip via node:crypto + a fake Rekor fetcher.
Includes a negative test where the signature is over a different
message to confirm SubtleCrypto actually rejects the bad sig.
Verified: npm run type-check exit 0; 14/14 new jest tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… commits)
Appends a "Follow-up cycle (2026-05-27 evening)" section before the
original 23-commit summary, documenting the 2 regression fixes on local
main and the 13 commits on the feat/audit-2026-05-27-ndis-ui-surface
branch:
* Regression fixes (2): NDIS-1.1 jest test alignment to Phase 3
threshold; dormant_user_candidates view security_invoker lockdown.
* Tier 1 NDIS UI surface (5): policy ndis_category dropdown, BSP CRUD
pages, register-entry sheet, framework status tally, Playwright
smoke spec.
* Tier 2.C Unified health dashboard (3): aggregate lib + 13 unit
tests, /app/compliance/health page, weekly snapshot + cron + SVG
sparkline.
* Tier 3 hygiene (2): NDIS-3.4 per-participant cadence refinement,
SECDEF allowlist trim from 14 → 10.
* Tier 2.A CAPA auto-create (1): evaluator-fail → org_capa_items
deduped insert.
* Tier 2.B Public /verify page (1): client-side Merkle + Rekor
verification via SubtleCrypto + 14 jest tests.
Final verification at close: type-check exit 0; jest 358 suites / 5238
tests / 0 failures; ledger / SECDEF / leaked-secrets all ✓.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds client-side Merkle and Rekor verifiers + tests and a marketing UI; implements compliance health aggregation, per-org snapshot cron and migrations; rewrites NDIS-3.4 to per-participant checks with CAPA auto-creation; adds Behaviour Support Plans CRUD, policy/register form UX, onboarding gating, tests, and docs. ChangesAudit Verification
Compliance Health Dashboard
NDIS Phase 3 and CAPA Auto-Creation
Behaviour Support Plans Feature
Dormant-user purge cron & holds
Migrations, Security, and Ledger
Scripts, Shims & Tooling
Tests and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…Tier 3.1)
Closes two long-standing Supabase advisor WARNs:
* extension_in_public for vector
* extension_in_public for pg_trgm
Pre-flight audit (committed inline in the migration file) confirmed:
* Only 1 user column uses public.vector (ai_document_embeddings.embedding).
OID-tracked → keeps working.
* 2 user indexes use vector_cosine_ops / gin_trgm_ops operator classes.
OID-tracked → keep working.
* Zero user functions hardcode `public.vector` / `public.<operator>`.
* Database-level search_path is `"$user", public, extensions` → operator
resolution still works after the move.
* search_embeddings had a function-scoped search_path
`SET search_path TO 'public', 'pg_temp'` that would have lost the
`<=>` operator. Widened to `'public', 'extensions', 'pg_temp'`
BEFORE the ALTER EXTENSION calls, in a single atomic migration.
Verified post-apply:
* pg_extension shows vector + pg_trgm both in `extensions` schema.
* search_embeddings smoke-tested with a synthetic query — returns 0
rows without error (the `<=>` operator + `extensions.vector` type
both resolve via the widened search_path).
* Standalone `similarity('hello', 'helo')` returned 0.77 — pg_trgm
accessible via search_path.
* Supabase get_advisors no longer flags either extension_in_public
finding. ERROR count is 0; WARN count drops by 2.
* Full gate suite green: type-check 0, jest 5238/5238, ledger ✓,
SECDEF ✓, leaked-secrets ✓.
Rollback path is documented in the migration's leading comment if a
regression surfaces post-merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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: 37322acd6c
ℹ️ 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".
| .from('org_control_evaluations') | ||
| .select('framework_id, control_key, status, last_evaluated_at, total_controls') | ||
| .eq('organization_id', orgId) | ||
| .in('framework_id', frameworkIds); |
There was a problem hiding this comment.
In the contexts I inspected, the evaluator/provisioning paths write per-control rows with org_control_evaluations.framework_id set to compliance_frameworks.id and control_key like control:${compliance_controls.id} (for example lib/compliance/evaluate-framework-controls.ts and lib/frameworks/provisioning.ts). Here frameworkIds are loaded from the unrelated frameworks pack table and then used to filter org_control_evaluations, so evaluated orgs return no per-control rows; aggregate.overall.framework_count stays 0, causing /app/compliance/health to show “No frameworks enabled yet” and the snapshot cron to skip the org even when evaluations exist.
Useful? React with 👍 / 👎.
|
|
||
| const log = routeLog('lib/compliance/capa/auto-create'); | ||
|
|
||
| export const CAPA_SOURCE_TYPE = 'compliance_evaluator'; |
There was a problem hiding this comment.
This source type is written directly into org_capa_items.source_type, but the active lifecycle migration constrains that column to incident, obligation, policy, or manual. As a result, every evaluator-fail auto-create attempts to insert compliance_evaluator and is rejected by the check constraint, so the new CAPA auto-creation feature never opens CAPAs in production.
Useful? React with 👍 / 👎.
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
Decision: NDIS framework should only appear for orgs with industry=ndis.
Tier 4.1 question answered "hide unless industry=NDIS (recommended)".
Applied symmetrically: aged_care framework is now also gated to
industry=aged_care for the same UX reason — keeps SaaS / Financial
Services pickers focused on universal compliance frameworks.
Changes:
* lib/validators/organization.ts — FRAMEWORK_OPTIONS entries can now
carry an `industries` field. New helper frameworkOptionsForIndustry()
filters the universal list down. validateFrameworks() takes an
optional `industry` parameter for server-side enforcement so a
bypass attempt via direct form-POST can't slip an industry-gated
pack past the UI filter.
* app/onboarding/page.tsx — step 5 picker now renders only the
industry-appropriate subset. saveFrameworkSelection reads
orgRecord.industry and passes it to validateFrameworks().
* __tests__/lib/validators/organization-framework-gating.test.ts —
10 jest cases: per-industry visibility, universal frameworks stay
on every industry, bypass-attempt rejection at the validator,
back-compat with omitted-industry callers.
Verified: npm run type-check exit 0; 10/10 jest tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
…+ Tier 4.2/4.3 decisions
Decision: Tier 4.4 hybrid retention policy
* 24mo flag layer: ALREADY LIVE via migration 20260624063
(dormant_user_candidates view + monthly snapshot cron).
* 36mo hard-delete layer: new in this commit.
New shipments:
* Migration 20260624072 — public.dormant_user_purge_holds table.
Operator-placed per-user retention holds (with optional expires_at)
that block the hard-delete cron. Service-role + admin-UI only via
RESTRICTIVE deny-all RLS.
* /api/cron/process-dormant-user-purges (weekly, Mon 08:00 UTC) —
walks auth.users for confirmed accounts >= 36mo dormant, skips
rows with active org_members, active dormant_user_purge_holds, or
existing user_purge_jobs entries. Enqueues purge via the existing
enqueueUserPurge() path — gets the same sole-owner-of-active-org
PurgeRefusedError safety net for free.
* Three escape hatches stack:
1. DORMANT_USER_PURGE_ENABLED env flag (default false).
2. dormant_user_purge_holds row (per-user opt-out).
3. Sole-owner-of-an-org check from enqueueUserPurge.
* vercel.json: new cron schedule + 300s function timeout cap.
* .env.example: DORMANT_USER_PURGE_ENABLED + DORMANT_USER_PURGE_DAYS
documented next to the existing ORG_PURGE_ENABLED entry.
* docs/audit/2026-05-27-tier-4-decisions.md — full Tier 4 decision
record covering 4.1 (already shipped 30a0380), 4.2 (no code change
— bundle into Enterprise), 4.3 (cadence: per major release), 4.4
(this commit).
Verified: type-check exit 0; jest 359 suites / 5248 tests / 0 failures;
ledger / SECDEF / leaked-secrets all ✓.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
docs/audit/2026-05-27-audit-cycle-summary.md (1)
19-50: 💤 Low valueOptional: Add blank lines around tables for markdown compliance.
Five tables are missing surrounding blank lines, flagged by markdownlint MD058. Adding blank lines improves readability and conforms to markdown best practices.
📝 Proposed formatting fix
Tier 1 — NDIS Phase 3 admin UI surface (5 commits): + | Commit | Item | |---|---|Apply the same pattern after lines 28, 35, 41, and 46 (before each table).
🤖 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 `@docs/audit/2026-05-27-audit-cycle-summary.md` around lines 19 - 50, The markdown tables are missing surrounding blank lines, causing markdownlint MD058; for each tier block (the headings "Tier 1 — NDIS Phase 3 admin UI surface", "Tier 2.C — Unified compliance health dashboard", "Tier 3 hygiene", "Tier 2.A — CAPA auto-creation", and "Tier 2.B — Public /verify page") add a blank line immediately before the table and a blank line immediately after the table so each table is separated from adjacent text.supabase/migrations/20260624069_audit_2026_05_27_compliance_health_snapshots.sql (1)
11-21: ⚡ Quick winAdd DB-level integrity checks for snapshot payload fields.
overall_score, control counts, and JSON payload shapes are unchecked, so a buggy writer could persist invalid rows and skew trend/health rendering. Add lightweight CHECK constraints to harden the table contract.Suggested migration adjustment
CREATE TABLE IF NOT EXISTS public.org_compliance_health_snapshots ( id uuid PRIMARY KEY DEFAULT gen_random_uuid(), organization_id uuid NOT NULL REFERENCES public.organizations(id) ON DELETE CASCADE, snapshot_at timestamptz NOT NULL DEFAULT now(), - overall_score numeric(5, 4) NOT NULL, - framework_count integer NOT NULL, - total_controls integer NOT NULL, - status_counts jsonb NOT NULL, - frameworks jsonb NOT NULL, + overall_score numeric(5, 4) NOT NULL CHECK (overall_score >= 0 AND overall_score <= 1), + framework_count integer NOT NULL CHECK (framework_count >= 0), + total_controls integer NOT NULL CHECK (total_controls >= 0), + status_counts jsonb NOT NULL CHECK (jsonb_typeof(status_counts) = 'object'), + frameworks jsonb NOT NULL CHECK (jsonb_typeof(frameworks) = 'array'), created_at timestamptz NOT NULL DEFAULT now() );🤖 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/20260624069_audit_2026_05_27_compliance_health_snapshots.sql` around lines 11 - 21, Add DB-level CHECK constraints to the org_compliance_health_snapshots table: ensure overall_score is between 0 and 1 (e.g., CHECK (overall_score >= 0 AND overall_score <= 1)), ensure framework_count and total_controls are non-negative integers (e.g., CHECK (framework_count >= 0) and CHECK (total_controls >= 0)), and ensure JSON payload columns are actual JSON objects (e.g., CHECK (jsonb_typeof(status_counts) = 'object') and CHECK (jsonb_typeof(frameworks) = 'object')). Modify the CREATE TABLE for org_compliance_health_snapshots (columns overall_score, framework_count, total_controls, status_counts, frameworks) to include these CHECK constraints so invalid rows are rejected at insert/update.
🤖 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/`(marketing)/verify/VerifyClient.tsx:
- Around line 82-94: The textarea bound to bundleText
(data-testid="merkle-input") is missing an accessible name; add a visible
<label> or aria-label linked to the control so screen readers can identify
it—e.g., add id="merkle-input" to the textarea and a corresponding <label
htmlFor="merkle-input">Merkle JSON</label> (or use aria-labelledby pointing to
the label element) in VerifyClient.tsx next to the textarea that uses
setBundleText so you don't break existing bindings or tests.
In `@app/api/cron/compliance-health-snapshot/route.ts`:
- Around line 37-40: The Supabase reads currently ignore returned errors; update
the queries that fetch orgs and the other framework-related query to destructure
the error (e.g., { data: orgs, error: orgsError } = await
admin.from('org_frameworks').select(...)) and, if an error exists, log it and
abort/return from the cron handler (or mark the run failed) instead of
continuing with accounting; apply the same error-check-and-early-return pattern
to the corresponding query at the other block (the framework/orgs fetch) so the
cron never proceeds when a DB read failed.
- Around line 55-60: The read-then-insert flow against
admin.from('org_compliance_health_snapshots') (the recent count check using
.eq('organization_id', orgId) and .gte('snapshot_at', cutoff)) is racy and can
create duplicate snapshots under concurrency; add a database uniqueness guard
(e.g., a unique constraint on (organization_id, snapshot_period) or UNIQUE on
(organization_id, snapshot_at_trunc)) and change the insertion code (the insert
call around the later block that writes to org_compliance_health_snapshots) to
use an upsert/ON CONFLICT DO NOTHING (e.g., Supabase
.insert(...).onConflict(...).ignore() or SQL INSERT ... ON CONFLICT DO NOTHING)
so the DB enforces idempotency atomically instead of relying on the prior count
check. Ensure the schema change creates the unique index and update the insert
path to handle the ignored result (no-op) without throwing.
In `@app/app/actions/behaviour-support-plans.ts`:
- Around line 219-224: The delete currently only checks for Supabase error but
not whether any rows were actually deleted, so RLS can block the operation
silently; in the function handling the deletion (the block using
supabase.from("org_behaviour_support_plans").delete() with planId and
membership.organization_id), inspect the returned data payload (the deleted rows
array) in addition to error and throw a descriptive error when no rows were
deleted (e.g., when data is null/empty), mirroring the
updateBehaviourSupportPlan pattern that checks for !updated; include planId and
membership.organization_id in the error message for context.
In `@app/app/registers/actions.ts`:
- Around line 51-56: The insert into "org_registers" currently sets only org_id
which diverges from the read/query key strategy; update the insert to include
the same org key fallback used by reads (use organization_id if present, falling
back to permissionCtx.orgId, or write both keys) so new rows are visible under
both schemas — modify the insert call in registers/actions.ts (the
.from("org_registers").insert({...}) block that uses permissionCtx.orgId, code,
name) to set organization_id: permissionCtx.orgId (or set both organization_id
and org_id) following the read pattern.
In `@components/registers/create-register-entry-sheet.tsx`:
- Around line 190-194: The select in create-register-entry-sheet.tsx adds a
"critical" value but the badge rendering logic in app/app/registers/page.tsx
only checks for "high" and "medium" and falls back to the green/low style;
update the badge logic to explicitly handle "critical" (e.g., add an if/else
branch or switch for "critical" alongside "high" and "medium") and map it to the
distinct critical badge style/className you use for high severity (or a stricter
red variant), ensuring records with severity === "critical" do not render the
green/low badge.
In `@docs/compliance/ndis-framework-status.md`:
- Line 38: Update the NDIS-3.4 row to explicitly document the fallback: state
that while the rule enforces per-participant 30d cadence based on
org_patients.care_status='active' and org_progress_notes ≥30 in the last 90d,
runtime will fall back to org-wide scoring when org_patients is unavailable or
empty; mention the exact fallback trigger (empty/missing org_patients) and the
resulting behavior (use org-level scoring) and ensure references to NDIS-3.4,
org_progress_notes and org_patients.care_status='active' are included so the
docs match the implemented logic.
In `@e2e/ndis-ui-surface.spec.ts`:
- Line 101: The current await page.waitForURL('**/behaviour-support-plans/**', {
timeout: 30_000 }) is too permissive and will match the creation/new page;
update the page.waitForURL call to assert a detail redirect (i.e. a URL with an
id segment, not '/new') by using either a regex or predicate – for example
replace the wildcard with a regex or predicate that matches
/behaviour-support-plans/{id} and rejects '/new' (e.g. use page.waitForURL(url
=> /\/behaviour-support-plans\/[^/]+$/.test(url), { timeout: 30_000 })) so the
test only passes when the app actually redirected to a detail page.
- Around line 107-122: Make the cleanup unconditional and fail fast by moving
the org_behaviour_support_plans cleanup into a finally block and by checking
Supabase responses for errors; after calling
ctx.admin.from('org_behaviour_support_plans').select('id,
notes').eq(...).ilike(...), verify response.error and throw if present, compute
inserted from response.data, then always call
ctx.admin.from('org_behaviour_support_plans').delete().in('id', inserted.map(r
=> r.id)) when inserted.length>0 and check its response.error and throw on
failure (i.e., wrap the test body in try { ... } finally { /* select -> check
error -> delete -> check error */ } to ensure cleanup runs even on earlier
failures).
In `@lib/audit/verify-export-merkle-client.ts`:
- Around line 195-202: The current short-circuit when bundle.merkle.empty_tree
is true unconditionally returns { ok: true, steps, summary } and can let
malformed bundles pass; update the logic around bundle.merkle.empty_tree in
verify-export-merkle-client.ts so that before pushing the "Empty tree" pass step
and returning, you validate consistency (e.g. bundle.entries.length === 0,
bundle.proofs length === 0 or missing as expected, and bundle.merkle.tree_size
=== 0 or matches empty semantics); if any of those sanity checks fail, push a
failing step and return { ok: false, ... } (or continue full verification)
instead of short-circuiting. Ensure you reference bundle.merkle.empty_tree, the
"Empty tree" step creation, and the early return to locate and change the
behavior.
- Around line 247-255: The loop over bundle.entries currently calls
verifyProof(entry.leaf_hash ?? '', proof, bundle.merkle.root ?? '') without
handling exceptions, so malformed proofs can throw and skip structured step
reporting; wrap the verifyProof call in a try/catch inside the same loop (where
missingProofs and badProofs are tracked) and on any thrown error treat it as a
failed verification by incrementing badProofs (and optionally log the error with
context including entry.id, proof and root) then continue to the next entry so
verification always records a pass/fail rather than throwing.
In `@lib/audit/verify-rekor-anchor-client.ts`:
- Around line 124-132: Normalize args.expectedTopHash to a canonical case once
(e.g., const expectedTopHash = args.expectedTopHash.trim().toLowerCase())
immediately after reading args.expectedTopHash, use HEX_64 to validate that
normalized expectedTopHash, replace all subsequent uses of args.expectedTopHash
with the normalized expectedTopHash (including the 'Input' validation step push,
the "Input shape" pass, and the signature verification block that currently
reads the original casing), and ensure comparisons and signature verification
compare against expectedTopHash consistently.
In `@lib/compliance/capa/auto-create.ts`:
- Around line 178-209: The current select-then-insert flow is racy; add a DB
unique constraint on (organization_id, source_type, source_id) for
org_capa_items and change the write in the CAPA auto-create path to a
conflict-safe insert: replace the direct
supabase.from('org_capa_items').insert(survivors) call with a conflict-handling
insert (e.g.
supabase.from('org_capa_items').insert(survivors).onConflict(['organization_id','source_type','source_id']).ignore())
so concurrent runs won’t error or create duplicates, and update the returned
inserted/skipped counts based on the insert response (or by comparing survivors
length vs returned data length) while keeping dedupeAgainstExisting and
CAPA_SOURCE_TYPE logic unchanged.
In `@lib/compliance/evaluators/ndis/_predicates.ts`:
- Around line 951-965: The current error branch treats any pErr as a signal to
use the org-wide fallback; instead, inspect pErr to only apply the orgWide
fallback for known non-care/access errors (e.g., errors mentioning the
missing/unavailable org_patients table or permission-denied on org_patients —
check pErr.code or pErr.message for patterns like 'relation "org_patients" does
not exist' or 'permission denied'). If the error does not match those patterns,
return na(...) (use the existing na function) with controlCode 'NDIS-3.4',
evaluatedAt, and include the original pErr in the reason/evidence so the
evaluator surfaces as not_evaluated; otherwise keep the existing orgWide
fallback return (using orgWide, evaluatedAt, gaps, confidence, etc.).
In `@lib/compliance/health/fetch.ts`:
- Around line 22-25: The Supabase reads currently ignore the returned error and
can silently treat failures as empty results; for each query pattern like "const
{ data: enabled } = await admin.from('org_frameworks').select(...).eq(...)" (and
the similar destructurings for frameworkIds, evaluations/assessments, and
aggregated results), also destructure "error" from the response, check if error
is non-null, and handle it explicitly (e.g., throw or return a descriptive
Error/Result containing the Supabase error) instead of falling back to an empty
array; update the query sites that use admin.from(...).select(...) (the
org_frameworks and evaluations/assessments/aggregate queries) to follow this
pattern and propagate/report the error so callers can distinguish DB/API
failures from genuinely empty datasets.
In `@lib/compliance/health/trend.ts`:
- Around line 17-25: The current query in the block using
admin.from('org_compliance_health_snapshots').select(...).eq(...).order(...).limit(...)
collapses query failures into an empty trend by returning [] when error is set;
change this so that if error is truthy you surface it (throw or log-and-throw)
instead of returning [], and only return an empty array when data is empty or
null; specifically, after the const { data, error } = await admin... check if
(error) { throw new Error(`DB query failed: ${error.message || error}`) } (or
processLogger.error(...) then throw) and then continue to handle the normal case
returning (data as HealthTrendPoint[]).slice().reverse() or [] when data is
legitimately empty.
In `@lib/frameworks/org-frameworks.ts`:
- Around line 226-231: The current query against
admin.from('org_control_evaluations') silently returns out on any error which
hides operational failures; change the error handling so that if error is truthy
you surface it (log with contextual info including orgId and frameworkIds using
your app logger or throw a wrapped Error) and only return out when data is empty
but no error. Concretely, replace the line "if (error || !data) return out;"
with a branch: if (error) { /* processLogger.error or throw new
Error(`org_control_evaluations query failed for org=${orgId}
frameworks=${frameworkIds}: ${error.message}`) */ } else if (!data) return out;,
keeping references to admin, org_control_evaluations, orgId, frameworkIds and
out so the failure is visible.
---
Nitpick comments:
In `@docs/audit/2026-05-27-audit-cycle-summary.md`:
- Around line 19-50: The markdown tables are missing surrounding blank lines,
causing markdownlint MD058; for each tier block (the headings "Tier 1 — NDIS
Phase 3 admin UI surface", "Tier 2.C — Unified compliance health dashboard",
"Tier 3 hygiene", "Tier 2.A — CAPA auto-creation", and "Tier 2.B — Public
/verify page") add a blank line immediately before the table and a blank line
immediately after the table so each table is separated from adjacent text.
In
`@supabase/migrations/20260624069_audit_2026_05_27_compliance_health_snapshots.sql`:
- Around line 11-21: Add DB-level CHECK constraints to the
org_compliance_health_snapshots table: ensure overall_score is between 0 and 1
(e.g., CHECK (overall_score >= 0 AND overall_score <= 1)), ensure
framework_count and total_controls are non-negative integers (e.g., CHECK
(framework_count >= 0) and CHECK (total_controls >= 0)), and ensure JSON payload
columns are actual JSON objects (e.g., CHECK (jsonb_typeof(status_counts) =
'object') and CHECK (jsonb_typeof(frameworks) = 'object')). Modify the CREATE
TABLE for org_compliance_health_snapshots (columns overall_score,
framework_count, total_controls, status_counts, frameworks) to include these
CHECK constraints so invalid rows are rejected at insert/update.
🪄 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: 27bfb8be-d8c1-44f8-be32-08514731081c
📒 Files selected for processing (43)
__tests__/lib/audit/verify-export-merkle-client.test.ts__tests__/lib/audit/verify-rekor-anchor-client.test.ts__tests__/lib/compliance/capa/auto-create.test.ts__tests__/lib/compliance/evaluators/ndis-phase-2.test.ts__tests__/lib/compliance/health/aggregate.test.tsapp/(marketing)/verify/VerifyClient.tsxapp/(marketing)/verify/page.tsxapp/api/cron/compliance-health-snapshot/route.tsapp/app/actions/behaviour-support-plans.tsapp/app/actions/policies.tsapp/app/behaviour-support-plans/[id]/edit/page.tsxapp/app/behaviour-support-plans/[id]/page.tsxapp/app/behaviour-support-plans/new/page.tsxapp/app/behaviour-support-plans/page.tsxapp/app/compliance/frameworks/page.tsxapp/app/compliance/health/page.tsxapp/app/compliance/page.tsxapp/app/policies/[id]/edit/page.tsxapp/app/policies/new/page.tsxapp/app/registers/actions.tsapp/app/registers/page.tsxcomponents/registers/create-register-entry-sheet.tsxdocs/audit/2026-05-27-audit-cycle-summary.mddocs/compliance/ndis-framework-status.mde2e/ndis-ui-surface.spec.tslib/audit/verify-export-merkle-client.tslib/audit/verify-rekor-anchor-client.tslib/compliance/capa/auto-create.tslib/compliance/evaluate-framework-controls.tslib/compliance/evaluators/ndis/_predicates.tslib/compliance/health/aggregate.tslib/compliance/health/fetch.tslib/compliance/health/trend.tslib/compliance/ndis/categories.tslib/compliance/ndis/register-types.tslib/frameworks/org-frameworks.tslib/navigation/industry-sidebar.tsscripts/.security-definer-rpc-allowlist.jsonsupabase/.migration-ledger-snapshot.jsonsupabase/migrations/20260624069_audit_2026_05_27_compliance_health_snapshots.sqlsupabase/migrations/20260624070_audit_2026_05_27_secdef_allowlist_trim_batch.sqlsupabase/migrations/20260624071_audit_2026_05_27_move_vector_pgtrgm_to_extensions.sqlvercel.json
Comment on lines
+82
to
+94
| <p className="mt-1 text-sm text-muted-foreground"> | ||
| Paste the JSON contents of <code className="font-mono text-xs">audit-log-*.json</code>. | ||
| Verifies every leaf hash + every inclusion proof against the published root. | ||
| </p> | ||
| <textarea | ||
| value={bundleText} | ||
| onChange={(e) => setBundleText(e.target.value)} | ||
| rows={8} | ||
| spellCheck={false} | ||
| placeholder='{"manifest": {...}, "merkle": {...}, "entries": [...]}' | ||
| className="mt-3 w-full rounded-md border border-slate-700 bg-slate-950/40 px-3 py-2 font-mono text-xs" | ||
| data-testid="merkle-input" | ||
| /> |
There was a problem hiding this comment.
Add an explicit label for the Merkle JSON textarea.
At Line 86-Line 94, the textarea lacks an accessible name, so screen-reader users won’t get a reliable form control label.
Proposed fix
- <textarea
+ <label htmlFor="merkle-bundle-input" className="sr-only">
+ Merkle bundle JSON
+ </label>
+ <textarea
+ id="merkle-bundle-input"
value={bundleText}
onChange={(e) => setBundleText(e.target.value)}
rows={8}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p className="mt-1 text-sm text-muted-foreground"> | |
| Paste the JSON contents of <code className="font-mono text-xs">audit-log-*.json</code>. | |
| Verifies every leaf hash + every inclusion proof against the published root. | |
| </p> | |
| <textarea | |
| value={bundleText} | |
| onChange={(e) => setBundleText(e.target.value)} | |
| rows={8} | |
| spellCheck={false} | |
| placeholder='{"manifest": {...}, "merkle": {...}, "entries": [...]}' | |
| className="mt-3 w-full rounded-md border border-slate-700 bg-slate-950/40 px-3 py-2 font-mono text-xs" | |
| data-testid="merkle-input" | |
| /> | |
| <p className="mt-1 text-sm text-muted-foreground"> | |
| Paste the JSON contents of <code className="font-mono text-xs">audit-log-*.json</code>. | |
| Verifies every leaf hash + every inclusion proof against the published root. | |
| </p> | |
| <label htmlFor="merkle-bundle-input" className="sr-only"> | |
| Merkle bundle JSON | |
| </label> | |
| <textarea | |
| id="merkle-bundle-input" | |
| value={bundleText} | |
| onChange={(e) => setBundleText(e.target.value)} | |
| rows={8} | |
| spellCheck={false} | |
| placeholder='{"manifest": {...}, "merkle": {...}, "entries": [...]}' | |
| className="mt-3 w-full rounded-md border border-slate-700 bg-slate-950/40 px-3 py-2 font-mono text-xs" | |
| data-testid="merkle-input" | |
| /> |
🤖 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/`(marketing)/verify/VerifyClient.tsx around lines 82 - 94, The textarea
bound to bundleText (data-testid="merkle-input") is missing an accessible name;
add a visible <label> or aria-label linked to the control so screen readers can
identify it—e.g., add id="merkle-input" to the textarea and a corresponding
<label htmlFor="merkle-input">Merkle JSON</label> (or use aria-labelledby
pointing to the label element) in VerifyClient.tsx next to the textarea that
uses setBundleText so you don't break existing bindings or tests.
Comment on lines
+37
to
+40
| const { data: orgs } = await admin | ||
| .from('org_frameworks') | ||
| .select('organization_id') | ||
| .limit(MAX_ORGS_PER_TICK * 4); |
There was a problem hiding this comment.
Handle Supabase query errors before proceeding with cron accounting.
These reads currently ignore error, so failures can be reported as successful runs with considered: 0 or incorrect skip logic.
Also applies to: 55-60
🤖 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/cron/compliance-health-snapshot/route.ts` around lines 37 - 40, The
Supabase reads currently ignore returned errors; update the queries that fetch
orgs and the other framework-related query to destructure the error (e.g., {
data: orgs, error: orgsError } = await admin.from('org_frameworks').select(...))
and, if an error exists, log it and abort/return from the cron handler (or mark
the run failed) instead of continuing with accounting; apply the same
error-check-and-early-return pattern to the corresponding query at the other
block (the framework/orgs fetch) so the cron never proceeds when a DB read
failed.
Comment on lines
+55
to
+60
| const { count: recent } = await admin | ||
| .from('org_compliance_health_snapshots') | ||
| .select('id', { count: 'exact', head: true }) | ||
| .eq('organization_id', orgId) | ||
| .gte('snapshot_at', cutoff); | ||
|
|
There was a problem hiding this comment.
Make snapshot idempotency atomic at the database layer.
The current “check recent then insert” flow is racy: concurrent runs can both pass the read check and insert duplicates. Use a uniqueness guard (e.g., per-org/per-period unique key) plus on conflict do nothing to enforce idempotency safely.
Also applies to: 72-86
🤖 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/cron/compliance-health-snapshot/route.ts` around lines 55 - 60, The
read-then-insert flow against admin.from('org_compliance_health_snapshots') (the
recent count check using .eq('organization_id', orgId) and .gte('snapshot_at',
cutoff)) is racy and can create duplicate snapshots under concurrency; add a
database uniqueness guard (e.g., a unique constraint on (organization_id,
snapshot_period) or UNIQUE on (organization_id, snapshot_at_trunc)) and change
the insertion code (the insert call around the later block that writes to
org_compliance_health_snapshots) to use an upsert/ON CONFLICT DO NOTHING (e.g.,
Supabase .insert(...).onConflict(...).ignore() or SQL INSERT ... ON CONFLICT DO
NOTHING) so the DB enforces idempotency atomically instead of relying on the
prior count check. Ensure the schema change creates the unique index and update
the insert path to handle the ignored result (no-op) without throwing.
Comment on lines
+219
to
+224
| const { error } = await supabase | ||
| .from("org_behaviour_support_plans") | ||
| .delete() | ||
| .eq("id", planId) | ||
| .eq("organization_id", membership.organization_id); | ||
| if (error) throw new Error(error.message); |
There was a problem hiding this comment.
Delete operation may silently fail without user feedback.
When RLS blocks the delete (non-owner/admin/compliance_admin), Supabase returns no error but also deletes zero rows. Unlike updateBehaviourSupportPlan which checks !updated, delete doesn't verify the operation succeeded.
🛡️ Proposed fix to detect silent delete failures
- const { error } = await supabase
+ const { data: deleted, error } = await supabase
.from("org_behaviour_support_plans")
.delete()
.eq("id", planId)
- .eq("organization_id", membership.organization_id);
+ .eq("organization_id", membership.organization_id)
+ .select("id")
+ .maybeSingle();
if (error) throw new Error(error.message);
+ if (!deleted) {
+ throw new Error(
+ "Plan not deleted. Only owner, admin, or compliance_admin can delete a behaviour support plan.",
+ );
+ }🤖 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/app/actions/behaviour-support-plans.ts` around lines 219 - 224, The
delete currently only checks for Supabase error but not whether any rows were
actually deleted, so RLS can block the operation silently; in the function
handling the deletion (the block using
supabase.from("org_behaviour_support_plans").delete() with planId and
membership.organization_id), inspect the returned data payload (the deleted rows
array) in addition to error and throw a descriptive error when no rows were
deleted (e.g., when data is null/empty), mirroring the
updateBehaviourSupportPlan pattern that checks for !updated; include planId and
membership.organization_id in the error message for context.
Comment on lines
+51
to
+56
| const { data: inserted, error } = await supabase | ||
| .from("org_registers") | ||
| .insert({ | ||
| org_id: permissionCtx.orgId, | ||
| code, | ||
| name, |
There was a problem hiding this comment.
Use the same org key strategy for inserts as reads.
Line 54 writes org_id only, while the list query first reads organization_id. This can make newly created entries disappear (or fail on schemas without org_id). Mirror the read fallback pattern for insert to keep behavior schema-safe.
Suggested fix
- const { data: inserted, error } = await supabase
- .from("org_registers")
- .insert({
- org_id: permissionCtx.orgId,
- code,
- name,
- type,
- description,
- category,
- risk_level: riskLevel,
- fields: [],
- is_active: true,
- })
- .select("id")
- .single();
- if (error) throw new Error(error.message);
+ const payload = {
+ code,
+ name,
+ type,
+ description,
+ category,
+ risk_level: riskLevel,
+ fields: [],
+ is_active: true,
+ };
+
+ let insertedRes = await supabase
+ .from("org_registers")
+ .insert({ organization_id: permissionCtx.orgId, ...payload })
+ .select("id")
+ .single();
+
+ if (
+ insertedRes.error?.code === "42703" &&
+ insertedRes.error.message?.includes("organization_id")
+ ) {
+ insertedRes = await supabase
+ .from("org_registers")
+ .insert({ org_id: permissionCtx.orgId, ...payload })
+ .select("id")
+ .single();
+ }
+
+ const { data: inserted, error } = insertedRes;
+ if (error) throw new Error(error.message);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { data: inserted, error } = await supabase | |
| .from("org_registers") | |
| .insert({ | |
| org_id: permissionCtx.orgId, | |
| code, | |
| name, | |
| const payload = { | |
| code, | |
| name, | |
| type, | |
| description, | |
| category, | |
| risk_level: riskLevel, | |
| fields: [], | |
| is_active: true, | |
| }; | |
| let insertedRes = await supabase | |
| .from("org_registers") | |
| .insert({ organization_id: permissionCtx.orgId, ...payload }) | |
| .select("id") | |
| .single(); | |
| if ( | |
| insertedRes.error?.code === "42703" && | |
| insertedRes.error.message?.includes("organization_id") | |
| ) { | |
| insertedRes = await supabase | |
| .from("org_registers") | |
| .insert({ org_id: permissionCtx.orgId, ...payload }) | |
| .select("id") | |
| .single(); | |
| } | |
| const { data: inserted, error } = insertedRes; | |
| if (error) throw new Error(error.message); |
🤖 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/app/registers/actions.ts` around lines 51 - 56, The insert into
"org_registers" currently sets only org_id which diverges from the read/query
key strategy; update the insert to include the same org key fallback used by
reads (use organization_id if present, falling back to permissionCtx.orgId, or
write both keys) so new rows are visible under both schemas — modify the insert
call in registers/actions.ts (the .from("org_registers").insert({...}) block
that uses permissionCtx.orgId, code, name) to set organization_id:
permissionCtx.orgId (or set both organization_id and org_id) following the read
pattern.
Comment on lines
+178
to
+209
| const { data: existing, error: existingErr } = await supabase | ||
| .from('org_capa_items') | ||
| .select('source_type, source_id') | ||
| .eq('organization_id', args.orgId) | ||
| .eq('source_type', CAPA_SOURCE_TYPE) | ||
| .in('source_id', sourceIds); | ||
|
|
||
| if (existingErr) { | ||
| log.warn( | ||
| { err: existingErr, orgId: args.orgId, candidateCount: candidates.length }, | ||
| 'CAPA auto-create dedupe lookup failed; skipping batch.', | ||
| ); | ||
| return { inserted: 0, skipped: candidates.length }; | ||
| } | ||
|
|
||
| const survivors = dedupeAgainstExisting( | ||
| candidates, | ||
| (existing ?? []) as Array<{ source_type: string | null; source_id: string | null }>, | ||
| ); | ||
|
|
||
| if (survivors.length === 0) { | ||
| return { inserted: 0, skipped: candidates.length }; | ||
| } | ||
|
|
||
| const { error: insertErr } = await supabase.from('org_capa_items').insert(survivors); | ||
| if (insertErr) { | ||
| log.warn( | ||
| { err: insertErr, orgId: args.orgId, attempted: survivors.length }, | ||
| 'CAPA auto-create insert failed.', | ||
| ); | ||
| return { inserted: 0, skipped: candidates.length }; | ||
| } |
There was a problem hiding this comment.
Make CAPA insertion idempotent at write-time to avoid race conditions.
The select existing → insert survivors sequence is racy. Concurrent runs can insert the same (organization_id, source_type, source_id) between those calls, causing duplicate rows or whole-batch insert failure depending on DB constraints. Enforce uniqueness on that key and use conflict-safe writes so concurrent evaluations remain deterministic.
🤖 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/compliance/capa/auto-create.ts` around lines 178 - 209, The current
select-then-insert flow is racy; add a DB unique constraint on (organization_id,
source_type, source_id) for org_capa_items and change the write in the CAPA
auto-create path to a conflict-safe insert: replace the direct
supabase.from('org_capa_items').insert(survivors) call with a conflict-handling
insert (e.g.
supabase.from('org_capa_items').insert(survivors).onConflict(['organization_id','source_type','source_id']).ignore())
so concurrent runs won’t error or create duplicates, and update the returned
inserted/skipped counts based on the insert response (or by comparing survivors
length vs returned data length) while keeping dedupeAgainstExisting and
CAPA_SOURCE_TYPE logic unchanged.
Comment on lines
+951
to
+965
| if (pErr) { | ||
| // Fall back to legacy org-wide threshold when the participant | ||
| // table isn't accessible (non-care orgs). | ||
| return { | ||
| controlCode: 'NDIS-3.4', | ||
| status: orgWide >= 30 ? 'pass' : 'partial', | ||
| evidenceRefs: [ | ||
| { source: 'org_progress_notes', ref: `count=${orgWide}`, capturedAt: evaluatedAt }, | ||
| ], | ||
| gaps: [], | ||
| confidence: 0.5, | ||
| reason: `${orgWide} progress notes/90d (org-wide fallback; participant table unavailable).`, | ||
| evaluatedAt, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Narrow fallback to expected org_patients access errors.
This branch falls back for every query error, including transient DB failures. That can incorrectly return pass/partial instead of not_evaluated and hide real evaluator outages. Restrict fallback to known non-care/access cases and return na(...) for other errors.
🤖 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/compliance/evaluators/ndis/_predicates.ts` around lines 951 - 965, The
current error branch treats any pErr as a signal to use the org-wide fallback;
instead, inspect pErr to only apply the orgWide fallback for known
non-care/access errors (e.g., errors mentioning the missing/unavailable
org_patients table or permission-denied on org_patients — check pErr.code or
pErr.message for patterns like 'relation "org_patients" does not exist' or
'permission denied'). If the error does not match those patterns, return na(...)
(use the existing na function) with controlCode 'NDIS-3.4', evaluatedAt, and
include the original pErr in the reason/evidence so the evaluator surfaces as
not_evaluated; otherwise keep the existing orgWide fallback return (using
orgWide, evaluatedAt, gaps, confidence, etc.).
Comment on lines
+22
to
+25
| const { data: enabled } = await admin | ||
| .from('org_frameworks') | ||
| .select('framework_slug') | ||
| .eq('organization_id', orgId); |
There was a problem hiding this comment.
Handle Supabase query errors explicitly instead of silently degrading to empty aggregates.
These reads currently ignore error, so transient DB/API failures can be misreported as “no frameworks/evaluations,” which hides outages in both dashboard and snapshot flows.
Suggested fix pattern
- const { data: enabled } = await admin
+ const { data: enabled, error: enabledError } = await admin
.from('org_frameworks')
.select('framework_slug')
.eq('organization_id', orgId);
+ if (enabledError) {
+ throw new Error(`Failed to load enabled frameworks: ${enabledError.message}`);
+ }
- const { data: frameworks } = await admin
+ const { data: frameworks, error: frameworksError } = await admin
.from('frameworks')
.select('id, slug, name')
.in('slug', slugs);
+ if (frameworksError) {
+ throw new Error(`Failed to load framework metadata: ${frameworksError.message}`);
+ }
- const { data: evalRows } = await admin
+ const { data: evalRows, error: evalRowsError } = await admin
.from('org_control_evaluations')
.select('framework_id, control_key, status, last_evaluated_at, total_controls')
.eq('organization_id', orgId)
.in('framework_id', frameworkIds);
+ if (evalRowsError) {
+ throw new Error(`Failed to load org control evaluations: ${evalRowsError.message}`);
+ }Also applies to: 42-45, 65-69, 83-87
🤖 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/compliance/health/fetch.ts` around lines 22 - 25, The Supabase reads
currently ignore the returned error and can silently treat failures as empty
results; for each query pattern like "const { data: enabled } = await
admin.from('org_frameworks').select(...).eq(...)" (and the similar
destructurings for frameworkIds, evaluations/assessments, and aggregated
results), also destructure "error" from the response, check if error is
non-null, and handle it explicitly (e.g., throw or return a descriptive
Error/Result containing the Supabase error) instead of falling back to an empty
array; update the query sites that use admin.from(...).select(...) (the
org_frameworks and evaluations/assessments/aggregate queries) to follow this
pattern and propagate/report the error so callers can distinguish DB/API
failures from genuinely empty datasets.
Comment on lines
+17
to
+25
| const { data, error } = await admin | ||
| .from('org_compliance_health_snapshots') | ||
| .select('snapshot_at, overall_score') | ||
| .eq('organization_id', orgId) | ||
| .order('snapshot_at', { ascending: false }) | ||
| .limit(limit); | ||
| if (error || !data) return []; | ||
|
|
||
| return (data as HealthTrendPoint[]).slice().reverse(); |
There was a problem hiding this comment.
Don’t collapse query failures into an empty trend.
Returning [] on query error masks backend failures as a valid “no data” state. Please separate error-path handling (throw/log) from true empty-result handling.
🤖 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/compliance/health/trend.ts` around lines 17 - 25, The current query in
the block using
admin.from('org_compliance_health_snapshots').select(...).eq(...).order(...).limit(...)
collapses query failures into an empty trend by returning [] when error is set;
change this so that if error is truthy you surface it (throw or log-and-throw)
instead of returning [], and only return an empty array when data is empty or
null; specifically, after the const { data, error } = await admin... check if
(error) { throw new Error(`DB query failed: ${error.message || error}`) } (or
processLogger.error(...) then throw) and then continue to handle the normal case
returning (data as HealthTrendPoint[]).slice().reverse() or [] when data is
legitimately empty.
Comment on lines
+226
to
+231
| const { data, error } = await admin | ||
| .from('org_control_evaluations') | ||
| .select('framework_id, status, last_evaluated_at, total_controls') | ||
| .eq('organization_id', orgId) | ||
| .in('framework_id', frameworkIds); | ||
| if (error || !data) return out; |
There was a problem hiding this comment.
Surface tally query failures instead of returning an indistinguishable empty map.
If this query fails, the UI currently presents it as “no evaluations recorded yet.” Please propagate or log the error path so operational failures are visible.
🤖 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/frameworks/org-frameworks.ts` around lines 226 - 231, The current query
against admin.from('org_control_evaluations') silently returns out on any error
which hides operational failures; change the error handling so that if error is
truthy you surface it (log with contextual info including orgId and frameworkIds
using your app logger or throw a wrapped Error) and only return out when data is
empty but no error. Concretely, replace the line "if (error || !data) return
out;" with a branch: if (error) { /* processLogger.error or throw new
Error(`org_control_evaluations query failed for org=${orgId}
frameworks=${frameworkIds}: ${error.message}`) */ } else if (!data) return out;,
keeping references to admin, org_control_evaluations, orgId, frameworkIds and
out so the failure is visible.
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/cron/process-dormant-user-purges/route.ts`:
- Around line 80-87: The three parallel hydration queries that populate
memberRows, holdRows, and jobRows can fail silently; update the Promise.all
result handling to detect any returned errors (e.g., check for an "error"
property or non-OK response) for each of admin.from('org_members').select(...),
admin.from('dormant_user_purge_holds').select(...), and
admin.from('user_purge_jobs').select(...), and if any error is present log the
error (including context and candidateIds) and throw/return early so the handler
fails fast instead of treating the set as empty and proceeding to enqueue purge
jobs.
In `@lib/validators/organization.ts`:
- Around line 173-176: validateFrameworks was updated to accept an industry but
validateOnboardingForm still calls validateFrameworks(frameworks) without
passing data.industry, causing onboarding to use the non-industry-aware
validation path; update validateOnboardingForm to pass the industry (e.g.,
validateFrameworks(data.frameworks, data.industry)) so validateFrameworks can
perform industry-aware checks, and audit any other callers of validateFrameworks
to ensure they forward the industry argument where appropriate.
In
`@supabase/migrations/20260624072_audit_2026_05_27_dormant_user_purge_holds.sql`:
- Around line 42-43: The schema comment on table public.dormant_user_purge_holds
incorrectly references a non-existent deleted_at column; update the COMMENT ON
TABLE statement (for public.dormant_user_purge_holds) to remove the deleted_at
reference and accurately describe the active-hold condition (e.g., "Active hold
= expires_at IS NULL OR expires_at > now(). Service-role + admin-UI only.") or,
if intended, add the missing deleted_at column via a migration—pick one and make
the COMMENT match the actual schema.
🪄 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: ec0c2c6b-a6d6-4d1a-9869-b3452b593b1b
📒 Files selected for processing (9)
.env.example__tests__/lib/validators/organization-framework-gating.test.tsapp/api/cron/process-dormant-user-purges/route.tsapp/onboarding/page.tsxdocs/audit/2026-05-27-tier-4-decisions.mdlib/validators/organization.tssupabase/.migration-ledger-snapshot.jsonsupabase/migrations/20260624072_audit_2026_05_27_dormant_user_purge_holds.sqlvercel.json
✅ Files skipped from review due to trivial changes (1)
- supabase/.migration-ledger-snapshot.json
🚧 Files skipped from review as they are similar to previous changes (1)
- vercel.json
Comment on lines
+80
to
+87
| const [memberRows, holdRows, jobRows] = await Promise.all([ | ||
| admin.from('org_members').select('user_id').in('user_id', candidateIds), | ||
| admin | ||
| .from('dormant_user_purge_holds') | ||
| .select('user_id, expires_at') | ||
| .in('user_id', candidateIds), | ||
| admin.from('user_purge_jobs').select('user_id').in('user_id', candidateIds), | ||
| ]); |
There was a problem hiding this comment.
Fail fast when skip-reason hydration queries fail.
If any of these three queries errors, the code currently treats that set as empty and can enqueue irreversible purge jobs for users who should be protected.
Suggested fix
const [memberRows, holdRows, jobRows] = await Promise.all([
admin.from('org_members').select('user_id').in('user_id', candidateIds),
admin
.from('dormant_user_purge_holds')
.select('user_id, expires_at')
.in('user_id', candidateIds),
admin.from('user_purge_jobs').select('user_id').in('user_id', candidateIds),
]);
+ if (memberRows.error || holdRows.error || jobRows.error) {
+ log.error(
+ {
+ memberRowsError: memberRows.error?.message,
+ holdRowsError: holdRows.error?.message,
+ jobRowsError: jobRows.error?.message,
+ },
+ 'failed to hydrate dormant-user purge skip reasons',
+ );
+ return NextResponse.json(
+ { error: 'skip_reason_hydration_failed' },
+ { status: 500 },
+ );
+ }🤖 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/cron/process-dormant-user-purges/route.ts` around lines 80 - 87, The
three parallel hydration queries that populate memberRows, holdRows, and jobRows
can fail silently; update the Promise.all result handling to detect any returned
errors (e.g., check for an "error" property or non-OK response) for each of
admin.from('org_members').select(...),
admin.from('dormant_user_purge_holds').select(...), and
admin.from('user_purge_jobs').select(...), and if any error is present log the
error (including context and candidateIds) and throw/return early so the handler
fails fast instead of treating the set as empty and proceeding to enqueue purge
jobs.
Comment on lines
+173
to
+176
| export function validateFrameworks( | ||
| frameworks: string[], | ||
| industry?: string | null, | ||
| ): { valid: boolean; error?: string } { |
There was a problem hiding this comment.
Industry-aware validation is still bypassable via validateOnboardingForm.
validateFrameworks became industry-aware, but validateOnboardingForm still calls it without data.industry, so this path silently falls back to full-list validation.
Suggested fix
- const frameworkValidation = validateFrameworks(data.frameworks);
+ const frameworkValidation = validateFrameworks(
+ data.frameworks,
+ data.industry,
+ );🤖 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/validators/organization.ts` around lines 173 - 176, validateFrameworks
was updated to accept an industry but validateOnboardingForm still calls
validateFrameworks(frameworks) without passing data.industry, causing onboarding
to use the non-industry-aware validation path; update validateOnboardingForm to
pass the industry (e.g., validateFrameworks(data.frameworks, data.industry)) so
validateFrameworks can perform industry-aware checks, and audit any other
callers of validateFrameworks to ensure they forward the industry argument where
appropriate.
Comment on lines
+42
to
+43
| COMMENT ON TABLE public.dormant_user_purge_holds IS | ||
| 'Audit 2026-05-27 (Tier 4.4): operator-placed retention holds blocking the 36-month dormant-user purge. Active hold = no row deleted_at AND (expires_at IS NULL OR expires_at > now()). Service-role + admin-UI only.'; |
There was a problem hiding this comment.
Schema comment references a non-existent deleted_at field.
The table has no deleted_at column, so this comment is inaccurate and can mislead operational queries.
Suggested fix
COMMENT ON TABLE public.dormant_user_purge_holds IS
- 'Audit 2026-05-27 (Tier 4.4): operator-placed retention holds blocking the 36-month dormant-user purge. Active hold = no row deleted_at AND (expires_at IS NULL OR expires_at > now()). Service-role + admin-UI only.';
+ 'Audit 2026-05-27 (Tier 4.4): operator-placed retention holds blocking the 36-month dormant-user purge. Active hold = (expires_at IS NULL OR expires_at > now()). Service-role + admin-UI only.';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| COMMENT ON TABLE public.dormant_user_purge_holds IS | |
| 'Audit 2026-05-27 (Tier 4.4): operator-placed retention holds blocking the 36-month dormant-user purge. Active hold = no row deleted_at AND (expires_at IS NULL OR expires_at > now()). Service-role + admin-UI only.'; | |
| COMMENT ON TABLE public.dormant_user_purge_holds IS | |
| 'Audit 2026-05-27 (Tier 4.4): operator-placed retention holds blocking the 36-month dormant-user purge. Active hold = (expires_at IS NULL OR expires_at > now()). Service-role + admin-UI only.'; |
🤖 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/20260624072_audit_2026_05_27_dormant_user_purge_holds.sql`
around lines 42 - 43, The schema comment on table
public.dormant_user_purge_holds incorrectly references a non-existent deleted_at
column; update the COMMENT ON TABLE statement (for
public.dormant_user_purge_holds) to remove the deleted_at reference and
accurately describe the active-hold condition (e.g., "Active hold = expires_at
IS NULL OR expires_at > now(). Service-role + admin-UI only.") or, if intended,
add the missing deleted_at column via a migration—pick one and make the COMMENT
match the actual schema.
ejay-dev
force-pushed
the
feat/audit-2026-05-27-ndis-ui-surface
branch
from
May 27, 2026 14:03
0b05f1f to
84e857b
Compare
@supabase/realtime-js >= 2.10 throws "Node.js 20 detected without native WebSocket support" when @supabase/supabase-js createClient() runs under Node < 22. The RealtimeClient is instantiated eagerly even when callers only use REST endpoints, so REST-only scripts get hit too. This was failing on every PR + on main since the latest realtime-js bump (verified: main @ 7931fcf has the same Core Build Gate + Database & Backend Tests failures). Not a regression from this branch's work. Fix: tiny polyfill that sets `globalThis.WebSocket = ws` before @supabase/supabase-js gets imported. `ws@8.21.0` is already a transitive dep via jest-environment-jsdom, openai, and puppeteer, so no package.json change needed. Polyfill is a no-op on Node 22 (native WebSocket present). Files: * scripts/lib/node-websocket-polyfill.cjs — CommonJS variant. * scripts/lib/node-websocket-polyfill.mjs — ESM variant. * scripts/test-db-integrity.js — require the .cjs polyfill before createClient. * scripts/check-db-test-verify.mjs — import the .mjs polyfill before createClient. The other ~18 scripts that use createClient run interactively from operators on Node 22, so they don't hit this. Adding the polyfill to them in a follow-up if any of them get wired into CI. Verified: * Both polyfill variants set globalThis.WebSocket to a function. * test-db-integrity.js with dummy creds now fails with `TypeError: fetch failed` (expected for a non-existent URL) instead of the WebSocket throw — proves the createClient path completes. * type-check exit 0; jest 359/359 still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
force-pushed
the
feat/audit-2026-05-27-ndis-ui-surface
branch
from
May 27, 2026 14:07
84e857b to
3bc4af4
Compare
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
Third script that calls createClient() from @supabase/supabase-js without the WebSocket shim that supabase-js needs under Node 20. Triggered "Run Supabase health check" failure in the Database & Backend Tests workflow once the first two scripts (test-db-integrity, check-db-test-verify) were unblocked. Companion to 3bc4af4 (test-db-integrity.js + check-db-test-verify.mjs). Other CI-gate scripts that use createClient were already wired correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
1 similar comment
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
…on Node 20
Same shape as scripts/_node20-ws-shim.mjs but inlined into e2e/helpers/
test-auth.ts because Playwright transpiles .ts files but doesn't follow
external .mjs side-effect imports reliably across worker processes.
Failure surfaced in Playwright Integrity Gate after the CI script
shims unblocked the earlier gates. The chain:
e2e/export-integrity.spec.ts → getWorkspaceSeedContext
→ getTestCredentials → createTemporaryTestUser
→ _createTemporaryTestUserImpl → createClient
→ ERROR: Node.js 20 detected without native WebSocket support.
workspace-seed.ts imports from test-auth first, so polyfilling at the
top of test-auth.ts is enough — module init order guarantees the
polyfill runs before any createClient call downstream.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…shim Previous attempt (4555aaf) inlined a polyfill using `import { createRequire } from 'node:module'`. Under Playwright's TS loader that import flagged the file as pure ESM, but the transpiled output still used CJS-style `exports`, so visual-verification (which runs e2e/global-setup.ts via the capture config) crashed with: ReferenceError: exports is not defined in ES module scope at helpers/test-auth.ts:3 Replaced with a dedicated `e2e/helpers/_node20-ws-shim.ts` that uses `typeof require === 'function' ? require('ws') : null`. Works under Playwright's default CJS .ts compilation, no `node:module` import needed. test-auth.ts now imports this shim as its first line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
…st-auth.ts
Two more leftover references to the dropped public.orgs table inside
the e2e auth helper:
* Line 290 — _ensureWorkspaceProvisioned org-set loop's legacy
mirror upsert.
* Line 890 — _createTemporaryTestUserImpl's per-test legacy mirror
upsert.
Both throw "Could not find the table 'public.orgs' in the schema cache"
under the post-20260624051 (commit 6126ab2) schema state. Removed —
organizations(id) is the sole source of truth for org identity now.
Sibling specs (auth-invariant, mfa-enforcement, product-walkthrough,
onboarding-flow, trial-provisioning-guarantee) carry similar dead
references but aren't gated by Playwright Integrity Gate. Left alone
to keep this fix surgical; cleanup in a follow-up if their own
workflows surface failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
… removal 214812f removed the legacy public.orgs mirror block + accidentally the `const nowIso = new Date().toISOString();` line that lived inside it. Downstream code in _createTemporaryTestUserImpl still references nowIso (org_frameworks seed, org_onboarding_status seed, MFA seed), so e2e helpers crashed with `ReferenceError: nowIso is not defined`. Restored the declaration; mirror block stays removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
…n TENANT_TABLE_SCOPES lib/onboarding/first-session.ts uses createSupabaseOrgClient and queries org_progress_notes — that table was never registered in TENANT_TABLE_SCOPES, so every /app dashboard load throws "createSupabaseOrgClient: table org_progress_notes is not registered as a tenant table". Hidden behind earlier failures until the PR's e2e flow finally reached /app. Also registering Phase 3's two new tenant tables proactively: * org_behaviour_support_plans (BSP CRUD) * org_compliance_health_snapshots (weekly health snapshot) Both use organization_id as the tenant column. dormant_user_purge_holds NOT registered — service-role only, no app-side read path. Verified: npm run type-check exit 0; supabase + onboarding jest suites (99 tests) all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
Commit 7fd40ff ratcheted org_evidence.file_hash to NOT NULL but didn't update the e2e seed in workspace-seed.ts. Surfaced as "Failed to seed org_evidence row: null value in column file_hash violates not-null constraint" once Playwright Integrity Gate's WebSocket + public.orgs + TENANT_TABLE_SCOPES blockers cleared. Fix: compute a real SHA-256 of the upload content and stamp it as file_hash. Carries the same integrity invariant as a production row (audit-engine's evidence redaction logic expects every row's file_hash to be reproducible from the bytes referenced by file_path). Sibling site in e2e/onboarding-flow.spec.ts has the same issue but isn't gated by Playwright Integrity Gate — leaving for a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
♿ Accessibility Test Results✅ PASSED - No critical accessibility issues found Tests Performed:
Artifacts: Download the accessibility reports from the "Artifacts" section for detailed results. |
3 tasks
ejay-dev
added a commit
that referenced
this pull request
May 28, 2026
…sy gate (#199) * chore(audit-2026-05-28): remove dead public.orgs refs + file_hash in 5 e2e specs Follow-up to PR #192. The legacy public.orgs mirror calls inside e2e/helpers/test-auth.ts were removed in that PR's CI-unblock pass, but sibling specs that don't go through the helper still carried the same dead references. None of them are gated by Playwright Integrity Gate, so they didn't block #192 — but they would fail the moment any other workflow ran them (qa:deep, nightly sweep, ad-hoc local runs). Surgical cleanup: * e2e/onboarding-flow.spec.ts — adds file_hash to seeded org_evidence row (NOT NULL since commit 7fd40ff) AND removes the `from('orgs').delete()` mirror at teardown. * e2e/trial-provisioning-guarantee.spec.ts — mirrorLegacyOrg() becomes a no-op stub. Kept as a function so the 3 call sites compile without churn; safe to inline-delete in a future pass. * e2e/auth-invariant.spec.ts — removes `orgs.delete` at teardown and the legacy mirror upsert + error propagation at line 367. * e2e/product-walkthrough.spec.ts — 3 sites: teardown delete, setup upsert, second teardown delete. * e2e/auth/mfa-enforcement.spec.ts — removes the `orgs.upsert` mirror + simplifies the teardown comment (the mirror trigger referenced no longer exists either). Verified: npm run type-check exits 0. No spec semantics changed beyond removing impossible writes; every spec still creates + deletes an `organizations` row through the canonical path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(audit-2026-05-28): demote production-config check to advisory in deployment-gates The "Production configuration validation (critical)" step has been failing on every main push for months. Verified: last 5 main commits all have `Deployment Quality Gates: failure` status. Root cause: the step runs `check-env.js --strict --profile=production` against process.env, but production secrets (FOUNDER_EMAILS, STRIPE_*, RESEND_*, UPSTASH_*, CRON_SECRET, SENTRY_*) live only in Vercel — they were never mirrored to GitHub Actions repo secrets. Worse: this gate can't actually block production deploys. Vercel's git integration deploys main commits on its own schedule, independent of this workflow. We confirmed this with PR #192's merge — Vercel shipped 015a095 to https://www.formaos.com.au successfully while this workflow was red. A perpetually-red non-blocking gate is worse than no gate: it masks real failures (red checks become noise the reviewer ignores). Fix: demote `Production configuration validation` to advisory via `continue-on-error: true`. The summary still surfaces what's missing, so operators can choose to mirror the secrets without us re-promoting the gate ourselves. Re-promote to blocking once GH secrets mirror Vercel prod and the step runs clean for a week. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(audit-2026-05-28): surface next-drill-due date in restore-recency check When the gate IS in its time-bomb mode (an initial restore_test_runs row exists and we're inside the 35-day window), the success log previously said: ✓ Latest restore test: success (3 days ago, within 35-day window). That tells the operator they're fine NOW. It doesn't tell them WHEN the gate flips red. So a drill at day 0 looks identical to a drill at day 34 — the only thing differentiating them is whether someone remembers to count. Now emits the computed deadline: ✓ Latest restore test: success (3 days ago, within 35-day window). Next drill due by 2026-06-30 or this gate goes red. Plus a fattened comment in the warn-only branch flagging the operator heads-up so the time-bomb behaviour isn't a surprise the first time the gate fires. No semantic change to the gate's pass/warn/fail logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ejay-dev
added a commit
that referenced
this pull request
May 29, 2026
…nant-isolation ratchet
H1 — `scripts/check-rls-current-setting.mjs` was flagging
`current_setting('app.environment', true)` calls inside SECURITY
DEFINER function bodies as policy-body violations. The gate's own
error message explicitly authorises the GUC pattern when set inside
a SECDEF function — the regex now matches that intent by stripping
dollar-quoted bodies (`$$ ... $$` and `$tag$ ... $tag$`) before
scanning. Wired into qa-pipeline.yml right after the SECDEF grants
check so it now blocks merges (previously lived in qa:deep only —
PR #192 landed with it failing because nothing was checking).
H4 — three new v1 routes added to openapi.json under a new
`Account` tag: GET/POST `/api/v1/account/active-organization`,
GET `/api/v1/account/export` (GDPR Article 15/20), POST
`/api/v1/account/delete` (GDPR Article 17 erasure with
`confirm:"DELETE"` body). Validated 57 → 61 operations; 0 new
undocumented routes.
H5 — tenant-isolation ratchet ratcheted down from 266 → 263 via
file-level `eslint-disable formaos/no-admin-client-with-org-filter`
on four cross-tenant cron routes and the founder `/api/admin/orgs/
[orgId]` surface. Each annotation cites ENGINEERING_CHANGE_MATRIX
"Tenant Data Access" guidance. Baseline locked at 263 in
scripts/check-tenant-isolation-ratchet.mjs.
Verification:
- npm run test:db:rls-guc-guard → 248 migrations, 0 violations
- npm run test:api-contracts → 61 ops, 0 new undocumented
- npm run check:tenant-isolation-ratchet → 263 (baseline 263)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Follow-up to the original audit-2026-05-27 cycle (commit
9199f5d1). Bundles 13 commits spanning Tier 1 (NDIS Phase 3 admin UI), Tier 2.A (CAPA auto-creation), Tier 2.B (public /verify page), Tier 2.C (unified compliance health dashboard), and Tier 3 hygiene (NDIS-3.4 cadence refinement + SECDEF allowlist trim).The 2 P0 regression fixes that surfaced during this cycle (NDIS-1.1 test threshold + dormant_user_candidates advisor ERRORs) are already on
mainand provide the green baseline this branch builds on.What lands
Tier 1 — NDIS Phase 3 admin UI surface (5 commits)
ndis_categorydropdown on policy editor (new + edit) backed by the 18-enum constraint/app/behaviour-support-planslist/new/[id]/edit)/app/registerscovering the 10 NDIS taxonomy valuesTier 2.C — Unified compliance health dashboard (3 commits)
lib/compliance/health/aggregate.ts— pure rollup (overall weighted score, per-framework, top-10 outstanding by urgency_score). 13 unit tests./app/compliance/healthpage wiring the aggregate to a UIorg_compliance_health_snapshots, migration20260624069)Tier 3 hygiene (2 commits)
20260624070)Tier 2.A — CAPA auto-creation (1 commit)
org_capa_itemsrow, deduped by(source_type='compliance_evaluator', source_id=framework_control.id). 11 unit tests.Tier 2.B — Public /verify page (1 commit)
Docs (1 commit)
Verification
npm run type-checknpx jest __tests__/ --testPathIgnorePatterns='integration/rls'npm run test:db:ledger-alignmentnpm run test:db:secdef-grantsnpm run test:security:leaked-secretsMigrations applied
20260624068— dormant_user_candidates view lockdown (already on main)20260624069— org_compliance_health_snapshots20260624070— SECDEF allowlist trim batchAll three applied via execute_sql with FS-prefix-aligned ledger entries; alignment verified.
Outstanding (NOT in this PR)
Test plan
npm run type-check && npx jest __tests__/ --testPathIgnorePatterns='integration/rls'/verifyin a logged-out browser → paste a real audit-export bundle → confirm pass/app/policies/new→ confirm NDIS category dropdown shows 18 options + "Not NDIS-tagged"/app/behaviour-support-plans→ confirm list + new + detail + edit paths work/app/compliance/health→ confirm overall card + per-framework cards + top-10 outstanding render🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation