fix(test): tolerate delayed Windows process exit observation

[RomneyDa]

Summary

• keep the bounded cleanup latency assertions separate from eventual process-exit observation
• allow Windows up to five seconds to report termination after the helper returns
• apply the same invariant to timeout, active cancellation, and already-canceled-token coverage

Regression

PR #1607 hit BoundedProcessWaitTests.WaitAsync_CancellationKillsProcessWithoutUsingTimeoutBudget in Core and CLI tests. BoundedProcessWait intentionally returns after its one-second cleanup budget, so Windows can report HasExited == false briefly even though the asynchronous kill is still completing.

Required proof pools

• none: test-only stabilization with no product, UI, gateway, MCP, or installer behavior change

Validation

Exact head 03f301e3:

• Parallels Windows 11 ARM64 ./build.ps1: passed
• Parallels Shared tests: 4,222 passed, 36 skipped
• Parallels Tray tests: 3,890 passed
• Parallels focused BoundedProcessWaitTests: 30/30 repeated runs passed, 6 tests per run
• macOS focused BoundedProcessWaitTests: 6 passed
• macOS focused suite repeated 30 times: 30/30 runs passed
• AutoReview: clean, no accepted or actionable findings

Real behavior proof

• the tests still require the helper to return cancellation or timeout within three seconds
• the tests still require the fixture process to terminate, now through a separately bounded five-second exit observation
• no production code changed

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 2, 2026, 10:42 AM ET / 14:42 UTC (Revision 2).

ClawSweeper review

What this changes

Three process-cleanup tests now allow five seconds to observe termination while preserving the existing timeout and cancellation latency checks.

Merge readiness

✅ Ready for maintainer review

The PR remains useful: current main still contains the premature exit assertions. No introduced correctness or security defect was found, and the previous review has no outstanding findings.

Priority: P2
Reviewed head: 03f301e30266f8e504f4be73a6a1e3667629ad77

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused test repair preserves the important invariants and has reported repeated validation on Windows and macOS.
Proof confidence	🌊 off-meta tidepool	Not applicable: This MEMBER-authored test-only PR has no changed production owner requiring contributor runtime proof. Reported exact-head fixture runs exercise timeout, active cancellation, and already-canceled tokens; no stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: This MEMBER-authored test-only PR has no changed production owner requiring contributor runtime proof. Reported exact-head fixture runs exercise timeout, active cancellation, and already-canceled tokens; no stored-data contract changes.
Evidence reviewed	6 items	Introduced test change: The pinned introduction changes only three exit assertions and adds a five-second observation helper. The two existing three-second latency assertions remain intact. Existing cleanup contract: Cleanup schedules process-tree termination asynchronously and returns after cleanup completes or its one-second budget expires. Immediate exit observation is therefore stronger than the documented best-effort contract. Still necessary on main and release: Fetched main and v2026.9.4 both retain the immediate HasExited assertions. The requested stabilization is not already present.
Findings	None	None.
Security	None	None.

How this fits together

The shared process-wait helper bounds child-process execution and output collection for Piper voice extraction. These tests check that cancellation and timeout return promptly and terminate the fixture process.

flowchart TD
 A[Child process fixture] --> B[Shared process wait]
 C[Timeout or cancellation] --> B
 B --> D[Bounded cleanup]
 D --> E[Prompt return assertion]
 D --> F[Eventual exit observation]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed exit checks	3 assertions updated; 1 five-second helper added	Termination remains mandatory without extending the measured cleanup latency.

Technical review

Best possible solution:

Keep cleanup latency and eventual termination as separate bounded invariants without changing production behavior.

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

Yes, source establishes the assertion race: asynchronous termination can outlast the helper's one-second cleanup budget. The captured body reports a Windows failure and repeated after-fix passes; this review did not execute tests.

Is this the best way to solve the issue?

Yes, separating prompt return from eventual exit matches the existing contract, and the thirty-second fixture cannot pass merely by naturally finishing within the new observation window.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4e285bbc6595.

Labels

Label changes:

No label changes.

Label justifications:

• P2: This is a bounded test-reliability repair with no production behavior change.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This MEMBER-authored test-only PR has no changed production owner requiring contributor runtime proof. Reported exact-head fixture runs exercise timeout, active cancellation, and already-canceled tokens; no stored-data contract changes.

Evidence

What I checked:

• Introduced test change: The pinned introduction changes only three exit assertions and adds a five-second observation helper. The two existing three-second latency assertions remain intact. (tests/OpenClaw.Shared.Tests/BoundedProcessWaitTests.cs:167, 03f301e30266)
• Existing cleanup contract: Cleanup schedules process-tree termination asynchronously and returns after cleanup completes or its one-second budget expires. Immediate exit observation is therefore stronger than the documented best-effort contract. (src/OpenClaw.Shared/Audio/BoundedProcessWait.cs:96, 03f301e30266)
• Still necessary on main and release: Fetched main and v2026.9.4 both retain the immediate HasExited assertions. The requested stabilization is not already present. (tests/OpenClaw.Shared.Tests/BoundedProcessWaitTests.cs:32, 4e285bbc6595)
• Release comparison: The latest supplied release also uses immediate exit assertions in all three affected tests. (tests/OpenClaw.Shared.Tests/BoundedProcessWaitTests.cs:32, 3c43751b2bac)
• Feature-history routing: Blame ties the bounded cleanup implementation to this commit. GitHub verifies fix(audio): unify Piper extraction deadline #1294 (fix(audio): unify Piper extraction deadline) as merged, authored and merged by shanselman, and its commit metadata identifies both central files as added. Local full historical diff inspection encountered unavailable promisor blobs; current source and GitHub metadata were successfully inspected. (src/OpenClaw.Shared/Audio/BoundedProcessWait.cs:96, 1d32722cada4)
• Validation and review continuity: The captured PR body reports exact-head Windows build, 4,222 Shared passes with 36 skips, 3,890 Tray passes, and 30 repeated focused passes on both Windows and macOS. Provided checks show Core and CLI tests succeeded. The previous review covered this same head and retained no findings or rank-up moves. No build, test, or target-bundled reviewer was executed during this read-only review. (03f301e30266)

Likely related people:

• Scott Hanselman: Raw commit 1d32722 adds src/OpenClaw.Shared/Audio/BoundedProcessWait.cs:96 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 1d32722cada4; files: src/OpenClaw.Shared/Audio/BoundedProcessWait.cs)

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 (1 earlier review cycle)

• reviewed 2026-10-02T14:30:38.078Z sha 03f301e :: needs maintainer review before merge. :: none