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 new file mode 100644 index 000000000..b6955482f --- /dev/null +++ b/apps/web/src/pages/new-workbench-picker.test.ts @@ -0,0 +1,92 @@ +// 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. +// +// 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 WorkbenchPreconditionError (no setup agent, unavailable template) is shown verbatim", () => { + expect( + describeWorkbenchCreateFailure( + 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.", + ); + expect(describeWorkbenchCreateFailure(new ApiQueryError("boom", 401))).toBe( + "You don't have access to this.", + ); + }); + + test("an ApiQueryError never leaks its raw message (path, status text) into the toast", () => { + expect( + describeWorkbenchCreateFailure( + new ApiQueryError( + "The server answered 500.", + 500, + "/api/tenants/tnt_1/template-blocks/code-review/deploy", + ), + ), + ).not.toContain("/api/tenants"); + }); + + 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 44d0c3359..6c088528d 100644 --- a/apps/web/src/pages/new-workbench-picker.tsx +++ b/apps/web/src/pages/new-workbench-picker.tsx @@ -10,8 +10,14 @@ 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"; import type { ConnectGithubRepo } from "@corbits/chat-ui"; @@ -20,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"; @@ -31,6 +38,33 @@ import { } from "../workbench-templates"; 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."; + +/** + * 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 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 GENERIC_CREATE_FAILURE; +} + type RepoPickerState = { readonly orgName: string; readonly repos: readonly ConnectGithubRepo[]; @@ -100,8 +134,16 @@ 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 instanceof ChatApiError + ? cause.status + : undefined, + path: cause instanceof ApiQueryError ? cause.path : undefined, + }); + toast(describeWorkbenchCreateFailure(cause)); setCreating(false); } } 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(); }); 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,