diff --git a/apps/web/components/task/task-changes-panel.test.ts b/apps/web/components/task/task-changes-panel.test.ts new file mode 100644 index 0000000000..2149830603 --- /dev/null +++ b/apps/web/components/task/task-changes-panel.test.ts @@ -0,0 +1,38 @@ +import { describe, it, expect } from "vitest"; +import { shouldCloseFileDiffPanel } from "./task-changes-panel"; + +const PATH = "src/foo.ts"; + +describe("shouldCloseFileDiffPanel", () => { + it("returns false when gitStatus is undefined (not loaded yet)", () => { + expect(shouldCloseFileDiffPanel(undefined, PATH)).toBe(false); + }); + + it("returns true when gitStatus.files is undefined (loaded, no changes)", () => { + expect(shouldCloseFileDiffPanel({}, PATH)).toBe(true); + }); + + it("returns true when the file is missing from gitStatus.files (discarded)", () => { + expect(shouldCloseFileDiffPanel({ files: {} }, PATH)).toBe(true); + }); + + it("returns false when the file has a non-empty uncommitted diff", () => { + const gitStatus = { files: { [PATH]: { diff: "@@ -1 +1 @@\n-a\n+b\n" } } }; + expect(shouldCloseFileDiffPanel(gitStatus, PATH)).toBe(false); + }); + + it("returns true when the file entry exists but diff is an empty string", () => { + const gitStatus = { files: { [PATH]: { diff: "" } } }; + expect(shouldCloseFileDiffPanel(gitStatus, PATH)).toBe(true); + }); + + it("returns true when the file entry exists but diff is undefined", () => { + const gitStatus = { files: { [PATH]: {} } }; + expect(shouldCloseFileDiffPanel(gitStatus, PATH)).toBe(true); + }); + + it("is not affected by unrelated files in gitStatus.files", () => { + const gitStatus = { files: { "other/file.ts": { diff: "diff content" } } }; + expect(shouldCloseFileDiffPanel(gitStatus, PATH)).toBe(true); + }); +}); diff --git a/apps/web/components/task/task-changes-panel.tsx b/apps/web/components/task/task-changes-panel.tsx index 16d0cb6577..226761bbb7 100644 --- a/apps/web/components/task/task-changes-panel.tsx +++ b/apps/web/components/task/task-changes-panel.tsx @@ -109,6 +109,16 @@ function addPRFiles(fileMap: Map, files: PRDiffFile[]) { } } +// Returns true only after gitStatus loads and the file's uncommitted diff is gone. +export function shouldCloseFileDiffPanel( + gitStatus: { files?: Record } | undefined, + filePath: string, +): boolean { + if (!gitStatus) return false; + const entry = gitStatus.files?.[filePath]; + return !entry?.diff; +} + /** Merge PR + uncommitted + committed files into a single sorted list */ function mergeReviewFiles( gitStatus: ReturnType, @@ -211,6 +221,7 @@ function useChangesData(selectedDiff: SelectedDiff | null, onClearSelected: () = totalCommentCount, fileRefs, cumulativeLoading, + gitStatus, }; } @@ -329,6 +340,40 @@ function useChangesActions(activeSessionId: string | null | undefined, allFiles: }; } +function useAutoCloseWhenEmpty(opts: { + mode: "all" | "file"; + filePath: string | undefined; + gitStatus: { files?: Record } | undefined; + visibleCount: number; + onBecameEmpty: (() => void) | undefined; +}) { + const { mode, filePath, gitStatus, visibleCount, onBecameEmpty } = opts; + const prevVisibleCountRef = useRef(null); + const prevFileSeenRef = useRef(false); + + useEffect(() => { + if (!onBecameEmpty) return; + + if (mode === "file" && filePath) { + // File-mode: close when the file no longer has an uncommitted diff, + // regardless of whether it still appears in PR/cumulative diff sources. + const shouldClose = shouldCloseFileDiffPanel(gitStatus, filePath); + if (prevFileSeenRef.current && shouldClose) { + onBecameEmpty(); + return; + } + if (!shouldClose) prevFileSeenRef.current = true; + return; + } + + const prevCount = prevVisibleCountRef.current; + if (prevCount !== null && prevCount > 0 && visibleCount === 0) { + onBecameEmpty(); + } + prevVisibleCountRef.current = visibleCount; + }, [mode, filePath, gitStatus, onBecameEmpty, visibleCount]); +} + const TaskChangesPanel = memo(function TaskChangesPanel({ mode = "all", filePath, @@ -349,6 +394,7 @@ const TaskChangesPanel = memo(function TaskChangesPanel({ totalCommentCount, fileRefs, cumulativeLoading, + gitStatus, } = useChangesData(selectedDiff, onClearSelected); const visibleFiles = useMemo(() => { if (mode === "file" && filePath) { @@ -385,16 +431,13 @@ const TaskChangesPanel = memo(function TaskChangesPanel({ ); const totalCount = visibleFiles.length; const progressPercent = totalCount > 0 ? (reviewedCount / totalCount) * 100 : 0; - const prevVisibleCountRef = useRef(null); - - useEffect(() => { - if (!onBecameEmpty) return; - const prevCount = prevVisibleCountRef.current; - if (prevCount !== null && prevCount > 0 && visibleFiles.length === 0) { - onBecameEmpty(); - } - prevVisibleCountRef.current = visibleFiles.length; - }, [onBecameEmpty, visibleFiles.length]); + useAutoCloseWhenEmpty({ + mode, + filePath, + gitStatus, + visibleCount: visibleFiles.length, + onBecameEmpty, + }); if (isArchived) return ; diff --git a/apps/web/e2e/tests/git/diff-update.spec.ts b/apps/web/e2e/tests/git/diff-update.spec.ts index 488035d0df..a831cfb80f 100644 --- a/apps/web/e2e/tests/git/diff-update.spec.ts +++ b/apps/web/e2e/tests/git/diff-update.spec.ts @@ -206,6 +206,34 @@ test.describe("Diff update on file change", () => { timeout: 15_000, }); }); + + test("diff panel closes when uncommitted change is undone via hunk Undo", async ({ + testPage, + apiClient, + seedData, + }) => { + // Regression: Undo must close the diff tab even when PR/cumulative diffs keep the file visible. + await seedDiffUpdateTask(testPage, apiClient, seedData); + await openChangesTab(testPage); + await openFileDiff(testPage, "diff_update_test.txt"); + + const diffTab = testPage.locator(".dv-default-tab", { hasText: "diff_update_test.txt" }); + await expect(diffTab).toBeVisible({ timeout: 10_000 }); + + const diffsContainer = getDiffsContainer(testPage); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // Button is CSS-hidden until hover; dispatchEvent bypasses pointer-events:none. + const undoBtn = diffsContainer.locator("[data-undo-btn] button").first(); + await expect(undoBtn).toHaveCount(1, { timeout: 10_000 }); + await undoBtn.dispatchEvent("click"); + + // The Diff tab should close automatically. + await expect(diffTab).toHaveCount(0, { timeout: 15_000 }); + }); }); test.describe("Untracked file diff update", () => {