docs: add openspec for SANDBOX-1806 shared types and bash session

openspec/changes/archive/2026-05-08-shared-types-and-bash-session/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-05-08

openspec/changes/archive/2026-05-08-shared-types-and-bash-session/design.md

@@ -0,0 +1,137 @@
+## Context
+
+The CLI MCP Server architecture has two binaries: the MCP server (control plane, manages pod lifecycle) and the sandbox agent (data plane, runs inside each sandbox pod). They communicate via HTTP — the server sends commands to the agent, the agent executes them in a persistent bash session and returns structured results.
+
+SANDBOX-1806 delivers the two foundational pieces: the shared HTTP contract types and the persistent bash session. These sit at the bottom of the call stack — every command execution flows through them.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Define a clean, simple HTTP contract between server and agent (`ExecRequest`, `ExecResponse`, `AssignRequest`)
+- Implement a persistent bash session that preserves state between calls (env vars, working directory, files)
+- Handle timeouts by killing the command without killing bash
+- Recover from bash crashes gracefully (respawn on next call)
+- Keep `pkg/sandbox` self-contained with no dependency on `pkg/agent` wire format
+- Return output as-is — no truncation, no transformation (TARSy handles compression)
+
+**Non-Goals:**
+- HTTP handler implementation (SANDBOX-1807)
+- Agent entry point and HTTP server setup (SANDBOX-1807)
+- Authentication/authorization (SANDBOX-1807)
+- Container image (SANDBOX-1808)
+- Unit tests (SANDBOX-1809)
+- Output truncation or transformation (design principle: "dumb proxy")
+
+## Decisions
+
+### Decision 1: Pipe-based bash session, not PTY
+Use `os/exec` with `StdinPipe()`, `StdoutPipe()`, `StderrPipe()` instead of a pseudo-terminal.
+
+**Rationale:**
+- Clean stdout/stderr separation — important for structured JSON responses
+- No terminal control sequences that have no value for LLM consumption
+- Pure Go stdlib, no external dependencies
+- Well-known pattern used by OpenHands, Claude Code
+
+**Alternative considered:** PTY-based session
+- Rejected because PTY merges stdout/stderr, adds ANSI escape sequences, and requires external dependencies — complexity with no benefit for LLM consumption
+
+### Decision 2: Concurrent pipe readers with shared buffers
+Use two goroutines writing to `strings.Builder` buffers, synchronized with `sync.WaitGroup`.
+
+**Rationale:**
+- Avoids pipe buffer deadlocks that occur with sequential reads (a command writing heavily to stderr blocks while we wait for stdout)
+- Simpler than channel-based approach — just accumulate into buffers
+- No streaming needed since we return the full response at the end
+
+**Alternative considered:** Sequential read (stdout first, then stderr)
+- Rejected due to pipe buffer deadlock risk
+
+**Alternative considered:** Channel-based concurrent readers
+- Rejected — adds ~15-20 lines of complexity (custom result types, channel coordination) for zero functional benefit
+
+### Decision 3: SIGKILL to process group with 5-second hard deadline
+On timeout, send SIGKILL to the command's process group. Wait up to 5 seconds for exit. If process won't die, mark session dead and trigger crash recovery on next call.
+
+**Rationale:**
+- Process group isolation (`Setpgid: true`) means SIGKILL kills the command but not bash
+- 5-second wait is a reasonable trade-off — timeouts are already exceptional
+- Reuses crash recovery path for the "process won't die" edge case
+- Clean exit is more important than saving 5 seconds
+
+**Alternative considered:** Fire-and-forget SIGKILL
+- Rejected — risks orphaned processes and corrupted pipe state
+
+**Alternative considered:** Async cleanup goroutine
+- Rejected — adds ~30-40 lines of concurrency code (generation counter, cleaning flag) for race conditions that are hard to get right
+
+### Decision 4: Crash recovery deferred to next call
+When bash dies (pipe EOF), fail the current Execute() and respawn on the next call. Prepend "[session state was reset]" to stderr.
+
+**Rationale:**
+- Simple — no retry logic, clear failure semantics
+- Avoids retrying a potentially crash-causing command (e.g., OOM)
+- Matches the parent design docs description
+- Extra round-trip cost is negligible (~0.01% crash frequency, LLM handles retry naturally)
+
+**Alternative considered:** Immediate respawn and retry
+- Rejected — risks crash-looping on OOM-causing commands
+
+### Decision 5: Domain-specific ExecResult type
+BashSession returns `ExecResult` (in `pkg/sandbox`), not `agent.ExecResponse`.
+
+**Rationale:**
+- Keeps `pkg/sandbox` self-contained with no import of `pkg/agent`
+- Uses idiomatic `time.Duration` instead of `int64` milliseconds
+- Handler does the mapping (~5 lines): `DurationMs: result.Duration.Milliseconds()`
+
+**Note:** The parent design has Execute() returning `*ExecResponse` directly. This is a deliberate refinement for cleaner package separation.
+
+### Decision 6: Config struct constructor
+`NewBashSession(cfg BashConfig)` instead of bare `NewBashSession()`.
+
+**Rationale:**
+- Stable constructor signature — adding settings later doesn't break callers
+- Consistent with `SandboxConfig` pattern in the parent design
+- Config is currently empty but provides a clear extension point
+
+**Note:** The parent design has `NewBashSession()` with no parameters. This refinement is functionally equivalent since `BashConfig` is currently empty.
+
+### Decision 7: Eager initialization
+Bash process starts in the constructor, not on first Execute().
+
+**Rationale:**
+- Fail-fast — if bash isn't available, the agent knows at startup
+- Zero first-command latency
+- Simpler health checks (alive = bash running, dead = not running)
+- Idle bash costs <1MB RSS — negligible even for warm pool pods
+
+### Decision 8: No output truncation
+Output is returned as-is with no size limits.
+
+**Rationale:**
+- Parent design explicitly states: "The MCP server returns command output as-is — no truncation, no transformation"
+- Design principle: "dumb proxy" — output treatment is TARSy's responsibility
+- Truncation risks losing information needed for accurate TARSy summarization
+- OOM risk mitigated by command timeouts (60-300s) and pod restart via `RestartPolicy: OnFailure`
+
+## Risks / Trade-offs
+
+**Risk:** Commands producing very large output could OOM the sandbox agent
+→ **Mitigation:** Command timeouts (default 60s, max 300s) limit duration. Pod `RestartPolicy: OnFailure` restarts the agent. Crash recovery handles it on the next call. TARSy's summarization compresses the response downstream.
+
+**Risk:** UUID-based delimiter could theoretically collide with command output
+→ **Mitigation:** 8-character UUID suffix gives ~1/4 billion collision probability per command. Practically impossible.
+
+**Risk:** Concurrent health check reads `dead` flag while Execute() modifies it
+→ **Mitigation:** `IsAlive()` acquires the mutex before reading `dead`.
+
+**Trade-off:** ExecResult adds a type + mapping code vs. using ExecResponse directly
+→ **Accepted:** Clean package separation is worth ~5 lines of mapping in the handler.
+
+**Trade-off:** BashConfig is currently empty
+→ **Accepted:** Provides a stable extension point. Better to have it from the start than to change the constructor signature later.
+
+## Open Questions
+
+None — all design decisions are resolved.

