Features: #296, #297, #298 — spawn leaks, REST parity, released replies

Dai Ha
2026-09-04 12:25:52 +07:00
parent b763ebff5c
commit 49211149bc
+74
@@ -3569,3 +3569,77 @@ failed underneath it.
knob. If either ask ceiling (`FleetMcp`'s or the REST face's, both 115 seconds today) is ever raised
past it, this delay must be raised with it, or the re-check fires while the question is still open,
finds nothing to sweep, and the single attempt is spent.
## The REST face reports what the MCP face reports
**What.** `GET /profiles` now carries the two backend outage states that `fleet_profiles` has always
reported: `quarantined` (the backend said it is out of capacity) and `coolingOff` (that credential
threw repeated non-exhaustion errors). Each names the `credentialId` and the seconds remaining. The
two checks are independent, so a profile can appear in both maps at once, and each map is present
only when at least one profile is in that state. Separately, `GET /agents` and `GET /members` now
map a herdr transport failure into the same `{error, detail}` envelope every other route uses,
instead of letting it escape as a bare 500.
**On.** Always on. Nothing to configure.
**Why it exists.** REST is the door a lead falls back to when its MCP mount drops — `listMembers`
says so in its own comment. So the weaker door was weakest exactly when it was load-bearing. A lead
on the fallback during an outage could not tell "busy, frees in 40 seconds" from "broken", and a
monitoring client that parses the JSON error envelope broke at the moment it was asking why the
fleet looked unreachable. Neither gap ever caused a wrong spawn: a REST spawn goes through the same
placement gate, so a refusal was still a refusal. These were visibility gaps, not state defects.
**One thing to know for maintenance.** Both doors render the profiles body from **one** method,
`FleetMcp.profilesView`, and are handed the **same** `BackendQuarantine`/`BackendOutagePolicy`
instances. Both halves are needed and neither is sufficient. Sharing the instances stops the doors
reading different facts; sharing the builder stops them reporting those facts differently. The first
version of this change shared the instances and copied the loop into `FleetApp`, which is the fleetd
#284 shape — one rule in two places, where the next edit lands on one and not the other. If you add
a field to that body, add it in `profilesView` and both doors get it.
## A released reply goes back to the broker instead of being dropped
**What.** When a member's session is torn down, `AmqpReplyInbox.release` now cancels that target's
consumer and then nacks every delivery it still holds **with requeue**, so an undrained reply stays
recoverable. Before, it dropped its local record and left the message unacked on a still-open
channel — invisible to `fleet_poll` and `peek`, and freed only when the whole AMQP connection
happened to drop.
**On.** Always on, for the durable (AMQP) inbox. The in-memory inbox is unaffected and correctly
still drops on release, because it is soft state with no broker behind it.
**Why it exists.** This was silent loss of the one artefact the bridge exists to carry. The path was
ordinary: a member replies with no live waiter, so the reply is held; the lead sends that target a
fresh task, which clears the stranded-reply *flag* but never drains the actual entry; the session is
later stopped, so the teardown skips recovery because the flag says there is nothing to recover, and
then drops the entry. The lead sees a member that finished and reported nothing, with no way to tell
that from a member that genuinely never replied.
**One thing to know for maintenance.** The order is cancel first, then nack — not the reverse. A
nack-with-requeue while the consumer is still attached hands the message straight back to that same
consumer as soon as a prefetch slot frees, racing `release`'s own cleanup and leaving a stale entry.
A delivery tag stays valid for `basicNack` on an open channel whether or not its consumer is
attached, so cancelling first costs nothing. This was found against a real broker, not by reading
the code, and it is why `AmqpReplyInboxContractTest` needs Docker.
## A failed spawn no longer leaves a live pane behind
**What.** Two spawn-side exits created a pane and left it running when the spawn failed: the
readiness gate propagated an unrelated herdr error without teardown, and pane placement never closed
the pane it had just split when the peer failed to start. Both now close the pane, best-effort, and
still hand the caller the original exception unchanged.
**On.** Always on.
**Why it exists.** Nothing downstream could clean these up. `SessionManager.acquire` never learns the
pane id — `spawn` throws before it returns one — so its failure path removed the worktree and the
branch and could do nothing about the pane. The result was a live backend process with a deleted
working directory, absent from `fleet_list`, holding a real backend seat that nothing decremented.
The pane-placement leak was the worse of the two: with no named agent started, the orphan reaper
cannot see it either, so no restart ever reclaimed it.
**One thing to know for maintenance.** The readiness gate still refuses to *interpret* an error it
does not recognise — it rethrows it unchanged, and a test pins that. Closing the pane and converting
the exception are different things, and only the first was missing. Do not "simplify" this by
mapping the error to `PeerUnreachableException`; that reintroduces the exact behaviour #176 wrote
`failFastOnGoneBackend` to keep distinct.