Add proof-bound WSL container start transport - #122
AviBackToBlack wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The canonical roadmap status omits the implemented wait operation listed in the other updated documentation.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds a proof-bound Docker Desktop WSL operation for starting an exact container without enabling the WSL frontend.
Changes:
- Adds Linux start transport and non-Linux fail-closed stub.
- Validates requests and treats HTTP 304 as failure.
- Adds tests and updates WSL roadmap documentation.
| File | Description |
|---|---|
internal/wsldocker/wsldocker.go |
Updates package documentation. |
internal/wsldocker/start.go |
Implements validated start requests. |
internal/wsldocker/start_linux.go |
Exposes Linux start transport. |
internal/wsldocker/start_other.go |
Adds non-Linux rejection stub. |
internal/wsldocker/start_test.go |
Tests request binding and failures. |
docs/wsl.md |
Documents start semantics and remaining work. |
docs/roadmap-implementation-requirements.md |
Updates WSL implementation status. |
docs/roadmap-decisions.md |
Updates the canonical WSL roadmap status. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| the separately constrained proof-bound attach transport and exact | ||
| container-start operation are implemented but not yet wired into an | ||
| enabled frontend; multiplexed output, container creation, terminal, |
CherylSnowVeil
left a comment
There was a problem hiding this comment.
Review
Verified the implementation directly against internal/wsldocker: startContainer mirrors waitContainer exactly — nil-context check, exact 64-hex lowercase ID validation, dependency completeness check, then execute, which re-runs the endpoint proof, binds the request to the proven socket dev/inode before and after the call, enforces root peer UID, bounds the response, and maps any non-204 status (including Docker's 304 "already started") to a typed *APIError. The Linux export uses the 30s operationTimeout and maxOperationOutput bounds (appropriate for a bounded request rather than the wait long-poll), and the non-Linux stub fails closed identically to WaitContainer. Test coverage is on par with the wait/attach suites: exact request shape, pre-proof rejection, and the fail-closed 304 path. CI is green. No code findings.
Findings
- 🟡 [important]
docs/roadmap-decisions.md(~L346-350) — the canonical "Remaining WSL2" implemented-capabilities list now reads "proof-bound bounded control requests, the separately constrained proof-bound attach transport and exact container-start operation" but omits the exact context-bound container-wait operation merged in #117, while bothdocs/wsl.mdanddocs/roadmap-implementation-requirements.mdenumerate wait and start. The omission predates this PR, but this change edits exactly that enumeration and leaves the status docs disagreeing about shipped lifecycle primitives. (Also flagged by Copilot review; still unresolved.) Remediation: include the wait transport in this list.
Summary
Sound, minimal addition that correctly extends the established proof-bound operation pattern. The HTTP 304 fail-closed choice is implemented and documented as intended. Only remaining issue is the docs inconsistency above.

Summary
POST /containers/{id}/start, accept only HTTP 204, and fail closed on Docker's HTTP 304 already-running responseRoadmap: #2
Validation
gofmtgo vet ./...go test -race ./...git diff --checkCommit
7fcc31cis hardware-signed.