diff --git a/openspec/changes/archive/2026-05-14-agent-http-handlers/.openspec.yaml b/openspec/changes/archive/2026-05-14-agent-http-handlers/.openspec.yaml new file mode 100644 index 0000000..66dd08a --- /dev/null +++ b/openspec/changes/archive/2026-05-14-agent-http-handlers/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-14 diff --git a/openspec/changes/archive/2026-05-14-agent-http-handlers/design.md b/openspec/changes/archive/2026-05-14-agent-http-handlers/design.md new file mode 100644 index 0000000..a06568c --- /dev/null +++ b/openspec/changes/archive/2026-05-14-agent-http-handlers/design.md @@ -0,0 +1,136 @@ +## Context + +The CLI MCP Server architecture has two binaries: the MCP server (control plane) and the sandbox agent (data plane). SANDBOX-1806 delivered the shared HTTP contract types and the persistent bash session. SANDBOX-1807 adds the HTTP layer and binary entry point — the glue between the network and the bash session. + +The sandbox agent exposes three HTTP endpoints on port 8090. The MCP server calls `/exec` to run commands and `/assign` to deliver auth tokens to warm-pool pods. Kubernetes kubelet calls `/health` for readiness probes. + +## Goals / Non-Goals + +**Goals:** +- Expose POST /exec with bearer token auth (constant-time via `hmac.Equal()`) +- Expose POST /assign for one-time token delivery to warm-pool pods +- Expose GET /health as an unauthenticated readiness probe +- Implement agent state machine: unassigned (warm pool) → assigned (has token) +- Wire cmd/agent/main.go with HTTP server, signal handling, graceful shutdown +- Keep the handler as a thin HTTP layer — no business logic beyond parsing, auth, and serialization + +**Non-Goals:** +- Container image (SANDBOX-1808) +- Unit tests (SANDBOX-1809) +- MCP server-side agent HTTP client (SANDBOX-1812) +- HMAC key derivation / per-session token computation (MCP server's responsibility) +- TLS (handled by kube-rbac-proxy on the MCP server side) +- Request logging or metrics (observability is a server-side concern) +- Content-Type validation or body size limits (network isolation + pod memory limits suffice) + +## Decisions + +### Decision 1: Mutex-protected AgentState struct +Use a struct with `sync.Mutex` grouping the `assigned` flag and `token` string together. + +**Rationale:** +- The state machine has two coupled fields (assigned ↔ token non-empty) — a mutex groups them in one critical section +- Matches internal patterns (e.g. `VMShutdownTracker` with `RWMutex` in `mcp-server-devsandbox`) +- Negligible mutex cost for a per-pod agent serving one investigation + +**Alternative considered:** Atomic operations (`atomic.Bool` + `atomic.Value`) +- Rejected — requires coordinating two atomic fields for a compound invariant; lock-free performance advantage is irrelevant at per-pod scale + +**Alternative considered:** `sync.Once` for assignment +- Rejected — can't distinguish "first call" from "already called" without additional state + +### Decision 2: `hmac.Equal()` for token comparison +Use `crypto/hmac.Equal()` for constant-time bearer token comparison. + +**Rationale:** +- Returns clean `bool` (vs `subtle.ConstantTimeCompare` returning `int`) +- Functionally identical — `hmac.Equal()` is a thin wrapper around `subtle.ConstantTimeCompare()` +- Semantically appropriate since the token is HMAC-derived by the MCP server (`HMAC-SHA256(HMACKey, sessionID)`) + +**Alternative considered:** `crypto/subtle.ConstantTimeCompare()` +- Rejected — awkward `int` return; no functional difference + +### Decision 3: Handler struct with method handlers +Use a `Handler` struct holding `*BashSession` and `*AgentState`, with `HandleExec`, `HandleAssign`, `HandleHealth` as methods. + +**Rationale:** +- Three handlers share the same two dependencies — a struct groups them once +- Easy to test: construct with real or mock dependencies +- Standard Go HTTP handler pattern + +**Alternative considered:** Closures over shared state +- Rejected — would repeat `(session, state)` parameter lists three times; closures are better when handlers have different dependencies + +### Decision 4: `net/http.Server` with `Shutdown()` for graceful drain +Use `http.Server.Shutdown()` with a 310-second timeout (MaxTimeout 300s + 10s buffer). + +**Rationale:** +- In-flight `/exec` requests can run up to 300s and must drain before exit +- `Shutdown()` stops accepting new connections, closes idle keepalives, waits for active requests +- 310s ensures even the longest command finishes with buffer for HTTP overhead +- Kubernetes pod spec must set `terminationGracePeriodSeconds: 310` to match + +**Alternative considered:** Bare `http.ListenAndServe` + immediate exit +- Rejected — kills in-flight `/exec` requests mid-execution + +### Decision 5: Go 1.22+ method-based ServeMux routing +Use `mux.HandleFunc("POST /exec", ...)` instead of manual method checks. + +**Rationale:** +- Repo is Go 1.24+; `use-modern-go` skill says to use modern idioms +- Automatic 405 Method Not Allowed for wrong methods — correct HTTP semantics for free +- Less boilerplate in handlers + +**Alternative considered:** Plain paths with manual method checks +- Rejected — boilerplate in every handler, pre-1.22 pattern in a 1.24+ repo + +### Decision 6: Timeout clamping in BashSession only +The handler converts `int` seconds to `time.Duration` and passes it through. BashSession applies defaults and caps internally (lines 135-139 of `bash.go`). + +**Rationale:** +- BashSession already clamps correctly — no duplication needed +- No change to existing SANDBOX-1806 code +- Follows "parse at boundaries, trust internals" — BashSession is the authority on its own limits + +**Alternative considered:** Handler clamps too (defense in depth) +- Rejected — duplicate logic, two places to update if limits change + +### Decision 7: Handler lives in `pkg/sandbox/handler.go` +Co-located with `bash.go` since the handler is the sole consumer of `BashSession`. + +**Rationale:** +- Matches the parent design doc, implementation plan, and stories +- Cross-package import on `pkg/agent` is a one-way dependency on pure data types +- Handler is ~100 lines — doesn't warrant its own package + +### Decision 8: POST /assign returns 200 with empty body +Simple, consistent with design docs. No response body needed. + +**Alternative considered:** 204 No Content +- Rejected — minor deviation from design docs with no practical benefit + +### Decision 9: No input validation (Content-Type, body size) +Agent runs in an isolated pod network (NetworkPolicy restricts ingress to MCP server only). Pod memory limits (512Mi) are the backstop. + +**Rationale:** +- The MCP server is our code sending tiny well-formed payloads +- Body size limits protect against a scenario requiring a bug in our own code, in an isolated network, with existing safeguards +- Simplicity first + +## Risks / Trade-offs + +**Risk:** `/assign` is unauthenticated — anyone who can reach the agent can deliver a token +→ **Mitigation:** NetworkPolicy restricts ingress to pods labeled `app: cli-mcp-server` only. The warm-pool pod has no token yet, so there's nothing to authenticate with. + +**Risk:** 310s shutdown timeout means pod deletion takes up to ~5 minutes worst case +→ **Mitigation:** Only occurs during active investigation cleanup or node drain. The alternative (killing in-flight commands) is worse. + +**Risk:** `writeError` ignores `json.Encode` error return +→ **Mitigation:** `ErrorResponse` is a simple struct that can't fail to serialize. Match `mcp-server-devsandbox` pattern with `_ = json.NewEncoder(w).Encode(...)` to satisfy `errcheck` linter. + +**Trade-off:** Handler doesn't clamp timeout — relies entirely on BashSession +→ **Accepted:** BashSession already handles this correctly. A negative or zero timeout from the request is treated as "use default" by BashSession's `<= 0` check. + +## Open Questions + +None — all design decisions are resolved. diff --git a/openspec/changes/archive/2026-05-14-agent-http-handlers/proposal.md b/openspec/changes/archive/2026-05-14-agent-http-handlers/proposal.md new file mode 100644 index 0000000..ddb7749 --- /dev/null +++ b/openspec/changes/archive/2026-05-14-agent-http-handlers/proposal.md @@ -0,0 +1,37 @@ +## Why + +The sandbox agent has a working persistent bash session (SANDBOX-1806) but no way to receive commands from the network. The MCP server needs to send commands to the agent over HTTP, deliver auth tokens to warm-pool pods, and probe health for Kubernetes readiness. Without HTTP handlers and an entry point, the bash session is a library with no runtime — nothing starts it, nothing talks to it, nothing shuts it down. + +## What Changes + +- Add `pkg/sandbox/handler.go` with: + - `AgentState` struct (mutex-protected) tracking unassigned/assigned state and bearer token + - `Handler` struct with method handlers for three HTTP endpoints + - `HandleExec` — bearer token auth via `hmac.Equal()`, parses `ExecRequest`, calls `BashSession.Execute()`, maps `ExecResult` to `ExecResponse` + - `HandleAssign` — one-time token delivery for warm-pool pods, transitions state from unassigned to assigned + - `HandleHealth` — unauthenticated readiness probe, returns 200 if bash alive, 503 if dead + - `ErrorResponse` type and `writeError` helper for consistent JSON error responses +- Update `cmd/agent/main.go` with: + - `SANDBOX_AUTH_TOKEN` env var reading for initial state + - `BashSession` creation with eager initialization + - HTTP mux with Go 1.22+ method-based routing (`POST /exec`, `POST /assign`, `GET /health`) + - `net/http.Server` on `:8090` with graceful shutdown via `Shutdown()` (310s timeout) + - Signal handling for `SIGTERM`/`SIGINT` via `signal.NotifyContext()` + +## Capabilities + +### New Capabilities +- `agent-http-handlers`: Defines the HTTP endpoints (`POST /exec`, `POST /assign`, `GET /health`), authentication via bearer token with constant-time comparison, agent state machine (unassigned → assigned), and error response format +- `agent-entry-point`: Defines the agent binary entry point — HTTP server lifecycle, signal handling, graceful shutdown with 310s timeout coordinated with Kubernetes `terminationGracePeriodSeconds` + +### Modified Capabilities + + +## Impact + +- **Affected code**: + - `pkg/sandbox/handler.go` — New file with AgentState, Handler, and HTTP endpoint implementations + - `cmd/agent/main.go` — Updated from scaffold placeholder to full agent entry point +- **API changes**: New HTTP API exposed on port 8090 (POST /exec, POST /assign, GET /health). No changes to existing Go APIs — `BashSession` and shared types from SANDBOX-1806 are consumed as-is. +- **Dependencies**: Depends on SANDBOX-1806 (shared types + BashSession). Depended on by SANDBOX-1808 (agent container image), SANDBOX-1809 (agent unit tests), SANDBOX-1812 (agent HTTP client). +- **No breaking changes** — `cmd/agent/main.go` is updated from a placeholder, all other code is new. diff --git a/openspec/changes/archive/2026-05-14-agent-http-handlers/specs/agent-entry-point/spec.md b/openspec/changes/archive/2026-05-14-agent-http-handlers/specs/agent-entry-point/spec.md new file mode 100644 index 0000000..965a639 --- /dev/null +++ b/openspec/changes/archive/2026-05-14-agent-http-handlers/specs/agent-entry-point/spec.md @@ -0,0 +1,67 @@ +## ADDED Requirements + +### Requirement: Agent reads initial state from environment +The agent binary SHALL read `SANDBOX_AUTH_TOKEN` from the environment to determine its initial state. + +#### Scenario: Token present starts agent in assigned state +- **WHEN** `SANDBOX_AUTH_TOKEN` environment variable is set to a non-empty value +- **THEN** the agent SHALL start with `AgentState` in assigned mode and log "starting in assigned state" + +#### Scenario: Token absent starts agent in unassigned state +- **WHEN** `SANDBOX_AUTH_TOKEN` environment variable is empty or not set +- **THEN** the agent SHALL start with `AgentState` in unassigned mode and log "starting in unassigned state (warm pool)" + +### Requirement: Agent creates BashSession eagerly +The agent SHALL create a `BashSession` at startup using `NewBashSession(NewDefaultBashConfig())` and exit fatally if bash fails to start. + +#### Scenario: Bash starts successfully +- **WHEN** the agent starts and bash is available in PATH +- **THEN** a BashSession SHALL be created and the agent SHALL proceed to start the HTTP server + +#### Scenario: Bash unavailable causes fatal exit +- **WHEN** the agent starts but bash is not available +- **THEN** the agent SHALL log the error and exit with a non-zero status code + +### Requirement: Agent starts HTTP server on port 8090 +The agent SHALL start an `http.Server` listening on `:8090` with routes registered using Go 1.22+ method-based routing. + +#### Scenario: Server starts and listens +- **WHEN** the agent starts successfully +- **THEN** an HTTP server SHALL listen on `:8090` and log "listening on :8090" + +#### Scenario: Routes are registered with method patterns +- **WHEN** the HTTP mux is configured +- **THEN** it SHALL register `"POST /exec"`, `"POST /assign"`, and `"GET /health"` using Go 1.22+ method-based patterns + +### Requirement: Agent shuts down gracefully on SIGTERM +The agent SHALL handle `SIGTERM` and `SIGINT` signals, drain in-flight HTTP requests via `http.Server.Shutdown()`, and close the BashSession before exiting. + +#### Scenario: SIGTERM triggers graceful shutdown +- **WHEN** the agent receives SIGTERM +- **THEN** it SHALL call `srv.Shutdown()` with a 310-second timeout context, then call `session.Close()`, then exit 0 + +#### Scenario: In-flight exec request completes during shutdown +- **WHEN** SIGTERM is received while a POST /exec request is in progress +- **THEN** the server SHALL wait for the in-flight request to complete (up to 310 seconds) before closing + +#### Scenario: Shutdown timeout exceeded +- **WHEN** `srv.Shutdown()` exceeds the 310-second timeout +- **THEN** the agent SHALL log the shutdown error, close the BashSession, and exit + +#### Scenario: SIGINT also triggers shutdown +- **WHEN** the agent receives SIGINT +- **THEN** it SHALL follow the same graceful shutdown sequence as SIGTERM + +### Requirement: Shutdown timeout accommodates maximum command duration +The shutdown timeout SHALL be 310 seconds (`MaxTimeout` 300s + 10s buffer) to ensure the longest possible in-flight command completes. + +#### Scenario: Kubernetes terminationGracePeriodSeconds coordination +- **WHEN** the agent pod spec is configured +- **THEN** `terminationGracePeriodSeconds` SHALL be set to 310 to match the agent's shutdown timeout + +### Requirement: Agent logs version at startup +The agent SHALL print version information to stderr at startup. + +#### Scenario: Version logged on start +- **WHEN** the agent starts +- **THEN** it SHALL print `sandbox-agent (built