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
11 changes: 11 additions & 0 deletions .changeset/brand-invariant-storyboard-runner.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@adcp/client': patch
---

Storyboard runner: enforce a brand/account invariant on every outgoing request.

Sellers that scope session state by brand (required for per-tenant isolation) derive a session key from `brand.domain`. Before this fix, a storyboard's `create_*` step could send one brand while the follow-up `get_*` / `update_*` / `delete_*` / `validate_*` step sent another — or omitted brand entirely and hit the default `test.example`. The list created in one session was then invisible to the lookup in a different session, surfacing as `NOT_FOUND` across `property_governance`, `collection_governance`, `media_buy_seller`, and any storyboard that exercises stateful CRUD.

The runner now merges `options.brand` into every request after builder / `sample_request` resolution, overriding any conflicting brand and filling in `account.brand` when the request carries an `account` object. A storyboard run now lands in one session regardless of per-tool authorship. Storyboards that don't configure a brand (e.g. `universal/security.yaml` probes) pass through unchanged.

Fixes #579.
8 changes: 6 additions & 2 deletions src/lib/testing/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import type { TestOptions, TestStepResult, AgentProfile, TaskResult, Logger } fr
import { TOOL_RESPONSE_SCHEMAS } from '../utils/response-schemas';
import { parseCapabilitiesResponse } from '../utils/capabilities';

const DEFAULT_BRAND_REF: BrandReference = { domain: 'test.example' };

