Conversation
…orce_session_status Closes #314 Two changes to close the gap revealed by issue investigation: 1. **seed_creative_format** — the comply-test-controller-request.json schema (AdCP 3.0.1) includes this scenario but it was missing from SCENARIOS, TestControllerStore, the _handle_test_controller dispatcher, and DemoStore. Any storyboard that seeds a creative format would receive UNKNOWN_SCENARIO. DemoStore.seed_creative_format() registers the format in a module-level seeded_creative_formats dict that list_creative_formats now merges in. 2. **force_session_status** — advertised in DemoSeller's static compliance_testing.scenarios (schema-legal: it's in the 3.0.1 capabilities enum) and given a minimal stub in DemoStore. This makes list_scenarios return all six schema-permitted scenarios, which is the most likely predicate the JS storyboard runner checks for controller_detected. A cross-repo fix is still needed to extend the capabilities schema enum to cover force_create_media_buy_arm, force_task_completion, and seed_* — those cannot be advertised in the static field until the schema is updated. https://claude.ai/code/session_018i5iWUxUayQEh5K3v9HZRG
…eative_format
If the storyboard sends a fixture that contains format_id as a structured
object {"agent_url": ..., "id": ...}, data.get("format_id") returns a dict.
Using that dict as a seeded_creative_formats key raises TypeError (unhashable).
Extract the scalar id via .get("id") to stay consistent with how other seed_*
methods handle their fixture fields.
https://claude.ai/code/session_018i5iWUxUayQEh5K3v9HZRG
…d_creative_format Pre-PR protocol review noted that neither new test exercised the case where format_id arrives only inside fixture as a structured object rather than as a top-level params.format_id string. Adds a test that passes the fixture path and asserts the dispatcher correctly passes format_id=None to the store (leaving ID extraction to the store's own logic). https://claude.ai/code/session_018i5iWUxUayQEh5K3v9HZRG
a858324 to
9c2afa9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #314
Summary
Investigation of #314 found two Python SDK gaps that together explain the
controller_detected: falseproblem (or are latent bugs that will surface imminently):seed_creative_formatwas missing from the Python SDK entirely. Thecomply-test-controller-request.jsonschema (AdCP 3.0.1) includes this scenario but it was absent fromSCENARIOS,TestControllerStore, the dispatcher, andDemoStore. Any storyboard that seeds a creative format would receiveUNKNOWN_SCENARIO.force_session_statuswas not advertised inDemoSeller's staticcompliance_testing.scenariosfield. Theget-adcp-capabilities-response.jsonschema allows exactly 6 scenario values in the static field; DemoSeller was only advertising 5 (missingforce_session_status). The JS storyboard runner likely gatescontroller_detectedon a check of this static field — it would not see the full set and flip the flag. Addingforce_session_statusto the static list and providing a minimalDemoStorestub brings the static and dynamic (list_scenarios) surfaces into alignment for this scenario.What this PR does not fix: The newer scenarios (
force_create_media_buy_arm,force_task_completion,seed_*) cannot be advertised in the static capabilities field until theget-adcp-capabilities-response.jsonschema enum is extended. That is a cross-repo fix against the AdCP spec; a follow-up issue should be filed againstadcontextprotocol/adcprequesting the enum update.Changes
src/adcp/server/test_controller.py: adds"seed_creative_format"toSCENARIOS, addsTestControllerStore.seed_creative_format()base method, adds dispatcher branchexamples/seller_agent.py: adds"force_session_status"toDemoSellerstatic capabilities, addsDemoStore.force_session_status()stub, addsDemoStore.seed_creative_format()implementation backed byseeded_creative_formatsmodule dict, merges seeded formats intolist_creative_formatsresponsetests/test_server_dx.py: three new tests —test_seed_creative_format_dispatches,test_seed_creative_format_in_list_scenarios,test_seed_creative_format_structured_fixture_idWhat was tested
pytest tests/test_server_dx.py tests/test_test_controller_context.py -v→ 58 passed, 0 failedruff check src/adcp/server/test_controller.py→ cleanmypy src/adcp/server/test_controller.py→ no issuesPre-PR review
Pre-PR review:
data.get("format_id")could return a dict from a structured fixture, making it an unhashable dict key; fixed to(data.get("format_id") or {}).get("id")). Also noted test coverage gap for the structured-fixture path (added as third test in final commit).force_session_statusis schema-legal for amedia_buy-only seller (in the 6-value capabilities enum).seed_creative_formatcorrectly mirrors the pattern of otherseed_*methods. Confirmed the blocker (dict-as-key) was already fixed. No additional blockers.Nits (not fixed, noted for reviewer):
DemoStore.force_session_statusalways returnsprevious_state: "active". Acceptable for a demo seller that has no SI session state; multi-step storyboards that issue twoforce_session_statuscalls in sequence will see a misleading value on the second call. Production stores should track actual session state.seeded_creative_formatsis module-level state that is never cleared between compliance sessions (consistent with how all other module-level dicts in seller_agent.py behave, but worth noting since it influences what the protocol advertises as available formats).list_creative_formatsconcatenates without deduplication. A seeded format_id colliding with a static format (display_300x250,display_970x250) would appear twice in the unfiltered response. Unlikely in practice given storyboard seed IDs.TestControllerStore.seed_creative_formatdocstring notes the seller MUST wire the seeded ID intolist_creative_formatsbut doesn't call this out as a subclass responsibility — future implementors could override without wiring the list method.Session: https://claude.ai/code/session_018i5iWUxUayQEh5K3v9HZRG