fix(server): defensive copy before hook call eliminates in-place mutation footgun

Pass dict(params) to pre_validation_hook so hooks that mutate their
argument in-place still leave raw_params (the context-echo snapshot)
untouched. The "must return a new dict" restriction is removed from
docstrings; either mutation style is now safe.

Adds test_in_place_mutation_is_safe_for_context_echo to prove the
invariant. Adds a docstring cross-reference to #623 for the account-omission
case per adopter feedback on #629.

Co-authored-by: bokelley <bokelley@users.noreply.github.com>

https://claude.ai/code/session_0115Pruuy4MbdaxhPvgTBFoo

Author: claude

src/adcp/decisioning/serve.py

@@ -452,8 +452,9 @@ def serve(
             )
 
         Hook exceptions surface as ``INVALID_REQUEST`` on the wire.
-        The hook must return a new dict; mutating the input in-place is
-        a bug — the original is captured separately for context echo.
+        The hook receives a shallow copy of the wire args, so it may
+        mutate its argument freely or return a new dict — either style
+        is safe. Context echo always reflects the original wire input.
     """
     # Local import to avoid a circular at module-load time. Adopter
     # serves never run during foundation imports anyway.

src/adcp/server/mcp_tools.py

@@ -1901,17 +1901,25 @@ def create_tool_caller(
     dispatcher-level enforcement.
 
     **Pre-validation hook (issue #614).** When ``pre_validation_hook`` is
-    supplied, it is called with ``(tool_name, raw_args_dict)`` and must
-    return a (possibly modified) ``dict`` that replaces the wire args
-    before schema validation and Pydantic ``model_validate`` run. Use
-    this to apply spec-mandated defaults for pre-v3 buyers that omit
-    required fields (e.g. ``buying_mode``, ``format_id`` shape coercion,
-    ``asset_type`` inference). The hook runs on every call; keep it fast.
+    supplied, it is called with ``(tool_name, shallow_copy_of_args)`` and
+    must return a ``dict`` that replaces the wire args before schema
+    validation and Pydantic ``model_validate`` run. The framework passes
+    a shallow copy of the incoming params dict, so the hook may mutate
+    its argument freely or return a brand-new dict — either style is safe.
+    The original wire params are captured before the copy is made, so
+    context echo always reflects what the buyer sent. Use this to apply
+    spec-mandated defaults for pre-v3 buyers that omit required fields
+    (e.g. ``buying_mode``, ``format_id`` shape coercion, ``asset_type``
+    inference). The hook runs on every call; keep it fast.
     Exceptions from the hook surface as ``INVALID_REQUEST`` — do not raise
     for missing-but-defaultable fields, only for structurally unusable args.
-    The hook must return a new dict (or the original unchanged); mutating
-    the input dict in-place is a bug — the original is captured separately
-    for the context-echo path.
+
+    .. note::
+        For the specific case of buyers omitting ``account``, see issue
+        #623 ("Typed dispatcher rejects valid request when ``account`` is
+        omitted") — that will be the canonical spec-level fix for that
+        field. Once #623 lands you can drop any ``account`` placeholder
+        hook entry.
 
     Args:
         handler: The ADCP handler instance
@@ -1960,7 +1968,7 @@ async def call_tool(params: dict[str, Any], context: ToolContext | None = None)
 
         if pre_validation_hook is not None:
             try:
-                params = pre_validation_hook(method_name, params)
+                params = pre_validation_hook(method_name, dict(params))
             except Exception as exc:
                 raise ADCPTaskError(
                     operation=method_name,

tests/test_pre_validation_hooks.py

@@ -7,7 +7,7 @@
 - pre_validation_hooks=None (default) is a no-op (hot path unchanged).
 - A hook for tool X is not called when tool Y is dispatched.
 - Hook runs before validate_request in strict validation mode.
-- The hook must not mutate raw_params used for context echo.
+- In-place mutation of hook args is safe (framework passes a shallow copy).
 """
 
 from __future__ import annotations
@@ -173,6 +173,32 @@ def stripping_hook(tool_name: str, args: dict[str, Any]) -> dict[str, Any]:
     assert result.get("context") == wire_context
 
 
+@pytest.mark.asyncio
+async def test_in_place_mutation_is_safe_for_context_echo() -> None:
+    """Hook that mutates its argument in-place must not corrupt context echo.
+
+    The framework passes a shallow copy to the hook, so in-place mutation
+    of the hook argument leaves the original wire params untouched for the
+    context-echo path. This test exercises the ``args["key"] = val; return args``
+    pattern that the original docstring labelled a "bug".
+    """
+    wire_context = {"correlation_id": "req-xyz"}
+    wire_args = {"buyer_field": "original", "context": wire_context}
+
+    def mutating_hook(tool_name: str, args: dict[str, Any]) -> dict[str, Any]:
+        args["server_default"] = "injected"
+        del args["buyer_field"]
+        return args
+
+    handler = _MinimalHandler()
+    caller = create_tool_caller(handler, "get_products", pre_validation_hook=mutating_hook)
+    result = await caller(dict(wire_args))
+
+    assert result["params_received"].get("server_default") == "injected"
+    assert "buyer_field" not in result["params_received"]
+    assert result.get("context") == wire_context
+
+
 # ---------------------------------------------------------------------------
 # MCPToolSet threading
 # ---------------------------------------------------------------------------