Add Windows CI and harden runtime lifecycle#69
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb12f76aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const child = spawn(invocation.command, invocation.args, { | ||
| ...spawnOptions, | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| windowsVerbatimArguments: invocation.windowsVerbatimArguments |
There was a problem hiding this comment.
When --cloudflared, --ngrok, or --tailscale points at a .cmd/.bat shim, this now records the spawned cmd.exe wrapper in spawnedChildren; cleanupChildren() later only calls child.kill() on that wrapper. On Windows, terminating the wrapper does not terminate the long-running process that the batch file launched, so q/SIGTERM shutdown can leave the tunnel process running after the launcher exits. Please either spawn the underlying executable directly or tear down the process tree for these wrapped invocations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 21f7812. Windows batch-file invocations now carry a process-tree cleanup marker, and shutdown uses taskkill.exe /PID <pid> /T /F for the wrapper and its descendants, with the existing direct-child kill as a fallback.
I also added a Windows integration regression in settings-smoke.mjs: the fake cloudflared.cmd records its real Node descendant PID, and the test verifies that PID is gone after the non-TTY q shutdown path.
Verified with npm run build, node scripts/settings-smoke.mjs, and the full npm run smoke suite.
1 participant
Summary
.cmd/.batinvocation path for tunnel binaries and proxiedcurlqshutdown for non-TTY launchers so runtime and temporary tunnel credentials are cleaned upWhy
CodexPro supports Windows, but CI currently runs only on Ubuntu. On Windows, the documented validation flow failed in several places before reaching the end of the suite:
analysis-smokebuilt a doubled drive path such asC:\\C:\\...HOMEbehaviorThe runtime-status failure is user-visible: the local settings/status page can report a stopped launcher as active after an abrupt Windows termination.
Impact
.cmd/.battunnel shims use the same quotedcmd.exewrapper already used by the handoff executor.qto stdin.This does not change authentication, tunnel exposure, tool permissions, or default write/bash policy.
Validation
npm ci(0 vulnerabilities)npm run buildnpm run smokenpm run stressnpm pack --dry-rungit diff --checkAll checks passed locally on Windows.
Contributor attribution
One small attribution request: if this is merged directly or ported into another branch, could you please preserve the author metadata from commit
fb12f76(or addCo-authored-by: Zao <zawonyee@163.com>) so GitHub can credit the contribution? Please use whichever merge strategy best fits the project. Thank you.