Skip to content

fix(schema_converter): allOf containing a false member must reject every value - #3921

Merged
Alberto Schiabel (jkomyno) merged 4 commits into
ComposioHQ:nextfrom
manjunathbhaskar:fix/schema-converter-allof-false
Jul 28, 2026
Merged

fix(schema_converter): allOf containing a false member must reject every value#3921
Alberto Schiabel (jkomyno) merged 4 commits into
ComposioHQ:nextfrom
manjunathbhaskar:fix/schema-converter-allof-false

Conversation

@manjunathbhaskar

@manjunathbhaskar manjunath bhaskar (manjunathbhaskar) commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserves JSON Schema boolean-schema semantics throughout the Python schema converter:

  • an unsatisfiable allOf now propagates through nested combiners instead of being dropped;
  • impossible array items, object properties, and local definition references remain rejecting;
  • composed impossible schemas use a Pydantic type that rejects every value, including JSON null;
  • the existing json_schema_to_pydantic_type(False) is None compatibility behavior remains unchanged.

Problem

The original fix handled only a literal false directly inside one allOf. Recursive filtering still interpreted the same None value as both "discard this union branch" and "the whole schema is impossible." That widened nested allOf schemas to str, removed impossible properties/items/definitions, and allowed JSON null because Pydantic treats a bare None annotation as NoneType.

Fix

  • Add a private unsatisfiable marker with Pydantic core- and JSON-schema hooks.
  • Drop that marker only from anyOf/oneOf; propagate it through allOf.
  • Preserve rejecting semantics for nested object properties, array items, and local $defs/definitions references.
  • Emit {"not": {}} from the rejecting Pydantic type.
  • Correct the related empty-allOf and all-false-anyOf expectations.

Testing

  • uv run pytest tests/test_schema_converter.py tests/test_schema_parser.py -q - 106 passed
  • uv run pytest -m schema -q - 71 passed
  • make tst - 928 passed
  • make chk - Ruff and mypy passed across core, providers, tests, and scripts

Regression coverage includes direct and nested allOf, JSON null, empty arrays with impossible items, optional and required impossible properties, local definition references, generated JSON Schema, and shared model consumers.

Note

#3876 adds coverage for the same function's anyOf handling but does not change this allOf propagation path.

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

manjunath bhaskar (@manjunathbhaskar) is attempting to deploy a commit to the Composio Team on Vercel.

A member of the Team first needs to authorize it.

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 453dbae. Configure here.

Comment thread python/composio/utils/schema_converter.py Outdated
@manjunathbhaskar

Copy link
Copy Markdown
Contributor Author

Good catch. None -> str was dead code for dict inputs before this PR (a dict-input call to _filter_boolean_schemas never returned None previously), and this PR was the first thing to make that path reachable -- with the wrong fallback for it.

Fixed by mirroring the existing bare-false-schema convention two lines above (returning None/NoneType instead of str), rather than inventing a new impossible-type mechanism. Verified: string, int, and dict inputs are now all correctly rejected (only bare None still validates, matching the pre-existing bare-false behavior -- not a regression).

Added test_schema_converter_allof_with_false_member_rejects_non_dict_values_too covering this. Full suite: 266 passed, 0 failed.

@jkomyno
Alberto Schiabel (jkomyno) merged commit 8cb290b into ComposioHQ:next Jul 28, 2026
13 of 15 checks passed
@jkomyno

Copy link
Copy Markdown
Collaborator

Thanks!

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.

2 participants