Skip to content

fix(compliance): thread brand through session-scoped storyboard steps (#2236)#2526

Merged
bokelley merged 1 commit into
mainfrom
bokelley/issue-2236
Apr 20, 2026
Merged

fix(compliance): thread brand through session-scoped storyboard steps (#2236)#2526
bokelley merged 1 commit into
mainfrom
bokelley/issue-2236

Conversation

@bokelley

@bokelley bokelley commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Three media_buy_seller scenarios had brand: { domain: "acmeoutdoor.example" } on the opening get_products / create_media_buy step but omitted it on the follow-up get_media_buys / update_media_buy / sync_creatives steps. That made each run land in open:default for the later calls while create_media_buy wrote to open:acmeoutdoor.example, so any seller scoping session state by brand (spec-required for multi-tenant isolation) correctly refused to return the buy — and the storyboard scored it as a failure.
  • Added brand: { domain: "acmeoutdoor.example" } to every session-scoped step across inventory_list_targeting, invalid_transitions, pending_creatives_to_start (9 steps, matching the already-passing inventory_list_no_match pattern).
  • Left sync_accounts / sync_governance / comply_test_controller steps alone — their handlers are global / test-control and don't derive from sessionKeyFromArgs.

Why

Closes the verification gap in #2236. Training-agent handlers for get_collection_list / update_collection_list / create_media_buy with targeting.collection_list are correct (see server/src/training-agent/inventory-governance-handlers.ts and task-handlers.ts:138-149). The remaining failures on live test-agent.adcontextprotocol.org were entirely driven by the storyboard harness asking the wrong session.

@adcp/client 5.2.0 added applyBrandInvariant as a runner-layer safety net (PR adcp-client#586), but it only fires when the caller sets options.brand — the CLI doesn't currently expose that flag, so each step's sample_request.brand is authoritative in the typical invocation. Fixing the YAMLs is the right layer until the CLI lands a passthrough.

Protocol review

ad-tech-protocol-expert confirmed per-request brand is spec-correct — AdCP has no session-login primitive, MCP is stateless per call, and relying on prior-step brand would break horizontal scaling. Task classification (which tasks need tenant scoping vs. which don't) matches sessionKeyFromArgs semantics in server/src/training-agent/state.ts:307-330.

Follow-ups (filed, non-blocking)

Close #2236 once the next @adcp/client release pulls this into the compliance cache and inventory_list_targeting scores green end-to-end.

Test plan

  • npm run build:compliance succeeds with the edited YAMLs.
  • node -e parses all three files via js-yaml; every session-scoped step's sample_request.brand.domain === "acmeoutdoor.example".
  • Precommit hook passed (587 tests, typecheck clean).
  • After release + cache sync, re-run npx @adcp/client storyboard run test-mcp media_buy_seller/inventory_list_targeting against test-agent.adcontextprotocol.org and confirm 5/5 steps pass.

🤖 Generated with Claude Code

…dia_buy_seller storyboards (#2236)

Sellers derive session keys from brand.domain per multi-tenant isolation. Three scenarios had brand on the opening get_products/create_media_buy step but omitted it on subsequent get/update/sync steps, landing those calls in a different session and failing against any brand-scoped seller. Fixed to match the already-passing inventory_list_no_match pattern. Filed adcp-client#637 for the latent CLI --file parsing bug that blocks end-to-end local verification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
…lations (#2527)

Every step that invokes a tenant-scoped task must carry brand/account identity in sample_request, so consecutive steps land in the same per-tenant session on brand-scoped sellers. Wired into npm run build:compliance — missing identity fails the build.

Fixes the 47 violations discovered by the lint (40 added brand, 7 marked scoping: global for negative-path probes) across 15 storyboards. Authoring convention documented in docs/contributing/storyboard-authoring.md.

Prevents the class of bug #2236 exposed, stacked on #2526.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
… brand, operator } (#2528)

Convert 36 top-level brand references to the strictly-more-expressive account-wrapper form across 24 buyer-side storyboards, and strip 32 redundant top-level brand blocks from steps that already had an account. Document the canonical shape in docs/accounts/overview.mdx.

No behavior change — both shapes resolve to the same session key on reference sellers. Consistency/clarity improvement only. Stacked on #2527 (lint) and #2526 (bug fix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
… brand, operator } (#2528)

Convert 36 top-level brand references to the strictly-more-expressive account-wrapper form across 24 buyer-side storyboards, and strip 32 redundant top-level brand blocks from steps that already had an account. Document the canonical shape in docs/accounts/overview.mdx.

No behavior change — both shapes resolve to the same session key on reference sellers. Consistency/clarity improvement only. Stacked on #2527 (lint) and #2526 (bug fix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit f5bb39c into main Apr 20, 2026
12 checks passed
bokelley added a commit that referenced this pull request Apr 20, 2026
… brand, operator } (#2528) (#2534)

* refactor(compliance): standardize buyer-side storyboards on account { brand, operator } (#2528)

Convert 36 top-level brand references to the strictly-more-expressive account-wrapper form across 24 buyer-side storyboards, and strip 32 redundant top-level brand blocks from steps that already had an account. Document the canonical shape in docs/accounts/overview.mdx.

No behavior change — both shapes resolve to the same session key on reference sellers. Consistency/clarity improvement only. Stacked on #2527 (lint) and #2526 (bug fix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(compliance): cover scope gaps flagged in PR #2534 review

Protocol-expert review of #2534 flagged two buyer-side storyboards that were excluded by the path-based scope rules but actually have caller.role: buyer_agent:

- protocols/governance/index.yaml — 6 buyer-side steps migrated to account { brand, operator } (get_products, check_governance denied/approved, create_media_buy, report_plan_outcome, get_plan_audit_logs). binding.account unchanged as reference target.
- specialisms/signal-marketplace/index.yaml — 5 buyer-side steps migrated (get_signals by-spec/by-ids/verify_provenance, activate_signal on platform and on agent).

Operator pinnacle-agency.com on both files (matches sibling buyer-side specialisms).

Also addresses protocol-expert narrative gap and code-reviewer taste nit on docs/accounts/overview.mdx: note the self-buying advertiser case (operator == brand.domain), and convert the absolute GitHub link to a Docusaurus-relative path for consistency.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
…#2532)

* feat(compliance): scoping lint + authoring guide, fix 47 existing violations (#2527)

Every step that invokes a tenant-scoped task must carry brand/account identity in sample_request, so consecutive steps land in the same per-tenant session on brand-scoped sellers. Wired into npm run build:compliance — missing identity fails the build.

Fixes the 47 violations discovered by the lint (40 added brand, 7 marked scoping: global for negative-path probes) across 15 storyboards. Authoring convention documented in docs/contributing/storyboard-authoring.md.

Prevents the class of bug #2236 exposed, stacked on #2526.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(storyboard-authoring): address docs-expert review

- Add account.operator as a required sibling to account.brand.domain per AccountRef schema
- Soften tenant-scoping framing from "spec-required" to "sellers that isolate tenants by brand..."
- Correct sync_accounts exemption rationale: identity in payload array, not envelope-circular
- Add "how to run locally" snippet with sample failure output

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(compliance): standardize buyer-side storyboards on account { brand, operator } (#2528) (#2534)

* refactor(compliance): standardize buyer-side storyboards on account { brand, operator } (#2528)

Convert 36 top-level brand references to the strictly-more-expressive account-wrapper form across 24 buyer-side storyboards, and strip 32 redundant top-level brand blocks from steps that already had an account. Document the canonical shape in docs/accounts/overview.mdx.

No behavior change — both shapes resolve to the same session key on reference sellers. Consistency/clarity improvement only. Stacked on #2527 (lint) and #2526 (bug fix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(compliance): cover scope gaps flagged in PR #2534 review

Protocol-expert review of #2534 flagged two buyer-side storyboards that were excluded by the path-based scope rules but actually have caller.role: buyer_agent:

- protocols/governance/index.yaml — 6 buyer-side steps migrated to account { brand, operator } (get_products, check_governance denied/approved, create_media_buy, report_plan_outcome, get_plan_audit_logs). binding.account unchanged as reference target.
- specialisms/signal-marketplace/index.yaml — 5 buyer-side steps migrated (get_signals by-spec/by-ids/verify_provenance, activate_signal on platform and on agent).

Operator pinnacle-agency.com on both files (matches sibling buyer-side specialisms).

Also addresses protocol-expert narrative gap and code-reviewer taste nit on docs/accounts/overview.mdx: note the self-buying advertiser case (operator == brand.domain), and convert the absolute GitHub link to a Docusaurus-relative path for consistency.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
…fallback (#2527, #2529, #2531)

Three guardrails against the class of bug #2236/#2526 exposed — steps that
silently write to open:default because sample_request lacks brand/account.

- #2527: scripts/lint-storyboard-scoping.cjs, wired into
  npm run build:compliance. TENANT_SCOPED_TASKS enumerates every tool that
  calls getSession(sessionKeyFromArgs(...)). Each step invoking one must
  carry sample_request.{account.account_id | account.brand.domain |
  brand.domain | plans[0].brand.domain (sync_plans)}. Per-step opt-out via
  `scoping: global` for intentionally cross-tenant probes. Fixed 26
  pre-existing violations across 10 storyboards.

- #2529: tests/lint-storyboard-scoping.test.cjs parity guard. Parses
  HANDLER_MAP from server/src/training-agent/task-handlers.ts and asserts
  every registered task is classified in exactly one of
  TENANT_SCOPED_TASKS / EXEMPT_FROM_LINT. Also catches stale entries in
  those sets after a rename. Added as npm run test:storyboard-scoping.

- #2531: sessionKeyFromArgs falls back to plans[0].brand.domain before
  open:default. sync_plans carries brand identity inside plans[], not at
  the envelope level — without this fallback, governance storyboards that
  only identify the tenant via plans land in the wrong session. Matches
  what the scoping lint already accepts as a valid identity shape.

- Authoring guide: docs/contributing/storyboard-authoring.md documents
  valid identity shapes, scoping: global opt-out, and local lint usage.

build:compliance, test:schemas, typecheck, and training-agent.test.ts
all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 20, 2026
…fallback (#2527, #2529, #2531) (#2563)

* refactor(compliance): normalize storyboard brand/account consistency (#2528, #2530, #2533)

Mechanical storyboard-consistency pass across static/compliance/source/:

- #2530: Normalize to RFC 2606 .example TLD. acmeoutdoor.com →
  acmeoutdoor.example, amsterdam-steakhouse.com → .example,
  pinnacle-agency.com → .example, and fix the acme-outdoor.example.com
  typo in creative-template and creative-generative.

- #2533: Fix operator = brand.domain in property-lists and
  collection-lists. These specialisms are meant to demonstrate an agency
  operating on behalf of a brand, not the brand operating directly.
  Switch to pinnacle-agency.example to match the rest of the buyer-side
  storyboards.

- #2528: Standardize buyer-side storyboards on account { brand, operator }.
  When a step already had an account block, drop the redundant top-level
  brand. Otherwise, promote top-level brand into the account wrapper with
  operator: "pinnacle-agency.example".

No runtime behavior change: both shapes resolve to the same session key
via sessionKeyFromArgs. Fresh build:compliance and test:schemas pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(compliance): address review — TLD sweep, list_accounts identity, create_media_buy brand (#2528, #2530)

Review findings from ad-tech-protocol-expert on #2562:

- #2530 follow-up: sales-broadcast-tv/index.yaml missed the initial TLD
  sweep. Normalize `novamotors.com → .example` (6 sites) and
  `videoamp.com → .example` (2 sites) so spec examples don't resolve to
  real third-party domains.

- sales-social/index.yaml: list_accounts had an injected
  `account { brand, operator }` block, but the list-accounts-request
  schema has no `account` field (listing all accounts for the
  authenticated caller). Drop the identity block; keep only the context
  wrapper. additionalProperties: true would silently accept it, but the
  semantics were misleading to spec readers.

- #2528 follow-up (H2): create-media-buy-request schema requires BOTH
  top-level `brand` and `account`. The initial standardization to
  `account { brand, operator }` dropped top-level brand as "redundant";
  it isn't — `brand` and `account` are distinct schema fields. Restore
  top-level `brand: { domain }` alongside the existing account block on
  20 create_media_buy steps so the requests validate against the
  published schema.

build:compliance and test:schemas pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(compliance): storyboard scoping lint + sessionKeyFromArgs plans fallback (#2527, #2529, #2531)

Three guardrails against the class of bug #2236/#2526 exposed — steps that
silently write to open:default because sample_request lacks brand/account.

- #2527: scripts/lint-storyboard-scoping.cjs, wired into
  npm run build:compliance. TENANT_SCOPED_TASKS enumerates every tool that
  calls getSession(sessionKeyFromArgs(...)). Each step invoking one must
  carry sample_request.{account.account_id | account.brand.domain |
  brand.domain | plans[0].brand.domain (sync_plans)}. Per-step opt-out via
  `scoping: global` for intentionally cross-tenant probes. Fixed 26
  pre-existing violations across 10 storyboards.

- #2529: tests/lint-storyboard-scoping.test.cjs parity guard. Parses
  HANDLER_MAP from server/src/training-agent/task-handlers.ts and asserts
  every registered task is classified in exactly one of
  TENANT_SCOPED_TASKS / EXEMPT_FROM_LINT. Also catches stale entries in
  those sets after a rename. Added as npm run test:storyboard-scoping.

- #2531: sessionKeyFromArgs falls back to plans[0].brand.domain before
  open:default. sync_plans carries brand identity inside plans[], not at
  the envelope level — without this fallback, governance storyboards that
  only identify the tenant via plans land in the wrong session. Matches
  what the scoping lint already accepts as a valid identity shape.

- Authoring guide: docs/contributing/storyboard-authoring.md documents
  valid identity shapes, scoping: global opt-out, and local lint usage.

build:compliance, test:schemas, typecheck, and training-agent.test.ts
all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(compliance): upgrade brand-only injections to account { brand, operator }

Follow-up self-review: the initial scoping-lint fix injected top-level
brand.domain into 47 steps. For create_media_buy/update_media_buy/
sync_creatives that is schema-invalid — AccountRef requires operator
whenever the natural-key (brand) form is used, so the canonical identity
shape is account { brand, operator }. Top-level brand on those tasks is
either a separate schema field (the campaign's brand on create_media_buy)
or not a schema field at all.

Changes:

- Upgrade existing brand-only injections to account { brand, operator }
  across 18 storyboards. Where a step already had an account block, drop
  the redundant brand. Keep top-level brand only where the storyboard
  legitimately expresses the campaign brand (separate from identity).

- Lint: also accept plans[0].account.brand.domain (canonical shape for
  sync_plans inside the plans array) in addition to plans[0].brand.domain.

- Runtime (sessionKeyFromArgs): mirror the lint — check
  plans[0].account.brand.domain before plans[0].brand.domain. New test
  coverage added.

- Authoring guide: stop documenting top-level brand as "shorthand
  identity." Document account { brand, operator } as canonical, with a
  clear note that top-level brand is a training-agent routing fallback,
  not a spec-sanctioned shape.

- Lint error message: recommend the account shape directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(compliance): sync_plans plan-level brand + harden parity test (review: H1, should-fix)

Review feedback from ad-tech-protocol-expert and code-reviewer on PR #2563:

H1 (blocking): sync-plans-request.json declares plan items with
`required: [plan_id, brand, ...]` and `additionalProperties: false`.
My amendment mis-wrapped plan identity in `account { brand, operator }`,
which AJV rejects. Revert to `brand: { domain }` at the plan-item level
across five storyboards (governance, governance-spend-authority × 2,
governance-delivery-monitor, brand-rights/governance_denied). Strip the
`plans[0].account.brand.domain` shape from the lint, the runtime
sessionKeyFromArgs fallback, tests, and the authoring guide so no-one
gets misled again.

Should-fix (parity test robustness):
- Strip `// comments` before matching handler-map entries so a `// TODO`
  beside a handler line doesn't silently drop the task from lint coverage.
- Drop the end-of-line anchor so an inline type annotation after the
  handler name doesn't fail the match.
- Add a canary: assert the parser captured ≥30 tasks. If the regex ever
  chops the body (e.g. future nested braces), the canary fires before
  the two downstream assertions pass vacuously.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(compliance): add top-level brand on PR B's create_media_buy injections (review: H2)

Review feedback from ad-tech-protocol-expert on #2563:

The scoping-lint fix added `account { brand, operator }` to 11 create_media_buy
steps that lacked any identity, but the create-media-buy-request schema
requires BOTH `brand` (top-level) and `account`. Adding only `account`
makes the probes fail on missing-brand validation before exercising the
error path they're meant to test (negative budget, reversed dates, etc.).

Add top-level `brand: { domain }` alongside the injected account on:
- universal/error-compliance.yaml (5 steps)
- universal/schema-validation.yaml (3 steps)
- protocols/media-buy/state-machine.yaml (1)
- protocols/governance/index.yaml (1)
- specialisms/governance-spend-authority/index.yaml (1)

The companion top-level-brand restoration for PR A's create_media_buy
steps landed in #2562.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ci): add SEO frontmatter to storyboard-authoring doc

check-seo CI requires description + og:title on every doc page.
Match the sibling testable-*.md frontmatter style.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Training agent: collection list CRUD is half-implemented (get/update/targeting fail)

1 participant