Skip to content

Commit 7b7387b

Browse files
jottakkaclaudeEricGustin
authored
fix(core): enforce input-model strictness and required params in ToolCatalog [TOO-771] (#829)
## Summary Closes TOO-771 (https://linear.app/arcadedev/issue/TOO-771) Auto-generated Pydantic input models for `@tool`-decorated functions currently: 1. Silently drop unknown kwargs (no `extra='forbid'`), so `edit_requests=...` instead of `requests=...` produces `isError: false` with zero indication anything was wrong. 2. Default omitted required params to `None` (every field is effectively optional at validation time, because `create_func_models` passes `default=None` unconditionally), so the wire schema's `required=True` never turns into a Pydantic error. 3. Silently drop typos inside nested TypedDict request dicts (`{"txt": "x"}` instead of `{"text": "x"}`), because the raw TypedDict → Pydantic path ignores extras. Reported by francisco@arcade.dev after ~1200 Google Docs edit requests were discarded across a multi-hour session; the agent had no signal that anything was being ignored. Production reproductions against `GoogleDocs.InsertTextAtEndOfDocument@5.2.0` and `GoogleDocs.EditDocument@5.2.0` confirm the same silent drops at the deployed Arcade Cloud MCP. ## Changes — [libs/arcade-core/arcade_core/catalog.py](libs/arcade-core/arcade_core/catalog.py) - **Top-level strictness**: `create_func_models` now sets `ConfigDict(extra='forbid')` on the generated input model. Unknown kwargs raise `ValidationError`, surfaced as `ToolInputError` / `ErrorKind.TOOL_RUNTIME_BAD_INPUT_VALUE` by the existing `ToolExecutor._serialize_input` branch. - **Real required fields**: Required params are emitted without `default=` so Pydantic enforces them at validation time. Adds a `ParamInfo.has_explicit_default` flag to distinguish "no default" from "default=None", preventing regressions on `def f(x: str = None)` style signatures (which are now correctly typed as `Optional[str]` with `default=None`). - **Nested TypedDict strictness (input only)**: New `create_model_from_typeddict(..., strict=True)` path wraps input-side TypedDicts in a `_StrictTypedDictBaseModel(extra='forbid')`. Output-side TypedDicts keep the permissive default so tools returning dicts with extra keys from upstream APIs (e.g. Google, Slack) don't break. - **`@model_serializer(mode='wrap')`** on `_TypedDictBaseModel` drops unset fields during nested serialization. The previous `model_dump()` Python-level override was bypassed by Pydantic v2's Rust-level outer serializer. - **Edge cases handled**: `Optional[TypedDict]`, `list[TypedDict]`, `list[Optional[TypedDict]]`, `list[TypedDict]` inside a parent TypedDict, two params sharing a TypedDict type (param-name-qualified model names to avoid `$defs` collisions), and bare `list` (no type arg) destructuring. ## Version bumps `arcade-core` 4.7.0 → 4.8.0 (minor: validation semantics change). `arcade-tdk` and `arcade-mcp-server` both pin `arcade-core>=4.8.0,<5.0.0`. ## Review history This PR went through 3 rounds of local ACR review; round 3 findings about theoretical `model_serializer` kwarg forwarding (1/5) and hypothetical non-None TypedDict field defaults (1/5) are documented in-source and not addressed since the actual call paths are covered by tests. ## Test plan - [x] 22 new tests in [libs/tests/core/test_input_model_strictness.py](libs/tests/core/test_input_model_strictness.py) covering all three bug shapes + edge cases + regressions. - [x] Full test suite: 2698 pass, 1 skip, 0 regressions. - [x] `make check` clean on `arcade-core` (pre-commit, ruff, mypy). - [x] End-to-end verified: `ToolExecutor.run` now returns `ErrorKind.TOOL_RUNTIME_BAD_INPUT_VALUE` for both unknown kwargs and missing required fields (previously both silently succeeded). - [ ] CI green across Python 3.10–3.14 / Ubuntu / Windows / macOS. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes runtime validation semantics for all tool invocations; previously accepted bad/missing inputs may now error, which is intended but can affect agents relying on silent coercion. > > **Overview** > **Tightens auto-generated Pydantic input models** for `@tool` functions so bad or incomplete arguments fail validation instead of running tools with silently wrong inputs. > > Generated input models now use **`extra='forbid'`**, so misspelled top-level fields (e.g. `edit_requests` vs `requests`) raise `ValidationError` and surface as `TOOL_RUNTIME_BAD_INPUT_VALUE` through `ToolExecutor`. > > **Required parameters** are emitted without a Pydantic default (via new **`has_explicit_default`** on `ParamInfo` / `ToolParamInfo`), fixing the case where missing required args defaulted to `None`. Signatures like `x: str = None` still count as optional. > > **Input-side TypedDicts** are wrapped through **`create_model_from_typeddict(..., strict=True)`** and **`_wrap_typeddicts_as_models`** so nested dict and `list[TypedDict]` typos are rejected; output TypedDicts stay permissive for extra API keys. **`@model_serializer`** on `_TypedDictBaseModel` drops unset `total=False` fields on dump. > > Adds **`libs/tests/core/test_input_model_strictness.py`** and bumps **`arcade-core`** 4.9.0 plus dependent package pins. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 868f2ec. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Eric Gustin <eric@arcade.dev>
1 parent 57179b7 commit 7b7387b

7 files changed

Lines changed: 891 additions & 40 deletions

File tree

libs/arcade-core/arcade_core/catalog.py

Lines changed: 157 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
get_type_hints,
2323
)
2424

