From 5dae9b0198a334268d85d6363f4509a1d8e4e50f Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Thu, 17 Sep 2026 18:50:36 +0300 Subject: [PATCH 1/3] fix(peer): A2A interop with real external agents (CLEAN-97) A live run of 41 public A2A 1.0 agents from a2aregistry.org through the real import and delegation code showed the circuit works and interop does not: 40 imported, 6 answered with content. After this change the same run gives 36 answers with content; the remaining 4 are agents that require structured input, and they now say so. - Message replies. A blocking SendMessage may be answered with a Message instead of a Task; most public agents do exactly that. The client returns either, and delegation reads the answer from both. - Reply text. Every artifact part is read, not only the first artifact's text: data parts become compact JSON, links stay links, binary is named but not inlined. An empty artifact list falls back to the task's status message. Only when nothing anywhere is readable is the reply recorded as an explicit empty answer, and the tool tells the model not to invent one. - Interface choice. The first JSON-RPC interface on 1.0 is used, not the first interface listed, in import and delegation alike. A 1.0 card without JSON-RPC is refused at import with PEER_BINDING. - The card's own address. Import and external refresh now vet the chosen interface URL: SSRF-guarded, and refused when it points back into this installation. Rows saved before this fail delegation with PEER_ADDRESS_REFUSED or PEER_UNSUPPORTED instead of "it answered with an error". - Self-imports. "Is this our own agent" compares a normalized address (host case, trailing dot, port, decoded and collapsed path, scheme ignored) instead of a string prefix. Doubled slashes and %61gents no longer pass, and a card whose interface names our base is refused whatever host served it. - Honest causes. A 0.3 card reports its version instead of "not an agent card". A JSON-RPC error sent with a 4xx status passes the peer's reason on instead of "could not be reached". Redirects are still never followed, but are now reported with their target, so the operator knows what to paste. - Card URLs that already name a .json document are kept as they are, so /.well-known/agent.json and custom card paths are no longer mangled. Co-Authored-By: Claude Opus 5 (1M context) --- .../slices/agent/peer/askAgent.tool.spec.ts | 26 +- api/src/slices/agent/peer/askAgent.tool.ts | 13 +- .../agent/peer/domain/a2a.client.spec.ts | 160 +++++++++- .../slices/agent/peer/domain/a2a.client.ts | 150 ++++++++-- .../agent/peer/domain/a2a.types.spec.ts | 174 +++++++++++ api/src/slices/agent/peer/domain/a2a.types.ts | 84 ++++++ .../peer/domain/delegation.service.spec.ts | 273 +++++++++++++++++- .../agent/peer/domain/delegation.service.ts | 76 +++-- .../agent/peer/domain/delegationStep.ts | 4 + .../agent/peer/domain/peer.service.spec.ts | 262 ++++++++++++++++- .../slices/agent/peer/domain/peer.service.ts | 178 ++++++++++-- .../slices/agent/peer/domain/peer.types.ts | 17 +- .../agent/peer/dtos/agentDelegation.dto.ts | 5 +- 13 files changed, 1327 insertions(+), 95 deletions(-) create mode 100644 api/src/slices/agent/peer/domain/a2a.types.spec.ts diff --git a/api/src/slices/agent/peer/askAgent.tool.spec.ts b/api/src/slices/agent/peer/askAgent.tool.spec.ts index d17450e0..87a1eba3 100644 --- a/api/src/slices/agent/peer/askAgent.tool.spec.ts +++ b/api/src/slices/agent/peer/askAgent.tool.spec.ts @@ -170,7 +170,9 @@ describe('AskAgentTool — the description the model reads', () => { const description = (await tool.describeForRequest(agentRequest())) ?? ''; // FR-011: the model must try a plausible peer before saying I don't know. - const giveUp = description.search(/before you answer that you do not know/i); + const giveUp = description.search( + /before you answer that you do not know/i, + ); const caveat = description.search(/not a first resort/i); expect(giveUp).toBeGreaterThanOrEqual(0); expect(caveat).toBeGreaterThan(giveUp); @@ -298,6 +300,28 @@ describe('AskAgentTool — calling it', () => { expect(text).toContain('Shoes can be returned within 30 days.'); }); + it('says outright that a peer answered with nothing, so the model does not fill the gap (CLEAN-97)', async () => { + const { tool, args } = makeHarness({ + outcome: { + kind: 'done', + status: DelegationStatuses.Answered, + peerName: 'Support Bot', + contextId: 'ctx-1', + durationMs: 800, + text: '', + }, + }); + + const result = await tool.ask(args, null, agentRequest()); + + expect(result.isError).toBeUndefined(); + const text = textOf(result); + expect(text).toContain('«Support Bot» answered'); + expect(text).toContain('the reply was empty'); + expect(text).toMatch(/do not invent/i); + expect(text).not.toContain('Reply from'); + }); + it('tells the model not to answer for a peer it could not reach', async () => { const { tool, args } = makeHarness({ outcome: { diff --git a/api/src/slices/agent/peer/askAgent.tool.ts b/api/src/slices/agent/peer/askAgent.tool.ts index 4060b844..2f095fc6 100644 --- a/api/src/slices/agent/peer/askAgent.tool.ts +++ b/api/src/slices/agent/peer/askAgent.tool.ts @@ -203,8 +203,15 @@ export class AskAgentTool const took = `${(outcome.durationMs / 1000).toFixed(1)}s`; if (outcome.status === DelegationStatuses.Answered) { + if (!outcome.text) { + // Said explicitly: an empty tool result reads to a model like "no + // news", and it will fill the gap itself (CLEAN-97). + return ok( + `«${outcome.peerName}» answered (context_id: ${outcome.contextId}, ${took}), but the reply was empty — no text, data or links. Tell the user it returned nothing; do not invent what it might have said.`, + ); + } return ok( - `Reply from «${outcome.peerName}» (context_id: ${outcome.contextId}, ${took}):\n\n${outcome.text ?? ''}`, + `Reply from «${outcome.peerName}» (context_id: ${outcome.contextId}, ${took}):\n\n${outcome.text}`, ); } @@ -234,7 +241,9 @@ function describePeer(connection: IAgentPeerData): string { // External peers have no agent id here; the connection id names them just // as reliably — matchPeer resolves both (CLEAN-95). - const parts = [`- "${name}" (peer: ${connection.peerAgentId ?? connection.id})`]; + const parts = [ + `- "${name}" (peer: ${connection.peerAgentId ?? connection.id})`, + ]; if (description) parts.push(` — ${description}`); if (skills) parts.push(` Skills: ${skills}`); return parts.join(''); diff --git a/api/src/slices/agent/peer/domain/a2a.client.spec.ts b/api/src/slices/agent/peer/domain/a2a.client.spec.ts index 4628e8ed..bf383317 100644 --- a/api/src/slices/agent/peer/domain/a2a.client.spec.ts +++ b/api/src/slices/agent/peer/domain/a2a.client.spec.ts @@ -43,11 +43,17 @@ function respond(options: { json?: unknown; text?: string; jsonThrows?: boolean; + location?: string; }) { const status = options.status ?? 200; return { ok: options.ok ?? (status >= 200 && status < 300), status, + type: 'basic', + headers: { + get: (name: string) => + name.toLowerCase() === 'location' ? (options.location ?? null) : null, + }, json: async () => { if (options.jsonThrows) throw new Error('not json'); return options.json; @@ -86,10 +92,29 @@ describe('A2aClient.fetchCard', () => { CARD_URL, expect.objectContaining({ headers: expect.objectContaining({ Authorization: `Bearer ${TOKEN}` }), - // Redirects are an SSRF-guard bypass; the client refuses them. - redirect: 'error', + // Redirects are an SSRF-guard bypass: never followed, only reported. + redirect: 'manual', + }), + ); + }); + + it('refuses a redirecting card and names where it points, without going there', async () => { + fetchMock.mockResolvedValue( + respond({ + status: 301, + location: 'https://agent.example/.well-known/agent-card.json', }), ); + + await expect( + new A2aClient().fetchCard(CARD_URL, TOKEN), + ).rejects.toMatchObject({ + kind: 'invalid', + message: expect.stringContaining( + 'redirects to https://agent.example/.well-known/agent-card.json', + ), + }); + expect(fetchMock).toHaveBeenCalledTimes(1); }); it('reports an unreachable host in words an operator can act on', async () => { @@ -105,16 +130,19 @@ describe('A2aClient.fetchCard', () => { await new A2aClient().fetchCard(CARD_URL); - const headers = fetchMock.mock.calls[0][1].headers as Record; + const headers = fetchMock.mock.calls[0][1].headers as Record< + string, + string + >; expect(headers).not.toHaveProperty('Authorization'); }); it('marks not-a-card answers as invalid, not unreachable (CLEAN-95)', async () => { fetchMock.mockResolvedValue(respond({ json: { hello: 'world' } })); - await expect(new A2aClient().fetchCard(CARD_URL, TOKEN)).rejects.toMatchObject( - { kind: 'invalid' }, - ); + await expect( + new A2aClient().fetchCard(CARD_URL, TOKEN), + ).rejects.toMatchObject({ kind: 'invalid' }); }); it('names a timeout as a timeout rather than an abort', async () => { @@ -149,9 +177,40 @@ describe('A2aClient.fetchCard', () => { it('refuses a document that parses but is not an agent card', async () => { fetchMock.mockResolvedValue(respond({ json: { hello: 'world' } })); - await expect(new A2aClient().fetchCard(CARD_URL, TOKEN)).rejects.toThrow( - /not an agent card/, + await expect( + new A2aClient().fetchCard(CARD_URL, TOKEN), + ).rejects.toMatchObject({ + kind: 'invalid', + message: expect.stringMatching(/not an agent card/), + }); + }); + + it('names the protocol version of a pre-1.0 card instead of calling it "not a card"', async () => { + // The shape most a2a-samples still serve: version at the top level, a + // single `url`, no supportedInterfaces. + fetchMock.mockResolvedValue( + respond({ + json: { + protocolVersion: '0.3.0', + name: 'Legacy Agent', + description: 'x', + url: 'https://legacy.example/', + preferredTransport: 'JSONRPC', + version: '1.0.0', + capabilities: {}, + defaultInputModes: ['text'], + defaultOutputModes: ['text'], + skills: [], + }, + }), ); + + await expect( + new A2aClient().fetchCard(CARD_URL, TOKEN), + ).rejects.toMatchObject({ + kind: 'version', + message: 'This agent speaks A2A 0.3.0; only 1.0 is supported', + }); }); it('refuses a card with no way to reach the agent', async () => { @@ -181,7 +240,7 @@ describe('A2aClient.sendMessage', () => { const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; expect(url).toBe(RPC_URL); expect(init.method).toBe('POST'); - expect(init.redirect).toBe('error'); + expect(init.redirect).toBe('manual'); expect(init.headers).toMatchObject({ Authorization: `Bearer ${TOKEN}`, 'Content-Type': 'application/json', @@ -199,7 +258,7 @@ describe('A2aClient.sendMessage', () => { await expect( new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), - ).resolves.toEqual(task); + ).resolves.toEqual({ task }); }); it('calls a revoked credential what it is', async () => { @@ -231,6 +290,56 @@ describe('A2aClient.sendMessage', () => { }); }); + it('passes on the reason a peer gives in a JSON-RPC error sent with a 4xx status', async () => { + // Real public agents do this: the request arrived, the peer refused it and + // said why. That is an answer from the peer, not a network problem. + fetchMock.mockResolvedValue( + respond({ + status: 400, + text: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + error: { code: -32602, message: 'Send a structured DataPart' }, + }), + }), + ); + + await expect( + new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), + ).rejects.toMatchObject({ + code: DelegationErrorCodes.Error, + message: 'Send a structured DataPart', + }); + }); + + it('reports a redirecting peer as unreachable, with the target, and does not follow it', async () => { + fetchMock.mockResolvedValue( + respond({ status: 302, location: 'http://10.0.0.1/rpc' }), + ); + + await expect( + new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), + ).rejects.toMatchObject({ + code: DelegationErrorCodes.Unreachable, + message: + 'redirects to http://10.0.0.1/rpc, and redirects are not followed', + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('keeps a 4xx without a JSON-RPC error as unreachable', async () => { + fetchMock.mockResolvedValue( + respond({ status: 404, text: 'Not Found' }), + ); + + await expect( + new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), + ).rejects.toMatchObject({ + code: DelegationErrorCodes.Unreachable, + message: expect.stringContaining('404'), + }); + }); + it('reports a network failure as unreachable', async () => { fetchMock.mockRejectedValue(new Error('socket hang up')); @@ -256,19 +365,40 @@ describe('A2aClient.sendMessage', () => { }); }); - it('refuses an answer that is not a task, having nothing to poll with', async () => { - fetchMock.mockResolvedValue( - respond({ json: { result: { message: { parts: [] } } } }), - ); + it('accepts a plain message reply — the spec allows it and most public agents use it', async () => { + const message = { + messageId: 'm-reply', + role: 'ROLE_AGENT', + parts: [{ text: 'Hello from a message' }], + }; + fetchMock.mockResolvedValue(respond({ json: { result: { message } } })); + + await expect( + new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), + ).resolves.toEqual({ message }); + }); + + it('refuses a result that is neither a task nor a message', async () => { + fetchMock.mockResolvedValue(respond({ json: { result: { hello: 1 } } })); await expect( new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), ).rejects.toMatchObject({ code: DelegationErrorCodes.Error, - message: 'answered without a task', + message: 'answered with neither a task nor a message', }); }); + it('refuses a message without parts, which carries no reply at all', async () => { + fetchMock.mockResolvedValue( + respond({ json: { result: { message: { messageId: 'm' } } } }), + ); + + await expect( + new A2aClient().sendMessage(RPC_URL, TOKEN, params, 1000), + ).rejects.toMatchObject({ code: DelegationErrorCodes.Error }); + }); + it('waits longer than the peer own limit, so its stated timeout wins', async () => { fetchMock.mockResolvedValue(respond({ json: { result: { task } } })); const spy = jest.spyOn(AbortSignal, 'timeout'); diff --git a/api/src/slices/agent/peer/domain/a2a.client.ts b/api/src/slices/agent/peer/domain/a2a.client.ts index 4f799577..0b03c076 100644 --- a/api/src/slices/agent/peer/domain/a2a.client.ts +++ b/api/src/slices/agent/peer/domain/a2a.client.ts @@ -6,7 +6,9 @@ import { A2A_VERSION, A2A_VERSION_HEADER, A2aMethods, + type A2aSendMessageResult, type IA2aAgentCard, + type IA2aMessage, type IA2aSendMessageParams, type IA2aTask, type IJsonRpcResponse, @@ -26,6 +28,10 @@ const CLIENT_TIMEOUT_MARGIN_MS = 5_000; /** Enough of a failing body to diagnose, not enough to flood a log line. */ const BODY_EXCERPT_CHARS = 200; +/** A failed RPC body is read further than an excerpt: a JSON-RPC error + * envelope must parse whole to be recognised as one. */ +const FAILED_RPC_BODY_CHARS = 8_000; + /** * The outbound half of A2A (CLEAN-74): reading a peer's card and handing it a * task. Every failure is translated into a code the caller can turn into @@ -52,8 +58,11 @@ export class A2aClient { Accept: 'application/json', }, // A redirect would let a vetted public URL bounce the request onto a - // private address AFTER the SSRF guard ran. No card needs one. - redirect: 'error', + // private address AFTER the SSRF guard ran, so none is followed. + // 'manual' rather than 'error' keeps the 3xx and its Location, so the + // operator is told which address to paste instead of "fetch failed" + // (CLEAN-97). The target is never requested either way. + redirect: 'manual', signal: AbortSignal.timeout(CARD_TIMEOUT_MS), }); } catch (err) { @@ -62,6 +71,16 @@ export class A2aClient { ); } + const redirect = redirectTarget(response); + if (redirect !== null) { + throw new PeerCardUnreachableError( + `The card at ${cardUrl} redirects${redirect ? ` to ${redirect}` : ''}. ` + + 'Redirects are not followed — import the final address instead.', + response.status, + 'invalid', + ); + } + if (!response.ok) { const body = await excerpt(response); throw new PeerCardUnreachableError( @@ -82,6 +101,17 @@ export class A2aClient { } if (!isCard(card)) { + // A 0.3 card is a real card in the wrong dialect: it names its version + // at the top level and has no supportedInterfaces. Saying "not a card" + // there sends the operator hunting for a broken URL (CLEAN-97). + const legacyVersion = legacyProtocolVersion(card); + if (legacyVersion) { + throw new PeerCardUnreachableError( + `This agent speaks A2A ${legacyVersion}; only ${A2A_VERSION} is supported`, + response.status, + 'version', + ); + } throw new PeerCardUnreachableError( `The document at ${cardUrl} is not an agent card`, response.status, @@ -93,26 +123,28 @@ export class A2aClient { } /** - * Hands a task to a peer and waits for the finished task. + * Hands a task to a peer and waits for its reply. * * SECURITY NOTE. `interfaceUrl` comes from a stored card snapshot, and a * card is remote content. For internal rows every snapshot was read from * this API's own route — the URL is ours. External rows (CLEAN-95) are - * exactly the foreseen case: the callers guard both the imported card URL - * and the card's interface URL with `assertPublicPeerAddress` before any - * request goes out. Literal-address checks only; DNS rebinding is accepted - * as out of scope for v1. + * exactly the foreseen case: import vets the card's interface URL before + * anything is saved (CLEAN-97), and delegation re-checks it with + * `assertPublicPeerAddress` / `assertResolvesPublic` before any request + * goes out. DNS rebinding between the check and the connect is accepted as + * out of scope. * - * A peer that answers with a message instead of a task is treated as an - * error: this client asked for blocking work and has nothing to poll with, - * so a message would leave the delegation with no outcome to report. + * The spec lets a peer answer a blocking SendMessage with either a task or + * a plain message, and most public agents choose the message for a direct + * reply. Both are returned as-is; reading the answer out of them is the + * caller's job (CLEAN-97). */ async sendMessage( interfaceUrl: string, token: string | undefined, params: IA2aSendMessageParams, timeoutMs: number, - ): Promise { + ): Promise { let response: Response; try { response = await fetch(interfaceUrl, { @@ -128,8 +160,9 @@ export class A2aClient { method: A2aMethods.SendMessage, params, }), - // Same reason as fetchCard: a redirect is an SSRF-guard bypass. - redirect: 'error', + // Same reason as fetchCard: a redirect is an SSRF-guard bypass, so it + // is reported, never followed. + redirect: 'manual', signal: AbortSignal.timeout(timeoutMs + CLIENT_TIMEOUT_MARGIN_MS), }); } catch (err) { @@ -139,6 +172,14 @@ export class A2aClient { ); } + const redirect = redirectTarget(response); + if (redirect !== null) { + throw new DelegationError( + DelegationErrorCodes.Unreachable, + `redirects${redirect ? ` to ${redirect}` : ''}, and redirects are not followed`, + ); + } + if (response.status === 401 || response.status === 403) { throw new DelegationError( DelegationErrorCodes.Unauthorized, @@ -147,10 +188,17 @@ export class A2aClient { } if (!response.ok) { - const body = await excerpt(response); + const body = await excerpt(response, FAILED_RPC_BODY_CHARS); + // Some servers send a JSON-RPC error with a 4xx status. The peer was + // reached and said why it refused; "could not be reached" would send + // the operator after the network instead of the request (CLEAN-97). + const rpcError = jsonRpcErrorMessage(body); + if (rpcError) { + throw new DelegationError(DelegationErrorCodes.Error, rpcError); + } throw new DelegationError( DelegationErrorCodes.Unreachable, - `answered ${response.status}${body ? `: ${body}` : ''}`, + `answered ${response.status}${body ? `: ${body.slice(0, BODY_EXCERPT_CHARS)}` : ''}`, ); } @@ -171,18 +219,33 @@ export class A2aClient { ); } - const task = (payload.result as { task?: IA2aTask } | undefined)?.task; - if (!task?.status) { - throw new DelegationError( - DelegationErrorCodes.Error, - 'answered without a task', - ); + const result = payload.result as + | { task?: IA2aTask; message?: IA2aMessage } + | undefined; + + if (result?.task?.status) return { task: result.task }; + if (result?.message && Array.isArray(result.message.parts)) { + return { message: result.message }; } - return task; + throw new DelegationError( + DelegationErrorCodes.Error, + 'answered with neither a task nor a message', + ); } } +/** The version a pre-1.0 card declares at its top level, when it looks like one. */ +function legacyProtocolVersion(value: unknown): string | null { + if (!value || typeof value !== 'object') return null; + const card = value as Record; + if (Array.isArray(card.supportedInterfaces)) return null; + return typeof card.protocolVersion === 'string' && + typeof card.name === 'string' + ? card.protocolVersion + : null; +} + /** * SSRF guard for operator-supplied peer addresses (CLEAN-95). External card * URLs and the interface URLs inside fetched cards are remote content, so a @@ -194,7 +257,8 @@ export class A2aClient { * `::ffff:` mapped v4 and unabbreviated loopback; curl-style numeric * shorthand like `2130706433` or `0x7f000001` is refused outright); * (2) `assertResolvesPublic` below, a pre-flight DNS check for hostnames; - * (3) `redirect: 'error'` on every outbound fetch. Residual risk, accepted + * (3) no redirect is ever followed (`redirect: 'manual'`, reported as a + * refusal with its target). Residual risk, accepted * and documented: the connection itself may re-resolve (DNS rebinding * TOCTOU). `A2A_ALLOW_PRIVATE_PEERS=true` lifts the guard for local * development, where the mock peer IS loopback. @@ -316,11 +380,47 @@ function describe(err: unknown): string { return String(err); } -async function excerpt(response: Response): Promise { +async function excerpt( + response: Response, + limit: number = BODY_EXCERPT_CHARS, +): Promise { try { const text = await response.text(); - return text.slice(0, BODY_EXCERPT_CHARS).trim(); + return text.slice(0, limit).trim(); } catch { return ''; } } + +/** + * For a 3xx answer: where it points ('' when it names nowhere). Null for any + * other status. An opaque redirect (status 0, as browsers report it) counts + * as a redirect with no known target. + */ +function redirectTarget(response: Response): string | null { + const opaque = response.type === 'opaqueredirect'; + if (!opaque && (response.status < 300 || response.status >= 400)) { + return null; + } + return response.headers?.get?.('location') ?? ''; +} + +/** The message of a JSON-RPC error envelope, when a failed body is one. */ +function jsonRpcErrorMessage(body: string): string | null { + if (!body) return null; + try { + const parsed = JSON.parse(body) as { + error?: { code?: unknown; message?: unknown }; + }; + const error = parsed?.error; + if (!error || typeof error !== 'object') return null; + if (typeof error.message === 'string' && error.message.trim()) { + return error.message.trim(); + } + return typeof error.code === 'number' + ? `protocol error ${error.code}` + : null; + } catch { + return null; + } +} diff --git a/api/src/slices/agent/peer/domain/a2a.types.spec.ts b/api/src/slices/agent/peer/domain/a2a.types.spec.ts new file mode 100644 index 00000000..969883cf --- /dev/null +++ b/api/src/slices/agent/peer/domain/a2a.types.spec.ts @@ -0,0 +1,174 @@ +import { + renderReplyParts, + replyTextOfTask, + selectJsonRpcInterface, + type IA2aTask, +} from './a2a.types'; + +/** + * The two decisions every external delegation leans on (CLEAN-97): which of a + * card's interfaces Ranch dials, and what of a peer's reply the calling model + * gets to read. Both are pure, so they are pinned here directly as well as + * through the services that use them. + */ +describe('selectJsonRpcInterface', () => { + it('picks the first JSON-RPC interface on 1.0, not simply the first one', () => { + const iface = selectJsonRpcInterface({ + supportedInterfaces: [ + { + url: 'https://x/grpc', + protocolBinding: 'GRPC', + protocolVersion: '1.0', + }, + { + url: 'https://x/rpc-a', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + { + url: 'https://x/rpc-b', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ], + }); + + expect(iface?.url).toBe('https://x/rpc-a'); + }); + + it('ignores a JSON-RPC interface on another protocol version', () => { + expect( + selectJsonRpcInterface({ + supportedInterfaces: [ + { + url: 'https://x/old', + protocolBinding: 'JSONRPC', + protocolVersion: '0.3', + }, + ], + }), + ).toBeNull(); + }); + + it('accepts the binding name in any letter case', () => { + expect( + selectJsonRpcInterface({ + supportedInterfaces: [ + { + url: 'https://x/rpc', + protocolBinding: 'jsonrpc', + protocolVersion: '1.0', + }, + ], + })?.url, + ).toBe('https://x/rpc'); + }); + + it('survives a card with no interfaces or a malformed entry', () => { + expect(selectJsonRpcInterface(null)).toBeNull(); + expect(selectJsonRpcInterface({ supportedInterfaces: [] })).toBeNull(); + expect( + selectJsonRpcInterface({ + supportedInterfaces: [ + { protocolBinding: 'JSONRPC', protocolVersion: '1.0' } as never, + ], + }), + ).toBeNull(); + }); +}); + +describe('renderReplyParts', () => { + it('passes text through, trimmed, and separates parts with a blank line', () => { + expect(renderReplyParts([{ text: ' one ' }, { text: 'two' }])).toBe( + 'one\n\ntwo', + ); + }); + + it('turns structured data into compact JSON', () => { + expect(renderReplyParts([{ data: { a: 1, b: ['x'] } }])).toBe( + '{"a":1,"b":["x"]}', + ); + }); + + it('treats empty data as nothing rather than as "{}"', () => { + expect(renderReplyParts([{ data: {} }, { data: [] }, { data: null }])).toBe( + '', + ); + }); + + it('keeps a link, labelled by its file name when there is one', () => { + expect( + renderReplyParts([ + { url: 'https://x/a' }, + { url: 'https://x/b', filename: 'b.html' }, + ]), + ).toBe('https://x/a\n\nb.html: https://x/b'); + }); + + it('names a binary part instead of pasting base64 into a prompt', () => { + expect(renderReplyParts([{ raw: 'QUJD', mediaType: 'image/png' }])).toBe( + '[binary attachment not included: image/png]', + ); + }); + + it('skips shapes it does not recognise', () => { + expect( + renderReplyParts([{ something: 'else' } as never, { text: 'ok' }]), + ).toBe('ok'); + expect(renderReplyParts(undefined)).toBe(''); + }); +}); + +describe('replyTextOfTask', () => { + const base: IA2aTask = { + id: 't', + contextId: 'c', + status: { + state: 'TASK_STATE_COMPLETED', + timestamp: '2026-09-17T00:00:00Z', + }, + artifacts: [], + history: [], + }; + + it('reads every artifact in order', () => { + expect( + replyTextOfTask({ + ...base, + artifacts: [ + { artifactId: 'a', parts: [{ text: 'first' }] }, + { artifactId: 'b', parts: [{ data: { n: 2 } }] }, + ], + }), + ).toBe('first\n\n{"n":2}'); + }); + + it('falls back to the status message only when the artifacts say nothing', () => { + const status = { + ...base.status, + message: { + messageId: 's', + role: 'ROLE_AGENT' as const, + parts: [{ text: 'status words' }], + }, + }; + + expect(replyTextOfTask({ ...base, status })).toBe('status words'); + expect( + replyTextOfTask({ + ...base, + status, + artifacts: [{ artifactId: 'a', parts: [{ text: 'artifact words' }] }], + }), + ).toBe('artifact words'); + }); + + it('is empty when nothing anywhere is readable', () => { + expect( + replyTextOfTask({ + ...base, + artifacts: [{ artifactId: 'a', parts: [{ data: {} }] }], + }), + ).toBe(''); + }); +}); diff --git a/api/src/slices/agent/peer/domain/a2a.types.ts b/api/src/slices/agent/peer/domain/a2a.types.ts index 198ba40f..cf9922a9 100644 --- a/api/src/slices/agent/peer/domain/a2a.types.ts +++ b/api/src/slices/agent/peer/domain/a2a.types.ts @@ -312,6 +312,90 @@ export function hasNonTextPart(parts: A2aPart[] | undefined): boolean { return parts.some((p) => !isTextPart(p)); } +/** The only transport Ranch calls peers over. */ +export const A2A_JSONRPC_BINDING = 'JSONRPC'; + +/** + * The interface Ranch talks to on a card: the first JSON-RPC one on the + * protocol version this server speaks (CLEAN-97). + * + * A card lists interfaces in preference order, but "preferred" is the + * agent's view, not ours: an agent that prefers HTTP+JSON and also offers + * JSON-RPC is perfectly reachable. Import, delegation and the console's + * address line all go through here, so what an operator approves is what + * the delegation dials. + */ +export function selectJsonRpcInterface( + card: Pick | null | undefined, +): IA2aAgentInterface | null { + const interfaces = card?.supportedInterfaces; + if (!Array.isArray(interfaces)) return null; + return ( + interfaces.find( + (i) => + typeof i?.url === 'string' && + String(i.protocolBinding ?? '').toUpperCase() === A2A_JSONRPC_BINDING && + i.protocolVersion === A2A_VERSION, + ) ?? null + ); +} + +/** + * A peer's reply as text the calling model can read (CLEAN-97). + * + * Text parts pass through. Structured data is handed over as compact JSON + * rather than dropped — for many agents the data IS the answer, and a model + * reads JSON well. Links stay links. Binary payloads are named but not + * inlined: base64 in a prompt costs tokens and tells the model nothing. + */ +export function renderReplyParts(parts: A2aPart[] | undefined): string { + if (!Array.isArray(parts)) return ''; + return parts + .map((part) => renderReplyPart(part)) + .filter((chunk) => chunk.length > 0) + .join('\n\n'); +} + +function renderReplyPart(part: A2aPart): string { + if (!part || typeof part !== 'object') return ''; + if (isTextPart(part)) return part.text.trim(); + + if ('data' in part && part.data !== undefined && part.data !== null) { + try { + const json = JSON.stringify(part.data); + return json === '{}' || json === '[]' ? '' : json; + } catch { + return ''; + } + } + + if ('url' in part && typeof part.url === 'string' && part.url) { + return part.filename ? `${part.filename}: ${part.url}` : part.url; + } + + if ('raw' in part && typeof part.raw === 'string' && part.raw) { + const label = part.filename ?? part.mediaType ?? 'unnamed file'; + return `[binary attachment not included: ${label}]`; + } + + return ''; +} + +/** + * The reply carried by a finished task. Artifacts are the answer; when a + * peer leaves them empty and puts its words in the status message instead — + * a pattern real agents use — that message is the answer. Empty string only + * when neither says anything. + */ +export function replyTextOfTask(task: IA2aTask): string { + const fromArtifacts = (task.artifacts ?? []) + .map((artifact) => renderReplyParts(artifact?.parts)) + .filter((chunk) => chunk.length > 0) + .join('\n\n'); + if (fromArtifacts) return fromArtifacts; + return renderReplyParts(task.status?.message?.parts); +} + /** ISO 8601 UTC with the trailing Z the spec asks for. */ export function a2aTimestamp(at: Date = new Date()): string { return at.toISOString(); diff --git a/api/src/slices/agent/peer/domain/delegation.service.spec.ts b/api/src/slices/agent/peer/domain/delegation.service.spec.ts index 09b163c8..b6b59490 100644 --- a/api/src/slices/agent/peer/domain/delegation.service.spec.ts +++ b/api/src/slices/agent/peer/domain/delegation.service.spec.ts @@ -1,5 +1,10 @@ import { DelegationService } from './delegation.service'; -import { A2aTaskStates, type IA2aAgentCard, type IA2aTask } from './a2a.types'; +import { + A2aTaskStates, + type A2aSendMessageResult, + type IA2aAgentCard, + type IA2aTask, +} from './a2a.types'; import { DelegationError, DelegationErrorCodes, @@ -110,6 +115,8 @@ function makeHarness( options: { connections?: IAgentPeerData[]; task?: IA2aTask; + /** The whole client result, for message replies (CLEAN-97). */ + reply?: A2aSendMessageResult; sendThrows?: Error; activeTurn?: { clientId: string; turnId: string; ts: number } | null; } = {}, @@ -151,7 +158,7 @@ function makeHarness( const sendMessage = jest.fn(async (..._args: unknown[]) => { order.push('send'); if (options.sendThrows) throw options.sendThrows; - return options.task ?? completed(); + return options.reply ?? { task: options.task ?? completed() }; }); const client = { sendMessage } as unknown as A2aClient; @@ -568,7 +575,13 @@ describe('DelegationService.run — external peers (CLEAN-95)', () => { const outcome = await run({ peer: 'Foreign Bot' }); expect(sendMessage).not.toHaveBeenCalled(); - expect(outcome).toMatchObject({ kind: 'done', status: 'failed' }); + // A named cause, not "it answered with an error": nothing was sent, so + // the peer answered nothing (CLEAN-97). + expect(outcome).toMatchObject({ + kind: 'done', + status: 'failed', + errorCode: DelegationErrorCodes.AddressRefused, + }); }); it('an internal peer keeps using its pair token', async () => { @@ -584,3 +597,257 @@ describe('DelegationService.run — external peers (CLEAN-95)', () => { ); }); }); + +describe('DelegationService.run — reading what a real peer sends back (CLEAN-97)', () => { + const task = (overrides: Partial): IA2aTask => ({ + id: 't', + contextId: 'ctx-1', + status: { + state: A2aTaskStates.Completed, + timestamp: '2026-09-17T10:00:00Z', + }, + artifacts: [], + history: [], + ...overrides, + }); + + it('takes a plain message as the answer — most public agents reply that way', async () => { + const { run, rows } = makeHarness({ + reply: { + message: { + messageId: 'm-reply', + role: 'ROLE_AGENT', + parts: [{ text: 'Crown Hill Senior Care Home, Seattle, WA' }], + }, + }, + }); + + const outcome = await run(); + + expect(outcome).toMatchObject({ + status: 'answered', + text: 'Crown Hill Senior Care Home, Seattle, WA', + }); + const [row] = Object.values(rows); + expect(row).toMatchObject({ status: 'answered', errorCode: null }); + }); + + it('hands structured data over as JSON instead of dropping it', async () => { + const { run } = makeHarness({ + task: task({ + artifacts: [ + { artifactId: 'a', parts: [{ data: { accepted: true, count: 3 } }] }, + ], + }), + }); + + await expect(run()).resolves.toMatchObject({ + status: 'answered', + text: '{"accepted":true,"count":3}', + }); + }); + + it('keeps links as links and names binary parts without inlining them', async () => { + const { run } = makeHarness({ + task: task({ + artifacts: [ + { + artifactId: 'a', + parts: [ + { url: 'https://example.test/page', filename: 'page.html' }, + { + raw: 'aGVsbG8=', + filename: 'report.pdf', + mediaType: 'application/pdf', + }, + ], + }, + ], + }), + }); + + const outcome = (await run()) as { text: string }; + + expect(outcome.text).toContain('page.html: https://example.test/page'); + expect(outcome.text).toContain( + '[binary attachment not included: report.pdf]', + ); + expect(outcome.text).not.toContain('aGVsbG8='); + }); + + it('joins every artifact, not only the first', async () => { + const { run } = makeHarness({ + task: task({ + artifacts: [ + { artifactId: 'a', parts: [{ text: 'first' }] }, + { artifactId: 'b', parts: [{ text: 'second' }] }, + ], + }), + }); + + await expect(run()).resolves.toMatchObject({ text: 'first\n\nsecond' }); + }); + + it('reads the status message when a finished task leaves its artifacts empty', async () => { + const { run } = makeHarness({ + task: task({ + status: { + state: A2aTaskStates.Completed, + timestamp: '2026-09-17T10:00:00Z', + message: { + messageId: 's', + role: 'ROLE_AGENT', + parts: [{ text: 'No public places matched.' }], + }, + }, + }), + }); + + await expect(run()).resolves.toMatchObject({ + status: 'answered', + text: 'No public places matched.', + }); + }); + + it('prefers the artifacts over a status message that only says it is done', async () => { + const { run } = makeHarness({ + task: task({ + artifacts: [{ artifactId: 'a', parts: [{ text: 'the answer' }] }], + status: { + state: A2aTaskStates.Completed, + timestamp: '2026-09-17T10:00:00Z', + message: { + messageId: 's', + role: 'ROLE_AGENT', + parts: [{ text: 'Request is completed!' }], + }, + }, + }), + }); + + await expect(run()).resolves.toMatchObject({ text: 'the answer' }); + }); + + it('records a truly empty reply as answered, and says so', async () => { + const { run, rows, sent } = makeHarness({ + task: task({ artifacts: [{ artifactId: 'a', parts: [{ data: {} }] }] }), + }); + + const outcome = await run(); + + expect(outcome).toMatchObject({ status: 'answered', text: '' }); + const [row] = Object.values(rows); + expect(row.excerpt).toBe( + 'The peer answered, but its reply had no text, data or links.', + ); + expect(sent[1].data.step.label).toBe('Answered by «Support Bot»'); + }); + + it('treats a message with nothing readable the same way', async () => { + const { run, rows } = makeHarness({ + reply: { message: { messageId: 'm', role: 'ROLE_AGENT', parts: [] } }, + }); + + await expect(run()).resolves.toMatchObject({ + status: 'answered', + text: '', + }); + const [row] = Object.values(rows); + expect(row.excerpt).toMatch(/no text, data or links/); + }); + + it('quotes a failure reason given as data, not just as text', async () => { + const { run, rows } = makeHarness({ + task: task({ + status: { + state: A2aTaskStates.Failed, + timestamp: '2026-09-17T10:00:00Z', + message: { + messageId: 's', + role: 'ROLE_AGENT', + parts: [{ data: { error: 'quota exceeded' } }], + }, + }, + }), + }); + + await run(); + + const [row] = Object.values(rows); + expect(row.excerpt).toBe('{"error":"quota exceeded"}'); + }); +}); + +describe('DelegationService.run — which interface gets called (CLEAN-97)', () => { + const withInterfaces = ( + supportedInterfaces: IA2aAgentCard['supportedInterfaces'], + ): IAgentPeerData => + connection({ + id: 'peer-ext', + peerAgentId: null, + origin: PeerOrigins.External, + token: null, + outboundToken: null, + cardSnapshot: { + ...connection().cardSnapshot, + name: 'Foreign Bot', + supportedInterfaces, + }, + cardUrl: 'https://other.example/.well-known/agent-card.json', + }); + + it('calls the JSON-RPC interface even when the card prefers another binding', async () => { + const { run, sendMessage } = makeHarness({ + connections: [ + withInterfaces([ + { + url: 'https://other.example/rest', + protocolBinding: 'HTTP+JSON', + protocolVersion: '1.0', + }, + { + url: 'https://other.example/jsonrpc', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + ], + }); + + await run({ peer: 'Foreign Bot' }); + + expect(sendMessage).toHaveBeenCalledWith( + 'https://other.example/jsonrpc', + undefined, + expect.anything(), + 120_000, + ); + }); + + it('refuses without sending when the card offers no JSON-RPC 1.0 interface', async () => { + const { run, sendMessage } = makeHarness({ + connections: [ + withInterfaces([ + { + url: 'https://other.example/rest', + protocolBinding: 'HTTP+JSON', + protocolVersion: '1.0', + }, + { + url: 'https://other.example/old', + protocolBinding: 'JSONRPC', + protocolVersion: '0.3', + }, + ]), + ], + }); + + const outcome = await run({ peer: 'Foreign Bot' }); + + expect(sendMessage).not.toHaveBeenCalled(); + expect(outcome).toMatchObject({ + status: 'failed', + errorCode: DelegationErrorCodes.Unsupported, + }); + }); +}); diff --git a/api/src/slices/agent/peer/domain/delegation.service.ts b/api/src/slices/agent/peer/domain/delegation.service.ts index 4ea4f38e..a5f5409d 100644 --- a/api/src/slices/agent/peer/domain/delegation.service.ts +++ b/api/src/slices/agent/peer/domain/delegation.service.ts @@ -12,7 +12,10 @@ import { buildDelegationStep, causeText } from './delegationStep'; import { A2aRoles, A2aTaskStates, - textOfParts, + renderReplyParts, + replyTextOfTask, + selectJsonRpcInterface, + type A2aSendMessageResult, type IA2aTask, } from './a2a.types'; import { @@ -20,6 +23,7 @@ import { DelegationError, DelegationErrorCodes, DelegationStatuses, + EMPTY_REPLY_NOTE, PeerOrigins, type DelegationErrorCode, type DelegationStatus, @@ -90,7 +94,8 @@ export class DelegationService { }; } - const peerName = peer.cardSnapshot?.name ?? peer.peerAgentId ?? peer.cardUrl; + const peerName = + peer.cardSnapshot?.name ?? peer.peerAgentId ?? peer.cardUrl; const contextId = input.contextId ?? `ctx-${crypto.randomUUID()}`; const matchedSkills = matchSkills(peer, `${input.reason} ${input.task}`); @@ -121,14 +126,31 @@ export class DelegationService { let excerpt: string | null = null; try { - const interfaceUrl = peer.cardSnapshot.supportedInterfaces[0].url; + // The interface import approved, not whichever the card lists first — + // an agent may prefer HTTP+JSON and still speak JSON-RPC (CLEAN-97). + const iface = selectJsonRpcInterface(peer.cardSnapshot); + if (!iface) { + throw new DelegationError( + DelegationErrorCodes.Unsupported, + 'its card offers no JSON-RPC interface on A2A 1.0', + ); + } + const interfaceUrl = iface.url; if (peer.origin === PeerOrigins.External) { // The interface URL inside a foreign card is remote content — never // let it point the platform at a private address (SSRF, CLEAN-95). - assertPublicPeerAddress(interfaceUrl); - await assertResolvesPublic(interfaceUrl); + // Import vets it too; rows saved before that check land here. + try { + assertPublicPeerAddress(interfaceUrl); + await assertResolvesPublic(interfaceUrl); + } catch (err) { + throw new DelegationError( + DelegationErrorCodes.AddressRefused, + err instanceof Error ? err.message : String(err), + ); + } } - const task = await this.client.sendMessage( + const reply = await this.client.sendMessage( interfaceUrl, peer.origin === PeerOrigins.External ? (peer.outboundToken ?? undefined) @@ -154,11 +176,19 @@ export class DelegationService { timeoutMs, ); - ({ status, errorCode, text } = readTask(task)); - excerpt = - status === DelegationStatuses.Answered - ? (text ?? '').slice(0, DELEGATION_EXCERPT_CHARS) - : (statusText(task) ?? causeText(errorCode ?? null)); + ({ status, errorCode, text } = readReply(reply)); + if (status === DelegationStatuses.Answered) { + // An answer with nothing in it is still an answer — recorded as one, + // and said out loud, so neither the audit row nor the model mistakes + // silence for content (CLEAN-97). + excerpt = text + ? text.slice(0, DELEGATION_EXCERPT_CHARS) + : EMPTY_REPLY_NOTE; + } else { + excerpt = + ('task' in reply ? statusText(reply.task) : null) ?? + causeText(errorCode ?? null); + } } catch (err) { errorCode = err instanceof DelegationError ? err.code : DelegationErrorCodes.Error; @@ -273,16 +303,29 @@ function matchSkills(peer: IAgentPeerData, context: string): IMatchedSkill[] { return chosen.map((s) => ({ id: s.id, name: s.name })); } -function readTask(task: IA2aTask): { +type ReadReply = { status: DelegationStatus; errorCode?: DelegationErrorCode; text?: string; -} { +}; + +/** A plain message is a direct, finished answer; a task says how it went. */ +function readReply(reply: A2aSendMessageResult): ReadReply { + if ('message' in reply) { + return { + status: DelegationStatuses.Answered, + text: renderReplyParts(reply.message.parts), + }; + } + return readTask(reply.task); +} + +function readTask(task: IA2aTask): ReadReply { switch (task.status.state) { case A2aTaskStates.Completed: return { status: DelegationStatuses.Answered, - text: textOfParts(task.artifacts?.[0]?.parts), + text: replyTextOfTask(task), }; case A2aTaskStates.Rejected: return { @@ -314,7 +357,6 @@ function readTask(task: IA2aTask): { /** The peer's own words about what went wrong, when it offered any. */ function statusText(task: IA2aTask): string | null { - const parts = task.status.message?.parts; - const text = textOfParts(parts); - return text || null; + const text = renderReplyParts(task.status.message?.parts); + return text ? text.slice(0, DELEGATION_EXCERPT_CHARS) : null; } diff --git a/api/src/slices/agent/peer/domain/delegationStep.ts b/api/src/slices/agent/peer/domain/delegationStep.ts index 6f9af865..766edf23 100644 --- a/api/src/slices/agent/peer/domain/delegationStep.ts +++ b/api/src/slices/agent/peer/domain/delegationStep.ts @@ -31,6 +31,10 @@ const CAUSES: Record = { 'it refused the credential — the connection may have been removed', [DelegationErrorCodes.Unreachable]: 'it could not be reached', [DelegationErrorCodes.Error]: 'it answered with an error', + [DelegationErrorCodes.AddressRefused]: + 'its address is private or local, so it was not called', + [DelegationErrorCodes.Unsupported]: + 'its card offers no interface Ranch can call', }; export function causeText(code: DelegationErrorCode | null): string { diff --git a/api/src/slices/agent/peer/domain/peer.service.spec.ts b/api/src/slices/agent/peer/domain/peer.service.spec.ts index b9a1da91..b04f823e 100644 --- a/api/src/slices/agent/peer/domain/peer.service.spec.ts +++ b/api/src/slices/agent/peer/domain/peer.service.spec.ts @@ -36,6 +36,22 @@ const card = (name: string): IA2aAgentCard => ({ ], }); +/** + * A card served from outside this installation. Its interface must point + * somewhere other than `api.test`: since CLEAN-97 an import whose card sends + * delegations back into this installation is refused as a self-import. + */ +const foreignCard = ( + name = 'Foreign Bot', + supportedInterfaces: IA2aAgentCard['supportedInterfaces'] = [ + { + url: 'https://other.example/a2a/agents/agent-x', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ], +): IA2aAgentCard => ({ ...card(name), supportedInterfaces }); + function makeHarness( options: { agents?: Array<{ id: string; name: string; status: string }>; @@ -126,7 +142,7 @@ function makeHarness( const fetchCard = jest.fn(async (url: string, _token?: string) => { if (options.fetchCardThrows) throw options.fetchCardThrows; if (!url.startsWith('https://api.test/')) { - return options.externalCard ?? card('Foreign Bot'); + return options.externalCard ?? foreignCard(); } return card('Support Bot'); }); @@ -386,9 +402,9 @@ describe('PeerService — importing an external agent by URL (CLEAN-95)', () => it('rejects garbage and private addresses as PEER_URL_INVALID', async () => { const { service } = makeHarness(); - await expect(service.connectByUrl('a', 'not a url')).rejects.toMatchObject( - { response: { code: 'PEER_URL_INVALID' } }, - ); + await expect(service.connectByUrl('a', 'not a url')).rejects.toMatchObject({ + response: { code: 'PEER_URL_INVALID' }, + }); await expect( service.connectByUrl('a', 'ftp://other.example/x'), ).rejects.toMatchObject({ response: { code: 'PEER_URL_INVALID' } }); @@ -411,7 +427,11 @@ describe('PeerService — importing an external agent by URL (CLEAN-95)', () => it('tells "not a card" (400) apart from "unreachable" (502)', async () => { const invalid = makeHarness({ - fetchCardThrows: new PeerCardUnreachableError('not a card', 200, 'invalid'), + fetchCardThrows: new PeerCardUnreachableError( + 'not a card', + 200, + 'invalid', + ), }); await expect( @@ -420,7 +440,7 @@ describe('PeerService — importing an external agent by URL (CLEAN-95)', () => }); it('refuses a card speaking another protocol version', async () => { - const externalCard = card('Foreign Bot'); + const externalCard = foreignCard(); externalCard.supportedInterfaces[0].protocolVersion = '2.0'; const { service, rows } = makeHarness({ externalCard }); @@ -441,7 +461,9 @@ describe('PeerService — previewing an external URL (CLEAN-95)', () => { expect(card.name).toBe('Foreign Bot'); expect(Object.keys(rows)).toHaveLength(0); - expect((peers as unknown as { create: jest.Mock }).create).not.toHaveBeenCalled(); + expect( + (peers as unknown as { create: jest.Mock }).create, + ).not.toHaveBeenCalled(); }); it('runs the same refusals as the import', async () => { @@ -499,3 +521,229 @@ describe('PeerService — armed state (CLEAN-95)', () => { expect((await harness.service.peersState('a')).armed).toBe(false); }); }); + +describe('PeerService — telling our own agents apart from external ones (CLEAN-97)', () => { + // Every one of these reaches this installation's card route; a string + // prefix check let the last five through. + it.each([ + ['the plain form', 'https://api.test/a2a/agents/b'], + ['an upper-case host', 'https://API.Test/a2a/agents/b'], + ['a doubled slash', 'https://api.test//a2a/agents/b'], + ['a percent-encoded path', 'https://api.test/a2a/%61gents/b'], + ['a trailing dot on the host', 'https://api.test./a2a/agents/b'], + ['http instead of https', 'http://api.test/a2a/agents/b'], + ['an upper-case path', 'https://api.test/A2A/Agents/b'], + ])('refuses %s before any request goes out', async (_label, url) => { + const { service, fetchCard, rows } = makeHarness(); + + await expect(service.connectByUrl('a', url)).rejects.toMatchObject({ + response: { code: 'PEER_SELF_URL' }, + }); + expect(fetchCard).not.toHaveBeenCalled(); + expect(Object.keys(rows)).toHaveLength(0); + }); + + it('refuses a card read from another host that sends delegations back to us', async () => { + // An alias of our host the address check cannot know about: the card it + // serves is ours, and our card names our real base. + const { service, rows } = makeHarness({ + externalCard: foreignCard('Rancher', [ + { + url: 'https://api.test/a2a/agents/agent-rancher', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + }); + + await expect( + service.connectByUrl( + 'a', + 'https://alias.example/a2a/agents/agent-rancher', + ), + ).rejects.toMatchObject({ response: { code: 'PEER_SELF_URL' } }); + expect(Object.keys(rows)).toHaveLength(0); + }); + + it('still imports a genuinely external agent', async () => { + const { service, rows } = makeHarness(); + + await service.connectByUrl('a', 'https://other.example/a2a/agents/agent-x'); + + expect(Object.keys(rows)).toHaveLength(1); + }); +}); + +describe('PeerService — the address inside the card (CLEAN-97)', () => { + const EXT_BASE = 'https://other.example/a2a/agents/agent-x'; + + it('refuses a card whose interface points at a private address, saving nothing', async () => { + const { service, rows } = makeHarness({ + externalCard: foreignCard('Sneaky', [ + { + url: 'http://127.0.0.1:9999', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + }); + + await expect(service.connectByUrl('a', EXT_BASE)).rejects.toMatchObject({ + response: { + code: 'PEER_URL_INVALID', + message: expect.stringContaining('http://127.0.0.1:9999'), + }, + }); + expect(Object.keys(rows)).toHaveLength(0); + }); + + it('applies the same refusal to a preview, so nothing looks importable that is not', async () => { + const { service } = makeHarness({ + externalCard: foreignCard('Sneaky', [ + { + url: 'http://169.254.169.254/latest', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + }); + + await expect(service.previewByUrl('a', EXT_BASE)).rejects.toMatchObject({ + response: { code: 'PEER_URL_INVALID' }, + }); + }); + + it('imports a card that prefers HTTP+JSON but also offers JSON-RPC', async () => { + const { service, rows } = makeHarness({ + externalCard: foreignCard('Two Doors', [ + { + url: 'https://other.example/rest', + protocolBinding: 'HTTP+JSON', + protocolVersion: '1.0', + }, + { + url: 'https://other.example/jsonrpc', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + }); + + await service.connectByUrl('a', EXT_BASE); + + expect(Object.keys(rows)).toHaveLength(1); + }); + + it('says plainly when a 1.0 agent offers no JSON-RPC at all', async () => { + const { service, rows } = makeHarness({ + externalCard: foreignCard('REST Only', [ + { + url: 'https://other.example/rest', + protocolBinding: 'HTTP+JSON', + protocolVersion: '1.0', + }, + ]), + }); + + await expect(service.connectByUrl('a', EXT_BASE)).rejects.toMatchObject({ + response: { + code: 'PEER_BINDING', + message: + 'This agent offers A2A 1.0 only over HTTP+JSON; Ranch calls agents over JSON-RPC', + }, + }); + expect(Object.keys(rows)).toHaveLength(0); + }); + + it('turns a pre-1.0 card into a version message, not "not a card"', async () => { + const { service } = makeHarness({ + fetchCardThrows: new PeerCardUnreachableError( + 'This agent speaks A2A 0.3.0; only 1.0 is supported', + 200, + 'version', + ), + }); + + await expect(service.connectByUrl('a', EXT_BASE)).rejects.toMatchObject({ + response: { + code: 'PEER_VERSION', + message: 'This agent speaks A2A 0.3.0; only 1.0 is supported', + }, + }); + }); +}); + +describe('PeerService — card URLs that are already JSON documents (CLEAN-97)', () => { + it('keeps a pre-1.0 agent.json address as it is', async () => { + const { service, fetchCard } = makeHarness(); + + await service.connectByUrl( + 'a', + 'https://other.example/.well-known/agent.json', + ); + + expect(fetchCard).toHaveBeenLastCalledWith( + 'https://other.example/.well-known/agent.json', + undefined, + ); + }); + + it('keeps a custom card path as it is', async () => { + const { service, fetchCard } = makeHarness(); + + await service.connectByUrl( + 'a', + 'https://other.example/a2a/agentverse/agent-card.json', + ); + + expect(fetchCard).toHaveBeenLastCalledWith( + 'https://other.example/a2a/agentverse/agent-card.json', + undefined, + ); + }); + + it('still appends the well-known path to a bare base address', async () => { + const { service, fetchCard } = makeHarness(); + + await service.connectByUrl('a', 'https://other.example'); + + expect(fetchCard).toHaveBeenLastCalledWith( + 'https://other.example/.well-known/agent-card.json', + undefined, + ); + }); +}); + +describe('PeerService — refreshing holds a card to the import bar (CLEAN-97)', () => { + const EXT_BASE = 'https://other.example/a2a/agents/agent-x'; + + it('refuses a refreshed card that moved its interface somewhere private, keeping the old one', async () => { + const { service, fetchCard, rows } = makeHarness(); + const imported = await service.connectByUrl('a', EXT_BASE); + fetchCard.mockResolvedValueOnce( + foreignCard('Foreign Bot', [ + { + url: 'http://10.0.0.5/jsonrpc', + protocolBinding: 'JSONRPC', + protocolVersion: '1.0', + }, + ]), + ); + + await expect(service.refresh('a', imported.id)).rejects.toMatchObject({ + response: { code: 'PEER_URL_INVALID' }, + }); + expect(Object.values(rows)[0].cardSnapshot.supportedInterfaces[0].url).toBe( + 'https://other.example/a2a/agents/agent-x', + ); + }); + + it('leaves internal peers alone: their card interface IS this installation', async () => { + const { service } = makeHarness(); + const connected = await service.connect('a', 'b'); + + await expect(service.refresh('a', connected.id)).resolves.toMatchObject({ + id: connected.id, + }); + }); +}); diff --git a/api/src/slices/agent/peer/domain/peer.service.ts b/api/src/slices/agent/peer/domain/peer.service.ts index f99e090e..99c2990a 100644 --- a/api/src/slices/agent/peer/domain/peer.service.ts +++ b/api/src/slices/agent/peer/domain/peer.service.ts @@ -2,6 +2,7 @@ import { BadGatewayException, BadRequestException, ConflictException, + HttpException, Injectable, Logger, NotFoundException, @@ -15,7 +16,13 @@ import { assertPublicPeerAddress, assertResolvesPublic, } from './a2a.client'; -import { A2A_CARD_PATH, A2A_VERSION } from './a2a.types'; +import { + A2A_CARD_PATH, + A2A_VERSION, + selectJsonRpcInterface, + type IA2aAgentCard, + type IA2aAgentInterface, +} from './a2a.types'; import { EXTERNAL_PEER_STATUS, PEER_TOKEN_BYTES, @@ -181,11 +188,7 @@ export class PeerService { * reviews before Connect (FR-002). The same read connectByUrl performs, so * what they approve is literally what gets stored. */ - async previewByUrl( - agentId: string, - rawUrl: string, - outboundToken?: string, - ) { + async previewByUrl(agentId: string, rawUrl: string, outboundToken?: string) { const { card } = await this.readExternalCard( agentId, rawUrl, @@ -210,6 +213,13 @@ export class PeerService { } const cardUrl = this.canonicalCardUrl(rawUrl); + + // Compared after normalizing, not as a string prefix: `//a2a`, `%61gents` + // or a trailing dot on the host all reach our own card route, and each + // used to slip past a startsWith (CLEAN-97). + const ownBase = await this.cards.ownA2aBase(); + if (isOwnA2aAddress(cardUrl, ownBase)) throw this.selfUrl(); + try { assertPublicPeerAddress(cardUrl); await assertResolvesPublic(cardUrl); @@ -217,16 +227,6 @@ export class PeerService { throw this.invalidUrl(err instanceof Error ? err.message : String(err)); } - const ownBase = await this.cards.ownA2aBase(); - if (cardUrl.startsWith(ownBase)) { - throw new BadRequestException({ - code: PeerErrorCodes.SelfUrl, - message: - 'This address belongs to an agent of this installation — pick it ' + - 'in the agent list instead of importing it by URL.', - }); - } - let card; try { card = await this.client.fetchCard(cardUrl, token); @@ -234,15 +234,74 @@ export class PeerService { throw this.importUnreadable(err, cardUrl); } - const version = card.supportedInterfaces?.[0]?.protocolVersion; - if (version !== A2A_VERSION) { + await this.vetExternalCard(card, ownBase); + + return { cardUrl, card }; + } + + /** + * What an external card must satisfy before it is stored, beyond "it is a + * card" (CLEAN-97). The pasted address was already checked; these checks + * are about the card's own claims, which is where delegation will actually + * send requests: + * + * - it offers a JSON-RPC interface on the version we speak — picked the + * same way delegation picks it, so the operator approves what gets dialled; + * - that interface is not this installation — our own card names our real + * base, so this catches every alias of our host that the address check + * cannot know about; + * - that interface is publicly reachable — otherwise the row saves fine and + * every delegation fails later with nothing in the console to explain it. + */ + private async vetExternalCard( + card: IA2aAgentCard, + ownBase: string, + ): Promise { + const iface = selectJsonRpcInterface(card); + if (!iface) { + const interfaces = card.supportedInterfaces ?? []; + const versions = unique(interfaces.map((i) => i?.protocolVersion)); + if (!versions.includes(A2A_VERSION)) { + throw new BadRequestException({ + code: PeerErrorCodes.Version, + message: `This agent speaks A2A ${versions.join(', ') || 'unknown'}; only ${A2A_VERSION} is supported`, + }); + } + const bindings = unique( + interfaces + .filter((i) => i?.protocolVersion === A2A_VERSION) + .map((i) => i?.protocolBinding), + ); throw new BadRequestException({ - code: PeerErrorCodes.Version, - message: `This agent speaks A2A ${version ?? 'unknown'}; only ${A2A_VERSION} is supported`, + code: PeerErrorCodes.Binding, + message: `This agent offers A2A ${A2A_VERSION} only over ${bindings.join(', ') || 'an unnamed transport'}; Ranch calls agents over JSON-RPC`, }); } - return { cardUrl, card }; + if (isOwnA2aAddress(iface.url, ownBase)) throw this.selfUrl(); + + try { + assertPublicPeerAddress(iface.url); + await assertResolvesPublic(iface.url); + } catch (err) { + throw new BadRequestException({ + code: PeerErrorCodes.UrlInvalid, + message: `This card sends delegations to ${iface.url}, which cannot be used: ${ + err instanceof Error ? err.message : String(err) + }`, + }); + } + + return iface; + } + + private selfUrl(): BadRequestException { + return new BadRequestException({ + code: PeerErrorCodes.SelfUrl, + message: + 'This address belongs to an agent of this installation — pick it ' + + 'in the agent list instead of importing it by URL.', + }); } /** @@ -267,6 +326,12 @@ export class PeerService { await assertResolvesPublic(cardUrl); } const card = await this.client.fetchCard(cardUrl, credential); + if (external) { + // A refreshed card is new remote content: held to the same bar as an + // import, so a peer cannot move its interface somewhere private (or + // onto us) between import and refresh (CLEAN-97). + await this.vetExternalCard(card, await this.cards.ownA2aBase()); + } const updated = await this.peers.updateSnapshot(row.id, { cardSnapshot: card, cardUrl, @@ -274,6 +339,9 @@ export class PeerService { }); return this.toView(updated); } catch (err) { + // A refusal from vetting already names its cause and code; only + // transport failures need translating. + if (err instanceof HttpException) throw err; throw this.cardUnreachable(err, row.cardSnapshot?.name ?? 'the peer'); } } @@ -341,7 +409,10 @@ export class PeerService { url.hash = ''; url.search = ''; let path = url.pathname.replace(/\/+$/, ''); - if (!path.endsWith(`/${A2A_CARD_PATH}`)) { + // An address that already names a JSON document is a card URL as it is: + // the pre-1.0 `/.well-known/agent.json` and custom card paths used to get + // `/.well-known/agent-card.json` glued onto them (CLEAN-97). + if (!path.toLowerCase().endsWith('.json')) { path = `${path}/${A2A_CARD_PATH}`; } url.pathname = path; @@ -360,6 +431,12 @@ export class PeerService { private importUnreadable(err: unknown, cardUrl: string): Error { const detail = err instanceof Error ? err.message : String(err); this.logger.warn(`External card read failed at ${cardUrl}: ${detail}`); + if (err instanceof PeerCardUnreachableError && err.kind === 'version') { + return new BadRequestException({ + code: PeerErrorCodes.Version, + message: detail, + }); + } if (err instanceof PeerCardUnreachableError && err.kind === 'invalid') { return new BadRequestException({ code: PeerErrorCodes.UrlInvalid, @@ -426,3 +503,60 @@ export class PeerService { }; } } + +function unique(values: Array): string[] { + return [ + ...new Set( + values.filter((v): v is string => typeof v === 'string' && v.length > 0), + ), + ]; +} + +/** + * An address reduced to what decides which route it reaches: host without a + * trailing dot, explicit port, and a path decoded once, collapsed and + * lower-cased — the way the server itself resolves it. The scheme is left + * out on purpose: http and https of our host are both us. + */ +function normalizeAddress( + raw: string, +): { host: string; port: string; path: string } | null { + let url: URL; + try { + url = new URL(raw.trim()); + } catch { + return null; + } + + let path = url.pathname; + try { + path = decodeURIComponent(path); + } catch { + // Malformed escapes: compare the raw form rather than give up. + } + path = path.replace(/\/{2,}/g, '/').toLowerCase(); + try { + path = new URL(path, 'http://normalize.invalid').pathname; + } catch { + // Keep the collapsed form. + } + if (!path.endsWith('/')) path = `${path}/`; + + return { + host: url.hostname.toLowerCase().replace(/\.+$/, ''), + port: url.port, + path, + }; +} + +/** True when `candidate` points into this installation's A2A surface. */ +function isOwnA2aAddress(candidate: string, ownBase: string): boolean { + const address = normalizeAddress(candidate); + const own = normalizeAddress(ownBase); + if (!address || !own) return false; + return ( + address.host === own.host && + address.port === own.port && + address.path.startsWith(own.path) + ); +} diff --git a/api/src/slices/agent/peer/domain/peer.types.ts b/api/src/slices/agent/peer/domain/peer.types.ts index 35acfec3..f5f92d1a 100644 --- a/api/src/slices/agent/peer/domain/peer.types.ts +++ b/api/src/slices/agent/peer/domain/peer.types.ts @@ -72,6 +72,8 @@ export const PeerErrorCodes = { UrlUnreachable: 'PEER_URL_UNREACHABLE', Version: 'PEER_VERSION', SelfUrl: 'PEER_SELF_URL', + // A 1.0 card with no JSON-RPC interface (CLEAN-97). + Binding: 'PEER_BINDING', } as const; export type PeerErrorCode = @@ -165,8 +167,18 @@ export const DelegationErrorCodes = { Unauthorized: 'PEER_UNAUTHORIZED', Unreachable: 'PEER_UNREACHABLE', Error: 'PEER_ERROR', + // The stored card's interface resolves to a private or local address, so + // no request was sent (CLEAN-97). + AddressRefused: 'PEER_ADDRESS_REFUSED', + // The stored card offers no JSON-RPC interface on A2A 1.0 (CLEAN-97). + Unsupported: 'PEER_UNSUPPORTED', } as const; +/** What the audit row and the visible step say when a peer answered with + * nothing readable — no text, no data, no link (CLEAN-97). */ +export const EMPTY_REPLY_NOTE = + 'The peer answered, but its reply had no text, data or links.'; + export type DelegationErrorCode = (typeof DelegationErrorCodes)[keyof typeof DelegationErrorCodes]; @@ -225,12 +237,13 @@ export interface IFinishDelegationData { /** The peer's card could not be read at connect or refresh time. * `kind` separates "could not reach it" from "reached it, not a card" so an - * external import can answer 502 vs 400 honestly (CLEAN-95). */ + * external import can answer 502 vs 400 honestly (CLEAN-95), and "it is a + * card, of a protocol version we do not speak" from both (CLEAN-97). */ export class PeerCardUnreachableError extends Error { constructor( message: string, public readonly status?: number, - public readonly kind: 'unreachable' | 'invalid' = 'unreachable', + public readonly kind: 'unreachable' | 'invalid' | 'version' = 'unreachable', ) { super(message); this.name = 'PeerCardUnreachableError'; diff --git a/api/src/slices/agent/peer/dtos/agentDelegation.dto.ts b/api/src/slices/agent/peer/dtos/agentDelegation.dto.ts index efda9ede..923c6778 100644 --- a/api/src/slices/agent/peer/dtos/agentDelegation.dto.ts +++ b/api/src/slices/agent/peer/dtos/agentDelegation.dto.ts @@ -53,7 +53,10 @@ export class AgentDelegationDto { description: 'Why it did not produce an answer: PEER_NOT_RUNNING, PEER_TIMEOUT, ' + 'PEER_REJECTED_LOOP, PEER_REJECTED_DEPTH, PEER_UNAUTHORIZED, ' + - 'PEER_UNREACHABLE or PEER_ERROR. Null while waiting and on success.', + 'PEER_UNREACHABLE, PEER_ADDRESS_REFUSED (the card points at a private ' + + 'or local address, so nothing was sent), PEER_UNSUPPORTED (the card ' + + 'offers no JSON-RPC interface on A2A 1.0) or PEER_ERROR. Null while ' + + 'waiting and on success — including an empty reply, which is answered.', example: null, }) errorCode: string | null; From 5bf998800ecdc61d20d8da0a2464b9c5fa4cccd1 Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Thu, 17 Sep 2026 18:50:36 +0300 Subject: [PATCH 2/3] fix(admin): show the A2A address delegations actually call (CLEAN-97) The card address line showed the first interface a card lists. The API now calls the first JSON-RPC interface on 1.0, so a card that prefers HTTP+JSON showed one address while delegations went to another. Both the card view and the A2A tab header use the same rule as the API now. The delegation feed also names the two new causes: a private interface address that was refused before anything was sent, and a card with no JSON-RPC interface. Co-Authored-By: Claude Opus 5 (1M context) --- .../agent/peer/components/peer/CardView.vue | 6 ++--- .../peer/components/peer/Delegations.vue | 3 +++ .../slices/agent/peer/components/peer/Tab.vue | 5 ++--- admin/slices/agent/peer/domain/cardAddress.ts | 22 +++++++++++++++++++ admin/slices/agent/peer/domain/index.ts | 1 + 5 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 admin/slices/agent/peer/domain/cardAddress.ts diff --git a/admin/slices/agent/peer/components/peer/CardView.vue b/admin/slices/agent/peer/components/peer/CardView.vue index 78c17c67..c5b51f64 100644 --- a/admin/slices/agent/peer/components/peer/CardView.vue +++ b/admin/slices/agent/peer/components/peer/CardView.vue @@ -1,5 +1,6 @@