fix(update): guard Windows task autostart during package updates#88292
fix(update): guard Windows task autostart during package updates#88292vincentkoc wants to merge 2 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 4:40 PM ET / 20:40 UTC. Summary 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.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +177, Tests +427. Total +604 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
690b55d to
52fc3ef
Compare
52fc3ef to
dfc06a5
Compare
dfc06a5 to
da348c1
Compare
|
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. |
Summary
--no-restartstopped/unknown cases where the task can still logon-start during replacement<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/ENABLErecovery if service stop fails after autostart was suspended, so the operator is not left with silently disabled logon autostartschtasks, missing tasks, unreadable XML state, and already-disabled tasks unchangedFixes #87993.
Verification
node scripts/run-vitest.mjs src/daemon/schtasks.stop.test.ts src/cli/update-cli.test.ts -- --run- 147 passednode_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.tsnode_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.tsgit diff --check origin/main...HEAD && git diff --check.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main- clean, patch correctrun_80f36e0c133e,pnpm check:changedpassedrun_16c0e77f1c9a,scheduled_task_disable_enable_xml_live=okPT3M/P9999D<Settings><Enabled>false</Enabled>/DISABLEwrites<Settings><Enabled>false</Enabled>/ENABLEremoves the explicit disabled state againrun_373d5c3d81fcobservedERROR: 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
/ENABLErecovery 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:
Evidence after fix: Tests assert
/DISABLEhappens before service stop/package install and/ENABLEhappens after install, before post-core fresh-process handoff. Tests also cover stopped and unknown installed task runtimes,--no-restartstopped/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/ENABLErecovery 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
schtasksbut 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.