SIGN IN SIGN UP

fix(broker): stop an orphaned receipt from jamming a parked agent forever (#1639)

* fix(broker): stop an orphaned receipt from jamming a parked agent forever

A `manual_flush` queue stamps each held message with the `agent_id` that
was live when it was queued, and the flush gate looked that cursor up by
`receipt.agent_id`. That key is not stable: `bind_authoritative_identity`
(spawn register, token identity resolve, inventory repair) retires the
previous `agent_id` and drops its cursor, and a node-control resume
handshake's `seed_cursor` moves the cumulative position to Relaycast's
own. Either event left already-parked receipts pointing at a cursor that
could never accept them.

`can_ack_receipt` returned false for that case, which is indistinguishable
from "not ACKable yet", so the flush stopped at the head message forever.
`flushed: 0`, and a partial flush deliberately pins the worker back into
`manual_flush`, so every later DM parked behind the poisoned head. The
agent went permanently deaf while `send_dm` kept returning
`recipientMatched: true` (relay#1593, relay#1559).

Split the gate into `receipt_ackability` -> Ready / Blocked / Orphaned.
Orphaned messages are injected and dropped without an ACK: Relaycast keeps
ownership of the frame so its redelivery policy is untouched. Withholding
the ACK was always the safe half; withholding the message was the bug. A
genuine ordering gap still classifies Blocked and still holds the queue.

Tradeoff: an orphaned frame Relaycast later redelivers under the new
identity can now be seen twice. Duplicate delivery beats permanent
deafness, and the old behaviour bought nothing here since a retired
identity is never redelivered to.

Also surface the failure. `POST /api/spawned/{name}/flush` returned a bare
count while `flush_result.failure` went only to a `tracing::warn!`, so
`node agent message flush` printed `{"flushed": 0}` with no error. That
ambiguity is why the flush evidence on relay#1593 was retracted as "0 is
the expected answer and proved nothing" -- it was a swallowed failure, not
a null result. The route now returns `held` and `blocked_reason` too.

Verified red then green with a byte-identical probe:
  red  (source at origin/main): 2 FAILED, left: 0 / right: 2
  green (fix applied):          8 passed; 0 failed
The guards `manual_flush_still_stops_on_a_genuine_sequence_gap` and
`manual_flush_failure_retains_failed_message_and_suffix_without_ack` pass
in both directions, so ordering is still enforced.

Refs: #1593, #1559

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

* style: auto-format with Prettier

* ci: restore a human-authored head so CI actually runs

The Prettier Auto-Format workflow pushed 2a6e87eb (author:
github-actions[bot]) on top of this branch, reformatting two line
wraps in client.ts. That made the PR head bot-authored, and every
workflow on that head completed `action_required` at 0s without
executing -- relay#1549.

Measured on this one PR, same branch, consecutive heads:
  b3ffbb54 (human): 15 runs, executing (Rust Auto-Format, Prettier
                    Auto-Format, Relay Evals, Large File Check all
                    success; CI/Test/E2E in progress)
  2a6e87eb (bot):   11 runs, ALL action_required, none executed

The same head change also failed RelayFlow PR Proof, which aborted
with "Pull request head changed during dispatch: status targets
b3ffbb54..., preparation resolved 2a6e87eb...".

This empty commit restores a human-authored head so the suite runs
against the code that will actually merge. The bot's formatting is
kept as-is (prettier --check is clean); nothing is rewritten.

Refs: #1549

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

* fix(broker): dead-letter orphaned parked receipts instead of injecting them

Addresses review on #1639. Two findings were correct and changed the design.

cubic P1 (node_control.rs) — an `agent_id` can be rebound to a *different
name* while the old name still holds a parked queue. `bind_identity` carries
the cursor across and rewrites `cursor.agent_name`, so a lookup by `agent_id`
alone still found a live cursor and classified the stale receipt `Ready`.
Committing it made `commit_acked_receipt` rewrite `cursor.agent_name` back to
the old name and ACK against it, after which `observe` rejects every delivery
for the identity's current name as an identity conflict — corrupting a healthy
agent in order to unjam a stale one. `receipt_ackability` now orphans any
receipt whose `agent` does not match the cursor's current `agent_name`.
Regression test included; disabling the guard makes it fail with `flushed: 1`.

CodeRabbit (major) — injecting an orphaned message hands one identity's
private messages to whatever process holds that name now. Orphaned receipts
are no longer injected at all: they are dead-lettered with reason
`orphaned_delivery_receipt:{identity_retired|cursor_moved_past}` and surface
through `node deadletters`. That still removes the real harm (permanent
deafness to every *subsequent* message) without cross-identity delivery, and
it satisfies relay#1593's explicit ask that an undeliverable injection
dead-letter with a distinguishing reason rather than being silently dropped.
A genuine ordering gap still holds the queue, unchanged.

Also from review:
- Restore the doc comment pairing broken by inserting `FlushPendingOk` between
  `SetInboundDeliveryMode`'s docs and its struct (cubic).
- Update the flush route contract comment, which still advertised only
  `flushed` (cubic).
- Add a route test for the load-bearing case, a blocked flush, asserting all
  four JSON fields (cubic).
- Reuse the built expected message in the sequence-gap test; building it twice
  re-stamps `queued_at_ms` and could flake (cubic).
- Correct the comment claiming a retired identity can never return.
  `bind_authoritative_identity` calls `forget_retired_identity` and can
  re-adopt it; the receipt is still un-ACKable because re-adoption rebuilds
  the cursor from Relaycast's position (cubic).
- Expose the new fields on the Swift SDK `FlushResult` (Codex, cubic).
- Lead the changelog entry with user-facing impact (cubic).

`dead_lettered` joins `flushed`, `held`, and `blocked_reason` on the flush
route and both SDK clients.

Not taken: CodeRabbit asks for `## [Unreleased]`. CLAUDE.md requires the
pending release level (`[Unreleased - Minor]`) on the first user-visible
change, so the suffix stays.

Verified: 1026 passed / 5 failed, the 5 being `spawner::tests::broker_hook_*`,
which fail identically on untouched origin/main on this machine (leaked global
core.hooksPath) and pass on CI ubuntu and macOS. fmt, clippy -D warnings,
prettier, and `swift build` all clean.

Refs: #1593, #1559

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

* test(relayflows): prove the parked-agent jam against the real broker binary

Adds the RelayFlow proof case this PR owed, and in building it the reachability
question this PR could not previously answer is settled.

The trigger is a duplicate spawn that is itself REJECTED. `workers.spawn` bails
with `agent '<name>' already exists` at worker.rs:579 — but that guard runs
*inside* `workers.spawn`, which api.rs reaches long after it has already
re-registered the name with Relaycast (register_fleet_agent, api.rs:366/450).
So a spawn that visibly fails has already called
`bind_authoritative_identity`, retired the live agent's `agent_id`, and dropped
its cursor. Nothing rolls that back, and only release or permanent-death clears
`delivery_states`, so the parked queue survives pointing at a dead identity.

That is the shape of relay#1604 / #1554 ("every fleet spawn dispatches twice;
spawn survives only because agent names collide"). Those duplicate dispatches
are not benign: the losing twin silently deafens the winning agent.

Measured on both arms with the real `agent-relay-broker` binary, observed
through `GET /api/spawned/{name}/pending`, which exists on both:

  base daf8a7c7c: pending after flush = 1  -> parked_message_never_leaves_the_queue
  head           : pending after flush = 0  -> parked_message_dead_lettered_and_queue_drains

The case is self-contained: tests/e2e/fleet/harness.ts needs a sibling relaycast
checkout that the proof sandbox does not have, so the case ships a fake
Relaycast speaking enough HTTP plus a real node-control websocket to park a
message and rebind an identity.

Two things the fake has to get right, recorded because neither is discoverable
from an error message: `POST /v1/nodes` must return every non-`default` field of
`NodeRosterEntry` (CreateNodeResponse flattens it) or the broker retries forever
with no stated cause; and the node token must be non-empty.

Refs: #1593, #1559, #1604, #1554

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

* fix(relayflows): stop scraping the broker's log for its API port

The proof case launched the broker with `--api-port 0` and discovered the
assigned port by matching `API listener bound on 127.0.0.1:(\d+)` in its
output. That line is a startup diagnostic, not an interface. It held on macOS
and did not on the Linux CI runner, so the runner timed out waiting for a port
that was already listening:

  Timed out waiting for broker API listener to bind.
  Case runner failed with exit 1; expected-red behavior must be reported as a
    successful structured observation
  ✗ prove-base — FAILED: output does not contain "PR_PROOF_ARM_COMPLETE arm=base"

A non-zero runner exit is an infrastructure failure under the case contract, so
this took down the arm that was supposed to report red rather than producing an
observation.

The runner now picks a free port itself, passes it explicitly, and waits on
`GET /api/status` — probing the thing it actually needs instead of a log line.
It also fails fast with the broker's exit code if the process dies during
startup, so a real startup fault reports as one instead of a generic timeout.

Also bounds the dead-letter event emit (broker side). `send_broker_event` ends
in `tx.send(...).await`, which blocks on a full channel, and the orphan branch
calls it inside the flush loop — once per parked message, up to
`MAX_PENDING_PER_WORKER`, on the runtime event loop. A full or unattended SDK
channel could therefore stall every other API request, which is what a hung
`status` looks like. It now uses the existing `emit_http_api_event_with_timeout`
and writes the dead-letter record before emitting, so a dropped event costs
observability on one entry and never the record itself.

Both arms re-verified locally against the real broker binaries:
  head -> parked_message_dead_lettered_and_queue_drains
  base -> parked_message_never_leaves_the_queue

Refs: #1593, #1559

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

* test(relayflows): strengthen parked-agent recovery proof

Session-Id: 01a060ff-9135-7bf0-9f39-1618ec160c82

* fix(broker): keep orphan flush nonblocking

Session-Id: 01a060ff-9135-7bf0-9f39-1618ec160c82

* test(relayflows): prove #1593 against a real self-hosted Relaycast engine

The previous case used an in-case fake, and the fake decided the one thing the
bug depends on. It had a `rotate_agent_id` command: the case simply asserted
that re-registering a live name yields a new immutable `agent_id`. The real
engine refuses that — `409 agent_already_exists`, verified by driving
`@relaycast/engine` directly — so the case was proving a trigger production
does not have.

This replaces the fake with a real self-hosted engine. `@relaycast/engine`
publishes `dist/bin/serve.js`, which runs standalone against a local sqlite
file, so the case now uses a real workspace, a real node mint, real
`agent.register` identity rules, and a real `POST /v1/dm`. The engine decides,
not the case.

The trigger is corrected accordingly. It is NOT a duplicate spawn (the engine
409s that, and the broker keeps its existing id). It is the agent record being
deleted out from under a still-live worker that holds a parked queue, after
which the broker re-registers the freed name and receives a genuinely new
immutable id — a release issued elsewhere, a dashboard release, or a roster
reaper. `bind_authoritative_identity` then retires the cursor every parked
receipt points at.

Getting this wrong once more was instructive and is recorded in the case: an
intermediate revision deleted AND recreated the record, so the broker's own
register collided (409), it kept the old id, nothing was orphaned, and the case
correctly refused to pass with `{"dead_lettered":0,"flushed":1}`. The broker
must mint the replacement, not the case.

Measured on both arms with real broker binaries against the real engine:

  base daf8a7c7c: pending after flush = 1
                  -> parked_message_never_leaves_the_queue
  head          : pending after flush = 0, flush reports
                  {"dead_lettered":1,"flushed":0,"held":0,"blocked_reason":null}
                  -> parked_message_dead_lettered_and_queue_drains
  identity replaced: 220857008603648000 -> 220857009920659456

The head arm additionally requires that the orphan was dead-lettered rather
than injected, that the worker's screen never shows it, and that a follow-up DM
sent afterwards injects normally — so a silent removal that leaves the queue
broken cannot pass as a fix.

One operational note recorded in the runner: the broker must be launched with a
clean environment. Inheriting the caller's `RELAY_*` variables authenticates it
against production instead of the engine under test, which surfaces as
"all configured multi-workspace memberships were rejected".

Refs: #1593, #1559

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3p9VEngj9kLDFFhrzHNd

Session-Id: 246fba6e-6436-46ae-bc50-6bb3cca0d95e

---------

Co-authored-by: Proactive Runtime Bot <agent@agent-relay.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Miya <khaliqgant+miya@gmail.com>
K
Khaliq committed
e6988e0e9b660ab07868c36de919a9ab767d56e3
Parent: 2559e66
Committed by GitHub <noreply@github.com> on 9/2/2026, 12:02:51 PM