fix(billing): close the access-cutoff loop and stop the daily repeat email (#550)

[guillermoscript]

What

Closes #550

Fixes the three access-cutoff defects from §4.6 and adds the first coverage
reconcileAccessCutoff has ever had.

1. Nothing cleared the cutoff when a school did what the notice asked. Course
   archive, course delete and member removal now reconcile. The admin billing page
   gets an explicit Re-check limits button, so recovery never depends on a
   scheduler at all.
2. A failed ledger write no longer reports success. deliverCutoffStage
   returns delivered: false on an upsert error, and the schedule branch
   consults the ledger instead of forcing the scheduled rung.
3. A missing platform_plans row no longer clears live enforcement.

Why

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

§1 — all six reconcileAccessCutoff call sites were plan-change events, but
the cutoff email's remediation is a usage action ("20 active courses exceed the
Free plan's limit of 15"). A downgrade scheduled a cutoff synchronously; a usage
reduction cleared nothing. A complying school stayed locked out until the next
daily sweep — or indefinitely, since nothing on the Dokploy host currently calls
/api/cron/* (#513). The only recovery paths were buying a bigger plan or a super
admin editing the row.

§2 — deliverCutoffStage logged the ledger upsert error and returned
{ delivered: true } anyway. The rung stayed unrecorded, so fetchSentStages
re-derived it on the next sweep, and the cron counted every repeat under notified
and reported a healthy run. RLS on access_cutoff_notifications is enabled with
no policy, so every non-service-role caller hit this on every send. Result: every
tenant admin received the identical cutoff email daily until the cutoff cleared —
exactly the failure mode #517 was written to prevent, inverted.

§3 — a platform_plans lookup miss yielded violations = [], which
decideAccessCutoffAction read as "compliant" and used to clear a live cutoff.

Three of the issue's premises needed correcting first

• There was no member-removal path to hang a call off. The issue's scope says
  "Both already hold or can obtain an admin client", implying one exists. It does
  not. deactivateUser only stamps profiles.deactivated_at — a column grep
  finds read nowhere except the two admin users screens, purely for display —
  while countTenantUsage counts tenant_users rows with
  role = 'student' AND status = 'active'. Every other tenant_users write in the
  repo is an INSERT. So a "deactivated" student still counted against the plan
  limit and still held their membership. Acceptance criterion 2 was not "add a
  call"; it was "build the removal the call would hang off".
• docs/OPERATIONS_GUIDE.md has no access-cutoff content to correct.
  grep -n "cutoff\|enforce-plan-limits\|reconcile" returns nothing across all 732
  lines; line 218 is Stripe webhook setup. The drift is confined to
  MONETIZATION.md:277, which is fixed. That file is left untouched.
• The schedule-branch race fixed itself. On a fresh 14-day cutoff
  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.

Design decisions worth a reviewer's attention

Member removal keeps the row (status = 'removed') rather than deleting it:
it preserves joined_at, keeps FK-owned rows from cascading away, and lets
joinCurrentSchool reinstate the member. Without that reinstate path removal would
be a one-way door — the old code rejected any existing membership row with "You
are already a member of this school", stranding the user permanently. Reinstatement
runs the same max_students pre-check a first-time join runs, so it is not a
limit bypass, and a removed admin comes back as a student or teacher, never an
admin — re-joining a school is not a way to restore privilege you were stripped of.
The membership read also moved to the admin client, since a removed member's own row
may be invisible to them under RLS and a false "no membership" would fall through to
an INSERT that dies on tenant_users_unique with an opaque error.

§3's trade is deliberate and asymmetric. decideAccessCutoffAction grows an
optional limitsKnown flag. An unresolvable plan lookup now means "limits unknown",
not "limits met": scheduling stays fail-open (nothing is enforced off limits never
read), while an existing cutoff is left standing. A lookup miss is an operator error
— a renamed slug, a deleted row. Clearing on it lifts enforcement school-wide with
only a plan purchase or a super admin row edit to recover; preserving it costs
nothing, because the very next successful reconcile clears it if the school really
is compliant. This is a narrower rule than checkPlanLimits's blanket fail-open,
which only ever governs whether a new restriction is applied.

Reconciliation never fails the caller. reconcileAccessCutoffSafely wraps the
reconciler in a try/catch that logs and swallows. The archive already succeeded by
the time it runs — throwing there would punish the school for doing exactly what the
cutoff email asked. notifyDueStages stays off on these paths, so the existing rule
that user-facing actions never pay email latency is intact.

How to QA

Fresh npm run db:reset, then npm run dev. Everything below is on
default.lvh.me:3000.

A — archiving clears the cutoff synchronously (AC 1)

1. Put the school over its course limit and force a cutoff in the past, so
   enforcement is live rather than merely scheduled:

   -- docker exec -i supabase_db_lms-front psql -U postgres -d postgres
   update platform_plans set limits = '{"max_courses": 1, "max_students": 50}'
     where slug = 'free';
   update tenants set plan = 'free', access_cutoff_at = now() - interval '1 day'
     where id = '00000000-0000-0000-0000-000000000001';

2. As student@e2etest.com, open any enrolled course. You land on
   /dashboard/student/access-suspended. Leave that tab open.
3. As owner@e2etest.com, go to Teacher → Courses and archive courses until only
   one is left unarchived.
4. Reload the student tab → the student is redirected back into the course.
   No cron is running. Before this PR the student stayed suspended.
5. Confirm in SQL: select access_cutoff_at from tenants where id = '000…001'; → NULL.

B — member removal does the same (AC 2)

1. Reset: update platform_plans set limits = '{"max_courses": 100, "max_students": 1}' where slug = 'free'; and set access_cutoff_at = now() - interval '1 day' again.
2. As owner@e2etest.com, Admin → Users → (a student) → ⋯ → Remove from School.
3. access_cutoff_at is NULL and the student's tenant_users row reads
   status = 'removed'. The user vanishes from the users list.
4. Reinstate: as that student, visit /join-school and join. They come back with
   status = 'active' — and, if the school is at its student limit, they are blocked
   by the same "reached its student limit" message a new joiner sees.

C — the manual re-check (AC 3)

1. With a cutoff live, go to Admin → Billing. An amber row in the usage section
   offers Re-check limits; it renders only while a cutoff exists.
2. Still over the limit → warning toast "Still over the plan limit…". Drop under the
   limit in SQL, click again → success toast "Access restored…" and the banner clears.

D — the ladder defects (AC 4, 5) are covered by
npx vitest run tests/unit/access-cutoff-reconcile.test.ts. To see them fail against
the old code, revert the three hunks in lib/billing/access-cutoff.ts — 4 of the 17
tests fail, one per defect. That is how they were validated (see below).

Restore the seeded limits afterwards:
update platform_plans set limits = '{"max_courses": 5, "max_students": 50}' where slug = 'free';

Screenshots / GIF

Not attached. The UI surface is two small controls reusing existing components
(a Remove from School item in the admin user-actions dropdown, and an amber
Re-check limits row in the billing usage section that renders only while
accessCutoffAt is set). Both were exercised in a real browser — the verified
outcomes are recorded under Browser verification below. Happy to record a GIF
if a reviewer wants one.

Testing

reconcileAccessCutoff had zero coverage: the existing suites only reach the pure
helpers. The untested 76 lines held every stateful decision, which is how a
return { delivered: true } sitting directly beneath if (error) console.error(...)
survived review.

Adds tests/unit/access-cutoff-reconcile.test.ts — 17 table-driven cases over
(violations × currentCutoffAt × notifyDueStages × sendEmail result × ledger write
result), built on the real-rows fake-supabase.ts because every assertion is about
what a second call sees after the first one wrote (or failed to write). The fake
gains a failWrites option, since an RLS-refused write returns { data: null, error }
rather than throwing and could not be expressed at all before.

Validated by reverting each fix: against the pre-fix module, exactly 4 tests fail —
one per defect (notifyFailed accounting, the twice-run sweep, the schedule-branch
double-send, and the missing-plan-row clear) — and all 17 pass after. This is not a
suite written to match the new behaviour.

npm run test:unit   → 540 passed (44 files); 523 before this PR
npm run typecheck   → clean
npm run build       → passes
npx eslint <changed files> → 0 errors (3 pre-existing warnings, none in new code)

The issue quotes a 370-test baseline at 146a8cfa; the tree is at 523 today.

Browser verification

Driven end to end in Chromium against local Supabase on default.lvh.me:3005,
with no cron running, then the database restored to its seeded baseline
(plan limits, cutoff, course statuses and memberships all confirmed identical
afterwards). Every step below was performed through the real UI unless it says
SQL.

#	Step	Result
1	SQL: max_courses = 1, access_cutoff_at = now() - 1 day	has_course_access false for both courses
2	Student opens /dashboard/student/courses/1001	redirected to /access-suspended
3	Admin dashboard	cutoff banner: "Course access is paused for your whole school…"
4	Admin → Courses → ⋯ → Archive → confirm	access_cutoff_at → NULL, has_course_access true again
5	Student reloads /access-suspended	redirected back to /dashboard/student
6	SQL: max_courses = 0 + cutoff in past; Billing → Re-check limits	warning toast "Still over the plan limit…", cutoff preserved
7	SQL: restore limits; Re-check limits again	success toast "Access restored…", cutoff NULL
8	SQL: max_students = 0 + cutoff in past; Admin → Users → ⋯ → Remove from School	toast confirms, row → status = 'removed', active students 0, cutoff NULL
9	Removed student logs in	proxy redirects to /join-school
10	Re-join while max_students = 0	refused: "This school has reached its student limit."
11	SQL: restore limit; re-join	lands on /dashboard/student; one tenant_users row, student / active — updated in place, no duplicate

Two defects this found that code reading had missed, both fixed in the
second commit:

• archiveCourse exists twice. The admin course-status screen calls
  app/actions/admin/courses.ts, not the teacher action I had patched — so
  step 4 initially did nothing. restoreCourse (which raises the count) now
  reconciles as well.
• The /join-school page gated on a membership row existing at all, so a
  removed member redirected there to re-join was met with "You're Already a
  Member!" and no way forward — step 9 dead-ended. The server action already
  handled status; the page did not.

Both are the kind of gap unit tests structurally cannot catch: the logic was
right, the wiring pointed at a different function.

Checklist

• npm run typecheck and npm run test:unit pass
• npm run build passes
• Every new tenant-scoped query filters by tenant_id (or the table genuinely has no such column)
• Tested with every relevant role (student / teacher / admin) — in-browser, see Browser verification
• Loading and error states handled — the re-check button reports three distinct outcomes (cleared / still over limit / no cutoff) so "checked" is never mistaken for "fixed"
• New UI strings added to both messages/en.json and messages/es.json
• Migration (if any) applies cleanly on npm run db:reset — no migration; tenant_users.status is an existing free-text column with no CHECK constraint, and 'removed' needs no schema change

[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.