You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If this PR changes Cargo.lock, the dependency-feed-check check passes (see docs/pull-requests.md)
📋 Issue Type
Bug fix
Feature
Task
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.
If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.
Branden Bonaby (bbonaby)
changed the title
[SDK] Add caller-owned PTY APIs for IsolationSession
[SDK] Add caller-owned PTY APIs for backends
Oct 2, 2026
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds caller-owned pseudo-terminal (PTY) execution paths for IsolationSession across the Rust engine/public SDK surface, the C FFI layer, and the Node.js/.NET SDKs, including resize and merged output support.
Changes:
Introduces MxcPty / PtySize abstractions and PTY-aware dispatch in wxc_common + mxc_engine, with IsolationSession backend implementations.
Extends mxc_ffi with PTY spawn/exec + resize APIs and routes Node.js/.NET PTY features through the existing lifecycle handle.
Adds Node.js and .NET SDK APIs, docs, and unit tests for PTY spawning and in-container PTY exec.
File
Description
src/ffi/mxc_ffi/src/streaming.rs
Switches sandbox handle to a trait object to support both piped and PTY-backed live processes; exposes PTY resize hook.
src/ffi/mxc_ffi/src/state_aware.rs
Adds state-aware exec entry point that returns a caller-owned PTY handle.
src/ffi/mxc_ffi/src/pty.rs
New FFI entry points for one-shot PTY spawn + PTY resize.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
The reason will be displayed to describe this comment to others. Learn more.
Hmm, passing the terminal in could simplify some backends, but I'm not sure it would make all of this code go away. Each type of sandbox connects a process to a terminal differently. In particular, IsolationSession creates that terminal connection inside its own service and gives us access to it afterward, so the caller does not already have something it can pass in in that case no?
we could technically support caller provided terminals as an additional option where it works but then we end up with some fragmentation for the ones that don't support it.
Design: I will restate that I do no think this is the right approach.
1: PTY should be an input parameter to a Spawn function. (This simplifies everything, is more general, and less code.)
2: I don't think this is correctly generalizing the "attached" behavior that the IsoSession wants.
They want to spawn process in "the current TTY" and maybe get a pipe to drive the console (they don't necessarily want to manage a PTY handles). I think they'd be fine with CreateProcessW(inherit=TRUE), tbh.
That said, I think this can work and enable the scenario.
Jeff Whiteside (@jsidewhite) Passing a PTY makes sense for backends that can consume one, but what concrete cross-SDK object would we pass? ProcessContainer would probably need HPCON , Unix needs file descriptors, and IsolationSession has no PTY-injection API since the service creates its own ConPTY. Are you proposing backend-specific PTY inputs for ProcessContainer/Unix while retaining the separate attached path for IsolationSession?
Remove the private portable-pty adapter and delegate directly to MXC's PTY process contract so Cargo builds use only feed-approved dependencies. Import the V1 .NET namespace in the PTY tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 433e5b70-c2bd-496d-9b8d-bd2085bb2706
Include state-aware PTY handles in the ABI safety contract
src/ffi/mxc_ffi/src/pty.rs:99
This safety contract excludes handles returned by mxc_state_aware_exec_pty, even though that API returns the same PTY-backed MxcSandbox and the Node/.NET state-aware paths resize it through this function. Include both valid handle-producing APIs so C callers are not told that a supported state-aware resize violates the ABI contract.
pty_process starts the workload before this initial ResizeConsole call. A fast command can therefore observe the service's default dimensions—or perform side effects before a resize failure is returned—even though the SDK exposes this value as the initial terminal size. The dimensions need to be applied as part of launch, or workload execution must be gated until the resize succeeds.
Branden Bonaby (bbonaby)
changed the title
[SDK] Add caller-owned PTY APIs for backends
[SDK] Add caller-controlled PTY process APIs for IsolationSession
Oct 3, 2026
This existing-container example can block forever after WaitAsync() when a background descendant retains the ConPTY output. The PTY exposes StandardOutputCloser specifically to end that read after foreground completion; acquire it after Output, close it after the wait, and only then await outputTask.
Fix broken intra-doc link to state-aware entry point
src/ffi/mxc_ffi/src/state_aware.rs:286
The referenced function does not exist; the streaming state-aware entry point is mxc_exec_state_aware_json. This creates a broken intra-doc link and fails to direct C callers to the ownership contract being reused.
Restrict PTY API to supported IsolationSession backend
sdk/node/src/state-aware.ts:383
The new public type signature advertises PTY support for every state-aware backend, including SandboxId<'wslc'>, even though native dispatch and the updated lifecycle documentation make this API IsolationSession-only. This defeats the SDK's backend-branded ID typing and turns an unsupported call into a runtime error. Restrict the signature to the supported backend until another implementation is added.
Preflight Node version before spawning PTY processes
sdk/node/src/bindings/pty.ts:184
The shared PTY creation path invokes the native spawn before applying the native-stdio Node version gate used by the existing streaming APIs (streaming.ts:467-490,559). On Windows Node 24 versions below 24.21.0, this can create an IsolationSession and only then fail while adopting its handles. Run the same ensureSupportedNodeVersion preflight before both PTY spawn entry points so unsupported runtimes fail without launch side effects.
Constrain API generics to isolation_session branded IDs and configs
sdk/node/src/state-aware.ts:380
The public signature accepts branded WSLC IDs/configs even though this API is IsolationSession-only, while the updated lifecycle documentation declares SandboxId<'isolation_session'>. This defeats the SDK's compile-time backend branding and turns a statically invalid call into a native runtime failure. Constrain both generic arguments to isolation_session.
PTY poll error leaves uncancelled drain thread and pipe handle
src/core/mxc-sdk/src/sandbox.rs:356
A non-timeout try_wait error returns here before the automatically spawned output drain is cancelled or joined. Because dropping the closer does not call close, a blocked PTY reader (especially one held open by a descendant) leaves a detached thread and native pipe handle indefinitely. Preserve the poll error, run the same drain cleanup used after final wait, and then return it.
Branden Bonaby (bbonaby)
changed the title
[SDK] Add caller-controlled PTY process APIs for IsolationSession
[SDK] Add caller-controlled PTY process APIs
Oct 3, 2026
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The Node state-aware PTY signature incorrectly advertises support for backends that always reject the operation.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Narrow generic backend type to supported IsolationSession backend
sdk/node/src/state-aware.ts:383
The public generic advertises PTY support for every state-aware backend, so TypeScript accepts wslc: and Windows Sandbox IDs even though this implementation and the lifecycle documentation are IsolationSession-only and those calls always fail natively. Narrow the signature to the supported backend, as execInSandbox already narrows its backend set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
MxcPtyProcessAPIs across the Rust, Node.js, and .NET SDKs.mxc_ffilive-process handle.ContainerConfigin the Node.jsspawnWithPtyAPI so containment, command, and policy remain config-owned.exec_attachedas the blocking API for attaching directly to the caller's current console.flowchart LR Caller[Caller] Rust[Rust SDK] Node[Node.js SDK] DotNet[.NET SDK] FFI[mxc_ffi opaque handle] Engine[mxc_engine] Isolation[IsolationSession] ConPTY[Service-owned ConPTY] Pty[MxcPtyProcess] Caller --> Rust Caller --> Node Caller --> DotNet Rust --> Engine Node --> FFI DotNet --> FFI FFI --> Engine Engine --> Isolation Isolation --> ConPTY ConPTY --> Pty Pty -->|input / resize / wait / kill| Caller Pty -->|merged output| Caller🔗 References
🔍 Validation
.\build.bat --with-isolation-session— passed.npm run buildfromsdk/node— passed with Node.js 24.cargo fmt --all -- --checkfromsrc— passed.mode con;exit 7producedexitCode=7;timedOut=false.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow