diff --git a/.changeset/setup-url-guard.md b/.changeset/setup-url-guard.md new file mode 100644 index 000000000..cb0542909 --- /dev/null +++ b/.changeset/setup-url-guard.md @@ -0,0 +1,5 @@ +--- +'@adcp/client': patch +--- + +Response builders now throw a descriptive error when `setup` is placed at the top level of a media buy response. The IO-signing setup URL belongs inside `account.setup` (a field on `Account`), not on the media buy itself. This was a silent trap because `DomainHandler` accepts `Record` so the strict type wasn't catching it. Affects `mediaBuyResponse`, `updateMediaBuyResponse`, and `getMediaBuysResponse`. diff --git a/src/lib/server/responses.ts b/src/lib/server/responses.ts index 1a9e77b42..35e123337 100644 --- a/src/lib/server/responses.ts +++ b/src/lib/server/responses.ts @@ -68,6 +68,21 @@ export function toStructuredContent(data: object): Record { return data as unknown as Record; } +// `setup` is only ever nested inside an `Account` (the IO-signing / pending_approval +// path). A top-level `setup` on a media buy response means the builder read the +// storyboard's "setup.url" shorthand as a top-level field. The strict handler types +// would catch this, but `DomainHandler` accepts `Record` for DX, +// so the error has to move to runtime. +function assertNoTopLevelSetup(data: unknown, builder: string): void { + if (data != null && typeof data === 'object' && 'setup' in data) { + throw new Error( + `${builder}: \`setup\` is not a field on the media buy — it belongs inside \`account.setup\`. ` + + `Move \`{ setup: { url, message } }\` to \`{ account: { ..., setup: { url, message } } }\`. ` + + `The setup URL is a property of the Account (returned alongside \`status: 'pending_approval'\`), not the MediaBuy.` + ); + } +} + /** * Build a get_adcp_capabilities response. * @@ -101,6 +116,7 @@ export function productsResponse(data: GetProductsResponse, summary?: string): M * `status` is provided but `valid_actions` is not */ export function mediaBuyResponse(data: CreateMediaBuySuccess, summary?: string): McpToolResponse { + assertNoTopLevelSetup(data, 'mediaBuyResponse'); const withDefaults = { ...data }; if (withDefaults.revision === undefined) { withDefaults.revision = 1; @@ -161,6 +177,7 @@ export function listCreativeFormatsResponse(data: ListCreativeFormatsResponse, s * `valid_actions` from `validActionsForStatus()`. */ export function updateMediaBuyResponse(data: UpdateMediaBuySuccess, summary?: string): McpToolResponse { + assertNoTopLevelSetup(data, 'updateMediaBuyResponse'); const withDefaults = { ...data }; if (withDefaults.valid_actions === undefined && withDefaults.status != null) { withDefaults.valid_actions = validActionsForStatus(withDefaults.status); @@ -175,6 +192,11 @@ export function updateMediaBuyResponse(data: UpdateMediaBuySuccess, summary?: st * Build a get_media_buys response. */ export function getMediaBuysResponse(data: GetMediaBuysResponse, summary?: string): McpToolResponse { + if (Array.isArray(data.media_buys)) { + for (const buy of data.media_buys) { + assertNoTopLevelSetup(buy, 'getMediaBuysResponse'); + } + } return { content: [ { diff --git a/test/server-responses.test.js b/test/server-responses.test.js index 2b42f98f9..931012a5e 100644 --- a/test/server-responses.test.js +++ b/test/server-responses.test.js @@ -107,6 +107,34 @@ describe('mediaBuyResponse', () => { }); assert.deepStrictEqual(result.structuredContent.valid_actions, ['cancel']); }); + + it('throws when `setup` is placed at the top level instead of inside account', () => { + assert.throws( + () => + mediaBuyResponse({ + media_buy_id: 'mb_1', + packages: [], + status: 'pending_approval', + setup: { url: 'https://example.com/sign', message: 'Review IO' }, + }), + /`setup` is not a field on the media buy.*belongs inside `account\.setup`/ + ); + }); + + it('accepts setup nested under account', () => { + const result = mediaBuyResponse({ + media_buy_id: 'mb_1', + packages: [], + status: 'pending_approval', + account: { + account_id: 'acct_1', + name: 'Acme', + status: 'pending_approval', + setup: { url: 'https://example.com/sign', message: 'Review IO' }, + }, + }); + assert.strictEqual(result.structuredContent.account.setup.url, 'https://example.com/sign'); + }); }); describe('deliveryResponse', () => { @@ -172,6 +200,18 @@ describe('updateMediaBuyResponse', () => { }); assert.deepStrictEqual(result.structuredContent.valid_actions, ['cancel']); }); + + it('throws when `setup` is placed at the top level instead of inside account', () => { + assert.throws( + () => + updateMediaBuyResponse({ + media_buy_id: 'mb_1', + status: 'pending_approval', + setup: { url: 'https://example.com/sign', message: 'Review IO' }, + }), + /`setup` is not a field on the media buy.*belongs inside `account\.setup`/ + ); + }); }); describe('getMediaBuysResponse', () => { @@ -179,6 +219,23 @@ describe('getMediaBuysResponse', () => { const result = getMediaBuysResponse({ media_buys: [{ media_buy_id: 'mb_1' }] }); assert.strictEqual(result.content[0].text, 'Found 1 media buy'); }); + + it('throws when any media buy has `setup` at the top level', () => { + assert.throws( + () => + getMediaBuysResponse({ + media_buys: [ + { media_buy_id: 'mb_1' }, + { + media_buy_id: 'mb_2', + status: 'pending_approval', + setup: { url: 'https://example.com/sign', message: 'Review IO' }, + }, + ], + }), + /getMediaBuysResponse.*`setup` is not a field on the media buy/ + ); + }); }); describe('performanceFeedbackResponse', () => {