Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/cron-dispatch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ name: Cron dispatch
# is being billed or delivered; raising delivery needs a scheduler that keeps
# its word (Vercel Pro's native sub-daily crons) rather than a different cron
# string here. See scripts/cron-dispatch.mjs for the per-route shortfall.
#
# Catch-up: a tick that arrives hours late used to evaluate only its own last
# five minutes, so every sub-daily firing in the gap was lost. The tick now asks
# the Actions API for the previous successful SCHEDULED run of this workflow and
# passes its start as CRON_SINCE; the dispatcher evaluates every minute since
# then (never less than five, capped at a day, daily routes left to Vercel, one
# call per route however many of its minutes fell in the gap). If the lookup
# fails the tick still runs with its five-minute window and says so. This
# recovers the firing, not the rate β€” see the dispatcher header.
# On Vercel Pro, move the schedules back into vercel.json and disable this.

on:
Expand All @@ -35,6 +44,8 @@ on:

permissions:
contents: read
# Listing this workflow's own runs (the catch-up boundary) needs actions: read.
actions: read

concurrency:
group: cron-dispatch
Expand All @@ -61,10 +72,37 @@ jobs:
with:
node-version-file: .nvmrc

- name: Find the previous successful tick (catch-up boundary)
id: previous
env:
# The job's GITHUB_TOKEN; `gh` reads it from GH_TOKEN.
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
RUN_ID: ${{ github.run_id }}
run: |
# The previous SCHEDULED success, not any success: a manual single-route
# run dispatches one route, so taking it as the boundary would skip every
# other route due since the real previous tick. The current run is
# excluded by id. The dispatcher never narrows below its fixed window, so
# a boundary that is too recent costs nothing, and one that is too old is
# capped there. Failure here must not fail the tick: the dispatcher then
# runs its fixed five-minute window exactly as it did before catch-up.
since="$(gh api \
"repos/${REPO}/actions/workflows/cron-dispatch.yml/runs?status=success&event=schedule&per_page=5" \
--jq "[.workflow_runs[] | select(.id != ${RUN_ID})][0].run_started_at // empty" 2>/dev/null)" || since=''
if [[ "$since" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]]; then
echo "Previous successful tick started at ${since}; catching up from there."
else
echo "No usable previous tick (got '${since:0:80}'); dispatching with the fixed five-minute window."
since=''
fi
echo "since=${since}" >> "$GITHUB_OUTPUT"

- name: Validate configuration and dispatch due routes
env:
CRON_SECRET: ${{ secrets.CRON_SECRET }}
CRON_BASE_URL: ${{ vars.CRON_BASE_URL }}
CRON_SINCE: ${{ steps.previous.outputs.since }}
ROUTE: ${{ github.event.inputs.route }}
run: |
if [ -n "$ROUTE" ]; then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ No test executed the handler before this change.
| `tests/admin-digest.test.ts` | `buildAdminDigest`, `buildHeadline`, `digestSubject`, `renderAdminDigestHtml`, `summarizeDigestDelivery` | pure-function unit tests |
| `tests/admin-digest.test.ts` | The route's source text contains `readAll<DigestRow>(`, `{ max: 20_000 }`, `if (feedError)` before `buildAdminDigest(` with `{ status: 502 }`, and `summary.ok ? 200 : 502` | static source checks |
| `tests/a-mirrored-cron-must-be-idempotent.test.ts` | Lists admin-digest in `KNOWN_DOUBLE_SEND`: both schedulers name `30 12 * * *`, and the route source has no dedupe marker | static; schedules and source text |
| `tests/a-late-tick-drops-a-cron.test.ts` | The dispatcher's 5-minute look-back (mentions the admin-digest duplicate) | dispatcher only |
| `tests/a-late-tick-catches-up.test.ts` (then `a-late-tick-drops-a-cron`) | The dispatcher's 5-minute look-back (mentions the admin-digest duplicate); since 2026-10-03 the catch-up window, which leaves daily routes such as admin-digest to Vercel | dispatcher only |

**Gaps before this change:**
- Paging, the truncation β†’ 502 path and the partial-failure β†’ 502 mapping were pinned only as source text, never run.
Expand Down
30 changes: 30 additions & 0 deletions docs/final-audit/cron-dispatch-cycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,33 @@ An additional direct comparison against `c7b56eff:scripts/cron-dispatch.mjs` ver
- These tests validate orchestration boundaries with synthetic responses, not actual provider delivery, database writes, cron idempotency, or worker lease recovery.

The falsely green missing-secret dispatch defect is fixed and locally retested. Production readiness remains **NO** pending the remaining scheduler and application audit workflows.

## 2026-10-03 β€” Catch-up window (the first "remaining scheduler scope" item)

Baseline: main `1421c745`. Owned files: `scripts/cron-dispatch.mjs`, `.github/workflows/cron-dispatch.yml`, `tests/cron-dispatch.test.ts`, `tests/cron-dispatch-execution.test.ts`, `tests/a-late-tick-catches-up.test.ts` (renamed with `git mv` from `tests/a-late-tick-drops-a-cron.test.ts`, so history follows), and this record. `SCHEDULES` is byte-identical, `TICK_MINUTES` is still 5, the workflow's cron string, sparse checkout, concurrency group, timeout and repository guard are unchanged. No endpoint, secret or repository setting was touched; the Actions API call was exercised only by a local simulation of the step's shell with a fake `gh`.

### The defect

`dueRoutes` looked back a fixed `TICK_MINUTES` ending at the invocation time and stored nothing about the previous tick. GitHub starts this workflow late as a rule (median gap 209 minutes over its first 95 runs, maximum 396), so a due minute outside the last five was dropped silently. `a-late-tick-drops-a-cron` characterised it; the same file, renamed, now proves the catch-up.

### Design

- **Boundary.** A new step reads the previous successful *scheduled* run of this workflow (`gh api repos/…/actions/workflows/cron-dispatch.yml/runs?status=success&event=schedule&per_page=5`, the current run excluded by id, `run_started_at` taken) under `permissions: actions: read`, and passes it to the dispatcher as `CRON_SINCE`. Only scheduled runs count, because a manual `--route` run dispatches one route; taking it as the boundary would skip every other route due since the real previous tick. A lookup failure, an empty result or an unusable value never fails the tick: the step prints why and hands over an empty boundary, and the dispatcher runs its fixed five-minute window exactly as before, saying so in its own log line.
- **Window.** `tickWindow(now, since)` evaluates `(start, now]` in whole minutes, with `start = max(min(since, now βˆ’ TICK_MINUTES), now βˆ’ CATCH_UP_MAX_MINUTES)`. It never narrows below the fixed window, so a boundary two minutes ago (a quick pair of runs) costs nothing. A missing, unparseable or future boundary leaves the fixed window alone and is named in the log (`; CRON_SINCE ignored: not a timestamp` / `in the future`).
- **Cap, and why it clamps rather than discards.** `CATCH_UP_MAX_MINUTES = 1440`. Every route the catch-up serves fires more than once a day (that is the rule that admits it), so it has a due minute in any 24-hour window and a wider window cannot add a route β€” only minutes to scan. A boundary older than the cap is therefore *clamped* to it: a `since` from weeks ago (a workflow re-enabled after being disabled) yields a 24-hour window, which fires every sub-daily route once. It is not discarded, because discarding would drop the catch-up on exactly the longest gaps β€” the measured maximum of 396 minutes already exceeds six hours β€” and the clamp costs at most one call per sub-daily route. A test computes each sub-daily route's longest gap over a week and fails if one exceeds the cap, so a route that fires less often than daily-but-not-once-a-day cannot be added silently.
- **Once per route.** A route is called at most once per invocation however many of its minutes fell in the window: the routes are sweeps over pending work, not replays of one slot. This recovers the lost *firing*, not the *rate*; a `*/5` route still runs once per tick, so `a-cron-cadence-is-a-promise-nothing-keeps` is unchanged and still true.
- **Daily routes are not caught up.** `isSubDaily(expr)` β€” `!(minute.size === 1 && hour.size === 1)`, the negation of Vercel Hobby's rule β€” decides. Its complement is exactly the set `vercel.json` mirrors at the dispatcher's own minute (a test now pins that equivalence), and Vercel fires on time; a catch-up hours later would be a second daily run, for `admin-digest` a second email. Those routes keep the fixed window, so `admin-digest-replay-contract`'s pin (`dueRoutes` selects it at 12:30–12:34 only) holds, and the known same-minute double send is neither added to nor removed.
- **Log.** `cron-dispatch at <now> (window 187 min from <start>): …`. With no boundary the line is byte-identical to before; `--route` runs ignore the boundary.

### Tests

`tests/cron-dispatch.test.ts` (+12): the window for no / blank / unparseable / future / recent / three-hour / older-than-cap boundaries, the minute count at second-level boundaries, the cap-versus-longest-gap check over the whole table; `dueRoutes` fires a route due in the gap but outside five minutes, once however many minutes matched, leaves `notifications`, `chore-reminders` and `admin-digest` to Vercel while their fixed window is unchanged, pins `isSubDaily` ≑ `!runsAtMostDaily` and the mirror in both directions, a 48-hour boundary fires what was due inside the cap and not before it, and a bad or future boundary evaluates exactly the fixed window. `tests/cron-dispatch-execution.test.ts` (+6, real CLI subprocess with fetch replaced): the 13 routes due in (08:58, 12:05] are dispatched once each and `notifications` is not, an unusable boundary dispatches the fixed six and says so, a boundary from eleven days earlier dispatches all 17 sub-daily routes once and no daily one, a dry run with a boundary stays credential-free, and `--route` ignores the boundary; the harness also clears `CRON_SINCE` from the inherited environment. `tests/a-late-tick-catches-up.test.ts` (7): keeps the drop visible for a tick with no boundary, shows the 09:07 tick with an 08:55 boundary fires everything the 09:00 tick would have, shows a 03:00 β†’ 09:07 gap fires all 17 GitHub-only routes once and none of the daily routes that were due in it (each of which `vercel.json` fires at the same minute), and shows 73 matching minutes become one call.

Mutation checks, each reverted in turn and restored: removing the once-per-route `break` fails 4 tests; dropping the clamp fails 3; dropping the future check fails 2 (the route set is unchanged by construction, since the window never narrows below the fixed one β€” the check exists to name the bad input in the log); catching up every route fails 4. The workflow step's shell was run locally under `bash -eo pipefail` with a fake `gh` that succeeded, failed with 403, returned nothing, returned `null`, and was absent: the boundary was set in the first case and empty with exit 0 and a stated fallback in the other four.

### Still open

- The rate deficit in `DISPATCHER_DEPENDENT` is unchanged: a tick GitHub does not deliver still delivers nothing, and a `*/5` route still runs about seven times a day.
- The `admin-digest` same-minute double send (Vercel and an on-time GitHub tick both at 12:30) is unchanged.
- `run_started_at` precedes the previous script's own clock by the checkout and setup time, so adjacent windows can overlap by a minute or two; for these idempotent routes that is a possible repeat call, never a loss. A failed previous run is not a boundary either, so its window is re-covered by the next success β€” again a repeat, not a loss.
- Live Actions API behaviour, `actions: read` on the real token, and production reachability were not exercised.
Loading
Loading