Repository navigation
Let an in-process caller opt in to the experimental backends - #954
Merged
adpa-ms merged 4 commits intoAug 19, 2026
Merged
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds explicit experimental-backend opt-in support to in-process state-aware APIs.
Changes:
- Threads the opt-in through Rust SDK, engine, and C ABI entry points.
- Prevents provisioning when the C ABI output pointer is null.
- Adds opt-in tests and updates lifecycle documentation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/ffi/mxc_ffi/src/state_aware.rs |
Extends C ABI and tests. |
src/core/wxc_common/src/state_aware_backend.rs |
Updates backend contract docs. |
src/core/wxc_common/src/exec_stream.rs |
Clarifies streaming behavior. |
src/core/mxc-sdk/tests/state_aware.rs |
Tests opt-in propagation. |
src/core/mxc-sdk/src/sandbox.rs |
Updates lifecycle documentation. |
src/core/mxc-sdk/src/lib.rs |
Extends public Rust APIs. |
src/core/mxc-sdk/README.md |
Documents new signatures. |
src/core/mxc_engine/src/state_aware.rs |
Applies opt-in after parsing. |
src/backends/isolation_session/common/src/state_aware.rs |
Updates testing-gap notes. |
sdk/dotnet/Microsoft.Mxc.Sdk/MxcLifecycle.cs |
Adapts P/Invoke calls. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
added 4 commits
August 19, 2026 10:41
Every state-aware backend is experimental -- WindowsSandbox, IsolationSession and WSLc -- and the engine already refuses all three unless the request carries the experimental flag. No in-process caller could set it: `config_parser` hardcodes `experimental_enabled: false` for state-aware requests, and the only setter sits on the one-shot builder. So the entire in-process state-aware surface was unreachable, refused before dispatch on every call. The two JSON entry points now take the flag alongside the `dry_run` they already took, and it is applied where the parser leaves off. Nothing about the gate changes: `require_experimental_optin` was already written and already applied by both dispatchers, so this rung supplies the missing way to satisfy it rather than inventing a refusal. It is an API parameter rather than a field in the request JSON because the alternative lets a config grant itself experimental access, which is what the flag exists to withhold. The C ABI mirrors the `dry_run` precedent beside it: an `i32` compared against zero. Three doc comments described the absence as permanent and are now false. Two sit in crates this change does not otherwise touch, which is why they were found by searching for the contract rather than by reading the diff: the SDK's `WaitOutcome::TimedOut` note, and the `ExecConsumer` contract in `wxc_common`. The third, on the IsolationSession backend, was only half false -- its end-to-end testing gap has two causes, and this closes exactly one. The remaining one is named there now, so the note still says what it takes to close it. The refusal carries no API-failure detail, and a test pins that: nothing was in flight when the gate fired, so it takes the same shape as a malformed request. Measured rather than assumed: dropping the flag on the way down, so the parameter is accepted and ignored, fails exactly one test at each layer -- the SDK's and the C ABI's `the_optin_admits_an_experimental_backend` -- and leaves every other test green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02c36d36-fb3d-446f-9108-4e4a01b01af1
The C ABI now requires the opt-in, so these two call sites have to pass something. They pass zero, which keeps every method on this type failing at the engine's experimental gate exactly as it did before the parameter existed. That is deliberate rather than conservative-by-default. MxcLifecycle builds every request with containment = isolation_session, and two separate things stand between it and a working lifecycle. The project builds mxc_ffi without the engine's isolation_session feature, so that backend has no dispatch arm in the library this project produces. And the request shape predates that backend's Preview migration on more than one count. So opting in would not make this surface work; it would only change which refusal the caller sees. Flip the value in the change that clears both obstacles -- the dedicated C# workstream, which owns this type -- so the surface starts working and stops misreporting in the same commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02c36d36-fb3d-446f-9108-4e4a01b01af1
mxc_state_aware ran the requested phase and only then checked whether it had anywhere to put the answer. With no opt-in that ordering was harmless, because every state-aware backend is experimental and the gate refused all of them before any work happened. Making the opt-in reachable makes the ordering reachable too: a non-dry-run provision with a null out-pointer would create a real sandbox, discard the response carrying its id, and return null_argument -- leaving a sandbox nothing can reclaim, because the only handle to it was thrown away. The check moves ahead of the dispatch, where the sibling mxc_state_aware_exec has always had it. The effect is not observable from a unit test: proving no phase ran needs a live backend, and there is no seam to fake one. The evidence is the sibling entry point's ordering and the code path itself. The streaming entry point had no test that passed the opt-in. Both entry points reach the same gate by separate routes, so hardcoding the flag in exec alone left every test green -- the earlier mutation run only covered the shared helper and missed this. A wsb id discriminates without a host: the gate refuses it without the opt-in, and with the opt-in it reaches unsupported_phase because Windows Sandbox has no streaming exec arm. A wslc id would not work here, since its feature-off arm answers with the same code the gate returns. Measured: with the flag dropped in exec_state_aware_json alone, exactly the new test fails at each layer and every envelope test stays green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02c36d36-fb3d-446f-9108-4e4a01b01af1
Several documentation claims in and around this crate outran the code, and the opt-in made three of them load-bearing by putting the state-aware surface within a library caller's reach for the first time. Which backend serves a streaming exec. exec_sandbox reaches dispatch_state_aware_exec, which asks each backend it dispatches to for an ExecConsumer::Library handle. IsolationSession serves it. WSLc refuses that consumer on the first line of its exec, before doing any work, because it relays to the executor's stdio and cannot serve an in-process caller -- it streams only under ExecConsumer::Executor, which is the wxc-exec relay. Windows Sandbox refuses it the same way and has no streaming arm to dispatch to at all. So IsolationSession is the only backend serving this path, and the crate docs, the README, the trait contract and the exec-stream adapter now agree on that. None of those refusals is new: the code did not drift, the documentation did, and an earlier commit on this branch acted on the opposite belief before it was corrected. How many state-aware backends there are. Three -- IsolationSession, WSLc and Windows Sandbox -- since the latter two gained the lifecycle. The claim that IsolationSession is the only one appeared in seven places; the two in this crate are corrected here. One sat directly above tests that drive windows_sandbox through a wsb sandbox id, and one contradicted a paragraph a hundred lines below it in the same file. The adapter's module doc likewise said the other two return null handles on every path, when under Library they return no handle at all -- the same file already said so further down. Five further copies live in the C# SDK, where they describe a surface that really does expose one backend; they belong to the workstream that widens it. Which exec path the end-to-end suites cover. They drive ExecConsumer::Executor, because they go through wxc-exec. The Library path exec_sandbox uses has no end-to-end coverage at all, which the testing-gap note on IsolationSession's exec records. The test module claimed the suites covered the real exec paths; it now names both consumers and explains why its own assertions stop at the facade's contract. What compiling a backend in requires, and how each absence is refused. Windows Sandbox requires nothing: its dependency is not optional and its arm is gated on the target OS alone. IsolationSession and WSLc each need an engine feature, and this crate forwards only the latter. Those two absences are refused differently, which the timeout note now states rather than conflating: without the opt-in the gate rejects the request before dispatch, while without the feature it passes the gate and finds no arm, which is a different code. The README carries the matrix and tells callers to branch on the code for that reason. Its provision example carries the canonical unrestricted-network acknowledgment, without which that backend refuses the phase -- the example previously advertised a sandbox id it could not have returned. Why the C# lifecycle keeps passing zero. Two obstacles stand between that type and a working lifecycle. The project builds mxc_ffi without the engine's isolation_session feature, so the request ends at unsupported_phase without reaching the backend, while build.bat --with-isolation-session enables it across the workspace and cargo unifies it into the engine this library links -- so the shipped binaries and the tested ones differ. And the request shape predates the Preview migration on more than one count. Flipping the value alone would trade backend_unavailable for unsupported_phase, which the C# tests already tolerate, so the change would look green while fixing nothing. Both obstacles are named at the call site, and the file is otherwise untouched: the only lines removed from it are the two native calls whose signature changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02c36d36-fb3d-446f-9108-4e4a01b01af1
adpa-ms
force-pushed
the
user/adibpa/copilot-state-aware-experimental-optin
branch
from
August 19, 2026 18:09
1030b29 to
8263a4d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/ffi/mxc_ffi/src/state_aware.rs:372
- This test does not cover the ordering regression fixed above. With
experimentalset to0, the experimental gate prevents backend dispatch even if the null-output check is moved back afterstate_aware_inner, so the old orphaning implementation still passes. Add a test using an injected/mock dispatcher that records no invocation for a null output pointer with the opt-in enabled; invoking a real backend here would be host-dependent and could recreate the leak.
let status = unsafe { mxc_state_aware(j.as_ptr(), 0, 0, ptr::null_mut()) };
src/core/mxc-sdk/README.md:328
- This error-code guidance contradicts both the example above and the dispatcher. Only feature-disabled WSLc has a dedicated
BackendUnavailablearm; feature-disabled IsolationSession falls through toUnsupportedPhase(mxc_engine/src/state_aware.rs:79-93, 110-135). Document the backend-specific mapping so callers do not branch on the wrong code.
`exec` in-process. Branch on the error code: a backend whose feature is not
compiled in answers `ErrorCode::BackendUnavailable`, and one with no arm on the
path you called answers `ErrorCode::UnsupportedPhase`.
This was referenced Sep 28, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Every state-aware backend is experimental, and the engine refuses all of them unless the request carries the experimental flag. No in-process caller could set it: the parser hardcodes
experimental_enabled: falsefor state-aware requests, and the only setter sits on the one-shot builder. The in-process state-aware surface was unreachable, refused before dispatch on every call.run_state_aware_json,exec_sandbox, and the two C ABI entry points now take the opt-in beside thedry_runparameter they already had, applied after parsing. The C ABI usesi32compared against zero, matchingdry_run.Four public signatures change: two on the Rust SDK, two on the C ABI.
Notes for reviewers
mxc_state_awarechecked its out-pointer after running the phase. The opt-in makes that reachable: a non-dry-run provision with a null out-pointer would create a sandbox and discard the id needed to reclaim it. The check now runs first, asmxc_state_aware_execalways did.MxcLifecyclepasses0. That project buildsmxc_ffiwithout the engine'sisolation_sessionfeature, and the request shape it emits predates that backend's Preview migration, so opting in would change which error it returns rather than make it work.ExecConsumer::Libraryis served by IsolationSession alone; WSLc and Windows Sandbox refuse it on entry. Comments in this crate claimed otherwise.🔗 References
Resolves #953
🔍 Validation
wxc_host_prep(16 tests) run elevated: fmt, clippy-D warnings, workspace tests with the isolation-session feature on and off, arm64 cross-build, six versioning gates, both C# SDK gates,dotnet test(49), and the Node build, unit, pack and integration legs.IsoSessionCli list-usersagreed atFound 0 agent user(s).✅ Checklist
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow