fix(workflow): prevent scheduled scan gaps and starvation

[bmurphy201]

Summary

• drive the scheduler with a fixed ticker and an exact window from the prior successful scan, so scan and execution time cannot create blind cron gaps
• retain the prior successful cursor after a failed or partial database scan, and coalesce a prolonged gap to the latest due cron instant
• keyset-page every enabled scheduled workflow instead of permanently truncating the global scan at 1,000 rows
• freeze each scan at its start time and add a matching partial database index so concurrent inserts cannot extend one scan indefinitely

Root cause

The loop previously slept for 60 seconds after completing each scan but looked back only 60 seconds for due cron instants. Any scan or execution work made the real interval longer than the lookup window. The global query also returned only the oldest 1,000 enabled scheduled workflows on every tick, leaving all later rows permanently unvisited.

This was observed as a hosted scheduled workflow failing to fire while manual execution remained healthy. Hosted relay logs were unavailable, so the production incident is consistent with these defects but does not prove that either was the only contributing cause.

Behavior

The durable (community_id, workflow_id, scheduled_for) claim remains the cross-pod at-most-once boundary. After an extended failed scan, the scheduler claims only the latest due cron instant rather than replaying an unbounded burst of side effects.

Validation

• env -u BUZZ_ACP_SESSION_POLICY bin/just ci
• affected package suites: buzz-db 122 passed / 253 ignored; buzz-workflow 171 passed / 2 ignored
• PostgreSQL test discovery guard passed; the new 1,002-row snapshot-bound pagination regression is included in the ignored PostgreSQL CI lane
• exact head tested: 028762d6ac33bd52a8fd49b96b31d536b738576e

Related work

• Fixes Scheduled (cron) workflows never fire on a hosted multi-tenant relay; manual trigger works #7065
• refreshes the current-main implementation path for the pagination work in draft Fix workflow scheduler pagination #5092
• overlaps the delayed-tick concern in fix(workflows): tolerate delayed cron ticks #3884
• fix(workflow): catch up missed cron fires #7506 proposes stronger persistent per-workflow catch-up across process restarts; this change is compatible in intent but deliberately preserves latest-only recovery semantics

AI-assisted implementation, reviewed and tested against current main before submission.

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is a61239f0d8036aff58176f5c0ce7f080c66e21b7...858acc8ff322d908502f90abb20663f047b5fb03.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 858acc8ff322d908502f90abb20663f047b5fb03 to authorize a new review.
Any previous review applies only to its recorded range.

[bmurphy201]

Maintainers: this fixes two production scheduler gaps we reproduced in a hosted Buzz workflow: elapsed scan time can leave cron blind spots, and the enabled-schedule query stops after 1,000 rows. Bill Murphy has authorized merge and deployment for our affected workspace. The exact PR head passed the full local just ci gate, plus the database and workflow regression suites. GitHub reports the PR mergeable, DCO, Semgrep, and zizmor passing. Please review and merge when the repository gates are satisfied; our account cannot merge or enable auto-merge under the base policy.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed head 028762d6ac33bd52a8fd49b96b31d536b738576e. Changes requested for the two scheduler regressions below. The keyset pagination and elapsed cron window are the right repair boundary; this does not call for durable replay or a workflow-engine redesign.

P2: Fixed-rate ticks can skip an otherwise due interval

The new fixed ticker at crates/buzz-workflow/src/lib.rs:495–505 is incompatible with the unchanged elapsed-wall-clock interval prefilter. Successful/duplicate interval claims save the actual tick’s now (:673,754), and interval_should_fire requires (now - last).num_seconds() >= interval_secs (:916–919). A fixed ticker does not guarantee at least 60 seconds between actual wake-ups.

Concrete one-minute case: the prior fire is at 09:00:00.120; the next tick wakes at 09:01:00.020. The elapsed duration is 59.900 seconds, so the prefilter rejects it even though the scheduler entered the next one-minute claim bucket. It does not fire until 09:02, and varying wake-up latency can repeat this skip during ordinary operation. The old sleep-after-processing loop guaranteed at least 60 seconds between scans and did not introduce this condition. Longer intervals aligned to the tick can also acquire an extra minute of delay.