openspec/changes/archive/2026-05-08-shared-types-and-bash-session/proposal.md

@@ -0,0 +1,37 @@
+## Why
+
+The CLI MCP Server needs a reliable mechanism for executing arbitrary shell commands inside sandbox pods and capturing their output in a structured way. The MCP server (control plane) and the sandbox agent (data plane) are separate binaries that communicate via HTTP — they need a shared set of request/response types. The sandbox agent needs a persistent bash session that preserves state (environment variables, working directory) between calls, giving the LLM the same investigative experience a developer gets in a terminal.
+
+Without these foundational primitives, the sandbox agent has no way to receive or respond to command requests, and each command would start in a fresh shell — losing the investigative context that makes the agent productive.
+
+## What Changes
+
+- Add `pkg/agent/types.go` with `ExecRequest`, `ExecResponse`, and `AssignRequest` structs defining the shared HTTP contract between server and agent
+- Add `pkg/sandbox/bash.go` implementing `BashSession` with:
+  - Persistent `bash --norc --noprofile` process managed via stdin/stdout/stderr pipes
+  - UUID-based delimiter protocol for isolating command output from the continuous bash stream
+  - Concurrent pipe readers using shared `strings.Builder` buffers with `sync.WaitGroup`
+  - Timeout handling via SIGKILL to command process group (not bash itself) with 5-second hard deadline
+  - Crash recovery: detect EOF on pipe reads, mark session dead, respawn on next Execute() call
+  - Eager initialization (bash starts in constructor for fail-fast behavior)
+  - Mutex-serialized access for thread safety
+- Add `pkg/sandbox/bash.go` domain-specific `ExecResult` type (separate from wire format `ExecResponse`)
+- Add `BashConfig` struct for stable constructor signature
+
+## Capabilities
+
+### New Capabilities
+- `shared-types`: Defines the HTTP contract types (`ExecRequest`, `ExecResponse`, `AssignRequest`) shared between the MCP server and sandbox agent
+- `persistent-bash-session`: Defines the `BashSession` struct and its behavior — command execution, delimiter protocol, timeout handling, crash recovery, and lifecycle management
+
+### Modified Capabilities
+<!-- None — these are new foundational components -->
+
+## Impact
+
+- **Affected code**:
+  - `pkg/agent/types.go` — New file with shared HTTP contract types
+  - `pkg/sandbox/bash.go` — New file with BashSession implementation
+- **API changes**: New internal Go API only. No external HTTP or MCP API changes (those are in SANDBOX-1807).
+- **Dependencies**: Depends on SANDBOX-1805 (repo scaffold). Depended on by SANDBOX-1807 (agent HTTP handlers), SANDBOX-1809 (agent unit tests), SANDBOX-1812 (agent HTTP client).
+- **No breaking changes** — all new code, no modifications to existing components.

