Skip to content

fix: route delegated peers to their owning runtime - #482

Merged
jacoblee-io merged 27 commits into
mainfrom
fix/cross-runtime-delegation-routing
Aug 11, 2026
Merged

fix: route delegated peers to their owning runtime#482
jacoblee-io merged 27 commits into
mainfrom
fix/cross-runtime-delegation-routing

Conversation

@LikiosSedo

@LikiosSedo LikiosSedo commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Root cause

Direct chat routes an agent to its configured Runtime, but coordinator delegation always called the source Runtime's local AgentBoxManager. When the coordinator and peer belonged to different Runtimes, the peer was created in the coordinator's network environment and received the wrong MCP/tool surface.

What changed

  • resolve and authorize peer placement before creating an AgentBox
  • preserve the existing local fast path for same-Runtime delegation
  • route cross-Runtime starts and aborts through the management-plane Runtime mesh
  • relay the target Runtime's live chat events back to the coordinator
  • fail closed when placement or delegated-session persistence cannot be proven
  • declare and reassert skipInitialPersistence so the target Runtime cannot duplicate the delegated user row
  • always derive the final remote answer from durable session history, so partial live-frame loss cannot return a truncated success
  • treat the remote timeout as an event-silence watchdog, configurable with SICLAW_REMOTE_DELEGATION_IDLE_TIMEOUT
  • prevent Stop received during session-lock contention or target cold-start from dispatching a remote turn afterward
  • document the Runtime placement, persistence, relay, and rollout contracts

User impact

A coordinator can delegate to a peer assigned to another Runtime without changing that peer's MCP bindings, cluster reachability, session history, live progress, or Stop behavior.

Validation

  • npm test — 245 test files passed; 5085 tests passed, 2 skipped
  • affected delegation/runtime regression tests passed after final hardening
  • npm run build
  • git diff --check
  • companion management-plane go test ./internal/siclaw/...
  • companion relay concurrency packages pass under go test -race

The management-plane routing support should be deployed before rolling out this Runtime change; older management planes fail closed instead of creating a peer in the wrong Runtime.

Resolve peer placement before creating an AgentBox, relay cross-runtime chat events through the management plane, and fail closed when the route or delegated session cannot be established.\n\nTested: npm test; npm run build\nConfidence: high\nScope-risk: moderate
@LikiosSedo
LikiosSedo marked this pull request as ready for review August 10, 2026 07:55

@jacoblee-io jacoblee-io left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the routing change. Direction looks right — making the placement decision before touching the local AgentBoxManager and failing closed is the correct shape, and reusing the reverse WS lane avoids exposing private Gateway addresses.

For the record on scope: this is not a regression. src/gateway/delegate-api.ts has exactly one prior commit (641b898a, "feat(delegation): siclaw-native agent-to-agent delegation"), and that first version already did deps.agentBoxManager.getOrCreate(peerAgentId) unconditionally. There has never been any Runtime-placement logic on the delegation path, and docs/design/coordinator-routing.md never states the single-Runtime assumption. So every multi-Runtime deployment has had this since delegation shipped.

Two things I think should be fixed before merge (details inline):

  1. skipInitialPersistence has no producer in this repo. server.ts consumes it, but the delegation.start payload built here does not set it. The de-duplication the PR describes only happens if the management plane injects the flag while forwarding to chat.send.
  2. The remote path can return a successful-but-empty result. finalText / steps / artifact are rebuilt only from relayed events, so a dropped frame plus a delivered prompt_done yields ok:true, status:"done", finalText: undefined, which the coordinator relays as a real answer.

Also: this adds a new cross-Runtime contract (delegation.resolveRoute / delegation.start / delegation.abort, the reverse delegation.event lane, and skipInitialPersistence) with no documentation change. Per CLAUDE.md's documentation rule these are contracts, not implementation details — worth a section in docs/design/coordinator-routing.md plus Change Impact Matrix rows for delegate-api.ts and frontend-ws-client.ts, including the deploy-ordering requirement.

Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/delegate-api.ts
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/server.ts
Recover dropped relay output from durable history, fail empty completion, make timeout activity-based, prevent stopped turns from dispatching, and document the Runtime placement contract.

Tests: npm test; npm run build; git diff --check

@jacoblee-io jacoblee-io left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed against 0c4570b1. All six points are addressed, several better than what I suggested — the idle watchdog instead of a total-duration cap, and the route.local !== (source === target) consistency assertion, are both improvements on the original comments.

