From 640756d04d82357a583c0cae2cce55f6849214a9 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 23 Aug 2026 21:49:21 +0200 Subject: [PATCH 1/3] fix(fleet): treat absence from a readable presence list as confirmed offline (factory-cloud#55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#assertAgentNotLive` refused to reclaim an orphaned agent row whenever presence did not list the agent: "presence does not list this agent, so it cannot be confirmed offline". That is inverted. Absence from a presence list we actually READ is the strongest evidence the engine can give that the agent is offline. The consequence was a permanent latch. Every registration attempt hit the same refusal, the bounded attempt budget only resets on success, and dispatch stayed gated for a week. Fix 1 (#343) stops the `factory status` preflight planting a fresh orphan; this makes any orphan that already exists recoverable, so the next hard crash or ungraceful exit does not reproduce the outage. The two presence outcomes are now routed apart, and neither is collapsed into the other: - presence UNREADABLE (threw, timed out, or came back as something other than a list) is evidence of nothing and stays fail-closed. Split into `#readPresenceRows`, whose every failure message names presence as "unreadable" so an operator can tell it from the other case. - presence READABLE and omitting the agent is confirmed absence, and reclaim proceeds. An empty list reads the same way: a single-agent cloud factory whose only row is the dead one sees exactly that, and refusing it would re-latch the outage in its most common shape. A row that IS listed still has to clear the status allow-list, so a missing or unrecognised status remains fail-closed. Tests: a must-fire/must-not-fire pair on the real RelayFleetClient with only the transport stubbed — readable-and-absent (and readable-and-empty) reclaims and issues the takeover for the right agent id; unreadable presence refuses and never calls takeover. A control arm runs both shapes through one harness that differs in nothing but the presence stub and asserts the outcomes differ, which fails an inert fixture. Verified against a swapped-branch mutant: it fails 4 tests, including the assertion that a refusal must not claim absence. Co-Authored-By: Claude Opus 5 --- .claude/settings.json | 7 ++ src/fleet/relay-fleet-client.test.ts | 98 +++++++++++++++++++++++++++- src/fleet/relay-fleet-client.ts | 81 +++++++++++++++-------- 3 files changed, 155 insertions(+), 31 deletions(-) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..5123f1d7 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,7 @@ +{ + "permissions": { + "allow": [ + "mcp__relaycast__*" + ] + } +} diff --git a/src/fleet/relay-fleet-client.test.ts b/src/fleet/relay-fleet-client.test.ts index be4f40b0..164a60a1 100644 --- a/src/fleet/relay-fleet-client.test.ts +++ b/src/fleet/relay-fleet-client.test.ts @@ -1378,13 +1378,17 @@ describe('RelayFleetClient', () => { }, ) - // A stale `offline` record plus an unreadable presence must never authorise a + // A stale `offline` record plus an UNREADABLE presence must never authorise a // seizure: the cost of being wrong is stranding a LIVE factory's credential, // and the token taken is the one it would have needed to recover. + // + // "Unreadable" means exactly that — the request failed, or the body was not a + // list. A presence list that WAS read and simply omits this agent is the + // opposite case and is covered by the must-fire tests below; conflating the + // two is the defect this pair exists to pin. it.each([ ['presence request throws', () => { throw new Error('presence unavailable') }], ['presence returns a non-list', () => ({ not: 'a list' })], - ['presence omits this agent', () => [{ agentName: 'someone-else', status: 'offline' }]], ['presence entry carries no status', () => [{ agentName: 'factory' }]], ])('fails closed and does not take over when %s', async (_label, impl) => { const messaging = new FakeMessaging() @@ -1411,11 +1415,99 @@ describe('RelayFleetClient', () => { const error = await fleet.roster().then(() => undefined, (err: unknown) => err) expect((error as Error).name).toBe('FactoryAgentRegistrationError') - expect((error as Error).message).toMatch(/cannot be confirmed offline|could not read presence/) + expect((error as Error).message).toMatch(/cannot be confirmed offline/) + // The refusal must not claim absence from presence — that is the other, + // now-permitted case, and an operator has to be able to tell them apart. + expect((error as Error).message).not.toMatch(/does not list this agent/) // The seizure must never have been attempted. expect(takeovers).toBe(0) }) + // The must-fire half of the pair. Absence from a presence list we actually + // READ is the strongest evidence the engine can give that the agent is + // offline; refusing to conclude it is what left an orphaned row permanently + // unreclaimable and gated dispatch (factory-cloud#55). The empty-list arm is + // the shape a single-agent cloud factory sees, where the dead row is the only + // agent there is. + it.each([ + ['presence lists only other agents', () => [{ agentName: 'someone-else', status: 'online' }]], + ['presence is readable and empty', () => []], + ])('reclaims the identity when %s', async (_label, impl) => { + const messaging = new FakeMessaging() + const { agents, presenceImpl } = deprecatedAliasAgents() + await (agents.register as (i: { name: string }) => Promise)({ name: 'factory' }) + presenceImpl.value = impl as () => unknown + const bootstrap: RelayClientLike = { messaging: { agents } as unknown as RelayMessaging } + const calls: Array<{ url: string; body: Record }> = [] + const fleet = new RelayFleetClient({ + workspaceKey: 'rk_live_test', + nodeId: 'node_test_1', + env: {}, + sleep: immediateSleep, + pollIntervalMs: 0, + fetch: (async (url: string, init: { body: string }) => { + calls.push({ url, body: JSON.parse(init.body) as Record }) + return new Response( + JSON.stringify({ ok: true, data: { agent_id: 'agent-1', name: 'factory', token: 'at_live_seized' } }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ) + }) as unknown as typeof globalThis.fetch, + createRelay: (options) => (options.agentToken ? { messaging: messaging.asMessaging() } : bootstrap), + }) + + await expect(fleet.roster()).resolves.toBeDefined() + + // Resolving is not enough on its own: the takeover must actually have been + // issued, for this record, rather than the conflict being routed elsewhere. + expect(calls).toHaveLength(1) + expect(calls[0]?.url).toBe('https://cast.agentrelay.com/v1/agents/factory/takeover') + expect(calls[0]?.body.expected_agent_id).toBe('agent-1') + }) + + // Control arm. Both presence shapes run through ONE harness that differs in + // nothing but the presence stub, so no setup difference can absorb a swap of + // the two branches. The `not.toBe` assertion additionally kills the inert + // fixture — the failure mode where both arms take the same path for a reason + // unrelated to presence, which is exactly what would let a swapped + // implementation pass a pair of separately-written tests. + it('separates readable-and-absent from unreadable presence, and would notice a swap', async () => { + const run = async (impl: () => unknown): Promise<{ seized: boolean; message: string }> => { + const messaging = new FakeMessaging() + const { agents, presenceImpl } = deprecatedAliasAgents() + await (agents.register as (i: { name: string }) => Promise)({ name: 'factory' }) + presenceImpl.value = impl + const bootstrap: RelayClientLike = { messaging: { agents } as unknown as RelayMessaging } + let takeovers = 0 + const fleet = new RelayFleetClient({ + workspaceKey: 'rk_live_test', + env: {}, + sleep: immediateSleep, + pollIntervalMs: 0, + fetch: (async () => { + takeovers += 1 + return new Response( + JSON.stringify({ ok: true, data: { agent_id: 'agent-1', name: 'factory', token: 'at_live_seized' } }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ) + }) as unknown as typeof globalThis.fetch, + createRelay: (options) => (options.agentToken ? { messaging: messaging.asMessaging() } : bootstrap), + }) + const error = await fleet.roster().then(() => undefined, (err: unknown) => err) + return { seized: takeovers > 0, message: error instanceof Error ? error.message : '' } + } + + const absent = await run(() => [{ agentName: 'someone-else', status: 'online' }]) + const unreadable = await run(() => { throw new Error('presence unavailable') }) + + expect(absent.seized).toBe(true) + expect(unreadable.seized).toBe(false) + // Fails if the harness cannot tell the two apart at all. + expect(absent.seized).not.toBe(unreadable.seized) + // And the refusal has to say WHICH of the two it was. + expect(unreadable.message).toMatch(/presence is unreadable/) + expect(absent.message).toBe('') + }) + it('re-reads the agent id and retries once when the identity moved mid-takeover', async () => { const messaging = new FakeMessaging() const { agents } = deprecatedAliasAgents() diff --git a/src/fleet/relay-fleet-client.ts b/src/fleet/relay-fleet-client.ts index e06b7609..e75c00ae 100644 --- a/src/fleet/relay-fleet-client.ts +++ b/src/fleet/relay-fleet-client.ts @@ -921,44 +921,40 @@ export class RelayFleetClient implements FleetClient { } // Presence is the canonical liveness signal, and the agent record can be - // stale. Everything below fails CLOSED: only a presence row we actually - // read, for this agent, carrying a status we recognise as not-live, may - // authorise a seizure. A request that failed, a body we cannot parse, or a - // missing row is absence of evidence — not evidence of death. + // stale. Two outcomes are routed apart here, and collapsing them swaps one + // wrong conclusion for another: // - // This asymmetry is deliberate. Refusing to boot costs us a recoverable - // outage of our own; seizing a live agent's credential strands another - // running process and takes the very token it would need to recover. - let presence: unknown - try { - presence = await agents.presence() - } catch (error) { - throw new FactoryAgentRegistrationError( - this.#agentName, - `could not read presence to confirm the agent is not live: ${errorMessage(error)}`, - { cause: error }, - ) - } - if (!Array.isArray(presence)) { - throw new FactoryAgentRegistrationError( - this.#agentName, - 'presence did not return a list, so the agent cannot be confirmed offline', - ) - } + // presence UNREADABLE (threw, timed out, or came back as something other + // than a list) — evidence of nothing. Fails CLOSED, because seizing a + // live agent's credential strands another running process and takes the + // very token it would need to recover. + // + // presence READABLE and omitting this agent — the strongest evidence the + // engine can give that the agent is offline. Proceeds. Treating this as + // inconclusive is what made an orphaned row permanently unreclaimable + // (factory-cloud#55): every registration attempt burned the bounded + // budget, which only a success resets, and dispatch stayed gated. + // + // A row that IS listed still has to clear the status check below: presence + // listing us is the case where the engine may still be holding the + // identity, so an unrecognised or missing status stays fail-closed. + const presence = await this.#readPresenceRows(agents) const entry = presence .map((row) => asRecord(row)) .find((row) => readString(row, 'agentName', 'agent_name', 'name') === this.#agentName) if (!entry) { - throw new FactoryAgentRegistrationError( - this.#agentName, - 'presence does not list this agent, so it cannot be confirmed offline', - ) + // Confirmed absence. Note this is also the correct reading of an empty + // list: a single-agent cloud factory whose only row is the dead one sees + // exactly that, and refusing it would re-latch the outage in its most + // common shape. + this.#log(`Presence is readable and does not list ${this.#agentName}; treating the existing record as offline`) + return } const seenStatus = readString(entry, 'status') if (seenStatus === undefined) { throw new FactoryAgentRegistrationError( this.#agentName, - 'presence reported no status for this agent, so it cannot be confirmed offline', + 'presence lists this agent with no status, so it cannot be confirmed offline', ) } if (LIVE_AGENT_STATUSES.has(seenStatus)) { @@ -969,6 +965,35 @@ export class RelayFleetClient implements FleetClient { } } + /** + * Read the presence list, or fail closed. + * + * Every failure here means presence could not be read at all — a transport + * error, a timeout, or a body that is not a list. None of them say anything + * about whether the agent is alive, so none of them may authorise a seizure. + * The messages all name presence as UNREADABLE so an operator can tell this + * apart from a presence list that was read and simply did not list us. + */ + async #readPresenceRows(agents: RelayMessaging['agents']): Promise { + let presence: unknown + try { + presence = await agents.presence() + } catch (error) { + throw new FactoryAgentRegistrationError( + this.#agentName, + `presence is unreadable, so the agent cannot be confirmed offline: ${errorMessage(error)}`, + { cause: error }, + ) + } + if (!Array.isArray(presence)) { + throw new FactoryAgentRegistrationError( + this.#agentName, + 'presence is unreadable (it did not return a list), so the agent cannot be confirmed offline', + ) + } + return presence + } + async #takeOverAgent(workspaceKey: string, expectedAgentId: string): Promise { const base = (this.#options.baseUrl ?? DEFAULT_RELAY_BASE_URL).replace(/\/+$/, '') const url = `${base}/v1/agents/${encodeURIComponent(this.#agentName)}/takeover` From 6029899cd4ce66b3821141f21a9061b98df894b1 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 23 Aug 2026 22:01:21 +0200 Subject: [PATCH 2/3] fix(fleet): fail closed when presence rows carry no recognisable name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 on #347. `#readPresenceRows` accepted any array, so a well-formed list of rows we cannot name — `[{}]`, or rows whose naming field the SDK renamed — passed as "readable". Every row would then fail the name match, a LIVE agent's row included, and omission would be read as confirmed absence: the one path by which the newly-permitted branch could strand a running factory's credential. Omission only means absence if a row FOR this agent would have been recognised, so the check belongs on the read, not inferred from the lookup missing. An unnameable row now makes the whole list unreadable and fails closed, naming the offending row index. Two arms added to the fail-closed table (no name; unrecognised naming field). Verified fail-first: with only this guard ablated, exactly those two fail. Co-Authored-By: Claude Opus 5 --- src/fleet/relay-fleet-client.test.ts | 5 +++++ src/fleet/relay-fleet-client.ts | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/fleet/relay-fleet-client.test.ts b/src/fleet/relay-fleet-client.test.ts index 164a60a1..4de8e8d8 100644 --- a/src/fleet/relay-fleet-client.test.ts +++ b/src/fleet/relay-fleet-client.test.ts @@ -1390,6 +1390,11 @@ describe('RelayFleetClient', () => { ['presence request throws', () => { throw new Error('presence unavailable') }], ['presence returns a non-list', () => ({ not: 'a list' })], ['presence entry carries no status', () => [{ agentName: 'factory' }]], + // Omission only means absence if a row FOR this agent would have been + // recognised. A row we cannot name breaks that: if the SDK renamed the + // naming field, a LIVE agent's row stops matching and reads as absent. + ['a presence row carries no agent name', () => [{}]], + ['presence rows use an unrecognised naming field', () => [{ agent: 'factory', status: 'online' }]], ])('fails closed and does not take over when %s', async (_label, impl) => { const messaging = new FakeMessaging() const { agents, presenceImpl } = deprecatedAliasAgents() diff --git a/src/fleet/relay-fleet-client.ts b/src/fleet/relay-fleet-client.ts index e75c00ae..a49b7677 100644 --- a/src/fleet/relay-fleet-client.ts +++ b/src/fleet/relay-fleet-client.ts @@ -991,6 +991,22 @@ export class RelayFleetClient implements FleetClient { 'presence is unreadable (it did not return a list), so the agent cannot be confirmed offline', ) } + // A list whose rows we cannot name is not readable either, however + // well-formed the array around them is. Omission only means absence if a + // row FOR this agent would have been recognised — so if the SDK renames the + // naming field, or a row arrives without one, every row silently stops + // matching and a live agent reads as absent. That is the one way the + // permitted branch below could strand a running factory, so it is checked + // here, on the read, rather than inferred from the lookup missing. + const unnamed = presence.findIndex( + (row) => readString(asRecord(row), 'agentName', 'agent_name', 'name') === undefined, + ) + if (unnamed !== -1) { + throw new FactoryAgentRegistrationError( + this.#agentName, + `presence is unreadable (row ${unnamed} carries no agent name), so the agent cannot be confirmed offline`, + ) + } return presence } From dfa67784c9bf8b5ac4a17d45da9c29454602e291 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Sun, 23 Aug 2026 23:42:43 +0200 Subject: [PATCH 3/3] chore: drop unrelated .claude/settings.json permission grant from #347 A permissions broadening does not belong in a fleet reclaim fix, and it is the principal's decision rather than a lane's. Removed so #347 carries only the reclaim-inversion change. Raise the MCP allow-list separately if wanted. --- .claude/settings.json | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json deleted file mode 100644 index 5123f1d7..00000000 --- a/.claude/settings.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "permissions": { - "allow": [ - "mcp__relaycast__*" - ] - } -}