[WSLC] Harden state-aware daemon (PR 2a/3): typed errors, validate-then-admit exec, idle-watchdog

[Soham Das (SohamDas2021)]

Summary

PR 1/3 of the WSLc state-aware split landed the per-user daemon + owner-only named-pipe IPC. This PR (call it 2a/3) hardens the daemon internals with no public wire or schema surface, so it lands independently.

Four changes:

• Typed worker errors — WorkerError{NotProvisioned, NotStarted, Backend} with a kind() -> ErrKind mapping instead of collapsing every worker failure to ErrKind::Backend. The control server returns the real classification, so clients can distinguish an unknown/not-started sandbox from a backend fault without string-matching. Display preserves the existing "unknown sandbox" message.
• Validate-then-admit exec — the Exec worker command now carries two reply channels (admit + done). The worker calls validate_exec() (sandbox exists + started) and writes the admit decision before running, so an unknown/not-started sandbox comes back as a pre-admission typed Err rather than a post-admission stream Error frame. Validation and run share one worker handler with no yield between them, so admission is atomic against concurrent Stop/Deprovision.
• Idle-watchdog hardening — a monotonic activity counter bumped on each accepted connection; the watchdog declares idle only when the container count is zero, no request is in flight, and the activity generation is unchanged since the last poll (generation read last, so a connect that starts and finishes between two polls is not missed).
• Dedup — the one-shot runner now uses the shared container_steps::sdk_error instead of a byte-identical private copy.

Testing

• Daemon unit tests (incl. new worker-error kind + validate_exec assertions) and the #[ignore]d WSL2-host lifecycle tests (in-proc and over the pipe): provision → start → exec → exec → stop → deprovision.
• wslc_common suite; full run_wslc_all_tests.ps1 corpus 24/24.
• cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --check clean.
• Reviewed with a 3-model pass; the one flagged finding (idle-watchdog atomic read ordering) is fixed.

Coming in the pipeline

• PR 2b/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]

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

[Copilot]

Pull request overview

Hardens the internal WSLC state-aware daemon’s error handling, exec admission, and idle shutdown behavior.

Changes:

• Adds typed worker errors and protocol mappings.
• Adds pre-admission exec validation.
• Tracks connection activity and deduplicates SDK error handling.

Reviewed changes

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

File	Description
src/backends/wslc/daemon/src/session_manager.rs	Adds typed errors and exec validation commands.
src/backends/wslc/daemon/src/main.rs	Hardens idle-watchdog activity tracking.
src/backends/wslc/daemon/src/control_server.rs	Returns typed errors and validates exec admission.
src/backends/wslc/common/src/wsl_container_runner.rs	Reuses the shared SDK error helper.

[azure-pipelines]

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

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

• This does put ErrKind on the wire, but the repository's DaemonClient immediately converts DaemonResponse::Err { kind, message } into anyhow!("daemon error [{kind:?}]: ...") (common/src/daemon_client.rs:297-300). Consequently actual client callers still cannot distinguish NotProvisioned/NotStarted without parsing the error string, contrary to this PR's typed-error goal. Preserve ErrKind in a concrete client error type (or a typed result) so callers can inspect it directly.

fn worker_err_response(e: WorkerError) -> DaemonResponse {
    DaemonResponse::Err {
        kind: e.kind(),
        message: e.to_string(),

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

• The added admission tests cover only the None branch; no test exercises this new NotStarted classification, even though distinguishing an unstarted sandbox is one of the core behaviors introduced here. Add an exec-before-start assertion (for example in the WSL-host lifecycle test), or factor the lifecycle state from the SDK handle so this branch can be unit-tested without WSL.

            Some(entry) if !entry.started => Err(WorkerError::NotStarted(sandbox_id.to_string())),

[Copilot]

Pull request overview

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

Suppressed comments (1)

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

• This still has a final-check race: a pipe can connect after generation is loaded but before notify_one(). Because activity is incremented only when the control server's connect branch runs (control_server.rs:127), the watchdog can publish shutdown while that connection is pending; the select may take shutdown, drop the connected pipe, and fail a phase request. Coordinate the idle decision with the accept loop (for example, pass a generation-tagged shutdown candidate that the server revalidates while prioritizing a ready connect) so accepting a request and committing shutdown have a single ordering point.

        let active = active_clients.load(Ordering::SeqCst);
        let generation = activity.load(Ordering::SeqCst);
        let idle = count == 0 && active == 0 && generation == last_activity;
        last_activity = generation;

        if idle {
            idle_for += IDLE_POLL;
            if idle_for >= IDLE_TIMEOUT {
                shutdown.notify_one();

[Gudge (MGudgin)]

Medium (reliability) - an exec can start before the client receives the admission response.

This ordering is introduced here: session.exec(config).await returns once the worker sends admit, and that worker immediately enters the blocking execution. If the subsequent DaemonResponse::Ok write fails because the client disconnected, the completion receiver is dropped but the command continues with real side effects. A retry can therefore execute a non-idempotent script twice, with no record of the orphaned result.

Fix: Add an acknowledged start phase so execution begins only after the admission frame is written, or provide cancellation plus explicit logging/result capture when that write fails.

[Soham Das (SohamDas2021)]

Fixed here- if the DaemonResponse::Ok / completion-channel write fails after admission, the worker now logs the orphaned result instead of silently discarding it.

Deferred and planned in PR 2c: the full acknowledged-start phase- begin execution only after the admission frame is written. In 2a's design admission and run-start are deliberately atomic on the single worker thread, which is what closes the Stop/Deprovision interleave window. Decoupling them to gate on the write reintroduces that window, so a clean fix belongs with the streaming/cancellation rework in 2c where the frame protocol already needs revisiting.

[Copilot]

Pull request overview

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

Suppressed comments (3)

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

• There is still a shutdown/admission race here. After the watchdog reads generation at line 224, the accept task can read draining == false; the watchdog can then set draining, and that accept task can increment activity and provision a sandbox. The watchdog has already committed to returning, so the server drains that request and teardown invalidates the ID it just returned. Make connection admission and the final idle transition atomic (for example, increment activity before checking draining, then set draining and recheck the generation before committing, or protect both with one lock/state machine).

                // Enter the draining state *before* signalling shutdown so the
                // accept loop, once it wakes, refuses any client that connected
                // after this final sample instead of provisioning it into a
                // session that is about to be released.
                draining.store(true, Ordering::SeqCst);

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

• The wire now carries the real kind, but DaemonClient immediately converts every DaemonResponse::Err { kind, message } into a plain anyhow::Error string (daemon_client.rs:167, 199, 387-394). Consequently callers still cannot branch on ErrKind without string-matching, contrary to this PR's typed-error goal and preventing the follow-up backend from reliably mapping these errors. Preserve ErrKind in a structured/downcastable client error (or in the client method return type).

fn worker_err_response(e: WorkerError) -> DaemonResponse {
    DaemonResponse::Err {
        kind: e.kind(),
        message: e.to_string(),

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

• Returning immediately when the admission write fails can still silently lose a completed exec result. A fast exec may have already sent into done successfully while this write was in progress; dropping done here then loses that value, and the worker does not emit its orphan warning because its send succeeded. On this error path, retain/await the completion receiver and log its result (or add an explicit delivery acknowledgement) before returning the transport error.

    write_frame(pipe, &DaemonResponse::Ok).await?;