State-aware exec streaming + typed-error contract gaps (FFI in-process + CLI executor framing)
- Dominant language
- Rust
- Stars
- 1.3k
- Forks
- 79
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 117
Description
### Description of the task
Track and fix the state-aware **exec** streaming contract gaps surfaced as review comments on #806 and #810, plus a related typed-error gap surfaced on #1045 (defect 4). These are four distinct failure modes that share one theme — how exec output and typed errors are framed across the executor/library boundary. Defects 1-3 are best fixed together because they touch the same code; defect 4 is independent and can land on its own.
Defects 1-3 all still stand on `main` after #829 ("Relay live exec pipes to executor stdio"), which added the `ExecConsumer` (`Executor` vs `Library`) plumbing and a generic `relay_exec_to_stdio`, but did **not** update WSLc's `exec` (it takes `_consumer: ExecConsumer` and still returns null pipes) or change the executor's stdout error framing.
**1. The `require_experimental_optin` gate blocks the in-process (FFI/Rust-SDK) path.**
`require_experimental_optin` (`src/core/mxc_engine/src/state_aware.rs:43,52,73,109`) rejects the experimental backends (WSLc / IsolationSession / WindowsSandbox) unless `request.experimental_enabled` is set. The CLI sets this from `--experimental`, but the JSON/FFI entry points parse via `convert_wire_state_aware` → `convert_wire_config`, which hardcodes `experimental_enabled = false` (`src/core/wxc_common/src/config_parser.rs:1335`). `mxc_sdk::exec_sandbox` / `mxc_state_aware_exec` expose no opt-in. Result: every in-process SDK/FFI exec against a `wslc:` (or IsolationSession/WindowsSandbox) sandbox returns `backend_unavailable` before dispatch.
_Fix (small, ~2 lines):_ set `experimental_enabled = true` on the trusted JSON/FFI entry points after parse (linking the native library is itself the opt-in), or thread an explicit opt-in through the SDK/FFI request path.
**2. WSLc `exec` returns null pipe handles and blocks (the `ExecConsumer::Library` path).**
`WslcStateAwareRunner::exec` (`src/backends/wslc/common/src/state_aware.rs:135-206`) relays the daemon stream synchronously into process-global stdout/stderr, blocks to exit, then returns an `ExecHandle` with `null_pipe_handle()` for stdout/stderr/stdin. Post-#829 it receives an `ExecConsumer` but ignores it (`_consumer`, line 140). Correct for the CLI/`ExecConsumer::Executor` path (self-relay + null pipes is the documented call-through), but the FFI path (`state_aware_dispatch.rs:97` passes `ExecConsumer::Library`) wraps the handle in `ExecSandboxProcess`, which expects **live pipes + non-blocking spawn** — so `mxc_state_aware_exec` blocks until exit and `take_stdout`/`take_stderr` return `None`, contradicting the live `MxcSandboxProcess` contract.
_Fix (moderate, ~150-250 lines):_ when `consumer == ExecConsumer::Library`, create real OS pipes, run `exec_streaming` on a background thread whose callback writes to the pipe write-ends, and return an `ExecHandle` carrying the read-ends + a waiter that joins the thread. Keep the `ExecConsumer::Executor` branch on the internal self-relay (or hand live pipes to the generic `relay_exec_to_stdio` #829 landed). Add stdin forwarding and handle-ownership care. IsolationSession and WindowsSandbox share the identical null-pipe pattern and should adopt the same shared helper.
**3. Executor stdout error-framing collision on post-admission failures (`ExecConsumer::Executor`).**
On the CLI/executor path, exec streams user bytes to stdout as they arrive. On a **post-admission** failure (timeout mid-output, daemon truncation, backend failure after some bytes streamed), `exec_streaming` returns `Err`, and `run_state_aware_main` finalizes via `StateAwareExit::Error(json)` → `println!(envelope); exit(1)` (`src/core/wxc/src/main.rs:457,484`) — appending the JSON error envelope to the **same** stdout that already carries user output. On the SDK side, `execInSandboxAsync` calls `tryParseErrorEnvelope`, which runs `JSON.parse(stdout.trim())` over the **whole** buffer (`sdk/node/src/state-aware-helper.ts:172-185`); mixed `[user output] + {"error":…}` is not valid JSON → returns `null` → no typed `MxcError` is thrown → the call resolves with `{ exitCode: 1, stdout: }`. Pre-admission failures (bad policy, sandbox not found) still stream nothing and produce a clean envelope, so only post-admission failures are affected.
_Fix (moderate; coordinated Rust + SDK):_ give the exec path separable framing for post-admission failures — e.g., route the exec error envelope to **stderr** (leaving user output as the sole owner of stdout) with the SDK reading the typed error from stderr, or length-prefix/frame the streams. Whatever is chosen must keep pre-admission envelopes working and keep the CLI human-readable.
**4. The daemon protocol flattens every backend failure to `backend_error`, discarding its typed classification.**
Surfaced as a review comment on #1045. #794 gave the WSLc backend a typed `WslcError` whose variants carry a `FailurePhase` (`Unavailable` → `BackendUnavailable`, `Rejected` → `Rejected`, etc.), so the **one-shot / streaming** surface now reaches the Rust SDK with an accurate code via `mxc_engine::dispatch::map_spawn_error`. The **state-aware** surface does not, because the classification is destroyed crossing the daemon's named pipe: `sr_err` (`src/backends/wslc/daemon/src/session_manager.rs:59-61`) reduces the step helper's `ScriptResponse` to `anyhow!(resp.error_message)`, dropping `failure_phase`; that becomes `WorkerError::Backend` → `ErrKind::Backend` (`:82`); and `map_daemon_error` (`src/backends/wslc/common/src/state_aware.rs:299-301`) maps `ErrKind::Backend` to `MxcError::backend_error`.
Concretely: provisioning on a host with missing WSLc components returns `backend_error` to state-aware Rust SDK callers, where the one-shot surface returns `backend_unavailable`. There is no local preflight that could catch it earlier — `state_aware.rs` has no `ensure_host_supported` / `load_sdk_checked` call, and its only `backend_unavailable` (`connect_daemon`) covers an unreachable daemon, a different condition. So a caller still has to string-match the message to distinguish an unusable host from a rejected policy.
_Fix (moderate; daemon wire change):_ widen `ErrKind` with `Unavailable` and `Rejected` rather than plumbing `FailurePhase` through — `failure_phase` is a `ScriptResponse` concept that the state-aware boundary does not use, whereas `map_daemon_error` already maps `ErrKind` 1:1 onto `MxcError` codes, so the client side is a new match arm. Requires: a `PROTOCOL_VERSION` bump, somewhere on `WorkerError` to carry the classification so `sr_err` stops discarding it, skew handling for an **already-running** persistent daemon speaking the old protocol, and a `docs/wsl/wslc-state-aware.md` update.
### Additional context
- **Defects 1 and 2 are coupled — do not land #1 without #2.** The gate in #1 is currently the only thing making #2's dead-pipe/blocking bug unreachable from the FFI path; fixing #1 alone exposes the FFI WSLc-streaming path directly onto the null-pipe blocking `exec`.
- **Not a #806 / #810 blocker.** #806 ships only the CLI/daemon streaming path, where null pipes are the intended contract (validated by the 57/57 state-aware E2E suite); #810 is TypeScript-only. Defects 1-3 are pre-existing behavior on `main` (2b/#801 + #829) — neither PR introduces them, and no in-tree consumer uses the FFI WSLc-streaming path today (C# SDK is run-to-completion only; TS SDK uses the CLI/PTY path).
- **#829 is the foundation, not the fix.** It added `ExecConsumer` and a generic `relay_exec_to_stdio` (for backends that surface live pipes) but left WSLc on the internal-relay/null-pipe path and did not change the executor's stdout error framing.
- **Defect 4 is independent of 1-3 and can land on its own.** It is the only item here that is not exec-specific: the flattening affects every phase (provision / start / stop / deprovision as well as exec), because all of them return through the same `sr_err` → `ErrKind::Backend` → `backend_error` chain. It is grouped here because it is the same underlying concern — typed errors losing fidelity across the executor/library boundary.
- **Defect 4 is pre-existing and was not introduced by #1045.** #794's scope was the one-shot/streaming surface; the daemon path flattened errors before that PR and still does. #1045 was left as-is deliberately rather than growing a wire-protocol change late in review.
- Source review comments: #806 (`mxc_engine/state_aware.rs:109`, `wslc/common/src/state_aware.rs:203`), #810 (`wslc/common/src/state_aware.rs:170`, comment id 3778810220), and #1045 (`wslc/common/src/error.rs:6-12`).
Contributor guide
Assessment
This issue has not been assessed yet.