fix(server): snapshot raw_params before hook to preserve context-echo contract

Both code-reviewer and dx-expert expert review flagged that assigning
raw_params after the hook would echo server-injected defaults back to the
buyer as if they were sent on the wire, violating the AdCP context-echo
contract. Move the snapshot to before the hook call (raw_params = params),
so inject_context always uses the original wire dict regardless of what
the hook returns or mutates.

Also strengthens test_hook_does_not_pollute_context_echo: the previous test
passed a copy of the outer dict so the assertion was trivially true. The
new test sends a wire payload with a context field, uses a stripping hook
that returns a completely new dict (no context key), and asserts the
response context echo still carries the original wire context — only possible
if raw_params was captured before the hook ran.

https://claude.ai/code/session_015hHQqXRbn2jX9WTu564gjZ

Author: claude

src/adcp/server/mcp_tools.py

@@ -1956,6 +1956,8 @@ def create_tool_caller(
     async def call_tool(params: dict[str, Any], context: ToolContext | None = None) -> Any:
         ctx = context if context is not None else ToolContext()
 
+        raw_params = params  # Preserve original wire params for context echo.
+
         if pre_validation_hook is not None:
             try:
                 params = pre_validation_hook(method_name, params)
@@ -1970,8 +1972,6 @@ async def call_tool(params: dict[str, Any], context: ToolContext | None = None)
                     ],
                 ) from exc
 
-        raw_params = params  # Preserve the (possibly hook-modified) dict for context echo.
-
         if request_mode is not None and request_mode != "off":
             outcome = validate_request(method_name, params)
             if not outcome.valid:

tests/test_pre_validation_hooks.py

@@ -142,25 +142,35 @@ def bad_hook(tool_name: str, args: dict[str, Any]) -> dict[str, Any]:
 
 
 @pytest.mark.asyncio
-async def test_hook_return_does_not_mutate_original_dict() -> None:
-    """The hook must return a new dict; the original must remain unmodified.
-
-    The context-echo path uses raw_params after the handler returns — if
-    the hook mutated the input in-place and then also returned it, the
-    echo would include server-injected fields as if the buyer had sent
-    them, violating the AdCP context-echo contract.
+async def test_hook_does_not_pollute_context_echo() -> None:
+    """raw_params must snapshot the original wire dict BEFORE the hook runs.
+
+    inject_context echoes the wire ``context`` field from raw_params back into
+    the response. If raw_params were assigned after the hook, a hook that
+    returns a new dict (dropping ``context``) would silently suppress the echo.
+    Conversely, a hook that adds keys would cause server-injected fields to
+    appear in the echo as if the buyer sent them.
+
+    We exercise both directions:
+    - A hook that strips all fields and adds "server_default" (no context key
+      in its return) still produces context echo from the original wire params.
+    - The handler result carries hook-modified fields, confirming the hook ran.
     """
-    original = {"buyer_field": "x"}
+    wire_context = {"correlation_id": "req-abc"}
+    wire_args = {"buyer_field": "x", "context": wire_context}
 
-    def mutating_hook(tool_name: str, args: dict[str, Any]) -> dict[str, Any]:
-        # Return a new dict; original is untouched
-        return {**args, "server_default": "y"}
+    def stripping_hook(tool_name: str, args: dict[str, Any]) -> dict[str, Any]:
+        # Returns a brand-new dict — deliberately omits "context"
+        return {"server_default": "y"}
 
     handler = _MinimalHandler()
-    caller = create_tool_caller(handler, "get_products", pre_validation_hook=mutating_hook)
-    await caller(dict(original))  # pass a copy to preserve the sentinel
-    # original dict (the buyer's view) must not include server_default
-    assert "server_default" not in original
+    caller = create_tool_caller(handler, "get_products", pre_validation_hook=stripping_hook)
+    result = await caller(dict(wire_args))
+
+    # Hook ran: handler received hook-modified params, not original
+    assert result["params_received"] == {"server_default": "y"}
+    # Context echo used raw_params (pre-hook snapshot), not hook return
+    assert result.get("context") == wire_context
 
 
 # ---------------------------------------------------------------------------