Skip to content

fix(sdk-generator): treat binary-format application/json responses as raw - #79

Merged
rob-archastro merged 3 commits into
mainfrom
fix/sdk-generator-binary-raw
Aug 16, 2026
Merged

fix(sdk-generator): treat binary-format application/json responses as raw#79
rob-archastro merged 3 commits into
mainfrom
fix/sdk-generator-binary-raw

Conversation

@rob-archastro

@rob-archastro rob-archastro commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Review on ArchCode

Problem and author intent

GET /api/v1/trajectories/{trajectory}/contents returns a raw JSON blob: the spec truthfully declares the 200 response as application/json with schema {type: string, format: binary} (the platform action is returns(:raw, content_type: "application/json")). The generator's extractSuccessResponse takes the typed path for any application/json schema, so the Elixir SDK emits decode: :string for this op. Req auto-decodes the JSON body to a map before the SDK's decode runs, so Codec.decode(map, :string) raises on every successful non-empty fetch — fixture-proven by remote-eval slice 4's adversarial sweep (the op only ever looked green because eval worlds had zero trajectories).

Intent: make the generator route binary-format responses down the existing raw-response path regardless of content type, so the SDK returns the body without a typed decode. The spec is already truthful; no spec change is involved.

What changed

  • src/frontend/operation-parser.ts extractSuccessResponse: an application/json schema with format: "binary" no longer takes the typed path; it falls through to the existing raw-response branch (rawResponse: true, string return type) that already handles non-JSON content types.
  • Frontend parser tests: new fixture ops covering the binary-format application/json case (must be raw) and a plain application/json {type: string} control (must stay typed with a string return type).
  • Full-pipeline backend test (second commit, from the post-open adversarial audit): parses both production shapes below through parseOpenApiSpecgenerateElixir and asserts raw: true emission with no decode: :string, plus the typed control.

Exactly two production ops change classification (enumerated over specs/platform-openapi.json; these are the only format: binary occurrences in the spec, both top-level):

  1. GET /api/v1/trajectories/{trajectory}/contents — the named target.
  2. GET /api/v1/private_service_definitions/{app_id}/{private_service_id} — same shape, same bug: its current typed emission crashes identically on every non-empty success, so the flip is the fix for it too.

Both flips are signature changes in every SDK on regen (Elixir {:ok, String.t()}{:ok, Req.Response.t()}, TypeScript Promise<string>Promise<{content, mimeType}>, and the analogous raw shapes in Python/Go/Swift/Rust). No working consumer can exist for either op today — the typed path throws on any non-empty body — so nothing functioning breaks.

All backends consume op.rawResponse from the shared AST, so every SDK language picks up the corrected classification on its next regen. The companion SDK-side fix (suppressing Req body auto-decode for raw requests) is archastro-elixir PR 18.

Scope

Generator-only (packages/sdk-generator frontend + tests). No spec changes, no emitter changes.

Risk assessment

Low. The change narrows the typed path by exactly one shape — application/json + top-level format: binary — and both ops that hit it crash today, so no working consumer can regress. Non-binary JSON schemas are untouched (control tests pin this). Two accepted tradeoffs, called out honestly:

  • Raw ops inherit the raw path's error behavior: non-2xx JSON error bodies surface as the generic "ArchAstro API returned HTTP <status>" error instead of the structured message/code — a pre-existing property of the raw path, newly inherited by these two ops.
  • The guard reads the top-level format only; a $ref or allOf-wrapped binary scalar would still take the typed path. The spec contains no such shape today (verified by full-spec walk); noted as a robustness follow-up rather than handled speculatively.

User impact

None until SDKs are regenerated; then Trajectories.contents and PrivateServiceDefinitions.get become usable.

