From 3a5cdc5108b463b33717e6986928851692a54b23 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 18 Jul 2026 15:41:43 +0200 Subject: [PATCH] CB-306: spawn-readiness gate design note (delegation spec) --- docs/CB-306-Spawn-Readiness-Gate.md | 143 ++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 docs/CB-306-Spawn-Readiness-Gate.md diff --git a/docs/CB-306-Spawn-Readiness-Gate.md b/docs/CB-306-Spawn-Readiness-Gate.md new file mode 100644 index 0000000..b9ee1cb --- /dev/null +++ b/docs/CB-306-Spawn-Readiness-Gate.md @@ -0,0 +1,143 @@ +# CB-306 — Spawn-Readiness Gate (launcher-owned terminal readiness) + +**Status:** design note / delegation spec (branch `worker/cb-306-readiness`) +**Issue:** gitea `lms/claude-bridge` #4 +**Owner of the behaviour:** `ClaudeCodeLauncher` (the `PeerLauncher` adapter) — NOT core. + +## 1. Problem + +`bridge_spawn` today returns a session the instant the herdr pane is started. The pane is not +yet a usable Claude REPL — it may still be sitting at the folder-trust prompt, or the CLI may +never come up at all. Nothing blocks or times out on that. Consequences: + +- A `bridge_send` to a not-yet-ready worker surfaces as a **~60 s MCP-client timeout** (the send + blocks waiting for a turn that can't start) instead of a fast, explicit spawn failure. +- A worker stuck at the folder-trust prompt lingers in `SPAWNING` forever; nothing fails it. + +We want **fail-fast spawn**: `spawn()` returns only once the peer is genuinely up and usable in +its terminal, or throws a clean error (and leaves no orphan pane) within a bounded timeout. + +## 2. What already exists (do NOT rebuild) + +`SessionManager` + `PresenceBridge.markPresent()` already flip a session `SPAWNING → READY` on +**any MCP contact from the worker** (`onReady(terminal)` → `transitionByTerminal(SPAWNING, READY)`, +SessionManager ~line 249, "worker became available on the bridge MCP"). That is the **delivery +lifecycle** and it stays exactly as-is. CB-306 does **not** touch it and does **not** replace it. + +CB-306 adds a *complementary, launcher-side* gate: the launcher guarantees the **terminal** is a +live, interactive REPL before it hands a handle back. The two signals are layered: + +| Signal | Owner | Means | CB-306 | +|---|---|---|---| +| terminal reaches interactive REPL (herdr `IDLE`) | `ClaudeCodeLauncher` (this ticket) | pane is past folder-trust, CLI is up | **NEW — the spawn gate** | +| first MCP contact → `SPAWNING→READY` | core (`SessionManager`/`PresenceBridge`) | worker spoke to the bridge | unchanged | + +## 3. The readiness predicate (herdr status) + +`AgentStatus.fromWire` maps herdr's wire strings to `IDLE | WORKING | BLOCKED | DONE | UNKNOWN`. +A freshly started pane that has **not** reached an interactive Claude — including one stalled at +the folder-trust prompt — reports **`UNKNOWN`** (herdr has not detected a Claude REPL yet). Once +the CLI is up and settled at its prompt it reports **`IDLE`**. + +**Predicate:** the pane is *ready* when `AgentControl.status(target)` first returns an +**injectable** state (`IDLE`, `BLOCKED`, or `DONE` — reuse `AgentStatus.injectable()`). `UNKNOWN` += not ready. `WORKING` alone is ambiguous this early and should not by itself satisfy readiness; +wait for an injectable state. (We do not need to know *why* a pane isn't ready — a trust stall, +a crash, and a slow start all present as "never becomes injectable" and all correctly time out.) + +## 4. Contract change on `spawn(SpawnRequest)` + +`ClaudeCodeLauncher.spawn(SpawnRequest)` becomes **block-until-ready-or-throw**: + +1. Start the pane exactly as today (`spawn(profile, cwd, callerCwd) → Agent`, build env + guard + + argv, `spawnInTab`/`spawnAsPane`, unique-named). +2. **Poll** `agentControl.status(paneId)` every `pollIntervalMs` (~300 ms) until it is `injectable()` + or `spawnReadyTimeoutMs` elapses. +3. **Ready** → return the `WorkerHandle(paneId, terminalId)` as today. +4. **Timeout** → the launcher **closes the pane it started** (and its tab, via the same path + `release`/`stop` uses) and throws **`PeerUnreachableException`** (new, in `dev.ltms.bridged.peer`). + No orphan pane is left behind — the launcher cleans up its own failed birth. + +`spawnReadyTimeoutMs == 0` (or unset) **disables** the gate = legacy non-blocking behaviour, so the +change is opt-in per deployment and existing tests that don't configure it keep their old semantics. + +### Testability seam (required) + +Do **not** call `Thread.sleep` directly in the poll loop against a real clock — unit tests must not +real-sleep. Introduce a small injectable seam, mirroring the existing `StatusPoller` style: + +- a `LongSupplier nowMillis` (monotonic clock) **and** a sleep/wait hook (e.g. a + `Sleeper`/`Waiter` functional interface, or reuse whatever `StatusPoller` already uses), both + defaulting to the real implementations in the production constructor and overridable in tests. + +Unit tests (add to the existing `ClaudeCodeLauncher` test): +- fake `AgentControl` returns `UNKNOWN` a few times then `IDLE` → `spawn` returns the handle; assert + no `close` was called. +- fake `AgentControl` always `UNKNOWN` → `spawn` throws `PeerUnreachableException`; assert the pane + **was closed** (verify `close(paneId)` invoked) and the fake clock advanced past the timeout. +- `spawnReadyTimeoutMs == 0` → `spawn` returns immediately without polling (legacy path). + +## 5. Config + +Add to the launcher-level config (a bridged-level knob, not per-profile) in `bridged.yaml` + +`BridgedConfig`: + +```yaml +spawn_ready_timeout_ms: 20000 # 0 disables the gate (legacy non-blocking spawn) +spawn_ready_poll_ms: 300 +``` + +Jackson ignores unknown keys, so omitting them in existing YAML is safe; pick sane defaults in code +(`20000` / `300`). Keep the names consistent with existing config field style in `BridgedConfig`. + +## 6. Core / MCP propagation + +`SessionManager.acquire(...)` already calls `launcher.spawn(req)`. A thrown +`PeerUnreachableException` must propagate out as a **clean spawn failure**: + +- The **worktree** acquire path already has a try/catch that cleans up a provisioned worktree when + `spawn` throws — verify the new exception flows through it (worktree removed, nothing registered). +- The **non-worktree** path registers the session only *after* `spawn` returns, so a throw means no + half-live `SPAWNING` session is ever registered — confirm this and add a test. +- `bridge_spawn` (MCP verb) must return an **error result** carrying the exception message, not a + success with a dead session. Trace `BridgeMcp`/`BridgedApp` spawn handlers and make sure the + exception becomes a clean tool error, not an uncaught 500 with a stack trace. + +**Out of scope (do NOT do here):** gating `bridge_send` on session `READY` (existing status-gate + +this spawn gate already close the window), MCP-handshake-as-readiness signal, the CB-307 broker, +any config `kind:` discriminator, any second adapter. + +## 7. Definition of done + +- `ClaudeCodeLauncher.spawn` blocks until injectable or throws `PeerUnreachableException` + + self-reaps the pane; gate disabled when timeout is 0. +- New `PeerUnreachableException` in `dev.ltms.bridged.peer`. +- Config knobs wired (`spawn_ready_timeout_ms`, `spawn_ready_poll_ms`) with safe defaults. +- Existing `SPAWNING→READY` MCP-contact transition untouched. +- New unit tests (ready / timeout+reap / disabled) green; **all existing tests still pass unchanged**. +- Build clean via the worker's own `mvn` (primary re-runs the authoritative IDE + `mvn clean install` + gate — self-reports are not verified facts). + +## 8. Sequence + +```mermaid +sequenceDiagram + participant SM as SessionManager.acquire + participant L as ClaudeCodeLauncher.spawn + participant T as herdr (AgentControl) + SM->>L: spawn(SpawnRequest) + L->>T: start pane (env+guard+argv) + loop until injectable or timeout + L->>T: status(paneId) + T-->>L: UNKNOWN / IDLE + end + alt reached injectable + L-->>SM: PeerHandle(id, terminalId) + else timed out + L->>T: close(paneId) + tab + L-->>SM: throw PeerUnreachableException + SM-->>SM: no session registered / worktree cleaned + end +``` + +*Figure — the launcher blocks in `spawn` until the pane is a usable REPL, else self-reaps and throws.*