From 8b435f01d99642defb87023eb008d7db2dcb96b8 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Wed, 12 Aug 2026 22:43:21 +0800 Subject: [PATCH 1/9] fix: surface a clear error on non-JSON API responses instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a proxy, gateway or CDN returns an HTTP 200 with an HTML body (an error or interstitial page) instead of JSON, the generated SDK client JSON-parses it and throws a raw `JSON Parse error: Unrecognized token '<'`. `parseAs` falls back to "json" whenever Content-Type is missing or unrecognized, so a non-JSON body reaches the parser. The error path was already guarded; the success path was not. Guard the JSON parse in both the v1 and v2 generated clients: on a parse failure, throw an actionable error (non-JSON response, likely a proxy/gateway error page, with HTTP status + content-type) instead of the raw parse crash. In v1, "json" is split out of the shared fall-through group so the other parse modes (arrayBuffer/blob/formData/text) keep dispatching via response[parseAs](). Both hunks are wrapped in `altimate_change start — upstream_fix:` markers, the repo convention for local deviations that should survive upstream bridge merges and eventually land upstream. Surfaced from telemetry as a recurring extension sendMessageError. --- packages/sdk/js/src/gen/client/client.gen.ts | 20 ++++++++++++++++++- .../sdk/js/src/v2/gen/client/client.gen.ts | 13 +++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index 34a8d0bece..1d805ca6ca 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -114,10 +114,28 @@ export const createClient = (config: Config = {}): Client => { case "arrayBuffer": case "blob": case "formData": - case "json": case "text": data = await response[parseAs]() break + // altimate_change start — upstream_fix: guard JSON parse against non-JSON (HTML) response bodies + // "json" is split out of the fall-through group above so its parse can be guarded: a 200 + // whose body is an HTML error page from a proxy/gateway/CDN otherwise crashes with a raw + // "JSON Parse error: Unrecognized token '<'". The body is read OUTSIDE the guard so a + // network/body-read failure (socket reset, abort) keeps its own error; only an actual + // JSON syntax failure gets the actionable message (mirrors the v2 client). + case "json": { + const text = await response.text() + try { + data = text ? JSON.parse(text) : {} + } catch { + throw new Error( + `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + + `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, + ) + } + break + } + // altimate_change end case "stream": return opts.responseStyle === "data" ? response.body diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 627e98ec42..01eb095776 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -169,7 +169,18 @@ export const createClient = (config: Config = {}): Client => { // Some servers return 200 with no Content-Length and empty body. // response.json() would throw; read as text and parse if non-empty. const text = await response.text() - data = text ? JSON.parse(text) : {} + // altimate_change start — upstream_fix: guard JSON parse against non-JSON (HTML) response bodies + // A 200 whose body is an HTML error page from a proxy/gateway/CDN otherwise crashes with a + // raw "JSON Parse error: Unrecognized token '<'". Surface an actionable error instead. + try { + data = text ? JSON.parse(text) : {} + } catch { + throw new Error( + `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + + `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, + ) + } + // altimate_change end break } case "stream": From 095cff2719d8128d20d5340d25bf80e7a46025b1 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Thu, 13 Aug 2026 08:36:00 +0800 Subject: [PATCH 2/9] fix: preserve the original parse error as the Error cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review: the guard's actionable message discarded the underlying SyntaxError (token/position detail). Attach it via new Error(msg, { cause }) — the SDK's existing convention (error-interceptor.ts). --- packages/sdk/js/src/gen/client/client.gen.ts | 3 ++- packages/sdk/js/src/v2/gen/client/client.gen.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index 1d805ca6ca..c5b4760b74 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -127,10 +127,11 @@ export const createClient = (config: Config = {}): Client => { const text = await response.text() try { data = text ? JSON.parse(text) : {} - } catch { + } catch (cause) { throw new Error( `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, + { cause }, ) } break diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 01eb095776..67915f39ac 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -174,10 +174,11 @@ export const createClient = (config: Config = {}): Client => { // raw "JSON Parse error: Unrecognized token '<'". Surface an actionable error instead. try { data = text ? JSON.parse(text) : {} - } catch { + } catch (cause) { throw new Error( `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, + { cause }, ) } // altimate_change end From b9fe7e2747d08705483b1017e403dd4209f3fc05 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Fri, 21 Aug 2026 02:36:53 +0800 Subject: [PATCH 3/9] fix: ship the json guard through codegen; widen coverage; traceable errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the human review: - the v2 guard now ships: script/build.ts re-applies it after codegen (clean: true wipes src/v2/gen on every release build), using the SseFn-patch pattern — needle-match against raw codegen output (trailing semicolon included) with a loud failure if the template drifts. Verified by running the full build: the regenerated tree carries the guard. Drift canaries pin both halves. - the v2 html interceptor normalizes content-type before comparing, so 'text/html; charset=utf-8' — the form proxies actually send — is caught instead of returned as a string payload. - the error carries request identity (method + URL), names the content-type honestly, and keeps a 200-char body slice on cause for debugging; telemetry stays body-free. - live-server tests drive both failure shapes end to end: HTML mislabeled as application/json (guard) and honestly-labeled text/html with charset (interceptor). --- packages/opencode/test/sdk-json-guard.test.ts | 75 +++++++++++++++++++ packages/sdk/js/script/build.ts | 30 ++++++++ packages/sdk/js/src/gen/client/client.gen.ts | 7 +- packages/sdk/js/src/v2/client.ts | 5 +- .../sdk/js/src/v2/gen/client/client.gen.ts | 8 +- 5 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 packages/opencode/test/sdk-json-guard.test.ts diff --git a/packages/opencode/test/sdk-json-guard.test.ts b/packages/opencode/test/sdk-json-guard.test.ts new file mode 100644 index 0000000000..55e2927792 --- /dev/null +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, beforeAll, afterAll } from "bun:test" +import { createClient } from "../../sdk/js/src/v2/gen/client/client.gen" +import { createOpencodeClient } from "../../sdk/js/src/v2/client" + +// The JSON-parse guard lives in GENERATED code that `script/build.ts` wipes +// (clean: true) and re-applies on every release build. These tests pin both +// halves: the drift canaries fail if either copy of the patch disappears, and +// the live-server tests exercise the actual failure shapes (a proxy serving +// an HTML error page as application/json, and one labeling it honestly). + +describe("sdk json guard — drift canaries", () => { + const read = (p: string) => Bun.file(new URL(p, import.meta.url).pathname).text() + + it("both generated clients carry the guard", async () => { + for (const p of [ + "../../sdk/js/src/gen/client/client.gen.ts", + "../../sdk/js/src/v2/gen/client/client.gen.ts", + ]) { + const src = await read(p) + expect(src).toContain("guard JSON parse against non-JSON") + expect(src).toContain("but the body was not JSON") + } + }) + + it("build.ts re-applies the v2 guard after codegen with a matching needle", async () => { + const build = await read("../../sdk/js/script/build.ts") + expect(build).toContain("json-guard patch did not apply") + expect(build).toContain('const jsonGuardNeedle = " data = text ? JSON.parse(text) : {};"') + expect(build).toContain("but the body was not JSON") + }) +}) + +describe("sdk json guard — live failure shapes", () => { + let server: ReturnType + let base: string + const html = "502 Bad Gateway" + + beforeAll(() => { + server = Bun.serve({ + port: 0, + fetch(req) { + const path = new URL(req.url).pathname + if (path.endsWith("/lying-proxy")) + return new Response(html, { status: 200, headers: { "content-type": "application/json" } }) + // every other route: an honest proxy error page with charset + return new Response(html, { status: 200, headers: { "content-type": "text/html; charset=utf-8" } }) + }, + }) + base = `http://localhost:${server.port}` + }) + afterAll(() => server.stop(true)) + + it("HTML mislabeled as application/json rejects with an actionable error", async () => { + const client = createClient({ baseUrl: base }) + const err = await client + .get({ url: "/lying-proxy" }) + .then(() => null) + .catch((e: unknown) => e as Error & { cause?: { body?: string } }) + expect(err).not.toBeNull() + expect(err!.message).toContain("but the body was not JSON") + expect(err!.message).toContain("/lying-proxy") + expect(err!.message).toContain("content-type application/json") + expect(err!.cause?.body).toContain("502 Bad Gateway") + }) + + it("honestly-labeled text/html (with charset) rejects at the interceptor", async () => { + const oc = createOpencodeClient({ baseUrl: base }) + const err = await oc.app + .log({ service: "t", level: "info", message: "x" }) + .then(() => null) + .catch((e: unknown) => e as Error) + expect(err).not.toBeNull() + expect(String(err)).toContain("text/html") + }) +}) diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 72f4e3f3e9..f1ef437e40 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -58,6 +58,36 @@ if (sseTypesPatched === sseTypesSource) { } await Bun.write(sseTypesPath, sseTypesPatched) +// Re-apply the JSON-parse guard: `clean: true` above wipes src/v2/gen, so an +// edit inside client.gen.ts alone would be deleted on every release build +// (script/publish.ts runs this file in prepareReleaseFiles). A 200 whose body +// is an HTML error page from a proxy/gateway/CDN otherwise crashes with a raw +// "JSON Parse error: Unrecognized token '<'". +const jsonGuardPath = "./src/v2/gen/client/client.gen.ts" +const jsonGuardFile = Bun.file(jsonGuardPath) +const jsonGuardSource = await jsonGuardFile.text() +const jsonGuardNeedle = " data = text ? JSON.parse(text) : {};" +const jsonGuardBlock = [ + " // altimate_change start — upstream_fix: guard JSON parse against non-JSON (HTML) response bodies", + " // Re-applied by script/build.ts after codegen; edit it THERE, not here.", + " try {", + " data = text ? JSON.parse(text) : {}", + " } catch (cause) {", + " throw new Error(", + " \`Expected a JSON response from \${request.method} \${request.url} but the body was not JSON \` +", + " \`(HTTP \${response.status}, content-type \${response.headers.get(\"content-type\") ?? \"unset\"}). \` +", + " \`This is usually a proxy or gateway error page, not the API.\`,", + " { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } },", + " )", + " }", + " // altimate_change end", +].join("\n") +const jsonGuardPatched = jsonGuardSource.replace(jsonGuardNeedle, jsonGuardBlock) +if (jsonGuardPatched === jsonGuardSource) { + throw new Error(`json-guard patch did not apply; @hey-api/client-fetch output may have changed (${jsonGuardPath})`) +} +await Bun.write(jsonGuardPath, jsonGuardPatched) + await $`bun prettier --write src/gen` await $`bun prettier --write src/v2` await $`rm -rf dist` diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index c5b4760b74..1b8af7f24c 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -129,9 +129,10 @@ export const createClient = (config: Config = {}): Client => { data = text ? JSON.parse(text) : {} } catch (cause) { throw new Error( - `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + - `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, - { cause }, + `Expected a JSON response from ${request.method} ${request.url} but the body was not JSON ` + + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + + `This is usually a proxy or gateway error page, not the API.`, + { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, ) } break diff --git a/packages/sdk/js/src/v2/client.ts b/packages/sdk/js/src/v2/client.ts index c1956cffe0..798bd17453 100644 --- a/packages/sdk/js/src/v2/client.ts +++ b/packages/sdk/js/src/v2/client.ts @@ -83,8 +83,11 @@ export function createOpencodeClient(config?: Config & { directory?: string; exp ) client.interceptors.response.use((response) => { const contentType = response.headers.get("content-type") - if (contentType === "text/html") + // altimate_change start — upstream_fix: normalize before comparing; proxies and CDNs + // send "text/html; charset=utf-8", which exact equality silently let through + if (contentType?.split(";")[0]?.trim().toLowerCase() === "text/html") throw new Error("Request is not supported by this version of OpenCode Server (Server responded with text/html)") + // altimate_change end return response }) diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 67915f39ac..5436e3874c 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -172,13 +172,15 @@ export const createClient = (config: Config = {}): Client => { // altimate_change start — upstream_fix: guard JSON parse against non-JSON (HTML) response bodies // A 200 whose body is an HTML error page from a proxy/gateway/CDN otherwise crashes with a // raw "JSON Parse error: Unrecognized token '<'". Surface an actionable error instead. + // Re-applied by script/build.ts after codegen (clean: true wipes this tree); edit it THERE. try { data = text ? JSON.parse(text) : {} } catch (cause) { throw new Error( - `Expected a JSON response but received ${response.headers.get("content-type") || "an unknown content type"} ` + - `(HTTP ${response.status}). This is usually a proxy or gateway error page, not the API.`, - { cause }, + `Expected a JSON response from ${request.method} ${request.url} but the body was not JSON ` + + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + + `This is usually a proxy or gateway error page, not the API.`, + { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, ) } // altimate_change end From bcdbe473ca593db538aebd3f041cc29f48896b4b Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Tue, 25 Aug 2026 18:33:06 +0800 Subject: [PATCH 4/9] chore: mark the build.ts json-guard step as an altimate change The re-apply step lives in an upstream-shared file; the marker guard rightly flagged it unmarked. --- packages/sdk/js/script/build.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index f1ef437e40..015787fb1c 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -58,6 +58,7 @@ if (sseTypesPatched === sseTypesSource) { } await Bun.write(sseTypesPath, sseTypesPatched) +// altimate_change start — upstream_fix: re-apply the JSON-parse guard after codegen // Re-apply the JSON-parse guard: `clean: true` above wipes src/v2/gen, so an // edit inside client.gen.ts alone would be deleted on every release build // (script/publish.ts runs this file in prepareReleaseFiles). A 200 whose body @@ -87,6 +88,7 @@ if (jsonGuardPatched === jsonGuardSource) { throw new Error(`json-guard patch did not apply; @hey-api/client-fetch output may have changed (${jsonGuardPath})`) } await Bun.write(jsonGuardPath, jsonGuardPatched) +// altimate_change end await $`bun prettier --write src/gen` await $`bun prettier --write src/v2` From ec5d602e193b62e96dbe9e2672a743f3c41b9358 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Wed, 26 Aug 2026 13:04:37 +0800 Subject: [PATCH 5/9] fix: query-free request identity; reproducible guard block; v1 live coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the error names method + pathname, never request.url: the query string carries directory= on every GET (percent-encoded, so path masking could not have caught it) — all three copies - build.ts emits the same comment lines the committed block carries, so a release build round-trips client.gen.ts byte-for-byte (verified by running the build); CI asserts that for the patched file. main's types.gen.ts itself does not round-trip today — tracked separately - drift detection now checks the needle against the generator TEMPLATE on disk (resolved through the sdk package root), not build.ts's own literal; the literal pin stays as friction - v1 has runtime coverage: mislabeled-JSON error (query-free), parseAs text dispatch through the split switch, chunked-empty 200 -> {} - fileURLToPath instead of URL.pathname (Windows drive-letter paths) --- .github/workflows/ci.yml | 16 +++ packages/opencode/test/sdk-json-guard.test.ts | 103 ++++++++++++------ packages/sdk/js/script/build.ts | 6 +- packages/sdk/js/src/gen/client/client.gen.ts | 2 +- .../sdk/js/src/v2/gen/client/client.gen.ts | 2 +- 5 files changed, 91 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8826a5579e..d1951a8cb4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,6 +115,22 @@ jobs: # such hang) at --max-concurrency=2, which stays green even under heavy load (~43s locally at # load 21). Do not reintroduce OPENCODE_TEST_CLI for these tests without fixing that binary hang. + - name: SDK codegen is reproducible + # The v2 gen tree is committed AND regenerated on every release build + # (script/build.ts, clean: true) with post-codegen patches re-applied. + # A clean checkout must round-trip to itself, or a release ships + # something the review never saw. + working-directory: packages/sdk/js + run: | + # build.ts ends with tsc; main's committed types.gen.ts currently + # does not round-trip (tracked separately), so only the file the + # post-codegen patch targets is asserted here. openapi.json must + # exist afterwards: a codegen failure would leave the tree untouched + # and turn this into a false pass. + bun script/build.ts || echo "::warning::build.ts exited non-zero (see log); codegen+patch output is asserted below" + test -s openapi.json + git diff --exit-code -- src/v2/gen/client/client.gen.ts + - name: Run tests working-directory: packages/opencode # Cloud E2E tests (Snowflake, BigQuery, Databricks) auto-skip when diff --git a/packages/opencode/test/sdk-json-guard.test.ts b/packages/opencode/test/sdk-json-guard.test.ts index 55e2927792..3443bd86bb 100644 --- a/packages/opencode/test/sdk-json-guard.test.ts +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -1,33 +1,44 @@ import { describe, expect, it, beforeAll, afterAll } from "bun:test" -import { createClient } from "../../sdk/js/src/v2/gen/client/client.gen" +import path from "path" +import { fileURLToPath } from "url" +import { createClient as createV2Client } from "../../sdk/js/src/v2/gen/client/client.gen" +import { createClient as createV1Client } from "../../sdk/js/src/gen/client/client.gen" import { createOpencodeClient } from "../../sdk/js/src/v2/client" // The JSON-parse guard lives in GENERATED code that `script/build.ts` wipes -// (clean: true) and re-applies on every release build. These tests pin both -// halves: the drift canaries fail if either copy of the patch disappears, and -// the live-server tests exercise the actual failure shapes (a proxy serving -// an HTML error page as application/json, and one labeling it honestly). +// (clean: true) and re-applies on every release build. These tests pin the +// chain end to end: the needle still matches the generator TEMPLATE on disk +// (the real drift detector), both copies carry the guard, build.ts carries +// the re-apply step, and live-server tests drive the actual failure shapes +// against BOTH clients (v1 is the root `@opencode-ai/sdk` export plugins use). -describe("sdk json guard — drift canaries", () => { - const read = (p: string) => Bun.file(new URL(p, import.meta.url).pathname).text() +const sdk = fileURLToPath(new URL("../../sdk/js/", import.meta.url)) +const read = (rel: string) => Bun.file(path.join(sdk, rel)).text() - it("both generated clients carry the guard", async () => { - for (const p of [ - "../../sdk/js/src/gen/client/client.gen.ts", - "../../sdk/js/src/v2/gen/client/client.gen.ts", - ]) { - const src = await read(p) - expect(src).toContain("guard JSON parse against non-JSON") - expect(src).toContain("but the body was not JSON") - } +describe("sdk json guard — codegen drift", () => { + it("the needle still matches @hey-api/client-fetch's template on disk", async () => { + // build.ts throws mid-release if this stops matching; catching it here + // moves the failure to CI (resolved through the sdk package root: the + // generator exports only ".", "./internal" and "./package.json") + const root = path.dirname(require.resolve("@hey-api/openapi-ts/package.json", { paths: [sdk] })) + const tpl = await Bun.file(path.join(root, "dist/clients/fetch/client.ts")).text() + expect(tpl).toContain(" data = text ? JSON.parse(text) : {};") }) - it("build.ts re-applies the v2 guard after codegen with a matching needle", async () => { - const build = await read("../../sdk/js/script/build.ts") - expect(build).toContain("json-guard patch did not apply") + it("build.ts pins the needle literal and re-applies the guard", async () => { + const build = await read("script/build.ts") expect(build).toContain('const jsonGuardNeedle = " data = text ? JSON.parse(text) : {};"') + expect(build).toContain("json-guard patch did not apply") expect(build).toContain("but the body was not JSON") }) + + it("both generated clients carry the guard", async () => { + for (const rel of ["src/gen/client/client.gen.ts", "src/v2/gen/client/client.gen.ts"]) { + const src = await read(rel) + expect(src).toContain("guard JSON parse against non-JSON") + expect(src).toContain("but the body was not JSON") + } + }) }) describe("sdk json guard — live failure shapes", () => { @@ -39,10 +50,18 @@ describe("sdk json guard — live failure shapes", () => { server = Bun.serve({ port: 0, fetch(req) { - const path = new URL(req.url).pathname - if (path.endsWith("/lying-proxy")) + const p = new URL(req.url).pathname + if (p.endsWith("/lying-proxy")) return new Response(html, { status: 200, headers: { "content-type": "application/json" } }) - // every other route: an honest proxy error page with charset + if (p.endsWith("/plain-text")) + return new Response("just text", { status: 200, headers: { "content-type": "text/plain" } }) + if (p.endsWith("/empty-chunked")) { + // a 200 with an empty streamed body and no Content-Length reaches + // the json switch arm (the 204 / Content-Length:0 early return + // does not cover it) + const body = new ReadableStream({ start(c) { c.close() } }) + return new Response(body, { status: 200, headers: { "content-type": "application/json" } }) + } return new Response(html, { status: 200, headers: { "content-type": "text/html; charset=utf-8" } }) }, }) @@ -50,20 +69,36 @@ describe("sdk json guard — live failure shapes", () => { }) afterAll(() => server.stop(true)) - it("HTML mislabeled as application/json rejects with an actionable error", async () => { - const client = createClient({ baseUrl: base }) - const err = await client - .get({ url: "/lying-proxy" }) - .then(() => null) - .catch((e: unknown) => e as Error & { cause?: { body?: string } }) - expect(err).not.toBeNull() - expect(err!.message).toContain("but the body was not JSON") - expect(err!.message).toContain("/lying-proxy") - expect(err!.message).toContain("content-type application/json") - expect(err!.cause?.body).toContain("502 Bad Gateway") + for (const [name, make] of [["v2", createV2Client], ["v1", createV1Client]] as const) { + it(`${name}: HTML mislabeled as application/json rejects with a traceable, query-free error`, async () => { + const client = make({ baseUrl: base }) + const err = await client + .get({ url: "/lying-proxy", query: { directory: "/Users/jdoe/secret-project" } }) + .then(() => null) + .catch((e: unknown) => e as Error & { cause?: { body?: string } }) + expect(err).not.toBeNull() + expect(err!.message).toContain("but the body was not JSON") + expect(err!.message).toContain("GET /lying-proxy") + expect(err!.message).not.toContain("directory=") + expect(err!.message).not.toContain("secret-project") + expect(err!.message).toContain("content-type application/json") + expect(err!.cause?.body).toContain("502 Bad Gateway") + }) + } + + it("v1: parseAs text still dispatches through the split switch", async () => { + const client = createV1Client({ baseUrl: base }) + const res = await client.get({ url: "/plain-text", parseAs: "text" }) + expect(res.data).toBe("just text") + }) + + it("v1: a chunked empty 200 yields {} (declared alignment with v2; was SyntaxError)", async () => { + const client = createV1Client({ baseUrl: base }) + const res = await client.get({ url: "/empty-chunked" }) + expect(res.data).toEqual({}) }) - it("honestly-labeled text/html (with charset) rejects at the interceptor", async () => { + it("honestly-labeled text/html (with charset) rejects at the v2 interceptor", async () => { const oc = createOpencodeClient({ baseUrl: base }) const err = await oc.app .log({ service: "t", level: "info", message: "x" }) diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 015787fb1c..3cc81651c8 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -70,12 +70,14 @@ const jsonGuardSource = await jsonGuardFile.text() const jsonGuardNeedle = " data = text ? JSON.parse(text) : {};" const jsonGuardBlock = [ " // altimate_change start — upstream_fix: guard JSON parse against non-JSON (HTML) response bodies", - " // Re-applied by script/build.ts after codegen; edit it THERE, not here.", + " // A 200 whose body is an HTML error page from a proxy/gateway/CDN otherwise crashes with a", + " // raw \"JSON Parse error: Unrecognized token '<'\". Surface an actionable error instead.", + " // Re-applied by script/build.ts after codegen (clean: true wipes this tree); edit it THERE.", " try {", " data = text ? JSON.parse(text) : {}", " } catch (cause) {", " throw new Error(", - " \`Expected a JSON response from \${request.method} \${request.url} but the body was not JSON \` +", + " \`Expected a JSON response from \${request.method} \${new URL(request.url).pathname} but the body was not JSON \` +", " \`(HTTP \${response.status}, content-type \${response.headers.get(\"content-type\") ?? \"unset\"}). \` +", " \`This is usually a proxy or gateway error page, not the API.\`,", " { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } },", diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index 1b8af7f24c..d4ff366a5b 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -129,7 +129,7 @@ export const createClient = (config: Config = {}): Client => { data = text ? JSON.parse(text) : {} } catch (cause) { throw new Error( - `Expected a JSON response from ${request.method} ${request.url} but the body was not JSON ` + + `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + `This is usually a proxy or gateway error page, not the API.`, { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 5436e3874c..35da479ef2 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -177,7 +177,7 @@ export const createClient = (config: Config = {}): Client => { data = text ? JSON.parse(text) : {} } catch (cause) { throw new Error( - `Expected a JSON response from ${request.method} ${request.url} but the body was not JSON ` + + `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + `This is usually a proxy or gateway error page, not the API.`, { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, From 4b5335d34281fe86daad469e5075d724ac9accd3 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Wed, 26 Aug 2026 13:21:32 +0800 Subject: [PATCH 6/9] ci: restore the sdk gen tree after the reproducibility check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check ran build.ts, which regenerates the (drifted) gen tree on the runner, and the test step then ran against the mutated checkout — two SDK tests failed on the regenerated types. The check itself passed; it now restores src/v2/gen and removes openapi.json before exiting. --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1951a8cb4..22e6c8e70a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -127,9 +127,14 @@ jobs: # post-codegen patch targets is asserted here. openapi.json must # exist afterwards: a codegen failure would leave the tree untouched # and turn this into a false pass. + # The build mutates the checkout (regenerated gen tree, openapi.json, + # dist); restore it afterwards so the test steps below see the + # committed tree, whatever this check concludes. bun script/build.ts || echo "::warning::build.ts exited non-zero (see log); codegen+patch output is asserted below" test -s openapi.json - git diff --exit-code -- src/v2/gen/client/client.gen.ts + rc=0; git diff --exit-code -- src/v2/gen/client/client.gen.ts || rc=$? + git checkout -- src/v2/gen; rm -f openapi.json + exit $rc - name: Run tests working-directory: packages/opencode From e6d0732c78977ca483205409083dd200694c6ab8 Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Wed, 26 Aug 2026 21:01:28 +0800 Subject: [PATCH 7/9] fix(sdk): post-codegen patches assert one site; cause.body only for markup; honest codegen check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 3: - build.ts: `patchOnce` — String.replace patches the first match only, so both the SseFn and JSON-guard patches now fail the build on zero OR many sites; the template canary in the test asserts an exact count too - the response body rides on `cause` only when it looks like markup: util/error.ts serializes `cause` into logs, and a truncated or malformed real JSON response must not put its first 200 characters there (build.ts template + both generated clients; live test drives a sentinel secret through errorData for v1 and v2) - CI "SDK codegen is reproducible": cleanup is an unconditional EXIT trap covering src/gen, src/v2/gen (tracked and untracked), dist and openapi.json; the asserted file is removed before the build so an early codegen failure can no longer pass on the committed copy; a non-zero build is tolerated only when the file was regenerated AND round-trips — tsc on the regenerated tree fails until #1148, which the step records; drop the tolerance with it Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GayJFsfg2q2FoAG2XSVsZF --- .github/workflows/ci.yml | 44 +++++++++++++------ packages/opencode/test/sdk-json-guard.test.ts | 28 +++++++++++- packages/sdk/js/script/build.ts | 30 +++++++++---- packages/sdk/js/src/gen/client/client.gen.ts | 6 ++- .../sdk/js/src/v2/gen/client/client.gen.ts | 6 ++- 5 files changed, 87 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22e6c8e70a..3493ae8e81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,20 +122,36 @@ jobs: # something the review never saw. working-directory: packages/sdk/js run: | - # build.ts ends with tsc; main's committed types.gen.ts currently - # does not round-trip (tracked separately), so only the file the - # post-codegen patch targets is asserted here. openapi.json must - # exist afterwards: a codegen failure would leave the tree untouched - # and turn this into a false pass. - # The build mutates the checkout (regenerated gen tree, openapi.json, - # dist); restore it afterwards so the test steps below see the - # committed tree, whatever this check concludes. - bun script/build.ts || echo "::warning::build.ts exited non-zero (see log); codegen+patch output is asserted below" - test -s openapi.json - rc=0; git diff --exit-code -- src/v2/gen/client/client.gen.ts || rc=$? - git checkout -- src/v2/gen; rm -f openapi.json - exit $rc - + # The build mutates the checkout (both gen trees, openapi.json, dist); + # restore it whatever happens so later steps see the committed tree. + # An EXIT trap runs on `bash -e` aborts too. + cleanup() { + git checkout -- src/gen src/v2/gen 2>/dev/null + git clean -qfd src/gen src/v2/gen 2>/dev/null + rm -rf dist openapi.json + } + trap cleanup EXIT + # Prove codegen actually ran: a sentinel comment on the asserted file + # (a comment, because the generator itself imports this client) must + # be gone afterwards. A failure anywhere before the clean:true wipe + # leaves the committed copy — and the sentinel — in place, so it can + # no longer pass the drift check untouched. + sed -i '1i // CI-SENTINEL: not regenerated' src/v2/gen/client/client.gen.ts + build_rc=0; bun script/build.ts || build_rc=$? + if grep -q 'CI-SENTINEL: not regenerated' src/v2/gen/client/client.gen.ts; then + echo "::error::codegen did not regenerate src/v2/gen/client/client.gen.ts (build exit $build_rc)" + exit 1 + fi + drift_rc=0; git diff --exit-code -- src/v2/gen/client/client.gen.ts || drift_rc=$? + # build.ts ends with `bun tsc`, which fails on a clean main until the + # types.gen.ts round-trip is fixed (issue #1148). A non-zero build is + # tolerated ONLY when the file was regenerated AND round-trips — the + # two assertions above — i.e. the failure is downstream of + # codegen+patch. Remove this tolerance (exit on build_rc) with #1148. + if [ "$build_rc" -ne 0 ]; then + echo "::warning::build.ts exited $build_rc (tsc on the regenerated tree, #1148); codegen+patch asserted above" + fi + exit "$drift_rc" - name: Run tests working-directory: packages/opencode # Cloud E2E tests (Snowflake, BigQuery, Databricks) auto-skip when diff --git a/packages/opencode/test/sdk-json-guard.test.ts b/packages/opencode/test/sdk-json-guard.test.ts index 3443bd86bb..981a024932 100644 --- a/packages/opencode/test/sdk-json-guard.test.ts +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from "url" import { createClient as createV2Client } from "../../sdk/js/src/v2/gen/client/client.gen" import { createClient as createV1Client } from "../../sdk/js/src/gen/client/client.gen" import { createOpencodeClient } from "../../sdk/js/src/v2/client" +import { errorData } from "../src/util/error" // The JSON-parse guard lives in GENERATED code that `script/build.ts` wipes // (clean: true) and re-applies on every release build. These tests pin the @@ -22,13 +23,15 @@ describe("sdk json guard — codegen drift", () => { // generator exports only ".", "./internal" and "./package.json") const root = path.dirname(require.resolve("@hey-api/openapi-ts/package.json", { paths: [sdk] })) const tpl = await Bun.file(path.join(root, "dist/clients/fetch/client.ts")).text() - expect(tpl).toContain(" data = text ? JSON.parse(text) : {};") + // exactly one site: build.ts patches the first string match only, and + // asserts the count — a second site in a future template must fail here + expect(tpl.split(" data = text ? JSON.parse(text) : {};").length - 1).toBe(1) }) it("build.ts pins the needle literal and re-applies the guard", async () => { const build = await read("script/build.ts") expect(build).toContain('const jsonGuardNeedle = " data = text ? JSON.parse(text) : {};"') - expect(build).toContain("json-guard patch did not apply") + expect(build).toContain("post-codegen patch expects exactly one site") expect(build).toContain("but the body was not JSON") }) @@ -53,6 +56,11 @@ describe("sdk json guard — live failure shapes", () => { const p = new URL(req.url).pathname if (p.endsWith("/lying-proxy")) return new Response(html, { status: 200, headers: { "content-type": "application/json" } }) + if (p.endsWith("/truncated-json")) + return new Response('{"token":"SENTINEL_SECRET_VALUE","more":', { + status: 200, + headers: { "content-type": "application/json" }, + }) if (p.endsWith("/plain-text")) return new Response("just text", { status: 200, headers: { "content-type": "text/plain" } }) if (p.endsWith("/empty-chunked")) { @@ -83,6 +91,22 @@ describe("sdk json guard — live failure shapes", () => { expect(err!.message).not.toContain("secret-project") expect(err!.message).toContain("content-type application/json") expect(err!.cause?.body).toContain("502 Bad Gateway") + // the markup case keeps its diagnostic through the repo's error serializer + expect(JSON.stringify(errorData(err))).toContain("502 Bad Gateway") + }) + + it(`${name}: a malformed REAL JSON body never reaches serialized error data`, async () => { + const client = make({ baseUrl: base }) + const err = await client + .get({ url: "/truncated-json" }) + .then(() => null) + .catch((e: unknown) => e as Error & { cause?: { body?: string } }) + expect(err).not.toBeNull() + expect(err!.message).toContain("but the body was not JSON") + expect(err!.cause?.body).toBeUndefined() + // util/error.ts serializes `cause` into structured logs and stderr + expect(JSON.stringify(errorData(err))).not.toContain("SENTINEL_SECRET_VALUE") + expect(JSON.stringify(err!.cause)).not.toContain("SENTINEL_SECRET_VALUE") }) } diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 3cc81651c8..982669a27e 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -49,13 +49,24 @@ await createClient({ const sseTypesPath = "./src/v2/gen/client/types.gen.ts" const sseTypesFile = Bun.file(sseTypesPath) const sseTypesSource = await sseTypesFile.text() -const sseTypesPatched = sseTypesSource.replace( +// altimate_change start — upstream_fix: post-codegen patches must apply exactly once +// String.prototype.replace with a string needle patches the FIRST match only: +// a template that grows a second site would leave one arm unpatched with a +// green build, and zero matches is a silent no-op. Assert exactly one. +const patchOnce = (source: string, needle: string, replacement: string, where: string) => { + const matches = source.split(needle).length - 1 + if (matches !== 1) { + throw new Error(`post-codegen patch expects exactly one site, found ${matches} in ${where}: ${needle.trim()}`) + } + return source.replace(needle, replacement) +} +const sseTypesPatched = patchOnce( + sseTypesSource, "=> Promise>", "=> Promise>", + sseTypesPath, ) -if (sseTypesPatched === sseTypesSource) { - throw new Error(`SseFn patch did not apply; @hey-api/openapi-ts output may have changed (${sseTypesPath})`) -} +// altimate_change end await Bun.write(sseTypesPath, sseTypesPatched) // altimate_change start — upstream_fix: re-apply the JSON-parse guard after codegen @@ -76,19 +87,20 @@ const jsonGuardBlock = [ " try {", " data = text ? JSON.parse(text) : {}", " } catch (cause) {", + " // The body rides on `cause` only when it looks like markup (the proxy/gateway page this", + " // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or", + " // malformed REAL JSON response must not put its first 200 characters there.", + " const body = text.trimStart().startsWith(\"<\") ? text.slice(0, 200) : undefined", " throw new Error(", " \`Expected a JSON response from \${request.method} \${new URL(request.url).pathname} but the body was not JSON \` +", " \`(HTTP \${response.status}, content-type \${response.headers.get(\"content-type\") ?? \"unset\"}). \` +", " \`This is usually a proxy or gateway error page, not the API.\`,", - " { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } },", + " { cause: { parseError: cause, status: response.status, body } },", " )", " }", " // altimate_change end", ].join("\n") -const jsonGuardPatched = jsonGuardSource.replace(jsonGuardNeedle, jsonGuardBlock) -if (jsonGuardPatched === jsonGuardSource) { - throw new Error(`json-guard patch did not apply; @hey-api/client-fetch output may have changed (${jsonGuardPath})`) -} +const jsonGuardPatched = patchOnce(jsonGuardSource, jsonGuardNeedle, jsonGuardBlock, jsonGuardPath) await Bun.write(jsonGuardPath, jsonGuardPatched) // altimate_change end diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index d4ff366a5b..f723901976 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -128,11 +128,15 @@ export const createClient = (config: Config = {}): Client => { try { data = text ? JSON.parse(text) : {} } catch (cause) { + // The body rides on `cause` only when it looks like markup (the proxy/gateway page this + // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or + // malformed REAL JSON response must not put its first 200 characters there. + const body = text.trimStart().startsWith("<") ? text.slice(0, 200) : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + `This is usually a proxy or gateway error page, not the API.`, - { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, + { cause: { parseError: cause, status: response.status, body } }, ) } break diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 35da479ef2..02c036dd0e 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -176,11 +176,15 @@ export const createClient = (config: Config = {}): Client => { try { data = text ? JSON.parse(text) : {} } catch (cause) { + // The body rides on `cause` only when it looks like markup (the proxy/gateway page this + // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or + // malformed REAL JSON response must not put its first 200 characters there. + const body = text.trimStart().startsWith("<") ? text.slice(0, 200) : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + `This is usually a proxy or gateway error page, not the API.`, - { cause: { parseError: cause, status: response.status, body: text.slice(0, 200) } }, + { cause: { parseError: cause, status: response.status, body } }, ) } // altimate_change end From 21f6529e5d5aa1c886ef22859ae647406e26005e Mon Sep 17 00:00:00 2001 From: ralphstodomingo Date: Wed, 26 Aug 2026 21:17:42 +0800 Subject: [PATCH 8/9] fix(sdk): cause.body is the page title only; CI whitelists the tsc failure by signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a proxy/gateway page can echo the request URL, query included — only its rides on cause now (all three copies); live test with an echoing Express-style page for v1 and v2 - the codegen CI step tolerates a non-zero build only when the log carries the "tsc" exited with code signature (#1148) — any earlier failure exits with the build's status; the drift check is also cleaned of build.log Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GayJFsfg2q2FoAG2XSVsZF --- .github/workflows/ci.yml | 21 ++++++++++++------- packages/opencode/test/sdk-json-guard.test.ts | 21 ++++++++++++++++++- packages/sdk/js/script/build.ts | 8 +++---- packages/sdk/js/src/gen/client/client.gen.ts | 8 +++---- .../sdk/js/src/v2/gen/client/client.gen.ts | 8 +++---- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3493ae8e81..e0c636b740 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -128,7 +128,7 @@ jobs: cleanup() { git checkout -- src/gen src/v2/gen 2>/dev/null git clean -qfd src/gen src/v2/gen 2>/dev/null - rm -rf dist openapi.json + rm -rf dist openapi.json build.log } trap cleanup EXIT # Prove codegen actually ran: a sentinel comment on the asserted file @@ -137,19 +137,26 @@ jobs: # leaves the committed copy — and the sentinel — in place, so it can # no longer pass the drift check untouched. sed -i '1i // CI-SENTINEL: not regenerated' src/v2/gen/client/client.gen.ts - build_rc=0; bun script/build.ts || build_rc=$? + build_rc=0; bun script/build.ts > build.log 2>&1 || build_rc=$? + cat build.log if grep -q 'CI-SENTINEL: not regenerated' src/v2/gen/client/client.gen.ts; then echo "::error::codegen did not regenerate src/v2/gen/client/client.gen.ts (build exit $build_rc)" exit 1 fi drift_rc=0; git diff --exit-code -- src/v2/gen/client/client.gen.ts || drift_rc=$? # build.ts ends with `bun tsc`, which fails on a clean main until the - # types.gen.ts round-trip is fixed (issue #1148). A non-zero build is - # tolerated ONLY when the file was regenerated AND round-trips — the - # two assertions above — i.e. the failure is downstream of - # codegen+patch. Remove this tolerance (exit on build_rc) with #1148. + # types.gen.ts round-trip is fixed (issue #1148). That ONE failure is + # whitelisted by its signature in the log, and only when the file was + # regenerated AND round-trips (the two assertions above); any other + # non-zero build — codegen, a patch, prettier — fails the step. + # Remove the whitelist (exit on build_rc) with #1148. if [ "$build_rc" -ne 0 ]; then - echo "::warning::build.ts exited $build_rc (tsc on the regenerated tree, #1148); codegen+patch asserted above" + if grep -q '"tsc" exited with code' build.log; then + echo "::warning::build.ts exited $build_rc at tsc on the regenerated tree (#1148); codegen+patch asserted above" + else + echo "::error::build.ts exited $build_rc before tsc — not the whitelisted #1148 failure" + exit "$build_rc" + fi fi exit "$drift_rc" - name: Run tests diff --git a/packages/opencode/test/sdk-json-guard.test.ts b/packages/opencode/test/sdk-json-guard.test.ts index 981a024932..8ef406f650 100644 --- a/packages/opencode/test/sdk-json-guard.test.ts +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -47,7 +47,7 @@ describe("sdk json guard — codegen drift", () => { describe("sdk json guard — live failure shapes", () => { let server: ReturnType<typeof Bun.serve> let base: string - const html = "<!DOCTYPE html><html><body>502 Bad Gateway</body></html>" + const html = "<!DOCTYPE html><html><head><title>502 Bad Gateway502 Bad Gateway" beforeAll(() => { server = Bun.serve({ @@ -56,6 +56,12 @@ describe("sdk json guard — live failure shapes", () => { const p = new URL(req.url).pathname if (p.endsWith("/lying-proxy")) return new Response(html, { status: 200, headers: { "content-type": "application/json" } }) + if (p.endsWith("/echo-page")) + // an Express-style page echoes the request target, query included + return new Response(`
Cannot GET ${new URL(req.url).pathname}${new URL(req.url).search}
`, { + status: 200, + headers: { "content-type": "application/json" }, + }) if (p.endsWith("/truncated-json")) return new Response('{"token":"SENTINEL_SECRET_VALUE","more":', { status: 200, @@ -95,6 +101,19 @@ describe("sdk json guard — live failure shapes", () => { expect(JSON.stringify(errorData(err))).toContain("502 Bad Gateway") }) + it(`${name}: a page that echoes the request URL contributes nothing but its title`, async () => { + const client = make({ baseUrl: base }) + const err = await client + .get({ url: "/echo-page", query: { directory: "/Users/jdoe/secret-project" } }) + .then(() => null) + .catch((e: unknown) => e as Error & { cause?: { body?: string } }) + expect(err).not.toBeNull() + expect(err!.message).toContain("but the body was not JSON") + expect(err!.cause?.body).toBeUndefined() + expect(JSON.stringify(errorData(err))).not.toContain("secret-project") + expect(JSON.stringify(errorData(err))).not.toContain("directory=") + }) + it(`${name}: a malformed REAL JSON body never reaches serialized error data`, async () => { const client = make({ baseUrl: base }) const err = await client diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 982669a27e..1bce5a7997 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -87,10 +87,10 @@ const jsonGuardBlock = [ " try {", " data = text ? JSON.parse(text) : {}", " } catch (cause) {", - " // The body rides on `cause` only when it looks like markup (the proxy/gateway page this", - " // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or", - " // malformed REAL JSON response must not put its first 200 characters there.", - " const body = text.trimStart().startsWith(\"<\") ? text.slice(0, 200) : undefined", + " // Only the page rides on `cause` (\"502 Bad Gateway\", \"Access Denied\", \"Sign in\" — the", + " // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs,", + " // and a page body can echo the request URL (query included) or be a malformed real response.", + " const body = text.trimStart().startsWith(\"<\") ? /<title>([^<]{1,200})<\\/title>/i.exec(text)?.[1] : undefined", " throw new Error(", " \`Expected a JSON response from \${request.method} \${new URL(request.url).pathname} but the body was not JSON \` +", " \`(HTTP \${response.status}, content-type \${response.headers.get(\"content-type\") ?? \"unset\"}). \` +", diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index f723901976..035225c3f6 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -128,10 +128,10 @@ export const createClient = (config: Config = {}): Client => { try { data = text ? JSON.parse(text) : {} } catch (cause) { - // The body rides on `cause` only when it looks like markup (the proxy/gateway page this - // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or - // malformed REAL JSON response must not put its first 200 characters there. - const body = text.trimStart().startsWith("<") ? text.slice(0, 200) : undefined + // Only the page <title> rides on `cause` ("502 Bad Gateway", "Access Denied", "Sign in" — the + // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs, + // and a page body can echo the request URL (query included) or be a malformed real response. + const body = text.trimStart().startsWith("<") ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 02c036dd0e..3ba1239508 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -176,10 +176,10 @@ export const createClient = (config: Config = {}): Client => { try { data = text ? JSON.parse(text) : {} } catch (cause) { - // The body rides on `cause` only when it looks like markup (the proxy/gateway page this - // guard exists for): util/error.ts serializes `cause` into logs, and a truncated or - // malformed REAL JSON response must not put its first 200 characters there. - const body = text.trimStart().startsWith("<") ? text.slice(0, 200) : undefined + // Only the page <title> rides on `cause` ("502 Bad Gateway", "Access Denied", "Sign in" — the + // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs, + // and a page body can echo the request URL (query included) or be a malformed real response. + const body = text.trimStart().startsWith("<") ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + From bcb8d151376e5866abc623acdf5925930b700fef Mon Sep 17 00:00:00 2001 From: ralphstodomingo <ralph@altimate.ai> Date: Wed, 26 Aug 2026 22:42:30 +0800 Subject: [PATCH 9/9] fix(sdk): drop a page title that echoes the request target from cause.body A proxy/CDN page can render the request path and query inside its <title>; a title carrying any of / ? = % is dropped (a gateway title never has them). Live fixtures with raw and percent-encoded echoed titles for v1 and v2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GayJFsfg2q2FoAG2XSVsZF --- packages/opencode/test/sdk-json-guard.test.ts | 25 +++++++++++++++++++ packages/sdk/js/script/build.ts | 4 ++- packages/sdk/js/src/gen/client/client.gen.ts | 6 ++++- .../sdk/js/src/v2/gen/client/client.gen.ts | 6 ++++- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/opencode/test/sdk-json-guard.test.ts b/packages/opencode/test/sdk-json-guard.test.ts index 8ef406f650..f7df04bb7f 100644 --- a/packages/opencode/test/sdk-json-guard.test.ts +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -62,6 +62,17 @@ describe("sdk json guard — live failure shapes", () => { status: 200, headers: { "content-type": "application/json" }, }) + if (p.endsWith("/echo-title")) + // a CDN/proxy page that renders the request target in its <title> + return new Response(`<!DOCTYPE html><html><head><title>Page not found at ${new URL(req.url).pathname}${new URL(req.url).search}404`, { + status: 200, + headers: { "content-type": "application/json" }, + }) + if (p.endsWith("/echo-title-encoded")) + return new Response(`Not found: ${encodeURIComponent(new URL(req.url).pathname + new URL(req.url).search)}404`, { + status: 200, + headers: { "content-type": "application/json" }, + }) if (p.endsWith("/truncated-json")) return new Response('{"token":"SENTINEL_SECRET_VALUE","more":', { status: 200, @@ -114,6 +125,20 @@ describe("sdk json guard — live failure shapes", () => { expect(JSON.stringify(errorData(err))).not.toContain("directory=") }) + it(`${name}: a that echoes the request target is dropped, raw or percent-encoded`, async () => { + const client = make({ baseUrl: base }) + for (const url of ["/echo-title", "/echo-title-encoded"]) { + const err = await client + .get({ url, query: { directory: "/Users/jdoe/secret-project" } }) + .then(() => null) + .catch((e: unknown) => e as Error & { cause?: { body?: string } }) + expect(err).not.toBeNull() + expect(err!.cause?.body).toBeUndefined() + expect(JSON.stringify(errorData(err))).not.toContain("secret-project") + expect(JSON.stringify(errorData(err))).not.toContain("directory") + } + }) + it(`${name}: a malformed REAL JSON body never reaches serialized error data`, async () => { const client = make({ baseUrl: base }) const err = await client diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 1bce5a7997..5ee63a4998 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -90,7 +90,9 @@ const jsonGuardBlock = [ " // Only the page <title> rides on `cause` (\"502 Bad Gateway\", \"Access Denied\", \"Sign in\" — the", " // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs,", " // and a page body can echo the request URL (query included) or be a malformed real response.", - " const body = text.trimStart().startsWith(\"<\") ? /<title>([^<]{1,200})<\\/title>/i.exec(text)?.[1] : undefined", + " // A title that echoes the request target (any `/ ? = %`) is dropped too.", + " const title = text.trimStart().startsWith(\"<\") ? /<title>([^<]{1,200})<\\/title>/i.exec(text)?.[1] : undefined", + " const body = title && !/[\\/?=%]/.test(title) ? title : undefined", " throw new Error(", " \`Expected a JSON response from \${request.method} \${new URL(request.url).pathname} but the body was not JSON \` +", " \`(HTTP \${response.status}, content-type \${response.headers.get(\"content-type\") ?? \"unset\"}). \` +", diff --git a/packages/sdk/js/src/gen/client/client.gen.ts b/packages/sdk/js/src/gen/client/client.gen.ts index 035225c3f6..96d37d7a90 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -131,7 +131,11 @@ export const createClient = (config: Config = {}): Client => { // Only the page <title> rides on `cause` ("502 Bad Gateway", "Access Denied", "Sign in" — the // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs, // and a page body can echo the request URL (query included) or be a malformed real response. - const body = text.trimStart().startsWith("<") ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] : undefined + // A title that echoes the request target (any `/ ? = %`) is dropped too. + const title = text.trimStart().startsWith("<") + ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] + : undefined + const body = title && !/[\/?=%]/.test(title) ? title : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` + diff --git a/packages/sdk/js/src/v2/gen/client/client.gen.ts b/packages/sdk/js/src/v2/gen/client/client.gen.ts index 3ba1239508..4db98d6df9 100644 --- a/packages/sdk/js/src/v2/gen/client/client.gen.ts +++ b/packages/sdk/js/src/v2/gen/client/client.gen.ts @@ -179,7 +179,11 @@ export const createClient = (config: Config = {}): Client => { // Only the page <title> rides on `cause` ("502 Bad Gateway", "Access Denied", "Sign in" — the // diagnostic part of a proxy/gateway/CDN page): util/error.ts serializes `cause` into logs, // and a page body can echo the request URL (query included) or be a malformed real response. - const body = text.trimStart().startsWith("<") ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] : undefined + // A title that echoes the request target (any `/ ? = %`) is dropped too. + const title = text.trimStart().startsWith("<") + ? /<title>([^<]{1,200})<\/title>/i.exec(text)?.[1] + : undefined + const body = title && !/[\/?=%]/.test(title) ? title : undefined throw new Error( `Expected a JSON response from ${request.method} ${new URL(request.url).pathname} but the body was not JSON ` + `(HTTP ${response.status}, content-type ${response.headers.get("content-type") ?? "unset"}). ` +