25-
from pydantic import BaseModel, Field, create_model
25+
from pydantic import BaseModel, ConfigDict, Field, create_model, model_serializer
2626
from pydantic.fields import FieldInfo
2727
from pydantic_core import PydanticUndefined
2828

@@ -517,10 +517,11 @@ def create_input_definition(func: Callable) -> ToolInput:
517517

518518
tool_field_info = extract_field_info(param)
519519

520-
# If the field has a default value, it is not required
521-
# If the field is optional, it is not required
522-
has_default_value = tool_field_info.default is not None
523-
is_required = not tool_field_info.is_optional and not has_default_value
520+
# If the field has an explicit default in the signature, it is not required.
521+
# If the annotation is Optional[...], it is not required.
522+
# (A bare `param: str = None` has has_explicit_default=True but
523+
# is_optional=False, and must still be treated as not-required.)
524+
is_required = not tool_field_info.is_optional and not tool_field_info.has_explicit_default
524525

525526
input_parameters.append(
526527
InputParameter(
@@ -663,6 +664,10 @@ class ParamInfo:
663664
field_type: type
664665
description: str | None = None
665666
is_optional: bool = True
667+
# True iff the Python signature (or pydantic Field) supplies a default.
668+
# Distinct from `default is not None` — `def f(x: str = None)` has an
669+
# explicit default (None) and must not be treated as required.
670+
has_explicit_default: bool = False
666671

667672

668673
@dataclass
@@ -679,6 +684,7 @@ class ToolParamInfo:
679684
description: str | None = None
680685
is_optional: bool = True
681686
is_inferrable: bool = True
687+
has_explicit_default: bool = False
682688

683689
@classmethod
684690
def from_param_info(
@@ -696,6 +702,7 @@ def from_param_info(
696702
is_optional=param_info.is_optional,
697703
wire_type_info=wire_type_info,
698704
is_inferrable=is_inferrable,
705+
has_explicit_default=param_info.has_explicit_default,
699706
)
700707

701708

@@ -987,16 +994,21 @@ def extract_python_param_info(param: inspect.Parameter) -> ParamInfo:
987994
f"Parameter {param.name} is a union type. Only optional types are supported."
988995
)
989996

997+
has_explicit_default = param.default is not inspect.Parameter.empty
990998
return ParamInfo(
991999
name=param.name,
992-
default=param.default if param.default is not inspect.Parameter.empty else None,
1000+
default=param.default if has_explicit_default else None,
9931001
is_optional=is_optional,
9941002
original_type=original_type,
9951003
field_type=field_type,
1004+
has_explicit_default=has_explicit_default,
9961005
)
9971006

9981007

9991008
def extract_pydantic_param_info(param: inspect.Parameter) -> ParamInfo:
1009+
has_explicit_default = (
1010+
param.default.default is not PydanticUndefined or param.default.default_factory is not None
1011+
)
10001012
default_value = None if param.default.default is PydanticUndefined else param.default.default
10011013

10021014
if param.default.default_factory is not None:
@@ -1027,6 +1039,7 @@ def extract_pydantic_param_info(param: inspect.Parameter) -> ParamInfo:
10271039
is_optional=is_optional,
10281040
original_type=original_type,
10291041
field_type=field_type,
1042+
has_explicit_default=has_explicit_default,
10301043
)
10311044

10321045

@@ -1069,32 +1082,106 @@ def get_wire_type(
10691082
raise ToolDefinitionError(f"Unsupported parameter type: {_type}")
10701083

10711084

1085+
def _wrap_typeddicts_as_models(field_type: Any, model_name_prefix: str) -> Any:
1086+
"""Route TypedDict input-field types through a strict Pydantic model.
1087+
1088+
The raw TypedDict → Pydantic validator path does NOT honor extra='forbid',
1089+
so typos inside nested request dicts are silently dropped. Wrapping the
1090+
TypedDict in a strict Pydantic model closes that hole. Handles:
1091+
- TypedDict (direct)
1092+
- list[TypedDict]
1093+
- list[Optional[TypedDict]]
1094+
Top-level Optional[TypedDict] is already unwrapped by
1095+
extract_*_param_info before this helper runs; the caller in
1096+
create_func_models re-wraps with Optional[...] when is_optional is true.
1097+
"""
1098+
if is_typeddict(field_type):
1099+
return create_model_from_typeddict(
1100+
field_type, f"{model_name_prefix}_{field_type.__name__}", strict=True
1101+
)
1102+
# Unwrap Optional[TypedDict] inside a list so the TypedDict can be wrapped
1103+
# and then re-Optionalized.
1104+
if is_strict_optional(field_type):
1105+
inner = next(arg for arg in get_args(field_type) if arg is not type(None))
1106+
wrapped = _wrap_typeddicts_as_models(inner, model_name_prefix)
1107+
if wrapped is not inner:
1108+
return Optional[wrapped]
1109+
return field_type
1110+
if get_origin(field_type) is list:
1111+
args = get_args(field_type)
1112+
if not args:
1113+
# Bare `list` (no type parameter) — nothing to wrap.
1114+
return field_type
1115+
inner = args[0]
1116+
wrapped = _wrap_typeddicts_as_models(inner, model_name_prefix)
1117+
if wrapped is not inner:
1118+
return list[wrapped] # type: ignore[valid-type]
1119+
return field_type
1120+
1121+
10721122
def create_func_models(func: Callable) -> tuple[type[BaseModel], type[BaseModel]]:
10731123
"""
10741124
Analyze a function to create corresponding Pydantic models for its input and output.
10751125
"""
1076-
input_fields = {}
1126+
input_fields: dict[str, Any] = {}
10771127
# TODO figure this out (Sam)
10781128
if asyncio.iscoroutinefunction(func) and hasattr(func, "__wrapped__"):
10791129
func = func.__wrapped__
1130+
model_prefix = snake_to_pascal_case(func.__name__)
10801131
for name, param in inspect.signature(func, follow_wrapped=True).parameters.items():
10811132
# Skip ToolContext parameters (including subclasses like arcade_mcp_server.Context)
10821133
ann = param.annotation
10831134
if isinstance(ann, type) and issubclass(ann, ToolContext):
10841135
continue
10851136

1086-
# TODO make this cleaner
10871137
tool_field_info = extract_field_info(param)
1088-
param_fields = {
1089-
"default": tool_field_info.default,
1090-
"description": tool_field_info.description
1091-
if tool_field_info.description
1092-
else "No description provided.",
1093-
# TODO more here?
1094-
}
1095-
input_fields[name] = (tool_field_info.field_type, Field(**param_fields))
10961138

1097-
input_model = create_model(f"{snake_to_pascal_case(func.__name__)}Input", **input_fields) # type: ignore[call-overload]
1139+
# A param is required iff the annotation is not Optional[...] AND
1140+
# there is no explicit default in the signature. Uses the
1141+
# has_explicit_default flag (not `default is not None`) so that
1142+
# `def f(x: str = None)` is correctly treated as optional.
1143+
is_required = not tool_field_info.is_optional and not tool_field_info.has_explicit_default
1144+
1145+
# extract_field_info raises ToolInputSchemaError when the Annotated
1146+
# description is missing, so by this point it is guaranteed non-None.
1147+
description = tool_field_info.description
1148+
1149+
# Route TypedDict fields (and list[TypedDict]) through Pydantic models
1150+
# so the extra='forbid' rule applies inside nested request dicts too.
1151+
# Include the parameter name in the model-name prefix so two params
1152+
# of the same TypedDict type don't collide in Pydantic's $defs.
1153+
field_type = _wrap_typeddicts_as_models(
1154+
tool_field_info.field_type, f"{model_prefix}_{name}"
1155+
)
1156+
1157+
# extract_*_param_info unwraps Optional[T] to T before this point, so
1158+
# re-wrap when the original annotation permitted None — otherwise the
1159+
# field would be annotated as a non-nullable Pydantic model and callers
1160+
# omitting the field (which falls back to default=None) would fail
1161+
# validation at access time. Also re-wrap when the signature has an
1162+
# explicit `= None` default on a non-Optional annotation
1163+
# (`def f(x: str = None)`), which is legal Python and should behave as
1164+
# `Optional[str]`.
1165+
if tool_field_info.is_optional or (
1166+
tool_field_info.has_explicit_default and tool_field_info.default is None
1167+
):
1168+
field_type = Optional[field_type]
1169+
1170+
if is_required:
1171+
# Omit `default=` entirely — Pydantic treats this as a required field
1172+
# and raises ValidationError when the caller omits it.
1173+
input_fields[name] = (field_type, Field(description=description))
1174+
else:
1175+
input_fields[name] = (
1176+
field_type,
1177+
Field(default=tool_field_info.default, description=description),
1178+
)
1179+
1180+
input_model = create_model(
1181+
f"{model_prefix}Input",
1182+
__config__=ConfigDict(extra="forbid"),
1183+
**input_fields,
1184+
)
10981185

10991186
output_model = determine_output_model(func)
11001187
return input_model, output_model
@@ -1212,21 +1299,54 @@ def determine_output_model(func: Callable) -> type[BaseModel]:
12121299

12131300

12141301
class _TypedDictBaseModel(BaseModel):
1215-
"""Base for Pydantic models derived from TypedDict.
1216-
1217-
Defaults model_dump() to exclude_unset=True so that absent optional
1218-
fields (total=False) don't appear as None in serialized output.
1302+
"""Base for Pydantic models derived from TypedDict (output side).
1303+
1304+
Drops unset fields at serialization time so absent optional fields
1305+
(total=False) don't appear as None. Uses @model_serializer instead
1306+
of overriding model_dump() because Pydantic v2's outer serializer
1307+
does not call Python-level model_dump() on nested models — the
1308+
override is only respected when the Python method is invoked
1309+
directly.
1310+
1311+
NOTE: this base does NOT set extra='forbid' — it is used for output
1312+
models, and tools that return TypedDicts may include extra keys from
1313+
upstream APIs that shouldn't fail serialization. Input-side strictness
1314+
is applied via the `strict` parameter in create_model_from_typeddict.
12191315
"""
12201316

1317+
@model_serializer(mode="wrap")
1318+
def _drop_unset_fields(self, handler: Any) -> dict[str, Any]:
1319+
result: dict[str, Any] = handler(self)
1320+
for name in type(self).model_fields.keys() - self.model_fields_set:
1321+
result.pop(name, None)
1322+
return result
1323+
12211324
def model_dump(self, **kwargs: Any) -> dict[str, Any]:
12221325
kwargs.setdefault("exclude_unset", True)
12231326
return super().model_dump(**kwargs)
12241327

12251328

1226-
def create_model_from_typeddict(typeddict_class: type, model_name: str) -> type[BaseModel]:
1329+
class _StrictTypedDictBaseModel(_TypedDictBaseModel):
1330+
"""Strict input-side base: rejects unknown keys.
1331+
1332+
Typos inside nested request dicts (e.g. {"txt": "x"} instead of
1333+
{"text": "x"}) raise ValidationError instead of being silently dropped.
1334+
"""
1335+
1336+
model_config = ConfigDict(extra="forbid")
1337+
1338+
1339+
def create_model_from_typeddict(
1340+
typeddict_class: type, model_name: str, strict: bool = False
1341+
) -> type[BaseModel]:
12271342
"""
12281343
Create a Pydantic model from a TypedDict class.
1229-
This enables runtime validation of TypedDict structures.
1344+
1345+
When strict=True, the generated model (and all nested TypedDict models)
1346+
forbid unknown keys. Used for input-side TypedDicts so typos surface as
1347+
ValidationError. Output-side callers leave strict=False (default) to
1348+
preserve the pass-through behavior for tools whose return dicts contain
1349+
extra keys from upstream APIs.
12301350
"""
12311351
# Get type hints for the TypedDict
12321352
type_hints = get_type_hints(typeddict_class, include_extras=True)
@@ -1245,7 +1365,9 @@ def create_model_from_typeddict(typeddict_class: type, model_name: str) -> type[
12451365

12461366
# Handle nested TypedDict (works for both T and Optional[T] after unwrapping)
12471367
if is_typeddict(inner_type):
1248-
nested_model = create_model_from_typeddict(inner_type, f"{model_name}_{field_name}")
1368+
nested_model = create_model_from_typeddict(
1369+
inner_type, f"{model_name}_{field_name}", strict=strict
1370+
)
12491371
if is_required:
12501372
if is_optional_type:
12511373
field_definitions[field_name] = (Optional[nested_model], Field())
@@ -1254,13 +1376,20 @@ def create_model_from_typeddict(typeddict_class: type, model_name: str) -> type[
12541376
else:
12551377
field_definitions[field_name] = (Optional[nested_model], Field(default=None))
12561378
else:
1379+
# In strict mode, propagate wrapping into list[TypedDict] fields
1380+
# so typos inside list elements also raise.
1381+
effective_type = field_type
1382+
if strict:
1383+
effective_type = _wrap_typeddicts_as_models(
1384+
field_type, f"{model_name}_{field_name}"
1385+
)
12571386
if is_required:
1258-
field_definitions[field_name] = (field_type, Field())
1387+
field_definitions[field_name] = (effective_type, Field())
12591388
else:
1260-
field_definitions[field_name] = (field_type, Field(default=None))
1389+
field_definitions[field_name] = (effective_type, Field(default=None))
12611390

1262-
# Create and return the Pydantic model
1263-
return create_model(model_name, __base__=_TypedDictBaseModel, **field_definitions)
1391+
base = _StrictTypedDictBaseModel if strict else _TypedDictBaseModel
1392+
return create_model(model_name, __base__=base, **field_definitions)
12641393

12651394

12661395
def to_tool_secret_requirements(

libs/arcade-core/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "arcade-core"
3-
version = "4.8.0"
3+
version = "4.9.0"
44
description = "Arcade Core - Core library for Arcade platform"
55
readme = "README.md"
66
license = { text = "MIT" }

libs/arcade-mcp-server/pyproject.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "arcade-mcp-server"
7-
version = "1.22.2"
7+
version = "1.23.0"
88
description = "Model Context Protocol (MCP) server framework for Arcade.dev"
99
readme = "README.md"
1010
authors = [{ name = "Arcade.dev" }]
@@ -21,9 +21,9 @@ classifiers = [
2121
]
2222
requires-python = ">=3.10"
2323
dependencies = [
24-
"arcade-core>=4.7.3,<5.0.0",
25-
"arcade-serve>=3.2.4,<4.0.0",
26-
"arcade-tdk>=3.7.0,<4.0.0",
24+
"arcade-core>=4.9.0,<5.0.0",
25+
"arcade-serve>=3.3.0,<4.0.0",
26+
"arcade-tdk>=3.9.0,<4.0.0",
2727
"arcadepy>=1.5.0",
2828
"pydantic>=2.0.0",
2929
"fastapi>=0.100.0",

libs/arcade-serve/pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "arcade-serve"
3-
version = "3.2.5"
3+
version = "3.3.0"
44
description = "Arcade Serve - Serving infrastructure for Arcade tools and workers"
55
readme = "README.md"
66
license = {text = "MIT"}
@@ -19,7 +19,7 @@ classifiers = [
1919
]
2020
requires-python = ">=3.10"
2121
dependencies = [
22-
"arcade-core>=4.7.1,<5.0.0",
22+
"arcade-core>=4.9.0,<5.0.0",
2323
"fastapi>=0.115.3",
2424
"uvicorn>=0.30.0",
2525
"watchfiles>=1.0.5",

libs/arcade-tdk/pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "arcade-tdk"
3-
version = "3.8.0"
3+
version = "3.9.0"
44
description = "Arcade TDK - Toolkit Development Kit for building Arcade tools"
55
readme = "README.md"
66
license = { text = "MIT" }
@@ -16,7 +16,7 @@ classifiers = [
1616
"Programming Language :: Python :: 3.13",
1717
]
1818
requires-python = ">=3.10"
19-
dependencies = ["arcade-core>=4.7.0,<5.0.0", "pydantic>=2.7.0"]
19+
dependencies = ["arcade-core>=4.9.0,<5.0.0", "pydantic>=2.7.0"]
2020

2121
[project.optional-dependencies]
2222
dev = [

0 commit comments

Comments
 (0)