fix(scheduler): dedicated context for post-fire UPDATE — F1089 follow-up to #1241 - #1244
Conversation
The post-fire UPDATE after s.proxy.ProxyA2ARequest() was using fireCtx, which derives from the outer ctx passed into fireSchedule(). If that ctx is cancelled — HTTP timeout, graceful shutdown, or any upstream deadline — ExecContext returns context.Canceled and the UPDATE is silently skipped, leaving next_run_at stale and causing the schedule to re-fire on the next tick. Fix: create a dedicated updateCtx from context.Background() with a 5s deadline, independent of the outer ctx hierarchy. Also improved the error log to include schedule name for easier debugging. Complements PR #1241 (fix/f1089-scheduler-ctx-fix-main) which fixes the goroutine-panic path in tick() — this fix covers the wider case of normal-return + ctx-cancelled after the proxy call. F1089 | Severity: HIGH+security Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review: #1244 — fix(scheduler): dedicated context for post-fire UPDATE — F1089 follow-up to #1241
Quality: HIGH — best F1089 PR yet. LGTM, recommend merge.
Fix analysis ✅
The context.WithTimeout(context.Background(), 5*time.Second) pattern is the correct approach for the normal-return path:
- Dedicated child context:
context.Background()parent + 5s timeout = fully independent of outerfireCtxcancellation chain - 5s deadline is right: DB UPDATE is fast (<100ms typically). 5s gives headroom for DB pressure without leaving an orphaned query if the DB is unresponsive
- Clean defer:
updateCancel()is called after ExecContext returns — correct defer ordering
Improvement over prior F1089 PRs
This is the most complete F1089 fix:
- PR #1211:
context.Background()in panic-recovery defer (goroutine panic path) - PR #1241: error logging on panic-recovery UPDATE
- PR #1244: dedicated context + test for the normal-return path
The normal-return path (fireSchedule completing successfully, then UPDATE) is the wider attack surface — most executions complete normally but may have their ctx cancelled by an upstream timeout. This PR addresses it definitively.
Test quality ✅
TestFireSchedule_OuterCtxCancelled_UpdateStillFires — injects a cancelled ctx to fireSchedule, asserts the mock DB receives the UPDATE on updateCtx. Proves the dedicated context is used and the outer ctx cancellation doesn't affect it. This is the right test.
Note on PR #1241 relationship
PR #1241 and this PR are complementary and both should merge. Together they cover:
- Panic path:
context.Background()in panic-recovery defer (#1241) - Normal-return path:
context.WithTimeout(context.Background(), 5s)for post-fire UPDATE (this PR)
Overall: Clean, well-commented, well-tested. This is the definitive F1089 fix. Recommend merge.
…#1244) The post-fire UPDATE after s.proxy.ProxyA2ARequest() was using fireCtx, which derives from the outer ctx passed into fireSchedule(). If that ctx is cancelled — HTTP timeout, graceful shutdown, or any upstream deadline — ExecContext returns context.Canceled and the UPDATE is silently skipped, leaving next_run_at stale and causing the schedule to re-fire on the next tick. Fix: create a dedicated updateCtx from context.Background() with a 5s deadline, independent of the outer ctx hierarchy. Also improved the error log to include schedule name for easier debugging. Complements PR #1241 (fix/f1089-scheduler-ctx-fix-main) which fixes the goroutine-panic path in tick() — this fix covers the wider case of normal-return + ctx-cancelled after the proxy call. F1089 | Severity: HIGH+security Co-authored-by: Molecule AI Infra Lead <infra-lead@agents.moleculesai.app> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Fixes F1089: the post-fire
UPDATE workspace_schedulesinfireSchedule()was usingfireCtx(derived from the outerctxpassed intofireSchedule). If that ctx iscancelled — HTTP timeout, graceful shutdown, or any upstream deadline — the UPDATE
returns
context.Canceledand is silently skipped, leavingnext_run_atstale.A stale
next_run_atmeans the schedule is immediately re-fired on the next tick,causing duplicate executions.
Fix
Replaced
ExecContext(ctx, ...)with a dedicatedupdateCtxfromcontext.Background()+ 5s deadline. Independent of the outer ctx hierarchy socancellations cannot silently swallow the UPDATE.
Also improved error log to include
sched.Namefor easier operator debugging.Relationship to PR #1241
PR #1241 (
fix/f1089-scheduler-ctx-fix-main) fixes the goroutine-panic path intick()— the deferred recover that catches panics fromfireSchedule. This PRcovers the normal-return + ctx-cancelled path in
fireScheduleitself, which isa wider attack surface. Both fixes are needed for complete F1089 coverage.
Test
Added
TestFireSchedule_OuterCtxCancelled_UpdateStillFiresinscheduler_test.go: passes a cancelled ctx tofireSchedule, asserts the mock DBsatisfies all UPDATE expectations, proving the dedicated context is used.
F1089 | HIGH+security
🤖 Generated with Claude Code