5 Commits

Author	SHA1	Message	Date
Bohan Jiang	a252f47337	fix(scheduler): advance autopilot next_run_at after each scheduled dispatch (MUL-3749) (#4618) ... * fix(scheduler): advance autopilot next_run_at after each scheduled dispatch The display-only autopilot_trigger.next_run_at column was written only on trigger create/update and never advanced afterward, so for a recurring schedule it froze at a past slot and the list rendered it as a 'next run' in the past (e.g. '53m ago'). The intended AdvanceTriggerNextRun query was dead code with zero callers. Wire it up at the scheduler's existing post-dispatch seam (replacing the last_fired_at-only TouchAutopilotTriggerFiredAt bump, which AdvanceTrigger- NextRun already supersets). The advanced value is computed on the app local clock via ComputeNextRun — the same path create/update use — so the whole next_run_at display column is owned by one clock and stays consistent; scheduling itself is untouched and still runs off DB time via NextOccurrencesUTC. On a cron/timezone parse failure we fall back to the last_fired_at-only bump. Adds a deterministic regression test for the reported scenario (hourly cron in America/New_York) and documents the local-clock ownership on ComputeNextRun. MUL-3749 Co-authored-by: multica-agent <github@multica.ai> * fix(scheduler): floor next_run_at advance at plan_time to survive clock skew Addresses review feedback on the next_run_at write-back (MUL-3749): - The post-dispatch advance computed the value from time.Now() alone. The handler is entered only after DB time judged the plan due, so if this app instance's clock lags the DB clock at a period boundary, time.Now() could recompute the slot that just fired and next_run_at would not advance — the original staleness bug, at the boundary. Extract advancedNextRun, which anchors at max(now, plan_time) via NextOccurrenceAfterUTC so the written value is always strictly after the fired plan_time while still tracking the local clock in the normal case. - Add scheduler-layer tests asserting the written value is strictly after plan_time across skew / on-slot / normal cases. The previous service-layer test only exercised the helper with an explicit after, not this path. - Sync the stale ListSchedulableAutopilotTriggers comment: the scheduler now writes last_fired_at via AdvanceTriggerNextRun (sqlc regenerated). MUL-3749 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>	2026-06-26 18:54:51 +08:00
Multica Eve	00b9668cd2	fix(autopilot): cold-start planner honors trigger.last_fired_at (MUL-3551) (#4495) ... Post-deploy of the new scheduled-dispatch scheduler (PR #4444), an autopilot configured for "weekdays 17:10 Asia/Shanghai" fired at ~12:30 Beijing the day after deploy — ~4h 38m before the next scheduled time the UI showed. Traced to a cold-start regression in the planner hook: Old behaviour ------------- On the first tick after migration the hook found no `sys_cron_executions` row for the trigger (`latestPlan(...).Found == false`) and anchored on the trigger's `created_at`, then applied the 24h replay cap: after := cfg.CreatedAt if oldest := now.Add(-replayWindow); after.Before(oldest) { after = oldest // now - 24h } For a trigger created days/weeks earlier and last fired by the legacy goroutine at Mon 17:10 Beijing (= Mon 09:10 UTC), this set `after = Tue 04:13 UTC - 24h ≈ Mon 04:13 UTC`. The half-open enumeration `(Mon 04:13 UTC, Tue 04:13 UTC]` STILL contained Mon 09:10 UTC — the occurrence the legacy code had already handled — so the new scheduler dispatched it again the moment it took over. The result: a SCHEDULED-source autopilot_run with planned_at = Mon 17:10 Beijing but a wall-clock dispatch at Tue ~12:30 Beijing. Timezone math was correct; the bug was purely the cold-start anchor not respecting prior-fire history. Fix Co-authored-by: multica-agent <github@multica.ai> --- The `autopilot_trigger.last_fired_at` column is maintained by both the legacy goroutine and the new scheduler (via TouchAutopilotTriggerFiredAt), so it is the authoritative "most-recent successful fire" cursor across the migration boundary. The planner hook now anchors cold-start enumeration on it: case latest.Found: after = latest.PlanTime case lastFiredAt != zero: after = lastFiredAt default: after = cfg.CreatedAt For the regressed case, `after = Mon 17:10 Beijing`, the next enumeration window is `(Mon 17:10, Tue 12:30]`, and Tue 17:10 is in the future — the hook returns nothing and the trigger waits quietly for Tue 17:10 as the UI promised. For brand-new triggers (last_fired_at NULL), the original `created_at` path still applies. For long-dormant triggers the `replayWindow` cap remains. Changes ------- * `ListSchedulableAutopilotTriggers` SQL now returns `last_fired_at`. * `autopilotTriggerConfig.LastFiredAt` is populated by the scope provider on every tick. * `autopilotPlansForScope` cold-start branch uses the new anchor. Tests ----- * TestAutopilotScheduleJobColdStartHonorsLastFiredAt — seeds the exact dev-environment shape (created 3 days ago, last_fired_at 5 hours ago, no sys_cron_executions row), runs a tick, asserts zero exec rows AND zero autopilot_run rows. Without the fix this test produces one of each at a historical plan_time. * TestAutopilotScheduleJobColdStartBrandNewTriggerStillFires — asserts a brand-new trigger (last_fired_at NULL) still fires its first due occurrence on cold start. All existing `TestAutopilotScheduleJob*` tests still pass. Refs MUL-3551 Co-authored-by: Eve <eve@multica-ai.local>	2026-06-24 13:01:59 +08:00
Multica Eve	131ca80a6c	refactor(autopilot): migrate scheduled dispatch to scheduler.Manager (3/3 MUL-3551) (#4444) ... * refactor(autopilot): migrate scheduled dispatch to scheduler.Manager PR 3 of 3 for the scheduled-Autopilot refactor on MUL-3551. Replaces the legacy cmd/server/autopilot_scheduler.go goroutine (30 s app-clock polling, app-time cron advancement, weak crash recovery) with a JobSpec registered on the existing scheduler.Manager. sys_cron_executions is now the lease + audit table for scheduled Autopilot occurrences, and the unique key on (job_name, scope_kind, scope_id, plan_time) is the primary guarantee that the same planned fire time cannot produce two runs. What changed * server/internal/scheduler/jobs_autopilot.go New AutopilotScheduleDispatchJob factory: - scope_kind = "autopilot_trigger", scope_id = trigger.id - PlansForScope hook (from PR 1) enumerates cron occurrences in (lastPlan, dbNow] and collapses missed fires to the most recent one (CatchUpLatestOnly — same policy the legacy goroutine had, now provable via a one-row-per-tick audit). - Handler re-loads trigger + autopilot inside the handler so a between-tick state change (paused, disabled, deleted) takes effect immediately and is recorded as a no-op SUCCESS row with skipped_reason in the result JSON. - Calls AutopilotService.DispatchAutopilotForPlan (from PR 2) for the actual run creation; that path is itself idempotent on (trigger_id, planned_at), so a stale-steal retry reuses the run created by the prior attempt instead of duplicating. - RunTimeout=2m, StaleTimeout=5m, HeartbeatInterval=30s, AllowStaleReentry=true, MaxAttempts=3, RetryBackoff [1m, 5m, 15m], MaxPlansPerTick=5 (safety cap). * server/internal/scheduler/manager.go Manager.runOnce promoted to RunOnce (exported) so external test packages can drive deterministic ticks; existing call sites in this package + cmd/server tests updated. * server/internal/service/cron.go NextOccurrenceAfterUTC and NextOccurrencesUTC: cron evaluators that take an explicit "now" instant. Callers pass dbNow() so schedule decisions stay consistent across app instances with clock skew. Legacy ComputeNextRun is preserved (delegating to NextOccurrenceAfterUTC with time.Now()) for the display-only autopilot_trigger.next_run_at write path — scheduling decisions no longer use it. * server/pkg/db/queries/autopilot.sql ListSchedulableAutopilotTriggers replaces the legacy ClaimDueScheduleTriggers (the new path no longer mutates autopilot_trigger.next_run_at on claim). RecoverLostTriggers removed — sys_cron_executions lease theft now handles crash recovery without an in-handler restart sweep. * server/cmd/server/main.go The "go runAutopilotScheduler(...)" line is gone. The new JobSpec is registered alongside TaskUsageHourlyJob on the existing schedulerMgr (still using sweepCtx for lifecycle). * server/cmd/server/autopilot_scheduler.go DELETED. Tests * server/internal/service/cron_test.go — unit tests for the cron helpers: timezone-aware enumeration, half-open (after, until] window, plan_time-exclusive "after", invalid inputs surface parse errors, and the "ignores wall clock" property the scheduler relies on. * server/cmd/server/autopilot_schedule_job_test.go — DB-backed integration tests: - DispatchesOnce: one tick → 1 SUCCESS exec row + 1 autopilot_run with planned_at set; a second tick does not regress the count. - MissedSchedulesCollapse: an hour of missed */5 fires produce a single autopilot_run, not 12. - CrashRecovery: simulated stale RUNNING lease at the same plan_time → second tick reclaims it and DOES NOT duplicate autopilot_run. - TwoRunnersSingleWinner: two concurrent scheduler.Manager instances on the same trigger → per-plan_time uniqueness holds (sys_cron_executions never has two RUNNING rows at the same plan_time, autopilot_run count == exec row count). - DisabledTriggerSkips: a trigger disabled between scope-list and tick produces no exec row. - PausedAutopilotSkipsAtHandler: an autopilot paused after the first tick does not produce a new exec row. - BadCronFailsLoudly: an invalid cron expression never fires dispatch (parse error surfaces in the plan hook). Existing autopilot listener / squad / dispatch tests still pass. * server/internal/scheduler/plans_for_scope_test.go from PR 1 still passes (RunOnce rename only). Verification * go build ./... * go vet ./... * go test ./internal/scheduler ./internal/service ./cmd/server ./internal/handler — all green. Rollback * Reverting this commit re-introduces the legacy goroutine. Migration 124 (PR 2) and the scheduler hook (PR 1) stay in place. Autopilot data on disk is forward- and backward- compatible: planned_at columns are nullable, the legacy goroutine never reads planned_at and the new job never reads autopilot_trigger.next_run_at. Refs MUL-3551 Co-authored-by: multica-agent <github@multica.ai> * fix(autopilot): scheduler hook retries FAILED plans + tighten tests Review fix for #4444 (MUL-3551). Blocker: hook planner skipped the FAILED-with-retry plan_time `autopilotPlansForScope` unconditionally set `after = latest.PlanTime` when `latest.Found`, then enumerated cron occurrences in the half-open interval `(after, dbNow]`. That EXCLUDED the FAILED plan_time itself, so `tryClaim`'s "FAILED-with-retry" branch — which only fires when the planner returns the same plan_time — never ran. A claim + crash sequence left the FAILED row stuck at attempt<max_attempts forever and the scheduled occurrence was lost (MUL-3551 acceptance ③). Fix: hook now branches on `latest.RetryEligible(now)` BEFORE computing `after`. When the most recent stored row is FAILED with attempts remaining and next_retry_at <= dbNow, the hook returns `[latest.PlanTime]` unchanged. tryClaim's retry-from-FAILED path fires, attempt increments, the run is retried, and the audit row reaches SUCCESS at the same plan_time. Mirrors the cadence planner's `info.RetryEligible(now)` branch in manager.plansForTick. Tests * TestAutopilotScheduleJobCrashRecovery rewritten to actually pin the retry contract instead of just "no duplicate run": - assert first attempt completes at attempt=1 with a real task_id linkage (the "complete" snapshot the retry must reuse); - simulate a crash mid-dispatch (status=RUNNING, expired stale_after, ghost lease_token); - assert tick 2 transitions the SAME exec row (same plan_time) to status=SUCCESS at attempt=2 (proving the planner did NOT skip past the FAILED bucket); - assert autopilot_run stays at exactly one row, reused from the first attempt — proving DispatchAutopilotForPlan's complete-run reuse path is what closes the loop. * TestAutopilotScheduleJobPausedAutopilotSkipsAtHandler rewritten to invoke `job.Handler` directly (the previous version drove `mgr.RunOnce` which short-circuited at the scope-list SQL filter and never reached the handler). The new test pauses the autopilot AFTER setup, calls the handler with a fabricated HandlerInput, and asserts the handler returns skipped_reason=autopilot_inactive without creating an autopilot_run. * TestAutopilotScheduleJobBadCronFailsLoudly renamed to TestAutopilotScheduleJobBadCronStaysSilent and updated to match the real implementation: a parse error in the plan hook surfaces as a manager-level warning log, NOT a sys_cron_executions row (no plan_time was ever claimed). The test now asserts zero exec rows AND zero autopilot_run rows, documenting that bad cron is a permanent configuration error (caught at HTTP create/update time first), not a transient failure that belongs in the retry envelope. Refs MUL-3551 Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai>	2026-06-24 12:09:07 +08:00
Multica Eve	f9ed62f075	refactor(scheduler): add PlansForScope hook for non-cadence jobs (#4442) ... The current Manager.plansForTick assumes a uniform Cadence grid: plan_times are derived via FloorPlan(db_now, Cadence). That works for rollup_task_usage_hourly but not for the upcoming Autopilot schedule dispatch job, where each trigger has its own cron expression and the plan_times do not snap to a single global grid. This change adds an optional JobSpec.PlansForScope hook. When set: * Manager loads the latest stored plan for (job, scope) and passes a new LatestPlanInfo to the hook (exported from the previously private latestPlanInfo). The hook returns the plan_times to attempt this tick. * Cadence, CatchUpMode and CatchUpWindow are bypassed; the hook is in full control of plan_time selection. * MaxPlansPerTick still acts as a safety cap on the hook's output. * All other timing fields (RunTimeout / StaleTimeout / HeartbeatInterval / MaxAttempts / RetryBackoff / AllowStaleReentry) and the lease/heartbeat/terminal-write SQL primitives are reused unchanged. JobSpec.validate now allows Cadence=0 when PlansForScope is set, and makes the every_plan MaxPlansPerTick > 0 invariant fire only on Cadence-driven every_plan jobs. Existing rollup_task_usage_hourly behaviour is unchanged — that JobSpec leaves PlansForScope nil. Tests: * TestJobSpecValidatePlansForScopeRelaxesCadence — validate() rules. * TestManagerPlansForScopeHookDrivesPlans — end-to-end hook delegation through the manager (DB-backed), proving that hook-returned plan_times go through the same tryClaim path, MaxPlansPerTick truncates without erroring, and LatestPlanInfo is populated on the second tick. * TestManagerPlansForScopeHookEmptyIsNoOp — empty hook output is a valid no-op. No behaviour change for callers that don't set PlansForScope. Refs MUL-3551 Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai>	2026-06-24 12:00:34 +08:00
LinYushen	3caba86b09	feat(scheduler): DB-backed execution-record scheduler [MUL-2957]	2026-06-05 13:46:26 +08:00