feat: Scheduled tasks (cron jobs) for recurring prompt execution#380
feat: Scheduled tasks (cron jobs) for recurring prompt execution#380
Conversation
PureWeen
force-pushed
the
copilot/add-cron-job-for-clause-tasks
branch
2 times, most recently
from
109fbaf to
01825de
Compare
PureWeen
changed the title
🤖 Multi-Model Code Review — PR #380feat: Scheduled tasks (cron jobs) for recurring prompt execution 🔴 CRITICAL — Scheduled task service never starts unless the page is visitedFile:
Impact: The core feature (background recurring execution) silently does nothing for users who don't visit the page after every app launch. Fix: Eagerly resolve the service after var app = builder.Build();
app.Services.GetRequiredService<ScheduledTaskService>();
return app;🔴 CRITICAL — Cron tasks never fire (off-by-one in time calculation)File: var candidate = new DateTime(local.Year, local.Month, local.Day,
local.Hour, local.Minute, 0).AddMinutes(1);The candidate always starts 1 minute in the future. Impact: Cron-type schedules never auto-execute. Fix: Start from the current minute rather than next minute: var candidate = new DateTime(local.Year, local.Month, local.Day,
local.Hour, local.Minute, 0, DateTimeKind.Local);Rely on 🔴 CRITICAL — Weekly schedule skips missed same-day slotsFile: if (candidateUtc <= now && i == 0) continue; // today's slot already passedIf today is a matching weekday and the scheduled time has passed, the slot is unconditionally skipped — even if the task hasn't run today. The Scenario: Weekly Mon-Fri 09:00, Impact: Weekly tasks miss ~1 day of execution per occurrence when the timer fires after the slot time. Fix: Mirror the Daily logic: check if 🔴 CRITICAL — UI edit overwrites run history (data loss race)File:
if (idx >= 0) _tasks[idx] = task; // replaces entire objectIf the background timer executed the task between Impact: Run history data loss whenever a task executes while the user has the edit form open. Fix: 🟡 MODERATE — Non-atomic file write risks total task lossFile: File.WriteAllText(TasksFilePath, json);If the app crashes or is killed during Fix: Write to a temp file, then atomic rename: var tempPath = TasksFilePath + ".tmp";
File.WriteAllText(tempPath, json);
File.Move(tempPath, TasksFilePath, overwrite: true);🟡 MODERATE — Disk I/O and serialization while holding lockFile:
Impact: UI thread freezes on Fix: Snapshot Notable Single-Model Findings (informational — did not meet 2-of-3 consensus)
📋 Summary
Recommended action: Four blockers:
Should also fix before merge: |
Review findings addressed (all from multi-model code review): 🔴 CRITICAL fixes: 1. Service eager start — ScheduledTaskService now resolved eagerly in MauiProgram.cs so the background timer starts on app launch, not just when the user visits the Scheduled Tasks page. 2. Cron off-by-one — GetNextCronTimeUtc now starts from the current minute (not +1). LastRunAt prevents re-firing within the same minute. 3. Weekly missed-day — Weekly schedule now mirrors Daily logic: only skips today's passed slot if LastRunAt shows the task already ran today. 4. Edit preserves run history — UpdateTask now merges only user-editable fields onto the canonical instance. LastRunAt and RecentRuns are never overwritten by stale clones from the edit form. 🟡 MODERATE fixes: 5. Atomic file write — SaveTasks uses write-to-temp + File.Move to prevent data loss on crash during write. 6. I/O outside lock — SaveTasks snapshots tasks under lock, then does serialization and file I/O outside the lock to prevent UI freezes. Additional hardening: - Start() checks _disposed guard to prevent zombie timers after Dispose - Per-task exception isolation in EvaluateTasksAsync foreach loop - Fixed 5 flaky timing tests in TurnEndFallbackTests using TCS pattern - Fixed minute-boundary race in CronSchedule_IsDue_ReturnsFalseWhenAlreadyRanThisMinute - Added tests: UpdateTask preserves run history, atomic write verification, Start-after-Dispose guard All 3,088 tests pass (0 failures). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
All 6 review findings addressed in commit ad10b28. See commit message for details. 3,088 tests pass, 0 failures. |
🤖 Multi-Model Code Review (Round 2) — PR #380feat: Scheduled tasks (cron jobs) for recurring prompt execution R1 Findings — Fix VerificationAll 6 findings from Round 1 are genuinely fixed with correct logic and good regression tests:
Fix details:
New Findings (Round 2)🟡 MODERATE — Shared temp file path creates save raceFile: var tempPath = TasksFilePath + ".tmp";
Fix: Use a unique temp file per call: var tempPath = TasksFilePath + $".{Guid.NewGuid():N}.tmp";🟢 MINOR —
|
| Finding | Model | Severity |
|---|---|---|
Impossible cron (0 9 31 2 *) holds _lock for ~50-500ms per eval cycle (527K iterations) |
Sonnet | 🔴 |
AddTask stores caller's reference directly — leaky abstraction |
Sonnet | 🟡 |
Start()/Dispose() race can create zombie timer (no memory barrier on _disposed) |
Sonnet | 🟡 |
run.Success = true means "prompt sent", not "AI responded" |
Opus | 🟡 |
| Cron DOM×DOW uses AND semantics; standard cron uses OR when both restricted | Codex | 🟢 |
| DST transitions may shift cron fire times (~1 misfire/year) | Opus | 🟢 |
✅ Positive Observations (all 3 models)
- Test coverage is excellent: 75+ tests covering model, CRUD, serialization, clone isolation, cron parsing, run history preservation, atomic write, and exact R1 regression scenarios
- Test isolation is proper:
SetTasksFilePathForTestinginTestSetup.csmodule initializer - Clone pattern is thorough: Deep copies
DaysOfWeek+RecentRuns;GetTasks()/GetTask()always return clones - CSS follows conventions: All
font-sizevalues usevar(--type-*)variables - Lazy property pattern:
TasksFilePathuses??=— nostatic readonlywith platform API calls - TurnEndFallbackTests improvements: Replaced fragile
Task.Delaytiming withTaskCompletionSourcesynchronization
📋 Summary
- CI:
⚠️ No checks configured for this branch - Prior comments: R1 review (6 findings) + author response (all addressed in commit
ad10b28) - R1 findings: All 6 verified fixed ✅
- New findings: 1 moderate (shared tmp path race), 1 minor (stale IsEnabled check)
- Test coverage: Strong — regression tests for all R1 fixes, clone isolation, cron edge cases
Recommended action: ✅ Approve
All 4 critical and 2 moderate R1 findings are genuinely fixed with good test coverage. The one new moderate finding (shared .tmp path) is a real race but low-probability in practice (concurrent saves within the same millisecond window) and has a trivial one-line fix that could be addressed in a follow-up. The PR is ship-ready.
Review findings addressed (all from multi-model code review): 🔴 CRITICAL fixes: 1. Service eager start — ScheduledTaskService now resolved eagerly in MauiProgram.cs so the background timer starts on app launch, not just when the user visits the Scheduled Tasks page. 2. Cron off-by-one — GetNextCronTimeUtc now starts from the current minute (not +1). LastRunAt prevents re-firing within the same minute. 3. Weekly missed-day — Weekly schedule now mirrors Daily logic: only skips today's passed slot if LastRunAt shows the task already ran today. 4. Edit preserves run history — UpdateTask now merges only user-editable fields onto the canonical instance. LastRunAt and RecentRuns are never overwritten by stale clones from the edit form. 🟡 MODERATE fixes: 5. Atomic file write — SaveTasks uses write-to-temp + File.Move to prevent data loss on crash during write. 6. I/O outside lock — SaveTasks snapshots tasks under lock, then does serialization and file I/O outside the lock to prevent UI freezes. Additional hardening: - Start() checks _disposed guard to prevent zombie timers after Dispose - Per-task exception isolation in EvaluateTasksAsync foreach loop - Fixed 5 flaky timing tests in TurnEndFallbackTests using TCS pattern - Fixed minute-boundary race in CronSchedule_IsDue_ReturnsFalseWhenAlreadyRanThisMinute - Added tests: UpdateTask preserves run history, atomic write verification, Start-after-Dispose guard All 3,088 tests pass (0 failures). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
force-pushed
the
copilot/add-cron-job-for-clause-tasks
branch
from
9179d1a to
65565a1
Compare
- RunNow: fire-and-forget ExecuteTaskAsync to avoid blocking the UI - SetEnabled: guard SaveTasks/OnTasksChanged with null check on task - IsValidTimeOfDay: reject negative TimeSpan values (add >= 0 check) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Multi-Model Code Review — PR #380 (R3)PR: feat: Scheduled tasks (cron jobs) for recurring prompt execution Review Method3 parallel sub-agent reviews: Claude Opus 4.6 (architecture, logic), Claude Sonnet 4.6 (patterns, security), GPT-5.3-Codex (edge cases). Only findings flagged by 2+ models are included. Consensus Findings🟡 MODERATE-1:
|
| # | Severity | Finding | Models |
|---|---|---|---|
| 1 | 🟡 MODERATE | SendPromptAsync from ThreadPool violates IsProcessing UI-thread invariant | Opus, Codex |
| 2 | 🟡 MODERATE | Failed runs block retry until next cycle | Opus, Sonnet |
| 3 | 🟡 MODERATE | Cron DOM+DOW AND vs standard OR (persists from R1) | All 3 |
| 4 | 🟡 MODERATE | Start/Dispose race on non-volatile _disposed | Opus, Sonnet |
| 5 | 🟡 MODERATE | No tests for badge logic | Opus, Codex |
| 6 | 🟢 MINOR | RunNow fire-and-forget swallows exceptions | Sonnet, Codex |
| 7 | 🟢 MINOR | Badge count grows unbounded | Opus, Codex |
| 8 | 🟢 MINOR | Cron search O(527K) per tick | Opus, Codex |
Prior R1/R2 Issues
- ✅ Lazy DI — fixed
- ✅ Cron off-by-one — fixed
⚠️ Cron DOM+DOW OR semantics — still present (MODERATE-3 above)- ✅ Weekly skip — fixed
- ✅ Edit data loss — fixed
- ✅ Non-atomic file write — fixed (atomic-write pattern in place)
- ✅ Disk I/O under lock — fixed (clone/snapshot pattern)
Test Coverage
The clone/snapshot pattern, TurnEndFallbackTests, and DiagnosticsLogTests are well-tested. However, badge increment/clear logic (MODERATE-5) and scheduled task edge cases (failed-run retry, DOM+DOW cron matching) lack test coverage.
Recommended Action
🔍 Multi-Model Code Review (Round 3) — PR #380feat: Scheduled tasks (cron jobs) for recurring prompt execution CI: R1/R2 Fixes — All Verified ✅
🟡 MODERATE — Cron DOM+DOW uses AND semantics instead of standard ORFlagged by: Opus ✓ · Codex ✓ (2/3) if (cron.Months.Contains(candidate.Month) &&
cron.DaysOfMonth.Contains(candidate.Day) &&
cron.DaysOfWeek.Contains((int)candidate.DayOfWeek) &&Standard POSIX cron semantics: when both day-of-month and day-of-week are non-wildcard, they should be OR'd (fire if either matches). This implementation uses AND — a cron like Fix: Track whether DOM/DOW fields were wildcards in 🟢 MINOR — Cron brute-force O(527K) iteration per task per 30s tickFlagged by: Opus ✓ · Sonnet ✓ (2/3) The loop iterates up to Impact: Low for a handful of well-formed tasks (~10ms). Becomes a CPU concern with misconfigured expressions or many tasks. Fix: Cache 🟢 MINOR — Badge count inflates for reflection-cycle sub-turnsFlagged by: Opus ✓ · Sonnet ✓ (2/3)
Fix: Only badge when Notable 1-of-3 Findings (not consensus, but worth awareness)These were flagged by only one model but have real substance:
Test Coverage
|
🔍 Multi-Model Code Review — PR #380 R5 (post-fix re-review)CI status: Re-review status of the prior blocking findings
Remaining findingsNone. This re-review did not find any new high-confidence correctness, concurrency, persistence, or lifecycle issues in the updated diff. Test coverageCoverage is now materially stronger for the risky lifecycle edges introduced by the hardening work. In particular, the suite now includes dedicated regression tests for:
Recommended action✅ Approve — the blocking scheduler issues from the prior review are addressed, the new edge cases are covered by regression tests, and the PR is in good shape to merge. |
Adds a cron-like scheduled tasks system for recurring prompt execution — daily stand-ups, periodic reviews, automated checks, etc. - Three schedule types: Interval (every N minutes), Daily (at time), Weekly (days + time) - Cron expression support: 5-field (min hour dom month dow) with wildcards, ranges, lists, and step values (e.g., '0 9 * * 1-5' for weekdays at 9am) - Full CRUD UI page at /scheduled-tasks with form validation - Task cards with toggle, edit, run-now, delete (with confirmation), and history - Expandable run history showing last 10 executions with timestamps and error details - Background timer evaluates due tasks every 30 seconds with overlap guard - Executes by sending prompt to existing session or creating a new one per run - Persists to ~/.polypilot/scheduled-tasks.json - Test isolation via SetTasksFilePathForTesting wired into TestSetup.cs - Fix Interval snap-forward: missed intervals return the last boundary, not 'now' - Fix Daily schedule: consistent local-time date comparison instead of mixed UTC/local - Fix SaveTasks race: RecordRunAndSave now holds lock through save - Fix CSS font-family enforcement: use var(--font-mono) not raw monospace - Fix .NET 10 Blazor type inference: use text input for time-of-day instead of type=time - Model: serialization, schedule descriptions, time parsing, due detection - Cron: valid/invalid expressions, ranges, steps, lists, JsonRoundTrip, next-run - Validation: IsValidTimeOfDay, ScheduleType.Cron enum, max interval - Schedule edge cases: daily never-run, daily already-ran-today, interval snap-forward - Service: persistence, CRUD, execution, error handling, corrupt file handling Fixes #367 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… canonical instance - ScheduledTask.Clone() deep-copies all fields including RecentRuns and DaysOfWeek - GetTasks() and GetTask() return clones so UI mutations cannot race with the timer - EvaluateTasksAsync collects task IDs (not direct references) before releasing lock - ExecuteTaskAsync(string taskId, ...) snapshots task data under lock, then executes async operations against the snapshot — no lock held across awaits - Convenience overload ExecuteTaskAsync(ScheduledTask, ...) delegates to ID-based version - RecordRunAndSave(string taskId, ...) looks up the canonical instance by ID under lock so stale UI clones can never corrupt the internal task's run history - 4 new tests: Clone independence, GetTasks/GetTask mutation isolation, stale-clone execution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review findings addressed (all from multi-model code review): 🔴 CRITICAL fixes: 1. Service eager start — ScheduledTaskService now resolved eagerly in MauiProgram.cs so the background timer starts on app launch, not just when the user visits the Scheduled Tasks page. 2. Cron off-by-one — GetNextCronTimeUtc now starts from the current minute (not +1). LastRunAt prevents re-firing within the same minute. 3. Weekly missed-day — Weekly schedule now mirrors Daily logic: only skips today's passed slot if LastRunAt shows the task already ran today. 4. Edit preserves run history — UpdateTask now merges only user-editable fields onto the canonical instance. LastRunAt and RecentRuns are never overwritten by stale clones from the edit form. 🟡 MODERATE fixes: 5. Atomic file write — SaveTasks uses write-to-temp + File.Move to prevent data loss on crash during write. 6. I/O outside lock — SaveTasks snapshots tasks under lock, then does serialization and file I/O outside the lock to prevent UI freezes. Additional hardening: - Start() checks _disposed guard to prevent zombie timers after Dispose - Per-task exception isolation in EvaluateTasksAsync foreach loop - Fixed 5 flaky timing tests in TurnEndFallbackTests using TCS pattern - Fixed minute-boundary race in CronSchedule_IsDue_ReturnsFalseWhenAlreadyRanThisMinute - Added tests: UpdateTask preserves run history, atomic write verification, Start-after-Dispose guard All 3,088 tests pass (0 failures). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…afety The 1024-byte threshold was too tight — when xUnit runs DiagnosticsLogTests in parallel (12 Theory cases + 4 Fact tests share one log file), concurrent writes can exceed 1KB. Bumped to 10KB which still validates the test's intent (file is nowhere near the 10MB rotation threshold). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add BadgeHelper.cs (MacCatalyst) using UNUserNotificationCenter.SetBadgeCountAsync (Mac Catalyst 16+) with UIApplication fallback for 15.x - Track _pendingCompletionCount in CopilotService; increment in CompleteResponse for non-worker, non-active sessions only - Clear badge in SwitchSession, window.Activated, and OnResume so the count resets whenever the user returns to the app Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- RunNow: fire-and-forget ExecuteTaskAsync to avoid blocking the UI - SetEnabled: guard SaveTasks/OnTasksChanged with null check on task - IsValidTimeOfDay: reject negative TimeSpan values (add >= 0 check) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Per-task in-flight guard: prevent RunNow + timer double-execution of same task - Unique temp file path per SaveTasks() call to eliminate concurrent .tmp race - Gate dock badge on reflection cycle completion (skip sub-turn increments) - Badge code refactored: read cycle once, check IsActive before both badge and reflection paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Marshal scheduled Copilot calls back to the captured UI context, preserve enable toggles from stale edit snapshots, prevent stale save writes from winning, and keep daily/weekly tasks due after failed runs. Add regression tests for the retry and stale-edit cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
force-pushed
the
copilot/add-cron-job-for-clause-tasks
branch
from
9c77e34 to
f3cd0f1
Compare
Have scheduled runs wait for the session to actually finish before recording success, with an OnSessionComplete/poll fallback and timeout. This prevents false-positive green runs when the prompt dispatch succeeds but the turn later errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prevent long-running scheduled runs from starving unrelated due tasks, fail fast when the target session closes mid-run, and add regression coverage for the new lifecycle edges. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Extends the scheduled tasks feature (PR #380) with additional integration test scenarios, UI scaffolding, and bridge support. ### Changes **Tests:** - New integration test scenarios in `scheduled-task-scenarios.json` (desktop entrypoint, target existing session, slash command, persistence after relaunch, session close disables task) - New unit tests in `ScheduledTaskTests.cs` covering the added scenarios - `ScenarioReferenceTests` updated with structural coverage for all new scenarios - `BridgePromptQueueTests` hardened with cancellation token support and increased timeout - `SlashCommandAutocompleteTests` minor fixes **UI:** - `SessionSidebar.razor` — scheduled task indicators and overflow menu link to /scheduled-tasks - `SessionSidebar.razor.css` — styles for scheduled task indicators - `Dashboard.razor` — scheduled task UI scaffolding, injected `ScheduledTaskService` **Bridge:** - `WsBridgeServer.cs` — scheduled task command handling with session busy check **Model:** - `ScheduledTask.cs` — additional model support (149 lines) **Misc:** - `DemoService.cs` — removed unused imports - `index.html` — minor update ### Testing All 3,351 tests pass ✅ --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a cron-like scheduled tasks system for recurring prompt execution — daily stand-ups, periodic reviews, automated checks, etc.
Fixes #367
Features
0 9 * * 1-5for weekdays at 9am)/scheduled-taskswith form validation~/.polypilot/scheduled-tasks.jsonBug Fixes (from original Copilot bot PR)
nowvar(--font-mono)not rawmonospaceTests (71 total)
Files Changed
PolyPilot/Models/ScheduledTask.csPolyPilot/Services/ScheduledTaskService.csPolyPilot/Components/Pages/ScheduledTasks.razorPolyPilot/Components/Pages/ScheduledTasks.razor.cssPolyPilot/Components/Layout/SessionSidebar.razorPolyPilot/MauiProgram.csPolyPilot.Tests/ScheduledTaskTests.csPolyPilot.Tests/TestSetup.csPolyPilot.Tests/PolyPilot.Tests.csproj