🤖 refactor: right-sidebar panes and dialogs keep shown data while reloading

src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx

@@ -1,14 +1,16 @@
-import React, { useEffect, useState } from "react";
+import React, { useEffect, useRef, useState } from "react";
 import { CopyButton } from "../CopyButton/CopyButton";
 import { Dialog, DialogContent, DialogHeader, DialogTitle } from "../Dialog/Dialog";
 import { DetailContent } from "@/browser/features/Tools/Shared/ToolPrimitives";
 import { useAPI } from "@/browser/contexts/API";
 import { useBackgroundProcesses } from "@/browser/stores/BackgroundBashStore";
+import { isConnectionLostError } from "@/browser/contexts/connectionLink";
 import {
   appendLiveBashOutputChunk,
   type LiveBashOutputInternal,
 } from "@/browser/utils/messages/liveBashOutputBuffer";
 import { BASH_TRUNCATE_MAX_TOTAL_BYTES } from "@/common/constants/toolLimits";
+import { getErrorMessage } from "@/common/utils/errors";
 
 const BACKGROUND_BASH_INITIAL_TAIL_BYTES = 64_000;
 const BACKGROUND_BASH_POLL_INTERVAL_MS = 500;
@@ -47,11 +49,25 @@ export const BackgroundBashOutputDialog: React.FC<BackgroundBashOutputDialogProp
         )}
       </DialogHeader>
 
-      <BackgroundBashOutputViewer workspaceId={props.workspaceId} processId={props.processId} />
+      {/* Keyed by the process, so another process starts from an empty viewer, while a new API
+          client (a reconnect) only re-runs the viewer's read and keeps the output it shows. */}
+      <BackgroundBashOutputViewer
+        key={`${props.workspaceId}:${props.processId}`}
+        workspaceId={props.workspaceId}
+        processId={props.processId}
+      />
     </DialogContent>
   </Dialog>
 );
 
+/** The output pane: "Loading…" until the first read, then the output or a placeholder. */
+function outputPaneText(state: { isLoading: boolean; text: string; hasError: boolean }): string {
+  if (state.isLoading) return "Loading…";
+  if (state.text.length > 0) return state.text;
+  // The error shows above the pane; an empty pane under it reads better than "No output yet".
+  return state.hasError ? "" : "No output yet";
+}
+
 const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: string }> = (
   props
 ) => {
@@ -69,19 +85,24 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str
   const [truncatedStart, setTruncatedStart] = useState(false);
   const [error, setError] = useState<string | null>(null);
   const [isLoading, setIsLoading] = useState(true);
+  // True once a read landed: the viewer then shows output (or "No output yet"), not "Loading…".
+  const hasReadRef = useRef(false);
 
   useEffect(() => {
-    setOutput(undefined);
-    setStatus("running");
-    setTruncatedStart(false);
+    // No reset of the shown output here: the first read below replaces it.
     setError(null);
-    setIsLoading(true);
 
     if (!api) {
+      // Nothing can load without a client: a fresh viewer shows only the error, not a "Loading…"
+      // that never ends. A viewer that already read keeps its output.
       setIsLoading(false);
       setError("API unavailable");
       return;
     }
+    // Until the first read lands, the viewer shows "Loading…" (also after an offline start).
+    if (!hasReadRef.current) {
+      setIsLoading(true);
+    }
 
     let cancelled = false;
 
@@ -109,8 +130,12 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str
         }
 
         setStatus(result.data.status);
-        if (result.data.truncatedStart) {
-          setTruncatedStart(true);
+        // The first read is a fresh tail (the only read that can be truncated at its start), so
+        // it replaces any output kept from an earlier run (a reconnect); later reads append.
+        if (offset === undefined) {
+          setTruncatedStart(result.data.truncatedStart);
+          setOutput(undefined);
+          hasReadRef.current = true;
         }
 
         offset = result.data.nextOffset;
@@ -133,12 +158,18 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str
       }
     };
 
-    // A rejected read (e.g. a lost server connection) stops polling with the error shown instead
-    // of escaping as an unhandled rejection; reopening the dialog starts a fresh read.
+    // A rejected read stops polling with the error shown instead of escaping as an unhandled
+    // rejection; reopening the dialog starts a fresh read. Only a lost connection
+    // (ConnectionLostError) shows no error and keeps the output: the next client polls again. A
+    // server cancel on a live socket is a plain AbortError, and no new client would ever re-run
+    // this poll, so it shows its error.
     run().catch((err: unknown) => {
       if (cancelled) return;
+      // A viewer that already read keeps its output for the next client. A fresh one (its first
+      // read cut off) shows the error, not a "Loading…" with nothing behind it.
+      if (isConnectionLostError(err) && hasReadRef.current) return;
       setIsLoading(false);
-      setError(err instanceof Error ? err.message : String(err));
+      setError(getErrorMessage(err));
     });
 
     return () => {
@@ -184,7 +215,7 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str
           min-h keeps a visible pane for "No output yet" while still letting very
           short windows shrink it instead of clipping it at the dialog edge. */}
       <DetailContent className="max-h-none min-h-24 flex-1 px-2 py-1.5">
-        {isLoading ? "Loading…" : text.length > 0 ? text : error ? "" : "No output yet"}
+        {outputPaneText({ isLoading, text, hasError: error != null })}
       </DetailContent>
     </div>
   );

src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx

@@ -2,7 +2,7 @@ import { useCallback, useEffect, useRef, useState } from "react";
 import { ArrowUpRight, BookOpen } from "lucide-react";
 
 import { ChatInputDecoration } from "@/browser/components/ChatPane/ChatInputDecoration";
-import { useAPI } from "@/browser/contexts/API";
+import { useAPI, useLiveAPI } from "@/browser/contexts/API";
 import {
   type AdditionalSystemContextSnapshot,
   getAdditionalSystemContextFocusGeneration,
@@ -14,6 +14,7 @@ import {
   useAdditionalSystemContextSnapshot,
 } from "@/browser/utils/additionalSystemContextStore";
 import { focusInstructionsTab } from "@/browser/utils/instructionsTabFocus";
+import { isConnectionLostError } from "@/browser/contexts/connectionLink";
 import { cn } from "@/common/lib/utils";
 import { getErrorMessage } from "@/common/utils/errors";
 
@@ -40,77 +41,117 @@ interface ScratchpadState {
   content: string;
   enabled: boolean;
   loading: boolean;
+  /**
+   * Editing is off while the text loads, and while there is no live connection to save to (the
+   * editor keeps showing the text).
+   */
+  disabled: boolean;
   /** True once the save has been in flight longer than the indicator delay. */
   savingVisible: boolean;
   error: string | null;
   setContent: (content: string) => void;
   setEnabled: (enabled: boolean) => void;
 }
 
-export function useChatInstructions(workspaceId: string): ScratchpadState {
+/**
+ * Loads the workspace's instructions into the store; returns whether the first load is pending.
+ *
+ * A re-run for the shown workspace (a new API client after a reconnect) keeps the editor as it
+ * is: it does not flash "Loading…" or take focus from the textarea. A local edit the backend has
+ * not settled (`dirtyRef`) is not replaced by the load.
+ */
+function useChatInstructionsLoad(
+  workspaceId: string,
+  dirtyRef: { current: boolean },
+  setError: (error: string | null) => void
+): boolean {
   const { api } = useAPI();
-  const snapshot = useAdditionalSystemContextSnapshot(workspaceId);
   const [loading, setLoading] = useState(true);
-  const [savingVisible, setSavingVisible] = useState(false);
-  const [error, setError] = useState<string | null>(null);
-  const dirtyRef = useRef(false);
-  const mountedRef = useRef(true);
-  const savingTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
-
-  const clearSavingTimer = () => {
-    if (savingTimerRef.current != null) {
-      clearTimeout(savingTimerRef.current);
-      savingTimerRef.current = null;
-    }
-  };
-
-  useEffect(() => {
-    mountedRef.current = true;
-    return () => {
-      mountedRef.current = false;
-      clearSavingTimer();
-    };
-  }, []);
+  // The workspace whose instructions the editor shows (loaded, or failed to load).
+  const loadedWorkspaceIdRef = useRef<string | null>(null);
 
   useEffect(() => {
-    dirtyRef.current = false;
-    setLoading(true);
-    setSavingVisible(false);
-    clearSavingTimer();
+    if (loadedWorkspaceIdRef.current !== workspaceId) {
+      loadedWorkspaceIdRef.current = null;
+      dirtyRef.current = false;
+      setLoading(true);
+    }
     setError(null);
 
     if (!api) return;
 
     const loadVersion = getAdditionalSystemContextVersion(workspaceId);
+    // Set by the cleanup, which also runs on unmount.
     let cancelled = false;
     api.workspace
       .getAdditionalSystemContext({ workspaceId })
       .then((result) => {
-        if (cancelled || !mountedRef.current) return;
+        if (cancelled) return;
         if (!dirtyRef.current && getAdditionalSystemContextVersion(workspaceId) === loadVersion) {
           updateAdditionalSystemContextSnapshot(workspaceId, {
             content: result.content,
             enabled: result.enabled,
           });
         }
         setLoading(false);
+        loadedWorkspaceIdRef.current = workspaceId;
       })
       .catch((err) => {
-        if (cancelled || !mountedRef.current) return;
+        // A lost connection is not a load error: keep the editor as it is for the next client.
+        if (cancelled || isConnectionLostError(err)) return;
         setError(getErrorMessage(err));
         setLoading(false);
+        loadedWorkspaceIdRef.current = workspaceId;
       });
 
     return () => {
       cancelled = true;
     };
-  }, [api, workspaceId]);
+  }, [api, workspaceId, dirtyRef, setError]);
+
+  return loading;
+}
+
+export function useChatInstructions(workspaceId: string): ScratchpadState {
+  // Saves need a live connection. While Xum reconnects the editor keeps showing the text but is
+  // read-only, so no edit waits for a later client.
+  const liveApi = useLiveAPI();
+  const snapshot = useAdditionalSystemContextSnapshot(workspaceId);
+  const [savingVisible, setSavingVisible] = useState(false);
+  const [error, setError] = useState<string | null>(null);
+  // An edit the backend has not settled yet: a load must not replace it. A failed save settles
+  // it too, so a later reload shows what the backend stored.
+  const dirtyRef = useRef(false);
+  const mountedRef = useRef(true);
+  const savingTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const loading = useChatInstructionsLoad(workspaceId, dirtyRef, setError);
+
+  const clearSavingTimer = () => {
+    if (savingTimerRef.current != null) {
+      clearTimeout(savingTimerRef.current);
+      savingTimerRef.current = null;
+    }
+  };
+
+  useEffect(() => {
+    mountedRef.current = true;
+    return () => {
+      mountedRef.current = false;
+      clearSavingTimer();
+    };
+  }, []);
+
+  // Another workspace starts without the previous one's "Saving…".
+  useEffect(() => {
+    setSavingVisible(false);
+    clearSavingTimer();
+  }, [workspaceId]);
 
   const queue = useCallback(
     (next: AdditionalSystemContextSnapshot) => {
+      if (!liveApi) return;
       dirtyRef.current = true;
       updateAdditionalSystemContextSnapshot(workspaceId, next);
-      if (!api) return;
       setError(null);
       // Only show the "Saving…" message if the save takes more than a beat —
       // sub-second writes shouldn't flicker the UI on every keystroke.
@@ -119,19 +160,20 @@ export function useChatInstructions(workspaceId: string): ScratchpadState {
         if (mountedRef.current) setSavingVisible(true);
         savingTimerRef.current = null;
       }, SAVING_INDICATOR_DELAY_MS);
-      queueAdditionalSystemContextSave(api, workspaceId, next, {
+      queueAdditionalSystemContextSave(liveApi, workspaceId, next, {
         onError: (err) => {
           if (!mountedRef.current) return;
           setError(getErrorMessage(err));
         },
         onIdle: () => {
+          dirtyRef.current = false;
           if (!mountedRef.current) return;
           clearSavingTimer();
           setSavingVisible(false);
         },
       });
     },
-    [api, workspaceId]
+    [liveApi, workspaceId]
   );
 
   const setContent = useCallback(
@@ -152,6 +194,7 @@ export function useChatInstructions(workspaceId: string): ScratchpadState {
     content: snapshot.content,
     enabled: snapshot.enabled,
     loading,
+    disabled: loading || liveApi == null,
     savingVisible,
     error,
     setContent,
@@ -229,15 +272,15 @@ export function ChatInstructionsEditor(props: ChatInstructionsEditorProps) {
           props.textareaClassName
         )}
         aria-label={`${CHAT_INSTRUCTIONS_LABEL} editor`}
-        disabled={state.loading}
+        disabled={state.disabled}
       />
       <div className="text-muted flex min-h-4 items-center justify-between gap-2 text-[10px]">
         <label className="flex cursor-pointer items-center gap-1.5 select-none">
           <input
             type="checkbox"
             className="accent-[var(--color-accent)]"
             checked={state.enabled}
-            disabled={state.loading}
+            disabled={state.disabled}
             onChange={(event) => state.setEnabled(event.currentTarget.checked)}
             aria-label="Enable Chat Instructions"
           />

src/browser/components/InstructionsTab/InstructionsTab.tsx

@@ -80,7 +80,9 @@ function InstructionsTabImpl(props: InstructionsTabProps) {
       <div className="min-h-0 flex-1 overflow-y-auto">
         <ChatInstructionsPanel workspaceId={props.workspaceId} />
         {error && <ErrorBanner message={error} />}
-        {!error && !loading && data?.files.length === 0 && <EmptyState />}
+        {/* Not gated on `loading`: a reload (refresh, or a new API client after a reconnect)
+            keeps the empty state on screen, as it keeps a file list. */}
+        {!error && data?.files.length === 0 && <EmptyState />}
         {data && data.files.length > 0 && <InstructionsBody data={data} />}
         {loading && !data && <LoadingState />}
       </div>