feat(generator): add xml response_format for XML-backed APIs by coopdogGGs · Pull Request #3065 · mvanhorn/cli-printing-press

coopdogGGs · 2026-06-18T02:02:27Z

Intent

Generated CLIs/MCP servers could not cleanly consume APIs that return XML. The
HTTP layer already let XML bytes through as text, but with the default
response_format: json the body was opaque to --json, --select, table
output, and MCP tools. This adds first-class XML support so an XML API behaves
like any JSON API in a printed CLI.

Issue: N/A

Approach

Add xml as a fifth response_format, mirroring how csv already turns a
non-JSON body into a generic JSON-compatible structure inside the generated
client. The decision was XML→JSON normalization (Design A) over an
xml_extract selector contract mirroring html_extract (Design B): A matches
the CSV precedent, needs no per-mode DOM/XPath machinery or profiler
propagation, and is enough for the motivating consumer (read-only XML APIs).

Three layers:

spec: ResponseFormatXML, Endpoint.UsesXMLResponse,
APISpec.HasXMLResponse, and validation accepts xml.
parser: XML-only success responses are auto-detected and set
response_format: xml plus an Accept: application/xml override so the
server returns XML instead of 406-ing the default application/json. Mixed
JSON+XML responses keep the JSON default.
generator/client: when any endpoint uses response_format: xml, emit a
cliutil.XMLToJSON helper (stdlib encoding/xml, BadgerFish-lite: @attr,
#text, repeated siblings → array, graceful raw pass-through on decode
failure). The client runs XML-only success bodies through it before
returning, so every downstream consumer sees ordinary JSON.

The helper, the client branch, and the isXMLResponseContentType helper are
all gated on HasXMLResponse, so JSON-only CLIs are byte-identical to before.

Scope

Primary area: internal/spec, internal/openapi, internal/generator
(templates + emission).

Why this belongs in this repo: this is core Printing Press generation behavior
— a new response-format class that any future XML-backed spec can use — not a
fix to one printed CLI.

Catalog Justification

