Phase 3a: decouple ProcessContainer backend selection from schema version by MGudgin · Pull Request #580 · microsoft/mxc

MGudgin · 2026-06-26T19:46:12Z

This change makes the Windows ProcessContainer AppContainer-vs-BaseContainer
choice entirely host-capability driven: BaseContainer is preferred when usable,
and AppContainer is the downlevel fallback. Schema version and
--experimental no longer affect this selection. It also incorporates updates
that clarify diagnostics and make the streaming fallback explicit.

Details:

Route the executor through the capability-driven dispatcher unconditionally
and remove the schema-version/experimental gate and obsolete parser helper.
Select the SDK streaming backend using the same BaseContainer usability probe;
on downlevel hosts it explicitly creates an AppContainer runner.
Reword BaseContainer E_NOTIMPL diagnostics to describe an OS-disabled feature
without internal terminology, and remove stale schema-version guidance.
Update backend-agnostic regression coverage and tidy related comments.

Tests:

cargo test -p wxc_common -p appcontainer_common -p mxc-sdk
cargo clippy --workspace --all-targets -- -D warnings
cargo fmt --all -- --check
check-schema-codegen.js and check-sdk-types-codegen.js

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Copilot-Session: 6c58a97a-f189-43eb-b159-78965a028dc9
Generated-with: gpt-5.6-terra

Microsoft Reviewers: Open in CodeFlow

…comments This change addresses bbonaby's review feedback on the Phase 3a PR. It reworks the BaseContainer E_NOTIMPL diagnostic to describe the feature as simply not enabled on the OS build (pointing at the Windows experimental-features settings) rather than referencing internal "velocity keys", and removes internal iterative-review item numbers that had leaked into test comments. Details: - launch_diagnostics: drop "velocity keys" from the three user-facing E_NOTIMPL messages in favour of neutral "feature is not enabled on this OS build" wording, and surface disabled ids as "disabled feature flags". Internal identifiers (check_velocity_keys, REQUIRED_VELOCITY_KEYS) are unchanged. - sandbox.rs: strip the "#1"/"#2" review-item references from the ProcessContainer test comments and trim the process_container_request doc comment. Tests: - cargo test -p appcontainer_common (126 passed) and -p mxc-sdk (all passed); updated the api_not_implemented assertion to match the new message. - cargo fmt + clippy -D warnings clean on both crates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8

Copilot

Pull request overview

Phase 3a of the versioning remediation: removes schema-version (and --experimental) influence over Windows ProcessContainer backend selection so AppContainer vs BaseContainer is chosen purely by host capability, aligning executor and SDK behavior and unblocking a future schema-floor raise (Phase 3b).

Changes:

Route ContainmentBackend::ProcessContainer unconditionally through the capability-driven dispatcher in wxc-exec (removing schema/experimental gating and the plain-AppContainer arm).
Update mxc-sdk streaming dispatch to select BaseContainer vs AppContainer based on a direct host-capability probe (instead of schema version), and refresh tests/docs/comments accordingly.
Refactor SDK wire conformance tests: extract shared type-assertion helpers and add a new state-aware wire conformance oracle + wire it into npm run test:unit.

Show a summary per file

File	Description
src/core/wxc/src/main.rs	Always dispatch ProcessContainer through capability-driven fallback selection; remove version/experimental gate.
src/core/wxc_common/src/models.rs	Update ProcessContainer docstring to reflect capability-driven selection.
src/core/wxc_common/src/config_parser.rs	Remove `is_base_container_version`/threshold constant and associated unit tests.
src/core/mxc-sdk/tests/sandbox.rs	Reframe Windows integration tests as backend-agnostic ProcessContainer regression guards.
src/core/mxc-sdk/src/policy.rs	Update policy backend comment to describe ProcessContainer’s runtime resolution.
src/core/mxc-sdk/src/dispatch.rs	Change streaming ProcessContainer selection to use `is_base_container_usable()` probe.
src/backends/appcontainer/common/src/launch_diagnostics.rs	Refresh BaseContainer-not-enabled diagnostics messaging + adjust test assertion accordingly.
src/backends/appcontainer/common/src/base_container_runner.rs	Update BaseContainer “API unavailable” hint to remove schema/experimental-based guidance.
src/backends/appcontainer/common/src/appcontainer_runner.rs	Reword BFS-not-available error message to remove schema-version-based guidance.
sdk/tests/unit/wire-conformance.test.ts	Move shared type-level helpers to a dedicated module.
sdk/tests/unit/wire-conformance-state-aware.test.ts	Add state-aware public-types ↔ generated-wire conformance oracle.
sdk/tests/unit/conformance-helpers.ts	New shared type-level assertion helper module for conformance tests.
sdk/package.json	Include the new state-aware conformance test in `test:unit`.
docs/schema-codegen.md	Document the two conformance oracles and shared helper module.

Review details

Files reviewed: 14/14 changed files
Comments generated: 1
Review effort level: Low

… explicit This change fixes a misleading comment on the mxc-sdk streaming dispatch path, flagged in review of #580. The comment claimed the non-BaseContainer streaming fallback used "plain AppContainer", but AppContainerScriptRunner::new() defaults to FilesystemMode::Bfs, so the fallback is really the AppContainer+BFS tier (still dependent on bfscfg.exe), and the AppContainer+DACL tier is genuinely unavailable on the streaming path. Details: - Reword the comment to state that streaming offers only two tiers: BaseContainer when usable, otherwise AppContainer in BFS mode (the dispatcher's Tier 2), with no Tier 3 AppContainer-DACL fallback. - Construct the fallback runner via with_filesystem_mode(FilesystemMode::Bfs) instead of new(), so the BFS choice is explicit and cannot silently drift if the new() default changes. Tests: - cargo test -p mxc-sdk (all passed); clippy -D warnings and fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8

…comments This change addresses bbonaby's review feedback on the Phase 3a PR. It reworks the BaseContainer E_NOTIMPL diagnostic to describe the feature as simply not enabled on the OS build (pointing at the Windows experimental-features settings) rather than referencing internal "velocity keys", and removes internal iterative-review item numbers that had leaked into test comments. Details: - launch_diagnostics: drop "velocity keys" from the three user-facing E_NOTIMPL messages in favour of neutral "feature is not enabled on this OS build" wording, and surface disabled ids as "disabled feature flags". Internal identifiers (check_velocity_keys, REQUIRED_VELOCITY_KEYS) are unchanged. - sandbox.rs: strip the "#1"/"#2" review-item references from the ProcessContainer test comments and trim the process_container_request doc comment. Tests: - cargo test -p appcontainer_common (126 passed) and -p mxc-sdk (all passed); updated the api_not_implemented assertion to match the new message. - cargo fmt + clippy -D warnings clean on both crates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8

… explicit This change fixes a misleading comment on the mxc-sdk streaming dispatch path, flagged in review of #580. The comment claimed the non-BaseContainer streaming fallback used "plain AppContainer", but AppContainerScriptRunner::new() defaults to FilesystemMode::Bfs, so the fallback is really the AppContainer+BFS tier (still dependent on bfscfg.exe), and the AppContainer+DACL tier is genuinely unavailable on the streaming path. Details: - Reword the comment to state that streaming offers only two tiers: BaseContainer when usable, otherwise AppContainer in BFS mode (the dispatcher's Tier 2), with no Tier 3 AppContainer-DACL fallback. - Construct the fallback runner via with_filesystem_mode(FilesystemMode::Bfs) instead of new(), so the BFS choice is explicit and cannot silently drift if the new() default changes. Tests: - cargo test -p mxc-sdk (all passed); clippy -D warnings and fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8

bbonaby · 2026-07-07T20:23:04Z

just need to fix the merge conflict and we should be good

MGudgin · 2026-07-10T22:43:30Z

Closing to try to 'jog' license/cla gate.

