Skip to content

Add Node and .NET request probe APIs - #1276

Merged
Huzaifa Danish (huzaifa-d) merged 37 commits into
mainfrom
user/modanish/probe-api-parity
Sep 30, 2026
Merged

Huzaifa Danish (huzaifa-d) merged 37 commits into
mainfrom
user/modanish/probe-api-parity

Conversation

@huzaifa-d

@huzaifa-d Huzaifa Danish (huzaifa-d) commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Adds Windows ProcessContainer request-probe parity to Node.js and .NET through the shared native engine.

  • exposes probeSandboxSupport(config?) and MxcSandbox.Probe(request?)
  • calls mxc_ffi in-process, without an executor dependency
  • preserves versioned policy semantics and structured native errors
  • leaves existing backend discovery and native-library ownership unchanged

huzaifa-msft and others added 12 commits September 23, 2026 15:56
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 25, 2026 03:08

This comment was marked as resolved.

Comment thread sdk/node/src/probe.ts Outdated
Copilot AI review requested due to automatic review settings September 25, 2026 17:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This comment was marked as resolved.

This comment was marked as resolved.

Co-authored-by: huzaifa-d <16077119+huzaifa-d@users.noreply.github.com>

This comment was marked as resolved.

Comment thread src/core/mxc-sdk/README.md Outdated
Keep the Node, .NET, Rust, engine, and FFI request-probe surfaces plus the fixes requested during review. Remove unrelated backend-discovery and line-ending changes from the PR.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 20:54
Remove the stale Bubblewrap contract dependency while retaining the direct mxc_ffi probe dependencies.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The lockfile is inconsistent, documented 0.8 LXC coverage is removed, and substantial unrelated LXC work obscures the probe API change.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Comment thread docs/architecture.md
Copilot AI balanced review requested due to automatic review settings September 30, 2026 20:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Public type and architecture documentation currently misidentify or omit the new in-process native path.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 21:05
Document the full Node policy-to-probe flow and explain why request-aware tier probing is specific to ProcessContainer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The new Node.js and .NET native ABI paths lack end-to-end integration coverage, and one public description incorrectly attributes capabilities to an executor.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Add native integration coverage for the generated Probe P/Invoke

sdk/​dotnet/​Microsoft.Mxc.Sdk/​Native/​RequestProbeInterop.cs:31

All added managed Probe tests replace RequestProbeInterop, so this generated P/Invoke call is never executed. Add a Windows native integration test that invokes MxcSandbox.Probe() with the built mxc_ffi.dll; otherwise entry-point and ABI drift can ship despite the fake-interoperability tests passing.

Medium severity Add Windows integration coverage for the Koffi sandbox probe binding

sdk/​node/​src/​bindings/​probe.ts:77

The Koffi declaration itself is not exercised by the added tests: they inject either the JSON reader or a fake ProbeNativeFacade, so an incorrect symbol name, pointer level, or error-detail declaration would still pass. Add a Windows integration test that loads the packaged mxc_ffi and calls probeSandboxSupport() through this binding.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 21:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The cross-language native ABI and Windows host-capability behavior warrant final human validation.

Review effort: Balanced
Findings: None

Use the supported process authoring intent and the exported platform-support availability field in the request-probe example.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The new Node and .NET ABI paths lack real-native Windows integration coverage.

Review effort: Balanced
Findings: None

Previously missed (3)

In code that hasn't changed since last review

Medium severity Add Windows integration coverage for the generated P/Invoke entry point

sdk/​dotnet/​Microsoft.Mxc.Sdk/​Native/​RequestProbeInterop.cs:34

All added managed tests replace RequestProbeInterop with a fake, so this generated P/Invoke entry point is never invoked. Please add a Windows integration test using the real native unit to cover export generation, marshalling, and packaged-library loading end to end.

Medium severity Add Windows integration coverage for the real Node native binding

sdk/​node/​src/​bindings/​probe.ts:84

The new Node tests inject ProbeNativeFacade, so they never exercise this Koffi symbol lookup or pointer signature against the packaged DLL. A Windows integration test should call probeSandboxSupport() through the real mxc_ffi; otherwise an export/signature or native-asset staging mismatch can ship while every added test passes.

Low severity Update architecture docs to mention Node and C# bindings

docs/​architecture.md:125

The preceding sentence still says mxc_ffi is used by only the C# SDK, although this PR adds the Node in-process binding shown in the diagram. Update it so the architecture guide reflects both language bindings.

Preserve the owned native pointer for explicit freeing, unload the per-call library handle on every path, restore the top-level probe documentation, and cover real repeated native calls.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 22:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The shared implementation preserves exact parsing and ownership contracts with comprehensive cross-language coverage.

Review effort: Balanced
Findings: None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@huzaifa-d
Huzaifa Danish (huzaifa-d) merged commit 0fefe53 into main Sep 30, 2026
31 checks passed
@huzaifa-d
Huzaifa Danish (huzaifa-d) deleted the user/modanish/probe-api-parity branch September 30, 2026 23:04
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Attention Requires attention or a decision from the MXC maintainers. label Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants