feat(agent): validate tool arguments against the schema

HOSTED_TOOL_CONTRACTS was advisory. Dispatch went from the model's
arguments straight to capability.handler(**kwargs), so
`additionalProperties: False` enforced nothing and every undeclared
handler parameter stayed reachable from the agent.

That mattered most for valuation, which accepts base_growth_pct,
terminal_growth_pct, and beta without declaring them. An LLM that can set
its own growth rate, terminal growth, or beta can make a DCF agree with
whatever it already believed — the failure the locked-DCF rule in the
idea-loop design exists to prevent, and until now that rule held only as
long as a caller remembered to whitelist kwargs by hand.

Validation runs at the tool boundary: after alias normalisation, so
documented aliases still work, and before dispatch. It is keyed on the
capability name, not the tool name. A backgroundable capability is also
exposed as start_<capability>_task, which is not a HOSTED_TOOL_CONTRACTS
key, so keying on the tool name would leave all 24 task variants
validating vacuously — including start_valuation_task, i.e. the tilt
parameters would stay reachable through the one capability this check
exists to protect. _build_tool_definition hands the task variant the
capability's own input_schema, so the contract enforced here is the one
the model was shown. start_task's own `description` kwarg appears in no
contract, so a model passing it is rejected, which is what the task tool's
`additionalProperties: false` already advertises.

Rejections come back in the adapter's existing accepted:false shape with a
modelHint, matching how tool misuse is already reported, rather than
raising.

Only the agent surface is constrained. Routes, the service, and a future
controller keep the full signature by design — a deliberate internal
caller is trusted to choose inputs; a model is not.

The validator is hand-rolled rather than pulling in jsonschema, because
the contracts use a small fixed subset: type, enum, minimum, maximum,
minLength, maxItems, anyOf, items, required, additionalProperties. Three
details are load-bearing:

- It returns the arguments to dispatch, not just a verdict. An integral
float is accepted for an integer field, since models routinely emit
20.0, but it is also narrowed to int. Accepting without narrowing only
relocates the failure: relative_strength reaches `ordered[:top_n]` and
raises `TypeError: slice indices must be integers`, which
_classify_tool_error does not recognise, so the run aborts instead of
returning a tool error.
- A null-valued required argument is rejected. It satisfies "key present"
but not the handler, which goes on to call .upper() on it, producing the
same unclassified-abort path. Absent and null are different errors and
both are errors; a null *optional* stays allowed, as models commonly
send one for an omitted field.
- An anyOf property reports the matching branch's errors when exactly one
branch matches on type. `tickers` is anyOf[string, array], so an
over-long list otherwise produced "expected string or array, got list" —
unactionable, because the value is a list. The model would resend it,
hit the same error fingerprint, and burn the retry budget instead of
learning about maxItems.

Booleans are rejected for integer and string fields, since bool is an int
subclass in Python but a JSON boolean is neither.

Tests assert the property rather than a sample: every tool the default
agent is handed must resolve to a contract, so a future execution mode
with a new name shape cannot reintroduce the task-variant hole silently.
The hidden-parameter test runs against both the invoke and task surfaces.

Two parity tests keep the surface honest: declared properties must be
acceptable by the real handler, and any handler parameter absent from the
schema must be named as deliberately internal, so a new one fails rather
than quietly widening what the model could reach. Both build the registry
against the real service *and* the real chart opener — passing
fake_chart_opener measured a double that is wider than production
open_chart, which hid its undeclared `client` parameter. Measured drift is
3 capabilities of 34: valuation's three tilt inputs, open_chart's
in-process client, and the runtime-injected session_id.

One behaviour change: an empty consult question is now rejected by schema
validation as a retryable tool error instead of raising ValueError. The
contract already declared minLength 1; the raise predated validation. The
adapter's own guard still covers a whitespace-only question, which passes
minLength.

Not covered, deliberately: the argument-repair retry re-dispatches without
a second validation pass. It only rewrites the *value* of name/ticker and
adds no keys, and it now operates on the narrowed dict, so it cannot
introduce an undeclared or wrongly-typed argument.

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

src/TerraFin/agent/contracts/tool_contracts.py

@@ -1,3 +1,4 @@
+from collections.abc import Mapping
 from copy import deepcopy
 from typing import Any
 
@@ -464,3 +465,149 @@ def get_hosted_tool_contract(capability_name: str) -> dict[str, Any]:
         return deepcopy(HOSTED_TOOL_CONTRACTS[capability_name])
     except KeyError as exc:
         raise KeyError(f"No explicit hosted tool contract registered for capability '{capability_name}'.") from exc
+
+
+# ---------------------------------------------------------------------------
+# Argument validation
+#
+# The schemas above were advisory until this existed: the dispatch path went
+# straight from the model's arguments to `capability.handler(**kwargs)`, so
+# `additionalProperties: False` enforced nothing. That mattered because several
+# handlers accept parameters the schema deliberately hides — `valuation` takes
+# `base_growth_pct`, `terminal_growth_pct`, and `beta`, which let a caller tilt
+# a valuation into agreeing with whatever it already believed.
+#
+# Validating here makes the tool surface the schema says it is. Internal Python
+# callers (routes, the service, a controller) are unaffected by design: they
+# hold the full signature, and locking the *agent* is the point.
+#
+# Hand-rolled rather than pulling in `jsonschema`, because the contracts use a
+# small fixed subset: type, enum, minimum, maximum, minLength, maxItems, anyOf,
+# items, required, additionalProperties.
+# ---------------------------------------------------------------------------
+
+_TYPE_NAMES = {
+    "string": str,
+    "boolean": bool,
+    "array": list,
+    "object": dict,
+}
+
+
+def _type_error(value: Any, expected: str) -> str | None:
+    """None when `value` satisfies `expected`, else a human-readable reason."""
+    if expected == "integer":
+        # bool is an int subclass in Python; a JSON boolean is not an integer.
+        if isinstance(value, bool):
+            return "expected an integer, got a boolean"
+        if isinstance(value, int):
+            return None
+        # Models routinely emit 7.0 for an integer field; accept it losslessly.
+        if isinstance(value, float) and value.is_integer():
+            return None
+        return f"expected an integer, got {type(value).__name__}"
+    if expected == "number":
+        if isinstance(value, bool):
+            return "expected a number, got a boolean"
+        return None if isinstance(value, (int, float)) else f"expected a number, got {type(value).__name__}"
+    expected_type = _TYPE_NAMES.get(expected)
+    if expected_type is None:
+        return None  # unknown type keyword: nothing to assert
+    if expected == "string" and isinstance(value, bool):
+        return "expected a string, got a boolean"
+    return None if isinstance(value, expected_type) else f"expected {expected}, got {type(value).__name__}"
+
+
+def _check_value(name: str, value: Any, schema: dict[str, Any]) -> list[str]:
+    if "anyOf" in schema:
+        branches: list[dict[str, Any]] = schema["anyOf"]
+        reasons = [_check_value(name, value, branch) for branch in branches]
+        if any(not branch_errors for branch_errors in reasons):
+            return []
+        # Exactly one branch matching on type means the value's *shape* is right
+        # and something inside it is wrong — a bound, or an item's type. Report
+        # that branch's errors. The generic "expected string or array, got list"
+        # would restate the value's own type back at the model, which cannot act
+        # on it and would resend the same argument until the retry budget ends.
+        matched = [
+            branch_errors
+            for branch, branch_errors in zip(branches, reasons)
+            if branch.get("type") and not _type_error(value, branch["type"])
+        ]
+        if len(matched) == 1:
+            return matched[0]
+        allowed = " or ".join(str(branch.get("type", "?")) for branch in branches)
+        return [f"{name}: expected {allowed}, got {type(value).__name__}"]
+
+    errors: list[str] = []
+    declared_type = schema.get("type")
+    if declared_type:
+        reason = _type_error(value, declared_type)
+        if reason:
+            return [f"{name}: {reason}"]
+
+    if "enum" in schema and value not in schema["enum"]:
+        errors.append(f"{name}: {value!r} is not one of {schema['enum']}")
+    if "minimum" in schema and isinstance(value, (int, float)) and value < schema["minimum"]:
+        errors.append(f"{name}: {value} is below the minimum of {schema['minimum']}")
+    if "maximum" in schema and isinstance(value, (int, float)) and value > schema["maximum"]:
+        errors.append(f"{name}: {value} is above the maximum of {schema['maximum']}")
+    if "minLength" in schema and isinstance(value, str) and len(value) < schema["minLength"]:
+        errors.append(f"{name}: must be at least {schema['minLength']} character(s)")
+    if "maxItems" in schema and isinstance(value, list) and len(value) > schema["maxItems"]:
+        errors.append(f"{name}: at most {schema['maxItems']} item(s), got {len(value)}")
+    if isinstance(value, list) and isinstance(schema.get("items"), dict):
+        for index, item in enumerate(value):
+            errors.extend(_check_value(f"{name}[{index}]", item, schema["items"]))
+    return errors
+
+
+def validate_tool_arguments(
+    tool_name: str, arguments: Mapping[str, Any]
+) -> tuple[list[str], dict[str, Any]]:
+    """Validate model-supplied arguments against a tool's declared input schema.
+
+    Returns `(problems, arguments_to_dispatch)`. `problems` is a list of
+    human-readable strings, empty when the arguments are acceptable. The second
+    element is the dict the caller should actually dispatch: integral floats are
+    narrowed to `int` for integer-typed fields, because accepting `20.0` without
+    narrowing it only moves the failure downstream — `relative_strength(top_n=20.0)`
+    reaches `ordered[:top_n]` and raises `TypeError: slice indices must be
+    integers`, which no error classifier recognises, so the whole run aborts.
+
+    Unknown tools validate vacuously — an unregistered tool is a different error,
+    reported elsewhere. Callers must pass the *capability* name: task variants are
+    exposed as `start_<capability>_task` but are handed the capability's own
+    schema, so they validate against the same contract.
+    """
+    contract = HOSTED_TOOL_CONTRACTS.get(tool_name)
+    if contract is None:
+        return [], dict(arguments)
+    schema = contract.get("input_schema") or {}
+    properties: dict[str, Any] = schema.get("properties") or {}
+
+    errors: list[str] = []
+    for required_name in schema.get("required") or []:
+        if required_name not in arguments:
+            errors.append(f"{required_name}: required")
+        elif arguments[required_name] is None:
+            # An explicit null satisfies "key present" but not the handler, which
+            # goes on to call `.upper()` on it. Optional nulls stay allowed below.
+            errors.append(f"{required_name}: required, but was null")
+
+    if schema.get("additionalProperties") is False:
+        unknown = sorted(set(arguments) - set(properties))
+        if unknown:
+            errors.append(
+                f"unknown argument(s) {unknown}; this tool accepts only {sorted(properties)}"
+            )
+
+    normalized = dict(arguments)
+    for name, value in arguments.items():
+        declared = properties.get(name)
+        if declared is None or value is None:
+            continue
+        errors.extend(_check_value(name, value, declared))
+        if declared.get("type") == "integer" and isinstance(value, float) and value.is_integer():
+            normalized[name] = int(value)
+    return errors, normalized

src/TerraFin/agent/tools/adapter.py

@@ -4,7 +4,11 @@ from collections.abc import Mapping
 from typing import TYPE_CHECKING, Any
 
 from ..contracts.definitions import TerraFinAgentDefinition, is_internal_agent_definition
-from ..contracts.tool_contracts import HOSTED_TOOL_CONTRACT_VERSION, get_hosted_tool_contract
+from ..contracts.tool_contracts import (
+    HOSTED_TOOL_CONTRACT_VERSION,
+    get_hosted_tool_contract,
+    validate_tool_arguments,
+)
 from ..runtime.capability import TerraFinCapability
 from ..runtime.errors import TerraFinAgentApprovalRequiredError
 from ..runtime.hosted import TerraFinHostedAgentRuntime
@@ -19,9 +23,9 @@ from .normalize import (
     _repair_symbol_or_name,
 )
 from .types import (
-    ToolExecutionMode,
     TerraFinToolDefinition,
     TerraFinToolInvocationResult,
+    ToolExecutionMode,
     _ToolErrorDisposition,
 )
 
@@ -75,6 +79,13 @@ class TerraFinHostedToolAdapter:
         payload: dict[str, Any]
         task: TerraFinTaskRecord | None = None
         resolved_arguments = _normalize_common_alias_arguments(dict(arguments or {}))
