Repository navigation
[WSLC] (3/4) Add the session lifecycle phases and dispatch routing - #1503
Conversation
5eb8b8d to
8c425b6
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The internal additions preserve existing caller contracts, reject unsupported operations, and have focused regression coverage with no blocking findings.
0 open findings
What changed in this PR
Adds internal WSLc session lifecycle operations and routing, preparing multi-session support for caller exposure in part 4 without changing accepted request JSON.
Changes:
- Adds session phases, identity-aware routing, binding, and result handling.
- Rejects unsupported session operations, including dry runs, and includes session sizing in policy hashes.
- Adds regression tests and updates lifecycle documentation.
| File | Description |
|---|---|
src/mxc-sdk/src/state_aware_sdk.rs |
Preserves session IDs and rejects unexpected result fields. |
src/mxc-sdk/src/core/mxc_engine/state_aware.rs |
Handles session outcomes and tests engine routing. |
src/mxc-sdk/src/core/mxc_common/state_aware_request.rs |
Defines session phases and target identity categories. |
src/mxc-sdk/src/core/mxc_common/state_aware_operation.rs |
Adds WSLc-only session operations. |
src/mxc-sdk/src/core/mxc_common/state_aware_dispatch.rs |
Dispatches session operations and encodes results. |
src/mxc-sdk/src/core/mxc_common/state_aware_binding.rs |
Preserves session configuration and IDs during binding. |
src/mxc-sdk/src/core/mxc_common/state_aware_binding_tests.rs |
Tests binding and validation/execution ordering. |
src/mxc-sdk/src/core/mxc_common/state_aware_backend.rs |
Adds session methods and rejecting validation defaults. |
src/mxc-sdk/src/core/mxc_common/splice.rs |
Supports session identity members in routing JSON. |
src/mxc-sdk/src/core/mxc_common/sdk_input.rs |
Restricts typed session input to the development contract. |
src/mxc-sdk/src/core/mxc_common/policy_identity.rs |
Hashes session sizing while excluding session IDs. |
src/mxc-sdk/src/core/mxc_common/models.rs |
Defines optional session sizing configuration. |
src/mxc-sdk/src/core/mxc_common/config_parser.rs |
Handles session routing while preserving JSON rejection. |
docs/development/glossary.md |
Expands lifecycle terminology to include session phases. |
docs/development/architecture/container-lifecycle.md |
Documents session interfaces, routing, and validation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Gudge (MGudgin)
left a comment
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Summary
Changes requested for this phase: Exercise the new session operations through run_typed_state_aware_request with WSLc present and absent, including dry runs; the added feature-gate tests currently call only the separate envelope router. Pin the canonical session policy identity rather than just comparing unequal hashes, so a field-key or phase change cannot silently change that identity. These are checks of #1503's own new path, not inherited #1489 issues. The comments below also describe the newly exposed telemetry sweep and a false exact-contract comment; the latter is a claim mismatch, not an independent merge condition.
Verified: Base 9398655 is the merge-base of head 8c425b6; the local 15-file diff equals gh pr diff 1503. Exact JSON contracts intentionally reject the two session spellings until 4/4; backend trait execution and dry-run validation default to typed unsupported_phase. The existing telemetry correlation-store source is byte-identical across this PR (blob e4d2bda3), but the new ID-less phases call its existing sweep when telemetry is active. I did not rerun live WSL2 scripts; the seven Windows Instant failures in #1503 CI come from unchanged #1489 code, and are not attributed to this PR.
Findings outside added-line anchors
- Low — Architecture dispatch example omits the new session arms.
docs/development/architecture/container-lifecycle.md:1436-1514—newly_exposed_by_change: the example's original five-arm match was not edited in the PR, but the nearby updatedTypedDispatchOutcomeenum now adds session outcomes. The sketch is illustrative, not executable code; extend it to show validation, dry run and result encoding for the new variants. - Low — Typed deprovision-session ID has no common format check.
state_aware_operation.rs:118-141—newly_exposed_by_change: unlike sandbox phases, the newly added typed session operation does not go through prefix parsing;validate()accepts empty or sandbox-shaped session IDs. The default backend still refuses the phase until 4/4, so this is latent. Add a shared non-empty/shape check before implementing release; do not route by the ID tag. - Low — CLI diagnostics mention a flag that does not yet exist.
config_parser.rs:692-707—introduced_by_change: the new--session-iderror text is staged beforewxchas the flag, and one none-ID arm always names--container-id. Registered JSON contracts still reject session phases, so this is phase-4 diagnostic cleanup, not a current CLI failure. - Low — Normalized session fields lack direct assertions.
config_parser.rs:1653-1705—introduced_by_change: new containment and network-normalization cases are checked only indirectly via errors fromoperation.containment(), not fromrequest.containmentor the normalized network fields. Assert the intended WSLc containment and network outcome in targeted normalization tests. - Low — Trait overview omits session exceptions.
docs/development/architecture/container-lifecycle.md:1080-1085—newly_exposed_by_change: the overview's associated-type generalization predates this PR, but the new concrete WSLc session config and no-metadata result create an exception, documented later in the guide. Qualify the overview to sandbox phases.
Verified pre-existing — not attributed to this PR
src/mxc-sdk/src/core/mxc_common/telemetry/correlation_state.rsis byte-identical. Its sweep is cited only because the new ID-less phases make that existing branch newly relevant.- The fresh-Windows-host
Instant::now() - 24hpanic and daemon shutdown/slot ownership paths are in #1489's base. Their CI failures do not make them #1503 findings. - A
wslcsess:tag is not registered as a backend-routing prefix, so the changed comment saying session IDs carry no backend prefix is not a routing defect. The facade intentionally rejects passing a minted session result through sandbox-only converters. These two speculative items from the initial review were withdrawn.
| /// exist yet) mints a fresh vector; every later sandbox phase recalls the same | ||
| /// lifecycle root persisted by `provision` and spins a distinct child off it — | ||
| /// see [`telemetry::correlation_state`]. | ||
| /// see [`telemetry::correlation_state`]. Session phases carry no `sandboxId`, |
There was a problem hiding this comment.
Medium (performance) — ID-less session phases trigger a correlation-store sweep before dispatch.
Attribution: newly_exposed_by_change — this added comment describes two new phases with no sandboxId; phase_correlation passes is_provision=false and no ID to the byte-identical pre_dispatch_vector. With telemetry enabled, that path calls prune_stale_records(None) and walks the correlation store before it seeds a vector—even for dry-run or unsupported internal session operations. It adds filesystem-scale latency and lock contention to the new typed path; telemetry-disabled calls do not pay it.
Fix: Seed a session-phase vector without sweeping sandbox records, or perform stale-record pruning only for operations that manage a persisted sandbox lifecycle root.
There was a problem hiding this comment.
Leaving this for 4/4, where the session phases first become reachable; today every provision, dry runs included, already runs the same sweep.
microsoft-github-policy-service
Bot
added
the
Needs-Author-Feedback
8c425b6 to
c90c0bd
Compare
c90c0bd to
d1df0ea
Compare
|
Deferred to 4/4:
|
microsoft-github-policy-service
Bot
added
Needs-Attention
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
d1df0ea to
30dbe04
Compare
30dbe04 to
91e9a44
Compare
9c7cba4 to
d1d473e
Compare
d1d473e to
0565dd1
Compare
Gudge (MGudgin)
left a comment
Gudge (MGudgin)
left a comment
There was a problem hiding this comment.
Re-verified #1503 at head 0565dd1. The typed session-route tests now cover both feature configurations and dry runs; canonical sizing/phase identity, session-ID validation, normalization, and the architecture dispatch sketch are corrected. Targeted local checks for the new route, hash, and normalization passed (5 tests); current PR checks have no failures or pending jobs. I did not rerun live WSL2 tests. Session-phase correlation still sweeps the sandbox store in this phase, but #1512 changes that path and adds a regression test. This approval covers the staged internal phase only, not #1512 or an independently published session API.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cde09ffb-ee78-46e1-bea1-830884f0a318
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cde09ffb-ee78-46e1-bea1-830884f0a318
…ng it Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cde09ffb-ee78-46e1-bea1-830884f0a318
0565dd1 to
84152da
Compare
microsoft-github-policy-service
Bot
removed
the
Needs-Attention
📖 Description
The daemon can hold several sessions since #1489, but nothing can ask it to: there is no vocabulary for creating or releasing one. This PR adds that vocabulary to the lifecycle surface and routes it, without making it reachable from any caller's JSON — no contract root accepts the new phase strings yet, so the whole change is invisible to callers until (4/4).
Phasegainsprovision-sessionanddeprovision-session. Routing became a property of the phase rather than a rule re-derived at each site:target_id_kind()says which identity addresses a phase, androutes_by_containment()is derived from it, because only a sandbox id carries a backend prefix. A session id does not, which is why both session phases declare a containment andbackend_from_prefixdeliberately gains nowslcsessentry. The_arms onPhaseare gone, so the next phase addition is a compile error rather than a silent misroute.The two session operations are WSLc-only in the type system; no other backend is expressible. Their four trait members — both operations and both validation hooks — default to a typed
unsupported_phase, so the other backends need no changes at all. Defaulting the validation hooks matters: a permissive default would let a dry run of an unsupported session operation report success on a backend that cannot perform it.Session sizing is enforcement-relevant, so
cpuCount/memoryMb/gpuenter the policy identity with presence preserved. The opaque session id does not, under the existing rule for sandbox ids. A sandbox joining an existing session declares no sizing and the hash is computed before dispatch, so a sandbox's identity cannot cover its session's sizing; that limit is documented at the projection.🔗 References
Part of #1460. Third of four PRs; stacked on #1489.
🔍 Validation
From
src/:cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p mxc-sdk --features wslc --libcargo test -p mxc-sdk --features wslc --test mxc_contract_v1_1_0_alphacargo test -p mxc-sdk --features wslc --test mxc_contract_v1_0_0cargo test -p mxc-sdk --features wslc --test mxc_contract_v0_9_0_alphacargo test -p mxc-sdk --features wslc --test mxc_contract_version_boundariescargo test -p mxc-sdk --features wslc --test state_awarecargo test -p mxc-sdk --features wslc --test wxc_e2e_tests_e2e_state_awarecargo test -p wxcThe session phases run the same validation / execution / dry-run ordering matrix as every other phase, and a Windows-gated test drives the full path — typed input, normalization, routing, binding, dispatch — to confirm a session phase reaches WSLc and is refused
unsupported_phaseuntil (4/4) implements it. A complementary test pins that a build without thewslcfeature reportsbackend_unavailablerather than a malformed request.Not run:
tests\scripts\run_wslc_*.ps1. They need a live WSL2 host and would exercise only the five sandbox phases, since nothing here is reachable from caller JSON. (4/4) is the first PR they can test.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes📋 Issue Type
Microsoft Reviewers: Open in CodeFlow