Enforce probe coverage for every backend and isolation tier by theelliotm · Pull Request #1491 · microsoft/mxc

Elliot (theelliotm) · 2026-10-13T22:46:39Z

📖 Description

Make it impossible to add a containment backend or isolation tier without deciding whether the host probe reports it, and prove that decision against the real reporting code.

backend_registry is the single source of truth: every registration declares ProbeCoverage::Detected(&[target_os]) or NotDetected(reason). Vm and MicroVm are recorded as undetected, making a previously silent probe gap explicit.
The platform reporting functions take their host checks as parameters, so tests can force every check to pass. Two gates then run the production reporting path: every backend declared detected on the current target must be emitted, and a NotDetected one must not be. Registering a backend as detected without adding a branch to available_backends() fails.
probe.rs derives its canonical tier list from IsolationTier::ALL instead of a hand-written array, and gains a test that every tier is reachable from select_tier.
platform.rs drops its duplicate wire-name table and builds available_methods from wire_name() rather than raw literals.

Behavior change

The gate found a real gap: Hyperlight runs on Linux under KVM (resolve_hyperlight, tools/lxc), but available_backends() only reported it on Windows. It is now reported on Linux too, when the hyperlight feature is built on x86_64 and /dev/kvm is readable and writable.

This affects the available_backends() array and lxc-exec --available-backends only. platform_support() is unchanged, and the Node SDK's Linux consumer scans that payload for the bubblewrap entry alone, so the added entry is inert there. No wire type, field, or name changed.

🔗 References

Resolves #769

Rescoped from the original ask: with backend_registry's exhaustive match already in place, compile-time enforcement plus unit tests cover this without a new CI job.

🔍 Validation

From src/:

cargo fmt --all -- --check — passed
cargo clippy -p mxc-sdk --all-targets -- -D warnings — passed
cargo test -p mxc-sdk --lib -- mxc_engine — passed, 106 tests
cargo test -p mxc-sdk --lib --features tier2_bfs -- mxc_engine::probe mxc_engine::backend_registry — passed, 24 tests
cargo check -p mxc-sdk --lib --target x86_64-unknown-linux-gnu — passed
cargo check -p mxc-sdk --lib --profile test --target x86_64-unknown-linux-gnu — passed

From the repository root:

node scripts/check-dotnet-api-parity.js — passed

Each gate was confirmed to fail on a simulated regression (reverted):

Registering Vm as Detected(&["windows"]) fails with vm is registered as detected on windows, but the probe never reports it even with every host check passing.
Removing the BFS rung from select_tier fails with select_tier can never report "appcontainer-bfs"; the probe tier ladder is missing a rung.
Dropping a backend from the canonical table fails the registry set-equality test.

Not covered locally: --features hyperlight --target x86_64-unknown-linux-gnu cannot build here (the ring build script needs a Linux C cross-toolchain), so the Linux KVM branch is compiled only by CI's Linux x64 Hyperlight build. Its cfg and call path match the existing resolve_hyperlight call site exactly.

Two mxc_common::filesystem_canonical tests fail on this machine for lack of symlink privileges; unrelated and pre-existing.

✅ Checklist

Signed the Contributor License Agreement
Linked to an issue
Updated documentation (if applicable)
Updated Copilot instructions (if build, architecture, or conventions changed)
If this PR changes Cargo.lock, the dependency-feed-check check passes

📋 Issue Type

Bug fix
Feature
Task

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-10-13T22:46:49Z

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

Copilot

🟡 Changes recommended

Detected registrations are not exhaustively connected to probe mappings, so the intended regression gate remains bypassable.

2 open findings

What changed in this PR

Centralizes backend probe metadata and strengthens isolation-tier coverage checks.

Changes:

Adds explicit backend probe coverage classifications.
Derives tier checks from IsolationTier::ALL.
Reuses canonical backend wire names across platform probes and documentation.

File	Description
`backend_registry.rs`	Adds probe coverage metadata and invariants.
`probe.rs`	Adds backend and tier coverage tests.
`platform.rs`	Replaces backend string literals with `wire_name()`.
`backend-support-probe-api.md`	Documents the updated coverage approach.

🧠 Review effort: Balanced

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

Address review feedback on #1491: - ProbeCoverage::Detected was inert metadata. Add an exhaustive host_detector() mapping in probe.rs and a test requiring Detected to name a detector and NotDetected to have none, so a backend cannot be registered as detected without a branch in available_backends(). - Correct the wslc test-plan bullet: availability comes from wslc_common::is_available() (WSL2 plus the WSLC runtime), not from resolving wslcsdk.dll. Fix the Repository consistency check: check-dotnet-api-parity.js read tier wire names from probe.rs's CANONICAL_TIERS, which this branch removed. Read the isolation_tiers! ladder in fallback_detector.rs instead, so the parity check compares against the canonical declaration rather than a test mirror. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25b86546-f178-4ab4-bda2-7443aa01f541

