diff --git a/apps/web/components/editors/file-actions-dropdown.tsx b/apps/web/components/editors/file-actions-dropdown.tsx index f268397240..e0f8cc5629 100644 --- a/apps/web/components/editors/file-actions-dropdown.tsx +++ b/apps/web/components/editors/file-actions-dropdown.tsx @@ -28,12 +28,16 @@ type FileActionsDropdownProps = { size?: "sm" | "xs" | "touch"; /** Optional toast callback after copy */ onCopied?: () => void; + /** Whether the menu includes its absolute worktree path copy action. */ + includeCopyPath?: boolean; }; type FileActionsMenuItemsProps = Pick< FileActionsDropdownProps, "filePath" | "sessionId" | "onCopied" ->; +> & { + includeCopyPath?: boolean; +}; function fileActionsButtonClass(size: NonNullable): string { if (size === "xs") return "h-6 w-6 p-0 cursor-pointer opacity-60 hover:opacity-100"; @@ -107,6 +111,7 @@ function useFileActions({ export function FileActionsMenuItems(props: FileActionsMenuItemsProps) { const { t } = useTranslation(); + const { includeCopyPath = true } = props; const { folderDisabled, defaultEditorId, @@ -139,10 +144,12 @@ export function FileActionsMenuItems(props: FileActionsMenuItemsProps) { )} - - - {t("editors:copyPath")} - + {includeCopyPath && ( + + + {t("editors:copyPath")} + + )} {t("editors:openWith")} - + ); diff --git a/apps/web/components/review/review-diff-header.tsx b/apps/web/components/review/review-diff-header.tsx index 54cdbe8fe8..83ea017bed 100644 --- a/apps/web/components/review/review-diff-header.tsx +++ b/apps/web/components/review/review-diff-header.tsx @@ -290,7 +290,6 @@ export function ReviewDiffHeader({ (file.repository_name ? undefined : fallbackBaseBranch); const toolbar = ( ({ isMobile: false })); +const clipboardMocks = vi.hoisted(() => ({ copyToClipboard: vi.fn() })); +const toastMocks = vi.hoisted(() => ({ toast: vi.fn() })); +const unsafePathCases = [ + { name: "newline", path: "src/line\nbreak.ts" }, + { name: "escape", path: "src/escape\u001b.ts" }, + { name: "delete", path: "src/delete\u007f.ts" }, +]; + +vi.mock("@/lib/utils/copy-to-clipboard", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, copyToClipboard: clipboardMocks.copyToClipboard }; +}); +vi.mock("@/components/toast-provider", () => ({ useToast: () => toastMocks })); vi.mock("@/components/editors/external-vcs-file-link", () => ({ ExternalVcsFileLink: (props: Record) => ( @@ -14,8 +27,18 @@ vi.mock("@/components/editors/external-vcs-file-link", () => ({ })); vi.mock("@/components/editors/file-actions-dropdown", () => ({ - FileActionsDropdown: () => , - FileActionsMenuItems: () => , + FileActionsDropdown: (props: { includeCopyPath?: boolean }) => ( + + ), + FileActionsMenuItems: (props: { includeCopyPath?: boolean }) => ( + + ), })); vi.mock("@/hooks/use-global-view-mode", () => ({ @@ -30,6 +53,8 @@ import { FileDiffToolbar, type FileDiffToolbarProps } from "./review-diff-toolba afterEach(() => { cleanup(); + clipboardMocks.copyToClipboard.mockReset(); + toastMocks.toast.mockReset(); mocks.isMobile = false; }); @@ -43,7 +68,6 @@ describe("Markdown preview actions", () => { render( { render( { it("offers Show diff while a Markdown row is in preview mode", () => { const onToggleMarkdownPreview = vi.fn(); const props: FileDiffToolbarProps & { markdownPreview: boolean } = { - diff: "# README", filePath: "README.md", sessionId: "session-1", source: "pr", @@ -124,7 +146,6 @@ describe("FileDiffToolbar", () => { render( { size: "xs", }); expect(screen.getByTestId("file-actions-dropdown")).toBeTruthy(); - expect(screen.getByRole("button", { name: "Copy diff" })).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Copy diff" })).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Copy path" })); + expect(clipboardMocks.copyToClipboard).toHaveBeenCalledWith("src/new-name.ts"); + expect(screen.getByTestId("file-actions-dropdown").dataset.includeCopyPath).toBe("false"); expect(screen.queryByRole("button", { name: /More actions for/ })).toBeNull(); }); @@ -168,7 +192,6 @@ describe("FileDiffToolbar", () => { render( { const menu = screen.getByTestId("review-file-actions-menu"); expect(menu).toBeTruthy(); - expect(screen.getByRole("menuitem", { name: "Copy diff" })).toBeTruthy(); + expect(screen.queryByRole("menuitem", { name: "Copy diff" })).toBeNull(); + const copyPath = screen.getByRole("menuitem", { name: "Copy path" }); + expect(copyPath.className).toContain("min-h-11"); + expect(screen.getByTestId("file-actions-menu-items").dataset.includeCopyPath).toBe("false"); const expand = screen.getByRole("menuitemcheckbox", { name: "Expand unchanged lines" }); const wrap = screen.getByRole("menuitemcheckbox", { name: "Wrap long lines" }); expect(expand.getAttribute("aria-checked")).toBe("false"); @@ -208,5 +234,74 @@ describe("FileDiffToolbar", () => { fireEvent.click(trigger); fireEvent.click(screen.getByRole("menuitemcheckbox", { name: "Expand unchanged lines" })); expect(onToggleExpandUnchanged).toHaveBeenCalledOnce(); + + fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); + fireEvent.click(trigger); + fireEvent.click(screen.getByRole("menuitem", { name: "Copy path" })); + expect(clipboardMocks.copyToClipboard).toHaveBeenCalledWith("src/app.ts"); }); }); + +describe("FileDiffToolbar path safety", () => { + it.each(unsafePathCases)( + "refuses to copy a $name control-character path from the desktop toolbar", + async ({ path: filePath }) => { + render( + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: "Copy path" })); + + expect(clipboardMocks.copyToClipboard).not.toHaveBeenCalled(); + await waitFor(() => + expect(toastMocks.toast).toHaveBeenCalledWith({ + description: "This path contains control characters and cannot be copied.", + }), + ); + }, + ); + + it.each(unsafePathCases)( + "refuses to copy a $name control-character path from the phone toolbar menu", + async ({ path: filePath }) => { + mocks.isMobile = true; + render( + + + , + ); + + const trigger = screen.getByRole("button", { name: /More actions for/ }); + fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); + fireEvent.click(trigger); + fireEvent.click(screen.getByRole("menuitem", { name: "Copy path" })); + + expect(clipboardMocks.copyToClipboard).not.toHaveBeenCalled(); + await waitFor(() => + expect(toastMocks.toast).toHaveBeenCalledWith({ + description: "This path contains control characters and cannot be copied.", + }), + ); + }, + ); +}); diff --git a/apps/web/components/review/review-diff-toolbar.tsx b/apps/web/components/review/review-diff-toolbar.tsx index e3b593ccdb..8c3dec0e89 100644 --- a/apps/web/components/review/review-diff-toolbar.tsx +++ b/apps/web/components/review/review-diff-toolbar.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useState } from "react"; +import { useState } from "react"; import { IconArrowBackUp, IconCopy, @@ -34,7 +34,7 @@ import { ExternalVcsFileMenuItem, } from "@/components/editors/external-vcs-file-link"; import { useGlobalViewMode } from "@/hooks/use-global-view-mode"; -import { copyToClipboard } from "@/lib/utils/copy-to-clipboard"; +import { useCopyRepositoryPath } from "@/hooks/use-copy-repository-path"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { isMarkdownFile } from "@/lib/utils/file-types"; import { useTranslation } from "react-i18next"; @@ -47,7 +47,6 @@ const mobileMenuItem = "cursor-pointer gap-3 text-sm"; const mobileMenuIcon = "size-4 text-muted-foreground"; export type FileDiffToolbarProps = { - diff: string; filePath: string; previousPath?: string | null; status?: string | null; @@ -196,8 +195,8 @@ function MobileDiffViewMenuItems({ function MobileFileMenuItems(props: FileDiffToolbarProps) { const { t } = useTranslation(); + const copyRepositoryPath = useCopyRepositoryPath(); const { - diff, filePath, previousPath, status, @@ -217,14 +216,14 @@ function MobileFileMenuItems(props: FileDiffToolbarProps) { onToggleWordWrap, repo, } = props; - const handleCopyDiff = useCallback(() => { - void copyToClipboard(diff || ""); - }, [diff]); return ( <> - + void copyRepositoryPath(filePath)} + > - {t("review:copyDiff")} + {t("task:copyPath")} {onOpenFile && ( onOpenFile(filePath, repo)}> @@ -249,7 +248,7 @@ function MobileFileMenuItems(props: FileDiffToolbarProps) { publishedBranch={publishedBranch} baseBranch={baseBranch} /> - + { - void copyToClipboard(diff || ""); - }, [diff]); - const handleToggleViewMode = useCallback( - () => setGlobalViewMode(globalViewMode === "split" ? "unified" : "split"), - [globalViewMode, setGlobalViewMode], - ); + const handleToggleViewMode = () => + setGlobalViewMode(globalViewMode === "split" ? "unified" : "split"); return (
@@ -371,7 +365,10 @@ function DesktopFileDiffToolbar(props: FileDiffToolbarProps) { )} - + void copyRepositoryPath(filePath)} + tooltip={t("task:copyPath")} + > )} - + {source === "uncommitted" && ( ({ isFinePointer: true, isMobile: false })); +const clipboardMocks = vi.hoisted(() => ({ copyToClipboard: vi.fn() })); +const toastMocks = vi.hoisted(() => ({ toast: vi.fn() })); + +vi.mock("@/lib/utils/copy-to-clipboard", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, copyToClipboard: clipboardMocks.copyToClipboard }; +}); +vi.mock("@/components/toast-provider", () => ({ useToast: () => toastMocks })); vi.mock("@/hooks/use-responsive-breakpoint", () => ({ useResponsiveBreakpoint: () => responsive, @@ -11,11 +19,18 @@ vi.mock("@/hooks/use-responsive-breakpoint", () => ({ afterEach(() => { cleanup(); + clipboardMocks.copyToClipboard.mockReset(); + toastMocks.toast.mockReset(); responsive.isFinePointer = true; responsive.isMobile = false; }); const moreActionsLabel = "Show more actions"; +const unsafePathCases = [ + { name: "newline", path: "packages/ui/src/line\nbreak.tsx" }, + { name: "escape", path: "packages/ui/src/escape\u001b.tsx" }, + { name: "delete", path: "packages/ui/src/delete\u007f.tsx" }, +]; const noop = () => {}; const noopSelect = () => false; @@ -28,7 +43,7 @@ const baseFile = { oldPath: undefined, }; -function renderRow(path: string) { +function renderRow(path: string, onOpenDiff = noop) { return render(
    @@ -36,7 +51,7 @@ function renderRow(path: string) { file={{ ...baseFile, path }} isPending={false} onSelect={noopSelect} - onOpenDiff={noop} + onOpenDiff={onOpenDiff} onStage={noop} onUnstage={noop} onDiscard={noop} @@ -47,6 +62,68 @@ function renderRow(path: string) { ); } +describe("FileRow copy path", () => { + it("copies the repository-relative path from the desktop actions without opening the diff", () => { + const onOpenDiff = vi.fn(); + const path = "packages/ui/src/button.tsx"; + const { getByRole } = renderRow(path, onOpenDiff); + + fireEvent.click(getByRole("button", { name: "Copy path" })); + + expect(clipboardMocks.copyToClipboard).toHaveBeenCalledWith(path); + expect(onOpenDiff).not.toHaveBeenCalled(); + }); + + it("copies the repository-relative path from the phone menu without opening the diff", async () => { + responsive.isFinePointer = false; + const onOpenDiff = vi.fn(); + const path = "packages/ui/src/mobile-button.tsx"; + const { getByRole, findByRole } = renderRow(path, onOpenDiff); + + fireEvent.keyDown(getByRole("button", { name: moreActionsLabel }), { key: "Enter" }); + const copyPath = await findByRole("menuitem", { name: "Copy path" }); + fireEvent.click(copyPath); + + expect(clipboardMocks.copyToClipboard).toHaveBeenCalledWith(path); + expect(onOpenDiff).not.toHaveBeenCalled(); + expect(copyPath.className).toContain("min-h-11"); + }); + + it.each(unsafePathCases)( + "refuses to copy a $name control-character path from the desktop actions", + async ({ path }) => { + const { getByRole } = renderRow(path); + + fireEvent.click(getByRole("button", { name: "Copy path" })); + + expect(clipboardMocks.copyToClipboard).not.toHaveBeenCalled(); + await waitFor(() => + expect(toastMocks.toast).toHaveBeenCalledWith({ + description: "This path contains control characters and cannot be copied.", + }), + ); + }, + ); + + it.each(unsafePathCases)( + "refuses to copy a $name control-character path from the phone menu", + async ({ path }) => { + responsive.isFinePointer = false; + const { getByRole, findByRole } = renderRow(path); + + fireEvent.keyDown(getByRole("button", { name: moreActionsLabel }), { key: "Enter" }); + fireEvent.click(await findByRole("menuitem", { name: "Copy path" })); + + expect(clipboardMocks.copyToClipboard).not.toHaveBeenCalled(); + await waitFor(() => + expect(toastMocks.toast).toHaveBeenCalledWith({ + description: "This path contains control characters and cannot be copied.", + }), + ); + }, + ); +}); + describe("FileRow truncation (regression: path overlaps diff stats in narrow panel)", () => { it("file name span allows truncation so a long name does not overflow visually", () => { // Bug: when the panel is narrow, a long file name renders past its container diff --git a/apps/web/components/task/changes-panel-file-row.tsx b/apps/web/components/task/changes-panel-file-row.tsx index b7e50f963e..3b8b2d6837 100644 --- a/apps/web/components/task/changes-panel-file-row.tsx +++ b/apps/web/components/task/changes-panel-file-row.tsx @@ -9,6 +9,7 @@ import { IconCheck, IconLoader2, IconPencil, + IconCopy, } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; @@ -18,6 +19,7 @@ import { LineStat } from "@/components/diff-stat"; import { FileStatusIcon } from "@/components/shared/file-status-icon"; import { FileIcon } from "@/components/ui/file-icon"; import { getFileCategory } from "@/lib/utils/file-types"; +import { useCopyRepositoryPath } from "@/hooks/use-copy-repository-path"; import type { ChangedFile } from "./changes-panel-helpers"; import type { OpenDiffOptions } from "./changes-diff-target"; import { useTranslation } from "react-i18next"; @@ -59,11 +61,16 @@ export type FileRowProps = { indentPx?: number; }; -export type FileRowContentProps = FileRowProps & { folder: string; name: string }; +export type FileRowContentProps = FileRowProps & { + folder: string; + name: string; + onCopyPath: () => void; +}; export function FileRow(props: FileRowProps) { const { file, isSelected, isActive, onSelect, onEditFile, onOpenDiff } = props; const { isMobile, isFinePointer } = useResponsiveBreakpoint(); + const copyRepositoryPath = useCopyRepositoryPath(); const touchMode = isMobile || !isFinePointer; const { folder, file: name } = splitPath(file.path); @@ -83,6 +90,10 @@ export function FileRow(props: FileRowProps) { } }; + const handleCopyPath = () => { + void copyRepositoryPath(file.path); + }; + return (
  • {touchMode ? ( - + ) : ( - + )}
    • ); @@ -118,6 +129,7 @@ function DesktopFileRowContent({ indentPx, folder, name, + onCopyPath, }: FileRowContentProps) { const showFolder = !treeMode && folder; return ( @@ -163,6 +175,7 @@ function DesktopFileRowContent({ @@ -173,7 +186,7 @@ function DesktopFileRowContent({ function FileRowStats({ file }: { file: ChangedFile }) { return ( -
    +
    @@ -285,11 +298,13 @@ function StageButton({ function FileRowActions({ path, repo, + onCopyPath, onDiscard, onEditFile, }: { path: string; repo?: string; + onCopyPath: () => void; onDiscard: (path: string, repo?: string, anchor?: HTMLElement) => void; onEditFile: (path: string, repo?: string) => void; }) { @@ -297,7 +312,7 @@ function FileRowActions({ return (
    @@ -331,6 +346,22 @@ function FileRowActions({ {t("common:edit")} + + + + + {t("task:copyPath")} +
    ); } diff --git a/apps/web/components/task/changes-panel-touch-file-row.tsx b/apps/web/components/task/changes-panel-touch-file-row.tsx index 70ee93404f..75f5308e26 100644 --- a/apps/web/components/task/changes-panel-touch-file-row.tsx +++ b/apps/web/components/task/changes-panel-touch-file-row.tsx @@ -9,6 +9,7 @@ import { IconMinus, IconPencil, IconPlus, + IconCopy, } from "@tabler/icons-react"; import { DropdownMenu, @@ -22,7 +23,7 @@ import { FileIcon } from "@/components/ui/file-icon"; import { FileStatusIcon } from "@/components/shared/file-status-icon"; import { SymlinkIndicator } from "@/components/shared/symlink-indicator"; import { LineStat } from "@/components/diff-stat"; -import type { FileRowProps, FileRowContentProps } from "./changes-panel-file-row"; +import type { FileRowContentProps } from "./changes-panel-file-row"; export function TouchFileRowContent(props: FileRowContentProps) { const { file, treeMode, indentPx, isPending, folder, name } = props; @@ -64,11 +65,15 @@ export function TouchFileRowContent(props: FileRowContentProps) { function TouchFileRowActions({ file, isPending, + onCopyPath, onStage, onUnstage, onEditFile, onDiscard, -}: FileRowProps) { +}: Pick< + FileRowContentProps, + "file" | "isPending" | "onCopyPath" | "onStage" | "onUnstage" | "onEditFile" | "onDiscard" +>) { const { t } = useTranslation(); const triggerRef = useRef(null); const StageIcon = file.staged ? IconMinus : IconPlus; @@ -94,6 +99,10 @@ function TouchFileRowActions({ {file.path} + + + {t("task:copyPath")} + { await expect(stage).toBeVisible(); expect((await stage.boundingBox())!.height).toBeLessThanOrEqual(24); await expect(row.getByRole("button", { name: "Show more actions" })).toHaveCount(0); + if (prCapture.capturing) { + await row.hover(); + await waitForFiniteAnimations(row.getByTestId("file-row-hover-actions")); + await expect(row.getByRole("button", { name: "Copy path" })).toBeVisible(); + } await prCapture.screenshot("desktop-changes", { - caption: "Desktop retains compact inline staging and hover actions.", + caption: "Desktop Changes rows expose Copy path alongside compact hover actions.", }); await testPage.setViewportSize({ width: 767, height: 851 }); @@ -577,6 +582,56 @@ test.describe("Git Changes Panel", () => { await expect(session.changes.getByText("test-file.txt")).toBeVisible({ timeout: 15_000 }); }); + test("copies a changed file path without opening its diff", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + await testPage.context().grantPermissions(["clipboard-read", "clipboard-write"]); + const title = "Copy changed file path"; + await apiClient.createTaskWithAgent(seedData.workspaceId, title, seedData.agentProfileId, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const session = await openTaskSession(testPage, title); + const filePath = "copy-path-e2e/copy-me.txt"; + const git = new GitHelper(path.join(backend.tmpDir, "repos", "e2e-repo"), { + ...process.env, + HOME: backend.tmpDir, + }); + fs.mkdirSync(path.join(backend.tmpDir, "repos", "e2e-repo", "copy-path-e2e"), { + recursive: true, + }); + git.createFile(filePath, "COPY_PATH_MARKER\n"); + + await session.clickTab("Changes"); + await session.expandChangesSection("unstaged-files-section"); + const row = testPage.getByTestId(`file-row-${filePath.replace(/[/\\]/g, "-")}`); + await expect(row).toBeVisible(); + const fileIdentity = row.getByTitle(filePath, { exact: true }); + const copyPath = row.getByRole("button", { name: "Copy path" }); + const rowButtonCount = await row.getByRole("button").count(); + await fileIdentity.focus(); + for (let index = 0; index < rowButtonCount; index += 1) { + await testPage.keyboard.press("Tab"); + if (await copyPath.evaluate((element) => element === document.activeElement)) break; + } + await expect(copyPath).toBeFocused(); + await expect + .poll(() => + row + .getByTestId("file-row-hover-actions") + .evaluate((element) => getComputedStyle(element).opacity), + ) + .toBe("1"); + await testPage.keyboard.press("Enter"); + await expect.poll(() => testPage.evaluate(() => navigator.clipboard.readText())).toBe(filePath); + await expect(testPage.locator("diffs-container")).toHaveCount(0); + }); + // @covers AC-UI-CHANGES-FILE-ACTION-FEEDBACK-001.1 // @covers AC-UI-CHANGES-FILE-ACTION-FEEDBACK-001.2 // @covers AC-UI-CHANGES-FILE-ACTION-FEEDBACK-001.3 diff --git a/apps/web/e2e/tests/review/mobile-review-file-status.spec.ts b/apps/web/e2e/tests/review/mobile-review-file-status.spec.ts index 00b8eab987..06cb522077 100644 --- a/apps/web/e2e/tests/review/mobile-review-file-status.spec.ts +++ b/apps/web/e2e/tests/review/mobile-review-file-status.spec.ts @@ -17,7 +17,9 @@ test.describe("Review file status on mobile", () => { apiClient, seedData, backend, + prCapture, }) => { + await testPage.context().grantPermissions(["clipboard-read", "clipboard-write"]); await apiClient.mockGitHubReset(); await apiClient.mockGitHubSetUser("reviewer"); await apiClient.mockGitHubAddPRs([ @@ -176,7 +178,7 @@ test.describe("Review file status on mobile", () => { expect(headerGeometry.moreActionsWidth).toBeGreaterThanOrEqual(44); expect(headerGeometry.moreActionsHeight).toBeGreaterThanOrEqual(44); - await moreActions.click(); + await moreActions.tap(); const actionsMenu = testPage.getByTestId("review-file-actions-menu"); await expect(actionsMenu).toBeVisible(); await actionsMenu.evaluate((element) => @@ -208,9 +210,10 @@ test.describe("Review file status on mobile", () => { expect(menuGeometry.bottom).toBeLessThanOrEqual(menuGeometry.viewportHeight); expect(menuGeometry.minItemHeight).toBeGreaterThanOrEqual(44); - await expect(actionsMenu.getByRole("menuitem", { name: "Copy diff" })).toBeVisible(); + await expect(actionsMenu.getByRole("menuitem", { name: "Copy diff" })).toHaveCount(0); await expect(actionsMenu.getByRole("menuitem", { name: "Edit file" })).toBeVisible(); await expect(actionsMenu.getByRole("menuitem", { name: "Copy path" })).toBeVisible(); + await expect(actionsMenu.getByRole("menuitem", { name: "Copy path" })).toHaveCount(1); await expect(actionsMenu.getByRole("menuitem", { name: "Open folder" })).toBeVisible(); await expect(actionsMenu.getByRole("menuitem", { name: "Revert changes" })).toBeVisible(); await expect( @@ -220,9 +223,22 @@ test.describe("Review file status on mobile", () => { const initialWrapState = await wrapLines.getAttribute("aria-checked"); expect(["true", "false"]).toContain(initialWrapState); - await wrapLines.click(); + const copyPath = actionsMenu.getByRole("menuitem", { name: "Copy path" }); + const copyPathBox = (await copyPath.boundingBox())!; + expect(copyPathBox.width).toBeGreaterThanOrEqual(44); + expect(copyPathBox.height).toBeGreaterThanOrEqual(44); + await prCapture.screenshot("review-copy-path-mobile", { + caption: "Phone Review file menu offers Copy path for the current file.", + }); + await copyPath.tap(); + await expect + .poll(() => testPage.evaluate(() => navigator.clipboard.readText())) + .toBe(MOBILE_FILE); + + await moreActions.tap(); + await wrapLines.tap(); await expect(actionsMenu).not.toBeVisible(); - await moreActions.click(); + await moreActions.tap(); await expect( testPage .getByTestId("review-file-actions-menu") diff --git a/apps/web/e2e/tests/review/review-file-status.spec.ts b/apps/web/e2e/tests/review/review-file-status.spec.ts index c35e4095a7..b551a5e0fc 100644 --- a/apps/web/e2e/tests/review/review-file-status.spec.ts +++ b/apps/web/e2e/tests/review/review-file-status.spec.ts @@ -25,6 +25,7 @@ test.describe("Review file status", () => { backend, prCapture, }) => { + await testPage.context().grantPermissions(["clipboard-read", "clipboard-write"]); await testPage.addInitScript(({ key, width }) => sessionStorage.setItem(key, width), { key: REVIEW_SIDEBAR_LIMITS.storageKey, width: String(REVIEW_SIDEBAR_LIMITS.minWidth), @@ -230,5 +231,17 @@ test.describe("Review file status", () => { movedSection.getByText(`Moved from ${MOVED_FROM_PATH}; no textual changes`), ).toBeVisible(); await expect(movedSection.getByText("Loading diff...")).toHaveCount(0); + + const movedActions = movedHeader.getByTestId("review-file-actions"); + await expect(movedActions.getByRole("button", { name: "Copy diff" })).toHaveCount(0); + const copyPath = movedActions.getByRole("button", { name: "Copy path" }); + await expect(copyPath).toBeVisible(); + await prCapture.screenshot("review-copy-path-desktop", { + caption: "Review diff toolbar offers Copy path for the current file.", + }); + await copyPath.click(); + await expect + .poll(() => testPage.evaluate(() => navigator.clipboard.readText())) + .toBe(MOVED_PATH); }); }); diff --git a/apps/web/e2e/tests/task/mobile-changes-panel.spec.ts b/apps/web/e2e/tests/task/mobile-changes-panel.spec.ts index df1b14edf4..130e8a673a 100644 --- a/apps/web/e2e/tests/task/mobile-changes-panel.spec.ts +++ b/apps/web/e2e/tests/task/mobile-changes-panel.spec.ts @@ -95,6 +95,7 @@ test.describe("Mobile changes panel", () => { backend, prCapture, }) => { + await testPage.context().grantPermissions(["clipboard-read", "clipboard-write"]); const initialLayout = (await apiClient.getUserSettings()).settings.changes_panel_layout; try { await apiClient.saveUserSettings({ changes_panel_layout: "tree" }); @@ -164,6 +165,20 @@ test.describe("Mobile changes panel", () => { }), ).toBe(true); + await more.tap(); + const actionsMenu = testPage.getByRole("menu"); + await waitForFiniteAnimations(actionsMenu); + const copyPath = actionsMenu.getByRole("menuitem", { name: "Copy path" }); + await expect(copyPath).toBeVisible(); + const copyPathBox = (await copyPath.boundingBox())!; + expect(copyPathBox.width).toBeGreaterThanOrEqual(44); + expect(copyPathBox.height).toBeGreaterThanOrEqual(44); + await copyPath.tap(); + await expect + .poll(() => testPage.evaluate(() => navigator.clipboard.readText())) + .toBe(filePath); + await expect(testPage.locator("diffs-container")).toHaveCount(0); + await apiClient.saveUserSettings({ changes_panel_layout: "flat" }); await testPage.reload(); await session.waitForLoad(); diff --git a/apps/web/hooks/use-copy-repository-path.ts b/apps/web/hooks/use-copy-repository-path.ts new file mode 100644 index 0000000000..5a24144da8 --- /dev/null +++ b/apps/web/hooks/use-copy-repository-path.ts @@ -0,0 +1,18 @@ +import { useCallback } from "react"; +import { useToast } from "@/components/toast-provider"; +import { copyPathToClipboard } from "@/lib/utils/copy-repository-path"; +import { useTranslation } from "react-i18next"; + +export function useCopyRepositoryPath() { + const { t } = useTranslation(); + const { toast } = useToast(); + + return useCallback( + async (filePath: string) => { + if ((await copyPathToClipboard(filePath)) === "unsafe") { + toast({ description: t("task:copyPathWithControlCharacters") }); + } + }, + [t, toast], + ); +} diff --git a/apps/web/lib/utils/copy-repository-path.test.ts b/apps/web/lib/utils/copy-repository-path.test.ts new file mode 100644 index 0000000000..c963f2f3f2 --- /dev/null +++ b/apps/web/lib/utils/copy-repository-path.test.ts @@ -0,0 +1,34 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const clipboardMocks = vi.hoisted(() => ({ copyToClipboard: vi.fn() })); + +vi.mock("./copy-to-clipboard", () => clipboardMocks); + +import { copyPathToClipboard } from "./copy-repository-path"; + +describe("copyPathToClipboard", () => { + beforeEach(() => { + clipboardMocks.copyToClipboard.mockReset().mockResolvedValue(true); + }); + + it("rejects every C0 and DEL control character before writing to the clipboard", async () => { + const controlCharacters = [ + ...Array.from({ length: 0x20 }, (_, codePoint) => String.fromCharCode(codePoint)), + String.fromCharCode(0x7f), + ]; + + for (const controlCharacter of controlCharacters) { + await expect(copyPathToClipboard(`src/file${controlCharacter}.ts`)).resolves.toBe("unsafe"); + } + + expect(clipboardMocks.copyToClipboard).not.toHaveBeenCalled(); + }); + + it("copies other repository paths exactly", async () => { + const path = "src/current-name.ts"; + + await expect(copyPathToClipboard(path)).resolves.toBe("copied"); + + expect(clipboardMocks.copyToClipboard).toHaveBeenCalledWith(path); + }); +}); diff --git a/apps/web/lib/utils/copy-repository-path.ts b/apps/web/lib/utils/copy-repository-path.ts new file mode 100644 index 0000000000..870d0113a1 --- /dev/null +++ b/apps/web/lib/utils/copy-repository-path.ts @@ -0,0 +1,10 @@ +import { copyToClipboard } from "./copy-to-clipboard"; + +const unsafeRepositoryPathControlCharacter = /[\u0000-\u001f\u007f]/; + +export type CopyPathResult = "copied" | "unsafe" | "failed"; + +export async function copyPathToClipboard(path: string): Promise { + if (unsafeRepositoryPathControlCharacter.test(path)) return "unsafe"; + return (await copyToClipboard(path)) ? "copied" : "failed"; +} diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index a6b081a92e..caad970c1c 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -360,6 +360,7 @@ "copyMessage": "Copy message", "copyMessageToClipboard": "Copy message to clipboard", "copyPath": "Copy path", + "copyPathWithControlCharacters": "This path contains control characters and cannot be copied.", "copyToClipboard": "Copy to clipboard", "copyUrl": "Copy URL", "copyWorkspacePath": "Copy workspace path", diff --git a/apps/web/src/locales/ja/task.json b/apps/web/src/locales/ja/task.json index 43d3a48639..e9661fc999 100644 --- a/apps/web/src/locales/ja/task.json +++ b/apps/web/src/locales/ja/task.json @@ -360,6 +360,7 @@ "copyMessage": "メッセージをコピー", "copyMessageToClipboard": "メッセージをクリップボードにコピー", "copyPath": "パスをコピー", + "copyPathWithControlCharacters": "このパスには制御文字が含まれているため、コピーできません。", "copyToClipboard": "クリップボードにコピー", "copyUrl": "URLをコピー", "copyWorkspacePath": "ワークスペースのパスをコピー", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 0a4f74cfdd..1f04f586bb 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -360,6 +360,7 @@ "copyMessage": "Ćōƥŷ ḿēśśàĝē", "copyMessageToClipboard": "Ćōƥŷ ḿēśśàĝē ţō ćĺĩƥƀōàŕď", "copyPath": "Ćōƥŷ ƥàţĥ", + "copyPathWithControlCharacters": "Ţĥĩś ƥàţĥ ćōńţàĩńś ćōńţŕōĺ ćĥàŕàćţēŕś àńď ćàńńōţ ƀē ćōƥĩēď.", "copyToClipboard": "Ćōƥŷ ţō ćĺĩƥƀōàŕď", "copyUrl": "Ćōƥŷ ŨŔĹ", "copyWorkspacePath": "Ćōƥŷ ŵōŕķśƥàćē ƥàţĥ", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index 8b50ebf0a4..c561fa50c7 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -350,6 +350,7 @@ "copyMessage": "Copiar a mensagem", "copyMessageToClipboard": "Copiar a mensagem para a área de transferência", "copyPath": "Copiar o caminho", + "copyPathWithControlCharacters": "Este caminho contém caracteres de controlo e não pode ser copiado.", "copyToClipboard": "Copiar para a área de transferência", "copyUrl": "Copiar o URL", "copyWorkspacePath": "Copiar o caminho da área de trabalho", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index 99e6a7e975..834b90ddce 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -350,6 +350,7 @@ "copyMessage": "复制消息", "copyMessageToClipboard": "复制消息到剪贴板", "copyPath": "复制路径", + "copyPathWithControlCharacters": "此路径包含控制字符，无法复制。", "copyToClipboard": "复制到剪贴板", "copyUrl": "复制 URL", "copyWorkspacePath": "复制工作区路径", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index 9653af786b..5275b0daed 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -350,6 +350,7 @@ "copyMessage": "複製訊息", "copyMessageToClipboard": "複製訊息到剪貼板", "copyPath": "複製路徑", + "copyPathWithControlCharacters": "此路徑包含控制字符，無法複製。", "copyToClipboard": "複製到剪貼板", "copyUrl": "複製 URL", "copyWorkspacePath": "複製工作區路徑", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index 3bed92720b..ba8e599251 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -350,6 +350,7 @@ "copyMessage": "複製訊息", "copyMessageToClipboard": "複製訊息到剪貼簿", "copyPath": "複製路徑", + "copyPathWithControlCharacters": "此路徑包含控制字元，無法複製。", "copyToClipboard": "複製到剪貼簿", "copyUrl": "複製 URL", "copyWorkspacePath": "複製工作區路徑", diff --git a/docs/plans/copy-file-path-actions/plan.md b/docs/plans/copy-file-path-actions/plan.md new file mode 100644 index 0000000000..19ef6ceceb --- /dev/null +++ b/docs/plans/copy-file-path-actions/plan.md @@ -0,0 +1,168 @@ +--- +created: 2026-09-24 +status: implemented +requirements: + - REQ-UI-COPY-FILE-PATH-ACTIONS-001 +system_design: + - ../../specs/ui/system-design/copy-file-path-actions.md +legacy_specs: [] +--- + +# Implementation Plan: Copy File Path Actions + +## Overview + +Add Copy path to working-tree rows in the task Changes panel and replace Copy +diff with Copy path in the Review diff toolbar. Implement the two surfaces +together so they use the same repository-relative path meaning and clipboard +behavior. Refuse to copy paths containing C0 or DEL ASCII control characters +and report that outcome to prevent terminal input injection. + +## Scope + +### In scope + +- Add Copy path to desktop hover and keyboard-focus actions for Changes rows. +- Add Copy path to the existing touch action menu for Changes rows. +- Replace Copy diff with Copy path in desktop and phone Review diff file actions. +- Reuse `copyToClipboard` for safe paths and the existing `task:copyPath` + translation. +- Reject C0 and DEL ASCII control characters from every Copy path entry point + and show localized refusal feedback. +- Add focused unit and desktop/phone E2E coverage for the copied value and + action reachability. + +### Out of scope + +- Copy diff controls in task diff toolbars, Monaco, or other editor surfaces. +- Path normalization, repository data, Git behavior, APIs, or persistence. +- Public documentation changes. + +## Technical approach + +- Extend `FileRowActions` and `TouchFileRowActions` with + the same copy handler for `ChangedFile.path`. Stop propagation so + copying does not open the diff. Ensure desktop actions become visible on + keyboard focus as well as hover. +- Replace the Review toolbar's desktop Copy diff button and phone menu item in + `review-diff-toolbar.tsx`. Copy the current `filePath`, + including the new path for renamed files. +- Use `copyPathToClipboard` in both surfaces to reject paths with C0 or DEL + ASCII control characters, then use `copyToClipboard` for safe paths. Use + `task:copyPath` for localized accessible names and a localized refusal + message for rejected paths. +- Suppress the shared editor menu's absolute-worktree Copy path item in the + Review toolbar menus, so the Review surface exposes one Copy path action with + the repository-relative value. Keep those shared menus unchanged elsewhere. +- Update existing row and Review toolbar unit tests, then extend the existing + Changes and Review browser scenarios for desktop and phone. + +## ASCII UI preview + +UI-01: Changes panel working-tree file row, desktop pointer hover or keyboard +focus. The path and statistics stay in the row; Copy path joins the existing +secondary actions. Copy path and row activation are separate controls. + +```text +[go] browser/programme_test.go +12 -12 [M] [undo] [edit] [copy] +``` + +UI-02: Changes panel working-tree file row, phone. Tapping the row opens the +diff; the visible 44px menu trigger opens a menu whose rows are at least 44px. +Copy path is a menu item, not a hover action. + +```text +[go] programme_test.go [+12 -12 M] [more] + go/internal/.../browser + +Menu: Full path + Copy path + Stage / Unstage + Edit + Discard +``` + +UI-03: Review diff file toolbar, desktop. Copy path replaces Copy diff and +copies the current repository-relative path. + +```text +[comment] path/to/programme_test.go +12/-12 [copy path] [external] [view] [edit] [...] +``` + +UI-04: Review diff file toolbar, phone. The existing 44px action trigger opens +the file menu; Copy path replaces Copy diff there. The diff keeps its existing +single content scroll region. + +```text +Diff header: .../path/to/programme_test.go [status] +[more] + +Menu: Copy path + Edit + Preview / diff controls +``` + +UI-05: If either surface receives a path containing a C0 or DEL ASCII control +character, Copy path leaves the clipboard unchanged and shows a localized +refusal notice. The notice does not change the row or active diff. + +```text +Copy path: refused +Notice: This path contains control characters and cannot be copied. +``` + +The drawings show action order, access, and hierarchy. Icon appearance and +spacing are illustrative. UI-01 through UI-04 map to +`AC-UI-COPY-FILE-PATH-ACTIONS-001.1` through `.4`; the +targeted browser checks prove the copied value and desktop/phone access. +UI-05 maps to `AC-UI-COPY-FILE-PATH-ACTIONS-001.6`. + +## Tests + +| Acceptance criterion | Evidence | +| ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------- | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.1`, `.5` | `components/task/changes-panel-file-row.test.tsx`: desktop and phone tests assert the exact path and no diff navigation | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.2`, `.3`, `.5` | `components/review/review-diff-toolbar.test.tsx`: desktop copies the current path for a rename; phone copies its active path | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.4` | Desktop and phone E2E checks below assert keyboard/pointer or menu reachability and 44px phone controls | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.6` | Component suites reject newline, ESC, and DEL across all four entry points; utility tests cover the full C0/DEL range | + +## E2E tests + +- `tests/git/git-changes-panel.spec.ts` (chromium): + `copies a changed file path without opening its diff`. +- `tests/task/mobile-changes-panel.spec.ts` (mobile-chrome): + `compact file rows preserve names and expose touch actions` also copies from + the row menu. +- `tests/review/review-file-status.spec.ts` (chromium): + the Review status scenario copies the new path for a renamed file. +- `tests/review/mobile-review-file-status.spec.ts` (mobile-chrome): + the mobile header scenario copies the current path from its file menu. + +Component tests also verify control-character paths are rejected from all four +desktop and phone entry points with localized refusal feedback. + +Each flow asserts the exact repository-relative clipboard value. The phone flows +also assert the relevant menu or control has a 44px hit area and the page has no +horizontal overflow. + +## Work orders + +- [x] [Task 01: Add copy path actions](task-01-copy-path-actions.md) (done) + +## Verification results + +- `pnpm exec vitest run components/task/changes-panel-file-row.test.tsx components/review/review-diff-toolbar.test.tsx lib/utils/copy-repository-path.test.ts`: passed (39 tests, including control-character rejection across all four desktop and phone entry points). +- `pnpm e2e:run --host --project chromium tests/git/git-changes-panel.spec.ts tests/review/review-file-status.spec.ts`: passed (28 tests; production builds succeeded). +- `pnpm e2e:run --no-build --host --project mobile-chrome tests/task/mobile-changes-panel.spec.ts tests/review/mobile-review-file-status.spec.ts`: passed (10 tests; using the production bundle built by the preceding managed mobile run). +- After formatting, `pnpm run build` passed, and focused desktop and mobile E2E reruns each passed (2 tests per project). +- Targeted ESLint, Prettier, typecheck, and `pnpm run i18n:ratchet`: passed. +- `python3 scripts/list-docs.py validate`, `python3 scripts/lint-spec-files.py --all`, and `git diff --check`: passed. +- After PR review remediation, the focused desktop Chromium E2E suite passed (28 tests) and the phone mobile-chrome suite passed (10 tests); both managed runs rebuilt the backend and Vite assets. +- After remediation, web typecheck, targeted ESLint and Prettier, `i18n:check`, `i18n:ratchet`, specification validation and lint all passed. + +## Risks + +- The additional Changes row control must not reduce the visible path or + overlap the trailing statistics at the panel's supported minimum width. +- Renamed files must copy the current path from the diff header, not the + previous path retained for rename context. diff --git a/docs/plans/copy-file-path-actions/task-01-copy-path-actions.md b/docs/plans/copy-file-path-actions/task-01-copy-path-actions.md new file mode 100644 index 0000000000..699be8802c --- /dev/null +++ b/docs/plans/copy-file-path-actions/task-01-copy-path-actions.md @@ -0,0 +1,177 @@ +--- +id: "01-copy-path-actions" +title: "Add copy path actions" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-COPY-FILE-PATH-ACTIONS-001 +acceptance_criteria: + - AC-UI-COPY-FILE-PATH-ACTIONS-001.1 + - AC-UI-COPY-FILE-PATH-ACTIONS-001.2 + - AC-UI-COPY-FILE-PATH-ACTIONS-001.3 + - AC-UI-COPY-FILE-PATH-ACTIONS-001.4 + - AC-UI-COPY-FILE-PATH-ACTIONS-001.5 + - AC-UI-COPY-FILE-PATH-ACTIONS-001.6 +system_design: + - ../../specs/ui/system-design/copy-file-path-actions.md +--- + +# Task 01: Add copy path actions + +## Summary + +Add Copy path to working-tree file rows in the Changes panel and replace the +Review diff toolbar's Copy diff action with Copy path. Both actions copy the +current repository-relative path through the shared clipboard helper when it +contains no C0 or DEL ASCII control character. Unsafe paths are refused with +localized feedback. + +## In scope + +- Add a Copy path action to desktop Changes row hover and keyboard-focus + actions. +- Add Copy path to the touch Changes row action menu without changing row-tap + diff navigation. +- Replace Copy diff in the desktop and phone Review diff file actions. +- Refuse paths containing C0 or DEL ASCII control characters from each Copy + path entry point and report the refusal. +- Suppress the Review toolbar's existing absolute-worktree Copy path item so + the repository-relative action is the single Copy path choice there. +- Add focused unit and desktop/phone browser coverage for path value and access. +- Add desktop/phone component coverage for control-character rejection. + +## Out of scope + +- Copy diff actions in task diff, Monaco, and other editor surfaces. +- Path conversion, Git behavior, backend/API changes, and persistence. +- Public documentation changes. + +## Acceptance + +- Changes row Copy path copies the exact repository-relative path on desktop + and phone; using it does not open the diff. +- Review diff exposes Copy path in place of Copy diff on desktop and phone and + copies the active path, including the current path for a rename. +- Paths with C0 or DEL ASCII control characters are not copied from either + surface, and the UI reports the refusal. +- Phone controls and menu items meet the 44px target requirement, and desktop + row actions are available by keyboard focus as well as pointer hover. + +## ASCII UI preview + +UI-01 desktop and UI-02 phone from the +[full preview](plan.md#ascii-ui-preview): + +```text +Desktop: [go] browser/programme_test.go +12 -12 [M] [undo] [edit] [copy] +Phone: [go] programme_test.go [+12 -12 M] [more] + go/internal/.../browser +Menu: Full path / Copy path / Stage or Unstage / Edit / Discard +``` + +UI-03 desktop and UI-04 phone from the +[full preview](plan.md#ascii-ui-preview): + +```text +Desktop: [comment] path/to/programme_test.go [copy path] [external] [view] [...] +Phone: Diff header .../path/to/programme_test.go [more] + Menu: Copy path / Edit / Preview / diff controls +``` + +UI-05 from the [full preview](plan.md#ascii-ui-preview) shows the refusal +notice when a path contains a C0 or DEL ASCII control character. The clipboard +and the selected row or diff remain unchanged. + +```text +Copy path: refused +Notice: This path contains control characters and cannot be copied. +``` + +AC-001.1 through AC-001.6 require the current repository-relative value for +safe paths, separate row and copy actions, current rename path, desktop/phone +access, and control-character rejection. The full plan preview defines fixed +versus scrollable content and each control's placement. + +## Verification + +Run from the repository root after `pnpm install --frozen-lockfile` in `apps`: + +```bash +(cd apps/web && pnpm exec vitest run components/task/changes-panel-file-row.test.tsx components/review/review-diff-toolbar.test.tsx lib/utils/copy-repository-path.test.ts) +(cd apps/web && pnpm e2e:run --host --project chromium tests/git/git-changes-panel.spec.ts tests/review/review-file-status.spec.ts) +(cd apps/web && pnpm e2e:run --host --project mobile-chrome tests/task/mobile-changes-panel.spec.ts tests/review/mobile-review-file-status.spec.ts) +(cd apps/web && pnpm exec eslint components/task/changes-panel-file-row.tsx components/task/changes-panel-touch-file-row.tsx components/review/review-diff-toolbar.tsx components/task/changes-panel-file-row.test.tsx components/review/review-diff-toolbar.test.tsx) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm run i18n:ratchet) +``` + +## Files likely touched + +- `apps/web/components/task/changes-panel-file-row.tsx` +- `apps/web/components/task/changes-panel-touch-file-row.tsx` +- `apps/web/components/task/changes-panel-file-row.test.tsx` +- `apps/web/components/review/review-diff-toolbar.tsx` +- `apps/web/components/review/review-diff-toolbar.test.tsx` +- `apps/web/hooks/use-copy-repository-path.ts` +- `apps/web/lib/utils/copy-repository-path.ts` +- `apps/web/lib/utils/copy-repository-path.test.ts` +- `apps/web/src/locales/*/task.json` +- `apps/web/components/editors/file-actions-dropdown.tsx` +- `apps/web/e2e/tests/git/git-changes-panel.spec.ts` +- `apps/web/e2e/tests/task/mobile-changes-panel.spec.ts` +- `apps/web/e2e/tests/review/review-file-status.spec.ts` +- `apps/web/e2e/tests/review/mobile-review-file-status.spec.ts` + +## Dependencies + +None. + +## Risks + +Keep the Changes row statistics contained at the minimum panel width. Copy the +current path for renamed files and retain the previous path only as context. + +## Parallelism + +`sequential` + +## Inputs + +- [Copy File Path Actions requirements](../../specs/ui/requirements/copy-file-path-actions.md) +- [Copy File Path Actions system design](../../specs/ui/system-design/copy-file-path-actions.md) +- Existing `copyToClipboard`, Changes row, touch action menu, and + Review file toolbar patterns. + +## Results + +Implemented desktop and phone Copy path actions for Changes rows, replaced +Copy diff in the Review toolbar, and suppressed the duplicate absolute-path +copy item inside Review file menus. + +Verification passed: + +- Focused Vitest suites: 25 tests passed. +- Desktop Chromium E2E: 28 tests passed; managed backend and Vite builds passed. +- Mobile Chrome E2E: 10 tests passed after the menu animation settled before + hit-area measurement. +- After formatting, `pnpm run build` and focused desktop/mobile browser reruns + passed (2 tests per project). +- Targeted ESLint and Prettier, web typecheck, i18n ratchet, specification + validation, specification lint, and `git diff --check` passed. + +PR review remediation removed the unused Review toolbar `diff` prop, made the +keyboard-focus E2E assertion find Copy path without a fixed Tab count, and +added a shared C0/DEL path-copy guard with localized refusal feedback. The +system design now has valid requirement-mapping anchors, and acceptance +criterion AC-UI-COPY-FILE-PATH-ACTIONS-001.6 records the refusal contract. + +Verification after remediation passed: + +- Focused Changes row, Review toolbar, and path-copy utility Vitest suites: 39 + tests. +- Desktop Chromium E2E: 28 tests; the managed backend and Vite builds passed. +- Phone mobile-chrome E2E: 10 tests; the managed backend and Vite builds passed. +- Web typecheck, targeted ESLint and Prettier, `i18n:check`, + `i18n:ratchet`, specification validation and lint, and `git diff --check`. diff --git a/docs/specs/ui/requirements/copy-file-path-actions.md b/docs/specs/ui/requirements/copy-file-path-actions.md new file mode 100644 index 0000000000..02c44a3abb --- /dev/null +++ b/docs/specs/ui/requirements/copy-file-path-actions.md @@ -0,0 +1,64 @@ +--- +status: active +system: ui +created: 2026-09-24 +owners: + - kandev +--- + +# Copy File Path Actions Requirements + +## Overview + +People can open a changed file's diff but must select its path manually to copy +it. The UI system owns the transient copy actions and their responsive +presentation. Task and workspace systems continue to own the file and +repository identities supplied to the UI. + +## Requirements + +### REQ-UI-COPY-FILE-PATH-ACTIONS-001: Copy a changed file path + +**Intent:** Let people copy the repository-relative path of a changed file from +the Changes list or its Review diff header. + +**User story:** As a person reviewing file changes, I want to copy a file path +directly so that I can reuse it without selecting the text. + +#### Acceptance criteria + +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.1:** Given a working-tree file row in the + Changes panel whose path has no C0 or DEL ASCII control character, when a + user activates Copy path, the clipboard shall receive the exact + repository-relative path represented by that row, and the action shall not + open the file diff. +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.2:** Given a file in the Review diff + panel whose path has no C0 or DEL ASCII control character, when the user + activates Copy path, the clipboard shall receive the current + repository-relative file path. Copy path shall replace Copy diff in that + panel's file toolbar. +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.3:** For a renamed file, Copy path shall + copy the current path shown by the file row or diff header when that path has + no C0 or DEL ASCII control character. The previous path remains available + only through the existing rename context. +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.4:** Copy path shall be keyboard and + pointer reachable on desktop and touch reachable on phones. It shall not + depend on hover as the only access path, and its phone controls shall have + hit areas of at least 44px in both dimensions. +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.5:** The copied value shall not include a + repository label or an absolute worktree path, including in a multi-repository + task. +- **AC-UI-COPY-FILE-PATH-ACTIONS-001.6:** Given a file path containing a C0 or + DEL ASCII control character, when a user activates Copy path in the Changes + panel or Review diff panel, the clipboard shall remain unchanged and the UI + shall report that the path cannot be copied. + +## Out of scope + +- Changing file-path data, repository routing, diff selection, or Git actions. +- Copying the entire diff from other editor or diff surfaces. +- Adding clipboard persistence or a new clipboard implementation. + +## Implementation plans + +- [Copy file path actions](../../../plans/copy-file-path-actions/plan.md) diff --git a/docs/specs/ui/system-design/copy-file-path-actions.md b/docs/specs/ui/system-design/copy-file-path-actions.md new file mode 100644 index 0000000000..cd3e2b0d7a --- /dev/null +++ b/docs/specs/ui/system-design/copy-file-path-actions.md @@ -0,0 +1,136 @@ +--- +status: current +system: ui +requirements: + - REQ-UI-COPY-FILE-PATH-ACTIONS-001 +--- + +# Copy File Path Actions System Design + +## Purpose and boundaries + +The UI system owns the transient copy controls and their desktop and phone +presentation. Existing task and workspace data remains authoritative for file +paths, repository identity, and diff selection. The change adds no backend +contract, persistent state, or path conversion. + +## Requirement mapping + +| Criterion | Design section | +| --------------------------------------------- | -------------------------------------------------------------------- | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.1` | [Changes row action](#changes-row-action) | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.2`, `001.3` | [Review diff action](#review-diff-action), [Path value](#path-value) | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.4` | [Responsive composition](#responsive-composition) | +| `AC-UI-COPY-FILE-PATH-ACTIONS-001.5` | [Path value](#path-value) | + +## Components and responsibilities + +- `ChangesPanel` continues to provide each working-tree + `ChangedFile.path`. + +### Changes row action + +- `FileRowActions` in + `apps/web/components/task/changes-panel-file-row.tsx` adds the + desktop Copy path control beside existing row actions. +- `TouchFileRowActions` in + `apps/web/components/task/changes-panel-touch-file-row.tsx` adds + Copy path to the existing row action menu. The row itself remains the primary + diff action. + +### Review diff action + +- `DesktopFileDiffToolbar` and `MobileFileMenuItems` in + `apps/web/components/review/review-diff-toolbar.tsx` replace the + Review toolbar's Copy diff action with Copy path. +- The shared file-action menus also offer Copy path, but that action copies an + absolute worktree path. The Review toolbar suppresses that generic item in + its desktop and phone menus so the Review surface has one unambiguous Copy + path action. Other editor menus keep their existing behavior. +- `copyToClipboard` in + `apps/web/lib/utils/copy-to-clipboard.ts` remains the clipboard + boundary, including its secure-context fallback. +- The existing `task:copyPath` translation supplies the accessible + name and tooltip. `task:copyPathWithControlCharacters` supplies refusal + feedback when a path contains control characters. + +## Path value + +Copy path passes the repository-relative path already represented by the +surface: `file.path` for a Changes row and the current +`filePath` for the Review diff toolbar. It does not prepend the +repository name, resolve a local filesystem root, or normalize the value. A +renamed file copies its current path; the existing previous-path value remains +reserved for rename cues and external links. Each copy action stops row-click +propagation where applicable. + +Paths containing C0 or DEL ASCII control characters are rejected before the +clipboard write. For these paths the action reports the refusal and leaves the +clipboard unchanged. Other paths are copied exactly as represented by the +surface. + +## Control flow + +1. The row or active diff supplies its current path. +2. The shared path-copy helper rejects values containing C0 or DEL ASCII + control characters and reports the refusal. +3. For other values, the helper calls the shared clipboard helper with the + exact repository-relative path. +4. The Changes row remains selected and the Review diff remains open. +5. If the modern clipboard API is unavailable or rejects the write, the shared + helper attempts its existing DOM fallback. The action remains available for + retry if copying still fails. + +## Responsive composition + +- **Desktop Changes outcome:** the existing file row keeps its path and + statistics layout. Copy path joins the row's hover actions and becomes + visible while keyboard focus is within the actions. It does not replace the + row's click-to-open behavior. +- **Phone Changes entry point and surface:** the existing Changes bottom + navigation opens `MobileChangesPanel`. The visible 44px row menu + trigger exposes Copy path with the existing Stage/Unstage, Edit, and Discard + actions. The row identity continues to open the diff. +- **Nearest mobile exemplar:** `TouchFileRowActions` already presents + temporary row actions in a responsive menu with 44px controls. Reuse that + menu and its existing full-path label. +- **Review diff outcome:** fine-pointer desktop keeps a direct Copy path icon + in the file toolbar. Phone users open the existing 44px file-actions trigger + and select Copy path from its menu. +- **Shared behavior:** desktop and phone use the same path value and clipboard + helper. No new scroll owner, drawer, saved preference, or page-level + horizontal overflow is introduced. + +## Failure and recovery + +Clipboard writes use the shared helper and its current fallback. A failed +attempt does not alter the selected file, current diff, or path value; users can +retry from the same control. A path containing C0 or DEL ASCII control +characters is not copied and displays localized refusal feedback. A normal +clipboard failure remains available for retry from the same control. + +## Persistence + +None. Clipboard actions are transient. + +## Security + +Repository filenames are untrusted input. C0 and DEL ASCII control characters +are refused before clipboard access to prevent a copied name from injecting +additional terminal input. For other paths, only the repository-relative value +already shown by the UI is copied. No filesystem root, credential, or +repository label is added. + +## Observability + +No new logs or metrics are needed. + +## Related specifications + +- [Clipboard browser fallbacks](../../auth/requirements/secure-context-browser-fallbacks.md) +- [Changes file row containment](../requirements/changes-file-row-containment.md) +- [Review file status cues](../requirements/review-file-status.md) + +## Implementation plan + +- [Copy file path actions](../../../plans/copy-file-path-actions/plan.md)