Reconcile orphaned native WSL runtime containers

[AviBackToBlack]

Summary

• add exact Docker label discovery plus fresh ownership/configuration proof for retained native-WSL run containers
• add per-run process-held leases and a private state-directory coordinator lock that closes the container-create-to-lease race without mutating --check
• reconcile proven stopped/running orphans automatically before each run and expose explicit cb wsl cleanup --check|--apply recovery
• retain lease evidence whenever exact container removal is not proven, and keep production WSL activation gated on state commands, integration coverage, and real qualification

Safety contract

• discovery filters never authorize mutation; every candidate is re-inspected and matched against its full ID, labels, retention mode, and stream configuration
• all candidate and lease proofs complete before the first mutation
• locked leases preserve active runs; missing or unlocked leases are orphaned only while the namespace coordinator is held
• running orphans receive SIGKILL, are waited, and then use the existing proof-bound non-force removal
• unsafe/replaced lease files, ambiguous Docker state, or incomplete identity fail closed

Validation

• go test -count=50 ./internal/wslreconcile ./internal/wslrun ./internal/wsldocker
• go test -race ./...
• go vet ./...
• release-style Windows/Linux amd64/arm64 builds with -trimpath -buildvcs=false -ldflags "-s -w -X main.version=v2.0.0-citest"
• canonical Windows amd64 cb.exe version => container-bin v2.0.0-citest

---

[devin-ai-integration]

Devin Review found 3 potential issues.

[Copilot]

Copilot review overview

🟡 Changes recommended

Coordinator failure can discard lease evidence, and duplicate run identities can be reconciled ambiguously.

Review effort: Balanced
Findings: 3

Open (3)

• Use a list-specific executor for the 4 MiB output limit · New
• Reject duplicate run IDs during container discovery · New
• Retain lease until container cleanup is proven successful · New

What changed in this PR

Adds proof-bound recovery for orphaned native-WSL runtime containers while keeping production activation gated.

Changes:

• Adds per-run leases and namespace coordination.
• Adds automatic reconciliation and cb wsl cleanup.
• Updates tests, help text, and WSL documentation.

File	Description
README.md	Documents WSL cleanup and activation status.
main.go	Dispatches and documents cleanup commands.
internal/​wslrun/​runner.go	Integrates run leases into execution.
internal/​wslrun/​runner_test.go	Tests lease lifecycle ordering.
internal/​wslrun/​run_linux.go	Wires production reconciliation.
internal/​wslrun/​plan.go	Carries layout into execution.
internal/​wslreconcile/​reconcile.go	Implements reconciliation and guards.
internal/​wslreconcile/​reconcile_test.go	Tests reconciliation behavior.
internal/​wslreconcile/​production_other.go	Adds non-Linux fail-closed wiring.
internal/​wslreconcile/​production_linux.go	Connects Linux Docker operations.
internal/​wslreconcile/​lease_other.go	Adds non-Linux lease stubs.
internal/​wslreconcile/​lease_linux.go	Implements file locks and leases.
internal/​wslreconcile/​lease_linux_test.go	Tests lease path safety.
internal/​wslreconcile/​command.go	Implements the cleanup CLI.
internal/​wslinstall/​install.go	Updates activation status output.
internal/​wsldocker/​reconcile.go	Adds candidate discovery and proof.
internal/​wsldocker/​reconcile_test.go	Tests discovery and ownership proof.
internal/​hostenv/​hostenv.go	Updates WSL gate messaging.
docs/​wsl.md	Documents reconciliation lifecycle.
docs/​wsl-process-contract.md	Defines lease and recovery semantics.
docs/​security-model.md	Updates WSL cleanup guarantees.
docs/​roadmap-implementation-requirements.md	Records reconciliation completion.
docs/​roadmap-decisions.md	Updates WSL roadmap status.
docs/​architecture.md	Describes reconciliation architecture.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[CherylSnowVeil]

Reviewed the full diff at b718b68 (current head) against main: internal/wslreconcile (coordinator/lease machinery, reconcile pass, cb wsl cleanup command), internal/wsldocker/reconcile.go discovery+proof, the wslrun create-to-lease wiring, main.go dispatch, and the docs. Also traced the surrounding contracts in wsldocker (create, inspect, remove, signal, wait), wslfs layout validation, and hostenv layout derivation.

Findings

