From 23b30b72853730f1884ad472b5ce0707e120ba4d Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Mon, 27 Apr 2026 16:38:20 +0700 Subject: [PATCH] fix: remember last active sidebar tab per agent session (#24631) closes CODAGT-142 The Agents right-panel tabs (Git, Terminal, Desktop) reset to the default tab (Git) every time the user switches between agent sessions. This happens because `KeyedAgentChatPage` forces a full remount on `agentId` change, discarding the `useState` that holds the active tab. Persist the active tab per agent session in `localStorage`, following the existing `agents.draft-input.` pattern. When the user returns to a session, the last tab they selected is restored. If the stored tab is temporarily unavailable (e.g. Desktop while the workspace is stopped), `SidebarTabView`'s existing fallback to the first available tab applies, and the stored value survives so it can be honoured once that tab reappears. Archiving a chat clears its stored tab entry so unarchiving starts fresh with the default tab. --- .../pages/AgentsPage/AgentChatPage.test.ts | 104 ++++++++++++ site/src/pages/AgentsPage/AgentChatPage.tsx | 47 +++++- .../AgentsPage/AgentChatPageView.stories.tsx | 152 ++++++++++++++++++ .../pages/AgentsPage/AgentChatPageView.tsx | 17 +- site/src/pages/AgentsPage/AgentsPage.tsx | 3 + 5 files changed, 319 insertions(+), 4 deletions(-) diff --git a/site/src/pages/AgentsPage/AgentChatPage.test.ts b/site/src/pages/AgentsPage/AgentChatPage.test.ts index a7f0be7658..83de6462bd 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.test.ts +++ b/site/src/pages/AgentsPage/AgentChatPage.test.ts @@ -3,10 +3,14 @@ import { createRef } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type * as TypesGen from "#/api/typesGenerated"; import { + clearPersistedSidebarTabId, draftInputStorageKeyPrefix, filterWorkspaceOptionsByOrganization, getPersistedDraftInputValue, + getPersistedSidebarTabId, + lastActiveSidebarTabStorageKeyPrefix, restoreOptimisticRequestSnapshot, + savePersistedSidebarTabId, submitEditAndScroll, useConversationEditingState, waitForPendingChatSettingsSyncs, @@ -912,3 +916,103 @@ describe("submitEditAndScroll", () => { expect(editMessage).toHaveBeenCalled(); }); }); + +describe("sidebar tab persistence", () => { + beforeEach(() => { + localStorage.clear(); + }); + + describe("getPersistedSidebarTabId", () => { + it("returns null when no value is stored for that chat", () => { + expect(getPersistedSidebarTabId("chat-1")).toBeNull(); + }); + + it("returns the stored string when one is present", () => { + localStorage.setItem( + `${lastActiveSidebarTabStorageKeyPrefix}chat-1`, + "terminal", + ); + expect(getPersistedSidebarTabId("chat-1")).toBe("terminal"); + }); + + it("returns null when chatID is undefined", () => { + expect(getPersistedSidebarTabId(undefined)).toBeNull(); + }); + + it("returns null when chatID is empty string", () => { + expect(getPersistedSidebarTabId("")).toBeNull(); + }); + + it("reads from the key agents.last-active-tab.", () => { + const chatID = "chat-xyz"; + localStorage.setItem(`agents.last-active-tab.${chatID}`, "git"); + expect(getPersistedSidebarTabId(chatID)).toBe("git"); + }); + }); + + describe("savePersistedSidebarTabId", () => { + it("writes tabID to agents.last-active-tab.", () => { + savePersistedSidebarTabId("chat-1", "desktop"); + expect( + localStorage.getItem(`${lastActiveSidebarTabStorageKeyPrefix}chat-1`), + ).toBe("desktop"); + }); + + it("is a no-op when chatID is undefined", () => { + savePersistedSidebarTabId(undefined, "desktop"); + expect(localStorage.length).toBe(0); + }); + + it("is a no-op when chatID is empty string", () => { + savePersistedSidebarTabId("", "desktop"); + expect(localStorage.length).toBe(0); + }); + + it("can be round-tripped with getPersistedSidebarTabId", () => { + savePersistedSidebarTabId("chat-rt", "terminal"); + expect(getPersistedSidebarTabId("chat-rt")).toBe("terminal"); + }); + + it("does not collide across different chatIDs", () => { + savePersistedSidebarTabId("chat-a", "git"); + savePersistedSidebarTabId("chat-b", "desktop"); + expect(getPersistedSidebarTabId("chat-a")).toBe("git"); + expect(getPersistedSidebarTabId("chat-b")).toBe("desktop"); + }); + }); + + describe("clearPersistedSidebarTabId", () => { + it("removes agents.last-active-tab. from storage", () => { + savePersistedSidebarTabId("chat-1", "terminal"); + clearPersistedSidebarTabId("chat-1"); + expect(getPersistedSidebarTabId("chat-1")).toBeNull(); + }); + + it("is a no-op when nothing is stored", () => { + // Calling twice should not throw. + clearPersistedSidebarTabId("chat-1"); + clearPersistedSidebarTabId("chat-1"); + expect(getPersistedSidebarTabId("chat-1")).toBeNull(); + }); + + it("is a no-op when chatID is undefined", () => { + savePersistedSidebarTabId("chat-1", "git"); + clearPersistedSidebarTabId(undefined); + expect(getPersistedSidebarTabId("chat-1")).toBe("git"); + }); + + it("is a no-op when chatID is empty string", () => { + savePersistedSidebarTabId("chat-1", "git"); + clearPersistedSidebarTabId(""); + expect(getPersistedSidebarTabId("chat-1")).toBe("git"); + }); + + it("only affects the target chat's entry", () => { + savePersistedSidebarTabId("chat-a", "git"); + savePersistedSidebarTabId("chat-b", "desktop"); + clearPersistedSidebarTabId("chat-a"); + expect(getPersistedSidebarTabId("chat-a")).toBeNull(); + expect(getPersistedSidebarTabId("chat-b")).toBe("desktop"); + }); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentChatPage.tsx b/site/src/pages/AgentsPage/AgentChatPage.tsx index 1a7100fd81..2bf6d40c23 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.tsx @@ -96,6 +96,8 @@ export const RIGHT_PANEL_OPEN_KEY = "agents.right-panel-open"; const lastModelConfigIDStorageKey = "agents.last-model-config-id"; /** @internal Exported for testing. */ export const draftInputStorageKeyPrefix = "agents.draft-input."; +/** @internal localStorage key prefix for the per-chat active sidebar tab. Exported for testing. */ +export const lastActiveSidebarTabStorageKeyPrefix = "agents.last-active-tab."; const clearChatPlanMode = "" satisfies ChatPlanModeOrClear; @@ -109,7 +111,7 @@ type PlanModeSwitch = TypesGen.ChatPlanMode | "clear"; export function getPersistedDraftInputValue( chatID: string | undefined, ): string { - if (typeof window === "undefined" || !chatID) { + if (!chatID) { return ""; } return parseStoredDraft( @@ -117,6 +119,49 @@ export function getPersistedDraftInputValue( ).text; } +/** + * Read the persisted active sidebar tab ID for a given chat. Returns + * `null` when no value is stored or the chat ID is missing. + */ +export function getPersistedSidebarTabId( + chatID: string | undefined, +): string | null { + if (!chatID) { + return null; + } + return localStorage.getItem( + `${lastActiveSidebarTabStorageKeyPrefix}${chatID}`, + ); +} + +/** + * Persist the active sidebar tab ID for a given chat so it can be + * restored across session switches. No-op when the chat ID is missing. + */ +export function savePersistedSidebarTabId( + chatID: string | undefined, + tabID: string, +): void { + if (!chatID) { + return; + } + localStorage.setItem( + `${lastActiveSidebarTabStorageKeyPrefix}${chatID}`, + tabID, + ); +} + +/** + * Remove the persisted active sidebar tab ID for a given chat. Called + * when a chat is archived so a future unarchive starts fresh. + */ +export function clearPersistedSidebarTabId(chatID: string | undefined): void { + if (!chatID) { + return; + } + localStorage.removeItem(`${lastActiveSidebarTabStorageKeyPrefix}${chatID}`); +} + /** @internal Exported for testing. */ export const restoreOptimisticRequestSnapshot = ( store: Pick< diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index afd0e138df..ba1dd22607 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -16,6 +16,7 @@ import { withProxyProvider, withWebSocket, } from "#/testHelpers/storybook"; +import { lastActiveSidebarTabStorageKeyPrefix } from "./AgentChatPage"; import { AgentChatPageLoadingView, AgentChatPageNotFoundView, @@ -1011,3 +1012,154 @@ export const TerminalFocusOnTabSwitch: Story = { ); }, }; + +const sidebarTabStorageKey = `${lastActiveSidebarTabStorageKeyPrefix}${AGENT_ID}`; + +/** + * When localStorage contains a persisted tab ID for this chat, the sidebar + * should restore it on mount. Seed localStorage with "terminal" and verify + * that the Terminal tab is selected instead of the default Git tab. + */ +export const RestoresPersistedSidebarTab: Story = { + beforeEach: () => { + localStorage.setItem(sidebarTabStorageKey, "terminal"); + return () => { + localStorage.removeItem(sidebarTabStorageKey); + }; + }, + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor(() => { + const terminalTab = canvas.getByRole("tab", { name: "Terminal" }); + expect(terminalTab).toHaveAttribute("aria-selected", "true"); + }); + + const gitTab = canvas.getByRole("tab", { name: "Git" }); + expect(gitTab).toHaveAttribute("aria-selected", "false"); + }, +}; + +/** + * Clicking a sidebar tab persists the selection to localStorage so that + * it is restored across session switches. + */ +export const PersistsSidebarTabClick: Story = { + beforeEach: () => { + localStorage.removeItem(sidebarTabStorageKey); + return () => { + localStorage.removeItem(sidebarTabStorageKey); + }; + }, + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor(() => { + const gitTab = canvas.getByRole("tab", { name: "Git" }); + expect(gitTab).toHaveAttribute("aria-selected", "true"); + }); + + const terminalTab = canvas.getByRole("tab", { name: "Terminal" }); + await userEvent.click(terminalTab); + + await waitFor(() => { + expect(terminalTab).toHaveAttribute("aria-selected", "true"); + }); + + expect(localStorage.getItem(sidebarTabStorageKey)).toBe("terminal"); + }, +}; + +/** + * When localStorage holds a tab ID whose tab is not currently available + * (e.g. `"terminal"` while the workspace is stopped), the sidebar + * should fall back to the first available tab (Git) and the stored + * value must be preserved so it can be honoured once the tab reappears. + * + * This locks down the contract described in the PR: `getEffectiveTabId` + * only reads `sidebarTabId` and never writes back. A future write-back + * in the fallback path would silently break restore-after-recovery, so + * this story exists to catch that regression. + */ +export const PreservesUnavailableSidebarTab: Story = { + beforeEach: () => { + localStorage.setItem(sidebarTabStorageKey, "terminal"); + return () => { + localStorage.removeItem(sidebarTabStorageKey); + }; + }, + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor(() => { + const gitTab = canvas.getByRole("tab", { name: "Git" }); + expect(gitTab).toHaveAttribute("aria-selected", "true"); + }); + + expect(canvas.queryByRole("tab", { name: "Terminal" })).toBeNull(); + + expect(localStorage.getItem(sidebarTabStorageKey)).toBe("terminal"); + }, +}; + +/** + * When a chat is archived, clicking a sidebar tab must not persist the + * selection to localStorage. The archive flow clears the entry on + * purpose so that a subsequent unarchive starts from the default tab; + * persisting here would silently recreate the entry for any tab the + * user clicks while viewing the read-only archived view. + * + * This locks down the fix for the codex P2 review comment. + */ +export const DoesNotPersistForArchivedChat: Story = { + beforeEach: () => { + localStorage.removeItem(sidebarTabStorageKey); + return () => { + localStorage.removeItem(sidebarTabStorageKey); + }; + }, + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor(() => { + const gitTab = canvas.getByRole("tab", { name: "Git" }); + expect(gitTab).toHaveAttribute("aria-selected", "true"); + }); + + const terminalTab = canvas.getByRole("tab", { name: "Terminal" }); + await userEvent.click(terminalTab); + + await waitFor(() => { + expect(terminalTab).toHaveAttribute("aria-selected", "true"); + }); + + expect(localStorage.getItem(sidebarTabStorageKey)).toBeNull(); + }, +}; diff --git a/site/src/pages/AgentsPage/AgentChatPageView.tsx b/site/src/pages/AgentsPage/AgentChatPageView.tsx index 2ab08280d9..e4e54c8b21 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.tsx @@ -14,6 +14,10 @@ import type * as TypesGen from "#/api/typesGenerated"; import type { ChatDiffStatus, ChatMessagePart } from "#/api/typesGenerated"; import { cn } from "#/utils/cn"; import { pageTitle } from "#/utils/page"; +import { + getPersistedSidebarTabId, + savePersistedSidebarTabId, +} from "./AgentChatPage"; import { AgentChatInput, type ChatMessageInputRef, @@ -268,9 +272,16 @@ export const AgentChatPageView: FC = ({ const effectiveScrollToBottomRef = scrollToBottomRef ?? internalScrollToBottomRef; - // State for programmatically switching the sidebar tab (e.g. when - // the user clicks the inline desktop preview card). - const [sidebarTabId, setSidebarTabId] = useState(null); + const [sidebarTabId, setSidebarTabIdState] = useState(() => + getPersistedSidebarTabId(agentId), + ); + + const setSidebarTabId = (tabId: string) => { + setSidebarTabIdState(tabId); + if (!isArchived) { + savePersistedSidebarTabId(agentId, tabId); + } + }; const handleOpenDesktop = () => { onSetShowSidebarPanel(true); diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index ba074f37c0..919da2fce2 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -39,6 +39,7 @@ import { DeleteDialog } from "#/components/Dialogs/DeleteDialog/DeleteDialog"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { useDashboard } from "#/modules/dashboard/useDashboard"; import { createReconnectingWebSocket } from "#/utils/reconnectingWebSocket"; +import { clearPersistedSidebarTabId } from "./AgentChatPage"; import { AgentsPageView } from "./AgentsPageView"; import { emptyInputStorageKey } from "./components/AgentCreateForm"; import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings"; @@ -164,6 +165,7 @@ const AgentsPage: FC = () => { ...archiveChatBase, onSuccess: (_data, chatId) => { clearChatErrorReason(chatId); + clearPersistedSidebarTabId(chatId); }, onError: (error, chatId, context) => { archiveChatBase.onError(error, chatId, context); @@ -186,6 +188,7 @@ const AgentsPage: FC = () => { ), onSuccess: async ({ chatId }) => { clearChatErrorReason(chatId); + clearPersistedSidebarTabId(chatId); await invalidateChatListQueries(queryClient); await queryClient.invalidateQueries({ queryKey: chatKey(chatId),