fix(ci): stop kill-grace from bounding Windows helper spawns

[tthayer]

Split out of #1426 at maintainer request — process-control change, reviewed on its own.

Problem

The Windows shard failed Step 0i (parallel suite scheduler contract) with cleanup failed: hang_after_summary: leader exited leaving live descendants while its sibling shard passed. Nothing was actually leaking a process.

run-test-wave.py passed --kill-grace as the subprocess timeout for two external Windows helpers: taskkill.exe /T /F and the powershell.exe Get-CimInstance descendant probe. Those are different quantities. kill_grace budgets how long a doomed process may take to die; the helpers also pay process spawn plus, for PowerShell, CIM startup. Contract fixtures run with --kill-grace 1, which is not reliably enough to launch either helper on a loaded runner.

The two timeouts compounded: a timed-out taskkill is reported as "could not prove process-tree cleanup", which raises out of the wave loop; the finally-block cleanup re-enters with the leader already dead, falls to the descendant probe, which times out on the same one-second budget and takes its "cannot prove absence -> assume the worst" branch. Phantom descendants, red wave.

Change

• scripts/run-test-wave.py: helper invocations get their own floor, max(kill_grace, WINDOWS_HELPER_TIMEOUT_SECONDS=30). kill_grace still governs every actual death wait, so nothing fails open — the timeout-race contract still refuses with rc=2 over a genuinely surviving descendant, now because PowerShell answered rather than because it timed out. The production path already passed --kill-grace 15 and is unchanged in behaviour.
• tests/test_parallel_harness_contract.sh: the flat 8s budget for the scheduler's refusal now reads the floor out of the scheduler on Windows rather than restating it — hard-coding it turns a slow runner into a harness failure the moment the two drift. The refusal path can spend the floor twice (proving descendants, then cleanup re-entry), so it allows both plus interpreter startup. POSIX is unchanged at 8s: the refusal there is signal-driven.

The two halves ship together because the contract imports WINDOWS_HELPER_TIMEOUT_SECONDS from the scheduler.

Validation

tests/test_parallel_harness_contract.sh passes locally on darwin arm64. The Windows behaviour it guards is exercised by the Windows shard in CI.

🤖 Generated with Claude Code

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you for splitting the Windows process-control fix into a focused PR and for documenting the difference between helper startup time and process death grace. The contribution queue is quite full, so the full review may take a little time. We have the PR routed and will come back with grounded feedback as soon as possible.

[DeusData]

Approved, and this one is worth more than its 39 lines: it closes #1963, which was filed from this queue after the same failure reddened an unrelated pull request.

Your causal chain is exactly right, and it is the part most people would not have found. A timed-out taskkill reports "could not prove process-tree cleanup", which raises out of the wave loop; the finally-block cleanup then re-enters with the leader already dead, falls through to the descendant probe, times out on the same one-second budget, and takes its "cannot prove absence → assume the worst" branch. Two independent timeouts compounding into a phantom-descendant report, while the sibling shard passes. That is why it looked like flakiness rather than a bug.

And the underlying confusion is stated in one sentence: kill_grace budgets how long a doomed process may take to die, but the helpers also pay process spawn and, for PowerShell, CIM startup. Those are different quantities that happened to share a variable.

I verified the safety claim rather than taking it. On main, kill_grace bounds six things. Your change touches exactly two — the windows_descendants probe at :170 and the taskkill invocation at :188, both helper spawns. All four genuine death waits are untouched:

:196  process.wait(timeout=kill_grace)
:203  process.wait(timeout=kill_grace)
:233  wait_for_group_exit(... + kill_grace)
:239  wait_for_group_exit(... + kill_grace)

So the timeout-race contract still refuses with rc=2 over a genuinely surviving descendant — now, as you say, because PowerShell answered rather than because it timed out. Nothing fails open, and the production path at --kill-grace 15 is behaviourally unchanged.

Having the contract read the floor out of the scheduler is the right call. module.WINDOWS_HELPER_TIMEOUT_SECONDS * 2 + 10 rather than a restated 8 means the two cannot drift into a harness failure that looks like a product failure. Your * 2 reasoning is sound too — the refusal path really can spend the floor twice, proving descendants and then re-entering cleanup. That is the same class of fix as deriving a loop bound from its own array instead of borrowing an unrelated constant.

Shipping both halves together is correct given the import.

Status

Clearance is REVIEW(1) on scripts/run-test-wave.py — the established shape for a test-harness script here. It needs a maintainer marker, not a change from you; I will handle it.

This is CLEAN and green, so it should move quickly once cleared — it is CI-unblocking, and a fix for something currently reddening other people's work goes ahead of ordinary queue order.

One clarification so the scope stays honest: this addresses test-windows shard 1 and its parallel-scheduler contract. There is a separate cluster on test-windows-guards — preconditions failing because the guards job's own setup cannot index an ASCII fixture — which I have seen on four unrelated PRs and which this does not touch. Different failure, also ours.