Skip to content

Commit 6724acb

Browse files
authored
Fix error notification dismissal regressions (#490)
1 parent 3345ee5 commit 6724acb

2 files changed

Lines changed: 76 additions & 3 deletions

File tree

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,73 @@ describe("ErrorNotificationBar", () => {
8585
expect(markup).toContain("Worktree thread could not start");
8686
expect(markup).toContain("Base branch 'main' does not resolve to a commit yet.");
8787
});
88+
89+
it("re-shows thread errors when the message changes after dismissal", async () => {
90+
const onDismissThreadError = vi.fn();
91+
let renderer: ReactTestRenderer | null = null;
92+
93+
await act(async () => {
94+
renderer = create(
95+
<ErrorNotificationBar
96+
threadError={THREAD_ERROR}
97+
showAuthFailuresAsErrors
98+
showNotificationDetails={false}
99+
includeDiagnosticsTipsInCopy={false}
100+
providerStatus={null}
101+
isMobileCompanion={false}
102+
onDismissThreadError={onDismissThreadError}
103+
/>,
104+
);
105+
});
106+
107+
const dismissAll = renderer!.root.findByProps({ "aria-label": "Dismiss notifications" });
108+
await act(async () => {
109+
dismissAll.props.onClick();
110+
});
111+
112+
expect(renderer!.toJSON()).toBeNull();
113+
114+
await act(async () => {
115+
renderer!.update(
116+
<ErrorNotificationBar
117+
threadError="Codex CLI is not authenticated. Run `codex login` and try again."
118+
showAuthFailuresAsErrors
119+
showNotificationDetails={false}
120+
includeDiagnosticsTipsInCopy={false}
121+
providerStatus={null}
122+
isMobileCompanion={false}
123+
onDismissThreadError={onDismissThreadError}
124+
/>,
125+
);
126+
});
127+
128+
expect(renderer!.toJSON()).not.toBeNull();
129+
expect(renderer!.root.findByProps({ "aria-label": "Show 1 notification" })).toBeTruthy();
130+
});
131+
132+
it("does not hide non-dismissible provider notifications via dismiss all", async () => {
133+
let renderer: ReactTestRenderer | null = null;
134+
135+
await act(async () => {
136+
renderer = create(
137+
<ErrorNotificationBar
138+
threadError={null}
139+
showAuthFailuresAsErrors
140+
showNotificationDetails={false}
141+
includeDiagnosticsTipsInCopy={false}
142+
providerStatus={makeProviderStatus()}
143+
isMobileCompanion={false}
144+
/>,
145+
);
146+
});
147+
148+
const dismissAll = renderer!.root.findByProps({ "aria-label": "Dismiss notifications" });
149+
await act(async () => {
150+
dismissAll.props.onClick();
151+
});
152+
153+
expect(renderer!.toJSON()).not.toBeNull();
154+
expect(renderer!.root.findByProps({ "aria-label": "Show 1 notification" })).toBeTruthy();
155+
expect(JSON.stringify(renderer!.toJSON())).toContain("OpenAI (Codex CLI) needs verification");
156+
});
88157
});

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ interface NotificationItem {
5454
onDismiss?: () => void;
5555
}
5656

57+
function buildThreadErrorNotificationId(error: string): string {
58+
return `thread-error:${error}`;
59+
}
60+
5761
export const ErrorNotificationBar = memo(function ErrorNotificationBar({
5862
threadError,
5963
showAuthFailuresAsErrors = true,
@@ -130,7 +134,7 @@ export const ErrorNotificationBar = memo(function ErrorNotificationBar({
130134
if (showAuthFailuresAsErrors || !isAuthenticationThreadError(threadError)) {
131135
const presentation = humanizeThreadError(threadError);
132136
items.push({
133-
id: "thread-error",
137+
id: buildThreadErrorNotificationId(threadError),
134138
kind: "thread-error",
135139
icon: CircleAlertIcon,
136140
title: presentation.title ?? "Error",
@@ -210,8 +214,8 @@ export const ErrorNotificationBar = memo(function ErrorNotificationBar({
210214
notif.onDismiss();
211215
}
212216
}
213-
setDismissedIds(new Set(notifications.map((n) => n.id)));
214-
}, [visibleNotifications, notifications]);
217+
setDismissedIds(new Set(visibleNotifications.filter((n) => n.dismissible).map((n) => n.id)));
218+
}, [visibleNotifications]);
215219

216220
// Nothing to show
217221
if (visibleNotifications.length === 0) return null;

0 commit comments

Comments
 (0)