fix: surface reviewer response failures
This commit is contained in:
@@ -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") {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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(<App />);
|
||||
|
||||
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();
|
||||
});
|
||||
@@ -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
|
||||
|
||||
@@ -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<void>((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(<WidgetHost sessionId="s1" />);
|
||||
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();
|
||||
});
|
||||
@@ -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 (
|
||||
<ErrorBoundary resetKeys={[pending.id]} label="step">
|
||||
<Renderer descriptor={pending} onRespond={onRespond} sessionId={sessionId ?? undefined} />
|
||||
<div className="space-y-2">
|
||||
<fieldset disabled={responding} className="min-w-0 border-0 p-0">
|
||||
<Renderer descriptor={pending} onRespond={onRespond} sessionId={sessionId ?? undefined} />
|
||||
</fieldset>
|
||||
{responding && (
|
||||
<p role="status" aria-live="polite" className="text-sm text-muted-foreground">
|
||||
Sending response…
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</ErrorBoundary>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -167,7 +167,12 @@ export const useSessionStore = create<SessionState>((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") {
|
||||
|
||||
Reference in New Issue
Block a user