CB-306: spawn-readiness gate design note (delegation spec)
This commit is contained in:
Dai Ha
@@ -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.*
|
||||
Reference in New Issue
Block a user