Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/setup-url-guard.md
Original file line number Diff line number Diff line change
@@ -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<string, unknown>` so the strict type wasn't catching it. Affects `mediaBuyResponse`, `updateMediaBuyResponse`, and `getMediaBuysResponse`.
22 changes: 22 additions & 0 deletions src/lib/server/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@ export function toStructuredContent(data: object): Record<string, unknown> {
return data as unknown as Record<string, unknown>;
}

// `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<string, unknown>` 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.
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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: [
{
Expand Down
57 changes: 57 additions & 0 deletions test/server-responses.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -172,13 +200,42 @@ 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', () => {
it('returns media buy count in default summary', () => {
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', () => {
Expand Down
Loading