Skip to content

fix(exec): bound output capture and Windows shutdown - #63

Open
tang-vu wants to merge 6 commits into
mainfrom
fix/bounded-command-capture
Open

fix(exec): bound output capture and Windows shutdown#63
tang-vu wants to merge 6 commits into
mainfrom
fix/bounded-command-capture

Conversation

@tang-vu

@tang-vu tang-vu commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Problem

The bounded command runner stopped reading at its output cap and timed only direct-child exit. Verbose checks could fail on a closed pipe, and inherited writers could delay capture indefinitely. The initial fix bounded the async call, but Windows still took about eight seconds to exit for a one-second deadline because Tokio's anonymous-pipe adapter retained blocking reads.

Change

  • Drain stdout and stderr concurrently while retaining at most 64 KiB per stream
  • Apply one absolute command/run deadline to child exit and capture; keep direct-child cleanup bounded
  • On Windows, use two connected inbound named pipes with overlapped readers and ordinary synchronous child writers
  • Connect before spawn, then drop the Command builder's writer copies immediately after spawn to avoid short-child and EOF races
  • Reject remote clients and namespace collisions; keep each stream in its own single-instance pipe
  • Assert whole-host lifetime, external cancellation, live inherited writers, exact output digest, short-child success and Windows handle cleanup

Only runner.rs and the operator guide change. Direct-child termination, command classification, environment scrubbing and fail-closed evidence behavior are preserved. Descendants may survive and later see a closed output pipe. This provides no process-tree sandbox and does not bound unrelated application shutdown work.

Verification

Current head: f2a1b04. The production correction is f9a4324; the follow-up changes only Windows cleanup tests. Published trees and reviewed file hashes were verified.

  • Formatting passes on Rust 1.99.0
  • Independent focused Linux harness: 29 embedded runner/classifier tests and five additional output/concurrency/fast-child tests pass; strict Clippy passes
  • Linux timeout probe: runner 1001 ms, runtime destruction 1 ms, host exit 1033 ms
  • Linux cancellation probe: runner 62 ms, runtime destruction 0 ms, host exit 91 ms
  • Both probes confirm a descendant is still alive after runtime destruction, then release the finite fixture
  • Previous exact-head Windows baseline: runner 1011 ms, runtime destruction 7018 ms, host exit 8057 ms (job evidence)

The first correction's hosted run compiled and exercised the Windows production path:

  • Windows timeout: runner 1006 ms, runtime destruction 0 ms, host exit 1070 ms
  • Windows cancellation: runner 96 ms, runtime destruction 0 ms, host exit 145 ms
  • The actual descendant survived runtime destruction in both cases
  • Hosted formatting/Clippy, Rust 1.88 MSRV, package checks and all Linux/macOS runner tests passed

That run also exposed a Windows cleanup test assuming immediate pipe-name reuse. Mio can retain a handle until a queued IOCP event is processed. The follow-up keeps the same-name resource check, gives the reactor progress for at most one second, retries only ERROR_PIPE_BUSY, and fails immediately on other errors. Production is unchanged. The first run is not a complete Windows pass.

Final Windows cleanup qualification and exact-head checks are pending in CI run 37581650170. A full-workspace pass is not claimed.

The existing main-branch path-validation, Windows PATH-scrubbing and rustls audit failures are addressed separately by #61. No #61 implementation or dependency/workflow changes are included here. This PR stays draft pending Windows qualification, baseline fixes, CI and required maintainer approval.

AI-assisted implementation and separate technical review are disclosed. That review does not substitute for independent human maintainer approval.

@tang-vu
fix(exec): bound command capture and shutdown
f9fe185 
Drain excess output without retaining it and await the direct child plus
both readers under one absolute deadline. Own reader futures directly,
bound cleanup, and reject unsupported timeout arithmetic.

Add focused regressions for noisy output, inherited pipes, read errors,
cancellation and extreme timeouts. Document direct-child-only cleanup.
@vercel

vercel Bot commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
contribai Ready Ready Preview Oct 7, 2026 7:05am UTC

@tang-vu tang-vu changed the title fix(exec): bound command capture and shutdown fix(exec): drain capped output and bound capture futures Oct 3, 2026
@tang-vu
test(exec): distinguish runner and runtime shutdown deadlines
888ee35 
Assert call wall-clock bounds and record call return, runtime teardown
and host exit separately in a finite real-process diagnostic.

Document Tokio's Windows blocking-pipe teardown limitation instead of
claiming a hard CLI-process exit deadline. Execution logic is unchanged.
@tang-vu
fix(exec): cancel Windows pipe reads before runtime shutdown
f9a4324 
Use connected overlapped named-pipe readers with synchronous child writers.
Keep direct-child cleanup and bounded evidence semantics, and assert real
host lifetime, descendant liveness, output integrity and handle cleanup.

AI-assisted implementation with independent source and Linux regression review.
Windows qualification follows on the existing draft pull request.
@tang-vu tang-vu changed the title fix(exec): drain capped output and bound capture futures fix(exec): bound output capture and Windows shutdown Oct 7, 2026
@tang-vu
test(exec): allow bounded Windows pipe completion cleanup
f2a1b04 
Give the IOCP reactor time to release its queued pipe handle before
reopening the same name. Retry only ERROR_PIPE_BUSY within one second;
all other errors still fail immediately. Production code is unchanged.
@tang-vu
fix(exec): integrate merged baseline fixes
4f86716 
Merge main cea4725 into the existing bounded-command-capture branch.
Preserve case-insensitive environment normalization and the Python
bytecode override before the reviewed connected-pipe setup.

Only runner.rs and the operator guide differ from main. Dependency
files remain identical to main; PR64 changes are not included.
@tang-vu
fix(exec): keep capture buffers off the Windows CLI stack
b485123 
Allocate each bounded 8 KiB scratch buffer on the heap so nested async
futures do not overflow the Windows debug binary's main-thread stack.
Preserve scoped read ownership, cancellation, deadlines and evidence caps.

Add broad inline-size budgets for capture, run and run_indirect futures.
The regression fails before this change; focused tests and Clippy pass.
Actual Windows CLI and resource tests remain required in hosted CI.
@tang-vu
tang-vu marked this pull request as ready for review October 7, 2026 07:20

This branch was successfully deployed

1 active deployment
Preview — b485123f Deployed Oct 7, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tang-vu