Give the cron Run Now safety timer a per-run token #2076

Activity

1. shanselman

   self-assigned this

   on Nov 8, 2026
2. shanselman

   added

   leverLever delivery tracking issueLever delivery tracking issue

   on Nov 8, 2026
3. github-project-automation

   added this to Windows Companion Appon Nov 8, 2026
4. github-project-automation

   moved this to Backlog in Windows Companion Appon Nov 8, 2026
5. shanselman

   added a parent issue on Nov 8, 2026

   • Fix dangerous implicit state machines (audit lanes) #2071
6. clawsweeper

   added

   clawsweeper:intake:bugPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   clawsweeper:intake:needs-environmentPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   clawsweeper:intake:needs-reproPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   P2Normal priority bug or improvement with limited blast radius.Normal priority bug or improvement with limited blast radius.

   clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.ClawSweeper found a high-confidence source-level issue reproduction.

   impact:ux-frictionUser-facing flow adds avoidable confusion or support burden without fully blocking progress.User-facing flow adds avoidable confusion or support burden without fully blocking progress.

   issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.Very strong issue quality with high-confidence source-level or clear reproduction.

   and removed

   clawsweeper:intake:bugPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   clawsweeper:intake:needs-environmentPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   clawsweeper:intake:needs-reproPreliminary submission classification; full source review pending.Preliminary submission classification; full source review pending.

   on Nov 8, 2026
7. clawsweeper commented on Nov 8, 2026

   

   clawsweeperBot

   on Nov 8, 2026 – with ClawSweeper

   More actions

   Codex review: this still needs some work.

   Summary
   Keep open: current main still permits an earlier run's safety timer to clear a newer run's state, and this collaborator-authored issue requires maintainer judgment before closure.

   Reproducibility: Source establishes the sequence: complete run A, start run B before A's 90-second timeout, then let A's callback clear B by the shared job ID; no builds, tests, or live reproduction were run.

   Next step
   Coordinate the protected audit lane with the pending page replacement, then establish a failing stale-timer regression and guard UI-thread cleanup with per-run identity.

   Review details

   Best possible solution:

   Keep run identity and timeout cleanup under UI-thread ownership so stale callbacks leave newer runs and their refresh loops untouched.

   Do we have a high-confidence way to reproduce the issue?

   Source establishes the sequence: complete run A, start run B before A's 90-second timeout, then let A's callback clear B by the shared job ID; no builds, tests, or live reproduction were run.

   Is this the best way to solve the issue?

   A per-run token checked inside the dispatched callback directly addresses the defect; moving only the HashSet lookup onto the UI thread would leave stale-run cleanup possible.

   AGENTS.md: found and applied where relevant.

   Remaining risk / open question:

   • The timing sequence is established from source but has no executed regression or visible UI proof.
   • The pending Automations replacement changes the same production owner, so repair placement needs coordination.

   Codex review notes: model internal, reasoning medium; reviewed against e70292487958.

   Label changes

   Label changes:

   • add P2: A source-proven race affects manual cron status and action gating without evidence of a blocked core workflow or failed job execution.
   • add impact:ux-friction: A newer run can lose its running indicator and refresh loop while job actions become available prematurely.
   • add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
   • add clawsweeper:source-repro: Current issue advisory state selects this label.

   Label justifications:

   • P2: A source-proven race affects manual cron status and action gating without evidence of a blocked core workflow or failed job execution.
   • impact:ux-friction: A newer run can lose its running indicator and refresh loop while job actions become available prematurely.

   Evidence reviewed

   What I checked:

   • Current default branch retains the defect: REST confirmed main equals the checkout. OnRunNowClick records only jobId; its 90-second continuation reads _runningJobIds before dispatch and subsequently clears by jobId without checking run identity. (src/OpenClaw.Tray.WinUI/Pages/CronPage.xaml.cs:238, e70292487958)
   • Completion permits the stale-timer sequence: Completion updates clear the first run before its timer expires, permitting another manual run. ClearRunningJob removes the running marker, cancels the current refresh loop, and re-enables actions. For example, run A starts at 0 seconds, completes at 10, and run B starts at 20; A's timer can clear B at 90. (src/OpenClaw.Tray.WinUI/Pages/CronPage.xaml.cs:312, e70292487958)
   • Existing coverage protects a different latch condition: CronRunningLatchTests exercise next-run advancement and Gateway running timestamps; CronPageContractTests check source patterns. Neither establishes that an expired timer cannot affect a replacement run. (tests/OpenClaw.Tray.Tests/CronRunningLatchTests.cs:5, e70292487958)
   • Discussion and overlapping work: The parent audit Fix dangerous implicit state machines (audit lanes) #2071 requests deterministic regression coverage for this lane. Cron Remove deletes on the first click and Run Now starts a disabled job #2032 concerns removal and disabled-job confirmation. Open draft feat(automations): native Automations page with web parity #1893 replaces CronPage with an AutomationsStore host, but that replacement has not landed; its existence does not establish a fix on main.
   • Feature-history ownership: RBrid's merged Improve Cron run history UX #814 added response-aware runs, refresh loops, and ClearRunningJob while carrying the existing timer forward. SebTardif's merged fix(tray): a cron job stays marked running after the gateway says it is not #1666 added the next-run latch repair. Local historical blob gaps were supplemented with REST commit patches; the timer's original introduction was not established. (src/OpenClaw.Tray.WinUI/Pages/CronPage.xaml.cs:312, 638386ed73ae)

   How this review workflow works

   • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
   • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
   • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
   • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
   • Maintainers can also comment @clawsweeper review to request a fresh review only.
   • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge; those execution lanes are retired.
   • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

   Reviewed November 8, 2026, 1:29 AM ET / 06:29 UTC.
8. shanselman commented on Nov 11, 2026

   

   shanselman

   on Nov 11, 2026

   CollaboratorAuthor

   More actions

   Hi, I'm a Copilot triage agent doing an overnight sweep (not Scott). A heads-up for whoever owns this lane, no action needed from me.

   On current main the code this issue describes is gone. The 90 s Task.Delay continuation and the _runningJobIds set in CronPage.xaml.cs were replaced when #1893 (feat(automations): native Automations page with web parity) landed. Run ownership now lives in AutomationsStore.RunNowAsync (src/OpenClaw.Shared/Cron/AutomationsStore.cs):

   • each Run Now creates a RunInvocation, and the acknowledgement and failure callbacks only act if OwnsRun(jobId, invocation) still matches
   • the run watcher, including its safety timeout, only clears the running state if OwnsWatcher(jobId, cts) still matches, so an older timer should not clear a newer run
   • AutomationsStoreConcurrencyTests already covers related ordering: LateAcknowledgementCannotRetireOrReplaceANewerInvocation and SuccessiveRunsRequireTheAcceptedRunIdDespiteStaleHistoryAndListState

   What I did not do: run the app or reproduce the old race. This is from reading main only. I also could not find a test that exercises the safety-timeout path specifically (the OwnsWatcher guard), so that might be a small follow-up if the owner wants a direct regression for the original scenario.

   If a maintainer agrees this is covered, it looks ready to close. I'm leaving that decision to you, and I'm not closing anything. Related: #2082 (fix: keep older Run Now attempts from clearing newer runs) targets the same old code and no longer applies.
9. bkudiess commented on Nov 13, 2026

   

   bkudiess

   on Nov 13, 2026

   Collaborator

   More actions

   Closing as fixed by #1893 (feat(automations): native Automations page with web parity). AutomationsStore gives each Run Now its own invocation, and the safety timeout checks that it still owns the job before clearing it, so an older attempt can no longer clear a newer run. The superseded fix in #2082 is closed; details are in #2082 (comment).
10. bkudiess

    closed this as completedon Nov 13, 2026