[Proposal] Unify one-shot SDKs on typed requests

[Branden Bonaby (bbonaby)]

📖 Description

Proposes a shared, strongly typed one-shot SDK contract for Rust, C#, and Node. The design introduces versioned request and run/spawn option types, carries them through a typed mxc_ffi boundary, and moves Node in-process through a thin Koffi adapter.

The existing versioned JSON path remains supported for executor binaries. This PR is a design proposal only; implementation will follow in reviewable stages.

Read the rendered proposal

🔗 References

None.

🔍 Validation

Documentation-only proposal; no runtime behavior changed.

✅ 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 (see docs/pull-requests.md)

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

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

[Copilot]

Copilot review overview

🟡 Changes recommended

The proposal misstates the parser path, omits an authorization option, and has contradictory Node threading semantics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 · 1

New issues introduced by this change (3)

Severity	Finding
	docs/​design/​json-first-one-shot-sandbox-apis.md — Preserve the testing-feature authorization flag View comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Resolve the synchronous spawn threading contradiction View comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Describe the actual one-shot parser path View comment

What changed in this PR

Proposes JSON-first one-shot APIs across Rust, C#, and Node while preserving existing APIs.

Changes:

• Defines cross-language run and spawn APIs.
• Proposes C ABI reuse through Koffi for Node.
• Outlines migration and parity criteria.

File	Description
docs/​design/​json-first-one-shot-sandbox-apis.md	Documents the proposed architecture, APIs, and migration.

---

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

[Gudge (MGudgin)]

I don't agree with this approach. The SDK APIs don't go through JSON at all with the new parser/versioning stack and should not need to.

[Copilot]

Copilot review overview

🟡 Changes recommended

Timeout preservation, asynchronous spawning, native packaging, dry-run semantics, and ABI option rules need clarification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 2

New issues introduced by this change (3)

Severity	Finding
	docs/​design/​json-first-one-shot-sandbox-apis.md — Include the native runtime unit in Node migration View comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Define the optional-options ABI contract View comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Make the Koffi spawn API asynchronous View comment

Issues resolved since last review (3)

Severity	Finding
	docs/​design/​json-first-one-shot-sandbox-apis.md — Describe the actual one-shot parser path View resolved comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Resolve the synchronous spawn threading contradiction View resolved comment
	docs/​design/​json-first-one-shot-sandbox-apis.md — Preserve the testing-feature authorization flag View resolved comment

Previously missed findings (2)

In code that hasn't changed since last review

docs/design/json-first-one-shot-sandbox-apis.md:98

• RunOptions advertises dryRun, but run_config returns a process Output even though no process or WaitOutcome exists in a dry run; the current streaming SDK run path also rejects requests marked dry-run. Specify whether this uses the engine's validation-only path and what Output/MxcRunResult contains, or use a dedicated validation result/remove this option, so each language does not invent different semantics.
  docs/design/json-first-one-shot-sandbox-apis.md:132
• A thin wrapper cannot inspect process.timeout, but the current C# streaming wrapper reads SandboxPolicy.TimeoutMs and enforces an absolute deadline because mxc_sandbox_try_wait does not enforce it and a later blocking wait starts a fresh timeout budget. Require the returned native handle to retain and enforce the parsed deadline from spawn time, or expose that deadline to wrappers; otherwise SpawnConfig can silently run longer than the same config through current APIs/CLI.

[Copilot]

Copilot review overview

🟡 Changes recommended

The PR metadata contradicts the proposal, and ABI evolution and cancellable-wait handling need clarification.

Get a fresh assessment by requesting another Copilot review.

Review tier: Balanced
Findings: 1 · 1 · 1

Open findings (3)

• Define an actually extensible V1 representation · New
• Keep cancellation from blocking behind the wait call · New
• Align the proposal with the JSON-first PR description · New

Resolved findings (3)

• Make the Koffi spawn API asynchronous
• Define the optional-options ABI contract
• Include the native runtime unit in Node migration

[Copilot]

Copilot review overview

🟡 Changes recommended

C ABI extensibility, V1 normalization semantics, and Node PTY compatibility remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review tier: Balanced
Findings: 2 · 1

Open (3)

• Preserve Node's PTY spawn contract · New
• Define an actually extensible V1 representation
• Keep cancellation from blocking behind the wait call

Resolved since last review (1)

• Align the proposal with the JSON-first PR description

Previously missed (1)

In code that hasn't changed since last review

Define V1's normalization version

docs/​design/​typed-one-shot-sandbox-apis.md:130

Removing the runtime version leaves normalization underspecified. The current Rust builder requires SandboxPolicy.version, copies it into ExecutionRequest.schema_version (src/core/mxc_engine/src/policy.rs:767-785,820-824), and backends branch on that value for different enforcement contracts (for example src/backends/bubblewrap/common/src/bwrap_command.rs:133-150). Without a fixed mapping, the same SandboxRequestV1 could receive 0.7 or 0.9 behavior depending on implementation/release, and the “equivalent executor JSON” criterion is ambiguous. Bind V1 to explicit, stable normalization semantics (and a schema version for JSON conversion), with semantic changes requiring V2.