Testing

  • npx vitest run in packages/sdk-generator: 407/407 pass.
  • Canonical proof: __tests__/backends/elixir.test.ts, "emits raw ops for binary-format json responses and typed ops for plain strings" — runs the real frontend-to-backend pipeline over both production shapes and the typed control, asserting the emitted Elixir source. Written red-first (verified failing against the pre-fix parser via git checkout origin/main -- src/frontend/operation-parser.ts). The frontend classification itself is pinned by __tests__/frontend/parse-spec.test.ts ("marks binary-format application/json responses as raw", also watched red first). This is a pure in-process transform with no process/network boundary to cross; the wire-level end-to-end proof lives in archastro-elixir PR 18's HTTP-level test and ultimately in the slice-4 strict typed e2e rerun at 0.3.4.

Follow-ups

  • archastro-elixir PR 18 (next in the 0.3.4 chain): decode_body: false for raw requests + the Codec.encode Code.ensure_loaded fix.
  • Chain: merge → release.yml package=sdk-generator patch → npm 0.11.2 → Elixir regen picks up both raw ops.
  • Robustness (not urgent): detect format: binary through $ref/allOf wrappers if such a shape ever enters the spec.

🤖 Generated with Claude Code

rob-archastro and others added 2 commits August 16, 2026 10:28
… raw

An application/json success response whose schema is type: string,
format: binary (a raw blob served with a JSON content type, e.g.
GET /trajectories/{trajectory}/contents) was typed as a plain string
op. HTTP clients auto-decode the JSON body before the SDK sees it, so
the typed string decode fails on every successful non-empty response.
Route such responses down the raw-response path like other binary
content.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… pipeline

Adversarial audit of the binary-raw fix found the classification change
also flips GET /private_service_definitions/{app_id}/{private_service_id}
(same binary-format application/json shape, same always-crashing typed
emission today) and that no test proved the generated output. Add a
parse-to-emission test covering both production shapes plus the typed
plain-string control.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rob-archastro

Copy link
Copy Markdown
Contributor Author

Review — round 1 (head 747ed84)

Verdict: no findings — meets bar. Verified independently:

  • The one-line narrowing in extractSuccessResponse routes application/json + format: binary into the pre-existing rawResponse: true branch (fall-through confirmed — the content-length check catches it). Schemas without format are untouched; $ref schemas have no format key and stay typed.
  • Ran the suite at head: 406/406 green.
  • Built the generator at head and at main, regenerated the Elixir SDK from both against the current spec, diffed: exactly two ops flip to raw: trueTrajectories.contents (the reported bug) and PrivateServiceDefinitions.get, whose platform action I verified is also returns(:raw, content_type: "application/json") (private_services/enrollment_definition.ex:18) — i.e. the fix silently repairs a second latent instance of the same always-crash class. Nothing else changes but content hashes.
  • The emitted return type flips to {:ok, Req.Response.t()}, consistent with the existing raw-op contract; the companion archastro-elixir decode_body: false change governs what arrives in .body.

Chain note: after merge this needs the npm publish (release.yml package=sdk-generator patch → 0.11.2) before the elixir regen.

Records why a JSON content type does not make a binary blob typed, and
that the check reads a top-level format only (no spec shape wraps
binary in a $ref or allOf today).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rob-archastro

Copy link
Copy Markdown
Contributor Author

Round 2 (head 18d38fe) — meets bar, no findings

The delta is responsive to round 1: the new backend test pins both production ops through the full parse→emit pipeline, including PrivateServiceDefinitions.get (the second latent instance I found in the regen diff), with a plain application/json {type: string} control asserting it stays decode: :string. That closes the gap where the fix's blast radius was only verified by my out-of-band regen rather than by a test in the repo.

The added comment on the check also documents the one real limitation honestly — only a top-level format is inspected, and no current spec shape wraps binary in a $ref or allOf. CI green on both Node legs.

Ready for merge. Chain: merge → release.yml package=sdk-generator bump=patch → npm 0.11.2 → then the archastro-elixir regen picks up both raw ops.

@rob-archastro
rob-archastro merged commit de24b27 into main Aug 16, 2026
3 checks passed
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.

1 participant