Copilot

🟡 Changes recommended

The coverage test uses a separate manual mapping rather than verifying runtime detector dispatch, and the plan retains stale follow-up items.

1 open finding

Tie ProbeCoverage to live backend detector dispatch · New

2 resolved since last review

Previously missed (1)

In code that hasn't changed since last review

Remove completed backend detection items from the follow-up list

docs/development/plans/backend-support-probe-api.md:187

This updated follow-up list still describes Windows Sandbox, IsolationSession, and LXC detection as future work, but available_backends() already invokes all three detectors and this PR registers them as Detected. Remove those completed items so the plan agrees with the implementation and the new coverage declarations.

🧠 Review effort: Balanced

The probe-coverage gate compared a declaration against another declaration: a backend registered as Detected only had to name a detector string, which a typo could satisfy. Make the gate run the production reporting code instead. ProbeCoverage::Detected now names the target_os values a backend is detected on. The platform reporting functions take their host checks as parameters, so tests can force every check to pass and assert that each backend declared detected on the current target is actually emitted, and that a NotDetected one still is not. Registering a backend as detected without adding a branch to available_backends() now fails. Also drop the completed windows_sandbox, isolation_session, and lxc detector items from the plan doc's follow-up list and gap tables, which no longer match the code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25b86546-f178-4ab4-bda2-7443aa01f541

Copilot

🔵 Needs a closer look

Hyperlight’s existing Linux KVM availability path is excluded from the new coverage registry and tests.

0 open findings

1 resolved since last review

Tie ProbeCoverage to live backend detector dispatch

Previously missed (2)

In code that hasn't changed since last review

Linux invariant tests omit runnable Hyperlight backend

src/mxc-sdk/src/core/mxc_engine/backend_registry.rs:69

Hyperlight is also runnable on Linux: resolve_hyperlight() gates it with hyperlight_common::is_kvm_available() (run.rs:475-499), and the Linux x64 CI build enables the feature. Restricting coverage to Windows makes the Linux invariant test ignore Hyperlight, so available_backends() can keep omitting a backend that the host can run. Include linux here and wire the feature/architecture-gated KVM check through linux_backends_with.

Document Linux KVM probe in Hyperlight detector summary

docs/development/plans/backend-support-probe-api.md:141

This detector summary omits Linux's existing is_kvm_available() probe, even though Hyperlight is supported on Linux as well as Windows. Document both OS-specific probes so the plan does not imply that Hyperlight coverage is Windows-only.

🧠 Review effort: Balanced

Hyperlight runs on Linux under KVM, but available_backends() only reported it on Windows, so the probe omitted a backend the host can run. Registering it as detected on Windows alone also kept the Linux coverage test from catching that. linux_backends_with() now takes a hyperlight availability input, and hyperlight_host_available() resolves WHP on Windows and KVM on Linux behind the same feature and x86_64 gates that resolve_hyperlight() uses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 25b86546-f178-4ab4-bda2-7443aa01f541

Copilot

🟡 Changes recommended

The coverage gates remain incomplete, and Linux Hyperlight reporting contradicts the stated no-behavior-change scope.

1 open finding

Document or remove Linux Hyperlight availability behavior change · New

Previously missed (3)

In code that hasn't changed since last review

Ensure every backend variant maps to its own registry entry

src/mxc-sdk/src/core/mxc_engine/backend_registry.rs:203

This test compares two manually maintained registry-derived lists, so it still passes if a new ContainmentBackend variant is omitted from both. The exhaustive registration() match only forces an arm; an accidental arm pointing to an existing index also passes every registry test and gives the new backend another backend's metadata. Derive an ALL list and the registry from the enum's single declaration (for example, with a macro), or add an enum-derived exhaustive list and assert that each variant maps to a registration whose backend equals that variant.

Require exact equality between declared and reported target sets

src/mxc-sdk/src/core/mxc_engine/probe.rs:427

This checks only that the declared set is a subset of the reported set. If a backend remains reported on Linux but "linux" is accidentally removed from its Detected targets, both this test and the NotDetected test still pass, so the registry is not actually the source of truth for per-target coverage. Compare the two sets for equality so undeclared reporting fails too.

Add missing comma in “check, which”

docs/development/plans/backend-support-probe-api.md:101

Add the missing separator in “check, which.”

🧠 Review effort: Balanced

Huzaifa Danish (huzaifa-d) · 2026-10-14T15:22:33Z

