diff --git a/apps/web/components/diff/diff-header-toolbar.test.tsx b/apps/web/components/diff/diff-header-toolbar.test.tsx index d09e4f492f..22bca04153 100644 --- a/apps/web/components/diff/diff-header-toolbar.test.tsx +++ b/apps/web/components/diff/diff-header-toolbar.test.tsx @@ -87,7 +87,19 @@ describe("useDiffHeaderToolbar", () => { renderToolbar(node as ReactElement); fireEvent.click(screen.getByLabelText("Edit")); - expect(onOpenFile).toHaveBeenCalledWith("src/renamed.ts"); + expect(onOpenFile).toHaveBeenCalledWith("src/renamed.ts", undefined); + }); + + it("forwards the repo subpath to onOpenFile so multi-repo files open under the right repository", () => { + const onOpenFile = vi.fn(); + const { result } = renderHook(() => + useDiffHeaderToolbar({ ...baseOpts, onOpenFile, repo: "enrichment-commons" }), + ); + const node = result.current(headerProp("src/foo.ts")); + renderToolbar(node as ReactElement); + + fireEvent.click(screen.getByLabelText("Edit")); + expect(onOpenFile).toHaveBeenCalledWith("src/foo.ts", "enrichment-commons"); }); it("falls back to opts.filePath when the callback prop has no fileDiff.name", () => { @@ -97,7 +109,7 @@ describe("useDiffHeaderToolbar", () => { renderToolbar(node as ReactElement); fireEvent.click(screen.getByLabelText("Edit")); - expect(onOpenFile).toHaveBeenCalledWith(FILE); + expect(onOpenFile).toHaveBeenCalledWith(FILE, undefined); }); it("shows the markdown preview button only for .md/.mdx files", () => { diff --git a/apps/web/components/diff/diff-header-toolbar.tsx b/apps/web/components/diff/diff-header-toolbar.tsx index bc663039c1..322d27f130 100644 --- a/apps/web/components/diff/diff-header-toolbar.tsx +++ b/apps/web/components/diff/diff-header-toolbar.tsx @@ -56,9 +56,11 @@ interface DiffHeaderToolbarOptions { onToggleWordWrap: () => void; viewMode: ViewMode; onToggleViewMode: () => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; + /** Multi-repo subpath (repository_name) so Edit opens under the right repo. */ + repo?: string; expandUnchanged?: boolean; onToggleExpandUnchanged?: () => void; } @@ -81,6 +83,7 @@ function DiffHeaderToolbarButtons({ onToggleViewMode, onOpenFile, onPreviewMarkdown, + repo, isMarkdownFile, }: ToolbarButtonsProps) { return ( @@ -139,7 +142,7 @@ function DiffHeaderToolbarButtons({ )} {onOpenFile && ( - onOpenFile(resolvedPath)} tooltip="Edit"> + onOpenFile(resolvedPath, repo)} tooltip="Edit"> )} @@ -163,6 +166,7 @@ export function useDiffHeaderToolbar(opts: DiffHeaderToolbarOptions) { onOpenFile, onPreviewMarkdown, onRevert, + repo, expandUnchanged, onToggleExpandUnchanged, } = opts; @@ -182,6 +186,7 @@ export function useDiffHeaderToolbar(opts: DiffHeaderToolbarOptions) { onOpenFile={onOpenFile} onPreviewMarkdown={onPreviewMarkdown} onRevert={onRevert} + repo={repo} expandUnchanged={expandUnchanged} onToggleExpandUnchanged={onToggleExpandUnchanged} /> @@ -197,6 +202,7 @@ export function useDiffHeaderToolbar(opts: DiffHeaderToolbarOptions) { onOpenFile, onPreviewMarkdown, onRevert, + repo, expandUnchanged, onToggleExpandUnchanged, ], diff --git a/apps/web/components/diff/diff-viewer-resolver.tsx b/apps/web/components/diff/diff-viewer-resolver.tsx index 4e3a8549b9..d194c55a84 100644 --- a/apps/web/components/diff/diff-viewer-resolver.tsx +++ b/apps/web/components/diff/diff-viewer-resolver.tsx @@ -22,7 +22,7 @@ interface DiffViewerResolverProps { className?: string; compact?: boolean; hideHeader?: boolean; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; enableAcceptReject?: boolean; diff --git a/apps/web/components/diff/diff-viewer.tsx b/apps/web/components/diff/diff-viewer.tsx index f33c85fd60..1f36e42aa8 100644 --- a/apps/web/components/diff/diff-viewer.tsx +++ b/apps/web/components/diff/diff-viewer.tsx @@ -30,7 +30,7 @@ interface DiffViewerProps { className?: string; compact?: boolean; hideHeader?: boolean; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; enableAcceptReject?: boolean; @@ -124,7 +124,7 @@ type WiringArgs = { data: FileDiffData; state: ReturnType; onCommentRun?: (comment: DiffComment) => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; enableComments: boolean; @@ -136,6 +136,7 @@ type WiringArgs = { expandUnchanged: boolean; toggleExpandUnchanged: () => void; wrapperRef: React.RefObject; + repo?: string; }; /** @@ -181,6 +182,7 @@ function useDiffViewerWiring(args: WiringArgs) { enableExpansion: canUseExpansion, expandUnchanged: args.expandUnchanged, onToggleExpandUnchanged: canUseExpansion ? args.toggleExpandUnchanged : undefined, + repo: args.repo, }); return { ...opts, renderAnnotation }; } @@ -250,6 +252,7 @@ export const DiffViewer = memo(function DiffViewer({ expandUnchanged, toggleExpandUnchanged, wrapperRef, + repo, }); const controlledSelection = state.showCommentForm ? state.selectedLines : null; diff --git a/apps/web/components/diff/file-diff-viewer.tsx b/apps/web/components/diff/file-diff-viewer.tsx index 754ea1b3e5..d941767a93 100644 --- a/apps/web/components/diff/file-diff-viewer.tsx +++ b/apps/web/components/diff/file-diff-viewer.tsx @@ -19,7 +19,7 @@ interface FileDiffViewerProps { className?: string; compact?: boolean; hideHeader?: boolean; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; enableAcceptReject?: boolean; diff --git a/apps/web/components/diff/use-diff-options.tsx b/apps/web/components/diff/use-diff-options.tsx index 529fafabe3..22c0f6b458 100644 --- a/apps/web/components/diff/use-diff-options.tsx +++ b/apps/web/components/diff/use-diff-options.tsx @@ -88,9 +88,11 @@ type UseDiffOptionsArgs = { handleLineSelectionEnd: (range: SelectedLineRange | null) => void; onLineEnter: (props: { lineType?: string; lineNumber?: number; annotationSide?: string }) => void; onLineLeave: () => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onRevert?: (filePath: string) => void; + /** Multi-repo subpath (repository_name) so Edit opens under the right repo. */ + repo?: string; /** Enable diff expansion (requires full deletionLines/additionLines in metadata) */ enableExpansion?: boolean; /** Number of lines to expand per click (default: 20) */ @@ -122,6 +124,7 @@ export function useDiffOptions(args: UseDiffOptionsArgs): UseDiffOptionsResult { onOpenFile, onPreviewMarkdown, onRevert, + repo, enableExpansion = false, expansionLineCount = 20, expandUnchanged, @@ -148,6 +151,7 @@ export function useDiffOptions(args: UseDiffOptionsArgs): UseDiffOptionsResult { onOpenFile, onPreviewMarkdown, onRevert, + repo, expandUnchanged, onToggleExpandUnchanged, }); diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx index 8a3b1f624e..3a4cddf217 100644 --- a/apps/web/components/review/review-diff-list.tsx +++ b/apps/web/components/review/review-diff-list.tsx @@ -28,7 +28,7 @@ type ReviewDiffListProps = { selectedFile?: string | null; onToggleReviewed: (path: string, reviewed: boolean) => void; onDiscard: (path: string) => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; fileRefs: Map>; }; @@ -119,7 +119,7 @@ type FileDiffSectionProps = { forceLoad?: boolean; onToggleReviewed: (key: string, reviewed: boolean) => void; onDiscard: (key: string) => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; sectionRef?: React.RefObject; scrollContainer: React.RefObject; @@ -204,7 +204,7 @@ type FileDiffHeaderProps = { expandUnchanged: boolean; onCheckboxChange: (checked: boolean | "indeterminate") => void; onDiscard: () => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onToggleCollapse: () => void; onToggleExpandUnchanged: () => void; @@ -268,6 +268,7 @@ function FileDiffHeader({ onPreviewMarkdown={onPreviewMarkdown} onToggleExpandUnchanged={onToggleExpandUnchanged} onToggleWordWrap={onToggleWordWrap} + repo={file.repository_name} /> ); diff --git a/apps/web/components/review/review-diff-toolbar.tsx b/apps/web/components/review/review-diff-toolbar.tsx index 54fc50fde8..e68ad216c2 100644 --- a/apps/web/components/review/review-diff-toolbar.tsx +++ b/apps/web/components/review/review-diff-toolbar.tsx @@ -33,10 +33,13 @@ export type FileDiffToolbarProps = { wordWrap: boolean; expandUnchanged: boolean; onDiscard: () => void; - onOpenFile?: (filePath: string) => void; + onOpenFile?: (filePath: string, repo?: string) => void; onPreviewMarkdown?: (filePath: string) => void; onToggleExpandUnchanged: () => void; onToggleWordWrap: () => void; + /** Multi-repo subpath (repository_name) so the Edit action opens the file + * under the right repository instead of the bare task root. */ + repo?: string; }; function ToolbarIconBtn({ @@ -85,6 +88,7 @@ export function FileDiffToolbar(props: FileDiffToolbarProps) { onPreviewMarkdown, onToggleExpandUnchanged, onToggleWordWrap, + repo, } = props; const [globalViewMode, setGlobalViewMode] = useGlobalViewMode(); const handleCopyDiff = useCallback(() => { @@ -129,7 +133,7 @@ export function FileDiffToolbar(props: FileDiffToolbarProps) { )} {onOpenFile && ( - onOpenFile(filePath)} tooltip="Edit"> + onOpenFile(filePath, repo)} tooltip="Edit"> )} diff --git a/apps/web/components/task/file-editor-panel.tsx b/apps/web/components/task/file-editor-panel.tsx index 09544da18c..58c0a89d0a 100644 Binary files a/apps/web/components/task/file-editor-panel.tsx and b/apps/web/components/task/file-editor-panel.tsx differ diff --git a/apps/web/hooks/file-editors-sync.test.ts b/apps/web/hooks/file-editors-sync.test.ts index 7899d9bafa..e1f63dde56 100644 --- a/apps/web/hooks/file-editors-sync.test.ts +++ b/apps/web/hooks/file-editors-sync.test.ts @@ -110,7 +110,7 @@ describe("syncOpenFileFromWorkspace", () => { updateFileState, }); - expect(mockRequestFileContent).toHaveBeenCalledWith(FAKE_CLIENT, SESSION_ID, PATH); + expect(mockRequestFileContent).toHaveBeenCalledWith(FAKE_CLIENT, SESSION_ID, PATH, undefined); expect(updateFileState).toHaveBeenCalledTimes(1); expect(updateFileState).toHaveBeenCalledWith( PATH, @@ -175,6 +175,67 @@ describe("syncOpenFileFromWorkspace", () => { }); }); +describe("syncOpenFileFromWorkspace repo scoping", () => { + let updateFileState: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + updateFileState = vi.fn(); + }); + + it("forwards the file's repo subpath so multi-repo files resolve under the right repository", async () => { + // Multi-repo task: foo.ts lives inside the "enrichment-commons" repo. The + // open editor buffer carries that repo. Re-syncing must pass `repo` to the + // backend, otherwise it stats /src/foo.ts (bare task root) and + // fails with "file not found" — the reported "Failed to edit" bug. + seedOpenFile({ + content: "v1", + originalContent: "v1", + originalHash: "h:2:v1", + repo: "enrichment-commons", + }); + mockRequestFileContent.mockResolvedValueOnce({ + content: "v2", + is_binary: false, + resolved_path: PATH, + }); + + await syncOpenFileFromWorkspace({ + client: FAKE_CLIENT, + sessionId: SESSION_ID, + path: PATH, + updateFileState, + }); + + expect(mockRequestFileContent).toHaveBeenCalledWith( + FAKE_CLIENT, + SESSION_ID, + PATH, + "enrichment-commons", + ); + }); + + it("drops the response if the tab was swapped to a different repo mid-fetch", async () => { + // The fetch is issued for repo "repoA". While it is in flight the same path + // key is reused for a file from "repoB". Writing repoA's content into the + // repoB buffer would be wrong, so the stale response must be discarded. + seedOpenFile({ content: "v1", originalContent: "v1", repo: "repoA" }); + mockRequestFileContent.mockImplementationOnce(async () => { + seedOpenFile({ content: "other", originalContent: "other", repo: "repoB" }); + return { content: "repoA-content", is_binary: false, resolved_path: PATH }; + }); + + await syncOpenFileFromWorkspace({ + client: FAKE_CLIENT, + sessionId: SESSION_ID, + path: PATH, + updateFileState, + }); + + expect(updateFileState).not.toHaveBeenCalled(); + }); +}); + function makeStatus(files: Record, timestamp: string): GitStatusEntry { return { branch: "main", diff --git a/apps/web/hooks/file-editors-sync.ts b/apps/web/hooks/file-editors-sync.ts index 5ad2915fef..08676c5b61 100644 --- a/apps/web/hooks/file-editors-sync.ts +++ b/apps/web/hooks/file-editors-sync.ts @@ -52,9 +52,14 @@ export async function syncOpenFileFromWorkspace({ }: SyncOpenFileArgs): Promise { if (!client) return; try { - const response = await requestFileContent(client, sessionId, path); + const repo = useDockviewStore.getState().openFiles.get(path)?.repo; + const response = await requestFileContent(client, sessionId, path, repo); const latest = useDockviewStore.getState().openFiles.get(path); if (!latest) return; + // The tab may have been swapped to a different repo's file at the same path + // key while the fetch was in flight; the response is then for the old repo. + // Drop it rather than writing stale content into the new buffer. + if (latest.repo !== repo) return; const remoteHash = await calculateHash(response.content); if (latest.isDirty) { diff --git a/apps/web/hooks/use-file-editors.build-state.test.ts b/apps/web/hooks/use-file-editors.build-state.test.ts new file mode 100644 index 0000000000..d0ea66a9eb --- /dev/null +++ b/apps/web/hooks/use-file-editors.build-state.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { FileContentResponse } from "@/lib/types/backend"; + +const mockRequestFileContent = vi.fn(); + +vi.mock("@/lib/utils/file-diff", () => ({ + calculateHash: async (s: string) => `h:${s.length}`, +})); + +vi.mock("@/lib/ws/workspace-files", () => ({ + requestFileContent: (...args: unknown[]) => mockRequestFileContent(...args), +})); + +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => ({}), +})); + +import { buildFileEditorState, fetchFileEditorState } from "./use-file-editors"; + +const PATH = "src/foo.ts"; +const REPO = "enrichment-commons"; +const SESSION_ID = "sess-1"; + +const RESPONSE: FileContentResponse = { + path: PATH, + content: "v1", + is_binary: false, +} as FileContentResponse; + +const FAKE_CLIENT = {} as NonNullable< + ReturnType +>; + +describe("buildFileEditorState", () => { + it("carries the repo subpath so subsequent save/sync calls scope to the right repository", async () => { + // Multi-repo open: opening foo.ts from the "enrichment-commons" repo must + // record `repo` on the editor state, otherwise later save/sync requests + // drop it and the backend stats the bare task root → "file not found". + const state = await buildFileEditorState(PATH, RESPONSE, REPO); + expect(state.repo).toBe(REPO); + }); + + it("leaves repo undefined for single-repo tasks", async () => { + const state = await buildFileEditorState(PATH, RESPONSE); + expect(state.repo).toBeUndefined(); + }); +}); + +describe("fetchFileEditorState", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns the built state (with repo) when the session is unchanged", async () => { + mockRequestFileContent.mockResolvedValueOnce(RESPONSE); + const ref = { current: SESSION_ID }; + + const state = await fetchFileEditorState(FAKE_CLIENT, SESSION_ID, PATH, REPO, ref); + + expect(mockRequestFileContent).toHaveBeenCalledWith(FAKE_CLIENT, SESSION_ID, PATH, REPO); + expect(state?.repo).toBe(REPO); + }); + + it("returns null when the active session changed while the fetch was in flight", async () => { + // User switches tasks mid-fetch: the late response must not be applied to + // the new session's editor state. + const ref = { current: SESSION_ID }; + mockRequestFileContent.mockImplementationOnce(async () => { + ref.current = "sess-2"; + return RESPONSE; + }); + + const state = await fetchFileEditorState(FAKE_CLIENT, SESSION_ID, PATH, undefined, ref); + + expect(state).toBeNull(); + }); +}); diff --git a/apps/web/hooks/use-file-editors.ts b/apps/web/hooks/use-file-editors.ts index 253bed1588..77925e3454 100644 --- a/apps/web/hooks/use-file-editors.ts +++ b/apps/web/hooks/use-file-editors.ts @@ -44,15 +44,22 @@ function getOpenFiles() { return useDockviewStore.getState().openFiles; } -/** Build a FileEditorState from a file content response. */ -async function buildFileEditorState( +/** + * Build a FileEditorState from a file content response. `repo` is the + * multi-repo subpath (repository_name) the file belongs to; it is recorded on + * the state so subsequent save/sync/delete requests stay scoped to the right + * repository instead of resolving against the bare task root. + */ +export async function buildFileEditorState( filePath: string, response: FileContentResponse, + repo?: string, ): Promise { const fileName = filePath.split("/").pop() || filePath; const hash = await calculateHash(response.content); return { path: filePath, + repo, name: fileName, content: response.content, originalContent: response.content, @@ -63,6 +70,47 @@ async function buildFileEditorState( }; } +/** + * Fetch a file's content and build its editor state, returning null if the + * active session changed while the request was in flight — a late response must + * not write content for a file the user has navigated away from. + */ +export async function fetchFileEditorState( + client: NonNullable>, + sessionId: string, + filePath: string, + repo: string | undefined, + activeSessionIdRef: React.MutableRefObject, +): Promise { + const response = await requestFileContent(client, sessionId, filePath, repo); + if (activeSessionIdRef.current !== sessionId) return null; + return buildFileEditorState(filePath, response, repo); +} + +/** + * Apply an editor buffer change, auto-promoting a preview tab to pinned on the + * first edit so the user's unsaved changes aren't discarded when another file + * is opened. Promote BEFORE updating state so the openFiles subscription sees + * the promoted flag when it fires from updateFileState. + */ +function applyFileChange( + path: string, + newContent: string, + updateFileState: (path: string, updates: Partial) => void, + promotePreviewToPinned: (type: "file-editor") => void, +) { + const file = getOpenFiles().get(path); + if (!file) return; + const nextIsDirty = newContent !== file.originalContent; + if (nextIsDirty && !file.isDirty) { + const preview = useDockviewStore.getState().api?.getPanel(PREVIEW_FILE_EDITOR_ID); + if ((preview?.params as Record | undefined)?.previewItemId === path) { + promotePreviewToPinned("file-editor"); + } + } + updateFileState(path, { content: newContent, isDirty: nextIsDirty }); +} + /** Build the sessionStorage tab records from live openFiles + dockview state. */ function buildPersistedTabs( api: ReturnType["api"], @@ -72,27 +120,39 @@ function buildPersistedTabs( const previewParams = preview?.params as Record | undefined; const previewPath = (previewParams?.previewItemId ?? null) as string | null; const isPromoted = previewParams?.promoted === true; - return Array.from(openFiles.values()).flatMap(({ path, name, markdownPreview }) => { + return Array.from(openFiles.values()).flatMap(({ path, name, repo, markdownPreview }) => { const isPinned = !!api?.getPanel(`file:${path}`); const isPreview = !isPinned && path === previewPath; if (!isPinned && !isPreview) return []; // Promoted previews persist as pinned so edits survive refresh const persistAsPinned = isPinned || (isPreview && isPromoted); return [ - { path, name, ...(markdownPreview ? { markdownPreview } : {}), pinned: persistAsPinned }, + { + path, + name, + ...(repo ? { repo } : {}), + ...(markdownPreview ? { markdownPreview } : {}), + pinned: persistAsPinned, + }, ]; }); } type RestoreTabsParams = { activeSessionId: string; - savedTabs: Array<{ path: string; name: string; markdownPreview?: boolean; pinned?: boolean }>; + savedTabs: Array<{ + path: string; + name: string; + repo?: string; + markdownPreview?: boolean; + pinned?: boolean; + }>; savedActiveTab: string; setFileState: (path: string, state: FileEditorState) => void; addFileEditorPanel: ( path: string, name: string, - opts?: { quiet?: boolean; pin?: boolean }, + opts?: { quiet?: boolean; pin?: boolean; repo?: string }, ) => void; }; @@ -139,6 +199,7 @@ async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Pr addFileEditorPanel(savedTab.path, savedTab.name, { quiet: true, pin: savedTab.pinned, + repo: savedTab.repo, }); // Seed a placeholder file state synchronously, carrying the restored // `markdownPreview` flag. This makes `openFiles.has(path)` true the moment @@ -149,6 +210,7 @@ async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Pr // restored preview flag is clobbered and the tab reopens in code view. setFileState(savedTab.path, { path: savedTab.path, + repo: savedTab.repo, name: savedTab.name, content: "", originalContent: "", @@ -159,10 +221,16 @@ async function loadAndRestoreTabs(params: RestoreTabsParams, retryCount = 0): Pr } for (const savedTab of savedTabs) { try { - const response = await requestFileContent(client, activeSessionId, savedTab.path); + const response = await requestFileContent( + client, + activeSessionId, + savedTab.path, + savedTab.repo, + ); const hash = await calculateHash(response.content); setFileState(savedTab.path, { path: savedTab.path, + repo: savedTab.repo, name: savedTab.name, content: response.content, originalContent: response.content, @@ -187,7 +255,7 @@ type FileEditorEffectsParams = { addFileEditorPanel: ( path: string, name: string, - opts?: { quiet?: boolean; pin?: boolean }, + opts?: { quiet?: boolean; pin?: boolean; repo?: string }, ) => void; clearFileStates: () => void; removeFileState: (path: string) => void; @@ -275,7 +343,7 @@ type FileEditorActionsParams = { addFileEditorPanel: ( path: string, name: string, - opts?: { quiet?: boolean; pin?: boolean }, + opts?: { quiet?: boolean; pin?: boolean; repo?: string }, ) => void; promotePreviewToPinned: (type: "file-editor") => void; setSavingFiles: React.Dispatch>>; @@ -292,26 +360,29 @@ function useFileEditorActions({ toast, }: FileEditorActionsParams) { const openFile = useCallback( - async (filePath: string) => { + async (filePath: string, repo?: string) => { const client = getWebSocketClient(); const currentSessionId = activeSessionIdRef.current; if (!client || !currentSessionId) return; const files = getOpenFiles(); if (files.has(filePath)) { - addFileEditorPanel(filePath, filePath.split("/").pop() || filePath); + const tabName = filePath.split("/").pop() || filePath; + addFileEditorPanel(filePath, tabName, { repo: files.get(filePath)?.repo }); return; } try { - const response: FileContentResponse = await requestFileContent( + const state = await fetchFileEditorState( client, currentSessionId, filePath, + repo, + activeSessionIdRef, ); - const state = await buildFileEditorState(filePath, response); + if (!state) return; // Create the panel BEFORE setting file state. The openFiles subscription // triggers tab persistence — it needs the dockview panel to already exist // so buildPersistedTabs can detect whether the file is preview or pinned. - addFileEditorPanel(filePath, state.name); + addFileEditorPanel(filePath, state.name, { repo }); setFileState(filePath, state); } catch (error) { toast({ @@ -325,22 +396,8 @@ function useFileEditorActions({ ); const handleFileChange = useCallback( - (path: string, newContent: string) => { - const file = getOpenFiles().get(path); - if (!file) return; - const nextIsDirty = newContent !== file.originalContent; - // VSCode-style: editing a preview file auto-promotes its tab so the - // user's unsaved changes aren't silently discarded when they open - // another file. Promote BEFORE updating file state so the openFiles - // subscription sees the promoted flag when it fires from updateFileState. - if (nextIsDirty && !file.isDirty) { - const preview = useDockviewStore.getState().api?.getPanel(PREVIEW_FILE_EDITOR_ID); - if ((preview?.params as Record | undefined)?.previewItemId === path) { - promotePreviewToPinned("file-editor"); - } - } - updateFileState(path, { content: newContent, isDirty: nextIsDirty }); - }, + (path: string, newContent: string) => + applyFileChange(path, newContent, updateFileState, promotePreviewToPinned), [updateFileState, promotePreviewToPinned], ); @@ -352,24 +409,27 @@ function useFileEditorActions({ }); const openFileInMarkdownPreview = useCallback( - async (filePath: string) => { + async (filePath: string, repo?: string) => { const client = getWebSocketClient(); const currentSessionId = activeSessionIdRef.current; if (!client || !currentSessionId) return; const files = getOpenFiles(); if (files.has(filePath)) { updateFileState(filePath, { markdownPreview: true }); - addFileEditorPanel(filePath, filePath.split("/").pop() || filePath); + const name = filePath.split("/").pop() || filePath; + addFileEditorPanel(filePath, name, { repo: files.get(filePath)?.repo }); return; } try { - const response: FileContentResponse = await requestFileContent( + const state = await fetchFileEditorState( client, currentSessionId, filePath, + repo, + activeSessionIdRef, ); - const state = await buildFileEditorState(filePath, response); - addFileEditorPanel(filePath, state.name); + if (!state) return; + addFileEditorPanel(filePath, state.name, { repo }); setFileState(filePath, { ...state, markdownPreview: true }); } catch (error) { toast({ diff --git a/apps/web/hooks/use-file-save-delete.test.ts b/apps/web/hooks/use-file-save-delete.test.ts new file mode 100644 index 0000000000..0c7a7874ca --- /dev/null +++ b/apps/web/hooks/use-file-save-delete.test.ts @@ -0,0 +1,104 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useRef } from "react"; +import type { FileEditorState } from "@/lib/state/dockview-store"; + +const mockUpdateFileContent = vi.fn(); +const mockDeleteFile = vi.fn(); +const mockGetWebSocketClient = vi.fn(); +let openFilesMap = new Map(); + +vi.mock("@/lib/ws/workspace-files", () => ({ + updateFileContent: (...args: unknown[]) => mockUpdateFileContent(...args), + deleteFile: (...args: unknown[]) => mockDeleteFile(...args), +})); + +vi.mock("@/lib/ws/connection", () => ({ + getWebSocketClient: () => mockGetWebSocketClient(), +})); + +vi.mock("@/lib/state/dockview-store", () => ({ + useDockviewStore: { + getState: () => ({ openFiles: openFilesMap, api: null }), + }, +})); + +vi.mock("@/lib/utils/file-diff", () => ({ + generateUnifiedDiff: () => "@@ -1 +1 @@\n-v1\n+v2", + calculateHash: async (s: string) => `h:${s.length}`, +})); + +import { useSaveDeleteActions, type SaveDeleteParams } from "./use-file-save-delete"; + +const FAKE_CLIENT = {} as ReturnType; +const SESSION_ID = "sess-1"; +const PATH = "src/foo.ts"; +const REPO = "enrichment-commons"; + +function seedOpenFile(state: Partial = {}) { + openFilesMap = new Map([ + [ + PATH, + { + path: PATH, + name: "foo.ts", + content: "v2", + originalContent: "v1", + originalHash: "h:2", + isDirty: true, + ...state, + }, + ], + ]); +} + +function renderActions() { + return renderHook(() => { + const activeSessionIdRef = useRef(SESSION_ID); + const params: SaveDeleteParams = { + activeSessionIdRef, + updateFileState: vi.fn(), + setSavingFiles: vi.fn(), + toast: vi.fn(), + }; + return useSaveDeleteActions(params); + }); +} + +describe("useSaveDeleteActions repo threading", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockGetWebSocketClient.mockReturnValue(FAKE_CLIENT); + }); + + it("saveFile forwards the file's repo so the diff applies under the right repository", async () => { + // Multi-repo task: editing foo.ts inside "enrichment-commons" and saving + // must scope the update to that repo, else the backend writes against the + // bare task root and the save fails with "file not found". + seedOpenFile({ repo: REPO }); + mockUpdateFileContent.mockResolvedValueOnce({ success: true, new_hash: "h:2" }); + + const { result } = renderActions(); + await act(async () => { + await result.current.saveFile(PATH); + }); + + expect(mockUpdateFileContent).toHaveBeenCalledWith( + FAKE_CLIENT, + SESSION_ID, + expect.objectContaining({ path: PATH, repo: REPO }), + ); + }); + + it("deleteFile forwards the file's repo", async () => { + seedOpenFile({ repo: REPO }); + mockDeleteFile.mockResolvedValueOnce({ success: true }); + + const { result } = renderActions(); + await act(async () => { + await result.current.deleteFileAction(PATH); + }); + + expect(mockDeleteFile).toHaveBeenCalledWith(FAKE_CLIENT, SESSION_ID, PATH, REPO); + }); +}); diff --git a/apps/web/hooks/use-file-save-delete.ts b/apps/web/hooks/use-file-save-delete.ts index d44ff2ac94..00af7fd822 100644 --- a/apps/web/hooks/use-file-save-delete.ts +++ b/apps/web/hooks/use-file-save-delete.ts @@ -30,6 +30,20 @@ export function updatePanelAfterSave(path: string, name: string) { } } +/** Close the pinned (or preview) editor panel for a path after a remote delete. */ +function closeFileEditorPanel(path: string) { + const dockApi = useDockviewStore.getState().api; + const pinned = dockApi?.getPanel(`file:${path}`); + if (pinned) { + dockApi?.removePanel(pinned); + return; + } + const preview = dockApi?.getPanel(PREVIEW_FILE_EDITOR_ID); + if (preview && (preview.params as Record)?.previewItemId === path) { + dockApi?.removePanel(preview); + } +} + export type SaveDeleteParams = { activeSessionIdRef: React.MutableRefObject; updateFileState: (path: string, updates: Partial) => void; @@ -51,6 +65,7 @@ async function performSaveFile(path: string, params: SaveDeleteParams) { diff, originalHash: file.originalHash, desiredContent: file.content, + repo: file.repo, }); if (response.success && response.new_hash) { // Re-read current state: user may have typed more while the save was @@ -107,7 +122,8 @@ export function useSaveDeleteActions(params: SaveDeleteParams) { const currentSessionId = activeSessionIdRef.current; if (!client || !currentSessionId) return; try { - const response = await deleteFile(client, currentSessionId, path); + const repo = getOpenFiles().get(path)?.repo; + const response = await deleteFile(client, currentSessionId, path, repo); if (!response.success) { toast({ title: "Delete failed", @@ -126,16 +142,7 @@ export function useSaveDeleteActions(params: SaveDeleteParams) { return; } // Close the panel only after the remote delete succeeds. - const dockApi = useDockviewStore.getState().api; - const pinned = dockApi?.getPanel(`file:${path}`); - if (pinned) { - dockApi?.removePanel(pinned); - } else { - const preview = dockApi?.getPanel(PREVIEW_FILE_EDITOR_ID); - if (preview && (preview.params as Record)?.previewItemId === path) { - dockApi?.removePanel(preview); - } - } + closeFileEditorPanel(path); }, [activeSessionIdRef, toast], ); diff --git a/apps/web/hooks/use-panel-actions.test.ts b/apps/web/hooks/use-panel-actions.test.ts index 57ec17439f..07379c555e 100644 --- a/apps/web/hooks/use-panel-actions.test.ts +++ b/apps/web/hooks/use-panel-actions.test.ts @@ -74,10 +74,20 @@ describe("usePanelActions", () => { }); expect(dockActions.addPlanPanel).toHaveBeenCalledOnce(); - expect(editorActions.openFile).toHaveBeenCalledWith("README.md"); + expect(editorActions.openFile).toHaveBeenCalledWith("README.md", undefined); expect(layoutActions.openDocument).not.toHaveBeenCalled(); }); + it("forwards the repo subpath when opening a multi-repo file", () => { + const { result } = renderHook(() => usePanelActions()); + + act(() => { + result.current.openFile("src/foo.ts", "enrichment-commons"); + }); + + expect(editorActions.openFile).toHaveBeenCalledWith("src/foo.ts", "enrichment-commons"); + }); + it("keeps non-workbench tablet fallbacks on the legacy layout store", () => { responsiveState.value = { usesDesktopWorkbench: false }; const { result } = renderHook(() => usePanelActions()); diff --git a/apps/web/hooks/use-panel-actions.ts b/apps/web/hooks/use-panel-actions.ts index 90dce84889..14e87f628c 100644 --- a/apps/web/hooks/use-panel-actions.ts +++ b/apps/web/hooks/use-panel-actions.ts @@ -93,18 +93,18 @@ export function usePanelActions() { }, [usesDesktopWorkbench, dockAddVscode]); const openFile = useCallback( - (filePath: string) => { + (filePath: string, repo?: string) => { if (usesDesktopWorkbench) { - dockOpenFile(filePath); + dockOpenFile(filePath, repo); } }, [usesDesktopWorkbench, dockOpenFile], ); const openFileInMarkdownPreview = useCallback( - (filePath: string) => { + (filePath: string, repo?: string) => { if (usesDesktopWorkbench) { - dockOpenFileInPreview(filePath); + dockOpenFileInPreview(filePath, repo); } }, [usesDesktopWorkbench, dockOpenFileInPreview], diff --git a/apps/web/lib/local-storage.test.ts b/apps/web/lib/local-storage.test.ts index 07c3fde030..c0e869973e 100644 --- a/apps/web/lib/local-storage.test.ts +++ b/apps/web/lib/local-storage.test.ts @@ -3,10 +3,12 @@ import { cleanupTaskStorage, clearGlobalSidebarWidth, getGlobalSidebarWidth, + getOpenFileTabs, markPRClosedBannerDismissed, markPRMergedBannerDismissed, restoreAttachmentPreview, setGlobalSidebarWidth, + setOpenFileTabs, wasPRClosedBannerDismissed, wasPRMergedBannerDismissed, } from "./local-storage"; @@ -105,6 +107,39 @@ describe("global sidebar width storage", () => { }); }); +describe("open file tabs storage", () => { + beforeEach(() => { + window.sessionStorage.clear(); + }); + + it("round-trips the multi-repo repo subpath so a restored tab refetches under the right repo", () => { + setOpenFileTabs("sess-1", [ + { + path: "src/foo.ts", + name: "foo.ts", + repo: "enrichment-commons", + markdownPreview: true, + pinned: true, + }, + ]); + + const tabs = getOpenFileTabs("sess-1"); + expect(tabs).toHaveLength(1); + expect(tabs[0]).toEqual({ + path: "src/foo.ts", + name: "foo.ts", + repo: "enrichment-commons", + markdownPreview: true, + pinned: true, + }); + }); + + it("leaves repo undefined for single-repo tabs", () => { + setOpenFileTabs("sess-1", [{ path: "src/foo.ts", name: "foo.ts", pinned: true }]); + expect(getOpenFileTabs("sess-1")[0].repo).toBeUndefined(); + }); +}); + describe("chat draft attachment storage", () => { it("normalizes invalid restored image delivery modes to prompt", () => { const restored = restoreAttachmentPreview({ diff --git a/apps/web/lib/local-storage.ts b/apps/web/lib/local-storage.ts index abb17c2c58..4e0c71ede5 100644 --- a/apps/web/lib/local-storage.ts +++ b/apps/web/lib/local-storage.ts @@ -443,6 +443,9 @@ const ACTIVE_TAB_KEY = "kandev.activeTab"; export interface StoredFileTab { path: string; name: string; + /** Multi-repo subpath (repository_name) so a restored tab re-fetches its + * content under the right repository after a refresh. */ + repo?: string; markdownPreview?: boolean; pinned?: boolean; } diff --git a/apps/web/lib/state/dockview-panel-actions.ts b/apps/web/lib/state/dockview-panel-actions.ts index 0730919a86..ed5bb24ddb 100644 --- a/apps/web/lib/state/dockview-panel-actions.ts +++ b/apps/web/lib/state/dockview-panel-actions.ts @@ -233,6 +233,9 @@ export type OpenPanelOpts = { quiet?: boolean; /** Force the per-item pinned panel instead of the shared preview slot. */ pin?: boolean; + /** Multi-repo subpath (repository_name) recorded on the panel params so + * FileEditorPanel's fallback content fetch resolves under the right repo. */ + repo?: string; }; export const PREVIEW_FILE_EDITOR_ID = "preview:file-editor"; @@ -247,7 +250,7 @@ function buildFileEditorAction(get: StoreGet) { type: "file-editor", itemId: path, title: name, - params: { path }, + params: { path, ...(opts?.repo ? { repo: opts.repo } : {}) }, groupId: centerGroupId, quiet: opts?.quiet, pin: opts?.pin, diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 0f60ba0b7f..aaaa42a2a4 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -67,6 +67,14 @@ export { applyLayoutFixups } from "./dockview-layout-builders"; export type FileEditorState = { path: string; + /** + * Multi-repo subpath (the repository_name, e.g. "enrichment-commons") this + * file belongs to; `path` is interpreted relative to it. Undefined for + * single-repo tasks. Threaded into every workspace file request so the + * backend resolves the file under the right repository directory instead of + * the bare task root. + */ + repo?: string; name: string; content: string; originalContent: string;