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) { 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();