From 214a8f5cce6a2e384286bb3589c7346a8af7ae43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:59:05 +0100 Subject: [PATCH] fix: scope multi-repo file editor open/save to the right repo (#1382) * fix: scope multi-repo file editor open/save to the right repo The editor open/save/sync/delete chain dropped the repository_name in multi-repo task workspaces, so the backend resolved paths against the bare task root and failed with "file not found". Thread the repo subpath through openFile, save, sync, and delete, and store it on FileEditorState. * fix: pass repo from diff Edit button to multi-repo file editor The diff/changes "Edit" button opened files without their repository scope, so multi-repo files resolved against the bare task root and failed with "file not found". Thread repository_name through the diff and review toolbars into onOpenFile. * fix: preserve repo across tab restore and stale syncs Address review feedback: persisted file tabs dropped the repo subpath, so a multi-repo file reopened against the bare task root after a browser refresh. Persist and restore `repo` on StoredFileTab. Also guard the workspace sync against a tab being swapped to a different repo mid-fetch, and widen the remaining onOpenFile type declarations to carry repo. * fix: thread repo into editor panel fallback fetch Address review feedback: FileEditorPanel's useFileLoader fallback fetch (triggered when the open-files map is transiently empty, e.g. after a session switch) called requestFileContent without the repo subpath, re-introducing the multi-repo "file not found" failure. Record repo on the panel params and forward it through useFileLoader. Also assert the full restored-tab shape in the local-storage round-trip test. * fix: guard editor fetches against stale session/repo races Address review feedback: file open and the panel fallback fetch could apply a late response after the user navigated away. openFile now drops the result if the active session changed mid-fetch (via fetchFileEditorState), and the panel loader keys its in-flight guard by session+path+repo so a changed target re-fetches and stale responses are discarded. --- .../diff/diff-header-toolbar.test.tsx | 16 ++- .../components/diff/diff-header-toolbar.tsx | 10 +- .../components/diff/diff-viewer-resolver.tsx | 2 +- apps/web/components/diff/diff-viewer.tsx | 7 +- apps/web/components/diff/file-diff-viewer.tsx | 2 +- apps/web/components/diff/use-diff-options.tsx | 6 +- .../components/review/review-diff-list.tsx | 7 +- .../components/review/review-diff-toolbar.tsx | 8 +- .../web/components/task/file-editor-panel.tsx | Bin 9630 -> 10242 bytes apps/web/hooks/file-editors-sync.test.ts | 63 ++++++++- apps/web/hooks/file-editors-sync.ts | 7 +- .../use-file-editors.build-state.test.ts | 75 ++++++++++ apps/web/hooks/use-file-editors.ts | 130 +++++++++++++----- apps/web/hooks/use-file-save-delete.test.ts | 104 ++++++++++++++ apps/web/hooks/use-file-save-delete.ts | 29 ++-- apps/web/hooks/use-panel-actions.test.ts | 12 +- apps/web/hooks/use-panel-actions.ts | 8 +- apps/web/lib/local-storage.test.ts | 35 +++++ apps/web/lib/local-storage.ts | 3 + apps/web/lib/state/dockview-panel-actions.ts | 5 +- apps/web/lib/state/dockview-store.ts | 8 ++ 21 files changed, 469 insertions(+), 68 deletions(-) create mode 100644 apps/web/hooks/use-file-editors.build-state.test.ts create mode 100644 apps/web/hooks/use-file-save-delete.test.ts 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 09544da18c073408821524cd945a7b5eb2252f4a..58c0a89d0af83ec07cf0120497871cb1205facaf 100644 GIT binary patch delta 791 zcmaKqQH#?+6onO5X;U8*6uO{tTvpiHZoAJevFwYm?u+P~DC{)3X@*T_!px+l)TQEI zh~S&}6NHMt#Ygcs_$G*Vk}9lVpOQ?@J?FcJ`Ec^~>8FFn5-^nO;X_zwq#E~v4i*70 z7~q8Fa4A8m{Unp))L~o*6Js=oC2OTt-CVe|>*xD8P9W4AI`K0TM5$0?vY4eZquNg( zAs3~P7N$_jhrum5K+z18BosOSgF%FG7A!Yp)Ai7v;V3d}E>vwdB6fgfd*!k@MwyPz zDDE&tTw$|KM!cNOwX(d0MQn7QQw*&jjFD53;d}Px9jvY$Bdr{paju?J7Bi!LM7`|5 z?}o4$B+Eyj@(!-!Uy18HeY}~r@KToJ4%gP7MCs%vbbtzr5*WCbd?;nWree?5{Ut={4 zUR^%gF!>jMGs>6NqczIvN2`79;7{>YRUvX=#KiWiFJaeL8?RBQm=dWdUIp#1TS2`{ QhdV0s>+RLA+aDXh0ir+;r~m)} delta 201 zcmZn)nCHF0fJ0SNp_)rUAvr&7v^W*W&`3+nDNdcdj#Enr zA*+{MT2z#pS2Fnur;;S5Fi>+zQEBSrIxZI}tg;X@?{j%g*5}rl+|RvwvlmYwBR;iv fc { 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;