diff --git a/.changeset/a2a-submitted-parser-fix.md b/.changeset/a2a-submitted-parser-fix.md new file mode 100644 index 000000000..6629d1d8a --- /dev/null +++ b/.changeset/a2a-submitted-parser-fix.md @@ -0,0 +1,77 @@ +--- +'@adcp/client': patch +--- + +Fix `ProtocolResponseParser.getStatus` and `getTaskId` to read AdCP +work-layer fields from A2A wrapped Task responses instead of the +transport-layer fields. Closes #973. + +Per #899's two-lifecycle contract, A2A `Task.state` reflects the +HTTP-call lifecycle (always `'completed'` for AdCP submitted arms — +the call returned with a queued AdCP task), and `Task.id` is the +SDK-generated transport handle (pinned to one HTTP call). The AdCP +work lifecycle and work handle live on the artifact: +`artifact.parts[0].data.status` and `artifact.metadata.adcp_task_id` +respectively. + +**Pre-fix behavior**: + +- `getStatus` for an A2A submitted-arm response returned + `'completed'` (read from `result.status.state`), preventing + `TaskExecutor.handleAsyncResponse` from ever entering the + SUBMITTED branch. Buyers thought async operations finished + synchronously — `result.submitted` was undefined; no + `SubmittedContinuation` was issued. +- `getTaskId` returned the A2A Task.id, which the seller's AdCP + `tasks/get` tool would not recognize (the seller knows the AdCP + task handle, not the transport id). + +**Fix**: when `result.kind === 'task'` AND the artifact's first +DataPart carries an AdCP payload, prefer the AdCP-layer fields: + +- `getStatus`: read `artifact.parts[0].data.status` if it's an + `ADCP_STATUS` enum value; fall back to `result.status.state`. +- `getTaskId`: read `artifact.metadata.adcp_task_id` if present and + passes the session-id safety guard; fall back to `result.id`. + +Non-AdCP A2A responses (no artifact, no DataPart, or `data.status` +not in the AdCP enum) keep the previous behavior — the transport- +layer fields are authoritative. + +**End-to-end consequence**: combined with #966 (server-task-id +plumbing) and #967 (AdCP `tasks/get` request/response shape), A2A +submitted-arm polling now works end-to-end against any +`createA2AAdapter`-backed seller. Probe before this PR: + +``` +result.status = completed ← WRONG, treated as sync completion +result.submitted = undefined +result.metadata.serverTaskId = +``` + +After: + +``` +result.status = submitted +result.submitted.taskId = tk_seller_handle_99 ← AdCP work handle +``` + +**Tests**: + +- `test/lib/protocol-response-parser-a2a-submitted.test.js` — 15 + unit tests covering AdCP-layer reads (submitted/working/failed), + fallback paths (no artifact, no DataPart, malformed status, no + metadata), interaction with MCP `structuredContent` (untouched), + and session-id safety guards. +- `test/server-a2a-submitted-end-to-end.test.js` — full submitted → + working → working → completed roundtrip against a real + `createA2AAdapter`. Asserts (1) SDK classifies as submitted, + (2) `SubmittedContinuation.taskId` is the AdCP handle, (3) + polling dispatches `tasks/get` with snake_case `task_id`, (4) + the spec-shape `tasks/get` response resolves + `waitForCompletion()` with `result.media_buy_id`. + +This is the third and final landmark of the A2A submitted-arm +polling story (#966 → #967 → #973). With it, A2A buyers can drive +guaranteed-buy / IO-signing / governance-review / batch-processing +flows end-to-end through the SDK without webhook-only fallbacks. diff --git a/src/lib/core/ProtocolResponseParser.ts b/src/lib/core/ProtocolResponseParser.ts index 0196860b1..9caf6fa8d 100644 --- a/src/lib/core/ProtocolResponseParser.ts +++ b/src/lib/core/ProtocolResponseParser.ts @@ -87,6 +87,57 @@ function isSafeSessionId(v: unknown): v is string { return SESSION_ID_PATTERN.test(v); } +/** + * Extract the AdCP work-layer status from an A2A wrapped Task result, + * if present. The AdCP `submitted` / `working` / `completed` lifecycle + * lives on `artifact.parts[0].data.status` (per adcp-client#899's + * two-lifecycle contract); the transport-layer `result.status.state` + * tracks the HTTP-call lifecycle and is `'completed'` for AdCP + * submitted arms. + * + * Returns `undefined` for non-AdCP A2A responses (no artifact, no + * DataPart, or `data.status` not in the AdCP enum) so callers can + * fall back to the transport-layer status. + */ +function extractAdcpStatusFromA2aTaskResult(result: any): ADCPStatus | undefined { + if (result == null || typeof result !== 'object' || Array.isArray(result)) return undefined; + if (result.kind !== 'task') return undefined; + const artifacts = result.artifacts; + if (!Array.isArray(artifacts) || artifacts.length === 0) return undefined; + const artifact = artifacts[0]; + if (artifact == null || typeof artifact !== 'object') return undefined; + const parts = artifact.parts; + if (!Array.isArray(parts) || parts.length === 0) return undefined; + const firstPart = parts[0]; + if (firstPart == null || typeof firstPart !== 'object' || firstPart.kind !== 'data') return undefined; + const data = firstPart.data; + if (data == null || typeof data !== 'object' || Array.isArray(data)) return undefined; + const status = (data as Record).status; + if (typeof status === 'string' && (Object.values(ADCP_STATUS) as string[]).includes(status)) { + return status as ADCPStatus; + } + return undefined; +} + +/** + * Extract the AdCP task handle from an A2A wrapped Task result. The + * handle lives on `artifact.metadata.adcp_task_id` (per + * adcp-client#899). Returns `undefined` for non-AdCP A2A responses or + * when the metadata extension wasn't emitted, so callers fall back + * to the transport-layer `result.id`. + */ +function extractAdcpTaskIdFromA2aTaskResult(result: any): string | undefined { + if (result == null || typeof result !== 'object' || Array.isArray(result)) return undefined; + if (result.kind !== 'task') return undefined; + const artifacts = result.artifacts; + if (!Array.isArray(artifacts) || artifacts.length === 0) return undefined; + const artifact = artifacts[0]; + if (artifact == null || typeof artifact !== 'object') return undefined; + const metadata = artifact.metadata; + if (metadata == null || typeof metadata !== 'object' || Array.isArray(metadata)) return undefined; + return firstSafeSessionId((metadata as Record).adcp_task_id); +} + /** Return the first argument that passes {@link isSafeSessionId}, else `undefined`. */ function firstSafeSessionId(...candidates: unknown[]): string | undefined { for (const c of candidates) { @@ -145,7 +196,20 @@ export class ProtocolResponseParser { * Get ADCP status from response */ getStatus(response: any): ADCPStatus | null { - // Check A2A JSON-RPC wrapped status (result.status.state) + // For A2A wrapped Task responses (`result.kind === 'task'`), the + // transport-layer `result.status.state` reflects the HTTP-call + // lifecycle (always `'completed'` for AdCP submitted arms per + // adcp-client#899), NOT the AdCP work lifecycle. Prefer the AdCP + // status surfaced on the artifact's DataPart when it's set — + // that's the layer the buyer cares about for polling decisions. + // See adcp-client#973 for the regression class this catches. + const adcpStatusFromArtifact = extractAdcpStatusFromA2aTaskResult(response?.result); + if (adcpStatusFromArtifact) return adcpStatusFromArtifact; + + // Check A2A JSON-RPC wrapped status (result.status.state) — used + // when the artifact didn't surface an AdCP status (non-AdCP A2A + // responses, or sync-completed responses where transport state is + // authoritative). if (response?.result?.status?.state && Object.values(ADCP_STATUS).includes(response.result.status.state)) { return response.result.status.state as ADCPStatus; } @@ -268,9 +332,17 @@ export class ProtocolResponseParser { if (response == null) return undefined; if (response.result) { - // A2A Task result carries its own id; Message results carry `taskId` - // when bound to a task. + // A2A wrapped Task with AdCP payload — `artifact.metadata.adcp_task_id` + // is the AdCP work handle (what the buyer polls with via AdCP + // `tasks/get`). The transport-layer `result.id` is the A2A + // Task.id (always pinned to one HTTP call per adcp-client#899's + // two-lifecycle contract); using it as a polling key would + // address the wrong thing. Prefer the AdCP handle when present; + // fall back to `result.id` for non-AdCP A2A responses where no + // artifact metadata was emitted. See adcp-client#973. if (response.result.kind === 'task') { + const adcpHandle = extractAdcpTaskIdFromA2aTaskResult(response.result); + if (adcpHandle) return adcpHandle; const taskKindId = firstSafeSessionId(response.result.id); if (taskKindId) return taskKindId; } diff --git a/test/lib/protocol-response-parser-a2a-submitted.test.js b/test/lib/protocol-response-parser-a2a-submitted.test.js new file mode 100644 index 000000000..19a0ffee0 --- /dev/null +++ b/test/lib/protocol-response-parser-a2a-submitted.test.js @@ -0,0 +1,164 @@ +// Regression tests for ProtocolResponseParser — issue #973. +// +// For A2A wrapped Task responses (`result.kind === 'task'`), the parser +// must prefer the AdCP work-layer fields surfaced via the artifact +// (`artifact.parts[0].data.status`, `artifact.metadata.adcp_task_id`) +// over the transport-layer fields (`result.status.state`, `result.id`). +// Per adcp-client#899's two-lifecycle contract: +// +// - `Task.state` reflects the HTTP-call lifecycle — always +// `'completed'` for AdCP submitted arms (the call returned, the +// work is queued). +// - `data.status` reflects the AdCP work lifecycle — `'submitted'`, +// `'working'`, `'completed'`, etc. +// +// - `Task.id` is the A2A SDK-generated transport handle (pinned to +// one HTTP call). +// - `artifact.metadata.adcp_task_id` is the AdCP work handle (the +// thing the buyer polls with). +// +// Pre-fix: `getStatus` returned `'completed'` for every A2A submitted +// arm (read from `result.status.state`), preventing +// `TaskExecutor.handleAsyncResponse` from ever entering the SUBMITTED +// branch. `getTaskId` returned the A2A Task.id (which the seller's +// AdCP `tasks/get` tool would not recognize). + +const { test, describe } = require('node:test'); +const assert = require('node:assert'); + +const { ProtocolResponseParser, ADCP_STATUS } = require('../../dist/lib/index.js'); + +const parser = new ProtocolResponseParser(); + +function a2aWrappedSubmittedResponse({ adcpTaskId = 'tk_X', a2aTaskId = 'a2a-uuid', adcpStatus = 'submitted' } = {}) { + return { + jsonrpc: '2.0', + id: 1, + result: { + kind: 'task', + id: a2aTaskId, + contextId: 'ctx-uuid', + // Per #899: A2A Task.state is 'completed' for AdCP submitted arms. + // Pre-fix the parser read this and called the response 'completed'. + status: { state: 'completed', timestamp: '2026-04-25T00:00:00Z' }, + artifacts: [ + { + artifactId: 'art-uuid', + name: 'submitted', + parts: [ + { + kind: 'data', + data: { status: adcpStatus, task_id: adcpTaskId }, + }, + ], + metadata: { adcp_task_id: adcpTaskId }, + }, + ], + }, + }; +} + +describe('ProtocolResponseParser.getStatus — A2A submitted arm (#973)', () => { + test('reads AdCP `data.status` from the artifact for submitted arms (NOT transport `Task.state`)', () => { + const response = a2aWrappedSubmittedResponse({ adcpStatus: 'submitted' }); + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.SUBMITTED); + }); + + test('handles AdCP `working` status from the artifact', () => { + const response = a2aWrappedSubmittedResponse({ adcpStatus: 'working' }); + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.WORKING); + }); + + test('handles AdCP `failed` status from the artifact', () => { + const response = a2aWrappedSubmittedResponse({ adcpStatus: 'failed' }); + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.FAILED); + }); + + test('falls back to transport `Task.state` for non-AdCP A2A responses (no artifact)', () => { + // Pure A2A Task with no artifact-borne AdCP payload — return the + // transport-layer state. This is the pre-fix behavior preserved + // for non-AdCP A2A responses. + const response = { + result: { + kind: 'task', + id: 'a2a-uuid', + contextId: 'ctx-uuid', + status: { state: 'completed', timestamp: '2026-04-25T00:00:00Z' }, + artifacts: [], + }, + }; + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.COMPLETED); + }); + + test('falls back when artifact has no DataPart', () => { + const response = a2aWrappedSubmittedResponse(); + response.result.artifacts[0].parts = [{ kind: 'text', text: 'hi' }]; + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.COMPLETED); + }); + + test('falls back when DataPart `data.status` is not an ADCP_STATUS value', () => { + const response = a2aWrappedSubmittedResponse(); + response.result.artifacts[0].parts[0].data.status = 'pending_approval'; // not an AdCP task status + // The artifact extractor returns undefined → fall through to the + // transport-layer state. + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.COMPLETED); + }); + + test('AdCP completed surfaces from artifact even when transport says completed (idempotent)', () => { + const response = a2aWrappedSubmittedResponse({ adcpStatus: 'completed' }); + // Both layers say completed; either path returns COMPLETED. Test + // pins that the AdCP-layer extractor doesn't error on the happy + // path. + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.COMPLETED); + }); + + test('does not touch non-A2A responses (MCP structuredContent unaffected)', () => { + const response = { + structuredContent: { status: 'submitted', task_id: 'tk_X' }, + }; + assert.strictEqual(parser.getStatus(response), ADCP_STATUS.SUBMITTED); + }); +}); + +describe('ProtocolResponseParser.getTaskId — A2A submitted arm (#973)', () => { + test('reads `artifact.metadata.adcp_task_id` for AdCP submitted arms (NOT A2A `result.id`)', () => { + const response = a2aWrappedSubmittedResponse({ adcpTaskId: 'tk_seller', a2aTaskId: 'a2a-transport-id' }); + assert.strictEqual(parser.getTaskId(response), 'tk_seller'); + }); + + test('falls back to transport `result.id` for A2A responses without metadata', () => { + const response = a2aWrappedSubmittedResponse(); + delete response.result.artifacts[0].metadata; + assert.strictEqual(parser.getTaskId(response), 'a2a-uuid'); + }); + + test('falls back when artifact metadata has no `adcp_task_id`', () => { + const response = a2aWrappedSubmittedResponse(); + response.result.artifacts[0].metadata = { other_extension: 'value' }; + assert.strictEqual(parser.getTaskId(response), 'a2a-uuid'); + }); + + test('falls back when artifacts array is empty', () => { + const response = a2aWrappedSubmittedResponse(); + response.result.artifacts = []; + assert.strictEqual(parser.getTaskId(response), 'a2a-uuid'); + }); + + test('rejects malformed `adcp_task_id` (control chars, overlong) and falls back', () => { + const response = a2aWrappedSubmittedResponse(); + response.result.artifacts[0].metadata.adcp_task_id = 'tk\x00with-null'; + // Malformed value rejected by `firstSafeSessionId` (control chars + // banned). Falls through to `result.id`. + assert.strictEqual(parser.getTaskId(response), 'a2a-uuid'); + }); + + test('does not touch MCP responses', () => { + const response = { structuredContent: { task_id: 'mcp-tk-1' } }; + assert.strictEqual(parser.getTaskId(response), 'mcp-tk-1'); + }); + + test('flat AdCP envelope (no result wrapping) reads response.task_id directly', () => { + const response = { task_id: 'flat-tk-2' }; + assert.strictEqual(parser.getTaskId(response), 'flat-tk-2'); + }); +}); diff --git a/test/server-a2a-submitted-end-to-end.test.js b/test/server-a2a-submitted-end-to-end.test.js new file mode 100644 index 000000000..37ea054d1 --- /dev/null +++ b/test/server-a2a-submitted-end-to-end.test.js @@ -0,0 +1,166 @@ +// Integration test: drives a full A2A submitted → completed +// roundtrip through the SDK, exercising: +// +// 1. `responseParser.getStatus` correctly classifies the seller's +// submitted-arm response as `'submitted'` (not `'completed'`) +// via the artifact's `data.status` (#973). +// 2. `responseParser.getTaskId` extracts the AdCP work handle from +// `artifact.metadata.adcp_task_id` (not the A2A Task.id). +// 3. `setupSubmittedTask` plumbs the AdCP handle into +// `SubmittedContinuation.taskId` (#966). +// 4. `pollTaskCompletion` dispatches AdCP `tasks/get` with snake_case +// `task_id` (#967). +// 5. The seller's AdCP `tasks/get` tool returns the spec-shape +// response, which the SDK maps to `TaskInfo` and resolves on +// `waitForCompletion`. +// +// This is the regression-class anchor for the entire A2A submitted-arm +// polling cycle end-to-end. Before #966/#967/#973 it didn't work for +// any spec-conformant seller. + +const { describe, it } = require('node:test'); +const assert = require('node:assert'); +const express = require('express'); + +const { createAdcpServer: _createAdcpServer } = require('../dist/lib/server/create-adcp-server'); +const { createA2AAdapter } = require('../dist/lib/server/a2a-adapter'); +const { InMemoryStateStore } = require('../dist/lib/server/state-store'); +const { TaskExecutor } = require('../dist/lib/index'); + +function createAdcpServer(config) { + return _createAdcpServer({ + ...config, + stateStore: config?.stateStore ?? new InMemoryStateStore(), + validation: { requests: 'off', responses: 'off', ...(config?.validation ?? {}) }, + }); +} + +async function startA2aFixture(handlers) { + const adcp = createAdcpServer(handlers); + const app = express(); + app.use(express.json()); + const server = app.listen(0); + await new Promise(resolve => server.once('listening', resolve)); + const { port } = server.address(); + const cardUrl = `http://127.0.0.1:${port}/a2a`; + const a2a = createA2AAdapter({ + server: adcp, + agentCard: { + name: 'Async A2A seller', + description: 'submitted → completed roundtrip', + url: cardUrl, + version: '1.0.0', + provider: { organization: 'Test', url: 'https://test.example' }, + securitySchemes: { bearer: { type: 'http', scheme: 'bearer' } }, + }, + }); + a2a.mount(app); + return { + server, + url: cardUrl, + close: () => new Promise(resolve => server.close(resolve)), + }; +} + +describe('A2A submitted → completed end-to-end (#966 + #967 + #973)', () => { + it('SDK classifies submitted, polls with AdCP task_id, and resolves on completion', async () => { + const SELLER_TASK_ID = 'tk_seller_async_1'; + const SELLER_MEDIA_BUY_ID = 'mb_completed_42'; + let pollCount = 0; + let observedPollParam; + let observedPollSkill; + + const fixture = await startA2aFixture({ + mediaBuy: { + createMediaBuy: async () => ({ + status: 'submitted', + task_id: SELLER_TASK_ID, + message: 'IO signature pending', + }), + }, + // Custom AdCP `tasks/get` tool dispatched as a buyer-callable + // tool over `message/send`. First two polls return working; + // third returns completed with the result data. + }); + // Drive polls via a `ProtocolClient.callTool` monkey-patch + // rather than registering a `tasks/get` tool on the seller — + // we want to assert on the exact arguments the SDK dispatches + // (snake_case `task_id`), and a real seller-side tool + // registration would obscure that surface. + const { ProtocolClient } = require('../dist/lib/index'); + const originalCallTool = ProtocolClient.callTool; + ProtocolClient.callTool = async (agent, toolName, params, ...rest) => { + if (toolName === 'tasks/get') { + observedPollSkill = toolName; + observedPollParam = params; + pollCount += 1; + if (pollCount < 3) { + return { + task_id: params.task_id, + task_type: 'create_media_buy', + protocol: 'media-buy', + status: 'working', + created_at: '2026-04-25T10:00:00Z', + updated_at: new Date().toISOString(), + }; + } + return { + task_id: params.task_id, + task_type: 'create_media_buy', + protocol: 'media-buy', + status: 'completed', + created_at: '2026-04-25T10:00:00Z', + updated_at: new Date().toISOString(), + result: { media_buy_id: SELLER_MEDIA_BUY_ID, packages: [] }, + }; + } + return originalCallTool.call(ProtocolClient, agent, toolName, params, ...rest); + }; + + try { + const executor = new TaskExecutor({ pollingInterval: 5 }); + const submittedResult = await executor.executeTask( + { id: 't', name: 't', agent_uri: fixture.url, protocol: 'a2a' }, + 'create_media_buy', + { + brand: { brand_id: 'b' }, + account: { account_id: 'a' }, + start_time: '2026-01-01T00:00:00Z', + end_time: '2026-02-01T00:00:00Z', + } + ); + + // (1) + (2) + (3): SDK saw the response as submitted, with + // the AdCP handle (not the A2A Task.id). + assert.strictEqual(submittedResult.status, 'submitted', 'SDK classifies as submitted'); + assert.ok(submittedResult.submitted, 'submitted continuation present'); + assert.strictEqual( + submittedResult.submitted.taskId, + SELLER_TASK_ID, + 'continuation surfaces the AdCP task handle, not the A2A Task.id' + ); + + // Poll through to completion. + const completion = await submittedResult.submitted.waitForCompletion(5); + + // (4): poll dispatched `tasks/get` with snake_case `task_id` + // carrying the AdCP handle. + assert.strictEqual(observedPollSkill, 'tasks/get'); + assert.strictEqual( + observedPollParam.task_id, + SELLER_TASK_ID, + 'poll addresses the AdCP task handle (snake_case task_id)' + ); + assert.strictEqual(observedPollParam.taskId, undefined, 'no legacy camelCase taskId'); + + // (5): SDK mapped the spec-shape response and resolved. + assert.strictEqual(completion.success, true); + assert.strictEqual(completion.status, 'completed'); + assert.deepStrictEqual(completion.data, { media_buy_id: SELLER_MEDIA_BUY_ID, packages: [] }); + assert.ok(pollCount >= 3, `expected ≥3 polls (working/working/completed), got ${pollCount}`); + } finally { + ProtocolClient.callTool = originalCallTool; + await fixture.close(); + } + }); +});