Skip to content

fix(app): openapi importer - resolve $ref'd path items and form bodies, honor consumes - #208

Merged
athrvk merged 3 commits into
masterfrom
claude/vibrant-noether-3hpd59
Jul 30, 2026
Merged

fix(app): openapi importer - resolve $ref'd path items and form bodies, honor consumes#208
athrvk merged 3 commits into
masterfrom
claude/vibrant-noether-3hpd59

Conversation

@athrvk

@athrvk athrvk commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Description

Part of #192. All seven findings in #193 were re-verified against current master (9fc4b9d) before editing - the line anchors had shifted slightly, but every defect is still present exactly as described.

Plan (root cause, approach, rejected alternative). The root cause of the two silent-loss items is the same one twice: the parsers resolve $ref at some levels and not others, and the unresolved level then reads as "nothing here". A path item is only checked for method keys, so a {"$ref": ...} item contributes none and its whole path disappears; the form-body branch reads content[ct].schema?.properties directly, so a schema behind $ref/allOf yields zero fields while the JSON branch two lines up resolves the same shape fine. So the approach in both cases is to route the read through the resolution the parser already owns: resolvePathItem before the method loop, and schemaFieldNames - which samples the schema and takes the stub's own keys - for the form branch. For the form fields I rejected writing a second properties-walker that merges allOf: it would resolve further than the JSON body next to it, so the same schema would import differently depending on its media type. Going through sampleSchema gives form and JSON bodies provably the same reach (first branch only), which is the parity the issue asks for.

The helpers live in a new openapi-shared.ts, imported by both parsers, rather than as a copy in each. The two parsers are structural clones - which is precisely how both grew the same unguarded parameters spread and the same hardcoded skipped: [] - and three of these fixes had to land in both. For item 4 I took route (a), support the keywords, over (b) narrowing the detector: detect() matching 3.1.x is right (a 3.1 spec imports fine), the gap was only in the sampler, and telling users "3.1 detected but not really sampled" is a worse contract than reading three keywords.

Findings, and what changed

# Finding Fix
1 Path-item-level $ref never resolved - a whole path silently vanishes resolvePathItem (openapi-shared.ts) resolves a {"$ref": ...} path item before the HTTP_METHODS loop, in both parsers. One hop only, matching the parameter / requestBody refs (a ref-to-a-ref would need a cycle guard and is not a shape generators emit). A path item that is not an object, or whose $ref does not resolve to one, is now counted rather than skipped in silence.
2 Urlencoded/multipart body schema behind $ref or allOf yields zero form fields Field names come from the new schemaFieldNames(schema, resolveRef) (schema-sampler.ts), which samples the schema and returns Object.keys() of the stub. $ref and allOf now resolve for form bodies exactly as far as they already did for JSON bodies - first branch only, deliberately, so the two cannot disagree. A schema that samples to a non-object contributes no names.
3 Swagger 2 formData always multipart, ignoring consumes buildSwaggerOp derives formMode from the operation's (else the spec's) consumes: urlencoded when it names application/x-www-form-urlencoded and not multipart; form-data when it names multipart, names both (only multipart can carry a type: file field), or names neither - so today's default is preserved for a spec with no consumes. Compared on the media type alone, so ; charset=utf-8 still matches.
4 Detector claims 3.1; sampler ignores 3.1 keywords Route (a): schema-sampler.ts handles Array.isArray(schema.type) (samples the first non-"null" member; an only-"null" type and the scalar type: "null" sample as null) and reads const. const is placed above example - JSON Schema makes it the only permitted value, where example is an annotation.
5 TRACE dropped with no skipped record A trace operation is counted as a new unsupported_method SkippedItem and stays out of requestCount. UNSUPPORTED_METHODS is the list and trace is its only member (a path item defines exactly eight methods). HttpMethod is not touched, per the issue.
6 Non-array parameters crashes the whole parse Both parsers' parameter lists go through SkipTally.params, which returns [] for a non-array and counts a malformed_spec. Absent stays uncounted - only a present-but-wrong shape is a defect. Every other path in the file still imports.

Beyond the issue's literal list (both noted in the docs): the sampler also reads 3.1's plural examples (first entry, when there is no singular example). The issue's Problem section names it as never read and route (a) is "support the keywords"; leaving it out would have meant a 3.1 spec's request-body example still being ignored, and the doc caveating it. Second, item 6's "ideally, record it" is implemented rather than skipped - silently treating malformed input as empty is the same silent-loss shape as the findings being fixed here.

