diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8826a5579e..e0c636b740 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,6 +115,50 @@ 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: | + # 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 build.log + } + 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.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). 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 + 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 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 new file mode 100644 index 0000000000..f7df04bb7f --- /dev/null +++ b/packages/opencode/test/sdk-json-guard.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it, beforeAll, afterAll } from "bun:test" +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" +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 +// 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). + +const sdk = fileURLToPath(new URL("../../sdk/js/", import.meta.url)) +const read = (rel: string) => Bun.file(path.join(sdk, rel)).text() + +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() + // 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("post-codegen patch expects exactly one site") + 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", () => { + let server: ReturnType + let base: string + const html = "502 Bad Gateway502 Bad Gateway" + + beforeAll(() => { + server = Bun.serve({ + port: 0, + fetch(req) { + 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("/echo-title")) + // a CDN/proxy page that renders the request target in its + 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, + 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")) { + // 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" } }) + }, + }) + base = `http://localhost:${server.port}` + }) + afterAll(() => server.stop(true)) + + 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") + // the markup case keeps its diagnostic through the repo's error serializer + 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 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 + .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") + }) + } + + 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 v2 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..5ee63a4998 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -49,15 +49,63 @@ 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<ServerSentEventsResult<TData, TError>>", "=> Promise<ServerSentEventsResult<TData>>", + 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 +// 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", + " // 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) {", + " // 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.", + " // 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\"}). \` +", + " \`This is usually a proxy or gateway error page, not the API.\`,", + " { cause: { parseError: cause, status: response.status, body } },", + " )", + " }", + " // altimate_change end", +].join("\n") +const jsonGuardPatched = patchOnce(jsonGuardSource, jsonGuardNeedle, jsonGuardBlock, jsonGuardPath) +await Bun.write(jsonGuardPath, jsonGuardPatched) +// altimate_change end + 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 34a8d0bece..96d37d7a90 100644 --- a/packages/sdk/js/src/gen/client/client.gen.ts +++ b/packages/sdk/js/src/gen/client/client.gen.ts @@ -114,10 +114,38 @@ 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 (cause) { + // 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. + // 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"}). ` + + `This is usually a proxy or gateway error page, not the API.`, + { cause: { parseError: cause, status: response.status, body } }, + ) + } + break + } + // altimate_change end case "stream": return opts.responseStyle === "data" ? response.body 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 627e98ec42..4db98d6df9 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,29 @@ 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. + // Re-applied by script/build.ts after codegen (clean: true wipes this tree); edit it THERE. + try { + data = text ? JSON.parse(text) : {} + } catch (cause) { + // 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. + // 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"}). ` + + `This is usually a proxy or gateway error page, not the API.`, + { cause: { parseError: cause, status: response.status, body } }, + ) + } + // altimate_change end break } case "stream":