Access-cutoff loop closure: nothing clears the cutoff, and a ledger failure emails admins daily forever

[guillermoscript]

Part of EPIC #540. §4.6. Follow-up to #494 / #517, and interacts with the still-open #513.

1. Nothing clears access_cutoff_at when a school does what the notice tells it to do 🔴

reconcileAccessCutoff has exactly five call sites — lib/billing/downgrade-tenant.ts:64, lib/payments/platform-plan-change.ts:111,162, app/actions/admin/billing.ts:363,540 — plus app/api/cron/enforce-plan-limits/route.ts:82. All five are plan-change events. Nothing on the usage side.

But the cutoff email's remediation is a usage action: lib/billing/access-cutoff.ts:116-121 tells the admin "20 active courses exceed the Free plan's limit of 15". Archiving courses or removing members is what the school is being asked to do — and app/actions/teacher/courses.ts (archive/delete) and app/actions/join-school.ts do not import reconcileAccessCutoff at all (verified by full-repo grep).

So the asymmetry is: a plan downgrade schedules a cutoff synchronously; a usage reduction does not clear it. A school that complies stays locked out until the next daily sweep — or, if #513's answer is "nothing on the Dokploy host calls /api/cron/*", indefinitely. The only recovery paths today are buying a bigger plan or a super admin editing the row. app/[locale]/dashboard/student/access-suspended/page.tsx correctly redirects the student out as soon as the cutoff clears; nothing clears it.

Doc drift: docs/MONETIZATION.md:277 states "Event-driven call sites (join-school, course creation, plan changes) do not pass the flag" — asserting join-school and course creation are call sites. They are not. docs/OPERATIONS_GUIDE.md:218 needs the same correction.

2. A failed ledger write makes the notification ladder re-send the same email daily, forever 🔴

lib/billing/access-cutoff.ts:198-207 — the access_cutoff_notifications upsert's error is logged (if (error) console.error(...)) and then the function returns { delivered: true } unconditionally.

:279-284 — the next sweep computes the due stage from fetchSentStages(...), reading that same ledger. An unwritten row means the rung is still "unsent", so :113 (reached.find((stage) => !sent.has(stage))) returns the same rung again. And app/api/cron/enforce-plan-limits/route.ts:87-89 counts it under notified, so the run reports healthy.

deliverCutoffStage's own contract comment says a row is written only after at least one recipient send succeeded. The inverse — a send succeeded but was never recorded — is silently violated. Trigger conditions: RLS is enabled on access_cutoff_notifications with no policy (20260725100000_access_cutoff_notifications.sql:45), so any non-service-role caller of reconcileAccessCutoff writes nothing at all; plus any constraint or transient error.

Result: every tenant admin gets the identical cutoff email every day until the cutoff clears. That is precisely the failure mode #517 was written to prevent, inverted.

Related, smaller: the schedule branch (:275-277) forces stage 'scheduled' without consulting the ledger, so a cron/plan-change race double-sends the first rung even though ignoreDuplicates keeps the ledger itself clean.

3. A missing platform_plans row clears live enforcement 🟡

lib/billing/access-cutoff.ts:246-249 — a lookup miss yields violations = [], which clears an existing cutoff rather than leaving it alone. Fail-open is documented for checkPlanLimits, but "clear live enforcement on a lookup miss" is a different tradeoff than that doc covers. Decide it deliberately.

Scope

In scope

• Call reconcileAccessCutoff (fire-and-forget, try/catch that logs and swallows — reconciliation must never fail the write that already succeeded) after every action that can reduce usage: course archive, course delete, tenant-member removal. Both already hold or can obtain an admin client. The function is idempotent by design (:62-71), so extra calls are no-ops.
• Add an explicit "re-check limits" action to the admin billing page, so recovery never depends on a scheduler at all.
• deliverCutoffStage: treat the ledger write as part of delivery — if (error) return { delivered: false } — so notifyFailures counts it instead of notified.
• Consider consulting the ledger on the schedule branch too.
• Decide and document the missing-plan-row behaviour in §3.
• Fix docs/MONETIZATION.md:277 and docs/OPERATIONS_GUIDE.md:218.

