fix(actions): Merge path-item parameters and flatten allOf in request bodies#12660
fix(actions): Merge path-item parameters and flatten allOf in request bodies#12660CarterPape wants to merge 1 commit intodanny-avila:mainfrom
Conversation
… bodies
Two bugs in openapiToFunction caused broken tool schemas when importing
OpenAPI specs that use path-item-level shared parameters or allOf in
request bodies (e.g. the FireCrawl v2 spec).
Bug 1 — path-item parameters:
Object.entries(pathItem) iterated ALL keys including non-method ones like
`parameters`, `summary`, `description`, and `servers`. This created ghost
tools (e.g. `parameters_/crawl/{id}`) and silently dropped the shared
path params from the real operations.
Fix: restrict the inner loop to the 8 valid HTTP method keys using a
`HTTP_METHODS` Set, then explicitly extract path-item-level `parameters`
and merge them into each operation's own parameter list, with operation-
level entries winning on (name, in) collision per OpenAPI 3.0 §4.8.9.
Bug 2 — allOf in request bodies:
resolveRef only unwraps $ref; it does not expand allOf. Request bodies
like `{ allOf: [ScrapeOptions, { properties: { url } }] }` resulted in
resolvedSchema.properties being undefined, so no body fields were added
to the tool.
Fix: add a flattenAllOf() helper that recursively resolves $ref and
merges allOf member schemas (properties, required, type), with a cycle
guard. Use it in place of the bare resolveRef call on request body schemas.
Adds 8 new unit tests covering: shared params merge, no ghost tools,
operation-level param wins on collision, non-method keys ignored,
inline allOf body, $ref-inside-allOf, nested allOf via $ref, and
properties alongside allOf.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gitnexus index |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac124cc62
ℹ️ 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".
| components, | ||
| seen, | ||
| ); | ||
| merged.properties = { ...merged.properties, ...flat.properties }; |
There was a problem hiding this comment.
Preserve intersected constraints when merging allOf properties
When allOf members define the same property name, merged.properties = { ...merged.properties, ...flat.properties } replaces the entire earlier property schema with the later one, which drops constraints instead of intersecting them. For example, a first member can declare id as type: string and a later member add minLength; after this merge, type is lost. This produces incorrect tool/zod schemas for composed request bodies and can allow invalid payloads or reject valid ones depending on member order.
Useful? React with 👍 / 👎.
Fixes #12659
Summary
Two bugs in
openapiToFunction(packages/data-provider/src/actions.ts) corrupt tool schemas when importing OpenAPI 3 specs that use path-item-level sharedparametersorallOfin request bodies — both common, spec-compliant patterns.Discovered while importing the FireCrawl v2 spec (30 operations), where they together broke 20 of 26 tools.
Bug 1 — Path-item
parameterstreated as a method (ghost tools + missing path params)Object.entries(pathItem)iterated every key including non-method ones (parameters,summary,description,servers). Theparameterskey became a ghost tool, and the shared parameters were never merged into the real operations — so path params like{id}went missing.Fix: restrict the inner loop to the 8 valid HTTP method names via an
HTTP_METHODSset; extract path-itemparametersand merge them into each operation (operation-level wins on(name, in)collision per OpenAPI 3.0 §4.8.9).Before / After:
Bug 2 —
allOfin request body silently droppedresolveRefonly unwraps$ref; it doesn't expandallOf. A request body{ allOf: [A, B] }leftresolvedSchema.propertiesasundefined, so no body fields were added to the tool.Fix: add
flattenAllOf()— recursively resolves$refand mergesallOfmember schemas (properties, required, type), with a cycle guard. Used in place of the bareresolveRefcall on request body schemas.Before / After:
Test plan
packages/data-provider/specs/actions.spec.tsparameterskey(name, in)collisionsummary,description,servers) are ignoredallOfbody is flattened$refinsideallOfmembers resolves correctlyallOfvia$ref(component schema itself usesallOf) resolves fullyallOfon the same schema object are includedactions.spec.tsstill pass (196 total)tsc --noEmitclean🤖 Generated with Claude Code