Retracting my first P1. I claimed nothing sets skipInitialPersistence. I had only read this repo; the management plane injects it at its trust boundary. The source now sets it explicitly too, which is the right call — the flag is asserted on both sides rather than depending on one.

I also checked the two risks the new readback could have introduced, and both are clear:

  • recoverRemoteResult's boundary scan is safe: getMessages reverses internally, so limit: 500 is the most recent 500 rows, not the oldest.
  • No read-too-early race: prompt_done is emitted only after consumeAgentSse resolves, and every persistence flush inside it is awaited (sse-consumer.ts:1085). The assistant rows are committed before the terminal event can reach the source.

One finding left, inline.

Comment thread src/gateway/delegate-api.ts Outdated
Treat live relay frames as progress only and always build the final remote answer from persisted assistant rows, preventing partial frame loss from becoming a truncated success.

Tests: npm test; npm run build
Comment thread src/gateway/delegate-api.ts
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/server.ts Outdated
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/server.ts Outdated
AgentBox starts a run before it acknowledges /api/prompt, so a lost or
timed-out ack leaves a really-running turn behind a still-pending Gateway
state, where prompt()'s rejection never reaches the post-accept
compensation. The pending-only Stop branch skipped AgentBox abort
entirely to avoid arming a pre-spawn latch the cancelled send would never
consume, which left that turn running headless.

Decide on the condition that actually governs the latch — whether the box
holds the session — instead of on Gateway pending-vs-active state. A box
that is running the turn has the session, so aborting it is both required
and latch-free; a box that has nothing is left alone. The probe never
falls back to getOrCreate: spawning a pod would manufacture the very box
whose absence was the answer.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts Outdated
jacoblee-io and others added 6 commits August 10, 2026 21:45
…dary

Stopping only on Stop left the larger leak open: AgentBox starts a run
before acknowledging /api/prompt, so an ack lost during an ORDINARY send
strands a turn with no consumer, and a Stop that probed just before the
box created the session missed it too. Move the guarantee to the rejection
path, where the dispatch outcome is genuinely unknown, and keep the
Stop-time probe as the fast path that avoids waiting out the timeout.

Both sites now share abortIfBoxHoldsSession: holding the session is what
separates "a turn is running" from "aborting would arm a pre-spawn latch
the next send would consume", and an unanswerable probe now leaves the box
alone rather than forcing an abort that could plant that latch.

Recovery also no longer caps a turn by its own length. Tool rows are
messages, so a tool-heavy peer turn could bury its opening row past a
single 500-row window and have its finished answer reported as
unrecoverable; page backwards to the boundary instead, bounding total rows
walked rather than one turn's size.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… window

Three separate ways a delegation could still end wrong.

A Stop the box never confirmed was answered ok:true, which tells the
control plane to stop retrying and tear down its supervision. Probe and
abort failures now surface, matching what the non-pending path already did;
only "the box does not have this session" still counts as already stopped.
A failed compensation on the prompt-rejection path is logged loudly instead,
because there the original prompt failure is what the caller must see.

Recovery paged by timestamp cursor, but created_at has one-second
granularity, so a cursor set to a page's oldest row skipped every other row
written in that same second — most of a busy turn. Read a growing window of
the newest rows instead: one extra query per step, and nothing can be
skipped.

A terminal that was consumed and then re-sent (its ack lost) had no live
consumer left and was rejected forever, which kept the sender's relay alive;
that relay's idle expiry aborts by (agent, session) and would land on a new
turn reusing the peer session. Recently settled delegations are now
remembered and acknowledge a re-delivery.

Also clamp the remote idle window to the control plane's relay lease —
waiting past it cannot succeed, since the events have no route left.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Abandoning a turn's stream after a failed abort leaves the box running with
nobody consuming it, and a single warning was the only trace. The box
demonstrably holds the session at that point — it just accepted the prompt —
so retrying is safe and cannot plant a pre-spawn latch. Retry briefly, then
say so at error level rather than warn.

The growing recovery window also doubled past its own ceiling on the last
step (12800 → 25600 against a stated 20000); land exactly on it.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A session id names a CONVERSATION, and delegation deliberately reuses a peer
session across turns, so "abort session S" is ambiguous the moment a turn
ends. Every workaround in this area came from that: a probe that guessed
whether a dispatch had landed, a pre-spawn latch that a later prompt could
consume, a cold-start Stop skipped entirely to avoid arming it, and a
control-plane abort arriving after its turn finished.

