[WSLC] State-aware lifecycle (PR 1/3): persistent daemon + named-pipe IPC control plane by SohamDas2021 · Pull Request #745 · microsoft/mxc

Soham Das (SohamDas2021) · 2026-08-04T23:35:37Z

📖 Description

First of three PRs delivering the state-aware WSLc lifecycle. The WSLc SDK (2.9.3) has no cross-process re-attach — WslcSession/WslcContainer/WslcProcess handles are in-process only — so a persistent, per-Windows-user daemon
must hold the handles across the separate provision/start/exec/stop/deprovision phase processes. This PR lands that daemon and its IPC control plane. It has no product-visible surface (no wire.rs/schema changes; nothing invokes the daemon yet), so it merges independently.

What's included

New wxc-wslc-daemon crate (src/backends/wslc/daemon/):
- main.rs — bootstrap, bind-before-publish daemon record, dedicated WSLc worker thread/COM apartment, idle watchdog (teardown on no live containers + no in-flight clients).
- control_server.rs — owner-only-SDDL named pipe, length-prefixed JSON frame dispatch
  (PROVISION/START/EXEC/STOP/DEPROVISION/PING).
- session_manager.rs — real WslcSession/container/process lifecycle over container_steps, holding the refcounted sandbox_id -> WslcContainer map on a thread-affine worker.
wslc_common additions: daemon_protocol.rs (control frames + internal per-phase config structs, deliberately separate from the public wire schema), daemon_record.rs (discovery record, pid+creation-time liveness, owner-only DACL hardening, transition lock), daemon_client.rs (spawn/poll, connect, exec result accumulation — live bidirectional stdio bridge deferred to PR 2).
New container_steps.rs: daemon-side WSLc step primitives (create session/container, exec, stop, delete) adapted for a warm keepalive-init session. The one-shot wsl_container_runner.rs is unchanged except two visibility relaxations so the daemon steps can reuse its import_image_from_tar + wslc_prerequisite_error.
Build wiring: build.bat stages the daemon exe next to wxc-exec + SDK bin.
Tests: Rust integration test drives the daemon directly over the pipe (real container + 2× exec on a warm container); the full-lifecycle test is #[ignore]d (requires a WSL2 host with alpine:latest pre-pulled). Unit tests for framing, refcount, idle-teardown, transition lock, and record trust run by default.

Coming in the pipeline

PR 2/3 — state-aware backend + wire/schema + E2E: wslc/common/state_aware.rs (StatefulSandboxBackend, prefix wslc) translating the public experimental.wslc.* wire schema into daemon protocol frames; mxc_engine state-aware arm + config-parser wiring; regenerated dev schema + generated TS wire types; multi-invocation E2E script with warm-reuse + idle-teardown assertions;
PR 3/3 — TypeScript SDK: Wslc*Config/*Result types + branded SandboxId<'wslc'> and helper prefix wiring, mirroring the LXC state-aware SDK surface.

🔗 References

🔍 Validation

✅ 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

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 GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-08-04T23:35:46Z

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

Copilot

Pull request overview

Introduces the persistent per-user WSLc daemon and internal IPC foundation for the upcoming state-aware lifecycle.

Changes:

Adds named-pipe protocol, daemon discovery, lifecycle worker, and client.
Refactors WSLc operations into reusable container steps.
Adds build integration and daemon IPC tests.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`src/Cargo.toml`	Registers the daemon crate.
`src/Cargo.lock`	Locks new crate dependencies.
`src/backends/wslc/daemon/tests/daemon_ipc.rs`	Tests pipe and lifecycle IPC.
`src/backends/wslc/daemon/src/session_manager.rs`	Owns WSLc sessions and containers.
`src/backends/wslc/daemon/src/main.rs`	Bootstraps and monitors the daemon.
`src/backends/wslc/daemon/src/control_server.rs`	Implements named-pipe request dispatch.
`src/backends/wslc/daemon/Cargo.toml`	Defines the daemon package.
`src/backends/wslc/daemon/build.rs`	Embeds executable version metadata.
`src/backends/wslc/common/src/wslc_bindings.rs`	Requires process-signalling support.
`src/backends/wslc/common/src/wsl_container_runner.rs`	Uses shared WSLc construction helpers.
`src/backends/wslc/common/src/lib.rs`	Exports daemon support modules.
`src/backends/wslc/common/src/daemon_record.rs`	Manages discovery and transition locking.
`src/backends/wslc/common/src/daemon_protocol.rs`	Defines IPC messages and framing.
`src/backends/wslc/common/src/daemon_client.rs`	Discovers and calls the daemon.
`src/backends/wslc/common/src/container_steps.rs`	Extracts reusable WSLc lifecycle operations.
`src/backends/wslc/common/Cargo.toml`	Adds protocol dependencies.
`build.bat`	Stages the daemon for the Node SDK.

Copilot

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/backends/wslc/daemon/src/control_server.rs:260

Exec is admitted before checking whether the sandbox exists or is started. Those failures consequently arrive as an untyped StreamFrame::Error after Ok, rather than the documented DaemonResponse::Err with NotProvisioned/NotStarted. Split admission/state validation from execution and send Ok only after admission succeeds.

    write_frame(&mut pipe, &DaemonResponse::Ok).await?;
    let terminal = match session.exec(config).await {
        Ok(code) => StreamFrame::Exit { code },
        Err(e) => StreamFrame::Error {

src/backends/wslc/daemon/src/control_server.rs:273

Every start/stop/deprovision failure is classified as Backend, including the explicit unknown-sandbox and invalid-state paths. This makes the stable ErrKind::NotProvisioned/NotStarted contract unusable by the later state-aware adapter. Preserve a typed worker error and map each state failure to its declared protocol kind instead of erasing it into anyhow.

fn ok_or_err(result: Result<()>) -> DaemonResponse {
    match result {
        Ok(()) => DaemonResponse::Ok,
        Err(e) => err_response(ErrKind::Backend, e),
    }

src/backends/wslc/daemon/src/main.rs:133

The zero-client check is not coordinated with pipe acceptance, and the discovery record remains ready while shutdown begins. A phase can discover this daemon just after the check, then either lose its listening pipe or be accepted while run() shuts down the worker, causing a transient lifecycle failure. Make shutdown an atomic admission state (retire the record/listener under the transition protocol and drain admitted clients), or make the client rediscover/spawn after a stale-pipe open failure.

            Ok(0) if active_clients.load(Ordering::SeqCst) == 0 => {
                idle_for += IDLE_POLL;
                if idle_for >= IDLE_TIMEOUT {
                    shutdown.notify_waiters();
                    return;

src/backends/wslc/daemon/src/session_manager.rs:331

ExecConfig::working_directory is documented as an in-container path (and the protocol test uses /work), but this call reaches ProcessSettings::build, which only accepts Windows drive paths via windows_path_to_container_path. A normal container path therefore gets silently omitted. Add a daemon exec builder/path mode that passes the container cwd through verbatim while retaining Windows-path mapping for the one-shot runner.

                &config.working_directory,

src/backends/wslc/daemon/src/session_manager.rs:344

The PR describes an exec stdio bridge and the protocol promises stdout/stderr frames, but the captured buffers are discarded here, so echo hi returns an empty ExecResult. Return the captured outcome through the worker and emit Stdout/Stderr frames before Exit; the IPC lifecycle test should assert the output rather than only the exit code.

        // NOTE: outcome.stdout/stderr are captured but not yet forwarded — live
        // stdio streaming over the control pipe is a later fill-in; the PR1
        // contract returns the exit code only.
        Ok(outcome.exit_code)

Address HIGH + MEDIUM findings from the multi-model review of the WSLc state-aware daemon (PR 1): - Fix a use-after-free of the SDK-owned Arc<IoContext>: reclaim the reference inside exit_callback (leak-on-kill rather than free while the SDK may still call back); drop IoCtxRawGuard. - Order Worker fields so the SDK/DLL drops last, after handle guards. - Rewrite deprovision to delete the container before removing bookkeeping, leaving the entry retryable on failure. - Add WslcSignalProcess to the required-symbol check. - Bind the control pipe before publishing a single ready record. - Handle pipe-accept errors locally instead of killing the daemon. - Gate the idle watchdog on no in-flight clients and no live containers. - Verify record + directory-chain ownership and validate the pipe name on the read path; harden every directory component, not just the leaf. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3ae1af2-7b79-4340-a5ce-a5402e7ede3d

Copilot

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/backends/wslc/daemon/src/main.rs:132

The zero-client check is not synchronized with the accept loop. A connection can be accepted immediately after this load, after which the watchdog still requests shutdown and run tears down the worker while that provision/exec is in flight. Coordinate shutdown with the server: stop accepting, drain/recheck active requests and container count, then commit teardown atomically.

            Ok(0) if active_clients.load(Ordering::SeqCst) == 0 => {
                idle_for += IDLE_POLL;
                if idle_for >= IDLE_TIMEOUT {
                    shutdown.notify_waiters();

src/backends/wslc/common/src/daemon_protocol.rs:106

This contract says callers provide an in-container path, but ProcessSettings::build_inner passes the value through windows_path_to_container_path; /work is therefore ignored and the process runs in its default directory. The state-aware request also carries the existing host-side working_directory, so document the actual Windows-host-path contract.

    /// Working directory inside the container (empty = container default).
    #[serde(default)]
    pub working_directory: String,

+            if !*exited {
+                let _ = writeln!(
+                    logger,
+                    "[WSLC][daemon] Warning: exit callback did not fire within 30s"
+                );


azure-pipelines · 2026-08-05T22:17:13Z

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

Branden Bonaby (bbonaby) · 2026-08-08T00:29:30Z

+// ---------------------------------------------------------------------------
+// Shared error helper
+// ---------------------------------------------------------------------------


thought: probably would do away with these kinds of comments since they take up space, no clue why the agent always does them. They do it to me too.

Branden Bonaby (bbonaby) · 2026-08-08T00:31:37Z

+    } else {
+        format!("{}: {} (HRESULT 0x{:08X})", context, sdk_msg, hr as u32)
+    };
+    ScriptResponse::error(&msg)


question: I thought for ScriptResponse errors now we try to encapsulate the error in an enum error type since this would help the rust sdk. Could be wrong though but that seemed like a good error pattern.

Branden Bonaby (bbonaby) · 2026-08-08T00:48:54Z

+/// SDK guarantees no I/O callback fires after exit, and the owning
+/// [`ProcessSettings`] holds an independent reference, so the pointer stays valid
+/// for every callback. The SDK guarantees `data` is valid for `data_size` bytes.
+unsafe extern "C" fn io_callback(


issue: can we move all the unsafe functions to a wslc_sys.rs file under src/backends/wslc/common/src/ ? That would be helpful so we have all the unsafe functions in one place that we can review.

Branden Bonaby (bbonaby) · 2026-08-08T00:57:36Z

+    _sh: Vec<u8>,
+    _dash_c: Vec<u8>,
+    _script_cstr: Vec<u8>,
+    _argv: Vec<PCSTR>,
+    _env_cstrings: Vec<Vec<u8>>,
+    _env_ptrs: Vec<PCSTR>,
+    _cwd_cstr: Option<Vec<u8>>,


question: why the need for underscores here? Is it because these fields won't be used in the PR but another one?

Branden Bonaby (bbonaby) · 2026-08-08T01:01:40Z

+    // creation fails before a live process adopts it. Disarmed by
+    // `mark_process_created` once a process is created, after which
+    // `exit_callback` (or a deliberate leak on kill) owns reclamation.
+    sdk_io_ref: SdkIoRef,


thought: haven't read through the rest of this yet but seeing deliberate leak on kill makes me think we need a simple design doc (mostly diagrams, headings and brief text, nothing long form). That said maybe it'll make sense further down.

Branden Bonaby (bbonaby)

issue (blocking): wxc-wslc-daemon.exe is required beside wxc-exec.exe, but it is absent from the ADO signing/copy pattern, GitHub artifact upload, and playground resources. Could we add it to all three manifests so release builds sign and ship the daemon?

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+/// to be: a process with its PID is currently running AND its creation time
+/// matches the recorded one.
+pub fn daemon_alive(record: &DaemonRecord) -> bool {
+    running_process_creation_time(record.pid) == Some(record.pid_creation_time)


praise: Pairing PID liveness with creation time and validating the record owner and pipe prefix is a strong fail-closed discovery design.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+name = "wxc-wslc-daemon"
+path = "src/main.rs"
+
+[dependencies]


issue: This workspace member imports Windows-only APIs unconditionally, so documented non-Windows workspace commands fail. We should target-gate the dependencies/code and provide a non-Windows stub main.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+    match io_handle {
+        WslcProcessIOHandle::WSLC_PROCESS_IO_HANDLE_STDOUT => {
+            let mut buf = ctx.stdout.lock().unwrap_or_else(|e| e.into_inner());
+            buf.extend_from_slice(bytes);


issue (blocking): stdout/stderr comes from untrusted code and grows these buffers without limit, so one exec can OOM the daemon and terminate every owned sandbox. Let's cap each stream or use bounded streaming/backpressure.

Good catch- proper bounded streaming/backpressure is coupled to the 2c stdio work, but I'll add an interim per-stream byte cap (truncate with a marker) in this PR so a single exec can't OOM the daemon and take down every owned sandbox.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+            windows::Win32::Foundation::HANDLE(exit_event),
+            wait_ms,
+        );
+        if wait_result == windows::Win32::Foundation::WAIT_TIMEOUT {


issue (blocking): A timeout is reported even if SIGKILL fails, and unexpected waits or a missing exit callback still fall through to WslcGetProcessExitCode (possibly STILL_ACTIVE). Could we require positive exit confirmation and retire the warm container when termination cannot be confirmed?

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+        // Working directory (mapped Windows -> container path; skip if unmapped).
+        let mut cwd_cstr: Option<Vec<u8>> = None;
+        if !working_directory.is_empty() {
+            if let Some(container_cwd) =


issue: working_directory is an in-container path, but /work is treated as a Windows path and silently ignored. We can avoid that fallback by validating an absolute container path and passing it directly to the SDK.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+                let session = session.clone();
+                let active = active_clients.clone();
+                active.fetch_add(1, Ordering::SeqCst);
+                tokio::spawn(async move {


issue: These handlers are detached, so shutdown can tear down the worker while an accepted request is still running. We should retain the tasks, stop admission atomically, and drain them before worker shutdown.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+
+/// Service exactly one request on a freshly-connected pipe instance.
+async fn handle_client(mut pipe: NamedPipeServer, session: SessionHandle) -> Result<()> {
+    let request: DaemonRequest = read_frame(&mut pipe).await?;


issue: The first frame has no deadline and each connection creates an unbounded task, so an abandoned same-user client can retain daemon resources indefinitely. Could we add a short request timeout and a bounded client semaphore?

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+    // Ensure the watchdog and worker are torn down.
+    shutdown.notify_waiters();
+    watchdog.abort();
+    let _ = session.shutdown().await;


issue: The record remains ready while the server stops and is removed outside the transition lock, so a client can select a daemon that is already disappearing. Let's mark it not-ready and serialize admission shutdown, draining, worker teardown, and record removal with the transition lock.

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+        let _ = self.child.wait();
+        // The daemon was killed before it could clean up; drop the stale record
+        // so it does not confuse a later run.
+        let _ = remove_daemon_record();


issue: This test uses and unconditionally deletes the real per-user daemon record, which can orphan a live daemon or race another test run. Could we use an isolated test state root and remove only the record owned by this child?

Branden Bonaby (bbonaby) · 2026-08-08T01:27:21Z

+use crate::daemon_record::{live_daemon, DaemonRecord, TransitionLock};
+
+/// Max wait to acquire the daemon spawn/teardown transition lock.
+const SPAWN_LOCK_TIMEOUT: Duration = Duration::from_secs(30);


issue: This 30-second lock can expire before the 60-second readiness wait it protects, so a second client can fail while the first daemon is still within its valid startup window. We should make the lock timeout longer than READY_TIMEOUT.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+            match OpenOptions::new()
+                .read(true)
+                .write(true)
+                .open(&self.pipe_name)


src/backends/wslc/common/src/daemon_client.rs:220

High (security) - authenticate the pipe server and restrict impersonation.

This opens the endpoint named by the same-user-owned record with default named-pipe QoS. Windows defaults that to SecurityImpersonation, so a hostile same-user pipe server can impersonate an elevated phase process after planting a valid-looking record containing its own PID, creation time, and prefixed pipe name. The client also never verifies that the connected server PID matches the record. The existing diagnostic-pipe client already demonstrates the repository's GetNamedPipeServerProcessId plus integrity-token verification pattern.

Fix: open with SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION (or a stricter level), then verify the connected server PID/creation time against the trusted record or use a per-spawn nonce authenticated on the wire.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+impl OwnerOnlySecurity {
+    fn new() -> Result<Self> {
+        let sid = current_user_sid_string()?;
+        let sddl = format!("D:P(A;;FA;;;{sid})(A;;FA;;;SY)");


src/backends/wslc/daemon/src/control_server.rs:132

Medium (security) - the pipe ACL does not preserve the caller's integrity boundary.

The ACL grants full access to the user SID and SYSTEM but does not add a mandatory label or authenticate the connecting token. Elevated and non-elevated processes for a split-token user have the same user SID, so a medium-integrity process can drive a daemon spawned elevated. Provision then passes caller-supplied host mounts to the SDK without validating that the caller could access those paths itself, turning the elevated daemon into a confused deputy.

Fix: reject clients whose integrity/elevation does not match the daemon (or stamp the appropriate mandatory label), and validate host mount paths under the authenticated caller's security context.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+/// affinity) and processes [`WorkerCommand`]s until a [`WorkerCommand::Shutdown`]
+/// is received or the command channel closes.
+pub fn spawn() -> Result<SessionHandle> {
+    let (tx, mut rx) = mpsc::unbounded_channel::<WorkerCommand>();


src/backends/wslc/daemon/src/session_manager.rs:428

High (reliability/performance) - a long exec freezes the entire daemon while requests accumulate without bound.

All lifecycle operations share this one worker and unbounded channel. WorkerCommand::Exec calls exec_in_container, which synchronously waits on the process exit event; timeout_ms == 0 becomes an effectively unbounded wait. During that time, unrelated provision/start/stop/deprovision requests and the watchdog's ContainerCount request queue indefinitely, consuming memory and preventing recovery.

Fix: use a bounded channel with explicit backpressure/Busy, and move process-exit waiting off the SDK command worker (or introduce per-container execution workers) so unrelated sandboxes remain serviceable. Add a regression test with one blocked exec and a second sandbox operation.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+                }
+                let connected = server;
+                // Create the next instance before servicing this one.
+                server = create_secured_instance(&pipe_name, &security, false)?;


src/backends/wslc/daemon/src/control_server.rs:82

Medium (reliability) - failure to create the next pipe instance terminates every live sandbox.

This ? runs after a client has connected but before that connection is handed to handle_client. A transient handle/resource-exhaustion failure therefore abandons the accepted client, exits the control server, and causes main to shut down the shared session and all containers. The connect-error path above has the same propagation when re-creating its instance.

Fix: retry instance creation with bounded backoff while retaining or explicitly failing the accepted client; do not turn one transient accept-capacity failure into daemon-wide teardown.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+    match io_handle {
+        WslcProcessIOHandle::WSLC_PROCESS_IO_HANDLE_STDOUT => {
+            let mut buf = ctx.stdout.lock().unwrap_or_else(|e| e.into_inner());
+            buf.extend_from_slice(bytes);


src/backends/wslc/common/src/container_steps.rs:100

High (reliability/performance) - daemon exec output can exhaust process memory.

The new daemon callback appends all stdout/stderr into unbounded vectors until the process exits. A container command that continuously writes output grows the long-lived daemon without limit; because exec also occupies the sole worker, other lifecycle requests cannot make progress while memory is consumed.

Fix: enforce a documented per-stream or aggregate capture limit, stop extending after the limit, and report truncation in the terminal result. Cover a command that exceeds the limit.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+    if len > MAX_FRAME_SIZE {
+        bail!("incoming frame length {len} exceeds maximum {MAX_FRAME_SIZE}");
+    }
+    let mut body = vec![0u8; len];


src/backends/wslc/daemon/src/control_server.rs:291

Medium (security/performance) - each unauthenticated connection can reserve 16 MiB and stall forever.

After sending only a four-byte prefix, a client can make the daemon allocate up to MAX_FRAME_SIZE and then stop sending. Pipe instances and handler tasks are not globally bounded, and there is no read timeout, so repeated same-user connections can exhaust memory and also keep active_clients nonzero indefinitely.

Fix: bound concurrent clients and aggregate in-flight frame bytes, apply a read/idle timeout, and read large bodies incrementally instead of reserving the full declared size immediately.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+        let mut cwd_cstr: Option<Vec<u8>> = None;
+        if !working_directory.is_empty() {
+            if let Some(container_cwd) =
+                policy_mapping::windows_path_to_container_path(working_directory)


src/backends/wslc/common/src/container_steps.rs:305

Medium (correctness) - the documented in-container working directory is treated as a Windows host path.

ExecConfig::working_directory is documented as a path inside the container, and the protocol round-trip test even uses /work. This code instead calls windows_path_to_container_path; a normal Linux path returns None, after which the setting is silently omitted and the command runs in the container default directory.

Fix: pass an already-container-native path directly, or explicitly define and validate a host-path contract. Do not silently discard a non-empty value; add an exec test using /tmp or /work.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+            Ok(0) if active_clients.load(Ordering::SeqCst) == 0 => {
+                idle_for += IDLE_POLL;
+                if idle_for >= IDLE_TIMEOUT {
+                    shutdown.notify_waiters();


src/backends/wslc/daemon/src/main.rs:132

High (correctness/reliability) - the idle shutdown notification can be lost permanently.

Notify::notify_waiters() does not retain a permit when no task is currently waiting. The control server awaits shutdown.notified() only inside its select; if the watchdog reaches this line while the server is executing the connected-client branch between select iterations, the notification is lost. The watchdog then returns, leaving the daemon alive indefinitely with no future idle checks. The separate container_count and active_clients observations also make the final-idle boundary race-prone.

Fix: use a cancellation primitive or watch channel with persistent state, or at minimum a retained notify_one permit, and add a deterministic test that triggers idle expiry while the accept branch is active.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+        DaemonRequest::Provision(config) => {
+            let resp = match session.provision(config).await {
+                Ok(sandbox_id) => DaemonResponse::Provisioned { sandbox_id },
+                Err(e) => err_response(ErrKind::Backend, e),


src/backends/wslc/daemon/src/control_server.rs:224

Medium (correctness/API contract) - every production failure is classified as Backend.

The protocol defines NotProvisioned, NotStarted, Busy, NotReady, and Protocol specifically so the later state-aware client can map failures without parsing strings. Production dispatch constructs only Backend, even for explicit unknown sandbox and not started errors, and no single-flight path emits Busy. The advertised stable classification is therefore unusable before PR 2 consumes it.

Fix: introduce a typed worker error and map each state/protocol failure to its declared ErrKind; add dispatch tests that assert the wire classification, not just the message.

Gudge (MGudgin) · 2026-08-08T20:46:08Z

+        // script heaps; all are owned by the returned struct.
+        let sh = b"/bin/sh\0".to_vec();
+        let dash_c = b"-c\0".to_vec();
+        let script_cstr = format!("{}\0", script_code).into_bytes();


src/backends/wslc/common/src/container_steps.rs:269

Medium (correctness/security) - interior NULs silently truncate values passed to the SDK.

The protocol accepts arbitrary JSON strings, but command, environment, image, and container-path C strings are created by appending \0. An input containing \u0000 is accepted in full by Rust and then observed only up to the first NUL by the SDK, so validation/logging and execution can disagree about the command or path.

Fix: construct every SDK C string with CString::new and reject interior NULs at the protocol boundary with a typed protocol error. Add round-trip rejection tests for command, environment, image, and mount fields.

Copilot AI balanced review requested due to automatic review settings August 4, 2026 23:35

Copilot started reviewing on behalf of Soham Das (SohamDas2021) August 4, 2026 23:36 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 5, 2026 15:45

Copilot started reviewing on behalf of Soham Das (SohamDas2021) August 5, 2026 15:45 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Soham Das (SohamDas2021) and others added 3 commits August 5, 2026 11:40

Add WSLc state-aware daemon and IPC control plane

80049fb

Addressed PR comments

bd4b3a7

Copilot AI review requested due to automatic review settings August 5, 2026 20:02

Soham Das (SohamDas2021) force-pushed the user/sodas/wslc-state-aware-daemon branch from 945d371 to bd4b3a7 Compare August 5, 2026 20:02

Copilot started reviewing on behalf of Soham Das (SohamDas2021) August 5, 2026 20:03 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Comment thread src/backends/wslc/common/src/container_steps.rs

Comment on lines +851 to +855

if !*exited {

let _ = writeln!(

logger,

"[WSLC][daemon] Warning: exit callback did not fire within 30s"

);

Soham Das (SohamDas2021) requested review from Gudge (MGudgin) and Branden Bonaby (bbonaby) August 5, 2026 22:13

Soham Das (SohamDas2021) marked this pull request as ready for review August 5, 2026 22:17

Soham Das (SohamDas2021) requested a review from a team as a code owner August 5, 2026 22:17

Branden Bonaby (bbonaby) reviewed Aug 8, 2026

View reviewed changes

Gudge (MGudgin) requested changes Aug 8, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned Soham Das (SohamDas2021) Aug 8, 2026

microsoft-github-policy-service Bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Aug 8, 2026

Uh oh!

Conversation

Soham Das (SohamDas2021) commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

What's included

Coming in the pipeline

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

azure-pipelines Bot commented Aug 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Branden Bonaby (bbonaby) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Soham Das (SohamDas2021) commented Aug 4, 2026 •

edited

Loading