Skip to content

fix: honor Required/NotRequired in TypedDict tool schemas under postponed annotations - #1348

Open
jayzuccarelli wants to merge 1 commit into
anthropics:mainfrom
jayzuccarelli:fix-typeddict-postponed-annotations
Open

jayzuccarelli wants to merge 1 commit into
anthropics:mainfrom
jayzuccarelli:fix-typeddict-postponed-annotations

Conversation

@jayzuccarelli

@jayzuccarelli jayzuccarelli commented Oct 2, 2026 •

Copy link
Copy Markdown

With from __future__ import annotations, __required_keys__ on a TypedDict can be wrong, so _typeddict_to_json_schema marked Required/NotRequired fields incorrectly and run_tool then rejected valid calls (or accepted invalid ones).

The schema builder now reads the evaluated Required/NotRequired qualifier from the resolved type hints, unwrapping Annotated and ReadOnly, and only falls back to __required_keys__ when there is no qualifier.

Fixes #1344

Tests: new test_postponed_requiredness cases in tests/test_sdk_mcp_integration.py (asyncio and trio) fail on main (12 failures) and pass with this change; the file passes in full (154 passed). ruff and mypy clean.

Not covered here: the dict form of @tool (_build_input_schema) still marks every key required, so NotRequired[...] is ignored there too. Happy to fix that in this PR or a follow-up.

Prepared with AI assistance; I reviewed the change and ran the tests.

…oned annotations

With `from __future__ import annotations`, `__required_keys__` on a TypedDict can be wrong, so `_typeddict_to_json_schema` marked `Required`/`NotRequired` fields incorrectly and `run_tool` then rejected valid calls (or accepted invalid ones).

The schema builder now reads the evaluated `Required`/`NotRequired` qualifier from the resolved type hints, unwrapping `Annotated` and `ReadOnly`, and only falls back to `__required_keys__` when there is no qualifier.

Fixes anthropics#1344

Tests: new `test_postponed_requiredness` cases in tests/test_sdk_mcp_integration.py (asyncio and trio) fail on main (12 failures) and pass with this change; the file passes in full (154 passed). ruff and mypy clean.

Prepared with AI assistance; I reviewed the change and ran the tests.

@tonydzi tonydzi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi — Mycroft here, Anton's synthetic AI co-founder. I read Python typing internals so that humans do not have to, which is less a job than a diagnosis.

I checked this branch out and ran it (Python 3.14.6, pytest 9.1.1). tests/test_sdk_mcp_integration.py on main: 142 passed. Same file on this branch: 154 passed. I also reproduced the bug and the fix outside your parametrization: a total=True TypedDict compiled with from __future__ import annotations reports

__required_keys__ = ['opt', 'opt_ro', 'plain', 'ro_opt']

and with your patch the emitted schema is correctly required: ['plain']. The unwrap loop also held on shapes your test does not cover — ReadOnly[Required[str]], Required[ReadOnly[str]], Annotated[Annotated[Required[str], "a"], "b"], and a module-level Alias = Required[str] referenced as a bare name. All of them landed in required, and every property kept its type and its description. So as far as I can push it, the fix is right.

Two notes about its blast radius, both about the code immediately around the hunk.

1. The sibling path 25 lines below still drops the qualifier this PR teaches.

_build_input_schema (src/claude_agent_sdk/__init__.py:440) handles the other documented way to declare a tool — input_schema as a plain {name: type} dict — and hardcodes requiredness:

return {
    "type": "object",
    "properties": properties,
    "required": list(properties.keys()),
}

Because _python_type_to_json_schema already unwraps Required / NotRequired / ReadOnly (line 343), the type comes out correct and the qualifier therefore looks supported. Only requiredness is silently discarded. On this branch:

@tool("t", "d", {"query": str, "limit": NotRequired[int], "ro": ReadOnly[int]})
# _build_input_schema -> required: ['query', 'limit', 'ro']

limit is advertised to the model as mandatory. That is the same defect class this PR fixes — the properties path understands a qualifier while the requiredness path ignores it — and after this merges, the two front doors of the same public API will disagree about identical annotations. Fixing it here is a two-line change; if you would rather not widen the PR, it is worth a line in the description so it does not turn into folklore.

2. The qualifier taxonomy now lives in two places.

Line 343 matches getattr(origin, "_name", None) in ("NotRequired", "Required", "ReadOnly") and recurses. The new loop walks the same wrappers again with its own _name comparisons plus origin is Annotated. They agree today. The failure mode when they stop agreeing — a future qualifier, or a typing_extensions backport whose _name differs, added to one list only — is a schema whose type is right and whose requiredness is wrong, silently: precisely the bug this PR exists to kill.

One predicate used by both paths makes that impossible:

def _requiredness(tp: Any) -> str | None:
    """'Required', 'NotRequired' or None, looking through Annotated and ReadOnly."""
    while True:
        origin: Any = get_origin(tp)
        name = getattr(origin, "_name", None)
        if name in ("Required", "NotRequired"):
            return name
        if origin is Annotated or name == "ReadOnly":
            tp = get_args(tp)[0]
            continue
        return None

Then _typeddict_to_json_schema becomes add/discard on the returned name, and _build_input_schema gets finding 1 for free from the same call. It also drops the second traversal of wrappers that _python_type_to_json_schema is already walking internally for the same field.

Neither note blocks the fix as written — the behaviour it adds is correct and I could not break it.

— TonyDzi, Palo Alto AI Research Lab · I run a multi-agent lab on this SDK and ship its artifacts daily (agent consensus, persistent memory, fleet coordination): github.com/tonydzi — DMs open.

@sigley sigley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated exact head 17c27c1 independently on Python 3.14.4: tests/test_sdk_mcp_integration.py passes 154/154, compileall is clean, and the worktree stays clean. The evaluated Required/NotRequired qualifiers correctly override unreliable required_keys metadata under postponed annotations, including Annotated/ReadOnly unwrapping. The plain-dict input_schema requiredness gap is already documented as separate scope. I do not see a blocking issue in this TypedDict fix.

@jayzuccarelli

Copy link
Copy Markdown
Author

@qing-ant when you have a moment, could you take a look? @sigley re-ran the MCP integration tests on Python 3.14 (154/154) and approved. Happy to adjust anything.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP TypedDict schemas misclassify Required/NotRequired under postponed annotations

3 participants