feat(testing): per-specialism storyboard routing — runStoryboard({ agents }) (#1066)

[bokelley]

Summary

Storyboards that span specialisms (e.g. signal_marketplace/governance_denied: sync_governance then activate_signal) can now route each step to the tenant that actually owns the tool, instead of fanning every call at one URL and relying on each tenant accepting "foreign" comply seeds. Closes #1066.

runStoryboard('', storyboard, {
  auth: { type: 'bearer', token: '...' },
  agents: {
    sales:      { url: 'https://test-agent.adcontextprotocol.org/sales/mcp' },
    signals:    { url: 'https://test-agent.adcontextprotocol.org/signals/mcp' },
    governance: { url: 'https://test-agent.adcontextprotocol.org/governance/mcp' },
  },
  default_agent: 'sales',
});

CLI:

adcp storyboard run --agents-map ./agents.yaml signal_marketplace/governance_denied

The 6-tenant prod test-agent (adcontextprotocol/adcp:server/src/training-agent/index.ts) is the canonical target: signals, sales, governance, creative, creative-builder, brand. Shared bearer auth across all six (per-tenant auth override is supported but not required).

How routing works

1. Explicit step.agent (escape hatch for cross-domain tools)
2. TASK_FEATURE_MAP[task] → first protocol → unique agent that claims it via get_adcp_capabilities
3. options.default_agent
4. Throw RoutingError (fails the step with a precise diagnostic)

Multi-claim conflicts (two agents claim the same protocol AND a step lacks agent: override) fail-fast at storyboard-build time, before any non-discovery network calls. Error names the conflicting agents and the affected step IDs.

Discovery runs in parallel across all agents at storyboard start. Any tenant's discovery failure is a HARD storyboard failure (not a per-step skip) — a broken tenant in a multi-tenant flow is a topology bug.

API additions

• StoryboardRunOptions.agents?: Record<string, AgentEntry>
• StoryboardRunOptions.default_agent?: string
• StoryboardStep.agent?: string (per-step override; YAML escape hatch)
• AgentEntry { url; auth?; transport? } — per-tenant overrides shadow run-level defaults
• StoryboardResult.agent_map?: Record<string, string> — echoes resolved key → url for JUnit/CI consumers
• Per-step agent_url/agent_index populated in routed mode (previously only in multi-instance replica mode)

CLI

• --agents-map <path> — YAML or JSON file with agents: (and optional top-level default_agent:)
• --agent <key>=<url> — repeatable, overrides file entries
• --default-agent <key> — fallback for unmapped tools
• URL validation matches --url: HTTPS in prod, HTTP only with --allow-http, no embedded credentials

Mutual exclusion

Three combinations throw at runStoryboard() entry, each with a specific error message:

• agents + multi_instance_strategy (replica round-robin is a different concept — replicas test horizontal scaling of one agent; routing dispatches per-tool across different agents)
• agents + _client (single client cannot serve multiple agents)
• agents + non-empty positional URL (ambiguous routing intent)

Empty agents map, missing url, default_agent not in map, and step.agent not in map are also caught at entry.

Tests

13 new tests in test/lib/agent-routing.test.js:

• 7 cover protocol-index build + conflict detection + per-step override + default-agent fallback (using buildRoutingContextFromProfiles test seam — no live agents)
• 6 cover the entry-guard error paths through runStoryboard()

Full lib suite: 5839 pass, 0 fail (no regressions).

Out of scope (filed as separate issues)

• Hello adapter for governance specialism #1332 — Hello adapter for governance specialism
• Hello adapter for creative specialism #1333 — Hello adapter for creative specialism
• Hello adapter for brand-rights specialism #1334 — Hello adapter for brand-rights specialism
• Hello cluster orchestrator: boot N specialism adapters with one command #1335 — Hello-cluster orchestrator (npm run hello-cluster) + manifest

The 2-agent integration smoke against the existing signals + sales adapters can land in this branch as a follow-up if reviewers prefer; today the routing logic is unit-tested with mocked profiles and the entry guards are exercised end-to-end through runStoryboard().

Test plan

• npm run build clean
• npm run format:check clean
• node --test test/lib/agent-routing.test.js — 13/13 pass
• npm run test:lib — 5839/5839 pass, no regressions
• Manual smoke against prod test-agent: adcp storyboard run --agents-map ./examples/prod-test-agent.agents.yaml signal_marketplace/governance_denied --auth $TRAINING_AGENT_TOKEN — pending committed manifest file (will land alongside Hello cluster orchestrator: boot N specialism adapters with one command #1335)
• Manual smoke against local hello-cluster — pending Hello adapter for governance specialism #1332/1333/1334/1335

🤖 Generated with Claude Code

[bokelley]

Expert review

Four reviewers in parallel — protocol, code, security, product. Three said ship; code-reviewer caught one real bug, now fixed.

Code review — fixed in 6631e0f

Concern: _profile and agentTools leaked through buildAgentOptions into per-agent discovery views. getOrDiscoverProfile short-circuits when options._profile is set, so every per-agent discovery would hand back comply()'s primary-tenant profile instead of probing the real tenant — silently breaking the protocol-claim index.

Fix: spread-clear _profile and agentTools alongside _client and agents in buildAgentOptions, with a comment pinning the rationale. New regression test in agent-routing.test.js exercises a routing context built with options._profile set to one tenant's profile and asserts routing still reflects each agent's distinct supported_protocols. 14/14 tests pass; format clean.

Other code-review items (primary-profile heuristic, unioned-tools gate, discovery error path, per-step failure rendering, closeConnections semantics, CLI parsing) all returned OK.

Protocol review — ship

• TASK_FEATURE_MAP first-protocol rule for comply_test_controller is correct because parseCapabilitiesResponse normalizes the compliance_testing capability block into supported_protocols before routing reads it.
• step.agent YAML override doesn't break spec-authored storyboards — the loader has no additionalProperties: false enforcement and the storyboard schema is comment-doc only.
• Two follow-ups noted, neither blocking: (a) inline comment at KNOWN_PROTOCOLS documenting the parser-side normalization, (b) confirm synthesized phases (controller seeding, request-signing) preserve step.agent overrides — they're injected after build-time conflict detection so the walk may miss them.

Security review — ship

• Per-tenant credential isolation is sound. MCP connection pool keys by URL + sha256(authToken) so two agents with the same URL but different bearers get separate connections. Promise.all discovery holds each per-agent options view closed over its own entry — no shared mutable state.
• yaml npm parse is safe-by-default (eemeli/yaml v2, not js-yaml's historical !!js/function RCE).
• WHATWG new URL() validation rejects embedded user:pass@; path-smuggled @ parses correctly.
• step.agent smuggling is bounded by the operator's per-tenant authorization in the agents map.
• One low-priority follow-up: discovery error messages from upstream agents are propagated verbatim into CI logs. A malicious 401 echoing Authorization: Bearer <token> would leak the caller's own bearer for that tenant — same trust scope as today's --auth, so it's parity not regression. Mitigation: regex-scrub Bearer <token> patterns in agent-routing.ts:608 and client.ts:205.

Product review — ship

• Per-specialism URL split matches the prod test-agent and a minority of real deployments (Disney/NBCU-style separation). Dominant production shape is one URL per brand/seat — but the routed runner falls through to "kitchen-sink" mode with zero adoption cost (don't pass agents).
• Per-tenant AgentEntry.auth matches the moment a buyer integrates against a publisher who fronts signals via a partner (LiveRamp creds ≠ pub creds). Worth shipping from day one.
• CLI ergonomics fit DSP/CI conventions. Nit: support ADCP_AGENTS_MAP env var for ephemeral CI creds — additive, can land later.
• Two follow-ups, both additive and non-blocking: (a) per-storyboard default_agent: field at YAML top level (schema change in adcontextprotocol/adcp), (b) discover-tenants primitive or .well-known/adcp-agents.json so buyers can enumerate publisher topology programmatically.

Disposition

Ready to merge. Five follow-ups identified across reviewers — none blocking. Will track separately:

1. Inline comment at KNOWN_PROTOCOLS documenting parser-side normalization (~1 line)
2. Confirm synthesized-phase routing (controller seeding + request-signing inject post-walk)
3. Bearer <token> scrub in propagated error messages (defense-in-depth)
4. ADCP_AGENTS_MAP env var support (CLI ergonomics)
5. Per-storyboard default_agent: schema field (adcontextprotocol/adcp issue)
6. .well-known/adcp-agents.json tenant-discovery primitive (adcontextprotocol/adcp issue)

[bokelley]

Acknowledged. Review synthesis received — four reviewers converge on ship, the _profile/agentTools leak is fixed in 6631e0fa, and the six follow-ups are clearly scoped (three in this repo, two in adcontextprotocol/adcp). No blocking items outstanding.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01GsEvmh7bbaTBFZSTZBSEhE

---

Generated by Claude Code

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
32421079	Triggered	Generic High Entropy Secret	206d4b0	test/lib/agent-routing.test.js	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

[bokelley]

Follow-ups landed

All six items from the expert review batched into one follow-up commit (e93e61d) on this branch, plus two upstream issues filed against adcontextprotocol/adcp.

In this PR (e93e61d)

1. KNOWN_PROTOCOLS comment — documents that compliance_testing arrives via parseCapabilitiesResponse normalization, not as a wire-level entry in supported_protocols. Defends against future refactors.
2. Controller-seeding guard — validateAgentsMap rejects agents + prerequisites.controller_seeding: true unless skip_controller_seeding: true is also set. Today's seeding loop dispatches against the first per-agent client, which is wrong for cross-specialism storyboards whose fixtures: block declares seeds owned by different tenants. Per-tenant seed dispatch is a follow-up worth its own issue.
3. scrubAuthSecrets helper — redacts Authorization: Bearer …, standalone Bearer …, and ?token=…/&token=… patterns from upstream discovery error messages before they propagate into StoryboardResult.error / CI logs / JUnit. Defense-in-depth (the leakable token is the caller's own bearer for that tenant — already in their CI environment).
4. ADCP_AGENTS_MAP env var — fallback for --agents-map flag; suits CI matrices that prefer env injection over file mounts. CLI flags override the env var when both are set.

Tests: +4 (controller-seeding guard, three bearer-scrub regex shapes). 18/18 in test/lib/agent-routing.test.js. Full lib suite still 5845/0.

Upstream issues filed against adcontextprotocol/adcp

5. Storyboard schema: per-storyboard default_agent field for multi-agent runner routing adcp#3894 — Storyboard schema: per-storyboard default_agent field. Lets storyboard authors declare topology intent in YAML instead of every CI invocation re-asserting default_agent: sales.
6. Tenant discovery: .well-known/adcp-agents.json for multi-agent topology enumeration adcp#3895 — .well-known/adcp-agents.json tenant-discovery primitive. Lets buyers and runners enumerate publisher topology with one fetch instead of out-of-band Notion pages.

Disposition

Ready to merge. Three reviewers said ship; the one finding (_profile/agentTools leak) and the four follow-up items are all addressed in-PR. The two upstream items are tracked separately and are additive — neither blocks this branch.

[bokelley]

Acknowledged — follow-ups noted. The four in-PR items (KNOWN_PROTOCOLS comment, controller-seeding guard, scrubAuthSecrets, ADCP_AGENTS_MAP env var) and the two upstream issues (#3894, #3895) are all accounted for. No further action from triage.

---

Generated by Claude Code

[bokelley]

A few comments from the perspective of locking #1335 (hello-cluster orchestrator) against this parser. Listed by priority — recommend ship after the bug fix and the two doc tightenings.

Real bug — string shorthand is unreachable

In parseAgentsMapArgs:

if (!entry || typeof entry !== 'object') {
  console.error(`ERROR: ${filePath} agents.${key} must be an object with at least \`url\``);
  process.exit(2);
}
const url = typeof entry === 'string' ? entry : entry.url;

The string-handler on the second line is dead code — strings fail the typeof !== 'object' check above with a misleading "must be an object" error. Either drop the shorthand handler (and document objects-only) or move validation after it. The PR description doesn't promise shorthand, so dropping the dead line is probably right.

Doc contradicts code — AgentEntry.auth optionality

JSDoc says:

"Per-agent auth is required from day one — production multi-tenant deployments almost always use per-tenant credentials."

…but auth?: is optional and the next sentence says it falls back to StoryboardRunOptions.auth. Pick one. I'd keep it optional (uniform bearer across the 6-tenant test-agent is the canonical example) and rewrite the JSDoc to "per-tenant override; falls back to run-level auth." Current wording will push adopters into setting auth redundantly on every entry.

Doc overstates fail-fast — multi-claim conflicts

PR description says multi-claim conflicts "fail-fast at storyboard-build time, before any non-discovery network calls." Per the documented routing order, step.agent overrides resolve conflicts per-step — so multi-claim only fails-fast for steps without an explicit override. Suggest tightening to "for affected steps lacking step.agent override." Important framing: it tells adopters step.agent is the disambiguator, not just an esoteric escape hatch.

Framing — step.agent is broader than "conflict resolver"

StoryboardStep.agent JSDoc says:

"Use only when a tool's specialism is claimed by multiple agents..."

This is the explicit-routing primitive for cross-domain workflows too — e.g. signal_marketplace/governance_denied needs agent: governance on sync_governance and agent: signals on activate_signal because those tools are owned by different specialisms, not because of a conflict. Current framing makes it sound like an unusual escape hatch when it's the canonical multi-tenant pattern.

Open questions worth answering before merge (won't block, but adopters will ask)

1. Webhook receiver in routed mode. If a step uses expect_webhook*, which tenant gets the receiver URL in its notification config? One shared, or per-tenant? validateAutoTunnelArgs is called in handleAgentsRoutedStoryboardRun so it's wired — just undocumented.
2. Discovery hard-fail brittleness. "Any tenant's discovery failure is a HARD storyboard failure" — defensible, but in CI with hello-cluster running 6 tenants, one flaky boot fails the whole run even for storyboards that don't touch that tenant. Worth an opt-in discovery_resilient: true mode or a tracking issue.
3. Capability-driven full assessment in routed mode. Explicitly not supported. Real adopter ask the moment someone has a multi-tenant prod stack and wants "run everything." Worth filing now so the gap is visible.
4. agent_map completeness. Does it include declared-but-unrouted-to agents? JUnit consumers will want the full topology, not just the touched subset.

Minor

• The PR description's example uses default_agent: 'sales' for a signal_marketplace/governance_denied storyboard that touches signals and governance but not sales. Decorative — pick a default_agent that's actually exercised, otherwise readers infer the wrong semantics.
• The PR mixes "per-tenant" (prose) with "per-specialism" (actual routing semantics — agents claim specialisms, not tenants). Same idea, but adopters who deploy one tenant per business unit (vs one per specialism) will get confused. Pick one term.

Concrete adopter perspective from #1335

I locked the hello-cluster's manifest shape against this parser end-to-end. The shape works: default_agent at top level, agents.<key>.{url, auth} with bearer, plus a top-level pending: [...] sibling that the parser ignores cleanly (it only reads parsed.agents). The orchestrator emits a manifest that round-trips through yaml.parse to the exact shape runStoryboard({ agents }) expects. Once this lands, npm run hello-cluster --silent > agents.yaml && adcp storyboard run --agents-map agents.yaml signal_marketplace/governance_denied will be the one-line cross-specialism smoke for adopters.

[bokelley]

GitGuardian false-positives addressed in 57958b3:

• Test fixtures (test/lib/agent-routing.test.js) — replaced realistic-looking placeholder tokens (sk_live_abc123def456, eyJhbGc.eyJzdWI.signature, abc123/xyz789) with obviously-fake FAKE_BEARER_PLACEHOLDER_VALUE / FAKE.JWT.PLACEHOLDER / FAKE_QUERY_PLACEHOLDER_*. The regex matches on shape, not entropy, so placeholders verify the same redaction patterns.
• Regex source (agent-routing.ts:scrubAuthSecrets) — annotated with ggignore inline comments. The flagged line is the redaction pattern itself; no secret is encoded.

No behavior change. 18/18 tests still pass; format clean. The rescan should clear all three findings.

[bokelley]

Thanks for the fix-up — FAKE_BEARER_PLACEHOLDER_VALUE / FAKE.JWT.PLACEHOLDER are clearly inert and the ggignore annotation on the regex source is the right call there. The rescan should be clean.

---

Generated by Claude Code

[bokelley]

Agreed — FAKE_BEARER_PLACEHOLDER_VALUE / FAKE.JWT.PLACEHOLDER are clearly inert test fixtures, and the ggignore annotation keeps the regex-source line from re-triggering on future scans. No further action needed here.

---

Generated by Claude Code

[bokelley]

Acknowledged — confirmed, no action needed on the placeholder strings. The ggignore annotation is the right call there.

---

Generated by Claude Code

[bokelley]

Thanks for confirming — noted, no further action on those items.

---

Generated by Claude Code

[bokelley]

Review actions landed (2d0c0c0)

Real bug — fixed

• CLI dead code: parseAgentsMapArgs had unreachable typeof entry === 'string' ? entry : entry.url shorthand — strings fail the prior typeof !== 'object' guard with a misleading error. Dropped to const { url } = entry since the PR doesn't promise shorthand.

Doc tightenings — fixed

• AgentEntry.auth JSDoc rewritten. Was "required from day one" (contradicting the optional auth?: field); now "per-tenant override" with explicit guidance that the prod test-agent's shared-bearer pattern is canonical (set run-level auth once, leave entries empty).
• StoryboardRunOptions.agents JSDoc + changeset updated to clarify multi-claim conflicts fail-fast for affected steps lacking step.agent overrides. Steps WITH overrides route per the override and never trip the conflict — the override is the per-step disambiguator.
• StoryboardStep.agent JSDoc reframed. Was "use only when conflict-resolution can't pick one" (esoteric escape hatch); now positioned as the canonical primitive for cross-specialism storyboards (a signal_marketplace/governance_denied storyboard needs explicit agent: on each step because the tools are owned by different specialisms — protocol-based routing resolves automatically when claimants are unique, but explicit agent: is the documentation-friendly form).

Open questions — answered or filed

Q4 — agent_map completeness: confirmed complete. agentMap is populated by walking every entry in Object.entries(options.agents), regardless of whether routing actually picks each agent. Declared-but-unrouted-to agents appear too. JUnit consumers see the full topology.

The other three filed as separate issues:

• Multi-agent routing: webhook receiver topology in routed mode (#1066 follow-up) #1366 — Webhook receiver topology in routed mode (one shared vs per-tenant; document explicitly)
• Multi-agent routing: opt-in resilient discovery for hello-cluster CI (#1066 follow-up) #1367 — Opt-in discovery_resilient: true mode for hello-cluster CI graceful degradation
• Capability-driven full assessment in multi-agent routed mode (#1066 follow-up) #1368 — Capability-driven full assessment in multi-agent routed mode

Hello-cluster confirmation

Confirmed the parser's manifest shape locks against the hello-cluster (#1335) end-to-end: default_agent at top level, agents.<key>.{url, auth} with shared bearer, plus the orchestrator's pending: [...] sibling key passes through cleanly because the parser only reads parsed.agents. Once this lands and the cluster orchestrator ships, npm run hello-cluster --silent > agents.yaml && adcp storyboard run --agents-map agents.yaml signal_marketplace/governance_denied is the one-line cross-specialism smoke for adopters.

18/18 tests pass; format clean.

[bokelley]

Thanks for the thorough summary. The CLI dead-code fix, JSDoc clarifications, and filed follow-ups (#1366, #1367, #1368) all look like the right calls — especially repositioning step.agent as the canonical primitive for cross-specialism storyboards rather than an escape hatch. 18/18 green + format clean; nothing blocking on my end.

---

Triaged by Claude Code. Session: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

---

Generated by Claude Code

[bokelley]

Triage note from #1366: the webhook receiver topology question you flagged in your own review was filed as a tracking issue. The fix scope — JSDoc on StoryboardRunOptions.webhook_receiver and a routing module header comment documenting shared-vs-per-tenant receiver semantics — lands squarely in this PR's diff. Consider folding before merge; a docs-only follow-up would just be a second reviewer pass for the same files.

---

Generated by Claude Code

[bokelley]

Triage note — fold candidate: Issue #1367 ("opt-in discovery_resilient: true on StoryboardRunOptions") was filed as a follow-up to open question #2 from your own PR review. It's non-breaking (optional flag, default false), touches the same StoryboardRunOptions type and discovery code this PR already modifies, and comes from the same author. Consider folding the ~30-line change into this branch before merge to keep the diff cohesive and avoid a narrow follow-up PR. If the scope would complicate review sign-off, the tracking issue (#1367) will resurface after merge.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01TtF7pmpGEw6W5KvbbFgVLz

---

Generated by Claude Code

[bokelley]

Folded in ceaf1a4. Two docs-only changes:

• StoryboardRunOptions.webhook_receiver JSDoc — new multi-agent topology paragraph: one shared receiver per run, delivery correlation by step-keyed URL path (/step/<step_id>/<op_id>), no per-tenant receiver needed or supported.
• agent-routing.ts module header — "Webhook receiver topology in routed mode" section explaining why a shared receiver is sufficient, what {{runner.webhook_base}} resolves to across agents, and the disambiguation escape hatch (distinct step IDs → distinct URL paths, free).

Format clean, no behavior change. Closes #1366.

---

Generated by Claude Code

[bokelley]

Acknowledged — docs-only, no behavior change. Noted that ceaf1a4 closes #1366.

---

Generated by Claude Code