From f2c2488c5c3e443ac665d451331b083fa5f2821a Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 30 May 2026 14:30:08 -0400 Subject: [PATCH] fix: include operation ids in task webhooks --- .changeset/framework-webhook-operation-id.md | 6 + package-lock.json | 6 +- .../decisioning/runtime/from-platform.ts | 58 ++++++++-- test/server-decisioning-from-platform.test.js | 103 +++++++++++++++++- 4 files changed, 157 insertions(+), 16 deletions(-) create mode 100644 .changeset/framework-webhook-operation-id.md diff --git a/.changeset/framework-webhook-operation-id.md b/.changeset/framework-webhook-operation-id.md new file mode 100644 index 000000000..94f651842 --- /dev/null +++ b/.changeset/framework-webhook-operation-id.md @@ -0,0 +1,6 @@ +--- +'@adcp/sdk': patch +--- + +Include `operation_id` in framework-emitted task webhook payloads and validate +push notification operation identifiers at the request boundary. diff --git a/package-lock.json b/package-lock.json index 1a77264ca..3cd195efc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@adcp/sdk", - "version": "8.1.0-beta.16", + "version": "8.1.0-beta.17", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@adcp/sdk", - "version": "8.1.0-beta.16", + "version": "8.1.0-beta.17", "license": "Apache-2.0", "workspaces": [ ".", @@ -7356,7 +7356,7 @@ "version": "6.0.0", "license": "Apache-2.0", "dependencies": { - "@adcp/sdk": "^8.1.0-beta.16" + "@adcp/sdk": "^8.1.0-beta.17" }, "bin": { "adcp": "bin/adcp.js" diff --git a/src/lib/server/decisioning/runtime/from-platform.ts b/src/lib/server/decisioning/runtime/from-platform.ts index e8332d23f..0ff050048 100644 --- a/src/lib/server/decisioning/runtime/from-platform.ts +++ b/src/lib/server/decisioning/runtime/from-platform.ts @@ -2550,6 +2550,7 @@ interface DispatchHitlOpts { accountId: string; pushNotificationUrl?: string; pushNotificationToken?: string; + pushNotificationOperationId?: string; emitWebhook?: HandlerContext['emitWebhook']; observability?: DecisioningObservabilityHooks; logger: AdcpLogger; @@ -2678,7 +2679,7 @@ async function emitSyncCompletionWebhook(opts: DispatchHitlOpts, result: unknown const r = await opts.emitWebhook({ url: opts.pushNotificationUrl, payload: wirePayload, - operation_id: `${opts.tool}.${taskId}`, + operation_id: resolveWebhookDeliveryOperationId(opts, taskId), }); success = r?.delivered === true; if (r && Array.isArray(r.errors) && r.errors.length > 0) { @@ -2839,8 +2840,8 @@ async function dispatchHitl( * `result`. This helper builds that shape from the v6 task lifecycle data. * * Spec requires top-level: `idempotency_key`, `task_id`, `task_type`, `status`, - * `timestamp`. Optional: `protocol`, `context_id`, `message`, `result`, - * `operation_id`. We add `validation_token` (echoed from + * `timestamp`, `operation_id`. Optional: `protocol`, `context_id`, `message`, + * `result`. We add `validation_token` (echoed from * `push_notification_config.token`) outside the spec but consistent with the * intent — receivers that don't expect it ignore the extra property. * @@ -2859,6 +2860,7 @@ function buildTaskWebhookPayload( const idempotencyKey = randomUUID(); const payload: Record = { idempotency_key: idempotencyKey, + operation_id: resolveWebhookPayloadOperationId(opts, taskId), task_id: taskId, task_type: opts.tool, status, @@ -2887,6 +2889,14 @@ function buildTaskWebhookPayload( return payload; } +function resolveWebhookPayloadOperationId(opts: DispatchHitlOpts, taskId: string): string { + return opts.pushNotificationOperationId ?? `${opts.tool}.${taskId}`; +} + +function resolveWebhookDeliveryOperationId(opts: DispatchHitlOpts, taskId: string): string { + return `task-webhook:${opts.accountId}:${opts.tool}:${taskId}`; +} + // `protocolForTool` and `SPEC_WEBHOOK_TASK_TYPES` are exported from // `protocol-for-tool.ts` — see import at top of file. @@ -2922,7 +2932,7 @@ async function emitTaskWebhook( const result = await opts.emitWebhook({ url: opts.pushNotificationUrl, payload: wirePayload, - operation_id: `${opts.tool}.${taskId}`, + operation_id: resolveWebhookDeliveryOperationId(opts, taskId), }); success = result?.delivered === true; if (result && Array.isArray(result.errors) && result.errors.length > 0) { @@ -3522,12 +3532,14 @@ function extractPushConfig( params: unknown, _logger: AdcpLogger, opts: { allowPrivateWebhookUrls?: boolean } = {} -): { url?: string; token?: string } { +): { url?: string; token?: string; operationId?: string } { if (!params || typeof params !== 'object') return {}; const cfg = (params as { push_notification_config?: unknown }).push_notification_config; if (!cfg || typeof cfg !== 'object') return {}; - const rawUrl = (cfg as { url?: unknown }).url; - const rawToken = (cfg as { token?: unknown }).token; + const cfgObj = cfg as { url?: unknown; token?: unknown; operation_id?: unknown }; + const rawUrl = cfgObj.url; + const rawToken = cfgObj.token; + const rawOperationId = cfgObj.operation_id; let url: string | undefined; if (typeof rawUrl === 'string') { @@ -3559,9 +3571,28 @@ function extractPushConfig( token = rawToken; } + let operationId: string | undefined; + if (Object.prototype.hasOwnProperty.call(cfgObj, 'operation_id')) { + if (typeof rawOperationId !== 'string') { + throw new AdcpError('INVALID_REQUEST', { + message: 'push_notification_config.operation_id rejected: operation_id must be a string', + field: 'push_notification_config.operation_id', + }); + } + const validation = validatePushNotificationOperationId(rawOperationId); + if (!validation.ok) { + throw new AdcpError('INVALID_REQUEST', { + message: `push_notification_config.operation_id rejected: ${validation.reason}`, + field: 'push_notification_config.operation_id', + }); + } + operationId = rawOperationId; + } + return { ...(url !== undefined && { url }), ...(token !== undefined && { token }), + ...(operationId !== undefined && { operationId }), }; } @@ -3684,6 +3715,7 @@ function validatePushNotificationUrl(rawUrl: string, opts: { allowPrivate?: bool const TOKEN_MAX_LENGTH = 255; const TOKEN_CONTROL_CHAR_RE = /[\x00-\x1f\x7f]/; +const PUSH_OPERATION_ID_RE = /^[A-Za-z0-9_.:-]{1,255}$/; function validatePushNotificationToken(token: string): UrlValidationResult { if (token.length === 0) { @@ -3698,6 +3730,13 @@ function validatePushNotificationToken(token: string): UrlValidationResult { return { ok: true }; } +function validatePushNotificationOperationId(operationId: string): UrlValidationResult { + if (!PUSH_OPERATION_ID_RE.test(operationId)) { + return { ok: false, reason: `operation_id must match ${PUSH_OPERATION_ID_RE.source}` }; + } + return { ok: true }; +} + function buildMediaBuyHandlers

>( platform: P, taskRegistry: TaskRegistry, @@ -3771,6 +3810,7 @@ function buildMediaBuyHandlers

>( accountId: reqCtx.account.id, pushNotificationUrl: push.url, pushNotificationToken: push.token, + pushNotificationOperationId: push.operationId, emitWebhook: taskWebhookEmit ?? ctx.emitWebhook, autoEmitCompletion: pushOpts.autoEmitCompletionWebhooks, observability, @@ -3906,6 +3946,7 @@ function buildMediaBuyHandlers

>( accountId: reqCtx.account.id, pushNotificationUrl: push.url, pushNotificationToken: push.token, + pushNotificationOperationId: push.operationId, emitWebhook: taskWebhookEmit ?? ctx.emitWebhook, autoEmitCompletion: pushOpts.autoEmitCompletionWebhooks, observability, @@ -3984,6 +4025,7 @@ function buildMediaBuyHandlers

>( accountId: reqCtx.account.id, pushNotificationUrl: push.url, ...(push.token !== undefined && { pushNotificationToken: push.token }), + ...(push.operationId !== undefined && { pushNotificationOperationId: push.operationId }), emitWebhook: taskWebhookEmit ?? ctx.emitWebhook, ...(observability && { observability }), logger, @@ -4019,6 +4061,7 @@ function buildMediaBuyHandlers

>( accountId: reqCtx.account.id, pushNotificationUrl: push.url, pushNotificationToken: push.token, + pushNotificationOperationId: push.operationId, emitWebhook: taskWebhookEmit ?? ctx.emitWebhook, autoEmitCompletion: pushOpts.autoEmitCompletionWebhooks, observability, @@ -4270,6 +4313,7 @@ function buildCreativeHandlers

>( accountId: reqCtx.account.id, pushNotificationUrl: push.url, pushNotificationToken: push.token, + pushNotificationOperationId: push.operationId, emitWebhook: taskWebhookEmit ?? ctx.emitWebhook, autoEmitCompletion: pushOpts.autoEmitCompletionWebhooks, observability, diff --git a/test/server-decisioning-from-platform.test.js b/test/server-decisioning-from-platform.test.js index c8927765b..088556f9c 100644 --- a/test/server-decisioning-from-platform.test.js +++ b/test/server-decisioning-from-platform.test.js @@ -15,6 +15,14 @@ const { AccountNotFoundError } = require('../dist/lib/server/decisioning/account const { AdcpError } = require('../dist/lib/server/decisioning/async-outcome'); const { setStatusChangeBus, createInMemoryStatusChangeBus } = require('../dist/lib/server/decisioning/status-changes'); const { StaticJwksResolver, InMemoryReplayStore, InMemoryRevocationStore } = require('../dist/lib/signing/server.js'); +const { getSchemaValidatorByRef } = require('../dist/lib/validation/schema-loader'); + +const validateMcpWebhookPayload = getSchemaValidatorByRef('core/mcp-webhook-payload.json'); +assert.ok(validateMcpWebhookPayload, 'MCP webhook payload schema must compile'); + +function assertMcpWebhookPayloadValid(payload) { + assert.strictEqual(validateMcpWebhookPayload(payload), true, JSON.stringify(validateMcpWebhookPayload.errors)); +} function buildPlatform(overrides = {}) { return { @@ -2671,7 +2679,7 @@ describe('HITL push notification webhook on terminal state', () => { }; } - it('emits webhook on completed task with push_notification_config', async () => { + it('emits webhook on completed task with push_notification_config operation_id', async () => { const emits = []; const fakeEmitter = { emit: async params => { @@ -2699,7 +2707,11 @@ describe('HITL push notification webhook on terminal state', () => { start_time: '2026-05-01T00:00:00Z', end_time: '2026-06-01T00:00:00Z', account: { account_id: 'acc_1' }, - push_notification_config: { url: 'https://buyer.example.com/webhook', token: 'shhh' }, + push_notification_config: { + url: 'https://buyer.example.com/webhook', + token: 'webhook-token-1234', + operation_id: 'op_webhook_test', + }, }, }, }); @@ -2716,13 +2728,63 @@ describe('HITL push notification webhook on terminal state', () => { // carries the success-arm body. assert.strictEqual(emit.payload.task_id, taskId); assert.strictEqual(emit.payload.task_type, 'create_media_buy'); + assert.strictEqual(emit.payload.operation_id, 'op_webhook_test'); assert.strictEqual(emit.payload.status, 'completed'); assert.strictEqual(emit.payload.protocol, 'media-buy'); assert.ok(typeof emit.payload.idempotency_key === 'string' && emit.payload.idempotency_key.length >= 16); assert.ok(typeof emit.payload.timestamp === 'string'); assert.deepStrictEqual(emit.payload.result, { media_buy_id: 'mb_42', status: 'active' }); - assert.strictEqual(emit.payload.token, 'shhh'); - assert.ok(emit.operation_id.startsWith('create_media_buy.task_')); + assert.strictEqual(emit.payload.token, 'webhook-token-1234'); + assert.match(emit.operation_id, new RegExp(`^task-webhook:acc_1:create_media_buy:${taskId}$`)); + assert.notStrictEqual(emit.operation_id, 'op_webhook_test'); + assertMcpWebhookPayloadValid(emit.payload); + }); + + it('treats webhook URL as opaque when push config omits operation_id', async () => { + const emits = []; + const fakeEmitter = { + emit: async params => { + emits.push(params); + return { operation_id: params.operation_id, idempotency_key: 'k', attempts: 1, delivered: true, errors: [] }; + }, + }; + + const platform = buildHitlPlatform(async () => ({ media_buy_id: 'mb_42', status: 'active' })); + const server = createAdcpServerFromPlatform(platform, { + name: 'webhook', + version: '0.0.1', + validation: { requests: 'off', responses: 'off' }, + taskWebhookEmitter: fakeEmitter, + }); + + const result = await server.dispatchTestRequest({ + method: 'tools/call', + params: { + name: 'create_media_buy', + arguments: { + buyer_ref: 'b1', + idempotency_key: '11111111-1111-1111-1111-111111111111', + packages: [], + start_time: '2026-05-01T00:00:00Z', + end_time: '2026-06-01T00:00:00Z', + account: { account_id: 'acc_1' }, + push_notification_config: { + url: 'https://buyer.example.com/step/create_media_buy/op_url_must_not_be_parsed', + token: 'webhook-token-1234', + }, + }, + }, + }); + + assert.strictEqual(result.structuredContent.status, 'submitted'); + await server.awaitTask(result.structuredContent.task_id); + + assert.strictEqual(emits.length, 1, 'one webhook emitted on terminal completion'); + assert.ok(emits[0].payload.operation_id.startsWith('create_media_buy.')); + assert.match(emits[0].operation_id, /^task-webhook:acc_1:create_media_buy:task_/); + assert.notStrictEqual(emits[0].payload.operation_id, 'op_url_must_not_be_parsed'); + assert.notStrictEqual(emits[0].operation_id, 'op_url_must_not_be_parsed'); + assertMcpWebhookPayloadValid(emits[0].payload); }); it('emits webhook on failed task with structured error', async () => { @@ -2835,7 +2897,7 @@ describe('Push notification webhook URL/token validation (B5/B6)', () => { }; } - async function dispatchWithUrl(server, url, token) { + async function dispatchWithPushConfig(server, pushNotificationConfig) { return server.dispatchTestRequest({ method: 'tools/call', params: { @@ -2847,12 +2909,16 @@ describe('Push notification webhook URL/token validation (B5/B6)', () => { start_time: '2026-05-01T00:00:00Z', end_time: '2026-06-01T00:00:00Z', account: { account_id: 'acc_1' }, - push_notification_config: { url, ...(token != null && { token }) }, + push_notification_config: pushNotificationConfig, }, }, }); } + async function dispatchWithUrl(server, url, token) { + return dispatchWithPushConfig(server, { url, ...(token != null && { token }) }); + } + function makeServer({ warns, emits } = {}) { return createAdcpServerFromPlatform( buildHitlPlatform(async () => ({ media_buy_id: 'mb_1' })), @@ -2972,6 +3038,31 @@ describe('Push notification webhook URL/token validation (B5/B6)', () => { assert.strictEqual(emits.length, 0); }); + for (const [label, operationId, reasonFragment] of [ + ['non-string operation_id', 123, 'must be a string'], + ['empty operation_id', '', 'must match'], + ['operation_id over 255 chars', 'a'.repeat(256), 'must match'], + ['operation_id with invalid character', 'op/bad', 'must match'], + ['operation_id with control character', 'op_\n_bad', 'must match'], + ]) { + it(`rejects ${label}`, async () => { + const emits = []; + const server = makeServer({ emits }); + const result = await dispatchWithPushConfig(server, { + url: 'https://buyer.example.com/webhook', + operation_id: operationId, + }); + assert.strictEqual(result.isError, true); + assert.strictEqual(result.structuredContent.adcp_error.code, 'INVALID_REQUEST'); + assert.strictEqual(result.structuredContent.adcp_error.field, 'push_notification_config.operation_id'); + assert.ok( + result.structuredContent.adcp_error.message.includes(reasonFragment), + `expected rejection reason "${reasonFragment}", got: ${result.structuredContent.adcp_error.message}` + ); + assert.strictEqual(emits.length, 0); + }); + } + it('accepts a well-formed token', async () => { const emits = []; const server = makeServer({ emits });