• [nit] Stale activation-gate text in cb help output — main.go (usage footer, "Native WSL2" section) still prints activation awaits orphan reconciliation and real Docker Desktop qualification. Every other gate message was updated to the new wording ("native state commands, integration coverage and real qualification": internal/hostenv/hostenv.go, internal/wslinstall/install.go, README, docs), so cb help now contradicts the rest of the user-facing surface and implies this PR's feature is still missing.

• [nit] cb wsl usage error omits cleanup — internal/wslinstall/install.go run() returns usage: cb wsl prepare (--check | --apply) | cb wsl install (--check | --apply) for any unrecognized cb wsl <cmd>. Now that runWSL routes cleanup before wslinstall.Run, the usage hint should list it.

• [nit] Missing symmetric probe-contract guard — internal/wslreconcile/reconcile.go reconcileLocked validates leaseActive-with-handle and leaseOrphaned-without-handle, but a leaseMissing result carrying a non-nil lease handle is accepted and classified as an orphan whose lease then gets Remove()d under --apply. Production probeFileLease can't produce that combination, so this is only a gap in the fail-closed seam validation the surrounding checks already enforce — a symmetric status == leaseMissing && heldLease != nil rejection would close it.

Verified design/correctness points (no issues)

• Create-to-lease race: beginRun holds the state-directory flock across reconcileLocked and container creation until Adopt publishes the locked lease — a coordinator-holding reconciler can never observe a created-but-unleased container. Verified defer ordering in executeTool: the cleanup defer is registered after the guard.Close defer, so container cleanup runs first and containerGone is correctly propagated to Close(containerGone), which removes the lease path only after proof-bound removal succeeds.
• Proof-before-mutation: all candidate proofs and lease probes complete before the first signal/remove; each candidate is re-inspected and its full ID, labels, retention mode, and stdio contract re-verified (requireOwnedContainer + retention/stdio checks). Ambiguous state fails closed.
• Orphan lifecycle: running orphans get SIGKILL → wait → the existing non-force proof-bound removal; 404/409 completion races are tolerated appropriately; RemoveContainer re-proves and refuses removal of a running container.
• Lease file hardening: O_NOFOLLOW openat under the 0700 state dir, owner/mode/nlink/device checks, dev/ino revalidation before unlink, and lease retention when container removal is unproven all match the documented contract. --check performs no Docker mutation and no filesystem writes (flock on the dir/lease files only).
• Dispatch: cb wsl cleanup composes the same fixed-layout validation (wslfs.Check, complete layout required) as install/prepare, fails closed off-Linux and on incomplete deps, and is documented in README/wsl.md/process-contract/security-model consistently.
• Cross-frontend isolation: the cb.wsl_namespace label is distro/machine/UID-bound; Windows-side containers never carry cb.kind=run + the namespace label, so discovery can't collide.
• Tests: ordering tests cover begin→create→adopt→remove→close-lease sequencing, read-only --check, proof-before-mutation, 404/409 completion races, active-lease preservation, and lease symlink/mode rejection. Coverage matches the risk surface.

Overall: a careful, fail-closed implementation consistent with the codebase's proof-bound conventions. Only cosmetic nits above; nothing blocking from this review. CI is green.

[AviBackToBlack]

Addressed all three CherylSnowVeil nits at current head dbcf8a9:

• updated the stale Native WSL2 activation-gate text in cb help (landed in 05bb3bc);
• added cb wsl cleanup (--check | --apply) to the fallback WSL usage error and covered it with a regression test;
• made lease-probe seam validation fully symmetric by rejecting leaseMissing with a non-nil handle before mutation, while retaining that handle for deferred close; the regression test verifies no remove operation occurs and the unexpected handle is closed.

Validation on dbcf8a9:

• go test -count=50 ./internal/wslinstall ./internal/wslreconcile ./internal/wsldocker ./internal/wslrun
• go test -race ./...
• go vet ./...
• git diff --check

Please re-review the current head when convenient.

[CherylSnowVeil]

Re-review at dbcf8a9

Validation of previous findings

All three findings from the earlier review at b718b68 are fixed and verified against the current head:

• [nit] Stale activation-gate text in cb help — fixed. main.go now prints "activation awaits native state commands, integration coverage and real Docker Desktop qualification", consistent with hostenv.go, the install plan output, README and docs.
• [nit] cb wsl usage error omits cleanup — fixed. internal/wslinstall/install.go usage now lists cb wsl cleanup (--check | --apply), covered by TestUsageIncludesCleanupCommand.
• [nit] Missing symmetric probe-contract guard — fixed. reconcileLocked now rejects leaseMissing with a non-nil handle before any mutation; TestReconcileRejectsMissingLeaseWithHandle proves the handle is closed and no mutation occurs.

Fresh review of the current revision

Re-reviewed the complete current diff: internal/wslreconcile (coordinator/lease machinery, reconcile pass, cb wsl cleanup command), internal/wsldocker/reconcile.go discovery+proof, the wslrun begin→create→adopt→cleanup wiring, main.go dispatch, docs, and new tests — plus the surrounding wsldocker create/inspect/remove/signal/wait and wslfs layout contracts they rely on.

Findings

• [nit] Orphaned lease residue is never reaped once its container is gone — internal/wslreconcile/reconcile.go. Lease probing is driven by container discovery, so probeLease only runs for run IDs that still have a matching container. The paths that deliberately retain the lease pathname after the container is gone — coordinator-reacquire failure in RunGuard.Close (reconcile.go:176-188), unproven absence after reconcileOrphan (reconcile.go:287-289), or fileLease.Remove() refusal on a replaced inode (lease_linux.go:207-209) — leave run-*.lease files that no later pass ever revisits. Since Adopt is strictly create-then-lease, a lease implies a container once existed, so while the coordinator is held the reconcile pass could safely enumerate run-*.lease files and remove orphaned ones with no matching container. The consequence is only unbounded zero-byte-file accumulation across repeated ambiguous failures — consistent with the documented "recovery evidence" intent, but nothing ever consumes that evidence.

Verified (no issues)

• Create-to-lease race: beginRun holds the state-dir flock across reconcileLocked, container creation and Adopt publication; a coordinator-holding reconciler cannot observe a created-but-unleased container. Defer ordering in executeTool runs container cleanup before guard.Close(containerGone), so the lease path is removed only after proof-bound removal succeeds.
• Proof-before-mutation: all candidate re-inspections and lease probes complete before the first signal/remove; invalid or inconsistent probe results (unknown status, missing-with-handle, active-with-handle, orphaned-without-handle) fail closed.
• Orphan lifecycle: running orphans get SIGKILL → wait → proof-bound non-force RemoveContainer, which re-inspects, re-proves labels/retention and refuses removal of a running container; 404/409 completion races tolerated.
• Lease hardening: O_NOFOLLOW+openat under the 0700 state dir, owner/mode/nlink/device checks, dev/ino revalidation before unlink, refusal on replaced leases.
• Guard close ordering: RunGuard.Close(containerGone=true) reacquires the coordinator and holds it across Remove+Close; reacquire failure retains the locked lease pathname as evidence instead of unlinking it.
• Ambiguity rejection: discovery fails closed on duplicate container IDs or duplicate run IDs before any proof or mutation; the dedicated discovery transport asserts its own 4 MiB bound above the shared 1 MiB control limit (Linux test).
• Dispatch and gating: cb wsl cleanup is routed ahead of the host gate alongside prepare/install, requires the complete fixed layout (wslfs.Check + zero missing directories), fails closed off-Linux via empty production deps plus currentLayout error, and --check performs no Docker mutation.
• Docs/help consistency: README, wsl.md, wsl-process-contract.md, security-model.md, architecture.md and the roadmap docs now consistently describe reconciliation as wired and name state commands/integration/qualification as the remaining gates.
• Tests: ordering (begin→create→adopt→remove→close-lease), read-only --check, proof-before-mutation, 404/409 races, active-lease preservation, unsafe lease rejection, coordinator context handling, duplicate run-ID rejection, and report printing.

Nothing blocking from this review. CI is green.

[AviBackToBlack]

Addressed the fresh re-review nit in 1b0f538.

The coordinator-held pass now enumerates only strict managed run-<128-bit-id>.lease names, completes all container and detached-lease proofs before the first mutation, and reports lease-only state explicitly. --apply removes an unlocked lease only when complete namespace discovery has no matching retained container; locked lease-only records are always preserved. Invalid names, duplicate run IDs, unsafe files, and inconsistent probe/handle results fail closed. The regression coverage includes check/apply behavior, active preservation, handle cleanup, malformed/duplicate discovery, proof-before-container-mutation ordering, and Linux directory enumeration.

Validation:

• go test -count=50 ./internal/wslreconcile ./internal/wsldocker ./internal/wslrun ./internal/wslinstall
• go test -race ./...
• go vet ./...
• release-style Windows/Linux amd64/arm64 builds with v2.0.0-citest
• canonical Windows/amd64 cb.exe version smoke
• git diff --check

The commit is intentionally unsigned because commit signing is currently disabled. Please re-review the current head when convenient.

[CherylSnowVeil]

Re-review at 1b0f538

Validation of previous findings

• [nit] Orphaned lease residue is never reaped once its container is gone (flagged at dbcf8a9) — fixed and verified. reconcileLocked now calls deps.discoverLeases (discoverFileLeases on Linux) under the held coordinator, validates each enumerated name by round-tripping leaseName, rejects duplicate run identities, and probes every lease that has no matching retained container before the first mutation. --apply removes a detached lease only via item.lease.Remove() — which still re-proves dev/ino before unlink — and only for unlocked leases; locked lease-only records are classified active and preserved. Lease-missing TOCTOU results are skipped, all invalid status/handle combinations fail closed, and report/CLI output now surface lease-active/lease-residue/lease-reaped with totals. Regression coverage includes check/apply reaping, active preservation, proof-before-container-mutation ordering, invalid discovery, invalid probe contracts, and real directory enumeration.
• The three earlier findings (stale cb help gate text, cb wsl usage omission, asymmetric probe-contract guard) remain fixed at this head.

Fresh review of the current revision

Re-reviewed the complete current diff: internal/wslreconcile (coordinator/lease machinery, detached-lease enumeration and reaping, cb wsl cleanup), internal/wsldocker/reconcile.go discovery+proof, the wslrun begin→create→adopt→cleanup wiring, main.go dispatch, docs, and tests — plus the surrounding wsldocker create/inspect/remove/signal/wait and wslfs/wslvolume contracts.

Verified (no issues)

• Detached-lease reaping safety: the pass only runs while the namespace coordinator is held, so no cooperating process can be between container-create and lease publication. A detached unlocked lease therefore always denotes residue; reaping it cannot strand a live run. Locked detached leases (e.g., a run whose container was externally removed, or a RunGuard.Close still blocking on coordinator reacquire) are classified active and preserved.
• Proof-before-mutation preserved: all container proofs/probes and all detached-lease probes complete before the first signal/remove/unlink; every invalid probe contract (unknown status, missing/active with handle, orphaned without handle) fails closed.
• Lease replacement race: fileLease.Remove() re-opens the path and refuses on dev/ino mismatch; Remove+Close complete inside the coordinator hold, so a cooperating reconciler cannot observe a replacement pathname on a still-locked inode.
• Guard lifecycle: beginRun holds the coordinator across reconcileLocked; Adopt keeps a failed coordinator-release's locked lease attached for the proven-cleanup path; Close(containerGone) reacquires the coordinator before Remove+Close, and retains the pathname (unlocked, as evidence) on reacquire or removal failure — now correctly consumed by the detached reaper on the next pass.
• Create-to-lease race: executeTool defers run container cleanup before guard.Close(containerGone); the lease path is removed only after proof-bound removal confirms absence, and the deferred Adopt-failure path is covered.
• Discovery integrity: duplicate container IDs and duplicate run IDs are rejected before any proof or mutation; the dedicated discovery transport passes the declared 4 MiB bound (asserted above the shared 1 MiB limit by the Linux test).
• Dispatch and gating: cb wsl cleanup is routed ahead of the host gate, requires the complete exact layout (wslfs.Check, zero missing dirs), fails closed off-Linux via empty production deps plus layout error, and --check performs no Docker mutation or filesystem write.
• Docs/help consistency: README, wsl.md, wsl-process-contract.md, security-model.md, architecture.md and roadmap docs consistently describe the lease evidence lifecycle, detached reaping and the remaining activation gates.
• Tests: ordering, read-only check, proof-before-mutation, 404/409 completion races, active-lease preservation, unsafe lease rejection, detached reaping in both modes, invalid discovery/probe contracts, and coordinator context handling match the risk surface.

CI is green across Linux, Windows and Windows ARM64. The current revision was reviewed in full and no actionable issues were found.