Please make interval eligibility compatible with the chosen cadence while preserving cold-start suppression, restart behavior, and the durable cross-pod claim boundary. The smallest safe alternative is to retain the previous sleep-after-scan cadence while keeping the new elapsed cron window and pagination: those two changes already address the reported defects without changing interval timing. If retaining fixed-rate ticks, add a regression through the production interval scheduling path with decreasing wake-up latency (including the timestamps above). Do not hide it with an arbitrary tolerance or change the delivery contract implicitly.

P2: Later pages can fire a definition saved after the scan boundary

list_enabled_schedule_workflows_page runs each page independently and fences only created_at (crates/buzz-db/src/store/workflow.rs:500–523). Updating an existing workflow preserves that field while replacing definition and setting updated_at = NOW() (:354–365). The engine nevertheless evaluates every page against the single earlier now (crates/buzz-workflow/src/lib.rs:505,512,572–576).

Concrete case: scan window (08:59:30,09:00:30]; page 1 takes time to process. At 09:01:00, change an old workflow ahead of the cursor from an event trigger to a daily 0 9 * * * schedule. A subsequent page sees the new definition, passes the old created_at bound, and claims 09:00:00: the new schedule executes for a time before it was saved, even though it was not part of the first page's query snapshot. The old single-query scan could not admit that post-query edit into the current pass; its next 60-second window would exclude 09:00. This can produce an unexpected immediate side effect instead of waiting for the next daily occurrence.

Please fence post-boundary definition/eligibility updates out of the current pass (or provide equivalent snapshot consistency), and add a regression editing an old row ahead of the cursor during page processing. Preserve the bounded-page design; this does not require a general replay/versioning system.

Required integration and validation updates

• Rebase carefully: current main 779af8886caae1317b4de962082429867ab61503 already has migrations 0045_retain_push_revocation_tombstones.sql and 0046_storage_accounting_snapshots.sql. Renumber this PR’s index migration to the next free version and reconcile migration assertions without dropping either existing migration. The current PR is merge-conflicting.
• Move the old cron boundary/invalid-expression tests off the newly test-only cron_fire_instant (:842–863) onto scheduler_cron_fire_instant, retaining (previous_scan_at, now] boundary checks. Add coverage binding the production paginated scan and failure/watermark behavior, not just independent calls to the page query and cron helper.
• Obtain green CI on the updated head, including the discovered PostgreSQL regression. At my nonblocking snapshot the Rust jobs were still running and Security had failed; that is not a green validation result. No broad suites were rerun locally.

Evidence: complete changed-path/source tracing at the head above, dependency inspection (cron 0.16.0 and chrono 0.4.45), and a source-bound arithmetic reproduction of the interval decision. No production relay or naturally firing public canary was exercised; this review does not establish the live incident’s root cause.

[wesbillman]

Carl, an automated contributor, commenting via Wes’s GitHub account.

Wes asked me to repair the review findings on this PR. I’m preparing the narrow scheduler/test corrections and reconciling the migration with current main, preserving the existing pagination and claim design. Current head is 028762d; I’ll recheck it before updating this branch to avoid overwriting concurrent edits. Please flag any repair already in flight.

[baxen]

LGTM. The elapsed scan window and stable keyset pagination address the timing gaps and starvation cleanly while preserving the existing authorization and durable-claim safeguards. Two non-blocking testing nits inline.

[baxen]

Nit (non-blocking): could we port the existing cron boundary/invalid-expression cases to scheduler_cron_fire_instant and remove this old helper? Keeping cron_fire_instant under #[cfg(test)] means those tests still exercise the obsolete implementation rather than the production path.

[baxen]

Nit (non-blocking): would be good to add an orchestration regression where page 1 succeeds, page 2 fails, and the next scan recovers. Assert that previous_scan_at stays put after the partial scan, interval-cache entries are not prematurely pruned, and already-claimed occurrences are not duplicated. The helper tests cover date arithmetic but not this control flow. For the pagination fixture, identical creation timestamps across communities would also exercise the cursor tie-breakers.