fix(exec): timeout Windows taskkill on the timeout path

[SebTardif]

Windows command deadlines could still leave Clawpatch hung while waiting for taskkill /T /F. Cleanup now has its own five-second deadline (CLAWPATCH_TASKKILL_TIMEOUT_MS), kills a wedged cleanup process, and terminates the direct child even when tree cleanup fails. Invalid or unsupported timer values fall back to the default; descendant cleanup remains best effort if taskkill is unavailable or hung.

The regression coverage verifies that the hanging cleanup process actually starts, exercises the Windows runCommandArgs caller, and checks that the direct child dies. A dedicated Windows executor CI job also exposed a pre-existing quoted-shell-command bug; the command runner now preserves cmd.exe quoting, with portable shell quoting in the test fixtures. Configuration docs and the changelog describe the behavior.

Native Windows proof used the built CLI, a temporary Git repository with a local bare remote, and native gh.exe/taskkill.exe fault-injection shims that record their invocations before hanging. No real GitHub PR was created by the proof.

$env:CLAWPATCH_GH_PR_CREATE_TIMEOUT_MS = '1000'
$env:CLAWPATCH_TASKKILL_TIMEOUT_MS = '1000'
# CLAWPATCH_GH and PATH point to the verified native fault-injection shims.
node dist/cli.js open-pr --root <synthetic-repo> --patch pat_timeout --base main --json --no-color

Observed against the submitted implementation: the CLI printed error: gh pr create failed: command timed out after 1000ms, but remained alive at the 10-second outer deadline. Against the corrected implementation: the same CLI exited with its expected GitHub-failure code 7 in 3130ms; the direct child was gone; the gh shim ran once and taskkill ran twice with the expected /pid ... /T /F arguments. Final harness marker: CLI_EXITED_AND_CHILD_TERMINATED=true.

Validation: native Windows executor tests passed (20 passed, 1 Unix-only skip), local macOS executor tests passed (19 passed, 2 Windows-only skips), and the Linux full suite passed (916 passed, 2 skipped), along with typecheck, lint, formatting, build, and packaged CLI smoke (13 features, including 3 CUDA). The final source also runs through GitHub's Linux and Windows CI jobs. Codex autoreview returned scoped-clean at its default P0 threshold.

Related Go-mapper timeout work: #194. Land this shared cleanup fix first. Original contribution by @SebTardif is retained in the commit history and changelog.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 31, 2026, 8:17 PM ET / September 1, 2026, 00:17 UTC.

ClawSweeper review

What this changes

This PR bounds a hung Windows process-tree cleanup, terminates the direct child after failed cleanup, preserves Windows shell quoting, and adds tests, documentation, changelog text, and Windows CI coverage.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

✅ Ready for maintainer review

Keep this PR open for ordinary maintainer review. The bounded Windows cleanup path is implemented, documented, covered by a Windows-specific test and CI job, and supported by a native Windows before/after CLI trace; no actionable patch defect was identified.

Priority: P1
Reviewed head: a08843e342983fdda72766b98a666aa6112be717

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with strong platform-specific behavior proof, CI coverage, tests, and documentation.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.
Evidence reviewed	6 items	Bounded cleanup implementation: The Windows timeout path bounds each taskkill process, kills a timed-out cleanup helper, and then terminates the direct child before resolving. Windows caller regression coverage: The Windows-only test invokes the command runner, intercepts a hanging taskkill process, asserts timeout result output, and verifies the original child has exited. Feature-history provenance: The prior merged exec timeout implementation waited indefinitely for taskkill to close; this PR changes that exact cleanup boundary.
Findings	None	None.
Security	None	None.

How this fits together

Clawpatch’s shared command runner launches provider, Git, shell, and validation commands and returns a normalized command result to CLI workflows. On Windows, timeout handling invokes process-tree cleanup before returning control to the caller.

flowchart LR
  A[CLI workflow] --> B[Shared command runner]
  B --> C{Command deadline reached?}
  C -->|No| G[Command result]
  C -->|Windows timeout| D[Process-tree cleanup]
  D --> E[Cleanup deadline]
  E --> F[Terminate direct child]
  F --> G

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and regression-test delta	production +40, tests +112	Most of the change is focused regression coverage for a small process-lifecycle repair.
Platform coverage	1 Windows CI job added	The affected timeout path is Windows-specific and is now exercised on that platform.

Technical review

Best possible solution:

Land the focused timeout and quoting repair after ordinary exact-head maintainer review.

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

Yes. The supplied native Windows before/after built-CLI trace gives a concrete reproduction path, and the prior source shows taskkill could wait without a cleanup deadline; this read-only review did not rerun it.

Is this the best way to solve the issue?

Yes. Bounding the cleanup helper while retaining best-effort tree termination and a direct-child fallback is a narrow repair to the shared timeout contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 09900a5edd2b.

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.
• remove merge-risk: 🚨 automation: Current PR review selected no merge-risk labels.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
• remove merge-risk: 🚨 availability: Current PR review selected no merge-risk labels.

Label justifications:

• P1: A hung cleanup process can keep public CLI workflows alive after their configured deadline.
• 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 (terminal): The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the Windows command-timeout cleanup path in the shared command runner; the captured native Windows built-CLI run injects hanging gh/taskkill shims through open-pr and records the after-fix bounded exit, direct-child termination, and expected cleanup invocations.

Evidence

What I checked:

• Bounded cleanup implementation: The Windows timeout path bounds each taskkill process, kills a timed-out cleanup helper, and then terminates the direct child before resolving. (src/exec.ts:161, a08843e34298)
• Windows caller regression coverage: The Windows-only test invokes the command runner, intercepts a hanging taskkill process, asserts timeout result output, and verifies the original child has exited. (src/exec.test.ts:273, a08843e34298)
• Feature-history provenance: The prior merged exec timeout implementation waited indefinitely for taskkill to close; this PR changes that exact cleanup boundary. (src/exec.ts:145, f16b8467e1b5)
• Native Windows behavior proof: The captured PR body reports a built-CLI native Windows fault-injection run where the corrected path exited with code 7 in 3130ms, the direct child was gone, and taskkill received the expected process-tree arguments. (a08843e34298)
• Windows automation result: GitHub context records the introduced windows-exec job as successful on the exact PR head. (.github/workflows/ci.yml:30, a08843e34298)
• Current status: GitHub context identifies this as an open, unmerged PR; no merged same-repository replacement is identified. (09900a5edd2b)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Hunter Sadler: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

History

Review history (9 earlier review cycles; latest 8 shown)

• reviewed 2026-08-30T01:59:55.335Z sha e890097 :: needs real behavior proof before merge. :: [P3] Document the taskkill timeout override
• reviewed 2026-08-30T07:05:54.860Z sha e890097 :: needs real behavior proof before merge. :: [P3] Document the taskkill timeout override
• reviewed 2026-08-30T08:51:26.848Z sha e890097 :: needs real behavior proof before merge. :: [P3] Document the taskkill timeout override
• reviewed 2026-08-30T13:05:14.231Z sha e890097 :: needs real behavior proof before merge. :: [P3] Document the taskkill timeout override
• reviewed 2026-08-30T16:06:19.298Z sha e890097 :: needs real behavior proof before merge. :: [P3] Document the taskkill timeout override
• reviewed 2026-08-31T02:05:40.277Z sha e890097 :: needs real behavior proof before merge. :: [P2] Make the Windows hang shim a runnable process | [P3] Document the taskkill timeout override
• reviewed 2026-08-31T11:06:32.565Z sha e890097 :: needs real behavior proof before merge. :: [P2] Verify that the Windows test invokes the hanging killer | [P3] Document the taskkill timeout override
• reviewed 2026-09-01T00:11:40.118Z sha 117a250 :: needs real behavior proof before merge. :: none

[steipete]

Maintainer follow-up: completed the cleanup fix on this branch, preserving @SebTardif's contribution. The final head is a08843e342983fdda72766b98a666aa6112be717.

Native Windows proof now exercises the built open-pr CLI with verified hanging executable shims. The submitted implementation printed its timeout error but remained alive at the 10-second outer deadline. The corrected implementation exited with the expected error code 7 in 3130ms, and the direct child was terminated. The gh shim ran once and the taskkill shim ran twice with the expected arguments. The updated PR body contains the command and observed output.

The new Windows CI job also exposed and now covers quoted shell-command handling. Final CI passes both Linux and Windows. Typecheck, lint, formatting, full tests, build, and packaged CLI smoke passed; native Windows executor coverage is 20 passed / 1 Unix-only skip. Codex autoreview is scoped-clean at the default P0 threshold.

LAND recommended. This remains open and unmerged for the orchestrator. Land the shared executor change before the Go-mapper deadline in #194.