From 6eb01db3cd04b0429980853cd0b8f3b11477b070 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Tue, 13 Oct 2026 09:20:06 -0700 Subject: [PATCH 1/2] fix(fleet): stop reading the placed handler node as dispatch evidence The engine's spawn ack snapshots the node placement chose in handler_node_id before it sends anything, so an ack whose send failed still carries it next to dispatched_node_id: null. The SDK's resolveDispatchState and the CLI's dispatchEvidence both treated handler_node_id as proof of dispatch, so `fleet spawn` printed dispatchState "dispatched" for an invocation `fleet spawn-status` (which reads the engine record, where handler_node_id is not exposed) reported as not_dispatched. Only dispatched_node_id, or a status the node itself moved, is evidence. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 ++ .../cli/src/cli/lib/spawn-lifecycle.test.ts | 34 ++++++++++++++++- packages/cli/src/cli/lib/spawn-lifecycle.ts | 5 ++- packages/sdk/src/messaging/placement.test.mts | 37 +++++++++++++++++++ .../sdk/src/messaging/relaycast-placement.ts | 13 ++++--- 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b3ccb0d4..30810b59a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `fleet spawn` and the SDK placement no longer report `dispatchState: "dispatched"` for a spawn the engine never sent. The engine's acknowledgement always names the node placement chose in `handler_node_id`, even when sending to it failed, so that field is no longer read as dispatch evidence; only `dispatched_node_id` (or a status the node moved) is. `fleet spawn` and `fleet spawn-status` now agree. + ## [13.4.1] - 2026-10-12 ### Fixed diff --git a/packages/cli/src/cli/lib/spawn-lifecycle.test.ts b/packages/cli/src/cli/lib/spawn-lifecycle.test.ts index 9b0a722f1..9dda520ec 100644 --- a/packages/cli/src/cli/lib/spawn-lifecycle.test.ts +++ b/packages/cli/src/cli/lib/spawn-lifecycle.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { sanitizedSpawnReceipt, spawnLifecycleState } from './spawn-lifecycle.js'; +import { sanitizedSpawnReceipt, spawnLifecycleState, spawnPlacementReceipt } from './spawn-lifecycle.js'; describe('spawn lifecycle readiness', () => { it('rejects a terminal success without launch and readiness proof', () => { @@ -28,3 +28,35 @@ describe('spawn lifecycle readiness', () => { ).toEqual({ status: 'completed', output: { spawned: true, ready: false } }); }); }); + +describe('spawn dispatch evidence', () => { + // The engine's 201 for a placed spawn always names the chosen node in + // handler_node_id, before it sends anything. When the send fails the row stays + // pending with dispatched_node_id null; that is not a dispatch. + it('does not treat the placed handler node as dispatch evidence', () => { + expect( + spawnPlacementReceipt({ + invocation_id: 'inv_unsent', + status: 'pending', + handler_node_id: 'node_sf_frame', + dispatched_node_id: null, + }) + ).toEqual({ + state: 'accepted', + dispatchState: 'not_dispatched', + invocationId: 'inv_unsent', + handlerNodeId: 'node_sf_frame', + }); + }); + + it('reports dispatched once the engine records the dispatched node', () => { + expect( + spawnPlacementReceipt({ + invocation_id: 'inv_sent', + status: 'pending', + handler_node_id: 'node_sf_frame', + dispatched_node_id: 'node_sf_frame', + }) + ).toMatchObject({ dispatchState: 'dispatched', dispatchedNodeId: 'node_sf_frame' }); + }); +}); diff --git a/packages/cli/src/cli/lib/spawn-lifecycle.ts b/packages/cli/src/cli/lib/spawn-lifecycle.ts index 45042837f..91a8f9261 100644 --- a/packages/cli/src/cli/lib/spawn-lifecycle.ts +++ b/packages/cli/src/cli/lib/spawn-lifecycle.ts @@ -10,13 +10,14 @@ function text(value: unknown): string | undefined { return typeof value === 'string' && value.trim() ? value : undefined; } +// `handler_node_id` is the node placement chose, snapshotted before any send, +// so it is present even when the send failed; only `dispatched_node_id` (or a +// status the node itself moved) shows the node received the invocation. function dispatchEvidence(value: Record): SpawnDispatchState { const status = text(value.status)?.toLowerCase(); if ( text(value.dispatchedNodeId) || text(value.dispatched_node_id) || - text(value.handlerNodeId) || - text(value.handler_node_id) || (status !== undefined && (DISPATCHED.has(status) || SUCCESS.has(status))) ) { return 'dispatched'; diff --git a/packages/sdk/src/messaging/placement.test.mts b/packages/sdk/src/messaging/placement.test.mts index 17707462a..1baf3442a 100644 --- a/packages/sdk/src/messaging/placement.test.mts +++ b/packages/sdk/src/messaging/placement.test.mts @@ -947,6 +947,43 @@ describe('RelaycastMessagingClient placement', () => { expect((error as RelayPlacementError).dispatchState).toBe('not_dispatched'); }); + // The engine's 201 for a placed spawn snapshots the chosen node in + // handler_node_id before it sends. A send that fails leaves + // dispatched_node_id null, so the placed node alone is not a route. + it('reports dispatchState not_dispatched on timeout when the ack names only the placed handler node', async () => { + const { client, invoke } = createClient([LIVE_NODE_A], { + getInvocation: async (name, invocationId) => ({ + invocation_id: invocationId, + action_name: name, + status: 'pending', + dispatched_node_id: null, + }), + }); + invoke.mockResolvedValueOnce({ + invocation_id: 'inv-placed-unsent', + action_name: 'spawn', + handler_node_id: 'node_a', + dispatched_node_id: null, + status: 'pending', + }); + + const error = await client.placement + .spawn({ + capability: 'spawn:claude', + node: 'node-a', + repo: 'relay', + confirm: true, + confirmTimeoutMs: 60, + confirmPollIntervalMs: 10, + input: { name: 'worker-placed-unsent' }, + }) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(RelayPlacementError); + expect((error as RelayPlacementError).code).toBe('spawn_unconfirmed'); + expect((error as RelayPlacementError).dispatchState).toBe('not_dispatched'); + }); + it('reports dispatchState not_dispatched on a terminal failure when the ack never routed to a node', async () => { const { client, invoke } = createClient([LIVE_NODE_A], { getInvocation: async (name, invocationId) => ({ diff --git a/packages/sdk/src/messaging/relaycast-placement.ts b/packages/sdk/src/messaging/relaycast-placement.ts index 50a8872d7..2b904788a 100644 --- a/packages/sdk/src/messaging/relaycast-placement.ts +++ b/packages/sdk/src/messaging/relaycast-placement.ts @@ -45,18 +45,21 @@ const NOT_DISPATCHED_STATUSES = new Set(['pending', 'queued']); /** * Classify dispatch evidence from an invocation ack (or its terminal read * back), matching the CLI's `spawnLifecycleState`/`dispatchEvidence` - * semantics: a `dispatchedNodeId`/`handlerNodeId` is real evidence a node - * received the dispatch. Its absence, with a `pending`/`queued` status, means - * the invocation never actually routed — even if the caller later observes a + * semantics: a `dispatchedNodeId` is real evidence a node received the + * dispatch. Its absence, with a `pending`/`queued` status, means the + * invocation never actually routed — even if the caller later observes a * terminal (timeout or error) outcome for it. A timeout or terminal error is * a fact about the *caller's* wait, not evidence that dispatch happened. + * + * `handlerNodeId` is deliberately not evidence: the engine snapshots the + * node placement *chose* there before any send, so an ack whose send failed + * still carries it alongside `dispatched_node_id: null`. */ export function resolveDispatchState(ack: { dispatchedNodeId?: string | null; - handlerNodeId?: string | null; status?: string; }): RelaySpawnDispatchState { - if (ack.dispatchedNodeId || ack.handlerNodeId) return 'dispatched'; + if (ack.dispatchedNodeId) return 'dispatched'; const status = ack.status?.toLowerCase(); if (status && DISPATCH_EVIDENT_STATUSES.has(status)) return 'dispatched'; if (status && NOT_DISPATCHED_STATUSES.has(status)) return 'not_dispatched'; From 1b3e7294bf2c58cb7ac1edf47d2d203d2b183a18 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Tue, 13 Oct 2026 10:29:49 -0700 Subject: [PATCH 2/2] fix(sdk): take dispatch evidence from confirmation reads; align the 1563 proof Review on #1972 raised three points. - Confirmation fixed dispatchState at ack time. A pending ack that the engine dispatched afterwards was still reported as not_dispatched when confirmation later failed. A confirmation read that carries a dispatchedNodeId now upgrades the state. A read's status does not, because the normalizer defaults a missing status to `invoked`. - The relay#1563 RelayFlow denial fixture named the node only in handler_node_id, which is now placement, not evidence. node_a denied what it received, so the ack now also carries dispatched_node_id. - The changelog heading is now [Unreleased - Patch] and the entry is shorter. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 +- packages/sdk/src/messaging/placement.test.mts | 48 +++++++++++++++++++ packages/sdk/src/messaging/relaycast.ts | 19 ++++---- .../cases/1563-spawn-lifecycle-truth/run.mjs | 3 ++ 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30810b59a..13b81a0eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,11 @@ All notable changes to Agent Relay will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Patch] ### Fixed -- `fleet spawn` and the SDK placement no longer report `dispatchState: "dispatched"` for a spawn the engine never sent. The engine's acknowledgement always names the node placement chose in `handler_node_id`, even when sending to it failed, so that field is no longer read as dispatch evidence; only `dispatched_node_id` (or a status the node moved) is. `fleet spawn` and `fleet spawn-status` now agree. +- `fleet spawn` and SDK placement no longer report `dispatchState: "dispatched"` for a spawn that was placed but never sent. Only `dispatched_node_id`, or a status the node moved, counts as dispatch evidence, so `fleet spawn` and `fleet spawn-status` agree. ## [13.4.1] - 2026-10-12 diff --git a/packages/sdk/src/messaging/placement.test.mts b/packages/sdk/src/messaging/placement.test.mts index 1baf3442a..437de69fc 100644 --- a/packages/sdk/src/messaging/placement.test.mts +++ b/packages/sdk/src/messaging/placement.test.mts @@ -984,6 +984,54 @@ describe('RelaycastMessagingClient placement', () => { expect((error as RelayPlacementError).dispatchState).toBe('not_dispatched'); }); + // The engine can dispatch a pending invocation after acknowledging it + // (a node that reconnects drains it). A read that shows the route is + // evidence the ack lacked. + it('reports dispatchState dispatched when a confirmation read shows a route the pending ack lacked', async () => { + let reads = 0; + const { client, invoke } = createClient([LIVE_NODE_A], { + getInvocation: async (name, invocationId) => { + reads += 1; + return reads === 1 + ? { + invocation_id: invocationId, + action_name: name, + status: 'dispatched', + dispatched_node_id: 'node_a', + } + : { + invocation_id: invocationId, + action_name: name, + status: 'failed', + error: 'harness exited before readiness', + }; + }, + }); + invoke.mockResolvedValueOnce({ + invocation_id: 'inv-pending-then-dispatched', + action_name: 'spawn', + handler_node_id: 'node_a', + dispatched_node_id: null, + status: 'pending', + }); + + const error = await client.placement + .spawn({ + capability: 'spawn:claude', + node: 'node-a', + repo: 'relay', + confirm: true, + confirmTimeoutMs: 1_000, + confirmPollIntervalMs: 10, + input: { name: 'worker-pending-then-dispatched' }, + }) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(RelayPlacementError); + expect((error as RelayPlacementError).code).toBe('spawn_failed'); + expect((error as RelayPlacementError).dispatchState).toBe('dispatched'); + }); + it('reports dispatchState not_dispatched on a terminal failure when the ack never routed to a node', async () => { const { client, invoke } = createClient([LIVE_NODE_A], { getInvocation: async (name, invocationId) => ({ diff --git a/packages/sdk/src/messaging/relaycast.ts b/packages/sdk/src/messaging/relaycast.ts index 3855d2aed..5c1eba08b 100644 --- a/packages/sdk/src/messaging/relaycast.ts +++ b/packages/sdk/src/messaging/relaycast.ts @@ -1006,14 +1006,14 @@ export class RelaycastMessagingClient implements RelayMessagingClient { ): Promise { const { timeoutMs, pollIntervalMs, requireReadiness, ...errorContext } = context; const invocationId = ack.invocationId; - // Dispatch evidence is fixed at ack time: a node id here means the engine - // already routed the invocation to a node before this method starts - // polling. A `pending`/`queued` ack with no node id has not routed yet, - // and confirmation timing out or reading a terminal status later does not - // retroactively manufacture that evidence — see `resolveDispatchState`. - // Computed before either early exit below so every `spawn_unconfirmed` - // path, not just the timeout/failure paths, carries this evidence. - const dispatchState = resolveDispatchState(ack); + // Dispatch evidence starts from the ack: a `dispatchedNodeId` means the + // engine already routed the invocation to a node. A `pending`/`queued` ack + // without one has not routed yet. A read below that carries a + // `dispatchedNodeId` adds evidence (the engine may dispatch after + // acknowledging); a timeout or a terminal status never manufactures it — see + // `resolveDispatchState`. Computed before either early exit below so every + // `spawn_unconfirmed` path, not just the timeout/failure paths, carries it. + let dispatchState = resolveDispatchState(ack); if (!invocationId) { throw new RelayPlacementError( 'spawn_unconfirmed', @@ -1061,6 +1061,9 @@ export class RelaycastMessagingClient implements RelayMessagingClient { // A later success must not report an earlier transient failure. lastReadError = undefined; const invocation = outcome.value; + // Only a node id counts here: the read normalizer defaults a + // missing status to `invoked`, which is not evidence. + if (invocation?.dispatchedNodeId) dispatchState = 'dispatched'; const status = invocation?.status?.toLowerCase(); if (status && CONFIRM_SUCCESS_STATUSES.has(status)) { if ( diff --git a/tests/relayflows/cases/1563-spawn-lifecycle-truth/run.mjs b/tests/relayflows/cases/1563-spawn-lifecycle-truth/run.mjs index 017cdcd1f..6f0e7e2f7 100644 --- a/tests/relayflows/cases/1563-spawn-lifecycle-truth/run.mjs +++ b/tests/relayflows/cases/1563-spawn-lifecycle-truth/run.mjs @@ -454,10 +454,13 @@ try { // Confirm:false is also the CLI's --no-confirm path. A terminal denial in // the ack is already a known failure, and its invocation/receipt correlation // must survive without entering the confirmation poll. + // node_a received the invocation and denied it, so the ack names it as the + // dispatched node; handler_node_id alone is only placement (relay#1972). const immediateDenied = createSdkClient(async () => undefined, { invocation_id: 'inv_immediate_denied', action_name: 'spawn:claude', handler_node_id: 'node_a', + dispatched_node_id: 'node_a', status: 'denied', error: 'node denied the spawn', });