Report a timeout and a refused kill through the state-aware exec handle

[adpa-ms]

📖 Description

ExecHandle's two closures could not express two things a backend knows. The waiter returned a bare exit code, so a workload killed at its deadline was indistinguishable from one that exited with whatever code the kill produced. The terminator returned nothing, so a platform that refused to terminate had no way to say so and the caller could only assume it worked.

The waiter now yields ExecOutcome::Exited(i32) | TimedOut, and the terminator returns Result<(), MxcError>. A timeout is an outcome — the deadline was spent while the process ran and the process is no longer running — whereas Err means the exit could not be determined; collapsing them costs the caller the ability to tell them apart. TimedOut deliberately does not claim the backend killed the process: a workload that overruns its deadline and then exits on its own has still missed it, and reporting the code it happened to produce would hide that. Only an in-process caller can observe TimedOut: ScriptResponse has no timeout field, so the executor keeps reporting Exited and the CLI is unchanged.

Two defects surfaced while making the IsolationSession backend answer honestly, both of which would have shipped silently:

The timeout could never be reported. The service arms its own per-process timer at creation from the same process.timeout and enforces it by killing with an ordinary exit code — the host suite pins it as exit code 1. Our wait started later with an equal duration, so the service always won and a genuine timeout arrived looking exactly like a normal exit. The streaming path now arms the service timer with a margin, making the caller's deadline fire first and leaving the service timer as a watchdog. A deadline too large to move behind disarms that watchdog rather than saturating back into equality. The margin is documented as an ordering heuristic, not a guarantee: nothing in the interface distinguishes a service-enforced kill from an ordinary exit.

Exit code 259 was ambiguous twice. ExitCode() reads STILL_ACTIVE (259) for a live process, but 259 is also a legal exit code, so neither that nor WaitForExit's -1 proves a timeout alone. Their conjunction establishes whether the process was live when sampled. The same ambiguity lived one function away in the shutdown ladder, which reads only ExitCode() — an exited process is no longer routed through it. A spent deadline is also sticky: if the wait returns its sentinel and the process is then seen to have exited with some other code, that code cannot be what the wait reported, so the deadline provably elapsed and reporting the exit would hide it. The sibling WSLc backend already draws this line, tracking deadline_elapsed separately from timed_out.

Backends that cannot serve an in-process caller now refuse before running anything. WSLc and Windows Sandbox previously ignored the consumer, ran the workload, wrote its output to the host process's own stdout, and only then returned a handle the adapter rejected — so the caller was told "unsupported" about side effects that had already happened, and a retry ran the command twice.

Every teardown path now terminates and reaps only if the kill was accepted, through one helper rather than four open-coded copies. Reaping after a refused kill blocks on a waiter that may never return, and script_timeout defaults to zero — INFINITE — so that is the default configuration. Two of those paths run during construction, before the caller holds a handle it could interrupt; one runs inside wait, where blocking reintroduces the stall it exists to avoid; the fourth is the all-null backstop, which cannot assume the workload finished, since the IsolationSession library path passes through whatever handles the service returned. Abandoning the waiter leaks a thread and a COM reference — the better trade against an unbounded hang in a case that already means the sandbox is not responding. A confirmed exit retires the terminator entirely.

Closes three of the four gaps #839 documented: the terminator now reports refusal, WaitOutcome::TimedOut is reachable, and teardown is bounded against a reported refusal. Untaken stdin is still only dropped. Still unbounded: an accepted kill that never took effect, which needs the backend to confirm the process died.

Before this change a state-aware exec could not report a timeout at all, so four statements in the SandboxProcess trait and the public Rust SDK promising a whole-tree kill were true, satisfied by the process-spawning backends alone. Making the outcome reachable from a backend whose only primitive is the foreground process turned them false; they now leave the reach to the backend and still name the tree kill where it is real.

🔗 References

Continues #829 and #839.

Related Issues

• State-aware exec streaming contract gaps (FFI in-process + CLI executor framing) #843 — related, not closed. Its defect 2 is partially addressed: WSLc's exec now returns a typed refusal for ExecConsumer::Library instead of silently returning null pipes and blocking. That is not the real-pipe streaming the issue asks for, but it does loosen its "do not land #1 without Adding Microsoft SECURITY.MD #2" coupling — if the experimental opt-in lands first, the FFI path gets a clean error rather than dead pipes. Defects 1 and 3 are untouched.

🔍 Validation

Full local gate green on this tree: fmt, clippy --workspace --all-targets -D warnings, build and test with isolation_session on and off, arm64 build, versioning scripts, C# SDK, Node SDK unit + integration, and the elevated wxc_host_prep tests.

Every behavioural claim is mutation-verified: 25 mutations, 23 detected. The two survivors are recorded rather than quietly dropped — the shutdown ladder's post-kill survivor check and the waiter-thread-spawn site, each needing something a unit test cannot arrange (a live process object; an OS refusing a thread). Mutations are checked for compiling, because one that fails to build exits non-zero and would otherwise read as a caught regression.

On a host with the OS-side isolation service: end-to-end suites 90 passed / 0 failed / 0 skipped across all three harnesses, an empty account-leak delta corroborated by IsoSessionCli reporting zero agent users, and the interactive terminal tests passed at the console.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated documentation (if applicable)

📋 Issue Type

• Task

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates state-aware exec to report timeouts and termination refusals while preventing unsupported library execs from running workloads.

Changes:

• Adds ExecOutcome and fallible termination.
• Improves IsolationSession timeout handling.
• Rejects unsupported in-process exec before execution.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
state_aware_dispatch.rs	Adapts executor relay to new outcomes.
state_aware_backend.rs	Defines updated exec contracts.
exec_stream.rs	Propagates timeout and kill results.
wslc/common/src/state_aware.rs	Rejects library exec early.
windows_sandbox/lifecycle/src/state_aware.rs	Rejects library exec early.
isolation_session/common/src/state_aware.rs	Reports timeout and termination results.
process_options.rs	Adds service timeout grace.
manager.rs	Classifies timeout versus exit.
mxc-state-aware-sandbox-api.md	Documents the new API contract.
state-aware-rust.md	Documents IsolationSession behavior.

Suppressed comments (2)

src/core/wxc_common/src/exec_stream.rs:215

• The waiter-thread spawn failure path also ignores a reported kill refusal and then invokes the waiter inline. With an infinite process wait, the API call never returns—the exact refusal-induced teardown hang this PR is intended to prevent. Only reap after an accepted termination, and handle ownership explicitly when no waiter thread can be created.

                let _ = terminator();
                if let Some(waiter) = waiter_slot.lock().ok().and_then(|mut w| w.take()) {
                    let _ = waiter();

src/core/wxc_common/src/exec_stream.rs:328

• If the discard thread cannot start and the terminator refuses the kill, join_waiter() can block forever (especially when scriptTimeout is zero). Check the terminator result here; on refusal, retain that state and detach the existing waiter instead of joining it, matching the bounded behavior implemented in Drop.

        if let Some(terminator) = self.terminator.take() {
            // Discarded: the drain-spawn failure is the error being returned.
            let _ = terminator();
        }
        let _ = self.join_waiter();

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/core/wxc_common/src/exec_stream.rs:549

• This message contradicts the new outcome contract: TimedOutAfterDeadline returns TimedOut when the process exited on its own after the deadline, so the sandbox did not necessarily terminate it. Preserve the distinction in the observable error text.

            "the exec timed out and the sandbox terminated it",

src/core/wxc_common/src/exec_stream.rs:472

• A process can finish and its waiter thread can return before the caller invokes wait/try_wait; in that state self.exit is still None, so kill() invokes the fallible terminator against an already-dead process and can report a spurious refusal. The previous infallible terminator hid this race. Join an already-finished waiter before terminating, and ensure the terminator treats a concurrent natural exit as success.

        if self.exit.is_some() {
            self.terminator = None;
            self.kill_refused = false;
            return Ok(());
        }

src/core/wxc_common/src/exec_stream.rs:90

• This field comment still promises a process-tree kill, but the newly supported IsolationSession terminator only reaches the foreground process. Describe this as backend-defined termination so the internal contract matches ExecOutcome and the implementation.

    terminator: Option<Box<dyn FnOnce() -> Result<(), MxcError> + Send>>,

src/core/wxc_common/src/sandbox_process.rs:129

• This caveat only qualifies timeout handling; SandboxProcess::kill above still unconditionally promises termination of the process and all descendants. ExecSandboxProcess::kill now invokes an IsolationSession terminator that can only target the foreground process, so the trait's kill contract remains false and should carry the same backend-specific reach qualification.

    /// **How far that termination reaches is the implementation's to state.**
    /// The process-spawning implementations kill the whole tree — the child
    /// leads its own process group on Unix and is assigned a job object on
    /// Windows. An implementation whose only primitive is the foreground
    /// process (the state-aware exec adapter, over a backend with no tree API)

src/core/mxc-sdk/src/sandbox.rs:37

• The new backend-specific qualification conflicts with this public type's existing kill promises: the Sandbox rustdoc still says “and its whole tree” and Sandbox::kill still says “Kill the child and its process tree.” The same Sandbox wraps state-aware exec handles, so update both statements to describe backend-defined termination reach.

    /// How far "gone" reaches depends on the backend. A process-spawning
    /// backend kills the whole tree. A backend whose only primitive is the
    /// foreground process — the state-aware `exec` path over IsolationSession —
    /// confirms that process, and a descendant the workload backgrounded is
    /// reclaimed when the sandbox is stopped and deprovisioned rather than here.