Skip to content

fix: surface a clear error on non-JSON API responses instead of crashing #1093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ralphstodomingo wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: surface a clear error on non-JSON API responses instead of crashing #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b090fd4
fix: surface a clear error on non-JSON API responses instead of crashing
Aug 12, 2026
15a452a
fix: preserve the original parse error as the Error cause
Aug 13, 2026
fcfb24b
fix: ship the json guard through codegen; widen coverage; traceable e…
Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions packages/opencode/test/sdk-json-guard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Comment on lines +2 to +3

@sahrizvi sahrizvi Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both live tests import v2, so v1 has no runtime coverage — only a string grep.

v1's edit is the structurally riskier one: it pulled case "json" out of a four-way fall-through group, changing switch control flow. It is also the root @opencode-ai/sdk export that external plugin authors consume (packages/plugin/src/index.ts:12), and packages/opencode/test/server/sdk-v1-smoke.test.ts shows the harness already exists.

Three cases would close it, all against v1's createClient:

  1. HTML body mislabeled application/json → the actionable error (the twin of the /lying-proxy case below).
  2. parseAs: "text" → the exact string back, proving arrayBuffer/blob/formData/text still dispatch through response[parseAs]() after the switch split.
  3. 200, chunked, empty body → {}. This pins the declared behavior change (previously SyntaxError); right now that change is documented but nothing stops a future refactor from silently undoing it.


// 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()

@cubic-dev-ai cubic-dev-ai Bot Aug 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The read helper passes new URL(p, import.meta.url).pathname to Bun.file. URL.pathname is percent-encoded and, on Windows, is prefixed with a drive letter (e.g. /C:/...), so Bun.file fails to resolve the file there and the drift-canary tests spuriously fail. Bun.file accepts a URL directly, so drop the .pathname (or use fileURLToPath, as script/build.ts does).

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/test/sdk-json-guard.test.ts, line 12:

<comment>The `read` helper passes `new URL(p, import.meta.url).pathname` to `Bun.file`. `URL.pathname` is percent-encoded and, on Windows, is prefixed with a drive letter (e.g. `/C:/...`), so `Bun.file` fails to resolve the file there and the drift-canary tests spuriously fail. `Bun.file` accepts a `URL` directly, so drop the `.pathname` (or use `fileURLToPath`, as `script/build.ts` does).</comment>

<file context>
@@ -0,0 +1,75 @@
+// 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 () => {
</file context>
Suggested change
const read = (p: string) => Bun.file(new URL(p, import.meta.url).pathname).text()
const read = (p: string) => Bun.file(new URL(p, import.meta.url)).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")
})
Comment on lines +25 to +30

@sahrizvi sahrizvi Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This canary cannot detect the drift it is named for.

expect(build).toContain('const jsonGuardNeedle = "…"') asserts that build.ts contains its own source literal. It passes for exactly as long as nobody edits that line — which is not the failure mode. The named failure is the needle ceasing to match generator output, and if @hey-api/openapi-ts changed its template tomorrow this test would stay green while build.ts:86-88 threw. The only detector today is that runtime throw, which fires inside publish.ts → prepareReleaseFiles() — mid-release.

That matters because it is load-bearing for the claim that the canaries "pin both halves". They pin that the strings are present; they do not pin that the patch still applies.

Two changes:

  1. Rename this to what it does — it pins the needle literal. Useful friction (nobody changes the needle without consciously updating the test), just not drift detection.
  2. Add the real detector. The template is on disk, so it costs nothing — but resolve it through the package root, since @hey-api/openapi-ts exports only ., ./internal and ./package.json, and it is a dependency of packages/sdk/js, not packages/opencode:
const sdk = fileURLToPath(new URL("../../sdk/js/", import.meta.url))
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) : {};")

That fails in CI on the dependency-bump PR, which is where you want it.

Separately, in build.ts: String.prototype.replace silently takes the first match. Assert the insertion point is unique, so a template that grows a second occurrence fails loudly rather than patching the wrong one:

const matches = jsonGuardSource.split(jsonGuardNeedle).length - 1
if (matches !== 1) throw new Error(`expected one JSON guard insertion point, found ${matches}`)

})

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>"

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")
})
})
30 changes: 30 additions & 0 deletions packages/sdk/js/script/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
22 changes: 21 additions & 1 deletion packages/sdk/js/src/gen/client/client.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,30 @@ 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) : {}

@sahrizvi sahrizvi Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undeclared v1 behavior change: an empty body used to throw, now returns {}.

v1's case "json" was await response.json(), which throws SyntaxError: Unexpected end of JSON input on an empty body. text ? JSON.parse(text) : {} returns {} instead.

The early return above only covers status === 204 and Content-Length === "0" (client.gen.ts:100-107), so a chunked 200 with an empty body and no Content-Length reaches this switch and now silently yields {}.

