From db1a81cfa600a033f85408a94234c64a4da35d02 Mon Sep 17 00:00:00 2001 From: mptyl Date: Fri, 14 Aug 2026 12:06:25 +0200 Subject: [PATCH] fix: surface reviewer response failures --- backend/src/bridge/session-bridge.ts | 16 +++++- backend/test/session-bridge.test.ts | 24 +++++++++ .../src/shell/AppShell.notifications.test.tsx | 28 ++++++++++ frontend/src/shell/AppShell.tsx | 17 ++++++ .../src/shell/WidgetHost.response.test.tsx | 53 +++++++++++++++++++ frontend/src/shell/WidgetHost.tsx | 23 +++++++- frontend/src/store/sessionStore.test.ts | 1 + frontend/src/store/sessionStore.ts | 7 ++- 8 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 frontend/src/shell/AppShell.notifications.test.tsx create mode 100644 frontend/src/shell/WidgetHost.response.test.tsx diff --git a/backend/src/bridge/session-bridge.ts b/backend/src/bridge/session-bridge.ts index 1e65926a..f1dbf2c2 100644 --- a/backend/src/bridge/session-bridge.ts +++ b/backend/src/bridge/session-bridge.ts @@ -1,5 +1,19 @@ import type { RpcClient } from "../rpc/rpc-client.js"; +const GENERIC_MODEL_FAILURE = + "Model request failed. Check provider connectivity, then Resume the session."; +const SUBSCRIPTION_MODEL_FAILURE = + "The selected model is unavailable for the current subscription. Choose another model and start a new session."; + +function safeModelFailure(error: unknown): string { + const detail = typeof error === "string" ? error : ""; + const isSubscriptionFailure = + /\b429\b/.test(detail) && + /(subscription plan|code["':\s]+1311|does not yet include access)/i.test(detail); + + return isSubscriptionFailure ? SUBSCRIPTION_MODEL_FAILURE : GENERIC_MODEL_FAILURE; +} + export type ToolActivity = { kind: "tool"; toolCallId: string; @@ -53,7 +67,7 @@ export class SessionBridge { this.fan({ type: "info", level: "error", - text: "Model request failed. Check provider connectivity, then Resume the session.", + text: safeModelFailure(m.message.errorMessage), }); } } else if (m.type === "extension_ui_request" && m.method === "notify") { diff --git a/backend/test/session-bridge.test.ts b/backend/test/session-bridge.test.ts index fbfe1d50..f0a41341 100644 --- a/backend/test/session-bridge.test.ts +++ b/backend/test/session-bridge.test.ts @@ -105,6 +105,30 @@ test("assistant provider errors are sanitized and leave the turn failed", () => expect(JSON.stringify(seen)).not.toContain("DO_NOT_LEAK"); }); +test("subscription model errors become a safe actionable client message", () => { + const { rpc, fire } = fakeRpc(); + const bridge = new SessionBridge(rpc); + const seen: any[] = []; + bridge.onClientEvent((event) => seen.push(event)); + + fire({ + type: "message_end", + message: { + role: "assistant", + stopReason: "error", + errorMessage: "429: {\"code\":\"1311\",\"message\":\"Your current subscription plan does not yet include access to GLM-5.3\",\"secret\":\"DO_NOT_LEAK\"}", + }, + }); + + expect(seen).toContainEqual({ + type: "info", + level: "error", + text: "The selected model is unavailable for the current subscription. Choose another model and start a new session.", + }); + expect(JSON.stringify(seen)).not.toContain("GLM-5.3"); + expect(JSON.stringify(seen)).not.toContain("DO_NOT_LEAK"); +}); + test("markFailed records backend-detected failure without emitting raw detail", () => { const { rpc } = fakeRpc(); const bridge = new SessionBridge(rpc); diff --git a/frontend/src/shell/AppShell.notifications.test.tsx b/frontend/src/shell/AppShell.notifications.test.tsx new file mode 100644 index 00000000..4555c9ab --- /dev/null +++ b/frontend/src/shell/AppShell.notifications.test.tsx @@ -0,0 +1,28 @@ +import { act, render, screen } from "@testing-library/react"; +import { http, HttpResponse } from "msw"; +import { App } from "../App"; +import { queryClient } from "../app/queryClient"; +import { useSessionStore } from "../store/sessionStore"; +import { server } from "../test/msw"; + +beforeEach(() => { + queryClient.clear(); + useSessionStore.getState().resetSession(); + server.use( + http.get("/api/me", () => HttpResponse.json({ issuer: "local", subject: "dev", isAdmin: true })), + http.get("/api/sessions", () => HttpResponse.json([])), + http.get("/api/settings", () => HttpResponse.json({ workspace: "psd-clinical" })), + http.get("/api/workspaces", () => HttpResponse.json([])), + http.get("/api/models", () => HttpResponse.json({ models: [] })), + ); +}); +test("session errors queued in the store become visible notifications", async () => { + render(); + + act(() => useSessionStore.getState().pushToast({ + level: "error", + text: "The selected model is unavailable for this subscription.", + })); + + expect(await screen.findByText("The selected model is unavailable for this subscription.")).toBeInTheDocument(); +}); diff --git a/frontend/src/shell/AppShell.tsx b/frontend/src/shell/AppShell.tsx index 21af9116..ecc7ab8a 100644 --- a/frontend/src/shell/AppShell.tsx +++ b/frontend/src/shell/AppShell.tsx @@ -372,12 +372,29 @@ export function AppShell() { // condition the final workflow step — which ends with no follow-up gate — would // leave the working state on forever. const pendingWidget = useSessionStore((s) => s.pendingWidget); + const sessionToasts = useSessionStore((s) => s.toasts); const resetSession = useSessionStore((s) => s.resetSession); const recordLifecycle = useSessionStore((s) => s.recordLifecycle); const setPhase = useSessionStore((s) => s.setPhase); const setAgentActive = useSessionStore((s) => s.setAgentActive); const lastSystemEvent = useSessionStore((s) => s.lastSystemEvent); const agentActive = useSessionStore((s) => s.agentActive); + const deliveredToastCountRef = useRef(0); + + useEffect(() => { + if (sessionToasts.length < deliveredToastCountRef.current) { + deliveredToastCountRef.current = 0; + } + + for (const notification of sessionToasts.slice(deliveredToastCountRef.current)) { + if (notification.level === "error") toast.error(notification.text); + else if (notification.level === "success") toast.success(notification.text); + else if (notification.level === "warning") toast.warning(notification.text); + else toast.info(notification.text); + } + + deliveredToastCountRef.current = sessionToasts.length; + }, [sessionToasts]); const sessionViewOpen = Boolean(activeSessionId) || creatingSession; const working = sessionViewOpen && !pendingWidget && agentActive; // The workflow bar runs only while the harness works, not while a finalized diff --git a/frontend/src/shell/WidgetHost.response.test.tsx b/frontend/src/shell/WidgetHost.response.test.tsx new file mode 100644 index 00000000..fc92f1d4 --- /dev/null +++ b/frontend/src/shell/WidgetHost.response.test.tsx @@ -0,0 +1,53 @@ +import { act, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { http, HttpResponse } from "msw"; +import { server } from "../test/msw"; +import { useSessionStore } from "../store/sessionStore"; +import { WidgetHost } from "./WidgetHost"; + +beforeEach(() => useSessionStore.getState().resetSession()); + +test("a gate choice shows progress, prevents duplicate clicks, and confirms delivery", async () => { + let release!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + let requests = 0; + server.use(http.post("/api/sessions/s1/response", async () => { + requests += 1; + await held; + return new HttpResponse(null, { status: 204 }); + })); + useSessionStore.setState({ + pendingWidget: { + id: "gate-1", + widget: "select", + options: [{ id: "recommended", label: "Recommended answer" }], + }, + }); + + const user = userEvent.setup(); + render(); + const choice = screen.getByRole("button", { name: "Recommended answer" }); + await user.click(choice); + + expect(screen.getByRole("status")).toHaveTextContent("Sending response"); + expect(choice).toBeDisabled(); + await user.click(choice); + expect(requests).toBe(1); + + act(() => release()); + await waitFor(() => expect(useSessionStore.getState().pendingWidget).toBeNull()); + expect(useSessionStore.getState().toasts.at(-1)).toEqual({ + level: "success", + text: "Response sent. The model is continuing.", + }); + + act(() => useSessionStore.setState({ + pendingWidget: { + id: "gate-2", + widget: "select", + options: [{ id: "next", label: "Next answer" }], + }, + })); + expect(screen.getByRole("button", { name: "Next answer" })).toBeEnabled(); + expect(screen.queryByRole("status")).not.toBeInTheDocument(); +}); diff --git a/frontend/src/shell/WidgetHost.tsx b/frontend/src/shell/WidgetHost.tsx index da9ae89c..5393f04f 100644 --- a/frontend/src/shell/WidgetHost.tsx +++ b/frontend/src/shell/WidgetHost.tsx @@ -1,3 +1,4 @@ +import { useRef, useState } from "react"; import { useSessionStore } from "../store/sessionStore"; import { resolve } from "../widgets"; import { postResponse } from "../api/sessions"; @@ -9,9 +10,15 @@ export function WidgetHost({ sessionId }: { sessionId: string | null }) { const clearPending = useSessionStore((s) => s.clearPending); const pushToast = useSessionStore((s) => s.pushToast); const setLastUserEntry = useSessionStore((s) => s.setLastUserEntry); + const [responding, setResponding] = useState(false); + const responseInFlight = useRef(false); if (!pending) return null; const Renderer = resolve(pending.widget); const onRespond = async (r: UiResponse) => { + if (responseInFlight.current) return; + responseInFlight.current = true; + setResponding(true); + if (sessionId) { try { await postResponse(sessionId, r); @@ -24,9 +31,14 @@ export function WidgetHost({ sessionId }: { sessionId: string | null }) { ? `Failed to send response: ${err.message}` : "Failed to send response.", }); + responseInFlight.current = false; + setResponding(false); return; } } + responseInFlight.current = false; + setResponding(false); + pushToast({ level: "success", text: "Response sent. The model is continuing." }); setLastUserEntry({ kind: "choice", text: @@ -40,7 +52,16 @@ export function WidgetHost({ sessionId }: { sessionId: string | null }) { // widget render. resetKeys on the descriptor id so the next gate starts clean. return ( - +
+
+ +
+ {responding && ( +

+ Sending response… +

+ )} +
); } diff --git a/frontend/src/store/sessionStore.test.ts b/frontend/src/store/sessionStore.test.ts index b8dcb5d7..4cf65eae 100644 --- a/frontend/src/store/sessionStore.test.ts +++ b/frontend/src/store/sessionStore.test.ts @@ -235,6 +235,7 @@ test("an error-level info event flags the current phase", () => { expect(useSessionStore.getState().phaseError).toBe("F4"); // the message still lands in stepMessages expect(useSessionStore.getState().stepMessages.at(-1)).toEqual({ level: "error", text: "boom" }); + expect(useSessionStore.getState().toasts.at(-1)).toEqual({ level: "error", text: "boom" }); }); test("a non-error info event does not set phaseError", () => { diff --git a/frontend/src/store/sessionStore.ts b/frontend/src/store/sessionStore.ts index d2fcbe1c..926bff99 100644 --- a/frontend/src/store/sessionStore.ts +++ b/frontend/src/store/sessionStore.ts @@ -167,7 +167,12 @@ export const useSessionStore = create((set) => ({ ]; // An error during the active phase marks that phase red until the next gate. return e.level === "error" - ? { stepMessages, activityLog, phaseError: st.currentPhase } + ? { + stepMessages, + activityLog, + phaseError: st.currentPhase, + toasts: [...st.toasts, { level: "error", text: e.text }], + } : { stepMessages, activityLog }; } if (e.type === "system_event") {