Skip to content

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

Open
Soham Das (SohamDas2021) wants to merge 1 commit into
user/sodas/wslc-state-aware-daemonfrom
user/sodas/wslc-daemon-hardening
Open

[WSLC] Harden state-aware daemon (PR 2a/3): typed errors, validate-then-admit exec, idle-watchdog#767
Soham Das (SohamDas2021) wants to merge 1 commit into
user/sodas/wslc-state-aware-daemonfrom
user/sodas/wslc-daemon-hardening

Conversation

@SohamDas2021

@SohamDas2021 Soham Das (SohamDas2021) commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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 errorsWorkerError{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 — a ValidateExec worker command validates the sandbox exists and is started before the Ok admission is written, so an unknown/not-started sandbox comes back as a pre-admission typed Err rather than a post-admission stream Error frame.
  • 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

📋 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

Copilot AI balanced review requested due to automatic review settings August 7, 2026 00:37
@azure-pipelines

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

@SohamDas2021
Soham Das (SohamDas2021) changed the base branch from main to user/sodas/wslc-state-aware-daemon August 7, 2026 00:38
@SohamDas2021 Soham Das (SohamDas2021) changed the title [WSLC] Harden state-aware daemon: typed errors, validate-then-admit exec, idle-watchdog [WSLC] Harden state-aware daemon (2a/3): typed errors, validate-then-admit exec, idle-watchdog Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines 268 to 272
if let Err(e) = session.validate_exec(config.sandbox_id.clone()).await {
write_frame(&mut pipe, &worker_err_response(e)).await?;
return Ok(());
}
write_frame(&mut pipe, &DaemonResponse::Ok).await?;
Copilot AI review requested due to automatic review settings August 7, 2026 03:21
@SohamDas2021
Soham Das (SohamDas2021) force-pushed the user/sodas/wslc-daemon-hardening branch from 2336f9a to 884d80b Compare August 7, 2026 03:21
@SohamDas2021
Soham Das (SohamDas2021) marked this pull request as ready for review August 7, 2026 03:21
@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code owner August 7, 2026 03:21
@azure-pipelines

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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())),

Comment on lines +159 to +161
let active = active_clients.load(Ordering::SeqCst);
let generation = activity.load(Ordering::SeqCst);
let idle = count == 0 && active == 0 && generation == last_activity;
@SohamDas2021 Soham Das (SohamDas2021) changed the title [WSLC] Harden state-aware daemon (2a/3): typed errors, validate-then-admit exec, idle-watchdog [WSLC] Harden state-aware daemon (PR 2a/3): typed errors, validate-then-admit exec, idle-watchdog Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants