From c6dede536470a71661bb5dfecda52e985a34af6f Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Mon, 22 Jun 2026 16:14:38 +0530 Subject: [PATCH 1/4] spec: add openspec for pod cache and session manager (SANDBOX-1810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds design documents, BDD specs, and implementation tasks for the session manager that handles sandbox pod lifecycle — creation, discovery, command proxying, and cleanup. Co-authored-by: Cursor --- .../.openspec.yaml | 2 + .../design.md | 151 ++++++++++++++++++ .../proposal.md | 46 ++++++ .../specs/pod-cache/spec.md | 50 ++++++ .../specs/session-manager/spec.md | 131 +++++++++++++++ .../tasks.md | 57 +++++++ 6 files changed, 437 insertions(+) create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/design.md create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md create mode 100644 openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml b/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml new file mode 100644 index 0000000..38f7628 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-22 diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md new file mode 100644 index 0000000..e81d43f --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md @@ -0,0 +1,151 @@ +## Context + +The CLI MCP Server architecture has two binaries: the MCP server (control plane) and the sandbox agent (data plane). SANDBOX-1806/1807 delivered the agent side — persistent bash session, HTTP handlers, and entry point. SANDBOX-1810 adds the server-side session management layer — the bridge between incoming MCP tool calls (identified by `X-Session-ID`) and the agent pods that execute commands. + +The MCP server is stateless — any replica can serve any request. All durable state lives in Kubernetes labels on the sandbox pods. The session manager provides a thin caching layer (30s TTL) to avoid per-request K8s API calls while remaining correct across replicas (label-based discovery is the source of truth; cache is an optimization). + +## Goals / Non-Goals + +**Goals:** +- PodCache with thread-safe map and TTL-based expiry (~30s) +- SessionManager with `GetOrCreatePod` (label lookup, cache, create pod + auth Secret, wait for ready) +- SessionManager with `ExecuteCommand` (resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation) +- SessionManager with `CleanupSession` (delete pod + Secret) +- SessionManager with `CleanupStale` (periodic, idle-timeout based) +- Pod spec includes: security context (RunAsNonRoot, drop ALL), readiness probe on :8090/health, kubeconfig volume, workspace emptyDir, correct labels and annotations +- SandboxConfig struct for all tunables (image, CPU/mem, idle timeout, HMAC key, warm pool size) + +**Non-Goals:** +- Warm pod pool (SANDBOX-1811) +- Agent HTTP client abstraction (SANDBOX-1812 — the manager uses `net/http` directly for now) +- MCP tool registration (SANDBOX-1813) +- MCP server entry point (SANDBOX-1814) +- Server unit tests beyond those in this PR (SANDBOX-1816) +- Metrics or structured logging middleware +- Retry logic for pod creation failures (single attempt; can be added later) + +## Decisions + +### Decision 1: In-memory PodCache with short TTL (30s) + +Use a simple `sync.RWMutex`-protected map with 30-second TTL entries. + +**Rationale:** +- The stateless MCP server can't rely on per-session state across requests — but a short-lived cache avoids a K8s API call on every single command execution within a burst +- 30s TTL means stale entries self-correct quickly if a pod gets a new IP (crash/restart) +- Conditional invalidation (`Invalidate(sessionID, podIP)`) ensures we don't accidentally evict a freshly-updated entry after a reconnect +- No external dependencies (Redis, memcached) — simplicity first + +**Alternative considered:** No cache — always query K8s API +- Rejected — K8s API calls add latency (~50-100ms) to every command execution; with TTL cache, only the first request per 30s window hits the API + +**Alternative considered:** Longer TTL (5 minutes) +- Rejected — if a pod restarts and gets a new IP, requests would fail for up to 5 minutes before cache expiry forces rediscovery + +### Decision 2: HMAC-SHA256 deterministic token derivation + +Compute per-session auth tokens as `hex(HMAC-SHA256(HMACKey, sessionID))`. + +**Rationale:** +- Any MCP server replica can derive the same token for a given session without storing or looking up tokens +- Deterministic — same key + session ID always yields same token, enabling stateless operation +- Standard cryptographic construction; 256-bit output provides ample security margin +- The token is delivered to the sandbox agent via a K8s Secret (env var injection), so the agent never computes it — it just stores and compares + +**Alternative considered:** Random token stored in Secret, looked up by MCP server on each request +- Rejected — requires a K8s API call to read the Secret on each command execution, defeating the purpose of caching + +### Decision 3: Label-based pod discovery as source of truth + +Use label selectors (`tarsy.redhat.com/session-id=, tarsy.redhat.com/component=cli-mcp-sandbox`) for pod lookup. + +**Rationale:** +- Labels are indexed in etcd — list-by-label is efficient +- Any replica can discover any session's pod without shared state +- If cache misses (TTL expired, replica restart, first request), the K8s API call provides the correct answer +- Matches the parent design doc's architecture + +**Alternative considered:** In-memory session-to-pod mapping as primary store +- Rejected — breaks stateless multi-replica operation; a different replica wouldn't know about a pod another replica created + +### Decision 4: Create auth Secret before pod + +Create the per-session Secret first, then create the pod that references it via `secretKeyRef`. + +**Rationale:** +- The pod spec references the Secret via env var injection (`SANDBOX_AUTH_TOKEN` from `secretKeyRef`). If the pod starts before the Secret exists, the container will fail to start with `CreateContainerConfigError`. +- Secret-first ordering ensures the pod's env var resolution succeeds on first attempt +- If pod creation fails, we best-effort delete the orphaned Secret + +**Alternative considered:** Create pod first, then Secret +- Rejected — race condition where the container starts before the Secret exists + +### Decision 5: Poll-based `waitForReady` with 60s deadline + +Poll the pod status every 2 seconds for up to 60 seconds until Running + PodReady condition. + +**Rationale:** +- Pod startup time is typically 3-8 seconds (image pull is fast with pre-cached layers on nodes) +- 60s deadline accommodates cold image pulls or scheduling delays +- 2s poll interval is a good balance between latency and K8s API load +- Simple to implement and reason about; watch-based would add complexity for marginal latency improvement + +**Alternative considered:** Watch-based with informer +- Rejected — adds significant complexity; session creation is not a hot path (happens once per investigation), so poll latency is acceptable + +### Decision 6: Background annotation update after ExecuteCommand + +Update `last-activity` annotation in a background goroutine (`go m.updateLastActivity()`). + +**Rationale:** +- The annotation update is best-effort — it's used for stale cleanup but not for correctness +- Executing the patch synchronously would add ~50ms latency to every command response +- If the patch fails (e.g., pod deleted between exec and patch), it's logged and harmless + +**Alternative considered:** Synchronous annotation update +- Rejected — adds latency to the critical path for non-critical metadata + +### Decision 7: SessionManager directly uses `net/http` for agent communication + +The manager constructs HTTP requests inline rather than depending on a separate agent client package. + +**Rationale:** +- SANDBOX-1812 will introduce `pkg/agent/client.go` as a proper abstraction; the manager will be refactored to use it +- For now, inline HTTP keeps the session manager self-contained with zero internal dependencies beyond `pkg/agent` types +- The HTTP call is simple: POST JSON body, bearer header, decode JSON response + +**Alternative considered:** Block on SANDBOX-1812 first +- Rejected — creates unnecessary sequencing; inline HTTP can be replaced later with a one-line refactor + +### Decision 8: `CleanupStale` is caller-driven (no internal ticker) + +The `CleanupStale` method is called externally (e.g., by a goroutine in `cmd/server/main.go` on a ticker). It does not manage its own lifecycle. + +**Rationale:** +- Keeps the SessionManager testable — tests call `CleanupStale()` directly without dealing with timers +- The caller (server entry point) owns the ticker and can cancel it on shutdown +- Consistent with the design doc's "stale pod cleanup goroutine" in `runServer` + +**Alternative considered:** Internal background goroutine with ticker +- Rejected — harder to test, harder to shut down cleanly, mixes lifecycle management with business logic + +## Risks / Trade-offs + +**Risk:** 30s cache TTL means requests can be routed to a dead pod IP for up to 30s after pod restart +→ **Mitigation:** `Invalidate()` is called on HTTP errors (agent unreachable), immediately clearing the stale entry and forcing rediscovery on the next request. + +**Risk:** `waitForReady` polls every 2s — adds 0-2s latency to pod creation +→ **Mitigation:** Acceptable for session creation (happens once per investigation). The warm pool feature (SANDBOX-1811) eliminates this latency entirely for pre-warmed pods. + +**Risk:** Background `updateLastActivity` can silently fail +→ **Mitigation:** Logged at WARN level. Worst case: a pod gets cleaned up slightly before its true idle timeout. The pod would be recreated on the next request. + +**Trade-off:** Manager uses inline `net/http` instead of `pkg/agent/client` +→ **Accepted:** Will be refactored in SANDBOX-1812. Keeps this story self-contained. + +**Trade-off:** No retry on pod creation failure +→ **Accepted:** Single attempt returns error to caller. The MCP tool handler will propagate the error to TARSy, which can retry the tool call. + +## Open Questions + +None — all design decisions are resolved. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md new file mode 100644 index 0000000..f5e9f18 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md @@ -0,0 +1,46 @@ +## Why + +The MCP server needs to manage sandbox pod lifecycle — creating pods on demand, caching their IPs for fast routing, executing commands via the agent HTTP API, and cleaning up idle sessions. Without a session manager, the server has no way to map an incoming `X-Session-ID` header to a running sandbox pod. Without a cache, every request would require a Kubernetes API call for pod discovery. Without cleanup, abandoned pods would accumulate indefinitely. + +SANDBOX-1806/1807 delivered the sandbox agent (persistent bash + HTTP handlers). This story builds the MCP server-side counterpart that manages the agent pods from the outside. + +## What Changes + +- Add `pkg/session/config.go` with: + - `SandboxConfig` struct holding all tunables (image, CPU/mem requests/limits, idle timeout, HMAC key, warm pool size, namespace, service account, kubeconfig secret name) + - `DefaultConfig()` factory with sensible production defaults + +- Add `pkg/session/cache.go` with: + - `PodCache` struct — thread-safe `sync.RWMutex`-protected map of session ID → (pod IP, pod name, expiry) + - TTL-based expiry (~30s default) — expired entries return cache miss + - `Get`, `Set`, `Delete`, `Invalidate` (conditional on matching pod IP), `EvictExpired` + +- Add `pkg/session/manager.go` with: + - `SessionManager` struct holding Kubernetes clientset, config, cache, HTTP client, logger + - `GetOrCreatePod(ctx, sessionID)` — cache → label lookup → create pod + auth Secret → wait for ready + - `ExecuteCommand(ctx, sessionID, command, timeout)` — resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation + - `CleanupSession(ctx, sessionID)` — delete pod + Secret, clear cache + - `CleanupStale(ctx)` — iterate sandbox pods, delete those idle longer than IdleTimeout + - `buildPodSpec(sessionID)` — full pod manifest with security context, readiness probe, volumes, labels, annotations + - `buildAuthSecret(sessionID, token)` — per-session Secret for HMAC token delivery + - `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID) deterministic token derivation + +## Capabilities + +### New Capabilities +- `pod-cache`: Defines the in-memory TTL cache for pod IP lookups with thread-safe access and conditional invalidation +- `session-manager`: Defines the session manager responsible for pod lifecycle (create, discover, proxy, cleanup) using Kubernetes labels as the source of truth + +### Modified Capabilities + + +## Impact + +- **Affected code**: + - `pkg/session/config.go` — New file with SandboxConfig + - `pkg/session/cache.go` — New file with PodCache + - `pkg/session/manager.go` — New file with SessionManager +- **API changes**: New Go API in `pkg/session` package. No external HTTP endpoints added (those come in SANDBOX-1814). +- **Dependencies**: Depends on SANDBOX-1806 (`pkg/agent` types). Depended on by SANDBOX-1811 (warm pod pool), SANDBOX-1812 (agent HTTP client — provides the transport layer the manager uses), SANDBOX-1813 (bash tool handler), SANDBOX-1814 (MCP server entry point). +- **New Go dependencies**: `k8s.io/client-go`, `k8s.io/api`, `k8s.io/apimachinery` +- **No breaking changes** — entirely new package. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md new file mode 100644 index 0000000..e7b6562 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md @@ -0,0 +1,50 @@ +## ADDED Requirements + +### Requirement: PodCache provides thread-safe in-memory caching with TTL + +The `PodCache` struct SHALL map session IDs to pod IPs and names using a `sync.RWMutex`-protected map with per-entry TTL expiry. + +#### Scenario: Set and Get within TTL +- **WHEN** `Set` is called with sessionID "inv-1", podIP "10.0.0.1", podName "pod-abc" +- **AND** `Get` is called for "inv-1" within the TTL window +- **THEN** `Get` SHALL return podIP "10.0.0.1", podName "pod-abc", and ok=true + +#### Scenario: Get returns miss for unknown session +- **WHEN** `Get` is called for a session ID that was never stored +- **THEN** it SHALL return empty strings and ok=false + +#### Scenario: Get returns miss after TTL expiry +- **WHEN** `Set` is called and the TTL duration elapses +- **AND** `Get` is called for the same session ID +- **THEN** it SHALL return empty strings and ok=false + +#### Scenario: Delete removes entry unconditionally +- **WHEN** `Delete` is called with a session ID +- **THEN** subsequent `Get` calls for that session SHALL return ok=false + +#### Scenario: Invalidate removes entry only if pod IP matches +- **WHEN** `Invalidate` is called with sessionID and podIP +- **AND** the cached entry has the same podIP +- **THEN** the entry SHALL be removed + +#### Scenario: Invalidate preserves entry if pod IP differs +- **WHEN** `Invalidate` is called with sessionID and a podIP different from the cached entry +- **THEN** the cached entry SHALL NOT be removed + +#### Scenario: EvictExpired removes all expired entries +- **WHEN** `EvictExpired` is called +- **THEN** all entries whose TTL has expired SHALL be removed +- **AND** entries whose TTL has not expired SHALL be preserved +- **AND** the count of evicted entries SHALL be returned + +#### Scenario: Default TTL is 30 seconds +- **WHEN** `NewPodCache` is called with ttl <= 0 +- **THEN** it SHALL use a default TTL of 30 seconds + +#### Scenario: Concurrent access is safe +- **WHEN** `Get`, `Set`, `Delete`, and `Invalidate` are called concurrently from multiple goroutines +- **THEN** all operations SHALL be serialized via the RWMutex without data races + +#### Scenario: Set overwrites existing entries +- **WHEN** `Set` is called for a session ID that already has a cached entry +- **THEN** the entry SHALL be replaced with the new podIP, podName, and a fresh TTL diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md new file mode 100644 index 0000000..ba5e0e8 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md @@ -0,0 +1,131 @@ +## ADDED Requirements + +### Requirement: SandboxConfig holds all tunables with sensible defaults + +The `SandboxConfig` struct SHALL hold all configurable parameters for sandbox pod lifecycle, and `DefaultConfig()` SHALL return production-ready defaults. + +#### Scenario: DefaultConfig provides production defaults +- **WHEN** `DefaultConfig()` is called +- **THEN** CPURequest SHALL be "100m", CPULimit "500m", MemoryRequest "128Mi", MemoryLimit "512Mi" +- **AND** IdleTimeout SHALL be 30 minutes +- **AND** WarmPoolSize SHALL be 0 (disabled) +- **AND** Namespace SHALL be "tarsy" +- **AND** ServiceAccountName SHALL be "cli-mcp-investigation-sa" +- **AND** KubeconfigSecret SHALL be "cli-mcp-investigation-kubeconfig" + +### Requirement: GetOrCreatePod resolves or creates a sandbox pod for a session + +The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label-based K8s API discovery → create new pod. + +#### Scenario: Cache hit returns cached pod IP +- **WHEN** `GetOrCreatePod` is called for a session with a valid cache entry +- **THEN** it SHALL return the cached pod IP without querying the K8s API + +#### Scenario: Cache miss triggers label-based discovery +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** a Running pod exists with matching session-id label +- **THEN** it SHALL return the pod's IP and cache the result + +#### Scenario: No existing pod triggers creation +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** no pod exists with matching session-id label +- **THEN** it SHALL create an auth Secret, create a sandbox pod, wait for ready, cache the result, and return the pod IP + +#### Scenario: Pod creation creates auth Secret first +- **WHEN** a new sandbox pod is created for a session +- **THEN** the per-session auth Secret SHALL be created before the Pod +- **AND** the Secret name SHALL be `cli-mcp-sandbox-auth-` +- **AND** the Secret SHALL contain the HMAC-derived token in the `token` key + +#### Scenario: Pod creation waits for readiness +- **WHEN** a new sandbox pod is created +- **THEN** `GetOrCreatePod` SHALL poll the pod status every 2 seconds +- **AND** return success only when the pod is Running with PodReady condition true +- **AND** timeout after 60 seconds with an error + +### Requirement: Pod spec follows security and operational standards + +The pod manifest built by `buildPodSpec` SHALL include all required security context, volumes, probes, and labels from the design doc. + +#### Scenario: Pod has correct labels and annotations +- **WHEN** `buildPodSpec` is called with a session ID +- **THEN** the pod SHALL have label `tarsy.redhat.com/session-id` = sessionID +- **AND** label `tarsy.redhat.com/component` = "cli-mcp-sandbox" +- **AND** annotation `tarsy.redhat.com/created-at` = current UTC time in RFC3339 +- **AND** annotation `tarsy.redhat.com/last-activity` = current UTC time in RFC3339 + +#### Scenario: Pod has restrictive security context +- **WHEN** `buildPodSpec` is called +- **THEN** the pod-level SecurityContext SHALL set RunAsNonRoot=true, RunAsUser=1001, RunAsGroup=1001 +- **AND** the container-level SecurityContext SHALL set AllowPrivilegeEscalation=false +- **AND** the container-level Capabilities SHALL Drop ALL + +#### Scenario: Pod has readiness probe on agent health endpoint +- **WHEN** `buildPodSpec` is called +- **THEN** the container SHALL have an HTTP readiness probe on /health port 8090 +- **AND** InitialDelaySeconds SHALL be 2 +- **AND** PeriodSeconds SHALL be 10 + +#### Scenario: Pod has kubeconfig and workspace volumes +- **WHEN** `buildPodSpec` is called +- **THEN** the pod SHALL have a "kubeconfig" volume from the configured Secret (read-only mount at /config) +- **AND** a "workspace" emptyDir volume (writable mount at /workspace) + +#### Scenario: Pod env delivers auth token via SecretKeyRef +- **WHEN** `buildPodSpec` is called for a session +- **THEN** the SANDBOX_AUTH_TOKEN env var SHALL reference the per-session Secret's "token" key +- **AND** KUBECONFIG SHALL be "/config/kubeconfig" +- **AND** HOME SHALL be "/workspace" + +### Requirement: ExecuteCommand proxies commands to the sandbox agent + +The `ExecuteCommand` method SHALL resolve the pod, authenticate with HMAC, POST to the agent, and return the response. + +#### Scenario: Successful command execution +- **WHEN** `ExecuteCommand` is called with a valid session ID and command +- **THEN** it SHALL POST to `http://:8090/exec` with JSON body containing command and timeout +- **AND** include `Authorization: Bearer ` header +- **AND** return the decoded `ExecResponse` + +#### Scenario: HMAC token derivation is deterministic +- **WHEN** `computeToken` is called with the same session ID and HMAC key +- **THEN** it SHALL always return the same hex-encoded HMAC-SHA256 value +- **AND** different session IDs SHALL produce different tokens + +#### Scenario: Agent unreachable invalidates cache +- **WHEN** the HTTP request to the agent fails (connection error or non-200 status) +- **THEN** the cache entry SHALL be invalidated for that session and pod IP +- **AND** an error SHALL be returned + +#### Scenario: Last-activity annotation is updated in background +- **WHEN** a command executes successfully +- **THEN** the pod's `tarsy.redhat.com/last-activity` annotation SHALL be patched with the current UTC timestamp +- **AND** the patch SHALL run in a background goroutine (not blocking the response) + +### Requirement: CleanupSession deletes a session's pod and Secret + +The `CleanupSession` method SHALL remove all Kubernetes resources for a session and clear the cache. + +#### Scenario: Full session cleanup +- **WHEN** `CleanupSession` is called with a session ID +- **THEN** all pods with matching session-id label SHALL be deleted +- **AND** the auth Secret `cli-mcp-sandbox-auth-` SHALL be deleted +- **AND** the cache entry SHALL be removed + +### Requirement: CleanupStale removes idle sessions exceeding the timeout + +The `CleanupStale` method SHALL iterate all sandbox pods and clean up those whose last-activity exceeds IdleTimeout. + +#### Scenario: Stale pod is cleaned up +- **WHEN** `CleanupStale` is called +- **AND** a pod's `last-activity` annotation indicates idle time exceeding IdleTimeout +- **THEN** the session SHALL be cleaned up via `CleanupSession` + +#### Scenario: Fresh pod is preserved +- **WHEN** `CleanupStale` is called +- **AND** a pod's `last-activity` annotation indicates idle time within IdleTimeout +- **THEN** the pod SHALL NOT be deleted + +#### Scenario: Pod without last-activity uses created-at as fallback +- **WHEN** `CleanupStale` encounters a pod with no `last-activity` annotation +- **THEN** it SHALL use the `created-at` annotation for idle time calculation diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md new file mode 100644 index 0000000..af35d1f --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md @@ -0,0 +1,57 @@ +## 1. SandboxConfig + +- [ ] 1.1 Create `pkg/session/config.go` with `SandboxConfig` struct (Image, CPURequest, CPULimit, MemoryRequest, MemoryLimit, IdleTimeout, HMACKey, WarmPoolSize, Namespace, ServiceAccountName, KubeconfigSecret) +- [ ] 1.2 Implement `DefaultConfig()` returning sensible defaults (100m/500m CPU, 128Mi/512Mi mem, 30m idle, tarsy namespace) + +## 2. PodCache + +- [ ] 2.1 Create `pkg/session/cache.go` with `PodCache` struct (`sync.RWMutex`, `map[string]*podEntry`, `ttl`) +- [ ] 2.2 Define `podEntry` struct (podIP, podName, expiresAt) +- [ ] 2.3 Implement `NewPodCache(ttl)` — default to 30s if ttl <= 0 +- [ ] 2.4 Implement `Get(sessionID)` — return pod IP and name if entry exists and not expired +- [ ] 2.5 Implement `Set(sessionID, podIP, podName)` — store with fresh TTL +- [ ] 2.6 Implement `Delete(sessionID)` — unconditional removal +- [ ] 2.7 Implement `Invalidate(sessionID, podIP)` — conditional removal (only if stored IP matches) +- [ ] 2.8 Implement `EvictExpired()` — remove all expired entries, return count +- [ ] 2.9 Implement `Len()` — return entry count (including expired but not yet evicted) + +## 3. SessionManager — Pod Discovery and Creation + +- [ ] 3.1 Create `pkg/session/manager.go` with `SessionManager` struct (clientset, config, cache, httpClient, logger) +- [ ] 3.2 Implement `NewSessionManager(clientset, config, logger)` — initialize cache and HTTP client +- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return IP of first Running pod +- [ ] 3.4 Implement `buildPodSpec(sessionID)` — construct Pod manifest with: + - GenerateName `cli-mcp-sandbox-` + - Labels: `tarsy.redhat.com/session-id`, `tarsy.redhat.com/component` + - Annotations: `tarsy.redhat.com/created-at`, `tarsy.redhat.com/last-activity` + - SecurityContext: RunAsNonRoot, RunAsUser 1001, RunAsGroup 1001 + - Container security: AllowPrivilegeEscalation=false, Drop ALL capabilities + - ReadinessProbe: HTTPGet /health:8090, initialDelay 2s, period 10s + - VolumeMounts: kubeconfig (ro at /config), workspace (rw at /workspace) + - Env: KUBECONFIG, HOME, SANDBOX_AUTH_TOKEN (from SecretKeyRef) +- [ ] 3.5 Implement `buildAuthSecret(sessionID, token)` — create Secret with token in StringData +- [ ] 3.6 Implement `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID), hex-encoded +- [ ] 3.7 Implement `waitForReady(ctx, podName)` — poll every 2s for up to 60s until Running + PodReady +- [ ] 3.8 Implement `createSandboxPod(ctx, sessionID)` — create Secret, create Pod, wait for ready +- [ ] 3.9 Implement `GetOrCreatePod(ctx, sessionID)` — cache hit → label discovery → create new + +## 4. SessionManager — Command Execution + +- [ ] 4.1 Implement `ExecuteCommand(ctx, sessionID, command, timeoutSec)` — resolve pod, compute token, POST to agent /exec +- [ ] 4.2 On HTTP error, call `cache.Invalidate(sessionID, podIP)` before returning error +- [ ] 4.3 On success, decode `agent.ExecResponse` from JSON body +- [ ] 4.4 Launch background `updateLastActivity(sessionID)` goroutine after successful exec +- [ ] 4.5 Implement `updateLastActivity(sessionID)` — patch pod annotation with current RFC3339 timestamp + +## 5. SessionManager — Cleanup + +- [ ] 5.1 Implement `CleanupSession(ctx, sessionID)` — delete cache entry, list+delete pods by label, delete auth Secret +- [ ] 5.2 Implement `CleanupStale(ctx)` — list all sandbox pods, parse last-activity annotation, cleanup those exceeding IdleTimeout + +## 6. Unit Tests + +- [ ] 6.1 Cache tests: set/get, TTL expiry, delete, invalidate (match/mismatch), evict expired, concurrent access, overwrite +- [ ] 6.2 Manager tests: cache hit, discover existing pod, create pod spec validation, auth secret structure, token determinism +- [ ] 6.3 Manager tests: cleanup session (pod + secret deleted, cache cleared) +- [ ] 6.4 Manager tests: cleanup stale (stale pod deleted, fresh pod preserved) +- [ ] 6.5 Manager tests: execute command via httptest server (token sent, request proxied, response decoded) From c9d09bdbd8ef3fccad3a385170787bd9358ee700 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Tue, 23 Jun 2026 14:16:52 +0530 Subject: [PATCH 2/4] fix: address CodeRabbit review comments on session manager openspec - Make GetOrCreatePod idempotent across replicas via K8s AlreadyExists conflict handling (new Decision 2 in design.md) - Add namespace, service account, kubeconfig secret name to SandboxConfig - Require Ready pods (not Running) with deterministic oldest-first selection - Add session ID validation requirement (RFC 1123 DNS label format) - Document HTTP trust boundary for intra-cluster pod communication - Narrow cache invalidation to transport-level failures only - Make CleanupStale explicitly delegate to CleanupSession for Secret cleanup Co-authored-by: Cursor Signed-off-by: Feny Mehta --- .../design.md | 33 +++++++++++++------ .../proposal.md | 2 +- .../specs/session-manager/spec.md | 28 ++++++++++++++-- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md index e81d43f..6ee5062 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md @@ -8,12 +8,12 @@ The MCP server is stateless — any replica can serve any request. All durable s **Goals:** - PodCache with thread-safe map and TTL-based expiry (~30s) -- SessionManager with `GetOrCreatePod` (label lookup, cache, create pod + auth Secret, wait for ready) +- SessionManager with `GetOrCreatePod` (label lookup, cache, idempotent create-or-get for pod + auth Secret using K8s API conflict handling, wait for ready) - SessionManager with `ExecuteCommand` (resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation) - SessionManager with `CleanupSession` (delete pod + Secret) - SessionManager with `CleanupStale` (periodic, idle-timeout based) - Pod spec includes: security context (RunAsNonRoot, drop ALL), readiness probe on :8090/health, kubeconfig volume, workspace emptyDir, correct labels and annotations -- SandboxConfig struct for all tunables (image, CPU/mem, idle timeout, HMAC key, warm pool size) +- SandboxConfig struct for all tunables (image, CPU/mem, idle timeout, HMAC key, warm pool size, namespace, service account, kubeconfig secret name) **Non-Goals:** - Warm pod pool (SANDBOX-1811) @@ -42,7 +42,20 @@ Use a simple `sync.RWMutex`-protected map with 30-second TTL entries. **Alternative considered:** Longer TTL (5 minutes) - Rejected — if a pod restarts and gets a new IP, requests would fail for up to 5 minutes before cache expiry forces rediscovery -### Decision 2: HMAC-SHA256 deterministic token derivation +### Decision 2: Idempotent `GetOrCreatePod` using K8s API conflict handling + +Use deterministic resource names (derived from session ID) and handle `AlreadyExists` errors from the Kubernetes API to make pod + Secret creation idempotent across replicas. + +**Rationale:** +- With a stateless multi-replica server, two requests for the same `X-Session-ID` can both miss the cache and labels, then race into the create path +- By using deterministic names (`cli-mcp-sandbox-`, `cli-mcp-sandbox-auth-`) and treating `AlreadyExists` as success (fall through to discovery), only one set of resources is created +- The Kubernetes API's built-in atomicity on create operations provides the serialization point — no external locking needed +- The losing replica simply re-discovers the pod that the winning replica created + +**Alternative considered:** Distributed lock (e.g., K8s Lease) before creation +- Rejected — adds complexity, latency, and a failure mode (lock not released) for a race that rarely occurs and is harmless when handled via conflict detection + +### Decision 3: HMAC-SHA256 deterministic token derivation Compute per-session auth tokens as `hex(HMAC-SHA256(HMACKey, sessionID))`. @@ -55,7 +68,7 @@ Compute per-session auth tokens as `hex(HMAC-SHA256(HMACKey, sessionID))`. **Alternative considered:** Random token stored in Secret, looked up by MCP server on each request - Rejected — requires a K8s API call to read the Secret on each command execution, defeating the purpose of caching -### Decision 3: Label-based pod discovery as source of truth +### Decision 4: Label-based pod discovery as source of truth Use label selectors (`tarsy.redhat.com/session-id=, tarsy.redhat.com/component=cli-mcp-sandbox`) for pod lookup. @@ -68,7 +81,7 @@ Use label selectors (`tarsy.redhat.com/session-id=, tarsy.redhat.com/compone **Alternative considered:** In-memory session-to-pod mapping as primary store - Rejected — breaks stateless multi-replica operation; a different replica wouldn't know about a pod another replica created -### Decision 4: Create auth Secret before pod +### Decision 5: Create auth Secret before pod Create the per-session Secret first, then create the pod that references it via `secretKeyRef`. @@ -80,7 +93,7 @@ Create the per-session Secret first, then create the pod that references it via **Alternative considered:** Create pod first, then Secret - Rejected — race condition where the container starts before the Secret exists -### Decision 5: Poll-based `waitForReady` with 60s deadline +### Decision 6: Poll-based `waitForReady` with 60s deadline Poll the pod status every 2 seconds for up to 60 seconds until Running + PodReady condition. @@ -93,7 +106,7 @@ Poll the pod status every 2 seconds for up to 60 seconds until Running + PodRead **Alternative considered:** Watch-based with informer - Rejected — adds significant complexity; session creation is not a hot path (happens once per investigation), so poll latency is acceptable -### Decision 6: Background annotation update after ExecuteCommand +### Decision 7: Background annotation update after ExecuteCommand Update `last-activity` annotation in a background goroutine (`go m.updateLastActivity()`). @@ -105,7 +118,7 @@ Update `last-activity` annotation in a background goroutine (`go m.updateLastAct **Alternative considered:** Synchronous annotation update - Rejected — adds latency to the critical path for non-critical metadata -### Decision 7: SessionManager directly uses `net/http` for agent communication +### Decision 8: SessionManager directly uses `net/http` for agent communication The manager constructs HTTP requests inline rather than depending on a separate agent client package. @@ -117,7 +130,7 @@ The manager constructs HTTP requests inline rather than depending on a separate **Alternative considered:** Block on SANDBOX-1812 first - Rejected — creates unnecessary sequencing; inline HTTP can be replaced later with a one-line refactor -### Decision 8: `CleanupStale` is caller-driven (no internal ticker) +### Decision 9: `CleanupStale` is caller-driven (no internal ticker) The `CleanupStale` method is called externally (e.g., by a goroutine in `cmd/server/main.go` on a ticker). It does not manage its own lifecycle. @@ -132,7 +145,7 @@ The `CleanupStale` method is called externally (e.g., by a goroutine in `cmd/ser ## Risks / Trade-offs **Risk:** 30s cache TTL means requests can be routed to a dead pod IP for up to 30s after pod restart -→ **Mitigation:** `Invalidate()` is called on HTTP errors (agent unreachable), immediately clearing the stale entry and forcing rediscovery on the next request. +→ **Mitigation:** `Invalidate()` is called on transport-level failures (connection refused, timeout, network unreachable), immediately clearing the stale entry and forcing rediscovery on the next request. Application-level HTTP errors (4xx/5xx from a reachable agent) do not invalidate the cache, since the pod is healthy. **Risk:** `waitForReady` polls every 2s — adds 0-2s latency to pod creation → **Mitigation:** Acceptable for session creation (happens once per investigation). The warm pool feature (SANDBOX-1811) eliminates this latency entirely for pre-warmed pods. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md index f5e9f18..3332b35 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md @@ -20,7 +20,7 @@ SANDBOX-1806/1807 delivered the sandbox agent (persistent bash + HTTP handlers). - `GetOrCreatePod(ctx, sessionID)` — cache → label lookup → create pod + auth Secret → wait for ready - `ExecuteCommand(ctx, sessionID, command, timeout)` — resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation - `CleanupSession(ctx, sessionID)` — delete pod + Secret, clear cache - - `CleanupStale(ctx)` — iterate sandbox pods, delete those idle longer than IdleTimeout + - `CleanupStale(ctx)` — iterate sandbox pods, delete those idle longer than IdleTimeout (delegates to `CleanupSession`, which removes both the pod and the matching auth Secret) - `buildPodSpec(sessionID)` — full pod manifest with security context, readiness probe, volumes, labels, annotations - `buildAuthSecret(sessionID, token)` — per-session Secret for HMAC token delivery - `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID) deterministic token derivation diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md index ba5e0e8..196a1ca 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md @@ -13,9 +13,21 @@ The `SandboxConfig` struct SHALL hold all configurable parameters for sandbox po - **AND** ServiceAccountName SHALL be "cli-mcp-investigation-sa" - **AND** KubeconfigSecret SHALL be "cli-mcp-investigation-kubeconfig" +### Requirement: Session IDs are validated before use in Kubernetes resource names + +Session IDs are interpolated into Kubernetes resource names (pod names, Secret names) and label values. They MUST be validated or normalized before use to ensure compatibility with Kubernetes naming rules. + +#### Scenario: Valid session ID is accepted +- **WHEN** a session ID matches the pattern `^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$` (RFC 1123 DNS label) +- **THEN** it SHALL be accepted and used directly in resource names + +#### Scenario: Invalid session ID is rejected +- **WHEN** a session ID contains characters outside `[a-z0-9-]`, starts/ends with a hyphen, or exceeds 63 characters +- **THEN** `GetOrCreatePod` SHALL return a validation error without making any K8s API calls + ### Requirement: GetOrCreatePod resolves or creates a sandbox pod for a session -The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label-based K8s API discovery → create new pod. +The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label-based K8s API discovery → idempotent create-or-get new pod (handling `AlreadyExists` conflicts from concurrent replicas). #### Scenario: Cache hit returns cached pod IP - **WHEN** `GetOrCreatePod` is called for a session with a valid cache entry @@ -23,14 +35,21 @@ The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label #### Scenario: Cache miss triggers label-based discovery - **WHEN** `GetOrCreatePod` is called for a session with no cache entry -- **AND** a Running pod exists with matching session-id label +- **AND** a Ready pod exists with matching session-id label (PodReady condition is true) - **THEN** it SHALL return the pod's IP and cache the result +- **AND** if multiple matching Ready pods exist, it SHALL select the oldest by creation timestamp for deterministic behavior #### Scenario: No existing pod triggers creation - **WHEN** `GetOrCreatePod` is called for a session with no cache entry - **AND** no pod exists with matching session-id label - **THEN** it SHALL create an auth Secret, create a sandbox pod, wait for ready, cache the result, and return the pod IP +#### Scenario: Concurrent create is idempotent via conflict handling +- **WHEN** two replicas simultaneously attempt to create a pod for the same session ID +- **AND** one replica's K8s create call returns `AlreadyExists` +- **THEN** the losing replica SHALL fall through to label-based discovery and return the existing pod's IP +- **AND** no duplicate pods or Secrets SHALL be created + #### Scenario: Pod creation creates auth Secret first - **WHEN** a new sandbox pod is created for a session - **THEN** the per-session auth Secret SHALL be created before the Pod @@ -87,15 +106,18 @@ The `ExecuteCommand` method SHALL resolve the pod, authenticate with HMAC, POST - **AND** include `Authorization: Bearer ` header - **AND** return the decoded `ExecResponse` +**Trust boundary note:** HTTP (not HTTPS) is used because the MCP server and sandbox agent pods communicate over the Kubernetes pod network within the same cluster. The bearer token guards against unauthorized callers within the cluster network, not against network-level eavesdropping. If cross-cluster or external-network communication is needed in future, this should be upgraded to TLS/mTLS. + #### Scenario: HMAC token derivation is deterministic - **WHEN** `computeToken` is called with the same session ID and HMAC key - **THEN** it SHALL always return the same hex-encoded HMAC-SHA256 value - **AND** different session IDs SHALL produce different tokens #### Scenario: Agent unreachable invalidates cache -- **WHEN** the HTTP request to the agent fails (connection error or non-200 status) +- **WHEN** the HTTP request to the agent fails with a transport-level error (connection refused, timeout, DNS resolution failure, or network unreachable) - **THEN** the cache entry SHALL be invalidated for that session and pod IP - **AND** an error SHALL be returned +- **AND** application-level HTTP errors (4xx/5xx responses from a reachable agent) SHALL NOT trigger cache invalidation #### Scenario: Last-activity annotation is updated in background - **WHEN** a command executes successfully From 3fb9a35b9a2e0574cc032b75bdf0405ed5538780 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 24 Jun 2026 11:07:48 +0530 Subject: [PATCH 3/4] fix: document concurrent execution semantics and align tasks with spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add scenario for concurrent command serialization at the agent - Add trade-off entry for queued execution in design.md - Fix tasks.md: Running→Ready pod selection, HTTP→transport-level cache invalidation, add session ID validation task Co-authored-by: Cursor Signed-off-by: Feny Mehta --- .../changes/2026-06-22-pod-cache-session-manager/design.md | 3 +++ .../specs/session-manager/spec.md | 6 ++++++ .../changes/2026-06-22-pod-cache-session-manager/tasks.md | 7 ++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md index 6ee5062..1ebab69 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md @@ -156,6 +156,9 @@ The `CleanupStale` method is called externally (e.g., by a goroutine in `cmd/ser **Trade-off:** Manager uses inline `net/http` instead of `pkg/agent/client` → **Accepted:** Will be refactored in SANDBOX-1812. Keeps this story self-contained. +**Trade-off:** Concurrent commands for the same session queue at the agent +→ **Accepted:** The session manager is stateless and POSTs each command independently. The sandbox agent serializes execution via its `BashSession` mutex (single persistent bash process). If N parallel requests arrive, they queue and execute sequentially — no interleaving. This means the Nth request's wall-clock latency includes waiting for the previous N-1 commands. This is the correct behavior for a single bash process; users who need parallelism can use shell-level constructs (`&`, `&&`) within a single command. + **Trade-off:** No retry on pod creation failure → **Accepted:** Single attempt returns error to caller. The MCP tool handler will propagate the error to TARSy, which can retry the tool call. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md index 196a1ca..13ffe81 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md @@ -119,6 +119,12 @@ The `ExecuteCommand` method SHALL resolve the pod, authenticate with HMAC, POST - **AND** an error SHALL be returned - **AND** application-level HTTP errors (4xx/5xx responses from a reachable agent) SHALL NOT trigger cache invalidation +#### Scenario: Concurrent commands for the same session are serialized at the agent +- **WHEN** multiple `ExecuteCommand` calls arrive concurrently for the same session ID +- **THEN** the session manager SHALL POST each request independently to the agent (no server-side queuing) +- **AND** the sandbox agent SHALL serialize execution via its bash session mutex — each command runs to completion before the next begins +- **AND** each queued command's timeout SHALL start when it begins executing, not when the HTTP request was received + #### Scenario: Last-activity annotation is updated in background - **WHEN** a command executes successfully - **THEN** the pod's `tarsy.redhat.com/last-activity` annotation SHALL be patched with the current UTC timestamp diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md index af35d1f..3f1c459 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md @@ -19,7 +19,7 @@ - [ ] 3.1 Create `pkg/session/manager.go` with `SessionManager` struct (clientset, config, cache, httpClient, logger) - [ ] 3.2 Implement `NewSessionManager(clientset, config, logger)` — initialize cache and HTTP client -- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return IP of first Running pod +- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return IP of oldest Ready pod (PodReady condition true, deterministic selection by creation timestamp) - [ ] 3.4 Implement `buildPodSpec(sessionID)` — construct Pod manifest with: - GenerateName `cli-mcp-sandbox-` - Labels: `tarsy.redhat.com/session-id`, `tarsy.redhat.com/component` @@ -33,12 +33,13 @@ - [ ] 3.6 Implement `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID), hex-encoded - [ ] 3.7 Implement `waitForReady(ctx, podName)` — poll every 2s for up to 60s until Running + PodReady - [ ] 3.8 Implement `createSandboxPod(ctx, sessionID)` — create Secret, create Pod, wait for ready -- [ ] 3.9 Implement `GetOrCreatePod(ctx, sessionID)` — cache hit → label discovery → create new +- [ ] 3.9 Implement `validateSessionID(sessionID)` — reject IDs not matching RFC 1123 DNS label format (`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) +- [ ] 3.10 Implement `GetOrCreatePod(ctx, sessionID)` — validate session ID, then cache hit → label discovery → idempotent create-or-get (handle `AlreadyExists` conflicts) ## 4. SessionManager — Command Execution - [ ] 4.1 Implement `ExecuteCommand(ctx, sessionID, command, timeoutSec)` — resolve pod, compute token, POST to agent /exec -- [ ] 4.2 On HTTP error, call `cache.Invalidate(sessionID, podIP)` before returning error +- [ ] 4.2 On transport-level error (connection refused, timeout, network unreachable), call `cache.Invalidate(sessionID, podIP)` before returning error — application-level HTTP errors (4xx/5xx) SHALL NOT invalidate cache - [ ] 4.3 On success, decode `agent.ExecResponse` from JSON body - [ ] 4.4 Launch background `updateLastActivity(sessionID)` goroutine after successful exec - [ ] 4.5 Implement `updateLastActivity(sessionID)` — patch pod annotation with current RFC3339 timestamp From 15a75380095be086e3c3b8d7def367a8223415b3 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 25 Jun 2026 12:31:02 +0530 Subject: [PATCH 4/4] fix: handle non-Ready pods in discovery and fix pod naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add spec scenario for cache miss with non-Ready pod (wait for Ready instead of looping into AlreadyExists) - Fix tasks.md: GenerateName → deterministic Name (cli-mcp-sandbox-), required for idempotent create-or-get - Update discoverPod task to skip terminal pods (Failed/Succeeded) and wait for non-Ready pods via waitForReady Co-authored-by: Cursor Signed-off-by: Feny Mehta --- .../specs/session-manager/spec.md | 9 ++++++++- .../2026-06-22-pod-cache-session-manager/tasks.md | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md index 13ffe81..6eed863 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md @@ -39,9 +39,16 @@ The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label - **THEN** it SHALL return the pod's IP and cache the result - **AND** if multiple matching Ready pods exist, it SHALL select the oldest by creation timestamp for deterministic behavior +#### Scenario: Cache miss finds non-Ready pod +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** a non-terminal pod exists with matching session-id label (Pending or Running, but PodReady is false) +- **AND** no Ready pod exists for the session +- **THEN** it SHALL wait for the existing pod to become Ready (reusing `waitForReady` with the 60s deadline) +- **AND** return the pod's IP and cache the result once Ready + #### Scenario: No existing pod triggers creation - **WHEN** `GetOrCreatePod` is called for a session with no cache entry -- **AND** no pod exists with matching session-id label +- **AND** no non-terminal pod exists with matching session-id label (either no pods, or only Failed/Succeeded pods) - **THEN** it SHALL create an auth Secret, create a sandbox pod, wait for ready, cache the result, and return the pod IP #### Scenario: Concurrent create is idempotent via conflict handling diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md index 3f1c459..1c226f5 100644 --- a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md @@ -19,9 +19,9 @@ - [ ] 3.1 Create `pkg/session/manager.go` with `SessionManager` struct (clientset, config, cache, httpClient, logger) - [ ] 3.2 Implement `NewSessionManager(clientset, config, logger)` — initialize cache and HTTP client -- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return IP of oldest Ready pod (PodReady condition true, deterministic selection by creation timestamp) +- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return oldest Ready pod's IP (PodReady condition true, deterministic by creation timestamp). If no Ready pod but a non-terminal pod exists (Pending/Running), wait for it via `waitForReady`. Skip terminal pods (Failed/Succeeded). - [ ] 3.4 Implement `buildPodSpec(sessionID)` — construct Pod manifest with: - - GenerateName `cli-mcp-sandbox-` + - Name `cli-mcp-sandbox-` (deterministic, required for idempotent create-or-get via `AlreadyExists` handling) - Labels: `tarsy.redhat.com/session-id`, `tarsy.redhat.com/component` - Annotations: `tarsy.redhat.com/created-at`, `tarsy.redhat.com/last-activity` - SecurityContext: RunAsNonRoot, RunAsUser 1001, RunAsGroup 1001