diff --git a/apps/backend/internal/common/readselector/readselector.go b/apps/backend/internal/common/readselector/readselector.go index 71dedfbd8e..cf4a94c143 100644 --- a/apps/backend/internal/common/readselector/readselector.go +++ b/apps/backend/internal/common/readselector/readselector.go @@ -40,6 +40,9 @@ func Split(raw string) (path string, startLine, lineCount int) { return raw, 0, 0 } colon := lastSep + 1 + rel + if lastSep < 0 && isWindowsDrivePrefix(raw, colon) { + return raw, 0, 0 + } base, suffix := raw[:colon], raw[colon+1:] if base == "" || suffix == "" { return raw, 0, 0 @@ -51,6 +54,14 @@ func Split(raw string) (path string, startLine, lineCount int) { return base, start, count } +func isWindowsDrivePrefix(raw string, colon int) bool { + return colon == 1 && len(raw) >= 2 && isASCIIAlpha(raw[0]) +} + +func isASCIIAlpha(b byte) bool { + return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') +} + // parseReadSelector validates the full selector tail (everything after the // first ':' in the final segment). Selector parts are joined by ':' so combos // such as "2-4:raw" and "raw:2-4" are accepted; each part must be a line-spec diff --git a/apps/backend/internal/common/readselector/readselector_test.go b/apps/backend/internal/common/readselector/readselector_test.go index f3cb221b3e..63eef7adcb 100644 --- a/apps/backend/internal/common/readselector/readselector_test.go +++ b/apps/backend/internal/common/readselector/readselector_test.go @@ -62,6 +62,7 @@ func TestSplit_NoFalsePositives(t *testing.T) { "foo.go:bar", // non-numeric, non-keyword suffix "foo.go:1.2", // float-like, not a line spec `C:\Users\me\a.go`, // windows drive letter: suffix is not a line spec + `C:43`, // windows drive-relative path, not a selector "dir:1/file.go", // colon lives in a directory component, not the file ":43-94", // empty base "main.go:", // empty selector diff --git a/apps/web/components/shared/markdown-components.test.tsx b/apps/web/components/shared/markdown-components.test.tsx index d0b0b0dac0..6b9d2bd3f5 100644 --- a/apps/web/components/shared/markdown-components.test.tsx +++ b/apps/web/components/shared/markdown-components.test.tsx @@ -208,7 +208,7 @@ describe("markdownComponents omp read selectors", () => { }; }); - it("opens file links with omp range and multi-range selectors in the editor", () => { + it("opens file links with omp range selectors in the editor", () => { render({"[readme](docs/README.md:16-20)"}); fireEvent.click(screen.getByRole("link", { name: "readme" })); expect(openFile).toHaveBeenCalledWith("docs/README.md"); diff --git a/apps/web/components/task/chat/messages/tool-edit-message.tsx b/apps/web/components/task/chat/messages/tool-edit-message.tsx index 70b39bc41b..b7400b1904 100644 --- a/apps/web/components/task/chat/messages/tool-edit-message.tsx +++ b/apps/web/components/task/chat/messages/tool-edit-message.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, memo, useCallback } from "react"; +import { useState, memo } from "react"; import { IconCheck, IconX, @@ -16,7 +16,7 @@ import { DiffViewBlock } from "./diff-view-block"; import { ExpandableRow } from "./expandable-row"; import { transformFileMutation, type FileMutation } from "@/lib/diff"; import { useExpandState } from "./use-expand-state"; -import { setPendingCursorPosition, scrollEditorIfMounted } from "@/hooks/use-file-editors"; +import { useOpenFileAtLine } from "@/hooks/use-file-editors"; type ModifyFilePayload = { file_path?: string; @@ -187,21 +187,7 @@ export const ToolEditMessage = memo(function ToolEditMessage({ } }; - // Navigate the editor to the first changed line. For a newly opened file the - // pending position is consumed on editor mount; for an already-open file no - // mount happens, so scroll the live editor directly. - const handleOpenFile = useCallback( - (path: string) => { - if (startLine && startLine > 0) { - setPendingCursorPosition(path, startLine, 1); - onOpenFile?.(path); - scrollEditorIfMounted(path, worktreePath ?? null, startLine, 1); - return; - } - onOpenFile?.(path); - }, - [onOpenFile, startLine, worktreePath], - ); + const handleOpenFile = useOpenFileAtLine(onOpenFile, startLine, worktreePath); return ( { - if (startLine && startLine > 0) { - setPendingCursorPosition(path, startLine, 1); - onOpenFile?.(path); - scrollEditorIfMounted(path, worktreePath ?? null, startLine, 1); - return; - } - onOpenFile?.(path); - }, - [onOpenFile, startLine, worktreePath], - ); + const handleOpenFile = useOpenFileAtLine(onOpenFile, startLine, worktreePath); return ( vi.fn()); +const APP_PATH = "src/app.ts"; +const MISSING_PATH = "src/missing.ts"; +const WORKTREE_PATH = "/worktree"; +const WORKTREE_APP_PATH = "/worktree/src/app.ts"; + +vi.mock("@/components/editors/monaco/monaco-init", () => ({ + getMonacoInstance, +})); + +import { + consumePendingCursorPosition, + scrollEditorIfMounted, + setPendingCursorPosition, + useOpenFileAtLine, +} from "./use-file-editors"; + +function createEditor(modelPath: string | null) { + return { + getModel: vi.fn(() => (modelPath ? { uri: { path: modelPath } } : null)), + setPosition: vi.fn(), + revealLineInCenter: vi.fn(), + focus: vi.fn(), + }; +} + +describe("scrollEditorIfMounted", () => { + afterEach(() => { + getMonacoInstance.mockReset(); + consumePendingCursorPosition(APP_PATH); + consumePendingCursorPosition(MISSING_PATH); + }); + + it("scrolls the mounted Monaco editor that matches the worktree path", () => { + const editor = createEditor(WORKTREE_APP_PATH); + getMonacoInstance.mockReturnValue({ editor: { getEditors: () => [editor] } }); + setPendingCursorPosition(APP_PATH, 12, 1); + + expect(scrollEditorIfMounted(APP_PATH, WORKTREE_PATH, 42, 3)).toBe(true); + + expect(editor.setPosition).toHaveBeenCalledWith({ lineNumber: 42, column: 3 }); + expect(editor.revealLineInCenter).toHaveBeenCalledWith(42); + expect(editor.focus).toHaveBeenCalledTimes(1); + expect(consumePendingCursorPosition(APP_PATH)).toBeUndefined(); + }); + + it("returns false when no mounted Monaco editor matches the path", () => { + const editor = createEditor("/worktree/src/other.ts"); + getMonacoInstance.mockReturnValue({ editor: { getEditors: () => [editor] } }); + setPendingCursorPosition(MISSING_PATH, 5, 1); + + expect(scrollEditorIfMounted(MISSING_PATH, WORKTREE_PATH, 5, 1)).toBe(false); + + expect(editor.setPosition).not.toHaveBeenCalled(); + expect(consumePendingCursorPosition(MISSING_PATH)).toEqual({ line: 5, column: 1 }); + }); +}); + +describe("useOpenFileAtLine", () => { + afterEach(() => { + getMonacoInstance.mockReset(); + consumePendingCursorPosition(APP_PATH); + }); + + it("opens and scrolls an already-mounted file when a target line is present", () => { + const openFile = vi.fn(); + const editor = createEditor(WORKTREE_APP_PATH); + getMonacoInstance.mockReturnValue({ editor: { getEditors: () => [editor] } }); + const { result } = renderHook(() => useOpenFileAtLine(openFile, 88, WORKTREE_PATH)); + + act(() => result.current(APP_PATH)); + + expect(openFile).toHaveBeenCalledWith(APP_PATH); + expect(editor.setPosition).toHaveBeenCalledWith({ lineNumber: 88, column: 1 }); + expect(consumePendingCursorPosition(APP_PATH)).toBeUndefined(); + }); + + it("opens without setting a pending position when the target line is missing", () => { + const openFile = vi.fn(); + const { result } = renderHook(() => useOpenFileAtLine(openFile, undefined, WORKTREE_PATH)); + + act(() => result.current(APP_PATH)); + + expect(openFile).toHaveBeenCalledWith(APP_PATH); + expect(getMonacoInstance).not.toHaveBeenCalled(); + expect(consumePendingCursorPosition(APP_PATH)).toBeUndefined(); + }); +}); diff --git a/apps/web/hooks/use-file-editors.ts b/apps/web/hooks/use-file-editors.ts index 0841ff460c..c52c2f10b4 100644 --- a/apps/web/hooks/use-file-editors.ts +++ b/apps/web/hooks/use-file-editors.ts @@ -76,6 +76,25 @@ export function scrollEditorIfMounted( return false; } +export function useOpenFileAtLine( + onOpenFile: ((path: string) => void) | undefined, + startLine: number | undefined, + worktreePath: string | null | undefined, +) { + return useCallback( + (path: string) => { + if (startLine && startLine > 0) { + setPendingCursorPosition(path, startLine, 1); + onOpenFile?.(path); + scrollEditorIfMounted(path, worktreePath ?? null, startLine, 1); + return; + } + onOpenFile?.(path); + }, + [onOpenFile, startLine, worktreePath], + ); +} + /** Read openFiles from the store without subscribing to changes. */ function getOpenFiles() { return useDockviewStore.getState().openFiles;