meta.skipped gains its first producer in these parsers, so the two new kinds have a reader before they have a writer: ImportModal.tsx:506-514 already renders ${count} ${kind} for every meta.skipped entry, which is where 1 unsupported_method / 2 malformed_spec surface in the preview. No new UI was needed and none was added.

Blast radius traced. SkippedItem.kind is read in exactly two places - ImportModal's preview line and insomnia-v4.ts's cast - and neither switches exhaustively on the union, so widening it changes no existing behaviour (tsc confirms). sampleSchema has three call sites (both OpenAPI parsers' body paths); the added const/examples/type-array handling only fires on schemas that previously fell through to {} or ignored a pinned value, so existing fixtures are unaffected - the two parser suites and orchestrator.fixture-parity.test.ts pass unchanged. schemaFieldNames is v3-only (Swagger 2 form fields come from formData parameters, not a schema). No engine file, no POST /import/apply payload shape, and no MCP surface is touched - rg -n "openapi-v3|openapi-v2|schema-sampler" app/electron engine is still empty, matching the issue's "App changes: none".

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Testing

All run on this branch at eeee26d, on Linux.

  • cd app && pnpm test - 1947 passed, 230 files (1926 on the base commit; 21 tests added).
  • cd app && pnpm type-check - clean.
  • mkdocs build --strict -f .github/mkdocs.yml - clean (the new #consumes--form-encoding and cross-doc anchors resolve).
  • npx eslint on the eight touched app files - 29 problems, down from 31 on the base commit; every one is a pre-existing no-explicit-any in these parsers belonging to app: burn down the ESLint debt (100 errors) and enforce lint in the PR CI matrix #189's lint burn-down (two went away because the pathParams: any[] = [] defaults did). No new diagnostics.
  • npx prettier --check on the eight touched app files - clean (all seven pre-existing ones were prettier-clean before, so formatting them was in scope). The three docs/app/import-collections/*.md files are not prettier-clean and were not clean on the base commit either, so they were deliberately left unformatted per CLAUDE.md.
  • Engine build and ctest were not run: no C++ changed (the issue's own acceptance criteria include "no change to any engine file"), so no engine test could go from green to red.

New coverage: openapi-v3.test.ts (a components.pathItems $ref path item incl. its shared parameters, an unresolvable one, a $ref'd urlencoded body, an allOf multipart body, TRACE, a non-array parameters on both a path item and an operation, and a clean-fixture assertion that skipped stays []), openapi-v2.test.ts (the six consumes → form-encoding cases, spec-level consumes fallback, field rows preserved, path-item $ref, the parameters guard), and schema-sampler.test.ts (type arrays in three shapes, only-null, const incl. beating example and const: null, examples[0], plus a schemaFieldNames block).

Every fix was mutation-checked by script - reverted in place, its covering tests run, then restored from a copy. All fourteen reverts fail, and only the intended tests fail:

Reverted Failing tests
resolvePathItem in v3 → raw path item 2 ($ref'd item, unresolvable item)
schemaFieldNamesschema?.properties 2 ($ref form body, allOf form body)
the UNSUPPORTED_METHODS tally 1 (TRACE)
tally.params(op.parameters) in v3 1
tally.params(pathItem.parameters) in v3 1
mode: formModemode: "form-data" 3 (per-consumes, field rows, spec-level fallback)
resolvePathItem in v2 1
tally.params in v2 1
the type-array normalisation 2
the const branch 1
the examples[0] branch 1
case "null" 1
schemaFieldNames' non-object guard 1
SkipTally.params' malformed count 2 (one per parser)

The two skipped assertions on the committed fixtures are what stop the tally from becoming noise: a producer that fires on a healthy spec would fail them, not merely add a warning nobody notices.

Checklist

  • My code follows the style guidelines
  • I have performed a self-review
  • I have added tests (if applicable)
  • I have updated documentation

Related Issues

Closes #193

Docs updated in the same commit: docs/app/import-collections/openapi-v3.md (path-item $ref resolution and the malformed_spec count in Tree structure, a new trace paragraph, the form-body table row plus a paragraph on why field names go through the sampler, the rewritten const/example/examples precedence and type-array bullets, the null row, the parameter-guard paragraph, and a meta.skipped kind table replacing "this parser never emits SkippedItems"), docs/app/import-collections/openapi-v2.md (a new consumes → form encoding section with the four-case table, the formData mapping-table row, path-item $ref and parameter-guard paragraphs, the sampler bullets, the corrected "Dropped / not represented" and meta lines, and two rows in the v2-vs-v3 contrast table), and docs/app/import-collections/README.md (the widened SkippedItem union with what each new kind means, and the sampleSchema / schemaFieldNames bullets).


Generated by Claude Code

…s, honor consumes

Seven gaps in the OpenAPI 3 / Swagger 2 importers, the two silent-loss ones
first:

- A path item written as `{"$ref": ...}` has no method keys, so every operation
  under it was skipped with no request built, no `requestCount`, and nothing in
  `meta.skipped`. Both parsers now resolve it through `resolvePathItem`.
- An urlencoded/multipart body schema behind `$ref` or `allOf` has no literal
  `properties`, so the form branch produced the right body mode with an empty
  field list. Field names now come from `schemaFieldNames`, which samples the
  schema - the same `$ref`/`allOf` resolution JSON bodies already got.
- Swagger 2 `formData` was unconditionally multipart; it now picks
  `x-www-form-urlencoded` vs `form-data` from the operation's (else the spec's)
  `consumes`, matching what the endpoint actually accepts.
- The detector claims 3.1, so the sampler now reads the 3.1 keywords it was
  ignoring: type arrays (`["string", "null"]` samples the first non-null
  member), `const` (outranking `example`), and the plural `examples`.
- A `trace` operation cannot be represented (`HttpMethod` has no `"TRACE"`) and
  is now counted as an `unsupported_method` SkippedItem instead of vanishing
  from `requestCount`.
- A non-array `parameters` (the missing-`-` YAML mistake) threw `is not
  iterable` and aborted the whole file. It is stepped over, counted as
  `malformed_spec`, and every other path still imports.

`openapi-shared.ts` holds the three helpers both parsers needed - they are
structural clones, which is how both grew the same unguarded spread and the
same hardcoded `skipped: []`.

Closes #193
@github-actions github-actions Bot added documentation Documentation, guides, examples component:app Electron + React UI labels Jul 30, 2026
@athrvk

athrvk commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Fix the merge conflict

Resolves the SkippedItem.kind conflict with #207: both sides widened the union
(malformed_item there, unsupported_method + malformed_spec here), so the
resolution keeps all three and the doc comment now separates the kinds that
mean "Vayu cannot represent this" from the three that do not.

The README's SkippedItem line is updated to match - master documented
malformed_item in postman.md rather than there, so listing only this branch's
two new kinds would have left the union under-documented.

athrvk commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Merged master (e666276) in - conflict resolved, branch is mergeable again.

One conflict, in importers/types.ts: #207 and this branch both widened SkippedItem.kind. Kept all three new kinds (malformed_item from #207, unsupported_method + malformed_spec from here) and rewrote the doc comment to separate the kinds that mean "Vayu cannot represent this" from the three that do not.

I also updated the SkippedItem line in docs/app/import-collections/README.md. Git took this branch's version of that block wholesale, and master had left it listing only the five original kinds (#207 documented malformed_item in postman.md instead), so the auto-merged result would have described an 8-member union with 7 members - it now names all eight.

Everything else auto-merged. Worth noting the two semantic overlaps that a clean auto-merge does not prove correct, both checked: #207's normalizeVars(path, { pathTemplates: true }) call sites in the two OpenAPI parsers survived (the {{baseUrl}}/users/{{id}} assertion in the new path-item-$ref test would fail otherwise), and #207's new optional RequestDraft.followRedirects / maxRedirects are untouched here - OpenAPI has no equivalent field, so these parsers correctly leave both absent.

Re-verified on the merged tree (6ca3523): pnpm test 1985 passed, 231 files, pnpm type-check clean, mkdocs build --strict clean, prettier clean on the touched files, and eslint still at the same 29 pre-existing no-explicit-any from #189. The test/verification numbers in the PR body above were measured pre-merge at eeee26d.


Generated by Claude Code

@athrvk
athrvk merged commit 5281c96 into master Jul 30, 2026
14 checks passed
@athrvk
athrvk deleted the claude/vibrant-noether-3hpd59 branch July 30, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:app Electron + React UI documentation Documentation, guides, examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app(import): OpenAPI importer - unresolved $refs drop paths and form bodies, server variables lost, 3.1 gaps

1 participant