/**
* Extract a principal identifier from TestOptions auth.
* For bearer auth this is the token; for basic auth this is the username;
Expand All @@ -38,9 +36,15 @@ export function resolveAuthPrincipal(options: TestOptions): string | undefined {
}
}

const DEFAULT_BRAND_REF: BrandReference = { domain: 'test.example' };

/**
* Resolve the brand reference to use for a test call.
* Prefers the new brand field, falls back to converting a legacy brand_manifest.
*
* The runner enforces a storyboard-run-scoped brand invariant on every
* outgoing request, so a missing brand yields a stable default that stays
* consistent across create/get/update/delete within a single run.
*/
export function resolveBrand(options: TestOptions): BrandReference {
return (
Expand Down
48 changes: 48 additions & 0 deletions src/lib/testing/storyboard/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { executeStoryboardTask } from './task-map';
import { extractContext, injectContext, applyContextOutputs, applyContextInputs } from './context';
import { runValidations, type ValidationContext } from './validations';
import { buildRequest, hasRequestBuilder } from './request-builder';
import { resolveBrand } from '../client';
import {
PROBE_TASKS,
probeProtectedResourceMetadata,
Expand Down Expand Up @@ -355,6 +356,13 @@ async function executeStep(
request = applyContextInputs(request, step.context_inputs, context);
}

// Brand/account is a storyboard-run-scoped invariant: every step in a run
// targets the same brand, so every outgoing request's brand context must
// match the options. Enforcing this here (after builder + sample_request)
// prevents session-key divergence across create/get/update/delete steps
// when individual builders or sample_request YAML omit brand.
request = applyBrandInvariant(request, options);

// Detect unresolved $context placeholders — a prior step likely failed
// and didn't produce the expected output. Skip rather than sending garbage.
const unresolvedVars = findUnresolvedContextVars(request);
Expand Down Expand Up @@ -660,6 +668,46 @@ function evalContributesIf(expr: string | undefined, priorStepResults: Map<strin
return !!prior?.passed && !prior.skipped;
}

// ────────────────────────────────────────────────────────────
// Brand/account invariant
// ────────────────────────────────────────────────────────────

/**
* Force every outgoing request onto the storyboard's brand context.
*
* Sellers that scope session state by brand (spec-required for multi-tenant
* isolation) derive a session key from `brand.domain` — or, when brand is
* absent, from `account.brand.domain`. If any step in a run targets a
* 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.
*
* `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.
*/
export function applyBrandInvariant(
request: Record<string, unknown>,
options: StoryboardRunOptions
): Record<string, unknown> {
// Only force the invariant when the caller has actually supplied a brand.
// Storyboards that don't exercise brand-scoped tools (e.g. security
// probes) legitimately run without one and should pass through unchanged.
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 };
}
return result;
}

// ────────────────────────────────────────────────────────────
// Helpers
// ────────────────────────────────────────────────────────────
Expand Down
178 changes: 178 additions & 0 deletions test/lib/storyboard-brand-invariant.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/**
* Storyboard runner: brand/account invariant.
*
* Issue #579 — sellers that scope session state by brand lost cross-step
* state when a create step sent `brand: acmeoutdoor.example` but a follow-up
* get/update/delete step either omitted brand or let it default to
* `test.example`. The runner now overrides brand on every outgoing request
* after builder / sample_request resolution so a storyboard run lands in one
* session, regardless of per-tool authorship.
*/

const { describe, test, it } = require('node:test');
const assert = require('node:assert');
const http = require('http');

const { applyBrandInvariant, runStoryboard } = require('../../dist/lib/testing/storyboard/runner.js');

const BRAND = { domain: 'acmeoutdoor.example' };

describe('applyBrandInvariant', () => {
test('injects brand when the request omits it', () => {
const result = applyBrandInvariant({ list_id: 'pl-1' }, { brand: BRAND });
assert.deepStrictEqual(result.brand, BRAND);
assert.strictEqual(result.list_id, 'pl-1');
});

test('overrides a conflicting brand so every step shares one session', () => {
const result = applyBrandInvariant({ brand: { domain: 'other.example' }, list_id: 'pl-1' }, { brand: BRAND });
assert.deepStrictEqual(result.brand, BRAND);
});

test('fills in account.brand when the request carries an account', () => {
const result = applyBrandInvariant(
{ account: { operator: 'acmeoutdoor.example' }, list_id: 'pl-1' },
{ brand: BRAND }
);
assert.deepStrictEqual(result.account, { operator: 'acmeoutdoor.example', brand: BRAND });
});

test('overrides a conflicting account.brand', () => {
const result = applyBrandInvariant(
{ account: { brand: { domain: 'other.example' }, operator: 'other.example' } },
{ brand: BRAND }
);
assert.deepStrictEqual(result.account.brand, BRAND);
});

test('passes through when no brand is configured (e.g. security probes)', () => {
const input = { list_id: 'pl-1' };
const result = applyBrandInvariant(input, {});
assert.strictEqual(result, input);
});

test('resolves brand from brand_manifest when brand is not set', () => {
const result = applyBrandInvariant(
{ list_id: 'pl-1' },
{ brand_manifest: { name: 'Acme', url: 'https://acmeoutdoor.example' } }
);
assert.deepStrictEqual(result.brand, { domain: 'acmeoutdoor.example' });
});

test('leaves non-object account values alone', () => {
const result = applyBrandInvariant({ account: null }, { brand: BRAND });
assert.strictEqual(result.account, null);
});

test('leaves array account values alone (malformed but possible)', () => {
const account = [];
const result = applyBrandInvariant({ account }, { brand: BRAND });
assert.strictEqual(result.account, account);
});

test('does not add account when the request has no account', () => {
const result = applyBrandInvariant({ list_id: 'pl-1' }, { brand: BRAND });
assert.strictEqual(result.account, undefined);
});

test('does not mutate the input request', () => {
const input = { account: { operator: 'x' }, list_id: 'pl-1' };
applyBrandInvariant(input, { brand: BRAND });
assert.deepStrictEqual(input, { account: { operator: 'x' }, list_id: 'pl-1' });
});
});

// ────────────────────────────────────────────────────────────
// Integration: runner call-site regression
// ────────────────────────────────────────────────────────────

/**
* Reproduces the wire-level signature of issue #579: three steps in a run
* that each try to set a different brand (via sample_request, via context,
* and via omission). Before the fix, the outgoing MCP `tools/call` would
* carry three different brand domains. After the fix, all three must
* converge on `options.brand`. This catches regressions that move or drop
* the `applyBrandInvariant` call in `executeStep` even when the helper
* itself still behaves correctly.
*/
describe('runStoryboard: brand invariant on the wire', () => {
it('sends options.brand on every step regardless of sample_request authorship', async () => {
const seen = [];
const server = http.createServer(async (req, res) => {
const chunks = [];
for await (const c of req) chunks.push(c);
const rpc = JSON.parse(Buffer.concat(chunks).toString('utf8'));
seen.push({ name: rpc.params.name, args: rpc.params.arguments });
res.writeHead(401, { 'content-type': 'application/json', 'www-authenticate': 'Bearer realm="x"' });
res.end('{}');
});
await new Promise(r => server.listen(0, r));
const agentUrl = `http://127.0.0.1:${server.address().port}/mcp`;
try {
const storyboard = {
id: 'brand_invariant_sb',
version: '1.0.0',
title: 'Brand invariant',
category: 'compliance',
summary: '',
narrative: '',
agent: { interaction_model: '*', capabilities: [] },
caller: { role: 'buyer_agent' },
phases: [
{
id: 'p',
title: 'crud',
steps: [
{
id: 's1_sample_with_different_brand',
title: 'sample_request carries a different brand',
task: 'list_creatives',
auth: 'none',
expect_error: true,
sample_request: {
account: { brand: { domain: 'other.example' }, operator: 'other.example' },
},
validations: [{ check: 'http_status_in', allowed_values: [401], description: '' }],
},
{
id: 's2_sample_omits_brand',
title: 'sample_request omits brand entirely',
task: 'list_creatives',
auth: 'none',
expect_error: true,
sample_request: { list_id: 'pl-1' },
validations: [{ check: 'http_status_in', allowed_values: [401], description: '' }],
},
{
id: 's3_builder_path',
title: 'builder constructs account from options',
task: 'list_creatives',
auth: 'none',
expect_error: true,
validations: [{ check: 'http_status_in', allowed_values: [401], description: '' }],
},
],
},
],
};
await runStoryboard(agentUrl, storyboard, {
protocol: 'mcp',
allow_http: true,
brand: BRAND,
agentTools: ['list_creatives'],
_profile: { name: 'Test', tools: ['list_creatives'] },
_client: { getAgentInfo: async () => ({ name: 'Test', tools: [{ name: 'list_creatives' }] }) },
});

assert.strictEqual(seen.length, 3, `expected 3 tool calls, got ${seen.length}`);
for (const call of seen) {
assert.deepStrictEqual(call.args.brand, BRAND, `step ${call.name} brand diverged`);
if (call.args.account && typeof call.args.account === 'object' && !Array.isArray(call.args.account)) {
assert.deepStrictEqual(call.args.account.brand, BRAND, 'account.brand diverged');
}
}
} finally {
server.close();
}
});
});
Loading