diff --git a/.github/workflows/cron.yml b/.github/workflows/cron.yml new file mode 100644 index 00000000..63dfffb2 --- /dev/null +++ b/.github/workflows/cron.yml @@ -0,0 +1,137 @@ +# Scheduled invocation of the app's /api/cron/* routes (issue #513). +# +# Production runs on Dokploy, which does not read `vercel.json` — so the seven +# cron routes declared there had no scheduler at all and every non-payment +# lifecycle job (access-cutoff enforcement, platform-subscription expiry, daily +# digest, league rollover, payment reconciliation) was inert in production. +# +# Each route authenticates with the same `Authorization: Bearer $CRON_SECRET` +# header it already implements, so no application code changed to support this. +# +# Required repository settings (Settings → Secrets and variables → Actions): +# secret CRON_SECRET — must match the CRON_SECRET env var on the Dokploy app +# variable CRON_BASE_URL — the production origin, e.g. https://preciopana.com +# Both are checked at runtime; a missing one fails the run loudly rather than +# silently doing nothing, which is the failure mode this workflow exists to fix. +# +# Caveats worth knowing before relying on this: +# * GitHub delays scheduled runs under load, and drops high-frequency ones +# first — the */10 reconcilers are the most affected. If exact cadence +# matters for those, prefer a Dokploy scheduled task (see docs/DEPLOYMENT.md). +# * GitHub disables scheduled workflows after 60 days with no repository +# activity. Re-enable from the Actions tab if that happens. +# * Run ONE mechanism. If a Dokploy schedule is added later, remove the +# corresponding schedule here so routes don't fire twice. + +name: Scheduled cron routes + +on: + schedule: + # Keep these in sync with vercel.json and with the case block below. + - cron: '*/10 * * * *' # solana-reconcile + binance-personal-reconcile + - cron: '0 * * * *' # daily-digest — hourly by design (each tenant sends at its own local hour) + - cron: '0 0 * * *' # expire-subscriptions + - cron: '0 1 * * 1' # league-rollover (pg_cron also runs it Mondays 00:05; this is the fallback) + - cron: '0 2 * * *' # expire-platform-subscriptions + - cron: '0 3 * * *' # enforce-plan-limits + workflow_dispatch: + inputs: + route: + description: 'Cron route to run now' + required: true + type: choice + options: + - enforce-plan-limits + - expire-platform-subscriptions + - expire-subscriptions + - daily-digest + - league-rollover + - solana-reconcile + - binance-personal-reconcile + # Never scheduled: solana-pull submits on-chain USDC transfers and has + # never had a scheduler in vercel.json either. Available here for + # manual runs only — putting it on a timer needs its own review. + - solana-pull + +permissions: + contents: read + +concurrency: + # Serialize per trigger so a slow run can't overlap the next tick. + group: cron-${{ github.event.schedule || inputs.route }} + cancel-in-progress: false + +jobs: + invoke: + name: Invoke cron route(s) + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Resolve routes for this trigger + id: resolve + env: + SCHEDULE: ${{ github.event.schedule }} + DISPATCH_ROUTE: ${{ inputs.route }} + run: | + set -euo pipefail + if [ -n "${DISPATCH_ROUTE:-}" ]; then + routes="$DISPATCH_ROUTE" + else + case "${SCHEDULE:-}" in + '*/10 * * * *') routes='solana-reconcile binance-personal-reconcile' ;; + '0 * * * *') routes='daily-digest' ;; + '0 0 * * *') routes='expire-subscriptions' ;; + '0 1 * * 1') routes='league-rollover' ;; + '0 2 * * *') routes='expire-platform-subscriptions' ;; + '0 3 * * *') routes='enforce-plan-limits' ;; + *) + echo "::error::Schedule '${SCHEDULE:-}' has no route mapping. Add it to the case block in .github/workflows/cron.yml." + exit 1 + ;; + esac + fi + echo "Resolved routes: $routes" + echo "routes=$routes" >> "$GITHUB_OUTPUT" + + - name: Invoke + env: + CRON_SECRET: ${{ secrets.CRON_SECRET }} + CRON_BASE_URL: ${{ vars.CRON_BASE_URL }} + ROUTES: ${{ steps.resolve.outputs.routes }} + run: | + set -euo pipefail + + if [ -z "${CRON_BASE_URL:-}" ]; then + echo "::error::Repository variable CRON_BASE_URL is not set (expected e.g. https://preciopana.com)." + exit 1 + fi + if [ -z "${CRON_SECRET:-}" ]; then + echo "::error::Repository secret CRON_SECRET is not set. It must match the CRON_SECRET env var on the Dokploy app." + exit 1 + fi + + base="${CRON_BASE_URL%/}" + failed=0 + + for route in $ROUTES; do + url="$base/api/cron/$route" + echo "--- GET $url" + # The secret is passed via env into the header and never printed; + # GitHub also masks it in logs. Body is capped so a large digest + # response can't flood the log. + code="$(curl -sS -o /tmp/cron-body.txt -w '%{http_code}' \ + --max-time 600 --retry 2 --retry-delay 15 --retry-connrefused \ + -H "Authorization: Bearer $CRON_SECRET" \ + "$url")" || code='000' + + echo "HTTP $code" + head -c 2000 /tmp/cron-body.txt || true + echo + + if [ "$code" != '200' ]; then + echo "::error::$route returned HTTP $code" + failed=1 + fi + done + + exit "$failed" diff --git a/app/actions/join-school.ts b/app/actions/join-school.ts index 1d26e73c..87f74b74 100644 --- a/app/actions/join-school.ts +++ b/app/actions/join-school.ts @@ -6,6 +6,7 @@ import { getCurrentTenantId } from '@/lib/supabase/tenant' import { revalidatePath } from 'next/cache' import { sendEmail } from '@/lib/email/send' import { joinedSchoolTemplate } from '@/lib/email/templates/joined-school' +import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff' /** * Join the current tenant as a student @@ -109,6 +110,21 @@ export async function joinCurrentSchool() { return { success: false, error: 'Failed to join school. Please try again.' } } + // The membership row now exists, so the tenant's student count has changed — + // reconcile the access cutoff at the moment usage moves rather than waiting up + // to 24h for the nightly sweep (issue #513). The pre-flight check above blocks + // at `>=` while `computePlanLimitViolations` flags at `>`, so a legitimate join + // stops *at* the limit and normally produces no violation; this call exists to + // catch the cases where the two independent limit computations drift (the + // hardcoded 50-student fallback above vs. `platform_plans.limits`), and to + // clear a stale cutoff once a tenant drops back under its limit. + // Non-blocking: reconciliation must never fail a join that already succeeded. + try { + await reconcileAccessCutoff(adminClient, tenantId) + } catch (cutoffErr) { + console.error('Failed to reconcile access cutoff after join:', cutoffErr) + } + // Create gamification profile for this tenant (ignore if already exists) const { error: gamificationError } = await adminClient .from('gamification_profiles') diff --git a/app/actions/teacher/courses.ts b/app/actions/teacher/courses.ts index cc65e9f4..db1e84c1 100644 --- a/app/actions/teacher/courses.ts +++ b/app/actions/teacher/courses.ts @@ -6,6 +6,7 @@ import { getUserRole } from '@/lib/supabase/get-user-role' import { revalidatePath } from 'next/cache' import { sendEmail } from '@/lib/email/send' import { createAdminClient } from '@/lib/supabase/admin' +import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff' // Fallback plan limits (used if platform_plans table query fails) const PLAN_LIMITS_FALLBACK: Record = { @@ -187,6 +188,21 @@ export async function createCourse(courseData: CourseFormData) { throw new Error(`Failed to create course: ${error.message}`) } + // The course row now exists, so the tenant's course count has changed — + // reconcile the access cutoff at the moment usage moves rather than waiting up + // to 24h for the nightly sweep (issue #513). `checkCourseLimit` above blocks at + // `currentCount < limit` and counts *all* courses, while + // `computePlanLimitViolations` flags at `>` and counts only non-archived ones, + // so a legitimate creation normally produces no violation; this call exists to + // catch the cases where those two independent limit computations drift, and to + // clear a stale cutoff once a tenant drops back under its limit. + // Non-blocking: reconciliation must never fail a course that was already created. + try { + await reconcileAccessCutoff(adminClient, tenantId) + } catch (cutoffErr) { + console.error('Failed to reconcile access cutoff after course creation:', cutoffErr) + } + revalidatePath('/dashboard/teacher/courses') return course } diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index dae02a6f..28eff15f 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -258,10 +258,56 @@ After saving, **redeploy** the app. Allow 1-2 minutes for Let's Encrypt to issue ### 3.5 Cron Jobs -Since there's no Vercel cron, set up a system cron on the server: +`vercel.json` declares these schedules, but **Dokploy does not read `vercel.json`** — +something on this side has to call the routes or they never run at all. Every +`/api/cron/*` route authenticates with `Authorization: Bearer $CRON_SECRET` +(§3.3), so any scheduler that can issue an HTTP GET will do. + +**Pick exactly one of the three mechanisms below.** Running two means every route +fires twice; the routes are written to tolerate that, but it doubles the load and +makes logs hard to read. + +#### Option A — GitHub Actions (default, and what the repo ships) + +`.github/workflows/cron.yml` runs all seven schedules. It needs two repository +settings under **Settings → Secrets and variables → Actions**: + +| Kind | Name | Value | +|---|---|---| +| Secret | `CRON_SECRET` | Same value as the `CRON_SECRET` env var on the Dokploy app | +| Variable | `CRON_BASE_URL` | Production origin, e.g. `https://lmsplatform.com` | + +A missing setting fails the run loudly rather than silently no-opping. Verify it +end to end with a manual run: + +```bash +gh workflow run cron.yml -f route=enforce-plan-limits +gh run watch +``` + +Two caveats: 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 with no repository activity. If either matters +for your deployment, use Option B instead. + +#### Option B — Dokploy scheduled task + +In the Dokploy dashboard, open the LMS application → **Schedules** → create one +per line below, shell `bash`, command: + +```bash +curl -sS -f -H "Authorization: Bearer $CRON_SECRET" https://lmsplatform.com/api/cron/ +``` + +Most reliable of the three (it runs on the host, on time), but it is invisible to +the repository — nothing in code review will tell you it exists or that it broke. +If you choose this, disable the schedules in `.github/workflows/cron.yml`. + +#### Option C — host crontab ```bash # crontab -e +# Expire lapsed student subscriptions 0 0 * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/expire-subscriptions # Daily digest + streak nudge — must run HOURLY (each tenant sends at its own local hour) 0 * * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/daily-digest @@ -269,8 +315,19 @@ Since there's no Vercel cron, set up a system cron on the server: 0 1 * * 1 curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/league-rollover # Expire lapsed manual-transfer platform (school billing) subscriptions: reminders, grace, downgrade to free. Replaces the retired pg_cron job. 0 2 * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/expire-platform-subscriptions +# Reconcile access cutoffs for tenants that grew past their plan limits with no plan-change event +0 3 * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/enforce-plan-limits +# Confirm on-chain Solana payments +*/10 * * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/solana-reconcile +# Confirm Binance personal-wallet payments +*/10 * * * * curl -s -H "Authorization: Bearer YOUR_CRON_SECRET" https://lmsplatform.com/api/cron/binance-personal-reconcile ``` +> `/api/cron/solana-pull` exists but is deliberately not on any schedule here or +> in `vercel.json`. It submits on-chain USDC transfers for native Solana +> subscriptions; enabling it is a separate decision. Run it manually via +> `gh workflow run cron.yml -f route=solana-pull` if needed. + --- ## 4. How Subdomain Routing Works diff --git a/docs/OPERATIONS_GUIDE.md b/docs/OPERATIONS_GUIDE.md index 534d9bb9..81b86536 100644 --- a/docs/OPERATIONS_GUIDE.md +++ b/docs/OPERATIONS_GUIDE.md @@ -215,12 +215,35 @@ Events: checkout.session.completed, invoice.payment_failed, customer.subscriptio ``` → Copy the signing secret to `STRIPE_PLATFORM_WEBHOOK_SECRET` -### 4d. Configure Vercel Cron +### 4d. Configure the cron scheduler -The `vercel.json` file already configures a daily cron job that expires lapsed -school subscriptions. Vercel runs this automatically on Pro+ plans. +The app has seven scheduled jobs under `/api/cron/*` — student and school +subscription expiry, the daily digest, weekly league rollover, plan-limit +enforcement, and two payment reconcilers. They are declared in `vercel.json`, but +**that file only does anything on Vercel.** On any other host (this project +deploys to Dokploy) something has to call the routes or none of them ever run, +and the failures are silent: no cutoffs get scheduled, no lapsed subscriptions +expire, no digests go out. -The cron calls `/api/cron/expire-subscriptions` with your `CRON_SECRET` header. +The repo ships `.github/workflows/cron.yml` to cover this. Set two repository +settings under **Settings → Secrets and variables → Actions**: + +- Secret `CRON_SECRET` — same value as the `CRON_SECRET` env var on your host +- Variable `CRON_BASE_URL` — your production origin, e.g. `https://yourdomain.com` + +Then confirm it works end to end: + +```bash +gh workflow run cron.yml -f route=enforce-plan-limits +gh run watch +``` + +A `200` with a JSON body means the chain is wired correctly. A `401` means the +`CRON_SECRET` in Actions doesn't match the one on your host. + +If you'd rather not depend on GitHub Actions, `docs/DEPLOYMENT.md` §3.5 documents +a Dokploy scheduled task and a host crontab as alternatives — **use only one of +the three**, or every job fires more than once. --- diff --git a/tests/unit/access-cutoff-call-sites.test.ts b/tests/unit/access-cutoff-call-sites.test.ts new file mode 100644 index 00000000..392b1f4f --- /dev/null +++ b/tests/unit/access-cutoff-call-sites.test.ts @@ -0,0 +1,268 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' + +/** + * Pins the two NEW reconcileAccessCutoff() call sites added for issue #513: + * - joinCurrentSchool() (app/actions/join-school.ts) — after the tenant_users insert + * - createCourse() (app/actions/teacher/courses.ts) — after the courses insert + * + * Both call sites are wrapped in try/catch and MUST be non-blocking: a + * rejected reconcileAccessCutoff() must not fail the user's action. + */ + +interface TestState { + tenantId: string + user: { id: string; email: string; user_metadata?: Record } + role: 'student' | 'teacher' | 'admin' + // join-school + existingMembership: { id: string } | null + tenantPlan: string + planLimits: { max_students?: number; max_courses?: number } + studentCount: number + invitation: { id: string; role: string } | null + insertTenantUserError: { message: string } | null + gamificationError: { message: string } | null + metaError: { message: string } | null + // teacher/courses + courseCount: number + insertedCourse: { course_id: number } | null + insertCourseError: { message: string } | null +} + +const state: TestState = { + tenantId: 't1', + user: { id: 'user-1', email: 'student@test.com', user_metadata: { full_name: 'Test User' } }, + role: 'student', + existingMembership: null, + tenantPlan: 'free', + planLimits: { max_students: 50, max_courses: 5 }, + studentCount: 0, + invitation: null, + insertTenantUserError: null, + gamificationError: null, + metaError: null, + courseCount: 0, + insertedCourse: { course_id: 42 }, + insertCourseError: null, +} + +type Resp = { data?: unknown; error?: unknown; count?: number } + +/** + * Generic chainable + thenable query-builder mock, one instance per .from() call. + * `mode` reflects the write operation (insert/upsert/update/delete) once one has + * been called — a trailing `.select().single()` (e.g. `insert(...).select().single()`) + * must NOT reset it back to 'select', so writes only ever move mode away from + * 'select', never back. + */ +function makeBuilder(resolveFn: (mode: string, countMode: boolean) => Resp) { + let mode: 'select' | 'insert' | 'upsert' | 'update' | 'delete' = 'select' + let countMode = false + const b: Record = { + select: (_cols?: string, opts?: { count?: string; head?: boolean }) => { + countMode = !!opts?.count + return b + }, + insert: () => { + mode = 'insert' + return b + }, + upsert: () => { + mode = 'upsert' + return b + }, + update: () => { + mode = 'update' + return b + }, + delete: () => { + mode = 'delete' + return b + }, + eq: () => b, + is: () => b, + in: () => b, + order: () => b, + limit: () => b, + single: () => Promise.resolve(resolveFn(mode, countMode)), + maybeSingle: () => Promise.resolve(resolveFn(mode, countMode)), + then: (resolve: (r: Resp) => unknown, reject: (e: unknown) => unknown) => + Promise.resolve(resolveFn(mode, countMode)).then(resolve, reject), + } + return b +} + +function makeRegularClient() { + return { + auth: { + getUser: () => Promise.resolve({ data: { user: state.user }, error: null }), + updateUser: () => Promise.resolve({ data: {}, error: null }), + refreshSession: () => Promise.resolve({ data: {}, error: null }), + }, + from(table: string) { + return makeBuilder((mode, countMode) => { + if (table === 'tenant_users') { + return { + data: state.existingMembership, + error: state.existingMembership ? null : { message: 'no rows' }, + } + } + if (table === 'tenants') { + return { data: { plan: state.tenantPlan, name: 'Test School' }, error: null } + } + if (table === 'platform_plans') { + return { data: { limits: state.planLimits }, error: null } + } + if (table === 'courses') { + if (countMode) return { data: null, error: null, count: state.courseCount } + return { data: null, error: null } + } + return { data: null, error: null } + }) + }, + } +} + +function makeAdminClient() { + return { + from(table: string) { + return makeBuilder((mode, countMode) => { + if (table === 'tenants') { + return { data: { plan: state.tenantPlan, name: 'Test School' }, error: null } + } + if (table === 'platform_plans') { + return { data: { limits: state.planLimits }, error: null } + } + if (table === 'tenant_users') { + if (mode === 'insert') return { data: null, error: state.insertTenantUserError } + if (countMode) return { data: null, error: null, count: state.studentCount } + return { data: null, error: null } + } + if (table === 'tenant_invitations') { + if (mode === 'update') return { data: null, error: null } + return { + data: state.invitation, + error: state.invitation ? null : { message: 'no rows' }, + } + } + if (table === 'gamification_profiles') { + return { data: null, error: state.gamificationError } + } + if (table === 'profiles') { + return { data: null, error: null } + } + if (table === 'courses') { + if (mode === 'insert') { + return { + data: state.insertCourseError ? null : state.insertedCourse, + error: state.insertCourseError, + } + } + return { data: null, error: null } + } + return { data: null, error: null } + }) + }, + auth: { + admin: { + updateUserById: () => Promise.resolve({ error: state.metaError }), + getUserById: () => + Promise.resolve({ + data: { user: { email: state.user.email, user_metadata: state.user.user_metadata } }, + error: null, + }), + }, + }, + } +} + +vi.mock('@/lib/billing/access-cutoff', () => ({ reconcileAccessCutoff: vi.fn() })) +vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })) +vi.mock('@/lib/email/send', () => ({ sendEmail: vi.fn().mockResolvedValue(true) })) +vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(makeRegularClient()) })) +vi.mock('@/lib/supabase/admin', () => ({ createAdminClient: () => makeAdminClient() })) +vi.mock('@/lib/supabase/tenant', () => ({ + getCurrentTenantId: () => Promise.resolve(state.tenantId), + getCurrentUserId: () => Promise.resolve(state.user.id), +})) +vi.mock('@/lib/supabase/get-user-role', () => ({ getUserRole: () => Promise.resolve(state.role) })) + +import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff' +import { joinCurrentSchool } from '@/app/actions/join-school' +import { createCourse } from '@/app/actions/teacher/courses' + +const reconcileMock = vi.mocked(reconcileAccessCutoff) + +beforeEach(() => { + state.tenantId = 't1' + state.user = { id: 'user-1', email: 'student@test.com', user_metadata: { full_name: 'Test User' } } + state.role = 'student' + state.existingMembership = null + state.tenantPlan = 'free' + state.planLimits = { max_students: 50, max_courses: 5 } + state.studentCount = 0 + state.invitation = null + state.insertTenantUserError = null + state.gamificationError = null + state.metaError = null + state.courseCount = 0 + state.insertedCourse = { course_id: 42 } + state.insertCourseError = null + + reconcileMock.mockReset() + reconcileMock.mockResolvedValue({ action: 'none' }) +}) + +describe('joinCurrentSchool — reconcileAccessCutoff wiring (#513)', () => { + it('calls reconcileAccessCutoff once with the tenant id after a successful join', async () => { + const result = await joinCurrentSchool() + + expect(result).toEqual({ success: true }) + expect(reconcileMock).toHaveBeenCalledTimes(1) + expect(reconcileMock).toHaveBeenCalledWith(expect.anything(), 't1') + }) + + it('is non-blocking: a rejected reconcileAccessCutoff does not fail the join', async () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + reconcileMock.mockRejectedValue(new Error('reconcile boom')) + + const result = await joinCurrentSchool() + + expect(result).toEqual({ success: true }) + expect(reconcileMock).toHaveBeenCalledTimes(1) + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to reconcile access cutoff after join:', + expect.any(Error) + ) + + consoleErrorSpy.mockRestore() + }) +}) + +describe('createCourse — reconcileAccessCutoff wiring (#513)', () => { + it('calls reconcileAccessCutoff once with the tenant id after a successful course creation', async () => { + state.role = 'teacher' + + const result = await createCourse({ title: 'New Course' }) + + expect(result).toEqual(state.insertedCourse) + expect(reconcileMock).toHaveBeenCalledTimes(1) + expect(reconcileMock).toHaveBeenCalledWith(expect.anything(), 't1') + }) + + it('is non-blocking: a rejected reconcileAccessCutoff does not fail course creation', async () => { + state.role = 'teacher' + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + reconcileMock.mockRejectedValue(new Error('reconcile boom')) + + const result = await createCourse({ title: 'New Course' }) + + expect(result).toEqual(state.insertedCourse) + expect(reconcileMock).toHaveBeenCalledTimes(1) + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to reconcile access cutoff after course creation:', + expect.any(Error) + ) + + consoleErrorSpy.mockRestore() + }) +})