Aligning v1 with v2 is probably the right call, but it's outside the stated scope and the PR body's matrix says v1 already returned {} before — it didn't. One knock-on: responseValidator (client.gen.ts:150) now runs against {} for empty bodies where it was previously never reached.

Either call it out in the description or split it into its own commit.

@ralphstodomingo ralphstodomingo Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declared in fcfb24b9f's description update — you're right that the PR body's matrix claimed v1 already returned {} on empty bodies; it threw. The alignment with v2 is intentional and now stated explicitly, including the responseValidator knock-on (it now runs against {} for chunked-empty 200s where it was previously unreachable).

} 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) } },
)
}
break
}
// altimate_change end
case "stream":
return opts.responseStyle === "data"
? response.body
Expand Down
5 changes: 4 additions & 1 deletion packages/sdk/js/src/v2/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
16 changes: 15 additions & 1 deletion packages/sdk/js/src/v2/gen/client/client.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,21 @@ 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.
Comment on lines +172 to +175

@sahrizvi sahrizvi Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This committed block is not what the build produces, so build.ts is not actually authoritative for this region.

Running script/build.ts against a clean checkout and diffing the result against this file gives exactly one hunk:

           // 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.
+          // Re-applied by script/build.ts after codegen; edit it THERE, not here.

Everything else in the regenerated tree matches, and the executable code is byte-identical — so there is no runtime impact. But:

  • every release build leaves a spurious modification to a committed file, which reads as accidental drift and gets committed or reverted inconsistently;
  • the two lines explaining why the guard exists are dropped from the code that actually ships;
  • it blocks the check that would close the drift-detection gap for good.

Make jsonGuardBlock emit the same four comment lines this file carries. Then this becomes possible, and it validates the entire chain — needle match, identifier scope, prettier, tsc — without duplicating any of it:

- name: SDK codegen is reproducible
  working-directory: packages/sdk/js
  run: bun run script/build.ts && git diff --exit-code src/v2/gen

try {
data = text ? JSON.parse(text) : {}

@sahrizvi sahrizvi Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard only fires when parseAs resolves to "json", which leaves the common proxy shape untouched.

Both clients default to parseAs: "auto", so getParseAs() picks the arm (utils.gen.ts:61-90, v1 twin at :59-88):

Response content-type resolves to with an HTML body, after this PR
application/json (proxy lies) json ✅ fixed
unrecognized, e.g. foo/bar (?? "json") json ✅ fixed
text/html / text/html; charset=utf-8 text ❌ HTML returned as a string in data, no error
absent stream ❌ response.body returned as data, no error
application/octet-stream blob ❌ Blob returned as data, no error

Driven against a local server, both clients resolve rather than reject for text/html, text/html; charset=utf-8, and no Content-Type, under both throwOnError settings.

So this covers only the mislabeled-as-JSON case. A gateway that labels its error page honestly — most of them — still returns a "successful" result whose data is an HTML string, and fails further downstream with a worse message than the one this replaces.

Also, the description says parseAs falls back to "json" when Content-Type is missing. It doesn't — a missing Content-Type resolves to "stream" (utils.gen.ts:62-66).

The cheapest way to close most of this is one line in code this PR doesn't touch. packages/sdk/js/src/v2/client.ts:84-89 already guards this exact failure:

if (contentType === "text/html")
  throw new Error("Request is not supported by this version of OpenCode Server (Server responded with text/html)")

Exact equality misses text/html; charset=utf-8 — the form proxies and CDNs actually send. Normalizing it covers strictly more cases than this hunk does:

if (contentType?.split(";")[0]?.trim().toLowerCase() === "text/html")

(v1 has no such interceptor at all, so v1 has neither layer.) Pre-existing and outside the diff, raised only because it's load-bearing for the gap above and is a one-liner.

@ralphstodomingo ralphstodomingo Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fcfb24b9f — your one-liner is in: the v2 interceptor normalizes (split(";")[0].trim().toLowerCase()), so text/html; charset=utf-8 is caught. Verified with a live local server (bun test spins one up): honestly-labeled HTML with charset now rejects at the interceptor, mislabeled-as-json rejects at the guard. You're right about the description's parseAs claim — corrected (absent content-type resolves to stream, not json); the stream/blob columns of your table remain uncovered by this PR and the description now says so explicitly.

} catch (cause) {
throw new Error(
`Expected a JSON response from ${request.method} ${request.url} but the body was not JSON ` +

@sahrizvi sahrizvi Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request.url includes the query string, which carries the user's absolute working directory.

v2/client.ts:34-37 (and the v1 twin at client.ts:19-27) set directory=<encodeURIComponent(absolute path)> as a query parameter on every GET/HEAD. So this message can render as:

Expected a JSON response from GET http://host/api/session?directory=%2FUsers%2Fjane%2Fwork%2Fclient-repo but the body was not JSON …

For the default http://localhost:4096 base URL the internal-host rule at packages/opencode/src/altimate/telemetry/index.ts:1405 redacts the whole URL, query included, so the common path is safe today. For a non-internal base URL that rule does not match and the path survives; percent-encoding also defeats path-shaped masking, so a future path-masking rule would not catch it either.

Request identity was the goal, and method + path supplies that without the query:

Suggested change
`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 ` +

Same change needed in the v1 copy at packages/sdk/js/src/gen/client/client.gen.ts:132 and in the build.ts template at :77, since those are separate copies.

`(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) } },
)
Comment on lines +179 to +184

@sahrizvi sahrizvi Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message names the content-type, which in the only case that fires is application/json.

Because the guard runs only when parseAs resolved to "json", what users actually see is:

Expected a JSON response but received application/json (HTTP 200).

That's the string in the PR's own verification table, and it reads as self-contradictory — the content-type is the one field that isn't discriminating here.

The more concrete loss is that the error carries no request identity. packages/sdk/js/src/error-interceptor.ts (describe()) deliberately puts method + URL + status into every wrapped client error so formatters and telemetry have something traceable. A telemetry event carrying this message can't be traced to an endpoint or a host, and request is in scope at both sites.

Keeping the body out of the message is right — embedding a gateway page risks logging something sensitive — but it can live on cause for anyone debugging:

Suggested change
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 },
)
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) } },
)

Same applies to the v1 copy at gen/client/client.gen.ts:131-135.

@ralphstodomingo ralphstodomingo Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fcfb24b9f — your suggested message adopted nearly verbatim in both copies (and in the build.ts template, which is now the authoritative v2 source): method + URL for traceability, content-type named honestly with ?? "unset", body kept out of the message but a 200-char slice on cause alongside the parse error and status. The live-server test asserts the message carries the URL and the cause carries the body slice.

}
// altimate_change end
Comment on lines +172 to +186

@sahrizvi sahrizvi Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this hunk is deleted by the release build, so it never ships.

packages/sdk/js/script/build.ts:16-22 regenerates this whole tree:

await createClient({
  input: "./openapi.json",
  output: { path: "./src/v2/gen", tsConfigPath: ..., clean: true },
  ...
})

clean: true wipes src/v2/gen and regenerates client/client.gen.ts from the @hey-api/client-fetch template — no guard, no markers — and nothing re-applies it afterwards.

This isn't "if someone runs generate". script/publish.ts:19-28 calls ./packages/sdk/js/script/build.ts inside prepareReleaseFiles(), which runs on every release, before the CLI and SDK are packed. So the published @opencode-ai/sdk/v2 — and the altimate binary that bundles it — ships without this fix, and the crash in the telemetry keeps firing.

Two things worth flagging:

  1. The correct pattern is twelve lines below the generation call. build.ts:43-59 re-applies the SseFn codegen patch post-generation and throws if the needle stops matching. That's exactly what this needs:

    const v2ClientPath = "./src/v2/gen/client/client.gen.ts"
const v2ClientSource = await Bun.file(v2ClientPath).text()
const needle = "data = text ? JSON.parse(text) : {}"
const v2ClientPatched = v2ClientSource.replace(needle, guardedBlock)
if (v2ClientPatched === v2ClientSource) {
  throw new Error(`json-guard patch did not apply; @hey-api/client-fetch output may have changed (${v2ClientPath})`)
}
await Bun.write(v2ClientPath, v2ClientPatched)

    Pair it with a codegen-idempotence check (run build.ts, assert the guard is still there). The replace assertion catches a template change; the test catches someone removing the patch step.

  2. The markers don't protect these files. script/upstream/analyze.ts:707-721 excludes both gen trees from marker checks outright:

    const markerExcludePatterns = [ ..., "packages/sdk/js/src/gen/**", "packages/sdk/js/src/v2/gen/**", ... ]

    So the PR description's rationale — markers here mean the bridge-merge process sees and carries them — doesn't hold for these two paths. The marker format is right; the file is the problem. This is the first altimate_change marker to land inside src/v2/gen.

Note the asymmetry the description presents as equivalence: src/gen (v1) isn't regenerated by build.ts (only prettier --write), so the v1 hunk survives — by accident of v1 being a frozen snapshot, not because of the markers. Worth saying so in the v1 comment.

@ralphstodomingo ralphstodomingo Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fcfb24b9f — you caught the fatal one; thank you. build.ts now re-applies the guard after codegen using exactly the SseFn pattern you pointed at, with one addition learned by running the full build: raw codegen emits the statement with a trailing semicolon (prettier strips it later), so the needle includes it — my first needle left the ; dangling, saved only by landing inside a comment. Verified end-to-end: ran script/build.ts, confirmed the regenerated tree carries the guard. Drift canaries pin both halves (guard present in both gen files; build.ts contains the re-apply with the exact needle). The committed v2 hunk now says 'edit it in build.ts, not here', and the PR description drops the marker-protection claim for the gen trees per your analyze.ts point — the v1 note now says plainly it survives because v1 is a frozen snapshot.

break
}
case "stream":
Expand Down
Loading