openspec/changes/archive/2026-05-08-shared-types-and-bash-session/specs/persistent-bash-session/spec.md

@@ -0,0 +1,165 @@
+## ADDED Requirements
+
+### Requirement: BashSession manages a persistent bash process
+The `BashSession` struct SHALL manage a long-running `bash --norc --noprofile` process using stdin/stdout/stderr pipes, preserving state (environment variables, working directory, aliases) between Execute() calls.
+
+#### Scenario: Environment variables persist between calls
+- **WHEN** Execute is called with `export FOO=bar` followed by Execute with `echo $FOO`
+- **THEN** the second call SHALL return stdout containing `bar`
+
+#### Scenario: Working directory persists between calls
+- **WHEN** Execute is called with `cd /tmp` followed by Execute with `pwd`
+- **THEN** the second call SHALL return stdout containing `/tmp`
+
+#### Scenario: Bash started with no user configuration
+- **WHEN** NewBashSession is called
+- **THEN** bash SHALL be started with `--norc --noprofile` flags and `PS1=`, `PS2=` environment variables to suppress prompts
+
+### Requirement: BashSession initializes eagerly
+The `NewBashSession(cfg BashConfig)` constructor SHALL start the bash process immediately and return an error if bash fails to start.
+
+#### Scenario: Constructor starts bash process
+- **WHEN** NewBashSession is called with a valid BashConfig
+- **THEN** a bash process SHALL be running and pipes SHALL be open
+
+#### Scenario: Constructor fails if bash unavailable
+- **WHEN** NewBashSession is called but bash is not in PATH
+- **THEN** it SHALL return a non-nil error
+
+#### Scenario: IsAlive returns true after construction
+- **WHEN** NewBashSession succeeds
+- **THEN** IsAlive() SHALL return true
+
+### Requirement: Execute uses UUID-based delimiter protocol
+Each Execute() call SHALL wrap the command with unique UUID-based delimiters to isolate its output from the continuous bash stream.
+
+#### Scenario: Delimiter wraps command output
+- **WHEN** Execute is called with command `echo hello`
+- **THEN** the wrapped command written to stdin SHALL include `echo <DELIM>_START`, the command, exit code capture, `echo <DELIM>_END`, and `echo <DELIM>_EXIT_${__exit} >&2`
+
+#### Scenario: Delimiter is unique per call
+- **WHEN** Execute is called twice
+- **THEN** each call SHALL generate a different delimiter using `uuid.NewString()[:8]`
+
+#### Scenario: Stdout is captured between START and END delimiters
+- **WHEN** a command produces stdout output
+- **THEN** Execute SHALL return only the content between `<DELIM>_START` and `<DELIM>_END` in the Stdout field
+
+#### Scenario: Exit code is parsed from stderr delimiter
+- **WHEN** a command exits with code 2
+- **THEN** Execute SHALL parse the exit code from the `<DELIM>_EXIT_2` marker on stderr and return `ExitCode: 2`
+
+### Requirement: Execute reads stdout and stderr concurrently
+Execute() SHALL read stdout and stderr using two concurrent goroutines writing to shared `strings.Builder` buffers, synchronized with `sync.WaitGroup`, to avoid pipe buffer deadlocks.
+
+#### Scenario: Command writing to both streams does not deadlock
+- **WHEN** a command writes heavily to both stdout and stderr simultaneously
+- **THEN** Execute SHALL complete without deadlocking on pipe buffer pressure
+
+#### Scenario: Stderr is captured separately from stdout
+- **WHEN** a command writes `output` to stdout and `warning` to stderr
+- **THEN** Execute SHALL return `Stdout` containing `output` and `Stderr` containing `warning`
+
+### Requirement: Execute is serialized via mutex
+Only one command SHALL execute at a time. Execute() SHALL acquire a mutex before any operation and release it when complete.
+
+#### Scenario: Concurrent Execute calls are serialized
+- **WHEN** two goroutines call Execute simultaneously
+- **THEN** the second call SHALL block until the first completes
+
+### Requirement: Execute handles command timeout
+When a command exceeds its timeout, Execute SHALL send SIGKILL to the command's process group (not bash itself), wait up to 5 seconds for the process to exit, and return partial output with exit code 137.
+
+#### Scenario: Timed-out command is killed
+- **WHEN** Execute is called with `sleep 60` and timeout of 1 second
+- **THEN** the command SHALL be killed via SIGKILL to the process group and ExitCode SHALL be 137
+
+#### Scenario: Bash survives after timeout
+- **WHEN** a command is killed due to timeout
+- **THEN** the next Execute call SHALL succeed because bash itself was not killed
+
+#### Scenario: Partial output is returned on timeout
+- **WHEN** a command produces some output before being timed out
+- **THEN** Execute SHALL return whatever output was captured before the timeout in Stdout/Stderr
+
+#### Scenario: Unkillable process triggers crash recovery
+- **WHEN** SIGKILL is sent but the process does not exit within 5 seconds
+- **THEN** the session SHALL be marked dead and crash recovery SHALL trigger on the next Execute call
+
+### Requirement: Execute handles bash crash with deferred recovery
+When bash dies during Execute (detected via EOF on pipe reads), the current call SHALL return an error. The next Execute call SHALL respawn bash and prepend a session reset notice to stderr.
+
+#### Scenario: Current call returns error on crash
+- **WHEN** the bash process is killed externally during an Execute call
+- **THEN** Execute SHALL detect EOF on the pipes, mark the session as dead, and return an error
+
+#### Scenario: Next call respawns bash
+- **WHEN** Execute is called after a crash
+- **THEN** it SHALL respawn a new bash process and execute the command in the new session
+
+#### Scenario: Session reset notice is prepended to stderr
+- **WHEN** Execute runs in a respawned bash session
+- **THEN** the Stderr field SHALL be prepended with `[session state was reset — previous bash process exited]\n`
+
+#### Scenario: IsAlive returns false when bash is dead
+- **WHEN** the bash process has crashed
+- **THEN** IsAlive() SHALL return false
+
+#### Scenario: IsAlive returns true after respawn
+- **WHEN** Execute successfully respawns bash after a crash
+- **THEN** IsAlive() SHALL return true
+
+### Requirement: Execute returns ExecResult with duration
+Execute() SHALL return an `ExecResult` struct containing `Stdout`, `Stderr`, `ExitCode`, and `Duration` (as `time.Duration`). Duration SHALL measure wall-clock time from writing the command to receiving the exit code.
+
+#### Scenario: Duration measures wall-clock time
+- **WHEN** Execute is called with `sleep 1`
+- **THEN** the Duration field SHALL be approximately 1 second
+
+#### Scenario: Non-zero exit codes are valid results
+- **WHEN** a command returns exit code 1
+- **THEN** Execute SHALL return an ExecResult (not an error) with ExitCode 1
+
+#### Scenario: Only infrastructure failures return errors
+- **WHEN** bash crashes or stdin write fails
+- **THEN** Execute SHALL return an error (not an ExecResult)
+
+### Requirement: Output is returned as-is
+Execute() SHALL return command output without any truncation, transformation, or size limits. Output treatment is TARSy's responsibility.
+
+#### Scenario: Large output is not truncated
+- **WHEN** a command produces 5MB of stdout
+- **THEN** Execute SHALL return all 5MB in the Stdout field without truncation
+
+#### Scenario: Binary-like output is preserved
+- **WHEN** a command produces non-UTF-8 output
+- **THEN** Execute SHALL return the raw bytes as a string without modification
+
+### Requirement: BashSession can be closed gracefully
+The `Close()` method SHALL kill the bash process and close all pipes. It SHALL be idempotent.
+
+#### Scenario: Close kills bash process
+- **WHEN** Close is called on an active BashSession
+- **THEN** the bash process SHALL be terminated and all pipes SHALL be closed
+
+#### Scenario: Close is idempotent
+- **WHEN** Close is called twice
+- **THEN** the second call SHALL return without error
+
+#### Scenario: IsAlive returns false after Close
+- **WHEN** Close has been called
+- **THEN** IsAlive() SHALL return false
+
+### Requirement: IsAlive is thread-safe
+The `IsAlive()` method SHALL acquire the mutex before reading the `dead` flag to ensure thread safety with concurrent Execute() calls.
+
+#### Scenario: Concurrent IsAlive and Execute
+- **WHEN** IsAlive is called while Execute is running
+- **THEN** IsAlive SHALL block until the mutex is available and return a consistent result
+
+### Requirement: BashSession uses process group isolation
+The bash process SHALL be started with `SysProcAttr{Setpgid: true}` so that SIGKILL can target a command's process group without killing bash itself.
+
+#### Scenario: Process group is set on bash
+- **WHEN** NewBashSession creates the bash process
+- **THEN** `cmd.SysProcAttr` SHALL have `Setpgid: true`