+        # Validate AFTER alias normalisation (so documented aliases are accepted)
+        # and BEFORE dispatch (so nothing undeclared reaches a handler).
+        schema_result, resolved_arguments = self._preflight_argument_schema(
+            tool, session_id, resolved_arguments
+        )
+        if schema_result is not None:
+            return schema_result
         preflight_result = self._preflight_tool_misuse(tool, session_id, resolved_arguments)
         if preflight_result is not None:
             return preflight_result
@@ -275,6 +286,71 @@ class TerraFinHostedToolAdapter:
             },
         )
 
+    def _preflight_argument_schema(
+        self,
+        tool: TerraFinToolDefinition,
+        session_id: str,
+        arguments: Mapping[str, Any],
+    ) -> tuple[TerraFinToolInvocationResult | None, dict[str, Any]]:
+        """Reject arguments the tool's declared schema does not allow.
+
+        Returns `(rejection_or_None, arguments_to_dispatch)`; the second element
+        carries the validator's narrowing (see `validate_tool_arguments`) and must
+        be what the caller dispatches.
+
+        Without this the schema is decoration: dispatch went straight to
+        `capability.handler(**kwargs)`, so `additionalProperties: False` enforced
+        nothing and a handler's undeclared parameters stayed reachable. The
+        clearest case is `valuation`, whose hidden `base_growth_pct` /
+        `terminal_growth_pct` / `beta` let a caller tilt a valuation toward a
+        conclusion it already held.
+
+        Keyed on `capability_name`, not `name`: a backgroundable capability is also
+        exposed as `start_<capability>_task`, which is not a `HOSTED_TOOL_CONTRACTS`
+        key, so keying on `name` made all 24 task variants validate vacuously —
+        including `start_valuation_task`, leaving the tilt parameters reachable by
+        the one path this check exists to close. `_build_tool_definition` hands the
+        task variant the capability's own `input_schema`, so the model is shown the
+        contract validated here.
+
+        Only the agent surface is constrained. Internal Python callers keep the
+        full signature, which is the intent — a route or a controller is trusted
+        to choose inputs deliberately. `start_task`'s own `description` kwarg is
+        not in any contract, so a model that passes it is rejected; that matches
+        the `additionalProperties: false` schema the task tool advertises.
+        """
+        problems, normalized = validate_tool_arguments(tool.capability_name, arguments)
+        if not problems:
+            return None, normalized
+
+        message = f"Invalid arguments for {tool.name}: " + "; ".join(problems)
+        return TerraFinToolInvocationResult(
+            tool_name=tool.name,  # the name the model called, not the capability
+            capability_name=tool.capability_name,
+            session_id=session_id,
+            execution_mode=tool.execution_mode,
+            payload={
+                "accepted": False,
+                "error": {
+                    "code": "tool_invalid_arguments",
+                    "message": message,
+                    "detail": problems,
+                    "retryable": True,
+                    "modelHint": (
+                        "Retry with only the arguments this tool declares, using the types and "
+                        "ranges in its schema. Parameters that are not in the schema are not "
+                        "available through the tool surface, even if a related HTTP route or "
+                        "Python method accepts them."
+                    ),
+                },
+            },
+            task=None,
+            is_error=True,
+            retryable=True,
+            error_code="tool_invalid_arguments",
+            error_message=message,
+        ), normalized
+
     def _preflight_tool_misuse(
         self,
         tool: TerraFinToolDefinition,

tests/agent/test_capability_parity.py

@@ -60,6 +60,27 @@ def capabilities():
     return registry.list()
 
 
+@pytest.fixture(scope="module")
+def real_capabilities():
+    """Registry built against the REAL service *and* the real chart opener.
+
+    Signature checks must not measure a double. `BaseFakeService`'s stubs are
+    narrower than the handlers they stand in for, so a declared property the real
+    handler accepts (e.g. `market_snapshot`'s `force_refresh`) would read as a
+    mismatch. `fake_chart_opener` has the opposite problem: it is *wider* than
+    production `open_chart`, whose `client` parameter is undeclared and went
+    undetected here. Passing no `chart_opener` binds the real one.
+
+    Nothing is called — the handlers are only inspected — so this stays free of
+    network and env mutation.
+    """
+
+    from TerraFin.agent.service import TerraFinAgentService
+
+    registry = build_default_capability_registry(TerraFinAgentService())
+    return registry.list()
+
+
 @pytest.fixture(scope="module")
 def live_route_paths() -> set[str | None]:
     """Paths served by the assembled app.
@@ -197,6 +218,82 @@ def test_declared_cli_subcommands_are_wired(capabilities) -> None:
     assert not unwired, f"capabilities declaring a CLI subcommand that is not wired: {unwired}"
 
 
+# Handler parameters deliberately NOT in the tool schema. Each entry is a
+# decision, not an oversight, and the tool boundary rejects them (see
+# `tests/agent/test_tool_argument_validation.py`):
+#   valuation  — the three tilt inputs. An LLM that can set its own growth rate,
+#                terminal growth, or beta can make a DCF agree with whatever it
+#                already believed, which is the failure the locked-DCF rule in
+#                the idea-loop design exists to prevent.
+#   macro_focus, open_chart — `session_id` is injected by the runtime
+#                (`_apply_defaults`), never supplied by a caller.
+#   open_chart — `client` is the `TerraFinAgentClient` the CLI passes in-process;
+#                a model naming its own HTTP client makes no sense.
+INTERNAL_ONLY_HANDLER_PARAMS = {
+    "valuation": {"base_growth_pct", "terminal_growth_pct", "beta"},
+    "macro_focus": {"session_id"},
+    "open_chart": {"session_id", "client"},
+}
+
+
+def test_no_new_undeclared_handler_parameters(real_capabilities) -> None:
+    """A handler parameter absent from the tool schema must be a deliberate choice.
+
+    Adding one silently widens what the model could reach if validation were ever
+    bypassed, so new ones have to be named here on purpose.
+    """
+
+    import inspect
+
+    surprises: dict[str, list[str]] = {}
+    for capability in real_capabilities:
+        contract = HOSTED_TOOL_CONTRACTS.get(capability.name)
+        if contract is None:
+            continue
+        declared = set((contract.get("input_schema") or {}).get("properties") or {})
+        try:
+            parameters = inspect.signature(capability.handler).parameters
+        except (TypeError, ValueError):  # pragma: no cover - builtins
+            continue
+        accepted = {
+            name
+            for name, parameter in parameters.items()
+            if parameter.kind in (parameter.POSITIONAL_OR_KEYWORD, parameter.KEYWORD_ONLY)
+        }
+        undeclared = accepted - declared - INTERNAL_ONLY_HANDLER_PARAMS.get(capability.name, set())
+        if undeclared:
+            surprises[capability.name] = sorted(undeclared)
+
+    assert not surprises, (
+        "handler parameters that are not in the tool schema and not listed as "
+        f"deliberately internal: {surprises}"
+    )
+
+
+def test_declared_properties_are_acceptable_by_the_handler(real_capabilities) -> None:
+    """A declared property the handler cannot take is a TypeError waiting to happen."""
+
+    import inspect
+
+    broken: dict[str, list[str]] = {}
+    for capability in real_capabilities:
+        contract = HOSTED_TOOL_CONTRACTS.get(capability.name)
+        if contract is None:
+            continue
+        declared = set((contract.get("input_schema") or {}).get("properties") or {})
+        try:
+            parameters = inspect.signature(capability.handler).parameters
+        except (TypeError, ValueError):  # pragma: no cover - builtins
+            continue
+        if any(parameter.kind == parameter.VAR_KEYWORD for parameter in parameters.values()):
+            continue
+        missing = sorted(declared - set(parameters))
+        if missing:
+            broken[capability.name] = missing
+
+    assert not broken, f"schema declares properties the handler cannot accept: {broken}"
+
+
 def test_declared_http_routes_exist(capabilities, live_route_paths) -> None:
     missing = sorted(
         f"{c.name} -> {c.http_route_path}"

tests/agent/test_loop.py

@@ -1008,11 +1008,24 @@ def test_consult_tool_rejects_empty_question_argument() -> None:
         session_id="loop:consult-empty-question",
     )
 
+    # An empty string violates the contract's own `minLength: 1`, so schema
+    # validation now rejects it at the tool boundary and hands the model a
+    # retryable error instead of raising. The adapter's own guard still covers
+    # what the schema cannot see: a whitespace-only question passes minLength.
+    result = loop.tool_adapter.run_tool(
+        conversation.session_id,
+        "consult_warren_buffett",
+        {"question": ""},
+    )
+    assert result.is_error is True
+    assert result.error_code == "tool_invalid_arguments"
+    assert "question" in result.error_message
+
     with pytest.raises(ValueError, match="question"):
         loop.tool_adapter.run_tool(
             conversation.session_id,
             "consult_warren_buffett",
-            {"question": ""},
+            {"question": "   "},
         )
 
 

tests/agent/test_tool_argument_validation.py

@@ -0,0 +1,253 @@
+"""Argument validation at the tool boundary.
+
+The declared `input_schema` used to be advisory: dispatch went straight from the
+model's arguments to `capability.handler(**kwargs)`, so `additionalProperties:
+False` enforced nothing and every undeclared handler parameter stayed reachable
+from the agent. `valuation` is the case that matters — its hidden
+`base_growth_pct` / `terminal_growth_pct` / `beta` let a caller tilt a valuation
+toward a conclusion it already held, which is exactly what the locked-DCF rule
+in the idea-loop design exists to prevent.
+"""
+
+import pytest
+
+from TerraFin.agent.contracts.tool_contracts import validate_tool_arguments
+
+
+def _problems(tool_name: str, arguments: dict) -> list[str]:
+    """Just the problem list. `validate_tool_arguments` also returns the narrowed
+    arguments to dispatch; the tests that care about narrowing read it directly."""
+
+    return validate_tool_arguments(tool_name, arguments)[0]
+
+
+def test_valid_arguments_pass() -> None:
+    assert _problems("valuation", {"ticker": "AAPL", "projection_years": 10}) == []
+    assert _problems("market_data", {"name": "AAPL", "depth": "full"}) == []
+
+
+def test_undeclared_valuation_tilt_parameters_are_rejected() -> None:
+    """The whole point: these exist on the handler but not in the schema."""
+
+    for hidden in ("base_growth_pct", "terminal_growth_pct", "beta"):
+        problems = _problems("valuation", {"ticker": "AAPL", hidden: 42})
+        assert problems, f"{hidden} must not be reachable through the tool surface"
+        assert hidden in problems[0]
+
+
+def test_missing_required_argument_is_reported() -> None:
+    assert _problems("valuation", {}) == ["ticker: required"]
+
+
+def test_enum_violation_is_reported() -> None:
+    problems = _problems("market_data", {"name": "AAPL", "depth": "everything"})
+
+    assert len(problems) == 1
+    assert "not one of" in problems[0]
+
+
+def test_numeric_bounds_are_enforced() -> None:
+    assert _problems("relative_strength", {"top_n": 101})
+    assert _problems("relative_strength", {"top_n": 0})
+    assert _problems("relative_strength", {"top_n": 50}) == []
+
+
+def test_integral_floats_are_narrowed_not_merely_accepted() -> None:
+    """Models routinely emit 20.0 for an integer field; 20.5 is a real error.
+
+    Accepting 20.0 without narrowing it to `int` only relocates the failure:
+    `relative_strength` reaches `ordered[:top_n]` and raises `TypeError: slice
+    indices must be integers`, which no classifier recognises, so the run aborts
+    instead of returning a tool error. The narrowed value is the contract.
+    """
+
+    problems, normalized = validate_tool_arguments("relative_strength", {"top_n": 20.0})
+    assert problems == []
+    assert normalized["top_n"] == 20
+    assert isinstance(normalized["top_n"], int)
+
+    assert _problems("relative_strength", {"top_n": 20.5})
+
+
+def test_narrowing_leaves_other_arguments_untouched() -> None:
+    _, normalized = validate_tool_arguments(
+        "relative_strength", {"top_n": 20.0, "universe": "sp500", "period": "6mo"}
+    )
+
+    assert normalized == {"top_n": 20, "universe": "sp500", "period": "6mo"}
+
+
+def test_booleans_are_not_integers_or_strings() -> None:
+    """bool is an int subclass in Python; JSON booleans are neither."""
+
+    assert _problems("relative_strength", {"top_n": True})
+    assert _problems("valuation", {"ticker": True})
+
+
+def test_string_or_array_unions_accept_both_forms() -> None:
+    assert _problems("pattern_scan", {"tickers": "NVDA,AMD"}) == []
+    assert _problems("pattern_scan", {"tickers": ["NVDA", "AMD"]}) == []
+    assert _problems("pattern_scan", {"tickers": 123})
+
+
+def test_array_bounds_and_item_types_name_the_actual_problem() -> None:
+    """A union branch must not report the value's own type back at the model.
+
+    `tickers` is `anyOf: [string, array]`. Reporting "expected string or array,
+    got list" for an over-long list is unactionable — the value *is* a list — so
+    the model resends it, hits the same error fingerprint, and burns the retry
+    budget. Assert the wording, not mere truthiness.
+    """
+
+    too_many = _problems("pattern_scan", {"tickers": ["X"] * 501})
+    assert too_many == ["tickers: at most 500 item(s), got 501"]
+
+    bad_item = _problems("pattern_scan", {"tickers": ["NVDA", 7]})
+    assert bad_item == ["tickers[1]: expected string, got int"]
+
+    # No branch matches on type, so the generic union message is the right one.
+    assert _problems("pattern_scan", {"tickers": 123}) == [
+        "tickers: expected string or array, got int"
+    ]
+
+
+def test_minimum_length_is_enforced() -> None:
+    assert _problems("valuation", {"ticker": ""})
+
+
+def test_optional_nulls_are_left_to_the_handler() -> None:
+    """An omitted-as-null optional is common from models and is not a schema error."""
+
+    assert _problems("news", {"ticker": "NVDA", "query": None}) == []
+
+
+def test_a_null_required_argument_is_rejected() -> None:
+    """`{"ticker": None}` satisfies "key is present" but not the handler.
+
+    `service.valuation` calls `ticker.upper()`, so a null that passes validation
+    becomes an `AttributeError` the classifier does not recognise, aborting the
+    run. Absent and null are different errors, and both are errors.
+    """
+
+    assert _problems("valuation", {"ticker": None}) == ["ticker: required, but was null"]
+    assert _problems("valuation", {}) == ["ticker: required"]
+
+
+def test_unknown_tool_validates_vacuously() -> None:
+    """An unregistered tool is a different error, reported elsewhere."""
+
+    assert _problems("not_a_tool", {"anything": 1}) == []
+
+
+@pytest.mark.parametrize("tool_name", ["consensus", "news", "pattern_scan", "relative_strength"])
+def test_unknown_arguments_are_rejected_for_every_recent_tool(tool_name: str) -> None:
+    problems = _problems(tool_name, {"definitely_not_a_field": 1})
+
+    # Not problems[0]: a missing required argument is reported first for tools
+    # that have one.
+    assert any("unknown argument" in problem for problem in problems), problems
+
+
+def _adapter_and_session():
+    """A tool adapter over the real registry with a stub service.
+
+    Mirrors `tests/agent/test_tools.py::_adapter` — the runtime needs the service
+    as well as the registry, and the session id lives at
+    `session.session.session_id`.
+    """
+
+    from fakes import BaseFakeService, fake_chart_opener
+
+    from TerraFin.agent.contracts.definitions import DEFAULT_HOSTED_AGENT_NAME
+    from TerraFin.agent.runtime import build_default_capability_registry
+    from TerraFin.agent.runtime.hosted import TerraFinHostedAgentRuntime
+    from TerraFin.agent.tools import TerraFinHostedToolAdapter
+
+    service = BaseFakeService()
+    registry = build_default_capability_registry(service, chart_opener=fake_chart_opener)
+    runtime = TerraFinHostedAgentRuntime(service=service, capability_registry=registry)
+    adapter = TerraFinHostedToolAdapter(runtime)
+    session = runtime.create_session(DEFAULT_HOSTED_AGENT_NAME, session_id="tool:schema")
+    return adapter, session.session.session_id
+
+
+@pytest.mark.parametrize("tool_name", ["valuation", "start_valuation_task"])
+def test_tool_path_rejects_a_hidden_valuation_parameter(tool_name: str) -> None:
+    """End to end: the schema is enforced on BOTH surfaces the model can call.
+
+    `start_valuation_task` is the case that regressed. Validation keyed on
+    `tool.name`, and the task variant is named `start_valuation_task`, which is not
+    a `HOSTED_TOOL_CONTRACTS` key — so it validated vacuously and the tilt
+    parameters stayed reachable through the exact capability this check protects.
+    Every backgroundable capability had the same hole.
+    """
+
+    adapter, session_id = _adapter_and_session()
+
+    result = adapter.run_tool(session_id, tool_name, {"ticker": "AAPL", "base_growth_pct": 40})
+
+    assert result.is_error is True
+    assert result.error_code == "tool_invalid_arguments"
+    assert "base_growth_pct" in result.error_message
+    assert result.payload["accepted"] is False
+    assert result.payload["error"]["retryable"] is True
+    assert "not available through the tool surface" in result.payload["error"]["modelHint"]
+
+
+def test_every_exposed_tool_validates_against_a_contract() -> None:
+    """No tool the model is handed may fall through validation.
+
+    The task-variant bypass was invisible because the tests only ever exercised
+    invoke-mode names. This asserts the property directly, over the full tool list
+    the default agent actually sees, so a future execution mode with a new name
+    shape cannot reintroduce it silently.
+    """
+
+    from TerraFin.agent.contracts.definitions import DEFAULT_HOSTED_AGENT_NAME
+    from TerraFin.agent.contracts.tool_contracts import HOSTED_TOOL_CONTRACTS
+
+    adapter, _ = _adapter_and_session()
+    tools = adapter.list_tools_for_agent(DEFAULT_HOSTED_AGENT_NAME)
+
+    assert len(tools) > 40, "expected the full tool surface, not a filtered subset"
+    unvalidated = sorted(t.name for t in tools if t.capability_name not in HOSTED_TOOL_CONTRACTS)
+    assert not unvalidated, f"tools whose arguments no contract constrains: {unvalidated}"
+
+
+def test_task_variants_still_start_with_valid_arguments() -> None:
+    """The D1 fix must reject the undeclared argument, not the whole task path."""
+
+    adapter, session_id = _adapter_and_session()
+
+    result = adapter.run_tool(session_id, "start_valuation_task", {"ticker": "AAPL"})
+
+    assert result.is_error is False
+    assert result.payload["accepted"] is True
+    assert result.task is not None
+    assert result.task.input_payload["ticker"] == "AAPL"
+
+
+def test_tool_path_still_accepts_declared_arguments() -> None:
+    adapter, session_id = _adapter_and_session()
+
+    result = adapter.run_tool(session_id, "valuation", {"ticker": "AAPL", "projection_years": 10})
+
+    assert result.is_error is False
+    assert result.payload["ticker"] == "AAPL"
+
+
+def test_internal_callers_keep_the_full_signature() -> None:
+    """Locking the agent must not lock a route or controller.
+
+    The service method deliberately still accepts the tilt parameters — a
+    deliberate internal caller is trusted; the model is not.
+    """
+
+    import inspect
+
+    from TerraFin.agent.service import TerraFinAgentService
+
+    parameters = inspect.signature(TerraFinAgentService.valuation).parameters
+
+    for hidden in ("base_growth_pct", "terminal_growth_pct", "beta"):
+        assert hidden in parameters

tests/fakes.py

@@ -162,7 +162,26 @@ class BaseFakeService:
             "processing": processing(),
         }
 
-    def valuation(self, ticker: str) -> dict[str, object]:
+    def valuation(
+        self,
+        ticker: str,
+        *,
+        projection_years: int | None = None,
+        fcf_base_source: str | None = None,
+        breakeven_year: int | None = None,
+        breakeven_cash_flow_per_share: float | None = None,
+        post_breakeven_growth_pct: float | None = None,
+    ) -> dict[str, object]:
+        """Accepts the schema's declared optionals, deliberately NOT the hidden
+        tilt parameters (`base_growth_pct`, `terminal_growth_pct`, `beta`) — those
+        must not be reachable from the tool surface."""
+        _ = (
+            projection_years,
+            fcf_base_source,
+            breakeven_year,
+            breakeven_cash_flow_per_share,
+            post_breakeven_growth_pct,
+        )
         return {
             "ticker": ticker,
             "dcf": {"status": "ready", "intrinsic_value": 120.0},