Repository navigation
Wire native WSL tool runtime - #124
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Signal handling can leak retained containers, fast TTY exits can lose their exit status, and incomplete installations are reported as enabled.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Adds native WSL2 managed-tool execution through Docker Desktop while retaining release qualification gates.
Changes:
- Adds fixed-layout WSL dispatch, planning, streaming, signals, and cleanup.
- Extends Docker container retention and explicit lockfile resolution.
- Updates tests and WSL documentation.
| File | Description |
|---|---|
| README.md | Updates WSL2 status and limitations. |
| main.go | Dispatches native WSL tool shims. |
| main_test.go | Tests WSL dispatch behavior. |
| internal/wslrun/terminal_linux.go | Implements terminal and signal handling. |
| internal/wslrun/runner.go | Orchestrates container execution and cleanup. |
| internal/wslrun/runner_test.go | Tests runtime lifecycle behavior. |
| internal/wslrun/run_other.go | Rejects WSL execution outside Linux. |
| internal/wslrun/run_linux.go | Wires production WSL dependencies. |
| internal/wslrun/plan.go | Builds commands, mounts, and environment. |
| internal/wslrun/plan_test.go | Tests runtime planning. |
| internal/wslrun/frontend.go | Validates installation and dispatch identity. |
| internal/wslrun/frontend_test.go | Tests fixed-layout frontend validation. |
| internal/wslinstall/install.go | Reports runtime availability. |
| internal/wslinstall/install_test.go | Updates installation-report assertions. |
| internal/wslfs/command.go | Updates preflight documentation. |
| internal/wsldocker/remove.go | Supports retained-container cleanup. |
| internal/wsldocker/remove_test.go | Tests retained-container removal. |
| internal/wsldocker/remove_linux.go | Documents retention verification. |
| internal/wsldocker/create.go | Adds configurable container retention. |
| internal/wsldocker/create_test.go | Tests retained container creation. |
| internal/lockfile/lockfile.go | Adds explicit-path image resolution. |
| internal/lockfile/lockfile_test.go | Tests explicit WSL lock paths. |
| internal/hostenv/wsl_layout.go | Documents enabled layout consumption. |
| internal/hostenv/hostenv.go | Enables canonical native WSL2 hosts. |
| internal/hostenv/hostenv_test.go | Tests the revised host boundary. |
| docs/wsl.md | Documents implemented WSL runtime behavior. |
| docs/wsl-process-contract.md | Defines WSL process semantics. |
| docs/shell-contract.md | References the implemented WSL frontend. |
| docs/security-model.md | Adds WSL runtime security guarantees. |
| docs/roadmap-implementation-requirements.md | Updates remaining WSL gates. |
| docs/roadmap-decisions.md | Records completed runtime wiring. |
| docs/architecture.md | Documents the new orchestration package. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Initial review of head 41f4924 (this is the first review round — no prior reviews or comments on the PR).
The composition is coherent: the frontend revalidates the fixed layout, managed binary, registry and shim identities before touching Docker; the plan validates the complete spec before any volume mutation; create/attach/start/wait ordering is right (attach before start, retained container so wait always collects the exit code); and cleanup is conservative without force-deleting ambiguous state. The RetainUntilCleanup change to wsldocker create/remove correctly closes the fast-process auto-remove race, including the inspect re-proof. host_mounts and Windows-shaped state correctly fail closed, and docs match the implementation I read.
Two actionable findings:
[important] Non-terminal character devices are treated as interactive, so routine redirections hard-fail
terminal.Interactive() (internal/terminal/terminal.go) only checks os.ModeCharDevice on stdin/stdout. /dev/null (and any other non-tty char device) satisfies that check, so deps.interactive() in runFrontend (internal/wslrun/frontend.go:109) selects spec.TTY = true. Then in internal/wslrun/terminal_linux.go:
getTermios(os.Stdin.Fd())issuesTCGETS— fails withENOTTYwhen stdin is/dev/null,/dev/zero, etc.;terminalSize(os.Stdout.Fd())issuesTIOCGWINSZ— fails withENOTTYwhen stdout is/dev/null.
Concrete failure mode: cb tool >/dev/null in a real terminal fails with "read native WSL terminal size: inappropriate ioctl for device"; cb tool </dev/null or cb tool </dev/null >/dev/null fails with "inspect native WSL terminal mode: …". These are ordinary scripting/cron patterns, and the Windows frontend handles them today (the docker run -t path never performs host termios ioctls). Cleanup runs correctly, but the tool never executes.
Remediation direction: select TTY mode on real terminal-ness (e.g. a TCGETS/isatty probe on stdin — matching the docker -t convention of gating on stdin), not character-device-ness; or treat ENOTTY as non-interactive before spec.TTY is committed. Fixing this in the interactive seam keeps the Windows behavior untouched.
[important] Signal/resize forwarding can race container exit and mask the real exit status
In internal/wslrun/runner.go the event cases call deps.resize/deps.signal unconditionally (:243-249). The Engine returns an error (HTTP 409 "container is not running", or 404) when the container has already exited, and the select loop may still consume a queued SIGWINCH/signal event after wait has effectively decided the outcome — select picks randomly among ready cases, and a signal arriving between container exit and the waitDone read also forwards to a dead container.
Concrete consequence: Ctrl-C, SIGHUP on terminal close, or a resize arriving just as a fast tool exits converts a completed run into "forward signal N to native WSL tool container: Docker Desktop Engine API … HTTP 409" and exit code 120 instead of the tool's real status. The deferred cleanup then behaves correctly (non-force remove of the stopped container), so only the reported result is wrong — but it is intermittently wrong in a common interactive gesture.
Remediation direction: tolerate not-running/not-found responses for forwarded signals and resizes (the forward is best-effort once the container is gone), or stop consuming events once the wait result is known.
Summary
The wiring is disciplined and fail-closed throughout: identity proofs are repeated at each boundary, the spec is validated before any mutation, transport-ambiguous start failures get a conservative kill/wait/remove path, and exit codes propagate exactly. The two findings above are real behavioral defects in the new runtime path (spurious failures for standard stream redirections, and a forwarding race that can misreport a completed run's exit status); neither undermines the security model, but both are worth addressing before the qualification gates the PR describes are exercised. CI was still pending at the time of this review; Go toolchain was unavailable in my environment, so findings are from source inspection of the current head.
|
Addressed both important findings from the initial review at
Validation on the current head: @CherylSnowVeil please re-review current head |
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of head e47dba5 (second round — initial review covered 41f4924).
Validation of previous findings
Both findings from the initial review are fixed, verified against the current source, not just the replies:
- Non-terminal character devices → interactive (important): fixed.
interactiveHostTerminalnow probes stdin with a realTCGETSioctl (internal/wslrun/terminal_linux.go:58), so/dev/null,/dev/zeroand friends classify as non-interactive.prepareHostTerminalandterminalSizealso operate on the stdin fd, socb tool </dev/null,cb tool >/dev/nulland combined redirections no longer hard-fail — the exact failure modes from round one.TestInteractiveHostTerminalRejectsNonTerminalCharacterDevicecovers the/dev/nullcase. - Signal/resize race masking exit status (important): fixed. The wait goroutine now starts immediately after a successful
start(internal/wslrun/runner.go:165-169), before the initial resize, andcontainerCompletionRacetolerates Engine 404/409 from the initial and any queued resize/signal forwards while the already-started wait stays authoritative.TestExecuteToolPreservesFastTTYExitAcrossInitialResizeRaceandTestExecuteToolPreservesExitAcrossQueuedControlEventspin both cases.
The other review-round findings also hold up under re-verification: finishToolResult non-blockingly collects a completed stdin copy/half-close error before reporting a successful status; lifecycleErr is joined on every cleanup path; SIGPIPE is now intercepted so broken output writes surface as EPIPE through the normal error/cleanup path; the install report distinguishes INSTALL REQUIRED from INSTALLED; and production dispatch remains behind requireHostFrontend pending orphan reconciliation, so the retained-container orphan risk cannot be exercised in this build.
Fresh findings (complete re-review of the current head)
[important] A zero-size host terminal hard-fails every interactive run, and a transient size-read error kills a running tool
terminalSize (internal/wslrun/terminal_linux.go:147) treats TIOCGWINSZ returning 0 rows or columns as fatal. prepareHostTerminal therefore errors after interactiveHostTerminal has already selected TTY mode (a pty passes TCGETS regardless of winsize), so executeTool aborts before start and the invocation exits 120. Programmatic ptys that never set a size — pexpect/expect-style harnesses, creack/pty-based runners and similar — report 0x0, and those are plausible consumers of tool shims. docker run -t proceeds in the same environment (it never gates the run on the host winsize).
Same class of strictness mid-run: a SIGWINCH whose terminalSize read fails is delivered as event.err (terminal_linux.go:100-104) and runner.go:240 returns it, which aborts the session and force-kills the container. An unmeasurable resize should not kill the tool.
Consequence: deterministic failure of a legitimate environment at startup, and a fragile abort path during resize. Remediation direction: treat a 0x0 winsize as "unknown" — skip the initial resize or substitute a conventional default — and drop/swallow unreadable resize events rather than failing the run.
[suggestion] stdin-only TTY selection diverges from the Windows frontend for redirected stdout
interactiveHostTerminal selects TTY whenever stdin is a real terminal; stdout is never consulted. The Windows frontend's shared terminal.Interactive() requires both stdin and stdout to be character devices. So cb tool > out.txt inside an interactive WSL session now runs the tool with a pty — emitting ONLCR \r\n and any TTY-only output such as colors/progress into out.txt — while the same invocation on Windows produces plain non-TTY output. wsl-process-contract.md documents this ("Stdout may be redirected"), so it is a deliberate contract choice rather than an oversight, and it does fix the hard-failure reported in round one. Flagging it once for the record because it is a real cross-frontend byte-level divergence; if intentional, no action needed.
[nit] Stale import edge in docs/architecture.md
The regenerated import-edge list claims wslrun -> …, terminal, …, but internal/wslrun does not import internal/terminal (actual edges: hostenv, lockfile, policy, registry, wsldocker, wslfs, wslpathmap, wslproject, wslshim, wslvolume) — a leftover from when interactivity used the shared helper. Since the doc presents the list as exact go list output, worth regenerating.
Summary
The round-two hardening is real and the design remains disciplined: attach-before-start, retained-until-wait container identity, proof-bound mounts/volumes/labels, conservative transport-ambiguous cleanup, exact exit propagation, and every operation repeating the endpoint proof. The activation gate stays intact; docs accurately describe the wired-but-gated state. CI is fully green at this head (Go toolchain was unavailable in my environment, so findings are from source inspection of e47dba5). One non-blocking observation for the interactive path: SIGTSTP is forwarded to the container while the shim itself cannot suspend, so Ctrl-Z in a terminal wedges the session (a stopped container makes even SIGINT pending until SIGCONT) — this mirrors docker attach semantics and the alternative of letting the shim stop would strand a retained running container, but it is worth being aware of before activation.
|
Addressed the complete second-round review at unsigned head
Added regressions for unknown initial TTY size, zero/error dimension classification, and both-stream TTY selection. Validation on current head:
@CherylSnowVeil please re-review current head |
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of head 36f6965 (third round — initial review covered 41f4924, second e47dba5).
Validation of previous findings
All three round-two findings are resolved, verified against source rather than the replies:
- Zero/unreadable terminal dimensions (important): fixed.
usableTerminalSize(internal/wslrun/terminal_linux.go:141) now classifies a failed or zero-dimensionTIOCGWINSZas unknown.prepareHostTerminalrecords 0x0 rather than failing, the initial resize is skipped when dimensions are unknown (runner.go:170), and an unmeasurableSIGWINCHis dropped atterminal_linux.go:102-107instead of aborting the session.TestUsableTerminalSizeandTestExecuteToolSkipsUnknownInitialTTYSizepin the behavior. - stdin-only TTY divergence (suggestion): adopted.
interactiveTerminalPairrequires a realTCGETSresult on both stdin and stdout (terminal_linux.go:54-64), matching the Windows frontend's two-stream rule —cb tool > out.txtnow selects non-TTY execution on both frontends.TestInteractiveTerminalPairRequiresBothStreamscovers all four combinations, and the contract doc was updated to match. - Stale architecture import edge (nit): fixed.
docs/architecture.mdnow listswslrun -> hostenv, lockfile, policy, registry, wsldocker, wslfs, wslpathmap, wslproject, wslshim, wslvolume, which I re-verified against the package's actual imports — noterminaledge remains.
The previously noted SIGTSTP observation is unchanged and remains non-blocking: forwarding TSTP to the container while the shim itself stays running mirrors docker attach semantics, and the alternative strands a running container.
Fresh findings (complete re-review of the current head)
[suggestion] stopEvents() restores default signal disposition before the cleanup window, reopening a catchable orphan path
In executeTool's deferred cleanup (internal/wslrun/runner.go:96-141), stopEvents() runs first — before stream.Close() and before the bounded signal(9)/wait/remove sequence (up to cleanupTimeout = 30s). ossignal.Stop restores default handling, so a SIGTERM/SIGHUP/SIGINT arriving during cleanup kills the shim instantly and can strand a still-running retained container — the exact orphan class the activation gate cites for SIGKILL, except these signals are catchable and the window is avoidable in-process. Moving stopEvents() after the removal block would keep the signals suppressed through cleanup. The trade-off is that the shim then appears signal-immune for the cleanup bound (the user retains SIGKILL), so this is a judgment call rather than a defect; given the gate explicitly exists for this orphan class, narrowing the window seems worthwhile.
[nit] pythonBootstrap is duplicated verbatim across both frontends
The bootstrap literal if [ ! -x /venv/bin/python ]; then ... __CB_PIP__ ... exists identically in internal/wslrun/plan.go:28 and internal/dockerrun/dockerrun.go:266. Nothing ties them together; a future fix to one (e.g., the sentinel ordering) silently diverges Windows and WSL Python behavior. A shared constant or a parity test would prevent drift.
Summary
The round-three hardening is real and verified in code: terminal size is advisory end to end, TTY selection now matches the Windows contract on both streams, wait remains authoritative over completion-race 404/409s, and every boundary repeats the fixed layout, managed-binary, registry, shim, volume and socket/peer proofs. The activation gate stays intact — RequireFrontend still rejects WSL2Native before the dispatch branch, so the wired runtime cannot be exercised in production, and the docs accurately describe the wired-but-gated state including the retained-container orphan caveat. CI is fully green at this head; Go toolchain was unavailable in my environment, so findings are from source inspection of 36f6965. No blocking or important issues found.
|
Addressed both third-round observations at unsigned head
Validation:
@CherylSnowVeil please re-review current head |
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of head 804a6cb (fourth round — initial review covered 41f4924, second e47dba5, third 36f6965).
Validation of previous findings
Both third-round observations are resolved, verified against source rather than the replies:
stopEvents()restoring default disposition before cleanup (suggestion): adopted.internal/wslrun/runner.go:135-143now invokesstopEvents()only afterstream.Close(), terminal restoration, the boundedsignal(9)/waitsequence and proof-bound removal. Catchable HUP/INT/TERM remain intercepted through the entire cleanup window, closing the in-process variant of the orphan path the activation gate exists for.TestExecuteToolForceStopsOwnedContainerAfterStreamFailurenow assertsremoveprecedesstop-events, anddocs/wsl-process-contract.mddocuments the guarantee including the remaining uncatchable-SIGKILL caveat.pythonBootstrapduplicated across frontends (nit): fixed.internal/dockerrun/dockerrun.go:35-37exportsPythonBootstrap, the Windows frontend consumes the same constant (dockerrun.go:270), andTestPythonBootstrapMatchesWindowsFrontendpins exact parity without adding a production dependency edge between the parallel frontends.
The earlier SIGTSTP observation is unchanged and remains non-blocking: TSTP is still forwarded to the container while the shim itself cannot suspend, which mirrors docker attach semantics; the alternative strands a retained running container.
Fresh findings (complete re-review of the current head)
[nit] A failed stream.Close() diagnostic is dropped when the run already failed
In the deferred cleanup (internal/wslrun/runner.go:98-101) the attach-stream close error is assigned to retErr only when retErr == nil; on an already-failed run it is silently discarded. term.restore and lifecycleErr on the same path are joined unconditionally, so this one case is inconsistent with the PR's otherwise careful preserve-every-cleanup-diagnostic policy. Optional: retErr = errors.Join(retErr, fmt.Errorf("close native WSL attach stream: %w", err)) matches the surrounding treatment. Not blocking.
Summary
The round-four changes are exactly as described and verified in code: catchable signals stay suppressed through the full bounded cleanup, and the Python bootstrap literal is parity-locked to the Windows frontend by test. The broader composition still holds up under a fresh pass — attach-before-start, retained-until-wait identity, wait-authoritative 404/409 completion-race arbitration, exact 0..255 exit propagation into osExit, advisory terminal sizing, two-stream TTY selection matching the Windows contract, proof-bound mounts/volumes/labels/socket checks, and conservative transport-ambiguous cleanup with joined diagnostics. The activation gate remains intact: requireHostFrontend still rejects WSL2Native before the dispatch branch, so the wired runtime cannot be exercised in production, and the docs accurately describe the wired-but-gated state including the retained-container orphan caveat. CI was partially pending at review time (zizmor and govulncheck green; build/test jobs still running); the Go toolchain is unavailable in my environment, so findings are from source inspection of 804a6cb. No blocking or important issues found — only the single nit above.
|
Addressed the optional fourth-round cleanup-diagnostics nit at unsigned head
Validation:
@CherylSnowVeil please re-review current head |
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of head bef9801 (fifth round — initial review covered 41f4924, second e47dba5, third 36f6965, fourth 804a6cb).
Validation of previous findings
The fourth-round nit is resolved, verified against source rather than the reply:
stream.Close()diagnostic dropped on already-failed runs (nit): fixed.internal/wslrun/runner.go:99-101now joins the attach-stream close error unconditionally viaerrors.Join(retErr, fmt.Errorf("close native WSL attach stream: %w", err)), matching theterm.restoreandlifecycleErrtreatment on the same path.TestExecuteToolForceStopsOwnedContainerAfterStreamFailurenow produces both a stream decode failure and a close failure in one run and requires both diagnostics.
The earlier SIGTSTP observation is unchanged and remains non-blocking: TSTP is still forwarded to the container while the shim itself cannot suspend, which mirrors docker attach semantics; the alternative strands a running container.
Fresh findings (complete re-review of the current head)
[important] A remote-close write failure on the stdin copy masks the tool's real exit status
internal/wslrun/runner.go:191-198 feeds io.Copy(stream, deps.stdin) and stream.CloseWrite() results into inputDone, and both the select loop (:233-237) and finishToolResult (:269-282) treat any completed non-io.ErrClosedPipe input error as fatal — deliberately, so genuinely truncated piped input is not reported as success.
The problem is the classification boundary: io.ErrClosedPipe is only produced by AttachStream.Write after a local close (stdinClosed/Close). When the daemon closes the attach connection because the container exited — the normal case for a tool that finishes without draining stdin — the in-flight or next write instead fails with *net.OpError wrapping syscall.EPIPE ("write: broken pipe"), and CloseWrite can similarly surface ENOTCONN/EPIPE on a reset connection. Neither matches the tolerated sentinel.
Concrete failure mode: yes | cb tool or cat bigfile | cb tool where the tool exits before consuming all of stdin (the standard SIGPIPE-style pipeline) — the EPIPE lands in inputDone essentially at container exit, before the wait round-trip completes. The inputErr case or finishToolResult's non-blocking read then reports "copy native WSL tool input: write unix …: broken pipe" and the invocation exits 120 instead of the tool's real status (including a successful 0). The same is reachable in TTY mode if the user types in the window between container exit and the wait result.
This is the same completion-race class the PR already fixed for resize/signal forwards (containerCompletionRace): forwarding stdin to a container that is already gone is best-effort, and the Engine wait is authoritative for the outcome. It also diverges from the Windows frontend, where exec.Cmd deliberately ignores stdin-copy failures after the child exits. A remote-close EPIPE is not truncated input — the tool exited; it consumed what it wanted — so it should not win over the tool status the way a real source-read failure should.
Remediation direction: distinguish source-read failures (keep reporting; they are genuine truncation) from sink-write failures caused by a gone peer — e.g., wrap stream.Write/CloseWrite so errors.Is(err, syscall.EPIPE) (and net.ErrClosed-class errors) maps to the same treatment as io.ErrClosedPipe, or make input errors non-fatal once wait has produced a result. A regression test could use a fake attach whose Write fails EPIPE while wait returns a real status.
Summary
The round-five change is exactly as described and verified in code: stream-close diagnostics are preserved on both success and failure paths, and the broader composition still holds up under a fresh pass — attach-before-start, retained-until-wait identity, wait-authoritative 404/409 completion-race arbitration, exact 0..255 exit propagation, advisory terminal sizing, two-stream TTY selection matching the Windows contract, proof-bound mounts/volumes/labels/socket checks, and conservative transport-ambiguous cleanup with joined diagnostics. The activation gate remains intact: requireHostFrontend still rejects WSL2Native before the dispatch branch, so the wired runtime cannot be exercised in production, and the docs accurately describe the wired-but-gated state including the retained-container orphan caveat. CI is fully green at this head; the Go toolchain is unavailable in my environment, so findings are from source inspection of bef9801. One important issue above — the same completion-race shape as the already-fixed resize/signal case, this time on the stdin forward path.
|
Addressed the fifth-round important finding at unsigned head
Validation:
@CherylSnowVeil please re-review current head |
CherylSnowVeil
left a comment
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Re-review of head 42473ab (sixth round — initial review covered 41f4924, second e47dba5, third 36f6965, fourth 804a6cb, fifth bef9801).
Validation of previous findings
The fifth-round important finding is resolved, verified against source rather than the reply:
- Remote-close stdin sink failure masking the tool exit status (important): fixed.
copyToolInput(internal/wslrun/runner.go:269-279) now runsio.CopythroughinputSinkWriter(runner.go:255-267), which normalizes only attach-sink errors classified byinputPeerClosedtoio.ErrClosedPipe. The classifier (internal/wslrun/input_error.go,input_error_linux.go) coversio.ErrClosedPipe,net.ErrClosed, and LinuxEPIPE/ENOTCONN/ECONNRESET— the exact*net.OpError-wrapped sentinels the real hijacked Unix connection produces on a dead peer. Source-read failures bypass the wrapper and remain fatal,CloseWriteis still attempted unconditionally, and non-peerCloseWriteerrors remain fatal. Both tolerance sites (runner.go:229select case andfinishToolResultatrunner.go:295) already acceptio.ErrClosedPipe, so a tool that exits without draining stdin —yes | cb tool,cat bigfile | cb tool, TTY input racing container exit — now reports the authoritative Engine wait status instead of "copy native WSL tool input: … broken pipe" and exit 120.TestExecuteToolPreservesExitAcrossClosedInputSinkexercises the failure end-to-end with a non-empty stdin (verifiedsuccessfulRunDependenciesfeedsstrings.NewReader("input"), soWriteis really invoked),TestCopyToolInputTreatsLinuxPeerCloseAsCompletionpins the errno set, andTestCopyToolInputPreservesSourceReadFailure/TestCopyToolInputPreservesUnexpectedSinkFailurepin both preserved-failure classes. The contract docs (docs/wsl-process-contract.md,docs/wsl.md) describe the same source-fatal/sink-tolerated boundary.
One note on the classification breadth: normalizing ECONNRESET on the sink does not mask a mid-run transport failure, because stdin and output share the same attach connection — a dead connection surfaces independently through the output-copy error path, while a clean container exit with a closed sink correctly defers to the wait result. The boundary is sound.
The earlier SIGTSTP observation is unchanged and remains non-blocking: TSTP is still forwarded to the container while the shim itself cannot suspend, which mirrors docker attach semantics; the alternative strands a retained running container.
Fresh findings (complete re-review of the current head)
None. The current revision was reviewed in full — runner orchestration, frontend identity proofs, plan/env/mount validation, terminal handling, the new input classifier, attach/create/remove proof-bound operations, RuntimeImageForToolAt, the main.go dispatch boundary, install reporting, docs, and the new tests — and no actionable issues were found.
Summary
The round-six change is exactly as described and verified in code: the stdin forward path now applies the same completion-race treatment the resize/signal forwards already had, without weakening the deliberate fail-closed behavior for genuine source truncation. The broader composition still holds under a fresh pass — attach-before-start, retained-until-wait identity, wait-authoritative 404/409 arbitration, exact 0..255 exit propagation, advisory terminal sizing, two-stream TTY selection matching the Windows contract, proof-bound mounts/volumes/labels/socket checks, and conservative transport-ambiguous cleanup with joined diagnostics and signals held intercepted through the whole cleanup window. The activation gate remains intact: requireHostFrontend still rejects WSL2Native before the dispatch branch (main.go:78-87), so the wired runtime cannot be exercised in production, and the docs accurately describe the wired-but-gated state including the retained-container orphan caveat. CI is green except the two Windows build jobs still pending at review time; the Linux test job that covers the new code passed. Go toolchain is unavailable in my environment, so findings are from source inspection of 42473ab. No blocking, important, or nit-level issues found.
Summary
host_mountsfail-closedValidation
gofmt -l .go vet ./...go test ./...go test -race ./...python -m unittest -v internal/registry/pipx_wrapper_test.py internal/cli/pipx_discovery_test.pypowershell.exe -NoLogo -NoProfile -ExecutionPolicy Bypass -File .\scripts\test-compare-startup-benchmarks.ps1Remaining WSL v2 activation gates
This intentionally does not enable production WSL tool dispatch or claim WSL release support. Proof-bound retained-container orphan reconciliation, the necessary native state-management surface, Windows-filesystem/WSL-filesystem and mixed-boundary integration coverage, and real WSL2 + Docker Desktop qualification remain.