This change makes the Windows ProcessContainer AppContainer-vs-BaseContainer choice entirely host-capability driven: BaseContainer is preferred when usable, and AppContainer+BFS is the downlevel fallback. Schema version and --experimental no longer affect this selection. It also incorporates updates that clarify diagnostics and make the streaming fallback explicit. Details: - Route the executor through the capability-driven dispatcher unconditionally and remove the schema-version/experimental gate and obsolete parser helper. - Select the SDK streaming backend using the same BaseContainer usability probe; on downlevel hosts it explicitly creates an AppContainer+BFS runner. - Reword BaseContainer E_NOTIMPL diagnostics to describe an OS-disabled feature without internal terminology, and remove stale schema-version guidance. - Update backend-agnostic regression coverage and tidy related comments. Tests: - cargo test -p wxc_common -p appcontainer_common -p mxc-sdk - cargo clippy --workspace --all-targets -- -D warnings - cargo fmt --all -- --check - check-schema-codegen.js and check-sdk-types-codegen.js Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6c58a97a-f189-43eb-b159-78965a028dc9 Generated-with: gpt-5.6-terra

Integrate 4 upstream commits (Windows Sandbox Phase 0 plumbing, schema floor raise to 0.6.0 (microsoft#581), ProcessContainer/schema decoupling (microsoft#580), mxc_telemetry license fix). Resolve the sole conflict in sdk/tests/unit/sandbox.test.ts: keep this branch's "should accept proxy configuration on macOS (Seatbelt)" test (proxy accepted, version 0.7.0-alpha) over upstream's stale "not supported on macOS" assertion, since this branch adds the Seatbelt proxy support. 0.7.0-alpha remains within the raised 0.6.0-0.8.0 range. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3191c12-2deb-452b-8379-3abbcd5eeb68 Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

MGudgin requested a review from a team as a code owner June 26, 2026 19:46

Base automatically changed from user/gudge/versioning_phase2_5_state_aware_conformance to main June 30, 2026 18:28

bbonaby reviewed Jul 2, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 7, 2026 14:19

Copilot started reviewing on behalf of MGudgin July 7, 2026 14:20 View session

This was referenced Jul 7, 2026

Emit wxc executor errors and warnings as structured ScriptResponse JSON #612

Open

Evaluate flatc/FlatBuffers-driven codegen vs hand-rolled schema/type conformance oracles #613

Open

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread src/core/mxc-sdk/src/dispatch.rs Outdated

MGudgin force-pushed the user/gudge/versioning_phase3a_decouple_backend branch from b1085ad to 5b052ba Compare July 7, 2026 14:29

MGudgin force-pushed the user/gudge/versioning_phase3a_decouple_backend branch from 5b052ba to 1f56d9e Compare July 9, 2026 16:33

bbonaby approved these changes Jul 10, 2026

View reviewed changes

MGudgin closed this Jul 10, 2026

MGudgin reopened this Jul 10, 2026

MGudgin force-pushed the user/gudge/versioning_phase3a_decouple_backend branch from 1f56d9e to 6548b10 Compare July 10, 2026 23:07

bbonaby approved these changes Jul 10, 2026

View reviewed changes

MGudgin merged commit 0e7c3dd into main Jul 10, 2026
22 checks passed

MGudgin deleted the user/gudge/versioning_phase3a_decouple_backend branch July 10, 2026 23:21

caarlos0 mentioned this pull request Jul 13, 2026

feat(sdk): expose Windows isolation tier #638

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Phase 3a: decouple ProcessContainer backend selection from schema version#580

Phase 3a: decouple ProcessContainer backend selection from schema version#580
MGudgin merged 1 commit into
mainfrom
user/gudge/versioning_phase3a_decouple_backend

MGudgin commented Jun 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bbonaby commented Jul 7, 2026

Uh oh!

MGudgin commented Jul 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

MGudgin commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

bbonaby commented Jul 7, 2026

Uh oh!

MGudgin commented Jul 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MGudgin commented Jun 26, 2026 •

edited

Loading