[bug] A negatively-jittered cron run computes its next slot from the early fire time — "@daily" reliably fires twice in one night

[serge-ivo]

A negatively-jittered run makes the next slot minutes away, so "@daily" fires twice in one night

The mechanism

lib/triggers.ts:473, after a cron trigger runs:

next = applyJitter(nextRunAt(trigger.schedule, now, cfg.timezone), cfg.jitterMinutes);

now is the moment the sweep fired it — the jittered time, not the scheduled slot. And
applyJitter (:159) is symmetric:

const offsetMs = (Math.random() * 2 - 1) * j * 60_000;   // ±j minutes
const floor = Date.now() + 60_000;
return new Date(Math.max(Date.parse(iso) + offsetMs, floor)).toISOString();

So a run whose jitter landed negative happens before its slot, and the next slot is then computed
from that earlier moment — landing only minutes ahead instead of a day.

Concretely, with @daily and jitterMinutes: 90:

1. run fires at 23:36 (slot 00:00, jitter −24m)
2. nextRunAt("@daily", now = 23:36) → 00:00 tonight — 24 minutes away
3. jitter ±90 around that, floored at now + 60s → next ∈ [23:37, 01:30]

Every outcome of step 3 is above the floor, so it does not merely risk running again — it always
does, within about two hours. If that second run also lands before midnight the cycle repeats.

Live evidence, with a prediction you can check tonight

The one cron trigger on this account, GET /v1/triggers:

name        Daily friend sweep
schedule    @daily            jitterMinutes 90
action      run_browse        url https://www.facebook.com/friends/requests
lastRunAt   2026-08-08 00:16:13      ← slot 00:00, jitter +16m
nextRunAt   2026-08-08T23:36:00Z     ← computed from 00:16 → slot 00:00 Aug-9, jitter −24m
failureCount 0   lastError null

nextRunAt is before midnight. So when it fires at 23:36 tonight, step 2 computes the next slot
as Aug-9 00:00 — 24 minutes later — and it will browse Facebook again before ~01:30. Nothing is
broken-looking; failureCount stays 0 and no error is recorded, because each individual run
succeeds.

Why it matters more than a scheduling nicety

• The action is run_browse against a real logged-in account. "Daily" that is sometimes twice-a-night
  is the mass-action-frequency class [browser][trust] Trust & permission model for browser agents (design prerequisite) #75 cares about, arriving through a bug rather than a policy.
• It is silent and self-concealing: every run succeeds, so the only symptom is a workload the
  owner did not ask for and a lastRunAt that looks reasonable in isolation.
• Every automatic run costs BYOK tokens and a browser session on the user's machine.

The cause, stated plainly

The trigger stores one time and uses it for two different things: when to fire (jittered) and
where we are in the schedule (the slot). Jitter is presentation; the slot is the state. Deriving
the next slot from the fire time lets presentation mutate state.

Fix

Compute the next slot from the SLOT, never from now. Keep the un-jittered scheduled time
alongside the jittered one — a next_slot_at column beside next_run_at, or store the slot and
apply jitter at dispatch — so:

slot(n+1) = nextRunAt(schedule, slot(n))        // pure cron, jitter-free
fire(n+1) = applyJitter(slot(n+1), jitter)      // presentation only

Then a −24m run still fires at 23:36 and the next slot is still Aug-9 00:00, a full day later.

Cheaper stop-gap if the column is unwelcome: compute from max(now, scheduledSlot) where the slot is
recoverable from nextRunAt minus the applied offset — but that means storing the offset, which is
the column again. The column is the honest version.

Also worth deciding: whether jitter should be symmetric at all. +0..j never moves a run into
the previous period and removes this class entirely; the anti-pattern-defence argument for jitter is
served just as well by a one-sided delay.

Acceptance

• A run whose jitter is negative does not shorten the interval to the next run.
• @daily fires once per day, whatever the jitter, with a test that steps a trigger through
  several negative-jitter runs and asserts the slot advances by exactly one period each time.
• Existing rows recover on their next run rather than needing a backfill.

Files: workers/api/src/lib/triggers.ts:156-166,473, workers/api/src/routes/triggers.ts:255,316,
workers/api/src/lib/cron-time.ts (nextRunAt).

[serge-ivo]

Shipped in 9a57fad. Deployed — API worker green, migration 0100 applied.

The fix, as the ticket specified it: the slot is state, the jitter is presentation. Migration 0100 adds agent_triggers.next_slot_at, and advanceCron (workers/api/src/lib/cron-schedule.ts) is the single place the pair is computed, so the sweep and create/update cannot drift apart:

slot(n+1) = nextRunAt(schedule, slot(n))     // pure cron, jitter-free
fire(n+1) = applyJitter(slot(n+1), jitter)   // presentation only

The slot rides inside the sweep’s existing compare-and-swap claim rather than a second statement — a row can never hold a fire time from this period beside a slot from the last.

One deviation worth naming. The base is max(slot, now), not the slot alone. Advancing from the slot is the fix, but a slot far in the past — a sweep resuming after an outage — would step to another past slot, get floored to now + 60s, fire, and repeat until it caught up. That is a catch-up storm of “missed” runs against a real logged-in account, which is worse than the missed runs. Your “cheaper stop-gap” paragraph named max(now, scheduledSlot) and observed it needs the offset stored; storing the slot gives it for free, so the column and the clamp are the same fix rather than alternatives.

presentTrigger now returns nextSlotAt, because this bug was unobservable from outside: a jittered nextRunAt alone cannot be checked against a schedule — “23:36 for an @daily trigger” is either a −24m jitter or a broken interval, and the field did not say which.

Live state, and the prediction — this is why I have NOT closed it

GET /v1/triggers now serves the field, on the same row your ticket describes:

name        Daily friend sweep
schedule    @daily            jitterMinutes 90
nextRunAt   2026-08-08T23:36:00.210Z
nextSlotAt  null                        ← legacy row, has not swept since 0100

nextSlotAt: null is the no-backfill design working as intended, and it has a cost I want on the record rather than glossed: that row’s current next_run_at was written by the buggy code and is already before its slot, so it will double-fire one final time tonight. Walking the live values through the shipped advanceCron:

run 1: fires 23:36 Aug-8  ->  slot 2026-08-09T00:00Z, fire in [23:37, 01:30]   ← the last double
run 2:                    ->  slot 2026-08-10T00:00Z
run 3:                    ->  slot 2026-08-11T00:00Z
run 4:                    ->  slot 2026-08-12T00:00Z

Exactly one period per run from run 2 onward. The check: tomorrow, nextSlotAt should read 2026-08-10T00:00:00.000Z and nextRunAt should sit within ±90m of it. If it does, this can be closed; if nextSlotAt is still null the sweep is not reaching the new path.

A backfill was considered and rejected for the reason in the migration: the slot is recoverable from a jittered next_run_at only if the applied offset is also stored, and a guessed slot persists, where a missing one heals itself. (For @daily + 90m the candidate happens to be unique; for every 15 minutes + 90m it is not, and I would rather not ship a repair that is only correct for the row I can see.)

Acceptance

• A run whose jitter is negative does not shorten the interval to the next run.
• @daily fires once per day whatever the jitter — cron-advance.test.ts steps a trigger through ten consecutive negative-jitter runs and asserts the slot advances by exactly one period each time.
• Existing rows recover on their next run rather than needing a backfill — with the one-final-double caveat above.

15 tests: 11 pure (the worked example, the ten-run walk, timezone phase held across runs, legacy/NaN/empty slots, the outage case, invalid-schedule still throws so the sweep can still disable the row) and 4 sweep-level pinning the wiring — a correct advanceCron called with the wrong argument, or whose result is dropped, reproduces the original bug exactly.

Two things I did NOT do, deliberately

1. Jitter symmetry. You flagged +0..j as “worth deciding”. It is a behaviour change for every existing trigger, and with the slot column the class is harmless either way, so I left it as a decision rather than taking it quietly. Happy to have it filed.
2. jitterMinutes is never compared to the period. It is clamped at 720 and nothing checks it against the schedule, so every 15 minutes with 90 minutes of jitter is creatable. With the slot it drifts rather than storming (a test documents this explicitly), which is the better of the two — but refusing it at the edge is the real answer and belongs to the create/update grammar, not to this function.

Structural note

The schedule arithmetic (normalizeSchedule, nextRunAt, previewRuns, applyJitter, plus advanceCron) moved out of triggers.ts into lib/cron-schedule.ts. Not tidying, and not scope I went looking for: triggers.ts is a dispatch module that sat one line under its size pin, so the choice was to split or to raise the pin to keep time arithmetic in a file about what a trigger does and to whom. That could not be defended, so it moved. The file went 852 → 771, under the 800-line threshold, so its ratchet entry is deleted rather than lowered. Nothing moved changed except its address.

Verified: pnpm typecheck (11 projects), pnpm vitest run (6,271 tests, 387 files, all green), biome --error-on-warnings, design tokens, file-size ratchet, migration hygiene (101 migrations, no new collision), docs-drift, test-isolation, agents-allowlist, qa-config.

[serge-ivo]

Fix verified. The stated close condition is not yet met — it needs tonight, and nothing else.

Checked at 2026-08-08 13:22 UTC. 9a57fad + migration 0100 are on main and in the deployed API
(ancestor of b6f6c9f6, deployed 13:02 UTC).

The fix is right

lib/cron-schedule.ts:160-168 — advanceCron implements the rule the ticket asked for:

slot(n+1) = nextRunAt(schedule, base)        // pure cron, jitter-free
fire(n+1) = applyJitter(slot(n+1), jitter)   // presentation only

with base = max(slot, now), and each of the three cases argued in the docstring: slot ahead of now is
the negative-jitter bug (advance from the slot — the fix); slot behind now is the ordinary case and also
an outage resume (advance from now, so a sweep coming back does not replay every missed slot one minute
apart); no slot at all falls back to now for exactly one more run.

Migration 0100 adds next_slot_at and deliberately does no backfill, for a reason I agree with:
inventing a slot from a jittered next_run_at is a guess carrying the full ±jitter error bar, and a
wrong slot persists where a missing one heals itself.

The live row, right now

GET /v1/triggers        "Daily friend sweep"   @daily   jitterMinutes 90
  lastRunAt    2026-08-08 00:16:13
  nextRunAt    2026-08-08T23:36:00.210Z      ← still the value the BUGGY code wrote
  nextSlotAt   null                          ← pre-0100 row, no slot yet
  failureCount 0   lastError null

next_run_at is 23:36 tonight, about ten hours from this check. So neither of the two runs that
resolve this has happened. The row still carries the defect's last output, exactly as predicted.

What happens tonight, and the check to make afterwards

Deterministic, from advanceCron and this row:

1. ~23:36 UTC Aug 8 — fires. slot is NULL → base = now → next_slot_at = 2026-08-09T00:00:00.000Z,
   next_run_at = jitter(that) ∈ [23:37 Aug 8, 01:30 Aug 9]. This is the one remaining double-fire,
   and it is unavoidable: it is the pre-0100 row taking the old behaviour for its final run, which is
   precisely the no-backfill trade.
2. Within ~2h of that — fires again. Now slot is known, so base = max(slot, now) and
   next_slot_at advances by exactly one period.

Close condition — check after ~02:00 UTC on 2026-08-09:

GET /v1/triggers → "Daily friend sweep"
  nextSlotAt == "2026-08-10T00:00:00.000Z"        ← a full day, from the SLOT
  nextRunAt  ∈ [2026-08-09T22:30Z, 2026-08-10T01:30Z]

If nextSlotAt reads 2026-08-10T00:00:00.000Z, the row has healed itself with no backfill and this
closes. If it reads anything nearer, the slot is still being derived from the fire time and the ticket
is live again.

Acceptance

• A run whose jitter is negative does not shorten the interval — advanceCron, base = max(slot, now).
• @daily fires once per day whatever the jitter, with a test stepping a trigger through several
  negative-jitter runs — cron-schedule tests.
• Existing rows recover on their next run rather than needing a backfill — the mechanism is in
  place and the row is queued to exercise it; it has not yet run. This is the one that needs the
  clock.

Needs nobody — no owner action, no agent work. Re-check the trigger row after 02:00 UTC tomorrow and
close on the value above.

[serge-ivo]

Re-checked for the clock condition. It is not yet due — the close check is ~4 hours early, not failing.

Checked at 2026-08-08 21:56 UTC.

GET /v1/triggers → "Daily friend sweep"   @daily   jitterMinutes 90
  lastRunAt    2026-08-08 00:16:13
  nextRunAt    2026-08-08T23:36:00.210Z      ← still the value the BUGGY code wrote
  nextSlotAt   null                          ← pre-0100 row, no slot yet

Unchanged from the 13:22 check, and correctly so: next_run_at is 23:36 tonight, about 1h40m from this reading. Neither of the two runs that resolve this has happened.

Worth stating explicitly, because it is what nearly sent me to the wrong conclusion: the close condition in your comment says "check after ~02:00 UTC on 2026-08-09", and my session's local clock reads 2026-08-09 (UTC+10). In UTC — which is what nextRunAt/nextSlotAt are in, and what advanceCron computes against — it is still 2026-08-08. Anyone re-checking this should read date -u, not the local date, or they will read a null nextSlotAt four hours early as "the sweep is not reaching the new path" and reopen a fix that is fine.

The prediction is unchanged and still deterministic from advanceCron and this row:

1. ~23:36 UTC tonight — fires. slot is NULL → base = now → next_slot_at = 2026-08-09T00:00:00.000Z, next_run_at ∈ [23:37 Aug-8, 01:30 Aug-9]. The one remaining double-fire, unavoidable, and precisely the no-backfill trade.
2. Within ~2h of that — fires again, now with a known slot, so next_slot_at advances by exactly one period.

Close condition, unchanged — re-check after 02:00 UTC on 2026-08-09:

nextSlotAt == "2026-08-10T00:00:00.000Z"
nextRunAt  ∈ [2026-08-09T22:30Z, 2026-08-10T01:30Z]

Nothing to do and nobody to ask; it needs the clock. No code was touched.

[serge-ivo]

The bug fix is confirmed implemented and covered with tests (commit fb8feac).

What was already on main: The core fix landed before this agent session — advanceCron in lib/cron-schedule.ts, next_slot_at column in TriggerRow (migration 0100_trigger_next_slot.sql), and runDueTriggers calling advanceCron(schedule, { now, slot: trigger.next_slot_at }) instead of nextRunAt(schedule, now). The presentTrigger function also exposes nextSlotAt so the field is visible in the API response.

What this commit adds (fb8feac):

• New workers/api/src/lib/cron-schedule.test.ts with 8 tests covering:
  • The exact scenario from the issue report: slot=midnight, fire=23:36 (−24 min), asserts next slot is midnight+1 day (not the same midnight 24 minutes away)
  • 5 consecutive negative-jitter runs each advancing exactly one period
  • Positive-jitter (late fire) still advances one period
  • NULL slot self-heal (rows written before migration 0100)
  • 100-trial jitter proof: fire time is always > 22 h away from the sweep
  • @hourly and every-N-minutes variants
  • Direct proof the old code path (nextRunAt('daily', fireTime)) returned the same slot — documenting the mechanism the fix closes
• next_slot_at: null in the trigger() test-fixture builder so it matches the updated TriggerRow interface

All 44 tests pass (8 new + 36 existing). Typecheck clean.