diff --git a/__tests__/components/features/chat/plan-preview.test.tsx b/__tests__/components/features/chat/plan-preview.test.tsx index 6af94df2d4..c48a09c48d 100644 --- a/__tests__/components/features/chat/plan-preview.test.tsx +++ b/__tests__/components/features/chat/plan-preview.test.tsx @@ -208,8 +208,7 @@ describe("PlanPreview", () => { await user.click(buildButton); // Assert - const pending = - useOptimisticUserMessageStore.getState().pendingMessages; + const pending = useOptimisticUserMessageStore.getState().pendingMessages; expect(pending).toHaveLength(1); expect(pending[0].text).toBe(expectedPrompt); expect(pending[0].status).toBe("sending"); @@ -381,9 +380,9 @@ describe("PlanPreview", () => { const viewButton = screen.getByTestId("plan-preview-view-button"); await user.click(viewButton); - // Assert: selectTab was called with 'planner' and the drawer opened - // (in-memory). The drawer-open state is session-only and must not - // touch localStorage; only the selected tab persists. + // Assert: selectTab was called with 'planner' and the drawer opened. + // Opening the drawer also mirrors `rightPanelShown` into the + // conversation's localStorage blob, alongside the selected tab. expect(useConversationStore.getState().selectedTab).toBe("planner"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); @@ -391,7 +390,7 @@ describe("PlanPreview", () => { localStorage.getItem(`conversation-state-${conversationId}`)!, ); expect(storedState.selectedTab).toBe("planner"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should call selectTab with 'planner' when Read more button is clicked", async () => { @@ -412,9 +411,9 @@ describe("PlanPreview", () => { const readMoreButton = screen.getByTestId("plan-preview-read-more-button"); await user.click(readMoreButton); - // Assert: selectTab was called with 'planner' and the drawer opened - // (in-memory). The drawer-open state is session-only and must not - // touch localStorage; only the selected tab persists. + // Assert: selectTab was called with 'planner' and the drawer opened. + // Opening the drawer also mirrors `rightPanelShown` into the + // conversation's localStorage blob, alongside the selected tab. expect(useConversationStore.getState().selectedTab).toBe("planner"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); @@ -422,6 +421,6 @@ describe("PlanPreview", () => { localStorage.getItem(`conversation-state-${conversationId}`)!, ); expect(storedState.selectedTab).toBe("planner"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); }); diff --git a/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx b/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx index 9617e3fcb7..dd796ff599 100644 --- a/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx +++ b/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx @@ -1,12 +1,39 @@ import { render, screen } from "@testing-library/react"; -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; import { ChatInterfaceWrapper } from "#/components/features/conversation/conversation-main/chat-interface-wrapper"; +import { useConversationStore } from "#/stores/conversation-store"; vi.mock("#/components/features/chat/chat-interface", () => ({ ChatInterface: () =>
, })); +vi.mock("#/components/features/conversation/conversation-overview-panel", () => ({ + ConversationOverviewPanel: () => ( +
+ ), +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => false, +})); + +const mockUseConversationOverviewColumnSpace = vi.fn(() => true); + +vi.mock("#/hooks/use-conversation-overview-column-space", () => ({ + useConversationOverviewColumnSpace: () => + mockUseConversationOverviewColumnSpace(), +})); + describe("ChatInterfaceWrapper", () => { + beforeEach(() => { + mockUseConversationOverviewColumnSpace.mockReturnValue(true); + useConversationStore.setState({ + isOverviewPanelShown: false, + isOverviewPanelPeeked: false, + isRightPanelShown: false, + }); + }); + it("renders the chat interface when the right panel is hidden", () => { render(); @@ -18,4 +45,38 @@ describe("ChatInterfaceWrapper", () => { expect(screen.getByTestId("chat-interface")).toBeInTheDocument(); }); + + it("uses the overview grid layout when space is available", () => { + useConversationStore.setState({ isOverviewPanelShown: true }); + render(); + + expect(screen.getByTestId("conversation-overview-column")).toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-panel")).toBeInTheDocument(); + }); + + it("keeps the thread in a height-constrained flex column when overview is shown", () => { + useConversationStore.setState({ isOverviewPanelShown: true }); + const { container } = render( + , + ); + + const threadColumn = container.querySelector(".overflow-hidden.flex-1"); + expect(threadColumn).toBeInTheDocument(); + expect(threadColumn).toHaveClass("min-h-0"); + }); + + it("falls back to the centered thread layout when the right column is too narrow", () => { + mockUseConversationOverviewColumnSpace.mockReturnValue(false); + useConversationStore.setState({ isOverviewPanelShown: true }); + + render(); + + expect( + screen.queryByTestId("conversation-overview-column"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-panel"), + ).not.toBeInTheDocument(); + expect(screen.getByTestId("chat-interface")).toBeInTheDocument(); + }); }); diff --git a/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx b/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx new file mode 100644 index 0000000000..679208430c --- /dev/null +++ b/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx @@ -0,0 +1,84 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationGitActionsToggle } from "#/components/features/conversation/conversation-git-actions-toggle"; +import { useConversationStore } from "#/stores/conversation-store"; + +const { breakpointIsMobile } = vi.hoisted(() => ({ + breakpointIsMobile: { value: false }, +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => breakpointIsMobile.value, +})); + +vi.mock("#/hooks/use-is-archived-conversation", () => ({ + useIsArchivedConversation: () => false, +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + git_provider: "github", + }, + }), +})); + +describe("ConversationGitActionsToggle", () => { + beforeEach(() => { + vi.clearAllMocks(); + breakpointIsMobile.value = false; + useConversationStore.setState({ messageToSend: null }); + }); + + it("stays visible on smaller screens", () => { + breakpointIsMobile.value = true; + + render(); + + expect( + screen.getByTestId("conversation-git-actions-toggle"), + ).toBeInTheDocument(); + }); + + it("opens a dropdown of git actions and fills the composer with prompts", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + + await user.click( + await screen.findByTestId("conversation-git-actions-commit"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "commit", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-pull")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-push")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "push", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-create-pr")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull request", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click( + screen.getByTestId("conversation-git-actions-create-new-branch"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "new branch", + ); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-name-with-status.test.tsx b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx index 9cb41ff550..00f2b9ea4d 100644 --- a/__tests__/components/features/conversation/conversation-name-with-status.test.tsx +++ b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx @@ -30,6 +30,9 @@ vi.mock("#/hooks/query/use-active-conversation", () => ({ vi.mock("#/hooks/use-conversation-id", () => ({ useConversationId: () => ({ conversationId: "test-conversation-id" }), + useOptionalConversationId: () => ({ + conversationId: "test-conversation-id", + }), })); vi.mock("#/hooks/mutation/use-unified-stop-conversation", () => ({ diff --git a/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx b/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx new file mode 100644 index 0000000000..9912d302f4 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx @@ -0,0 +1,140 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewDiffsRow } from "#/components/features/conversation/conversation-overview-diffs-row"; +import { useConversationStore } from "#/stores/conversation-store"; + +const navigateToTabMock = vi.fn(); +const closeDrawerMock = vi.fn(); + +vi.mock("#/hooks/use-conversation-overview-git-diff-stats", () => ({ + useConversationOverviewGitDiffStats: () => ({ + additions: 12, + deletions: 4, + changeCount: 2, + isLoading: false, + isError: false, + }), +})); + +const navigateToChangesMock = vi.fn(); + +vi.mock("#/hooks/use-select-conversation-tab", () => ({ + useSelectConversationTab: () => ({ + navigateToTab: navigateToTabMock, + navigateToChanges: navigateToChangesMock, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + git_provider: "github", + }, + }), +})); + +vi.mock( + "#/components/features/conversation/conversation-overview-drawer-context", + () => ({ + useConversationOverviewDrawerOptional: () => ({ + section: "skills", + openAdd: false, + openSection: vi.fn(), + closeDrawer: closeDrawerMock, + }), + }), +); + +describe("ConversationOverviewDiffsRow", () => { + beforeEach(() => { + vi.clearAllMocks(); + useConversationStore.setState({ messageToSend: null }); + }); + + it("opens the git actions menu and sends commit, pull, push, and create PR prompts", async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + + await user.click( + await screen.findByTestId("conversation-overview-diffs-git-commit"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "commit", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click(screen.getByTestId("conversation-overview-diffs-git-pull")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click(screen.getByTestId("conversation-overview-diffs-git-push")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "push", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click( + screen.getByTestId("conversation-overview-diffs-git-create-pr"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull request", + ); + }); + + it("keeps diff numbers hidden while the git menu is open", async () => { + const user = userEvent.setup(); + render(); + + const stats = screen.getByTestId( + "conversation-overview-diffs-additions", + ).parentElement; + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + + expect(stats).toHaveClass("opacity-0"); + }); + + it("opens Diff view and closes open drawers when the changes label is clicked", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-overview-diffs")); + + expect(closeDrawerMock).toHaveBeenCalled(); + expect(navigateToChangesMock).toHaveBeenCalled(); + expect(navigateToTabMock).not.toHaveBeenCalled(); + }); + + it("uses a full-row hover that clears when the git action is hovered", () => { + render(); + + const row = screen.getByTestId("conversation-overview-diffs").closest("li"); + const changesButton = screen.getByTestId("conversation-overview-diffs"); + const gitAction = screen.getByTestId( + "conversation-overview-diffs-git-action", + ); + + expect(row).toHaveClass("hover:bg-white/5"); + expect(row?.className).toContain( + "has-[.conversation-overview-diffs-git-action:hover]:bg-transparent", + ); + expect(changesButton).not.toHaveClass("hover:bg-white/5"); + expect(gitAction).toHaveClass("hover:bg-white/10"); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx b/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx new file mode 100644 index 0000000000..5ad02e370d --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx @@ -0,0 +1,260 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationOverviewDrawerContent } from "#/components/features/conversation/conversation-overview-drawer-content"; +import { + ConversationOverviewDrawerProvider, + useConversationOverviewDrawer, +} from "#/components/features/conversation/conversation-overview-drawer-context"; +import { CONVERSATION_OVERVIEW_DRAWER_SECTION } from "#/components/features/conversation/conversation-overview-drawer.types"; +import { ActiveBackendProvider } from "#/contexts/active-backend-context"; +import SettingsService from "#/api/settings-service/settings-service.api"; +import SkillsService from "#/api/skills-service"; +import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; +import type { SkillInfo } from "#/types/settings"; + +vi.mock("#/hooks/use-conversation-overview-stats", () => ({ + useConversationOverviewStats: () => ({ + workspaceName: "demo", + }), +})); + +vi.mock("#/hooks/use-conversation-primary-repository", () => ({ + useConversationPrimaryRepository: () => ({ + repository: "openhands/agent-canvas", + provider: "github" as const, + branch: "main", + isConnected: true, + }), +})); + +vi.mock("#/hooks/query/use-repository-git-items", () => ({ + useRepositoryPullRequests: () => ({ + data: [], + isLoading: false, + isError: false, + }), + useRepositoryIssues: () => ({ + data: [], + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { selected_workspace: "/workspace/project/demo" }, + }), +})); + +vi.mock("#/hooks/query/use-automation-health", () => ({ + useAutomationHealth: () => ({ + data: { status: "ok" }, + isLoading: false, + refetch: vi.fn(), + }), +})); + +vi.mock("#/hooks/query/use-automations", () => ({ + useAutomations: () => ({ + data: { automations: [], total: 0 }, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useToggleAutomation: () => ({ mutate: vi.fn() }), + useDeleteAutomation: () => ({ mutate: vi.fn(), isPending: false }), + useDispatchAutomation: () => ({ mutate: vi.fn() }), +})); + +vi.mock("#/hooks/use-tracking", () => ({ + useTracking: () => ({ + trackPrebuiltAutomationEnabled: vi.fn(), + }), +})); + +vi.mock("#/hooks/use-create-automation-in-chat", () => ({ + useCreateAutomationInChat: () => vi.fn(), +})); + +vi.mock("#/hooks/use-is-creating-conversation", () => ({ + useIsCreatingConversation: () => false, +})); + +vi.mock("#/hooks/mutation/use-create-conversation", () => ({ + useCreateConversation: () => ({ mutate: vi.fn(), isPending: false }), +})); + +function buildSkill(overrides: Partial = {}): SkillInfo { + return { + name: "deno", + type: "knowledge", + source: "/Users/test/.openhands/cache/skills/public-skills/skills/deno/SKILL.md", + description: "Use this skill for Deno projects.", + triggers: ["deno"], + version: "1.0.0", + license: "Apache-2.0", + compatibility: null, + metadata: null, + allowed_tools: null, + is_agentskills_format: true, + disable_model_invocation: false, + ...overrides, + }; +} + +function OpenSection({ + section, +}: { + section: (typeof CONVERSATION_OVERVIEW_DRAWER_SECTION)[keyof typeof CONVERSATION_OVERVIEW_DRAWER_SECTION]; +}) { + const { openSection } = useConversationOverviewDrawer(); + return ( + + ); +} + +function renderDrawer( + section: (typeof CONVERSATION_OVERVIEW_DRAWER_SECTION)[keyof typeof CONVERSATION_OVERVIEW_DRAWER_SECTION], +) { + return render( + + + + , + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); +} + +describe("ConversationOverviewDrawerContent", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + MOCK_DEFAULT_USER_SETTINGS, + ); + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([buildSkill()]); + }); + + it("places the close button left of the title and the add control on the right", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.skills); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + expect(header).not.toBeNull(); + expect(header).toHaveClass("h-10"); + expect(header).toHaveClass("min-h-10"); + expect(header).toHaveClass("pr-4"); + expect( + within(header as HTMLElement).getByTestId( + "conversation-overview-skills-add-skill-button", + ), + ).toHaveClass("h-7"); + + const headerItems = within(header as HTMLElement).getAllByRole("button"); + expect(headerItems[0]).toHaveAttribute( + "data-testid", + "conversation-overview-drawer-close", + ); + expect(headerItems[1]).toHaveAttribute( + "data-testid", + "conversation-overview-skills-add-skill-button", + ); + }); + + it("opens the add skill modal from the header add button", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.skills); + + await user.click(screen.getByTestId("open-drawer-section")); + await user.click( + await screen.findByTestId("conversation-overview-skills-add-skill-button"), + ); + + expect(await screen.findByTestId("add-skill-modal")).toBeInTheDocument(); + }); + + it("shows the automations add button in the header", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.automations); + + await user.click(screen.getByTestId("open-drawer-section")); + + expect( + await screen.findByTestId("conversation-overview-automations-add"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-automations-panel") + ?.querySelector( + '[data-testid="conversation-overview-automations-add"]', + ), + ).toBeNull(); + }); + + it("shows the mcp add button in the header", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.mcp); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + expect( + within(header as HTMLElement).getByTestId( + "conversation-overview-mcp-add-server", + ), + ).toBeInTheDocument(); + }); + + it("places the view-on-provider link in the header for pull requests", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.pull_requests); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + const externalLink = within(header as HTMLElement).getByTestId( + "conversation-overview-pull_requests-open-external", + ); + + expect(externalLink).toHaveAttribute( + "href", + "https://github.com/openhands/agent-canvas/pulls", + ); + expect(externalLink).toHaveTextContent( + "CONVERSATION$OVERVIEW_VIEW_ON_PROVIDER", + ); + expect( + screen + .getByTestId("conversation-overview-pull_requests-panel") + .querySelector( + '[data-testid="conversation-overview-pull_requests-open-external"]', + ), + ).toBeNull(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-panel.test.tsx b/__tests__/components/features/conversation/conversation-overview-panel.test.tsx new file mode 100644 index 0000000000..44b490704e --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-panel.test.tsx @@ -0,0 +1,289 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewPanel } from "#/components/features/conversation/conversation-overview-panel"; +import { NavigationProvider } from "#/context/navigation-context"; +import { ConversationOverviewDrawerProvider } from "#/components/features/conversation/conversation-overview-drawer-context"; +import { CONVERSATION_OVERVIEW_DRAWER_SECTION } from "#/components/features/conversation/conversation-overview-drawer.types"; + +const openSection = vi.fn(); +const closeDrawer = vi.fn(); +const navigateToCommits = vi.fn(); + +vi.mock("#/hooks/use-conversation-id", () => ({ + useConversationId: () => ({ conversationId: "conv-1" }), +})); + +vi.mock("#/hooks/use-conversation-overview-git-diff-stats", () => ({ + useConversationOverviewGitDiffStats: () => ({ + additions: 4161, + deletions: 1824, + changeCount: 3, + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/hooks/use-select-conversation-tab", () => ({ + useSelectConversationTab: () => ({ + navigateToTab: vi.fn(), + navigateToChanges: vi.fn(), + navigateToCommits, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + selected_workspace: "/workspace/project/demo", + llm_model: "openhands/test-model", + }, + }), +})); + +vi.mock("#/hooks/query/use-settings", () => ({ + useSettings: () => ({ + data: { + llm_model: "openhands/test-model", + agent_settings: { + mcp_config: { + mcpServers: { + example: { + url: "https://example.com/mcp", + }, + }, + }, + }, + }, + }), +})); + +vi.mock("#/hooks/use-conversation-primary-repository", () => ({ + useConversationPrimaryRepository: () => ({ + repository: "openhands/agent-canvas", + provider: "github" as const, + branch: "main", + isConnected: true, + }), +})); + +vi.mock("#/hooks/query/use-unified-git-commits", () => ({ + useUnifiedGitCommits: () => ({ + commits: [{ sha: "abc" }, { sha: "def" }, { sha: "ghi" }], + hasMore: false, + isUnsupported: false, + isLoading: false, + isFetching: false, + isSuccess: true, + isError: false, + }), +})); + +vi.mock("#/hooks/query/use-repository-git-items", () => ({ + useRepositoryPullRequests: () => ({ + data: [ + { + id: 1, + number: 10, + title: "Fix overview", + url: "https://github.com/openhands/agent-canvas/pull/10", + authorLogin: "dev", + updatedAt: null, + }, + ], + isLoading: false, + isError: false, + }), + useRepositoryIssues: () => ({ + data: [], + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/api/conversation-metadata-store", () => ({ + getStoredConversationMetadata: () => ({ + selected_workspace: "/workspace/project/demo", + }), +})); + +vi.mock( + "#/components/features/conversation/conversation-overview-drawer-context", + async (importOriginal) => { + const actual = await importOriginal< + typeof import("#/components/features/conversation/conversation-overview-drawer-context") + >(); + return { + ...actual, + useConversationOverviewDrawerOptional: () => ({ + section: null, + openAdd: false, + openSection, + closeDrawer, + }), + }; + }, +); + +function renderPanel() { + return render( + + + + + , + ); +} + +describe("ConversationOverviewPanel", () => { + beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + }); + + it("renders workspace and git changes without MCP, secrets, skills, or automations", () => { + renderPanel(); + + expect(screen.getByTestId("conversation-overview-panel")).toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-workspace")).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-git-title"), + ).not.toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-mcp"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-automations"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-skills"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-secrets"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-issues"), + ).not.toBeInTheDocument(); + }); + + it("shows changes inside the git area with commits and pull requests when a repo is connected", async () => { + const user = userEvent.setup(); + renderPanel(); + + const gitBlock = screen.getByTestId("conversation-overview-git-block"); + const diffs = screen.getByTestId("conversation-overview-diffs"); + expect(gitBlock).toContainElement(diffs); + + const workspace = screen.getByTestId("conversation-overview-workspace"); + const gitSection = screen.getByTestId("conversation-overview-git-section"); + // Workspace sits below the git content. + expect(gitSection.compareDocumentPosition(workspace)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + const repoLink = screen.getByTestId("conversation-overview-git-repo"); + expect(repoLink).toHaveTextContent("openhands/agent-canvas"); + expect(repoLink.getAttribute("href")).toContain("github.com"); + const branchLink = screen.getByTestId("conversation-overview-git-branch"); + expect(branchLink).toHaveTextContent("main"); + expect(branchLink).toHaveAttribute( + "href", + "https://github.com/openhands/agent-canvas/tree/main", + ); + expect( + screen.getByTestId("conversation-overview-commits-count"), + ).toHaveTextContent("3"); + expect( + screen.getByTestId("conversation-overview-pull-requests-count"), + ).toHaveTextContent("1"); + + await user.click(screen.getByTestId("conversation-overview-commits")); + expect(navigateToCommits).toHaveBeenCalled(); + + await user.click(screen.getByTestId("conversation-overview-pull-requests")); + expect(openSection).toHaveBeenCalledWith( + CONVERSATION_OVERVIEW_DRAWER_SECTION.pull_requests, + ); + }); + + it("lets users pin and unpin git changes from the overflow menu", async () => { + const user = userEvent.setup(); + renderPanel(); + + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-ellipsis")); + expect( + screen.getByTestId("conversation-overview-context-menu"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-divider-git"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "true"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ); + + expect( + screen.queryByTestId("conversation-overview-diffs"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "false"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ); + + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "true"); + }); + + it("lets users unpin the git section and individual git parts from the overflow menu", async () => { + const user = userEvent.setup(); + renderPanel(); + + expect( + screen.getByTestId("conversation-overview-git-section"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-ellipsis")); + expect( + screen.getByTestId("conversation-overview-menu-pin-git"), + ).toHaveAttribute("aria-pressed", "true"); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-branch"), + ).toHaveAttribute("aria-pressed", "true"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-branch"), + ); + expect( + screen.queryByTestId("conversation-overview-git-branch"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-git-repo"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-menu-pin-git")); + expect( + screen.queryByTestId("conversation-overview-git-block"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git"), + ).toHaveAttribute("aria-pressed", "false"); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx b/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx new file mode 100644 index 0000000000..ec89ba7b24 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx @@ -0,0 +1,106 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationOverviewSkillsPanel } from "#/components/features/conversation/conversation-overview-skills-panel"; +import SettingsService from "#/api/settings-service/settings-service.api"; +import SkillsService from "#/api/skills-service"; +import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; +import type { SkillInfo } from "#/types/settings"; +import { ActiveBackendProvider } from "#/contexts/active-backend-context"; + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { selected_workspace: "/workspace/project/demo" }, + }), +})); + +function buildSkill(overrides: Partial = {}): SkillInfo { + return { + name: "deno", + type: "knowledge", + source: "/Users/test/.openhands/cache/skills/public-skills/skills/deno/SKILL.md", + description: "Use this skill for Deno projects.", + triggers: ["deno"], + version: "1.0.0", + license: "Apache-2.0", + compatibility: null, + metadata: null, + allowed_tools: null, + is_agentskills_format: true, + disable_model_invocation: false, + ...overrides, + }; +} + +function renderPanel(openAdd = false) { + return render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); +} + +describe("ConversationOverviewSkillsPanel", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + MOCK_DEFAULT_USER_SETTINGS, + ); + }); + + it("opens the add skill modal when openAdd is true", async () => { + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([buildSkill()]); + + renderPanel(true); + + expect(await screen.findByTestId("add-skill-modal")).toBeInTheDocument(); + }); + + it("shows the empty state without an inline add skill button", async () => { + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([]); + + renderPanel(); + + expect( + await screen.findByTestId("conversation-overview-skills-empty"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-skills-add-skill-button"), + ).not.toBeInTheDocument(); + }); + + it("defaults to this-project scope and can show all skills", async () => { + const user = userEvent.setup(); + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([ + buildSkill({ + name: "project-skill", + source: "/workspace/project/demo/.openhands/skills/project/SKILL.md", + }), + buildSkill({ name: "public-skill" }), + ]); + + renderPanel(); + + expect( + await screen.findByTestId("conversation-overview-skills-scope"), + ).toBeInTheDocument(); + expect(await screen.findByText("project-skill")).toBeInTheDocument(); + expect(screen.queryByText("public-skill")).not.toBeInTheDocument(); + + await user.click( + screen.getByTestId("conversation-overview-skills-scope-option-all"), + ); + + expect(await screen.findByText("public-skill")).toBeInTheDocument(); + expect(screen.getByText("project-skill")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx b/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx new file mode 100644 index 0000000000..5d6cd92535 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx @@ -0,0 +1,126 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewToggle } from "#/components/features/conversation/conversation-overview-toggle"; +import { useConversationStore } from "#/stores/conversation-store"; + +const { breakpointIsMobile } = vi.hoisted(() => ({ + breakpointIsMobile: { value: false }, +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => breakpointIsMobile.value, +})); + +vi.mock("#/hooks/use-is-archived-conversation", () => ({ + useIsArchivedConversation: () => false, +})); + +vi.mock("#/components/features/conversation/conversation-overview-panel", () => ({ + ConversationOverviewPanel: () => ( +
+ ), +})); + +describe("ConversationOverviewToggle", () => { + beforeEach(() => { + breakpointIsMobile.value = false; + useConversationStore.setState({ + isOverviewPanelShown: false, + isOverviewPanelPeeked: false, + isRightPanelShown: false, + hasRightPanelToggled: false, + }); + }); + + it("toggles the overview panel when clicked", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(true); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + + it("closes the files drawer and shows overview when the files drawer is open", async () => { + const user = userEvent.setup(); + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + hasRightPanelToggled: true, + }); + render(); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + + const state = useConversationStore.getState(); + expect(state.isRightPanelShown).toBe(false); + expect(state.hasRightPanelToggled).toBe(false); + expect(state.isOverviewPanelShown).toBe(true); + }); + + it("closes the overview panel when the right drawer opens", () => { + useConversationStore.setState({ + isOverviewPanelShown: true, + isRightPanelShown: false, + }); + + const { rerender } = render(); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(true); + + useConversationStore.setState({ isRightPanelShown: true }); + rerender(); + + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + + it("peeks the overview on hover while the right drawer is open", async () => { + const user = userEvent.setup(); + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + }); + render(); + + await user.hover(screen.getByTestId("conversation-overview-toggle")); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + expect(screen.getByTestId("conversation-overview-peek")).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-panel"), + ).toBeInTheDocument(); + }); + + it("does not peek the overview on hover when the right drawer is closed", async () => { + const user = userEvent.setup(); + render(); + + await user.hover(screen.getByTestId("conversation-overview-toggle")); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(false); + expect( + screen.queryByTestId("conversation-overview-peek"), + ).not.toBeInTheDocument(); + }); + + it("stays visible and supports hover peek on smaller screens", async () => { + const user = userEvent.setup(); + breakpointIsMobile.value = true; + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + }); + render(); + + const toggle = screen.getByTestId("conversation-overview-toggle"); + expect(toggle).toBeInTheDocument(); + + await user.hover(toggle); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(true); + expect(screen.getByTestId("conversation-overview-peek")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx b/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx index 36c60f3387..6dd3f78d10 100644 --- a/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx +++ b/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx @@ -62,11 +62,18 @@ describe("ConversationTabsContextMenu", () => { it("should render all default tabs when open", () => { render(); - const expectedTabs = ["COMMON$FILES", "COMMON$TERMINAL", "COMMON$BROWSER"]; + const expectedTabs = [ + "COMMON$FILES", + "DIFF_VIEWER$COMMITS", + "COMMON$TERMINAL", + "COMMON$BROWSER", + ]; for (const tab of expectedTabs) { expect(screen.getByText(tab)).toBeInTheDocument(); } + expect(screen.queryByText("FILES$DIFF_VIEW")).not.toBeInTheDocument(); + // Planner is cloud-only; on the default (local) backend it is hidden. expect(screen.queryByText("COMMON$PLANNER")).not.toBeInTheDocument(); }); @@ -132,13 +139,14 @@ describe("ConversationTabsContextMenu", () => { const storeState = useConversationStore.getState(); expect(storeState.hasRightPanelToggled).toBe(true); - expect(storeState.selectedTab).toBe("terminal"); + // Next pinned tab after Files is Commits. + expect(storeState.selectedTab).toBe("commits"); const storedState = JSON.parse( localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, ); expect(storedState.unpinnedTabs).toContain("files"); - expect(storedState.selectedTab).toBe("terminal"); + expect(storedState.selectedTab).toBe("commits"); }); it("should not close the right panel when unpinning a non-active tab", async () => { diff --git a/__tests__/components/features/conversation/conversation-tabs.test.tsx b/__tests__/components/features/conversation/conversation-tabs.test.tsx index 11b01527e4..7a145ee986 100644 --- a/__tests__/components/features/conversation/conversation-tabs.test.tsx +++ b/__tests__/components/features/conversation/conversation-tabs.test.tsx @@ -82,6 +82,8 @@ const seedConversationState = ( JSON.stringify({ selectedTab: "files", unpinnedTabs: [], + unpinnedOverviewSections: [], + unpinnedOverviewGitParts: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, @@ -102,6 +104,7 @@ function seedActiveBackend(backend: Backend): void { const setActiveTabState = (tab: "files" | "planner") => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: tab, + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: tab, @@ -160,9 +163,7 @@ describe("ConversationTabs localStorage behavior", () => { const parsed = JSON.parse(storedState!); expect(parsed).toHaveProperty("selectedTab"); expect(parsed).toHaveProperty("unpinnedTabs"); - // The right-drawer open state is session-only and must never - // be persisted into the consolidated conversation-state blob. - expect(parsed).not.toHaveProperty("rightPanelShown"); + expect(parsed.rightPanelShown).toBe(true); }); }); @@ -186,16 +187,15 @@ describe("ConversationTabs localStorage behavior", () => { const terminalTab = screen.getByTestId("conversation-tab-terminal"); await user.click(terminalTab); - // Assert: Panel should be open and terminal tab selected (in-memory only). + // Assert: Panel should be open and terminal tab selected. expect(useConversationStore.getState().selectedTab).toBe("terminal"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); - // Tab selection persists to localStorage; drawer-open state does not. const storedState = JSON.parse( localStorage.getItem(`conversation-state-${REAL_CONVERSATION_ID}`)!, ); expect(storedState.selectedTab).toBe("terminal"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should close panel when clicking the same active tab", async () => { @@ -203,6 +203,10 @@ describe("ConversationTabs localStorage behavior", () => { const user = userEvent.setup(); // Arrange: Panel is open with editor tab selected + seedConversationState(REAL_CONVERSATION_ID, { + selectedTab: "files", + rightPanelShown: true, + }); useConversationStore.setState({ selectedTab: "files", isRightPanelShown: true, @@ -217,17 +221,13 @@ describe("ConversationTabs localStorage behavior", () => { const editorTab = screen.getByTestId("conversation-tab-files"); await user.click(editorTab); - // Assert: Panel should be closed (in-memory only). + // Assert: Panel should be closed and persisted. expect(useConversationStore.getState().hasRightPanelToggled).toBe(false); - // localStorage must NOT carry the drawer-open state — that's - // session-only by design. - const raw = localStorage.getItem( - `conversation-state-${REAL_CONVERSATION_ID}`, + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${REAL_CONVERSATION_ID}`)!, ); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + expect(storedState.rightPanelShown).toBe(false); }); it("should switch to different tab when clicking another tab while panel is open", async () => { @@ -235,6 +235,10 @@ describe("ConversationTabs localStorage behavior", () => { const user = userEvent.setup(); // Arrange: Panel is open with editor tab selected + seedConversationState(REAL_CONVERSATION_ID, { + selectedTab: "files", + rightPanelShown: true, + }); useConversationStore.setState({ selectedTab: "files", isRightPanelShown: true, @@ -289,7 +293,7 @@ describe("ConversationTabs localStorage behavior", () => { expect(refreshButtons).toHaveLength(0); }); - it("places the Files tab leftmost in the tab bar", () => { + it("places the Files tab leftmost, followed by Commits", () => { setActiveTabState("files"); render(, { @@ -300,8 +304,13 @@ describe("ConversationTabs localStorage behavior", () => { document.querySelectorAll('[data-testid^="conversation-tab-"]'), ); const testIds = tabs.map((t) => t.getAttribute("data-testid")); - // Files must be the first tab rendered in the bar. + // Files must be the first tab; Commits sits beside it as the git view. expect(testIds[0]).toBe("conversation-tab-files"); + expect(testIds).toContain("conversation-tab-commits"); + expect(testIds).not.toContain("conversation-tab-changes"); + expect(testIds.indexOf("conversation-tab-files")).toBeLessThan( + testIds.indexOf("conversation-tab-commits"), + ); }); it("keeps Files leftmost even when the task list tab is present", () => { @@ -335,6 +344,7 @@ describe("ConversationTabs localStorage behavior", () => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: "planner", unpinnedTabs: ["planner"], + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: "planner", @@ -365,6 +375,7 @@ describe("ConversationTabs localStorage behavior", () => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: "files", unpinnedTabs: ["planner"], + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: "files", diff --git a/__tests__/components/features/conversation/right-panel-toggle.test.tsx b/__tests__/components/features/conversation/right-panel-toggle.test.tsx index f3a17912b5..a4fd8dba39 100644 --- a/__tests__/components/features/conversation/right-panel-toggle.test.tsx +++ b/__tests__/components/features/conversation/right-panel-toggle.test.tsx @@ -61,10 +61,10 @@ describe("RightPanelToggle", () => { expect(storeState.hasRightPanelToggled).toBe(false); expect(storeState.isRightPanelShown).toBe(false); - const raw = localStorage.getItem(`conversation-state-${CONVERSATION_ID}`); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, + ); + expect(storedState.rightPanelShown).toBe(false); }); it("should show the panel when clicked while panel is hidden", async () => { @@ -84,10 +84,10 @@ describe("RightPanelToggle", () => { expect(storeState.hasRightPanelToggled).toBe(true); expect(storeState.isRightPanelShown).toBe(true); - const raw = localStorage.getItem(`conversation-state-${CONVERSATION_ID}`); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, + ); + expect(storedState.rightPanelShown).toBe(true); }); it("should have aria-pressed attribute reflecting panel state on desktop", () => { diff --git a/__tests__/components/features/diff-viewer/commit-list.test.tsx b/__tests__/components/features/diff-viewer/commit-list.test.tsx new file mode 100644 index 0000000000..1d97948999 --- /dev/null +++ b/__tests__/components/features/diff-viewer/commit-list.test.tsx @@ -0,0 +1,177 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi } from "vitest"; +import { CommitList } from "#/components/features/diff-viewer/commit-list"; +import type { GitCommit } from "#/api/open-hands.types"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, options?: { count?: number }) => { + if ( + key === "DIFF_VIEWER$UNCOMMITTED_FILE_COUNT" && + typeof options?.count === "number" + ) { + return options.count === 1 + ? `${options.count} file` + : `${options.count} files`; + } + return key; + }, + }), +})); + +vi.mock("#/hooks/query/use-commit-changes", () => ({ + useCommitChanges: () => ({ + data: undefined, + isLoading: false, + isSuccess: false, + }), +})); + +vi.mock("#/components/features/diff-viewer/diff-change-list", () => ({ + DiffChangeList: ({ + changes, + }: { + changes: Array<{ path: string; status: string }>; + }) => ( +
+ {changes.map((change) => ( +
{change.path}
+ ))} +
+ ), +})); + +const makeCommit = (overrides: Partial = {}): GitCommit => ({ + sha: "a".repeat(40), + shortSha: "aaaaaaa", + subject: "add logging", + author: "Agent", + timestamp: "2026-07-10T12:00:00+07:00", + ...overrides, +}); + +describe("CommitList", () => { + it("renders an Uncommitted accordion row above the commit rows", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + expect(screen.getByText("DIFF_VIEWER$UNCOMMITTED")).toBeInTheDocument(); + expect(screen.getByTestId("uncommitted-changes-count")).toHaveTextContent( + "1 file", + ); + const rows = screen.getAllByTestId(/^(uncommitted-changes-row|commit-row)$/); + expect(rows[0]).toHaveAttribute("data-testid", "uncommitted-changes-row"); + }); + + it("pluralizes the Uncommitted file count", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-count")).toHaveTextContent( + "2 files", + ); + }); + + it("expands Uncommitted into the working-tree file list", async () => { + // Arrange + const user = userEvent.setup(); + render( + , + ); + + // Act + await user.click(screen.getByTestId("uncommitted-changes-row-toggle")); + + // Assert + expect(await screen.findByText("src/a.ts")).toBeInTheDocument(); + }); + + it("collapses Uncommitted when a commit row is expanded", async () => { + // Arrange + const user = userEvent.setup(); + render( + , + ); + const uncommittedToggle = screen.getByTestId( + "uncommitted-changes-row-toggle", + ); + await user.click(uncommittedToggle); + expect(uncommittedToggle).toHaveAttribute("aria-expanded", "true"); + expect(await screen.findByText("src/a.ts")).toBeInTheDocument(); + + // Act + await user.click(screen.getByTestId("commit-row-toggle")); + + // Assert — single-open accordion: Uncommitted collapses when a commit opens. + expect(uncommittedToggle).toHaveAttribute("aria-expanded", "false"); + expect(screen.getByTestId("commit-row-toggle")).toHaveAttribute( + "aria-expanded", + "true", + ); + }); + + it("expands Uncommitted on request and clears the request", () => { + // Arrange + const onAutoExpandHandled = vi.fn(); + + // Act + render( + , + ); + + // Assert + expect( + screen.getByTestId("uncommitted-changes-row-toggle"), + ).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByText("src/a.ts")).toBeInTheDocument(); + expect(onAutoExpandHandled).toHaveBeenCalled(); + }); + + it("still renders Uncommitted when there are no working-tree changes", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/diff-viewer/diff-change-list.test.tsx b/__tests__/components/features/diff-viewer/diff-change-list.test.tsx new file mode 100644 index 0000000000..512cb0efe8 --- /dev/null +++ b/__tests__/components/features/diff-viewer/diff-change-list.test.tsx @@ -0,0 +1,84 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { DiffChangeList } from "#/components/features/diff-viewer/diff-change-list"; + +vi.mock("framer-motion", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + // Skip exit animations so open/close assertions are synchronous. + useReducedMotion: () => true, + }; +}); + +vi.mock("#/hooks/query/use-unified-git-diff", () => ({ + useUnifiedGitDiff: () => ({ + data: { original: "a", modified: "b" }, + isLoading: false, + isSuccess: true, + isRefetching: false, + }), +})); + +vi.mock("@monaco-editor/react", () => ({ + DiffEditor: () =>
, + Editor: () =>
, +})); + +describe("DiffChangeList", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("keeps only one file expanded at a time", async () => { + const user = userEvent.setup(); + render( + , + ); + + const [firstToggle, secondToggle] = screen.getAllByTestId("collapse"); + + await user.click(firstToggle); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[0].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeTruthy(); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[1].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeNull(); + + await user.click(secondToggle); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[0].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeNull(); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[1].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeTruthy(); + }); + + it("collapses the open file when its header is clicked again", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("collapse")); + expect(screen.getByTestId("file-diff-viewer")).toBeInTheDocument(); + + await user.click(screen.getByTestId("collapse")); + expect(screen.queryByTestId("file-diff-viewer")).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx b/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx index 8f3a49622f..7bdb37e28e 100644 --- a/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx +++ b/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx @@ -1,7 +1,10 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { FileDiffViewer } from "#/components/features/diff-viewer/file-diff-viewer"; +import { + FileDiffViewer, + MAX_DIFF_EDITOR_HEIGHT_PX, +} from "#/components/features/diff-viewer/file-diff-viewer"; const MOCK_DIFF = { original: "old content", modified: "new content" }; const MOCK_MD_DIFF = { @@ -48,20 +51,29 @@ describe("FileDiffViewer", () => { mockIsLoading = false; }); - it("starts collapsed with no view mode buttons", () => { - render(); - - expect(screen.queryByTestId("view-mode-old")).not.toBeInTheDocument(); - expect(screen.queryByTestId("view-mode-diff")).not.toBeInTheDocument(); - expect(screen.queryByTestId("view-mode-new")).not.toBeInTheDocument(); + it("caps opened editor panes at 600px", () => { + expect(MAX_DIFF_EDITOR_HEIGHT_PX).toBe(600); }); - it("shows view mode buttons when expanded", async () => { + it("keeps view mode controls reserved but inert while collapsed", () => { + render(); + + const viewModeGroup = screen.getByTestId("view-mode-diff").parentElement; + expect(viewModeGroup).toHaveClass("invisible"); + expect(screen.getByTestId("view-mode-old")).toHaveAttribute( + "tabIndex", + "-1", + ); + }); + + it("reveals view mode buttons when expanded", async () => { const user = userEvent.setup(); render(); await expand(user); + const viewModeGroup = screen.getByTestId("view-mode-diff").parentElement; + expect(viewModeGroup).not.toHaveClass("invisible"); expect(screen.getByTestId("view-mode-old")).toBeInTheDocument(); expect(screen.getByTestId("view-mode-diff")).toBeInTheDocument(); expect(screen.getByTestId("view-mode-new")).toBeInTheDocument(); diff --git a/__tests__/conversation-local-storage.test.ts b/__tests__/conversation-local-storage.test.ts index 07571b36e9..b2b6dc4308 100644 --- a/__tests__/conversation-local-storage.test.ts +++ b/__tests__/conversation-local-storage.test.ts @@ -50,51 +50,37 @@ describe("conversation localStorage utilities", () => { expect(state.selectedTab).toBe("terminal"); }); - it("silently drops the legacy rightPanelShown field from older persisted blobs", () => { - // Older builds persisted the right-drawer state alongside the - // selected tab. The schema no longer carries that field — verify - // the read path strips it instead of leaking the unknown property - // onto consumers (and that legacy `false` values don't somehow - // pin the panel closed forever). - const conversationId = "conv-legacy-right-panel"; - const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; - localStorage.setItem( - key, - JSON.stringify({ - selectedTab: "terminal", - rightPanelShown: false, - unpinnedTabs: ["browser"], - }), - ); + it("round-trips rightPanelShown through localStorage", () => { + const conversationId = "conv-right-panel"; + setConversationState(conversationId, { + selectedTab: "terminal", + rightPanelShown: true, + unpinnedTabs: ["browser"], + }); const state = getConversationState(conversationId); expect(state.selectedTab).toBe("terminal"); expect(state.unpinnedTabs).toEqual(["browser"]); - expect(state).not.toHaveProperty("rightPanelShown"); + expect(state.rightPanelShown).toBe(true); }); - it("also drops legacy rightPanelShown: true (not just the falsy variant)", () => { - // Older builds could persist either boolean. The previous test - // covered `false`; this one covers `true` so an upgrading user - // with the drawer open can't have it leak through into the new - // schema either. - const conversationId = "conv-legacy-right-panel-true"; + it("defaults rightPanelShown to false and drops corrupt values", () => { + expect(getConversationState("conv-right-panel-default").rightPanelShown).toBe( + false, + ); + + const conversationId = "conv-right-panel-corrupt"; const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; localStorage.setItem( key, JSON.stringify({ selectedTab: "terminal", - rightPanelShown: true, - unpinnedTabs: ["browser"], + rightPanelShown: "yes", }), ); - const state = getConversationState(conversationId); - - expect(state.selectedTab).toBe("terminal"); - expect(state.unpinnedTabs).toEqual(["browser"]); - expect(state).not.toHaveProperty("rightPanelShown"); + expect(getConversationState(conversationId).rightPanelShown).toBe(false); }); it("returns default state when key is missing or invalid", () => { @@ -160,6 +146,30 @@ describe("conversation localStorage utilities", () => { expect(state.subConversationTaskId).toBeNull(); expect(state.selectedTab).toBe("files"); expect(state.unpinnedTabs).toEqual([]); + expect(state.unpinnedOverviewSections).toEqual([]); + expect(state.unpinnedOverviewGitParts).toEqual([]); + }); + + it("persists and sanitizes unpinnedOverviewSections", () => { + const conversationId = "conv-overview-pins"; + setConversationState(conversationId, { + unpinnedOverviewSections: ["skills", "not-a-section", "mcp", "workspace"], + }); + + const state = getConversationState(conversationId); + // Legacy section ids (mcp/skills/secrets/…) are dropped by the allowlist. + expect(state.unpinnedOverviewSections).toEqual(["workspace"]); + }); + + it("persists and sanitizes unpinnedOverviewGitParts", () => { + const conversationId = "conv-overview-git-pins"; + setConversationState(conversationId, { + unpinnedOverviewGitParts: ["branch", "not-a-part", "issues"], + }); + + const state = getConversationState(conversationId); + // Legacy git part ids (issues) are dropped by the allowlist. + expect(state.unpinnedOverviewGitParts).toEqual(["branch"]); }); it("retrieves subConversationTaskId from localStorage when it exists", () => { @@ -217,14 +227,29 @@ describe("conversation localStorage utilities", () => { expect(state.selectedTab).toBe("files"); }); - it("filters obsolete tabs out of stored unpinnedTabs (changes / editor / served / app)", () => { - // Returning users may have unpinned the now-removed Changes, - // Editor, Served, or App tabs in a previous version. Those names + it("migrates a stored Diffs (changes) tab selection to Commits", () => { + const conversationId = "conv-123"; + const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + + localStorage.setItem( + consolidatedKey, + JSON.stringify({ + selectedTab: "changes", + unpinnedTabs: [], + }), + ); + + const state = getConversationState(conversationId); + + expect(state.selectedTab).toBe("commits"); + }); + + it("filters obsolete tabs out of stored unpinnedTabs (editor / served / app / changes)", () => { + // Returning users may have unpinned the now-removed Editor, Served, + // App, or Diffs (`changes`) tabs in a previous version. Those names // should not survive the read — otherwise they linger forever in // localStorage since the UI has no way to surface them again to be - // re-pinned. We cover ALL four removed names here (the previous - // version of this test missed `app` and the gap let a denylist-vs- - // whitelist regression slip through review). + // re-pinned. const conversationId = "conv-123"; const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; @@ -238,8 +263,7 @@ describe("conversation localStorage utilities", () => { const state = getConversationState(conversationId); - // Only the still-valid `terminal` entry survives; all four - // obsolete names are dropped. + // Obsolete names are dropped; still-valid `terminal` stays. expect(state.unpinnedTabs).toEqual(["terminal"]); }); }); @@ -537,53 +561,19 @@ describe("conversation localStorage utilities", () => { }); }); - describe("filesTabDiffView persistence", () => { - // The diff-view toggle is per-conversation: in a git repo it - // defaults to ON, in a plain workspace it defaults to OFF, but the - // user's last explicit choice should win. Verify the boolean - // round-trips through localStorage and that the unset case stays - // `null` (so the higher layer can apply the repo-aware default). - - it("defaults to null when nothing is stored", () => { - const state = getConversationState("files-diff-conv-1"); - expect(state.filesTabDiffView).toBeNull(); - }); - - it("round-trips `true` through localStorage", () => { - const conversationId = "files-diff-conv-2"; - setConversationState(conversationId, { filesTabDiffView: true }); + describe("filesTabDiffView preference", () => { + it("preserves filesTabDiffView from stored blobs on read", () => { + const conversationId = "files-diff-legacy"; + localStorage.setItem( + `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`, + JSON.stringify({ + selectedTab: "files", + filesTabDiffView: true, + }), + ); const state = getConversationState(conversationId); expect(state.filesTabDiffView).toBe(true); - - // Also verify the on-disk shape — important because the consumer - // code reads it back via `JSON.parse`, so a wrong-type value would - // be a silent regression. - const raw = localStorage.getItem( - `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`, - ); - expect(raw).not.toBeNull(); - expect(JSON.parse(raw as string).filesTabDiffView).toBe(true); - }); - - it("round-trips `false` through localStorage", () => { - const conversationId = "files-diff-conv-3"; - setConversationState(conversationId, { filesTabDiffView: false }); - - const state = getConversationState(conversationId); - expect(state.filesTabDiffView).toBe(false); - }); - - it("is isolated per conversation", () => { - setConversationState("files-diff-convA", { filesTabDiffView: true }); - setConversationState("files-diff-convB", { filesTabDiffView: false }); - - expect(getConversationState("files-diff-convA").filesTabDiffView).toBe( - true, - ); - expect(getConversationState("files-diff-convB").filesTabDiffView).toBe( - false, - ); }); }); @@ -658,4 +648,45 @@ describe("conversation localStorage utilities", () => { expect(state.filesTabContentViewMode).toBe("rich"); }); }); + + describe("files tab open-state / tree persistence", () => { + it("defaults to an expanded tree and no open files", () => { + const state = getConversationState("files-open-defaults"); + expect(state.filesTabTreeVisible).toBe(true); + expect(state.filesTabOpenPaths).toEqual([]); + expect(state.filesTabSelectedPath).toBeNull(); + }); + + it("round-trips tree visibility and open tabs", () => { + const conversationId = "files-open-roundtrip"; + setConversationState(conversationId, { + filesTabTreeVisible: false, + filesTabOpenPaths: ["README.md", "src/main.ts"], + filesTabSelectedPath: "src/main.ts", + }); + + const state = getConversationState(conversationId); + expect(state.filesTabTreeVisible).toBe(false); + expect(state.filesTabOpenPaths).toEqual(["README.md", "src/main.ts"]); + expect(state.filesTabSelectedPath).toBe("src/main.ts"); + }); + + it("sanitizes corrupt open-state fields", () => { + const conversationId = "files-open-corrupt"; + const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + localStorage.setItem( + key, + JSON.stringify({ + filesTabTreeVisible: "yes", + filesTabOpenPaths: ["ok.ts", 12, "", null], + filesTabSelectedPath: { path: "nope" }, + }), + ); + + const state = getConversationState(conversationId); + expect(state.filesTabTreeVisible).toBe(true); + expect(state.filesTabOpenPaths).toEqual(["ok.ts"]); + expect(state.filesTabSelectedPath).toBeNull(); + }); + }); }); diff --git a/__tests__/hooks/use-select-conversation-tab.test.ts b/__tests__/hooks/use-select-conversation-tab.test.ts index 2fa1bbcead..0d61701133 100644 --- a/__tests__/hooks/use-select-conversation-tab.test.ts +++ b/__tests__/hooks/use-select-conversation-tab.test.ts @@ -37,17 +37,16 @@ describe("useSelectConversationTab", () => { result.current.selectTab("files"); }); - // Assert: Panel should be open and tab selected (in-memory only). + // Assert: Panel should be open and tab selected. expect(useConversationStore.getState().selectedTab).toBe("files"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isRightPanelShown).toBe(true); - // Tab selection is persisted; the right-drawer open state is - // intentionally session-only and must NOT touch localStorage. const storedState = JSON.parse( localStorage.getItem(`conversation-state-${TEST_CONVERSATION_ID}`)!, ); expect(storedState.selectedTab).toBe("files"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should close panel when clicking the same active tab", () => { @@ -65,19 +64,14 @@ describe("useSelectConversationTab", () => { result.current.selectTab("files"); }); - // Assert: Panel should be closed (in-memory only). + // Assert: Panel should be closed and persisted. expect(useConversationStore.getState().hasRightPanelToggled).toBe(false); + expect(useConversationStore.getState().isRightPanelShown).toBe(false); - // The drawer-close shouldn't have written to localStorage at all - // (session-only behavior). If anything is persisted, it's just the - // pre-existing tab selection from earlier writes — never a - // `rightPanelShown` field. - const raw = localStorage.getItem( - `conversation-state-${TEST_CONVERSATION_ID}`, + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${TEST_CONVERSATION_ID}`)!, ); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + expect(storedState.rightPanelShown).toBe(false); }); it("should switch to different tab when panel is already open", () => { @@ -153,6 +147,77 @@ describe("useSelectConversationTab", () => { }); }); + describe("navigateToTab", () => { + it("always opens the panel even when isRightPanelShown is stale true", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: true, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToTab("files"); + }); + + expect(useConversationStore.getState().selectedTab).toBe("files"); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + + describe("navigateToChanges", () => { + it("opens the commits tab with Uncommitted requested", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: false, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + commitsAutoExpandSection: null, + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToChanges(); + }); + + expect(useConversationStore.getState().selectedTab).toBe("commits"); + expect(useConversationStore.getState().commitsAutoExpandSection).toBe( + "uncommitted", + ); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + + describe("navigateToCommits", () => { + it("opens the commits tab without requesting Uncommitted", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: false, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + commitsAutoExpandSection: "uncommitted", + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToCommits(); + }); + + expect(useConversationStore.getState().selectedTab).toBe("commits"); + expect( + useConversationStore.getState().commitsAutoExpandSection, + ).toBeNull(); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + describe("onTabChange", () => { it("should update both Zustand store and localStorage when changing tab", () => { // Arrange diff --git a/__tests__/routes/changes-tab.test.tsx b/__tests__/routes/changes-tab.test.tsx deleted file mode 100644 index 1891ab761e..0000000000 --- a/__tests__/routes/changes-tab.test.tsx +++ /dev/null @@ -1,132 +0,0 @@ -import { render, screen } from "@testing-library/react"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { describe, expect, it, vi } from "vitest"; -import { MemoryRouter } from "react-router"; -import { AxiosError } from "axios"; -import GitChanges from "#/routes/changes-tab"; -import { useUnifiedGetGitChanges } from "#/hooks/query/use-unified-get-git-changes"; -import { useAgentState } from "#/hooks/use-agent-state"; -import { AgentState } from "#/types/agent-state"; - -vi.mock("react-i18next", () => ({ - useTranslation: () => ({ - t: (key: string) => key, - }), -})); - -vi.mock("#/hooks/query/use-unified-get-git-changes"); -vi.mock("#/hooks/use-agent-state"); -vi.mock("#/hooks/use-conversation-id", () => ({ - useConversationId: () => ({ conversationId: "test-id" }), - useOptionalConversationId: () => ({ conversationId: "test-id" }), -})); - -const wrapper = ({ children }: { children: React.ReactNode }) => ( - - - {children} - - -); - -describe("Changes Tab", () => { - it("should show EmptyChangesMessage when there are no changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("DIFF_VIEWER$NO_CHANGES")).toBeInTheDocument(); - }); - - it("should not show EmptyChangesMessage when there are changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [{ path: "src/file.ts", status: "M" }], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect( - screen.queryByText("DIFF_VIEWER$NO_CHANGES"), - ).not.toBeInTheDocument(); - }); - - it("should render the Protip alongside the empty state when there are no changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("TIPS$PROTIP")).toBeInTheDocument(); - }); - - it("should hide the Protip when the git changes request errors", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: false, - isError: true, - error: new AxiosError("fatal: not a git repository"), - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.queryByText("TIPS$PROTIP")).not.toBeInTheDocument(); - expect( - screen.getByText("DIFF_VIEWER$NOT_A_GIT_REPO"), - ).toBeInTheDocument(); - }); - - it("should show the loading message while git changes are loading", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: true, - isFetching: true, - isSuccess: false, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("DIFF_VIEWER$LOADING")).toBeInTheDocument(); - }); -}); diff --git a/__tests__/routes/commits-tab.test.tsx b/__tests__/routes/commits-tab.test.tsx index 37872f092f..683d821970 100644 --- a/__tests__/routes/commits-tab.test.tsx +++ b/__tests__/routes/commits-tab.test.tsx @@ -55,10 +55,13 @@ describe("Commits Tab", () => { AgentServerGitService, "getCommitChanges", ); + const getGitChangesSpy = vi.spyOn(AgentServerGitService, "getGitChanges"); beforeEach(() => { getGitCommitsSpy.mockReset(); getCommitChangesSpy.mockReset(); + getGitChangesSpy.mockReset(); + getGitChangesSpy.mockResolvedValue([]); vi.mocked(useAgentState).mockReturnValue({ curAgentState: AgentState.RUNNING, }); @@ -112,6 +115,22 @@ describe("Commits Tab", () => { expect(await screen.findByText("add logging")).toBeInTheDocument(); expect(screen.getByText("fix tests")).toBeInTheDocument(); expect(screen.getByText("aaaaaaa")).toBeInTheDocument(); + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + }); + + it("shows Uncommitted alone when there are working-tree changes but no commits", async () => { + // Arrange + getGitCommitsSpy.mockResolvedValue({ commits: [], hasMore: false }); + getGitChangesSpy.mockResolvedValue([{ path: "src/a.ts", status: "M" }]); + + // Act + render(, { wrapper }); + + // Assert + expect( + await screen.findByTestId("uncommitted-changes-row"), + ).toBeInTheDocument(); + expect(screen.queryByTestId("commit-row")).not.toBeInTheDocument(); }); it("expanding a commit fetches and lists the files it changed", async () => { diff --git a/__tests__/routes/files-tab.test.tsx b/__tests__/routes/files-tab.test.tsx index 0cbaed3638..27afdf3f24 100644 --- a/__tests__/routes/files-tab.test.tsx +++ b/__tests__/routes/files-tab.test.tsx @@ -1,5 +1,4 @@ -/* eslint-disable react/jsx-props-no-spreading */ -import { render, screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { describe, it, expect, vi, beforeEach } from "vitest"; @@ -8,28 +7,15 @@ import { MemoryRouter } from "react-router"; import FilesTab from "#/routes/files-tab"; import { useFilesTabStore } from "#/stores/files-tab-store"; import { NavigationProvider } from "#/context/navigation-context"; +import { + LOCAL_STORAGE_KEYS, + setConversationState, +} from "#/utils/conversation-local-storage"; // Mocks must be declared before the SUT is imported. -const useHasAttachedSourceMock = vi.fn(); -const useHasGitCommitsMock = vi.fn(); -const useUnifiedGitCommitsMock = vi.fn(); const useWorkspaceFilesMock = vi.fn(); const useWorkspaceFileContentMock = vi.fn(); const useActiveConversationMock = vi.fn(); -const refetchGitChangesMock = vi.fn(); - -vi.mock("#/hooks/use-has-attached-source", () => ({ - useHasAttachedSource: () => useHasAttachedSourceMock(), -})); - -vi.mock("#/hooks/query/use-has-git-commits", () => ({ - useHasGitCommits: (opts?: { enabled?: boolean }) => - useHasGitCommitsMock(opts), -})); - -vi.mock("#/hooks/query/use-unified-git-commits", () => ({ - useUnifiedGitCommits: () => useUnifiedGitCommitsMock(), -})); vi.mock("#/hooks/query/use-workspace-files", () => ({ useWorkspaceFiles: () => useWorkspaceFilesMock(), @@ -44,21 +30,6 @@ vi.mock("#/hooks/query/use-active-conversation", () => ({ useActiveConversation: () => useActiveConversationMock(), })); -vi.mock("#/hooks/query/use-unified-get-git-changes", () => ({ - useUnifiedGetGitChanges: () => ({ - refetch: refetchGitChangesMock, - isFetching: false, - }), -})); - -vi.mock("#/routes/changes-tab", () => ({ - default: () =>
Diff View
, -})); - -vi.mock("#/routes/commits-tab", () => ({ - default: () =>
Commits View
, -})); - function renderTab(conversationId: string | null = null) { const client = new QueryClient({ defaultOptions: { queries: { retry: false } }, @@ -81,43 +52,22 @@ function renderTab(conversationId: string | null = null) { ); } +function openFile(path: string, conversationId: string | null = null) { + useFilesTabStore.getState().setSelectedPath(path, conversationId); +} + describe("FilesTab", () => { beforeEach(() => { - // `selectedPath` lives in a global Zustand store (useFilesTabStore) and - // the auto-select effect re-fires when the store is reset between tests, - // which can race with the Zustand mock's afterEach reset and leave the - // store polluted with the previous test's path. Resetting here, after - // the previous test's cleanup() has unmounted any FilesTab, defeats - // that race so each test starts with a clean selection. useFilesTabStore.setState({ selectedPath: null, selectedConversationId: null, + openPaths: [], }); + localStorage.clear(); - useHasAttachedSourceMock.mockReset(); - useHasGitCommitsMock.mockReset(); - useUnifiedGitCommitsMock.mockReset(); useWorkspaceFilesMock.mockReset(); useWorkspaceFileContentMock.mockReset(); useActiveConversationMock.mockReset(); - refetchGitChangesMock.mockReset(); - // Default: pretend the probe has already resolved with at least one - // commit. Individual tests can override this for "empty repo" cases. - useHasGitCommitsMock.mockReturnValue({ - hasCommits: true, - isLoading: false, - }); - // Default: the agent server supports the commits API (the third toggle - // segment is offered) but the conversation has no commits yet. - useUnifiedGitCommitsMock.mockReturnValue({ - commits: [], - hasMore: false, - isUnsupported: false, - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - }); useWorkspaceFilesMock.mockReturnValue({ data: ["index.html", "src/main.ts", "README.md"], @@ -142,93 +92,29 @@ describe("FilesTab", () => { }); }); - it("defaults to diff view when the user attached a source (repo or workspace)", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); - + it("renders the file browser without a Diff/Commits toggle", () => { renderTab(); - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); - // The Rich/Plain toggle is hidden when diff view is active. + expect(screen.getByTestId("files-tab")).toBeInTheDocument(); expect( screen.queryByTestId("files-tab-content-mode-toggle"), ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("files-tab-diff-toggle"), + ).not.toBeInTheDocument(); }); - it("defaults to files+rich view when the attached source has no commits (non-git workspace or unborn HEAD)", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); - useHasGitCommitsMock.mockReturnValue({ - hasCommits: false, - isLoading: false, - }); - + it("does not open file tabs until a file is selected", () => { renderTab(); - // Even though something is attached, the diff view is suppressed when - // there's nothing to diff against. - expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument(); + expect(useWorkspaceFileContentMock).toHaveBeenCalledWith(null); + expect(screen.queryByRole("tab")).not.toBeInTheDocument(); expect( - screen.getByTestId("files-tab-content-mode-toggle"), + screen.getByTestId("file-quick-row-tree-toggle"), ).toBeInTheDocument(); }); - it("does NOT probe for commits when no source is attached", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - - renderTab(); - - // The hook is still called (so the diff toggle has a value), but it - // must be called with enabled: false so we don't shell out to the - // workspace pointlessly. - expect(useHasGitCommitsMock).toHaveBeenCalledWith({ enabled: false }); - }); - - it("optimistically defaults to diff view while the attachment / has-commits probes are still loading", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); - useHasGitCommitsMock.mockReturnValue({ - hasCommits: null, - isLoading: true, - }); - - renderTab(); - - // The common case is a repo with commits, so to avoid a files→diff - // flash on initial mount we lean diff-view while loading. - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); - }); - - it("defaults to plain file viewer when no source is attached", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - - renderTab(); - - expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument(); - // Tree is collapsed by default — user expands via the caret. - expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); - expect( - screen.getByTestId("files-tab-content-mode-toggle"), - ).toBeInTheDocument(); - }); - - it("shows the active conversation workspace path in files view", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); + it("shows the active conversation workspace path", () => { useActiveConversationMock.mockReturnValue({ data: { workspace: { working_dir: "/workspace/project/worktree-123" }, @@ -242,48 +128,44 @@ describe("FilesTab", () => { ).toHaveTextContent("/workspace/project/worktree-123"); }); - it("lets users toggle diff view off even when a source is attached", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); + it("opens a tab when a file is selected and closes it from the tab strip", async () => { const user = userEvent.setup(); - + openFile("src/main.ts"); renderTab(); - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); - - // Click the "Files" segment of the diff-view toggle. - await user.click(screen.getByTestId("files-tab-diff-toggle-option-off")); - - await waitFor(() => { - expect( - screen.queryByTestId("changes-tab-content"), - ).not.toBeInTheDocument(); - }); - // Quick-row toggle exists and the file-viewer area is shown. expect( - screen.getByTestId("file-quick-row-tree-toggle"), + screen.getByTestId("file-quick-row-item-src/main.ts"), ).toBeInTheDocument(); + expect(screen.getByRole("tab", { selected: true })).toHaveTextContent( + "main.ts", + ); + + await user.click(screen.getByTestId("file-quick-row-close-src/main.ts")); + + expect( + screen.queryByTestId("file-quick-row-item-src/main.ts"), + ).not.toBeInTheDocument(); + expect(useFilesTabStore.getState().selectedPath).toBeNull(); + expect(useFilesTabStore.getState().openPaths).toEqual([]); }); - it("auto-selects the highest-priority file on first render", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - + it("keeps vertical edges on every open tab", () => { + openFile("README.md"); + openFile("src/main.ts"); renderTab(); - // Either index.html (top-priority entrypoint) should be selected. - expect(useWorkspaceFileContentMock).toHaveBeenCalledWith("index.html"); + const firstTab = screen.getByTestId( + "file-quick-row-item-README.md", + ).parentElement; + const secondTab = screen.getByTestId( + "file-quick-row-item-src/main.ts", + ).parentElement; + expect(firstTab).toHaveClass("border-l"); + expect(firstTab).toHaveClass("border-r"); + expect(secondTab).toHaveClass("border-r"); }); it("renders the binary fallback in plain mode for binary files", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFileContentMock.mockReturnValue({ data: { path: "logo.png", @@ -298,6 +180,7 @@ describe("FilesTab", () => { }); const user = userEvent.setup(); + openFile("logo.png"); renderTab(); await user.click( @@ -309,45 +192,56 @@ describe("FilesTab", () => { ).toBeInTheDocument(); }); - it("shows full file paths (not just basenames) as quick-row pills", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - + it("shows the file name (not the full path) on quick-row tabs", () => { + openFile("src/main.ts"); renderTab(); - // The pill for src/main.ts should display the full relative path. - const pill = screen.getByTestId("file-quick-row-item-src/main.ts"); - expect(pill).toHaveTextContent("src/main.ts"); + const tab = screen.getByTestId("file-quick-row-item-src/main.ts"); + expect(tab).toHaveTextContent("main.ts"); + expect(tab).toHaveAttribute("title", "src/main.ts"); + expect(tab).toHaveAttribute("role", "tab"); }); - it("collapses the file tree by default and expands it via the caret", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); + it("shows the file tree by default and collapses it via the caret", async () => { const user = userEvent.setup(); renderTab(); - // Hidden by default. - expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); - - await user.click(screen.getByTestId("file-quick-row-tree-toggle")); expect(screen.getByTestId("files-tab-tree")).toBeInTheDocument(); await user.click(screen.getByTestId("file-quick-row-tree-toggle")); expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); + + await user.click(screen.getByTestId("file-quick-row-tree-toggle")); + expect(screen.getByTestId("files-tab-tree")).toBeInTheDocument(); + }); + + it("exposes a grippable resize handle on the tree's right edge when expanded", () => { + window.localStorage.clear(); + + renderTab(); + + expect( + screen.getByTestId("files-tab-tree-resize-handle"), + ).toBeInTheDocument(); + expect(screen.getByTestId("files-tab-tree")).toHaveStyle({ + width: "224px", + }); + }); + + it("opens a tab from the file tree when a file is clicked", async () => { + const user = userEvent.setup(); + renderTab(); + + await user.click(screen.getByTestId("file-tree-file-README.md")); + + expect(useFilesTabStore.getState().openPaths).toContain("README.md"); + expect( + screen.getByTestId("file-quick-row-item-README.md"), + ).toBeInTheDocument(); }); it("renders markdown content via MarkdownRenderer in rich mode", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - // Only expose a markdown file so it is auto-selected as the first - // priority entry. useWorkspaceFilesMock.mockReturnValue({ data: ["README.md"], isLoading: false, @@ -365,6 +259,7 @@ describe("FilesTab", () => { isError: false, }); + openFile("README.md"); renderTab(); await waitFor(() => { @@ -373,26 +268,13 @@ describe("FilesTab", () => { ).toBeInTheDocument(); }); - // react-markdown turns "# Hello" into an

. expect( screen.getByRole("heading", { level: 1, name: "Hello" }), ).toBeInTheDocument(); expect(screen.getByText("bold").tagName.toLowerCase()).toBe("strong"); - // Markdown rendering uses MarkdownRenderer, not an iframe. - expect( - screen.queryByTestId("file-content-viewer-iframe"), - ).not.toBeInTheDocument(); - // The rich-rendered markdown container is mounted. - expect( - screen.getByTestId("file-content-viewer-markdown"), - ).toBeInTheDocument(); }); it("shows highlighted source (not rich markdown) when toggled to plain on a .md", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFilesMock.mockReturnValue({ data: ["README.md"], isLoading: false, @@ -411,10 +293,9 @@ describe("FilesTab", () => { }); const user = userEvent.setup(); + openFile("README.md"); renderTab(); - // Toggle to plain — markdown source should now be syntax-highlighted - // as `markdown`, not rendered. await user.click( screen.getByTestId("files-tab-content-mode-toggle-option-plain"), ); @@ -423,17 +304,12 @@ describe("FilesTab", () => { "file-content-viewer-highlighted", ); expect(highlighted.getAttribute("data-language")).toBe("markdown"); - // Confirm the rich-rendered

is gone. expect( screen.queryByRole("heading", { level: 1, name: "Hello" }), ).not.toBeInTheDocument(); }); it("uses the workspace fileserver URL as the iframe src for HTML files", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFilesMock.mockReturnValue({ data: ["index.html"], isLoading: false, @@ -452,31 +328,15 @@ describe("FilesTab", () => { isError: false, }); + openFile("index.html"); renderTab(); const iframe = await screen.findByTestId("file-content-viewer-iframe"); - expect(iframe).toBeInTheDocument(); - // The iframe src points at the workspace fileserver so relative - // asset references (`` etc.) resolve to - // sibling files. The `?v=` suffix is the - // cache-buster appended by the viewer so the browser re-fetches - // after each agent-side edit. expect(iframe).toHaveAttribute("src", `${staticUrl}?v=0`); - // The iframe is sandboxed with `allow-same-origin` only: `