+
+    #[cfg(target_os = "linux")]
+    fn backends_with_host_checks_passing() -> Vec<AvailableBackend> {
+        linux_backends_with(true, || Ok(()), true, true)


This test bypasses the production detector wiring. It still passes if linux_backends() uses the wrong detector or argument. Please test the production path with one detector enabled at a time.

The new tests check the reporting branches, but still bypass the real detector assignments in linux_backends() and the Windows path. For example, setting the production Hyperlight result to false would leave them passing. Please run the single-detector tests through the production assignment path so they catch a disconnected or wrongly assigned detector.

Huzaifa Danish (huzaifa-d) · 2026-10-14T15:22:36Z

-    fn backend_wire_names_match_the_canonical_table() {
-        for (backend, expected) in CANONICAL_BACKEND_NAMES {
-            assert_eq!(backend.wire_name(), *expected, "{backend:?}");
+    fn every_detected_backend_is_reported_when_its_host_checks_pass() {


Linux CI does not run these probe or registry tests. Windows CI cannot test Linux-only code. Please run these tests in the default and x64 Hyperlight Linux jobs.

Huzaifa Danish (huzaifa-d) · 2026-10-14T15:22:39Z

+    #[cfg(not(target_os = "windows"))]
+    #[test]
+    fn no_backend_reports_a_tier_off_windows() {
+        for entry in available_backends() {


This test checks nothing when available_backends() returns an empty list. Please use backends_with_host_checks_passing() so the test always checks reported backends.

Copilot

🟢 Approval recommended

The scoped changes preserve existing gates and add coverage checks, with no unresolved blocking issues identified.

0 open findings

1 resolved since last review

Document or remove Linux Hyperlight availability behavior change

🧠 Review effort: Balanced

Copilot

🟢 Approval recommended

The scoped reporting changes and coverage checks are consistent, with no unresolved blocking findings.

0 open findings

🧠 Review effort: Balanced

Enforce probe coverafe for every backend and isolation tier

7125ea0

Copilot AI balanced review requested due to automatic review settings October 13, 2026 22:46

Copilot started reviewing on behalf of Elliot (theelliotm) October 13, 2026 22:47

View session

Elliot (theelliotm) self-assigned this Oct 13, 2026

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Comment thread src/mxc-sdk/src/core/mxc_engine/backend_registry.rs Outdated

Comment thread docs/development/plans/backend-support-probe-api.md Outdated

Copilot AI balanced review requested due to automatic review settings October 13, 2026 23:00

Copilot started reviewing on behalf of Elliot (theelliotm) October 13, 2026 23:00

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Comment thread src/mxc-sdk/src/core/mxc_engine/probe.rs Outdated

Copilot AI balanced review requested due to automatic review settings October 13, 2026 23:49

Copilot started reviewing on behalf of Elliot (theelliotm) October 13, 2026 23:49

View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 14, 2026 00:06

Copilot started reviewing on behalf of Elliot (theelliotm) October 14, 2026 00:07

View session

Elliot (theelliotm) marked this pull request as ready for review October 14, 2026 00:07

Elliot (theelliotm) requested a review from a team as a code owner October 14, 2026 00:07

Elliot (theelliotm) requested a review from Huzaifa Danish (huzaifa-d) October 14, 2026 00:08

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread src/mxc-sdk/src/core/mxc_engine/probe.rs

Huzaifa Danish (huzaifa-d) reviewed Oct 14, 2026

View reviewed changes

Comment thread docs/development/plans/backend-support-probe-api.md Outdated

Huzaifa Danish (huzaifa-d) reviewed Oct 14, 2026

View reviewed changes

address feedback

1a8dabb

Copilot AI balanced review requested due to automatic review settings October 14, 2026 21:40

Copilot started reviewing on behalf of Elliot (theelliotm) October 14, 2026 21:41

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Elliot (theelliotm) requested a review from Huzaifa Danish (huzaifa-d) October 14, 2026 21:46

compare available_backends to detector results

bdcaee6

Copilot AI balanced review requested due to automatic review settings October 15, 2026 20:05

Copilot started reviewing on behalf of Elliot (theelliotm) October 15, 2026 20:06

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

Huzaifa Danish (huzaifa-d) approved these changes Oct 15, 2026

View reviewed changes

Elliot (theelliotm) merged commit 068a78d into main Oct 15, 2026
31 checks passed

Elliot (theelliotm) deleted the user/emichlin/investigate-769 branch October 15, 2026 20:31

Conversation

Elliot (theelliotm) commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

Behavior change

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Oct 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Elliot (theelliotm) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Elliot (theelliotm) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Elliot (theelliotm) Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Elliot (theelliotm) commented Oct 13, 2026 •

edited

Loading