N/A — no catalog/*.yaml or catalog/specs/** changes. (Motivated by
BoardGameGeek's XML API, but no catalog entry is added here; that remains a
separate follow-up gated on BGG application approval.)

Risk

Low and contained. All new generation is gated on HasXMLResponse, which is
false for every existing spec, so JSON/CSV/HTML/binary CLIs are unaffected. The
new parser branch only fires when a success response is XML-only (never when
JSON is also offered), so it cannot reroute existing JSON endpoints. Worst case
for a future XML API is a decode failure, which returns the raw XML bytes
unchanged rather than corrupting the body.

Output Contract

Generated output changes only for specs that declare/auto-detect
response_format: xml:

New emitted file internal/cliutil/xml_parse.go (gated on HasXMLResponse).
New isXMLResponseContentType helper and a normalization branch in the
client's success path (both gated).

Evidence:

Emitted-code + compiled-CLI assertion: TestGenerateXMLResponseParseHelper
generates an XML-API CLI, then compiles and runs the generated
cliutil.XMLToJSON against table cases (attributes, nesting, repeated
siblings → array, mixed attr+text, malformed pass-through).
Gating proof: TestGenerateJSONOnlyOmitsXMLResponseParseHelper asserts
JSON-only CLIs emit no xml_parse.go.
No-drift proof: regenerated the generate-golden-api fixture; all 39
generated .go files are byte-identical to the committed golden expected
output (only the copyright-owner line differs, an author-handle artifact).

Verification

go build ./... — pass
go vet ./internal/spec/... ./internal/openapi/... ./internal/generator/... — pass
gofmt -l on touched files — clean
go test ./internal/spec/... — pass
go test ./internal/openapi/... — pass except the pre-existing
Windows-only failure TestGenerateDataEnvelopeAllOfBodyFlags (builds the
binary without the .exe suffix at parser_test.go:1411); passes when
skipped. Confirmed it fails identically on main.
go test ./internal/generator/... -run XMLResponse — pass (compiles + runs
generated helper)
scripts/golden.sh verify — Not conclusive locally: on the author's Windows
box it reports 31 changed cases, but main reports the same 31 (CRLF
line-ending normalization + local Go import ordering), so the baseline itself
does not match here. None of the diffs are XML-related. The per-file
byte-identity check above isolates this change's zero drift; CI (Linux) runs
the authoritative golden verify.
Generator/template change: verified generated output, including emitted-code assertions or compiled generated CLI output
Generator/template change: covered the affected fallback or variant shape, not only happy-path fixtures
Generator/template change: checked emitted definitions and call sites for matching gates

AI / Automation Disclosure

Human-reviewed: AI or automation was used, and a human reviewed the work for intent, fit, and obvious issues before submission

Add ResponseFormatXML, Endpoint.UsesXMLResponse, APISpec.HasXMLResponse, and accept xml in response_format validation. Mirrors the existing csv/html/binary formats.

Detect XML-only success responses and set response_format: xml plus an Accept: application/xml override so the server returns XML rather than 406-ing the default application/json. Mixed JSON+XML responses keep the JSON default.

@attr

Emit a cliutil.XMLToJSON helper (stdlib encoding/xml, BadgerFish-lite: @attr, #text, repeated->array, graceful raw passthrough on decode failure) when any endpoint uses response_format: xml. The generated client runs XML-only success bodies through it so --json, --select, table output, and MCP tools work unchanged. Helper and client branch are gated on HasXMLResponse so JSON-only CLIs are byte-identical.

mergify · 2026-06-18T02:03:03Z

Merge Protections

🔴 2 of 2 protections blocking · waiting on 🤖 CI and 🙋 you

	Protection	Waiting on
🔴	require-ready-label-and-ci	🤖 CI and 🙋 you
🔴	🚦 Auto-queue	🙋 you

🔴 require-ready-label-and-ci

Waiting for

This rule is failing.

🔴 🚦 Auto-queue

Waiting for

label = ready-to-merge

This rule is failing.

When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.

label = ready-to-merge

greptile-apps · 2026-06-18T02:08:14Z

Greptile Summary

This PR adds xml as a fifth response_format so generated CLIs can consume XML-only APIs (e.g. BoardGameGeek). The XML→JSON normalization follows the existing CSV precedent: the parser auto-detects XML-only success responses, the generator emits a BadgerFish-lite cliutil.XMLToJSON helper gated on HasXMLResponse, and the client normalizes XML bodies before returning them so --json, --select, table output, and MCP tools all see ordinary JSON.

internal/spec/spec.go: adds ResponseFormatXML, UsesXMLResponse, HasXMLResponse, and validation support.
internal/openapi/parser.go: adds responseUsesXML auto-detection and pins Accept: application/xml per-endpoint via headerOverrides.
internal/generator/templates/client.go.tmpl: emits a spec-level isXMLResponseContentType check and changes the global Accept header fallback to application/xml when HasXMLResponse is true — both of which behave incorrectly in specs that mix XML and JSON endpoints.

Confidence Score: 4/5

Safe to merge for pure XML specs (the motivating BGG case); mixed XML+JSON specs would send the wrong Accept header on JSON endpoints.

The global Accept fallback is changed from application/json to application/xml for any spec that contains at least one XML endpoint. Since the spec-level HasXMLResponse flag drives both the Accept override and the response-normalization branch, a spec that legitimately mixes XML and JSON endpoints would have all its JSON endpoints default to Accept: application/xml — and have any application/xml-typed success response silently normalized, regardless of which endpoint produced it. The per-endpoint headerOverride already correctly pins Accept: application/xml for XML endpoints, so the global change is redundant for pure XML specs and broken for mixed ones.

internal/generator/templates/client.go.tmpl — both the Accept fallback and the XML normalization branch are gated at the spec level rather than the endpoint level.

Important Files Changed

Filename	Overview
internal/generator/templates/client.go.tmpl	Adds XML Accept-header fallback and response-normalization branch, both gated on spec-level HasXMLResponse — causes incorrect default Accept header and over-broad XML normalization in specs with mixed JSON+XML endpoints.
internal/generator/templates/cliutil_xml_parse.go.tmpl	New BadgerFish-lite XML→JSON normalizer: clean stdlib-only implementation with graceful raw pass-through on decode failure; namespace-collision and decode-error behaviors already noted in prior threads.
internal/openapi/parser.go	Adds responseUsesXML detection (XML-only = no JSON alternative) and pins Accept: application/xml via per-endpoint headerOverride; logic is correct and well-gated.
internal/spec/spec.go	Adds ResponseFormatXML constant, HasXMLResponse/UsesXMLResponse helpers, and updates validation to accept 'xml'; mirrors existing CSV/HTML precedents cleanly.
internal/generator/generator.go	Adds HasXMLResponse gate to emit cliutil_xml_parse.go.tmpl only when needed; mirrors the existing cliutil proxypath emission pattern correctly.
internal/generator/xml_response_test.go	New tests cover helper emission, helper omission for JSON-only specs, and in-process compiled execution of the generated XMLToJSON against multiple XML shapes.
internal/openapi/parser_binary_response_test.go	Updates existing 'XML response gets no Accept override' assertion to the new expected behavior (xml format + Accept override); straightforward test fix.
internal/spec/spec_test.go	Adds XML validation round-trip test and corrects the invalid-format test (was using 'xml' as a bad value; now correctly uses 'yaml').
skills/printing-press/SKILL.md	Adds a brief callout that XML-backed endpoints normalize to JSON via cliutil.XMLToJSON; no functional change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OpenAPI spec endpoint] --> B{responseUsesBinary?}
    B -- yes --> C[ResponseFormatBinary]
    B -- no --> D{responseUsesXML?\nXML types present,\nno JSON types}
    D -- yes --> E[ResponseFormatXML\n+ Accept: application/xml\nheaderOverride]
    D -- no --> F[ResponseFormatJSON default]

    E --> G[HasXMLResponse = true\nfor entire spec]
    F --> G

    G --> H{Generated client\ndoInternal}
    H --> I{Accept header empty?}
    I -- yes, binary --> J[Accept: */*]
    I -- yes, non-binary --> K{HasXMLResponse?}
    K -- yes --> L["Accept: application/xml\n⚠ applies to ALL endpoints\nincluding JSON ones"]
    K -- no --> M[Accept: application/json]
    I -- no, headerOverride set --> N[Uses pre-set Accept\nfrom headerOverride]

    H --> O{resp 2xx}
    O --> P{isBinaryResponseContentType?}
    P -- yes --> Q[wrapBinaryResponse]
    P -- no --> R{HasXMLResponse &&\nisXMLResponseContentType?\n⚠ spec-scoped, not endpoint-scoped}
    R -- yes --> S[cliutil.XMLToJSON]
    R -- no --> T[sanitizeJSONResponse]

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[OpenAPI spec endpoint] --> B{responseUsesBinary?}
    B -- yes --> C[ResponseFormatBinary]
    B -- no --> D{responseUsesXML?\nXML types present,\nno JSON types}
    D -- yes --> E[ResponseFormatXML\n+ Accept: application/xml\nheaderOverride]
    D -- no --> F[ResponseFormatJSON default]

    E --> G[HasXMLResponse = true\nfor entire spec]
    F --> G

    G --> H{Generated client\ndoInternal}
    H --> I{Accept header empty?}
    I -- yes, binary --> J[Accept: */*]
    I -- yes, non-binary --> K{HasXMLResponse?}
    K -- yes --> L["Accept: application/xml\n⚠ applies to ALL endpoints\nincluding JSON ones"]
    K -- no --> M[Accept: application/json]
    I -- no, headerOverride set --> N[Uses pre-set Accept\nfrom headerOverride]

    H --> O{resp 2xx}
    O --> P{isBinaryResponseContentType?}
    P -- yes --> Q[wrapBinaryResponse]
    P -- no --> R{HasXMLResponse &&\nisXMLResponseContentType?\n⚠ spec-scoped, not endpoint-scoped}
    R -- yes --> S[cliutil.XMLToJSON]
    R -- no --> T[sanitizeJSONResponse]

_{Reviews (2): Last reviewed commit: "feat(generator): default Accept to appli..." | Re-trigger Greptile}

…ormat XML-backed specs now negotiate application/xml on the if-empty Accept default instead of application/json, so direct client calls on XML-only APIs avoid a 406. Non-XML specs are byte-identical (still application/json).

tmchow · 2026-06-21T06:59:18Z

@coopdogGGs Your agents needs to follow directions in AGENTS.md if it's going to open PRs. Specifically, it needs to babysit the PRs to address code review feedback.

In addition to addressing code review feedback, please fix the Greptile comment:

The global Accept fallback is changed from application/json to application/xml for any spec that contains at least one XML endpoint. Since the spec-level HasXMLResponse flag drives both the Accept override and the response-normalization branch, a spec that legitimately mixes XML and JSON endpoints would have all its JSON endpoints default to Accept: application/xml — and have any application/xml-typed success response silently normalized, regardless of which endpoint produced it. The per-endpoint headerOverride already correctly pins Accept: application/xml for XML endpoints, so the global change is redundant for pure XML specs and broken for mixed ones.

When you've done your fixes, feel free to mark as ready for review

The xml response_format change added three explanatory comment lines to the if-empty Accept default in client.go.tmpl, which render into every generated client. Regenerate the five affected golden client.go fixtures so the golden/full-golden lanes pass.

Greptile flagged that gating XML behavior on HasXMLResponse (spec has at least one XML endpoint) is too broad for a mixed spec: a JSON endpoint in the same spec would receive the application/xml Accept default and have any xml-content-type response run through XMLToJSON, risking a 406 or a wrong normalization. Add APISpec.AllResponsesXML (true only when every endpoint is response_format: xml) and gate the spec-wide XML behavior on it instead: the application/xml Accept default, the XML to JSON normalization call site, the isXMLResponseContentType helper, and xml_parse.go emission. XML endpoints in a mixed spec still get their per-endpoint Accept override from the parser; only the spec-wide fallback is withheld, so JSON endpoints keep application/json and are never XML-normalized. BoardGameGeek and any other all-XML API are unaffected (every endpoint is XML, so AllResponsesXML is true). Existing golden cases are all non-XML, so both gates evaluate false and output is byte-identical. Adds spec unit tests for AllResponsesXML and a generator test that a mixed XML/JSON spec emits no spec-wide XML code.

coopdogGGs · 2026-06-23T23:42:28Z

Review follow-up (commit `b87db4b`)

Thanks for the thorough pass. Addressed the two P1 findings; the two P2 findings are deliberate scope decisions explained below.

✅ P1 — Global Accept override breaks JSON endpoints in mixed specs (`client.go.tmpl`)

✅ P1 — XML normalization is spec-scoped, not endpoint-scoped (`client.go.tmpl`)

Both stem from the same root cause: the spec-wide XML behavior was gated on HasXMLResponse() (spec has at least one XML endpoint). I added APISpec.AllResponsesXML() — true only when every endpoint is response_format: xml — and moved all four spec-wide gates onto it:

the application/xml Accept default,
the XMLToJSON normalization call site,
the isXMLResponseContentType helper,
xml_parse.go emission.

So a mixed spec now keeps application/json and never XML-normalizes its JSON endpoints. XML endpoints in a mixed spec still get their per-endpoint Accept: application/xml override from the parser (the layer you correctly identified) — only the spec-wide fallback is withheld. Pure-XML APIs (BoardGameGeek) are unchanged. Added a spec unit test for AllResponsesXML and a generator test asserting a mixed XML/JSON spec emits no spec-wide XML code. All golden cases are non-XML, so output is byte-identical.

Full per-endpoint XML in a mixed spec (so a mixed spec's XML endpoints still normalize) is intentionally out of scope here — it needs a per-request marker like binaryResponse. Filed as a follow-up; this PR keeps the conservative, safe behavior.

ℹ️ P2 — XML namespace collision (`cliutil_xml_parse.go.tmpl`)

Accepted scope. The mapper is intentionally BadgerFish-lite and keys by local name; as the review notes, this is correct for flat XML (BGG and the vast majority of XML REST APIs). Namespace-qualified keys would complicate the JSON shape for every consumer to handle a case no current spec hits. Documenting as a known limitation rather than expanding the output contract.

ℹ️ P2 — XML decode failure surfaces as opaque JSON error (`client.go.tmpl`)

Accepted scope. The raw-bytes passthrough on decode failure is a deliberate "never corrupt the body" choice — the caller still receives the original XML to inspect. A structured {"__xml_raw": …} envelope is a reasonable future enhancement, but changing the failure contract is beyond this PR; noting as a follow-up.

tmchow · 2026-06-26T07:42:33Z

@coopdogGGs still ned this fixed:

The global Accept fallback is changed from application/json to application/xml for any spec that contains at least one XML endpoint. Since the spec-level HasXMLResponse flag drives both the Accept override and the response-normalization branch, a spec that legitimately mixes XML and JSON endpoints would have all its JSON endpoints default to Accept: application/xml — and have any application/xml-typed success response silently normalized, regardless of which endpoint produced it. The per-endpoint headerOverride already correctly pins Accept: application/xml for XML endpoints, so the global change is redundant for pure XML specs and broken for mixed ones.

internal/generator/templates/client.go.tmpl — both the Accept fallback and the XML normalization branch are gated at the spec level rather than the endpoint level.

FortyNinepoint5 added 3 commits June 17, 2026 16:19

feat(spec): register xml response_format

95d46f1

Add ResponseFormatXML, Endpoint.UsesXMLResponse, APISpec.HasXMLResponse, and accept xml in response_format validation. Mirrors the existing csv/html/binary formats.

feat(openapi): auto-detect XML-only responses

a7c90a6

Detect XML-only success responses and set response_format: xml plus an Accept: application/xml override so the server returns XML rather than 406-ing the default application/json. Mixed JSON+XML responses keep the JSON default.

greptile-apps Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread internal/generator/templates/cliutil_xml_parse.go.tmpl

Comment thread internal/generator/templates/client.go.tmpl Outdated

coopdogGGs changed the title ~~feat: add xml response_format for XML-backed APIs~~ feat(generator): add xml response_format for XML-backed APIs Jun 18, 2026

greptile-apps Bot reviewed Jun 19, 2026

View reviewed changes

Comment thread internal/generator/templates/client.go.tmpl

Comment thread internal/generator/templates/client.go.tmpl Outdated

tmchow marked this pull request as draft June 21, 2026 08:17

FortyNinepoint5 added 2 commits June 23, 2026 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(generator): add xml response_format for XML-backed APIs#3065

feat(generator): add xml response_format for XML-backed APIs#3065
coopdogGGs wants to merge 6 commits into
mvanhorn:mainfrom
coopdogGGs:feat/xml-response-format

coopdogGGs commented Jun 18, 2026

Uh oh!

mergify Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jun 18, 2026 •

edited

Loading

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tmchow commented Jun 21, 2026 •

edited

Loading

Uh oh!

coopdogGGs commented Jun 23, 2026

Uh oh!

tmchow commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

coopdogGGs commented Jun 18, 2026

Intent

Approach

Scope

Catalog Justification

Risk

Output Contract

Verification

AI / Automation Disclosure

Uh oh!

mergify Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Protections

🔴 require-ready-label-and-ci

🔴 🚦 Auto-queue

Uh oh!

greptile-apps Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tmchow commented Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coopdogGGs commented Jun 23, 2026

Review follow-up (commit b87db4b)

✅ P1 — Global Accept override breaks JSON endpoints in mixed specs (client.go.tmpl)

✅ P1 — XML normalization is spec-scoped, not endpoint-scoped (client.go.tmpl)

ℹ️ P2 — XML namespace collision (cliutil_xml_parse.go.tmpl)

ℹ️ P2 — XML decode failure surfaces as opaque JSON error (client.go.tmpl)

Uh oh!

tmchow commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mergify Bot commented Jun 18, 2026 •

edited

Loading

greptile-apps Bot commented Jun 18, 2026 •

edited

Loading

tmchow commented Jun 21, 2026 •

edited

Loading

Review follow-up (commit `b87db4b`)

✅ P1 — Global Accept override breaks JSON endpoints in mixed specs (`client.go.tmpl`)

✅ P1 — XML normalization is spec-scoped, not endpoint-scoped (`client.go.tmpl`)

ℹ️ P2 — XML namespace collision (`cliutil_xml_parse.go.tmpl`)

ℹ️ P2 — XML decode failure surfaces as opaque JSON error (`client.go.tmpl`)