Skip to content

fix(update): guard Windows task autostart during package updates#88292

Closed
vincentkoc wants to merge 2 commits into
mainfrom
fix/windows-update-task-autostart
Closed

fix(update): guard Windows task autostart during package updates#88292
vincentkoc wants to merge 2 commits into
mainfrom
fix/windows-update-task-autostart

Conversation

@vincentkoc

@vincentkoc vincentkoc commented May 30, 2026

Copy link
Copy Markdown
Member

Summary

  • disables enabled Windows Scheduled Task autostart while a managed package update replaces the OpenClaw package tree
  • covers running, stopped, and unclassified installed task runtimes, including --no-restart stopped/unknown cases where the task can still logon-start during replacement
  • re-enables only tasks this update actually disabled, before post-core handoff, restart handoff, blocked-update exits, or failed-update exits
  • treats omitted <Settings><Enabled> in live Task Scheduler XML as enabled, matching native Windows output where enabled tasks are represented by absence of an explicit disabled state
  • reports failed /ENABLE recovery if service stop fails after autostart was suspended, so the operator is not left with silently disabled logon autostart
  • leaves startup-folder fallback installs, unavailable schtasks, missing tasks, unreadable XML state, and already-disabled tasks unchanged

Fixes #87993.

Verification

  • node scripts/run-vitest.mjs src/daemon/schtasks.stop.test.ts src/cli/update-cli.test.ts -- --run - 147 passed
  • node_modules/.bin/oxfmt --check src/daemon/schtasks.ts src/daemon/schtasks.stop.test.ts src/cli/update-cli/update-command.ts src/cli/update-cli.test.ts
  • node_modules/.bin/oxlint src/daemon/schtasks.ts src/daemon/schtasks.stop.test.ts src/cli/update-cli/update-command.ts src/cli/update-cli.test.ts
  • git diff --check origin/main...HEAD && git diff --check
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main - clean, patch correct
  • AWS Crabbox changed gate on rebased branch: run_80f36e0c133e, pnpm check:changed passed
  • AWS Crabbox native Windows Task Scheduler XML proof: run_16c0e77f1c9a, scheduled_task_disable_enable_xml_live=ok
    • seeded a legacy-style LogonTrigger task with PT3M / P9999D
    • observed enabled live XML represented by absence of <Settings><Enabled>false</Enabled>
    • verified /DISABLE writes <Settings><Enabled>false</Enabled>
    • verified /ENABLE removes the explicit disabled state again
    • verified PT3M/P9999D repetition metadata survives disable/enable
  • AWS Crabbox native Windows disabled-run probe: run_373d5c3d81fc observed ERROR: The scheduled task "OCProof..." could not run because it is disabled. The script intentionally failed because PowerShell promoted the expected stderr into a native-command error.

Real behavior proof

Behavior addressed: Windows package updates no longer leave an enabled Scheduled Task free to relaunch the gateway while the package tree is being replaced. If the task was suspended and service stop then fails, failed /ENABLE recovery is now surfaced instead of hidden.

Real environment tested: Focused local regression tests on current branch SHA da348c17f40; AWS Crabbox Linux changed gate; AWS Crabbox native Windows Task Scheduler with a seeded legacy-style PT3M/P9999D task.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/daemon/schtasks.stop.test.ts src/cli/update-cli.test.ts -- --run
node_modules/.bin/oxfmt --check src/daemon/schtasks.ts src/daemon/schtasks.stop.test.ts src/cli/update-cli/update-command.ts src/cli/update-cli.test.ts
node_modules/.bin/oxlint src/daemon/schtasks.ts src/daemon/schtasks.stop.test.ts src/cli/update-cli/update-command.ts src/cli/update-cli.test.ts
git diff --check origin/main...HEAD && git diff --check
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
node scripts/crabbox-wrapper.mjs run --provider aws --idle-timeout 90m --ttl 240m --timing-json --shell -- "pnpm check:changed"

Evidence after fix: Tests assert /DISABLE happens before service stop/package install and /ENABLE happens after install, before post-core fresh-process handoff. Tests also cover stopped and unknown installed task runtimes, --no-restart stopped/unknown runtimes, already-disabled task preservation, startup-fallback/no-schtasks no-op, Settings-level XML parsing, live-style omitted Settings Enabled defaults, NUL-separated XML output, and failed /ENABLE recovery after a service-stop failure. Native Windows proof showed Task Scheduler omits an explicit enabled setting for enabled tasks, writes explicit disabled state after /DISABLE, removes it after /ENABLE, and preserves legacy PT3M/P9999D repetition metadata.

Observed result after fix: package-update code suspends only a currently enabled Scheduled Task, resumes only if this update suspended it, and surfaces recovery failure if stop fails after suspension.

What was not tested: full native Windows OpenClaw package-update smoke remains unproven because the available AWS native Windows Crabbox image has schtasks but no Node/npm. WSL2 is not a touched runtime surface for this PR because WSL does not use the Windows Scheduled Task gateway service path.

@vincentkoc vincentkoc self-assigned this May 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime cli CLI command changes size: M maintainer Maintainer-authored PR labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 4:40 PM ET / 20:40 UTC.

Summary
The branch adds Windows Scheduled Task suspend/resume helpers and wires package-update tests and flow to disable enabled autostart during package replacement and re-enable it afterward.

PR surface: Source +177, Tests +427. Total +604 across 4 files.

Reproducibility: yes. source-reproducible: current main and v2026.6.10 stop the Windows Scheduled Task with /End and do not disable the PT3M repeated trigger before mutable package replacement. I did not run a native failing update in this read-only review.

Review metrics: 1 noteworthy metric.

  • Persistent service-state mutation: 1 temporary disable/enable cycle added. The updater now mutates durable Windows Task Scheduler enabled state, so recovery ownership and interruption behavior matter before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #87993
Summary: The canonical remaining work is the Windows Scheduled Task PT3M relaunch race during update; this PR is the maintainer-authored candidate fix, while the smaller external PR is an alternate candidate with broader stop-semantics risk.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase onto current main and rerun the focused update and Scheduled Task tests against the resolved code.
  • Attach or reference a native Windows OpenClaw package-update transcript or recording that shows suspension, replacement, restoration, and restart or --no-restart behavior.

Risk before merge

  • [P1] The PR is draft and dirty against current main, so the tested head is not the final merge result.
  • [P1] The updater temporarily mutates durable Windows Scheduled Task enabled state; an abrupt process termination after /DISABLE and before recovery can leave logon autostart disabled until operator action.
  • [P1] Current main now shares the mutable-update service stop lifecycle across package and git updates, so the rebase needs an explicit package-only versus shared-scope decision.
  • [P1] The PR body has focused tests and native Task Scheduler primitive proof, but not a full native Windows OpenClaw package-update run through replacement, restoration, and restart or --no-restart outcome.

Maintainer options:

  1. Rebase And Prove The Final Windows Lifecycle (recommended)
    Resolve the dirty branch against current main while preserving conditional restoration, then rerun focused tests plus native Windows package-update proof on the rebased head.
  2. Accept Hard-Termination Recovery Limits
    Maintainers may intentionally accept that an abrupt updater death can require manual schtasks recovery if diagnostics and support guidance are considered sufficient.
  3. Hold For Native Update Validation
    Keep the draft open but unmerged until a Windows environment can exercise package replacement, restoration, and restart or --no-restart outcomes.

Next step before merge

  • [P2] Manual review is needed because the PR has the protected maintainer label, is draft/dirty, and the remaining action is rebase plus recovery-risk and scope acceptance.

Security
Cleared: The diff changes Windows service-control logic and tests only; it does not touch dependencies, lockfiles, workflows, credentials, permissions, publishing, or downloaded code.

Review details

Best possible solution:

Rebase onto the current mutable-update lifecycle, keep suspension update-scoped, restore only tasks this invocation disabled, and validate the native Windows update path before merge.

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

Yes, source-reproducible: current main and v2026.6.10 stop the Windows Scheduled Task with /End and do not disable the PT3M repeated trigger before mutable package replacement. I did not run a native failing update in this read-only review.

Is this the best way to solve the issue?

Yes in direction, but not yet as a merge result. Update-scoped suspension is narrower than changing generic stop/restart semantics, and the branch needs rebase plus maintainer acceptance of the temporary-disable recovery behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 15de9d881a84.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies focused regression tests plus Crabbox native Windows Task Scheduler live-output proof for disable/enable XML; a full OpenClaw package-update smoke remains a merge-risk gap.

Label justifications:

  • P2: This is a bounded Windows package-update reliability fix with real availability impact but limited platform blast radius.
  • merge-risk: 🚨 availability: If temporary Scheduled Task disable is not recovered, the gateway may remain unavailable at logon until operator action restores it.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies focused regression tests plus Crabbox native Windows Task Scheduler live-output proof for disable/enable XML; a full OpenClaw package-update smoke remains a merge-risk gap.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies focused regression tests plus Crabbox native Windows Task Scheduler live-output proof for disable/enable XML; a full OpenClaw package-update smoke remains a merge-risk gap.
Evidence reviewed

PR surface:

Source +177, Tests +427. Total +604 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 185 8 +177
Tests 2 468 41 +427
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 653 49 +604

What I checked:

  • Repository policy read: Root AGENTS.md was fully read and applied; its ClawSweeper guidance treats maintainer/protected-label PRs as keep-open and calls out service startup/fallback changes as compatibility and availability-sensitive review surfaces. (AGENTS.md:1, 15de9d881a84)
  • Live PR state: GitHub reports the PR as open, draft=true, mergeable=false, mergeable_state=dirty, head da348c1, and labeled maintainer plus merge-risk availability. (da348c17f407)
  • Current main stop path: Current main stops Windows Scheduled Tasks with schtasks /End and does not disable the registered task, so repeated logon triggers can remain armed during an update window. (src/daemon/schtasks.ts:1391, 15de9d881a84)
  • Current main update caller: Current main stops the managed service before mutable package and git update work, making the stop lifecycle the relevant rebase/scope surface for this PR. (src/cli/update-cli/update-command.ts:3732, 15de9d881a84)
  • Latest release still lacks the fix: Tag v2026.6.10 contains /End calls in schtasks.ts and no /DISABLE or /ENABLE task autostart guard, so the fix is not shipped in the latest release. (src/daemon/schtasks.ts:1307, aa69b12d0086)
  • Branch update-scoped suspension: The PR branch suspends Windows Scheduled Task autostart before package update service stop and tracks whether this invocation actually suspended it. (src/cli/update-cli/update-command.ts:925, da348c17f407)

Likely related people:

  • vincentkoc: Authored this branch and recent current-main update/Windows service commits appear in the update-command and schtasks history. (role: recent area contributor and proposed-fix owner; confidence: high; commits: da348c17f407, c1a414bf286a, 62fad3da8606; files: src/cli/update-cli/update-command.ts, src/cli/update-cli.test.ts, src/daemon/schtasks.ts)
  • steipete: Recent history shows repeated Scheduled Task and daemon hardening work in the same Windows service-control surface. (role: Windows daemon lifecycle contributor; confidence: high; commits: f348284fa9eb, 625b793635e2, a3c44d53d18d; files: src/daemon/schtasks.ts, src/daemon/schtasks.stop.test.ts, src/daemon/service.ts)
  • tmimmanuel: Commit 0fef95b preserved Scheduled Task restart/install behavior and adjacent tests that share the enable/stop/restart invariant. (role: Windows Scheduled Task behavior contributor; confidence: high; commits: 0fef95b17d72; files: src/daemon/schtasks.ts, src/daemon/schtasks.stop.test.ts, src/daemon/schtasks.install.test.ts)
  • MoerAI: Recent merged XML Scheduled Task creation work is directly relevant to this PR's live XML enabled-state parsing. (role: Scheduled Task XML contributor; confidence: high; commits: d92a33306e57; files: src/daemon/schtasks.ts, src/daemon/schtasks.install.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

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.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T07:39:29.593Z sha da348c1 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 30, 2026
@vincentkoc
vincentkoc force-pushed the fix/windows-update-task-autostart branch 3 times, most recently from 690b55d to 52fc3ef Compare May 30, 2026 15:01
@vincentkoc
vincentkoc force-pushed the fix/windows-update-task-autostart branch from 52fc3ef to dfc06a5 Compare May 30, 2026 17:13
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 30, 2026
@vincentkoc
vincentkoc force-pushed the fix/windows-update-task-autostart branch from dfc06a5 to da348c1 Compare May 31, 2026 11:36
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 31, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jul 5, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Replayed and landed the canonical fix in #100757 as commit a13a198.

The landed version keeps the original bounded package-update guard, then adds preservation for already-disabled tasks, locale-independent Task Scheduler existence checks, signal-safe restoration, combined failure causality, focused regression coverage, exact-head green CI, and native Windows task-state proof.

This branch could not be updated because maintainer edits were disabled, so I recreated the fix on a maintainer branch while preserving contributor credit. Thank you, @vincentkoc. Enabling Allow edits by maintainers on future PRs lets us apply this kind of final hardening directly to the contributor branch.

@steipete steipete closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI command changes gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows — openclaw update 期间 Scheduled Task PT3M 重复触发器重新拉起 gateway 导致竞态

2 participants