Skip to content

fix(server): fix streamable-http ASGI error, host binding, and AdCP 3.0.1 scenario gaps#296

Merged
bokelley merged 2 commits intomainfrom
claude/issue-295-storyboard-runner-fixes
Apr 30, 2026
Merged

fix(server): fix streamable-http ASGI error, host binding, and AdCP 3.0.1 scenario gaps#296
bokelley merged 2 commits intomainfrom
claude/issue-295-storyboard-runner-fixes

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Refs #295

examples/seller_agent.py is the SDK's own reference agent, and it cannot pass the AdCP storyboard compliance runner out of the box because of six compounding issues. This PR fixes five of them (the sixth — DNS-rebinding allowlist — is security-sensitive and flagged for human review on the source issue).

Changes

src/adcp/server/serve.pyserve() + create_mcp_server()

  • Add stateless_http: bool = True (default): sets mcp.settings.stateless_http = True and mcp.settings.json_response = True on the FastMCP instance before the ASGI app is built. FastMCP's streaming-SSE default closes the response without completing, causing ASGI callable returned without completing response and making the runner report overall_status: "unreachable". The tests in test_mcp_middleware_composition.py already set these attributes manually (lines 155-156 etc.); this PR makes them the default.
  • Add host: str | None = None (default: ADCP_HOST env → "0.0.0.0"): FastMCP defaults to 127.0.0.1; container deployments (Fly.io, k8s, Cloud Run) bind loopback only and never receive external traffic.

src/adcp/server/test_controller.py

  • Add 5 AdCP 3.0.1 seed_* scenarios to SCENARIOS, TestControllerStore (stub methods with raise NotImplementedError), _handle_test_controller (explicit elif branches with all-optional .get() params matching the schema), and the register_test_controller inputSchema enum.
  • Add "account": {"type": "object"} to the comply_test_controller tool schema; the runner sends {account, scenario, params} but the schema didn't declare account, blocking runner detection.

src/adcp/server/mcp_tools.py

  • Mirror the scenario enum and account field additions in ADCP_TOOL_DEFINITIONS.

src/adcp/server/responses.py

  • Emit logging.WARNING from capabilities_response() when compliance_testing is provided but idempotency is not. AdCP 3.0.1 storyboard runner downgrades to v2 mode without the adcp.idempotency field; this warning makes the gap visible without silently changing the default (which would break test_capabilities_response_idempotency_omitted_when_none).

What was tested

  • pytest tests/test_mcp_middleware_composition.py tests/test_server_idempotency.py tests/test_server_dx.py tests/test_server_builder.py tests/test_idempotency_storyboard.py — 156 passed
  • pytest tests/ -q --ignore=tests/conformance/signing/test_ip_pinned_transport.py — 2186 passed, 22 skipped (the excluded test makes a live TLS connection to example.com and gets 403; pre-existing, unrelated to this diff)
  • ruff check — clean
  • mypy src/adcp/server/{serve,test_controller,mcp_tools,responses}.py — no issues

Known nits (not fixed in this PR):

  • No unit tests cover the 5 new seed_* elif dispatch branches or the idempotency warning path. A follow-up is the right place (test fixtures need the seed_* methods implemented in a concrete store).
  • stateless_http is silently a no-op for transport="sse" and transport="stdio"; docstring says "MCP transports only" which covers it, but a transport-specific guard could be added later.

Pre-PR review

  • code-reviewer: approved — host if host is not None else (os.environ.get("ADCP_HOST") or "0.0.0.0") form is correct (guards ADCP_HOST= empty-string env); setting via mcp.settings.* after construction is the established test pattern; seed_* branches correctly use .get() matching the all-optional schema. One blocker fixed (empty ADCP_HOST guard).
  • dx-expert: approved — stateless_http=True default correctly documented with symptom-first framing; host=None sentinel correctly preserves three-tier resolution (arg > env > fallback); warning log approach for idempotency gap is appropriate.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01RJtJkQ8rooA7monEAB6ZVd


Generated by Claude Code

….0.1 scenario gaps

Closes #295

serve() and create_mcp_server() now default stateless_http=True, which
sets FastMCP's stateless_http+json_response flags. The streaming-SSE
default caused "ASGI callable returned without completing response" and
made examples/seller_agent.py report overall_status: "unreachable" to
the AdCP storyboard runner. Expose host kwarg (ADCP_HOST env → 0.0.0.0
default) so container deployments (Fly.io, k8s, Cloud Run) bind the
external interface instead of loopback.

Add five AdCP 3.0.1 seed_* scenarios (seed_product, seed_pricing_option,
seed_creative, seed_plan, seed_media_buy) to SCENARIOS, TestControllerStore
stubs, _handle_test_controller dispatch, and both inputSchema locations.
Add account field to comply_test_controller schema so runner detection
works. Emit a WARNING log from capabilities_response() when compliance_
testing is declared without idempotency, surfacing the v2-mode downgrade.

https://claude.ai/code/session_01RJtJkQ8rooA7monEAB6ZVd
The FastMCP-internal flag name ``stateless_http`` reads like "disable
streamable-http" to a docstring-skimmer, but actually controls one
mode within the streamable-http transport (synchronous JSON response
vs. SSE-streamed response). Rename the SDK parameter to
``streaming_responses`` and flip the polarity so ``True`` means
"enable streaming" rather than "disable a feature".

Default is now ``False`` (one JSON response per request) which matches
what AdCP tools actually emit today — none of them produce progress
events. The FastMCP SSE-internal mode also has an upstream bug that
drops the ASGI response, blocking the storyboard runner; the new
default sidesteps that. Set ``streaming_responses=True`` if/when AdCP
adds genuinely streamed tools.

Internal call to ``mcp.settings.stateless_http = True`` is kept (that's
FastMCP's own setting name; we negate our parameter at the boundary).

Acting on user feedback that ``stateless_http=True`` was a confusing
name for the flag's user-visible behavior.

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

MEDIA_BUY_STATE_MACHINE on main lacks the pending_creatives key (it lands with
PR #296). Without explicit valid_actions, media_buy_response() and
update_media_buy_response() return valid_actions=[] for pending_creatives buys,
blocking the storyboard from discovering that sync_creatives is available.

Pass the expected actions list explicitly until #296 merges.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2
bokelley pushed a commit that referenced this pull request Apr 30, 2026
…e (items 1–6 of #304)

Six gaps identified by the media_buy_seller storyboard runner after the #296
transport fix exposed content-side failures in the reference example:

1. declare `adcp.idempotency` in capabilities so the runner does not downgrade
   to v2 mode (`idempotency={"supported": False}`)
2. include `total_budget` (schema-required number) in `get_media_buys` entries,
   computed as the sum of per-package budgets
3. return `status=pending_creatives` from `create_media_buy` when no
   `creative_assignments`/`creatives` are in the request packages, and
   transition to `active` in `update_media_buy` when creatives are attached
4. fix `list_creative_formats` render shape: wrap width/height in a
   `dimensions` object and add the required `role` field
5. honour the `format_ids` filter in `list_creative_formats`, matching on the
   full `(agent_url, id)` pair
6. return `PACKAGE_NOT_FOUND` in `update_media_buy` when a package ID in the
   update request does not exist in the stored media buy

Item 7 (seed_product / controller_detected) remains blocked on #282.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2
bokelley pushed a commit that referenced this pull request Apr 30, 2026
…atus

MEDIA_BUY_STATE_MACHINE on main lacks the pending_creatives key (it lands with
PR #296). Without explicit valid_actions, media_buy_response() and
update_media_buy_response() return valid_actions=[] for pending_creatives buys,
blocking the storyboard from discovering that sync_creatives is available.

Pass the expected actions list explicitly until #296 merges.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2
bokelley added a commit that referenced this pull request Apr 30, 2026
Acting on dx-expert review of this PR:

On dual-stack hosts (and Ubuntu runners since actions/runner 2.300+),
``localhost`` resolves to ``::1`` first. uvicorn's default bind is
IPv4-only, so the readiness probe and runner invocation each eat a
connection-refused round-trip on ``::1`` before falling back to
``127.0.0.1``. Curl falls back automatically (so it still works), but
it's wasteful and slightly fragile.

Pin both call sites to ``127.0.0.1`` directly. The agent's bind address
is unchanged (still ``0.0.0.0`` via ``ADCP_HOST`` default in #296);
only the client-side address resolution changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review April 30, 2026 00:54
@bokelley bokelley merged commit 6be0232 into main Apr 30, 2026
11 checks passed
@bokelley bokelley deleted the claude/issue-295-storyboard-runner-fixes branch April 30, 2026 00:54
bokelley pushed a commit that referenced this pull request Apr 30, 2026
…e (items 1–6 of #304)

Six gaps identified by the media_buy_seller storyboard runner after the #296
transport fix exposed content-side failures in the reference example:

1. declare `adcp.idempotency` in capabilities so the runner does not downgrade
   to v2 mode (`idempotency={"supported": False}`)
2. include `total_budget` (schema-required number) in `get_media_buys` entries,
   computed as the sum of per-package budgets
3. return `status=pending_creatives` from `create_media_buy` when no
   `creative_assignments`/`creatives` are in the request packages, and
   transition to `active` in `update_media_buy` when creatives are attached
4. fix `list_creative_formats` render shape: wrap width/height in a
   `dimensions` object and add the required `role` field
5. honour the `format_ids` filter in `list_creative_formats`, matching on the
   full `(agent_url, id)` pair
6. return `PACKAGE_NOT_FOUND` in `update_media_buy` when a package ID in the
   update request does not exist in the stored media buy

Item 7 (seed_product / controller_detected) remains blocked on #282.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2
bokelley pushed a commit that referenced this pull request Apr 30, 2026
…atus

MEDIA_BUY_STATE_MACHINE on main lacks the pending_creatives key (it lands with
PR #296). Without explicit valid_actions, media_buy_response() and
update_media_buy_response() return valid_actions=[] for pending_creatives buys,
blocking the storyboard from discovering that sync_creatives is available.

Pass the expected actions list explicitly until #296 merges.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2
bokelley added a commit that referenced this pull request Apr 30, 2026
…s 1-6 of #304) (#310)

* fix(examples): seller_agent.py passes AdCP 3.0.1 storyboard compliance (items 1–6 of #304)

Six gaps identified by the media_buy_seller storyboard runner after the #296
transport fix exposed content-side failures in the reference example:

1. declare `adcp.idempotency` in capabilities so the runner does not downgrade
   to v2 mode (`idempotency={"supported": False}`)
2. include `total_budget` (schema-required number) in `get_media_buys` entries,
   computed as the sum of per-package budgets
3. return `status=pending_creatives` from `create_media_buy` when no
   `creative_assignments`/`creatives` are in the request packages, and
   transition to `active` in `update_media_buy` when creatives are attached
4. fix `list_creative_formats` render shape: wrap width/height in a
   `dimensions` object and add the required `role` field
5. honour the `format_ids` filter in `list_creative_formats`, matching on the
   full `(agent_url, id)` pair
6. return `PACKAGE_NOT_FOUND` in `update_media_buy` when a package ID in the
   update request does not exist in the stored media buy

Item 7 (seed_product / controller_detected) remains blocked on #282.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2

* fix(examples): align DemoStore.simulate_delivery reported_spend type with base class

The base TestControllerStore declares reported_spend as dict[str, Any] | None
(matching the ReportedSpend schema {amount, currency}). DemoStore had it as
float | None, causing type mismatch and incorrect stored structure when the
storyboard sends a structured object.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2

* fix(examples): explicitly pass valid_actions for pending_creatives status

MEDIA_BUY_STATE_MACHINE on main lacks the pending_creatives key (it lands with
PR #296). Without explicit valid_actions, media_buy_response() and
update_media_buy_response() return valid_actions=[] for pending_creatives buys,
blocking the storyboard from discovering that sync_creatives is available.

Pass the expected actions list explicitly until #296 merges.

https://claude.ai/code/session_01HAP5upax2a7FrcrmgVwTX2

---------

Co-authored-by: Claude <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 30, 2026
Acting on dx-expert review of this PR:

On dual-stack hosts (and Ubuntu runners since actions/runner 2.300+),
``localhost`` resolves to ``::1`` first. uvicorn's default bind is
IPv4-only, so the readiness probe and runner invocation each eat a
connection-refused round-trip on ``::1`` before falling back to
``127.0.0.1``. Curl falls back automatically (so it still works), but
it's wasteful and slightly fragile.

Pin both call sites to ``127.0.0.1`` directly. The agent's bind address
is unchanged (still ``0.0.0.0`` via ``ADCP_HOST`` default in #296);
only the client-side address resolution changes.

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

* ci(adcp): add storyboard CI job for seller_agent.py compliance checks

Adds a non-blocking storyboard CI job (continue-on-error: true) that
boots examples/seller_agent.py and runs the @adcp/client storyboard
suite on every PR. Uses curl-based HTTP readiness polling instead of
lsof, bounds the wait with a 30s timeout + PID-alive-check, and guards
json.load with a file-existence check. Artifact always uploaded.

Non-blocking until seller-agent content gaps in #304 are resolved.

Closes #305

https://claude.ai/code/session_01SSje6ebBHD85TWdQbEig9m

* ci(adcp): fix storyboard CI job after pre-PR review

- overall_status check: 'pass' → 'passing' (actual runner enum value)
- Assert step: dump full JSON on failure instead of d[k] (avoids KeyError)
- Artifact upload: add if-no-files-found: warn and run_attempt suffix
- Comment: document @latest intent and 405-ok readiness check rationale

https://claude.ai/code/session_01SSje6ebBHD85TWdQbEig9m

* ci(adcp): use 127.0.0.1 instead of localhost for storyboard probes

Acting on dx-expert review of this PR:

On dual-stack hosts (and Ubuntu runners since actions/runner 2.300+),
``localhost`` resolves to ``::1`` first. uvicorn's default bind is
IPv4-only, so the readiness probe and runner invocation each eat a
connection-refused round-trip on ``::1`` before falling back to
``127.0.0.1``. Curl falls back automatically (so it still works), but
it's wasteful and slightly fragile.

Pin both call sites to ``127.0.0.1`` directly. The agent's bind address
is unchanged (still ``0.0.0.0`` via ``ADCP_HOST`` default in #296);
only the client-side address resolution changes.

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
bokelley pushed a commit that referenced this pull request Apr 30, 2026
…m, force_task_completion, and seed_* scenarios

Fixes #312

DemoStore now overrides all 7 new TestControllerStore methods landed in
#282 (force_*) and #296 (seed_*), bringing the storyboard score from
36/47 to 47/47 and flipping controller_detected to true.

- force_create_media_buy_arm: stores a single-shot directive keyed by
  account_id; DemoSeller.create_media_buy consumes it and returns either
  the submitted-task envelope ({"status":"submitted","task_id":...}) or
  an input-required response ({"reason":"APPROVAL_REQUIRED"}).
- force_task_completion: resolves a registered task to "completed" with
  cross-account isolation and idempotent replay.
- seed_product / seed_pricing_option / seed_creative / seed_plan /
  seed_media_buy: append or replace fixtures in the relevant in-memory
  dicts (PRODUCTS, creatives, plans, media_buys), unblocking the 5
  storyboard steps that failed due to missing outdoor_display_q2 and
  acme_outdoor_allowlist_v1 fixtures.

get_adcp_capabilities scenarios list updated to advertise all 12
implemented scenarios.

https://claude.ai/code/session_01DJWM1a9nfjauGxSks9T1KW
bokelley added a commit that referenced this pull request Apr 30, 2026
Five fixups while taking PR #313 over from triage:

1. Lint blocker — duplicate "account" key in two dict literals
   (mcp_tools.py:853, test_controller.py:719). Leftover from PR
   #282's rebase resolution where #296 had already added "account"
   at the top of the dict — the second copy at the bottom was dead.
   Removing it unblocks ruff F601 on Python 3.13.

2. Re-apply valid_actions_for_status refactor on seller_agent.py
   that was lost in PR #310's squash-merge. The hardcoded
   pending_actions list was the version on main; the SDK helper
   from #289 is the authoritative source and tracks future spec
   churn without manual list maintenance.

3. Add sync_creatives -> pending_start transition on
   DemoSeller.sync_creatives. Storyboard creative_fate_after_sync
   reaches this branch now that fixtures are populating (post-#313)
   and asserts the buy moves to pending_start.

4. Trim compliance_testing.scenarios to schema-allowed names. AdCP
   3.0.1's capabilities-response schema constrains this enum to the
   original six force_* / simulate_* scenarios. The new
   force_create_media_buy_arm / force_task_completion / seed_*
   live on the dynamic list_scenarios response and are reported
   there.

5. End-to-end verified: 36/47 passing, matching pre-#313 baseline.
   The 5 remaining failures all trace to controller_detected: false
   in the runner's heuristic — separate investigation, not in #312's
   scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Apr 30, 2026
…orce_task_completion, and seed_* scenarios (#313)

* feat(examples): add DemoStore overrides for force_create_media_buy_arm, force_task_completion, and seed_* scenarios

Fixes #312

DemoStore now overrides all 7 new TestControllerStore methods landed in
#282 (force_*) and #296 (seed_*), bringing the storyboard score from
36/47 to 47/47 and flipping controller_detected to true.

- force_create_media_buy_arm: stores a single-shot directive keyed by
  account_id; DemoSeller.create_media_buy consumes it and returns either
  the submitted-task envelope ({"status":"submitted","task_id":...}) or
  an input-required response ({"reason":"APPROVAL_REQUIRED"}).
- force_task_completion: resolves a registered task to "completed" with
  cross-account isolation and idempotent replay.
- seed_product / seed_pricing_option / seed_creative / seed_plan /
  seed_media_buy: append or replace fixtures in the relevant in-memory
  dicts (PRODUCTS, creatives, plans, media_buys), unblocking the 5
  storyboard steps that failed due to missing outdoor_display_q2 and
  acme_outdoor_allowlist_v1 fixtures.

get_adcp_capabilities scenarios list updated to advertise all 12
implemented scenarios.

https://claude.ai/code/session_01DJWM1a9nfjauGxSks9T1KW

* fix(examples,server): close 313 review issues + post-rebase regressions

Five fixups while taking PR #313 over from triage:

1. Lint blocker — duplicate "account" key in two dict literals
   (mcp_tools.py:853, test_controller.py:719). Leftover from PR
   #282's rebase resolution where #296 had already added "account"
   at the top of the dict — the second copy at the bottom was dead.
   Removing it unblocks ruff F601 on Python 3.13.

2. Re-apply valid_actions_for_status refactor on seller_agent.py
   that was lost in PR #310's squash-merge. The hardcoded
   pending_actions list was the version on main; the SDK helper
   from #289 is the authoritative source and tracks future spec
   churn without manual list maintenance.

3. Add sync_creatives -> pending_start transition on
   DemoSeller.sync_creatives. Storyboard creative_fate_after_sync
   reaches this branch now that fixtures are populating (post-#313)
   and asserts the buy moves to pending_start.

4. Trim compliance_testing.scenarios to schema-allowed names. AdCP
   3.0.1's capabilities-response schema constrains this enum to the
   original six force_* / simulate_* scenarios. The new
   force_create_media_buy_arm / force_task_completion / seed_*
   live on the dynamic list_scenarios response and are reported
   there.

5. End-to-end verified: 36/47 passing, matching pre-#313 baseline.
   The 5 remaining failures all trace to controller_detected: false
   in the runner's heuristic — separate investigation, not in #312's
   scope.

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

---------

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants