diff --git a/CLAUDE.md b/CLAUDE.md index f94a0a0..00425f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -215,11 +215,14 @@ must obey belongs in the charter, not here. reads the list with `git config --worktree --get-all fleet.neutralizedConfig`, and the consequence with `git config --worktree --get fleet.neutralizedConfigNote`. Never brief a worker to edit one of these files: the edit cannot be committed, and it will not tell you so. -- **Flows and the error model** — rendezvous, `fleet_ask`, detached delivery, turn-done fallback — - are diagrammed in `docs/MCP-Contract.md` **§6 only**. The rest of that page is a pre-build design - doc whose tool names, parameter names and REST paths never caught up with the code, so do not use - it as the tool reference (CB-609). Section 6 is kept out of this file because this file loads into - every session's context. +- **Flows and the error model** — rendezvous, `fleet_ask`, detached delivery, the turn-done + fallback and status gating — are diagrammed in `docs/MCP-Contract.md`. That page is now flows + only: its pre-build tool catalogue, parameter tables and REST paths were deleted rather than + corrected, because a hand-maintained second copy of the tool surface is what drifted for a month + while this line pointed every session at it (CB-609 / #114). **The live MCP schema is the tool + reference**, with the intent→tool table above as the short form. `McpContractDocTest` fails if + that page names a `fleet_*` tool the server does not register. The flows are kept out of this + file because this file loads into every session's context. ### Redeploying the daemon — the lead may do this (primary only) diff --git a/docs/MCP-Contract.md b/docs/MCP-Contract.md index 5281764..dd21413 100644 --- a/docs/MCP-Contract.md +++ b/docs/MCP-Contract.md @@ -1,311 +1,162 @@ -# MCP Contract — `fleetd`'s unified gateway +# MCP flows and error model — `fleetd` -> **Status: 🔴 HISTORICAL DESIGN — do NOT use as the tool reference.** Written 2026-07-14, before -> any MCP code existed. The system shipped and this page never caught up, so **its tool names, -> parameter names and REST paths are wrong today**. Audited 2026-08-17; the specific drift: +> **What this page is.** The **flows**: how a delegation, a clarification, a detached task and a +> silent member each travel through `fleetd`. These shapes are what shipped, and they are hard to +> read off the code because they span the MCP face, the rendezvous registry, the `Injector` and +> herdr. > -> - **Tools it names that do not exist:** `fleet_read`, `fleet_cancel`. -> - **Shipped tools it omits:** `fleet_poll`, `fleet_ack`, `fleet_profiles`, `fleet_whoami`. -> - **Parameter names are wrong nearly everywhere** — it says `message`/`target`/`timeout_seconds`/ -> `block` where the code takes `content`/`sessionId`/`timeoutMs`/`wait`; `text` where -> `fleet_reply` takes `content`; `target` where `fleet_stop` takes `paneId`. -> - **REST paths are wrong:** it says `POST /workers` and `DELETE /workers/{paneId}`; the daemon -> serves `POST /members` and `DELETE /members/{paneId}`. +> **What this page is NOT: a tool reference.** It deliberately holds no tool catalogue, no +> parameter tables and no REST paths. **The live MCP schema is the authority** — each tool's own +> description and parameters, as mounted — with the intent→tool table in `CLAUDE.md` as the short +> form. > -> **The authoritative tool surface is the live MCP schema** (each tool's own description and -> parameters, as mounted), with the intent→tool table in `CLAUDE.md` as the short form. Both were -> checked against `mcp/FleetMcp.java` on 2026-08-17 and are accurate. +> That absence is the fix for fleetd #114 (CB-609), and it is worth stating why. This page used to +> carry a full tool catalogue written in July 2026, before any MCP code existed. The code shipped; +> the page did not follow. By August it named two tools that do not exist, omitted five that do, +> had the wrong name for nearly every parameter, pointed at REST paths the daemon does not serve, +> and — worst — still described an identity model (*"any connection that does not map to a known +> worker is treated as a primary"*) that was a real privilege bug, fixed since by the ancestry +> walk in fleetd #161. Every one of those errors is the same error: **a second, hand-maintained +> copy of something the code already states**. So the second copy is gone rather than corrected. +> Only the flows remain, because a flow is a shape rather than a name, and shapes are what this +> page was ever good for. > -> What is still worth reading here is **§6 — the flows and the error model** (rendezvous, -> `fleet_ask`, detached delivery, the turn-done fallback). The shapes it describes are the ones -> that shipped; only the names around them drifted. Rewriting this page is tracked as **CB-609**. - -`fleetd` is the **sole communication gateway** for every Claude session in the bridge. Both -the **primary** (Opus, on subscription) and every **worker** (off-subscription Claude Code) -mount the *same* MCP server with a single `claude mcp add` line, and talk only through its -tools. No Claude session ever addresses a broker, a peer, or the network directly. - -This document defines every MCP tool that face must expose, who may call it, its blocking -semantics, and how it maps onto the code already in the tree. +> The names that do appear below are checked by `McpContractDocTest`, which fails if this page +> names a `fleet_*` tool the server does not register. That test is the whole reason it is safe to +> write a tool name here at all. --- -## 1. Design constraints (non-negotiable) +## 1. Rendezvous flows -These come from the project's core invariants and bound every decision below. +### 1.1 Delegation — happy path -1. **One server, both roles.** The primary and all workers mount an identical server. The - catalog must serve both, and `fleetd` must decide *who is calling* from the connection — - never from a caller-supplied argument that could be spoofed. -2. **Subscription-safe by construction.** No MCP tool ever reads, sets, or forwards - `ANTHROPIC_BASE_URL`. Mounting the bridge cannot move a session off subscription. - Enforced today by [`SubscriptionGuard`](1-Architecture). -3. **Blocking rendezvous, no busy-poll.** The primary consumes a worker's reply through a - *single* MCP call that `fleetd` holds open — never a cross-turn poll loop that would burn - subscription quota. -4. **Status-gated delivery.** Anything that puts text into a worker flows through the existing - [`Injector`](1-Architecture): delivered only when the worker is `idle`/`blocked`, at most - one message per turn. -5. **`fleetd` owns policy; herdr owns PTYs.** MCP tools express *intent*; `fleetd` - translates it into guard checks, rendezvous bookkeeping, and herdr `agent.*` calls. - ---- - -## 2. Topology - -Both faces live in the one daemon. The **north face** is MCP (this document); the **south -face** is the herdr Unix socket. REST/SSE remains only for non-Claude clients and dashboards. - -```mermaid -flowchart LR - OPUS["Opus — primary
(Claude Code, env CLEAN)
MCP client"] - subgraph BD["fleetd — standalone daemon"] - MCP["MCP server (north face)
fleet_send · fleet_reply
fleet_ask · fleet_status · lifecycle"] - RDV["rendezvous registry
(blocking-call waiters)"] - INJ["Injector + StatusPoller
(status-gated writer)"] - SOCK["herdr socket client (south face)"] - MCP --> RDV - RDV --> INJ - INJ --> SOCK - MCP --> SOCK - end - HERDR["herdr
panes · agent-status"] - W["worker claude pane
ANTHROPIC_BASE_URL set
MCP client"] - - OPUS -->|"fleet_send (blocks)"| MCP - W -.->|"fleet_reply / fleet_ask"| MCP - SOCK -->|"agent.start · agent.send
agent.get · pane.close"| HERDR - HERDR -->|"drives PTY"| W - - classDef ext fill:#2b6cb0,stroke:#1a365d,color:#ffffff; - classDef core fill:#2f855a,stroke:#22543d,color:#ffffff; - class OPUS,W ext - class MCP,RDV,INJ,SOCK core -``` - ---- - -## 3. Identity & addressing - -Because the same server is mounted by everyone, `fleetd` resolves the caller's role on every -request — this is the linchpin of the whole contract and has no code yet. - -- **Workers are known.** `fleetd` spawns every worker - ([`WorkerService`](1-Architecture)) and records its herdr session UUID / `terminal_id` on - the returned [`Agent`]. When a call arrives on a connection that maps to a known worker, - the caller is *that* worker — so **workers never pass a target**; routing is implicit. -- **The primary is "not a worker".** Any connection that does not map to a known worker is - treated as a primary. It addresses workers **explicitly** by `target` — a session UUID, - a `terminal_id`, or a friendly `profile` name. -- **Turn correlation.** A blocking `fleet_send` registers a *waiter* keyed by worker - identity. A worker's later `fleet_reply` / `fleet_ask` on the same identity resolves that - waiter. A `turn_id` is minted per exchange so a clarification round-trip - (§6.2) rejoins the right turn. - ---- - -## 4. Transport - -`fleetd` is a long-lived daemon serving **multiple** concurrent clients (one primary + N -workers), so a per-client stdio child is the wrong shape. The recommended transport is -**streamable-HTTP / SSE** on the same bind as the REST face: - -```bash -# identical on primary and every worker -claude mcp add --transport http fleetd http://127.0.0.1:8080/mcp -``` - -This adds an MCP-server dependency the pom does not yet carry. See [Open decisions](#10-open-decisions). - ---- - -## 5. Tool catalog - -| Tool | Caller | Blocks? | Backing (exists today?) | -|---|---|---|---| -| [`fleet_send`](#fleet_send) | primary | yes (default) | `Injector.enqueue` ✅ · rendezvous registry ❌ (CB-104) | -| [`fleet_reply`](#fleet_reply) | worker | no | rendezvous ❌ · pane injection via `Injector` ✅ | -| [`fleet_ask`](#fleet_ask) | worker | yes | reverse rendezvous ❌ | -| [`fleet_status`](#fleet_status) | either | no | `AgentControl.status` ✅ · `Injector.activeTargets` ✅ | -| [`fleet_spawn`](#lifecycle) | primary | no | `WorkerService.spawn` ✅ (`POST /workers`) | -| [`fleet_list`](#lifecycle) | either | no | `WorkerService.list` ✅ (`/agents`) | -| [`fleet_stop`](#lifecycle) | primary | no | `WorkerService.stop` ✅ (`DELETE /workers/{paneId}`) | -| [`fleet_read`](#fleet_read) | primary | no | `AgentControl.read` ✅ | -| [`fleet_cancel`](#fleet_cancel) | primary | no | — ❌ (future) | - -### Core: delegation & rendezvous - -#### `fleet_send` -*(primary → worker — the headline tool, CB-104)* - -- **Params:** `message` (required); `target` (optional — defaults to the sole worker / default - profile); `timeout_seconds` (default 600); `block` (default `true`); `auto_spawn` - (default `true`); `turn_id` (optional — supplied when answering a worker's `fleet_ask`). -- **Blocking (`block:true`):** enqueue `message` via the `Injector`, then hold the call open - until exactly one of: - - worker calls `fleet_reply` → `{ outcome:"reply", text }` - - worker calls `fleet_ask` → `{ outcome:"question", text, turn_id }` - - worker's `agent_status` reaches done/idle with no reply → `{ outcome:"turn_done", text: }` - - deadline elapses → `{ outcome:"timeout" }` - - worker gone → error `worker_gone` -- **Detached (`block:false`):** enqueue and return `{ outcome:"dispatched", dispatch_id }` - immediately. The eventual reply is injected into the primary's idle pane (§6.3), or drained - via `fleet_status` on a split-host primary. - -#### `fleet_reply` -*(worker → primary)* - -- **Params:** `text` (required); `final` (default `true`). -- **Behavior:** resolve the primary waiter registered against this worker with `text`. If no - waiter exists (detached delegation), `fleetd` **injects the primary's idle pane** instead. - Returns `{ delivered:true, mode:"resolved"|"injected" }`. No `target` — identity is implicit. - -#### `fleet_ask` -*(worker → primary — the reverse rendezvous)* - -- **Params:** `question` (required); `timeout_seconds`. -- **Behavior:** blocks the *worker's* call. Surfaces the question to the primary (resolving its - open `fleet_send` with `outcome:"question"`, or injecting its pane). When the primary - answers — a `fleet_send` carrying the matching `turn_id` — that unblocks this call and - returns `{ answer }` to the worker, which continues **in the same turn**. - -### Worker lifecycle - -Thin adapters over [`WorkerService`](1-Architecture) — parity with the existing REST routes. - -- **`fleet_spawn`** — `{ profile? }` → worker view (`sessionId`, `terminalId`, `paneId`, - `status`). Guard-checked; a boundary breach returns error `subscription_boundary` (the - REST `403`). -- **`fleet_list`** — no params → all workers + `agent_status`. Read-only, either role. -- **`fleet_stop`** — `{ target }` → tears down the pane and its dedicated tab. Idempotent. - -### Observability - -#### `fleet_status` -*(either role — the README's 4th named tool)* - -- **Params:** `target?`. -- **Behavior:** per-worker `agent_status`, queue depth (`Injector.activeTargets`), whether a - rendezvous is open, and ids. For the *calling* session it also reports/drains **pending - messages addressed to me** — the path a split-host primary's `Stop`-hook uses to wake and - collect replies without being injectable. Read-only, non-blocking. - -#### `fleet_read` -*(primary)* - -- **Params:** `target`; `source` ∈ `visible | recent | recent_unwrapped | detection`. -- **Behavior:** returns the worker's terminal text so the primary can peek at a *detached* - worker's progress. Adapter over `AgentControl.read`. - -### Control (future) - -#### `fleet_cancel` -*(primary)* - -- **Params:** `target`. Interrupt the worker's current turn / abandon the rendezvous. No - backing code yet. - ---- - -## 6. Rendezvous flows - -### 6.1 Delegation — happy path - -One blocking call, zero polls. +One blocking call, zero polls. The lead's call is held open by `fleetd` until the member answers. ```mermaid sequenceDiagram - participant P as Primary (Opus) - participant B as fleetd (MCP + Injector) + participant P as "Lead (primary)" + participant B as "fleetd (MCP + Injector)" participant H as herdr - participant W as Worker (Claude) + participant W as "Member" - P->>B: fleet_send("do X", target=w) — blocks - B->>B: register waiter(w) - B->>H: agent.send(w, "do X") (idle window) - H-->>W: prompt injected - W->>W: works the turn - W->>B: fleet_reply("result") - B->>B: resolve waiter(w) - B-->>P: { outcome:"reply", text:"result" } + P->>B: "fleet_send{sessionId, content} — blocks" + B->>B: "register waiter(sessionId)" + B->>H: "agent.send — only in an injectable window" + H-->>W: "prompt injected" + W->>W: "works the turn" + W->>B: "fleet_reply{content}" + B->>B: "resolve waiter" + B-->>P: "{ outcome: reply }" ``` -### 6.2 Clarification — reverse rendezvous (`fleet_ask`) +**The cap that matters:** a blocking `fleet_send` is bounded by the *caller's own* MCP client +timeout, about 60 seconds — not by the task. Anything slower than that must use the detached flow +in §1.3, or the lead's call returns while the member is still working. -The worker pauses mid-turn to ask; the primary answers; the worker resumes in the same turn. +### 1.2 Clarification — reverse rendezvous + +The member pauses mid-turn to ask, the lead answers, and the member resumes **the same turn** with +its context intact. ```mermaid sequenceDiagram - participant P as Primary + participant P as "Lead" participant B as fleetd - participant W as Worker + participant W as "Member" - P->>B: fleet_send("do X", target=w) — blocks - B-->>W: "do X" (injected) - W->>B: fleet_ask("which config?") — worker blocks - B-->>P: { outcome:"question", text:"which config?", turn_id } - P->>B: fleet_send("config.yaml", target=w, turn_id) — blocks again - B-->>W: resolve fleet_ask → { answer:"config.yaml" } - W->>W: resumes same turn - W->>B: fleet_reply("done") - B-->>P: { outcome:"reply", text:"done" } + P->>B: "fleet_send{sessionId, content} — blocks" + B-->>W: "content injected" + W->>B: "fleet_ask{question} — member blocks" + B-->>P: "{ outcome: question, turnId }" + P->>B: "fleet_send{turnId, content} — answers THIS turn" + B-->>W: "fleet_ask returns the answer" + W->>W: "resumes the same turn" + W->>B: "fleet_reply{content}" + B-->>P: "{ outcome: reply }" ``` -### 6.3 Detached delegation — pane injection +**Answer with `turnId`, never `sessionId`.** A `sessionId` send starts a new turn; it does not +resolve the waiting `fleet_ask`. -The primary does not block; the reply arrives later in its idle pane. +**The window is about 55 seconds and no nudge extends it.** So never brief a member to "ask me": +decide before delegating, or give the member an explicit default to fall back on. + +### 1.3 Detached delegation — the lead does not block + +The lead gets a ticket immediately and collects the answer later. This is the flow for any real +task, because of the ~60s cap in §1.1. ```mermaid sequenceDiagram - participant P as Primary + participant P as "Lead" participant B as fleetd - participant W as Worker + participant W as "Member" - P->>B: fleet_send("do X", target=w, block=false) - B-->>P: { outcome:"dispatched", dispatch_id } - P->>P: continues its own work - W->>B: fleet_reply("result") - Note over B: no waiter → detached path - B->>B: Injector.enqueue(primary_pane, "result") - B-->>P: injected into idle pane (status-gated) + P->>B: "fleet_send{sessionId, content, wait:false}" + B-->>P: "accepted — ticket" + P->>P: "continues its own work" + W->>B: "fleet_reply{content}" + Note over B: "no waiter is blocked — the reply is held" + B->>B: "nudge the lead's own pane (status-gated)" + P->>B: "fleet_poll{ticket}" + B-->>P: "the member's report" + P->>B: "fleet_ack{target, msgId}" ``` -### 6.4 Uncooperative worker — turn-done fallback +A terminal ticket nudges the lead's pane by itself, so a detached task does not need watching. The +nudge needs an injectable lead pane and is capped, so it is a convenience rather than a guarantee. -A worker that never calls `fleet_reply` still returns a result: `fleetd` reads its terminal -tail when the turn completes. +### 1.4 The member never replies — turn-done fallback + +A member that ends its turn without `fleet_reply` still produces something: `fleetd` reads its +pane tail. This is a **fallback, not a channel** — it is lossy in three separate ways, and every +one of them has produced a wrong answer in practice. ```mermaid sequenceDiagram - participant P as Primary + participant P as "Lead" participant B as fleetd - participant W as Worker + participant W as "Member" - P->>B: fleet_send("do X", target=w) — blocks - B-->>W: "do X" (injected) - W->>W: works, never calls fleet_reply - B->>B: StatusPoller sees agent_status → idle/done - B->>B: AgentControl.read(w, "recent") - B-->>P: { outcome:"turn_done", text: } + P->>B: "fleet_send — blocks or detaches" + B-->>W: "content injected" + W->>W: "works, never calls fleet_reply" + B->>B: "StatusPoller sees the turn end" + B->>B: "read the pane tail" + B->>B: "classify: exhausted? echoed brief? real report?" + B-->>P: "{ outcome: turn_done } or a named failure" ``` +The three ways it goes wrong, and what each looks like now: + +| What happened | What the lead used to get | What it gets today | +|---|---|---| +| The report is longer than the scrape window | The **end** silently cut off | Still clipped, but marked partial | +| The member never started — spent credential | The lead's **own brief** echoed back as a report | A named failure: backend exhausted | +| The member is simply slow | A tail of work in progress | Unchanged — read it as a hint, not a result | + +The echoed-brief case is the one to remember: it reads as a long, on-topic report with nothing in +it from the member. It is suppressed now, but the general rule stands — **check the member's +worktree with `git log` before believing a report you did not watch arrive.** + --- -## 7. Status gating +## 2. Status gating -Delivery only happens in a safe window. This is the state machine the `Injector` already -enforces via `AgentStatus.injectable()`; MCP `fleet_send` is simply its producer. +Delivery only happens in a safe window. `fleet_send` is a producer for the `Injector`, which +already enforces this through `AgentStatus.injectable()`. ```mermaid stateDiagram-v2 [*] --> IDLE - IDLE --> WORKING: message delivered / picks up - WORKING --> IDLE: turn done - WORKING --> BLOCKED: awaits input - BLOCKED --> WORKING: input delivered - IDLE --> UNKNOWN: detection glitch - BLOCKED --> UNKNOWN: detection glitch - UNKNOWN --> IDLE: re-detected + IDLE --> WORKING: "message delivered, picked up" + WORKING --> IDLE: "turn done" + WORKING --> BLOCKED: "awaits input" + BLOCKED --> WORKING: "input delivered" + IDLE --> UNKNOWN: "detection glitch" + BLOCKED --> UNKNOWN: "detection glitch" + UNKNOWN --> IDLE: "re-detected" note right of IDLE injectable — deliver head of FIFO @@ -321,69 +172,17 @@ stateDiagram-v2 end note ``` -At most one message is delivered per turn: after a send the `Injector` waits for a `WORKING` -pickup before delivering the next, with a `PICKUP_GRACE_POLLS` fallback for turns faster than -the poll interval. A herdr `events.subscribe` stream can later replace the sampling without -touching this state machine. +**At most one message per turn.** After a send, the `Injector` waits for a `WORKING` pickup before +delivering the next, with a grace-poll fallback for turns that finish faster than the poll +interval. ---- +Two consequences a lead feels directly: -## 8. Error model +- **A second send to a busy member never lands.** It reports as queued and times out. The member + is fine; the message simply waits, and then restarts the member when it next goes idle. +- **A spawned member is not deliverable until it has mounted the MCP.** Until then a send waits on + that gate for about 60 seconds and then fails without ever reaching the pane. -| Condition | `fleet_send` result | Notes | -|---|---|---| -| Worker replies | `{ outcome:"reply" }` | normal | -| Worker asks | `{ outcome:"question", turn_id }` | answer with `fleet_send(turn_id)` | -| Turn ends, no reply | `{ outcome:"turn_done" }` | terminal tail as text | -| Deadline elapsed | `{ outcome:"timeout" }` | message may still be queued/delivered | -| Worker vanished | error `worker_gone` | `Injector.drop` fails the queued future | -| Guard breach on spawn | error `subscription_boundary` | REST `403` parity | -| Delivery failed at herdr | error, message dropped | poisoned message not left blocking the FIFO | - -`fleet_reply` from a worker with no open waiter is **not** an error — it falls through to -detached pane injection (§6.3). - ---- - -## 9. Mapping to existing code - -The MCP face is a thin adapter layer; nearly every capability already exists behind the REST -seam. Only the **rendezvous registry** and the **caller-identity resolver** are new. - -| MCP tool | Existing collaborator | New work | -|---|---|---| -| `fleet_send` | `Injector.enqueue`, `AgentControl.send` | waiter registry, timeout, outcome mux (CB-104) | -| `fleet_reply` / `fleet_ask` | `Injector` (pane injection) | reverse rendezvous, identity resolver | -| `fleet_status` | `AgentControl.status`, `Injector.activeTargets` | pending-drain projection | -| `fleet_spawn` / `list` / `stop` | `WorkerService.{spawn,list,stop}` | MCP adapter only | -| `fleet_read` | `AgentControl.read` | MCP adapter only | - -Because the REST routes in `FleetApp` already exercise the collaborators, MCP tools are -validated by **parity** against those routes, not by re-testing behavior. - ---- - -## 10. Open decisions - -1. **`fleet_ask` direction.** This page defines it as *worker-asks-primary* (a genuine reverse - channel, matching the "inject the primary's pane" language). The alternative — a synonym for - a blocking primary→worker send — is weaker and produces different plumbing. **Recommend - worker-asks-primary.** -2. **Detached delivery shape.** A `block:false` param on `fleet_send` (keeps the catalog - small) vs. a separate `fleet_dispatch` tool. **Recommend the param.** -3. **Auto-spawn on send.** `fleet_send` provisions a worker per profile when none exists - (simplest primary UX) vs. requiring an explicit `fleet_spawn` first. **Recommend - auto-spawn, defaulting on.** -4. **Transport & SDK.** Streamable-HTTP/SSE co-located with the REST bind (recommended) vs. - stdio. Requires choosing a Java MCP server SDK and adding it to the pom. - ---- - -## 11. Implementation staging - -- **CB-104** — blocking `fleet_send` + rendezvous registry + caller-identity resolver - (the producer that finally drives the inert `StatusPoller`). -- **CB-1xx** — `fleet_reply` / `fleet_ask` reverse rendezvous + detached pane injection. -- **CB-1xx** — lifecycle + observability adapters (`fleet_spawn/list/stop/status/read`). -- **CB-1xx** — transport wiring + `claude mcp add` docs; parity tests vs. REST. -- **Later** — `fleet_cancel`; swap `StatusPoller` for herdr `events.subscribe`. +`UNKNOWN` is deliberately neither injectable nor a pickup. A pane whose status cannot be read is +not a pane that is safe to write to — see fleetd #176 for what happens when a gate treats an +unreadable pane as a ready one. diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java index e55ee4b..332e153 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -1315,7 +1315,11 @@ public final class FleetMcp { + "worktree: to provision an isolated git worktree. Pass resumeSessionId " + "to relaunch onto a prior conversation instead of starting cold — this requires an " + "explicit profile whose backend supports it (fleet_list shows agentSessionId for " - + "resumable members), and is refused otherwise rather than silently starting fresh. " + + "resumable members; it is absent for a member fleetd cannot reliably re-identify, " + + "e.g. an opencode member spawned without a worktree), and is refused otherwise " + + "rather than silently starting fresh. For an opencode profile, resumeSessionId " + + "itself also requires worktree:true/ on THIS spawn — without one fleetd can " + + "never re-verify which conversation it actually resumed (fleetd #249). " + "sessionName gives the member a display name in its own UI when the backend supports " + "one. Returns the member's sessionId (use with fleet_send) and paneId (use with " + "fleet_stop).", @@ -1326,7 +1330,7 @@ public final class FleetMcp { "worktree", Map.of("type", "string", "description", "'true' or a ticket slug — requests an isolated git worktree"), "ticket", stringProp("Ticket slug when worktree:true"), "sessionName", stringProp("Logical display name for the member's own session, when its backend supports one"), - "resumeSessionId", stringProp("A prior member's agentSessionId (from fleet_list) to resume — requires an explicit profile that supports it")), + "resumeSessionId", stringProp("A prior member's agentSessionId (from fleet_list) to resume — requires an explicit profile that supports it, and (for opencode) a worktree on this spawn too")), List.of())); } @@ -1347,9 +1351,14 @@ public final class FleetMcp { + "discover a peer lead without being told its address. 'members' are the " + "sessions delegated to — each with sessionId, paneId, role (architect/dev/" + "reviewer), profile (the backend it runs on), state, optional " - + "worktree/branch/owner/agentSessionId (the id to pass as fleet_spawn's " - + "resumeSessionId to relaunch onto that same conversation, when the backend " - + "supports it), and live herdr status. An empty 'members' " + + "worktree/branch/owner/agentSessionId, and live herdr status. agentSessionId, " + + "when present, is the id to pass as fleet_spawn's resumeSessionId to relaunch " + + "onto that same conversation. It is ABSENT — not a guess — for a member fleetd " + + "cannot reliably re-identify: some backends (e.g. opencode) resolve it from the " + + "member's working directory, which only uniquely identifies a member when it " + + "was spawned into its own fleetd-provisioned worktree (worktree:true/); a " + + "member spawned without one shares its directory with others and never reports " + + "an id, however long it runs (fleetd #249). An empty 'members' " + "means no members are spawned; it says nothing about peers. When capacity " + "facts are configured, a 'capacity' row per profile also reports free: 0 for " + "a quarantined profile's credential (see fleet_profiles), whatever its " diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java index 652b28d..01855ae 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java @@ -495,7 +495,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { *

Additive, not a rewrite. {@code .claude.json} is large (tens of KB, dozens of * projects) and Claude Code itself rewrites it while running, so this reads the file as a JSON * tree (missing or unreadable → treated as an empty object) and changes only - * {@code projects..hasTrustDialogAccepted} / {@code .hasCompletedProjectOnboarding} — + * {@code projects..hasTrustDialogAccepted} (that key alone — see fleetd #247) — * every other top-level key and every other project entry is written back untouched. Only the * one project entry for {@code cwd} is replaced/created; an existing entry for a DIFFERENT cwd * (or the operator's own project history) is never touched. @@ -505,7 +505,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { * peer that starts without the seed still starts; it just may hit the dialog fleetd #149 * describes. * - *

Gated to a provisioned worktree ({@link #isProvisionedWorktree}) — see that + *

Gated to a provisioned worktree ({@link HerdrPeerLauncher#isProvisionedWorktree}) — see that * method's javadoc for the incident that made this gate mandatory, not optional: this must * never run against a real checkout or an un-configured fallback cwd, only the exact * always-fresh-directory population fleetd #149 describes. @@ -516,7 +516,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { * sibling-temp-file + {@code ATOMIC_MOVE}, never a truncate-in-place) so a crash mid-write or a * concurrent reader never observes a half-written file, and through {@link #TRUST_JSON_LOCK} so * two concurrent spawns' entries both survive instead of the second write silently discarding - * the first. Both exist because of a real incident: see {@link #isProvisionedWorktree}'s javadoc + * the first. Both exist because of a real incident: see {@link HerdrPeerLauncher#isProvisionedWorktree}'s javadoc * and {@link #writeAtomically}'s javadoc. * * @param configDir the profile's {@code CLAUDE_CONFIG_DIR} ({@code cfg.configDir()}), or @@ -558,8 +558,18 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { if (!(projectNode instanceof ObjectNode)) { projects.set(cwd, project); } + // fleetd #247: ONLY hasTrustDialogAccepted. We used to write + // hasCompletedProjectOnboarding beside it; do not put it back. Measured on + // 2026-09-03, minutes after a live spawn seeded this file: 28 of 28 project + // entries carried hasTrustDialogAccepted and 0 of 28 carried the onboarding key + // — including the 27 entries Claude Code wrote for itself. Claude Code + // normalises the whole file when it saves and drops that key every time, so + // writing it achieved nothing except making the next reader think it mattered. + // The member reached idle with the trust flag alone, which is the only outcome + // this seed exists for. If a future Claude Code needs the second flag the + // symptom returns as the trust dialog fleetd #149 describes — re-measure then, + // do not restore it on a guess. project.put("hasTrustDialogAccepted", true); - project.put("hasCompletedProjectOnboarding", true); writeAtomically(target, TRUST_JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root)); } catch (Exception e) { log.debug("cannot seed workspace-trust entry for cwd '{}' into '{}'", cwd, target, e); @@ -578,7 +588,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { *

fleetd #149 incident. The original implementation used * {@code Files.writeString(target, content)} directly, which truncates {@code target} in place * before writing the replacement bytes. Combined with an ungated {@code cwd} (see - * {@link #isProvisionedWorktree}'s javadoc), a mutation-testing run hit that truncation window + * {@link HerdrPeerLauncher#isProvisionedWorktree}'s javadoc), a mutation-testing run hit that truncation window * against the operator's real {@code ~/.claude.json} and left it at 178 bytes. The gate closes * which file this can ever target; this closes how the target is written, so * that even a legitimate write against a real, live, concurrently-read {@code .claude.json} @@ -628,32 +638,6 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { } } - /** - * Whether {@code cwd} is a fleetd-provisioned git worktree — signalled the same way - * {@link #writeIdeOverlay} already gates on: a {@code .git} that is a regular - * file holding a {@code gitdir:} pointer, as opposed to a real checkout's {@code .git} - * directory. {@code null}/blank never qualifies. - * - *

Shared by every write that must land only in a worktree fleetd itself created for a - * member — never in a real checkout, an arbitrary configured directory, or (see the incident - * below) the daemon's own fallback cwd. - * - *

fleetd #149 incident. {@link #seedTrustDialog} originally ran unconditionally on - * any non-blank {@code cwd}. Most of this launcher's OWN tests spawn a profile with no - * {@code cwd} configured, so the base class's {@code resolveCwd} falls through to the real - * {@code user.dir} — and with no {@code configDir} either (also the common case in this - * file's fixtures), the seed's target falls through the same way to the real - * {@code ~/.claude.json}. Running this repo's own test suite corrupted the operator's actual - * config file (it shrank from ~72 KB to a single seeded entry) the first time a mutation - * happened to make the write non-additive. Gating both cwd-targeted writes on "this is a - * worktree fleetd provisioned" — exactly the population fleetd #149 describes - * ({@code worktree: true} always lands in a brand-new directory) — makes that class of write - * impossible against a real checkout or an untouched fallback cwd, in production or in tests. - */ - private static boolean isProvisionedWorktree(String cwd) { - return cwd != null && !cwd.isBlank() && Files.isRegularFile(Path.of(cwd, ".git")); - } - /** {@code s}, or {@code null} when {@code s} is null/blank — the charter-presence test used above. */ private static String nonBlank(String s) { return (s == null || s.isBlank()) ? null : s; diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java index 75c5ef8..3ae40d7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -358,6 +358,42 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return Files.isRegularFile(candidate) ? candidate : null; } + /** + * Whether {@code cwd} is a fleetd-provisioned git worktree — signalled the same way + * {@code ClaudeCodeLauncher#writeIdeOverlay} already gates on: a {@code .git} that is a + * regular file holding a {@code gitdir:} pointer, as opposed to a real + * checkout's {@code .git} directory. {@code null}/blank never qualifies. + * + *

Shared by every write (and, since fleetd #249, every identity read) that must land only + * in a worktree fleetd itself created for a member — never in a real checkout, an arbitrary + * configured directory, or (see the incident below) the daemon's own fallback cwd. Package- + * private (not {@code protected}) on purpose: {@link ClaudeCodeLauncher} and + * {@link OpenCodeLauncher} both call it, and same-package visibility is enough — no subclass + * outside this package needs it. + * + *

fleetd #149 incident. {@code ClaudeCodeLauncher#seedTrustDialog} originally ran + * unconditionally on any non-blank {@code cwd}. Most of that launcher's OWN tests spawn a + * profile with no {@code cwd} configured, so the base class's {@code resolveCwd} falls + * through to the real {@code user.dir} — and with no {@code configDir} either (also the + * common case in that file's fixtures), the seed's target falls through the same way to the + * real {@code ~/.claude.json}. Running this repo's own test suite corrupted the operator's + * actual config file (it shrank from ~72 KB to a single seeded entry) the first time a + * mutation happened to make the write non-additive. Gating both cwd-targeted writes on "this + * is a worktree fleetd provisioned" — exactly the population fleetd #149 describes + * ({@code worktree: true} always lands in a brand-new directory) — makes that class of write + * impossible against a real checkout or an untouched fallback cwd, in production or in tests. + * + *

fleetd #249. The same reasoning extends to a READ: {@code + * OpenCodeSessionDiscovery#sessionIdForDirectory} keys on {@code directory}, a heuristic that + * is only reliable when the directory is unique to this member — i.e., exactly the population + * this gate identifies. {@link OpenCodeLauncher} uses it to withhold {@code agentSessionId()} + * (report absence rather than a guess) and to refuse a {@code resumeSessionId} spawn that + * cannot be resolved reliably going forward. + */ + static boolean isProvisionedWorktree(String cwd) { + return cwd != null && !cwd.isBlank() && Files.isRegularFile(Path.of(cwd, ".git")); + } + // --- profile surface ----------------------------------------------------------------------- /** The configured peer profile names (what {@code spawn(profile)} accepts). */ diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java index e5aaefc..badcfc6 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -672,13 +672,29 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { /** Add lazy on-disk session discovery to the base handle. */ @Override public PeerHandle spawn(SpawnRequest req) { + String cwd = effectiveCwd(req); + // fleetd #249: refuse rather than silently resume into unverifiable territory. opencode's + // `-s ` flag itself resumes precisely — the resolved id is what fails, not the resume — + // but resolvedSessionId() below can never confirm (or later re-report) this handle's own + // identity without a fleetd-provisioned worktree (isProvisionedWorktree(cwd)), because the + // directory is shared and sessionIdForDirectory's "most recently updated row" heuristic can + // pick a sibling's session. Refusing here, before anything spawns, beats letting the member + // start and only then discovering fleetd can never again verify who it actually is. + if (req.resumeSessionId() != null && !req.resumeSessionId().isBlank() + && !isProvisionedWorktree(cwd)) { + throw new IllegalArgumentException("resumeSessionId requires a fleetd-provisioned " + + "worktree for an opencode profile — without one, this member's cwd is shared " + + "with other sessions, so fleetd can never reliably confirm (now or later) which " + + "conversation it is actually running (fleetd #249). Pass fleet_spawn{worktree:" + + "} to resume this member."); + } PeerHandle inner = super.spawn(req); // fleetd #175: the same profile config buildLaunch resolved for this spawn (requireProfile // is deterministic on req.profileName(), so re-resolving here costs a map lookup, not a // second decision) — SessionAwareHandle needs cfg.model() to know what THIS session should // be running. FleetConfig.Profile cfg = requireProfile(req.profileName()); - return new SessionAwareHandle(inner, discovery, effectiveCwd(req), cfg, + return new SessionAwareHandle(inner, discovery, cwd, cfg, this::memberHerdrSocketConfigured, discoveryUnavailableWarned, exhaustionSink); } @@ -718,6 +734,17 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { * the directory right now." */ private final AtomicReference resolvedSessionId = new AtomicReference<>(); + /** + * fleetd #249: whether {@link #cwd} is a fleetd-provisioned git worktree + * ({@link HerdrPeerLauncher#isProvisionedWorktree}), computed once at spawn time since + * {@code cwd} never changes for this handle. When {@code false} the directory is shared + * with other sessions (the default no-worktree spawn inherits the lead's own cwd), so + * {@link OpenCodeSessionDiscovery#sessionIdForDirectory}'s "most recently updated row for + * this directory" heuristic can and does pick another session's row — see that class's + * javadoc. {@link #agentSessionId()} refuses to guess in that case: it reports absent + * rather than a possibly-foreign id. + */ + private final boolean worktreeProvisioned; SessionAwareHandle(PeerHandle delegate, OpenCodeSessionDiscovery discovery, String cwd, FleetConfig.Profile cfg, @@ -731,6 +758,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { this.discoveryUnavailable = discoveryUnavailable; this.discoveryUnavailableWarned = discoveryUnavailableWarned; this.exhaustionSink = exhaustionSink; + this.worktreeProvisioned = isProvisionedWorktree(cwd); } @Override @@ -760,6 +788,9 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { // built from (see OpenCodeLauncher#defaultDiscoveryRoot's javadoc for the full // reasoning). Scanning fleetd's own $HOME under that config would only ever find "no // row" and read as "resume unsupported" — declare it unavailable instead, once, loudly. + // Checked before the fleetd #249 worktree gate below: this OS-user mismatch makes + // discovery unusable regardless of whether cwd happens to be a provisioned worktree, so + // it earns the one-time WARN either way. if (discoveryUnavailable.getAsBoolean()) { if (discoveryUnavailableWarned.compareAndSet(false, true)) { log.warn("opencode session discovery unavailable: memberHerdrSocket is " @@ -771,6 +802,16 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { } return null; } + // fleetd #249: cwd is shared with other sessions unless fleetd itself provisioned this + // worktree, and sessionIdForDirectory's directory-keyed heuristic cannot tell this + // member's row apart from a sibling's in that case (measured: a three-day-old row from + // a different profile). Refuse to guess — absent is the honest answer, and it is what + // this codebase already returns elsewhere for absent evidence (fleetd #175's UNKNOWN). + // No WARN here: unlike discoveryUnavailable above, this is the ordinary, expected shape + // of the large majority of spawns (no worktree requested), not a configuration gap. + if (!worktreeProvisioned) { + return null; + } // fleetd #234: once resolved, stay resolved. Re-deriving from `directory` on every call // would let this handle's identity drift to a sibling session that later shares the // same cwd and writes a newer row — see resolvedSessionId's javadoc. diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index 2ba37ec..bf07edf 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -6,11 +6,18 @@ import dev.ltms.fleet.peer.MemberRole; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.RecordComponent; +import java.lang.reflect.Type; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; +import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.junit.jupiter.api.Assertions.*; @@ -1366,9 +1373,18 @@ class FleetConfigTest { } /** - * Every optional knob the example documents must bind under the exact spelling used there. - * Keep this list in step with {@code fleetd.example.yaml}: a rename that updates the record - * but not the example (or vice versa) fails here instead of silently no-op'ing in production. + * A spot check that the knobs listed below bind under the exact spelling the example uses — + * it asserts real VALUES arrive in the record, which no name-matching guard can do. + * + *

This is NOT a coverage guard, and must not be read as one (fleetd #113). The list + * inside it is hand-written, so it only ever covers what someone remembered to add. Coverage + * — "is every key the code reads documented, and does every documented key bind?" — comes + * from {@link #everyNestedConfigKeyIsDocumentedInTheExample} and + * {@link #everyLiveKeyInTheExampleBindsToARecordComponent}, both of which derive their key + * set from the record tree and therefore cannot drift. + * + *

Adding a knob here is optional. Leaving one out is not a coverage gap, because the two + * derived guards above already fail on an undocumented or unbindable key. */ @Test void everyOptionalKnobDocumentedInTheExampleBinds(@TempDir Path dir) throws Exception { @@ -1525,6 +1541,230 @@ class FleetConfigTest { return p.matcher(yaml).find(); } + /** + * The nested half of {@link #everyKnownTopLevelKeyIsDocumentedInTheExample} (fleetd #113). + * + *

That guard walks {@link FleetConfig#KNOWN_TOP_LEVEL_KEYS} and anchors its regex at + * column 0, so it sees ONLY top-level keys. Every nested key — {@code profiles..model}, + * {@code health.paneProbeIntervalSeconds} and a hundred others — is outside its scope, and + * neither its name nor its output says so. A green run then reads as "the example documents + * the schema" when most of the schema was never looked at. + * + *

This walks the record tree rather than a name list, so a key added to any nested record + * is covered the moment it compiles, with no edit here. That is the point: a hand-maintained + * second copy of a list always drifts from the thing it mirrors. + * + *

Scope, stated on purpose (fleetd #113 criterion 3 — every check reports what it + * did and did not look at): + *

    + *
  • It checks each key NAME appears somewhere in the example as a YAML key, live or + * commented out. It does NOT check the key sits at the right path.
  • + *
  • It does NOT check a documented key is read by anything. {@code paneProbeIntervalSeconds} + * is parsed into {@link FleetConfig.Health} and used nowhere, and this guard passes it. + * Proving a key is live code needs a call graph, which this is not.
  • + *
+ */ + @Test + void everyNestedConfigKeyIsDocumentedInTheExample() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + assertTrue(Files.exists(example), "fleetd.example.yaml must ship next to the pom"); + String text = Files.readString(example); + + Map pathByName = configKeyPaths(); + + // The denominator. An under-counting walk passes every subset check vacuously, which is + // the exact shape fleetd #113 collects — so the walk has to prove it descended at all. + // The floor is DERIVED, not a literal: the nested walk must find substantially more keys + // than the top-level set the old guard used, or it has not gone below the first level. + int topLevel = FleetConfig.KNOWN_TOP_LEVEL_KEYS.size(); + assertTrue(pathByName.size() > topLevel * 2, + "the record walk found " + pathByName.size() + " config key(s) against " + + topLevel + " top-level key(s) — it has stopped descending into the " + + "nested records, so this guard would pass vacuously. Fix the walk " + + "before trusting a green run."); + + List undocumented = pathByName.entrySet().stream() + .filter(e -> !keyDocumentedAnywhere(text, e.getKey())) + .map(Map.Entry::getValue) + .sorted() + .toList(); + + assertTrue(undocumented.isEmpty(), () -> "checked " + pathByName.size() + + " config key(s) that FleetConfig can bind; " + undocumented.size() + + " appear nowhere in fleetd.example.yaml: " + undocumented + + " — document each one there, commented out if optional. fleetd.yaml is " + + "gitignored, so the example is the only committed description of the schema."); + } + + /** + * The other direction: a LIVE key in the example that {@link FleetConfig} cannot bind. That is + * a key an operator would copy into {@code fleetd.yaml} expecting it to do something, where it + * would be silently ignored. + * + *

Scope, stated on purpose: only live (uncommented) keys are checked. Most of the + * example is commented-out prose, and that prose contains lines like {@code # mode: token} + * that are indistinguishable from keys by text alone. Parsing them would produce false + * failures, so they are deliberately out of scope — and saying so here is the point, rather + * than letting a reader assume the whole file was validated. + */ + @Test + void everyLiveKeyInTheExampleBindsToARecordComponent() throws Exception { + Path example = Path.of("fleetd.example.yaml"); + String text = Files.readString(example); + + List> paths = liveKeyPaths(text); + assertTrue(paths.size() >= 20, + "only " + paths.size() + " live key path(s) were parsed out of the example — the " + + "parser is not seeing the file, so this guard would pass vacuously."); + + List unbindable = paths.stream() + .filter(path -> !pathBinds(path)) + .map(path -> String.join(".", path)) + .distinct() + .sorted() + .toList(); + + assertTrue(unbindable.isEmpty(), () -> "checked " + paths.size() + + " live key path(s) in fleetd.example.yaml; " + unbindable.size() + + " bind to nothing in FleetConfig: " + unbindable + + " — an operator copying one of these into fleetd.yaml gets silence, not an error."); + } + + /** + * Every configuration key {@link FleetConfig} can bind, at every depth, as + * {@code name -> a dotted path to one place it appears}. Derived from the record components, + * so it cannot drift from the code. + */ + private static Map configKeyPaths() { + Map out = new TreeMap<>(); + collectConfigKeys(FleetConfig.class, "", new HashSet<>(), out); + return out; + } + + private static void collectConfigKeys(Class type, String prefix, Set seen, + Map out) { + if (!type.isRecord() || !seen.add(type.getName())) { + return; + } + for (RecordComponent rc : type.getRecordComponents()) { + String path = prefix.isEmpty() ? rc.getName() : prefix + "." + rc.getName(); + out.putIfAbsent(rc.getName(), path); + Class nested = rc.getType(); + if (nested.isRecord()) { + collectConfigKeys(nested, path, seen, out); + } else if (Map.class.isAssignableFrom(nested) || List.class.isAssignableFrom(nested)) { + Class element = elementRecord(rc); + if (element != null) { + String childPrefix = Map.class.isAssignableFrom(nested) + ? path + "." : path + "[]"; + collectConfigKeys(element, childPrefix, seen, out); + } + } + } + } + + /** The record type inside a {@code Map} or {@code List} component, else null. */ + private static Class elementRecord(RecordComponent rc) { + if (rc.getGenericType() instanceof ParameterizedType pt) { + Type[] args = pt.getActualTypeArguments(); + if (args.length > 0 && args[args.length - 1] instanceof Class c && c.isRecord()) { + return c; + } + } + return null; + } + + /** + * True when {@code key} is documented in the example, in either of the two conventions that + * file actually uses: + *

    + *
  1. as a YAML key at any indentation, live or commented out ({@code key:}); or
  2. + *
  3. in a prose block that describes a section's sub-keys, one per line, as + * {@code # key → what it does}.
  4. + *
+ * + *

The second form is not decoration. {@code broker.uri} is documented ONLY that way, on + * purpose: writing it out as a copy-pasteable {@code uri: amqp://user:pass@host} invites an + * operator to paste a password into a file, which is the very thing {@code uriEnv} exists to + * avoid. A guard that demanded the key form would push the file toward doing that. So this + * encodes the convention the example really uses rather than imposing a new one. + */ + private static boolean keyDocumentedAnywhere(String yaml, String key) { + String quoted = Pattern.quote(key); + Pattern asYamlKey = Pattern.compile("(?m)^\\s*(?:#\\s*)?" + quoted + ":"); + Pattern asProseEntry = Pattern.compile("(?m)^\\s*#\\s*" + quoted + "\\s+\u2192"); + return asYamlKey.matcher(yaml).find() || asProseEntry.matcher(yaml).find(); + } + + /** Every live (uncommented) key in {@code yaml}, as a path from the document root. */ + private static List> liveKeyPaths(String yaml) { + Pattern keyLine = Pattern.compile("^(\\s*)([A-Za-z][A-Za-z0-9_]*):(\\s.*)?$"); + List stack = new ArrayList<>(); + List indents = new ArrayList<>(); + List> paths = new ArrayList<>(); + for (String line : yaml.split("\n", -1)) { + if (line.isBlank() || line.stripLeading().startsWith("#")) { + continue; + } + Matcher m = keyLine.matcher(line); + if (!m.matches()) { + continue; + } + int indent = m.group(1).length(); + while (!indents.isEmpty() && indents.get(indents.size() - 1) >= indent) { + indents.remove(indents.size() - 1); + stack.remove(stack.size() - 1); + } + indents.add(indent); + stack.add(m.group(2)); + paths.add(List.copyOf(stack)); + } + return paths; + } + + /** True when a dotted YAML path resolves to something {@link FleetConfig} can bind. */ + private static boolean pathBinds(List path) { + Class type = FleetConfig.class; + boolean nextSegmentIsAFreeFormName = false; + for (int i = 0; i < path.size(); i++) { + if (nextSegmentIsAFreeFormName) { + nextSegmentIsAFreeFormName = false; + continue; + } + RecordComponent rc = componentNamed(type, path.get(i)); + if (rc == null) { + return false; + } + Class t = rc.getType(); + if (t.isRecord()) { + type = t; + } else if (Map.class.isAssignableFrom(t)) { + Class element = elementRecord(rc); + if (element == null) { + return true; // Map: its entries are data, not schema + } + type = element; + nextSegmentIsAFreeFormName = true; + } else { + // A scalar or a list of scalars: nothing may legitimately nest under it. + return i == path.size() - 1; + } + } + return true; + } + + private static RecordComponent componentNamed(Class type, String name) { + if (type == null || !type.isRecord()) { + return null; + } + for (RecordComponent rc : type.getRecordComponents()) { + if (rc.getName().equals(name)) { + return rc; + } + } + return null; + } + @Test void placementDefaultsToFixedForExistingConfigs(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-placement.yaml"); diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java index c82ed4e..cf2b1ea 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/BackendOutageFlowTest.java @@ -2,6 +2,7 @@ package dev.ltms.fleet.inject; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import dev.ltms.fleet.Fleetd; import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.guard.SubscriptionGuard; import dev.ltms.fleet.herdr.AgentControl; @@ -146,40 +147,16 @@ class BackendOutageFlowTest { pushLoop = new ReplyPushLoop(registry, new AgentControl(leadClient), inbox, scheduler, 3, 50); AtomicReference pushLoopRef = new AtomicReference<>(pushLoop); - // --- mirrors Fleetd.main's backendErrorSink lambda EXACTLY: (1) mark BACKEND_ERROR, - // (2) resolve profile/credential via the roster, fail-loud + notify unmapped-target, - // (3) record in BackendOutagePolicy, (4) on a NEW incident, notify the lead. ------------ - BackendErrorSink backendErrorSink = (target, matchedLine, reason) -> { - sessions.onBackendError(target, reason); + // fleetd #248 follow-up: this used to be a 30-line hand-copy of Fleetd.main's + // backendErrorSink lambda, with a comment promising it mirrored production "EXACTLY". + // That promise is exactly the problem: a copy proves the copy. Editing or deleting the + // real sink left this whole flow test green, because it never touched the real sink. + // #248 made Fleetd.backendErrorSink public precisely so a cross-package test could + // drive the real object, so this now calls it. Every assertion below is about + // production code again. + BackendErrorSink backendErrorSink = + Fleetd.backendErrorSink(sessions, () -> profiles, outagePolicy, pushLoopRef::get); - String profileName = sessions.roster().stream() - .filter(session -> target.equals(session.terminalId())) - .findFirst() - .map(MemberSession::profile) - .orElse(null); - FleetConfig.Profile profile = profileName == null ? null : profiles.get(profileName); - if (profile == null) { - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendTargetUnmapped(target, reason); - } - return; - } - String credentialId = profile.effectiveCredentialId(); - Optional incident = outagePolicy.record(credentialId, target, reason); - incident.ifPresent(inc -> { - List affectedProfiles = profiles.values().stream() - .filter(p -> credentialId.equals(p.effectiveCredentialId())) - .map(FleetConfig.Profile::profile) - .sorted() - .toList(); - ReplyPushLoop loop = pushLoopRef.get(); - if (loop != null) { - loop.onBackendIncident(inc.id(), inc.targets(), credentialId, affectedProfiles, - (int) inc.remainingCoolOffSeconds()); - } - }); - }; BackendErrorPatternLookup patterns = target -> Pattern.compile("(?i)503 Service Unavailable"); resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none(), patterns, backendErrorSink); diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/McpContractDocTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/McpContractDocTest.java new file mode 100644 index 0000000..fbcf43e --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/McpContractDocTest.java @@ -0,0 +1,121 @@ +package dev.ltms.fleet.mcp; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #114 (CB-609): the guard that lets {@code docs/MCP-Contract.md} name a tool at all. + * + *

That page was written in July 2026, before any MCP code existed, and then did not follow the + * code. By August it named two tools that had never been built, omitted five that shipped, had the + * wrong name for nearly every parameter, and still described a caller-identity rule that was a + * privilege bug by then. Nothing failed, because nothing checked it — and {@code CLAUDE.md} sends + * every session in the fleet to that page. + * + *

The fix was to delete the tool catalogue rather than correct it: a hand-maintained second copy + * of the tool surface is the defect, not the particular errors it had accumulated. What survives is + * the flows, which are shapes rather than names. But the flows still have to say {@code fleet_send} + * somewhere to be readable, and that is exactly the sentence that rots. This test is what makes it + * safe to write. + * + *

It checks source text, not behaviour. It reads the Markdown and reads {@link FleetMcp}'s + * source, and it only catches a name in the doc that the server does not register. It cannot catch a + * flow that describes the wrong order, or a parameter name in prose — those are not name-shaped. The + * doc's own header carries that caveat for its readers. + */ +class McpContractDocTest { + + /** Tests run with the module directory as cwd, so the repo-root doc is one level up. */ + private static final Path DOC = Path.of("../docs/MCP-Contract.md"); + private static final Path MCP_SOURCE = Path.of("src/main/java/dev/ltms/fleet/mcp/FleetMcp.java"); + + private static Set matches(Path file, String regex) throws Exception { + Matcher m = Pattern.compile(regex).matcher(Files.readString(file)); + Set found = new LinkedHashSet<>(); + while (m.find()) { + found.add(m.group(1)); + } + return found; + } + + /** Every {@code fleet_*} the doc mentions, in prose or in a diagram. */ + private static Set toolsNamedInTheDoc() throws Exception { + return matches(DOC, "(fleet_[a-z_]+)"); + } + + /** Every tool {@link FleetMcp} actually registers, read from its {@code tool("…")} calls. */ + private static Set toolsTheServerRegisters() throws Exception { + return matches(MCP_SOURCE, "tool\\(\"(fleet_[a-z_]+)\""); + } + + @Test + @DisplayName("[SOURCE TEXT] every fleet_* tool named in MCP-Contract.md is one the server registers") + void theDocNamesNoToolThatDoesNotExist() throws Exception { + Set registered = toolsTheServerRegisters(); + Set named = toolsNamedInTheDoc(); + + Set unknown = new LinkedHashSet<>(named); + unknown.removeAll(registered); + + assertTrue(unknown.isEmpty(), + "docs/MCP-Contract.md names " + unknown + ", which FleetMcp does not register. " + + "Checked " + named.size() + " name(s) in the doc against " + registered.size() + + " registered tool(s): " + registered + ". This is the fleetd #114 defect " + + "recurring — the doc named fleet_read and fleet_cancel for weeks after the " + + "code shipped without them. Either fix the name or drop it from the page; do " + + "NOT weaken this test."); + } + + /** + * The denominator guard. The check above passes trivially if the doc stops naming any tool at + * all — an empty set is a subset of everything. A checker that can silently check nothing is the + * fleetd #113 shape, so this pins that the doc really is still describing the flows, and that + * the registration scrape really did find the server's tools. + */ + @Test + @DisplayName("[SOURCE TEXT] the doc/server name check is not vacuous — both sides found names") + void theCheckActuallyHasSomethingToCheck() throws Exception { + Set registered = toolsTheServerRegisters(); + Set named = toolsNamedInTheDoc(); + + assertTrue(registered.size() >= 10, + "scraped only " + registered.size() + " tool registrations from FleetMcp (" + registered + + "); the server registers eleven, so the tool(\"…\") scrape has stopped matching " + + "and the check above is now vacuous"); + assertTrue(named.size() >= 4, + "docs/MCP-Contract.md names only " + named.size() + " fleet_* tool(s) (" + named + "). " + + "The flows describe delegation, clarification, detached delivery and the " + + "turn-done fallback, so it should name several. Too few means the page has been " + + "gutted and this test is guarding nothing."); + } + + /** + * fleetd #114's actual lesson. The catalogue was deleted on purpose; a well-meaning "let me just + * document the tools here" restores the exact second copy that drifted for a month. + */ + @Test + @DisplayName("[SOURCE TEXT] MCP-Contract.md still says it is not the tool reference") + void theDocStillDisclaimsBeingTheToolReference() throws Exception { + String doc = Files.readString(DOC); + assertTrue(doc.contains("**What this page is NOT: a tool reference.**"), + "docs/MCP-Contract.md must keep saying it is not the tool reference. That sentence is " + + "the fix for fleetd #114: the page carried a hand-maintained tool catalogue that " + + "drifted from the code for a month while CLAUDE.md pointed every session at it."); + assertEquals(0, countTables(doc.substring(0, doc.indexOf("## 1. Rendezvous flows"))), + "the header of docs/MCP-Contract.md must not grow a tool/parameter table — that is the " + + "second copy fleetd #114 deleted"); + } + + private static int countTables(String markdown) { + return (int) markdown.lines().filter(l -> l.strip().startsWith("|")).count(); + } +} diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java index ea5f991..d995775 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java @@ -2159,8 +2159,7 @@ class ClaudeCodeLauncherTest { } JsonNode root = new ObjectMapper().readTree(claudeJson.toFile()); JsonNode project = root.path("projects").path(worktree.toString()); - seededBeforeStart.set(project.path("hasTrustDialogAccepted").asBoolean(false) - && project.path("hasCompletedProjectOnboarding").asBoolean(false)); + seededBeforeStart.set(project.path("hasTrustDialogAccepted").asBoolean(false)); } catch (IOException e) { seededBeforeStart.set(false); } @@ -2178,7 +2177,7 @@ class ClaudeCodeLauncherTest { } @Test - void seedTrustDialogWritesBothTrustFlagsForTheResolvedCwd( + void seedTrustDialogWritesOnlyTheTrustFlagAndNotTheOnboardingKey( @TempDir Path configDir, @TempDir Path worktree) throws Exception { markAsProvisionedWorktree(worktree); FakeHerdr herdr = new FakeHerdr(); @@ -2192,7 +2191,13 @@ class ClaudeCodeLauncherTest { JsonNode project = new ObjectMapper().readTree(claudeJson.toFile()) .path("projects").path(worktree.toString()); assertTrue(project.path("hasTrustDialogAccepted").asBoolean(false)); - assertTrue(project.path("hasCompletedProjectOnboarding").asBoolean(false)); + // fleetd #247: the onboarding key must NOT be written. Claude Code strips it on every + // save (measured: 0 of 28 live entries had it, including its own), so writing it only + // adds a contested key to a file two processes share. This assertion is the guard that + // stops it coming back as a plausible-looking "completeness" fix. + assertFalse(project.has("hasCompletedProjectOnboarding"), + "hasCompletedProjectOnboarding must not be written — Claude Code drops it on " + + "every save, and the member reaches idle on hasTrustDialogAccepted alone"); } /** @@ -2238,7 +2243,6 @@ class ClaudeCodeLauncherTest { JsonNode mine = root.path("projects").path(worktree.toString()); assertTrue(mine.path("hasTrustDialogAccepted").asBoolean(false)); - assertTrue(mine.path("hasCompletedProjectOnboarding").asBoolean(false)); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index 6fd791b..d10e98f 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -323,11 +323,39 @@ class OpenCodeLauncherTest { // --- CB-547: resume + post-hoc session discovery -------------------------------------------- + /** + * Give {@code dir} the exact signature {@link HerdrPeerLauncher#isProvisionedWorktree} checks + * for: a {@code .git} REGULAR FILE, never a directory. Content is never parsed by that gate, so + * any {@code gitdir:} pointer is fine. Mirrors {@code ClaudeCodeLauncherTest}'s helper of the + * same shape (fleetd #249). + */ + private static void markAsProvisionedWorktree(Path dir) throws IOException { + Files.writeString(dir.resolve(".git"), "gitdir: /tmp/not-a-real-gitdir"); + } + + /** + * A fresh subdirectory of {@code configRoot}, marked as a provisioned worktree (fleetd #249), + * for tests that predate this gate and stood in a bare {@code "/work/dir"} string as their + * member's cwd — a directory that never existed on disk and, post-#249, would never pass + * {@link HerdrPeerLauncher#isProvisionedWorktree} either. Those tests are about the model + * mismatch / late-resolve machinery (fleetd #175/#234/#209), not about the worktree gate + * itself, so they need a cwd the gate accepts without changing what each test demonstrates. + */ + private static String provisionedWorkDir(Path configRoot) throws IOException { + Path dir = Files.createDirectories(configRoot.resolve("work-dir")); + markAsProvisionedWorktree(dir); + return dir.toString(); + } + @Test - void aResumeSpawnPassesTheSessionIdAsDashS(@TempDir Path root) { + void aResumeSpawnIntoAProvisionedWorktreePassesTheSessionIdAsDashS(@TempDir Path root, + @TempDir Path worktree) + throws Exception { + markAsProvisionedWorktree(worktree); FakeHerdr herdr = new FakeHerdr(); service(herdr, root, opencodeCfg("google/gemini-2.5-pro", null, null)) - .spawn(new SpawnRequest(null, null, null, null, "ses_41b79fc90ffeI9E8uZv6VprUn2")); + .spawn(new SpawnRequest(null, worktree.toString(), null, null, + "ses_41b79fc90ffeI9E8uZv6VprUn2")); List args = startArgs(herdr); int s = args.indexOf("-s"); @@ -336,6 +364,27 @@ class OpenCodeLauncherTest { "the resume target id follows -s"); } + /** + * fleetd #249 acceptance criterion 3: without a fleetd-provisioned worktree, the member's cwd + * is shared with other sessions, so fleetd can never reliably confirm (now or later via {@link + * OpenCodeSessionDiscovery}) which conversation it is actually running. Refuse the spawn itself + * rather than silently launching opencode's {@code -s } into unverifiable territory. + */ + @Test + void aResumeSpawnWithoutAProvisionedWorktreeIsRefused(@TempDir Path root) { + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = service(herdr, root, + opencodeCfg("google/gemini-2.5-pro", null, null)); + + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> + launcher.spawn(new SpawnRequest(null, null, null, null, + "ses_41b79fc90ffeI9E8uZv6VprUn2"))); + + assertTrue(e.getMessage().contains("worktree"), e.getMessage()); + assertFalse(herdr.called("agent.start"), + "the refusal must happen before anything spawns — no pane, no process"); + } + @Test void aFreshSpawnCarriesNoSessionFlag(@TempDir Path root) { FakeHerdr herdr = new FakeHerdr(); @@ -348,25 +397,59 @@ class OpenCodeLauncherTest { @Test void theHandleDiscoversTheSessionIdForTheWorkersCwdOnlyAfterItAppears(@TempDir Path root, - @TempDir Path discRoot) + @TempDir Path discRoot, + @TempDir Path worktree) throws Exception { + markAsProvisionedWorktree(worktree); FakeHerdr herdr = new FakeHerdr(); OpenCodeLauncher launcher = new OpenCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), Map.of("gemini", opencodeCfg(null, null, null)), "gemini", _ -> null, 0, System::currentTimeMillis, () -> { }, root, discRoot); - PeerHandle handle = launcher.spawn(new SpawnRequest(null, "/work/dir", null)); + PeerHandle handle = launcher.spawn(new SpawnRequest(null, worktree.toString(), null)); // opencode writes the record only when the session is first persisted — the instant the // pane is ready it does not exist, so agentSessionId() is null (never a spawn failure). assertNull(handle.agentSessionId(), "no record yet → null, not a spawn-time block"); // Once the record appears (here: same cwd), lazy discovery resolves it — the handle's // session id matches its own worktree, not another's. - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_resolved", "/work/dir", 1000L); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_resolved", worktree.toString(), 1000L); assertEquals("ses_resolved", handle.agentSessionId(), "agentSessionId() re-scans and picks up a record that has since been written"); } + /** + * fleetd #249 acceptance criterion 1, exercised through the real caller path (the handle + * {@code fleet_list} actually reads), not {@link OpenCodeSessionDiscovery} directly. Without a + * fleetd-provisioned worktree the member's cwd is shared — the default no-worktree spawn + * inherits the lead's own long-lived cwd — so even once a matching row appears (here: + * simulating another profile's session that happens to share the directory) the handle must + * report absence rather than guess. Measured real-world case (2026-09-03): the row it would + * otherwise pick was three days old and belonged to a different profile. + */ + @Test + void theHandleNeverReportsAnIdForANonProvisionedCwdEvenAfterARowAppears(@TempDir Path root, + @TempDir Path discRoot) + throws Exception { + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = new OpenCodeLauncher(new AgentControl(herdr), + new WorkspaceControl(herdr), Map.of("gemini", opencodeCfg(null, null, null)), + "gemini", _ -> null, 0, System::currentTimeMillis, () -> { }, root, discRoot); + // No markAsProvisionedWorktree — this cwd has no .git file, the shared-cwd shape a + // no-worktree spawn (or a real checkout) actually has. + String sharedCwd = root.resolve("shared-cwd").toString(); + + PeerHandle handle = launcher.spawn(new SpawnRequest(null, sharedCwd, null)); + + assertNull(handle.agentSessionId(), "no record yet → null, same as the provisioned case"); + // A row for this exact directory now appears — e.g. a sibling member, or a stale session + // from days earlier, sharing the same unprovisioned cwd. + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_someone_elses", sharedCwd, 1000L); + assertNull(handle.agentSessionId(), + "a non-provisioned cwd must NEVER report an id, even once a row for it exists — " + + "the row could belong to any other session sharing this directory"); + } + @Test void foreignWorkerMatchesOpencodePrefixButNotClaude() { String nonce = "abc123"; @@ -920,6 +1003,7 @@ class OpenCodeLauncherTest { @Test void theRealSessionManagerLateResolvePathCatchesAModelMismatch(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); FakeHerdr herdr = new FakeHerdr(); // xf's real shape (fleetd #175): weight:80, model "opencode/nemotron-3-ultra-free", no // credentialId — the profile that actually escaped the fleet's accounting. @@ -929,7 +1013,7 @@ class OpenCodeLauncherTest { OpenCodeLauncher launcher = serviceWithSink(herdr, configRoot, discRoot, cfg, sink); SessionManager sessions = new SessionManager(launcher); - MemberSession acquired = sessions.acquire(cfg.profile(), "/work/dir", null, null); + MemberSession acquired = sessions.acquire(cfg.profile(), workDir, null, null); // Real late-resolve path, driven BEFORE opencode has written its session row — same shape // as production the instant a pane goes ready. @@ -940,7 +1024,7 @@ class OpenCodeLauncherTest { // opencode writes its row late, running gpt-5.6-sol (a PAID credential) instead of the // withdrawn free model the profile actually asked for — the exact fleetd #175 scenario. - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); // Drive the SAME real late-resolve path again: sessions.get() -> resolveAgentSessionId -> @@ -959,12 +1043,13 @@ class OpenCodeLauncherTest { @Test void aProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("openai/gpt-5.6-terra", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-terra\",\"providerID\":\"openai\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -975,12 +1060,13 @@ class OpenCodeLauncherTest { @Test void aGxProviderPrefixedModelMatchingBothIdAndProviderIsNotAMismatch(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("gx/deepseek-v4-flash", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"deepseek-v4-flash\",\"providerID\":\"gx\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -997,12 +1083,13 @@ class OpenCodeLauncherTest { @Test void aMissingProviderIdInTheEvidenceIsUnknownNotAMismatchWhenTheIdMatches( @TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("openai/gpt-5.6-terra", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-terra\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -1018,12 +1105,13 @@ class OpenCodeLauncherTest { @Test void aMissingProviderIdInTheEvidenceStillCatchesARealIdMismatch( @TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("openai/gpt-5.6-terra", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -1042,12 +1130,13 @@ class OpenCodeLauncherTest { @Test void aBareModelWithNoProviderPrefixMatchesOnIdAloneAndIsNotAMismatch(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("deepseek-v4-flash", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"deepseek-v4-flash\",\"providerID\":\"gx\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -1064,6 +1153,7 @@ class OpenCodeLauncherTest { @Test void aRealIdMismatchLogsAnErrorNamingBothModelsAndQuarantinesThroughTheSink( @TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(target + "|" + reason); FleetConfig.Profile cfg = opencodeCfg("opencode/nemotron-3-ultra-free", null, null); @@ -1075,8 +1165,8 @@ class OpenCodeLauncherTest { PeerHandle handle; try { handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); assertEquals("ses_x", handle.agentSessionId()); } finally { @@ -1111,11 +1201,12 @@ class OpenCodeLauncherTest { @Test void unknownOrUnparseableModelEvidenceNeverQuarantines(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("openai/gpt-5.6-terra", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); + .spawn(new SpawnRequest(null, workDir, null)); // No row yet at all. assertNull(handle.agentSessionId()); @@ -1137,12 +1228,13 @@ class OpenCodeLauncherTest { @Test void aProfileWithNoConfiguredModelIsNeverCheckedForAMismatch(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg(null, null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + .spawn(new SpawnRequest(null, workDir, null)); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"anything-at-all\",\"providerID\":\"anyone\"}"); assertEquals("ses_x", handle.agentSessionId()); @@ -1166,21 +1258,22 @@ class OpenCodeLauncherTest { @Test void modelCheckReadsTheResolvedSessionsOwnRowNotWhateverIsNewestInTheSharedDirectory( @TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); List exhausted = new ArrayList<>(); ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason); FleetConfig.Profile cfg = opencodeCfg("openai/gpt-5.6-terra", null, null); PeerHandle handle = serviceWithSink(new FakeHerdr(), configRoot, discRoot, cfg, sink) - .spawn(new SpawnRequest(null, "/work/dir", null)); + .spawn(new SpawnRequest(null, workDir, null)); // Our own session's row, correctly matching the profile's requested model. - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_ours", "/work/dir", 1000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_ours", workDir, 1000L, "{\"id\":\"gpt-5.6-terra\",\"providerID\":\"openai\"}"); assertEquals("ses_ours", handle.agentSessionId(), "resolves to our own session"); assertTrue(exhausted.isEmpty(), "matching model → no mismatch on first resolve: " + exhausted); // A sibling member, spawned later into the SAME shared directory (no worktree, fleetd // #234's default), writes a newer row running a totally different model. - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_sibling", "/work/dir", 9000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_sibling", workDir, 9000L, "{\"id\":\"deepseek-v4-flash\",\"providerID\":\"gx\"}"); assertEquals("ses_ours", handle.agentSessionId(), @@ -1212,6 +1305,7 @@ class OpenCodeLauncherTest { @Test void aSpawnTimeModelMismatchActuallyQuarantinesTheCredentialThroughTheRealAcquirePath( @TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); FleetConfig.Profile cfg = opencodeCfgWithCredential( "terra", "opencode/nemotron-3-ultra-free", "openai-shared"); Map profiles = Map.of(cfg.profile(), cfg); @@ -1232,14 +1326,14 @@ class OpenCodeLauncherTest { // The mismatching row exists BEFORE the spawn — reproducing fleetd #234's exact timing: // opencode's session table already carries evidence by the moment acquire() first asks. - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); assertFalse(quarantine.isQuarantined("openai-shared"), "nothing quarantined before the spawn"); // The real production entrypoint: acquire() builds the MemberSession by calling // handle.agentSessionId() BEFORE registry.put() runs. - MemberSession acquired = sessions.acquire(cfg.profile(), "/work/dir", null, null); + MemberSession acquired = sessions.acquire(cfg.profile(), workDir, null, null); assertEquals("ses_x", acquired.agentSessionId(), "the id itself still resolves correctly"); assertTrue(quarantine.isQuarantined("openai-shared"), @@ -1258,6 +1352,7 @@ class OpenCodeLauncherTest { @Test void aRosterOnlySinkSilentlyDropsTheSpawnTimeQuarantine(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); FleetConfig.Profile cfg = opencodeCfgWithCredential( "terra", "opencode/nemotron-3-ultra-free", "openai-shared"); BackendQuarantine quarantine = new BackendQuarantine(() -> 0L, TimeUnit.SECONDS.toNanos(1800)); @@ -1270,10 +1365,10 @@ class OpenCodeLauncherTest { OpenCodeLauncher launcher = serviceWithSink(herdr, configRoot, discRoot, cfg, rosterOnlySink); SessionManager sessions = new SessionManager(launcher); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); - MemberSession acquired = sessions.acquire(cfg.profile(), "/work/dir", null, null); + MemberSession acquired = sessions.acquire(cfg.profile(), workDir, null, null); assertEquals("ses_x", acquired.agentSessionId(), "the id itself still resolves correctly"); assertFalse(quarantine.isQuarantined("openai-shared"), @@ -1301,6 +1396,7 @@ class OpenCodeLauncherTest { @Test void theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop(@TempDir Path configRoot, @TempDir Path discRoot) throws Exception { + String workDir = provisionedWorkDir(configRoot); FleetConfig.Profile cfg = opencodeCfgWithCredential( "terra", "opencode/nemotron-3-ultra-free", "openai-shared"); Map profiles = Map.of(cfg.profile(), cfg); @@ -1332,12 +1428,12 @@ class OpenCodeLauncherTest { }; exhaustionSinkRef.set(realSink); - OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", "/work/dir", 1000L, + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L, "{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}"); assertFalse(quarantine.isQuarantined("openai-shared"), "nothing quarantined before the spawn"); - MemberSession acquired = sessions.acquire(cfg.profile(), "/work/dir", null, null); + MemberSession acquired = sessions.acquire(cfg.profile(), workDir, null, null); assertEquals("ses_x", acquired.agentSessionId(), "the id itself still resolves correctly"); assertTrue(quarantine.isQuarantined("openai-shared"),