authorgravatar for 56601613+myacoub91@users.noreply.github.comMina Yacoub <56601613+myacoub91@users.noreply.github.com> 2026-08-11 18:27:55+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-08-11 12:27:55-04:00
log57b1052679da56229dba45e3505718e5f5991b32
tree609a668299486dbc80a8a0528836bd5398672ad3
parent1e355a2a3444d4de0c9cf68cbe31ade370afd598
signature Signed by PGP key B5690EEEBB952194

fix(web): thread error banner dismiss survives reconnect and rerenders (#6123)

3 files changed, 127 insertions(+), 5 deletions(-)

apps/web/src/components/ChatView.tsx+32-4
...@@ -264,7 +264,13 @@ import {...@@ -264,7 +264,13 @@ import {
264 ProviderStatusBanner,264 ProviderStatusBanner,
265 shouldShowProviderStatusBanner,265 shouldShowProviderStatusBanner,
266} from "./chat/ProviderStatusBanner";266} from "./chat/ProviderStatusBanner";
267import { ThreadErrorBanner } from "./chat/ThreadErrorBanner";267import {
268 dismissThreadErrorBannerForSession,
269 getThreadErrorBannerKey,
270 isThreadErrorBannerDismissedForSession,
271 shouldShowThreadErrorBanner,
272 ThreadErrorBanner,
273} from "./chat/ThreadErrorBanner";
268import { resolveThreadPr } from "./ThreadStatusIndicators";274import { resolveThreadPr } from "./ThreadStatusIndicators";
269import { ComposerBannerStack, type ComposerBannerStackItem } from "./chat/ComposerBannerStack";275import { ComposerBannerStack, type ComposerBannerStackItem } from "./chat/ComposerBannerStack";
270import { ThreadSyncStatusPill } from "./chat/ThreadSyncStatusPill";276import { ThreadSyncStatusPill } from "./chat/ThreadSyncStatusPill";
...@@ -1493,6 +1499,24 @@ function ChatViewContent(props: ChatViewProps) {...@@ -1493,6 +1499,24 @@ function ChatViewContent(props: ChatViewProps) {
1493 const threadError = isServerThread1499 const threadError = isServerThread
1494 ? (localServerError ?? activeServerThread?.session?.lastError ?? null)1500 ? (localServerError ?? activeServerThread?.session?.lastError ?? null)
1495 : localDraftError;1501 : 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);
1496 const runtimeMode = composerRuntimeMode ?? activeThread?.runtimeMode ?? DEFAULT_RUNTIME_MODE;1520 const runtimeMode = composerRuntimeMode ?? activeThread?.runtimeMode ?? DEFAULT_RUNTIME_MODE;
1497 // Plan mode is legacy (Settings → Beta). With the flag off the effective1521 // Plan mode is legacy (Settings → Beta). With the flag off the effective
1498 // mode is forced to "default" — even for threads with a stored plan mode —1522 // mode is forced to "default" — even for threads with a stored plan mode —
...@@ -2618,7 +2642,7 @@ function ChatViewContent(props: ChatViewProps) {...@@ -2618,7 +2642,7 @@ function ChatViewContent(props: ChatViewProps) {
2618 )2642 )
2619 ? activeProviderStatus2643 ? activeProviderStatus
2620 : null;2644 : null;
2621 const hasTimelineTopBanner = Boolean(threadError) || visibleProviderStatus !== null;2645 const hasTimelineTopBanner = Boolean(visibleThreadError) || visibleProviderStatus !== null;
2622 const activeProjectCwd = activeProject?.workspaceRoot ?? null;2646 const activeProjectCwd = activeProject?.workspaceRoot ?? null;
2623 const activeThreadWorktreePath = activeThread?.worktreePath ?? null;2647 const activeThreadWorktreePath = activeThread?.worktreePath ?? null;
2624 const activeWorkspaceRoot = activeThreadWorktreePath ?? activeProjectCwd ?? undefined;2648 const activeWorkspaceRoot = activeThreadWorktreePath ?? activeProjectCwd ?? undefined;
...@@ -6146,8 +6170,12 @@ function ChatViewContent(props: ChatViewProps) {...@@ -6146,8 +6170,12 @@ function ChatViewContent(props: ChatViewProps) {
6146 </header>6170 </header>
61476171
6148 <ThreadErrorBanner6172 <ThreadErrorBanner
6149 error={threadError}6173 error={visibleThreadError}
6150 onDismiss={() => setThreadError(activeThread.id, null)}6174 onDismiss={() => {
6175 setThreadError(activeThread.id, null);
6176 dismissThreadErrorBannerForSession(threadErrorBannerKey);
6177 setThreadErrorBannerDismissTick((tick) => tick + 1);
6178 }}
6151 />6179 />
6152 {/* Main content area with optional plan sidebar */}6180 {/* Main content area with optional plan sidebar */}
6153 <div className="flex min-h-0 min-w-0 flex-1">6181 <div className="flex min-h-0 min-w-0 flex-1">
apps/web/src/components/chat/ThreadErrorBanner.test.tsx+66-1
...@@ -1,9 +1,74 @@...@@ -1,9 +1,74 @@
1import { renderToStaticMarkup } from "react-dom/server";1import { renderToStaticMarkup } from "react-dom/server";
2import { describe, expect, it } from "vite-plus/test";2import { describe, expect, it } from "vite-plus/test";
33
4import { ThreadErrorBanner } from "./ThreadErrorBanner";4import {
5 dismissThreadErrorBannerForSession,
6 getThreadErrorBannerKey,
7 isThreadErrorBannerDismissedForSession,
8 shouldShowThreadErrorBanner,
9 ThreadErrorBanner,
10} from "./ThreadErrorBanner";
511
6describe("ThreadErrorBanner", () => {12describe("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 });
7 it("aligns the warning and dismiss icons with the first line of a multi-line error", () => {72 it("aligns the warning and dismiss icons with the first line of a multi-line error", () => {
8 const markup = renderToStaticMarkup(73 const markup = renderToStaticMarkup(
9 <ThreadErrorBanner74 <ThreadErrorBanner
apps/web/src/components/chat/ThreadErrorBanner.tsx+29
...@@ -4,6 +4,35 @@ import { Button } from "../ui/button";...@@ -4,6 +4,35 @@ import { Button } from "../ui/button";
4import { CircleAlertIcon, XIcon } from "lucide-react";4import { CircleAlertIcon, XIcon } from "lucide-react";
5import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";5import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
66
7export function getThreadErrorBannerKey(threadKey: string, error: string | null): string | null {
8 return error === null ? null : `${threadKey}\u0000${error}`;
9}
10
11export 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.
24const sessionDismissedThreadErrorBannerKeys = new Set<string>();
25
26export function dismissThreadErrorBannerForSession(bannerKey: string | null): void {
27 if (bannerKey !== null) {
28 sessionDismissedThreadErrorBannerKeys.add(bannerKey);
29 }
30}
31
32export function isThreadErrorBannerDismissedForSession(bannerKey: string | null): boolean {
33 return bannerKey !== null && sessionDismissedThreadErrorBannerKeys.has(bannerKey);
34}
35
7export const ThreadErrorBanner = memo(function ThreadErrorBanner({36export const ThreadErrorBanner = memo(function ThreadErrorBanner({
8 error,37 error,
9 onDismiss,38 onDismiss,