Callers now name the turn they mean. AgentBox stores the turn a prompt is
running and answers an abort for any other turn as already stopped; its
pre-spawn latch records the turn that armed it and is consumed only by that
turn's prompt. The Runtime mints an id per turn, forwards it, returns it in
the chat.send ack, and tags every abort it sends. chat.abort accepts an
optional turnId and ignores a stale one — the Stop button still sends none
and still means "stop what is running".

That removes rather than adds machinery: the listSessions probe is gone, and
so is the special case that skipped a cold-start Stop, because a turn-scoped
latch can only ever be consumed by the prompt being cancelled.

Both fields are optional, so the deploy is additive in either order: an
older box ignores turnId and keeps session-wide semantics, and a Runtime
that sends none behaves exactly as before.

The cold-spawn regression test is superseded by "lets a retry run after a
cold-start Stop", which asserts the same no-dispatch and retry-runs
properties plus the turn tagging.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reverse leg was made reliable, but the leg INTO the control plane stayed
fire-and-forget. A human-facing turn survives losing its terminal because the
frontend refetches; a delegated turn has a machine waiting on it and nobody to
retry, so a terminal dropped there stranded the caller until its idle window
elapsed — reported as a failure for a turn that had in fact finished.

The target Runtime now also reports a delegated turn's terminal over an
acknowledged RPC, retried a few times. The control plane treats it exactly as
an observed terminal: supervision ends, and no abort is issued for a turn that
is already over. It is idempotent by construction, because the event lane may
deliver the same terminal and an unacknowledged attempt is retried.

Only delegated turns pay the extra round-trip; ordinary chat is unchanged. A
control plane without the method is not retried, and Portal-standalone answers
it directly, so neither mode sees a delivery failure it cannot act on.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were decided over several review rounds and lived only in code comments.
The doc now states what must hold: an abort names the turn it means because a
peer session is reused across turns; a pre-spawn latch belongs to the turn that
armed it; the Stop button still sends no turn and still means "stop what is
running"; both legs of terminal delivery are acknowledged, and a re-delivered
terminal must be acknowledged rather than rejected; durable readback widens a
window instead of walking a second-granular timestamp cursor; and the idle
window is bounded by the control plane's own lease.

Also states the two rollout facts a future operator cannot infer: the control
plane must precede the Runtime, while turn addressing spans the AgentBox image
and degrades safely in either order — and that cross-Runtime delegation assumes
a control plane whose Runtime connections all terminate in one process.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jacoblee-io

Copy link
Copy Markdown
Collaborator

All 14 threads are resolved. Summary of where this landed, since the shape of the fix changed materially after the first round.

The root cause behind most of the findings

Nearly every remaining issue traced to one thing: an abort was addressed by (agentId, sessionId). A session id names a conversation, and delegation deliberately reuses a peer session for follow-ups in one investigation — so "abort session S" becomes ambiguous the moment a turn ends, and a supervisor's abort can be delayed past that point by a lease expiry or a retry.

That single ambiguity produced the listSessions() probe (guessing whether a dispatch had landed), a pre-spawn latch a later prompt could consume, a cold-start Stop skipped entirely to avoid arming it, and a control-plane abort landing on a successor turn.

Aborts now name the turn they mean:

  • a prompt carries a turnId, the box records it as the running turn, and an abort naming any other turn is answered as already stopped;
  • the pre-spawn latch records the turn that armed it and is consumed only by that turn's prompt, so an orphan cannot cancel a deliberate retry;
  • chat.abort takes an optional turnId and ignores a stale one — the Stop button sends none and still means "stop what is running";
  • the id comes back in the chat.send ack so a supervisor can name it later.

This removed machinery rather than adding it: the probe is gone, and so is the special case that skipped a cold-start Stop. Compensation on a failed prompt is now unconditional, because an abort naming a turn the box never started is a no-op it cannot confuse with a later one — which is what closes the lost-ack window in both the Stop and no-Stop cases.

Both fields are optional, so this half of the rollout is additive in either order.

Reliability of the result path

  • Live relay text is never authoritative. Final remote text always comes from durable assistant rows after the current delegation boundary, so an individually dropped frame cannot yield a truncated success. Readback widens a window over the newest rows instead of walking a timestamp cursor — created_at is second-granular, so a cursor skips rows sharing a second.
  • Both legs of terminal delivery are acknowledged. The reverse leg already was; the target Runtime now also reports a delegated turn's terminal over an acknowledged RPC. Losing that first hop used to strand the source until its idle window elapsed, which it then reported as a failure for a turn that had finished.
  • A re-delivered terminal is acknowledged, not rejected. Its consumer is gone precisely because it consumed the original; rejecting it kept the sender retrying, which kept supervision alive over a finished turn — and that supervision's expiry was the abort that could hit a successor.
  • Supervision ends when a terminal is observed, whether or not it could be handed onwards.
  • An unconfirmed Stop is reported as a failure, never as success: answering ok tells the control plane to stop retrying and tear down supervision.
  • The idle window is event-silence based and clamped to the control plane's relay lease — waiting past it cannot succeed, since those events no longer have a route.

Rollout

One strict ordering and one that is not:

  • Strict: the control plane implementing route/start/abort and the reverse lane must be deployed before this Runtime. An older one makes every delegation route lookup fail closed, including same-Runtime delegation.
  • Not strict: turn addressing spans the AgentBox image (http-server.ts, session.ts). Each half degrades to the older session-wide meaning on its own, so that half can roll either way — but it does mean building the agentbox image, repointing SICLAW_AGENTBOX_IMAGE, and recycling pods.

Cross-Runtime delegation also assumes a control plane whose Runtime connections all terminate in one process. Placement, command delivery and event subscription resolve against process-local connections, so a multi-replica control plane fails closed on a split pair rather than misrouting.

Contracts are documented

docs/design/coordinator-routing.md now states the turn-addressing and two-leg delivery contracts, the readback rule, the idle-window bound, and both rollout facts. Change Impact Matrix rows were added/extended for delegate-api.ts, the reverse-delivery lane, and the agentbox turn protocol.

One knowingly-deferred item

The settledDelegations memory is per-process, so a source Runtime restart forgets it. With turn-scoped aborts the consequence is now just a warning line — the restart already ended the delegate stream, and a late abort names a turn that is gone.

Verification

npm test — 245 files, 5098 passed, 2 skipped. npx tsc --noEmit clean.

One test was replaced rather than dropped: does not dispatch prompt after Stop is acknowledged during cold spawn pinned the retired probe semantics (abortSessionCalls === []). lets a retry run after a cold-start Stop, because the latch is turn-scoped asserts the same no-dispatch and retry-runs properties, plus the turn tagging.

Comment thread src/gateway/server.ts Outdated
Comment thread src/gateway/server.ts Outdated
Comment thread src/gateway/server.ts Outdated
Comment thread src/agentbox/session.ts Outdated
jacoblee-io and others added 4 commits August 11, 2026 00:09
…lost

Three gaps in the first cut of turn addressing.

A second send registers its turn before it can acquire the session lock, so two
turns are live at once — one on the box, one queued. Remembering only the newest
made Stop name the queued turn: the box rejected the mismatch, the consumer was
torn down anyway, and the running turn continued headless. Track the live set and
name all of it, snapshotted BEFORE breaking the consumer — reading it afterwards
misses the very turn being stopped, because a settling turn removes itself.

The turn id was learned from the chat.send acknowledgement, which left the one
case that needs it most with nothing to name: a lost ack, whose compensation then
fell back to stopping the session and could hit a later turn. chat.send now
accepts a caller-supplied turnId and echoes it, so a supervisor fixes it before
dispatch.

A named pre-spawn latch is armed unconditionally. The "already has history on
disk" guard existed only because a session-wide latch could cancel an unrelated
later prompt; a turn-scoped one cannot, and skipping the guard is what lets a Stop
on a reused session — which a delegated peer thread always is — arm anything.

Also widen the terminal delivery retry budget past a WS reconnect, which cannot
complete faster than its own backoff.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The turn-addressing section read as if one turn were live per session and as if
the id could come from the acknowledgement. Both were the assumptions that had to
be corrected in code: a second send makes two turns live, and an acknowledgement
that never arrives is exactly the case a supervisor needs the id for.

Also records why a named pre-spawn latch skips the on-disk-history guard, that a
reported terminal is queued rather than acted on directly, and that the delivery
retry budget has to outlast a WS reconnect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It read as if one turn were live per session and as if the id could come from the
acknowledgement. Both were the assumptions that had to be corrected in code: a
second send makes two turns live, and an acknowledgement that never arrives is
exactly the case a supervisor needs the id for.

Also records why a named pre-spawn latch skips the on-disk-history guard, that a
reported terminal is queued rather than acted on directly, and that each delivery
retry budget has to outlast a WS reconnect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shutdown and box removal end a turn on behalf of its own reporting path, which is
then suppressed — so the one terminal a delegated caller cannot do without went
out fire-and-forget on exactly the path where the transport is about to disappear.
The caller was left to wait out its idle window and report a failure for a turn
that had been interrupted, with the cause already known here.

