[WSLC] State-aware lifecycle (PR 3/3): TypeScript SDK per-phase policy configs - #810
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds WSLc support to the TypeScript SDK’s state-aware lifecycle API.
Changes:
- Adds per-phase WSLc configuration and metadata types.
- Routes
wslc:IDs and defaults requests to schema0.8.0-alpha. - Updates exports, documentation, and unit coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
sdk/node/src/state-aware-types.ts |
Defines WSLc lifecycle types and metadata mapping. |
sdk/node/src/state-aware-helper.ts |
Adds WSLc versioning and ID-prefix routing. |
sdk/node/src/index.ts |
Exports the new public types. |
sdk/node/README.md |
Documents WSLc lifecycle usage. |
sdk/node/tests/unit/state-aware.test.ts |
Tests envelope construction and lifecycle routing. |
sdk/node/tests/unit/state-aware-types.test.ts |
Tests type constraints and branding. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
39c76fa to
9b110a8
Compare
97c516f to
3b534c6
Compare
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Summary
Requesting changes for two SDK consistency gaps: WSLc's new backend-specific phase fields are not added to the state-aware generated-wire conformance oracle, and prefix routing is the only backend registry in this design that is not compiler-enforced as exhaustive. The remaining two comments are non-blocking documentation cleanup.
Range verified: 9b110a87bf06b1e0239bd13dd24ce4fa6b39c5f6...3b534c67bc11e04bc4191ebf5828bfc7b620994d (6 files, +353/-7). The merge base exactly matches GitHub's PR base. The locally captured git diff and gh pr diff are byte-for-byte identical at 498 lines.
Verified clean, with receipts: the new tests cover WSLc version default/override, filesystem and network lifting, image/imageTarPath nesting, all five lifecycle routes, prefix inference, metadata absence, branded-id separation, and public config/export shapes. Existing generic state-aware tests already verify every wire error code, including policy_validation, maps to MxcError. Runtime Rust policy enforcement is unchanged by this SDK-only PR and fails closed for unsupported WSLc policy fields. No runtime WSL tests were run as part of this filing.
Verified pre-existing - not independently attributed to this PR
sdk/node/tests/unit/wire-conformance-state-aware.test.tsis byte-identical base vs head. The file itself states that every new backend-specific state-aware phase shape must be represented to prevent generated-wire/public-SDK drift; the finding is framed as a gap in this PR's new WSLc type addition, not as an old defect.docs/wsl/wslc-state-aware.mdand the Rust WSLc policy implementation are byte-identical base vs head. Their pre-existing content is context only and is not a merge condition in this review.
Requested findings not filed after verification
- The broad
NetworkConfigtypes are intentional: the PR description explicitly follows the Windows Sandbox precedent of permissive SDK shapes with runtime honor-matrix validation. The unchanged Rust backend rejects unsupported fields withpolicy_validation, so this is not a fail-open or reliability bug. - Backend-specific policy rejection tests were withdrawn: the SDK cannot prove Rust policy semantics with a fake spawn, while generic state-aware tests already verify typed
MxcErrorpropagation and the Rust layer owns the policy tests. - The honor-matrix omission claim was withdrawn: that pre-existing table is a policy matrix rather than a complete backend-config schema, and the new JSDoc already states that only
defaultPolicyis honored at provision.
9b110a8 to
bd05283
Compare
… add provision delegation regression tests
Address the two blocking review findings on the live exec-output stream: - enqueue_output now uses try_send with an overflowed latch instead of blocking_send, so the WSLc SDK I/O callback thread (which also delivers the exit callback) is never parked when a slow pipe client stops draining. - write_exec_result biases the select toward the done branch so a completed or leaked exec always writes its terminal frame; a latched overflow turns a clean Exit into a truncation Error via terminal_frame. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d
bd05283 to
f9d3f8f
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d
f9d3f8f to
7ac1559
Compare
…ing, and 0.8.0-alpha default
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d
3b534c6 to
527a731
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
src/backends/wslc/common/src/daemon_protocol.rs:40
- The PR description says this is a pure TypeScript SDK-surface addition with “no runtime behavior,” but this protocol bump is part of substantial daemon/runtime changes (live output streaming, base64 frame encoding, and provision-policy gating). Please update the PR scope and validation details, or rebase/split the inherited runtime changes, so the reviewed diff matches the stated purpose.
pub const PROTOCOL_VERSION: u32 = 2;
src/backends/wslc/daemon/src/session_manager.rs:168
- This new rationale is stale within the same change:
StreamFramepayloads are now base64 strings, not JSON number arrays. The 64 KiB limit remains frame-safe, but documenting a ~4× expansion misstates the wire format and can lead to incorrect future capacity calculations.
/// allocation and keeps the resulting `Stdout`/`Stderr` frame well under the
/// protocol's `MAX_FRAME_SIZE` (a `Vec<u8>` serializes as a JSON number array,
/// ~4x expansion), so a large callback can never overflow a frame and abort the
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Verified all four prior review findings at 527a731.
Validation:
- npm run build: passed
- npm test: 268 runnable tests passed, 0 failed, 6 skipped
- GitHub CI: all checks green
The branch is currently behind its declared base (merge-base b9d08f4 vs base 1c11f25), so it should be rebased and revalidated before merge.
…he state-aware lifecycle branch Three SDK files conflicted because microsoft#810 added the WSLc backend to the same registries this branch adds LXC to. Every conflict resolved as the union of both backends, not a choice between them. sdk/node/src/state-aware-types.ts StateAwareContainmentBackend now carries both 'lxc' and 'wslc'. sdk/node/src/state-aware-helper.ts microsoft#810 replaced the hand-written PREFIX_TO_BACKEND map with an exhaustive BACKEND_TO_PREFIX that PREFIX_TO_BACKEND is derived from, so this branch's copy of the old map is redundant and was dropped rather than merged. LXC is registered in BACKEND_TO_PREFIX and in the new DEFAULT_STATE_AWARE_VERSION map, which the Record type makes a compile error to omit. LXC defaults to STATE_AWARE_VERSION, the version it already sent; only WSLc was promoted at a later schema version. sdk/node/README.md Both backend lists merged, keeping the LXC-specific caveats that it is Linux-only, not experimental, and has no streaming exec. src/backends/lxc/common/src/state_aware.rs Upstream added a fifth parameter to StatefulSandboxBackend::exec, so the LXC implementation no longer compiled. Named _consumer to match the other three in-tree backends, none of which honors ExecConsumer::Library yet: LXC relays internally through attach_run and returns null pipe handles, which the trait documents as the executor-only path. Verified: cargo fmt --all --check, cargo clippy -D warnings, cargo check --workspace --all-targets, 875 Rust tests, and 286 SDK tests, all passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d79fea-ab96-467a-af61-dda2cebf1168
📖 Description
.github/copilot-instructions.md.Summary
Final increment of the WSLc state-aware sandbox lifecycle series: the TypeScript SDK surface. This is a pure SDK-surface addition — no runtime behavior. The daemon, Rust
StatefulSandboxBackend, wire model, and generated wire types all landed in the earlier increments; this PR exposes the WSLc backend through the existing generic state-aware functions (provisionSandbox/startSandbox/execInSandbox/execInSandboxAsync/stopSandbox/deprovisionSandbox).Changes:
state-aware-types.ts): adds'wslc'toStateAwareContainmentBackend; five per-phaseWslc*Configinterfaces;wslcentries in the closed config + metadata registries (WSLc returns no metadata for any phase). Configs mirror the backend honor matrix — provision takesfilesystem/network(cross-cutting) plus backend-specificimage/imageTarPath; exec takesprocessplus a cooperativenetwork.proxy; start/stop/deprovision takeversiononly.state-aware-helper.ts): adds thewslc:id prefix,SandboxId<'wslc'>branding viaPREFIX_TO_BACKEND, and a per-backend default-version map so WSLc envelopes default to0.8.0-alphawhileisolation_session/windows_sandboxstay0.6.0-alpha(the gate-lockedSTATE_AWARE_VERSIONis untouched).index.ts) + docs (README.md) + unit tests covering version default,experimental.wslc.provisionnesting, cross-cutting lifting, prefix routing, metadata-undefined, and brand distinctness.Consistent with the
windows_sandboxprecedent: SDK types are permissive where the Rust backend enforces the honor matrix at runtime (e.g. aproxyat provision is expressible but rejected withpolicy_validation).Validation:
tscclean · 268/274 SDK unit tests pass ·check-schema-versions.js/check-sdk-types-codegen.js/check-version-sync.jsgreen.Stacked on #801.
📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow