From 17468a234a0cb8d5fe835165a7b119ff6eaee0b1 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 05:54:55 +0200 Subject: [PATCH 01/20] M4: document fleet health design --- docs/M4-Fleet-Health.md | 915 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 915 insertions(+) create mode 100644 docs/M4-Fleet-Health.md diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md new file mode 100644 index 0000000..f6e1827 --- /dev/null +++ b/docs/M4-Fleet-Health.md @@ -0,0 +1,915 @@ +# M4 - Fleet health, recovery, routing, and capacity + +**Status:** Design accepted on 2026-08-15. No M4 implementation exists yet. +**Scope:** Fleet evidence, safe mechanical repair, lead routing, capacity reporting, and optional +human notification. +**Grounded in:** `inject/StatusPoller`, `inject/StatusRefiner`, `inject/CompletionResolver`, +`inject/Injector`, `session/SessionManager`, `msg/MessageService`, `msg/ReplyInbox`, +`msg/ReplyPushLoop`, `msg/LeadHeartbeatLoop`, `mcp/PrimaryRegistry`, and `herdr/AgentControl`. + +## 1. Problem and decision boundary + +The operator asked the bridge to detect idle agents, exceptions, stopped work, and broken +communication. The bridge may read an agent pane from time to time. It must notify a person when +the fleet cannot move forward. + +The four operator terms are not four equal health states. `IDLE` is a normal mode. An exception is +sometimes visible only as pane text. Stopped work may look the same as slow work. Broken +communication can occur on several links. + +M4 uses this boundary: + +- The bridge detects facts and joins evidence. +- The bridge repairs only mechanical failures with no judgement. +- The lead decides whether to stop, retry, replace, or reassign a member. +- A human is notified only when no healthy lead can act. +- n8n may route an outbound incident. It never classifies state or chooses recovery. + +An inbound n8n decider would need bridge authority. No narrow machine-decider role exists. Giving a +workflow engine lead authority is unsafe, while adding a new role is a separate authorization +design. An outbound sink needs no bridge role. + +The bridge must never replay a delivered task. That task may already have changed files, pushed a +branch, opened a pull request, or changed external state. A replay can run those side effects twice. +This rule must remain true even if later code stores delivered prompt text. + +## 2. Evidence model + +A health state is mainly a comparison between two views: + +- **herdr view:** current agents and raw live status from one `AgentControl.list()` call. +- **bridge view:** session FSM, MCP presence, accepted turns, tasks, inbox state, and lead ownership. + +A strong fault often appears as a disagreement between those views. For example, `BUSY` in the +session FSM and `DONE` in herdr means the bridge missed a turn boundary. Pane reads support this +model, but they are not the main monitor. + +`SessionManager.rosterView` already joins session state and live status. `AgentControl.list()` +already gets the whole live fleet in one call. M4 makes that join persistent and adds timers, +accepted-turn state, and incident state. + +### 2.1 Real traces behind the design + +The first trace was an architect that stopped making progress: + +```text +profile=opus role=architect state=busy liveStatus=done +``` + +The session moved from `DONE` to `BUSY` for turn 2. Eighteen minutes later, the session still said +`BUSY`, herdr still said `DONE`, the async task still said `PENDING`, and no completion fallback had +run. This is `TURN_BOUNDARY_LOST`, not a general slow-turn guess. + +The second trace had two async sends to the same pane, one second apart. The pane was then stopped. +One ticket became failed. The other stayed `pending - worker unknown`. Current +`MessageService.abandon` resolves only `Rendezvous.currentWaiter(target)`, while async tasks live in +a separate ticket map. CB-568 is intended to fix that bug. M4 still keeps an independent +post-teardown invariant so a later regression becomes `DELEGATION_ORPHANED`. + +### 2.2 Corrections made during design + +The first state table missed `BUSY` in bridged plus `IDLE` or `DONE` in herdr. It would have found +the real trace only through a late, weak stall timer. The final model adds +`TURN_BOUNDARY_LOST` as a strong disagreement state. + +The first notification design also required a webhook before `health.enabled` could turn on. That +removed useful local detection to avoid a narrower human-notification gap. The final design splits +detection from notification. Missing human escalation is shown as partial coverage instead of +disabling health. + +## 3. Classification precedence + +Evidence is applied in this order. A lower rule cannot hide a higher one. + +1. **Control link:** failed fleet list plus failed ping becomes `CONTROL_LINK_DOWN`. +2. **Definitive target loss:** `_not_found` becomes `GONE` or `LEAD_UNREACHABLE` when the control + link is healthy. +3. **Startup and teardown invariants:** readiness expiry becomes `NEVER_READY`; surviving tasks + after teardown become `DELEGATION_ORPHANED`. +4. **Bridge/live disagreement:** `BUSY` plus stable raw `IDLE` or `DONE` becomes + `TURN_BOUNDARY_LOST`. +5. **Known screen evidence:** a tested fatal signature becomes `ERROR_ON_SCREEN`. +6. **Timed suspicion:** unchanged sparse pane probes may become `STALL_SUSPECTED`. +7. **Communication quality:** completion fallback becomes `MUTE`; an old inbox entry becomes + `REPLY_STRANDED`. +8. **Normal mode:** `STARTING`, `IDLE`, `WORKING`, `WORK_PENDING`, or `BLOCKED_AMBIGUOUS`. + +The member flow in Figure 1 shows lifecycle states and the main fault exits. Fault states are +reported beside the session FSM; most are not new FSM values. + +```mermaid +flowchart TD + Registered["Member registered"] --> Starting["STARTING"] + Starting -->|"MCP presence"| Idle["IDLE"] + Starting -->|"Readiness grace expires"| NeverReady["NEVER_READY"] + Idle -->|"Accepted delivery"| Working["WORKING"] + Working -->|"Trusted turn boundary"| Idle + Working -->|"Bridge BUSY and herdr IDLE or DONE"| Lost["TURN_BOUNDARY_LOST"] + Working -->|"Known fatal screen"| Error["ERROR_ON_SCREEN"] + Working -->|"Long age and unchanged sparse probes"| Stall["STALL_SUSPECTED"] + Working -->|"Target not found"| Gone["GONE"] + Idle -->|"Inbox or queued delivery exists"| Pending["WORK_PENDING"] + Pending -->|"Delivery or collection finishes"| Idle + Idle -->|"Raw BLOCKED with an open turn"| Blocked["BLOCKED_AMBIGUOUS"] + Lost -->|"Strict guarded repair"| Repaired["DONE with reconciled completion"] + Lost -->|"Repair refused"| LeadDecision["Lead decision required"] +``` + +*Figure 1. The member lifecycle and the main health exits. Pane-based states never authorise an +automatic retry of the task.* + +## 4. State model + +### 4.1 Normal and transitional member states + +| State | Exact evidence | Meaning and certainty | +|---|---|---| +| `STARTING` | Session is `SPAWNING`; MCP presence is absent | Normal inside the startup grace. MCP contact is the readiness signal. | +| `IDLE` | Session is `READY` or `DONE`; live status is `IDLE` or `DONE`; no open turn or inbox item exists | Normal. Idle is not a fault. | +| `WORKING` | Session is `BUSY`; raw live status is `WORKING`; the accepted turn is open | Certain that herdr sees work. It does not prove useful progress. | +| `WORK_PENDING` | Queued delivery or inbox content exists while the target is injectable | Transitional. Existing injector or push logic should move it. | +| `BLOCKED_AMBIGUOUS` | An open turn exists and raw live status is `BLOCKED` | The bridge cannot tell whether this is permission, input, or a settled screen. | + +Idle may drive configured resource cleanup. It never opens an incident and never pages a person. + +### 4.2 Member fault and quality states + +| State | Exact evidence | Certainty and action | +|---|---|---| +| `NEVER_READY` | `SPAWNING`, no MCP presence, and an accepted delivery waits through the existing readiness grace | Delivery never became possible. The exact cause is unknown. Fail the send, stop the process, and preserve a provisioned worktree. | +| `GONE` | Per-target herdr call returns `_not_found` while fleet list or ping works | Certain target loss. Fail all target work. Do not replay it. | +| `TURN_BOUNDARY_LOST` | Same session turn stays `BUSY`; same accepted task stays open; two raw snapshots show `IDLE` or `DONE` | Strong disagreement. Strict reconciliation may repair it. | +| `ERROR_ON_SCREEN` | Suspicious non-working state survives grace; `detection` matches a tested adapter-specific fatal signature | Certain only for the matched signature. A bare word such as `Exception` is not enough. | +| `STALL_SUSPECTED` | Open turn is older than the configured threshold; two normalised `recent_unwrapped` digests are unchanged; no boundary or reply occurs | Not certain. A long valid API call can look the same. Lead decides. | +| `MUTE` | Turn resolves through completion fallback instead of `bridge_reply` | Certain that no structured reply won. It does not prove an MCP failure. A single event is a metric, not an incident. | +| `REPLY_STRANDED` | Typed reply or health message remains after owning-lead push reaches its cap | Collection failed. This does not explain whether the lead is busy, dead, or ignoring the nudge. | +| `DELEGATION_ORPHANED` | Target is gone, failed, or released, but one or more tasks remain `PENDING` after reconciliation grace | Certain bridge invariant failure. This is not an inbox-drain fault. | +| `WORK_PRODUCT_AT_RISK` | Provisioned branch has commits after its recorded base; member is `DONE`, `FAILED`, or preserved after release; no turn or inbox item remains; long-idle threshold passed | A warning, not proof of loss. Work may already have an open pull request or a squash merge. | + +`MUTE` opens an incident only after a small fixed rate threshold for one target or profile, or when +it appears with another fault. + +`WORK_PRODUCT_AT_RISK` must not become `WORK_PRODUCT_UNCOLLECTED`. The bridge does not know pull +request or merge state. If committed work appears with `REPLY_STRANDED` or +`DELEGATION_ORPHANED`, the existing incident gains `committedWorkAtRisk: true`. + +### 4.3 Control-link state + +| State | Exact evidence | Certainty and action | +|---|---|---| +| `CONTROL_LINK_DOWN` | Two full-fleet `agent.list` calls fail across the grace, and herdr `ping` also fails | Certain for the bridged-to-herdr link. Retry calls, record the incident, and use human escalation if no lead can be reached. | + +A failed fleet list alone is not a dead-member claim. A single `_not_found` with a healthy global +link is a target fault, not a control-link fault. + +### 4.4 Lead states + +| State | Exact evidence | Meaning and action | +|---|---|---| +| `LEAD_IDLE` | Expected lead is present with raw injectable status; no actionable state waits | Normal. Existing heartbeat may run under its own policy. | +| `LEAD_WORKING` | Expected lead is present with raw `WORKING`; stall threshold is not met | Reachable and busy. Never inject into the live turn. | +| `LEAD_STATUS_UNKNOWN` | Expected lead is present with raw `UNKNOWN` | Neither dead nor a healthy routing target. Retain evidence and retry. | +| `LEAD_UNREACHABLE` | Expected lead is absent from two successful live-agent snapshots while ping works, or targeted lookup returns `_not_found` with a healthy control link | Route to a healthy peer. If none exists, use human escalation. | +| `LEAD_UNRESPONSIVE` | Actionable state waits; lead stays injectable; bounded nudges exhaust; inbox remains uncollected | Route to a healthy peer or a person. | +| `LEAD_STALL_SUSPECTED` | Lead stays `WORKING` past threshold; two sparse pane probes show no progress | Not certain. Never kill or restart automatically. Route to peer or person. | + +The monitor retains the lead name and terminal, last successful sighting, raw status and age, +consecutive list absences, targeted errors, pane-probe facts, pending incident age, and nudge +outcomes. Current heartbeat and push loops discard much of this history. + +Expected lead identity comes from the same supplier used by `CallerResolver`. It is not liveness +evidence. `LeadTabScanner` keeps cached identity after a failed scan, so the health monitor compares +that identity with a fresh successful agent list. A dynamic identity also survives a two-successful- +snapshot retirement grace. This stops a dead lead from escaping health by disappearing from one map. + +### 4.5 Evidence limits + +M4 cannot tell these cases apart with current evidence: + +- A valid long call and a hung call may have the same status and pane digest. +- `BLOCKED` does not explain which input is needed. +- An idle prompt after failure may look like an idle prompt after success. +- A missing structured reply does not prove a broken MCP connection. +- An undrained inbox does not explain why the lead did not collect it. +- Arbitrary pane text cannot safely classify arbitrary exceptions. +- A branch ahead of its base does not prove that work was not collected. + +Logs are outputs, not classifier inputs. The monitor never parses its own logs. + +## 5. Automatic action and lead action + +### 5.1 Actions the bridge may take + +The bridge may: + +- retry transient herdr status, list, ping, and pane-read failures with bounded backoff; +- re-submit Enter after the existing paste/submit race; +- fail queued delivery after `NEVER_READY`; +- stop a never-ready process while preserving its provisioned worktree; +- fail all queued, accepted, and async tasks for a gone or released target; +- reconcile one lost boundary when every strict gate in Section 8 passes; +- hold typed messages, nudge the owning lead, and stop at the configured cap; +- use the existing bounded idle-lead heartbeat; +- deduplicate, route, update, and resolve incidents. + +These actions do not choose new work and do not replay old work. + +### 5.2 Decisions reserved for the lead + +Only the lead may: + +- stop or continue `BLOCKED_AMBIGUOUS`; +- stop, inspect, or wait on `ERROR_ON_SCREEN`; +- kill or continue `STALL_SUSPECTED`; +- spawn a replacement or reassign work; +- retry a delivered task; +- choose how to use partial work in a worktree; +- restart herdr or change network, model, credentials, backend, or configuration. + +Reports include literal safe tool calls such as `bridge_status(sessionId="...")`, +`bridge_poll(ticket="...")`, `bridge_list()`, and optional `bridge_stop(paneId="...")`. A judgement +state never presents stop as the only action. + +### 5.3 Release causes and worktree safety + +| Release cause | Process action | Provisioned worktree | +|---|---|---| +| `SPAWN_ROLLBACK` before registration or delivery | Stop and clean up | Remove | +| `COMPLETED` for `READY` or `DONE` without pending work, idle TTL, or successful context-cap completion | Stop | Remove under completed policy | +| `NEVER_READY` | Stop | Preserve | +| `GONE` | Best-effort stop | Preserve | +| `TURN_FAILED` or lead abort while `BUSY` or `FAILED` | Stop | Preserve | +| `RELEASE_WITH_PENDING_TASKS` | Stop | Preserve | +| `SHUTDOWN` | Stop | Preserve | + +Explicit stop is state-aware. `SPAWNING`, `BUSY`, `FAILED`, or any target with pending tasks uses a +preserving cause. + +Before abnormal release removes the live session, M4 writes an atomic manifest under the worktree +root. It records session identity, owner, role, profile, repository, path, branch, base commit, +release cause, release time, state, and pending task ids. `bridge_list.preservedWorktrees` loads these +manifests after restart. Stop output and WARN logs also name the path and cause. M4 never +auto-deletes a preserved worktree. + +## 6. Fleet health monitor + +Add `FleetHealthMonitor`. Do not widen `StatusPoller` into a policy loop. + +`StatusPoller` has a 250 ms delivery cadence and samples only injector targets with outstanding +work. Health needs all sessions, all leads, task state, inbox age, and global control evidence. One +loop cannot serve both cadences safely. + +Build the monitor like `LeadHeartbeatLoop`: + +- pure `decide(snapshot, priorState, now)` logic; +- a thin scheduler; +- an injected clock; +- edge-triggered state changes; +- no network work in the pure function; +- no sleeping in tests. + +Each enabled fleet tick reads: + +- one `AgentControl.list()` result for the whole fleet; +- one in-memory `SessionManager.roster()` snapshot; +- accepted turns and async task state; +- typed inbox depth, kind, and age; +- push and heartbeat outcomes; +- configured and discovered leads. + +Existing failure paths publish structured evidence to the monitor. The monitor does not infer events +from log text. + +### 6.1 Pane budget + +Healthy idle members, recent working members, and quiet leads cause no pane reads. + +A pane is eligible only for a stable lost boundary, sustained `BLOCKED` or `UNKNOWN`, work older +than the suspect threshold, or one final evidence read for a confirmed fault when the pane exists. + +Compiled brakes apply even if config asks for more: + +- per-target pane cooldown is at least 60 seconds; +- working age before the first progress probe is at least 300 seconds; +- at most two pane reads occur in one fleet tick; +- targets rotate fairly; +- only a normalised digest and optional clipped local excerpt are stored; +- no pane excerpt leaves bridged in a human webhook. + +Use `detection` for tested screen signatures. Use normalised `recent_unwrapped` only for progress +comparison. + +## 7. Typed inbox and routing + +### 7.1 Semantic record + +The typed inbox record carries: + +```text +schemaVersion +kind: reply | health +msgId, target, subjectTerminal, recipientLead +severity, state, evidence +createdAtEpochMillis, firstSeenEpochMillis, lastSeenEpochMillis +recoveryTried, suggestedToolCalls, content +``` + +A health message never calls `Rendezvous.resolve`. It cannot look like the member's task result. + +Both inbox adapters share field preservation, first-id-wins dedup, FIFO among decoded messages, +explicit ownership, ack, and release rules. The in-memory adapter stores typed records directly. It +does not copy AMQP migration logic. + +### 7.2 AMQP migration + +The reader uses AMQP `content_type`, never body sniffing: + +```text +Legacy v0: text/plain +Typed family: application/vnd.ltms.bridged.inbox-message+json +``` + +A legacy reply may begin with `{`. It remains plain text because its media type is `text/plain`. +Legacy text becomes `kind=reply` with exact UTF-8 content and absent typed metadata. + +Typed JSON has required integer `schemaVersion: 1`. Version 1 ignores unknown optional fields. +Missing required fields, invalid enums, malformed UTF-8 or JSON, and property/body identity mismatch +are invalid data. + +An unknown schema version is not partly decoded. It remains unacknowledged on the original queue and +creates one operator-visible `unsupported_version` failure. A newer daemon may read it later. + +Invalid known-format data is copied byte-for-byte to durable queue +`agent..inbox.quarantine`. A dedicated confirm-mode publisher confirms the persistent copy +before the original is acknowledged. A failed quarantine handoff leaves the original unacknowledged. +The raw body never enters logs. + +Decode failure creates a redacted WARN, metric, `bridge_list` summary, and routed health incident. +One bad entry never escapes the consumer callback and never stops later valid messages. + +Safe downgrade is not supported. The previous build ignores `content_type` and would show typed JSON +as ordinary reply text. If drained, it would acknowledge the message and lose typed meaning. Typed +queues must be drained or preserved before an old jar runs. + +The existing contract suite uses RabbitMQ. Production uses LavinMQ. The migration and lead-key +ownership cases must run once against production LavinMQ before release, or the release must state +that LavinMQ was not checked. + +### 7.3 Member routing + +A member incident first goes to the exact lead that owns its accepted delegation. +`PrimaryRegistry` needs a no-fallback `delegatingLeadFor(memberTarget)` query. Health routing must not +use the old singular-primary fallback when several leads exist. + +Publish the incident under the affected member target. Trigger the existing bounded push route. The +push waits until the owning lead is injectable, so it does not interrupt a live lead turn. + +### 7.4 Peer lead routing + +Figure 2 shows the route from incident to lead, peer, or person. + +```mermaid +flowchart TD + Incident["Open incident"] --> Member{"Member incident?"} + Member -->|"yes"| Known{"Exact delegation owner known?"} + Known -->|"no"| Sink{"Human webhook enabled and healthy?"} + Known -->|"yes"| Owner{"Owner lead healthy?"} + Owner -->|"yes"| OwnerInbox["Publish to owner lead path"] + Owner -->|"no"| PeerSet["Build healthy peer candidate set"] + Member -->|"no, lead incident"| PeerSet + PeerSet --> Peer{"Healthy peer exists?"} + Peer -->|"yes"| Select["Choose fewest assigned incidents
then stable name and terminal id"] + Select --> PeerInbox["Publish to peer lead inbox
and status-gated push"] + Peer -->|"no"| Sink + Sink -->|"yes"| Webhook["Send classified outbound incident"] + Sink -->|"no"| Passive["Keep incident open
show partial coverage on local surfaces"] +``` + +*Figure 2. Routing keeps delegation ownership separate from temporary peer fallback.* + +Peer candidates exclude the incident subject, failed owner, absent leads, raw-unknown leads, and +leads with an open unhealthy state. A reachable `WORKING` peer may be selected; its push waits for an +injectable window. + +Choose the candidate with the fewest assigned foreign incidents. Break ties by stable lead name, +then terminal id. Pin the recipient. Reassign only if that peer becomes unhealthy or retires. A +routing generation marks a reassignment, and old pending assignments become superseded. + +`bridge_list` lead rows show health, health age, assigned foreign incident count, and a bounded list +of incident id, subject, state, severity, age, and routing generation. The top-level view also shows +owner, recipient, and routing reason. + +A peer incident is published under the recipient lead's inbox key, not the failed subject's key. Its +status-gated nudge names the failed lead and gives the exact +`bridge_poll(target="")` call. + +### 7.5 Lead inbox ownership + +Add `LeadInboxRegistry`, driven by the same expected-lead supplier as `CallerResolver`. + +It calls `replyInbox.own(leadTerminal)` at startup for configured leads, after successful discovery, +after config adds a lead, and before publication. Ownership is not an authorization side effect. + +A missing lead keeps its key owned. Release happens only after confirmed retirement, all incidents +are reassigned or resolved, typed health messages move or ack, and the queue is empty. Own a +replacement terminal before moving messages from the old key. Never release a non-empty in-memory +lead key, because in-memory release clears local data. + +### 7.6 Single-lead deployment + +One lead and no peer is a normal mode, not an edge case. + +An idle, reachable lead may receive the existing bounded nudge. An unreachable or stalled sole lead +has no safe in-loop recovery. The bridge must not restart or replace it. A new lead would not have the +failed lead's plan or context, and an uncertain relaunch could create two orchestrators. + +With no webhook, only `bridge_list`, `/healthz`, metrics, WARN logs, and the incident journal remain. +These are passive surfaces. They are not a human notification. + +## 8. Lost-boundary reconciliation + +This is the only M4 path that reconstructs a result. It must prefer a visible stall over a fabricated +reply. + +### 8.1 Why normal completion rules are not enough + +Current `CompletionResolver.resolve` has two fail-open rules. It resolves when the delivery baseline +is missing. It also resolves an empty completion when the pane read fails. Those choices are valid +after a trusted `WORKING -> IDLE` boundary because the bridge knows the turn ran. They are unsafe +when health only guesses that a boundary was lost. + +M4 gives each accepted send an internal `TurnToken`. It ties target, exact waiter, session turn, +delivery baseline, and task outcome together. + +### 8.2 Delivery baseline + +Capture the baseline immediately after prompt send and before the delivery future completes. Store: + +```text +TurnToken +exact waiter identity +capture time and pane source +normalised assistant block clipped to MAX_SCRAPE_CHARS +whether a supported assistant marker was recognised +capture result: PRESENT | READ_FAILED | UNRECOGNISED +``` + +A failed or missing baseline never authorises repair. A late baseline is not valid evidence. After a +daemon restart, the old waiter, task, token, and baseline are gone, so the old turn cannot be +repaired. + +Automatic repair is enabled only for agent kinds with tested assistant-block fixtures. Current +extraction is Claude Code-specific and falls back to arbitrary raw text without `⏺`. That raw fallback +cannot authorise repair. OpenCode repair stays disabled until live pane fixtures exist. + +### 8.3 Strict gates and resolver result + +Figure 3 shows the repair gates. Any failed gate keeps the waiter unchanged. + +The two raw snapshots must describe the same `TurnToken` and session turn. No `WORKING`, +`BLOCKED`, `UNKNOWN`, missing-agent, reply, failure, or new-delivery observation may occur between +them. + +```mermaid +flowchart TD + Candidate["TURN_BOUNDARY_LOST candidate"] --> Stable{"Same TurnToken and BUSY turn
across two raw IDLE or DONE snapshots?"} + Stable -->|"no"| Resnapshot["Take a fresh snapshot"] + Stable -->|"yes"| Waiter{"Exact captured waiter
still open by identity?"} + Waiter -->|"no"| Stale["STALE_TURN or ALREADY_RESOLVED"] + Waiter -->|"yes"| Baseline{"Successful recognised
delivery baseline exists?"} + Baseline -->|"no"| Refuse["Refuse repair
leave ticket pending"] + Baseline -->|"yes"| Read{"Fresh pane read succeeds?"} + Read -->|"no"| Refuse + Read -->|"yes"| Output{"Recognised non-blank assistant block
differs from clipped baseline?"} + Output -->|"no"| Refuse + Output -->|"yes"| Resolve["Shared CompletionResolver guard core
resolves exact waiter"] + Resolve -->|"won race"| Repaired["RECONCILED_COMPLETION
same turn becomes DONE"] + Resolve -->|"lost race"| Resnapshot +``` + +*Figure 3. Repair needs stronger evidence than a normal observed turn boundary.* + +Refactor the current resolver into one guard core with two policies: + +```text +resolveCaptured(target, inFlight, OBSERVED_BOUNDARY) +resolveCaptured(target, inFlight, LOST_BOUNDARY_REPAIR) +``` + +The health monitor calls only: + +```text +CompletionResolver.reconcileLostBoundary(target, expectedTurnToken) +``` + +It returns `REPAIRED`, `ALREADY_RESOLVED`, `REFUSED_NO_CAPTURE`, `REFUSED_NO_BASELINE`, +`REFUSED_UNREADABLE`, `REFUSED_UNCHANGED`, `REFUSED_AMBIGUOUS_OUTPUT`, `STALE_TURN`, or +`RACE_LOST`. + +Only `REPAIRED` and same-turn `ALREADY_RESOLVED` may move that turn from `BUSY` to `DONE`. A +per-target reconciliation gate stops a queued second send from being accepted between waiter +resolution and the FSM transition. + +### 8.4 Lead-visible marker and refusal + +A repaired result uses distinct `RECONCILED_COMPLETION` values in `Rendezvous`, `MessageService`, +task poll source, and metrics. The lead sees: + +```text +[repaired completion - bridged detected a lost turn boundary. The member did not call +bridge_reply; pane-derived text follows and may be partial] +``` + +Clipped text also keeps the existing clipped-tail marker. + +A refused repair leaves `TURN_BOUNDARY_LOST` open and the ticket pending. The report states that no +reply was reconstructed and no task was replayed. `UNCHANGED`, `UNREADABLE`, and +`AMBIGUOUS_OUTPUT` get at most one delayed retry for the same token. Missing capture or baseline gets +no retry. After two refused scrapes, automatic repair stops for that token. + +### 8.5 Target-wide teardown invariant + +CB-568 owns the multi-ticket cancellation mechanism. M4 routes every terminal cause through that one +idempotent operation and checks this independent invariant after teardown: + +- no injector entry exists for the target; +- no accepted turn or completion record exists; +- no rendezvous waiter or ask exists; +- every async task is terminal or was already terminal; +- no thread waiting for the target send lock can later accept it; +- new sends fail immediately; +- each old task has one terminal outcome and one metric count. + +A violation becomes `DELEGATION_ORPHANED`. The monitor may call the same idempotent target-wide +failure operation once. It never recreates the task. + +## 9. Capacity and utilisation + +Capacity is a view, not a health state. + +`bridge_list` adds one block per profile: + +```text +profile, maxLoad, live, free, reclaimable +``` + +For an unlimited profile, `maxLoad` and `free` are null. `free` is +`max(0, maxLoad - live)` for a capped profile. + +The view must use the exact live-count function used by placement. A second calculation could show a +free slot that placement then refuses. Member rows add `idleForSeconds` only when state is `READY` or +`DONE`, no accepted turn exists, and the inbox is empty. `reclaimable` means only that the member +holds capacity without open bridge work. + +The existing idle-lead nudge gains a bounded capacity summary. It lists per-profile live, cap, free, +and reclaimable counts, plus at most three long-idle members. Capacity does not make +`FleetState.hasPending()` true. A changed capacity fingerprint may re-arm one capped heartbeat +sequence. The fingerprint excludes changing idle durations, so a static idle fleet cannot reset the +cap forever. Reply-push stand-down remains first. + +The bridge must never: + +- spawn a member because a slot is free; +- generate a task or acceptance criteria; +- move queued work to another member or profile; +- treat a free slot or idle member as an incident; +- stop an idle member only to improve utilisation. + +The bridge knows capacity facts but has no work list. Only the lead has the plan, task context, +side-effect history, and acceptance criteria. + +Capacity calculation is in memory and adds no pane reads. Work-product checks run on a terminal +session edge, not every fleet tick. + +This capacity design adds no automatic stop. The accepted `NEVER_READY` cleanup can still stop a +very slow startup after the existing grace, which is a known risk. Free capacity and long idle time +never trigger that path. + +## 10. Human escalation and notification + +### 10.1 Escalation rule + +Notify a person only when no healthy lead can act: + +- `CONTROL_LINK_DOWN` survives grace; +- a lead is unhealthy and no healthy peer can receive the incident; +- a member incident has no known owning lead; +- the only owning lead becomes unreachable, unresponsive, or stalled; +- incident publication or routing itself fails. + +Do not page a person for a member fault while a healthy owning lead exists. An uncollected member +incident feeds lead-health evidence. If the lead then becomes unhealthy, peer or human routing starts. + +### 10.2 Detection and notification switches + +`health.enabled` controls detection and bridge-local reporting. It does not require a webhook. + +`health.notifications.mode` is `disabled` or `webhook`. Disabled is valid and is the default. +Webhook mode requires a resolved environment variable. Turning notification off stops outbound +attempts but keeps incidents. Turning it back on resumes still-open human incidents. + +Without a sink, `bridge_list.healthCoverage` states that human escalation is unavailable. `/healthz` +keeps its existing HTTP liveness result and adds a nested `fleetHealth.status=partial` component. +Metrics and one startup or reload WARN expose the same limit. + +### 10.3 Incident and delivery deduplication + +One open incident uses this key: + +```text +(scope, subjectStableId, state, causeFingerprint) +``` + +The cause fingerprint includes stable error codes, dependency names, signature ids, or invariant +names. It excludes times, ages, retry counts, pane text, and changing digests. A later recurrence +after resolution gets a new generation and incident id. + +Each outbound event uses: + +```text +Idempotency-Key = hash(incidentId, eventType, eventRevision) +``` + +Event types are `open`, `severity_changed`, `reminder`, and `resolved`. Transport retries keep the +same key. + +An atomic owner-only journal beside the active config stores open incidents, routing, delivered +revisions, retry state, and resolution state. It stores no pane or task content. Journal failure does +not stop detection, but notification coverage becomes degraded. + +### 10.4 Retry, reminder, and resolve + +Send the first event immediately. Retry network errors, timeouts, HTTP 408, HTTP 429, and HTTP 5xx +with full-jitter exponential backoff: + +```text +base: 5 seconds +factor: 3 +maximum delay: 15 minutes +one outstanding attempt per event +``` + +Respect `Retry-After` up to 15 minutes. Other HTTP 4xx responses are permanent for that event until +config changes or a person requests replay. + +Transport retry is not an incident reminder. `humanRepeatSeconds` creates a new reminder revision +for an unresolved critical incident after the last successful human event. Disabled mode does not +build an unbounded reminder queue. + +Send `resolved` only if at least one human event for that incident was delivered. If an incident +resolves before its first successful delivery, cancel the pending open event and record local +resolution. + +### 10.5 Outbound payload boundary + +An outbound payload may contain incident id and event type, severity, state, scope, stable bridge +ids, role or profile, times, duration, structured evidence type and counts, recovery attempted, +routing reason, coverage, and safe tool calls. + +It must never contain: + +- raw pane text, pane excerpts, or pane digests; +- task briefs, prompts, or member reply content; +- source files, diffs, or worktree file content; +- worktree paths; +- environment values, tokens, credentials, headers, or webhook URL; +- raw exception messages or stack traces; +- arbitrary model output. + +The sink response body is ignored. A webhook cannot direct recovery. n8n remains outbound-only. + +### 10.6 Metrics + +M4 adds bounded-label series: + +```text +bridged_health_incidents{scope,state,severity} +bridged_health_incidents_total{event} +bridged_health_notifications_total{event,outcome} +bridged_health_notification_queue_depth +bridged_health_notification_last_success_seconds +bridged_health_notification_capability{mode,status} +bridged_lead_health{lead,state} +bridged_lead_assigned_incidents{lead} +``` + +Metric labels never include terminal ids, incident ids, URLs, or error text. + +## 11. Configuration + +The optional `health:` block is absent or disabled by default. The dormant monitor scheduler does no +herdr or pane work while disabled. Every listed key is hot because the monitor reads `ConfigRef` on +each tick or notification. + +| Key | Class | Default and hard bound | Purpose | +|---|---|---|---| +| `health.enabled` | Hot | `false` | Enable detection and bridge-local reporting. | +| `health.snapshotIntervalSeconds` | Hot | default 30, minimum 15 | Whole-fleet comparison cadence. | +| `health.workingSuspectAfterSeconds` | Hot | default 600, minimum 300 | Age before working-pane probes. | +| `health.paneProbeIntervalSeconds` | Hot | default 60, minimum 60 | Per-target pane cooldown. | +| `health.leadUnresponsiveAfterSeconds` | Hot | default 300, minimum 120 | Delay after exhausted actionable nudges before lead fault. | +| `health.humanRepeatSeconds` | Hot | default 3600, minimum 900 | Minimum repeat period for one open human incident. | +| `health.capacityLongIdleAfterSeconds` | Hot | default 900, minimum 300 | Long-idle threshold for capacity summaries. | +| `health.includePaneExcerpt` | Hot | `false` | Allow a clipped excerpt in local lead reports only. Human payloads still exclude it. | +| `health.notifications.mode` | Hot | `disabled` | Select `disabled` or `webhook`. | +| `health.notifications.webhookUrlEnv` | Hot | required in webhook mode | Name of the environment variable that holds the sink URL. | +| `health.notifications.requestTimeoutMs` | Hot | default 10000, range 1000-30000 | Whole webhook request limit. | + +Two consecutive snapshots are compiled floors for lost boundary, lead disappearance, and control +link failure. The two-pane-reads-per-tick limit is also compiled and cannot be weakened by config. + +## 12. Delivery units and acceptance + +### Unit 1 - Evidence model and fleet snapshot + +Scope: health state model, fleet join, clocks, evidence retention, and pane budget. + +Acceptance criteria: + +1. One `agent.list` call covers one enabled fleet tick. +2. Pure decision tests cover every state and every evidence limit in Section 4. +3. `BUSY` plus stable raw `DONE` opens `TURN_BOUNDARY_LOST` after two snapshots. +4. Healthy fleet snapshots perform zero pane reads. +5. Pane cooldown, two-read fleet budget, and fair rotation cannot be disabled by config. +6. Logs are outputs only; no log parsing exists. +7. Fleet snapshots expose the same profile live-count calculation that placement uses. +8. Capacity rows report cap, live, free, and reclaimable values without opening incidents. + +### Unit 2 - Lost boundary and task reconciliation + +Scope: accepted-turn identity, guarded repair, target-wide teardown, release causes, and preserved +worktree discovery. + +Acceptance criteria: + +1. Every accepted send receives a stable `TurnToken` tied to target, exact waiter, session turn, + delivery baseline, and task outcome. +2. Repair requires the same `BUSY` token, two raw `IDLE` or `DONE` snapshots, no conflicting + observation, exact open waiter, successful baseline, and new recognised assistant output. +3. Missing, failed, late, or post-restart baseline never authorises repair. +4. Repair is enabled only for agent kinds with tested assistant-block extraction. Raw-text fallback + without a recognised marker refuses repair. +5. Normal completion and repair use one resolver guard core. Waiter, scrape, clipping, unchanged, and + exact-turn guards are not duplicated. +6. `reconcileLostBoundary` returns every typed result named in Section 8.3. +7. Only `REPAIRED` and same-turn `ALREADY_RESOLVED` may move the same turn to `DONE`. +8. A per-target reconciliation gate blocks a queued second send during repair and FSM update. +9. Repaired completion has distinct rendezvous kind, message outcome, poll source, lead marker, and + metric. Clipping keeps its extra marker. +10. Unchanged, unreadable, or ambiguous evidence gets at most one delayed retry. Missing capture or + baseline gets none. +11. Refusal leaves the ticket pending and tells the lead that no result was rebuilt or replayed. +12. Release, gone, never-ready, and abnormal stop use CB-568's one idempotent target-wide failure + operation. +13. The post-teardown invariant in Section 8.5 is tested independently of CB-568 internals. +14. A violated teardown invariant creates `DELEGATION_ORPHANED` and retries only the idempotent + failure operation. +15. `SPAWN_ROLLBACK` and normal `COMPLETED` remove worktrees. Abnormal and shutdown causes preserve + them. +16. Explicit stop is state-aware. Any pending task or non-terminal state preserves the worktree. +17. Atomic preserved-worktree manifests reload after restart and appear in lead-only + `bridge_list.preservedWorktrees`. +18. Manifest failure preserves the worktree and opens an operator-visible health failure. +19. Provision records the base commit. Terminal, long-idle worktrees report + `WORK_PRODUCT_AT_RISK` only under the evidence in Section 4.2 and never auto-delete work. +20. No path replays a delivered task, rebuilds its brief, or retargets it, even when prompt text is + available. +21. Tests cover both real traces, all repair refusals, clipping, explicit-reply and next-turn races, + restart without capture, concurrent send and release, and preserved discovery after restart. + +### Unit 3 - Typed inbox and member routing + +Scope: semantic record, AMQP migration, both adapters, member routing, polling, and member health in +`bridge_list`. + +Acceptance criteria: + +1. AMQP selects legacy or typed decoding only from `content_type`; it never sniffs the body. +2. Persistent `text/plain` from the old build becomes `kind=reply` with exact UTF-8 content, + including content beginning with `{`. +3. New entries use the vendor media type, `schemaVersion: 1`, UTF-8, persistent delivery, and AMQP + message ids. +4. Version 1 ignores unknown optional fields but rejects missing fields and identity mismatch. +5. Unknown versions are not decoded or acked. They remain on the original queue and create one + deduplicated failure. +6. Invalid known data never escapes the callback, appears as a reply, or blocks later valid messages. +7. Invalid data reaches durable per-target quarantine before original ack. Failed handoff leaves the + original unacked. +8. Decode failures create redacted WARN, metric, `bridge_list` summary, and routed incident without + raw content. +9. Both adapters pass one semantic contract for fields, FIFO, dedup, ownership, ack, and release. +10. Lead keys require explicit ownership. Publication never claims a queue. +11. Unit codec tests cover legacy `{`, Unicode, malformed UTF-8, typed round trip, additive fields, + malformed JSON, missing fields, identity mismatch, media type, version, and dedup. +12. A live broker contract writes old wire data and reads it with the new adapter after reconnect. +13. Live contract tests cover mixed entries, quarantine confirm-before-ack, unsupported redelivery, + later progress past poison, property persistence, lead ownership, and ack removal. +14. Safe downgrade is documented as unsupported. +15. RabbitMQ contract tests pass with `mvn test -Pcontract`. The same cases run once on production + LavinMQ, or the release states that LavinMQ was not checked. +16. Member incidents route to the exact delegating lead and never resolve a task rendezvous. +17. `bridge_list` shows compact member health and capacity without pane content. Member + `idleForSeconds` is present only when no accepted turn or inbox item exists. + +### Unit 4 - Lead health and peer routing + +Scope: lead evidence, exact ownership, peer selection, explicit-recipient push, and lead inbox +lifecycle. + +Acceptance criteria: + +1. Lead identity uses the `CallerResolver` supplier. Liveness uses successful current agent data. +2. Two successful-list absences with healthy ping become `LEAD_UNREACHABLE`; global link failure does + not. +3. Raw `WORKING`, raw `UNKNOWN`, first-seen time, failures, last success, and error class persist + across ticks. +4. Heartbeat and push publish status and nudge outcomes before safe no-injection decisions. +5. Dynamic lead identity survives a two-successful-snapshot retirement grace. +6. Member incidents first use exact delegation ownership with no singular-primary fallback. +7. Peer selection follows the exclusions, load rule, and stable tie break in Section 7.4. +8. A selected working peer is not interrupted. Its push waits for an injectable window. +9. Recipient assignment stays pinned. Reassignment increments generation and supersedes old pending + assignment. +10. `bridge_list` shows bounded foreign assignments, recipient, reason, and generation without pane + content. +11. `LeadInboxRegistry` owns configured and discovered lead keys before publication. +12. Missing leads keep ownership. Retirement needs an empty queue and handled incidents. +13. Replacement owns the new key before messages move. Non-empty in-memory keys are not released. +14. Tests cover dead versus busy, unknown, global failure, stale scan cache, disappearance, one peer, + several peers, reassignment, and no peer. +15. Adapter tests cover lead ownership, restart re-ownership, retirement, and terminal replacement. + LavinMQ is checked or named as unchecked. +16. A sole unreachable or stalled lead is never restarted or replaced. Without a sink, only passive + evidence remains and every coverage surface says so. + +### Unit 5 - Human sink, hot config, metrics, and operator coverage + +Scope: generic webhook, config split, incident journal, retry, resolve, metrics, example config, and +operator documentation. + +Acceptance criteria: + +1. `health.enabled` works without a human sink. +2. Notification mode is hot, defaults to disabled, and supports disabled or webhook. +3. Webhook mode requires a resolved environment value. Bad notification config does not disable an + already valid detector. +4. Mode changes keep open incidents. Re-enable resumes eligible incidents. +5. `bridge_list`, `/healthz`, metrics, and one WARN show partial coverage without a sink. HTTP + liveness behavior stays unchanged. +6. One-lead, no-sink coverage states that lead failure has no active notification or recovery. +7. Incident and outbound dedupe use the stable keys in Section 10.3. +8. The owner-only local journal survives restart and contains no pane or task content. +9. Journal failure keeps detection running but marks notification coverage degraded. +10. Retry tests cover network failure, timeout, 408, 429, `Retry-After`, 5xx, permanent 4xx, jitter, + delay cap, config re-arm, and one outstanding attempt. +11. Reminders and transport retries remain separate. Disabled mode does not build an unbounded queue. +12. Resolve sends only after an earlier human event succeeded. Resolve-before-delivery cancels stale + open delivery. +13. Metrics use bounded labels and exclude ids, URLs, and error text. +14. Payload tests reject every content type forbidden in Section 10.5. +15. Webhook response bodies are ignored and cannot direct recovery. +16. Tests cover disabled mode, one lead without sink, open/update/reminder/resolve, restart, dedup, + reassignment, disable/re-enable, and sink failure while local health continues. +17. `bridged.example.yaml` documents all hot keys and compiled floors. +18. The operator Features wiki is updated separately. The portable `CLAUDE.md` block is checked and + changed only if shipped tool or inbox semantics make it untrue. +19. `mvn clean install` passes. + +## 13. Not checked and release gates + +These limits are part of the design, not optional follow-up notes. + +- **OpenCode pane status and assistant markers were not checked.** OpenCode lost-boundary repair is + disabled until live fixtures exist. +- **Permission-prompt status was not checked** for Claude Code or OpenCode. `BLOCKED` remains + ambiguous and has no automatic action. +- **`recent_unwrapped` stability was not checked** across all supported agent kinds. If normalisation + is not stable, `STALL_SUSPECTED` must say its evidence is weaker. +- **The real `BUSY + DONE` trace was not replayed against live herdr.** The design uses the observed + production trace and current poller behavior. +- **CB-568 was not present when Unit 2 was designed.** Unit 2 must inspect the landed API and keep its + independent teardown invariant. +- **Production LavinMQ was not checked.** Existing durable-inbox contracts use RabbitMQ. Migration, + quarantine, redelivery, lead ownership, and reassignment must run on LavinMQ before release or be + recorded as unchecked. +- **Live multi-lead routing was not checked.** Peer choice and reassignment are design rules backed by + fake-clock and adapter tests until a live exercise runs. +- **A live sole-lead failure with a webhook was not checked.** The no-peer path is a design result, + not a tested recovery. +- **No n8n, Slack, PagerDuty, or other receiver was checked.** The webhook remains generic and + outbound-only. +- **Deployment supervisor behavior for nested `/healthz` fields was not checked.** HTTP liveness + status stays unchanged to reduce this risk. +- **Incident-journal crash behavior was not checked** because the journal does not exist yet. Unit 5 + must test atomic replacement and restart recovery. +- **Worktree merge state cannot be checked reliably** without forge or explicit collection evidence. + `WORK_PRODUCT_AT_RISK` stays a warning. + +## 14. Locked exclusions + +M4 does not expose `agent.read` as a bridge tool. It does not add a workflow engine, inbound n8n +authority, automatic task assignment, task replay, automatic lead replacement, or automatic member +spawn for free capacity. + +The bridge remains a message bus with evidence and bounded mechanical repair. The lead remains the +place where judgement and work planning happen. From 20e0e68ad7eba3bc4ae14f41767581eaa6682052 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 05:56:00 +0200 Subject: [PATCH 02/20] M4: align design status with CB-573 --- docs/M4-Fleet-Health.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md index f6e1827..11e36c2 100644 --- a/docs/M4-Fleet-Health.md +++ b/docs/M4-Fleet-Health.md @@ -1,11 +1,13 @@ # M4 - Fleet health, recovery, routing, and capacity -**Status:** Design accepted on 2026-08-15. No M4 implementation exists yet. +**Status:** Design accepted on 2026-08-15. CB-573 part 1 has shipped the classification model and +the `bridge_list` capacity view; the remaining M4 units are not yet shipped. **Scope:** Fleet evidence, safe mechanical repair, lead routing, capacity reporting, and optional human notification. -**Grounded in:** `inject/StatusPoller`, `inject/StatusRefiner`, `inject/CompletionResolver`, -`inject/Injector`, `session/SessionManager`, `msg/MessageService`, `msg/ReplyInbox`, -`msg/ReplyPushLoop`, `msg/LeadHeartbeatLoop`, `mcp/PrimaryRegistry`, and `herdr/AgentControl`. +**Grounded in:** `health/FleetHealth`, `health/PaneBudget`, `inject/StatusPoller`, +`inject/StatusRefiner`, `inject/CompletionResolver`, `inject/Injector`, `session/SessionManager`, +`msg/MessageService`, `msg/ReplyInbox`, `msg/ReplyPushLoop`, `msg/LeadHeartbeatLoop`, +`mcp/PrimaryRegistry`, and `herdr/AgentControl`. ## 1. Problem and decision boundary From 16e17b32adb9a9825f6dfb3da6f2f39be10e827c Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:06:07 +0200 Subject: [PATCH 03/20] CB-568: preserve dropped turn causes --- .../main/java/dev/ltms/bridged/Bridged.java | 6 ++++ .../bridged/inject/CompletionResolver.java | 35 +++++++++++++------ .../dev/ltms/bridged/inject/Injector.java | 6 ++-- .../dev/ltms/bridged/inject/TurnListener.java | 8 +++++ .../dev/ltms/bridged/inject/InjectorTest.java | 24 +++++++++++++ .../ltms/bridged/msg/MessageServiceTest.java | 30 ++++++++++++++++ 6 files changed, 95 insertions(+), 14 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 85d057c..f0e2a85 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -285,6 +285,12 @@ public final class Bridged { completion.onTurnFailed(target); sessions.onTurnFailed(target); } + + @Override + public void onTurnFailed(String target, String reason) { + completion.onTurnFailed(target, reason); + sessions.onTurnFailed(target); + } }; Injector injector = new Injector(agents, turnListener, deliverableTo(presence, leads), presence::forget); diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java b/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java index 1d1be03..e504b96 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java @@ -137,7 +137,13 @@ public final class CompletionResolver implements TurnListener { @Override public void onTurnFailed(String target) { InFlight turn = inFlight.get(target); - Thread.ofVirtual().name("turn-failed-" + target).start(() -> fail(target, turn)); + Thread.ofVirtual().name("turn-failed-" + target).start(() -> fail(target, turn, null)); + } + + @Override + public void onTurnFailed(String target, String reason) { + InFlight turn = inFlight.get(target); + Thread.ofVirtual().name("turn-failed-" + target).start(() -> fail(target, turn, reason)); } /** Synchronous resolve (the unit-testable core of {@link #onTurnComplete}). */ @@ -192,6 +198,11 @@ public final class CompletionResolver implements TurnListener { /** Synchronous fail (the unit-testable core of {@link #onTurnFailed}). */ void fail(String target, InFlight turn) { + fail(target, turn, null); + } + + /** Synchronous fail with an optional reason supplied by a dropped worker queue. */ + void fail(String target, InFlight turn, String explicitReason) { // A never-delivered readiness failure has no in-flight record but still has a blocked send; // fall back to the currently-registered waiter (unambiguous — that send never completed, so // no next turn exists to confuse it with). @@ -201,16 +212,18 @@ public final class CompletionResolver implements TurnListener { inFlight.remove(target, turn); // nobody blocked on this worker — nothing to fail return; } - String reason; - try { - reason = clip(agents.read(target, SCRAPE_SOURCE)); - } catch (RuntimeException e) { - reason = ""; - } - if (reason.isBlank()) { - // No screen to scrape — either the worker is stuck (CB-109) or gone (CB-110). - reason = "worker did not reply; its turn ended in an unrecoverable state " - + "(worker unreachable or stuck)"; + String reason = explicitReason; + if (reason == null || reason.isBlank()) { + try { + reason = clip(agents.read(target, SCRAPE_SOURCE)); + } catch (RuntimeException e) { + reason = ""; + } + if (reason.isBlank()) { + // No screen to scrape — either the worker is stuck (CB-109) or gone (CB-110). + reason = "worker did not reply; its turn ended in an unrecoverable state " + + "(worker unreachable or stuck)"; + } } if (rendezvous.resolveFailure(waiter, reason)) { inFlight.remove(target, turn); diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java b/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java index b6848e2..6144c6b 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java @@ -410,8 +410,8 @@ public final class Injector { for (Pending p : pending) { p.delivered().completeExceptionally(cause); } - if (hadDeliveredTurn) { - turnListener.onTurnFailed(target); - } + // A queued send has no in-flight record, while a delivered turn does. CompletionResolver + // handles both forms and resolves its waiter at most once. + turnListener.onTurnFailed(target, cause.getMessage()); } } diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java b/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java index 6ac5062..68e377d 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java @@ -41,6 +41,14 @@ public interface TurnListener { default void onTurnFailed(String target) { } + /** + * As {@link #onTurnFailed(String)}, carrying the reason a worker became unreachable. The default + * keeps existing listeners working while allowing the completion resolver to report a useful cause. + */ + default void onTurnFailed(String target, String reason) { + onTurnFailed(target); + } + /** * A message was just delivered into {@code target}'s pane (CB-115). Fired so the completion * resolver can snapshot the pane's pre-turn content: a later {@link #onTurnComplete} whose diff --git a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java index 990b7a0..61a10b9 100644 --- a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java @@ -259,6 +259,7 @@ class InjectorTest { private static final class Captor implements TurnListener { final List completed = new ArrayList<>(); final List failed = new ArrayList<>(); + final List failureReasons = new ArrayList<>(); @Override public void onTurnComplete(String target) { @@ -269,6 +270,12 @@ class InjectorTest { public void onTurnFailed(String target) { failed.add(target); } + + @Override + public void onTurnFailed(String target, String reason) { + failed.add(target); + failureReasons.add(reason); + } } // ~30s of unknown at the 250ms prod poll interval; enough onStatus samples to trip the stall. @@ -322,6 +329,23 @@ class InjectorTest { assertTrue(f.isCompletedExceptionally(), "queued waiters unblock when the worker vanishes"); } + @Test + void dropPassesTheRealCauseForQueuedAndDeliveredWork() { + Captor cap = new Captor(); + Injector inj = new Injector(new AgentControl(herdr), cap); + CompletableFuture delivered = inj.enqueue(T, "delivered"); + CompletableFuture queued = inj.enqueue(T, "queued"); + + inj.onStatus(T, AgentStatus.IDLE); // deliver the first message + inj.onStatus(T, AgentStatus.WORKING); // its turn is now in flight; one remains queued + inj.drop(T, new HerdrException("agent target sol not found", "agent_not_found", null)); + + assertEquals(List.of(T), cap.failed, "drop signals one turn failure for both affected states"); + assertEquals(List.of("agent target sol not found"), cap.failureReasons); + assertTrue(delivered.isDone(), "the delivered future has already completed"); + assertTrue(queued.isCompletedExceptionally(), "the queued future fails with the drop cause"); + } + @Test void dropFailsTheTurnOfADeliveredMessageWhenTheWorkerVanishes() { // CB-110: the message was delivered (no longer queued), so failing queued waiters alone would diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index 5668890..5743816 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -120,6 +120,36 @@ class MessageServiceTest { assertFalse(reply.completed()); } + @Test + void droppedQueuedAndDeliveredTurnsExposeTheRealCauseExactlyOnce() throws Exception { + CompletableFuture first = sendAsync(); + awaitWaiting(); + injector.onStatus(T, AgentStatus.IDLE); // first delivery + injector.onStatus(T, AgentStatus.WORKING); // first turn in flight + + CompletableFuture queued = injector.enqueue(T, "second task"); + CompletableFuture waiter = rendezvous.currentWaiter(T); + injector.drop(T, new HerdrException("agent target sol not found", "agent_not_found", null)); + + MessageService.Reply reply = first.get(5, TimeUnit.SECONDS); + assertEquals(MessageService.Outcome.WORKER_FAILED, reply.outcome()); + assertEquals("agent target sol not found", reply.text()); + assertTrue(queued.isCompletedExceptionally(), "the queued delivery future also fails"); + assertFalse(rendezvous.resolveFailure(waiter, "second failure"), "the waiter fails exactly once"); + } + + @Test + void noDropReasonKeepsTheExistingFallbackText() throws Exception { + herdr.readText(""); + CompletableFuture waiter = rendezvous.open(T); + + completion.onTurnFailed(T); + + Rendezvous.Resolution resolution = waiter.get(2, TimeUnit.SECONDS); + assertEquals("worker did not reply; its turn ended in an unrecoverable state " + + "(worker unreachable or stuck)", resolution.text()); + } + // --- bridge_ask reverse rendezvous (CB-205) ------------------------------------------------ @Test From f556af5d4e61d5667e01a1bc82ab2b1d76eddf6f Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:10:24 +0200 Subject: [PATCH 04/20] CB-568: fail queued async tickets on teardown --- .../dev/ltms/bridged/msg/MessageService.java | 51 +++++++++++++----- .../ltms/bridged/msg/MessageServiceTest.java | 53 +++++++++++++++++++ 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index 58a0df8..f949539 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -9,6 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -170,8 +171,8 @@ public final class MessageService { private final Metrics metrics; // CB-502: nullable — no registry in unit tests private final ConcurrentHashMap sessionLocks = new ConcurrentHashMap<>(); private final ConcurrentHashMap tasks = new ConcurrentHashMap<>(); - /** The async task that currently owns a target's send lock. */ - private final ConcurrentHashMap asyncTasksByTarget = new ConcurrentHashMap<>(); + /** Async tasks that have accepted delivery for a target. */ + private final ConcurrentHashMap> asyncTasksByTarget = new ConcurrentHashMap<>(); /** Async tickets paused on a specific {@code bridge_ask} turn. */ private final ConcurrentHashMap asyncTasksByTurn = new ConcurrentHashMap<>(); private final AtomicLong ticketSeq = new AtomicLong(); @@ -300,14 +301,14 @@ public final class MessageService { */ public boolean abandon(String target, String reason) { CompletableFuture waiter = rendezvous.currentWaiter(target); - if (waiter == null || waiter.isDone()) { - return false; // nobody is blocked on this worker — nothing to abandon - } - boolean failed = rendezvous.resolveFailure(waiter, reason); + boolean failed = waiter != null && !waiter.isDone() && rendezvous.resolveFailure(waiter, reason); + boolean asyncFailed = tasks.values().stream() + .filter(task -> target.equals(task.target) && task.question == null) + .anyMatch(task -> task.future.complete(new Reply(Outcome.WORKER_FAILED, reason))); if (failed) { log.warn("abandoning the blocked send to {}: {}", target, reason); } - return failed; + return failed || asyncFailed; } /** @@ -354,6 +355,11 @@ public final class MessageService { * never earned. {@code null} disables the hook. */ public Reply send(String target, String content, long timeoutMillis, Runnable onAccepted) { + return send(target, content, timeoutMillis, onAccepted, null); + } + + /** Run a send, optionally stopping an async task that teardown already failed before acceptance. */ + private Reply send(String target, String content, long timeoutMillis, Runnable onAccepted, Task task) { long deadlineNanos = System.nanoTime() + timeoutMillis * 1_000_000L; ReentrantLock lock = sessionLocks.computeIfAbsent(target, _ -> new ReentrantLock()); @@ -361,6 +367,9 @@ public final class MessageService { return new Reply(Outcome.BUSY, null); // another send held the session the whole window } try { + if (task != null && task.future.isDone()) { + return task.future.getNow(null); + } if (hasAsyncQuestion(target)) { return new Reply(Outcome.BUSY, null); // the worker's current turn is paused for its lead } @@ -527,17 +536,18 @@ public final class MessageService { if (onAccepted != null) { onAccepted.run(); } - asyncTasksByTarget.put(target, task); + asyncTasksByTarget.computeIfAbsent(target, _ -> ConcurrentHashMap.newKeySet()).add(task); }; - Reply result = send(target, content, ASYNC_TIMEOUT_MS, trackingAccepted); + Reply result = send(target, content, ASYNC_TIMEOUT_MS, trackingAccepted, task); if (result.outcome() == Outcome.QUESTION) { - asyncTasksByTarget.remove(target, task); + // Keep the accepted owner until answer() finishes it. markAsyncQuestion may run + // just after resolveQuestion wakes this thread. } else { finishAsyncTask(task, result); } } catch (Throwable t) { task.future.completeExceptionally(t); - asyncTasksByTarget.remove(target, task); + untrackAsyncTarget(task); } }); pruneTerminalTickets(); @@ -600,7 +610,11 @@ public final class MessageService { /** Record the active question for an async ticket; blocking sends have no entry and stay unchanged. */ private void markAsyncQuestion(String target, String text, String turnId) { - Task task = asyncTasksByTarget.get(target); + Set targetTasks = asyncTasksByTarget.get(target); + Task task = targetTasks == null ? null : targetTasks.stream() + .filter(candidate -> !candidate.future.isDone()) + .findFirst() + .orElse(null); if (task != null) { task.question = new Reply(Outcome.QUESTION, text, turnId); task.turnId = turnId; @@ -623,7 +637,7 @@ public final class MessageService { /** Complete and detach an async ticket after its worker's actual terminal reply. */ private void finishAsyncTask(Task task, Reply result) { task.future.complete(result); - asyncTasksByTarget.remove(task.target, task); + untrackAsyncTarget(task); if (task.turnId != null) { asyncTasksByTurn.remove(task.turnId, task); } @@ -642,6 +656,17 @@ public final class MessageService { return asyncTasksByTurn.values().stream().anyMatch(task -> target.equals(task.target)); } + /** Stop tracking a task once it no longer owns an accepted target turn. */ + private void untrackAsyncTarget(Task task) { + Set targetTasks = asyncTasksByTarget.get(task.target); + if (targetTasks != null) { + targetTasks.remove(task); + if (targetTasks.isEmpty()) { + asyncTasksByTarget.remove(task.target, targetTasks); + } + } + } + /** Release the async executor. */ public void close() { asyncExecutor.shutdown(); diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index 5743816..7072a7d 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -639,4 +639,57 @@ class MessageServiceTest { assertTrue(view.detail() != null && view.detail().contains("released"), "and the detail says why, rather than 'worker unknown'"); } + + @Test + void abandonFailsEveryPendingAsyncTicketForTheReleasedTarget() throws Exception { + String first = messages.sendAsync(T, "first task"); + awaitWaiting(); // first task owns the target lock and rendezvous waiter + String second = messages.sendAsync(T, "second task"); // parked on the same lock, not yet queued + + assertTrue(messages.abandon(T, "agent target term_a not found")); + + assertFailedTicket(first, "agent target term_a not found"); + assertFailedTicket(second, "agent target term_a not found"); + } + + @Test + void abandonDoesNotFailAnAsyncTicketWaitingForAnAnswer() throws Exception { + String ticket = messages.sendAsync(T, "task that asks"); + awaitWaiting(); + injectDelivery(); + + CompletableFuture ask = + CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000)); + MessageService.TaskView asking = awaitTicketPhase(ticket, MessageService.Phase.ASKING); + + assertFalse(messages.abandon(T, "agent target term_a not found"), + "an asking ticket is an active turn, not a pending send to sweep"); + assertEquals(MessageService.Phase.ASKING, messages.poll(ticket).phase()); + + CompletableFuture answer = CompletableFuture.supplyAsync( + () -> messages.answer(asking.turnId(), "config.yaml", 5000)); + assertEquals("config.yaml", ask.get(5, TimeUnit.SECONDS).answer()); + awaitWaiting(); + assertTrue(rendezvous.resolve(T, "done")); + assertEquals(MessageService.Outcome.REPLIED, answer.get(5, TimeUnit.SECONDS).outcome()); + } + + private void assertFailedTicket(String ticket, String reason) throws Exception { + MessageService.TaskView view = awaitTicketPhase(ticket, MessageService.Phase.FAILED); + assertEquals(reason, view.detail()); + } + + private MessageService.TaskView awaitTicketPhase(String ticket, MessageService.Phase phase) throws Exception { + long deadline = System.currentTimeMillis() + 3000; + MessageService.TaskView view; + do { + view = messages.poll(ticket); + if (view.phase() == phase) { + return view; + } + Thread.sleep(5); + } while (System.currentTimeMillis() < deadline); + assertEquals(phase, view.phase()); + return view; + } } From 75f57cdba7b5421a429fb37e7a9fb16a6507f1b7 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:15:21 +0200 Subject: [PATCH 05/20] CB-568c: fail every queued async ticket --- .../main/java/dev/ltms/bridged/msg/MessageService.java | 10 +++++++--- .../java/dev/ltms/bridged/msg/MessageServiceTest.java | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index f949539..c50675e 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -302,9 +302,13 @@ public final class MessageService { public boolean abandon(String target, String reason) { CompletableFuture waiter = rendezvous.currentWaiter(target); boolean failed = waiter != null && !waiter.isDone() && rendezvous.resolveFailure(waiter, reason); - boolean asyncFailed = tasks.values().stream() - .filter(task -> target.equals(task.target) && task.question == null) - .anyMatch(task -> task.future.complete(new Reply(Outcome.WORKER_FAILED, reason))); + boolean asyncFailed = false; + for (Task task : tasks.values()) { + if (target.equals(task.target) && task.question == null + && task.future.complete(new Reply(Outcome.WORKER_FAILED, reason))) { + asyncFailed = true; + } + } if (failed) { log.warn("abandoning the blocked send to {}: {}", target, reason); } diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index 7072a7d..ecb1b5d 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -645,11 +645,13 @@ class MessageServiceTest { String first = messages.sendAsync(T, "first task"); awaitWaiting(); // first task owns the target lock and rendezvous waiter String second = messages.sendAsync(T, "second task"); // parked on the same lock, not yet queued + String third = messages.sendAsync(T, "third task"); // a second queued ticket proves the full sweep assertTrue(messages.abandon(T, "agent target term_a not found")); assertFailedTicket(first, "agent target term_a not found"); assertFailedTicket(second, "agent target term_a not found"); + assertFailedTicket(third, "agent target term_a not found"); } @Test From 826fffe05b61593e87cf92677a974301f80d9b40 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:10:29 +0200 Subject: [PATCH 06/20] CB-573: add dormant fleet health monitor --- bridged/bridged.example.yaml | 10 +++ .../main/java/dev/ltms/bridged/Bridged.java | 29 +++++++- .../ltms/bridged/config/BridgedConfig.java | 27 ++++++- .../bridged/health/FleetHealthMonitor.java | 70 +++++++++++++++++++ .../java/dev/ltms/bridged/mcp/BridgeMcp.java | 14 ++-- .../health/FleetHealthMonitorTest.java | 41 +++++++++++ .../ltms/bridged/mcp/BridgeMcpAuthzTest.java | 2 +- .../dev/ltms/bridged/mcp/BridgeMcpTest.java | 6 +- 8 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java create mode 100644 bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 6b68030..6bd6c0f 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -85,6 +85,16 @@ bind: # backoffMs: 60000 # quietNudgeCap: 3 +# Fleet health detection is dormant unless enabled. It reads one whole-fleet agent list per tick. +# It can run without a webhook; bridge_list then reports healthCoverage: detection-only. +# health: +# enabled: true +# intervalSeconds: 30 # minimum 15 +# workingSuspectAfterSeconds: 600 # minimum 300 +# paneProbeIntervalSeconds: 60 # minimum 60 +# notifications: +# mode: disabled # disabled (default) or webhook + # herdr Unix socket. Omit to use the client default # (${HERDR_SOCKET_PATH:-~/.config/herdr/herdr.sock}). herdrSocket: ~/.config/herdr/herdr.sock diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index f0e2a85..fbae9b7 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -23,6 +23,7 @@ import dev.ltms.bridged.mcp.BridgeMcp; import dev.ltms.bridged.mcp.ConnectionIdentity; import dev.ltms.bridged.metrics.BridgedMetrics; import dev.ltms.bridged.metrics.Metrics; +import dev.ltms.bridged.health.FleetHealthMonitor; import dev.ltms.bridged.mcp.PrimaryRegistry; import dev.ltms.bridged.mcp.LsofPeerPidLookup; import dev.ltms.bridged.mcp.LsofProcessCwdLookup; @@ -354,6 +355,26 @@ public final class Bridged { MessageService messages = new MessageService(agents, injector, rendezvous, replyInbox, pushLoop, metrics); + // Health is a slow whole-fleet observer. Keep it separate from the 250ms delivery poller. + final FleetHealthMonitor healthMonitor; + var healthScheduler = Executors.newSingleThreadScheduledExecutor(r -> + Thread.ofVirtual().name("bridge-health-").unstarted(r)); + if (cfg.health() != null && cfg.health().isEnabled()) { + healthMonitor = new FleetHealthMonitor(agents, sessions::roster, messages, healthScheduler, + System::nanoTime, cfg.health().intervalOrDefault()); + String coverage = FleetHealthMonitor.coverage(true, + cfg.health().notifications() != null && cfg.health().notifications().configured()); + if ("detection-only".equals(coverage)) { + log.warn("fleet health: {} (no notification sink configured)", coverage); + } else { + log.info("fleet health: {}", coverage); + } + healthMonitor.start(); + } else { + healthMonitor = null; + healthScheduler.shutdownNow(); + } + // CB-520: the reply inbox only consumes for agents this gateway owns. own on acquire, // release on teardown. Do this before CB-516 so the inbox is owned before any reply can land. sessions.onAcquire(replyInbox::own); @@ -393,7 +414,12 @@ public final class Bridged { profile -> { var configured = config.get().profiles().get(profile); return configured == null ? null : configured.maxLoad(); - }, () -> config.get().profiles().keySet(), System::nanoTime)); + }, () -> config.get().profiles().keySet(), System::nanoTime), + new BridgeMcp.HealthCoverageSource(() -> { + var health = config.get().health(); + return FleetHealthMonitor.coverage(health != null && health.isEnabled(), + health != null && health.notifications() != null && health.notifications().configured()); + })); // CB-559: opt-in config reload. With no `configReload:` block nothing is constructed, so an // upgraded daemon behaves exactly as before — the file is read once at boot and never again. @@ -414,6 +440,7 @@ public final class Bridged { messages.close(); pushLoop.close(); if (heartbeat != null) heartbeat.close(); // CB-551: stop the idle-lead heartbeat scheduler + if (healthMonitor != null) healthMonitor.stop(); if (configWatcher != null) configWatcher.stop(); // CB-559: stop polling the config file mcp.close(); if (reaper != null) reaper.stop(); diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index a8f7086..d072ff0 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -73,6 +73,7 @@ public record BridgedConfig( Primary primary, Fleet fleet, LeadHeartbeat leadHeartbeat, + Health health, String placement, Auth auth, ConfigReload configReload) { @@ -83,7 +84,16 @@ public record BridgedConfig( Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet, LeadHeartbeat leadHeartbeat, String placement, Auth auth) { this(bind, herdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, - spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, placement, auth, null); + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, null, placement, auth, null); + } + + /** Back-compat form before the optional {@code health:} block was added. */ + public BridgedConfig(Bind bind, String herdrSocket, Map profiles, Guard guard, + String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs, + Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet, + LeadHeartbeat leadHeartbeat, String placement, Auth auth, ConfigReload configReload) { + this(bind, herdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, null, placement, auth, configReload); } /** @@ -377,6 +387,17 @@ public record BridgedConfig( boolean clearAfterTurn) { } + /** Optional fleet detection. A missing block stays dormant. */ + @JsonIgnoreProperties(ignoreUnknown = true) + public record Health(Boolean enabled, Integer intervalSeconds, Integer workingSuspectAfterSeconds, + Integer paneProbeIntervalSeconds, Notifications notifications) { + public boolean isEnabled() { return Boolean.TRUE.equals(enabled); } + public int intervalOrDefault() { return Math.max(15, intervalSeconds == null ? 30 : intervalSeconds); } + public record Notifications(String mode) { + public boolean configured() { return "webhook".equalsIgnoreCase(mode); } + } + } + /** * External AMQP broker for durable, cross-restart reply delivery (CB-307 Stage 2). Its mere * presence swaps the in-memory {@code ReplyInbox} for the AMQP-backed adapter; absent, bridged @@ -812,7 +833,7 @@ public record BridgedConfig( private static final Set KNOWN_TOP_LEVEL_KEYS = Set.of( "bind", "herdrSocket", "profiles", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", - "leadHeartbeat", "placement", "auth", "configReload"); + "leadHeartbeat", "health", "placement", "auth", "configReload"); /** Load and validate config from {@code path}. */ public static BridgedConfig load(Path path) { @@ -1082,7 +1103,7 @@ public record BridgedConfig( // defaults the fields of a block that IS present. Defaulting it here would start watching // the file for every config that never asked to be watched. return new BridgedConfig(b, herdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs, - broker, primary, f, leadHeartbeat, placementOrDefault, a, configReload); + broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload); } /** diff --git a/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java new file mode 100644 index 0000000..c0d808c --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java @@ -0,0 +1,70 @@ +package dev.ltms.bridged.health; + +import dev.ltms.bridged.herdr.Agent; +import dev.ltms.bridged.herdr.AgentControl; +import dev.ltms.bridged.herdr.AgentStatus; +import dev.ltms.bridged.msg.MessageService; +import dev.ltms.bridged.session.MemberSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.LongSupplier; +import java.util.function.Supplier; + +/** Slow whole-fleet evidence collection. It is deliberately separate from the delivery poller. */ +public final class FleetHealthMonitor { + private static final Logger log = LoggerFactory.getLogger(FleetHealthMonitor.class); + private final AgentControl agents; + private final Supplier> roster; + private final MessageService messages; + private final ScheduledExecutorService scheduler; + private final LongSupplier clock; + private final long intervalSeconds; + private final Map priors = new HashMap<>(); + + public FleetHealthMonitor(AgentControl agents, Supplier> roster, MessageService messages, + ScheduledExecutorService scheduler, LongSupplier clock, long intervalSeconds) { + this.agents = agents; + this.roster = roster; + this.messages = messages; + this.scheduler = scheduler; + this.clock = clock; + this.intervalSeconds = intervalSeconds; + } + + /** Pure per-member decision seam. */ + static HealthDecision decide(HealthSnapshot snapshot, HealthPrior prior, long nowNanos) { + return FleetHealth.decide(snapshot, prior, nowNanos); + } + + public void start() { scheduler.schedule(this::tick, intervalSeconds, TimeUnit.SECONDS); } + public void stop() { scheduler.shutdownNow(); } + + // Package-private so tests can run one tick without waiting. + void tick() { + List agentsNow = agents.list(); // Exactly one list call for this complete observation. + Map live = new HashMap<>(); + for (Agent agent : agentsNow) live.put(agent.terminalId(), agent); + for (MemberSession session : roster.get()) { // One in-memory roster snapshot for the same tick. + Agent agent = live.get(session.terminalId()); + AgentStatus status = agent == null ? AgentStatus.UNKNOWN : agent.status(); + boolean accepted = messages.hasAcceptedDelivery(session.terminalId()); + HealthSnapshot snapshot = new HealthSnapshot(session.state(), status, accepted, false, + messages.hasInboxMessage(session.terminalId()), agent != null, false, false, + false, false, false, false); + HealthDecision decision = decide(snapshot, priors.getOrDefault(session.terminalId(), HealthPrior.NONE), + clock.getAsLong()); + priors.put(session.terminalId(), decision.prior()); + } + scheduler.schedule(this::tick, intervalSeconds, TimeUnit.SECONDS); + } + + public static String coverage(boolean enabled, boolean notificationConfigured) { + return !enabled ? "off" : notificationConfigured ? "full" : "detection-only"; + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java b/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java index a480aff..af11447 100644 --- a/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java +++ b/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java @@ -77,6 +77,7 @@ public final class BridgeMcp { private final CallerResolver authz; // CB-501: null → authorization not enforced (legacy) private final Metrics metrics; // CB-502: null → auth failures not counted private final CapacitySource capacity; + private final HealthCoverageSource healthCoverage; /** Capacity facts used by {@code bridge_list}; production must supply the placement live count. */ public record CapacitySource(Function liveCount, Function maxLoad, @@ -86,6 +87,9 @@ public final class BridgeMcp { boolean available() { return !configuredProfiles.get().isEmpty(); } } + /** Coverage is supplied by the health wiring, not inferred from a missing dependency. */ + public record HealthCoverageSource(Supplier value) { } + /** * @param callers resolves each call's {@link Principal}; {@code null} disables authorization. * This surface needs its own enforcement: {@code /mcp} is a raw servlet on @@ -95,8 +99,9 @@ public final class BridgeMcp { */ public BridgeMcp(MessageService messages, PeerLauncher workers, SessionManager sessions, ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry, - CallerResolver callers, Metrics metrics, CapacitySource capacity) { + CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage) { this.capacity = capacity; + this.healthCoverage = healthCoverage; McpJsonMapper json = new JacksonMcpJsonMapperSupplier().get(); this.transport = HttpServletStreamableServerTransportProvider.builder() .jsonMapper(json) @@ -210,7 +215,7 @@ public final class BridgeMcp { .toolCall(listTool(), (exchange, _) -> { McpSchema.CallToolResult denied = deny(exchange, Authz.Action.READ, null); if (denied != null) return denied; - return listFleet(workers, sessions, messages, capacity, + return listFleet(workers, sessions, messages, capacity, healthCoverage, callers == null ? Map.of() : callers.leads(), callerTerminal(exchange)); }) @@ -724,11 +729,11 @@ public final class BridgeMcp { */ static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, Map leads, String selfTerm) { - return listFleet(workers, sessions, null, CapacitySource.none(), leads, selfTerm); + return listFleet(workers, sessions, null, CapacitySource.none(), new HealthCoverageSource(() -> "off"), leads, selfTerm); } static McpSchema.CallToolResult listFleet(PeerLauncher workers, SessionManager sessions, MessageService messages, - CapacitySource capacity, + CapacitySource capacity, HealthCoverageSource healthCoverage, Map leads, String selfTerm) { try { Map live = workers.list().stream() @@ -747,6 +752,7 @@ public final class BridgeMcp { roster.stream().map(MemberSession::profile).forEach(profiles::add); Map result = new LinkedHashMap<>(); result.put("leads", leadRows); result.put("members", out); + result.put("healthCoverage", healthCoverage.value().get()); if (capacity.available()) result.put("capacity", profiles.stream() .map(profile -> capacityView(profile, capacity.liveCount(), capacity.maxLoad(), roster, messages, capacity.clock().getAsLong())).toList()); diff --git a/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java new file mode 100644 index 0000000..e133ad5 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java @@ -0,0 +1,41 @@ +package dev.ltms.bridged.health; + +import dev.ltms.bridged.herdr.AgentControl; +import dev.ltms.bridged.herdr.FakeHerdr; +import dev.ltms.bridged.inject.Injector; +import dev.ltms.bridged.msg.InMemoryReplyInbox; +import dev.ltms.bridged.msg.MessageService; +import dev.ltms.bridged.msg.Rendezvous; +import dev.ltms.bridged.session.SessionManager; +import dev.ltms.bridged.member.ClaudeCodeLauncher; +import dev.ltms.bridged.herdr.WorkspaceControl; +import dev.ltms.bridged.guard.SubscriptionGuard; +import dev.ltms.bridged.config.BridgedConfig; +import org.junit.jupiter.api.Test; + +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Executors; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class FleetHealthMonitorTest { + @Test void oneTickUsesOneFleetListForAnyRosterSize() { + FakeHerdr herdr = new FakeHerdr(); + AgentControl agents = new AgentControl(herdr); + BridgedConfig.Profile profile = new BridgedConfig.Profile("test", "http://test:1", null, + null, null, null, null, null, null, null, null, null); + ClaudeCodeLauncher launcher = new ClaudeCodeLauncher(agents, new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("test")), Map.of("test", profile), "test", _ -> "token"); + SessionManager sessions = new SessionManager(launcher); + sessions.acquire("test", null, null, null); + sessions.acquire("test", null, null, null); + herdr.calls.clear(); + MessageService messages = new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()); + var scheduler = Executors.newSingleThreadScheduledExecutor(); + FleetHealthMonitor monitor = new FleetHealthMonitor(agents, sessions::roster, messages, scheduler, () -> 1, 60); + monitor.tick(); + monitor.stop(); + assertEquals(1, herdr.calls.stream().filter(call -> call.method().equals("agent.list")).count()); + } +} diff --git a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java index fb2e067..7b1af8b 100644 --- a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java @@ -72,7 +72,7 @@ class BridgeMcpAuthzTest { new PrimaryRegistry(null), enforce ? CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, new MemberRegistry(null)) : null, - metrics, BridgeMcp.CapacitySource.none()); + metrics, BridgeMcp.CapacitySource.none(), new BridgeMcp.HealthCoverageSource(() -> "off")); return mcp; } diff --git a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java index 48d9b77..43ba54d 100644 --- a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java @@ -474,7 +474,7 @@ class BridgeMcpTest { sessions.acquire("ltms-local", null, null, null); McpSchema.CallToolResult res = BridgeMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, new BridgeMcp.CapacitySource(profile -> 2, profile -> 2, - () -> Set.of("ltms-local"), () -> 0), Map.of(), ""); + () -> Set.of("ltms-local"), () -> 0), new BridgeMcp.HealthCoverageSource(() -> "off"), Map.of(), ""); String out = textOf(res); assertTrue(out.contains("\"maxLoad\":2"), out); assertTrue(out.contains("\"live\":2"), out); @@ -487,7 +487,7 @@ class BridgeMcpTest { SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); String out = textOf(BridgeMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), sessions, null, new BridgeMcp.CapacitySource(profile -> 0, profile -> 2, - () -> Set.of("terra"), () -> 0), Map.of(), "")); + () -> Set.of("terra"), () -> 0), new BridgeMcp.HealthCoverageSource(() -> "off"), Map.of(), "")); assertTrue(out.contains("\"profile\":\"terra\""), out); assertTrue(out.contains("\"live\":0"), out); assertTrue(out.contains("\"free\":2"), out); @@ -499,7 +499,7 @@ class BridgeMcpTest { FakeHerdr h = new FakeHerdr(); String out = textOf(BridgeMcp.listFleet(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")), new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw"))), null, - BridgeMcp.CapacitySource.none(), Map.of(), "")); + BridgeMcp.CapacitySource.none(), new BridgeMcp.HealthCoverageSource(() -> "off"), Map.of(), "")); assertFalse(out.contains("\"capacity\":"), out); } From c76b2f149e668e8a2bd382cf362674611567d650 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:17:28 +0200 Subject: [PATCH 07/20] CB-573: keep health monitoring after failures --- .../bridged/health/FleetHealthMonitor.java | 65 +++++++++++++++---- .../health/FleetHealthMonitorTest.java | 40 ++++++++++++ 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java index c0d808c..79e4f84 100644 --- a/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java +++ b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java @@ -9,6 +9,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.concurrent.ScheduledExecutorService; @@ -26,6 +27,10 @@ public final class FleetHealthMonitor { private final LongSupplier clock; private final long intervalSeconds; private final Map priors = new HashMap<>(); + private final Map states = new HashMap<>(); + + // These facts need the evidence publishers introduced by later M4 units. They are not negatives. + private static final boolean NOT_YET_OBSERVED = false; public FleetHealthMonitor(AgentControl agents, Supplier> roster, MessageService messages, ScheduledExecutorService scheduler, LongSupplier clock, long intervalSeconds) { @@ -47,21 +52,53 @@ public final class FleetHealthMonitor { // Package-private so tests can run one tick without waiting. void tick() { - List agentsNow = agents.list(); // Exactly one list call for this complete observation. - Map live = new HashMap<>(); - for (Agent agent : agentsNow) live.put(agent.terminalId(), agent); - for (MemberSession session : roster.get()) { // One in-memory roster snapshot for the same tick. - Agent agent = live.get(session.terminalId()); - AgentStatus status = agent == null ? AgentStatus.UNKNOWN : agent.status(); - boolean accepted = messages.hasAcceptedDelivery(session.terminalId()); - HealthSnapshot snapshot = new HealthSnapshot(session.state(), status, accepted, false, - messages.hasInboxMessage(session.terminalId()), agent != null, false, false, - false, false, false, false); - HealthDecision decision = decide(snapshot, priors.getOrDefault(session.terminalId(), HealthPrior.NONE), - clock.getAsLong()); - priors.put(session.terminalId(), decision.prior()); + try { + List agentsNow = agents.list(); // Exactly one list call for this complete observation. + List rosterNow = roster.get(); // One in-memory roster snapshot for this tick. + Map live = new HashMap<>(); + for (Agent agent : agentsNow) live.put(agent.terminalId(), agent); + HashSet current = new HashSet<>(); + for (MemberSession session : rosterNow) { + current.add(session.terminalId()); + Agent agent = live.get(session.terminalId()); + AgentStatus status = agent == null ? AgentStatus.UNKNOWN : agent.status(); + boolean accepted = messages.hasAcceptedDelivery(session.terminalId()); + HealthSnapshot snapshot = new HealthSnapshot(session.state(), status, accepted, NOT_YET_OBSERVED, + messages.hasInboxMessage(session.terminalId()), agent != null, NOT_YET_OBSERVED, + NOT_YET_OBSERVED, NOT_YET_OBSERVED, NOT_YET_OBSERVED, NOT_YET_OBSERVED, NOT_YET_OBSERVED); + HealthDecision decision = decide(snapshot, priors.getOrDefault(session.terminalId(), HealthPrior.NONE), + clock.getAsLong()); + priors.put(session.terminalId(), decision.prior()); + reportTransition(session.terminalId(), decision.state()); + } + priors.keySet().retainAll(current); + states.keySet().retainAll(current); + } catch (Throwable error) { + // A list failure is health evidence, and must never kill the monitor's only scheduler task. + log.warn("fleet health collection failed; will retry next tick", error); + } finally { + if (!scheduler.isShutdown()) { + scheduler.schedule(this::tick, intervalSeconds, TimeUnit.SECONDS); + } } - scheduler.schedule(this::tick, intervalSeconds, TimeUnit.SECONDS); + } + + void reportTransition(String target, HealthState next) { + HealthState previous = states.put(target, next); + if (previous == next) return; + if (fault(next)) { + log.warn("fleet health member={} state={} previous={}", target, next, previous); + } else if (previous != null && fault(previous)) { + log.info("fleet health member={} recovered state={} previous={}", target, next, previous); + } + } + + private static boolean fault(HealthState state) { + return switch (state) { + case NEVER_READY, GONE, TURN_BOUNDARY_LOST, ERROR_ON_SCREEN, STALL_SUSPECTED, + MUTE, REPLY_STRANDED, DELEGATION_ORPHANED, CONTROL_LINK_DOWN -> true; + default -> false; + }; } public static String coverage(boolean enabled, boolean notificationConfigured) { diff --git a/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java index e133ad5..4d90b07 100644 --- a/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java @@ -1,5 +1,8 @@ package dev.ltms.bridged.health; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.inject.Injector; @@ -12,6 +15,7 @@ import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.config.BridgedConfig; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import java.util.Map; import java.util.Set; @@ -38,4 +42,40 @@ class FleetHealthMonitorTest { monitor.stop(); assertEquals(1, herdr.calls.stream().filter(call -> call.method().equals("agent.list")).count()); } + + @Test void failedTickDoesNotStopTheNextTick() { + FakeHerdr herdr = new FakeHerdr().healthy(false); + AgentControl agents = new AgentControl(herdr); + var scheduler = Executors.newSingleThreadScheduledExecutor(); + FleetHealthMonitor monitor = new FleetHealthMonitor(agents, java.util.List::of, + new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()), + scheduler, () -> 1, 60); + monitor.tick(); + herdr.healthy(true); + monitor.tick(); + monitor.stop(); + assertEquals(2, herdr.calls.stream().filter(call -> call.method().equals("agent.list")).count()); + } + + @Test void faultTransitionLogsOnlyOnceUntilItChanges() { + Logger logger = (Logger) LoggerFactory.getLogger(FleetHealthMonitor.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + FakeHerdr herdr = new FakeHerdr(); + AgentControl agents = new AgentControl(herdr); + var scheduler = Executors.newSingleThreadScheduledExecutor(); + FleetHealthMonitor monitor = new FleetHealthMonitor(agents, java.util.List::of, + new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()), + scheduler, () -> 1, 60); + monitor.reportTransition("term_a", HealthState.TURN_BOUNDARY_LOST); + monitor.reportTransition("term_a", HealthState.TURN_BOUNDARY_LOST); + monitor.stop(); + assertEquals(1, appender.list.stream().filter(event -> event.getFormattedMessage() + .contains("member=term_a state=TURN_BOUNDARY_LOST")).count()); + } finally { + logger.detachAppender(appender); + } + } } From e186c7945ab456ed10797e32d8a271463b32cf68 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:21:53 +0200 Subject: [PATCH 08/20] CB-577: correlate async questions to turns --- .../dev/ltms/bridged/msg/MessageService.java | 23 +++++++++++++------ .../ltms/bridged/msg/MessageServiceTest.java | 17 ++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index c50675e..cc99ff4 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -173,6 +173,9 @@ public final class MessageService { private final ConcurrentHashMap tasks = new ConcurrentHashMap<>(); /** Async tasks that have accepted delivery for a target. */ private final ConcurrentHashMap> asyncTasksByTarget = new ConcurrentHashMap<>(); + /** Async task that owns each exact forward rendezvous waiter. */ + private final ConcurrentHashMap, Task> asyncTasksByWaiter = + new ConcurrentHashMap<>(); /** Async tickets paused on a specific {@code bridge_ask} turn. */ private final ConcurrentHashMap asyncTasksByTurn = new ConcurrentHashMap<>(); private final AtomicLong ticketSeq = new AtomicLong(); @@ -385,6 +388,9 @@ public final class MessageService { // failed send leaves no stale waiter behind. CompletableFuture reply = rendezvous.open(target); try { + if (task != null) { + asyncTasksByWaiter.put(reply, task); + } // The send has won the lock; the accepted-delivery hook records delegator ownership // here (CB-548). It runs BEFORE enqueue so a throwing hook — onAccepted is now a // public callback — fails the send without queuing a message that would orphan. @@ -408,6 +414,7 @@ public final class MessageService { throw new IllegalStateException("interrupted awaiting reply from " + target, e); } } finally { + asyncTasksByWaiter.remove(reply); rendezvous.close(target, reply); } } finally { @@ -433,11 +440,15 @@ public final class MessageService { if (ticket.fresh()) { // Register the reverse waiter first, then surface the question — so the answer, which can // arrive the instant the primary reacts, always finds an open waiter to resolve. + CompletableFuture waiter = rendezvous.currentWaiter(workerSession); + Task task = markAsyncQuestion(waiter, question, ticket.turnId()); if (!rendezvous.resolveQuestion(workerSession, question, ticket.turnId())) { + if (task != null) { + clearAsyncQuestion(ticket.turnId(), true); + } rendezvous.closeAsk(ticket.turnId()); return new AskResult(AskOutcome.NO_WAITER, null); // no primary is blocked on this worker } - markAsyncQuestion(workerSession, question, ticket.turnId()); } try { String answer = ticket.answer().get(timeoutMillis, TimeUnit.MILLISECONDS); @@ -613,17 +624,14 @@ public final class MessageService { } /** Record the active question for an async ticket; blocking sends have no entry and stay unchanged. */ - private void markAsyncQuestion(String target, String text, String turnId) { - Set targetTasks = asyncTasksByTarget.get(target); - Task task = targetTasks == null ? null : targetTasks.stream() - .filter(candidate -> !candidate.future.isDone()) - .findFirst() - .orElse(null); + private Task markAsyncQuestion(CompletableFuture waiter, String text, String turnId) { + Task task = asyncTasksByWaiter.get(waiter); if (task != null) { task.question = new Reply(Outcome.QUESTION, text, turnId); task.turnId = turnId; asyncTasksByTurn.put(turnId, task); } + return task; } /** Clear an answered or lapsed question, but only when it matches the ticket's current turn. */ @@ -634,6 +642,7 @@ public final class MessageService { if (forgetTurn) { asyncTasksByTurn.remove(turnId, task); task.turnId = null; + untrackAsyncTarget(task); } } } diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index ecb1b5d..5577eba 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -676,6 +676,23 @@ class MessageServiceTest { assertEquals(MessageService.Outcome.REPLIED, answer.get(5, TimeUnit.SECONDS).outcome()); } + @Test + void unansweredAsyncQuestionReturnsTheTicketToPendingAndReleasesItsTarget() throws Exception { + String ticket = messages.sendAsync(T, "task that asks"); + awaitWaiting(); + injectDelivery(); + + assertEquals(MessageService.AskOutcome.TIMED_OUT, + messages.ask(T, "which config?", 200).outcome()); + assertEquals(MessageService.Phase.PENDING, messages.poll(ticket).phase(), + "only the question wait ended; the delegated turn may still finish"); + + String next = messages.sendAsync(T, "next task"); + awaitWaiting(); + assertTrue(rendezvous.resolve(T, "done")); + assertEquals(MessageService.Phase.DONE, awaitTicketPhase(next, MessageService.Phase.DONE).phase()); + } + private void assertFailedTicket(String ticket, String reason) throws Exception { MessageService.TaskView view = awaitTicketPhase(ticket, MessageService.Phase.FAILED); assertEquals(reason, view.detail()); From 5275922d1da971ace525d091723e0544f65e5ecd Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:22:22 +0200 Subject: [PATCH 09/20] CB-577: handle questions without async waiters --- bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index cc99ff4..024e274 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -625,7 +625,7 @@ public final class MessageService { /** Record the active question for an async ticket; blocking sends have no entry and stay unchanged. */ private Task markAsyncQuestion(CompletableFuture waiter, String text, String turnId) { - Task task = asyncTasksByWaiter.get(waiter); + Task task = waiter == null ? null : asyncTasksByWaiter.get(waiter); if (task != null) { task.question = new Reply(Outcome.QUESTION, text, turnId); task.turnId = turnId; From 927e0151d437a949c4d0790bc3819bef35e80a69 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:23:22 +0200 Subject: [PATCH 10/20] CB-577: test async question waiter ownership --- .../ltms/bridged/msg/MessageServiceTest.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index 5577eba..c14001b 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -10,6 +10,9 @@ import dev.ltms.bridged.inject.Injector; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.lang.reflect.Field; +import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -693,6 +696,38 @@ class MessageServiceTest { assertEquals(MessageService.Phase.DONE, awaitTicketPhase(next, MessageService.Phase.DONE).phase()); } + @Test + @SuppressWarnings("unchecked") + void asyncQuestionBelongsToTheTaskThatOwnsItsForwardWaiter() throws Exception { + String first = messages.sendAsync(T, "first task"); + awaitWaiting(); + String second = messages.sendAsync(T, "second task"); + + // Model the resolveQuestion/markAsyncQuestion race: another accepted task reached the target set. + Field tasksField = MessageService.class.getDeclaredField("tasks"); + tasksField.setAccessible(true); + Map tasks = (Map) tasksField.get(messages); + Field byTargetField = MessageService.class.getDeclaredField("asyncTasksByTarget"); + byTargetField.setAccessible(true); + Map> byTarget = (Map>) byTargetField.get(messages); + Set targetTasks = byTarget.get(T); + targetTasks.clear(); + targetTasks.add(tasks.get(second)); + + CompletableFuture ask = + CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000)); + assertEquals(MessageService.Phase.ASKING, awaitTicketPhase(first, MessageService.Phase.ASKING).phase()); + assertEquals(MessageService.Phase.PENDING, messages.poll(second).phase()); + + MessageService.TaskView asking = messages.poll(first); + CompletableFuture answer = CompletableFuture.supplyAsync( + () -> messages.answer(asking.turnId(), "config.yaml", 5000)); + assertEquals("config.yaml", ask.get(5, TimeUnit.SECONDS).answer()); + awaitWaiting(); + assertTrue(rendezvous.resolve(T, "done")); + assertEquals(MessageService.Outcome.REPLIED, answer.get(5, TimeUnit.SECONDS).outcome()); + } + private void assertFailedTicket(String ticket, String reason) throws Exception { MessageService.TaskView view = awaitTicketPhase(ticket, MessageService.Phase.FAILED); assertEquals(reason, view.detail()); From 74b0087ebb54ceaa926a005a0f6af09f64945711 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:24:05 +0200 Subject: [PATCH 11/20] CB-577: model async question ownership race --- .../java/dev/ltms/bridged/msg/MessageServiceTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index c14001b..a72804e 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -701,23 +701,21 @@ class MessageServiceTest { void asyncQuestionBelongsToTheTaskThatOwnsItsForwardWaiter() throws Exception { String first = messages.sendAsync(T, "first task"); awaitWaiting(); - String second = messages.sendAsync(T, "second task"); // Model the resolveQuestion/markAsyncQuestion race: another accepted task reached the target set. - Field tasksField = MessageService.class.getDeclaredField("tasks"); - tasksField.setAccessible(true); - Map tasks = (Map) tasksField.get(messages); Field byTargetField = MessageService.class.getDeclaredField("asyncTasksByTarget"); byTargetField.setAccessible(true); Map> byTarget = (Map>) byTargetField.get(messages); Set targetTasks = byTarget.get(T); + Class taskClass = Class.forName(MessageService.class.getName() + "$Task"); + var constructor = taskClass.getDeclaredConstructor(String.class); + constructor.setAccessible(true); targetTasks.clear(); - targetTasks.add(tasks.get(second)); + targetTasks.add(constructor.newInstance(T)); CompletableFuture ask = CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000)); assertEquals(MessageService.Phase.ASKING, awaitTicketPhase(first, MessageService.Phase.ASKING).phase()); - assertEquals(MessageService.Phase.PENDING, messages.poll(second).phase()); MessageService.TaskView asking = messages.poll(first); CompletableFuture answer = CompletableFuture.supplyAsync( From c884802b1312fae4f87e909440da92b7ac29211d Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:28:53 +0200 Subject: [PATCH 12/20] CB-577: remove obsolete async target tracking --- .../dev/ltms/bridged/msg/MessageService.java | 25 +------------------ .../ltms/bridged/msg/MessageServiceTest.java | 15 ----------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index 024e274..ca75d04 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -9,7 +9,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.List; -import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -171,8 +170,6 @@ public final class MessageService { private final Metrics metrics; // CB-502: nullable — no registry in unit tests private final ConcurrentHashMap sessionLocks = new ConcurrentHashMap<>(); private final ConcurrentHashMap tasks = new ConcurrentHashMap<>(); - /** Async tasks that have accepted delivery for a target. */ - private final ConcurrentHashMap> asyncTasksByTarget = new ConcurrentHashMap<>(); /** Async task that owns each exact forward rendezvous waiter. */ private final ConcurrentHashMap, Task> asyncTasksByWaiter = new ConcurrentHashMap<>(); @@ -547,13 +544,7 @@ public final class MessageService { tasks.put(ticket, task); asyncExecutor.submit(() -> { try { - Runnable trackingAccepted = () -> { - if (onAccepted != null) { - onAccepted.run(); - } - asyncTasksByTarget.computeIfAbsent(target, _ -> ConcurrentHashMap.newKeySet()).add(task); - }; - Reply result = send(target, content, ASYNC_TIMEOUT_MS, trackingAccepted, task); + Reply result = send(target, content, ASYNC_TIMEOUT_MS, onAccepted, task); if (result.outcome() == Outcome.QUESTION) { // Keep the accepted owner until answer() finishes it. markAsyncQuestion may run // just after resolveQuestion wakes this thread. @@ -562,7 +553,6 @@ public final class MessageService { } } catch (Throwable t) { task.future.completeExceptionally(t); - untrackAsyncTarget(task); } }); pruneTerminalTickets(); @@ -642,7 +632,6 @@ public final class MessageService { if (forgetTurn) { asyncTasksByTurn.remove(turnId, task); task.turnId = null; - untrackAsyncTarget(task); } } } @@ -650,7 +639,6 @@ public final class MessageService { /** Complete and detach an async ticket after its worker's actual terminal reply. */ private void finishAsyncTask(Task task, Reply result) { task.future.complete(result); - untrackAsyncTarget(task); if (task.turnId != null) { asyncTasksByTurn.remove(task.turnId, task); } @@ -669,17 +657,6 @@ public final class MessageService { return asyncTasksByTurn.values().stream().anyMatch(task -> target.equals(task.target)); } - /** Stop tracking a task once it no longer owns an accepted target turn. */ - private void untrackAsyncTarget(Task task) { - Set targetTasks = asyncTasksByTarget.get(task.target); - if (targetTasks != null) { - targetTasks.remove(task); - if (targetTasks.isEmpty()) { - asyncTasksByTarget.remove(task.target, targetTasks); - } - } - } - /** Release the async executor. */ public void close() { asyncExecutor.shutdown(); diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index a72804e..7f25043 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -10,9 +10,6 @@ import dev.ltms.bridged.inject.Injector; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.lang.reflect.Field; -import java.util.Map; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -697,22 +694,10 @@ class MessageServiceTest { } @Test - @SuppressWarnings("unchecked") void asyncQuestionBelongsToTheTaskThatOwnsItsForwardWaiter() throws Exception { String first = messages.sendAsync(T, "first task"); awaitWaiting(); - // Model the resolveQuestion/markAsyncQuestion race: another accepted task reached the target set. - Field byTargetField = MessageService.class.getDeclaredField("asyncTasksByTarget"); - byTargetField.setAccessible(true); - Map> byTarget = (Map>) byTargetField.get(messages); - Set targetTasks = byTarget.get(T); - Class taskClass = Class.forName(MessageService.class.getName() + "$Task"); - var constructor = taskClass.getDeclaredConstructor(String.class); - constructor.setAccessible(true); - targetTasks.clear(); - targetTasks.add(constructor.newInstance(T)); - CompletableFuture ask = CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000)); assertEquals(MessageService.Phase.ASKING, awaitTicketPhase(first, MessageService.Phase.ASKING).phase()); From aac29d604c9bcb98cd4ad3ea17a60595d9c45607 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:29:27 +0200 Subject: [PATCH 13/20] =?UTF-8?q?M4:=20correct=20unit=202=20criterion=201?= =?UTF-8?q?=20=E2=80=94=20TurnToken=20cannot=20carry=20the=20session=20tur?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The criterion required the token to bind the session turn number. Three independent refusals from the implementer showed why that is not implementable at this layer: MessageService owns acceptance but never learns of delivery, and CompletionResolver.onDelivered runs before SessionManager.onDelivered, so the turn number does not exist yet at the only point the token could capture it. Records both rejected alternatives and why, so the next reader does not re-derive them: a target-keyed registry restores the ambiguity the token exists to remove, and injecting a turn counter couples layers to fill a field nothing reads yet. --- docs/M4-Fleet-Health.md | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md index 11e36c2..f954a99 100644 --- a/docs/M4-Fleet-Health.md +++ b/docs/M4-Fleet-Health.md @@ -744,8 +744,28 @@ worktree discovery. Acceptance criteria: -1. Every accepted send receives a stable `TurnToken` tied to target, exact waiter, session turn, - delivery baseline, and task outcome. +1. Every accepted send receives a stable `TurnToken` tied to target, exact waiter, and delivery + baseline. + + **Corrected during implementation (2026-08-15).** This criterion first also required the session + turn number and the task outcome. That is not implementable at this layer, and the implementer + refused it three times rather than fabricate a value — correctly. The reason is an ordering fact + that is invisible from any single class: `MessageService` owns acceptance and holds the waiter and + the async `Task`, but it learns nothing about delivery, because the delivery event goes to + `CompletionResolver` through `TurnListener.onDelivered`. And `CompletionResolver.onDelivered` runs + *before* `SessionManager.onDelivered`, so the session turn number does not exist yet at the only + point where the token could capture it. + + Two ways out were rejected. A shared registry keyed by target reintroduces exactly the "whichever + send happens to be waiting" ambiguity the token exists to remove — the same weak claim + `Rendezvous.currentWaiter` warns about. Injecting a turn counter into `MessageService` adds a + required cross-layer dependency to populate a field that nothing in this slice reads, which is + speculative coupling across a boundary already shown to be fragile. + + So the token identifies the **accepted send**, and `SessionManager` keeps verifying its own + delivery separately. Repair (criterion 2) does need the session turn; binding it means resolving + that acceptance-versus-delivery ordering first, and that work belongs to the repair unit, not + here. The token record carries a comment saying the field is deliberately absent. 2. Repair requires the same `BUSY` token, two raw `IDLE` or `DONE` snapshots, no conflicting observation, exact open waiter, successful baseline, and new recognised assistant output. 3. Missing, failed, late, or post-restart baseline never authorises repair. From b745e159de7f02c86ab2e6c1506fc36100b65bcd Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:27:39 +0200 Subject: [PATCH 14/20] CB-573: carry accepted turn tokens on delivery --- .../main/java/dev/ltms/bridged/Bridged.java | 6 +++--- .../bridged/inject/CompletionResolver.java | 9 +++++---- .../dev/ltms/bridged/inject/Injector.java | 9 +++++---- .../dev/ltms/bridged/inject/TurnListener.java | 4 +++- .../dev/ltms/bridged/msg/MessageService.java | 3 ++- .../java/dev/ltms/bridged/msg/TurnToken.java | 20 +++++++++++++++++++ .../ltms/bridged/session/SessionManager.java | 3 ++- 7 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 bridged/src/main/java/dev/ltms/bridged/msg/TurnToken.java diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index fbae9b7..70a10d9 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -276,9 +276,9 @@ public final class Bridged { } @Override - public void onDelivered(String target) { - completion.onDelivered(target); - sessions.onDelivered(target); + public void onDelivered(String target, dev.ltms.bridged.msg.TurnToken token) { + completion.onDelivered(target, token); + sessions.onDelivered(target, token); } @Override diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java b/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java index e504b96..5259ffd 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/CompletionResolver.java @@ -2,6 +2,7 @@ package dev.ltms.bridged.inject; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.msg.Rendezvous; +import dev.ltms.bridged.msg.TurnToken; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,17 +84,17 @@ public final class CompletionResolver implements TurnListener { } @Override - public void onDelivered(String target) { + public void onDelivered(String target, TurnToken token) { // Capture the exact waiter this turn belongs to (CB-116) and snapshot the pane's pre-turn // content — what it shows *before* the just-delivered turn produces output — as the staleness // reference (CB-115). Done synchronously (like the delivering send itself) so both are in // place before this turn's completion can fire. - captureBaseline(target); + captureBaseline(target, token); } /** Capture the in-flight turn: its waiter and pre-turn baseline (the testable core of {@link #onDelivered}). */ - void captureBaseline(String target) { - CompletableFuture waiter = rendezvous.currentWaiter(target); + void captureBaseline(String target, TurnToken token) { + CompletableFuture waiter = token.waiter(); if (waiter == null) { inFlight.remove(target); // no send is waiting on this delivery — nothing to resolve later return; diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java b/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java index 6144c6b..eeb62f3 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/Injector.java @@ -2,6 +2,7 @@ package dev.ltms.bridged.inject; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.AgentStatus; +import dev.ltms.bridged.msg.TurnToken; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -129,7 +130,7 @@ public final class Injector { } /** A pending message and the future that completes when it has been delivered. */ - private record Pending(String text, CompletableFuture delivered) { + private record Pending(String text, TurnToken token, CompletableFuture delivered) { } /** Per-worker delivery state, guarded by its own monitor (single writer per worker). */ @@ -159,9 +160,9 @@ public final class Injector { *

Uses an atomic map update so a concurrent {@link #drop} cannot slip between "find the * target" and "queue the message" and orphan it in a target it just removed. */ - public CompletableFuture enqueue(String target, String text) { + public CompletableFuture enqueue(String target, String text, TurnToken token) { CompletableFuture delivered = new CompletableFuture<>(); - Pending p = new Pending(text, delivered); + Pending p = new Pending(text, token, delivered); targets.compute(target, (_, existing) -> { Target t = (existing != null) ? existing : new Target(); t.add(p); // synchronized on the Target monitor — atomic with a concurrent drop @@ -356,7 +357,7 @@ public final class Injector { } else { // Baseline the pane's pre-turn content so a misattributed completion (no new output) // can't resolve this send with the previous turn's stale answer (CB-115). - turnListener.onDelivered(target); + turnListener.onDelivered(target, sent.token()); sent.delivered().complete(null); } } diff --git a/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java b/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java index 68e377d..7d453ee 100644 --- a/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java +++ b/bridged/src/main/java/dev/ltms/bridged/inject/TurnListener.java @@ -1,5 +1,7 @@ package dev.ltms.bridged.inject; +import dev.ltms.bridged.msg.TurnToken; + /** * Notified when a worker's delegated turn is observed to complete — a confirmed * {@code WORKING → IDLE} transition after a delivery. This is the CB-106 completion signal the @@ -57,7 +59,7 @@ public interface TurnListener { * resolve the send with the previous turn's stale answer. A default no-op keeps the interface * functional for callers that don't scrape. */ - default void onDelivered(String target) { + default void onDelivered(String target, TurnToken token) { } /** No-op default for callers that only need delivery, not completion signalling. */ diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java index 024e274..4c0a151 100644 --- a/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java +++ b/bridged/src/main/java/dev/ltms/bridged/msg/MessageService.java @@ -391,13 +391,14 @@ public final class MessageService { if (task != null) { asyncTasksByWaiter.put(reply, task); } + TurnToken token = new TurnToken(target, reply); // The send has won the lock; the accepted-delivery hook records delegator ownership // here (CB-548). It runs BEFORE enqueue so a throwing hook — onAccepted is now a // public callback — fails the send without queuing a message that would orphan. if (onAccepted != null) { onAccepted.run(); } - CompletableFuture delivered = injector.enqueue(target, content); + CompletableFuture delivered = injector.enqueue(target, content, token); try { Rendezvous.Resolution r = reply.get(remainingMillis(deadlineNanos), TimeUnit.MILLISECONDS); return recorded(new Reply(outcomeOf(r.kind()), r.text(), r.turnId())); diff --git a/bridged/src/main/java/dev/ltms/bridged/msg/TurnToken.java b/bridged/src/main/java/dev/ltms/bridged/msg/TurnToken.java new file mode 100644 index 0000000..f3f8c2e --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/msg/TurnToken.java @@ -0,0 +1,20 @@ +package dev.ltms.bridged.msg; + +import java.util.concurrent.CompletableFuture; + +/** + * Identity for one accepted send. The session turn is deliberately absent: CompletionResolver's + * delivery callback runs before SessionManager.onDelivered, so binding it needs a later ordering design. + */ +public final class TurnToken { + private final String target; + private final CompletableFuture waiter; + + public TurnToken(String target, CompletableFuture waiter) { + this.target = target; + this.waiter = waiter; + } + + public String target() { return target; } + public CompletableFuture waiter() { return waiter; } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java index fdb5766..9ebc167 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java @@ -4,6 +4,7 @@ import dev.ltms.bridged.auth.MemberLifecycle; import dev.ltms.bridged.herdr.Agent; import dev.ltms.bridged.inject.TurnListener; import dev.ltms.bridged.inject.MemberPresence; +import dev.ltms.bridged.msg.TurnToken; import dev.ltms.bridged.peer.MemberRole; import dev.ltms.bridged.peer.PeerHandle; import dev.ltms.bridged.peer.PeerLauncher; @@ -425,7 +426,7 @@ public final class SessionManager implements TurnListener { * can be re-delivered for multi-turn reuse until it is released. */ @Override - public void onDelivered(String target) { + public void onDelivered(String target, TurnToken token) { MemberSession current = findByTerminal(target); if (current == null) return; if (current.state() != MemberSession.State.READY && current.state() != MemberSession.State.DONE) { From 0edc6615fcd226a1d8702964e78a0e7e2716393d Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:29:27 +0200 Subject: [PATCH 15/20] =?UTF-8?q?M4:=20correct=20unit=202=20criterion=201?= =?UTF-8?q?=20=E2=80=94=20TurnToken=20cannot=20carry=20the=20session=20tur?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The criterion required the token to bind the session turn number. Three independent refusals from the implementer showed why that is not implementable at this layer: MessageService owns acceptance but never learns of delivery, and CompletionResolver.onDelivered runs before SessionManager.onDelivered, so the turn number does not exist yet at the only point the token could capture it. Records both rejected alternatives and why, so the next reader does not re-derive them: a target-keyed registry restores the ambiguity the token exists to remove, and injecting a turn counter couples layers to fill a field nothing reads yet. --- docs/M4-Fleet-Health.md | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md index 11e36c2..f954a99 100644 --- a/docs/M4-Fleet-Health.md +++ b/docs/M4-Fleet-Health.md @@ -744,8 +744,28 @@ worktree discovery. Acceptance criteria: -1. Every accepted send receives a stable `TurnToken` tied to target, exact waiter, session turn, - delivery baseline, and task outcome. +1. Every accepted send receives a stable `TurnToken` tied to target, exact waiter, and delivery + baseline. + + **Corrected during implementation (2026-08-15).** This criterion first also required the session + turn number and the task outcome. That is not implementable at this layer, and the implementer + refused it three times rather than fabricate a value — correctly. The reason is an ordering fact + that is invisible from any single class: `MessageService` owns acceptance and holds the waiter and + the async `Task`, but it learns nothing about delivery, because the delivery event goes to + `CompletionResolver` through `TurnListener.onDelivered`. And `CompletionResolver.onDelivered` runs + *before* `SessionManager.onDelivered`, so the session turn number does not exist yet at the only + point where the token could capture it. + + Two ways out were rejected. A shared registry keyed by target reintroduces exactly the "whichever + send happens to be waiting" ambiguity the token exists to remove — the same weak claim + `Rendezvous.currentWaiter` warns about. Injecting a turn counter into `MessageService` adds a + required cross-layer dependency to populate a field that nothing in this slice reads, which is + speculative coupling across a boundary already shown to be fragile. + + So the token identifies the **accepted send**, and `SessionManager` keeps verifying its own + delivery separately. Repair (criterion 2) does need the session turn; binding it means resolving + that acceptance-versus-delivery ordering first, and that work belongs to the repair unit, not + here. The token record carries a comment saying the field is deliberately absent. 2. Repair requires the same `BUSY` token, two raw `IDLE` or `DONE` snapshots, no conflicting observation, exact open waiter, successful baseline, and new recognised assistant output. 3. Missing, failed, late, or post-restart baseline never authorises repair. From 8e2e4c5e73e7e510349975a464d641c9b8e116ba Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 06:36:35 +0200 Subject: [PATCH 16/20] M4 unit 2a: migrate test call sites to the required turn token The delivery callback now requires a TurnToken, so 54 test call sites had to pass one. They use an explicit TestTurnTokens.inert(target) rather than a defaulted overload, because a delivery with no token is the unbound baseline this unit forbids. The first version of inert() returned a fresh CompletableFuture as the waiter, which turned captureBaselineSkipsTheReadWhenNoSendIsWaiting red: the resolver saw a non-null waiter, concluded a turn was in flight, and scraped a pane no send was blocked on. An inert value must omit the fact, not invent it, so the waiter is now null and the production skip fires as designed. --- .../inject/CompletionResolverTest.java | 8 ++- .../dev/ltms/bridged/inject/InjectorTest.java | 69 ++++++++++--------- .../ltms/bridged/msg/MessageServiceTest.java | 2 +- .../dev/ltms/bridged/msg/TestTurnTokens.java | 20 ++++++ .../bridged/session/SessionManagerTest.java | 31 +++++---- .../session/WorktreeSessionManagerTest.java | 3 +- 6 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 bridged/src/test/java/dev/ltms/bridged/msg/TestTurnTokens.java diff --git a/bridged/src/test/java/dev/ltms/bridged/inject/CompletionResolverTest.java b/bridged/src/test/java/dev/ltms/bridged/inject/CompletionResolverTest.java index 8ae282d..6cf1a40 100644 --- a/bridged/src/test/java/dev/ltms/bridged/inject/CompletionResolverTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/inject/CompletionResolverTest.java @@ -7,6 +7,8 @@ import ch.qos.logback.core.read.ListAppender; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.msg.Rendezvous; +import dev.ltms.bridged.msg.TestTurnTokens; +import dev.ltms.bridged.msg.TurnToken; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; @@ -47,7 +49,7 @@ class CompletionResolverTest { Rendezvous rendezvous = new Rendezvous(); // no waiter opened CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous); - resolver.captureBaseline("term_a"); // no send to attribute a later completion to + resolver.captureBaseline("term_a", TestTurnTokens.inert("term_a")); // no send to attribute a later completion to assertFalse(herdr.called("agent.read"), "with no waiting send there is no turn to baseline — skip the scrape"); @@ -194,7 +196,7 @@ class CompletionResolverTest { Rendezvous rendezvous = new Rendezvous(); CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous); var waiter = rendezvous.open("term_a"); - resolver.captureBaseline("term_a"); + resolver.captureBaseline("term_a", new TurnToken("term_a", waiter)); herdr.readText("⏺ answer that /clear would erase\n❯ "); resolver.resolveBeforePostAction("term_a"); @@ -217,7 +219,7 @@ class CompletionResolverTest { CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous); var waiter = rendezvous.open("term_a"); // a send is blocked on this turn - resolver.captureBaseline("term_a"); // baseline is the clipped >cap block + resolver.captureBaseline("term_a", new TurnToken("term_a", waiter)); // baseline is the clipped >cap block var turn = resolver.inFlight("term_a"); assertEquals(CompletionResolver.MAX_SCRAPE_CHARS, turn.baseline().length(), "the delivery baseline is clipped to the same cap resolve() applies to the tail"); diff --git a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java index 61a10b9..fddd39a 100644 --- a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java @@ -8,6 +8,7 @@ import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.AgentStatus; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.herdr.HerdrException; +import dev.ltms.bridged.msg.TestTurnTokens; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; @@ -47,7 +48,7 @@ class InjectorTest { @Test void deliversWhenIdle() { - CompletableFuture f = injector.enqueue(T, "hello"); + CompletableFuture f = injector.enqueue(T, "hello", TestTurnTokens.inert(T)); assertFalse(f.isDone(), "not delivered until an injectable status arrives"); injector.onStatus(T, AgentStatus.IDLE); assertTrue(f.isDone()); @@ -59,7 +60,7 @@ class InjectorTest { // CB-113: idle alone is not enough — hold until the worker's MCP is connected (ready). java.util.Set ready = new java.util.HashSet<>(); Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, ready::contains); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // idle but not yet available → held out of the boot window assertEquals(List.of(), sent(), "must not deliver into a not-yet-available worker"); @@ -79,7 +80,7 @@ class InjectorTest { void resubmitsEnterWhenADeliveredMessageIsNotPickedUp() { // CB-113: the Enter at delivery can race the paste; while the worker stays idle (not picked // up), the injector re-nudges Enter so the pending paste submits. - injector.enqueue(T, "task"); + injector.enqueue(T, "task", TestTurnTokens.inert(T)); injector.onStatus(T, AgentStatus.IDLE); // deliver: paste + one Enter long afterDeliver = enterKeystrokes(); @@ -95,7 +96,7 @@ class InjectorTest { @Test void holdsWhileWorkingThenDeliversOnIdle() { - injector.enqueue(T, "later"); + injector.enqueue(T, "later", TestTurnTokens.inert(T)); injector.onStatus(T, AgentStatus.WORKING); assertEquals(List.of(), sent(), "must not inject mid-turn"); injector.onStatus(T, AgentStatus.IDLE); @@ -104,7 +105,7 @@ class InjectorTest { @Test void blockedIsInjectableButUnknownIsNot() { - injector.enqueue(T, "answer"); + injector.enqueue(T, "answer", TestTurnTokens.inert(T)); injector.onStatus(T, AgentStatus.UNKNOWN); assertEquals(List.of(), sent(), "unknown status is not safe to inject"); injector.onStatus(T, AgentStatus.BLOCKED); @@ -113,8 +114,8 @@ class InjectorTest { @Test void twoRapidDeliveriesNeverInterleave() { - injector.enqueue(T, "m1"); - injector.enqueue(T, "m2"); + injector.enqueue(T, "m1", TestTurnTokens.inert(T)); + injector.enqueue(T, "m2", TestTurnTokens.inert(T)); // First idle window delivers only m1, even if idle is observed twice before pickup. injector.onStatus(T, AgentStatus.IDLE); @@ -129,8 +130,8 @@ class InjectorTest { @Test void transientUnknownDoesNotReleaseThePickupLatch() { - injector.enqueue(T, "m1"); - injector.enqueue(T, "m2"); + injector.enqueue(T, "m1", TestTurnTokens.inert(T)); + injector.enqueue(T, "m2", TestTurnTokens.inert(T)); injector.onStatus(T, AgentStatus.IDLE); // m1 sent, awaiting pickup assertEquals(List.of("m1"), sent()); @@ -145,8 +146,8 @@ class InjectorTest { @Test void missedPickupEdgeIsReleasedByGraceSoTheQueueNeverWedges() { - injector.enqueue(T, "m1"); - injector.enqueue(T, "m2"); + injector.enqueue(T, "m1", TestTurnTokens.inert(T)); + injector.enqueue(T, "m2", TestTurnTokens.inert(T)); injector.onStatus(T, AgentStatus.IDLE); // m1 sent assertEquals(List.of("m1"), sent()); @@ -158,9 +159,9 @@ class InjectorTest { @Test void fifoOrderAcrossManyTurns() { - injector.enqueue(T, "a"); - injector.enqueue(T, "b"); - injector.enqueue(T, "c"); + injector.enqueue(T, "a", TestTurnTokens.inert(T)); + injector.enqueue(T, "b", TestTurnTokens.inert(T)); + injector.enqueue(T, "c", TestTurnTokens.inert(T)); for (int i = 0; i < 3; i++) { injector.onStatus(T, AgentStatus.IDLE); // deliver one injector.onStatus(T, AgentStatus.WORKING); // pickup @@ -172,7 +173,7 @@ class InjectorTest { @Test void activeWhileQueuedOrInFlightThenQuietAfterTurnCompletes() { assertTrue(injector.activeTargets().isEmpty()); - injector.enqueue(T, "x"); + injector.enqueue(T, "x", TestTurnTokens.inert(T)); assertEquals(Set.of(T), injector.activeTargets(), "active while a message is queued"); injector.onStatus(T, AgentStatus.IDLE); // delivers; awaiting pickup @@ -191,7 +192,7 @@ class InjectorTest { void firesTurnCompleteOnAConfirmedWorkingThenIdle() { List completed = new ArrayList<>(); Injector inj = new Injector(new AgentControl(herdr), completed::add); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver inj.onStatus(T, AgentStatus.WORKING); // pickup + turn running @@ -226,8 +227,8 @@ class InjectorTest { } ResetListener listener = new ResetListener(); Injector inj = new Injector(agents, listener); - inj.enqueue(T, "first"); - inj.enqueue(T, "second"); + inj.enqueue(T, "first", TestTurnTokens.inert(T)); + inj.enqueue(T, "second", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // first delegation inj.onStatus(T, AgentStatus.WORKING); @@ -246,7 +247,7 @@ class InjectorTest { void doesNotSynthesizeCompletionFromAnUnconfirmedTurn() { List completed = new ArrayList<>(); Injector inj = new Injector(new AgentControl(herdr), completed::add); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); // Deliver, then only ever idle — a `working` sample is never seen. The pickup grace unwedges // the queue but must NOT invent a completion: without a sampled turn there is no trustworthy @@ -285,7 +286,7 @@ class InjectorTest { void failsAnOutstandingDelegationWhoseWorkerWedgesInUnknown() { Captor cap = new Captor(); Injector inj = new Injector(new AgentControl(herdr), cap); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver inj.onStatus(T, AgentStatus.WORKING); // worker starts the turn @@ -300,7 +301,7 @@ class InjectorTest { void aTransientUnknownGlitchNeitherFailsNorBlocksCompletion() { Captor cap = new Captor(); Injector inj = new Injector(new AgentControl(herdr), cap); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver inj.onStatus(T, AgentStatus.WORKING); // confirmed turn @@ -315,7 +316,7 @@ class InjectorTest { void sendFailureDropsMessageAndFailsItsFuture() { FakeHerdr failing = new FakeHerdr().agentSendFailsWith("send_failed"); Injector inj = new Injector(new AgentControl(failing)); - CompletableFuture f = inj.enqueue(T, "boom"); + CompletableFuture f = inj.enqueue(T, "boom", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); assertTrue(f.isCompletedExceptionally()); @@ -324,7 +325,7 @@ class InjectorTest { @Test void dropFailsPendingWaiters() { - CompletableFuture f = injector.enqueue(T, "orphan"); + CompletableFuture f = injector.enqueue(T, "orphan", TestTurnTokens.inert(T)); injector.drop(T, new HerdrException("worker gone", "pane_not_found", null)); assertTrue(f.isCompletedExceptionally(), "queued waiters unblock when the worker vanishes"); } @@ -333,8 +334,8 @@ class InjectorTest { void dropPassesTheRealCauseForQueuedAndDeliveredWork() { Captor cap = new Captor(); Injector inj = new Injector(new AgentControl(herdr), cap); - CompletableFuture delivered = inj.enqueue(T, "delivered"); - CompletableFuture queued = inj.enqueue(T, "queued"); + CompletableFuture delivered = inj.enqueue(T, "delivered", TestTurnTokens.inert(T)); + CompletableFuture queued = inj.enqueue(T, "queued", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver the first message inj.onStatus(T, AgentStatus.WORKING); // its turn is now in flight; one remains queued @@ -352,7 +353,7 @@ class InjectorTest { // leave its send hanging. A vanished worker must fail that in-flight turn too. Captor cap = new Captor(); Injector inj = new Injector(new AgentControl(herdr), cap); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver inj.onStatus(T, AgentStatus.WORKING); // turn running @@ -367,7 +368,7 @@ class InjectorTest { // by an off-sub worker's review of CB-110, delegated through the bridge.) Captor cap = new Captor(); Injector inj = new Injector(new AgentControl(herdr), cap); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); // deliver; pickup never confirmed inj.drop(T, new HerdrException("worker gone", "pane_not_found", null)); @@ -386,7 +387,7 @@ class InjectorTest { Captor cap = new Captor(); List forgotten = new ArrayList<>(); Injector inj = new Injector(new AgentControl(herdr), cap, _ -> false, forgotten::add); - CompletableFuture f = inj.enqueue(T, "task"); + CompletableFuture f = inj.enqueue(T, "task", TestTurnTokens.inert(T)); for (int i = 0; i < READINESS_SAMPLES; i++) inj.onStatus(T, AgentStatus.IDLE); @@ -414,7 +415,7 @@ class InjectorTest { try { Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, _ -> false, _ -> { }); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); for (int i = 0; i < READINESS_SAMPLES; i++) inj.onStatus(T, AgentStatus.IDLE); @@ -439,7 +440,7 @@ class InjectorTest { Set ready = new java.util.HashSet<>(); Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, ready::contains, _ -> { }); - inj.enqueue(T, "task"); + inj.enqueue(T, "task", TestTurnTokens.inert(T)); for (int i = 0; i < 100; i++) inj.onStatus(T, AgentStatus.IDLE); // still booting, well under grace assertEquals(List.of(), sent()); @@ -455,7 +456,7 @@ class InjectorTest { // linger past the worker's life (MemberPresence.forget had no caller before this). List forgotten = new ArrayList<>(); Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, _ -> true, forgotten::add); - inj.enqueue(T, "orphan"); + inj.enqueue(T, "orphan", TestTurnTokens.inert(T)); inj.drop(T, new HerdrException("worker gone", "pane_not_found", null)); assertEquals(List.of(T), forgotten, "drop clears the gone worker's presence"); } @@ -476,7 +477,7 @@ class InjectorTest { try { Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, _ -> true, _ -> { }); - inj.enqueue(T, "orphan"); + inj.enqueue(T, "orphan", TestTurnTokens.inert(T)); inj.drop(T, new HerdrException("worker gone", "pane_not_found", null)); String warn = appender.list.stream() @@ -500,7 +501,7 @@ class InjectorTest { StatusPoller poller = new StatusPoller(new AgentControl(idle), inj, 10); poller.start(); try { - CompletableFuture delivered = inj.enqueue(T, "via-poller"); + CompletableFuture delivered = inj.enqueue(T, "via-poller", TestTurnTokens.inert(T)); delivered.get(2, TimeUnit.SECONDS); // completes when the poller drives the send } finally { poller.stop(); @@ -519,7 +520,7 @@ class InjectorTest { void deliveredFutureCarriesSendFailure() { FakeHerdr failing = new FakeHerdr().agentSendFailsWith("send_failed"); Injector inj = new Injector(new AgentControl(failing)); - CompletableFuture f = inj.enqueue(T, "boom"); + CompletableFuture f = inj.enqueue(T, "boom", TestTurnTokens.inert(T)); inj.onStatus(T, AgentStatus.IDLE); ExecutionException ex = assertThrows(ExecutionException.class, f::get); assertInstanceOf(HerdrException.class, ex.getCause()); diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java index a72804e..58e3d04 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/MessageServiceTest.java @@ -130,7 +130,7 @@ class MessageServiceTest { injector.onStatus(T, AgentStatus.IDLE); // first delivery injector.onStatus(T, AgentStatus.WORKING); // first turn in flight - CompletableFuture queued = injector.enqueue(T, "second task"); + CompletableFuture queued = injector.enqueue(T, "second task", TestTurnTokens.inert(T)); CompletableFuture waiter = rendezvous.currentWaiter(T); injector.drop(T, new HerdrException("agent target sol not found", "agent_not_found", null)); diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/TestTurnTokens.java b/bridged/src/test/java/dev/ltms/bridged/msg/TestTurnTokens.java new file mode 100644 index 0000000..35bee6b --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/msg/TestTurnTokens.java @@ -0,0 +1,20 @@ +package dev.ltms.bridged.msg; + +/** + * Explicit unbound tokens for tests that exercise delivery without an accepted send. + * + *

The waiter is {@code null} on purpose. "No accepted send" is an absence, and a helper + * that handed back a fresh {@code CompletableFuture} would invent one — which is how the first + * version of this class turned {@code captureBaselineSkipsTheReadWhenNoSendIsWaiting} red: the + * resolver saw a non-null waiter, decided a turn was in flight, and scraped a pane that no send was + * blocked on. An inert value must omit the fact, never fabricate it. + */ +public final class TestTurnTokens { + private TestTurnTokens() { + } + + /** A token for a delivery that no send is waiting on: it authorises nothing. */ + public static TurnToken inert(String target) { + return new TurnToken(target, null); + } +} diff --git a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java index febf37a..422831a 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java @@ -10,6 +10,7 @@ import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.member.ClaudeCodeLauncher; +import dev.ltms.bridged.msg.TestTurnTokens; import dev.ltms.bridged.peer.PeerUnreachableException; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; @@ -101,7 +102,7 @@ class SessionManagerTest { assertDoesNotThrow(() -> sessions.asPresence().markPresent(null), "the primary's null terminal must not blow up an unrelated tool call"); - assertDoesNotThrow(() -> sessions.onDelivered(null)); + assertDoesNotThrow(() -> sessions.onDelivered(null, TestTurnTokens.inert(null))); assertDoesNotThrow(() -> sessions.onTurnComplete(null)); assertDoesNotThrow(() -> sessions.onTurnFailed(null)); @@ -121,7 +122,7 @@ class SessionManagerTest { "MCP presence moves SPAWNING → READY"); assertTrue(sessions.asPresence().isPresent(terminal), "presence is also recorded"); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); assertEquals(MemberSession.State.BUSY, sessions.get(session.paneId()).orElseThrow().state(), "delivery moves READY → BUSY"); @@ -153,7 +154,7 @@ class SessionManagerTest { MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnFailed(terminal); @@ -181,7 +182,7 @@ class SessionManagerTest { MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnFailed(terminal); @@ -264,7 +265,7 @@ class SessionManagerTest { MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); clock[0] = 100; assertEquals(0, sessions.reapIdle(10), "BUSY session past TTL is never reaped"); @@ -281,7 +282,7 @@ class SessionManagerTest { MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnComplete(terminal); clock[0] = 21; @@ -299,7 +300,7 @@ class SessionManagerTest { MemberSession busy = sessions.acquire("ltms-local", "/busy", "/caller", "owner2"); sessions.asPresence().markPresent(ready.terminalId()); sessions.asPresence().markPresent(busy.terminalId()); - sessions.onDelivered(busy.terminalId()); + sessions.onDelivered(busy.terminalId(), TestTurnTokens.inert(busy.terminalId())); clock[0] = 50; assertEquals(1, sessions.reapIdle(30), "only READY past TTL is reaped"); @@ -317,9 +318,9 @@ class SessionManagerTest { String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnComplete(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnComplete(terminal); MemberSession updated = sessions.get(session.paneId()).orElseThrow(); @@ -337,13 +338,13 @@ class SessionManagerTest { String terminal = session.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnComplete(terminal); assertEquals(MemberSession.State.DONE, sessions.get(session.paneId()).orElseThrow().state(), "first turn completes without release"); - sessions.onDelivered(terminal); + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); sessions.onTurnComplete(terminal); assertTrue(sessions.get(session.paneId()).isEmpty(), "session released after cap reached"); @@ -359,7 +360,7 @@ class SessionManagerTest { MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); sessions.asPresence().markPresent(session.terminalId()); - sessions.onDelivered(session.terminalId()); + sessions.onDelivered(session.terminalId(), TestTurnTokens.inert(session.terminalId())); assertTrue(sessions.onTurnCompleteWithPostAction(session.terminalId())); MemberSession updated = sessions.get(session.paneId()).orElseThrow(); @@ -373,7 +374,7 @@ class SessionManagerTest { SessionManager sessions = sessionManager(herdr, () -> 0L, 1, true); MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); sessions.asPresence().markPresent(session.terminalId()); - sessions.onDelivered(session.terminalId()); + sessions.onDelivered(session.terminalId(), TestTurnTokens.inert(session.terminalId())); assertFalse(sessions.hasPostTurnAction(session.terminalId()), "a session at its cap will be released, not reset for reuse"); @@ -388,7 +389,7 @@ class SessionManagerTest { SessionManager sessions = sessionManager(herdr, () -> 0L, 0, false); MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); sessions.asPresence().markPresent(session.terminalId()); - sessions.onDelivered(session.terminalId()); + sessions.onDelivered(session.terminalId(), TestTurnTokens.inert(session.terminalId())); sessions.onTurnComplete(session.terminalId()); @@ -406,7 +407,7 @@ class SessionManagerTest { MemberSession busy = sessions.acquire("ltms-local", "/busy", "/caller", "ownerB"); sessions.asPresence().markPresent(ready.terminalId()); sessions.asPresence().markPresent(busy.terminalId()); - sessions.onDelivered(busy.terminalId()); + sessions.onDelivered(busy.terminalId(), TestTurnTokens.inert(busy.terminalId())); sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100)); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java index 45eb6f8..e0b07e7 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java @@ -7,6 +7,7 @@ import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.member.ClaudeCodeLauncher; +import dev.ltms.bridged.msg.TestTurnTokens; import dev.ltms.bridged.peer.MemberRole; import org.junit.jupiter.api.Test; @@ -204,7 +205,7 @@ class WorktreeSessionManagerTest { new WorktreeRequest("cb-544", null)); String terminal = s.terminalId(); sessions.asPresence().markPresent(terminal); - sessions.onDelivered(terminal); // BUSY, never completes → still BUSY when the timeout hits + sessions.onDelivered(terminal, TestTurnTokens.inert(terminal)); // BUSY, never completes → still BUSY when the timeout hits sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100)); From 9118ce253744c3c2c3ab54ff354fc86d757effd5 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 07:14:17 +0200 Subject: [PATCH 17/20] CB-576: release preserves a dirty worktree instead of deleting it --- .../ltms/bridged/session/GitWorktrees.java | 8 ++++ .../ltms/bridged/session/SessionManager.java | 10 ++++ .../dev/ltms/bridged/session/Worktrees.java | 8 ++++ .../ltms/bridged/session/FakeWorktrees.java | 12 +++++ .../bridged/session/GitWorktreesTest.java | 25 ++++++++++ .../session/WorktreeSessionManagerTest.java | 48 +++++++++++++++++++ docs/M4-Fleet-Health.md | 2 +- 7 files changed, 112 insertions(+), 1 deletion(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java index c574c5b..9dbcbd1 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java @@ -167,6 +167,14 @@ public final class GitWorktrees implements Worktrees { exec("git", "-C", repoRoot, "worktree", "remove", "--force", worktreePath); } + @Override + public boolean hasUncommitted(String worktreePath) { + // No --untracked-files=no: the exact shape of the work lost in CB-576 was a new file + // that was never added, so an untracked-only worktree is still dirty. + String out = exec("git", "-C", worktreePath, "status", "--porcelain"); + return !out.isBlank(); + } + @Override public void overlayParity(String repoRoot, String worktreePath, List overlay) { if (overlay == null || overlay.isEmpty()) { diff --git a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java index 9ebc167..4f5bc72 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java @@ -201,6 +201,16 @@ public final class SessionManager implements TurnListener { removed.paneId(), removed.terminalId(), removed.state(), cause); if (preserveWorktree && removed.worktree() != null) { logPreservedForShutdown(removed); + } else if (removed.worktree() != null && worktrees.hasUncommitted(removed.worktree())) { + // CB-576: a release that would otherwise remove the worktree finds it holding + // uncommitted work the bridge cannot see. A worker that ends a turn without + // committing (normally because it stopped to ask a question or refused the turn) + // has its only copy of that work in the worktree. Remove would --force-delete it, + // so preserve the directory and tell an operator where to find it. + preserveWorktree = true; + log.warn("release {} preserves dirty worktree {} for pane={} terminal={}: " + + "the worktree holds uncommitted changes that --force remove would destroy", + cause, removed.worktree(), removed.paneId(), removed.terminalId()); } // CB-516: a send still waiting on this worker can never be answered now. Tell the // listener BEFORE the pane is torn down, so a blocked caller fails fast with a real diff --git a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java index eaef94e..f858bf0 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java @@ -10,6 +10,14 @@ public interface Worktrees { /** git -C worktree remove --force . Idempotent (already-gone tolerated). */ void remove(String repoRoot, String worktreePath); + /** + * True when the worktree holds uncommitted changes the bridge cannot see: tracked + * modifications, staged files, or untracked files. {@code git status --porcelain} is the + * test; an empty result means clean. Callers use this to decide whether removing the + * worktree would silently destroy a worker's only copy of its work. + */ + boolean hasUncommitted(String worktreePath); + /** Copy each existing overlay path repoRoot→worktree; mark tracked ones --skip-worktree. */ void overlayParity(String repoRoot, String worktreePath, List overlay); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java b/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java index 4aa1dd3..c9b8d9d 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/FakeWorktrees.java @@ -29,6 +29,7 @@ public final class FakeWorktrees implements Worktrees { private final Set existingPaths = ConcurrentHashMap.newKeySet(); private final Set trackedPaths = ConcurrentHashMap.newKeySet(); private volatile RuntimeException addFailure; + private volatile boolean dirty = false; private volatile String repoRoot = "/repo"; private volatile String prefix = "/worktrees"; @@ -61,6 +62,12 @@ public final class FakeWorktrees implements Worktrees { return this; } + /** Mark the worktree dirty so {@link #hasUncommitted} reports true (simulates uncommitted work). */ + public FakeWorktrees withDirty(boolean dirty) { + this.dirty = dirty; + return this; + } + @Override public String add(String repoRoot, String branch, String baseRef) { addCalls.add(new AddCall(repoRoot, branch, baseRef)); @@ -77,6 +84,11 @@ public final class FakeWorktrees implements Worktrees { removeCalls.add(new RemoveCall(repoRoot, worktreePath)); } + @Override + public boolean hasUncommitted(String worktreePath) { + return dirty; + } + @Override public void overlayParity(String repoRoot, String worktreePath, List overlay) { List copied = new java.util.ArrayList<>(); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java index 3ebd8c2..b5c6a83 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java @@ -175,6 +175,31 @@ class GitWorktreesTest { assertTrue(Files.exists(Path.of(wt).resolve(".mcp.json")), ".mcp.json stub was dropped"); } + /** + * CB-576. {@code hasUncommitted} must treat a freshly-provisioned worktree as clean, but a + * worktree holding a brand-new, never-added file as dirty. The untracked-file-only shape is + * exactly the work lost in the incident — a worker's draft that compiled but was never + * committed because it stopped to ask its lead a question. + */ + @Test + void anUntrackedOnlyWorktreeCountsAsDirty(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String wt = gitWorktrees.add(repo.toString(), "cb-576-u", "HEAD"); + + assertFalse(gitWorktrees.hasUncommitted(wt), + "a freshly provisioned worktree must read as clean"); + + Files.writeString(Path.of(wt).resolve("brand-new.txt"), "draft that was never added\n"); + + assertTrue(gitWorktrees.hasUncommitted(wt), + "an untracked-only file must count as dirty"); + + Files.writeString(Path.of(wt).resolve("README.md"), "edited tracked file\n"); + assertTrue(gitWorktrees.hasUncommitted(wt), + "a tracked modification must also count as dirty"); + } + /** All three protected configs are covered: each one present in a worktree is neutralized and hidden. */ @Test void allThreeConfigsAreNeutralizedWhenPresent(@TempDir Path tmp) throws Exception { diff --git a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java index e0b07e7..5fa720b 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java @@ -6,10 +6,15 @@ import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.herdr.WorkspaceControl; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import dev.ltms.bridged.member.ClaudeCodeLauncher; import dev.ltms.bridged.msg.TestTurnTokens; import dev.ltms.bridged.peer.MemberRole; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import java.util.List; import java.util.Map; @@ -179,6 +184,49 @@ class WorktreeSessionManagerTest { assertTrue(sessions.get(paneId).isEmpty(), "released session is no longer retrievable"); } + /** + * CB-576. A normal {@code COMPLETED} release whose worktree holds uncommitted work must NOT + * remove it — {@code --force} would destroy the worker's only copy. The bridge cannot see + * uncommitted files, so the worktree is preserved and the release logged at WARN naming the + * path, the session, and the cause an operator needs to find the work. + */ + @Test + void releasePreservesDirtyWorktreeAndLogsWarn() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt") + .withDirty(true); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null, + new WorktreeRequest("cb-576", null)); + + LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory(); + ch.qos.logback.classic.Logger sessionLog = + (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class); + ListAppender appender = new ListAppender<>(); + appender.setContext(ctx); + appender.start(); + sessionLog.addAppender(appender); + sessionLog.setLevel(Level.WARN); + try { + sessions.release(s.paneId()); + + assertTrue(herdr.called("pane.close"), "release still tears the worker pane down"); + assertTrue(worktrees.removeCalls().isEmpty(), + "a dirty worktree is never removed — it holds the only copy of the work"); + String warn = appender.list.stream() + .filter(e -> e.getLevel().equals(Level.WARN)) + .map(ILoggingEvent::getFormattedMessage) + .filter(m -> m.contains("dirty worktree")) + .findFirst() + .orElse("no dirty-release WARN logged"); + assertTrue(warn.contains(s.worktree()), "the WARN names the worktree path: " + warn); + assertTrue(warn.contains(s.terminalId()), "the WARN names the session: " + warn); + assertTrue(warn.contains("COMPLETED"), "the WARN names the release cause: " + warn); + } finally { + sessionLog.detachAppender(appender); + } + } + @Test void drainAllPreservesWorktreeOfIdleSession() { FakeHerdr herdr = new FakeHerdr(); diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md index f954a99..c605172 100644 --- a/docs/M4-Fleet-Health.md +++ b/docs/M4-Fleet-Health.md @@ -237,7 +237,7 @@ state never presents stop as the only action. | Release cause | Process action | Provisioned worktree | |---|---|---| | `SPAWN_ROLLBACK` before registration or delivery | Stop and clean up | Remove | -| `COMPLETED` for `READY` or `DONE` without pending work, idle TTL, or successful context-cap completion | Stop | Remove under completed policy | +| `COMPLETED` for `READY` or `DONE` without pending work, idle TTL, or successful context-cap completion | Stop | Remove only if clean; preserve a dirty worktree (CB-576) | | `NEVER_READY` | Stop | Preserve | | `GONE` | Best-effort stop | Preserve | | `TURN_FAILED` or lead abort while `BUSY` or `FAILED` | Stop | Preserve | From 0af902ec438885a7e1d478d410672a01fc2791c2 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 07:54:28 +0200 Subject: [PATCH 18/20] CB-580: fail a ticket when its member reaches a terminal health state FleetHealthMonitor now requires a failTarget BiConsumer collaborator (no defaulting overload) and calls it exactly once when a member transitions into GONE or NEVER_READY, via CB-568's idempotent target-wide abandon() operation. The reason string names the real terminal state. failTarget invocation retries up to MAX_FAIL_TARGET_ATTEMPTS (3) within the same transition if it throws, and never refires on a later tick where the state is unchanged. Bridged.java wires messages::abandon as the production failTarget. --- .../main/java/dev/ltms/bridged/Bridged.java | 4 +- .../bridged/health/FleetHealthMonitor.java | 46 ++++++++- .../health/FleetHealthMonitorTest.java | 95 ++++++++++++++++++- 3 files changed, 140 insertions(+), 5 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 70a10d9..e38fa28 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -360,8 +360,10 @@ public final class Bridged { var healthScheduler = Executors.newSingleThreadScheduledExecutor(r -> Thread.ofVirtual().name("bridge-health-").unstarted(r)); if (cfg.health() != null && cfg.health().isEnabled()) { + // CB-580: a member found GONE/NEVER_READY must fail whatever ticket is waiting on it, + // through the same idempotent target-wide operation CB-516 already uses on release. healthMonitor = new FleetHealthMonitor(agents, sessions::roster, messages, healthScheduler, - System::nanoTime, cfg.health().intervalOrDefault()); + System::nanoTime, cfg.health().intervalOrDefault(), messages::abandon); String coverage = FleetHealthMonitor.coverage(true, cfg.health().notifications() != null && cfg.health().notifications().configured()); if ("detection-only".equals(coverage)) { diff --git a/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java index 79e4f84..f9ecd81 100644 --- a/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java +++ b/bridged/src/main/java/dev/ltms/bridged/health/FleetHealthMonitor.java @@ -12,34 +12,50 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; import java.util.function.LongSupplier; import java.util.function.Supplier; /** Slow whole-fleet evidence collection. It is deliberately separate from the delivery poller. */ public final class FleetHealthMonitor { private static final Logger log = LoggerFactory.getLogger(FleetHealthMonitor.class); + + /** Bounded attempts to run {@link #failTarget} for one transition. Never retried tick-to-tick (CB-580). */ + static final int MAX_FAIL_TARGET_ATTEMPTS = 3; + private final AgentControl agents; private final Supplier> roster; private final MessageService messages; private final ScheduledExecutorService scheduler; private final LongSupplier clock; private final long intervalSeconds; + private final BiConsumer failTarget; private final Map priors = new HashMap<>(); private final Map states = new HashMap<>(); // These facts need the evidence publishers introduced by later M4 units. They are not negatives. private static final boolean NOT_YET_OBSERVED = false; + /** + * @param failTarget CB-568's idempotent target-wide failure operation (e.g. {@code messages::abandon}), + * invoked once when a member transitions into a terminal health state. Required — + * there is deliberately no defaulting overload; a caller that does not want the + * fail-tickets-on-terminal-health behavior must pass an explicit inert value (see + * {@code TestTurnTokens.inert} / {@code BridgeMcp.CapacitySource.none()} for the pattern). + */ public FleetHealthMonitor(AgentControl agents, Supplier> roster, MessageService messages, - ScheduledExecutorService scheduler, LongSupplier clock, long intervalSeconds) { + ScheduledExecutorService scheduler, LongSupplier clock, long intervalSeconds, + BiConsumer failTarget) { this.agents = agents; this.roster = roster; this.messages = messages; this.scheduler = scheduler; this.clock = clock; this.intervalSeconds = intervalSeconds; + this.failTarget = Objects.requireNonNull(failTarget, "failTarget"); } /** Pure per-member decision seam. */ @@ -91,6 +107,34 @@ public final class FleetHealthMonitor { } else if (previous != null && fault(previous)) { log.info("fleet health member={} recovered state={} previous={}", target, next, previous); } + // CB-580: a member entering GONE/NEVER_READY must not leave its waiting tickets pending + // forever. Fire exactly once per transition — never on a tick where the state is unchanged, + // which is what made the rejected commit call abandon() once per tick for as long as a + // member stayed terminal. + if (terminal(next)) { + failTerminalTarget(target, next); + } + } + + private void failTerminalTarget(String target, HealthState state) { + String reason = "fleet health: member reached terminal state " + state.name(); + RuntimeException last = null; + for (int attempt = 1; attempt <= MAX_FAIL_TARGET_ATTEMPTS; attempt++) { + try { + failTarget.accept(target, reason); + return; + } catch (RuntimeException error) { + last = error; + log.warn("fleet health: failTarget attempt {}/{} failed for member={} state={}", + attempt, MAX_FAIL_TARGET_ATTEMPTS, target, state, error); + } + } + log.warn("fleet health: giving up on failTarget for member={} state={} after {} attempts", + target, state, MAX_FAIL_TARGET_ATTEMPTS, last); + } + + private static boolean terminal(HealthState state) { + return state == HealthState.GONE || state == HealthState.NEVER_READY; } private static boolean fault(HealthState state) { diff --git a/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java index 4d90b07..28c53e2 100644 --- a/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/health/FleetHealthMonitorTest.java @@ -20,8 +20,10 @@ import org.slf4j.LoggerFactory; import java.util.Map; import java.util.Set; import java.util.concurrent.Executors; +import java.util.function.BiConsumer; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class FleetHealthMonitorTest { @Test void oneTickUsesOneFleetListForAnyRosterSize() { @@ -37,7 +39,8 @@ class FleetHealthMonitorTest { herdr.calls.clear(); MessageService messages = new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()); var scheduler = Executors.newSingleThreadScheduledExecutor(); - FleetHealthMonitor monitor = new FleetHealthMonitor(agents, sessions::roster, messages, scheduler, () -> 1, 60); + FleetHealthMonitor monitor = new FleetHealthMonitor(agents, sessions::roster, messages, scheduler, () -> 1, 60, + (_, _) -> { }); monitor.tick(); monitor.stop(); assertEquals(1, herdr.calls.stream().filter(call -> call.method().equals("agent.list")).count()); @@ -49,7 +52,7 @@ class FleetHealthMonitorTest { var scheduler = Executors.newSingleThreadScheduledExecutor(); FleetHealthMonitor monitor = new FleetHealthMonitor(agents, java.util.List::of, new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()), - scheduler, () -> 1, 60); + scheduler, () -> 1, 60, (_, _) -> { }); monitor.tick(); herdr.healthy(true); monitor.tick(); @@ -68,7 +71,7 @@ class FleetHealthMonitorTest { var scheduler = Executors.newSingleThreadScheduledExecutor(); FleetHealthMonitor monitor = new FleetHealthMonitor(agents, java.util.List::of, new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()), - scheduler, () -> 1, 60); + scheduler, () -> 1, 60, (_, _) -> { }); monitor.reportTransition("term_a", HealthState.TURN_BOUNDARY_LOST); monitor.reportTransition("term_a", HealthState.TURN_BOUNDARY_LOST); monitor.stop(); @@ -78,4 +81,90 @@ class FleetHealthMonitorTest { logger.detachAppender(appender); } } + + // --- CB-580: a member that reaches GONE/NEVER_READY must fail its waiting tickets + + private static FleetHealthMonitor monitorWith(BiConsumer failTarget) { + FakeHerdr herdr = new FakeHerdr(); + AgentControl agents = new AgentControl(herdr); + var scheduler = Executors.newSingleThreadScheduledExecutor(); + return new FleetHealthMonitor(agents, java.util.List::of, + new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()), + scheduler, () -> 1, 60, failTarget); + } + + @Test void terminalTransitionFailsTheTargetOnce() { + RecordingFailTarget failTarget = new RecordingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.stop(); + assertEquals(1, failTarget.calls.size()); + assertEquals("term_a", failTarget.calls.get(0).target()); + assertTrue(failTarget.calls.get(0).reason().contains("GONE")); + } + + @Test void neverReadyNamesItselfAsTheReason() { + RecordingFailTarget failTarget = new RecordingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.NEVER_READY); + monitor.stop(); + assertEquals(1, failTarget.calls.size()); + assertTrue(failTarget.calls.get(0).reason().contains("NEVER_READY")); + } + + @Test void stayingInATerminalStateProducesOneFailureNotN() { + RecordingFailTarget failTarget = new RecordingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.stop(); + assertEquals(1, failTarget.calls.size()); + } + + @Test void aNonTerminalFaultStateDoesNotFailTheTarget() { + RecordingFailTarget failTarget = new RecordingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.TURN_BOUNDARY_LOST); + monitor.stop(); + assertEquals(0, failTarget.calls.size()); + } + + @Test void failTargetRetryIsBounded() { + AlwaysThrowingFailTarget failTarget = new AlwaysThrowingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.GONE); + monitor.stop(); + assertEquals(FleetHealthMonitor.MAX_FAIL_TARGET_ATTEMPTS, failTarget.calls); + } + + @Test void exhaustedRetryStillDoesNotRefireOnAnUnchangedTick() { + AlwaysThrowingFailTarget failTarget = new AlwaysThrowingFailTarget(); + FleetHealthMonitor monitor = monitorWith(failTarget); + monitor.reportTransition("term_a", HealthState.GONE); + int afterFirstTransition = failTarget.calls; + monitor.reportTransition("term_a", HealthState.GONE); + monitor.stop(); + assertEquals(afterFirstTransition, failTarget.calls); + } + + private record RecordedCall(String target, String reason) { } + + private static final class RecordingFailTarget implements BiConsumer { + final java.util.List calls = new java.util.ArrayList<>(); + + @Override public void accept(String target, String reason) { + calls.add(new RecordedCall(target, reason)); + } + } + + private static final class AlwaysThrowingFailTarget implements BiConsumer { + int calls = 0; + + @Override public void accept(String target, String reason) { + calls++; + throw new RuntimeException("boom"); + } + } } From 976eff8ad1d8f63b0f9ecfeaf39b192a9642da5e Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 08:05:21 +0200 Subject: [PATCH 19/20] CB-579: resolve a lead by its tab name, drop the terminal-id pin Leader.terminal -> Leader.tab (exact tab label, case-insensitive match). LeadTabScanner matches an exact tab->name map instead of stripping a shared tabPrefix, and no longer merges configured leads into every scan result -- a stale pin can no longer outlive its tab. LeadLauncher.tabLabel() returns the configured tab directly; the terminalId pinned-terminal fallback in liveLeads() is gone. Config load now rejects a leftover fleet.leaders.*.terminal key instead of silently ignoring it. primary.terminal is untouched. --- bridged/bridged.example.yaml | 33 ++-- .../main/java/dev/ltms/bridged/Bridged.java | 42 ++--- .../ltms/bridged/config/BridgedConfig.java | 102 +++++++---- .../dev/ltms/bridged/config/ConfigRef.java | 6 +- .../ltms/bridged/herdr/LeadTabScanner.java | 61 ++++--- .../dev/ltms/bridged/lead/LeadLauncher.java | 44 ++--- .../bridged/config/BridgedConfigTest.java | 164 +++++++++++------- .../bridged/herdr/LeadTabScannerTest.java | 132 ++++++++++---- .../ltms/bridged/lead/LeadLauncherTest.java | 51 +++--- 9 files changed, 399 insertions(+), 236 deletions(-) diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 6bd6c0f..35630a2 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -47,15 +47,18 @@ bind: # (say a Claude lead and an opencode lead) work as peers: the second is silently demoted and refused # every orchestration call. List each lead's pane here and all of them resolve as leads. # -# terminal → the ONLY field identity depends on; get it from that session's bridge_whoami +# tab → the ONLY field identity depends on (CB-579); the exact label of the tab hosting the lead. +# Label the tab yourself, or let bridged label one it launches — see `fleet.leaders:` below. # kind/model → descriptive; they document what runs in the pane and are echoed by bridge_whoami # -# A lead is never spawned — it pre-exists, which is exactly why it must be named rather than created. +# A lead's tab must already carry its label (or be launched by bridged, which labels it) — there is +# no terminal id to paste in and nothing to re-pin when the session restarts: the tab survives, so +# the same label resolves the same lead again on the next scan. # `bridge_whoami` reports `{"role":"primary","leader":""}`; role stays "primary" because a lead # IS a primary for authorization, so nothing that keys on the role breaks. # # KEEP `primary:` when adding leads: it still addresses the CB-307 push loop, which needs a single -# destination for its nudges. If both name the same terminal, the `fleet.leaders:` entry wins. +# destination for its nudges, and is a separate mechanism from lead identity — see `fleet.leaders:`. # # Leads are configured under `fleet.leaders:` — see THE FLEET further down. # @@ -309,15 +312,18 @@ fleet: # Panes that orchestrate rather than are orchestrated. A lead may now be CREATED as well as # recognised: give it a `profile:` and the daemon launches the shortfall when fewer than - # `instances` are live. Give it only a `terminal:` and it is recognise-only, as before. + # `instances` are live. Omit `profile:` and it is recognise-only, as before. # - # `tabPrefix` is the naming convention that finds a lead without pasting a terminal id: label the - # tab `lead: ` when you open it and the pane is recognised on the next rescan. Reopen the - # tab later and the id changes; the label does not. + # `tab:` (CB-579) is REQUIRED and is the only field identity depends on — the exact label of the + # tab hosting the lead, matched case-insensitively. Label the tab yourself and put that same + # string here, and the pane is recognised on the next rescan. Reopen the tab later, or the session + # inside it restarts — the terminal id changes; the tab, and its label, do not, so no config edit + # follows a restart. # - # A lead the daemon launches is labelled BY the daemon, using the same convention, so it is found + # A lead the daemon launches is labelled BY the daemon with this same `tab:` value, so it is found # by the same scan. A lead counts as live only when herdr also reports a running agent in that - # tab — a label left behind by a session that died does not block the relaunch. + # tab — a label left behind by a session that died does not block the relaunch, and a tab that is + # gone entirely drops out of the next scan rather than being remembered forever. # # An auto-launched lead is NOT a member: it gets no worker reply charter, is never registered with # the session lifecycle (the idle reaper would kill your orchestrator), and stays on the @@ -326,10 +332,9 @@ fleet: # opus-5.0: # profile: opus # omit to never create this lead, only recognise it # instances: 1 # desired live count; only the shortfall is launched. 0 = off - # terminal: term_0123456789abcd # optional hand-pin; usually found by tabPrefix instead. - # # A running agent on this terminal also counts as live, so a - # # lead you opened by hand is not relaunched under you. - # tabPrefix: "lead:" # `lead: opus-5.0` ⇒ a lead named opus-5.0 (case-insensitive) + # tab: "lead: opus-5.0" # REQUIRED — the exact tab label this lead lives in + # tabPrefix: "lead:" # only used to guard against a worker tabLabel colliding with + # # this convention at startup; plays no part in matching a lead # scanIntervalSeconds: 10 # rescan cadence, and the worst case before a new tab is seen # workspace: leads # where a launched lead's tab is created (default "leads"). # # MUST NOT be a member workspace — those are excluded from the @@ -337,7 +342,7 @@ fleet: # cwd: /path/to/repo # the launched lead's working directory (default: bridged's own) # kind: claude # descriptive; reported by bridge_whoami # gpt-sol-5.6: - # terminal: term_fedcba9876543 + # tab: "lead: gpt-sol-5.6" # kind: opencode # model: openai/gpt-5.6-terra diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 70a10d9..3b79210 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -193,34 +193,34 @@ public final class Bridged { if (leadTerminals.size() > 1) { log.info("leads: {} panes recognised {}", leadTerminals.size(), leadTerminals.values()); } - // CB-531: on top of the static registry, discover leads by the tab labels the operator - // writes. CB-557 moved the settings onto the lead they describe, so scanning is on whenever - // a `fleet.leaders:` entry exists — with no leads configured the supplier is a constant and - // never touches herdr, exactly as a missing `leadScan:` block used to behave. + // CB-531: on top of the legacy primary.terminal pin, discover leads by the tab labels the + // operator writes. CB-557 moved the settings onto the lead they describe, so scanning is on + // whenever a `fleet.leaders:` entry exists — with no leads configured the supplier is a + // constant and never touches herdr, exactly as a missing `leadScan:` block used to behave. + // CB-579: each lead now names its own exact `tab:` label, so one scanner discovers every + // configured lead regardless of how differently their tabs are labelled — the old + // single-shared-tabPrefix limitation (and its warning) is gone. final Supplier> leads; var leaders = cfg.fleet().leaders(); if (!leaders.isEmpty()) { - // One scanner, so one prefix and one interval. Distinct per-lead prefixes would need a - // scanner each; until a config actually wants that, take the first entry's settings and - // say so, rather than silently honouring one lead's prefix and dropping another's. - var scan = leaders.values().iterator().next(); Set memberSpaces = cfg.profiles().values().stream() .map(BridgedConfig.Profile::workspace) .filter(Objects::nonNull) .collect(Collectors.toSet()); - leads = new LeadTabScanner(herdr, scan.tabPrefix(), memberSpaces, leadTerminals, - TimeUnit.SECONDS.toNanos(scan.scanIntervalSeconds()), System::nanoTime); - log.info("lead scan: tabs labelled '{}…' host a lead (rescan every {}s, member spaces {} " - + "excluded)", - scan.tabPrefix(), scan.scanIntervalSeconds(), memberSpaces); - long distinctPrefixes = leaders.values().stream() - .map(BridgedConfig.Leader::tabPrefix).distinct().count(); - if (distinctPrefixes > 1) { - log.warn("fleet.leaders declares {} different tabPrefix values; only '{}' is scanned " - + "for. Give every lead the same tabPrefix, or leads under the others " - + "will not be discovered.", - distinctPrefixes, scan.tabPrefix()); - } + Map tabToName = new LinkedHashMap<>(); + leaders.forEach((name, leader) -> { + if (leader != null && leader.tab() != null && !leader.tab().isBlank()) { + tabToName.put(leader.tab(), name); + } + }); + // One shared rescan cadence: still taken from the first entry, as before — it is an + // operational cadence, not identity, so there is no correctness reason to give every + // lead its own scanner. + int scanIntervalSeconds = leaders.values().iterator().next().scanIntervalSeconds(); + leads = new LeadTabScanner(herdr, tabToName, memberSpaces, + TimeUnit.SECONDS.toNanos(scanIntervalSeconds), System::nanoTime); + log.info("lead scan: tabs {} host a lead (rescan every {}s, member spaces {} excluded)", + tabToName.keySet(), scanIntervalSeconds, memberSpaces); } else { leads = () -> leadTerminals; } diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index d072ff0..30f74b2 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -458,24 +458,32 @@ public record BridgedConfig( * pre-existed, which is why it had to be recognised by configuration rather than created. With * {@code profile} and {@code instances} the daemon may stand one up when none is live, so the * pane no longer has to exist before the daemon does. Recognition still comes first: a lead - * already running under {@code tabPrefix} is adopted, and only the shortfall is launched. + * already running in its configured {@code tab} is adopted, and only the shortfall is launched. + * + *

{@code tab} replaced {@code terminal} (CB-579). A herdr {@code terminal_id} changes + * every time the lead's session restarts, so pinning one cost a config edit and a daemon restart + * per restart. A tab is stable: a human opens it once, it holds exactly one pane, and its label + * survives restarts of the agent inside it — so identity is now the tab label alone. * * @param profile the {@code profiles:} entry to launch this lead on when one must * be created; {@code null} ⇒ recognise-only, never create - * @param terminal the lead's herdr {@code terminal_id} when pinned by hand; the only - * field identity depends on. {@code null} ⇒ found by {@code tabPrefix} + * @param tab the exact tab label hosting this lead, matched case-insensitively; + * the only field identity depends on. Required — a lead with no + * {@code tab} can never be discovered, launched or not * @param instances how many of this lead should be live (default 1). The daemon * launches only the shortfall, so a restart adopts rather than doubles - * @param tabPrefix label prefix marking this lead's tab, matched case-insensitively; - * the remainder is the lead's name ({@code "lead: opus"} → - * {@code opus}). Default {@code "lead:"} + * @param tabPrefix no longer used to find a lead's tab — {@code tab} is matched + * exactly. Its only remaining job is the startup collision guard + * ({@link #validateLeadTabPrefixes()}), which still uses it to refuse + * a worker {@code tabLabel} template that could be misread as a lead. + * Default {@code "lead:"} * @param scanIntervalSeconds how long a tab scan is cached before herdr is asked again; also the * worst case before a newly-labelled tab is recognised. Default 10 * @param kind which agent runs there ({@code claude}, {@code opencode}, …) * @param model the model or selector it runs, for operators reading the roster */ @JsonIgnoreProperties(ignoreUnknown = true) - public record Leader(String profile, String terminal, Integer instances, String tabPrefix, + public record Leader(String profile, String tab, Integer instances, String tabPrefix, Integer scanIntervalSeconds, String kind, String model, String workspace, String cwd) { @@ -493,12 +501,13 @@ public record BridgedConfig( (scanIntervalSeconds == null || scanIntervalSeconds <= 0) ? 10 : scanIntervalSeconds; workspace = (workspace == null || workspace.isBlank()) ? DEFAULT_WORKSPACE : workspace.strip(); + tab = (tab == null || tab.isBlank()) ? null : tab.strip(); } /** Back-compat 7-arg form — no workspace or cwd, so both take their defaults. */ - public Leader(String profile, String terminal, Integer instances, String tabPrefix, + public Leader(String profile, String tab, Integer instances, String tabPrefix, Integer scanIntervalSeconds, String kind, String model) { - this(profile, terminal, instances, tabPrefix, scanIntervalSeconds, kind, model, null, null); + this(profile, tab, instances, tabPrefix, scanIntervalSeconds, kind, model, null, null); } /** True when this lead may be launched by the daemon rather than only recognised. */ @@ -506,9 +515,9 @@ public record BridgedConfig( return profile != null && !profile.isBlank() && instances > 0; } - /** The tab label an auto-launched instance of this lead gets — what the scanner reads back. */ - public String tabLabel(String name) { - return tabPrefix + " " + name; + /** The tab label an auto-launched instance of this lead gets — its configured {@code tab}. */ + public String tabLabel() { + return tab; } } @@ -707,28 +716,20 @@ public record BridgedConfig( } /** - * The terminal → lead-name map that {@link dev.ltms.bridged.auth.CallerResolver} resolves - * against, merging the {@code leaders:} registry with the legacy singular {@code primary:} pin. + * The terminal → lead-name map seeded from the legacy singular {@code primary:} pin (CB-530). * - *

Precedence: an explicit {@code leaders:} entry wins over the {@code primary:} pin for the - * same terminal. The pin is the older, less expressive spelling of the same fact, so when both - * name a pane the named entry is the one an operator meant. The pin is still honoured on its - * own — a config carrying only {@code primary:} behaves exactly as it did before CB-530. + *

{@code fleet.leaders} no longer carries a per-entry terminal pin (CB-579): a lead's identity + * comes from its {@code tab} alone, resolved live by {@code LeadTabScanner}. This method now + * exists only for the {@code primary.terminal} fallback — a config that never migrated off it + * still resolves that one pane as a lead named {@code "primary"}, exactly as before CB-530. * - * @return an unmodifiable map, empty when neither block is configured (nothing is pinned, and - * every pane therefore resolves as a worker — the pre-CB-307 behaviour) + * @return an unmodifiable map, empty when {@code primary.terminal} is not configured (nothing is + * pinned, and every pane therefore resolves as a worker — the pre-CB-307 behaviour) */ public Map leaderTerminals() { Map byTerminal = new LinkedHashMap<>(); - if (fleet != null) { - fleet.leaders().forEach((name, leader) -> { - if (leader != null && leader.terminal() != null && !leader.terminal().isBlank()) { - byTerminal.put(leader.terminal(), name); - } - }); - } if (primary != null && primary.terminal() != null && !primary.terminal().isBlank()) { - byTerminal.putIfAbsent(primary.terminal(), "primary"); + byTerminal.put(primary.terminal(), "primary"); } return Collections.unmodifiableMap(byTerminal); } @@ -840,6 +841,7 @@ public record BridgedConfig( try { String yaml = Files.readString(path); rejectRenamedTopLevelKeys(yaml); + rejectLeaderTerminalKey(yaml); warnUnknownTopLevelKeys(yaml, path); rejectDuplicateMemberSlots(yaml); BridgedConfig cfg = YAML.readValue(yaml, BridgedConfig.class); @@ -1064,6 +1066,43 @@ public record BridgedConfig( } } + /** + * Reject a config whose {@code fleet.leaders.} still carries the retired {@code terminal:} + * pin (CB-579), naming {@code tab:} as its replacement. + * + *

{@code Leader} is {@code @JsonIgnoreProperties(ignoreUnknown = true)}, so simply dropping + * the record component would make a leftover {@code terminal:} key silently no-op — the daemon + * would start, the pin would never take effect, and nothing would say why. Fatal and specific + * instead, exactly like {@link #rejectRenamedTopLevelKeys}, which this mirrors for a key one + * level deeper than the ones that method covers. + * + * @param yaml the raw config text + * @throws IllegalStateException when any {@code fleet.leaders..terminal} key is present + */ + static void rejectLeaderTerminalKey(String yaml) { + Map raw; + try { + raw = YAML.readValue(yaml, Map.class); + } catch (IOException | IllegalArgumentException e) { + return; // a malformed file is reported by the real parse, not here + } + if (raw == null || !(raw.get("fleet") instanceof Map fleet) + || !(fleet.get("leaders") instanceof Map leaders)) { + return; + } + List bad = leaders.entrySet().stream() + .filter(e -> e.getValue() instanceof Map leader && leader.containsKey("terminal")) + .map(e -> String.valueOf(e.getKey())) + .sorted() + .toList(); + if (!bad.isEmpty()) { + throw new IllegalStateException("refusing to start: fleet.leaders entries [" + + String.join(", ", bad) + "] still use the retired 'terminal:' key — replace it " + + "with 'tab:', the exact tab label hosting the lead. A terminal_id changes on " + + "every restart of the lead's session; a tab label does not."); + } + } + static List unknownTopLevelKeys(String yaml) { Map raw; try { @@ -1313,10 +1352,9 @@ public record BridgedConfig( + "', which is not a configured profiles: entry (have: " + profiles.keySet() + ")."); } - if (!leader.isCreatable() && (leader.terminal() == null || leader.terminal().isBlank())) { - bad.add("fleet.leaders." + name + " can neither be found nor created — it pins no " - + "terminal: and names no profile: to launch one on. Give it one or the " - + "other, or drop the entry."); + if (leader.tab() == null || leader.tab().isBlank()) { + bad.add("fleet.leaders." + name + " has no tab: — a lead is now found (and, if " + + "auto-launched, labelled) purely by its tab, so every entry must name one."); } }); if (!bad.isEmpty()) { diff --git a/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java index 6393994..d145555 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java @@ -30,7 +30,11 @@ import java.util.function.Supplier; * {@code fleet:} (every role pool, {@code charters}, and {@code tabLabel}), * {@code placement:}, and an existing profile's {@code weight} / {@code maxLoad}. Those * three are read through a supplier on {@code CompositePeerLauncher}, which is what makes - * them hot — not the fact that they are config. + * them hot — not the fact that they are config. This does NOT include + * {@code fleet.leaders}: {@code Bridged.main} reads {@code cfg.fleet().leaders()} + * once at startup to build the {@code LeadTabScanner} and the {@code LeadLauncher}, and + * neither is reconstructed on reload — so a lead added, removed, or re-{@code tab}'d under + * {@code fleet.leaders} needs a restart, the same as any deferred key below. *

  • Deferred — accepted into the new snapshot, but the wiring built at startup * keeps the old value until a restart: {@code lifecycle:}, {@code leadHeartbeat:}, * {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code guard:}, diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java b/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java index 522d581..6297d75 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.function.LongSupplier; @@ -23,6 +24,15 @@ import java.util.function.Supplier; * by first starting the session and asking it. Scanning closes that loop: label the tab, and the * pane is recognised on the next resolve. * + *

    CB-579 — matched by name, not prefix. This used to strip one shared + * {@code tabPrefix} off a label to derive the lead's name, and merged a config-supplied + * {@code terminal_id} pin over every scan result so the pin could never expire. Both are gone: each + * lead now configures its own exact {@code tab} label ({@code fleet.leaders..tab}), so this + * class is handed a {@code tab → name} map up front and matches labels against it exactly + * (case-insensitively). There is no merge step — a scan result is the whole answer. That is the + * fix for the bug this replaces: a {@code terminal_id} pin surviving in config after the pane it + * named was gone, so the daemon kept treating a dead session as a live lead forever. + * *

    Direction of trust. The label names the lead; it never grants * anything a pane could take for itself. Three properties keep that honest: *

      @@ -48,7 +58,7 @@ import java.util.function.Supplier; * ever make a decision that removes something based on this map, add the same check. * The remaining hazard is an operator one — a worker {@code tabLabel} template that * happens to start with the same prefix would promote the whole fleet — and that is refused at - * startup by {@code BridgedConfig.validateLeadScan} rather than documented here. + * startup by {@code BridgedConfig.validateLeadTabPrefixes} rather than documented here. * *

      Caching. {@link #get()} is on the request path (every resolve), so the scan * is TTL-cached and a stale-but-valid map is preferred to a herdr round-trip. A failed scan keeps @@ -60,38 +70,46 @@ public final class LeadTabScanner implements Supplier> { private static final Logger log = LoggerFactory.getLogger(LeadTabScanner.class); private final HerdrClient herdr; - private final String tabPrefix; + private final Map tabToName; private final Set excludedWorkspaceLabels; - private final Map configuredLeads; private final long ttlNanos; private final LongSupplier clock; - private Map cached; + private Map cached = Map.of(); private long scannedAtNanos; private boolean everScanned; /** * @param herdr the herdr client to query ({@code workspace.list}, * {@code tab.list}, {@code pane.list} — all read-only) - * @param tabPrefix a tab whose label starts with this (case-insensitively) hosts a - * lead; the rest of the label, trimmed, is the lead's name + * @param tabToName every configured lead's exact tab label → its name + * ({@code fleet.leaders..tab}), matched case-insensitively * @param excludedWorkspaceLabels workspaces never scanned — the configured worker spaces - * @param configuredLeads the static {@code leaders:}/{@code primary:} registry, merged - * over every scan result. Explicit config outranks the - * convention, and survives a scan that cannot run at all * @param ttlNanos how long a scan result is reused before the next one * @param clock nanosecond time source ({@code System::nanoTime} in production) */ - public LeadTabScanner(HerdrClient herdr, String tabPrefix, Set excludedWorkspaceLabels, - Map configuredLeads, long ttlNanos, LongSupplier clock) { + public LeadTabScanner(HerdrClient herdr, Map tabToName, + Set excludedWorkspaceLabels, long ttlNanos, LongSupplier clock) { this.herdr = herdr; - this.tabPrefix = tabPrefix == null || tabPrefix.isBlank() ? "lead:" : tabPrefix.strip(); + this.tabToName = normalize(tabToName); this.excludedWorkspaceLabels = excludedWorkspaceLabels == null ? Set.of() : Set.copyOf(excludedWorkspaceLabels); - this.configuredLeads = configuredLeads == null ? Map.of() : Map.copyOf(configuredLeads); this.ttlNanos = ttlNanos; this.clock = clock; - this.cached = this.configuredLeads; + } + + /** Keys stripped and lower-cased once, so every lookup is a plain map hit. */ + private static Map normalize(Map tabToName) { + if (tabToName == null || tabToName.isEmpty()) { + return Map.of(); + } + Map out = new LinkedHashMap<>(); + tabToName.forEach((tab, name) -> { + if (tab != null && !tab.isBlank() && name != null && !name.isBlank()) { + out.put(tab.strip().toLowerCase(Locale.ROOT), name); + } + }); + return Collections.unmodifiableMap(out); } /** @@ -151,25 +169,20 @@ public final class LeadTabScanner implements Supplier> { } } } - byTerminal.putAll(configuredLeads); // an explicit pin outranks a label return Collections.unmodifiableMap(byTerminal); } /** - * The lead name a tab label declares, or {@code null} if it declares none. + * The lead name a tab label declares, or {@code null} if it names none of the configured leads. * - *

      {@code "lead: opus-5.0"} → {@code "opus-5.0"}. A bare {@code "lead:"} names nobody and is - * rejected: an unnamed lead would resolve as {@code PRIMARY} with nothing to attribute it to. + *

      Exact match (case-insensitive, ends stripped) against {@link #tabToName} — no prefix + * stripping, so an operator's {@code "lead: something-else"} tab is never mistaken for a + * configured lead just because it shares a prefix. */ private String leadNameOf(String label) { if (label == null) { return null; } - String l = label.strip(); - if (!l.regionMatches(true, 0, tabPrefix, 0, tabPrefix.length())) { - return null; - } - String name = l.substring(tabPrefix.length()).strip(); - return name.isEmpty() ? null : name; + return tabToName.get(label.strip().toLowerCase(Locale.ROOT)); } } diff --git a/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java b/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java index 0693893..d6d2f83 100644 --- a/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java @@ -59,7 +59,7 @@ public final class LeadLauncher { /** * @param agents herdr agent control (start, list) * @param spaces workspace / tab control (ensure, create, label, list) - * @param cfg the loaded config — {@code fleet.leaders}, {@code profiles} and the lead pins + * @param cfg the loaded config — {@code fleet.leaders}, {@code profiles} and each lead's tab */ public LeadLauncher(AgentControl agents, WorkspaceControl spaces, BridgedConfig cfg) { this.agents = agents; @@ -102,8 +102,8 @@ public final class LeadLauncher { continue; } if (!lead.isCreatable()) { - // A lead with a `terminal:` pin and no `profile:` is recognise-only by design: the - // operator opens it by hand. Say so once rather than looking like a silent failure. + // A lead with a `tab:` but no `profile:` is recognise-only by design: the operator + // opens it by hand. Say so once rather than looking like a silent failure. log.info("lead '{}' is not live, and names no profile — it can be recognised but not " + "launched. Add `profile:` under fleet.leaders.{} to have bridged start it.", name, name); @@ -127,18 +127,16 @@ public final class LeadLauncher { } /** - * How many live leads exist per configured name. + * How many live leads exist per configured name: a running agent in a tab labelled with that + * lead's exact {@code tab} (CB-579). Member workspaces are excluded, exactly as the scanner + * excludes them: a member must not be counted as a lead because it happens to sit in a matching + * tab. * - *

      Two independent pieces of evidence, because either alone double-spawns: - *

        - *
      • a running agent in a tab labelled {@code " "} — how an auto-launched - * lead, or an operator following the labelling convention, is found;
        • - *
      • a running agent on a terminal the config pins in {@code fleet.leaders..terminal} — - * how a lead the operator opened and pinned by hand is found. Without this, a pinned lead - * whose tab carries no matching label would be relaunched on every boot.
        • - *
      - * Member workspaces are excluded, exactly as the scanner excludes them: a member must not be - * counted as a lead because it happens to sit in a matching tab. + *

      There used to be a second path here — a running agent on the terminal a + * {@code fleet.leaders..terminal} pin named, for a lead opened and pinned by hand. That + * pin is retired: {@code tab} is now the only field identity depends on, and {@link Agent} + * already carries {@link Agent#tabId()} directly, so a hand-opened lead is found the same way an + * auto-launched one is — by labelling its tab to match. */ private Map liveLeads(Map leaders) { Set memberSpaces = cfg.profiles().values().stream() @@ -160,20 +158,9 @@ public final class LeadLauncher { } } - // terminalId → the lead name the config pins it to. - Map nameByPinnedTerminal = new LinkedHashMap<>(); - leaders.forEach((name, lead) -> { - if (lead.terminal() != null && !lead.terminal().isBlank()) { - nameByPinnedTerminal.put(lead.terminal().strip(), name); - } - }); - Map counts = new LinkedHashMap<>(); for (Agent a : agents.list()) { String name = nameByTab.get(a.tabId()); - if (name == null) { - name = nameByPinnedTerminal.get(a.terminalId()); - } if (name != null) { counts.merge(name, 1, Integer::sum); } @@ -184,7 +171,7 @@ public final class LeadLauncher { /** * The configured lead a tab label names, or {@code null} for a label that names none. * - *

      Matched against the declared lead names rather than by splitting on the prefix, so an + *

      Matched exactly (case-insensitively) against each lead's configured {@code tab}, so an * operator's {@code "lead: something-else"} tab is not mistaken for a configured lead. */ private String leadNameOf(String label, Map leaders) { @@ -193,7 +180,8 @@ public final class LeadLauncher { } String l = label.strip(); for (Map.Entry e : leaders.entrySet()) { - if (l.equalsIgnoreCase(e.getValue().tabLabel(e.getKey()).strip())) { + String tab = e.getValue().tabLabel(); + if (tab != null && l.equalsIgnoreCase(tab.strip())) { return e.getKey(); } } @@ -202,7 +190,7 @@ public final class LeadLauncher { /** Start one lead. Returns false (having logged) rather than throwing on any failure. */ private boolean launch(String name, BridgedConfig.Leader lead, BridgedConfig.Profile profile) { - String label = lead.tabLabel(name); + String label = lead.tabLabel(); String cwd = (lead.cwd() == null || lead.cwd().isBlank()) ? System.getProperty("user.dir") : lead.cwd(); diff --git a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java index 52f2284..475bc37 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -194,7 +194,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" """); BridgedConfig.Leader lead = BridgedConfig.load(f).fleet().leaders().get("opus"); @@ -212,7 +212,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "drive: opus" tabPrefix: "drive:" scanIntervalSeconds: 30 """); @@ -224,7 +224,9 @@ class BridgedConfigTest { /** * The pane no longer has to exist before the daemon does (CB-557): a lead naming a profile may - * be launched, while one that names only a terminal is recognised and never created. + * be launched, while one that names no profile is recognised and never created. Either way it + * still needs its own {@code tab:} (CB-579) — that part is unconditional, see + * {@link #aLeadWithNoTabRefusesToStart}. */ @Test void aLeadIsCreatableOnlyWhenItNamesAProfile(@TempDir Path dir) throws Exception { @@ -239,8 +241,9 @@ class BridgedConfigTest { leaders: launched: profile: opus + tab: "lead: launched" pinned: - terminal: term_opus + tab: "lead: pinned" """); var leaders = BridgedConfig.load(f).fleet().leaders(); @@ -249,8 +252,12 @@ class BridgedConfigTest { "no profile to launch on ⇒ recognise-only, the pre-CB-557 behaviour"); } + /** + * CB-579: {@code tab} is the only field a lead's identity depends on now, so it is required + * whether the entry is creatable or recognise-only — without it the entry can never be found. + */ @Test - void aLeadThatCanBeNeitherFoundNorCreatedRefusesToStart(@TempDir Path dir) throws Exception { + void aLeadWithNoTabRefusesToStart(@TempDir Path dir) throws Exception { Path f = dir.resolve("useless-lead.yaml"); Files.writeString(f, """ bind: @@ -264,6 +271,80 @@ class BridgedConfigTest { IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers); assertTrue(e.getMessage().contains("ghost"), "the message must name the useless entry"); + assertTrue(e.getMessage().contains("tab:"), "the message must say what is missing"); + } + + /** + * CB-579 acceptance (2): a config still spelling {@code fleet.leaders..terminal} must fail + * loudly at load, not be silently dropped by {@code Leader}'s {@code @JsonIgnoreProperties}. + */ + @Test + void aLeaderTerminalKeyFailsLoadAndNamesTabAsTheReplacement(@TempDir Path dir) throws Exception { + Path f = dir.resolve("stale-terminal.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + terminal: term_opus + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("opus"), "the message must name the offending entry"); + assertTrue(e.getMessage().contains("tab:"), "the message must name the replacement key"); + assertTrue(e.getMessage().contains("terminal"), "the message must name the retired key"); + } + + /** The same refusal, and it must name every offending entry, not just the first. */ + @Test + void everyLeaderStillUsingTerminalIsReportedAtOnce(@TempDir Path dir) throws Exception { + Path f = dir.resolve("stale-terminals.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + terminal: term_opus + sol: + terminal: term_sol + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("opus")); + assertTrue(e.getMessage().contains("sol")); + } + + /** A {@code terminal:} anywhere else in the document (not under a leader entry) is unaffected. */ + @Test + void aTerminalKeyOutsideFleetLeadersIsNotRejected(@TempDir Path dir) throws Exception { + Path f = dir.resolve("primary-terminal-ok.yaml"); + Files.writeString(f, "bind:\n port: 8080\nprimary:\n terminal: term_fixed\n"); + + assertDoesNotThrow(() -> BridgedConfig.load(f)); + } + + /** CB-579 acceptance (3): distinct `tab:` labels need no shared prefix — one scanner finds both. */ + @Test + void twoLeadersWithDifferentTabsAreBothConfigured(@TempDir Path dir) throws Exception { + Path f = dir.resolve("two-tabs.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + tab: "lead: opus" + sol: + tab: "captain: sol" + """); + + var leaders = BridgedConfig.load(f).fleet().leaders(); + assertEquals("lead: opus", leaders.get("opus").tab()); + assertEquals("captain: sol", leaders.get("sol").tab()); } // ── CB-551: the idle-lead heartbeat ───────────────────────────────────────────────────────── @@ -328,7 +409,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" tabPrefix: "lead:" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -349,7 +430,7 @@ class BridgedConfigTest { tabLabel: "lead: {role} {profile}" leaders: opus: - terminal: term_opus + tab: "lead: opus" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -374,7 +455,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" """); assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadTabPrefixes()); @@ -400,7 +481,7 @@ class BridgedConfigTest { "a label that collides with a convention nobody reads is not a problem"); } - // ── CB-530: the leaders registry ──────────────────────────────────────────────────────────── + // ── CB-530/CB-579: the leaders registry ───────────────────────────────────────────────────── @Test void leadersBlockRegistersEveryPaneByName(@TempDir Path dir) throws Exception { @@ -411,10 +492,10 @@ class BridgedConfigTest { fleet: leaders: opus-5.0: - terminal: term_opus + tab: "lead: opus-5.0" kind: claude gpt-sol-5.6: - terminal: term_sol + tab: "lead: gpt-sol-5.6" kind: opencode model: openai/gpt-5.6-terra """); @@ -425,9 +506,9 @@ class BridgedConfigTest { assertEquals(Set.of("opus-5.0", "gpt-sol-5.6"), leaders.keySet()); assertEquals("opencode", leaders.get("gpt-sol-5.6").kind()); assertEquals("openai/gpt-5.6-terra", leaders.get("gpt-sol-5.6").model()); - // The whole point: BOTH panes resolve as leads, so neither is demoted to worker. - assertEquals(Map.of("term_opus", "opus-5.0", "term_sol", "gpt-sol-5.6"), - cfg.leaderTerminals()); + // Identity is the tab now (CB-579) — both entries carry their own, distinct label. + assertEquals("lead: opus-5.0", leaders.get("opus-5.0").tab()); + assertEquals("lead: gpt-sol-5.6", leaders.get("gpt-sol-5.6").tab()); } @Test @@ -439,42 +520,6 @@ class BridgedConfigTest { "configs that never migrate must behave exactly as they did before CB-530"); } - @Test - void anExplicitLeadersEntryWinsOverThePinForTheSameTerminal(@TempDir Path dir) throws Exception { - Path f = dir.resolve("both.yaml"); - Files.writeString(f, """ - bind: - port: 8080 - primary: - terminal: term_shared - fleet: - leaders: - opus-5.0: - terminal: term_shared - """); - - assertEquals(Map.of("term_shared", "opus-5.0"), BridgedConfig.load(f).leaderTerminals(), - "the pin is the older spelling of the same fact; the named entry is what was meant"); - } - - @Test - void bothBlocksTogetherRegisterTheUnionOfTheirTerminals(@TempDir Path dir) throws Exception { - Path f = dir.resolve("union.yaml"); - Files.writeString(f, """ - bind: - port: 8080 - primary: - terminal: term_pinned - fleet: - leaders: - gpt-sol-5.6: - terminal: term_sol - """); - - assertEquals(Map.of("term_pinned", "primary", "term_sol", "gpt-sol-5.6"), - BridgedConfig.load(f).leaderTerminals()); - } - @Test void neitherBlockLeavesNothingRegistered(@TempDir Path dir) throws Exception { Path f = dir.resolve("none.yaml"); @@ -483,22 +528,25 @@ class BridgedConfigTest { assertTrue(BridgedConfig.load(f).leaderTerminals().isEmpty()); } - /** A lead entry with no terminal identifies nothing — it must not register a null key. */ + /** + * CB-579: {@code fleet.leaders} no longer feeds {@code leaderTerminals()} at all — a lead's + * identity comes from the live tab scan, not a config-held terminal map. This method now exists + * only for the {@code primary.terminal} fallback. + */ @Test - void aLeadWithoutATerminalIsNotRegistered(@TempDir Path dir) throws Exception { - Path f = dir.resolve("no-terminal.yaml"); + void fleetLeadersNeverContributesToLeaderTerminals(@TempDir Path dir) throws Exception { + Path f = dir.resolve("leaders-only.yaml"); Files.writeString(f, """ bind: port: 8080 fleet: leaders: - sketch: - kind: opencode - real: - terminal: term_real + opus-5.0: + tab: "lead: opus-5.0" """); - assertEquals(Map.of("term_real", "real"), BridgedConfig.load(f).leaderTerminals()); + assertTrue(BridgedConfig.load(f).leaderTerminals().isEmpty(), + "no primary.terminal pin ⇒ nothing registered, even with fleet.leaders configured"); } // ── CB-548: the architects registry ──────────────────────────────────────────────────────── diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java index 8ccb2d6..b06c405 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java @@ -15,8 +15,9 @@ import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.*; /** - * CB-531. A lead is never spawned, so the daemon has to find it: these assert that an - * operator-labelled tab is what makes a pane a lead, and — just as importantly — what does not. + * CB-531/CB-579. A lead is never spawned, so the daemon has to find it: these assert that + * an operator-labelled tab matching a configured {@code tab:} is what makes a pane a lead, and — + * just as importantly — what does not, and that a stale entry does not linger forever. */ class LeadTabScannerTest { @@ -120,23 +121,48 @@ class LeadTabScannerTest { .pane("w9:p1", "w9:t1", "term_worker"); } - private LeadTabScanner scanner(TopologyHerdr herdr, Map configured, + /** The {@code tab:} → name map {@code twoLeads()}'s two lead tabs are configured under. */ + private static Map twoLeadsConfigured() { + return Map.of("lead: opus-5.0", "opus-5.0", "lead: gpt-sol-5.6", "gpt-sol-5.6"); + } + + private LeadTabScanner scanner(TopologyHerdr herdr, Map tabToName, AtomicLong clock) { - return new LeadTabScanner(herdr, "lead:", Set.of("bridged-workers"), configured, TTL, - clock::get); + return new LeadTabScanner(herdr, tabToName, Set.of("bridged-workers"), TTL, clock::get); } @Test - void everyLabelledTabBecomesALeadNamedByItsLabel() { - Map leads = scanner(twoLeads(), Map.of(), new AtomicLong()).get(); + void everyConfiguredTabBecomesALeadNamedByItsEntry() { + Map leads = scanner(twoLeads(), twoLeadsConfigured(), new AtomicLong()).get(); assertEquals(Map.of("term_opus", "opus-5.0", "term_gpt", "gpt-sol-5.6"), leads, - "two leads discovered from labels alone — no terminal_id was ever configured"); + "two leads discovered by their configured tab — no terminal_id was ever configured"); } @Test - void anUnlabelledTabContributesNothing() { - assertFalse(scanner(twoLeads(), Map.of(), new AtomicLong()).get().containsKey("term_notes")); + void anUnconfiguredTabContributesNothing() { + assertFalse(scanner(twoLeads(), twoLeadsConfigured(), new AtomicLong()) + .get().containsKey("term_notes")); + } + + /** + * CB-579: matching is exact against the configured map now, not a shared prefix — two leads with + * completely different labels are both discovered by one scanner, no convention required. + */ + @Test + void twoLeadsWithCompletelyDifferentLabelsAreBothDiscovered() { + TopologyHerdr herdr = new TopologyHerdr() + .workspace("w1", "main") + .tab("w1:t1", "w1", "orchestrator: opus") + .tab("w1:t2", "w1", "captain: sol") + .pane("w1:p1", "w1:t1", "term_opus") + .pane("w1:p2", "w1:t2", "term_sol"); + Map tabToName = Map.of("orchestrator: opus", "opus", "captain: sol", "sol"); + + Map leads = scanner(herdr, tabToName, new AtomicLong()).get(); + + assertEquals(Map.of("term_opus", "opus", "term_sol", "sol"), leads, + "no shared prefix needed — each lead is matched by its own configured tab"); } /** @@ -148,25 +174,28 @@ class LeadTabScannerTest { void aTabInAWorkerSpaceIsNeverALeadEvenWhenItsLabelMatches() { TopologyHerdr herdr = twoLeads().tab("w9:t2", "w9", "lead: impostor") .pane("w9:p2", "w9:t2", "term_impostor"); + Map tabToName = new LinkedHashMap<>(twoLeadsConfigured()); + tabToName.put("lead: impostor", "impostor"); - assertFalse(scanner(herdr, Map.of(), new AtomicLong()).get().containsKey("term_impostor")); + assertFalse(scanner(herdr, tabToName, new AtomicLong()).get().containsKey("term_impostor")); } @Test - void aBarePrefixNamesNobodyAndIsRejected() { + void aLabelWithNoConfiguredEntryIsIgnored() { TopologyHerdr herdr = new TopologyHerdr().workspace("w1", "main") - .tab("w1:t1", "w1", "lead:").pane("w1:p1", "w1:t1", "term_a"); + .tab("w1:t1", "w1", "lead: nobody-configured").pane("w1:p1", "w1:t1", "term_a"); - assertEquals(Map.of(), scanner(herdr, Map.of(), new AtomicLong()).get(), - "a lead with no name would resolve as PRIMARY with nothing to attribute it to"); + assertEquals(Map.of(), scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get(), + "a label that names no configured lead resolves nobody"); } @Test - void thePrefixMatchesCaseInsensitivelyAndTheNameIsTrimmed() { + void matchingIsCaseInsensitiveAndToleratesSurroundingWhitespace() { TopologyHerdr herdr = new TopologyHerdr().workspace("w1", "main") - .tab("w1:t1", "w1", " LEAD: opus-5.0 ").pane("w1:p1", "w1:t1", "term_a"); + .tab("w1:t1", "w1", " LEAD: Opus-5.0 ").pane("w1:p1", "w1:t1", "term_a"); - assertEquals(Map.of("term_a", "opus-5.0"), scanner(herdr, Map.of(), new AtomicLong()).get()); + assertEquals(Map.of("term_a", "opus-5.0"), + scanner(herdr, Map.of("lead: Opus-5.0", "opus-5.0"), new AtomicLong()).get()); } @Test @@ -175,18 +204,51 @@ class LeadTabScannerTest { // nothing bridged placed can land here (see the worker-space test above). TopologyHerdr herdr = twoLeads().pane("w1:p1b", "w1:t1", "term_opus_split"); - assertEquals("opus-5.0", scanner(herdr, Map.of(), new AtomicLong()).get().get("term_opus_split")); + assertEquals("opus-5.0", + scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get().get("term_opus_split")); } + /** + * CB-579 acceptance (6): this is the bug the ticket closes. A stale pin used to be merged back + * over every scan and never expire; now a scan is the whole answer, so a lead whose tab is gone + * drops out on the very next scan. + */ @Test - void anExplicitlyConfiguredLeadIsMergedInAndOutranksALabel() { - Map configured = Map.of("term_opus", "pinned-name", "term_extra", "from-config"); + void aTabNoLongerPresentDropsTheLeadOnTheNextScan() { + TopologyHerdr herdr = twoLeads(); + AtomicLong clock = new AtomicLong(); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); + assertTrue(s.get().containsKey("term_opus")); - Map leads = scanner(twoLeads(), configured, new AtomicLong()).get(); + // The session behind term_opus restarted — herdr no longer reports that tab or pane at all. + herdr.tabs.remove("w1:t1"); + herdr.panes.remove("w1:p1"); + clock.addAndGet(TTL); - assertEquals("pinned-name", leads.get("term_opus"), "an explicit pin is the operator's last word"); - assertEquals("from-config", leads.get("term_extra"), "a configured lead needs no tab at all"); - assertEquals("gpt-sol-5.6", leads.get("term_gpt")); + assertFalse(s.get().containsKey("term_opus"), + "a stale entry must expire once the tab it named is gone, not be merged back forever"); + } + + /** + * CB-579 acceptance (5): the whole point of matching by tab instead of {@code terminal_id} — a + * restart changes the terminal, not the tab, so the lead resolves under the same name with no + * config edit. + */ + @Test + void aLeadRestartingInTheSameTabResolvesUnderTheSameName() { + TopologyHerdr herdr = twoLeads(); + AtomicLong clock = new AtomicLong(); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); + assertEquals("opus-5.0", s.get().get("term_opus")); + + // The session restarts: herdr assigns the pane a new terminal_id, same tab (w1:t1). + herdr.panes.remove("w1:p1"); + herdr.pane("w1:p1", "w1:t1", "term_opus_v2"); + clock.addAndGet(TTL); + + Map leads = s.get(); + assertEquals("opus-5.0", leads.get("term_opus_v2"), "the new terminal resolves immediately"); + assertFalse(leads.containsKey("term_opus"), "the old terminal_id is simply gone, not carried"); } // ── caching ───────────────────────────────────────────────────────────────────────────────── @@ -195,7 +257,7 @@ class LeadTabScannerTest { void aSecondLookupWithinTheTtlDoesNotTouchHerdr() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); s.get(); int afterFirst = herdr.calls; @@ -210,21 +272,23 @@ class LeadTabScannerTest { void aTabLabelledAfterStartupIsPickedUpOnceTheTtlExpires() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + Map tabToName = new LinkedHashMap<>(twoLeadsConfigured()); + tabToName.put("lead: late-arrival", "late-arrival"); + LeadTabScanner s = scanner(herdr, tabToName, clock); assertFalse(s.get().containsKey("term_notes")); herdr.tab("w1:t3", "w1", "lead: late-arrival"); // the operator renames their tab clock.addAndGet(TTL); assertEquals("late-arrival", s.get().get("term_notes"), - "the whole point over `leaders:`: no config edit, no restart"); + "the whole point over a config-held terminal_id: no config edit, no restart"); } @Test void aFailedScanKeepsTheLeadsAlreadyKnownRatherThanDemotingThem() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); Map before = s.get(); herdr.failing = true; @@ -235,14 +299,14 @@ class LeadTabScannerTest { } @Test - void aFailedFirstScanStillHonoursTheConfiguredLeads() { + void aFailedFirstScanReturnsEmptyRatherThanThrowing() { TopologyHerdr herdr = twoLeads(); herdr.failing = true; - Map leads = scanner(herdr, Map.of("term_x", "opus-5.0"), new AtomicLong()).get(); + Map leads = scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get(); - assertEquals(Map.of("term_x", "opus-5.0"), leads, - "config-named leads must not depend on herdr answering at all"); + assertEquals(Map.of(), leads, + "with nothing scanned yet and no override to fall back on, the map is simply empty"); } @Test @@ -250,7 +314,7 @@ class LeadTabScannerTest { TopologyHerdr herdr = twoLeads(); herdr.failing = true; AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); s.get(); int afterFirst = herdr.calls; diff --git a/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java index 03f8bb6..3b3d771 100644 --- a/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java @@ -41,8 +41,8 @@ class LeadLauncherTest { null, null, fleet, null, "fixed", null).withDefaults(); } - private static BridgedConfig.Leader lead(String profile, String terminal, int instances) { - return new BridgedConfig.Leader(profile, terminal, instances, "lead:", 10, null, null, + private static BridgedConfig.Leader lead(String profile, String tab, int instances) { + return new BridgedConfig.Leader(profile, tab, instances, "lead:", 10, null, null, "leads", "/repo"); } @@ -70,16 +70,16 @@ class LeadLauncherTest { void startsTheDeclaredLeadWhenNoneIsRunning() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertTrue(herdr.called("agent.start"), "a lead must actually be started"); assertEquals("lead-opus", startedName(herdr)); } - /** The tab is labelled so the scanner finds the lead on the next resolve. */ + /** The tab is labelled with the configured `tab:` so the scanner finds the lead on the next resolve. */ @Test - void labelsTheTabWithThePrefixTheScannerReadsBack() { + void labelsTheTabWithTheConfiguredTabValue() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); assertEquals("lead: opus", ((Map) herdr.lastCall("tab.rename").params()).get("label")); @@ -90,7 +90,7 @@ class LeadLauncherTest { void startsAsManyInstancesAsAreDeclared() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(2, launcher(herdr, configWith(lead("opus", null, 2))).ensureLeads()); + assertEquals(2, launcher(herdr, configWith(lead("opus", "lead: opus", 2))).ensureLeads()); assertEquals(2, herdr.calls.stream().filter(c -> c.method().equals("agent.start")).count()); } @@ -104,7 +104,7 @@ class LeadLauncherTest { .withTab("wL", "wL:t1", "lead: opus") .withAgent("lead-opus", "term_lead", "wL:p1", "wL:t1"); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start"), "the live lead must not be duplicated"); } @@ -118,20 +118,23 @@ class LeadLauncherTest { .withWorkspace("wL", "leads") .withTab("wL", "wL:t1", "lead: opus"); // label only — nothing running in it - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(), + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(), "a stale label is not a lead; the lead must be relaunched"); } /** - * A lead the operator opened by hand and pinned with `terminal:` is live even though its tab - * carries no matching label. Counting labels alone would relaunch it on every boot. + * A lead the operator opened by hand is live once its tab carries the configured `tab:` label — + * CB-579 retired the `terminal:` pin, so a hand-opened lead is found the same way an + * auto-launched one is, by its tab, not by a terminal id nobody wrote down in advance. */ @Test - void aPinnedTerminalWithARunningAgentCountsAsLive() { + void aHandOpenedLeadWithTheConfiguredTabLabelCountsAsLive() { FakeHerdr herdr = new FakeHerdr() - .withAgent("hand-opened", "term_pinned", "wX:p1", "wX:t1"); + .withWorkspace("wX", "main") + .withTab("wX", "wX:t1", "lead: opus") + .withAgent("hand-opened", "term_hand", "wX:p1", "wX:t1"); - assertEquals(0, launcher(herdr, configWith(lead("opus", "term_pinned", 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -143,7 +146,7 @@ class LeadLauncherTest { .withTab("wM", "wM:t1", "lead: opus") // a member tab that looks like a lead .withAgent("claude-opus-x", "term_m", "wM:p1", "wM:t1"); - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(), + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(), "a member in a lead-labelled tab is not a lead, so the real lead is still missing"); } @@ -152,7 +155,7 @@ class LeadLauncherTest { void anUncountableHerdrStartsNothing() { FakeHerdr herdr = new FakeHerdr().healthy(false); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -166,7 +169,7 @@ class LeadLauncherTest { @Test void theLeadNeverReceivesTheWorkerReplyCharter() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); List args = startedArgs(herdr); assertFalse(args.contains("--append-system-prompt"), @@ -178,7 +181,7 @@ class LeadLauncherTest { @Test void theLeadMountsTheBridgeMcpAndPinsItsModel() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); List args = startedArgs(herdr); assertTrue(args.contains("--mcp-config")); @@ -192,7 +195,7 @@ class LeadLauncherTest { @Test void theLeadEnvCarriesNoAnthropicBinding() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); Map env = tabEnv(herdr); assertNull(env.get("ANTHROPIC_BASE_URL")); @@ -205,7 +208,7 @@ class LeadLauncherTest { @Test void theLeadTabIsCreatedOutsideEveryMemberWorkspace() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); String label = (String) ((Map) herdr.lastCall("workspace.create").params()).get("label"); assertEquals("leads", label); @@ -214,12 +217,12 @@ class LeadLauncherTest { // ── recognise-only and misconfiguration ─────────────────────────────────────────────────── - /** A lead with a pin but no profile is recognise-only by design — not an error, not a launch. */ + /** A lead with a tab but no profile is recognise-only by design — not an error, not a launch. */ @Test void aLeadThatNamesNoProfileIsRecognisedButNeverLaunched() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead(null, "term_dead", 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead(null, "lead: dead", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -228,7 +231,7 @@ class LeadLauncherTest { void zeroInstancesLaunchesNothing() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 0))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 0))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -237,7 +240,7 @@ class LeadLauncherTest { void anUnknownProfileIsSkippedRatherThanThrown() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead("nope", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("nope", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } From b525b0f08fc4da3449b7ea82740c7629bb252758 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 08:47:12 +0200 Subject: [PATCH 20/20] CB-576: hasUncommitted tolerates an already-gone worktree --- .../dev/ltms/bridged/session/GitWorktrees.java | 8 ++++++++ .../java/dev/ltms/bridged/session/Worktrees.java | 4 ++++ .../ltms/bridged/session/GitWorktreesTest.java | 15 +++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java index 9dbcbd1..052b0c5 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/GitWorktrees.java @@ -169,6 +169,14 @@ public final class GitWorktrees implements Worktrees { @Override public boolean hasUncommitted(String worktreePath) { + // A worktree that is already gone holds no work to lose, and it must not break teardown: + // git -C status exits non-zero and would throw where release() is mid-way + // through stopping a pane. Mirror remove()'s already-gone tolerance by treating it as clean. + Path p = Path.of(worktreePath); + if (!Files.exists(p)) { + log.debug("worktree {} already gone — nothing can be uncommitted", worktreePath); + return false; + } // No --untracked-files=no: the exact shape of the work lost in CB-576 was a new file // that was never added, so an untracked-only worktree is still dirty. String out = exec("git", "-C", worktreePath, "status", "--porcelain"); diff --git a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java index f858bf0..dd532ce 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/Worktrees.java @@ -15,6 +15,10 @@ public interface Worktrees { * modifications, staged files, or untracked files. {@code git status --porcelain} is the * test; an empty result means clean. Callers use this to decide whether removing the * worktree would silently destroy a worker's only copy of its work. + * + *

      An already-gone worktree is reported as clean (no throw), matching {@link #remove}'s + * idempotent contract: a path that does not exist holds no work to lose, and must not break + * a teardown that is mid-way through stopping the pane. */ boolean hasUncommitted(String worktreePath); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java index b5c6a83..8dd65aa 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/GitWorktreesTest.java @@ -200,6 +200,21 @@ class GitWorktreesTest { "a tracked modification must also count as dirty"); } + /** + * CB-576 review. {@code hasUncommitted} must tolerate a missing worktree exactly like + * {@code remove}: an already-gone directory holds no work to lose, and throwing here would + * break teardown — SessionManager.release() calls it before stopping the pane, so an + * exception would orphan a live pane and skip the release notification (CB-516). + */ + @Test + void hasUncommittedOnAMissingWorktreeReturnsFalseWithoutThrowing(@TempDir Path tmp) { + GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString()); + String gone = tmp.resolve("wts").resolve("does-not-exist").toString(); + + assertFalse(gitWorktrees.hasUncommitted(gone), + "a missing worktree is reported clean, not an error"); + } + /** All three protected configs are covered: each one present in a worktree is neutralized and hidden. */ @Test void allThreeConfigsAreNeutralizedWhenPresent(@TempDir Path tmp) throws Exception {