Delegated turns are now tracked so the supervisor can report their terminals the
same acknowledged way, carrying the interruption cause it already names. Shutdown
waits briefly for those deliveries before closing the connection they travel
over — a live connection settles the first attempt in milliseconds, and a dead one
must not hold the process open for the whole retry budget. A box removal does not
wait: its connection stays up.

The per-turn deliverer moved out of chat.send to be shared, unchanged otherwise.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jacoblee-io

Copy link
Copy Markdown
Collaborator

Follow-up on the review of the turn-addressing work. Four P1s and three P2s, all addressed; head is b468e5a9.

P1 — two turns can be live at once

currentTurnIds was set before the background task takes the session lock, so a second send on the same session immediately replaced the running turn's id. Stop then named the queued turn, the box correctly rejected the mismatch, the Runtime consumer was torn down anyway, and the running turn continued headless.

Turns are now tracked as a set per session, and a Stop that names none names all of them: the box stops the one it is running and answers the rest as already stopped. Falling back to a session-wide abort instead would re-arm a session-wide pre-spawn latch, which is what the turn scoping exists to prevent.

Writing the test for this surfaced a second bug in the same code: chat.abort held a reference to the live set, and breaking the consumer lets a turn settle and remove itself before the reference was read — so it aborted only the queued turn. The set is now snapshotted before the consumer is broken.

P1 — a lost chat.send ack left compensation with nothing to name

Reading the turn id from the acknowledgement failed in exactly the case that needs it: no ack, so no id, so the compensating abort fell back to stopping the session and could hit whatever turn is running by then.

The control plane now mints the id before dispatch and passes it in the prompt; chat.send accepts a caller-supplied turnId and echoes it, minting one only when none is supplied. Every abort for a delegation names a turn, ack or no ack.

P1 — a reused session never armed a turn-scoped latch

markPendingAbort skipped arming when the session already had history on disk. That guard exists only because a session-wide latch could cancel an unrelated later prompt; a turn-scoped latch cannot, since only that turn's own prompt consumes it. Since a delegated peer thread always has history, the guard was suppressing the latch in precisely the case it was needed. It is now skipped when the abort names a turn, and kept for the legacy session-wide call.

P1 — a reported terminal could overtake queued control frames

delegation.terminal forwarded and retired the relay straight from the RPC, jumping ahead of artifact, error and clarification frames still queued on that delegation's subscription and dropping them.

A subscription can now be handed a payload for its own queue, so the reported terminal lands behind those frames and the ordinary handler interprets it in arrival order — including ending supervision, which is why nothing else happens in the RPC. The handler also validates the reported turn and accepts only a terminal event type: this path retires a relay, so an arbitrary frame must not be able to trigger it.

Verified the test detects the regression rather than merely passing: reverting to direct forwarding fails it.

P2s

  • Retry budget. 3 attempts over ~750 ms could not outlast a WS reconnect, which cannot complete faster than its own backoff. Now 5 attempts over ~15 s.
  • Supervisor terminals. Shutdown and box removal end a turn on behalf of its own reporting path, which is then suppressed — so that terminal went out fire-and-forget on exactly the path where the transport is about to disappear. Delegated turns are tracked so the supervisor reports their terminals the same acknowledged way, carrying the interruption cause it already names. This also exposed an ordering bug: close() called frontendClient.close() immediately after ending turns, killing the connection those deliveries need. Shutdown now waits briefly for them first; a box removal does not, since its connection stays up.
  • Terminal handler validation. Covered above: turn id checked, event type restricted to terminals.

Verification

npm test — 245 files, 5101 passed, 2 skipped. npx tsc --noEmit clean.

Docs updated alongside: the turn-addressing section previously read as if one turn were live per session and as if the id could come from the acknowledgement — both were exactly the assumptions that had to be corrected in code.

Deployment

Unchanged in shape, but worth restating since this now spans three images: build the agentbox image, repoint SICLAW_AGENTBOX_IMAGE, recycle agent pods, then roll Runtime and the control plane. The control plane must precede the Runtime for route/start/abort; turn addressing degrades to session-wide semantics on either side alone, so that half is order-independent. Assumes a single-process control plane and SICLAW_REMOTE_DELEGATION_IDLE_TIMEOUT within the relay lease (values above it are clamped with a warning).

jacoblee-io and others added 3 commits August 11, 2026 01:40
Naming a turn on the wire is only half of it; the Runtime still cancelled by
session, and three failures followed.

An abort naming a QUEUED turn aborted the session's controllers, so the RUNNING
turn's consumer was torn down while the box was told only about the queued one —
leaving the running turn with nobody reading it. Controllers are now held per turn
and only the named ones are cancelled.

