feat(compliance): storyboard scoping lint + sessionKeyFromArgs plans fallback (#2527, #2529, #2531)#2563
Merged
Merged
Conversation
…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>
Contributor
Schema Link Check ResultsCommit:
|
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…view: 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>
…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>
…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>
…d, 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>
…view: 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>
…tions (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>
bokelley
force-pushed
the
bokelley/storyboard-scoping-lint
branch
from
d058182 to
68a7297
Compare
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>
…coping-lint # Conflicts: # static/compliance/source/protocols/governance/index.yaml # static/compliance/source/protocols/media-buy/state-machine.yaml # static/compliance/source/specialisms/brand-rights/scenarios/governance_denied.yaml # static/compliance/source/specialisms/governance-spend-authority/index.yaml
This was referenced Apr 20, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three guardrails against the class of bug #2236/#2526 exposed — steps that silently write to
open:defaultbecausesample_requestlacks brand/account identity.Stacked on #2562 (mechanical storyboard cleanup). Merge after that.
#2527 — Scoping lint
scripts/lint-storyboard-scoping.cjs, wired intonpm run build:compliance.TENANT_SCOPED_TASKSenumerates every tool whose training-agent handler callsgetSession(sessionKeyFromArgs(...)). Each step invoking one must carry one of:sample_request.account.account_idsample_request.account.brand.domainsample_request.brand.domainsample_request.plans[0].brand.domain(sync_plans only)Per-step opt-out via
scoping: globalfor intentionally cross-tenant probes. Fixed 26 pre-existing violations across 10 storyboards by addingbrand: { domain }derived from each storyboard'sprerequisites.test_kit.#2529 — Handler-dispatch parity test
tests/lint-storyboard-scoping.test.cjsparsesHANDLER_MAPfromserver/src/training-agent/task-handlers.tsand asserts every registered task is classified in exactly one ofTENANT_SCOPED_TASKS/EXEMPT_FROM_LINT. Also catches stale entries after a rename. Added asnpm run test:storyboard-scopingand wired into the top-levelnpm run testchain.Prevents silent drift: without this, someone adds
get_new_thingto the dispatch table, forgets to classify it, and the lint never sees it.#2531 — sessionKeyFromArgs plans[0] fallback
sessionKeyFromArgsnow falls back toplans[0].brand.domainbeforeopen:default.sync_planscalls carry brand identity inside the plans array, not at the envelope level — without this fallback, governance storyboards that only identify the tenant viaplans[*].brand.domainland inopen:defaulteven though the lint accepts that shape as valid.Tests added for:
brandstill wins overplans[0].brand.domainopen:defaultAuthoring guide
docs/contributing/storyboard-authoring.mddocuments valid identity shapes, thescoping: globalopt-out, which tasks are tenant-scoped vs exempt, and how to run the lint locally.Closes
Test plan
npm run build:compliance→ scoping lint passes + clean dist buildnpm run test:schemas(7/7) andnpm run test:examples(31/31)npm run test:storyboard-scoping(3/3)npx vitest run server/tests/unit/training-agent.test.ts(326/326, including 4 new plans-fallback cases)npm run typecheckcleanStacking
PR A #2562 (mechanical cleanup) → this PR (lint + fallback).
🤖 Generated with Claude Code