From 6bdf94815668e9dc8f2f81432155618afc0d5769 Mon Sep 17 00:00:00 2001 From: Alex Lavaee Date: Wed, 21 Oct 2026 14:43:59 -0700 Subject: [PATCH 1/2] Report a gateway's empty error body by its HTTP status in chopin status A proxy answering for a Chopin that went away sends 502 with Content-Length 0. Bun reads that empty body as JSON null, so `chopin status` threw a TypeError reading `error` of null instead of saying Chopin answered http-502. Every connector read of a JSON answer now goes through one helper that treats a body that is not a JSON object as empty: the machine check in status, registration and pairing requests, the version refusal check, and `chopin update` asking Chopin which release to install, which said "null is not an object" instead of "it recommends none" for an empty answer. Assistant-model: Claude Opus 5.5 (fast) Assistant-workflow: inline (fast subagents) Assistant-verification: regression tests failed before the fix and pass after it Assistant-verification: bun test (connector suite and full suite) passed Assistant-verification: bun run types and bun run ci passed Co-authored-by: Alex Lavaee --- apps/connector/src/machine.ts | 6 ++---- apps/connector/src/mcp.ts | 15 +++++++++++---- apps/connector/src/status.test.ts | 11 +++++++++++ apps/connector/src/status.ts | 4 ++-- apps/connector/src/update.test.ts | 17 +++++++++++++++++ apps/connector/src/update.ts | 5 ++--- 6 files changed, 45 insertions(+), 13 deletions(-) diff --git a/apps/connector/src/machine.ts b/apps/connector/src/machine.ts index 4d6490a6c..74edec945 100644 --- a/apps/connector/src/machine.ts +++ b/apps/connector/src/machine.ts @@ -12,7 +12,7 @@ import { CONNECTOR_UPDATE_REQUIRED, } from "@chopin/protocol/connector"; import { redacted } from "./connection"; -import { CONNECTOR_HEADERS, ConnectorError, refusal } from "./mcp"; +import { CONNECTOR_HEADERS, ConnectorError, jsonObject, refusal } from "./mcp"; import type { Output } from "./output"; /** @@ -188,9 +188,7 @@ async function post( }); let refused = await refusal(response); if (refused) throw refused; - // Bun reads an empty body sent with Content-Length 0, as a gateway's 502 is, as JSON null. - let parsed: unknown = await response.json().catch(() => undefined); - let value = (parsed && typeof parsed === "object" ? parsed : {}) as Record; + let value = await jsonObject(response); if (!response.ok) { let code = typeof value.error === "string" ? value.error : `http-${response.status}`; throw new ConnectorError( diff --git a/apps/connector/src/mcp.ts b/apps/connector/src/mcp.ts index e166c9db9..020bad11f 100644 --- a/apps/connector/src/mcp.ts +++ b/apps/connector/src/mcp.ts @@ -19,16 +19,23 @@ export const CONNECTOR_HEADERS = { [CONNECTOR_PROTOCOL_HEADER]: String(CONNECTOR_PROTOCOL), }; +/** + * The JSON object a response carries, or an empty one when its body is anything else. Bun reads an + * empty body sent with Content-Length 0, as a gateway's 502 is, as JSON null rather than failing. + */ +export async function jsonObject(response: Response) { + let parsed: unknown = await response.json().catch(() => undefined); + return (parsed && typeof parsed === "object" ? parsed : {}) as Record; +} + /** * The server's refusal of this connector's version, which no retry can fix. Its message names the * release to install. */ export async function refusal(response: Response) { if (response.status !== 409) return; - let body = await response.clone().json().catch(() => undefined) as - | { error?: unknown; message?: unknown } - | undefined; - if (body?.error === CONNECTOR_UPDATE_REQUIRED && typeof body.message === "string") { + let body = await jsonObject(response.clone()); + if (body.error === CONNECTOR_UPDATE_REQUIRED && typeof body.message === "string") { return new ConnectorError(CONNECTOR_UPDATE_REQUIRED, body.message, 409); } } diff --git a/apps/connector/src/status.test.ts b/apps/connector/src/status.test.ts index 59c092ee6..2fb88db91 100644 --- a/apps/connector/src/status.test.ts +++ b/apps/connector/src/status.test.ts @@ -428,6 +428,17 @@ test("status still reports when Chopin is down, and says when it refuses the mac await hung.stop(true); }); +test("status reports the HTTP status of a gateway that answers with an empty body", async () => { + // A proxy such as Caddy answers for a Chopin that went away with 502 and Content-Length 0. + using gateway = chopin(() => new Response(null, { status: 502 })); + let paired = await machine(gateway.url); + let report = await collectStatus({ ...paired, server: gateway.url, source: "config" }); + expect(report.machine).toMatchObject({ check: "error", accepted: null, detail: "http-502" }); + expect(formatStatus(report).join("\n")).toContain( + "Machine: paired as build-box; Chopin's answer was http-502", + ); +}); + // Run under a temporary HOME, so the service check sees no unit and runs nothing. test.skipIf(process.platform === "win32")( "chopin status --json reads the base dir it is given and prints the report", diff --git a/apps/connector/src/status.ts b/apps/connector/src/status.ts index 7a3bba460..29891ca3d 100644 --- a/apps/connector/src/status.ts +++ b/apps/connector/src/status.ts @@ -1,6 +1,6 @@ import { type ConnectSettings, readConfig } from "./config"; import { machineLabel, machineToken, SELF } from "./machine"; -import { CONNECTOR_HEADERS, refusal } from "./mcp"; +import { CONNECTOR_HEADERS, jsonObject, refusal } from "./mcp"; import { logPath, type ServiceHost, type ServiceStatus, serviceStatus } from "./service"; import { appCannotPush } from "./github-token"; import { CONNECTOR_PROTOCOL, VERSION } from "./version"; @@ -130,7 +130,7 @@ async function check(url: string, token: string, timeoutMs: number) { if (refused) { return { check: "update-required" as const, accepted: false, detail: refused.message }; } - let body = await response.json().catch(() => ({})) as Record; + let body = await jsonObject(response); if (response.ok) { return { check: "accepted" as const, diff --git a/apps/connector/src/update.test.ts b/apps/connector/src/update.test.ts index e060f9d11..d1b9eb7ad 100644 --- a/apps/connector/src/update.test.ts +++ b/apps/connector/src/update.test.ts @@ -169,6 +169,23 @@ describe.skipIf(!binary)("chopin update", () => { expect(await readFile(at.chopin, "utf8")).toBe(fakeChopin("99.0.0")); }); + test("installs the newest release when the Chopin server answers with an empty body", async () => { + using release = releases(); + let empty = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: () => new Response(null) }); + let url = `http://127.0.0.1:${empty.port}`; + try { + let at = await installed(); + let result = await at.update({ ...release.env, CHOPIN_URL: url }); + expect(result.code).toBe(0); + expect(result.stdout).toContain( + `Could not ask ${url} which chopin to install (it recommends none), so installing the ` + + "newest release, 99.0.0, instead.", + ); + } finally { + await empty.stop(true); + } + }); + test("moves to an older release only with --allow-downgrade", async () => { using release = releases(); using chopin = chopinServer("0.0.0"); diff --git a/apps/connector/src/update.ts b/apps/connector/src/update.ts index 92eaabc65..15e32dc34 100644 --- a/apps/connector/src/update.ts +++ b/apps/connector/src/update.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs/promises"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; +import { jsonObject } from "./mcp"; import { VERSION } from "./version"; /** @@ -171,9 +172,7 @@ async function newest(api: string) { /** The release a Chopin server recommends to its connectors, if it can be asked. */ async function recommended(server: string) { try { - let body = await (await download(`${server}/api/connector/version`)).json() as { - version?: unknown; - }; + let body = await jsonObject(await download(`${server}/api/connector/version`)); if (typeof body.version !== "string") return { reason: "it recommends none" }; return { version: releaseVersion(body.version) }; } catch (error) { From 742d4a2ac6da3eb202caf11fe1460ead7dd35088 Mon Sep 17 00:00:00 2001 From: Alex Lavaee Date: Wed, 21 Oct 2026 14:43:59 -0700 Subject: [PATCH 2/2] Name a Build task row without a space before its state's comma A task row's button took its accessible name from its content: the title and a visually hidden ", done". Browsers set that absolutely positioned hidden text apart from the title with a space, so every row read as "Deploy on main , done", whatever its title or state. The row now carries an explicit label composed from the title's shown text, parsed as its inline Markdown renders, and its state, with single spaces and nothing before the comma. Assistant-model: Claude Opus 5.5 (fast) Assistant-workflow: inline (fast subagents) Assistant-verification: the browser test's exact name assertion failed before the fix and passes after it Assistant-verification: bun test (web unit tests and full suite) passed Assistant-verification: Build and machine browser tests passed Assistant-verification: bun run types and bun run ci passed Co-authored-by: Alex Lavaee --- apps/web/package.json | 2 ++ apps/web/src/build-model.test.ts | 17 +++++++++++++++++ apps/web/src/build-model.ts | 11 +++++++++++ apps/web/src/build-view.tsx | 5 +++-- apps/web/src/chat/inline-text.ts | 16 ++++++++++++++++ apps/web/src/chat/markdown.test.ts | 19 +++++++++++++++++++ bun.lock | 2 ++ e2e/implementation-launcher.e2e.ts | 15 ++++++++++----- 8 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 apps/web/src/chat/inline-text.ts diff --git a/apps/web/package.json b/apps/web/package.json index 65b683d9f..cc6b383ba 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -23,6 +23,8 @@ "@chopin/viewport": "workspace:*", "@fontsource-variable/inter": "^5.3.0", "@fontsource/lora": "^5.3.0", + "mdast-util-from-markdown": "2.0.2", + "mdast-util-to-string": "4.0.0", "react": "catalog:react", "react-dom": "catalog:react", "react-markdown": "^10.1.0" diff --git a/apps/web/src/build-model.test.ts b/apps/web/src/build-model.test.ts index 9239a055c..44a672565 100644 --- a/apps/web/src/build-model.test.ts +++ b/apps/web/src/build-model.test.ts @@ -28,9 +28,11 @@ import { startingHint, syncHint, syncStatus, + taskRowName, taskStartsOpen, unfinishedReason, } from "./build-model"; +import { inlineText } from "./chat/inline-text"; import type { BuildRequest, @@ -213,6 +215,21 @@ describe("presentation", () => { expect(taskStartsOpen("queued", false)).toBe(false); expect(taskStartsOpen("completed", true)).toBe(true); }); + + it("names a task row by its title as shown, then its state, with no space before a comma", () => { + let title = inlineText("Deploy on `main`"); + expect(taskRowName(title, "completed", true)).toBe("Deploy on main, done, no change needed"); + expect(taskRowName(title, "completed", false)).toBe("Deploy on main, done"); + expect(taskRowName(inlineText("Read [the docs](https://example.com)"), "blocked", false)) + .toBe("Read the docs, blocked"); + expect(taskRowName(" Wire the\nagent ", "in_progress", false)).toBe( + "Wire the agent, in progress", + ); + expect(taskRowName("Wire the agent", "queued", false)).toBe("Wire the agent, queued"); + // A title that shows no text, such as only an image, leaves the state alone. + expect(taskRowName(inlineText("![image](https://example.invalid/i.png)"), "queued", false)) + .toBe("queued"); + }); }); describe("attribution", () => { diff --git a/apps/web/src/build-model.ts b/apps/web/src/build-model.ts index fdc3b97c9..c58e1b039 100644 --- a/apps/web/src/build-model.ts +++ b/apps/web/src/build-model.ts @@ -344,6 +344,17 @@ export function taskStateLabel(state: TaskState, noChange: boolean): string { return state === "completed" && noChange ? "done, no change needed" : TASK_STATE_LABEL[state]; } +/** + * A task row's accessible name: the title as shown (`inlineText`), then its state. Given as a + * label, since a name read from the row's content gains a space before the comma: browsers set + * a visually hidden, absolutely positioned state apart from the title with one. + */ +export function taskRowName(title: string, state: TaskState, noChange: boolean): string { + let text = title.replace(/\s+/g, " ").trim(); + let label = taskStateLabel(state, noChange); + return text ? `${text}, ${label}` : label; +} + /** Active and blocked rows start open; so does a linked task. */ export function taskStartsOpen(state: TaskState, linked: boolean): boolean { return linked || state === "in_progress" || state === "blocked"; diff --git a/apps/web/src/build-view.tsx b/apps/web/src/build-view.tsx index d9e4e8967..ca2afbae0 100644 --- a/apps/web/src/build-view.tsx +++ b/apps/web/src/build-view.tsx @@ -27,8 +27,8 @@ import { pullRequestCommits, pullRequestNumber, retryAction, + taskRowName, taskStartsOpen, - taskStateLabel, unfinishedReason, } from "./build-model"; import { @@ -37,6 +37,7 @@ import { implementationResponse as response, stopBuild, } from "./build-start"; +import { inlineText } from "./chat/inline-text"; import { InlineMarkdown, MessageMarkdown } from "./chat/markdown"; import { LocalAgentNote } from "./local-agent-note"; import { MachineGlyph } from "./machine-button"; @@ -641,6 +642,7 @@ function TaskRow( {pullRequest && ( { expect(inline("raw ![image](https://example.invalid/i.png)")).toBe("raw "); expect(inline("one\n\ntwo")).toBe("one two"); }); + +test("the plain text of inline Markdown is the text it shows, with single spaces", () => { + let sources = [ + "Rewire `login` to **call** _three_ [helpers](https://example.com)", + "Deploy on `main`", + "Read [the docs](https://example.com)", + "raw ![image](https://example.invalid/i.png)", + "> quote & `a b`", + "one\n\ntwo", + ]; + for (let source of sources) { + let shown = inline(source, false).replace(/<[^>]+>/g, "").replaceAll(">", ">") + .replaceAll("&", "&"); + expect(inlineText(source)).toBe(shown.replace(/\s+/g, " ").trim()); + } + expect(inlineText("Deploy on `main`")).toBe("Deploy on main"); + expect(inlineText("raw ![image](https://example.invalid/i.png)")).toBe("raw"); +}); diff --git a/bun.lock b/bun.lock index 40c9df897..7d27e3b70 100644 --- a/bun.lock +++ b/bun.lock @@ -80,6 +80,8 @@ "@chopin/visuals": "workspace:*", "@fontsource-variable/inter": "^5.3.0", "@fontsource/lora": "^5.3.0", + "mdast-util-from-markdown": "2.0.2", + "mdast-util-to-string": "4.0.0", "react": "catalog:react", "react-dom": "catalog:react", "react-markdown": "^10.1.0", diff --git a/e2e/implementation-launcher.e2e.ts b/e2e/implementation-launcher.e2e.ts index 8cfde74fc..82dceb311 100644 --- a/e2e/implementation-launcher.e2e.ts +++ b/e2e/implementation-launcher.e2e.ts @@ -839,9 +839,12 @@ test("a build that ran for an hour reads as built when it finished, not when it test("a task that needed no change reads as done without a pull request, with its evidence", async ({ seed, join: enter }) => { let now = Date.now(); + let graph = preparedGraph(); + // A title ending in inline code reads with its state straight after it, as any title does. + graph.versions[0]!.definition.tasks[0]!.title = "Connect the local agent on `main`"; await seed( "# Checked build\n\nOne task was already satisfied.\n", - builtBuild(preparedGraph(), new Date(now - 10 * 60_000), new Date(now - 60_000), { + builtBuild(graph, new Date(now - 10 * 60_000), new Date(now - 60_000), { connect: { reason: "The connection already reports blockers on the base commit.", evidence: "- Ran `bun test connector` in run 4821: **36 passed**, 0 failed.", @@ -856,10 +859,12 @@ test("a task that needed no change reads as done without a pull request, with it await expect(view.getByText("1 pull request, 1 task needed no change", { exact: true })) .toBeVisible(); let tasks = view.getByRole("list", { name: "Tasks" }); - let checked = tasks.getByRole("button", { - name: /^Connect the local agent\s*, done, no change needed$/, - }); - await expect(checked).toBeVisible(); + let checked = tasks.getByRole("button", { name: /^Connect the local agent/ }); + await expect(checked).toHaveAccessibleName( + "Connect the local agent on main, done, no change needed", + ); + await expect(tasks.getByRole("button", { name: /^Review the connection/ })) + .toHaveAccessibleName("Review the connection, done"); // The task that needed no change has no pull request link; the other task keeps its own. await expect(tasks.getByRole("link", { name: /^Pull request/ })).toHaveCount(1); await expect(tasks.getByText("No change needed", { exact: true })).toBeVisible();