The supervisor reported every live turn as interrupted but cancelled only the
streaming one, so a queued turn could still start after its caller had been told
it was over. It now cancels every turn it reports, and the suppression of a turn's
own terminal is per turn too — otherwise the other live turn still emitted a plain
terminal, which reads as a turn that succeeded.

AgentBox treated an abort for a not-currently-running turn as already stopped, but
that turn's prompt may still be in flight or its session rebuilding, so the
cancellation was simply lost. It now records a latch for the named turn, and
latches are held per (session, turn): one slot per session let two turn-scoped
latches overwrite each other, which is an ordinary sequence when one turn is
cancelled while another runs.

Also raises the terminal delivery budget past a full worst-case reconnect (the
client's backoff caps at 30s plus up to 2s of jitter).

Tests: npm test; npx tsc --noEmit. Both new Runtime tests were verified to fail
against the previous behaviour rather than merely pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four, all in the seams rather than the idea.

The choice between per-turn and session-wide cancellation was made from state read
AFTER an awaited box lookup, so a turn settling in between dropped it into the
session-wide branch — breaking the consumer of a successor that had started
meanwhile. It is now taken from the snapshot and applied before the await.

Shutdown only considered sessions with a registered consumer, so a turn still
cold-starting was neither reported nor cancelled: its caller waited out an idle
window while the box could still start it. It now covers every session with work of
ours.

Terminal delivery was awaited inside the turn, so a budget measured in reconnects
held the session lock, the streaming registration and the supervisor's view of a
live turn for up to a minute — blocking the next turn on that session and letting a
SIGTERM re-report a turn that had already succeeded as interrupted. The turn now
settles at once and deregisters itself; the delivery continues in a tracked set
that shutdown flushes.

A queued turn cancelled by the supervisor reaches the pre-stream catch, which
emitted a second, PLAIN terminal — one without `aborted`, which reads as a turn
that completed. That path now honours the same per-turn suppression.

Tests: npm test; npx tsc --noEmit. The supervisor-cancellation test went through
three signals before landing on one that actually fails without the fix: sampling
after close(), and waiting on the session lock, both race the lock hand-off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cancelling on the Runtime side does not end a prompt: the consumer only notices its
signal when the next event arrives, and a dropped SSE subscription merely
unsubscribes. Boxes deliberately outlive a Runtime roll in K8s, so a turn already
reported as `runtime_restart` kept executing there with nobody left to read it. The
supervisor now asks the box to stop each DISPATCHED turn by name — `busyOn` is the
record of that placement, so a turn that never reached a box is left alone.

The terminals the supervisor starts were returned but never tracked, and the
box-roll callback discards the return value. A shutdown right after a box removal
therefore saw no live turn and no pending delivery, and closed the transport out
from under a `box_rolled` terminal still being retried — losing the one report that
tells the delegated caller its turn was interrupted rather than lost. Everything
endTurns starts is now tracked centrally, and shutdown waits for it.

Tests: npm test; npx tsc --noEmit. Both new tests were verified to fail against the
previous behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts Outdated
A turn stays live until its consumer settles, and a real consumer settles only when
its next event arrives — so a box removal followed by a shutdown reaches the same
turn twice. Both passes reported it, putting two authoritative terminals with
DIFFERENT causes in flight; whichever won the retry race named the cause, so a
`box_rolled` interruption could be reported as `runtime_restart`.

Each turn is now claimed before the first report. Later passes still cancel it and
still stop its box, and the delivery they would otherwise have started is already
tracked for shutdown to flush.

Two things found while writing the test:

The existing supervisor tests hid this window, because the mocked consumer settles
the moment its signal aborts — which a real one does not. The mock can now model a
consumer that ignores the abort, which is what makes the second pass reachable.

A box removal was also asking the removed box to stop its turn: that endpoint is
dead by definition, so the request only hung out its timeout and logged a misleading
warning. Skipped for that reason.

Tests: npm test; npx tsc --noEmit. The new test was verified to fail against the
previous behaviour (two terminals instead of one).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts Outdated
The previous commit skipped it on the premise that the endpoint is already dead.
That premise is wrong: the manager reports the interruption BEFORE asking the
spawner to stop the box, and a failed stop keeps the drain mark and retries on a
later tick. In that window the Runtime has already emitted an authoritative
`box_rolled` terminal and dropped its consumer, while the prompt keeps running — and
keeps producing tool side effects — on a box that is very much alive.

The abort is issued for a box roll as well. A box that has in fact gone answers
nothing, which is the outcome that was wanted, so a failure there is reported at log
level rather than as a warning; a shutdown, where the box is expected to answer,
still says so loudly.

Tests: npm test; npx tsc --noEmit. The new test fails against the skip (its abort
never arrives).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts Outdated
The same gap as the terminal deliveries, in the half I did not fix: the abort a
supervisor issues was only pushed onto endTurns' return value, and the box-removal
caller discards that. Once the local consumer settles the turn leaves liveTurnIds, so
a SIGTERM an instant later saw nothing pending and the process exited with the abort
in flight — and since K8s keeps the boxes, an already-reported turn would go on
running there.

Both kinds of outliving work now go through one central set that the bounded shutdown
wait drains, so it no longer matters what a caller does with the return value.

Two notes on the test:

It asserts what the reviewer asked for — block abortSession, let the consumer settle,
start close(), verify shutdown still waits — and fails against the untracked version,
where close() completes immediately.

It also needed its own session id. A test whose consumer never settles holds that
session's turn lock for the remainder of the file, because sessionTurnLocks is
process-wide; reusing the id made the new test hang in acquire() rather than test
anything, and only in a full-file run.

Tests: npm test; npx tsc --noEmit

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts
Shutdown looked once while every producer was still active: the command lane stays
open so terminals can still be delivered, the HTTP servers still listen, and the
manager's loops run until later. A turn admitted during that wait registered after the
drain had looked, so it was neither reported nor cancelled — and the box, which K8s
deliberately keeps, went on running it.

chat.send now refuses once the fence is up, and a turn registers before the
persistence awaits in its own handler, so a handler that got past the check is already
visible to the drain instead of appearing while it was in the database.

This REVERSES a documented decision (`server-shutdown-turns.test.ts`), which admitted
such a turn precisely so it could report itself rather than hang. Admitting it is the
worse half of that trade: a refusal is an explicit error the caller can act on by
placing the turn on a Runtime that will still be there, which is more than an
unsupervised turn offers. That test now asserts the refusal, and says why the trade
flipped. If you would rather keep the old contract, the alternative is a repeated
drain, and the fence should come back out.

Also: the supervisor now asks a box to stop a given turn only once, so a box roll
followed by a shutdown does not ask twice.

A rescan loop was written first and then removed: under the fence nothing reachable
turns up in a second pass — a turn finishing then finds itself already reported, and a
box roll finds every live turn claimed and already asked — and I could not construct a
test that distinguished it. Unexercised machinery on the shutdown path is its own
hazard.

Tests: npm test; npx tsc --noEmit. The fence half of the new test fails without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/server.ts
Comment thread src/gateway/server.ts
Two gaps in the previous commit.

The fence gated `chat.send` only, while `POST /api/internal/delegate` starts AgentBox
work of its own — a local peer prompt, or a remote relay — and kept being admitted
during the drain. It now honours the same fence and answers 503, which is something a
coordinator can act on; an unsupervised peer turn is not.

The box abort marked a turn as asked before the request resolved and kept the mark on
failure. The case that abort exists for is a box removal whose own `spawner.stop()` also
failed, so treating a timed-out abort as done spent the only retry a shutdown could have
made and left the prompt running on a box that was still there. A refused attempt now
clears the mark.

The invariant claim is narrowed to match the code rather than the other way round. One
snapshot is sufficient for the turns this drain has ever known — the chat.send and
delegation ingresses. The task coordinator's scheduled and fire-now jobs and capability
runs keep their own AgentBox clients and are registered nowhere here, so they are
neither fenced nor drained. That predates this drain, and closing it means giving those
paths the same admission gate and registration — a change of its own, not a line in
this one.

Tests: npm test; npx tsc --noEmit. The delegation-ingress fence has a test through that
ingress, and the abort-retry test fails when a refused attempt counts as asked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jacoblee-io

Copy link
Copy Markdown
Collaborator

Drawing the scope line explicitly, because this branch has been reviewed well past what it sets out to do.

The PR's purpose is Runtime placement for delegated peers. Everything in the first review round was about that, and it has been settled for several rounds. What followed came from one decision — addressing aborts by turn rather than by session — which is not a delegation requirement but a pre-existing ambiguity that delegation exposed. Fixing it pulled in the shutdown path, and the shutdown path pulled in producers this branch does not otherwise touch.

This branch stops here. Head is 622d1513: npm test 245 files / 5111 passed, npx tsc --noEmit clean, every thread answered. The paired control-plane change is confirmed reviewable as it stands.

Two known gaps are deliberately left, both pre-existing and now documented in docs/design/coordinator-routing.md rather than implied away:

  • The task coordinator's scheduled and fire-now jobs, and capability runs, start AgentBox work with their own clients and are registered nowhere in liveTurnIds. They are therefore neither fenced at shutdown nor drained — as was true before this branch. Giving them the same admission gate and registration is a shutdown-quiescence change across two subsystems, worth reviewing on its own.
  • settledDelegations is per-process, so a Runtime restart forgets it. With turn-scoped aborts the consequence is a log line, not a wrong outcome.

Further findings adjacent to cancellation or shutdown are welcome as issues rather than as blockers here. A finding that this PR's own subject — placement, the two-leg terminal contract, or the durable result readback — is wrong is of course still a blocker.

…self

Both gaps were in the interleavings, and both of my previous tests avoided them.

The delegation endpoint sampled the shutdown gate once, at entry, and then awaited the
roster, the model binding, the route, session reuse, persistence, the session lock and —
locally — a box spawn before dispatching. It could observe "not shutting down", pause in
any of those, and dispatch after shutdown had taken its one look; and a delegation
already under way was registered nowhere, so nothing wound it down. The gate is now
re-read at the dispatch boundary, and the handler registers the same wind-down a client
disconnect triggers, which shutdown runs before it takes stock so whatever that starts
is waited on too.

The box abort deduplicated on "asked", which a later caller cannot recover from: it
finds the mark set while the first attempt is still outstanding, and by the time that
attempt fails there is no later pass — and if the consumer settled meanwhile, the turn
has left the bookkeeping entirely. Each attempt now retries itself inside the promise
shutdown already awaits, and the mark is cleared only once every attempt has been
refused.

Tests: npm test; npx tsc --noEmit. Both new tests were verified against the previous
behaviour: the dispatch-boundary one hangs waiting for a relay it should never have
started, and the abort one never sees its second attempt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/delegate-api.ts Outdated
Comment thread src/gateway/delegate-api.test.ts
…tions

Three findings, one of which was making the whole suite exit non-zero.

The registered wind-down scheduled its abort and returned void, so the promise shutdown
tracked settled as soon as the request was dispatched — the transport could still close
underneath it. It now returns the abort it issues. A client disconnect can fire and
forget; a shutdown cannot.

Cancellation while `delegation.start` was still pending rejected `remoteDone` before
anything awaited it, an unhandled rejection that took the process exit code with it:
`npm test` reported 245 files passed and exited 1. `remoteDone` is now marked handled at
creation, which leaves the later await unchanged.

The hook was registered at the top of the handler but released only by the inner
`finally`, so every request rejected before dispatch — authorization, binding, route,
parent, persistence — left a closure in the set for the lifetime of the Runtime, and
shutdown would later invoke stale hooks. It is registered inside the guarded scope
instead; nothing before that point has peer work to wind down, and the gate re-read
covers that window.

On my own process: I had been checking the suite's summary lines and not its exit code,
which is why the unhandled rejection survived several rounds of me reporting the suite
as clean. `npm test` now exits 0.

Tests: npm test (exit 0); npx tsc --noEmit. The wind-down test fails against a
fire-and-forget hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/gateway/delegate-api.ts Outdated
Both halves of the finding hold.

The wind-down caught a refused abort and resolved, so an immediately failing abort read
as a completed wind-down. For a local peer there is no relay lease to fall back on and
the box outlives the Runtime, so the prompt simply kept running. The attempt now retries a
refusal, bounded, the same way the supervisor's own box abort does.

And a disconnect started that attempt fire-and-forget, letting the handler reach its
finally and unregister while the abort was still on the wire — a shutdown an instant later
found neither a hook nor tracked work and was free to exit underneath it. The attempt is
memoized, so a disconnect and a shutdown wait on the same one, and the hook stays
registered until it settles.

The test took three attempts to become discriminating, which is worth recording. Counting
aborts on the remote path proved nothing, because the handler issues one of its own;
asserting the hook before awaiting the handler proved nothing either, because it had not
reached its finally yet. It now drives the LOCAL path, where the wind-down is the only
source of abortSession, and asserts after the handler has settled. Both halves were
verified to fail without their fix.

Tests: npm test (exit 0); npx tsc --noEmit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jacoblee-io
jacoblee-io merged commit 374f14a into main Aug 11, 2026
3 checks passed
@jacoblee-io
jacoblee-io deleted the fix/cross-runtime-delegation-routing branch August 11, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants