From 7ea79e0a3952597bfeb725ff765321affa3c87b4 Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Sun, 10 May 2026 19:29:16 -0700 Subject: [PATCH] feat(files-tab): Files tab with diff + rich/plain file viewer, safe-HTML markdown (#284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add Files tab with file viewer and diff view modes Replaces the previous Changes and App (served-host) tabs with a unified Files tab that supports two top-level modes: - Diff View (default when working inside a git repo): renders the existing Changes UI in-place. - File Viewer: shows a quick-access row of the most important files (index.html, package.json, README.md, etc.) with an overflow dropdown, plus an on-demand expandable file tree. Selecting a file shows its content with a Rich/Plain toggle. Rich mode renders HTML, markdown, and images in a sandboxed iframe; Plain mode shows plaintext with a binary-fallback message. Includes a new useIsGitRepo hook and supporting workspace-files hooks plus utilities for sorting files by priority and building a tree from flat paths. Co-authored-by: openhands * fix(files-tab): only default to diff view when a repo was explicitly attached The previous detection relied on whether 'git status' succeeded against the workspace, but the agent-server initialises every workspace as an internal git worktree for change tracking. As a result, a brand-new conversation with no user-attached repo was incorrectly treated as a git repo and the Files tab opened in diff view. Drop the filesystem probe and use the conversation's 'selected_repository' as the sole signal — that's the field populated by the repo picker for 'an existing git repo' from the user's point of view. Co-authored-by: openhands * feat(files-tab): refresh button covers file list, auto-refresh on edits Two related fixes to the Files tab data lifecycle: 1. The toolbar refresh button used to only refetch git changes (the diff view). It now also invalidates the workspace file list and any cached file contents, so clicking it works as expected in both modes. 2. Add a useAutoRefreshFilesOnEdit hook mounted by FilesTab that watches the conversation event store and invalidates the workspace-files, workspace-file-content and file_changes queries whenever the agent produces a mutating file-editor observation (create / str_replace / insert / undo_edit). Read-only 'view' commands and non-file observations are ignored. The hook is array-position based so it processes each event exactly once. Tests: 4 new for the hook, all existing files-tab and conversation-tabs tests still pass. Co-authored-by: openhands * feat(files-tab): tree toggle, full paths, real markdown rendering UI changes to the workspace file viewer: - Replace the trailing 'more files' overflow dropdown with a single caret button on the LEFT of the quick-access row that toggles the left-hand file tree. Tree is shown by default; users who want more horizontal space for the content pane can collapse it. There is no longer a dropdown listing extra files — anything that doesn't fit in the pills row is reachable by opening the tree. - Pills in the quick-access row now display the full relative file path (e.g. 'src/main.ts') instead of just the basename, so users can distinguish between same-named files in different folders at a glance. The full path also serves as the tooltip. - Markdown files are now rendered via the existing MarkdownRenderer (react-markdown + remark-gfm + remark-breaks) inside a styled prose container. The old approach piped raw text into a sandboxed '; + const { container } = render({md}); + expect(container.querySelector("iframe")).toBeNull(); + }); + + it("strips raw HTML when allowHtml=false", () => { + const md = "Hello world"; + const { container } = render( + {md}, + ); + // should not be parsed; the text should still appear. + expect(container.querySelector("mark")).toBeNull(); + expect(container.textContent).toContain("world"); + }); +}); + +// Direct tests against MARKDOWN_SANITIZE_SCHEMA. End-to-end +// MarkdownRenderer tests can't reach these because our custom `anchor` +// component always hard-codes target/rel — so even a buggy schema (one +// that strips `rel` from HAST) would still produce a safe-looking final +// ``. We run `hast-util-sanitize` directly on hand-built HAST trees +// to assert what the schema does and doesn't pass through. +describe("MARKDOWN_SANITIZE_SCHEMA", () => { + function makeAnchor(properties: Record): Root { + return { + type: "root", + children: [ + { + type: "element", + tagName: "a", + properties, + children: [{ type: "text", value: "link" }], + } as Element, + ], + }; + } + + function firstAnchor(tree: Root): Element | null { + const node = tree.children[0]; + return node && node.type === "element" ? (node as Element) : null; + } + + it("preserves space-separated rel values on raw HTML anchors (regression for fc208bc)", () => { + // The old schema used `["rel", "noopener", "noreferrer", "nofollow"]`, + // which is rehype-sanitize's "exact match against allowed values" + // form — it would reject `rel="noopener noreferrer"` (the canonical + // safe-link incantation) because the *combined* string isn't in the + // allowed-values list. With the fix this test must pass: rel is + // preserved verbatim. + const tree = sanitize( + makeAnchor({ + href: "https://example.com", + target: "_blank", + rel: "noopener noreferrer", + }), + MARKDOWN_SANITIZE_SCHEMA, + ) as Root; + + const a = firstAnchor(tree); + expect(a).not.toBeNull(); + // hast-util-sanitize stores `rel` as an array of tokens; reassemble. + const relProp = a?.properties?.rel; + const rel = Array.isArray(relProp) ? relProp.join(" ") : relProp; + expect(rel).toBe("noopener noreferrer"); + expect(a?.properties?.target).toBe("_blank"); + expect(a?.properties?.href).toBe("https://example.com"); + }); + + it("preserves rel even when it carries unusual but-safe tokens like `nofollow ugc`", () => { + // `rel` keywords never execute code or navigate, so allowing any + // value is safe. This locks that property in. + const tree = sanitize( + makeAnchor({ + href: "https://example.com", + rel: "nofollow ugc", + }), + MARKDOWN_SANITIZE_SCHEMA, + ) as Root; + + const a = firstAnchor(tree); + const relProp = a?.properties?.rel; + const rel = Array.isArray(relProp) ? relProp.join(" ") : relProp; + expect(rel).toBe("nofollow ugc"); + }); +}); diff --git a/__tests__/conversation-local-storage.test.ts b/__tests__/conversation-local-storage.test.ts index 33e9e12a7e..9b285d3da8 100644 --- a/__tests__/conversation-local-storage.test.ts +++ b/__tests__/conversation-local-storage.test.ts @@ -29,7 +29,7 @@ describe("conversation localStorage utilities", () => { const state = getConversationState("task-uuid-123"); expect(state.conversationMode).toBe("code"); - expect(state.selectedTab).toBe("editor"); + expect(state.selectedTab).toBe("files"); expect(state.rightPanelShown).toBe(true); expect( localStorage.getItem( @@ -115,7 +115,7 @@ describe("conversation localStorage utilities", () => { const state = getConversationState(conversationId); expect(state.subConversationTaskId).toBeNull(); - expect(state.selectedTab).toBe("editor"); + expect(state.selectedTab).toBe("files"); expect(state.rightPanelShown).toBe(true); expect(state.unpinnedTabs).toEqual([]); }); @@ -154,10 +154,56 @@ describe("conversation localStorage utilities", () => { const state = getConversationState(conversationId); expect(state.subConversationTaskId).toBe("task-123"); - expect(state.selectedTab).toBe("editor"); + expect(state.selectedTab).toBe("files"); expect(state.rightPanelShown).toBe(true); expect(state.unpinnedTabs).toEqual([]); }); + + it("falls back to the default tab when stored selectedTab is no longer valid", () => { + const conversationId = "conv-123"; + const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + + // Persisted from a previous app version where "editor" was a tab. + localStorage.setItem( + consolidatedKey, + JSON.stringify({ + selectedTab: "editor", + rightPanelShown: true, + unpinnedTabs: [], + }), + ); + + const state = getConversationState(conversationId); + + 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 + // 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). + const conversationId = "conv-123"; + const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + + localStorage.setItem( + consolidatedKey, + JSON.stringify({ + selectedTab: "files", + rightPanelShown: true, + unpinnedTabs: ["editor", "changes", "served", "app", "terminal"], + }), + ); + + const state = getConversationState(conversationId); + + // Only the still-valid `terminal` entry survives; all four + // obsolete names are dropped. + expect(state.unpinnedTabs).toEqual(["terminal"]); + }); }); describe("setConversationState", () => { @@ -185,7 +231,7 @@ describe("conversation localStorage utilities", () => { localStorage.setItem( consolidatedKey, JSON.stringify({ - selectedTab: "changes", + selectedTab: "browser", rightPanelShown: false, unpinnedTabs: ["tab-1"], subConversationTaskId: "old-task-id", @@ -201,7 +247,7 @@ describe("conversation localStorage utilities", () => { const parsed = JSON.parse(stored!); expect(parsed.subConversationTaskId).toBe("new-task-id"); - expect(parsed.selectedTab).toBe("changes"); + expect(parsed.selectedTab).toBe("browser"); expect(parsed.rightPanelShown).toBe(false); expect(parsed.unpinnedTabs).toEqual(["tab-1"]); }); @@ -456,4 +502,110 @@ 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 }); + + 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); + }); + }); + + describe("filesTabContentViewMode persistence", () => { + // The rich/plain toggle for the file content viewer also persists + // per conversation. Default is "rich" — verified explicitly here so + // a careless change to the default field initializer doesn't slip + // through unnoticed (it would flip every existing user from rich to + // plain after deploy). + + it("defaults to 'rich' when nothing is stored", () => { + const state = getConversationState("files-view-conv-1"); + expect(state.filesTabContentViewMode).toBe("rich"); + }); + + it("round-trips 'plain' through localStorage", () => { + const conversationId = "files-view-conv-2"; + setConversationState(conversationId, { + filesTabContentViewMode: "plain", + }); + + expect( + getConversationState(conversationId).filesTabContentViewMode, + ).toBe("plain"); + + const raw = localStorage.getItem( + `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`, + ); + expect(JSON.parse(raw as string).filesTabContentViewMode).toBe("plain"); + }); + + it("round-trips 'rich' through localStorage (explicit save, not default)", () => { + const conversationId = "files-view-conv-3"; + setConversationState(conversationId, { + filesTabContentViewMode: "rich", + }); + + expect( + getConversationState(conversationId).filesTabContentViewMode, + ).toBe("rich"); + }); + + it("is isolated per conversation", () => { + setConversationState("files-view-convA", { + filesTabContentViewMode: "plain", + }); + setConversationState("files-view-convB", { + filesTabContentViewMode: "rich", + }); + + expect( + getConversationState("files-view-convA").filesTabContentViewMode, + ).toBe("plain"); + expect( + getConversationState("files-view-convB").filesTabContentViewMode, + ).toBe("rich"); + }); + }); }); diff --git a/__tests__/hooks/query/use-workspace-session.test.tsx b/__tests__/hooks/query/use-workspace-session.test.tsx new file mode 100644 index 0000000000..88dae68702 --- /dev/null +++ b/__tests__/hooks/query/use-workspace-session.test.tsx @@ -0,0 +1,193 @@ +import React from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { + joinWorkspaceUrl, + useWorkspaceSession, +} from "#/hooks/query/use-workspace-session"; + +// We mock the workspace factory rather than the lower-level HttpClient: +// that's where our wiring contract lives (we hand the typescript-client a +// conversation id and trust it to do the right POST + return a base URL). +const startWorkspaceSessionMock = vi.fn(); +const createRemoteWorkspaceMock = vi.fn(); + +vi.mock("#/api/typescript-client", async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createRemoteWorkspace: (...args: unknown[]) => { + createRemoteWorkspaceMock(...args); + return { + startWorkspaceSession: startWorkspaceSessionMock, + }; + }, + }; +}); + +const useActiveConversationMock = vi.fn(); +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => useActiveConversationMock(), +})); + +const useRuntimeIsReadyMock = vi.fn(); +vi.mock("#/hooks/use-runtime-is-ready", () => ({ + useRuntimeIsReady: () => useRuntimeIsReadyMock(), +})); + +function makeWrapper() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const Wrapper = function WorkspaceSessionTestWrapper({ + children, + }: { + children: React.ReactNode; + }) { + return ( + {children} + ); + }; + return Wrapper; +} + +// Yields back to the event loop a few microtasks deep so react-query has a +// chance to schedule (and, in the negative-path tests, to NOT schedule) the +// query. ESLint forbids returning the timer id from `new Promise(...)`, so +// we wrap setTimeout in a void callback. +function flushScheduler(ms = 10): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +beforeEach(() => { + startWorkspaceSessionMock.mockReset(); + createRemoteWorkspaceMock.mockReset(); + useActiveConversationMock.mockReset(); + useRuntimeIsReadyMock.mockReset(); + useRuntimeIsReadyMock.mockReturnValue(true); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("useWorkspaceSession", () => { + it("calls startWorkspaceSession and exposes the returned baseUrl", async () => { + useActiveConversationMock.mockReturnValue({ + data: { + id: "conv-1", + conversation_url: "https://agent.example.com/api/conversations/conv-1", + session_api_key: "key-abc", + }, + }); + startWorkspaceSessionMock.mockResolvedValue( + "https://agent.example.com/api/conversations/conv-1/workspace/", + ); + + const { result } = renderHook(() => useWorkspaceSession(), { + wrapper: makeWrapper(), + }); + + await waitFor(() => { + expect(result.current.data?.baseUrl).toBe( + "https://agent.example.com/api/conversations/conv-1/workspace/", + ); + }); + + expect(createRemoteWorkspaceMock).toHaveBeenCalledTimes(1); + expect(createRemoteWorkspaceMock).toHaveBeenCalledWith({ + conversationUrl: "https://agent.example.com/api/conversations/conv-1", + sessionApiKey: "key-abc", + }); + expect(startWorkspaceSessionMock).toHaveBeenCalledTimes(1); + expect(startWorkspaceSessionMock).toHaveBeenCalledWith("conv-1"); + }); + + it("does not call startWorkspaceSession until the runtime is ready", async () => { + useActiveConversationMock.mockReturnValue({ + data: { + id: "conv-1", + conversation_url: "https://agent.example.com/api/conversations/conv-1", + session_api_key: "key-abc", + }, + }); + useRuntimeIsReadyMock.mockReturnValue(false); + + const { result } = renderHook(() => useWorkspaceSession(), { + wrapper: makeWrapper(), + }); + + // Give react-query a tick to schedule (it shouldn't). + await flushScheduler(); + expect(startWorkspaceSessionMock).not.toHaveBeenCalled(); + expect(result.current.data).toBeNull(); + }); + + it("does not call startWorkspaceSession without a conversation id", async () => { + useActiveConversationMock.mockReturnValue({ data: undefined }); + + renderHook(() => useWorkspaceSession(), { wrapper: makeWrapper() }); + + await flushScheduler(); + expect(startWorkspaceSessionMock).not.toHaveBeenCalled(); + }); + + it("surfaces the error when the workspace-session POST fails", async () => { + useActiveConversationMock.mockReturnValue({ + data: { + id: "conv-1", + conversation_url: "https://agent.example.com/api/conversations/conv-1", + session_api_key: "bad-key", + }, + }); + startWorkspaceSessionMock.mockRejectedValue(new Error("401 Unauthorized")); + + const { result } = renderHook(() => useWorkspaceSession(), { + wrapper: makeWrapper(), + }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(result.current.error?.message).toMatch(/401/); + expect(result.current.data).toBeNull(); + }); +}); + +describe("joinWorkspaceUrl", () => { + const base = "https://agent.example.com/api/conversations/c1/workspace/"; + + it("returns the base URL when no relative path is supplied", () => { + expect(joinWorkspaceUrl(base)).toBe(base); + expect(joinWorkspaceUrl(base, "")).toBe(base); + expect(joinWorkspaceUrl(base, null)).toBe(base); + }); + + it("appends a single-segment path", () => { + expect(joinWorkspaceUrl(base, "index.html")).toBe(`${base}index.html`); + }); + + it("appends nested paths preserving separators", () => { + expect(joinWorkspaceUrl(base, "src/components/App.tsx")).toBe( + `${base}src/components/App.tsx`, + ); + }); + + it("strips leading slashes on the relative path", () => { + expect(joinWorkspaceUrl(base, "/index.html")).toBe(`${base}index.html`); + expect(joinWorkspaceUrl(base, "///deep/path.md")).toBe( + `${base}deep/path.md`, + ); + }); + + it("URL-encodes individual segments but not the separators", () => { + expect(joinWorkspaceUrl(base, "my files/has spaces.txt")).toBe( + `${base}my%20files/has%20spaces.txt`, + ); + expect(joinWorkspaceUrl(base, "tëst/résumé.pdf")).toBe( + `${base}t%C3%ABst/r%C3%A9sum%C3%A9.pdf`, + ); + }); +}); diff --git a/__tests__/hooks/use-auto-refresh-files-on-edit.test.tsx b/__tests__/hooks/use-auto-refresh-files-on-edit.test.tsx new file mode 100644 index 0000000000..f68d7cf7dc --- /dev/null +++ b/__tests__/hooks/use-auto-refresh-files-on-edit.test.tsx @@ -0,0 +1,377 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import React from "react"; + +import { useAutoRefreshFilesOnEdit } from "#/hooks/use-auto-refresh-files-on-edit"; +import { useEventStore } from "#/stores/use-event-store"; +import type { OHEvent } from "#/stores/use-event-store"; +import { useWorkspaceMutationCounter } from "#/stores/use-workspace-mutation-counter"; + +function makeWrapper(client: QueryClient) { + return ({ children }: { children: React.ReactNode }) => ( + {children} + ); +} + +function makeObservationEvent( + id: string, + kind: string, + command: string, +): OHEvent { + return { + id, + timestamp: new Date(Date.now() + Number(id.replace(/\D/g, "")) * 1000) + .toISOString(), + source: "environment", + tool_name: "str_replace_based_edit_tool", + tool_call_id: `tc-${id}`, + action_id: `act-${id}`, + observation: { + kind, + command, + path: "/workspace/project/foo.txt", + old_content: null, + new_content: "hello", + output: "ok", + }, + } as unknown as OHEvent; +} + +describe("useAutoRefreshFilesOnEdit", () => { + beforeEach(() => { + act(() => { + useEventStore.getState().clearEvents(); + // Reset the workspace mutation counter so per-test counter assertions + // don't see ticks bled over from earlier tests. + useWorkspaceMutationCounter.setState({ count: 0 }); + }); + }); + + it("invalidates workspace queries when a mutating file editor observation arrives", () => { + const client = new QueryClient(); + const spy = vi.spyOn(client, "invalidateQueries"); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + expect(spy).not.toHaveBeenCalled(); + + act(() => { + useEventStore + .getState() + .addEvent( + makeObservationEvent("1", "FileEditorObservation", "str_replace"), + ); + }); + + const invalidatedKeys = spy.mock.calls.map( + (call) => (call[0] as { queryKey: unknown[] }).queryKey[0], + ); + expect(invalidatedKeys).toContain("workspace-files"); + expect(invalidatedKeys).toContain("workspace-file-content"); + expect(invalidatedKeys).toContain("file_changes"); + }); + + it("ignores read-only `view` observations", () => { + const client = new QueryClient(); + const spy = vi.spyOn(client, "invalidateQueries"); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + act(() => { + useEventStore + .getState() + .addEvent(makeObservationEvent("1", "FileEditorObservation", "view")); + }); + + expect(spy).not.toHaveBeenCalled(); + }); + + it("ignores non-file observation kinds", () => { + const client = new QueryClient(); + const spy = vi.spyOn(client, "invalidateQueries"); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + act(() => { + useEventStore + .getState() + .addEvent( + makeObservationEvent("1", "ExecuteBashObservation", "ls"), + ); + }); + + expect(spy).not.toHaveBeenCalled(); + }); + + it("bumps the workspace mutation counter on each mutating observation so iframes / images cache-bust", () => { + const client = new QueryClient(); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + expect(useWorkspaceMutationCounter.getState().count).toBe(0); + + act(() => { + useEventStore + .getState() + .addEvent( + makeObservationEvent("1", "FileEditorObservation", "str_replace"), + ); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(1); + + act(() => { + useEventStore + .getState() + .addEvent( + makeObservationEvent( + "2", + "StrReplaceEditorObservation", + "create", + ), + ); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(2); + }); + + it("does NOT bump the workspace mutation counter for read-only / non-file observations", () => { + const client = new QueryClient(); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + act(() => { + useEventStore + .getState() + .addEvent(makeObservationEvent("1", "FileEditorObservation", "view")); + useEventStore + .getState() + .addEvent( + makeObservationEvent("2", "ExecuteBashObservation", "ls"), + ); + }); + + expect(useWorkspaceMutationCounter.getState().count).toBe(0); + }); + + it("still reacts to mutations that arrive out-of-order (older timestamp inserted between newer events)", () => { + // Regression test for a bug where the hook used `events.slice(processedCount)` + // to find new events. The event store re-sorts by timestamp on insert, + // so a late-arriving older event lands *between* two newer ones and + // the tail slice would miss it. + const client = new QueryClient(); + const spy = vi.spyOn(client, "invalidateQueries"); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + // First, push two newer events. The id-numbers drive the timestamp, + // so id "10" is later than id "5". Both land in the same effect run + // (we coalesce — one bump per batch, not per event), so count goes + // from 0 → 1. + act(() => { + useEventStore + .getState() + .addEvent(makeObservationEvent("10", "FileEditorObservation", "create")); + useEventStore + .getState() + .addEvent(makeObservationEvent("20", "FileEditorObservation", "create")); + }); + const callsAfterInitial = spy.mock.calls.length; + expect(callsAfterInitial).toBeGreaterThan(0); + const countAfterInitial = useWorkspaceMutationCounter.getState().count; + expect(countAfterInitial).toBe(1); + + // Now insert an OLDER event (id "5" → earliest timestamp). The store + // re-sorts so the events array becomes [e5, e10, e20]. The previous + // "slice from index 2" approach would return [e20] only and miss e5 + // entirely — no invalidation, no cache-bust, stale iframe. + act(() => { + useEventStore + .getState() + .addEvent(makeObservationEvent("5", "FileEditorObservation", "create")); + }); + + // We should have invalidated again and bumped the counter exactly once + // more for the late-arriving mutation (count: 1 → 2). + expect(spy.mock.calls.length).toBeGreaterThan(callsAfterInitial); + expect(useWorkspaceMutationCounter.getState().count).toBe( + countAfterInitial + 1, + ); + }); + + it("processes each id-less event distinctly (does NOT collapse them via an `undefined` Set key)", () => { + // The event store explicitly allows events without ids + // (`getEventId` returns undefined for them). If the hook keyed dedup + // on `event.id` naively, a single `undefined` entry in the Set would + // swallow every subsequent id-less event — silently dropping real + // mutations on the floor. + // + // Verifies via three SEPARATE act() calls (one per event) so each + // store mutation gets its own effect-flush. The counter bumps once + // per flush that found at least one new mutation; three flushes → + // counter ends at 3. Putting all three addEvent calls inside a + // single act() would batch them into one flush (counter=1) and + // verify nothing useful. + const client = new QueryClient(); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + // Three distinct id-less FileEditorObservation events (different + // timestamps so the store treats them as ordered, not duplicates). + const idlessEvent = (i: number): OHEvent => + ({ + // no `id` field at all → getEventId returns undefined + timestamp: new Date(2026, 0, 1, 0, 0, i).toISOString(), + source: "environment", + tool_name: "str_replace_based_edit_tool", + tool_call_id: `tc-idless-${i}`, + action_id: `act-idless-${i}`, + observation: { + kind: "FileEditorObservation", + command: "create", + path: `/workspace/project/foo${i}.txt`, + old_content: null, + new_content: "hello", + output: "ok", + }, + }) as unknown as OHEvent; + + act(() => { + useEventStore.getState().addEvent(idlessEvent(1)); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(1); + + act(() => { + useEventStore.getState().addEvent(idlessEvent(2)); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(2); + + act(() => { + useEventStore.getState().addEvent(idlessEvent(3)); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(3); + }); + + it("does NOT re-bump on subsequent renders for the same id-less event", () => { + // Companion to the previous test, targeting the *other* half of the + // id-less dedup contract: each id-less event must be processed + // exactly ONCE across the lifetime of the hook. Without + // reference-based dedup (`processedEventsRef` WeakSet) the events + // array — which is rebuilt on every store mutation but keeps stable + // element references — would cause the same id-less event to + // re-trigger the bump on every subsequent re-render, spamming + // cache invalidations. + const client = new QueryClient(); + + const { rerender } = renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + const idlessEvent: OHEvent = { + timestamp: new Date(2026, 0, 1, 0, 0, 0).toISOString(), + source: "environment", + tool_name: "str_replace_based_edit_tool", + tool_call_id: "tc-idless-stable", + action_id: "act-idless-stable", + observation: { + kind: "FileEditorObservation", + command: "create", + path: "/workspace/project/foo.txt", + old_content: null, + new_content: "hello", + output: "ok", + }, + } as unknown as OHEvent; + + act(() => { + useEventStore.getState().addEvent(idlessEvent); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(1); + + // Force several extra re-renders without adding new events. The + // id-less event still sits in the events array on every re-render, + // but the WeakSet dedup must prevent it from being re-processed. + rerender(); + rerender(); + rerender(); + expect(useWorkspaceMutationCounter.getState().count).toBe(1); + }); + + it("dedupes numeric event ids the same way as string ids", () => { + // The formal EventID type is `string`, but the event store carries + // `Set` defensively (use-event-store.ts:52) and + // `getEventId` returns `string | number | undefined`. The hook's + // processed-ids set is widened to match — a stray numeric id (legacy + // payload, hand-crafted test event, …) must still dedup correctly. + const client = new QueryClient(); + + renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + const numericEvent: OHEvent = { + id: 42 as unknown as string, // intentionally numeric at runtime + timestamp: new Date(2026, 0, 1, 0, 0, 1).toISOString(), + source: "environment", + tool_name: "str_replace_based_edit_tool", + tool_call_id: "tc-num", + action_id: "act-num", + observation: { + kind: "FileEditorObservation", + command: "create", + path: "/workspace/project/foo.txt", + old_content: null, + new_content: "hello", + output: "ok", + }, + } as unknown as OHEvent; + + act(() => { + useEventStore.getState().addEvent(numericEvent); + }); + const afterFirst = useWorkspaceMutationCounter.getState().count; + expect(afterFirst).toBe(1); + + // Re-adding the same numeric-id event must be a no-op for the + // counter (store dedups on id; hook must too). + act(() => { + useEventStore.getState().addEvent({ ...numericEvent }); + }); + expect(useWorkspaceMutationCounter.getState().count).toBe(afterFirst); + }); + + it("only invalidates once per new event batch", () => { + const client = new QueryClient(); + const spy = vi.spyOn(client, "invalidateQueries"); + + const { rerender } = renderHook(() => useAutoRefreshFilesOnEdit(), { + wrapper: makeWrapper(client), + }); + + act(() => { + useEventStore + .getState() + .addEvent(makeObservationEvent("1", "FileEditorObservation", "create")); + }); + + const callsAfterFirst = spy.mock.calls.length; + expect(callsAfterFirst).toBeGreaterThan(0); + + // Re-render without adding new events — should not re-invalidate. + rerender(); + expect(spy.mock.calls.length).toBe(callsAfterFirst); + }); +}); diff --git a/__tests__/hooks/use-draft-persistence.test.tsx b/__tests__/hooks/use-draft-persistence.test.tsx index 0734470324..8146a991ae 100644 --- a/__tests__/hooks/use-draft-persistence.test.tsx +++ b/__tests__/hooks/use-draft-persistence.test.tsx @@ -36,28 +36,34 @@ describe("useDraftPersistence", () => { // Default mock for useConversationLocalStorageState vi.mocked(conversationLocalStorage.useConversationLocalStorageState).mockReturnValue({ state: { - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }, setSelectedTab: vi.fn(), setRightPanelShown: vi.fn(), setUnpinnedTabs: vi.fn(), setConversationMode: vi.fn(), setDraftMessage: mockSetDraftMessage, + setFilesTabDiffView: vi.fn(), + setFilesTabContentViewMode: vi.fn(), }); // Default mock for getConversationState vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); }); @@ -74,12 +80,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef(); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: savedDraft, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); // Act @@ -97,12 +105,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef(existingContent); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: savedDraft, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); // Act @@ -118,12 +128,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef("Some stale content"); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); // Act @@ -206,27 +218,33 @@ describe("useDraftPersistence", () => { vi.mocked(conversationLocalStorage.useConversationLocalStorageState).mockReturnValue({ state: { - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: existingDraft, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }, setSelectedTab: vi.fn(), setRightPanelShown: vi.fn(), setUnpinnedTabs: vi.fn(), setConversationMode: vi.fn(), setDraftMessage: mockSetDraftMessage, + setFilesTabDiffView: vi.fn(), + setFilesTabContentViewMode: vi.fn(), }); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: existingDraft, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { result } = renderHook(() => @@ -306,20 +324,24 @@ describe("useDraftPersistence", () => { // First conversation has a draft vi.mocked(conversationLocalStorage.getConversationState) .mockReturnValueOnce({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: "Draft from conv A", + filesTabDiffView: null, + filesTabContentViewMode: "rich", }) .mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { rerender } = renderHook( @@ -342,20 +364,24 @@ describe("useDraftPersistence", () => { vi.mocked(conversationLocalStorage.getConversationState) .mockReturnValueOnce({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }) .mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: draftForConvB, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { rerender } = renderHook( @@ -409,12 +435,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef("Draft typed during init"); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { rerender } = renderHook( @@ -444,12 +472,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef(""); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { rerender } = renderHook( @@ -471,12 +501,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef("Some draft"); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); const { rerender } = renderHook( @@ -502,18 +534,22 @@ describe("useDraftPersistence", () => { vi.mocked(conversationLocalStorage.useConversationLocalStorageState).mockReturnValue({ state: { - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: "Existing draft", + filesTabDiffView: null, + filesTabContentViewMode: "rich", }, setSelectedTab: vi.fn(), setRightPanelShown: vi.fn(), setUnpinnedTabs: vi.fn(), setConversationMode: vi.fn(), setDraftMessage: mockSetDraftMessage, + setFilesTabDiffView: vi.fn(), + setFilesTabContentViewMode: vi.fn(), }); // Act @@ -545,12 +581,14 @@ describe("useDraftPersistence", () => { const chatInputRef = createMockChatInputRef(); vi.mocked(conversationLocalStorage.getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], conversationMode: "code", subConversationTaskId: null, draftMessage: "Draft to restore", + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); // Act diff --git a/__tests__/hooks/use-handle-plan-click.test.tsx b/__tests__/hooks/use-handle-plan-click.test.tsx index bc34cbc531..9cd54c9a4d 100644 --- a/__tests__/hooks/use-handle-plan-click.test.tsx +++ b/__tests__/hooks/use-handle-plan-click.test.tsx @@ -88,12 +88,14 @@ describe("useHandlePlanClick", () => { ); vi.mocked(getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], subConversationTaskId: null, conversationMode: "code", draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); }); @@ -118,12 +120,14 @@ describe("useHandlePlanClick", () => { ); vi.mocked(getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], subConversationTaskId: storedTaskId, conversationMode: "code", draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); renderHook(() => useHandlePlanClick()); @@ -157,12 +161,14 @@ describe("useHandlePlanClick", () => { ); vi.mocked(getConversationState).mockReturnValue({ - selectedTab: "editor", + selectedTab: "files", rightPanelShown: true, unpinnedTabs: [], subConversationTaskId: storedTaskId, conversationMode: "code", draftMessage: null, + filesTabDiffView: null, + filesTabContentViewMode: "rich", }); renderHook(() => useHandlePlanClick()); diff --git a/__tests__/hooks/use-select-conversation-tab.test.ts b/__tests__/hooks/use-select-conversation-tab.test.ts index d1857d511a..31ed15b210 100644 --- a/__tests__/hooks/use-select-conversation-tab.test.ts +++ b/__tests__/hooks/use-select-conversation-tab.test.ts @@ -33,11 +33,11 @@ describe("useSelectConversationTab", () => { // Act: Select a tab act(() => { - result.current.selectTab("editor"); + result.current.selectTab("files"); }); // Assert: Panel should be open and tab selected - expect(useConversationStore.getState().selectedTab).toBe("editor"); + expect(useConversationStore.getState().selectedTab).toBe("files"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); // Verify localStorage was updated @@ -46,14 +46,14 @@ describe("useSelectConversationTab", () => { `conversation-state-${TEST_CONVERSATION_ID}`, )!, ); - expect(storedState.selectedTab).toBe("editor"); + expect(storedState.selectedTab).toBe("files"); expect(storedState.rightPanelShown).toBe(true); }); it("should close panel when clicking the same active tab", () => { // Arrange: Panel is open with editor tab selected useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); @@ -62,7 +62,7 @@ describe("useSelectConversationTab", () => { // Act: Click the same tab again act(() => { - result.current.selectTab("editor"); + result.current.selectTab("files"); }); // Assert: Panel should be closed @@ -80,7 +80,7 @@ describe("useSelectConversationTab", () => { it("should switch to different tab when panel is already open", () => { // Arrange: Panel is open with editor tab selected useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); @@ -110,7 +110,7 @@ describe("useSelectConversationTab", () => { it("should return true when tab is selected and panel is visible", () => { // Arrange: Panel is open with editor tab selected useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); @@ -118,13 +118,13 @@ describe("useSelectConversationTab", () => { const { result } = renderHook(() => useSelectConversationTab()); // Assert: Editor tab should be active - expect(result.current.isTabActive("editor")).toBe(true); + expect(result.current.isTabActive("files")).toBe(true); }); it("should return false when tab is selected but panel is not visible", () => { // Arrange: Editor tab selected but panel is closed useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: false, hasRightPanelToggled: false, }); @@ -132,13 +132,13 @@ describe("useSelectConversationTab", () => { const { result } = renderHook(() => useSelectConversationTab()); // Assert: Editor tab should not be active - expect(result.current.isTabActive("editor")).toBe(false); + expect(result.current.isTabActive("files")).toBe(false); }); it("should return false when different tab is selected", () => { // Arrange: Panel is open with editor tab selected useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); @@ -181,7 +181,7 @@ describe("useSelectConversationTab", () => { it("should set tab to null when passing null", () => { // Arrange useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); @@ -224,7 +224,7 @@ describe("useSelectConversationTab", () => { it("should return current isRightPanelShown from store", () => { // Arrange useConversationStore.setState({ - selectedTab: "editor", + selectedTab: "files", isRightPanelShown: true, hasRightPanelToggled: true, }); diff --git a/__tests__/i18n/files-diff-label.test.ts b/__tests__/i18n/files-diff-label.test.ts new file mode 100644 index 0000000000..46c65e3006 --- /dev/null +++ b/__tests__/i18n/files-diff-label.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import fs from "fs"; +import path from "path"; + +// The Files-tab diff toggle was renamed from "Diff view" to just "Diff". +// Lock that down at the source-of-truth (translation.json) rather than the +// rendered label, because the test environment's i18next mock returns keys +// rather than translated strings. +describe("FILES$DIFF_VIEW label", () => { + const translationPath = path.join( + __dirname, + "../../src/i18n/translation.json", + ); + const translation = JSON.parse( + fs.readFileSync(translationPath, "utf-8"), + ) as Record>; + + it('uses "Diff" (not "Diff view") in English', () => { + expect(translation.FILES$DIFF_VIEW).toBeDefined(); + expect(translation.FILES$DIFF_VIEW.en).toBe("Diff"); + }); +}); diff --git a/__tests__/routes/files-tab.test.tsx b/__tests__/routes/files-tab.test.tsx new file mode 100644 index 0000000000..a7e3bfe540 --- /dev/null +++ b/__tests__/routes/files-tab.test.tsx @@ -0,0 +1,416 @@ +/* eslint-disable react/jsx-props-no-spreading */ +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"; +import { MemoryRouter } from "react-router"; + +import FilesTab from "#/routes/files-tab"; + +// Mocks must be declared before the SUT is imported. +const useIsGitRepoMock = vi.fn(); +const useHasGitCommitsMock = vi.fn(); +const useWorkspaceFilesMock = vi.fn(); +const useWorkspaceFileContentMock = vi.fn(); +const refetchGitChangesMock = vi.fn(); + +vi.mock("#/hooks/use-is-git-repo", () => ({ + useIsGitRepo: () => useIsGitRepoMock(), +})); + +vi.mock("#/hooks/query/use-has-git-commits", () => ({ + useHasGitCommits: (opts?: { enabled?: boolean }) => + useHasGitCommitsMock(opts), +})); + +vi.mock("#/hooks/query/use-workspace-files", () => ({ + useWorkspaceFiles: () => useWorkspaceFilesMock(), +})); + +vi.mock("#/hooks/query/use-workspace-file-content", () => ({ + useWorkspaceFileContent: (path: string | null) => + useWorkspaceFileContentMock(path), +})); + +vi.mock("#/hooks/query/use-unified-get-git-changes", () => ({ + useUnifiedGetGitChanges: () => ({ + refetch: refetchGitChangesMock, + isFetching: false, + }), +})); + +vi.mock("#/routes/changes-tab", () => ({ + default: () =>
Diff View
, +})); + +function renderTab() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + + + , + ); +} + +describe("FilesTab", () => { + beforeEach(() => { + useIsGitRepoMock.mockReset(); + useHasGitCommitsMock.mockReset(); + useWorkspaceFilesMock.mockReset(); + useWorkspaceFileContentMock.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, + }); + + useWorkspaceFilesMock.mockReturnValue({ + data: ["index.html", "src/main.ts", "README.md"], + isLoading: false, + }); + useWorkspaceFileContentMock.mockReturnValue({ + data: { + path: "index.html", + kind: "text", + text: "hello", + staticUrl: + "http://localhost:3000/api/conversations/c1/workspace/index.html", + mimeType: "text/html", + }, + isLoading: false, + isError: false, + }); + }); + + it("defaults to diff view when working inside a git repo", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false }); + + renderTab(); + + expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); + // The Rich/Plain toggle is hidden when diff view is active. + expect( + screen.queryByTestId("files-tab-content-mode-toggle"), + ).not.toBeInTheDocument(); + }); + + it("defaults to files+rich view in a git repo with zero commits (unborn HEAD)", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false }); + useHasGitCommitsMock.mockReturnValue({ + hasCommits: false, + isLoading: false, + }); + + renderTab(); + + // Even though it's an attached repo, the diff view is suppressed when + // there's nothing to diff against. + expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument(); + expect( + screen.getByTestId("files-tab-content-mode-toggle"), + ).toBeInTheDocument(); + }); + + it("does NOT probe for commits when there is no attached repo", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: 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 has-commits probe is still loading", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: 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 not in a git repo", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: 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("lets users toggle diff view off even when in a git repo", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false }); + const user = userEvent.setup(); + + 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"), + ).toBeInTheDocument(); + }); + + it("auto-selects the highest-priority file on first render", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + + renderTab(); + + // Either index.html (top-priority entrypoint) should be selected. + expect(useWorkspaceFileContentMock).toHaveBeenCalledWith("index.html"); + }); + + it("renders the binary fallback in plain mode for binary files", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + useWorkspaceFileContentMock.mockReturnValue({ + data: { + path: "logo.png", + kind: "binary", + text: null, + staticUrl: + "http://localhost:3000/api/conversations/c1/workspace/logo.png", + mimeType: "application/octet-stream", + }, + isLoading: false, + isError: false, + }); + const user = userEvent.setup(); + + renderTab(); + + await user.click( + screen.getByTestId("files-tab-content-mode-toggle-option-plain"), + ); + + expect( + screen.getByTestId("file-content-viewer-binary-fallback"), + ).toBeInTheDocument(); + }); + + it("shows full file paths (not just basenames) as quick-row pills", () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + + 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"); + }); + + it("collapses the file tree by default and expands it via the caret", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + 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(); + }); + + it("renders markdown content via MarkdownRenderer in rich mode", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: 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, + }); + useWorkspaceFileContentMock.mockReturnValue({ + data: { + path: "README.md", + kind: "text", + text: "# Hello\n\nSome **bold** text", + staticUrl: + "http://localhost:3000/api/conversations/c1/workspace/README.md", + mimeType: "text/markdown", + }, + isLoading: false, + isError: false, + }); + + renderTab(); + + await waitFor(() => { + expect( + screen.getByTestId("file-content-viewer-markdown"), + ).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 must paint the right-pane bg + // color (so it blends with the surrounding chrome) and project white + // text — both spelled out in the user's design ask. + const container = screen.getByTestId("file-content-viewer-markdown"); + expect(container.className).toContain("bg-[#25272D]"); + expect(container.className).toContain("text-white"); + }); + + it("shows highlighted source (not rich markdown) when toggled to plain on a .md", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + useWorkspaceFilesMock.mockReturnValue({ + data: ["README.md"], + isLoading: false, + }); + useWorkspaceFileContentMock.mockReturnValue({ + data: { + path: "README.md", + kind: "text", + text: "# Hello\n\nSome **bold** text", + staticUrl: + "http://localhost:3000/api/conversations/c1/workspace/README.md", + mimeType: "text/markdown", + }, + isLoading: false, + isError: false, + }); + const user = userEvent.setup(); + + 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"), + ); + + const highlighted = await screen.findByTestId( + "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 static workspace URL as the iframe src for HTML files", async () => { + useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false }); + useWorkspaceFilesMock.mockReturnValue({ + data: ["index.html"], + isLoading: false, + }); + const staticUrl = + "http://localhost:3000/api/conversations/abc/workspace/index.html"; + useWorkspaceFileContentMock.mockReturnValue({ + data: { + path: "index.html", + kind: "text", + text: "hi", + staticUrl, + mimeType: "text/html", + }, + isLoading: false, + isError: false, + }); + + renderTab(); + + const iframe = await screen.findByTestId("file-content-viewer-iframe"); + expect(iframe).toBeInTheDocument(); + // The iframe src starts with the workspace static URL and carries the + // mutation-counter cache-buster (`?v=`) so browser-cached responses + // are invalidated whenever the agent edits a file. + expect(iframe.getAttribute("src")).toMatch( + new RegExp(`^${staticUrl.replace(/[/.]/g, "\\$&")}\\?v=\\d+$`), + ); + // The iframe is sandboxed with `allow-same-origin` only: relative + // asset refs (CSS, images) load from the workspace fileserver + // origin, but `