fix(web-shell): report each connection error once to stop the inline onError re-render loop

packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx

@@ -62,26 +62,47 @@ vi.mock('@qwen-code/sdk/daemon', () => ({
 }));
 
 vi.mock('@qwen-code/web-shell', async () => {
-  const { useEffect } = await import('react');
+  const { useEffect, useMemo, useRef, useState } = await import('react');
   return {
     WebShellWithProviders: (props: CapturedProps) => {
       mocks.embeddedProps.current = props;
       // Mirror App.tsx's error-notification effect: while a connection error
-      // persists it re-runs whenever the onError prop identity changes — an
-      // unstable callback turns that into an infinite render loop.
+      // persists, each distinct error value is reported once. Hosts may pass
+      // an onError whose identity changes on every render, which re-runs the
+      // effect without re-delivering the already-reported error.
+      const onError = props.onError as ((error: Error) => void) | undefined;
+      const lastReportedError = useRef<string | undefined>(undefined);
+      const [churn, setChurn] = useState(0);
+      // A fresh wrapper identity whenever the host's onError identity
+      // changes mirrors a host passing an inline onError; the churn state
+      // below additionally forces the effect to re-run after a delivery,
+      // like the host re-render that delivering the error triggers.
+      const unstableOnError = useMemo(
+        () => (onError ? (error: Error) => onError(error) : undefined),
+        [onError],
+      );
       useEffect(() => {
         const message = mocks.connectionError.current;
-        if (!message) return;
+        if (!message) {
+          lastReportedError.current = undefined;
+          return;
+        }
+        if (lastReportedError.current === message) return;
+        // App.tsx returns before stamping when no handler is attached, so a
+        // handler that appears later still receives the persistent error.
+        if (!unstableOnError) return;
+        lastReportedError.current = message;
         mocks.errorNotifications.current += 1;
         if (mocks.errorNotifications.current > 3) {
-          // An unstable onError re-runs this effect on every render; fail
-          // fast instead of hanging on the infinite loop.
+          // Value-dedup makes a notify loop impossible; fail fast if this
+          // mirror ever regresses instead of hanging.
           throw new Error('onError notified in a loop');
         }
-        (props.onError as ((error: Error) => void) | undefined)?.(
-          new Error(message),
-        );
-      }, [props.onError]);
+        unstableOnError(new Error(message));
+        // Delivering an error re-renders the host; force one extra effect
+        // run under a fresh callback identity to mirror that churn.
+        if (churn < 1) setChurn((count) => count + 1);
+      }, [unstableOnError, churn]);
       return null;
     },
   };
