OpenAPI: rewrite $ref under propertyNames and patternProperties in _replace_ref_with_defs; add regression test for dict[StrEnum, Model] by manojPal23234 · Pull Request #3306 · PrefectHQ/fastmcp

manojPal23234 · 2026-02-26T08:36:21Z

Summary

This PR fixes a gap in the OpenAPI → JSON Schema normalization path. The function _replace_ref_with_defs already rewrote #/components/schemas/... to #/$defs/... in common locations (e.g., properties, items, additionalProperties, and combinators), but it did not recurse into propertyNames (and patternProperties).
As a result, schemas like dict[StrEnum, Model] (where keys are a StrEnum) produced a dangling propertyNames.$ref pointing to #/components/schemas/..., which some MCP clients failed to resolve.

This change adds explicit handling for both propertyNames and patternProperties inside the established OpenAPI normalization chokepoint.

Fixes #3303.

What changed

src/fastmcp/utilities/openapi/schemas.py
- In _replace_ref_with_defs(...), also recurse into:
  - propertyNames (mirrors the existing pattern used for additionalProperties)
  - patternProperties (recurses into each sub‑schema)
Tests (OpenAPI utilities)
- Added a focused regression test that reproduces the dict[StrEnum, Model] shape and asserts:
  - additionalProperties.$ref → #/$defs/ItemInfo
  - propertyNames.$ref → #/$defs/Category
  - No remaining #/components/schemas/ in the result
Test file path:
tests\utilities\openapi\test_propertynames_ref_rewrite.py

Rationale & scope

Keeps the fix localized to the OpenAPI layer (_replace_ref_with_defs) per review feedback, avoiding a second, parallel normalization path in generic JSON‑Schema utilities.
No collision/aliasing logic is introduced; this path is not expected to receive schemas with conflicting "$defs" and components.schemas.
Minimal, keyword‑targeted diff that closes the specific bug without altering broader behavior.

Regression test (excerpt)

from fastmcp.utilities.openapi.schemas import _replace_ref_with_defs

def test_replace_ref_with_defs_rewrites_propertyNames_ref():
    """
    Regression test for dict[StrEnum, Model] case:
    propertyNames.$ref and additionalProperties.$ref must both normalize to #/$defs/...
    """
    schema = {
        "type": "object",
        "propertyNames": {"$ref": "#/components/schemas/Category"},
        "additionalProperties": {"$ref": "#/components/schemas/ItemInfo"},
    }

    result = _replace_ref_with_defs(schema)

    assert result["additionalProperties"]["$ref"] == "#/$defs/ItemInfo"
    assert result["propertyNames"]["$ref"] == "#/$defs/Category"
    assert "#/components/schemas/" not in str(result)

…onents→$defs; add regression test

…gression test

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 653508ba56

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-02-26T08:41:19Z

+    if isinstance(comp_defs, dict) and comp_defs:
+        dst = schema.setdefault("$defs", {})
+        for k, v in comp_defs.items():
+            dst.setdefault(k, v)


Overwrite colliding $defs entries when migrating components

Using dst.setdefault(k, v) keeps any pre-existing $defs entry even after all #/components/schemas/... references are rewritten to #/$defs/..., so mixed schemas can dereference to the wrong definition. In a schema that already has $defs.User and also has components.schemas.User, every rewritten ref now points at the stale $defs.User instead of the component schema, which changes validation semantics after this commit. Please make migration deterministic for collisions (e.g., prefer components.schemas when normalizing component refs).

Useful? React with 👍 / 👎.

…on; preserve direct $defs refs via alias; add collision test

manojPal23234

Deterministic migration: replaced dst.setdefault(k, v) with a collision-aware policy.

If both $defs.Name and components.schemas.Name exist and differ:
- Preserve the old $defs.Name only if it’s referenced directly via #/$defs/Name, by moving it to an alias (e.g., Name__preexisting) and rewriting those direct refs.
- Then overwrite $defs[Name] with the component definition so normalized component refs resolve to the intended schema.
  This removes migration order dependence and avoids mixed schemas silently picking stale $defs entries.

jlowin

Thanks for digging into this — the issue is real and your investigation of the collision edge case shows careful thinking about the problem space.

That said, I think the fix belongs in a different layer. The function _replace_ref_with_defs in src/fastmcp/utilities/openapi/schemas.py is the established chokepoint for converting OpenAPI-style #/components/schemas/ refs into JSON Schema #/$defs/ refs. It already handles properties, items, additionalProperties, and combinators — it just doesn't recurse into propertyNames (or patternProperties, which has the same shape of problem).

This is essentially a one-line fix in _replace_ref_with_defs — add propertyNames handling the same way additionalProperties is handled. Adding a second, parallel normalization path in json_schema.py (which handles generic JSON Schema and shouldn't need to know about OpenAPI conventions) creates two different rewriting strategies that don't know about each other, which is a maintenance problem.

The collision handling (_migrate_components_to_defs with aliasing) isn't something we need — schemas coming through this path won't have both $defs and components.schemas with conflicting definitions. Let's keep this scoped to the actual bug.

So the ideal version of this PR would be:

Add propertyNames and patternProperties handling to _replace_ref_with_defs (same pattern as additionalProperties)
Add a regression test for the dict[StrEnum, Model] case from #3303, in the existing test suite under tests/utilities/openapi/

That should be a pretty small diff. Happy to help if you have questions about the codebase structure.

marvin-context-protocol · 2026-02-26T15:58:01Z

Note: This comment was edited with updated analysis.

Test Failure Analysis

Summary: The CI static_analysis job failed because the PR was submitted without running prek (pre-commit hooks) locally first. This is a pure formatting/linting issue — no logic errors.

Root Cause: The ruff check and ruff format hooks found issues in 3 files:

Unused import pytest in tests/utilities/openapi/test_propertynames_ref_rewrite.py
Formatting issues in src/fastmcp/utilities/openapi/schemas.py (indentation of a closing })
Missing trailing newline in src/fastmcp/utilities/json_schema.py

Suggested Solution: Run prek locally and commit the resulting changes:

```bash
uv run prek run --all-files
git add -u
git commit -m "Fix formatting"
```

Detailed Analysis

Failed Step: `Run prek`

Error 1 — ruff check (1 auto-fix applied):
```
ruff check...............................................................[Failed]

hook id: ruff-check
exit code: 1
files were modified by this hook

Found 1 error (1 fixed, 0 remaining).
```
Removed unused import pytest from tests/utilities/openapi/test_propertynames_ref_rewrite.py.

Error 2 — ruff format (3 files reformatted):
```
ruff format..............................................................[Failed]

hook id: ruff-format
files were modified by this hook

3 files reformatted, 622 files left unchanged
```

Diffs that need to be applied

src/fastmcp/utilities/json_schema.py — Add trailing newline:
```diff

return schema
\ No newline at end of file

return schema
```

src/fastmcp/utilities/openapi/schemas.py — Fix indentation of closing brace:
```diff
schema["patternProperties"] = {
pattern: _replace_ref_with_defs(subschema)
for pattern, subschema in pattern_properties.items()

```
   }
```

```
       }
```

```

tests/utilities/openapi/test_propertynames_ref_rewrite.py — Remove unused import and reformat dict literals/assertions to single-line style.

Related Files

src/fastmcp/utilities/json_schema.py — missing trailing newline
src/fastmcp/utilities/openapi/schemas.py — indentation issue in patternProperties block
tests/utilities/openapi/test_propertynames_ref_rewrite.py — unused import pytest, formatting issues

…ref_with_defs

manojPal23234 · 2026-02-26T17:53:30Z

Thanks for digging into this — the issue is real and your investigation of the collision edge case shows careful thinking about the problem space.

That said, I think the fix belongs in a different layer. The function _replace_ref_with_defs in src/fastmcp/utilities/openapi/schemas.py is the established chokepoint for converting OpenAPI-style #/components/schemas/ refs into JSON Schema #/$defs/ refs. It already handles properties, items, additionalProperties, and combinators — it just doesn't recurse into propertyNames (or patternProperties, which has the same shape of problem).

This is essentially a one-line fix in _replace_ref_with_defs — add propertyNames handling the same way additionalProperties is handled. Adding a second, parallel normalization path in json_schema.py (which handles generic JSON Schema and shouldn't need to know about OpenAPI conventions) creates two different rewriting strategies that don't know about each other, which is a maintenance problem.

The collision handling (_migrate_components_to_defs with aliasing) isn't something we need — schemas coming through this path won't have both $defs and components.schemas with conflicting definitions. Let's keep this scoped to the actual bug.

So the ideal version of this PR would be:

Add propertyNames and patternProperties handling to _replace_ref_with_defs (same pattern as additionalProperties)

Add a regression test for the dict[StrEnum, Model] case from propertyNames.$ref not converted by _replace_ref_with_defs #3303, in the existing test suite under tests/utilities/openapi/

That should be a pretty small diff. Happy to help if you have questions about the codebase structure.

Hi! Thanks for the guidance and for clarifying the right layer to fix this in.
I’ve updated the PR to:

Scope the normalization to the OpenAPI chokepoint (_replace_ref_with_defs) as suggested.
Add recursive handling for:

propertyNames (mirrors existing additionalProperties handling), and
patternProperties (recurses into each sub‑schema).

Include a regression test for the dict[StrEnum, Model] case (keys via propertyNames, values via additionalProperties) that verifies both refs are rewritten to #/$defs/... and no #/components/schemas/... remains.

Per your review, I removed the earlier generic normalization and the collision/aliasing logic from json_schema.py to avoid duplicating strategies and to keep this fix focused on the actual gap.

jlowin

Thanks! A couple small static failures I'll clean up.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e640700a7b

ℹ️ 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-02-26T18:27:18Z

+    if pattern_properties := schema.get("patternProperties"):
+        if isinstance(pattern_properties, dict):
+            schema["patternProperties"] = {
+                pattern: _replace_ref_with_defs(subschema)


Skip boolean subschemas in patternProperties

The new patternProperties rewrite path unconditionally calls _replace_ref_with_defs(subschema) for each value, but this helper expects a dict and starts with info.copy(). In JSON Schema/OpenAPI 3.1, boolean schemas are valid in patternProperties (for example "^x$": false), so this now raises an AttributeError and aborts schema normalization for otherwise valid specs. Please mirror the additionalProperties handling by bypassing recursion for non-dict entries.

Useful? React with 👍 / 👎.

manojPal23234 added 2 commits February 26, 2026 13:12

Normalize OpenAPI $ref everywhere (incl. propertyNames); migrate comp…

a1c6531

…onents→$defs; add regression test

Fix: normalize $ref in propertyNames and additionalProperties; add re…

653508b

…gression test

marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. openapi Related to OpenAPI integration, parsing, or code generation features. labels Feb 26, 2026

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Deterministic migration: components.schemas override $defs on collisi…

b9cb298

…on; preserve direct $defs refs via alias; add collision test

manojPal23234 changed the title ~~Normalize $ref inside propertyNames (and everywhere); migrate components→$defs; add regression test~~ Normalize $ref in propertyNames (and everywhere), deterministic components→$defs migration; add regression tests Feb 26, 2026

manojPal23234 commented Feb 26, 2026

View reviewed changes

jlowin requested changes Feb 26, 2026

View reviewed changes

Fix: rewrite $ref in propertyNames and patternProperties in _replace_…

d80f786

…ref_with_defs

manojPal23234 force-pushed the fix/normalize-propertynames-refs branch from ffbd7bf to d80f786 Compare February 26, 2026 17:33

manojPal23234 requested a review from jlowin February 26, 2026 18:00

jlowin approved these changes Feb 26, 2026

View reviewed changes

Fix syntax error, formatting, and stray files

e640700

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Skip boolean subschemas in patternProperties

d638f1d

jlowin merged commit 507e6b8 into PrefectHQ:main Feb 26, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenAPI: rewrite $ref under propertyNames and patternProperties in _replace_ref_with_defs; add regression test for dict[StrEnum, Model]#3306

OpenAPI: rewrite $ref under propertyNames and patternProperties in _replace_ref_with_defs; add regression test for dict[StrEnum, Model]#3306
jlowin merged 6 commits intoPrefectHQ:mainfrom
manojPal23234:fix/normalize-propertynames-refs

manojPal23234 commented Feb 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Feb 26, 2026

Uh oh!

manojPal23234 left a comment

Uh oh!

jlowin left a comment

Uh oh!

marvin-context-protocol Bot commented Feb 26, 2026 •

edited

Loading

Failed Step: `Run prek`

Diffs that need to be applied

Uh oh!

manojPal23234 commented Feb 26, 2026

Uh oh!

jlowin left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Feb 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

manojPal23234 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Rationale & scope

Regression test (excerpt)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

manojPal23234 left a comment

Choose a reason for hiding this comment

Uh oh!

jlowin left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Test Failure Analysis

Failed Step: Run prek

Diffs that need to be applied

Uh oh!

manojPal23234 commented Feb 26, 2026

Uh oh!

jlowin left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

manojPal23234 commented Feb 26, 2026 •

edited

Loading

marvin-context-protocol Bot commented Feb 26, 2026 •

edited

Loading

Failed Step: `Run prek`