From 1ca73d4a37295edaeb3d5569d3e075690e9ab44b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 19 Apr 2026 06:36:35 -0400 Subject: [PATCH] feat: --multi-instance storyboard mode Round-robin storyboard steps across N seller URLs to catch horizontal-scaling persistence bugs where (brand, account)-scoped state lives in-process rather than in a shared store. Implements the client-side half of the cross-instance persistence requirement introduced in adcontextprotocol/adcp#2363. - CLI: --url repeatable, positional agent mutually exclusive - Library: runStoryboard(agentUrls: string | string[], ...) - Failure attribution: replica map + NOT_FOUND signature + docs deep-link - --stateful filter on storyboard list - docs/examples/multi-instance/ reference harness - URL validation: reject javascript:/file://, reject userinfo, require --allow-http for http Closes adcontextprotocol/adcp#2267. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../multi-instance-storyboard-runner.md | 28 ++ bin/adcp.js | 334 ++++++++++++- docs/examples/multi-instance/.env.example | 9 + docs/examples/multi-instance/Caddyfile | 24 + docs/examples/multi-instance/README.md | 73 +++ .../multi-instance/docker-compose.yml | 102 ++++ docs/guides/MULTI-INSTANCE-TESTING.md | 109 +++++ src/lib/testing/storyboard/runner.ts | 211 ++++++++- src/lib/testing/storyboard/types.ts | 15 + test/lib/storyboard-multi-instance.test.js | 448 ++++++++++++++++++ 10 files changed, 1327 insertions(+), 26 deletions(-) create mode 100644 .changeset/multi-instance-storyboard-runner.md create mode 100644 docs/examples/multi-instance/.env.example create mode 100644 docs/examples/multi-instance/Caddyfile create mode 100644 docs/examples/multi-instance/README.md create mode 100644 docs/examples/multi-instance/docker-compose.yml create mode 100644 docs/guides/MULTI-INSTANCE-TESTING.md create mode 100644 test/lib/storyboard-multi-instance.test.js diff --git a/.changeset/multi-instance-storyboard-runner.md b/.changeset/multi-instance-storyboard-runner.md new file mode 100644 index 000000000..861021c6f --- /dev/null +++ b/.changeset/multi-instance-storyboard-runner.md @@ -0,0 +1,28 @@ +--- +'@adcp/client': minor +--- + +Storyboard runner: `--multi-instance` mode to catch horizontal-scaling persistence bugs. + +A seller deployed behind a load balancer with in-memory state passes every storyboard against a single URL but breaks in production when a follow-up step lands on a different machine. Single-URL runs never exercise this. `runStoryboard` now accepts an array of agent URLs and round-robins steps across them — writes on instance A must be visible on instance B or the read fails, and the runner attributes the failure with an instance→step map and a `write on [#A] → read on [#B] → NOT_FOUND` signature line matching the canonical horizontal-scaling bug. + +CLI: + +``` +npx @adcp/client storyboard run \ + --url https://a.your-agent.example/mcp/ \ + --url https://b.your-agent.example/mcp/ \ + account_and_audience \ + --auth $TOKEN +``` + +- Repeated `--url` engages multi-instance mode (minimum 2). Positional agent is disallowed in this mode — single-URL runs still use the positional shorthand. +- JSON output gains `agent_urls[]` and `multi_instance_strategy` on the result, and `agent_url` + `agent_index` on each step. +- `--dry-run` prints the per-step instance assignment plan. +- Full capability-driven assessment (no storyboard ID) is not yet multi-instance aware; use a specific storyboard or bundle ID. + +Error output mirrors the canonical failure example in the protocol docs (`create on replica [#1] … succeeded. read on replica [#2] … failed with NOT_FOUND. → Brand-scoped state is not shared across replicas.`) so developers pattern-match the page they'll click through to. Deep-links to [Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state). + +See `docs/guides/MULTI-INSTANCE-TESTING.md` for the full contract, including why the test asserts `(brand, account)`-keyed state, when false failures can occur, and how this fits alongside verify-by-architecture and verify-by-own-testing approaches. + +Implements the client-side half of the cross-instance persistence requirement introduced in [adcontextprotocol/adcp#2363](https://github.com/adcontextprotocol/adcp/pull/2363). Closes [adcontextprotocol/adcp#2267](https://github.com/adcontextprotocol/adcp/issues/2267). diff --git a/bin/adcp.js b/bin/adcp.js index b784e2b96..9e7696b67 100755 --- a/bin/adcp.js +++ b/bin/adcp.js @@ -828,6 +828,11 @@ EXAMPLES: async function handleStoryboardList(args) { const { listBundles, loadBundleStoryboards } = await import('../dist/lib/testing/storyboard/index.js'); const jsonOutput = args.includes('--json'); + // --stateful: keep only storyboards that contain at least one step marked + // `stateful: true`. That's the same predicate the multi-instance runner + // uses to identify storyboards whose write→read chains surface in-process + // state bugs, so this filter returns the "worth round-robining" set. + const statefulOnly = args.includes('--stateful'); let bundles; try { @@ -842,30 +847,38 @@ async function handleStoryboardList(args) { for (const ref of bundles) { const storyboards = loadBundleStoryboards(ref); if (storyboards.length === 0) continue; // skip schema/fixture YAMLs that aren't runnable - const summary = { - bundle_kind: ref.kind, - bundle_id: ref.id, - storyboards: storyboards.map(s => ({ - id: s.id, - title: s.title, - category: s.category, - summary: s.summary, - track: s.track, - step_count: s.phases.reduce((sum, p) => sum + p.steps.length, 0), - })), - }; + const summaryStoryboards = storyboards + .map(s => { + const allSteps = s.phases.flatMap(p => p.steps); + const statefulStepCount = allSteps.filter(step => step.stateful === true).length; + return { + id: s.id, + title: s.title, + category: s.category, + summary: s.summary, + track: s.track, + step_count: allSteps.length, + stateful_step_count: statefulStepCount, + }; + }) + .filter(s => !statefulOnly || s.stateful_step_count > 0); + if (summaryStoryboards.length === 0) continue; + const summary = { bundle_kind: ref.kind, bundle_id: ref.id, storyboards: summaryStoryboards }; grouped[ref.kind].push(summary); - for (const sb of summary.storyboards) { + for (const sb of summaryStoryboards) { flat.push({ ...sb, bundle_kind: ref.kind, bundle_id: ref.id }); } } if (jsonOutput) { - await writeJsonOutput({ bundles: grouped, storyboards: flat }); + await writeJsonOutput({ bundles: grouped, storyboards: flat, filter: statefulOnly ? 'stateful' : null }); return; } - console.log('\nCompliance storyboards (from local cache)\n'); + const heading = statefulOnly + ? 'Stateful compliance storyboards (1+ step marked stateful: true)' + : 'Compliance storyboards (from local cache)'; + console.log(`\n${heading}\n`); for (const kind of ['universal', 'protocol', 'specialism']) { if (grouped[kind].length === 0) continue; const header = @@ -874,13 +887,17 @@ async function handleStoryboardList(args) { for (const bundle of grouped[kind]) { console.log(` [${bundle.bundle_id}]`); for (const sb of bundle.storyboards) { - console.log(` ${sb.id} — ${sb.title} (${sb.step_count} steps)`); + const statefulSuffix = statefulOnly || sb.stateful_step_count > 0 ? `, ${sb.stateful_step_count} stateful` : ''; + console.log(` ${sb.id} — ${sb.title} (${sb.step_count} steps${statefulSuffix})`); if (sb.track) console.log(` Track: ${sb.track}`); } } console.log(); } - console.log(`${flat.length} storyboard(s) across ${bundles.length} bundle(s).`); + const suffix = statefulOnly ? ' with at least one stateful step' : ''; + console.log( + `${flat.length} storyboard(s)${suffix} across ${grouped.universal.length + grouped.protocol.length + grouped.specialism.length} bundle(s).` + ); } async function handleStoryboardShow(args) { @@ -961,6 +978,14 @@ async function handleStoryboardRun(args) { const opts = parseAgentOptions(args); const { authToken, protocolFlag, jsonOutput, debug, dryRun, positionalArgs } = opts; + // Multi-instance mode: repeated --url flags round-robin steps across N + // seller URLs. Must share a backing store to pass — catches horizontal + // scaling bugs where brand-scoped state lives in-process. + const extraUrls = extractRepeatedUrlFlags(args); + if (extraUrls.length > 0) { + return handleMultiInstanceStoryboardRun(args, opts, extraUrls); + } + const agentArg = positionalArgs[0]; const storyboardId = positionalArgs[1]; @@ -970,6 +995,7 @@ async function handleStoryboardRun(args) { if (!agentArg) { console.error('Usage: adcp storyboard run [storyboard_id|--file path] [options]'); + console.error(' Multi-instance: adcp storyboard run --url --url '); process.exit(2); } @@ -1094,6 +1120,280 @@ async function handleStoryboardRun(args) { process.exit(result.overall_passed ? 0 : 3); } +/** + * Extract every `--url ` occurrence from the CLI args and validate + * each against the same policy the single-instance agent argument enforces: + * + * - value must be a parseable URL + * - scheme must be http or https (http only allowed with --allow-http) + * - no userinfo (`https://user:pass@host/` is rejected — tokens go via --auth) + * + * Without these gates a hostile or typo'd `--url file:///etc/passwd` or + * `--url https://attacker@victim/mcp` would flow straight into the MCP + * transport and land in attribution output. + */ +function extractRepeatedUrlFlags(args) { + const allowHttp = args.includes('--allow-http'); + const values = []; + for (let i = 0; i < args.length; i++) { + if (args[i] !== '--url') continue; + const raw = args[i + 1]; + if (raw === undefined || raw.startsWith('--')) { + console.error('ERROR: --url requires a value (URL)'); + process.exit(2); + } + let parsed; + try { + parsed = new URL(raw); + } catch { + console.error(`ERROR: --url value is not a valid URL: ${raw}`); + process.exit(2); + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + console.error(`ERROR: --url must be http(s); got ${parsed.protocol} in "${raw}"`); + process.exit(2); + } + if (parsed.protocol === 'http:' && !allowHttp) { + console.error(`ERROR: --url with http:// requires --allow-http (got "${raw}")`); + process.exit(2); + } + if (parsed.username || parsed.password) { + console.error('ERROR: --url must not contain credentials (user:pass@). Pass tokens via --auth.'); + process.exit(2); + } + values.push(raw); + } + return values; +} + +/** + * Run a storyboard (or bundle) in multi-instance mode. Each step round-robins + * across the supplied URLs. Positional agent is disallowed — --url is the + * multi-instance path; positional is the single-instance path. + * + * Full capability-driven assessment (no storyboard ID) is intentionally not + * supported here: the compliance pipeline shares a single client across + * storyboards for connection reuse, which is incompatible with per-step URL + * dispatch. Use a specific storyboard or bundle ID. + */ +async function handleMultiInstanceStoryboardRun(args, opts, urls) { + const { authToken, protocolFlag, jsonOutput, dryRun, positionalArgs } = opts; + + if (urls.length < 2) { + console.error('ERROR: Multi-instance mode requires 2+ --url flags. Drop --url for single-instance.'); + process.exit(2); + } + + // Strip --url values that may have slipped past parseAgentOptions' positional filter. + const cleanPositional = positionalArgs.filter(p => !urls.includes(p)); + const firstPositional = cleanPositional[0]; + + if (firstPositional && (firstPositional.startsWith('http://') || firstPositional.startsWith('https://'))) { + console.error( + 'ERROR: Cannot combine a positional agent URL with --url flags. ' + + 'Use either a positional agent (single-instance) or repeated --url flags (multi-instance).' + ); + process.exit(2); + } + + const fileIndex = args.indexOf('--file'); + const filePath = fileIndex !== -1 ? args[fileIndex + 1] : null; + const storyboardId = firstPositional; + + if (filePath && storyboardId) { + console.error('ERROR: Cannot combine a storyboard ID with --file. Use one or the other.'); + process.exit(2); + } + + if (!filePath && !storyboardId) { + console.error( + 'ERROR: Multi-instance mode requires a storyboard ID, bundle ID, or --file. ' + + 'Capability-driven full assessment is not yet multi-instance aware.' + ); + process.exit(2); + } + + const { loadStoryboardFile, runStoryboard, getComplianceStoryboardById, loadBundleStoryboards, findBundleById } = + await import('../dist/lib/testing/storyboard/index.js'); + + // Load one or more storyboards (bundle IDs expand). + const storyboards = []; + if (filePath) { + try { + storyboards.push(loadStoryboardFile(filePath)); + } catch (err) { + console.error(`Failed to load storyboard from ${filePath}: ${err.message}`); + process.exit(2); + } + } else { + const bundle = findBundleById(storyboardId); + if (bundle) { + const bundleStoryboards = loadBundleStoryboards(storyboardId); + if (!bundleStoryboards || bundleStoryboards.length === 0) { + console.error(`ERROR: Bundle "${storyboardId}" is empty.`); + process.exit(2); + } + storyboards.push(...bundleStoryboards); + } else { + const sb = getComplianceStoryboardById(storyboardId); + if (!sb) { + console.error( + `ERROR: Unknown storyboard or bundle ID: ${storyboardId}\n` + `Run 'adcp storyboard list' to see all options.` + ); + process.exit(2); + } + storyboards.push(sb); + } + } + + // Auto-detect protocol from the first URL. Multi-instance deployments + // share a codebase across replicas, so one probe is representative. + let protocol = protocolFlag; + if (!protocol) { + if (!jsonOutput) console.error('Auto-detecting protocol from first URL...'); + try { + protocol = await detectProtocol(urls[0]); + if (!jsonOutput) console.error(`Detected protocol: ${protocol.toUpperCase()}\n`); + } catch (err) { + console.error(`ERROR: Failed to detect protocol: ${err.message}`); + process.exit(1); + } + } + + const totalSteps = storyboards.reduce( + (sum, sb) => sum + sb.phases.reduce((phaseSum, p) => phaseSum + p.steps.length, 0), + 0 + ); + + if (!jsonOutput) { + console.error(`Multi-instance storyboard run (round-robin across ${urls.length} instances):`); + urls.forEach((u, i) => console.error(` [#${i + 1}] ${u}`)); + console.error(` Protocol: ${protocol.toUpperCase()}`); + console.error(` Storyboards: ${storyboards.map(s => s.id).join(', ')}`); + console.error(` Total steps: ${totalSteps}\n`); + } + + // --dry-run: print the assignment plan and exit + if (dryRun) { + if (jsonOutput) { + await writeJsonOutput({ + agent_urls: urls, + multi_instance_strategy: 'round-robin', + protocol, + preview: true, + storyboards: storyboards.map(sb => { + const flat = sb.phases.flatMap(p => p.steps); + return { + storyboard_id: sb.id, + storyboard_title: sb.title, + assignments: flat.map((s, idx) => ({ + step_id: s.id, + task: s.task, + instance_index: (idx % urls.length) + 1, + agent_url: urls[idx % urls.length], + })), + }; + }), + }); + } else { + for (const sb of storyboards) { + console.log(`\n${sb.title} (${sb.id})`); + console.log('═'.repeat(50)); + let stepIdx = 0; + for (const phase of sb.phases) { + console.log(`\n── Phase: ${phase.title} ──`); + for (const step of phase.steps) { + const inst = (stepIdx % urls.length) + 1; + console.log(` [#${inst}] ${step.id}: ${step.title} → ${step.task}`); + stepIdx++; + } + } + } + console.log( + `\n${totalSteps} step(s) would be executed across ${urls.length} instances. Use without --dry-run to run.` + ); + } + return; + } + + const runOptions = { + protocol, + ...(authToken ? { auth: { type: 'bearer', token: authToken } } : {}), + ...(opts.allowHttp && { allow_http: true }), + }; + + const restoreLogs = jsonOutput ? captureStdoutLogs() : null; + const results = []; + let hadFailure = false; + try { + for (const sb of storyboards) { + const result = await runStoryboard(urls, sb, runOptions); + results.push(result); + if (!result.overall_passed) hadFailure = true; + } + } finally { + if (restoreLogs) restoreLogs(); + } + + if (jsonOutput) { + await writeJsonOutput( + results.length === 1 + ? results[0] + : { + agent_urls: urls, + multi_instance_strategy: 'round-robin', + storyboards: results, + overall_passed: !hadFailure, + } + ); + } else { + const SKIP_ICONS = { missing_test_harness: '🔧', not_testable: '⏭️', dependency_failed: '⏭️' }; + const SKIP_LABELS = { + missing_test_harness: ' [needs test harness]', + not_testable: ' [not testable]', + dependency_failed: ' [dependency failed]', + }; + for (const result of results) { + console.log(`\n${result.storyboard_title} (${result.storyboard_id})`); + console.log('═'.repeat(50)); + for (const phase of result.phases) { + console.log(`\n── Phase: ${phase.phase_title} ──────────────────────────────`); + for (const step of phase.steps) { + const icon = step.skipped ? (SKIP_ICONS[step.skip_reason] ?? '⏭️') : step.passed ? '✅' : '❌'; + const skipLabel = SKIP_LABELS[step.skip_reason] ?? ''; + const instTag = step.agent_index ? `[#${step.agent_index}] ` : ''; + console.log(`\n${icon} ${instTag}${step.title}${skipLabel} (${step.duration_ms}ms)`); + console.log(` Task: ${step.task}`); + if (step.error) { + console.log(` Error: ${step.error}`); + } + for (const v of step.validations) { + const vIcon = v.passed ? '✅' : '❌'; + console.log(` ${vIcon} ${v.description}`); + if (v.error) console.log(` ${v.error}`); + } + } + } + } + console.log(`\n${'─'.repeat(50)}`); + const totals = results.reduce( + (acc, r) => ({ + passed: acc.passed + r.passed_count, + failed: acc.failed + r.failed_count, + skipped: acc.skipped + r.skipped_count, + duration: acc.duration + r.total_duration_ms, + }), + { passed: 0, failed: 0, skipped: 0, duration: 0 } + ); + const overallIcon = !hadFailure ? '✅' : '❌'; + console.log( + `${overallIcon} ${totals.passed} passed, ${totals.failed} failed, ${totals.skipped} skipped (${totals.duration}ms) across ${urls.length} instances` + ); + } + + process.exit(hadFailure ? 3 : 0); +} + // Shared implementation: run all matching storyboards against an agent async function runFullAssessment(agentArg, rawArgs, parsedOpts) { const opts = parsedOpts || parseAgentOptions(rawArgs); diff --git a/docs/examples/multi-instance/.env.example b/docs/examples/multi-instance/.env.example new file mode 100644 index 000000000..3127d47db --- /dev/null +++ b/docs/examples/multi-instance/.env.example @@ -0,0 +1,9 @@ +# Multi-instance harness environment. +# Copy this file to `.env` and replace every placeholder. +# Both variables are required — `docker compose up` fails if either is unset. + +# Auth token your agent expects on /mcp requests. +AGENT_TOKEN= + +# Postgres password (local-only dev credential). +POSTGRES_PASSWORD= diff --git a/docs/examples/multi-instance/Caddyfile b/docs/examples/multi-instance/Caddyfile new file mode 100644 index 000000000..d09e10ce7 --- /dev/null +++ b/docs/examples/multi-instance/Caddyfile @@ -0,0 +1,24 @@ +# DEV ONLY — local storyboard-testing harness. Do NOT deploy this Caddyfile +# to production: `X-Replica` exposes internal container hostports, and the +# listener is plain HTTP without TLS. +# +# Round-robin proxy for the two-replica AdCP agent harness. +# http://localhost:4099/mcp/ → agent-a:8080 / agent-b:8080 round-robin. + +:4099 { + handle_path /* { + reverse_proxy agent-a:8080 agent-b:8080 { + lb_policy round_robin + # Surface which replica answered so failing storyboard runs + # can read it in the response headers. Leaks internal hostport + # information — acceptable for a dev harness, NOT for prod. + header_up X-Forwarded-For {remote_host} + header_down +X-Replica {upstream_hostport} + } + } + + log { + output stdout + format console + } +} diff --git a/docs/examples/multi-instance/README.md b/docs/examples/multi-instance/README.md new file mode 100644 index 000000000..34813e024 --- /dev/null +++ b/docs/examples/multi-instance/README.md @@ -0,0 +1,73 @@ +# Multi-instance AdCP agent harness + +Two replicas of an AdCP agent sharing one Postgres store, fronted by a round-robin Caddy proxy. Copy-paste scaffolding for local multi-instance storyboard testing. + +## What's in here + +| File | Purpose | +| ----------------- | ------------------------------------------------------------- | +| `docker-compose.yml` | 2 app replicas + Postgres + Caddy. Ports 4100, 4101, 4099. | +| `Caddyfile` | Round-robin reverse proxy on :4099 fronting both replicas. | +| `.env.example` | Secrets/config placeholders — copy to `.env` before starting.| + +## Quickstart + +1. Swap `image: CHANGEME/your-adcp-agent:latest` in `docker-compose.yml` for your agent image (or add a `build:` context). +2. Copy env: + ```bash + cp .env.example .env + ``` +3. Bring it up: + ```bash + docker compose up -d --wait + ``` +4. Run storyboards. Two valid shapes: + + **Runner-level round-robin** (client alternates per step, distinct MCP sessions per replica): + ```bash + npx @adcp/client storyboard run \ + --url http://localhost:4100/mcp/ \ + --url http://localhost:4101/mcp/ \ + property_lists --auth "$AGENT_TOKEN" --allow-http + ``` + + **LB-level rotation** (single MCP session; Caddy round-robins per request; matches production fronting): + ```bash + npx @adcp/client storyboard run http://localhost:4099/mcp/ \ + property_lists --auth "$AGENT_TOKEN" --allow-http + ``` + +Both exercise cross-replica state persistence. The runner-level shape gives per-step `[#1]/[#2]` attribution in failure output; the LB shape is closer to production traffic but has no per-step attribution. + +## Why two shapes + +[Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state) in the AdCP docs lists two test topologies: + +- **Explicit multi-URL** — you control which replica each step hits; `--url` repeated at the client. +- **LB-round-robin** — you deploy behind an LB, the client hits one endpoint, the LB distributes. + +Either is valid. The docker-compose here gives you both on the same stack so you can pick without reconfiguring. + +## Picking storyboards worth running + +The multi-instance failure mode only surfaces on storyboards that have write→read chains. List them with: + +```bash +npx @adcp/client storyboard list --stateful +``` + +That returns the ~40 compliance storyboards with at least one step marked `stateful: true`. Run those; stateless probes (capability discovery, schema validation, auth rejection) don't exercise the cross-replica invariant. + +## Caveats + +- **Placeholder image.** This compose file has `image: CHANGEME/your-adcp-agent:latest`. Replace with a real image that exposes `/healthz` and can connect to Postgres via env vars (`POSTGRES_PASSWORD` is injected; build `DATABASE_URL` from `PGHOST`/`PGUSER`/`POSTGRES_PASSWORD` inside the container, or override by adding `DATABASE_URL=...` to your `.env`). An agent that stores state in-process will correctly fail multi-instance testing with this harness. +- **Local-only.** `allow_http` is required because these are HTTP, not HTTPS. Do not run production traffic through this setup. +- **State schema.** Your agent needs to run migrations against the shared Postgres on startup (or you need to run them separately before `up`). That's agent-specific. +- **No default password.** `.env.example` ships placeholder values — copy to `.env` and replace every `<...>` with real values before running `docker compose up`. No defaults are inlined in `docker-compose.yml`, so compose fails fast if `.env` is missing. +- **Caddyfile is DEV ONLY.** The reverse proxy config exposes internal container hostports via `X-Replica` (for debugging) and terminates plain HTTP without TLS. Do not deploy this Caddyfile to production. + +## Related + +- Runner docs: [`docs/guides/MULTI-INSTANCE-TESTING.md`](../../guides/MULTI-INSTANCE-TESTING.md) +- Spec requirement: [State persistence and horizontal scaling](https://adcontextprotocol.org/docs/protocol/architecture#state-persistence-and-horizontal-scaling) +- Builder guidance: [Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state) diff --git a/docs/examples/multi-instance/docker-compose.yml b/docs/examples/multi-instance/docker-compose.yml new file mode 100644 index 000000000..7992a4737 --- /dev/null +++ b/docs/examples/multi-instance/docker-compose.yml @@ -0,0 +1,102 @@ +# Reference multi-instance harness for AdCP storyboard testing. +# +# Two replicas of your agent (agent-a, agent-b) share one Postgres store. +# Caddy fronts both with round-robin routing for the "single LB endpoint" +# test path. You can also hit the replicas directly via their host ports +# (4100, 4101) and pass both to `npx @adcp/client storyboard run --url`. +# +# Swap `image: CHANGEME/your-adcp-agent:latest` for your own image (or a +# build context). Everything else is reusable. +# +# Quickstart: +# cp .env.example .env # replace every +# docker compose up -d --wait +# +# Then run storyboards in one of two shapes: +# +# # (A) Runner-level round-robin across two direct URLs. Each request +# # gets a distinct MCP session and round-robins at the client. +# npx @adcp/client storyboard run \ +# --url http://localhost:4100/mcp/ \ +# --url http://localhost:4101/mcp/ \ +# property_lists --auth $AGENT_TOKEN --allow-http +# +# # (B) LB-level rotation at Caddy. Single MCP session; Caddy round-robins +# # per request. This matches what a production Fly.io / K8s deployment +# # looks like from the outside. +# npx @adcp/client storyboard run http://localhost:4099/mcp/ \ +# property_lists --auth $AGENT_TOKEN --allow-http +# +# Both shapes exercise cross-replica state persistence. (A) gives you +# per-step instance attribution in failure output; (B) is closer to what +# production traffic looks like. +# +# All credentials flow through `env_file: .env` — nothing is inlined in +# this compose file so secret scanners don't flag reference values. + +services: + postgres: + image: postgres:16-alpine + restart: unless-stopped + env_file: .env + environment: + POSTGRES_DB: adcp + POSTGRES_USER: adcp + volumes: + - pgdata:/var/lib/postgresql/data + healthcheck: + test: ['CMD-SHELL', 'pg_isready -U adcp -d adcp'] + interval: 2s + timeout: 2s + retries: 30 + + agent-a: &agent + image: CHANGEME/your-adcp-agent:latest + restart: unless-stopped + env_file: .env + environment: + # The replica label shows up in error attribution — set it so + # cross-replica failure messages are unambiguous. DATABASE_URL and + # AGENT_TOKEN are injected via env_file (.env), not inlined here. + REPLICA_ID: a + PGHOST: postgres + PGPORT: '5432' + PGDATABASE: adcp + PGUSER: adcp + ports: + - '4100:8080' + depends_on: + postgres: + condition: service_healthy + healthcheck: + test: ['CMD-SHELL', 'curl -fsS http://localhost:8080/healthz || exit 1'] + interval: 2s + timeout: 2s + retries: 30 + + agent-b: + <<: *agent + environment: + REPLICA_ID: b + PGHOST: postgres + PGPORT: '5432' + PGDATABASE: adcp + PGUSER: adcp + ports: + - '4101:8080' + + caddy: + image: caddy:2-alpine + restart: unless-stopped + ports: + - '4099:4099' + volumes: + - ./Caddyfile:/etc/caddy/Caddyfile:ro + depends_on: + agent-a: + condition: service_healthy + agent-b: + condition: service_healthy + +volumes: + pgdata: diff --git a/docs/guides/MULTI-INSTANCE-TESTING.md b/docs/guides/MULTI-INSTANCE-TESTING.md new file mode 100644 index 000000000..856344d5c --- /dev/null +++ b/docs/guides/MULTI-INSTANCE-TESTING.md @@ -0,0 +1,109 @@ +# Multi-instance Storyboard Testing + +Catch horizontal-scaling bugs in your AdCP agent before production. + +## The protocol requirement + +The AdCP spec requires that `(brand, account)`-scoped state survive across agent process instances — a write on one replica MUST be readable from any other. See [State persistence and horizontal scaling](https://adcontextprotocol.org/docs/protocol/architecture#state-persistence-and-horizontal-scaling) for the normative text, and [Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state) for the builder-facing guidance. + +## The bug class + +An agent deployed behind a load balancer — Fly.io with ≥2 machines, Kubernetes with >1 pod, an autoscaling fleet — can pass every storyboard against a single URL and still break in production: + +1. Storyboard step 1 creates an entity on replica A. +2. Storyboard step 2 reads that entity. The load balancer routes to replica B. +3. Replica B has no record because state lived in replica A's process memory. +4. The read returns `NOT_FOUND`. + +A single-URL storyboard never sees this. Multi-instance mode round-robins steps across two (or more) URLs that share a backing store. An agent whose state is `(brand, account)`-keyed and stored in a shared datastore passes; an agent whose state is per-process fails on the second request. + +## Usage + +``` +npx @adcp/client storyboard run \ + --url https://a.your-agent.example/mcp/ \ + --url https://b.your-agent.example/mcp/ \ + account_and_audience \ + --auth $AGENT_TOKEN +``` + +- Repeat `--url` once per instance (minimum 2). +- The positional agent argument is disallowed in multi-instance mode — use `--url` flags only. +- A storyboard ID, bundle ID, or `--file ` is required. Full capability-driven assessment is single-URL only in this release. +- `--dry-run` prints the per-step instance assignment plan without executing. +- `--json` emits a per-step `agent_url` and `agent_index`; the top-level result gains `agent_urls[]` and `multi_instance_strategy`. + +## What makes the test valid: (brand, account)-scoped state + +Multi-instance mode asserts the protocol requirement: state keyed by `(brand, account)` survives across replicas. If your agent keys state only by the MCP session ID, each `--url` client gets a distinct session, and round-robin produces legitimate isolation — you will see false failures. + +That is not a reason to disable multi-instance mode. It is a reason to fix the state-keying. An agent that only works inside one MCP session has already failed multi-tenant isolation, per the spec. + +## Failure signature + +When a step fails in multi-instance mode the runner appends a block to the error. The wording mirrors the canonical example in the protocol docs so you pattern-match the page you'll click through to: + +``` +create on replica [#1] (https://a.your-agent.example/mcp/) succeeded. +read on replica [#2] (https://b.your-agent.example/mcp/) failed with NOT_FOUND. +→ Brand-scoped state is not shared across replicas. +See: https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state +Replica → step map: + [#1] create — ok + [#2] read — FAIL +Reproduce single-replica: adcp storyboard run https://b.your-agent.example/mcp/ account_and_audience +``` + +The `Reproduce single-replica` command gives you the exact repro so you can confirm the bug is cross-replica, not intrinsic. Single-URL pass + multi-URL fail = horizontal-scaling bug. + +## Local harness + +A reference compose file ships at [`docs/examples/multi-instance/`](../examples/multi-instance/) — two app replicas + Postgres + Caddy round-robin proxy. Edit the agent image line, copy `.env.example` to `.env`, run `docker compose up -d --wait`. Supports both runner-level `--url` round-robin (hit both replicas directly at 4100, 4101) and LB-level rotation (hit Caddy at 4099). + +For local dev without Docker, the simplest setup is two `node` processes listening on different ports, both pointed at the same Postgres / SQLite file / Redis instance. + +## Live smoke test against the public test agent + +Quick way to verify the multi-instance code path end-to-end against the AdCP public test agent: + +```bash +npx @adcp/client storyboard run \ + --url "https://test-agent.adcontextprotocol.org/mcp/?replica=a" \ + --url "https://test-agent.adcontextprotocol.org/mcp/?replica=b" \ + property_lists \ + --auth $TEST_AGENT_TOKEN +``` + +The `?replica=a` / `?replica=b` query strings are a deliberate cache-busting trick. The MCP connection pool keys by URL, so two identical URL strings collapse onto one transport and one session — which defeats the whole test. Distinct query strings force two transports, two `Mcp-Session-Id`s, two independent `initialize` handshakes. Both requests still reach the same agent; the test exercises the full MCP-SDK multi-session code path even without per-replica DNS. + +The test agent is correctly implemented — it runs behind Fly.io's round-robin routing with a shared Postgres store — so this run should show the property-list chain (create → list → get → update → validate → delete) passing across replica boundaries. A failure here means either the spec requirement broke in the test agent, or the runner regressed. + +This command alternates which replica each step hits via round-robin. The runner's failure attribution also correctly distinguishes cross-replica failures from intrinsic ones — if a read fails on the *same* replica as its prior write (round-robin coincidence), the attribution message says so rather than falsely blaming cross-replica state. + +## Distribution strategy + +Currently only `round-robin` is implemented. Step N is dispatched to `urls[N % urls.length]`. The assignment is deterministic and reproducible — the same storyboard always hits the same URLs in the same order, so bug reports are stable. + +The `multi_instance_strategy` enum is reserved for future additions (random, reverse, sticky-by-step-index) without breaking the signature. + +## Limitations + +- **No full-assessment mode.** `adcp storyboard run --url ... --url ...` requires a storyboard or bundle ID. The full capability-driven assessment path shares a single client across storyboards for connection reuse, which is incompatible with per-step URL dispatch. Run the bundles you care about individually. +- **Single auth across URLs.** `--auth` applies to every instance. Per-URL credentials for migration-era deployments with per-host tokens are not supported yet. +- **Sequential execution.** The runner does not parallelize steps across instances. It round-robins serially. Concurrent-write races are not exercised by this mode. +- **State sensitivity is inferred from step-level `stateful: true`.** The runner does not round-robin probe-only storyboards (e.g., `oauth_auth_server_metadata`) any differently than stateful ones — all steps are round-robined uniformly. For probes, every instance should return consistent metadata, which is its own valid invariant to check. + +## Verify-by-architecture vs verify-by-multi-instance + +The upstream docs ([Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state)) call out three valid ways to prove the invariant. Multi-instance testing is one of them. + +- **Verify by architecture.** If you deploy on a managed serverless platform with a shared datastore (Lambda + DynamoDB, Cloud Run + Firestore, Vercel + Neon, etc.), the invariant holds by construction. Single-URL storyboards that pass are sufficient — you do not need `--url` flags. +- **Verify by multi-instance testing.** If you deploy long-running processes, use `--url` to round-robin explicitly (this doc), OR stand up ≥2 replicas behind round-robin routing and run storyboards against the shared endpoint (the LB does the rotation). +- **Verify by your own testing.** Property-based tests, chaos fault injection, or production observability that correlates writes and reads across replicas are all valid. The protocol cares about the invariant, not the methodology. + +## Related + +- Spec requirement: [State persistence and horizontal scaling](https://adcontextprotocol.org/docs/protocol/architecture#state-persistence-and-horizontal-scaling) +- Builder guidance: [Verifying cross-instance state](https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state) +- Upstream PR: [adcontextprotocol/adcp#2363](https://github.com/adcontextprotocol/adcp/pull/2363) +- Client issue: [adcontextprotocol/adcp#2267](https://github.com/adcontextprotocol/adcp/issues/2267) diff --git a/src/lib/testing/storyboard/runner.ts b/src/lib/testing/storyboard/runner.ts index 2d16278f6..3d95a1750 100644 --- a/src/lib/testing/storyboard/runner.ts +++ b/src/lib/testing/storyboard/runner.ts @@ -6,7 +6,7 @@ * - runStoryboardStep(): run a single step (stateless, LLM-friendly) */ -import { getOrCreateClient, getOrDiscoverProfile, runStep } from '../client'; +import { getOrCreateClient, getOrDiscoverProfile, runStep, type TestClient } from '../client'; import { closeConnections } from '../../protocols'; import { executeStoryboardTask } from './task-map'; import { extractContext, injectContext, applyContextOutputs, applyContextInputs } from './context'; @@ -44,19 +44,44 @@ import type { TaskResult } from '../types'; /** * Run an entire storyboard against an agent. + * + * Pass a single URL for the standard single-instance run. Pass an array of + * URLs to engage multi-instance mode: the runner round-robins each step + * across the provided URLs so that (brand, account)-scoped state created on + * one instance must be visible on the next. Sellers whose state lives only + * in-process will fail this mode — the failure signature is a prior write + * succeeding on instance A while a subsequent read returns NOT_FOUND or + * empty on instance B. */ export async function runStoryboard( - agentUrl: string, + agentUrlOrUrls: string | string[], storyboard: Storyboard, options: StoryboardRunOptions = {} ): Promise { validateTestKit(options.test_kit); const start = Date.now(); - const client = getOrCreateClient(agentUrl, options); + const agentUrls = Array.isArray(agentUrlOrUrls) ? agentUrlOrUrls : [agentUrlOrUrls]; + if (agentUrls.length === 0) { + throw new Error('runStoryboard: at least one agent URL required'); + } + const isMultiInstance = agentUrls.length > 1; + if (isMultiInstance && options._client) { + throw new Error( + 'runStoryboard: _client override is incompatible with multi-instance mode. ' + + 'Remove _client (or pass a single agent URL) to use round-robin dispatch.' + ); + } - // Discover agent profile and (for MCP) keep the transport alive. + // Build one client per URL. In single-URL mode `_client` (from comply()) is + // honored so the shared MCP transport is reused across storyboards. + const clients = agentUrls.map(url => getOrCreateClient(url, options)); + + // Discover agent profile against the first instance; all instances are + // expected to run the same code behind a shared state store, so one probe + // is sufficient. For multi-instance runs, skipping N-1 redundant + // get_agent_info calls also keeps CI output clean. if (!options._client) { - const { profile } = await getOrDiscoverProfile(client, options); + const { profile } = await getOrDiscoverProfile(clients[0]!, options); // Populate agentTools from discovered profile if not already set if (!options.agentTools && profile?.tools) { options = { ...options, agentTools: profile.tools }; @@ -74,6 +99,7 @@ export async function runStoryboard( // Flatten all steps for next-step preview lookups const allSteps = flattenSteps(storyboard); + const dispatch = createDispatcher(agentUrls, clients, options.multi_instance_strategy ?? 'round-robin'); for (const phase of storyboard.phases) { const phaseStart = Date.now(); @@ -113,12 +139,17 @@ export async function runStoryboard( continue; } - const result = await executeStep(client, step, phase.id, context, allSteps, options, { + const assignment = dispatch.nextFor(step); + const result = await executeStep(assignment.client, step, phase.id, context, allSteps, options, { contributions, priorStepResults, priorProbes, - agentUrl, + agentUrl: assignment.agentUrl, }); + if (isMultiInstance) { + result.agent_url = assignment.agentUrl; + result.agent_index = assignment.instanceIndex + 1; + } stepResults.push(result); priorStepResults.set(step.id, result); @@ -144,6 +175,11 @@ export async function runStoryboard( // configured) must not fail the storyboard by itself. if (!phase.optional) failedCount++; if (step.stateful) statefulFailed = true; + // In multi-instance mode, annotate the failure with the cross-instance + // attribution block so CI readers pattern-match it as a deployment bug. + if (isMultiInstance) { + annotateMultiInstanceFailure(result, storyboard, stepResults); + } } } @@ -170,7 +206,9 @@ export async function runStoryboard( const result: StoryboardResult = { storyboard_id: storyboard.id, storyboard_title: storyboard.title, - agent_url: agentUrl, + agent_url: agentUrls[0]!, + ...(isMultiInstance && { agent_urls: [...agentUrls] }), + ...(isMultiInstance && { multi_instance_strategy: options.multi_instance_strategy ?? 'round-robin' }), overall_passed: failedCount === 0 && requiredPhasesPassed, phases: phaseResults, context, @@ -181,7 +219,9 @@ export async function runStoryboard( tested_at: new Date().toISOString(), }; - // Close protocol connections when the runner created its own client + // Close protocol connections when the runner created its own client. The + // connection pool is keyed by URL+auth, so a single closeConnections() call + // evicts every instance's transport regardless of how many URLs we used. if (!options._client) { await closeConnections(options.protocol); } @@ -785,6 +825,159 @@ function getNextStepPreview( }; } +// ──────────────────────────────────────────────────────────── +// Multi-instance dispatch +// ──────────────────────────────────────────────────────────── + +interface StepAssignment { + client: TestClient; + agentUrl: string; + /** 0-based index into the agent URL list */ + instanceIndex: number; +} + +interface Dispatcher { + nextFor(step: StoryboardStep): StepAssignment; +} + +/** + * Build a dispatcher that picks an (agent URL, client) pair per step. + * + * Single-URL runs always return the same assignment. Multi-URL runs use the + * configured strategy — currently round-robin only (other strategies reserved + * in the enum; see tracking issue adcontextprotocol/adcp-client#607 for a + * dependency-aware variant that closes the 2-replica coverage gap). Each + * step increments the counter so step N hits `clients[N % N_urls]`, + * deterministic and reproducible for bug reports. + */ +function createDispatcher(agentUrls: string[], clients: TestClient[], _strategy: 'round-robin'): Dispatcher { + let counter = 0; + return { + nextFor(_step: StoryboardStep): StepAssignment { + const idx = counter % agentUrls.length; + counter++; + return { + client: clients[idx]!, + agentUrl: agentUrls[idx]!, + instanceIndex: idx, + }; + }, + }; +} + +const HORIZONTAL_SCALING_DOCS_URL = + 'https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state'; +const NOT_FOUND_PATTERN = /not[_ ]found|not-found|\b404\b/i; + +// Agent-controlled text (error messages, response payloads) lands in terminal +// output. Strip C0/C1 control chars so a hostile agent returning +// `\x1b[2J\x1b[H` (clear screen) or `\r` (overwrite prior line) can't mangle +// CI logs or forge terminal state. Tabs and newlines are preserved. +// The cap bounds JSON-stringification cost if an agent returns an enormous +// or deeply-nested response body. +const MAX_ATTRIBUTION_SNIPPET = 512; +function sanitizeAgentText(text: string): string { + return text.replace(/[\x00-\x08\x0b-\x1f\x7f-\x9f]/g, '').slice(0, MAX_ATTRIBUTION_SNIPPET); +} + +/** + * Detect the canonical horizontal-scaling failure signature on a step result. + * + * Reads structured fields the runner commonly populates (error string, + * nested response.error/code/message/status) rather than regex-matching the + * full stringified response — structured lookup is cheaper, resistant to an + * agent smuggling "NOT_FOUND" into an unrelated field to falsely trigger the + * canonical wording, and doesn't blow up on circular or oversized payloads. + */ +function isNotFoundSignature(result: StoryboardStepResult): boolean { + const candidates: Array = [result.error]; + const resp = result.response as Record | null | undefined; + if (resp && typeof resp === 'object' && !Array.isArray(resp)) { + candidates.push(resp.error, resp.code, resp.message, resp.status, resp.status_code); + } + for (const c of candidates) { + if (typeof c === 'string' && NOT_FOUND_PATTERN.test(c)) return true; + if (typeof c === 'number' && c === 404) return true; + } + return false; +} + +/** + * Mutate a failed step result to include cross-instance attribution. + * + * In multi-instance mode any step failure is worth attributing because the + * failure signature may not be NOT_FOUND — it can surface as 500, an empty + * array, PERMISSION_DENIED, or stale status. Attribution always emits: + * - which replica served this step and the immediate prior stateful write + * - a replica→step map for pattern-matching in CI logs + * - a single-replica repro command + * When the signature matches the canonical horizontal-scaling case (prior + * write on A, read fails on B with NOT_FOUND), the wording mirrors the + * protocol docs verbatim so developers pattern-match the page they'll + * eventually click through to. + */ +function annotateMultiInstanceFailure( + result: StoryboardStepResult, + storyboard: Storyboard, + priorResults: StoryboardStepResult[] +): void { + const currentInstance = result.agent_index; + const currentUrl = result.agent_url; + if (!currentInstance || !currentUrl) return; + + // Lookup stateful flag on step defs — needed to identify "prior writes". + const stepDefs = new Map(); + for (const phase of storyboard.phases) { + for (const s of phase.steps) stepDefs.set(s.id, s); + } + + const priorCrossInstanceWrite = [...priorResults].reverse().find(prior => { + if (!prior.passed || prior.skipped) return false; + if (!prior.agent_index || prior.agent_index === currentInstance) return false; + return stepDefs.get(prior.step_id)?.stateful === true; + }); + + const replicaMap = priorResults + .filter(r => !r.skipped && r.agent_index) + .map(r => ` [#${r.agent_index}] ${r.step_id} — ${r.passed ? 'ok' : 'FAIL'}`) + .join('\n'); + + const lines: string[] = []; + + if (priorCrossInstanceWrite) { + const writerIdx = priorCrossInstanceWrite.agent_index; + const writerUrl = priorCrossInstanceWrite.agent_url; + // Wording deliberately mirrors the failure example in the protocol docs + // (" on replica A returned …; on replica B returned NOT_FOUND; + // → Brand-scoped state is not shared across replicas.") so CI readers + // pattern-match the page they'll click through to. + lines.push(`${priorCrossInstanceWrite.step_id} on replica [#${writerIdx}] (${writerUrl}) succeeded.`); + lines.push( + `${result.step_id} on replica [#${currentInstance}] (${currentUrl}) failed${ + isNotFoundSignature(result) ? ' with NOT_FOUND' : '' + }.` + ); + lines.push('→ Brand-scoped state is not shared across replicas.'); + lines.push(`See: ${HORIZONTAL_SCALING_DOCS_URL}`); + } else { + lines.push( + `Multi-instance failure on replica [#${currentInstance}] (${currentUrl}). ` + + `No prior cross-replica stateful write found — the failure may be intrinsic to this replica.` + ); + } + + if (replicaMap) { + lines.push('Replica → step map:'); + lines.push(replicaMap); + } + lines.push(`Reproduce single-replica: adcp storyboard run ${currentUrl} ${storyboard.id}`); + + // Agent-controlled text goes in the base line; control chars are stripped + // so a hostile agent can't forge terminal escape sequences in CI output. + const base = sanitizeAgentText(result.error ?? 'Step failed'); + result.error = `${base}\n\n${lines.join('\n')}`; +} + /** * Get a preview of the first step in a storyboard (for showing what will happen). */ diff --git a/src/lib/testing/storyboard/types.ts b/src/lib/testing/storyboard/types.ts index 796729f5f..d61753357 100644 --- a/src/lib/testing/storyboard/types.ts +++ b/src/lib/testing/storyboard/types.ts @@ -220,6 +220,12 @@ export interface StoryboardRunOptions extends TestOptions { * in the report when used. */ allow_http?: boolean; + /** + * Distribution strategy across agent URLs in multi-instance mode. + * Only consulted when the runner is given 2+ URLs. Defaults to 'round-robin'. + * Reserved enum; additional strategies may land without a signature change. + */ + multi_instance_strategy?: 'round-robin'; } // ──────────────────────────────────────────────────────────── @@ -271,6 +277,10 @@ export interface StoryboardStepResult { error?: string; /** Preview of the next step (for LLM consumption) */ next?: StoryboardStepPreview; + /** Agent URL that served this step (multi-instance mode). Absent in single-URL mode. */ + agent_url?: string; + /** 1-based index of the agent instance (multi-instance mode). Absent in single-URL mode. */ + agent_index?: number; } export interface StoryboardPhaseResult { @@ -284,7 +294,12 @@ export interface StoryboardPhaseResult { export interface StoryboardResult { storyboard_id: string; storyboard_title: string; + /** Primary agent URL. In multi-instance mode this is the first URL — see agent_urls for the full list. */ agent_url: string; + /** All agent URLs used in multi-instance mode. Absent (or single-entry) in single-URL mode. */ + agent_urls?: string[]; + /** Distribution strategy used across agent_urls. Absent in single-URL mode. */ + multi_instance_strategy?: 'round-robin'; overall_passed: boolean; phases: StoryboardPhaseResult[]; /** Final accumulated context */ diff --git a/test/lib/storyboard-multi-instance.test.js b/test/lib/storyboard-multi-instance.test.js new file mode 100644 index 000000000..c9cedf6f1 --- /dev/null +++ b/test/lib/storyboard-multi-instance.test.js @@ -0,0 +1,448 @@ +/** + * Storyboard runner — multi-instance (round-robin) mode. + * + * Issue #2267: sellers deployed behind a load balancer with in-memory state + * pass every storyboard against a single URL but break in production because + * brand-scoped state isn't shared across machines. The runner's multi-URL + * mode round-robins steps across 2+ seller URLs so cross-machine persistence + * is exercised in CI. + * + * These tests stand up two local HTTP servers as fake MCP agents. Steps use + * `auth: 'none'` so dispatch goes through `rawMcpProbe` (no MCP SDK session + * handshake) — sufficient for the round-robin and attribution assertions, + * and keeps the tests free of MCP initialization concerns. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert'); +const http = require('http'); + +const { runStoryboard } = require('../../dist/lib/testing/storyboard/runner.js'); +const { GovernanceAgentStub } = require('../../dist/lib/testing/stubs/index.js'); +const { closeMCPConnections } = require('../../dist/lib/protocols/mcp.js'); + +// ──────────────────────────────────────────────────────────── +// Fake agent harness +// ──────────────────────────────────────────────────────────── + +/** + * Start a fake MCP agent on an ephemeral port. `state` is a Map the handler + * writes to for `create_*` tasks and reads from for `get_*` tasks — inject + * one shared Map across two agents to simulate a correctly shared backing + * store, or separate Maps to simulate the per-process bug. + */ +async function startFakeAgent({ state, label }) { + const requests = []; + 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')); + const toolName = rpc.params?.name; + const args = rpc.params?.arguments ?? {}; + requests.push({ tool: toolName, args, label }); + + const ok = structured => + res + .writeHead(200, { 'content-type': 'application/json' }) + .end(JSON.stringify({ jsonrpc: '2.0', id: rpc.id, result: { structuredContent: structured } })); + const notFound = msg => + res.writeHead(200, { 'content-type': 'application/json' }).end( + JSON.stringify({ + jsonrpc: '2.0', + id: rpc.id, + result: { isError: true, structuredContent: { error: msg, code: 'NOT_FOUND' } }, + }) + ); + + // Use non-AdCP tool names so the runner's request builder map doesn't + // rewrite the sample_request (`sync_accounts` and similar have builders + // that strip caller-provided IDs). + if (toolName === '__test_write') { + const key = args.key ?? 'default'; + state.set(key, { key, value: args.value ?? 'v', written_on: label }); + return ok({ key, stored: true }); + } + if (toolName === '__test_read') { + const key = args.key ?? 'default'; + const rec = state.get(key); + if (!rec) return notFound(`key ${key} NOT_FOUND on instance ${label}`); + return ok(rec); + } + if (toolName === '__test_probe') { + return ok({ instance: label }); + } + if (toolName === 'get_adcp_capabilities') { + return ok({ version: '1.0', protocols: [], specialisms: [] }); + } + return notFound(`unknown tool ${toolName} on instance ${label}`); + }); + await new Promise(r => server.listen(0, '127.0.0.1', r)); + const port = server.address().port; + return { server, url: `http://127.0.0.1:${port}/mcp`, requests }; +} + +function stopAgent(agent) { + return new Promise(r => agent.server.close(r)); +} + +// ──────────────────────────────────────────────────────────── +// Storyboard fixtures +// ──────────────────────────────────────────────────────────── + +function storyboardWith(steps) { + return { + id: 'multi_instance_sb', + version: '1.0.0', + title: 'Multi-instance test', + category: 'testing', + summary: '', + narrative: '', + agent: { interaction_model: '*', capabilities: [] }, + caller: { role: 'buyer_agent' }, + phases: [{ id: 'p', title: 'crud', steps }], + }; +} + +// Every step uses `auth: 'none'` so dispatch goes via rawMcpProbe. We set +// `agentTools` to pretend every tool is advertised (so the runner doesn't +// skip them for missing_tool) and inject a `_profile` to skip discovery. +const AGENT_TOOLS = ['__test_write', '__test_read', '__test_probe', 'get_adcp_capabilities']; +const RUN_OPTIONS_BASE = { + protocol: 'mcp', + allow_http: true, + agentTools: AGENT_TOOLS, + _profile: { name: 'fake', tools: AGENT_TOOLS.map(name => ({ name })) }, +}; + +// ──────────────────────────────────────────────────────────── +// Tests +// ──────────────────────────────────────────────────────────── + +describe('runStoryboard: multi-instance round-robin', () => { + let agentA; + let agentB; + + afterEach(async () => { + if (agentA) await stopAgent(agentA); + if (agentB) await stopAgent(agentB); + agentA = undefined; + agentB = undefined; + }); + + test('dispatches steps round-robin across URLs and records agent_index', async () => { + const shared = new Map(); + agentA = await startFakeAgent({ state: shared, label: 'A' }); + agentB = await startFakeAgent({ state: shared, label: 'B' }); + + const storyboard = storyboardWith([ + { id: 's1', title: 's1', task: '__test_probe', auth: 'none', sample_request: {} }, + { id: 's2', title: 's2', task: '__test_probe', auth: 'none', sample_request: {} }, + { id: 's3', title: 's3', task: '__test_probe', auth: 'none', sample_request: {} }, + { id: 's4', title: 's4', task: '__test_probe', auth: 'none', sample_request: {} }, + ]); + + const result = await runStoryboard([agentA.url, agentB.url], storyboard, RUN_OPTIONS_BASE); + + assert.deepStrictEqual(result.agent_urls, [agentA.url, agentB.url]); + assert.strictEqual(result.multi_instance_strategy, 'round-robin'); + const steps = result.phases[0].steps; + assert.strictEqual(steps[0].agent_index, 1); + assert.strictEqual(steps[1].agent_index, 2); + assert.strictEqual(steps[2].agent_index, 1); + assert.strictEqual(steps[3].agent_index, 2); + assert.strictEqual(steps[0].agent_url, agentA.url); + assert.strictEqual(steps[1].agent_url, agentB.url); + // Each instance received exactly 2 requests. + assert.strictEqual(agentA.requests.length, 2); + assert.strictEqual(agentB.requests.length, 2); + }); + + test('shared backing store: write on A, read on B succeeds', async () => { + const shared = new Map(); + agentA = await startFakeAgent({ state: shared, label: 'A' }); + agentB = await startFakeAgent({ state: shared, label: 'B' }); + + const storyboard = storyboardWith([ + { + id: 'create', + title: 'create', + task: '__test_write', + stateful: true, + auth: 'none', + sample_request: { key: 'k1', value: 'v1' }, + }, + { + id: 'read', + title: 'read', + task: '__test_read', + stateful: true, + auth: 'none', + sample_request: { key: 'k1' }, + }, + ]); + + const result = await runStoryboard([agentA.url, agentB.url], storyboard, RUN_OPTIONS_BASE); + assert.ok(result.overall_passed, `expected overall_passed, got error: ${result.phases[0].steps[1].error}`); + assert.strictEqual(result.phases[0].steps[0].agent_index, 1); + assert.strictEqual(result.phases[0].steps[1].agent_index, 2); + }); + + test('per-process state: write on A, read on B fails with attribution block', async () => { + // Each agent gets its own state map — the horizontal-scaling bug. + agentA = await startFakeAgent({ state: new Map(), label: 'A' }); + agentB = await startFakeAgent({ state: new Map(), label: 'B' }); + + const storyboard = storyboardWith([ + { + id: 'create', + title: 'create', + task: '__test_write', + stateful: true, + auth: 'none', + sample_request: { key: 'k1', value: 'v1' }, + }, + { + id: 'read', + title: 'read', + task: '__test_read', + stateful: true, + auth: 'none', + sample_request: { key: 'k1' }, + }, + ]); + + const result = await runStoryboard([agentA.url, agentB.url], storyboard, RUN_OPTIONS_BASE); + + assert.strictEqual(result.overall_passed, false); + const readStep = result.phases[0].steps[1]; + assert.strictEqual(readStep.passed, false); + assert.ok(readStep.error, 'expected an error message on the failed read step'); + // Wording mirrors the failure example in + // https://adcontextprotocol.org/docs/building/validate-your-agent#verifying-cross-instance-state + assert.match(readStep.error, /create on replica \[#1\]/); + assert.match(readStep.error, /read on replica \[#2\] .* failed with NOT_FOUND/); + assert.match(readStep.error, /→ Brand-scoped state is not shared across replicas\./); + assert.match( + readStep.error, + /validate-your-agent#verifying-cross-instance-state/, + 'expected deep link to upstream docs anchor' + ); + assert.match(readStep.error, /Reproduce single-replica: adcp storyboard run/); + }); + + test('single URL as string keeps backward-compatible result shape', async () => { + const shared = new Map(); + agentA = await startFakeAgent({ state: shared, label: 'A' }); + + const storyboard = storyboardWith([ + { id: 's1', title: 's1', task: '__test_probe', auth: 'none', sample_request: {} }, + ]); + + const result = await runStoryboard(agentA.url, storyboard, RUN_OPTIONS_BASE); + assert.strictEqual(result.agent_url, agentA.url); + assert.strictEqual(result.agent_urls, undefined); + assert.strictEqual(result.multi_instance_strategy, undefined); + assert.strictEqual(result.phases[0].steps[0].agent_index, undefined); + assert.strictEqual(result.phases[0].steps[0].agent_url, undefined); + }); + + test('rejects _client override in multi-instance mode', async () => { + await assert.rejects( + runStoryboard(['http://a', 'http://b'], storyboardWith([]), { + ...RUN_OPTIONS_BASE, + _client: {}, + }), + /incompatible with multi-instance mode/ + ); + }); + + test('rejects empty URL array', async () => { + await assert.rejects(runStoryboard([], storyboardWith([]), RUN_OPTIONS_BASE), /at least one agent URL required/); + }); +}); + +// ──────────────────────────────────────────────────────────── +// MCP SDK dispatch path (no auth: 'none' override — full MCP handshake) +// ──────────────────────────────────────────────────────────── + +/** + * The tests above use `auth: 'none'` which routes through `rawMcpProbe` + * (raw fetch, no MCP session handshake). That's sufficient for dispatch + * and attribution logic but doesn't exercise the real production path: + * two ADCPMultiAgentClient instances each with their own + * StreamableHTTPClientTransport, each doing an `initialize` + tool dispatch. + * This block covers that path using two GovernanceAgentStub servers. + */ +describe('runStoryboard: multi-instance through MCP SDK', () => { + let stubA; + let stubB; + let urls; + + beforeEach(async () => { + stubA = new GovernanceAgentStub(); + stubB = new GovernanceAgentStub(); + const a = await stubA.start(); + const b = await stubB.start(); + urls = [a.url, b.url]; + }); + + afterEach(async () => { + // Close the client-side MCP connection cache first so transports attached + // to the about-to-die servers don't linger and leak sockets into the next + // test's event loop. + await closeMCPConnections(); + await stubA.stop(); + await stubB.stop(); + }); + + test('round-robins a stateless storyboard across two MCP stubs via the SDK', async () => { + // check_governance is deterministic per plan_id — both stubs accept it and + // return identical responses, so this storyboard doesn't depend on state + // sharing. It exists solely to verify that the MCP SDK path (initialize + // handshake, tools/call serialization, session handling) works when the + // runner has two distinct transports rotating per step. + const storyboard = { + id: 'mcp_sdk_multi_instance', + version: '1.0.0', + title: 'MCP SDK multi-instance', + category: 'testing', + summary: '', + narrative: '', + agent: { interaction_model: '*', capabilities: [] }, + caller: { role: 'buyer_agent' }, + phases: [ + { + id: 'p', + title: 'alternating governance checks', + steps: [ + { + id: 's1', + title: 'check 1', + task: 'check_governance', + sample_request: { + plan_id: 'plan-mcp-sdk', + binding: 'proposed', + caller: 'buyer', + tool: 'create_media_buy', + payload: { budget: 100 }, + }, + }, + { + id: 's2', + title: 'check 2', + task: 'check_governance', + sample_request: { + plan_id: 'plan-mcp-sdk', + binding: 'proposed', + caller: 'buyer', + tool: 'create_media_buy', + payload: { budget: 200 }, + }, + }, + { + id: 's3', + title: 'check 3', + task: 'check_governance', + sample_request: { + plan_id: 'plan-mcp-sdk', + binding: 'proposed', + caller: 'buyer', + tool: 'create_media_buy', + payload: { budget: 300 }, + }, + }, + ], + }, + ], + }; + + const result = await runStoryboard(urls, storyboard, { + protocol: 'mcp', + allow_http: true, + agentTools: ['check_governance', 'get_adcp_capabilities'], + _profile: { + name: 'stub', + tools: [{ name: 'check_governance' }, { name: 'get_adcp_capabilities' }], + }, + }); + + assert.ok( + result.overall_passed, + `expected all stateless steps to pass; first error: ${result.phases[0].steps.find(s => !s.passed)?.error}` + ); + const steps = result.phases[0].steps; + assert.strictEqual(steps[0].agent_index, 1); + assert.strictEqual(steps[1].agent_index, 2); + assert.strictEqual(steps[2].agent_index, 1); + // We care about the check_governance calls — the SDK may also do a + // tools/list on first contact. Filter to the tool we dispatched. + const aChecks = stubA.getCallsForTool('check_governance'); + const bChecks = stubB.getCallsForTool('check_governance'); + assert.strictEqual(aChecks.length, 2, `stubA check_governance count: ${JSON.stringify(stubA.getCallLog())}`); + assert.strictEqual(bChecks.length, 1, `stubB check_governance count: ${JSON.stringify(stubB.getCallLog())}`); + }); + + test('does not echo the auth token in any step result or error', async () => { + // Regression for the security review's concern: a hostile agent could + // force a failure and embed the token if the runner ever placed it in + // attribution text. Verify empirically by passing a distinctive token + // and scanning the entire serialized result for it. + const SECRET = 'adcp-test-token-DO-NOT-ECHO-MUST-NOT-LEAK'; + // Deliberately call a step whose task isn't declared to force a skip + // that still exercises the multi-instance path. + const storyboard = { + id: 'token_no_echo', + version: '1.0.0', + title: 'Token no-echo', + category: 'testing', + summary: '', + narrative: '', + agent: { interaction_model: '*', capabilities: [] }, + caller: { role: 'buyer_agent' }, + phases: [ + { + id: 'p', + title: 'governance', + steps: [ + { + id: 's1', + title: 's1', + task: 'check_governance', + sample_request: { + plan_id: 'plan-no-echo', + binding: 'proposed', + caller: 'buyer', + tool: 'create_media_buy', + payload: {}, + }, + }, + { + id: 's2', + title: 's2', + task: 'check_governance', + sample_request: { + plan_id: 'plan-no-echo', + binding: 'proposed', + caller: 'buyer', + tool: 'create_media_buy', + payload: {}, + }, + }, + ], + }, + ], + }; + + const result = await runStoryboard(urls, storyboard, { + protocol: 'mcp', + allow_http: true, + auth: { type: 'bearer', token: SECRET }, + agentTools: ['check_governance', 'get_adcp_capabilities'], + _profile: { name: 'stub', tools: [{ name: 'check_governance' }] }, + }); + + const serialized = JSON.stringify(result); + assert.ok(!serialized.includes(SECRET), 'token leaked into result JSON'); + assert.ok(!serialized.includes(SECRET.slice(5)), 'token substring leaked into result JSON'); + }); +});