Out of scope

• Scheduling /api/cron/* — that is Access-cutoff reconciliation depends on a cron that may never run on Dokploy (#494 follow-up) #513, which has an open draft PR (fix(ops): schedule /api/cron/* from GitHub Actions and reconcile cutoff on usage change (#513) #519) blocked on two repo settings (secret CRON_SECRET, variable CRON_BASE_URL). This issue deliberately makes recovery work without a scheduler; Access-cutoff reconciliation depends on a cron that may never run on Dokploy (#494 follow-up) #513 makes the periodic sweep exist. Land Access-cutoff reconciliation depends on a cron that may never run on Dokploy (#494 follow-up) #513 too, but neither blocks the other.
• dueCutoffNotificationStage's ladder logic — audited and correct (most-urgent-unsent, failed-send retry, supersede-stale-rung, post-cutoff enforced-only, fresh ladder per cutoff; 14 test cases).
• The Access cutoff: single warning email, no in-app signal, undocumented tenant-wide blast radius (#494 follow-up) #517 notice audience split — AccessCutoffBanner is admin-only by design (app/[locale]/dashboard/layout.tsx:50) and students get the dedicated /access-suspended page; both clear correctly. Intentional, not a gap.
• has_course_access()'s cutoff clause (20260724130000:19-35) — correct; the SQL function does know about the cutoff, it is not app-layer-only.
• Cutoff-clearing ordering in app/actions/admin/billing.ts:363,540 — both reconcile after the tenants.plan write, so the clear decision sees the new plan. Correct.

Test coverage this must add

reconcileAccessCutoff — the function that actually enforces #494/#517 — has zero coverage. tests/unit/access-cutoff.test.ts:2 imports only decideAccessCutoffAction; access-cutoff-notifications.test.ts:10-12 imports only the three pure helpers. The untested 76 lines (lib/billing/access-cutoff.ts:221-297) hold every stateful decision: the tenants write, the three-way effectiveCutoffAt selection (:266-271), the schedule-branch shortcut, deliverCutoffStage's recipient counting, and defect §2 above.

Write table-driven tests over (violations × currentCutoffAt × notifyDueStages × sendEmail result × ledger write result). tests/unit/course-access-state.test.ts:17-43 is a good stub-client template.

Also untested: app/actions/platform/billing-health.ts:84-133, the four-query union (incl. the missingTenantIds follow-up) that feeds the well-covered pure functions.

Acceptance criteria

• Archiving enough courses to drop back under the limit clears access_cutoff_at synchronously, and the student's /access-suspended page redirects them back in without waiting for a cron.
• Removing a member below the student limit does the same.
• The admin billing page has a working "re-check limits" action.
• A failing ledger write results in delivered: false and is counted under notifyFailures, not notified — asserted with a stub whose upsert errors.
• Running the sweep twice in a row with a failing ledger does not send the same rung twice.
• docs/MONETIZATION.md and docs/OPERATIONS_GUIDE.md describe the call sites that actually exist.
• npm run test:unit green (370/370 baseline at 146a8cfa) plus new reconcileAccessCutoff coverage.

Effort: S–M · Risk: LOW — decideAccessCutoffAction is idempotent, and returning delivered: false on a ledger error costs at most one extra send next sweep, which is strictly better than unbounded repeats.

[guillermoscript]

Plan of attack

Surveyed the four defects and the code they name. Three of the issue's premises need
correcting before the plan makes sense, so those come first.

What the survey changed about the issue

a) There is no member-removal path to add a reconcileAccessCutoff call to.
The issue's scope says "Both already hold or can obtain an admin client", implying a
member-removal action exists. It does not. app/actions/admin/users.ts has
deactivateUser, but it only stamps profiles.deactivated_at — a column that
grep shows is read nowhere except the admin users list and detail page, purely
for display. countTenantUsage (lib/billing/plan-limits.ts:57-62) counts
tenant_users rows with role = 'student' AND status = 'active', which
deactivateUser never touches. Every other tenant_users write in the repo is an
INSERT (join-school.ts:99, onboarding.ts:49, checkout/actions.ts). So acceptance
criterion 2 is not "add a call" — it is "build the removal the call would hang off".

b) docs/OPERATIONS_GUIDE.md has no access-cutoff content to correct.
grep -n "cutoff\|enforce-plan-limits\|reconcile" docs/OPERATIONS_GUIDE.md returns
nothing across all 732 lines; line 218 is Stripe webhook setup. The drift is confined
to docs/MONETIZATION.md:277.

c) The schedule-branch race fixes itself. On a fresh schedule the ladder has
msRemaining = 14 days, so dueCutoffNotificationStage reaches exactly ['scheduled']
and returns it when unsent, null when the ledger already holds it. Routing the
schedule branch through the same function is both the de-race and a simplification —
no separate ledger check needed.

Root cause

Two independent asymmetries, plus one fail-open that reaches too far.

1. §1 — reconcileAccessCutoff's six call sites are all plan-change events. The
   cutoff email asks the school for a usage action (archive courses, remove members).
   Nothing on the usage side reconciles, so compliance is invisible until the daily
   sweep — or forever, if nothing on the host calls /api/cron/* (Access-cutoff reconciliation depends on a cron that may never run on Dokploy (#494 follow-up) #513).
2. §2 — deliverCutoffStage returns { delivered: true } unconditionally
   (access-cutoff.ts:198-207), so a failed ledger upsert leaves the rung unrecorded
   while the caller counts it under notified. fetchSentStages then re-derives the
   same rung tomorrow. RLS on access_cutoff_notifications is enabled with no
   policy (20260725100000:45), so any non-service-role caller hits this every time.
3. §3 — a missing platform_plans row yields violations = []
   (access-cutoff.ts:246-249), which decideAccessCutoffAction reads as "compliant"
   and clears a live cutoff. Fail-open on a lookup miss is right for scheduling
   new enforcement; silently lifting enforcement that is already in force is a
   different and much louder decision.

Approach

§1 — close the loop on the usage side

• Export reconcileAccessCutoffSafely(admin, tenantId) from lib/billing/access-cutoff.ts:
  a try/catch wrapper that logs and swallows. Reconciliation must never fail a write
  that already succeeded.
• Call it after archiveCourse and deleteCourse (app/actions/teacher/courses.ts),
  both of which already hold an admin client.
• Add removeTenantMember(userId) to app/actions/admin/users.ts: sets
  tenant_users.status = 'removed' for the current tenant, reuses the existing
  last-admin guard, then reconciles. This is the removal that criterion 2 needs.
• Companion fix, required for correctness: joinCurrentSchool currently rejects any
  existing membership row with "You are already a member of this school"
  (join-school.ts:31-33), which would strand a removed member permanently. Reinstate a
  non-active row through the same student-limit pre-check the insert path already runs.
• recheckPlanLimits() server action on app/actions/admin/billing.ts + a "Re-check
  limits" button in the usage section of components/admin/billing-overview.tsx,
  rendered while accessCutoffAt is set. Recovery then never depends on a scheduler.

§2 — treat the ledger write as part of delivery

• deliverCutoffStage: if (error) return { delivered: false } after logging, so
  reconcileAccessCutoff returns notifyFailed and the cron counts it under
  notifyFailures instead of notified.
• Route the schedule branch through dueCutoffNotificationStage (see premise (c)),
  which removes the double-send race for free.

§3 — decide the missing-plan-row behaviour deliberately

Preserve enforcement, do not extend it. decideAccessCutoffAction grows an optional
limitsKnown flag (default true); when the platform_plans lookup misses,
reconcileAccessCutoff passes false and the clear branch is suppressed — an
existing cutoff is left exactly as it was, while no new cutoff is ever scheduled off
unknown limits. Rationale to be written into the module comment and MONETIZATION.md:
a lookup miss is an operator error (a plan slug renamed or a row deleted), and the
sole recovery from a mistakenly-cleared cutoff is a super admin editing the row,
whereas a mistakenly-preserved cutoff is cleared by the very next successful sweep.

Docs — correct MONETIZATION.md:277 to name the call sites that actually exist,
document the new usage-side call sites and the manual re-check, and record the §3
decision. OPERATIONS_GUIDE.md is left alone per premise (b).

Risks

• removeTenantMember is new access-control surface. Mitigated by reusing
  verifyAdminAccess(), the existing last-admin guard, and a tenant-scoped filter, and
  by keeping the row (status = 'removed') rather than deleting it so history and the
  reinstate path both survive.
• The joinCurrentSchool reinstate path must not become a limit bypass — it runs the
  same max_students pre-check as a first-time join.
• §3's change means a genuinely-deleted plan row now holds a cutoff open instead of
  releasing it. That is the intended trade, and it is stated in the docs so an operator
  who hits it knows where to look.
• reconcileAccessCutoff on the archive/delete path adds two counting queries to those
  actions. Fire-and-forget after the write, with notifyDueStages left off, so no email
  latency is paid by a user-facing action (the existing rule stays intact).

Test plan

New tests/unit/access-cutoff-reconcile.test.ts, table-driven over
(violations × currentCutoffAt × notifyDueStages × sendEmail result × ledger write result),
built on tests/unit/support/fake-supabase.ts — extended with a small failWrites
option so an erroring upsert can actually be expressed (the fake has no error injection
today).

Cases: schedule-on-first-violation writes access_cutoff_at and sends scheduled;
clear-on-compliance nulls it; none leaves it alone; a failing ledger write yields
delivered: false / notifyFailed, and re-running the sweep sends the rung again but
never reports it as notified; a schedule racing an already-recorded scheduled rung
sends nothing; a missing platform_plans row leaves a live cutoff standing.

Plus npm run test:unit (370 baseline), npm run typecheck, npm run lint, and manual
QA of the archive → cutoff-clears → /access-suspended redirect loop.

[guillermoscript]

Shipped in #564 (squash-merged to master as 90c03569).

What landed

• §1 — reconcileAccessCutoffSafely is now called from every usage-reducing action: archiveCourse/deleteCourse (app/actions/teacher/courses.ts), archiveCourse/restoreCourse (app/actions/admin/courses.ts), and a new removeTenantMember (app/actions/admin/users.ts). A Re-check limits button on the admin billing page makes recovery independent of any scheduler.
• §2 — a failed access_cutoff_notifications write returns delivered: false, so it is counted under notifyFailures instead of notified. The schedule branch consults the ledger too, removing the double-send race.
• §3 — decideAccessCutoffAction grows limitsKnown: a missing platform_plans row no longer clears live enforcement.
• 17 new tests for reconcileAccessCutoff, which had none. Four fail against the pre-fix module, one per defect.

Corrections to the issue as written

1. There was no member-removal path. deactivateUser only stamps profiles.deactivated_at, which nothing outside the admin users screens reads, while the limit counts active tenant_users rows. AC 2 required building removal, not adding a call. It sets status = 'removed', and joinCurrentSchool reinstates such a row through the same student-limit pre-check.
2. docs/OPERATIONS_GUIDE.md has no access-cutoff content — line 218 is Stripe webhook setup. Only MONETIZATION.md:277 was drifted, and it is fixed.
3. AC 5 is not literally achievable, and the issue concedes it. With a ledger that cannot be written there is no state to de-duplicate against, so the rung is re-sent; the Risk note already accepts this ("costs at most one extra send next sweep"). What is guaranteed and tested: the repeat is never reported as notified, so it always surfaces as a failure rather than a healthy run.

Two defects found by browser QA that code review had missed

• archiveCourse exists twice, and the admin course-status screen calls the admin action, not the teacher one that had been patched — so the first archive clicked did nothing.
• The /join-school page gated on a membership row existing at all, so a removed member redirected there to re-join saw "You're Already a Member!" with no way forward.

Both fixed in the same PR and re-verified. Eleven flows were exercised end to end against local Supabase with no cron running, and the database restored to its seeded baseline afterwards.

No migration. This is app-layer only — tenant_users.status = 'removed' needs no schema change, since the column is free-text with no CHECK constraint. Nothing to apply to any environment.

Out of scope and still open: #513 (nothing on the Dokploy host calls /api/cron/*). This issue deliberately makes recovery work without that sweep, but the reminder ladder still needs it.