Skip to content

Complete V1.Dev exact-JSON SDK APIs - #1412

Open
Gudge (MGudgin) wants to merge 1 commit into
mainfrom
user/gudge/json_dev_apis
Open

Gudge (MGudgin) wants to merge 1 commit into
mainfrom
user/gudge/json_dev_apis

Conversation

@MGudgin

@MGudgin Gudge (MGudgin) commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

This PR adds caller-authored exact-JSON execution, lifecycle, and validation
APIs under V1.Dev in Rust, .NET, and Node. Each API retains the declared
contract version, keeps experimental authorization separate, and checks
lifecycle phases before dispatch.

Details

  • Exposes capture, pipe, and PTY modes for one-shot and existing-container
    execution, plus lifecycle and dry-run validation. Existing V1 result and
    handle ownership semantics are retained.
  • Routes Node ProcessContainer PTY through wxc-exec with the original JSON.
    Lifecycle and validation return the full native response JSON.
  • Updates the SDK READMEs, versioning guide, and versioned API references.

Node ProcessContainer PTY resolves when wxc-exec starts, before its policy
validation completes; its terminal channel cannot expose structured warnings
or output metadata. Node and .NET captured output is lossy UTF-8 text through
the existing ABI; Rust capture retains bytes.

Tests

  • From src: cargo fmt --all -- --check (passed);
    cargo check -p mxc-sdk --lib --features isolation_session,wslc --quiet
    (passed); cargo clippy --workspace --all-targets -- -D warnings (passed).
  • From src: cargo test -p mxc-sdk --lib dev:: --quiet (93 passed,
    1 host-gated); cargo test -p mxc-sdk --test state_aware --quiet
    (16 passed); cargo test -p mxc-sdk --lib --features isolation_session,wslc dev:: --quiet (93 passed, 1 host-gated).
  • From sdk\node: npm run typecheck (passed);
    npm test --silent (448 passed, 21 skipped).
  • From the repository root: dotnet build sdk\dotnet\Microsoft.Mxc.Sdk.Tests\Microsoft.Mxc.Sdk.Tests.csproj --no-restore --nologo -v:q (passed); dotnet build sdk\dotnet\Microsoft.Mxc.Sdk.AotSmokeTest\Microsoft.Mxc.Sdk.AotSmokeTest.csproj --no-restore --nologo -v:q (passed).
  • Direct xUnit: & sdk\dotnet\Microsoft.Mxc.Sdk.Tests\bin\Debug\net8.0\Microsoft.Mxc.Sdk.Tests.exe -class Microsoft.Mxc.Sdk.Tests.DevEntryPointTests (7 passed).
  • Direct xUnit: & sdk\dotnet\Microsoft.Mxc.Sdk.Tests\bin\Debug\net8.0\Microsoft.Mxc.Sdk.Tests.exe -class Microsoft.Mxc.Sdk.Tests.V1.V1ApiSurfaceTests (3 passed).

@MGudgin
Gudge (MGudgin) requested a review from a team as a code owner October 6, 2026 19:05
Copilot AI balanced review requested due to automatic review settings October 6, 2026 19:05
@azure-pipelines

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

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

Unresolved timeout handling can hang captured execution or misreport PTY outcomes.

Review effort: Balanced
Findings: 2 High severity · 3 Medium severity

Open (5)
What changed in this PR

Adds V1.Dev exact-JSON APIs alongside the typed Rust, .NET, and Node SDKs, preserving caller-declared contract versions and separate experimental authorization.

Changes:

  • Adds capture, pipe, PTY, lifecycle, and validation operations.
  • Routes Node ProcessContainer PTY requests through wxc-exec unchanged.
  • Adds tests and documents signatures, ownership, and platform limitations.
File Description
src/​mxc-sdk/​tests/​state_aware.rs Checks public Rust dev signatures and routing.
src/​mxc-sdk/​src/​lib.rs Exports the versioned dev module.
src/​mxc-sdk/​src/​dev/​state_aware.rs Adds lifecycle and existing-container APIs.
src/​mxc-sdk/​src/​dev/​one_shot.rs Adds one-shot execution APIs.
src/​mxc-sdk/​src/​dev.rs Defines exports and invocation options.
src/​mxc-sdk/​README.md Introduces Rust exact-JSON APIs.
sdk/​node/​tests/​unit/​v1-dev.test.ts Tests forwarding, validation, and capture.
sdk/​node/​tests/​unit/​state-aware-binding.test.ts Tests missing native responses.
sdk/​node/​tests/​unit/​process-container-pty.test.ts Tests unchanged JSON transport.
sdk/​node/​tests/​integration/​dev.test.ts Checks native contract rejection.
sdk/​node/​tests/​integration/​dev-api.ts Checks packaged dev signatures.
sdk/​node/​src/​v1/​dev/​index.ts Implements Node dev operations.
sdk/​node/​src/​bindings/​streaming.ts Adds raw-JSON pipe spawning.
sdk/​node/​src/​bindings/​state-aware.ts Rejects missing response JSON.
sdk/​node/​src/​bindings/​run.ts Adds raw-JSON capture binding.
sdk/​node/​src/​bindings/​pty.ts Adds raw-JSON PTY binding.
sdk/​node/​src/​bindings/​process-container-pty.ts Accepts original JSON for executor launches.
sdk/​node/​README.md Introduces Node dev APIs.
sdk/​node/​package.json Exports dev APIs and registers tests.
sdk/​dotnet/​README.md Introduces .NET dev APIs.
sdk/​dotnet/​Microsoft.Mxc.Sdk/​V1/​Dev/​MxcLifecycle.cs Adds lifecycle and existing-container APIs.
sdk/​dotnet/​Microsoft.Mxc.Sdk/​V1/​Dev/​MxcContainer.cs Adds one-shot execution APIs.
sdk/​dotnet/​Microsoft.Mxc.Sdk/​V1/​Dev/​JsonOptions.cs Defines authorization and PTY options.
sdk/​dotnet/​Microsoft.Mxc.Sdk/​V1/​Dev/​DevJsonRequest.cs Checks phases and encodes requests.
sdk/​dotnet/​Microsoft.Mxc.Sdk.Tests/​DevEntryPointTests.cs Tests dev validation and signatures.
docs/​versioning.md Explains exact-JSON version selection.
docs/​reference/​rust/​v1/​README.md Links Rust dev reference.
docs/​reference/​rust/​v1/​dev.md Documents Rust dev APIs.
docs/​reference/​rust/​v1/​api.md Distinguishes typed and dev surfaces.
docs/​reference/​README.md Links cross-SDK dev references.
docs/​reference/​node/​v1/​README.md Links Node dev reference.
docs/​reference/​node/​v1/​dev.md Documents Node APIs and executor limitations.
docs/​reference/​node/​v1/​api.md Distinguishes typed and dev surfaces.
docs/​reference/​dotnet/​v1/​README.md Links .NET dev reference.
docs/​reference/​dotnet/​v1/​dev.md Documents .NET APIs and cancellation behavior.
docs/​reference/​dotnet/​v1/​api.md Distinguishes typed and dev surfaces.

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

Comment thread sdk/dotnet/Microsoft.Mxc.Sdk/V1/Dev/MxcLifecycle.cs
Comment thread sdk/node/src/v1/dev/index.ts Outdated
Comment thread sdk/node/src/v1/dev/index.ts Outdated
Comment thread sdk/node/src/v1/dev/index.ts
Comment thread sdk/node/src/v1/dev/index.ts
Copilot AI balanced review requested due to automatic review settings October 6, 2026 19:16

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-SDK API expansion involves native interop, PTY routing, and lifecycle ownership that warrant final human review.

Review effort: Balanced
Findings: 2 High severity · 4 Medium severity

Open (6)

Comment thread sdk/dotnet/Microsoft.Mxc.Sdk/V1/Dev/DevJsonRequest.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 6, 2026 20:43
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/json_dev_apis branch from 60a7dbc to 1687a1d Compare October 6, 2026 20:43

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

Cross-SDK native-handle ownership and unresolved Windows timeout capture require platform-specific maintainer validation.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (6)

Comment thread sdk/node/src/v1/capture.ts
Copilot AI balanced review requested due to automatic review settings October 6, 2026 20:58
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/json_dev_apis branch from 1687a1d to 25604e2 Compare October 6, 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

Unresolved correctness issues and host-dependent native cleanup require fixes and human integration review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (1)

Comment thread sdk/node/src/v1/capture.ts Outdated
Comment thread sdk/dotnet/Microsoft.Mxc.Sdk/V1/Dev/JsonOptions.cs
Copilot AI balanced review requested due to automatic review settings October 6, 2026 21:20
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/json_dev_apis branch from 25604e2 to 15dd178 Compare October 6, 2026 21:20

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

Native PTY and output-reader cleanup behavior require final human review supported by host-dependent validation.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Comment thread docs/api-reference/README.md Outdated
Copilot AI balanced review requested due to automatic review settings October 6, 2026 23:02
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/json_dev_apis branch from 15dd178 to 3d92de7 Compare October 6, 2026 23:02

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

Cross-platform native process cleanup and executor-backed PTY behavior warrant final human review, particularly the host-gated timeout regression.

Review effort: Balanced
Findings: None

ExecutionResult RunJson(string json, JsonOptions? options = null);
Task<ExecutionResult> RunJsonAsync(string json, JsonOptions? options = null,
CancellationToken cancellationToken = default);
MxcProcess SpawnJson(string json, JsonOptions? options = null);

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.

I'm trying to reason over the overall design.

Is it a normal SDK pattern to have a complete a ::dev:: namespace that duplicates the API surface for active development?

Could it be something like,

  1. "dev" namespace just contains a request object (new ContainerRequestDev that just holds the json string)
  2. Change the real Spawn API to accept an interface, IContainerRequest

?

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.

that way there's only one Spawn function and it's easier for consumer to migrate code from "dev" to stable

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for raising the migration ergonomics. The separate V1.Dev surface is intentional: typed V1 Spawn(ContainerRequest) emits the SDK-owned published contract with no experimental opt-in, while SpawnJson preserves the caller-authored exact version and takes backend authorization separately. An IContainerRequest overload would mix those contracts in the stable facade, and it would not address lifecycle calls whose JSON owns the phase/id and whose response can contain evolving metadata. The one-shot result and process-handle types are already shared. Moving a promoted feature to typed V1 still requires converting the request fields; sharing the Spawn name would not remove that step. I would keep explicit *Json operations in this PR and consider a cross-SDK convenience surface separately if migration friction warrants it.

Comment thread docs/api-reference/node/v1/dev.md Outdated
Comment thread sdk/node/src/bindings/streaming.ts
@jsidewhite

Copy link
Copy Markdown
Member

consider: adding a sample to /samples/ folder

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:

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

Native process ownership and timeout cleanup need final maintainer validation on a prepared Windows host.

Review effort: Balanced
Findings: None

This PR adds caller-authored exact-JSON execution and lifecycle APIs in the
Rust, .NET, and Node SDKs. Requests retain their declared contract version
while authorization and operation selection remain separate. Captured
operations return partial output on timeout instead of waiting for inherited
output pipes to close.

Details

* Publish capture, pipe, PTY, lifecycle, and validation entry points with
  native exact-contract routing and versioned SDK references.
* Close timed-out .NET capture readers; settle Node captures without waiting
  for native stream close and reject read failures during process wait.
* Preserve the original capture error if disposal fails; route the historical
  appcontainer PTY alias and accept deep annotations in .NET requests.
* Register .NET Dev options for reflection-free JSON and Native AOT smoke.
* Document and test exit-only timeout reporting for executor-backed PTYs.

Tests

* From src: cargo fmt --all -- --check; cargo clippy --workspace
  --all-targets -- -D warnings; cargo test -p mxc-sdk --lib dev::;
  cargo test -p mxc-sdk --test state_aware (passed).
* From sdk\node: npm run typecheck --silent (passed); npm test --silent
  (458 passed, 21 skipped), including pending-read capture coverage.
* .NET AOT smoke ran reflection-free; Native AOT publish and run passed;
  DevEntryPointTests and V1ApiSurfaceTests passed (11 total).
  Host-dependent IsolationSession workloads were skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f136b563-7d18-4c2e-8f86-43af66889e6a
Generated-with: gpt-6-sol
Copilot AI balanced review requested due to automatic review settings October 7, 2026 02:54
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/json_dev_apis branch from e5265e4 to 4f1c892 Compare October 7, 2026 02:54

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

Cross-SDK native stream cleanup and PTY ownership behavior still need supported-host validation and final human review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment on lines +7 to +18
public class JsonOptions
{
/// <summary>Authorize experimental backends independently of the JSON version.</summary>
public bool Experimental { get; set; }
}

/// <summary>Invocation controls for caller-authored exact JSON with a PTY.</summary>
public sealed class PtyJsonOptions : JsonOptions
{
/// <summary>Initial terminal dimensions; defaults to 24 rows by 80 columns.</summary>
public MxcPtySize? Size { get; set; }
}

This branch has not been deployed

No deployments
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.

3 participants