Repository navigation
Complete V1.Dev exact-JSON SDK APIs - #1412
Open
Gudge (MGudgin) wants to merge 1 commit into
Open
Gudge (MGudgin) wants to merge 1 commit into
Gudge (MGudgin) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 6, 2026 19:15
32b5ee3 to
60a7dbc
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved timeout handling can hang captured execution or misreport PTY outcomes.
Review effort: Balanced
Findings: 2
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-execunchanged. - 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.
Contributor
There was a problem hiding this comment.
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
Open (6)
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 6, 2026 20:43
60a7dbc to
1687a1d
Compare
Contributor
There was a problem hiding this comment.
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
Open (1)
Resolved since last review (6)
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 6, 2026 20:58
1687a1d to
25604e2
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 6, 2026 21:20
25604e2 to
15dd178
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 6, 2026 23:02
15dd178 to
3d92de7
Compare
| 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); |
There was a problem hiding this comment.
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,
- "dev" namespace just contains a request object (new
ContainerRequestDevthat just holds the json string) - Change the real
SpawnAPI to accept an interface,IContainerRequest
?
There was a problem hiding this comment.
that way there's only one Spawn function and it's easier for consumer to migrate code from "dev" to stable
Member
Author
There was a problem hiding this comment.
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.
Member
|
consider: adding a sample to /samples/ folder |
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 7, 2026 02:28
3d92de7 to
e5265e4
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 7, 2026 02:54
e5265e4 to
4f1c892
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 7, 2026 15:33
4f1c892 to
a44a4fb
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 7, 2026 15:55
a44a4fb to
3bee51e
Compare
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 12, 2026 15:24
3bee51e to
a2073a3
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Node’s phase guard permits duplicate declarations to reach native dispatch, contrary to the stated phase-before-dispatch behavior.
9 open findings
🧠 Review effort: Balanced
| if (!isRecord(request)) { | ||
| throw new MxcError('malformed_request', `${name} requires a JSON object`); | ||
| } | ||
| if (phase === undefined ? Object.hasOwn(request, 'phase') : request.phase !== phase) { |
Comment on lines
+45
to
+46
| workload to exit. The Node ProcessContainer one-shot PTY route | ||
| passes the original UTF-8 JSON through `wxc-exec --config-base64`, including |
Comment on lines
+9
to
+10
| The separate [V1.Dev exact-JSON API](dev.md) accepts caller-authored requests | ||
| for one-shot execution and state-aware lifecycle operations. |
Comment on lines
+6
to
+7
| takes a second container ID. Invalid versions, unknown fields, and malformed | ||
| JSON are rejected by the exact contract parser. |
|
|
||
| Capture uses pipe-backed execution: a backend without piped execution | ||
| rejects this mode with `UnsupportedContainment` even when experimental | ||
| access is authorized. PTY and state-aware support also depend on the |
|
|
||
| namespace Microsoft.Mxc.Sdk.V1.Dev; | ||
|
|
||
| /// <summary>One-shot caller-authored exact-JSON entry points.</summary> |
| }); | ||
| } | ||
|
|
||
| /** Capture a one-shot request. */ |
Comment on lines
+15
to
+17
| V1.Dev runs caller-authored exact-version requests in-process. It provides | ||
| one-shot and existing-container capture, pipe and PTY spawns, lifecycle | ||
| operations, and dry-run validation. Lifecycle calls return the complete |
Comment on lines
+11
to
+15
| /// Run a caller-authored exact-version one-shot request to completion and capture its output. | ||
| /// | ||
| /// The registered `version` and one-shot request root are parsed by the engine | ||
| /// without stamping or reserializing the input. `options.experimental` only | ||
| /// authorizes an experimental backend; it does not select a contract version. |
Collaborator
|
should we take in these last copilot comments? They look like good doc updates |
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 21, 2026 22:15
a2073a3 to
d04e12f
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Successful Dev execution and lifecycle flows still need real-backend functional coverage in the scheduled SDK validation suites.
10 open issues
Review effort: Balanced
Comment on lines
+173
to
+175
| const proc = await spawnStateAwareBindingSandboxProcessAsync( | ||
| json, options.experimental === true, timeoutMs, | ||
| ); |
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. * Keep .NET Dev options sealed and independent with reflection-free JSON and Native AOT smoke coverage. * 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 (12 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
Gudge (MGudgin)
force-pushed
the
user/gudge/json_dev_apis
branch
from
October 23, 2026 14:23
d04e12f to
7906c27
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Successful V1.Dev execution and lifecycle scenarios still need host-backed functional coverage wired into scheduled CI.
11 open issues
Review effort: Balanced
Comment on lines
+19
to
+20
| pub fn run_json(json: &str, options: JsonOptions) -> Result<ExecutionResult, Error> { | ||
| crate::__ffi::run_json(json, options.experimental) |
This branch has not been deployed
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.
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
execution, plus lifecycle and dry-run validation. Existing V1 result and
handle ownership semantics are retained.
wxc-execwith the original JSON.Lifecycle and validation return the full native response JSON.
Node ProcessContainer PTY resolves when
wxc-execstarts, before its policyvalidation 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
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).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).sdk\node:npm run typecheck(passed);npm test --silent(448 passed, 21 skipped).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).& sdk\dotnet\Microsoft.Mxc.Sdk.Tests\bin\Debug\net8.0\Microsoft.Mxc.Sdk.Tests.exe -class Microsoft.Mxc.Sdk.Tests.DevEntryPointTests(7 passed).& sdk\dotnet\Microsoft.Mxc.Sdk.Tests\bin\Debug\net8.0\Microsoft.Mxc.Sdk.Tests.exe -class Microsoft.Mxc.Sdk.Tests.V1.V1ApiSurfaceTests(3 passed).