Skip to content

Pass json-schema-ref-no-deref conformance scenario (SEP-2106) #2924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kludex merged 12 commits into from
Jun 20, 2026
+12 −4
Merged

Pass json-schema-ref-no-deref conformance scenario (SEP-2106) #2924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
33774ff
Reject external JSON Schema $ref in tool schemas (SEP-2106)
Kludex Jun 20, 2026
69ea4fb
Drop conformance burn-down and migration docs from this PR
Kludex Jun 20, 2026
b0786d1
Reject external $ref via the tool schema generator
Kludex Jun 20, 2026
9b8d0aa
Inline the tool schema generator into func_metadata
Kludex Jun 20, 2026
f7366a6
Make ExternalSchemaRefError a hard error, not a ValueError
Kludex Jun 20, 2026
4d8f900
Burn down json-schema-ref-no-deref conformance scenario
Kludex Jun 20, 2026
a53b892
Align SEP-2106 $ref handling to the spec
Kludex Jun 20, 2026
7a2e7ab
Reuse the initialize handler for json-schema-ref-no-deref
Kludex Jun 20, 2026
a06d8e1
Give json-schema-ref-no-deref its own handler
Kludex Jun 20, 2026
8e255ad
Merge remote-tracking branch 'origin/main' into sep-2106-json-schema-ref
Kludex Jun 20, 2026
2c47fc4
Guard the no-deref test with a socket tripwire
Kludex Jun 20, 2026
c7522f2
Drop the Python no-deref unit test
Kludex Jun 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/actions/conformance/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,18 @@
logger.debug("Listed tools successfully")


@register("json-schema-ref-no-deref")
async def run_json_schema_ref_no_deref(server_url: str) -> None:
"""Initialize and list tools; the scenario fails only if the client fetches a network $ref.

ClientSession never walks inputSchema or resolves $refs, so listing is enough (SEP-2106).
"""
async with streamable_http_client(url=server_url) as (read_stream, write_stream):
async with ClientSession(read_stream, write_stream) as session:
await session.initialize()
await session.list_tools()

Check warning on line 189 in .github/actions/conformance/client.py

View check run for this annotation

Claude / Claude Code Review

PR description claims a client unit test that was dropped from the final diff 

The PR description states that a client test was added confirming a network $ref in an output schema is validated without being dereferenced, but that test was deliberately dropped in commit c7522f2 ('Drop the Python no-deref unit test') and the final diff contains only the conformance handler and the two expected-failures baselines. This is a description/documentation discrepancy only — either update the description (including the Verification section) to reflect that coverage now lives solely
Comment on lines +180 to +189

@claude claude Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 The PR description states that a client test was added confirming a network $ref in an output schema is validated without being dereferenced, but that test was deliberately dropped in commit c7522f2 ('Drop the Python no-deref unit test') and the final diff contains only the conformance handler and the two expected-failures baselines. This is a description/documentation discrepancy only — either update the description (including the Verification section) to reflect that coverage now lives solely in the external conformance scenario, or re-add the in-repo test.

Extended reasoning...

What the issue is. The PR description's "What changed" section claims: "Added a client test confirming a network $ref in an output schema, under a property the structured content does not set, is validated without being dereferenced." The Verification section likewise reads as if SDK-level test coverage exists. However, the final diff touches only three files — .github/actions/conformance/client.py, expected-failures.yml, and expected-failures.2026-07-28.yml — and contains no test file at all.

How this happened. The branch history shows the test did exist at one point: commit 2c47fc4 ("Guard the no-deref test with a socket tripwire") added/modified a test in tests/client/test_output_schema_validation.py, and the final commit c7522f2 ("Drop the Python no-deref unit test") deliberately removed it, on the rationale that the conformance scenario already proves the no-fetch behavior end to end. The drop was intentional and reasonable — but the PR description was never updated to match.

Step-by-step verification.

  1. The changed-files list for this PR is exactly: .github/actions/conformance/client.py, .github/actions/conformance/expected-failures.yml, .github/actions/conformance/expected-failures.2026-07-28.yml. No file under tests/ is touched.
  2. git log on the branch shows 2c47fc4 adding/guarding the unit test and c7522f2 removing it (≈43 lines deleted from tests/client/test_output_schema_validation.py).
  3. A search of tests/ for no-deref / dereference coverage of output-schema $refs finds nothing remaining.
  4. Yet the description still says the client test was "Added", so a reviewer reading the description would expect an in-repo regression test that does not exist in the merged content.

Why it matters. Reviewers and future maintainers use the PR description to understand what coverage exists. As written, it overstates the in-repo coverage: the only thing asserting the no-dereference behavior is the external json-schema-ref-no-deref conformance scenario, which lives in the conformance harness rather than the SDK's own test suite. If that scenario is ever reorganized or the harness pin changes, there is no local regression test backing the behavior — and anyone relying on the description would believe there is.

This is not a code defect. All verifiers agreed the test removal itself was a defensible scoping decision (the unit test was a redundant local mirror of the conformance scenario), so this is purely a documentation discrepancy and does not block the PR.

How to fix. Either (a) update the PR description — remove or reword the "Added a client test…" bullet and adjust the Verification section to say the no-deref behavior is covered by the json-schema-ref-no-deref conformance scenario only — or (b) re-add the dropped unit test from 2c47fc4 if maintainers prefer in-repo coverage.



@register("tools_call")
async def run_tools_call(server_url: str) -> None:
"""Connect, initialize, list tools, call add_numbers(a=5, b=3), close."""
Expand Down
2 changes: 0 additions & 2 deletions .github/actions/conformance/expected-failures.2026-07-28.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ client:
# SEP-2243 (HTTP standardization): no fixture handler / client header support yet.
- http-custom-headers
- http-invalid-tool-headers
# SEP-2106 (JSON Schema $ref handling): client still dereferences network $refs.
- json-schema-ref-no-deref
# SEP-2352 (authorization server migration): client does not re-register when
# PRM authorization_servers changes.
- auth/authorization-server-migration
Expand Down
2 changes: 0 additions & 2 deletions .github/actions/conformance/expected-failures.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ client:
# SEP-2243 (HTTP standardization): no fixture handler / client header support yet.
- http-custom-headers
- http-invalid-tool-headers
# SEP-2106 (JSON Schema $ref handling): client still dereferences network $refs.
- json-schema-ref-no-deref
# SEP-2352 (authorization server migration): client does not re-register when
# PRM authorization_servers changes.
- auth/authorization-server-migration
Expand Down
Loading