Skip to content

Commit 57b1052

Browse files
authored
fix(web): thread error banner dismiss survives reconnect and rerenders (#6123)
1 parent 1e355a2 commit 57b1052

3 files changed

Lines changed: 127 additions & 5 deletions

File tree

apps/web/src/components/ChatView.tsx

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,13 @@ import {
264264
ProviderStatusBanner,
265265
shouldShowProviderStatusBanner,
266266
} from "./chat/ProviderStatusBanner";
267-
import { ThreadErrorBanner } from "./chat/ThreadErrorBanner";
267+
import {
268+
dismissThreadErrorBannerForSession,
269+
getThreadErrorBannerKey,
270+
isThreadErrorBannerDismissedForSession,
271+
shouldShowThreadErrorBanner,
272+
ThreadErrorBanner,
273+
} from "./chat/ThreadErrorBanner";
268274
import { resolveThreadPr } from "./ThreadStatusIndicators";
269275
import { ComposerBannerStack, type ComposerBannerStackItem } from "./chat/ComposerBannerStack";
270276
import { ThreadSyncStatusPill } from "./chat/ThreadSyncStatusPill";
@@ -1493,6 +1499,24 @@ function ChatViewContent(props: ChatViewProps) {
14931499
const threadError = isServerThread
14941500
? (localServerError ?? activeServerThread?.session?.lastError ?? null)
14951501
: localDraftError;
1502+
// Dismissals can only mask the shown error, never clear it: a server thread
1503+
// keeps its error in session.lastError, so clearing the local shadow would
1504+
// just fall through to the persisted one. Mask the current error until a
1505+
// different error arrives, mirroring the provider status banner.
1506+
const threadErrorBannerKey = getThreadErrorBannerKey(routeThreadKey, threadError);
1507+
const visibleThreadError = shouldShowThreadErrorBanner(
1508+
routeThreadKey,
1509+
threadError,
1510+
isThreadErrorBannerDismissedForSession(threadErrorBannerKey),
1511+
)
1512+
? threadError
1513+
: null;
1514+
// Dismissing only mutates the session-scoped mask set, which does not
1515+
// trigger a render on its own; setThreadError(null) can also bail when the
1516+
// local shadow is already empty and the banner is driven purely by
1517+
// session.lastError. Bump a tick so the banner hides immediately. Mirrors
1518+
// the branch mismatch banner.
1519+
const [, setThreadErrorBannerDismissTick] = useState(0);
14961520
const runtimeMode = composerRuntimeMode ?? activeThread?.runtimeMode ?? DEFAULT_RUNTIME_MODE;
14971521
// Plan mode is legacy (Settings → Beta). With the flag off the effective
14981522
// mode is forced to "default" — even for threads with a stored plan mode —
@@ -2618,7 +2642,7 @@ function ChatViewContent(props: ChatViewProps) {
26182642
)
26192643
? activeProviderStatus
26202644
: null;
2621-
const hasTimelineTopBanner = Boolean(threadError) || visibleProviderStatus !== null;
2645+
const hasTimelineTopBanner = Boolean(visibleThreadError) || visibleProviderStatus !== null;
26222646
const activeProjectCwd = activeProject?.workspaceRoot ?? null;
26232647
const activeThreadWorktreePath = activeThread?.worktreePath ?? null;
26242648
const activeWorkspaceRoot = activeThreadWorktreePath ?? activeProjectCwd ?? undefined;
@@ -6146,8 +6170,12 @@ function ChatViewContent(props: ChatViewProps) {
61466170
</header>
61476171

61486172
<ThreadErrorBanner
6149-
error={threadError}
6150-
onDismiss={() => setThreadError(activeThread.id, null)}
6173+
error={visibleThreadError}
6174+
onDismiss={() => {
6175+
setThreadError(activeThread.id, null);
6176+
dismissThreadErrorBannerForSession(threadErrorBannerKey);
6177+
setThreadErrorBannerDismissTick((tick) => tick + 1);
6178+
}}
61516179
/>
61526180
{/* Main content area with optional plan sidebar */}
61536181
<div className="flex min-h-0 min-w-0 flex-1">

apps/web/src/components/chat/ThreadErrorBanner.test.tsx

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,74 @@
11
import { renderToStaticMarkup } from "react-dom/server";
22
import { describe, expect, it } from "vite-plus/test";
33

4-
import { ThreadErrorBanner } from "./ThreadErrorBanner";
4+
import {
5+
dismissThreadErrorBannerForSession,
6+
getThreadErrorBannerKey,
7+
isThreadErrorBannerDismissedForSession,
8+
shouldShowThreadErrorBanner,
9+
ThreadErrorBanner,
10+
} from "./ThreadErrorBanner";
511

612
describe("ThreadErrorBanner", () => {
13+
it("stays hidden after its current error is dismissed", () => {
14+
const bannerKey = getThreadErrorBannerKey("env:thread-a", "Aborted");
15+
dismissThreadErrorBannerForSession(bannerKey);
16+
17+
expect(
18+
shouldShowThreadErrorBanner(
19+
"env:thread-a",
20+
"Aborted",
21+
isThreadErrorBannerDismissedForSession(bannerKey),
22+
),
23+
).toBe(false);
24+
});
25+
26+
it("reappears when a new error arrives on the same thread", () => {
27+
dismissThreadErrorBannerForSession(getThreadErrorBannerKey("env:thread-b", "Turn failed"));
28+
const newErrorKey = getThreadErrorBannerKey("env:thread-b", "Provider crashed");
29+
30+
expect(isThreadErrorBannerDismissedForSession(newErrorKey)).toBe(false);
31+
expect(
32+
shouldShowThreadErrorBanner(
33+
"env:thread-b",
34+
"Provider crashed",
35+
isThreadErrorBannerDismissedForSession(newErrorKey),
36+
),
37+
).toBe(true);
38+
});
39+
40+
it("scopes dismissals to the thread that dismissed them", () => {
41+
dismissThreadErrorBannerForSession(getThreadErrorBannerKey("env:thread-c", "Aborted"));
42+
const otherThreadKey = getThreadErrorBannerKey("env:other-thread", "Aborted");
43+
44+
expect(isThreadErrorBannerDismissedForSession(otherThreadKey)).toBe(false);
45+
expect(
46+
shouldShowThreadErrorBanner(
47+
"env:other-thread",
48+
"Aborted",
49+
isThreadErrorBannerDismissedForSession(otherThreadKey),
50+
),
51+
).toBe(true);
52+
});
53+
54+
it("keeps a dismissal across visiting threads with no error", () => {
55+
const bannerKey = getThreadErrorBannerKey("env:thread-d", "Aborted");
56+
dismissThreadErrorBannerForSession(bannerKey);
57+
58+
expect(shouldShowThreadErrorBanner("env:thread-d", null, false)).toBe(false);
59+
expect(isThreadErrorBannerDismissedForSession(bannerKey)).toBe(true);
60+
expect(
61+
shouldShowThreadErrorBanner(
62+
"env:thread-d",
63+
"Aborted",
64+
isThreadErrorBannerDismissedForSession(bannerKey),
65+
),
66+
).toBe(false);
67+
});
68+
69+
it("never shows a null error", () => {
70+
expect(shouldShowThreadErrorBanner("env:thread-e", null, false)).toBe(false);
71+
});
772
it("aligns the warning and dismiss icons with the first line of a multi-line error", () => {
873
const markup = renderToStaticMarkup(
974
<ThreadErrorBanner

apps/web/src/components/chat/ThreadErrorBanner.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,35 @@ import { Button } from "../ui/button";
44
import { CircleAlertIcon, XIcon } from "lucide-react";
55
import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
66

7+
export function getThreadErrorBannerKey(threadKey: string, error: string | null): string | null {
8+
return error === null ? null : `${threadKey}\u0000${error}`;
9+
}
10+
11+
export function shouldShowThreadErrorBanner(
12+
threadKey: string,
13+
error: string | null,
14+
isDismissed: boolean,
15+
): boolean {
16+
return getThreadErrorBannerKey(threadKey, error) !== null && !isDismissed;
17+
}
18+
19+
// Session-scoped (module-level so it survives ChatView remounts, e.g. route
20+
// changes between threads). Mirrors the branch-mismatch banner: a dismissal
21+
// is remembered per thread key plus message, so navigating away to a thread
22+
// with no error cannot resurrect the banner, while a different error message
23+
// on the same thread still appears.
24+
const sessionDismissedThreadErrorBannerKeys = new Set<string>();
25+
26+
export function dismissThreadErrorBannerForSession(bannerKey: string | null): void {
27+
if (bannerKey !== null) {
28+
sessionDismissedThreadErrorBannerKeys.add(bannerKey);
29+
}
30+
}
31+
32+
export function isThreadErrorBannerDismissedForSession(bannerKey: string | null): boolean {
33+
return bannerKey !== null && sessionDismissedThreadErrorBannerKeys.has(bannerKey);
34+
}
35+
736
export const ThreadErrorBanner = memo(function ThreadErrorBanner({
837
error,
938
onDismiss,

0 commit comments

Comments
 (0)