Repository navigation
fix(service): bound every Windows manager command the guarded stop depends on - #6403
Conversation
…pends on Every execFileSync a Windows verdict or stop relies on ran without a timeout: all schtasks operations via runFile (/query, /end, /delete, /create, /xml), WinSW verbs including stopwait, and every sc.exe query/qc/stop/delete. A wedged Task Scheduler or Service Control Manager service would hang the CLI forever inside a guarded stop — observed natively as the bundled stop blowing the desktop takeover deadline while sibling probes (PowerShell, wrapper kill) are already bounded. Bounds: 15s for SCM round-trips and schtasks operations (all sub-second on a healthy system, far under the app's startup budget); 40s for WinSW stopwait so it outlasts the SCM <stoptimeout> (20s) in the generated service definition. The interactive install /p call stays unbounded by design — it waits on the operator's password input. A killed command throws into the same unreadable/ failed/unknown classification a nonzero exit would, so no verdict weakens. Source-level regressions pin the contract: every execFileSync in winsw.ts is bounded except the inherited-stdio interactive call, stopwait routes to the longer bound, and runFile carries SCHTASKS_TIMEOUT_MS. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/0 boxes ticked. Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked. |
…90861790-win-bounded-service-execs
…90861790-win-bounded-service-execs
|
Maintainer integration record (MAINTAINERS.md, dev-only)
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
schtasks,sc.exe, and ordinary WinSW verbs; 40 seconds for WinSWstopwait, exceeding its generated 20-second stop timeout.unknown/failed; it never establishes that a manager stopped.winsw install /premains unbounded because it waits for operator password input. No hung manager was observed in the original native timeout; these bounds address a separate indefinite-hang exposure.Verification
Refreshed from
devat06cc3815c06a10120d89ef986c46be95be37f28awithout rewriting branch history, including the accepted upstream role-route and owner-registry ACL fixture corrections. Targeted verification: role routes 7 passed; owner-registry ACL 6 passed. Required CI passed ate6bf0bae23bcf3ee5715002330bb002df2e61188: zero failed and zero pending checks (platform/path-inapplicable checks remain skipped).bun test tests/windows/: 497/498 passed. The hung-tray-probe test also failed on cleandevon the same machine.bun x tsc --noEmit,bun run structure:check: passed. Source-level regressions pin timeout wiring and the interactive exception.Full
test:changedexceeded the local Windows 900-second cap; CI remains the full-suite gate.Real registered-Scheduler CLI lifecycle passed on combined
d50be3427with fix(service): scope Windows scheduler probes to the root task and re-probe deleted registrations #6401 and fix(cli): stabilize guarded-stop re-verification and report the failing guard fact #6402. Starting from absent Scheduler/WinSW registrations, installation and exact runtime approval succeeded; guarded stop completed in 49.4s, returnedruntimeDown: true, left the task Ready, and removed its complete approved wrapper/proxy chain. Repair/start succeeded; uninstall plus explicitocx stoprestored the absent-manager baseline.No deliberately wedged SCM/Scheduler was induced. WinSW account/password-dependent lifecycle remains untested.
Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Link to Devin session: https://app.devin.ai/sessions/a8795b27d4114aa6be37a88565e404b2
Open in Devin Desktop: https://app.devin.ai/desktop/session/a8795b27d4114aa6be37a88565e404b2?variant=devin
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.