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/brand-invariant-construct-account.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@adcp/client': patch
---

Fix `applyBrandInvariant` scoping for tools whose schema declares `account` but not top-level `brand` (e.g. `get_media_buys`, `get_media_buy_delivery`, `list_creatives`). The helper was only injecting top-level `brand` and merging into an existing `account`; when the request-builder produced no `account`, `adaptRequestForServerVersion` would strip the unrecognized top-level `brand` and the run-scoped brand was lost on the wire. Now the helper constructs an `account` (via `resolveAccount(options)`) when the request omits one, so session scoping survives for every schema shape. Non-object `account` values (`null`, arrays) are still passed through unchanged. (adcp-client#643)
39 changes: 29 additions & 10 deletions src/lib/testing/storyboard/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from './context';
import { runValidations, type ValidationContext } from './validations';
import { buildRequest, hasRequestBuilder } from './request-builder';
import { resolveBrand } from '../client';
import { resolveAccount, resolveBrand } from '../client';
import { isMutatingTask } from '../../utils/idempotency';
import {
PROBE_TASKS,
Expand Down Expand Up @@ -1107,15 +1107,24 @@ function evalContributesIf(expr: string | undefined, priorStepResults: Map<strin
* different brand, it lands in a different session and can't see state
* created by earlier steps.
*
* This helper runs after builder / sample_request resolution and overwrites
* any conflicting brand on the request with `options.brand`. When the
* request carries an `account` object, we also set `account.brand` so both
* addressing forms converge.
* This helper runs after builder / sample_request resolution and writes the
* run-scoped brand into both addressing forms:
*
* - Top-level `brand` — for tools whose schema declares it (e.g.
* `get_products`, `create_media_buy`, signal tools).
* - `account.brand` — for tools whose schema declares `account` but not
* top-level `brand` (e.g. `get_media_buys`, `get_media_buy_delivery`,
* `list_creatives`). When the incoming request has no `account`, we
* construct one from `resolveAccount(options)` so the scoping survives
* `adaptRequestForServerVersion`'s schema-aware field stripping.
*
* For any given tool, only one of the two addressing forms is declared in
* its schema; the other is stripped downstream. Setting both here lets the
* helper stay tool-agnostic.
*
* `AccountReference` is a union of `{account_id}` or `{brand, operator, sandbox?}`.
* Injecting `brand` into an `{account_id}`-branch account still passes schema
* validation (request schemas use `.passthrough()`) but is semantically
* redundant. No storyboard currently uses the `{account_id}` branch.
* validation but is semantically redundant.
*/
export function applyBrandInvariant(
request: Record<string, unknown>,
Expand All @@ -1127,9 +1136,19 @@ export function applyBrandInvariant(
if (!options.brand && !options.brand_manifest) return request;
const brand = resolveBrand(options);
const result: Record<string, unknown> = { ...request, brand };
const existingAccount = request.account;
if (existingAccount && typeof existingAccount === 'object' && !Array.isArray(existingAccount)) {
result.account = { ...(existingAccount as Record<string, unknown>), brand };
if ('account' in request) {
// Caller sent an account — merge brand in when it's a plain object.
// Leave non-object values (null, array) alone so intentionally malformed
// requests aren't silently "corrected."
const existingAccount = request.account;
if (existingAccount && typeof existingAccount === 'object' && !Array.isArray(existingAccount)) {
result.account = { ...(existingAccount as Record<string, unknown>), brand };
}
} else {
// No account on the request — construct one so tools whose schema
// declares `account` but not top-level `brand` (e.g. get_media_buys,
// list_creatives) still carry the run-scoped brand on the wire.
result.account = resolveAccount(options);
}
return result;
}
Expand Down
10 changes: 8 additions & 2 deletions test/lib/storyboard-brand-invariant.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,15 @@ describe('applyBrandInvariant', () => {
assert.strictEqual(result.account, account);
});

test('does not add account when the request has no account', () => {
test('constructs account when the request omits it, so tools whose schema declares account but not top-level brand still carry the run-scoped brand on the wire (get_media_buys, list_creatives, etc.)', () => {
const result = applyBrandInvariant({ list_id: 'pl-1' }, { brand: BRAND });
assert.strictEqual(result.account, undefined);
assert.deepStrictEqual(result.account, { brand: BRAND, operator: BRAND.domain, sandbox: undefined });
assert.deepStrictEqual(result.brand, BRAND);
});

test('passes sandbox through into the constructed account when options.sandbox is set', () => {
const result = applyBrandInvariant({ list_id: 'pl-1' }, { brand: BRAND, sandbox: true });
assert.deepStrictEqual(result.account, { brand: BRAND, operator: BRAND.domain, sandbox: true });
});

test('does not mutate the input request', () => {
Expand Down
Loading