Merge pull request #7268 from senamakel/ci-e2e-green-followup

fix: stabilize onboarding and thread selection e2e

Author: senamakel

app/src/components/InitProgressScreen/InitProgressScreen.test.tsx

@@ -65,6 +65,8 @@ describe('InitProgressScreen', () => {
       <InitProgressScreen snapshot={snapshot()} onRetry={vi.fn()} onContinue={onContinue} />
     );
 
+    expect(screen.getByTestId('harness-init-background')).toBeInTheDocument();
+    expect(screen.queryByTestId('harness-init-continue-anyway')).not.toBeInTheDocument();
     fireEvent.click(screen.getByText('Run in background'));
     expect(onContinue).toHaveBeenCalledTimes(1);
   });
@@ -92,6 +94,8 @@ describe('InitProgressScreen', () => {
     );
 
     expect(screen.getByText('pip install timed out')).toBeInTheDocument();
+    expect(screen.getByTestId('harness-init-continue-anyway')).toBeInTheDocument();
+    expect(screen.queryByTestId('harness-init-background')).not.toBeInTheDocument();
 
     fireEvent.click(screen.getByText('Retry'));
     expect(onRetry).toHaveBeenCalledTimes(1);

app/src/components/InitProgressScreen/InitProgressScreen.tsx

@@ -82,6 +82,7 @@ export default function InitProgressScreen({
     <div className="fixed inset-0 z-9999 flex items-center justify-center bg-stone-950/90 p-4 backdrop-blur-sm">
       <div
         role="dialog"
+        data-testid="harness-init-dialog"
         aria-modal="true"
         aria-labelledby="harness-init-title"
         className="w-full max-w-md rounded-2xl border border-stone-700/60 bg-stone-900 p-6 shadow-2xl">
@@ -101,6 +102,7 @@ export default function InitProgressScreen({
             <p className="text-xs text-content-muted">{t('harnessInit.backgroundHint')}</p>
             <button
               type="button"
+              data-testid="harness-init-background"
               onClick={onContinue}
               className="shrink-0 rounded-lg border border-stone-700 px-3 py-1.5 text-sm text-content-faint hover:bg-stone-800 hover:text-white">
               {t('harnessInit.runInBackground')}
@@ -121,6 +123,7 @@ export default function InitProgressScreen({
             <div className="mt-4 flex justify-end gap-2">
               <button
                 type="button"
+                data-testid="harness-init-continue-anyway"
                 onClick={onContinue}
                 className="rounded-lg px-3 py-1.5 text-sm text-content-faint hover:text-white">
                 {t('harnessInit.continueAnyway')}

app/src/features/conversations/Conversations.tsx

@@ -1,5 +1,6 @@
 import debugFactory from 'debug';
 import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react';
+import { useStore } from 'react-redux';
 import { useLocation, useNavigate, useParams } from 'react-router-dom';
 
 import { type ChatSendError, chatSendError } from '../../chat/chatSendError';
@@ -59,6 +60,7 @@ import {
   useRustChat,
 } from '../../services/chatService';
 import { callCoreRpc } from '../../services/coreRpcClient';
+import type { RootState } from '../../store';
 import {
   beginInferenceTurn,
   clearRuntimeForThread,
@@ -79,6 +81,7 @@ import {
   clearThreadInferenceActive,
   createNewThread,
   deleteThread,
+  invalidateThreadSelection,
   loadThreadMessages,
   loadThreads,
   markThreadInferenceActive,
@@ -259,6 +262,7 @@ const Conversations = ({
   const composer = composerOverride ?? composerProp;
   const { t } = useT();
   const dispatch = useAppDispatch();
+  const store = useStore<RootState>();
   const navigate = useNavigate();
   const location = useLocation();
   const { threadId: routeThreadId } = useParams<{ threadId?: string }>();
@@ -347,10 +351,9 @@ const Conversations = ({
   const persistedFailedMessageByThreadRef = useRef<Map<string, ThreadMessage>>(new Map());
   const createThreadErrorRef = useRef(createThreadError);
   createThreadErrorRef.current = createThreadError;
-  // Startup thread restoration is asynchronous. If the user chooses a thread
-  // while its initial `loadThreads` is in flight, that older callback must not
-  // overwrite the newer selection when it resolves.
-  const threadSelectionIntentRef = useRef(0);
+  // The Redux selection intent is updated by every setSelectedThread action,
+  // including worker-thread cards rendered inside the transcript. Async
+  // startup/create continuations compare against it before selecting a thread.
   const displayedSendError = deriveChatErrorBanner(
     sendError,
     createThreadError,
@@ -726,11 +729,19 @@ const Conversations = ({
   const turnSignatureByThreadRef = useRef<Map<string, readonly unknown[]>>(new Map());
 
   const handleCreateNewThread = async (fromInitialLoad = false) => {
-    if (!fromInitialLoad) threadSelectionIntentRef.current += 1;
-    const selectionIntentAtCreate = threadSelectionIntentRef.current;
+    if (!fromInitialLoad) dispatch(invalidateThreadSelection());
+    const selectionIntentAtCreate = store.getState().thread.selectionIntentVersion ?? 0;
     try {
       const thread = await dispatch(createNewThread()).unwrap();
-      if (threadSelectionIntentRef.current !== selectionIntentAtCreate) return;
+      const currentSelectionIntent = store.getState().thread.selectionIntentVersion ?? 0;
+      if (currentSelectionIntent !== selectionIntentAtCreate) {
+        debug(
+          '[chat] create thread selection superseded; dropping result intent_at_create=%d current_intent=%d',
+          selectionIntentAtCreate,
+          currentSelectionIntent
+        );
+        return;
+      }
       dispatch(setSelectedThread(thread.id));
       void dispatch(loadThreadMessages(thread.id));
       if (shouldSyncChatRoute) {
@@ -790,12 +801,21 @@ const Conversations = ({
 
   useEffect(() => {
     let cancelled = false;
-    const selectionIntentAtLoad = threadSelectionIntentRef.current;
+    const selectionIntentAtLoad = store.getState().thread.selectionIntentVersion ?? 0;
 
     void dispatch(loadThreads())
       .unwrap()
       .then(data => {
-        if (cancelled || threadSelectionIntentRef.current !== selectionIntentAtLoad) return;
+        const currentSelectionIntent = store.getState().thread.selectionIntentVersion ?? 0;
+        if (cancelled || currentSelectionIntent !== selectionIntentAtLoad) {
+          debug(
+            '[chat] initial thread load selection superseded; dropping result cancelled=%s intent_at_load=%d current_intent=%d',
+            cancelled,
+            selectionIntentAtLoad,
+            currentSelectionIntent
+          );
+          return;
+        }
         // Match the sidebar's default General filter here so initial/resume
         // selection can't auto-pick a thread hidden by the selected tab.
         const visibleThreads = data.threads.filter(t => isThreadVisibleInTab(t, GENERAL_TAB_VALUE));
@@ -1889,7 +1909,6 @@ const Conversations = ({
       selectedThreadId={selectedThreadId ?? null}
       onCreateThread={() => handleCreateNewThread()}
       onSelectThread={id => {
-        threadSelectionIntentRef.current += 1;
         dispatch(setSelectedThread(id));
         void dispatch(loadThreadMessages(id));
         if (shouldSyncChatRoute) {
@@ -2257,7 +2276,6 @@ const Conversations = ({
             type="button"
             data-analytics-id="chat-header-back-to-parent-thread"
             onClick={() => {
-              threadSelectionIntentRef.current += 1;
               dispatch(setSelectedThread(selectedThreadParent.id));
               void dispatch(loadThreadMessages(selectedThreadParent.id));
               navigate(chatThreadPath(selectedThreadParent.id));

app/src/pages/__tests__/Conversations.render.test.tsx

@@ -267,6 +267,7 @@ async function openSidebar() {
 const emptyThreadState = {
   threads: [],
   selectedThreadId: null,
+  selectionIntentVersion: 0,
   activeThreadIds: {},
   welcomeThreadId: null,
   messagesByThreadId: {},
@@ -734,6 +735,43 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => {
     });
   });
 
+  it('keeps an explicit sidebar selection made while initial thread loading is pending', async () => {
+    const threads = [
+      makeThread({ id: 't-1', title: 'Initial Thread' }),
+      makeThread({ id: 't-2', title: 'Explicitly Selected Thread' }),
+    ];
+    let resolveThreads: ((value: { threads: Thread[]; count: number }) => void) | undefined;
+    mockGetThreads.mockImplementation(
+      () =>
+        new Promise(resolve => {
+          resolveThreads = resolve;
+        })
+    );
+
+    let store: ReturnType<typeof buildStore> | undefined;
+    await act(async () => {
+      store = await renderConversations({
+        thread: {
+          ...emptyThreadState,
+          threads,
+          selectedThreadId: 't-1',
+          messagesByThreadId: { 't-1': [], 't-2': [] },
+        },
+      });
+    });
+    await waitFor(() => expect(mockGetThreads).toHaveBeenCalled());
+
+    fireEvent.click(screen.getByTestId('thread-row-t-2'));
+    expect(store?.getState().thread.selectedThreadId).toBe('t-2');
+
+    await act(async () => {
+      resolveThreads?.({ threads, count: threads.length });
+    });
+
+    expect(store?.getState().thread.selectedThreadId).toBe('t-2');
+    expect(screen.getByTestId('thread-row-t-2')).toHaveClass('bg-surface/70');
+  });
+
   // Sidebar "New thread" button was removed in the composer flattening refactor.
   // The "+ New" header button (tested below) is the remaining create-thread entry point.
 

app/src/store/__tests__/threadSlice.test.ts

@@ -168,13 +168,15 @@ describe('threadSlice synchronous reducers', () => {
     store.dispatch(setSelectedThread('t-1'));
     store.dispatch(setActiveThread('t-1'));
 
+    const selectionVersion = store.getState().thread.selectionIntentVersion;
     store.dispatch(clearAllThreads());
     const state = store.getState().thread;
     expect(state.threads).toEqual([]);
     expect(state.messagesByThreadId).toEqual({});
     expect(state.selectedThreadId).toBeNull();
     expect(state.activeThreadIds).toEqual({});
     expect(state.messages).toEqual([]);
+    expect(state.selectionIntentVersion).toBeGreaterThan(selectionVersion);
   });
 
   it('clearStaleThread removes stale selection, cache, and active id', async () => {
@@ -192,12 +194,14 @@ describe('threadSlice synchronous reducers', () => {
     store.dispatch(setSelectedThread('t-1'));
     store.dispatch(setActiveThread('t-1'));
 
+    const selectionVersion = store.getState().thread.selectionIntentVersion;
     store.dispatch(clearStaleThread('t-1'));
 
     const state = store.getState().thread;
     expect(state.threads.map(thread => thread.id)).toEqual(['t-2']);
     expect(state.messagesByThreadId['t-1']).toBeUndefined();
     expect(state.selectedThreadId).toBeNull();
+    expect(state.selectionIntentVersion).toBeGreaterThan(selectionVersion);
     expect(state.activeThreadIds).toEqual({});
     expect(state.messages).toEqual([]);
   });
@@ -208,6 +212,44 @@ describe('threadSlice loadThreads thunk', () => {
     vi.clearAllMocks();
   });
 
+  it('does not restore threads from a load superseded by clearing all threads', async () => {
+    const store = createStore();
+    let resolveThreads!: (value: { threads: Thread[]; count: number }) => void;
+    mockedThreadApi.getThreads.mockImplementationOnce(
+      () => new Promise(resolve => (resolveThreads = resolve))
+    );
+
+    const request = store.dispatch(loadThreads());
+    store.dispatch(clearAllThreads());
+    resolveThreads({ threads: [makeThread({ id: 'stale' })], count: 1 });
+    await request;
+
+    expect(store.getState().thread.threads).toEqual([]);
+    expect(store.getState().thread.selectedThreadId).toBeNull();
+  });
+
+  it('does not restore a cleared stale thread from an older list response', async () => {
+    const store = createStore();
+    mockedThreadApi.getThreads.mockResolvedValueOnce({
+      threads: [makeThread({ id: 't-1' }), makeThread({ id: 't-2' })],
+      count: 2,
+    });
+    await store.dispatch(loadThreads());
+    store.dispatch(setSelectedThread('t-1'));
+
+    let resolveThreads!: (value: { threads: Thread[]; count: number }) => void;
+    mockedThreadApi.getThreads.mockImplementationOnce(
+      () => new Promise(resolve => (resolveThreads = resolve))
+    );
+    const request = store.dispatch(loadThreads());
+    store.dispatch(clearStaleThread('t-1'));
+    resolveThreads({ threads: [makeThread({ id: 't-1' }), makeThread({ id: 't-2' })], count: 2 });
+    await request;
+
+    expect(store.getState().thread.threads.map(thread => thread.id)).toEqual(['t-2']);
+    expect(store.getState().thread.selectedThreadId).toBeNull();
+  });
+
   it('sets isLoadingThreads while pending and stores threads on fulfilled', async () => {
     const store = createStore();
     const payload = { threads: [makeThread({ id: 'a' })], count: 1 };
@@ -230,6 +272,21 @@ describe('threadSlice loadThreads thunk', () => {
     expect(result.type).toBe('thread/loadThreads/rejected');
     expect(store.getState().thread.isLoadingThreads).toBe(false);
   });
+
+  it('preserves a newer selection when an older thread-list response omits it', async () => {
+    const store = createStore();
+    let resolveThreads: ((value: { threads: Thread[]; count: number }) => void) | undefined;
+    mockedThreadApi.getThreads.mockImplementationOnce(
+      () => new Promise(resolve => (resolveThreads = resolve))
+    );
+
+    const request = store.dispatch(loadThreads());
+    store.dispatch(setSelectedThread('worker-created-after-request'));
+    resolveThreads?.({ threads: [makeThread({ id: 'existing-thread' })], count: 1 });
+    await request;
+
+    expect(store.getState().thread.selectedThreadId).toBe('worker-created-after-request');
+  });
 });
 
 describe('threadSlice loadThreadMessages thunk', () => {