fix: strip discriminator after dereferencing schemas by jlowin · Pull Request #3682 · PrefectHQ/fastmcp

jlowin · 2026-03-28T21:22:47Z

DereferenceRefsMiddleware inlines $ref entries via jsonref.replace_refs and then removes $defs. But discriminator.mapping values are plain strings ("identify": "#/$defs/IdentifyPerson"), not $ref objects, so jsonref doesn't touch them — they end up pointing at definitions that no longer exist, producing invalid JSON Schema.

discriminator is an OpenAPI extension that's redundant after inlining anyway: each anyOf variant already carries const on the discriminant field. The fix strips discriminator keys from the schema after dereferencing, the same approach the codebase already uses for OpenAPI imports.

class IdentifyPerson(BaseModel):
    action: Literal["identify"]
    name: str

class PersonDelete(BaseModel):
    action: Literal["delete"]

# Before: discriminator.mapping points at removed $defs — invalid schema
# After: discriminator stripped, anyOf variants self-describe via const

Fixes #3679

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 232dd6dbf5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-28T21:24:16Z

+    """
+    if isinstance(obj, dict):
+        return {
+            k: _strip_discriminator(v) for k, v in obj.items() if k != "discriminator"


Preserve property names when stripping discriminator

_strip_discriminator removes every dictionary key named "discriminator" regardless of context, so it also deletes legitimate property definitions under properties (and keys inside defaults/examples data). For schemas that actually include a field named discriminator, dereference_refs now drops that field and can leave required pointing to a missing property, producing an invalid/broken schema. The stripping should be limited to the OpenAPI discriminator keyword in schema objects, not arbitrary map keys.

Useful? React with 👍 / 👎.

marvin-context-protocol · 2026-03-28T21:29:27Z

Test Failure Analysis

Summary: The Windows CI job timed out on a pre-existing test (test_run_mcp_config) that is unrelated to this PR. This PR only modifies src/fastmcp/utilities/json_schema.py and tests/utilities/test_json_schema.py; all Linux jobs passed cleanly.

Root Cause: tests/cli/test_run.py::TestMCPConfig::test_run_mcp_config was added in commit 90bd4ff (PR #3681, merged just before this PR was opened) and hit the 5-second unit-test timeout on the Windows runner. The test spawns a subprocess via stdio transport (StdioMCPServer(command='python', ...)) and connects a Client to it — process startup on Windows is significantly slower than on Linux, making this test intermittently flaky there. The same test passed on Windows in run 23695280394 (the push-to-main run for 90bd4ff), confirming this is a timing/flakiness issue, not a code regression.

Suggested Solution: The fix belongs in the test added by PR #3681, not in this PR. Options:

Increase the timeout for that specific test: mark it @pytest.mark.timeout(30) (or whatever is appropriate for a subprocess-spawning test).
Mark it as an integration test with @pytest.mark.integration, which uses a 30-second timeout and runs without parallelism — matching the nature of the test (it spawns a real subprocess).
Either change would be made in tests/cli/test_run.py in the TestMCPConfig class (lines 101–149).

This PR's changes are correct and unrelated to the failure. A re-run of the Windows job would likely pass.

Detailed Analysis

Failing job: Tests: Python 3.10 on windows-latest (job ID 69032057443)

Failing test: tests/cli/test_run.py::TestMCPConfig::test_run_mcp_config

Timeout log excerpt:

+++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Captured stderr ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  DEBUG  Stdio transport connected  stdio.py:218
...
  File "...selectors.py", line 315, in _select
    r, w, x = select.select(r, w, w, timeout)
+++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++
##[error]Process completed with exit code 1.

The test was waiting inside asyncio's select.select() for the stdio subprocess to become ready — normal for Windows subprocess startup. The 5-second unit-test timeout was reached before the subprocess finished initialising.

This PR's changes (only in src/fastmcp/utilities/json_schema.py and tests/utilities/test_json_schema.py) passed on all other jobs:

Tests: Python 3.10 on ubuntu-latest ✓
Tests: Python 3.13 on ubuntu-latest ✓
MCP conformance tests ✓
Integration tests ✓
Tests with lowest-direct dependencies ✓

Prior run confirming flakiness: Run 23695280394 (push of 90bd4ff to main, which introduced the test) succeeded on Tests: Python 3.10 on windows-latest in 6m31s — the test passed, but only narrowly within the overall job time, suggesting it is on the edge of the timeout.

Related Files

tests/cli/test_run.py — contains TestMCPConfig::test_run_mcp_config (lines 101–149), the test that timed out; added in PR fix: restore HTTP headers in worker execution path for background tasks #3681
src/fastmcp/utilities/json_schema.py — modified by this PR; unrelated to the failure
tests/utilities/test_json_schema.py — new tests added by this PR; unrelated to the failure

Fixes #3679

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca97861a0f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-28T23:07:33Z

+        skip = "discriminator" in obj and ("anyOf" in obj or "oneOf" in obj)
+        return {
+            k: _strip_discriminator(v)
+            for k, v in obj.items()
+            if not (k == "discriminator" and skip)


Restrict discriminator stripping to schema nodes

This recursion removes discriminator from any dictionary that also contains anyOf/oneOf, even when that dictionary is literal data under keywords like default, examples, or const rather than a schema object. In those cases dereference_refs now mutates user-provided payload values (e.g., default: {"anyOf": [...], "discriminator": "x"} loses discriminator), which changes schema semantics and can break downstream defaults/examples. The stripping logic should be constrained to actual schema-key contexts instead of arbitrary nested dict values.

Useful? React with 👍 / 👎.

@tool

Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: Marvin Context Protocol <41898282+Marvin Context [email protected]> Co-authored-by: voidborne-d <[email protected]> Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com> Co-authored-by: Claude <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Jeremiah Lowin <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Sumanshu Nankana <[email protected]> Co-authored-by: Eric Robinson <[email protected]> Co-authored-by: Martim Santos <[email protected]> Co-authored-by: d 🔹 <[email protected]> Co-authored-by: Matthieu B <[email protected]> Co-authored-by: Sascha Buehrle <[email protected]> Co-authored-by: Hakancan <[email protected]> Co-authored-by: nightcityblade <[email protected]> Co-authored-by: Matt Hallowell <[email protected]> Co-authored-by: nate nowack <[email protected]> Co-authored-by: Bill Easton <[email protected]> Co-authored-by: Marcus Shu <[email protected]> Co-authored-by: Rushabh Doshi <[email protected]> Co-authored-by: AIKAWA Shigechika <[email protected]> Co-authored-by: Jeremy Simon <[email protected]> Co-authored-by: Miguel Miranda Dias <[email protected]> Co-authored-by: Anthony James Padavano <[email protected]> Co-authored-by: Mostafa Kamal <[email protected]> Fix auto-close MRE script posting comment without closing (#3386) Fix WorkOS token scope verification bypass 🤖 Generated with Codex (#3407) Fix initialize McpError fallthrough 🤖 Generated with Codex (#3413) Fix transform arg collisions with passthrough params (#3431) Fix get_* returning None when latest version is disabled (#3439) Fix get_* returning None when latest version is disabled (#3421) Fix server lifespan overlap teardown (#3415) Fix $ref output schema object detection regression (#3420) resolved annotations (#3429) Fix async partial callables rejected by iscoroutinefunction (#3438) Fix async partial callables rejected by iscoroutinefunction (#3423) fix: add version to components (#3458) fix: use intent-based flag for OIDC scope patch in load_access_token (#3465) Fixes #3461 fix: normalize Google scope shorthands and surface valid_scopes (#3477) fix: resolve ty 0.0.23 type-checking errors and bump pin (#3481) fix: shield lifespan teardown from cancellation (#3480) fix: forward custom_route endpoints from mounted servers (#3462) fix updates _get_additional_http_routes() to traverse providers, Fixes #3457 fix: remove hardcoded version from CLI help text (#3456) fix: monty 0.0.8 compatibility, drop external_functions from constructor (#3468) fix: task test teardown hanging 5s per test (#3499) Closes #3498 fix: validate workspace path is a directory before cursor install (#3440) Fixes #3426 fix: handle re.error from malformed URI templates in build_regex (#3501) fix: reject empty/OIDC-only required_scopes in AzureProvider (#3503) fix: restrict $ref resolution to local refs only (SSRF/LFI) (#3502) fix warnings and timeouts (#3504) close upgrade check issue when build passes (#3505) Closes #3484 fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767) (#3507) fix: prevent path traversal in skill download (#3493) fix: prefer IdP-granted scopes over client-requested scopes in OAuthProxy (#3492) fix: remove unrelated transform and http.py changes from PR scope fix: remove forced follow_redirects from httpx_client_factory calls (#3496) fix: stop passing follow_redirects to httpx_client_factory fix: restore follow_redirects=True for custom httpx client factories Closes #3509 fix: CSRF double-submit cookie check in consent flow (#3519) fix: validate server names in install commands (#3522) fix: use raw strings for regex in pytest.raises match (#3523) fix: reject refresh tokens used as Bearer access tokens (#3524) fix: route ResourcesAsTools/PromptsAsTools through server middleware (#3495) fix: resolve Pyright "Module is not callable" on @tool, @resource, @prompt decorators (#3540) fix: filter warnings by message in KEY_PREFIX test (#3549) fix: suppress output schema for ToolResult subclass annotations (#3548) fix: increase sleep duration in proxy cache tests (#3567) fix: store absolute token expiry to prevent stale expires_in on reload (#3572) fix: preserve tool properties named 'title' during schema compression (#3582) Fix loopback redirect URI port matching per RFC 8252 §7.3 (#3589) Fix app tool routing: visibility check and middleware propagation (#3591) Fix query parameter serialization to respect OpenAPI explode/style settings (#3595) Fix dev apps form: union types, textarea support, JSON parsing (#3597) fix(google): replace deprecated /oauth2/v1/tokeninfo with /oauth2/v3/userinfo (#3603) fix: resolve EntraOBOToken dependency injection through MultiAuth (#3609) fix(docs): correct misleading stateless_http header (#3622) fix: filesystem provider import machinery (#3626) Closes #3625 (issues 2, 3, 6) fix: recover StdioTransport after subprocess exits (#3630) fix(server): preserve mounted tool task metadata (#3632) fix: scope deprecation warning filter to FastMCPDeprecationWarning (#3649) fix imports, add PrefabAppConfig (#3650) fix: resolve CurrentFastMCP/ctx.fastmcp to child server in mounted background tasks (#3651) Fix blocking docs issues: chart imports, Select API, Rx consistency (#3652) closed by default (#3657) Fix prompt caching middleware missing wrap/unwrap round-trip (#3666) fix: serialize object query params per OpenAPI style/explode rules (#3662) Fixes #2857 fix: HTTP request headers not accessible in background task workers (#3631) fix: restore HTTP headers in worker execution path for background tasks (#3681) fix: strip discriminator after dereferencing schemas (#3682) fix: remove stale ty:ignore directives for ty 0.0.26 (#3684) Fix docs gaps in app provider pages (#3690) fix: dev apps log panel UX improvements (#3698) fix dev server empty string args (#3700)

jlowin added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels Mar 28, 2026

chatgpt-codex-connector Bot reviewed Mar 28, 2026

View reviewed changes

fix: strip discriminator after dereferencing schemas

b41a858

Fixes #3679

jlowin force-pushed the fix/discriminator-mapping-dangle branch from 232dd6d to b41a858 Compare March 28, 2026 23:02

scope discriminator stripping to anyOf/oneOf siblings

ca97861

chatgpt-codex-connector Bot reviewed Mar 28, 2026

View reviewed changes

skip data-holding keys when stripping discriminator

9c00029

jlowin merged commit 923695b into main Mar 28, 2026
8 checks passed

jlowin deleted the fix/discriminator-mapping-dangle branch March 28, 2026 23:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: strip discriminator after dereferencing schemas#3682

fix: strip discriminator after dereferencing schemas#3682
jlowin merged 3 commits intomainfrom
fix/discriminator-mapping-dangle

jlowin commented Mar 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 28, 2026

Uh oh!

marvin-context-protocol Bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jlowin commented Mar 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

marvin-context-protocol Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Failure Analysis

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

marvin-context-protocol Bot commented Mar 28, 2026 •

edited

Loading