From 26e1c048dff95f2be842886ff88efe1c960d62d2 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 07:30:06 -0700 Subject: [PATCH 1/4] Add tests for honest workbench-create failure copy Pins the wording describeWorkbenchCreateFailure should pick for each shape of cause: a precondition Error shown verbatim, an ApiQueryError routed through describeApiError, and a status-less/network failure still landing on the generic try-again. --- .../src/pages/new-workbench-picker.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 apps/web/src/pages/new-workbench-picker.test.ts diff --git a/apps/web/src/pages/new-workbench-picker.test.ts b/apps/web/src/pages/new-workbench-picker.test.ts new file mode 100644 index 000000000..08fcd56b5 --- /dev/null +++ b/apps/web/src/pages/new-workbench-picker.test.ts @@ -0,0 +1,41 @@ +// CL-6495: the create-workbench catch used to discard whatever +// `createWorkbenchFromTemplate` threw behind a fixed "try again" toast — +// honest for a transient failure, a lie for a precondition this bench +// doesn't meet (no setup agent, an unavailable template). These pin the +// wording `describeWorkbenchCreateFailure` picks for each shape of cause. + +import { describe, expect, test } from "bun:test"; +import { ApiQueryError } from "@corbits/api-query"; + +import { describeWorkbenchCreateFailure } from "./new-workbench-picker"; + +describe("describeWorkbenchCreateFailure", () => { + test("a precondition Error (no setup agent, unavailable template) is shown verbatim, not flattened to a retry prompt", () => { + expect( + describeWorkbenchCreateFailure( + new Error("A code-review workbench isn't available here yet."), + ), + ).toBe("A code-review workbench isn't available here yet."); + }); + + test("an ApiQueryError runs through describeApiError so the status drives the wording", () => { + expect(describeWorkbenchCreateFailure(new ApiQueryError("boom", 404))).toBe( + "This isn't here anymore.", + ); + expect(describeWorkbenchCreateFailure(new ApiQueryError("boom", 401))).toBe( + "You don't have access to this.", + ); + }); + + test("a network-level ApiQueryError with no status still names retrying as the honest answer", () => { + expect( + describeWorkbenchCreateFailure(new ApiQueryError("Failed to fetch")), + ).toBe("Something went wrong creating this workbench. Try again."); + }); + + test("a non-Error cause falls back to the same generic, honest message", () => { + expect(describeWorkbenchCreateFailure("boom")).toBe( + "Something went wrong creating this workbench. Try again.", + ); + }); +}); From 2d3cef99d8e8940aeff3a8e116697375fdb42054 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 07:30:14 -0700 Subject: [PATCH 2/4] Workbench picker: surface why creation failed instead of a bare toast The create-workbench catch bound nothing, so whatever createWorkbenchFromTemplate threw was discarded and every failure showed the same 'try again' toast regardless of whether retrying could help. Now the cause is logged with its status/path so the next occurrence names which step fired, and the toast copy is honest: a precondition failure (no setup agent, an unavailable template) shows its own message instead of a retry prompt that would be a lie, and an ApiQueryError runs through describeApiError so the status drives the wording, matching the treatment other pages already use. --- apps/web/src/pages/new-workbench-picker.tsx | 29 +++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/apps/web/src/pages/new-workbench-picker.tsx b/apps/web/src/pages/new-workbench-picker.tsx index 44d0c3359..e703afb1c 100644 --- a/apps/web/src/pages/new-workbench-picker.tsx +++ b/apps/web/src/pages/new-workbench-picker.tsx @@ -12,6 +12,8 @@ import { Button, toast } from "@corbits/react-ui"; import { ChatCircle, GitPullRequest, Plus } from "@corbits/icons"; import { WorkbenchLoadingState } from "@corbits/chat-ui"; import { useState } from "react"; +import { getLogger } from "@corbits/client-log"; +import { ApiQueryError, describeApiError } from "@corbits/api-query"; import type { ConnectGithubRepo } from "@corbits/chat-ui"; @@ -31,6 +33,24 @@ import { } from "../workbench-templates"; import { GithubRepoSelectDialog } from "./github-repo-select-dialog"; +const log = getLogger("web.new-workbench-picker"); + +/** + * "Try again" is only honest advice for a transient failure (a bad + * connection, a 5xx) — `describeApiError` already speaks to that case. + * A missing setup agent or an unavailable template is a precondition + * this bench doesn't meet, not a fluke: `createWorkbenchFromTemplate` + * throws a plain `Error` with that exact, already-human message for + * those, and the honest move is to show it verbatim rather than + * flattening it into a generic retry prompt. + */ +export function describeWorkbenchCreateFailure(cause: unknown): string { + if (cause instanceof Error && !(cause instanceof ApiQueryError)) { + return cause.message; + } + return describeApiError(cause, "creating this workbench"); +} + type RepoPickerState = { readonly orgName: string; readonly repos: readonly ConnectGithubRepo[]; @@ -100,8 +120,13 @@ export function NewWorkbenchPickerRoute() { navigate, pickGithubRepos, ); - } catch { - toast("Couldn't create the workbench — try again."); + } catch (cause) { + log.error("Couldn't create the workbench", { + message: cause instanceof Error ? cause.message : String(cause), + status: cause instanceof ApiQueryError ? cause.status : undefined, + path: cause instanceof ApiQueryError ? cause.path : undefined, + }); + toast(describeWorkbenchCreateFailure(cause)); setCreating(false); } } From fcf28d73b742d39220b7a52cbffc45728e5e610b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 07:42:50 -0700 Subject: [PATCH 3/4] Workbench picker: stop the create-failure toast leaking internals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix denylisted ApiQueryError before showing an Error's message verbatim, which failed open: ChatApiError (thrown by createWorkbench, patchWorkbenchSettings, and the GitHub-connect steps) slipped straight through and put raw request paths and schema summaries in the toast — the exact leak class this ticket exists to close. Same for plain Errors from listPluginsForTenant and instantiateWorkbenchTemplate. Inverted to an allow-list: only WorkbenchPreconditionError (a new marker for the two intentionally user-facing precondition messages — no setup agent, an unavailable template) is shown verbatim. ApiQueryError and ChatApiError each go through their own describer (describeApiError, describeChatError, the latter now exported from chat-ui), and anything else falls to one generic message. A future error type therefore fails safe by default instead of leaking. Also reads status off ChatApiError for the create-failure log, since four of the five awaited steps throw that type and previously logged status: undefined regardless of what actually failed. --- apps/web/src/instant-agent-create.ts | 24 ++++++- .../src/pages/new-workbench-picker.test.ts | 67 ++++++++++++++++--- apps/web/src/pages/new-workbench-picker.tsx | 41 ++++++++---- packages/chat-ui/src/index.ts | 1 + 4 files changed, 110 insertions(+), 23 deletions(-) diff --git a/apps/web/src/instant-agent-create.ts b/apps/web/src/instant-agent-create.ts index 3b3071248..3160ca20a 100644 --- a/apps/web/src/instant-agent-create.ts +++ b/apps/web/src/instant-agent-create.ts @@ -46,6 +46,18 @@ import type { WorkbenchTemplateId } from "./workbench-templates"; export const NEW_WORKBENCH_TITLE = "New Workbench"; +/** + * Marks the two precondition failures below as intentionally + * user-facing: their `message` is authored copy, never a raw request + * path or schema summary, so a caller can show it verbatim. Every + * other throw on this path (`ApiQueryError`, `ChatApiError`, or a + * plain `Error` from a package that hasn't opted in) must go through + * that error type's own describer instead — allow-listing safe + * throws, rather than denylisting unsafe ones, so a new error type + * added later fails safe (masked) instead of leaking by default. + */ +export class WorkbenchPreconditionError extends Error {} + /** * Presents the connected org's repo list for the person to pick from once * the workbench exists — the create flow's own "select" half of CL-6386 @@ -72,7 +84,9 @@ export async function createAgentAndLaunch( const definitions = await listAgentDefinitions(tenantId); const template = findMyraDefinition(definitions); if (template === undefined) { - throw new Error("No default setup agent found for this workbench."); + throw new WorkbenchPreconditionError( + "No default setup agent found for this workbench.", + ); } await launchAgentChat(tenantId, template.id, navigate, NEW_WORKBENCH_TITLE); } @@ -104,7 +118,9 @@ export async function createWorkbenchFromTemplate( const definitions = await listAgentDefinitions(tenantId); const setupTemplate = findMyraDefinition(definitions); if (setupTemplate === undefined) { - throw new Error("No default setup agent found for this workbench."); + throw new WorkbenchPreconditionError( + "No default setup agent found for this workbench.", + ); } // The manifest comes from the bench library (CL-6344), never from a // hardcoded catalog import; reading it is what seeds the shelf @@ -119,7 +135,9 @@ export async function createWorkbenchFromTemplate( : ((await fetchWorkbenchTemplateManifest(tenantId, templateId)) ?? undefined); if (templateId !== "blank" && manifest === undefined) { - throw new Error(`A ${templateId} workbench isn't available here yet.`); + throw new WorkbenchPreconditionError( + `A ${templateId} workbench isn't available here yet.`, + ); } const requiresGithub = manifest?.requiredConnections.includes("github") ?? false; diff --git a/apps/web/src/pages/new-workbench-picker.test.ts b/apps/web/src/pages/new-workbench-picker.test.ts index 08fcd56b5..b6955482f 100644 --- a/apps/web/src/pages/new-workbench-picker.test.ts +++ b/apps/web/src/pages/new-workbench-picker.test.ts @@ -3,21 +3,45 @@ // honest for a transient failure, a lie for a precondition this bench // doesn't meet (no setup agent, an unavailable template). These pin the // wording `describeWorkbenchCreateFailure` picks for each shape of cause. +// +// The check is an allow-list (only `WorkbenchPreconditionError` is shown +// verbatim), not a denylist of `ApiQueryError` — a plain `Error` from +// anywhere else on the path (`listPluginsForTenant`, `instantiateWorkbenchTemplate`, +// or any future throw) must fall to the generic message rather than leak +// a raw request path or schema summary into the toast. import { describe, expect, test } from "bun:test"; import { ApiQueryError } from "@corbits/api-query"; +import { ChatApiError } from "@corbits/chat-ui"; import { describeWorkbenchCreateFailure } from "./new-workbench-picker"; +import { WorkbenchPreconditionError } from "../instant-agent-create"; + +const GENERIC = "Something went wrong creating this workbench. Try again."; describe("describeWorkbenchCreateFailure", () => { - test("a precondition Error (no setup agent, unavailable template) is shown verbatim, not flattened to a retry prompt", () => { + test("a WorkbenchPreconditionError (no setup agent, unavailable template) is shown verbatim", () => { expect( describeWorkbenchCreateFailure( - new Error("A code-review workbench isn't available here yet."), + new WorkbenchPreconditionError( + "A code-review workbench isn't available here yet.", + ), ), ).toBe("A code-review workbench isn't available here yet."); }); + test("a plain Error carrying internal detail is masked, not shown verbatim", () => { + // The exact shape `listPluginsForTenant`/`instantiateWorkbenchTemplate` + // throw: a message embedding a raw status code or schema summary. + expect( + describeWorkbenchCreateFailure( + new Error( + "Unexpected response shape resolving GitHub: must be an object", + ), + ), + ).toBe(GENERIC); + }); + test("an ApiQueryError runs through describeApiError so the status drives the wording", () => { expect(describeWorkbenchCreateFailure(new ApiQueryError("boom", 404))).toBe( "This isn't here anymore.", @@ -27,15 +51,42 @@ describe("describeWorkbenchCreateFailure", () => { ); }); - test("a network-level ApiQueryError with no status still names retrying as the honest answer", () => { + test("an ApiQueryError never leaks its raw message (path, status text) into the toast", () => { expect( - describeWorkbenchCreateFailure(new ApiQueryError("Failed to fetch")), - ).toBe("Something went wrong creating this workbench. Try again."); + describeWorkbenchCreateFailure( + new ApiQueryError( + "The server answered 500.", + 500, + "/api/tenants/tnt_1/template-blocks/code-review/deploy", + ), + ), + ).not.toContain("/api/tenants"); }); - test("a non-Error cause falls back to the same generic, honest message", () => { - expect(describeWorkbenchCreateFailure("boom")).toBe( - "Something went wrong creating this workbench. Try again.", + test("a ChatApiError — createWorkbench, patchWorkbenchSettings, and the GitHub-connect steps all throw this — runs through describeChatError", () => { + expect(describeWorkbenchCreateFailure(new ChatApiError("boom", 401))).toBe( + "You're signed out. Sign in again to continue.", + ); + expect(describeWorkbenchCreateFailure(new ChatApiError("boom", 403))).toBe( + "You don't have access to this.", + ); + expect(describeWorkbenchCreateFailure(new ChatApiError("boom", 500))).toBe( + "Something went wrong on our end. Try again in a moment.", ); }); + + test("a ChatApiError never leaks its raw message (it always embeds the request path)", () => { + expect( + describeWorkbenchCreateFailure( + new ChatApiError( + "The server answered 500 for /api/tenants/tnt_1/chat/workbenches.", + 500, + ), + ), + ).not.toContain("/api/tenants"); + }); + + test("a non-Error cause falls back to the same generic, honest message", () => { + expect(describeWorkbenchCreateFailure("boom")).toBe(GENERIC); + }); }); diff --git a/apps/web/src/pages/new-workbench-picker.tsx b/apps/web/src/pages/new-workbench-picker.tsx index e703afb1c..6c088528d 100644 --- a/apps/web/src/pages/new-workbench-picker.tsx +++ b/apps/web/src/pages/new-workbench-picker.tsx @@ -10,7 +10,11 @@ import { Button, toast } from "@corbits/react-ui"; import { ChatCircle, GitPullRequest, Plus } from "@corbits/icons"; -import { WorkbenchLoadingState } from "@corbits/chat-ui"; +import { + ChatApiError, + describeChatError, + WorkbenchLoadingState, +} from "@corbits/chat-ui"; import { useState } from "react"; import { getLogger } from "@corbits/client-log"; import { ApiQueryError, describeApiError } from "@corbits/api-query"; @@ -22,6 +26,7 @@ import { TemplateLibraryPage } from "../workbench-templates-api"; import { useBench } from "../bench-context"; import { createWorkbenchFromTemplate, + WorkbenchPreconditionError, type PickGithubRepos, } from "../instant-agent-create"; import { useNavigate } from "../navigation"; @@ -35,20 +40,29 @@ import { GithubRepoSelectDialog } from "./github-repo-select-dialog"; const log = getLogger("web.new-workbench-picker"); +const GENERIC_CREATE_FAILURE = + "Something went wrong creating this workbench. Try again."; + /** - * "Try again" is only honest advice for a transient failure (a bad - * connection, a 5xx) — `describeApiError` already speaks to that case. - * A missing setup agent or an unavailable template is a precondition - * this bench doesn't meet, not a fluke: `createWorkbenchFromTemplate` - * throws a plain `Error` with that exact, already-human message for - * those, and the honest move is to show it verbatim rather than - * flattening it into a generic retry prompt. + * Allow-lists what's safe to show verbatim, rather than denylisting + * what to hide — a new error type `createWorkbenchFromTemplate`'s path + * starts throwing later lands here unrecognized and falls to the + * generic message, not into the toast raw. Only + * `WorkbenchPreconditionError` carries authored, always-safe copy + * ("try again" is a lie for a missing template, so its own message + * says so instead); `ApiQueryError` and `ChatApiError` both embed raw + * request paths and schema summaries in `.message` and must go through + * their own describer, never shown directly. */ export function describeWorkbenchCreateFailure(cause: unknown): string { - if (cause instanceof Error && !(cause instanceof ApiQueryError)) { - return cause.message; + if (cause instanceof WorkbenchPreconditionError) return cause.message; + if (cause instanceof ApiQueryError) { + return describeApiError(cause, "creating this workbench"); + } + if (cause instanceof ChatApiError) { + return describeChatError(cause, GENERIC_CREATE_FAILURE); } - return describeApiError(cause, "creating this workbench"); + return GENERIC_CREATE_FAILURE; } type RepoPickerState = { @@ -123,7 +137,10 @@ export function NewWorkbenchPickerRoute() { } catch (cause) { log.error("Couldn't create the workbench", { message: cause instanceof Error ? cause.message : String(cause), - status: cause instanceof ApiQueryError ? cause.status : undefined, + status: + cause instanceof ApiQueryError || cause instanceof ChatApiError + ? cause.status + : undefined, path: cause instanceof ApiQueryError ? cause.path : undefined, }); toast(describeWorkbenchCreateFailure(cause)); diff --git a/packages/chat-ui/src/index.ts b/packages/chat-ui/src/index.ts index c382d758c..47801afdf 100644 --- a/packages/chat-ui/src/index.ts +++ b/packages/chat-ui/src/index.ts @@ -131,6 +131,7 @@ export { isKnownWorkbenchKind, MessageSender, ChatApiError, + describeChatError, listWorkbenches, listAllWorkbenches, workbenchesQueryKey, From c7d349ea97dde7f8ee23252b21b4527c3eb9fa84 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 21 Aug 2026 07:58:05 -0700 Subject: [PATCH 4/4] Assert the precondition wording in the one-toast test The stub serves an empty definitions list, so the create fails its precondition rather than the request. The generic toast used to hide that; now that preconditions are shown verbatim, the assertion names the failure the test actually exercises. --- apps/web/test/toast-single-system.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/web/test/toast-single-system.test.tsx b/apps/web/test/toast-single-system.test.tsx index 6d36a4f47..ecec0ac6e 100644 --- a/apps/web/test/toast-single-system.test.tsx +++ b/apps/web/test/toast-single-system.test.tsx @@ -142,8 +142,11 @@ describe("the one toast system (CL-6372)", () => { const shown = visibleToasts(); expect(shown.length).toBe(1); + // The stub serves an empty definitions list, so the create fails its + // precondition before any request is sent. That is a + // `WorkbenchPreconditionError`, which the picker shows verbatim. expect(shown[0]?.textContent).toBe( - "Couldn't create the workbench — try again.", + "No default setup agent found for this workbench.", ); await waitForClear(); });