Dai Ha
555715ced9
The repo moved lms/claude-bridge -> fleet/claude-bridge -> fleet/fleetd. Gitea redirects hold, so most of this is not urgent, but one line was a real break: the implementer skill posts a worker's PR to a hardcoded repo path, so every worker PR would have gone to the old address. .claude/skills/implementer/SKILL.md the worker PR endpoint (functional) .gitmodules wiki submodule URL CLAUDE.md + wiki/7-Use-Cases.md the canonical block, kept byte-identical README.md clone command and wiki link deploy/bridged.service Documentation= plugin/.claude-plugin/plugin.json homepage + repository docs/*.md issue and wiki links The wiki is not a separate repo. /repos/lms/claude-bridge.wiki returns 404 and lms owned no .wiki entity, so the wiki moved with the repo; both the old and the new wiki SSH URLs resolve to the same sha. Ticket step 4 assumed a second transfer that does not exist.
144 lines
7.5 KiB
Markdown
144 lines
7.5 KiB
Markdown
# CB-306 — Spawn-Readiness Gate (launcher-owned terminal readiness)
|
|
|
|
**Status:** design note / delegation spec (branch `worker/cb-306-readiness`)
|
|
**Issue:** gitea `fleet/fleetd` #4
|
|
**Owner of the behaviour:** `ClaudeCodeLauncher` (the `PeerLauncher` adapter) — NOT core.
|
|
|
|
## 1. Problem
|
|
|
|
`fleet_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 `fleet_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.
|
|
- `fleet_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 `fleet_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.*
|