diff --git a/.changeset/brand-manifest-create-update-media-buy.md b/.changeset/brand-manifest-create-update-media-buy.md new file mode 100644 index 000000000..ad61f3f01 --- /dev/null +++ b/.changeset/brand-manifest-create-update-media-buy.md @@ -0,0 +1,17 @@ +--- +"@adcp/client": patch +--- + +Fix v2/v3 backwards compatibility for create_media_buy, update_media_buy, and get_products + +**Inbound normalization (pre-strict-validation)** +- `brand_manifest` passed to `create_media_buy` is now converted to `brand` (BrandReference) and stripped before Zod strict validation fires, matching the existing `get_products` pattern. Previously these requests failed with "Request validation failed: Unrecognized key: brand_manifest". +- `update_media_buy` is no longer incorrectly included in the `brand_manifest` normalization block — neither the v2 nor v3 update schema has a `brand` field. + +**Outbound adaptation (v3 client → v2 server)** +- `adaptCreateMediaBuyRequestForV2` now converts `brand: { domain }` → `brand_manifest: 'https://'` before sending to v2 servers. Previously `brand` passed through unchanged and v2 servers rejected it as an unrecognised field. +- `adaptCreateMediaBuyRequestForV2` now preserves `brand` in the output when it cannot be converted (no `domain` present), consistent with `adaptGetProductsRequestForV2`. +- `adaptCreateMediaBuyRequestForV2` now throws a clear error when `proposal_id` is present with no packages — proposal mode is v3-only and v2 servers require an explicit `packages` array. +- `adaptGetProductsRequestForV2` now correctly strips the `account` field (was erroneously deleting `account_id`, a field that doesn't exist at the top level). +- `adaptPackageRequestForV2` now strips `catalog` from package items — it is a v3-only field not present in the v2 package schema. Applies to both `create_media_buy` and `update_media_buy` packages. +- Brand manifest URL format aligned: both `get_products` and `create_media_buy` now use the bare domain URL (`https://`) when converting `brand` → `brand_manifest` for v2 servers. diff --git a/package-lock.json b/package-lock.json index c534db6a0..493d90b3e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@adcp/client", - "version": "3.25.1", + "version": "4.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@adcp/client", - "version": "3.25.1", + "version": "4.0.0", "license": "MIT", "dependencies": { "better-sqlite3": "^12.4.1", @@ -5787,16 +5787,16 @@ } }, "node_modules/minimatch": { - "version": "10.2.1", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.1.tgz", - "integrity": "sha512-MClCe8IL5nRRmawL6ib/eT4oLyeKMGCghibcDWK+J0hh0Q8kqSdia6BvbRMVk6mPa6WqUa5uR2oxt6C5jd533A==", + "version": "10.2.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", + "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", "dev": true, "license": "BlueOak-1.0.0", "dependencies": { "brace-expansion": "^5.0.2" }, "engines": { - "node": "20 || >=22" + "node": "18 || 20 || >=22" }, "funding": { "url": "https://github.com/sponsors/isaacs" diff --git a/src/lib/core/SingleAgentClient.ts b/src/lib/core/SingleAgentClient.ts index f7048de4b..b7ccdb750 100644 --- a/src/lib/core/SingleAgentClient.ts +++ b/src/lib/core/SingleAgentClient.ts @@ -2014,12 +2014,31 @@ export class SingleAgentClient { * written against older schema versions keep working. */ private normalizeRequestParams(taskType: string, params: any): any { - if (taskType !== 'get_products' || !params) { + if (!params) { return params; } let normalized = { ...params }; + // Strip brand_manifest → brand for tasks whose schema has a brand field (get_products, create_media_buy). + // update_media_buy has no brand field in either v2 or v3 — excluded deliberately. + // brand takes precedence if both are supplied. + if (taskType === 'get_products' || taskType === 'create_media_buy') { + if (normalized.brand_manifest && !normalized.brand) { + const brand = brandManifestToBrandReference(normalized.brand_manifest); + if (brand) { + normalized.brand = brand; + } + } + delete normalized.brand_manifest; + } + + if (taskType !== 'get_products') { + return normalized; + } + + // get_products-specific normalization below + // Infer buying_mode from brief presence if not supplied if (!normalized.buying_mode) { normalized = { @@ -2028,16 +2047,6 @@ export class SingleAgentClient { }; } - // Convert legacy brand_manifest → brand (BrandReference) so strict validation passes. - // brand takes precedence if both are supplied. - if (normalized.brand_manifest && !normalized.brand) { - const brand = brandManifestToBrandReference(normalized.brand_manifest); - if (brand) { - normalized.brand = brand; - } - } - delete normalized.brand_manifest; - // Convert legacy product_selectors (v3 beta / v2 era) → catalog so strict validation passes. // catalog takes precedence if both are supplied. if (normalized.product_selectors && !normalized.catalog) { diff --git a/src/lib/utils/creative-adapter.ts b/src/lib/utils/creative-adapter.ts index aa64b2419..2e886e2b7 100644 --- a/src/lib/utils/creative-adapter.ts +++ b/src/lib/utils/creative-adapter.ts @@ -55,10 +55,10 @@ export interface PackageResponseV2 { /** * Adapt a v3-style package request for a v2 server. * Converts creative_assignments to creative_ids (dropping weight and placement_ids). - * Strips v3-only package fields (optimization_goals). + * Strips v3-only package fields (optimization_goals, catalog). */ export function adaptPackageRequestForV2(pkg: PackageRequestV3): PackageRequestV2 { - const { optimization_goals, ...rest } = pkg as any; + const { optimization_goals, catalog, ...rest } = pkg as any; if (!rest.creative_assignments) { return rest as PackageRequestV2; @@ -74,13 +74,32 @@ export function adaptPackageRequestForV2(pkg: PackageRequestV3): PackageRequestV /** * Adapt a create_media_buy request for a v2 server. - * Strips v3-only top-level fields and adapts packages. + * Strips v3-only top-level fields, converts brand → brand_manifest, and adapts packages. */ export function adaptCreateMediaBuyRequestForV2(request: any): any { - const { account, proposal_id, total_budget, artifact_webhook, ...rest } = request; + const { account, proposal_id, total_budget, artifact_webhook, brand, ...rest } = request; + + // Proposal mode is v3-only. If packages are also present we can still satisfy the request + // by dropping proposal_id/total_budget and using the explicit packages. + // Only throw when there are no packages — then there's nothing to send a v2 server. + if (proposal_id && !rest.packages?.length) { + throw new Error( + 'Proposal mode (proposal_id + total_budget) requires a v3 server. ' + + 'The connected server only supports AdCP v2. Provide an explicit packages array instead.' + ); + } + + // Convert v3 BrandReference → v2 brand_manifest URL (bare domain, consistent with get_products). + // normalizeRequestParams has already stripped any incoming brand_manifest and + // promoted it to brand, so brand is the canonical source here. + const brand_manifest = brand?.domain ? `https://${brand.domain}` : undefined; return { ...rest, + // If brand is present but has no domain, preserve it — consistent with adaptGetProductsRequestForV2 + // which also leaves brand on the object when it cannot convert it. + ...(brand && !brand_manifest && { brand }), + ...(brand_manifest !== undefined && { brand_manifest }), ...(rest.packages && { packages: rest.packages.map(adaptPackageRequestForV2) }), }; } diff --git a/src/lib/utils/pricing-adapter.ts b/src/lib/utils/pricing-adapter.ts index ec126815a..47401ab65 100644 --- a/src/lib/utils/pricing-adapter.ts +++ b/src/lib/utils/pricing-adapter.ts @@ -253,10 +253,8 @@ export function normalizeProductPricing(product: any): any { export function adaptGetProductsRequestForV2(request: any): any { const adapted: any = { ...request }; - // Convert v3 brand (BrandReference) → v2 brand_manifest (string URL). - // Use the base domain URL — v2 servers resolve brand info from there. - // Do not append /.well-known/brand.json; that path may not exist and - // would cause servers to return "brand_manifest must provide brand information". + // Convert v3 brand (BrandReference) → v2 brand_manifest (bare domain URL). + // v2 servers resolve brand info from the base domain URL. if (adapted.brand?.domain) { adapted.brand_manifest = `https://${adapted.brand.domain}`; delete adapted.brand; @@ -317,7 +315,7 @@ export function adaptGetProductsRequestForV2(request: any): any { delete adapted.buying_mode; delete adapted.buyer_campaign_ref; delete adapted.property_list; - delete adapted.account_id; + delete adapted.account; delete adapted.pagination; return adapted; diff --git a/test/lib/request-validation.test.js b/test/lib/request-validation.test.js index bf677c45e..831204d02 100644 --- a/test/lib/request-validation.test.js +++ b/test/lib/request-validation.test.js @@ -103,6 +103,107 @@ describe('SingleAgentClient Request Validation', () => { 'Should throw validation error for invalid create_media_buy request' ); }); + + test('should strip brand_manifest and convert to brand before strict validation', async () => { + const client = new AdCPClient([mockAgent]); + const agent = client.agent(mockAgent.id); + + // brand_manifest is a legacy v2.5 field — should be stripped before strict validation + await assert.doesNotReject(async () => { + try { + await agent.createMediaBuy({ + buyer_ref: 'buyer123', + account: { account_id: 'test-account' }, + packages: [], + brand_manifest: { name: 'Acme', url: 'https://acme.com/brand.json' }, + start_time: 'immediate', + end_time: '2025-12-31T23:59:59Z', + }); + } catch (err) { + if (err.message.includes('Request validation failed')) { + throw err; + } + } + }, 'brand_manifest should be stripped before strict validation, not cause a validation error'); + }); + + test('should verify brand_manifest is not forwarded as an object to the server', async () => { + // Mock A2AClient to capture the parameters actually sent to the server so we can + // verify normalization happened — not just that Zod didn't reject the request. + const capturedCalls = []; + const A2AClient = require('@a2a-js/sdk/client').A2AClient; + const originalFromCardUrl = A2AClient.fromCardUrl; + + A2AClient.fromCardUrl = async () => ({ + sendMessage: async payload => { + capturedCalls.push(payload.message.parts[0].data); + return { + jsonrpc: '2.0', + id: 'test-id', + result: { + kind: 'task', + id: 'task-123', + contextId: 'ctx-123', + status: { state: 'completed', timestamp: new Date().toISOString() }, + }, + }; + }, + }); + + try { + const client = new AdCPClient([mockAgent]); + const agent = client.agent(mockAgent.id); + + await agent.createMediaBuy({ + buyer_ref: 'buyer123', + account: { account_id: 'test-account' }, + packages: [], + brand_manifest: { name: 'Acme', url: 'https://acme.com/brand.json' }, + start_time: 'immediate', + end_time: '2025-12-31T23:59:59Z', + }); + } catch (err) { + assert.ok( + !err.message.includes('Request validation failed'), + `Validation should not reject brand_manifest: ${err.message}` + ); + } finally { + A2AClient.fromCardUrl = originalFromCardUrl; + } + + // If the request reached the server, verify brand_manifest was NOT forwarded as an object. + // (It should have been converted to either brand:{domain} for v3 or brand_manifest URL for v2.) + const mediaBuyCall = capturedCalls.find(d => d.skill === 'create_media_buy'); + if (mediaBuyCall) { + assert.ok( + typeof mediaBuyCall.parameters.brand_manifest !== 'object', + 'brand_manifest must not be forwarded as an object — it should be converted before sending' + ); + } + }); + + test('should prefer explicit brand over brand_manifest when both are supplied', async () => { + const client = new AdCPClient([mockAgent]); + const agent = client.agent(mockAgent.id); + + await assert.doesNotReject(async () => { + try { + await agent.createMediaBuy({ + buyer_ref: 'buyer123', + account: { account_id: 'test-account' }, + packages: [], + brand: { domain: 'example.com' }, + brand_manifest: { name: 'Acme', url: 'https://acme.com/brand.json' }, + start_time: 'immediate', + end_time: '2025-12-31T23:59:59Z', + }); + } catch (err) { + if (err.message.includes('Request validation failed')) { + throw err; + } + } + }, 'brand takes precedence; brand_manifest stripped without causing a validation error'); + }); }); // Note: AdCP v3 schemas have additionalProperties: true for extensibility diff --git a/test/lib/v3-compatibility.test.js b/test/lib/v3-compatibility.test.js index 46b1c213a..4d1502b2e 100644 --- a/test/lib/v3-compatibility.test.js +++ b/test/lib/v3-compatibility.test.js @@ -318,14 +318,28 @@ describe('Creative Assignment Adapter', () => { assert.strictEqual(result.optimization_goals, undefined); assert.deepStrictEqual(result.creative_ids, ['creative-1']); }); + + test('should strip catalog from package (v3-only field)', () => { + const v3Package = { + package_id: 'pkg-1', + budget: 5000, + catalog: { type: 'product', gtins: ['gtin-1', 'gtin-2'] }, + creative_assignments: [{ creative_id: 'creative-1' }], + }; + + const result = adaptPackageRequestForV2(v3Package); + + assert.strictEqual(result.catalog, undefined); + assert.strictEqual(result.budget, 5000); + assert.deepStrictEqual(result.creative_ids, ['creative-1']); + }); }); describe('adaptCreateMediaBuyRequestForV2', () => { - test('should strip v3-only top-level fields', () => { + test('should strip v3-only top-level fields and convert brand to brand_manifest', () => { const v3Request = { buyer_ref: 'buyer-1', account: { account_id: 'acc-1' }, - proposal_id: 'prop-1', total_budget: { amount: 10000, currency: 'USD' }, artifact_webhook: 'https://example.com/webhook', brand: { domain: 'example.com' }, @@ -335,11 +349,74 @@ describe('Creative Assignment Adapter', () => { const result = adaptCreateMediaBuyRequestForV2(v3Request); assert.strictEqual(result.account, undefined); - assert.strictEqual(result.proposal_id, undefined); assert.strictEqual(result.total_budget, undefined); assert.strictEqual(result.artifact_webhook, undefined); + assert.strictEqual(result.brand, undefined); assert.strictEqual(result.buyer_ref, 'buyer-1'); - assert.deepStrictEqual(result.brand, { domain: 'example.com' }); + // brand converted to v2 brand_manifest URL (bare domain) + assert.strictEqual(result.brand_manifest, 'https://example.com'); + }); + + test('should throw when proposal_id is present and no packages (v3-only feature, no fallback)', () => { + const v3Request = { + buyer_ref: 'buyer-1', + account: { account_id: 'acc-1' }, + proposal_id: 'prop-1', + total_budget: { amount: 10000, currency: 'USD' }, + brand: { domain: 'example.com' }, + // no packages — nothing for a v2 server to execute + }; + + assert.throws( + () => adaptCreateMediaBuyRequestForV2(v3Request), + err => err.message.includes('Proposal mode') && err.message.includes('v3 server'), + 'Should throw when proposal_id is present with no packages' + ); + }); + + test('should strip proposal_id and proceed when packages are also present', () => { + const v3Request = { + buyer_ref: 'buyer-1', + proposal_id: 'prop-1', + total_budget: { amount: 10000, currency: 'USD' }, + brand: { domain: 'example.com' }, + packages: [{ buyer_ref: 'pkg-1', product_id: 'prod-1', budget: 1000 }], + }; + + // Should NOT throw — packages provide a valid v2 fallback + const result = adaptCreateMediaBuyRequestForV2(v3Request); + + assert.strictEqual(result.proposal_id, undefined); + assert.strictEqual(result.total_budget, undefined); + assert.strictEqual(result.buyer_ref, 'buyer-1'); + assert.ok(result.packages?.length === 1); + }); + + test('should produce a valid v2 brand_manifest URL from brand with brand_id', () => { + const result = adaptCreateMediaBuyRequestForV2({ + buyer_ref: 'buyer-1', + brand: { domain: 'acme.com', brand_id: 'br_123' }, + packages: [], + }); + + // brand_id is v3-only metadata; v2 brand_manifest URL uses just the domain + assert.strictEqual(result.brand_manifest, 'https://acme.com'); + assert.strictEqual(result.brand, undefined); + }); + + test('should preserve brand when it has no domain (consistent with adaptGetProductsRequestForV2)', () => { + // If brand is present but has no domain we cannot produce a brand_manifest URL. + // Preserve brand in the output rather than silently dropping it, matching the + // behaviour of adaptGetProductsRequestForV2 which also leaves brand untouched + // when it cannot be converted. + const result = adaptCreateMediaBuyRequestForV2({ + buyer_ref: 'buyer-1', + brand: { brand_id: 'br_999' }, // no domain + packages: [], + }); + + assert.deepStrictEqual(result.brand, { brand_id: 'br_999' }); + assert.strictEqual(result.brand_manifest, undefined); }); }); @@ -356,6 +433,18 @@ describe('Creative Assignment Adapter', () => { assert.strictEqual(result.reporting_webhook, undefined); assert.strictEqual(result.media_buy_id, 'mb-1'); }); + + test('should not modify brand-related fields (neither v2 nor v3 update_media_buy schema has a brand field)', () => { + // The update_media_buy schema has no brand field in v2 or v3, so the adapter + // must not convert or strip any brand-related data — pass through unchanged. + const result = adaptUpdateMediaBuyRequestForV2({ + media_buy_id: 'mb-1', + brand: { domain: 'example.com' }, + }); + + assert.deepStrictEqual(result.brand, { domain: 'example.com' }); + assert.strictEqual(result.brand_manifest, undefined); + }); }); describe('normalizePackageResponse', () => {