From 3b740d84fcef53ac3d331324b91ea94491d387d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Apr 2026 23:23:53 +0000 Subject: [PATCH 1/3] feat(server): add force_create_media_buy_arm + force_task_completion controller scenarios Adds two new comply_test_controller scenarios for AdCP 3.0.1 storyboard parity. Sellers running the create_media_buy_async.yaml storyboard suite against a Python reference seller now grade `passing` rather than `not_applicable` on the submitted-arm phase. - Adds `force_create_media_buy_arm` and `force_task_completion` to SCENARIOS, TestControllerStore abstract base, and the dispatcher in _handle_test_controller. - Validates arm enum, conditional task_id-when-submitted, char limits, 256 KB result cap, and whitespace task_id stripping. - Updates register_test_controller inline schema (derived from SCENARIOS to prevent drift) and mcp_tools.py ADCP_TOOL_DEFINITIONS enum to include both. - Adds account field to both inline schemas so storyboard runners can drive cross-account isolation. - 20 new tests at parity with Node training-agent nine-test pattern. Closes #281 https://claude.ai/code/session_01KaGEJKsjnTEuLF6qnaRFqQ --- src/adcp/server/mcp_tools.py | 3 + src/adcp/server/test_controller.py | 176 ++++++++- ...media_buy_arm_and_force_task_completion.py | 363 ++++++++++++++++++ 3 files changed, 527 insertions(+), 15 deletions(-) create mode 100644 tests/test_force_create_media_buy_arm_and_force_task_completion.py diff --git a/src/adcp/server/mcp_tools.py b/src/adcp/server/mcp_tools.py index 0ec00f00..a3343b3e 100644 --- a/src/adcp/server/mcp_tools.py +++ b/src/adcp/server/mcp_tools.py @@ -847,6 +847,8 @@ "force_creative_status", "force_account_status", "force_media_buy_status", + "force_create_media_buy_arm", + "force_task_completion", "force_session_status", "simulate_delivery", "simulate_budget_spend", @@ -858,6 +860,7 @@ ], }, "params": {"type": "object"}, + "account": {"type": "object"}, "context": {"type": "object"}, }, "required": ["scenario"], diff --git a/src/adcp/server/test_controller.py b/src/adcp/server/test_controller.py index 94470b94..f9cc640d 100644 --- a/src/adcp/server/test_controller.py +++ b/src/adcp/server/test_controller.py @@ -49,6 +49,8 @@ async def force_account_status(self, account_id, status): "force_creative_status", "force_account_status", "force_media_buy_status", + "force_create_media_buy_arm", + "force_task_completion", "force_session_status", "simulate_delivery", "simulate_budget_spend", @@ -60,6 +62,10 @@ async def force_account_status(self, account_id, status): "seed_media_buy", ] +_MAX_TASK_ID = 128 +_MAX_MESSAGE = 2000 +_MAX_RESULT_BYTES = 256 * 1024 # 256 KB soft cap per AdCP 3.0.1 + class TestControllerError(Exception): """Typed error for test controller store methods. @@ -165,6 +171,89 @@ async def force_session_status( """ raise NotImplementedError + async def force_create_media_buy_arm( + self, + arm: str, + task_id: str | None = None, + message: str | None = None, + *, + account: dict[str, Any] | None = None, + context: ToolContext | None = None, + ) -> dict[str, Any]: + """Register a single-shot directive for the next create_media_buy call. + + The directive is consumed by the next create_media_buy call from the + same authenticated sandbox account, then cleared. A second registration + before consumption overwrites the first. + + Args: + arm: Response arm — ``'submitted'`` or ``'input-required'``. + task_id: Required when ``arm='submitted'``. The seller MUST emit + this exact value on the next create_media_buy task envelope + and accept it on subsequent tasks/get calls within the same + sandbox account. Max 128 chars. + message: Optional plain-text note surfaced on the response. + Max 2000 chars. + account: Caller-supplied account object from the MCP request. + Implementations use this for single-shot-per-account isolation. + context: Optional ToolContext from the server's context_factory. + + Returns: + ForcedDirectiveSuccess:: + + {"success": True, "forced": {"arm": str, "task_id"?: str}} + + Raises: + TestControllerError: with code ``"NOT_FOUND"`` if the caller + account is not recognized, or ``"INVALID_PARAMS"`` on + validation failure. + """ + raise NotImplementedError + + async def force_task_completion( + self, + task_id: str, + result: dict[str, Any], + *, + account: dict[str, Any] | None = None, + context: ToolContext | None = None, + ) -> dict[str, Any]: + """Resolve a previously-submitted task to ``'completed'``. + + Isolation and idempotency contract: + + - **Cross-account replay** — raise ``TestControllerError("NOT_FOUND", ...)`` + when the task_id was registered by a different sandbox account. + - **Identical-params replay** — idempotent; return the same + ``StateTransitionSuccess``. + - **Diverging-params replay** against a terminal task — raise + ``TestControllerError("INVALID_TRANSITION", ..., + current_state="completed")``. + + Args: + task_id: Task handle to resolve. Max 128 chars. + result: Completion payload (non-empty object). Implementations + SHOULD validate it against the response branch for the task's + original method and MUST reject payloads that fail that check + with ``TestControllerError("INVALID_PARAMS", ...)``. + account: Caller-supplied account object from the MCP request. + Used for cross-account isolation. + context: Optional ToolContext from the server's context_factory. + + Returns: + StateTransitionSuccess:: + + {"success": True, "previous_state": "submitted", + "current_state": "completed"} + + Raises: + TestControllerError: with code ``"NOT_FOUND"`` if the task_id + is unknown or owned by a different account, + ``"INVALID_TRANSITION"`` if the task is already terminal and + params diverge, or ``"INVALID_PARAMS"`` on validation failure. + """ + raise NotImplementedError + async def simulate_delivery( self, media_buy_id: str, @@ -414,6 +503,73 @@ async def _handle_test_controller( termination_reason=scenario_params.get("termination_reason"), **extra, ) + elif scenario == "force_create_media_buy_arm": + arm = scenario_params.get("arm") or "" + if arm not in ("submitted", "input-required"): + return _controller_error( + "INVALID_PARAMS", + "arm must be 'submitted' or 'input-required'", + ) + raw_task_id = scenario_params.get("task_id") + task_id: str | None = ( + raw_task_id.strip() if isinstance(raw_task_id, str) else None + ) + if not task_id: + task_id = None + if arm == "submitted" and not task_id: + return _controller_error( + "INVALID_PARAMS", + "task_id is required when arm is 'submitted'", + ) + if task_id and len(task_id) > _MAX_TASK_ID: + return _controller_error( + "INVALID_PARAMS", + f"task_id must be at most {_MAX_TASK_ID} characters", + ) + message = scenario_params.get("message") + if message is not None and len(str(message)) > _MAX_MESSAGE: + return _controller_error( + "INVALID_PARAMS", + f"message must be at most {_MAX_MESSAGE} characters", + ) + result = await method( + arm=arm, + task_id=task_id, + message=message, + account=params.get("account"), + **extra, + ) + elif scenario == "force_task_completion": + raw_task_id = scenario_params.get("task_id") + task_id = raw_task_id.strip() if isinstance(raw_task_id, str) else None + if not task_id: + return _controller_error( + "INVALID_PARAMS", + "Missing required parameter: 'task_id'", + ) + if len(task_id) > _MAX_TASK_ID: + return _controller_error( + "INVALID_PARAMS", + f"task_id must be at most {_MAX_TASK_ID} characters", + ) + result_value = scenario_params.get("result") + if not isinstance(result_value, dict) or not result_value: + return _controller_error( + "INVALID_PARAMS", + "result must be a non-empty object", + ) + result_bytes = len(json.dumps(result_value).encode("utf-8")) + if result_bytes > _MAX_RESULT_BYTES: + return _controller_error( + "INVALID_PARAMS", + f"result payload exceeds {_MAX_RESULT_BYTES // 1024} KB limit", + ) + result = await method( + task_id=task_id, + result=result_value, + account=params.get("account"), + **extra, + ) elif scenario == "simulate_delivery": result = await method( media_buy_id=scenario_params["media_buy_id"], @@ -546,29 +702,19 @@ async def comply_test_controller(**kwargs: Any) -> str: description="Compliance test controller. Sandbox only, not for production use.", ) - # Override schema with the proper comply_test_controller inputSchema + # Override schema with the proper comply_test_controller inputSchema. + # Derived from SCENARIOS so it can't drift from the dispatcher. tool.parameters = { "type": "object", "properties": { "account": {"type": "object"}, "scenario": { "type": "string", - "enum": [ - "list_scenarios", - "force_creative_status", - "force_account_status", - "force_media_buy_status", - "force_session_status", - "simulate_delivery", - "simulate_budget_spend", - "seed_product", - "seed_pricing_option", - "seed_creative", - "seed_plan", - "seed_media_buy", - ], + # Derived from SCENARIOS so the enum never drifts from the dispatcher. + "enum": ["list_scenarios"] + SCENARIOS, }, "params": {"type": "object"}, + "account": {"type": "object"}, "context": {"type": "object"}, }, "required": ["scenario"], diff --git a/tests/test_force_create_media_buy_arm_and_force_task_completion.py b/tests/test_force_create_media_buy_arm_and_force_task_completion.py new file mode 100644 index 00000000..987e5048 --- /dev/null +++ b/tests/test_force_create_media_buy_arm_and_force_task_completion.py @@ -0,0 +1,363 @@ +"""Tests for force_create_media_buy_arm and force_task_completion. + +Parity with adcp/server/tests/unit/training-agent-force-create-media-buy-arm.test.ts +and training-agent-force-task-completion.test.ts (adcp#3115, adcp#3194). + +Covers: valid registration, INVALID_PARAMS branches, replay idempotency, +diverging-replay INVALID_TRANSITION, cross-account isolation, and +list_scenarios advertisement. +""" + +from __future__ import annotations + +import json +from typing import Any + +import pytest + +from adcp.server.test_controller import ( + TestControllerError, + TestControllerStore, + _handle_test_controller, +) + +# --------------------------------------------------------------------------- +# Concrete store implementations for tests +# --------------------------------------------------------------------------- + +_ACCOUNT_A = {"id": "acct-a"} +_ACCOUNT_B = {"id": "acct-b"} + + +class _ArmStore(TestControllerStore): + """Stores a single pending force_create_media_buy_arm directive per + account. A second registration overwrites; it is consumed (cleared) + by the first create_media_buy call (not tested here — only the + registration side is in scope for this PR).""" + + def __init__(self) -> None: + self._directives: dict[str, dict[str, Any]] = {} + + async def force_create_media_buy_arm( + self, + arm: str, + task_id: str | None = None, + message: str | None = None, + *, + account: dict[str, Any] | None = None, + context: Any = None, + ) -> dict[str, Any]: + key = (account or {}).get("id", "__default__") + directive: dict[str, Any] = {"arm": arm} + if task_id is not None: + directive["task_id"] = task_id + self._directives[key] = directive + forced: dict[str, Any] = {"arm": arm} + if task_id is not None: + forced["task_id"] = task_id + return {"success": True, "forced": forced} + + +class _CompletionStore(TestControllerStore): + """Stores force_task_completion records with cross-account isolation, + idempotency, and INVALID_TRANSITION on diverging replay.""" + + def __init__(self) -> None: + # {task_id: {"result": ..., "owner": str}} + self._tasks: dict[str, dict[str, Any]] = {} + + async def force_task_completion( + self, + task_id: str, + result: dict[str, Any], + *, + account: dict[str, Any] | None = None, + context: Any = None, + ) -> dict[str, Any]: + owner = (account or {}).get("id", "__default__") + if task_id in self._tasks: + stored = self._tasks[task_id] + if stored["owner"] != owner: + raise TestControllerError("NOT_FOUND", f"task {task_id!r} not found") + if stored["result"] == result: + # Identical-params replay — idempotent + return {"success": True, "previous_state": "submitted", "current_state": "completed"} + raise TestControllerError( + "INVALID_TRANSITION", + f"task {task_id!r} already completed with different result", + current_state="completed", + ) + self._tasks[task_id] = {"result": result, "owner": owner} + return {"success": True, "previous_state": "submitted", "current_state": "completed"} + + +class _BothStore(_ArmStore, _CompletionStore): + """Implements both new scenarios for list_scenarios tests.""" + + def __init__(self) -> None: + _ArmStore.__init__(self) + _CompletionStore.__init__(self) + + +# --------------------------------------------------------------------------- +# Helper +# --------------------------------------------------------------------------- + + +def _arm_req(params: dict[str, Any], account: dict[str, Any] | None = None) -> dict[str, Any]: + req: dict[str, Any] = {"scenario": "force_create_media_buy_arm", "params": params} + if account is not None: + req["account"] = account + return req + + +def _completion_req( + params: dict[str, Any], account: dict[str, Any] | None = None +) -> dict[str, Any]: + req: dict[str, Any] = {"scenario": "force_task_completion", "params": params} + if account is not None: + req["account"] = account + return req + + +# =========================================================================== +# force_create_media_buy_arm +# =========================================================================== + + +@pytest.mark.asyncio +async def test_arm_submitted_valid_registration() -> None: + store = _ArmStore() + resp = await _handle_test_controller( + store, + _arm_req({"arm": "submitted", "task_id": "task-abc"}, account=_ACCOUNT_A), + ) + assert resp["success"] is True + assert resp["forced"]["arm"] == "submitted" + assert resp["forced"]["task_id"] == "task-abc" + + +@pytest.mark.asyncio +async def test_arm_input_required_no_task_id() -> None: + store = _ArmStore() + resp = await _handle_test_controller( + store, + _arm_req({"arm": "input-required"}, account=_ACCOUNT_A), + ) + assert resp["success"] is True + assert resp["forced"]["arm"] == "input-required" + assert "task_id" not in resp["forced"] + + +@pytest.mark.asyncio +async def test_arm_overwrite_before_consumption() -> None: + """A second registration before consumption overwrites the first.""" + store = _ArmStore() + await _handle_test_controller( + store, + _arm_req({"arm": "submitted", "task_id": "task-first"}, account=_ACCOUNT_A), + ) + resp = await _handle_test_controller( + store, + _arm_req({"arm": "submitted", "task_id": "task-second"}, account=_ACCOUNT_A), + ) + assert resp["success"] is True + assert resp["forced"]["task_id"] == "task-second" + assert store._directives[_ACCOUNT_A["id"]]["task_id"] == "task-second" + + +@pytest.mark.asyncio +async def test_arm_invalid_params_missing_arm() -> None: + store = _ArmStore() + resp = await _handle_test_controller(store, _arm_req({})) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + + +@pytest.mark.asyncio +async def test_arm_invalid_params_bad_arm_value() -> None: + store = _ArmStore() + resp = await _handle_test_controller(store, _arm_req({"arm": "completed"})) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + + +@pytest.mark.asyncio +async def test_arm_task_id_required_when_submitted() -> None: + store = _ArmStore() + resp = await _handle_test_controller(store, _arm_req({"arm": "submitted"})) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "task_id" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_arm_task_id_too_long() -> None: + store = _ArmStore() + resp = await _handle_test_controller( + store, _arm_req({"arm": "submitted", "task_id": "x" * 129}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "128" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_arm_message_too_long() -> None: + store = _ArmStore() + resp = await _handle_test_controller( + store, + _arm_req({"arm": "input-required", "message": "m" * 2001}), + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "2000" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_arm_whitespace_task_id_treated_as_missing() -> None: + """A whitespace-only task_id is stripped to empty, then treated as absent.""" + store = _ArmStore() + resp = await _handle_test_controller( + store, _arm_req({"arm": "submitted", "task_id": " "}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + + +# =========================================================================== +# force_task_completion +# =========================================================================== + +_GOOD_RESULT = {"media_buy_id": "mb-1", "packages": []} + + +@pytest.mark.asyncio +async def test_completion_valid() -> None: + store = _CompletionStore() + resp = await _handle_test_controller( + store, + _completion_req({"task_id": "task-1", "result": _GOOD_RESULT}, account=_ACCOUNT_A), + ) + assert resp["success"] is True + assert resp["previous_state"] == "submitted" + assert resp["current_state"] == "completed" + + +@pytest.mark.asyncio +async def test_completion_idempotent_same_params() -> None: + store = _CompletionStore() + params = {"task_id": "task-1", "result": _GOOD_RESULT} + await _handle_test_controller(store, _completion_req(params, account=_ACCOUNT_A)) + resp = await _handle_test_controller(store, _completion_req(params, account=_ACCOUNT_A)) + assert resp["success"] is True + assert resp["current_state"] == "completed" + + +@pytest.mark.asyncio +async def test_completion_diverging_replay_invalid_transition() -> None: + store = _CompletionStore() + await _handle_test_controller( + store, + _completion_req({"task_id": "task-1", "result": _GOOD_RESULT}, account=_ACCOUNT_A), + ) + different_result = {"media_buy_id": "mb-2", "packages": []} + resp = await _handle_test_controller( + store, + _completion_req({"task_id": "task-1", "result": different_result}, account=_ACCOUNT_A), + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_TRANSITION" + assert resp.get("current_state") == "completed" + + +@pytest.mark.asyncio +async def test_completion_cross_account_not_found() -> None: + store = _CompletionStore() + await _handle_test_controller( + store, + _completion_req({"task_id": "task-1", "result": _GOOD_RESULT}, account=_ACCOUNT_A), + ) + resp = await _handle_test_controller( + store, + _completion_req({"task_id": "task-1", "result": _GOOD_RESULT}, account=_ACCOUNT_B), + ) + assert resp["success"] is False + assert resp["error"] == "NOT_FOUND" + + +@pytest.mark.asyncio +async def test_completion_missing_task_id() -> None: + store = _CompletionStore() + resp = await _handle_test_controller( + store, _completion_req({"result": _GOOD_RESULT}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "task_id" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_completion_empty_result_object() -> None: + store = _CompletionStore() + resp = await _handle_test_controller( + store, _completion_req({"task_id": "task-1", "result": {}}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "non-empty" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_completion_task_id_too_long() -> None: + store = _CompletionStore() + resp = await _handle_test_controller( + store, _completion_req({"task_id": "t" * 129, "result": _GOOD_RESULT}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "128" in resp["error_detail"] + + +@pytest.mark.asyncio +async def test_completion_result_too_large() -> None: + store = _CompletionStore() + large_result = {"data": "x" * (256 * 1024 + 1)} + resp = await _handle_test_controller( + store, _completion_req({"task_id": "task-1", "result": large_result}) + ) + assert resp["success"] is False + assert resp["error"] == "INVALID_PARAMS" + assert "256" in resp["error_detail"] + + +# =========================================================================== +# list_scenarios advertisement +# =========================================================================== + + +@pytest.mark.asyncio +async def test_list_scenarios_both_implemented() -> None: + store = _BothStore() + resp = await _handle_test_controller(store, {"scenario": "list_scenarios"}) + assert resp["success"] is True + assert "force_create_media_buy_arm" in resp["scenarios"] + assert "force_task_completion" in resp["scenarios"] + + +@pytest.mark.asyncio +async def test_list_scenarios_neither_implemented() -> None: + store = TestControllerStore() + resp = await _handle_test_controller(store, {"scenario": "list_scenarios"}) + assert resp["success"] is True + assert "force_create_media_buy_arm" not in resp["scenarios"] + assert "force_task_completion" not in resp["scenarios"] + + +@pytest.mark.asyncio +async def test_list_scenarios_partial_only_arm() -> None: + store = _ArmStore() + resp = await _handle_test_controller(store, {"scenario": "list_scenarios"}) + assert resp["success"] is True + assert "force_create_media_buy_arm" in resp["scenarios"] + assert "force_task_completion" not in resp["scenarios"] From f0189e5bc202f0fb5e4129c70eb230f18e18881a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Apr 2026 23:31:24 +0000 Subject: [PATCH 2/3] fix(server): strip task_id for input-required arm; add coverage test Forced.extra='forbid' in the comply_test_controller response schema means a store that echoes task_id on arm='input-required' would produce an invalid Forced object. The dispatcher now nullifies task_id before the store call when arm='input-required', preventing protocol drift regardless of store implementation. Adds one test: test_arm_task_id_stripped_for_input_required. https://claude.ai/code/session_01KaGEJKsjnTEuLF6qnaRFqQ --- src/adcp/server/test_controller.py | 5 +++++ ...e_media_buy_arm_and_force_task_completion.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/adcp/server/test_controller.py b/src/adcp/server/test_controller.py index f9cc640d..d64d491b 100644 --- a/src/adcp/server/test_controller.py +++ b/src/adcp/server/test_controller.py @@ -526,6 +526,11 @@ async def _handle_test_controller( "INVALID_PARAMS", f"task_id must be at most {_MAX_TASK_ID} characters", ) + # Forced.task_id is only valid for arm='submitted'; strip it for + # 'input-required' so stores can't inadvertently echo it into the + # Forced object (which has extra="forbid" in the response schema). + if arm == "input-required": + task_id = None message = scenario_params.get("message") if message is not None and len(str(message)) > _MAX_MESSAGE: return _controller_error( diff --git a/tests/test_force_create_media_buy_arm_and_force_task_completion.py b/tests/test_force_create_media_buy_arm_and_force_task_completion.py index 987e5048..dabaf7cc 100644 --- a/tests/test_force_create_media_buy_arm_and_force_task_completion.py +++ b/tests/test_force_create_media_buy_arm_and_force_task_completion.py @@ -225,6 +225,23 @@ async def test_arm_whitespace_task_id_treated_as_missing() -> None: assert resp["error"] == "INVALID_PARAMS" +@pytest.mark.asyncio +async def test_arm_task_id_stripped_for_input_required() -> None: + """task_id is silently dropped for arm='input-required'. + + Forced.extra='forbid' in the response schema means a store that echoes + task_id on input-required would produce an invalid response. The + dispatcher strips it before calling the store. + """ + store = _ArmStore() + resp = await _handle_test_controller( + store, + _arm_req({"arm": "input-required", "task_id": "should-be-dropped"}, account=_ACCOUNT_A), + ) + assert resp["success"] is True + assert "task_id" not in resp["forced"] + + # =========================================================================== # force_task_completion # =========================================================================== From fdf558868d7afa335089bbda7f32d2322a4d3f1b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Apr 2026 23:35:44 +0000 Subject: [PATCH 3/3] refactor(server): address pre-PR review feedback - Extract _accepts_kwarg(method, name) so both context and account pass-through share one signature-inspection impl; _accepts_context_kwarg delegates to it. - Gate account kwarg via _accepts_kwarg in the shared `extra` dict so stores that omit account= don't receive an unexpected keyword and silently fall to INTERNAL_ERROR. - Replace len(str(message)) guard with isinstance + len for consistency with task_id handling. - Import SCENARIOS from test_controller in mcp_tools.py so the comply_test_controller inputSchema enum is always derived from the canonical list and can't drift on the next scenario addition. https://claude.ai/code/session_01KaGEJKsjnTEuLF6qnaRFqQ --- src/adcp/server/mcp_tools.py | 22 ++++---------- src/adcp/server/test_controller.py | 49 ++++++++++++++---------------- 2 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/adcp/server/mcp_tools.py b/src/adcp/server/mcp_tools.py index a3343b3e..bc0642b2 100644 --- a/src/adcp/server/mcp_tools.py +++ b/src/adcp/server/mcp_tools.py @@ -24,6 +24,7 @@ from typing import Any from adcp.server.base import ADCPHandler, ToolContext +from adcp.server.test_controller import SCENARIOS as _CONTROLLER_SCENARIOS from adcp.validation.client_hooks import ValidationHookConfig logger = logging.getLogger(__name__) @@ -842,22 +843,11 @@ "account": {"type": "object"}, "scenario": { "type": "string", - "enum": [ - "list_scenarios", - "force_creative_status", - "force_account_status", - "force_media_buy_status", - "force_create_media_buy_arm", - "force_task_completion", - "force_session_status", - "simulate_delivery", - "simulate_budget_spend", - "seed_product", - "seed_pricing_option", - "seed_creative", - "seed_plan", - "seed_media_buy", - ], + # Derived from test_controller.SCENARIOS so the static stub + # matches the dispatcher; the Pydantic-generated path also + # carries the new names because #292 ships them in the + # comply-test-controller-request schema. + "enum": ["list_scenarios"] + _CONTROLLER_SCENARIOS, }, "params": {"type": "object"}, "account": {"type": "object"}, diff --git a/src/adcp/server/test_controller.py b/src/adcp/server/test_controller.py index d64d491b..5d9f166c 100644 --- a/src/adcp/server/test_controller.py +++ b/src/adcp/server/test_controller.py @@ -389,34 +389,23 @@ def _controller_error(error: str, detail: str, current_state: str | None = None) return resp -def _accepts_context_kwarg(method: Any) -> bool: - """True when ``method``'s signature accepts ``context=`` by keyword. +def _accepts_kwarg(method: Any, name: str) -> bool: + """True when ``method``'s signature accepts ``name`` as a keyword argument. - TestControllerStore subclasses written against the original API - (pre-#227) don't declare ``context``; passing it would raise - ``TypeError`` at the call site. Signature inspection keeps the - dispatcher backward-compatible while letting stores opt in to - header-driven context by simply adding ``context=None`` to their - override. + Used by the dispatcher to decide whether to pass optional kwargs + (``context``, ``account``) to store methods. Methods that don't + declare the kwarg keep working unchanged; methods that do get the + value threaded in. Counts as an opt-in: - - ``*, context: ...`` — keyword-only (the documented recipe). - - ``context: ...`` as a regular positional-or-keyword parameter. - - ``**kwargs`` — accepts any keyword, including ``context``. + - ``*, name: ...`` — keyword-only (the documented recipe). + - ``name: ...`` as a regular positional-or-keyword parameter. + - ``**kwargs`` — accepts any keyword. Does **not** count: - - ``context`` as positional-only (before ``/``) — passing by - keyword raises ``TypeError``. - - ``context`` as ``*args`` (it's never a variadic positional). - - Caveat: ``inspect.signature`` follows ``__wrapped__`` set by - ``@functools.wraps``. A decorator that wraps a legacy store method - and exposes the legacy signature will look "not opted in" even if - the wrapper itself would accept ``context``. This matches the - behavior callers expect — the wrapped callable signature is the - authoritative contract. + - ``name`` as positional-only (before ``/``). """ try: sig = inspect.signature(method) @@ -429,11 +418,16 @@ def _accepts_context_kwarg(method: Any) -> bool: for param in sig.parameters.values(): if param.kind == inspect.Parameter.VAR_KEYWORD: return True - if param.name == "context" and param.kind in allowed: + if param.name == name and param.kind in allowed: return True return False +def _accepts_context_kwarg(method: Any) -> bool: + """True when ``method``'s signature accepts ``context=`` by keyword.""" + return _accepts_kwarg(method, "context") + + async def _handle_test_controller( store: TestControllerStore, params: dict[str, Any], @@ -474,6 +468,9 @@ async def _handle_test_controller( extra: dict[str, Any] = {} if context is not None and _accepts_context_kwarg(method): extra["context"] = context + account = params.get("account") + if account is not None and _accepts_kwarg(method, "account"): + extra["account"] = account try: if scenario == "force_creative_status": @@ -532,16 +529,17 @@ async def _handle_test_controller( if arm == "input-required": task_id = None message = scenario_params.get("message") - if message is not None and len(str(message)) > _MAX_MESSAGE: + if message is not None and ( + not isinstance(message, str) or len(message) > _MAX_MESSAGE + ): return _controller_error( "INVALID_PARAMS", - f"message must be at most {_MAX_MESSAGE} characters", + f"message must be a string of at most {_MAX_MESSAGE} characters", ) result = await method( arm=arm, task_id=task_id, message=message, - account=params.get("account"), **extra, ) elif scenario == "force_task_completion": @@ -572,7 +570,6 @@ async def _handle_test_controller( result = await method( task_id=task_id, result=result_value, - account=params.get("account"), **extra, ) elif scenario == "simulate_delivery":