@@ -489,9 +510,10 @@ describe('EmbeddedApp host wiring', () => {
   it('notifies once when a connection error persists instead of looping', async () => {
     mocks.connectionError.current = 'daemon connection lost';
 
-    // An unstable onError re-runs the mirrored notification effect on every
-    // render; the mock trips after three notifications instead of hanging
-    // on the infinite loop.
+    // The mirrored effect re-runs under a fresh onError identity on every
+    // re-render (like a host passing an inline onError); the value-dedup
+    // must still deliver the persistent error exactly once. The mock trips
+    // after three notifications instead of hanging if that ever regresses.
     await renderApp();
     const { container } = mounted[mounted.length - 1];
 
@@ -501,6 +523,34 @@ describe('EmbeddedApp host wiring', () => {
     expect(alerts[0].textContent).toContain('daemon connection lost');
   });
 
+  it('does not report or stamp an error while no onError handler is attached', async () => {
+    mocks.connectionError.current = 'daemon connection lost';
+    const { WebShellWithProviders } = await import('@qwen-code/web-shell');
+    const WebShell =
+      WebShellWithProviders as unknown as ComponentType<CapturedProps>;
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+    const root = createRoot(container);
+    mounted.push({ container, root });
+
+    await act(async () => {
+      root.render(<WebShell />);
+      await Promise.resolve();
+    });
+    // App.tsx returns before stamping when no handler exists; the mirror must
+    // leave the error unreported and unstamped here.
+    expect(mocks.errorNotifications.current).toBe(0);
+
+    // Because nothing was stamped, a handler attached later still receives
+    // the persistent error exactly once.
+    await act(async () => {
+      root.render(<WebShell onError={() => {}} />);
+      await Promise.resolve();
+    });
+    expect(mocks.errorNotifications.current).toBe(1);
+  });
+
   it('releases the panel when a session switch times out', async () => {
     sdkMocks.listWorkspaceSessionsPage.mockResolvedValueOnce({
       sessions: [

packages/vscode-ide-companion/src/webview/EmbeddedApp.tsx

@@ -363,9 +363,9 @@ export function EmbeddedApp() {
     [],
   );
 
-  // Stable identity: Web Shell re-runs its error-notification effect
-  // whenever this callback changes, so a fresh arrow every render would
-  // re-notify — and re-render — forever while a connection error persists.
+  // Web Shell reports each distinct connection error value only once, so a
+  // new identity here (e.g. when `t` is rebuilt on a language switch) re-runs
+  // its notification effect without re-delivering a persisted error.
   const handleShellError = useCallback(
     (error: Error) => {
       clearInsight();

packages/web-shell/client/App.test.tsx

@@ -1,6 +1,12 @@
 // @vitest-environment jsdom
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { act, createRef, type CSSProperties, type ReactNode } from 'react';
+import {
+  act,
+  createRef,
+  useState,
+  type CSSProperties,
+  type ReactNode,
+} from 'react';
 import { createRoot, type Root } from 'react-dom/client';
 import {
   DaemonHttpError,
@@ -24363,3 +24369,97 @@ describe('fileUploadEnabled customization plumbing', () => {
     expect(composer?.hasAttribute('data-file-upload-directory')).toBe(false);
   });
 });
+
+describe('App connection error reporting (#10406)', () => {
+  it('reports a persistent connection error once even while host re-renders pass a fresh inline onError', async () => {
+    // Daemon unreachable: connection.error persists. A host may store each
+    // reported error in its own state, which re-renders the host and hands
+    // App an onError with a fresh identity (inline or otherwise). Before the
+    // fix, every new callback identity re-fired the notification effect for
+    // the same persistent error — an infinite re-render loop.
+    mockConnection.error = 'daemon unreachable';
+    const calls: string[] = [];
+    const HOST_NOTICE_CAP = 5;
+
+    function Host() {
+      const [noticeCount, setNoticeCount] = useState(0);
+      return (
+        <App
+          sidebar={{ enabled: true }}
+          header={{}}
+          onError={(error: Error) => {
+            calls.push(error.message);
+            if (noticeCount < HOST_NOTICE_CAP) {
+              setNoticeCount((count) => count + 1);
+            }
+          }}
+        />
+      );
+    }
+
+    const container = document.createElement('div');
+    document.body.appendChild(container);
+    const root = createRoot(container);
+    await act(async () => {
+      root.render(<Host />);
+    });
+    await flush();
+    await act(async () => {
+      root.unmount();
+    });
+    container.remove();
+
+    expect(calls).toEqual(['daemon unreachable']);
+  });
+
+  it('still reports when the connection error changes to a different value', async () => {
+    mockConnection.error = 'daemon unreachable';
+    const calls: string[] = [];
+    const { rerender } = renderApp({
+      onError: (error) => calls.push(error.message),
+    });
+    await flush();
+    expect(calls).toEqual(['daemon unreachable']);
+
+    mockConnection.error = 'session missing';
+    rerender({ onError: (error) => calls.push(error.message) });
+    await flush();
+
+    expect(calls).toEqual(['daemon unreachable', 'session missing']);
+  });
+
+  it('reports a recurring error again after the connection recovers', async () => {
+    mockConnection.error = 'daemon unreachable';
+    const calls: string[] = [];
+    const onError = (error: Error) => calls.push(error.message);
+    const { rerender } = renderApp({ onError });
+    await flush();
+    expect(calls).toEqual(['daemon unreachable']);
+
+    mockConnection.error = undefined;
+    rerender({ onError });
+    await flush();
+
+    mockConnection.error = 'daemon unreachable';
+    rerender({ onError });
+    await flush();
+
+    expect(calls).toEqual(['daemon unreachable', 'daemon unreachable']);
+  });
+
+  it('delivers a persistent error once when the host attaches onError after it appears', async () => {
+    // onError is optional: a host may mount while a connection error is
+    // already active and only attach its handler on a later render. The
+    // pending error must still be delivered exactly once.
+    mockConnection.error = 'daemon unreachable';
+    const calls: string[] = [];
+    const { rerender } = renderApp({});
+    await flush();
+    expect(calls).toEqual([]);
+
+    rerender({ onError: (error) => calls.push(error.message) });
+    await flush();
+
+    expect(calls).toEqual(['daemon unreachable']);
+  });
+});

packages/web-shell/client/App.tsx

@@ -1094,7 +1094,12 @@ export interface WebShellProps {
    * at most once per animation frame during active generation.
    */
   onTranscriptChange?: (blocks: readonly DaemonTranscriptBlock[]) => void;
-  /** Called when a critical error occurs (auth failure, session gone, etc). */
+  /**
+   * Called when a critical error occurs (auth failure, session gone, etc).
+   * Each distinct connection error value is reported once; reporting resets
+   * when the connection recovers. Replacing the handler while an error
+   * persists does not re-deliver that error.
+   */
   onError?: (error: Error) => void;
   /** Called when `/bug` is invoked. Receives system info. If omitted, web-shell opens the report URL itself. */
   onBugReport?: (info: BugReportInfo) => void;
@@ -8191,11 +8196,21 @@ export function App({
     onConnectionChange?.(connection.status);
   }, [connection.status, onConnectionChange]);
 
+  // Report each distinct connection.error value only once. Hosts may pass an
+  // inline onError (or one whose identity changes) and update their own state
+  // when it fires; the resulting re-render then hands this effect a fresh
+  // callback identity, which would re-notify the same persistent error
+  // forever (#10406).
+  const lastReportedConnectionErrorRef = useRef<string | undefined>(undefined);
   useEffect(() => {
-    if (connection.error) {
-      const error = new Error(connection.error);
-      onError?.(error);
+    if (!connection.error) {
+      lastReportedConnectionErrorRef.current = undefined;
+      return;
     }
+    if (lastReportedConnectionErrorRef.current === connection.error) return;
+    if (!onError) return;
+    lastReportedConnectionErrorRef.current = connection.error;
+    onError(new Error(connection.error));
   }, [connection.error, onError]);
 
   useLayoutEffect(() => {

packages/web-shell/client/components/MessageList.tsx

@@ -5513,6 +5513,7 @@ export const MessageList = memo(
         onInsightReportOpen,
         onEditUserMessage,
         editableUserTurn,
+        hasOlderHistory,
         generateContent,
         headerOffset,
         visibleItems,