Skip to content

fix(ops): schedule /api/cron/* from GitHub Actions and reconcile cutoff on usage change (#513) - #519

Draft
guillermoscript wants to merge 1 commit into
masterfrom
fix/513-cron-scheduling
Draft

fix(ops): schedule /api/cron/* from GitHub Actions and reconcile cutoff on usage change (#513)#519
guillermoscript wants to merge 1 commit into
masterfrom
fix/513-cron-scheduling

Conversation

@guillermoscript

Copy link
Copy Markdown
Owner

Closes #513.

What was actually wrong

I checked the premise against production before writing any code. Querying the Dokploy API for schedules on the LMS app (schedule.list, wTNg0pha2N1VUYgzeB8-a), on the MCP app, and at dokploy-server scope all return [], and server.all is empty so there is no separate host holding a crontab either.

There is no scheduler in production. All seven /api/cron/* routes have never run there.

That is broader than the access-cutoff symptom the issue was filed for. tenants.access_cutoff_at is only written by reconcileAccessCutoff(), and five of its six call sites hang off a plan-change event; the sixth — enforce-plan-limits — is the only path that fires for a tenant that grows over its limits organically, and it's declared exclusively in vercel.json, which Dokploy does not read. The same silence applies to platform-subscription expiry (#462), the daily digest, league rollover, and both payment reconcilers.

Two corrections to the issue's evidence, both of which changed the shape of the fix:

  1. docs/DEPLOYMENT.md was not silent on scheduling. It already had a ### 3.5 Cron Jobs section with a host-crontab example — but listing only 4 of the 8 routes, and nobody ever ran it. So this was a "complete it and make it executable" job, not a "write it from scratch" job.
  2. There are 8 route directories under app/api/cron/ but only 7 entries in vercel.json. solana-pull has never been scheduled anywhere despite its own docstring saying it should run hourly. See "Deliberately out of scope" below.

Changes

1. .github/workflows/cron.yml (new) — schedules all seven routes, one on.schedule entry per distinct cadence already declared in vercel.json, with a case block mapping github.event.schedule to the route(s) that tick belongs to. Auth reuses the Authorization: Bearer $CRON_SECRET header every route already implements, so no application code changed to support this. A workflow_dispatch route picker makes any single job runnable on demand — that's what makes this verifiable at review time rather than something you find out about in 24 hours.

I chose GitHub Actions over a Dokploy scheduled task deliberately: it lives in the repo, it's reviewable in this PR, and a reviewer can verify it without prod console access. Its weaknesses are real and are documented in the file's header comment and in DEPLOYMENT.md — GitHub delays scheduled runs under load (dropping high-frequency ones first, so the */10 reconcilers are the most affected), and it disables scheduled workflows after 60 days of repository inactivity. Neither is disqualifying for daily billing sweeps on an active repo, but the operator now has the Dokploy scheduled task documented as the belt-and-braces alternative.

vercel.json is left as-is — harmless, and removing it would strand the project if it's ever deployed to Vercel.

2. reconcileAccessCutoff() at the two usage-crossing sitesapp/actions/join-school.ts after the tenant_users insert succeeds, and app/actions/teacher/courses.ts after the courses insert succeeds. Both already hold an admin client, which is what the function needs. Both calls are wrapped in try/catch that logs and swallows: reconciliation (or an email failure inside it) must never fail a join or a course creation that already succeeded. The function is idempotent by design — it only schedules when violations.length > 0 && !currentCutoffAt — so re-entry from the cron is safe.

Being straight about what this second change buys, because the issue slightly overstates it: it is defense-in-depth, not a live bug fix. computePlanLimitViolations compares with strict >, while both pre-flight checks block at >= (join-school.ts at count >= maxStudents, checkCourseLimit at currentCount < limit). A legitimate join or course creation therefore stops at the limit and cannot produce a violation. The value is in the drift between the duplicated checks — join-school.ts falls back to a hardcoded 50 when the platform_plans row is missing or inactive, and checkCourseLimit counts all courses while countTenantUsage counts only non-archived ones. Where those disagree a crossing can slip past the pre-flight, and this catches it at the moment it happens instead of up to 24h later. It also clears a stale cutoff the moment a tenant drops back under its limit.

3. Docsdocs/OPERATIONS_GUIDE.md §4d was telling the operator to "Configure Vercel Cron" and named only expire-subscriptions; it now describes the actual scheduler, its two required repo settings, and how to verify it. docs/DEPLOYMENT.md §3.5 now presents three mutually-exclusive options (Actions / Dokploy scheduled task / host crontab) with all seven routes listed in each, and states plainly that running two of them means every job fires twice.

Required before this does anything

This PR does not become effective on merge. Two repository settings must be added under Settings → Secrets and variables → Actions:

Kind Name Value
Secret CRON_SECRET Must match the CRON_SECRET env var already set on the Dokploy app
Variable CRON_BASE_URL https://preciopana.com

The workflow fails loudly if either is missing rather than silently no-opping, which is precisely the failure mode being fixed here. A 401 in the run log means the two CRON_SECRET values don't match.

Testing

  • npm run test:unit287/287 passing across 27 files (283 before this branch, plus 4 new). New tests/unit/access-cutoff-call-sites.test.ts covers both call sites: that reconcileAccessCutoff is called exactly once with the tenant id, and that a rejecting reconcileAccessCutoff still lets joinCurrentSchool() return { success: true } and createCourse() resolve with the course.
  • Mutation-tested, so those four aren't vacuous. I stripped the reconcileAccessCutoff call from both call sites and re-ran: all 4 fail (Test Files 1 failed, Tests 4 failed). Restored and re-verified the tree is clean.
  • npx tsc --noEmit — no errors in any changed file.
  • npx eslint app/actions/join-school.ts app/actions/teacher/courses.ts — clean.
  • npm run build — passes.
  • Workflow YAML parsed with a strict parser (6 unique schedules, no duplicates, all 7 vercel.json routes covered, 8 dispatch options). The case block was extracted and executed against all six schedule strings plus one unmapped string, confirming each maps to the right route and that an unmapped schedule exits non-zero instead of silently doing nothing. actionlint is not installed locally, so the workflow has not been through it — worth a glance from the reviewer.

QA script — the one check that proves this works

Everything above verifies the code; only this verifies the mechanism, and it can only run after the two settings above are added and this is merged to master (GitHub only offers workflow_dispatch for workflows present on the default branch):

gh workflow run cron.yml -f route=enforce-plan-limits
gh run watch

Expect HTTP 200 and a body like {"success":true,"scheduled":0,"cleared":0,"none":N,"errors":0} where N is your tenant count. 401 → the CRON_SECRET values don't match. Any other code → the route or the base URL is wrong.

Then confirm the schedules actually fire: check the Actions tab ~24h later for a green run on the 0 3 * * * tick.

Deliberately out of scope

Both worth filing as follow-ups rather than smuggling in here:

  • solana-pull is still not scheduled. It submits on-chain USDC transfers for native Solana subscriptions and has never had a scheduler in vercel.json either. Putting a money-moving job on a timer deserves its own reviewed change, so it's available under workflow_dispatch only and documented as intentionally unscheduled.
  • Three separate plan-limit implementations. lib/billing/plan-limits.ts, join-school.ts's inline check, and checkCourseLimit() each compute limits independently with different fallbacks, different comparison operators, and different course-count semantics. Consolidating them onto lib/billing/plan-limits.ts is the real fix for the drift that change Add paypal as a payment method #2 above merely compensates for.

No UI surface changed, so there are no screenshots.

…ff on usage change (#513)

Production runs on Dokploy, which does not read `vercel.json`. Querying the
Dokploy API for schedules on the LMS app, the MCP app, and at server scope all
return empty, and there is no separate host holding a crontab — so none of the
seven `/api/cron/*` routes have ever run in production. Access-cutoff
enforcement (#494), platform-subscription expiry (#462), the daily digest,
league rollover and both payment reconcilers were all inert.

Add `.github/workflows/cron.yml`, which schedules all seven routes using the
`Authorization: Bearer $CRON_SECRET` header each route already implements, one
schedule entry per cadence declared in `vercel.json`, plus a `workflow_dispatch`
route picker for on-demand runs. It requires a `CRON_SECRET` repository secret
and a `CRON_BASE_URL` repository variable, and fails loudly when either is
missing rather than silently no-opping. `vercel.json` is left in place.

Also call `reconcileAccessCutoff()` from `joinCurrentSchool()` and
`createCourse()` immediately after their inserts succeed, wrapped in try/catch
so a reconciliation or email failure can never fail a user action that already
succeeded. This is defense-in-depth rather than a live bug fix: both pre-flight
checks block at `>=` while `computePlanLimitViolations` flags at `>`, so a
legitimate join or course creation stops at the limit. The value is in catching
drift between the three independent limit implementations, and in clearing a
stale cutoff as soon as a tenant drops back under its limit.

Docs: `OPERATIONS_GUIDE.md` no longer tells the operator to configure Vercel
Cron, and `DEPLOYMENT.md` §3.5 now lists all seven routes across three
mutually-exclusive scheduling options instead of four routes in one.

`solana-pull` remains deliberately unscheduled — it submits on-chain USDC
transfers and belongs in its own reviewed change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xmiu65sTJVVgFjdn9xRKK2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working payments Payment provider / billing related severity:high High severity security finding Sub-task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Access-cutoff reconciliation depends on a cron that may never run on Dokploy (#494 follow-up)

1 participant