From e4a01185797050c3ebfb8e70df98b2c10815d8ac Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 24 Jun 2026 18:09:39 -0700 Subject: [PATCH 1/3] fix(workspace): suppress spurious "Not a git repository" toast in Git Manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening the Git Manager on a workspace project rendered the sub-repo dropdown correctly but ALSO toasted "Not a git repository". On open the section fetch fires immediately with no repoPath (selectedRepo unresolved), hitting the non-git browse-only workspace root; fetchWorkspaceRepos resolves a tick later and the fetch re-runs against a real sub-repo. We now track workspace detection in a ref and suppress that one benign root-race error (no repoPath + "Not a git repository" while detection is pending or has confirmed a workspace). A genuinely broken non-workspace project still surfaces the error: once detection settles as non-workspace, a single guarded re-fetch re-surfaces it (no redundant fetch in the common non-workspace path, preserving existing call-count expectations). Tests: add the missing fetchWorkspaceRepos api mock (pre-existing gap that broke the whole GitManagerModal suite at import), plus a positive (workspace → no toast) and negative-control (non-workspace broken → toast) regression. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/components/GitManagerModal.tsx | 63 ++++++++++++++++++- .../__tests__/GitManagerModal.test.tsx | 39 ++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/packages/dashboard/app/components/GitManagerModal.tsx b/packages/dashboard/app/components/GitManagerModal.tsx index e50f21bb78..3be5667be1 100644 --- a/packages/dashboard/app/components/GitManagerModal.tsx +++ b/packages/dashboard/app/components/GitManagerModal.tsx @@ -267,6 +267,24 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj const [workspaceRepos, setWorkspaceRepos] = useState([]); const [selectedRepo, setSelectedRepo] = useState(null); const gitRepoPath = selectedRepo ?? undefined; + /* + FNXC:Workspace 2026-06-25-00:10: + In a workspace the project root is a non-git browse-only directory. On modal open the section fetch + fires immediately with no repoPath (selectedRepo not yet resolved), so a git status against the root + returns "Not a git repository" and toasts a spurious error on every open — even though the repo + dropdown renders correctly. fetchWorkspaceRepos resolves a tick later and re-fetches against a real + sub-repo. We track detection status in a REF (read inside the async fetch catch without a stale + closure or extra render dep) so we can SUPPRESS that one benign root-race error: a "Not a git + repository" with no repoPath while detection is unresolved OR has detected a workspace. A genuine + broken non-workspace project (resolved, repos empty) still surfaces the error normally. + */ + const workspaceDetectionRef = useRef<{ resolved: boolean; isWorkspace: boolean }>({ resolved: false, isWorkspace: false }); + // Tracks whether the most recent fetch suppressed a root-race error, and a state tick that flips + // when detection resolves — together they let a genuinely-broken NON-workspace project re-surface + // the error (a single re-fetch) after detection settles, without adding a redundant fetch to the + // common non-workspace-OK path (where the first fetch already succeeded). + const suppressedRootRaceRef = useRef(false); + const [detectionResolved, setDetectionResolved] = useState(false); // ── Changes state const [fileChanges, setFileChanges] = useState([]); @@ -322,6 +340,7 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj if (!isOpen) return; setLoading(true); setSectionError(null); + suppressedRootRaceRef.current = false; try { switch (activeSection) { case "status": { @@ -375,8 +394,30 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj } } } catch (err) { - setSectionError(getErrorMessage(err) || t("git.failedToFetchData", "Failed to fetch git data")); - addToast(getErrorMessage(err) || t("git.failedToFetchData", "Failed to fetch git data"), "error"); + const message = getErrorMessage(err) || t("git.failedToFetchData", "Failed to fetch git data"); + /* + FNXC:Workspace 2026-06-25-00:10: + Suppress the benign workspace-root race: on open, the first fetch fires before selectedRepo + resolves (no repoPath → the non-git browse root), which fails "Not a git repository". A workspace + re-fetches against a real sub-repo a tick later. Only swallow this when there is NO repoPath AND + detection is still pending OR has confirmed a workspace; a resolved non-workspace project surfaces + a genuine "Not a git repository" normally. + */ + const detection = workspaceDetectionRef.current; + const isWorkspaceRootRace = + gitRepoPath === undefined && + /not a git repository/i.test(message) && + (!detection.resolved || detection.isWorkspace); + if (isWorkspaceRootRace) { + // Benign: defer reporting. A workspace re-fetches against its sub-repo (selectedRepo change); + // a non-workspace re-fetches once via the detection-resolved effect below, surfacing any real + // error then. + suppressedRootRaceRef.current = true; + setSectionError(null); + } else { + setSectionError(message); + addToast(message, "error"); + } } finally { setLoading(false); } @@ -909,20 +950,36 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj selectedRepo in the effect deps, preserving the projectId-keyed intent. */ useEffect(() => { + // Reset detection on project switch so a stale verdict can't suppress a real error. + workspaceDetectionRef.current = { resolved: false, isWorkspace: false }; + setDetectionResolved(false); fetchWorkspaceRepos(projectId) .then((result) => { const repos = result.repos; + workspaceDetectionRef.current = { resolved: true, isWorkspace: repos.length > 0 }; setWorkspaceRepos(repos); setSelectedRepo((current) => current && repos.includes(current) ? current : (repos[0] ?? null), ); }) .catch(() => { + workspaceDetectionRef.current = { resolved: true, isWorkspace: false }; setWorkspaceRepos([]); setSelectedRepo(null); - }); + }) + .finally(() => setDetectionResolved(true)); }, [projectId]); // keyed on projectId; selectedRepo is revalidated via the functional updater + // FNXC:Workspace 2026-06-25-00:10: once detection settles, re-surface a suppressed root-race error + // for a NON-workspace project (a genuinely broken/non-git repo). A workspace already re-fetches via + // the selectedRepo change, so we skip it here to avoid a redundant second fetch. + useEffect(() => { + if (isOpen && detectionResolved && suppressedRootRaceRef.current && !workspaceDetectionRef.current.isWorkspace) { + suppressedRootRaceRef.current = false; + void fetchSectionData(); + } + }, [isOpen, detectionResolved, fetchSectionData]); + const handleSyncIntegrationTip = useCallback(async () => { if (!status?.integrationBranch || status.isOnIntegrationBranch === false) return; const worktreePath = rootDir; diff --git a/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx b/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx index 9028f8049b..32f2859119 100644 --- a/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx @@ -70,6 +70,10 @@ vi.mock("../../api", async () => { fetchAheadCommits: vi.fn(), fetchRemoteCommits: vi.fn(), fetchBranchCommits: vi.fn(), + // FNXC:Test 2026-06-25-00:10: GitManagerModal detects workspace sub-repos on mount via + // fetchWorkspaceRepos; the mock was never added when that call landed, breaking the whole suite + // at import. Default to a non-workspace project ({ repos: [] }) so the root git path is exercised. + fetchWorkspaceRepos: vi.fn().mockResolvedValue({ repos: [] }), }; }); @@ -112,6 +116,7 @@ import { fetchAheadCommits, fetchRemoteCommits, fetchBranchCommits, + fetchWorkspaceRepos, } from "../../api"; import { subscribeSse } from "../../sse-bus"; @@ -284,6 +289,40 @@ describe("GitManagerModal", () => { (fetchRemoteCommits as any).mockResolvedValue([]); }); + // ── Workspace root-race toast suppression ─────────────────── + // FNXC:Workspace 2026-06-25-00:10: a workspace project's root is non-git, so the first git status + // (no repoPath yet) fails "Not a git repository". That benign race must NOT toast; a real + // non-workspace project with the same error must. + + it("does NOT toast 'Not a git repository' for a workspace project's initial root-race fetch", async () => { + (fetchWorkspaceRepos as any).mockResolvedValue({ repos: ["openvide", "swarmclaw"] }); + // Root (no repoPath) → not a git repo; a real sub-repo → resolves. + (fetchGitStatus as any).mockImplementation((_pid: unknown, _opts: unknown, repoPath?: string) => + repoPath + ? Promise.resolve({ branch: "main", commit: "abc1234", isDirty: false, ahead: 0, behind: 0 }) + : Promise.reject(new Error("Not a git repository")), + ); + + render(); + + // Wait until the re-fetch against the selected sub-repo has happened. + await waitFor(() => { + expect((fetchGitStatus as any).mock.calls.some((c: unknown[]) => c[2] === "openvide")).toBe(true); + }); + expect(mockAddToast).not.toHaveBeenCalledWith(expect.stringMatching(/not a git repository/i), "error"); + }); + + it("DOES toast 'Not a git repository' for a real non-workspace project", async () => { + (fetchWorkspaceRepos as any).mockResolvedValue({ repos: [] }); + (fetchGitStatus as any).mockRejectedValue(new Error("Not a git repository")); + + render(); + + await waitFor(() => { + expect(mockAddToast).toHaveBeenCalledWith(expect.stringMatching(/not a git repository/i), "error"); + }); + }); + // ── Basic Rendering ───────────────────────────────────────── it("renders nothing when not open", () => { From 98fc453f45f6140847404d0a2d4fc245fb6be4bf Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 24 Jun 2026 18:29:58 -0700 Subject: [PATCH 2/3] feat(workspace): show per-sub-repo changes in the task Changes tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Changes / Files-changed tab showed nothing for multi-repo workspace tasks: the task-diff backend is single-repo throughout, and a workspace task has null task.worktree/task.branch (its per-repo state lives in workspaceWorktrees), so every path fell back to git-diff against the non-git workspace root → empty. Backend (register-session-diff-routes): extract the single-repo per-worktree detailed-diff into one shared helper (computeWorktreeDetailedFiles) and add a workspace branch to BOTH /tasks/:id/diff and /tasks/:id/file-diffs that runs before the single-repo logic: iterate sorted workspaceWorktrees, compute each sub-repo's diff in its own live worktree against that repo's baseCommitSha (done tasks fall back to the per-repo landed range in the sub-repo root), and aggregate with `${repoRel}/`-prefixed paths. Single-repo behavior is byte-for-byte preserved (renamed→modified fold retained; 58 existing diff-route tests pass). Frontend: TaskChangesTab takes an isWorkspace prop and no longer shows the single-repo "No worktree available" empty state for workspace tasks; TaskDetailModal passes isWorkspace={isWorkspaceTask(workingTask)}. Tests: backend aggregation (repo-prefixed paths + stats) and frontend rendering of workspace changes instead of the empty state. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/components/TaskChangesTab.tsx | 15 +- .../app/components/TaskDetailModal.tsx | 2 +- .../__tests__/TaskChangesTab.test.tsx | 38 ++ .../__tests__/routes-diff-workspace.test.ts | 118 ++++++ .../routes/register-session-diff-routes.ts | 397 +++++++++++------- 5 files changed, 412 insertions(+), 158 deletions(-) create mode 100644 packages/dashboard/src/__tests__/routes-diff-workspace.test.ts diff --git a/packages/dashboard/app/components/TaskChangesTab.tsx b/packages/dashboard/app/components/TaskChangesTab.tsx index 5b220116eb..1ac2815139 100644 --- a/packages/dashboard/app/components/TaskChangesTab.tsx +++ b/packages/dashboard/app/components/TaskChangesTab.tsx @@ -19,6 +19,14 @@ interface TaskChangesTabProps { projectId?: string; column?: ColumnId; mergeDetails?: MergeDetails; + /** + * True for a workspace (multi-repo) task. Such a task has no singular + * `worktree`/`branch` — its changes live in per-sub-repo worktrees, which the + * backend `/tasks/:id/diff` now aggregates (repo-prefixed paths). Used to skip + * the single-repo "No worktree available" empty state, which would otherwise + * fire on every workspace task because `worktree` is undefined. + */ + isWorkspace?: boolean; /** * Files modified by the task during execution, captured from the worktree. * Used as a last-resort fallback when the live worktree diff is empty or the @@ -127,7 +135,7 @@ interface NormalizedFile { * modifiedFiles view instead of showing a hard error. This preserves the prior * graceful behavior while allowing FN-4563/FN-4576 lineage-backed parity. */ -export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetails, modifiedFiles }: TaskChangesTabProps) { +export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetails, modifiedFiles, isWorkspace }: TaskChangesTabProps) { const { t } = useTranslation("app"); const [files, setFiles] = useState([]); const [stats, setStats] = useState<{ filesChanged: number; additions: number; deletions: number }>({ filesChanged: 0, additions: 0, deletions: 0 }); @@ -309,7 +317,10 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai } // Non-done task without a worktree → only show fallback state when branch-fallback diff is empty. - if (!isDone && !worktree && files.length === 0) { + // A workspace task legitimately has no singular `worktree` (its changes come from the per-sub-repo + // aggregation), so it must NOT hit this "No worktree available" branch — fall through to the + // standard empty/populated rendering below. + if (!isDone && !worktree && !isWorkspace && files.length === 0) { if (modifiedFiles && modifiedFiles.length > 0) { return renderModifiedFilesFallback(modifiedFiles, false, undefined, "execution", t); } diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index 606724c8c9..66efd7109b 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -3399,7 +3399,7 @@ export function TaskDetailContent({ )} ) : activeTab === "changes" ? ( - + ) : activeTab === "review" ? ( { }); }); +// FNXC:Workspace 2026-06-25-00:40: a workspace task has no singular `worktree` — its changes come +// from the backend's per-sub-repo aggregation (repo-prefixed paths). It must render those instead of +// the single-repo "No worktree available" empty state. +describe("TaskChangesTab — workspace tasks", () => { + it("renders aggregated repo-prefixed files for a workspace task (no singular worktree)", async () => { + mockFetchTaskDiff.mockResolvedValue({ + files: [ + { path: "openvide/src/a.ts", status: "added", additions: 2, deletions: 0, patch: "@@ -0,0 +1,2 @@\n+a\n+aa" }, + { path: "swarmclaw/lib/b.ts", status: "modified", additions: 1, deletions: 1, patch: "@@ -1 +1 @@\n+b\n-old" }, + ], + stats: { filesChanged: 2, additions: 3, deletions: 1 }, + }); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByText("openvide/src/a.ts")).toBeTruthy(); + }); + expect(screen.getByText("swarmclaw/lib/b.ts")).toBeTruthy(); + expect(screen.queryByText("No worktree available for this task.")).toBeNull(); + }); + + it("does NOT show 'No worktree available' for an empty workspace task", async () => { + mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } }); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByText("No files modified.")).toBeTruthy(); + }); + expect(screen.queryByText("No worktree available for this task.")).toBeNull(); + }); +}); + describe("TaskChangesTab — commit-backed (done tasks)", () => { it("loads diff from fetchTaskDiff for done task with commitSha", async () => { mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF); diff --git a/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts b/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts new file mode 100644 index 0000000000..d1fa0d2c47 --- /dev/null +++ b/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts @@ -0,0 +1,118 @@ +/* +FNXC:Workspace 2026-06-25-00:40: +A workspace (multi-repo) task has no singular `worktree`/`branch` — its changes live in per-sub-repo +worktrees recorded in `task.workspaceWorktrees`. `/tasks/:id/diff` and `/tasks/:id/file-diffs` must +aggregate each sub-repo's diff (computed in that sub-repo's worktree) and prefix every file path with +the sub-repo key, instead of diffing the non-git workspace root (which returns empty). + +We mock runGitCommand (keyed by cwd so each sub-repo returns its own files) and node:fs/promises +access (so the sub-repo worktrees "exist") — no real/slow git. +*/ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { EventEmitter } from "node:events"; +import type { Task } from "@fusion/core"; + +const runGitCommandMock = vi.fn<(...args: any[]) => Promise>(); + +vi.mock("../routes/resolve-diff-base.js", () => ({ + // Per-repo base: the route passes the sub-repo's captured baseCommitSha through. + resolveDiffBase: vi.fn(async (task: any) => task.baseCommitSha), + runGitCommand: (...args: any[]) => runGitCommandMock(...args), +})); + +vi.mock("node:fs/promises", async () => { + const actual = await vi.importActual("node:fs/promises"); + return { ...actual, access: vi.fn(async () => undefined) }; +}); + +import { createServer } from "../server.js"; + +class MockStore extends EventEmitter { + private tasks = new Map(); + getRootDir(): string { return "/ws-root"; } + getFusionDir(): string { return "/ws-root/.fusion"; } + getDatabase() { + return { exec: vi.fn(), prepare: vi.fn().mockReturnValue({ run: vi.fn().mockReturnValue({ changes: 0 }), get: vi.fn(), all: vi.fn().mockReturnValue([]) }) }; + } + getMissionStore() { + return { + listMissions: vi.fn().mockResolvedValue([]), createMission: vi.fn(), getMission: vi.fn(), updateMission: vi.fn(), deleteMission: vi.fn(), + listTemplates: vi.fn().mockResolvedValue([]), createTemplate: vi.fn(), getTemplate: vi.fn(), updateTemplate: vi.fn(), deleteTemplate: vi.fn(), instantiateMission: vi.fn(), + }; + } + async listTasks(): Promise { return Array.from(this.tasks.values()); } + getTask(id: string): Task | undefined { return this.tasks.get(id); } + addTask(task: Task): void { this.tasks.set(task.id, task); } + async getTaskCommitAssociationsByLineageId(): Promise<[]> { return []; } +} + +function workspaceTask(): Task { + return { + id: "MULT-002", title: "ws task", description: "", column: "in-review", + dependencies: [], steps: [], currentStep: 0, log: [], + createdAt: "2026-06-24T00:00:00.000Z", updatedAt: "2026-06-24T00:00:00.000Z", + worktree: undefined, branch: undefined, + workspaceWorktrees: { + // Intentionally non-alphabetical insertion to prove sorted, deterministic output. + swarmclaw: { worktreePath: "/wt/swarmclaw", branch: "fusion/mult-002", baseCommitSha: "baseS" }, + openvide: { worktreePath: "/wt/openvide", branch: "fusion/mult-002", baseCommitSha: "baseO" }, + }, + } as Task; +} + +// Per-cwd git responses. Anything not listed throws — restrictActiveCommittedFilesToOwnTask's +// attribution probes hit that and are swallowed (display-only), preserving the broad diff. +const RESPONSES: Record> = { + "/wt/openvide": { + "diff --name-status -M baseO..HEAD": "A\tsrc/a.ts", + "diff --cached --name-status -M": "", + "diff --name-status -M": "", + "diff baseO -- src/a.ts": "+a\n+aa\n", + }, + "/wt/swarmclaw": { + "diff --name-status -M baseS..HEAD": "M\tlib/b.ts", + "diff --cached --name-status -M": "", + "diff --name-status -M": "", + "diff baseS -- lib/b.ts": "+b\n-old\n", + }, +}; + +describe("workspace task diff aggregation", () => { + beforeEach(() => { + vi.clearAllMocks(); + runGitCommandMock.mockImplementation(async (gitArgs: string[], cwd?: string) => { + const repo = (cwd && RESPONSES[cwd]) || {}; + const key = gitArgs.join(" "); + if (key in repo) return repo[key] ?? ""; + throw new Error(`Unexpected git command [${cwd}]: ${key}`); + }); + }); + afterEach(() => vi.restoreAllMocks()); + + it("/diff aggregates per-sub-repo files with repo-prefixed paths and summed stats", async () => { + const store = new MockStore(); + store.addTask(workspaceTask()); + const app = createServer(store as any); + + const { get } = await import("../test-request.js"); + const res = await get(app, "/api/tasks/MULT-002/diff"); + + expect(res.status).toBe(200); + expect(res.body.files.map((f: any) => f.path)).toEqual(["openvide/src/a.ts", "swarmclaw/lib/b.ts"]); + expect(res.body.files.find((f: any) => f.path === "openvide/src/a.ts").status).toBe("added"); + expect(res.body.stats).toEqual({ filesChanged: 2, additions: 3, deletions: 1 }); + }); + + it("/file-diffs returns repo-prefixed per-file patches", async () => { + const store = new MockStore(); + store.addTask(workspaceTask()); + const app = createServer(store as any); + + const { get } = await import("../test-request.js"); + const res = await get(app, "/api/tasks/MULT-002/file-diffs"); + + expect(res.status).toBe(200); + expect(res.body.map((f: any) => f.path)).toEqual(["openvide/src/a.ts", "swarmclaw/lib/b.ts"]); + expect(res.body.find((f: any) => f.path === "swarmclaw/lib/b.ts").diff).toContain("-old"); + }); +}); diff --git a/packages/dashboard/src/routes/register-session-diff-routes.ts b/packages/dashboard/src/routes/register-session-diff-routes.ts index b41ec8d5ec..3b848688ab 100644 --- a/packages/dashboard/src/routes/register-session-diff-routes.ts +++ b/packages/dashboard/src/routes/register-session-diff-routes.ts @@ -1,6 +1,8 @@ import { access } from "node:fs/promises"; +import { join } from "node:path"; import type { Request, Router } from "express"; import type { RunAuditEvent, RunAuditEventFilter } from "@fusion/core"; +import { isWorkspaceTask } from "@fusion/core"; import { ApiError, notFound, rethrowAsApiError } from "../api-error.js"; import { resolveDiffBase, runGitCommand } from "./resolve-diff-base.js"; import { countPatchLines } from "./diff-counts.js"; @@ -372,6 +374,188 @@ async function collectDoneRangeFiles(range: string, rootDir: string): Promise { + const diffBase = await resolveDiffBase(taskLike, cwd, "HEAD", undefined, { enableDisplayRecovery: true }); + + const fileMap = new Map(); + + if (diffBase) { + try { + const committedOutput = (await runGitCommand(["diff", "--name-status", "-M", `${diffBase}..HEAD`], cwd, timeoutMs)).trim(); + for (const line of committedOutput.split("\n").filter(Boolean)) { + const parsed = parseNameStatusLine(line); + if (!parsed) continue; + fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); + } + } catch { + // committed diff failed + } + } + + await restrictActiveCommittedFilesToOwnTask(fileMap, { + taskId: taskLike.id, + diffBase, + worktreePath: cwd, + runGit: (args) => runGitCommand(args, cwd, timeoutMs), + }); + + try { + const stagedOutput = (await runGitCommand(["diff", "--cached", "--name-status", "-M"], cwd, timeoutMs)).trim(); + for (const line of stagedOutput.split("\n").filter(Boolean)) { + const parsed = parseNameStatusLine(line); + if (!parsed || fileMap.has(parsed.path)) continue; + fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); + } + } catch { + // staged diff failed + } + + try { + const workingTreeOutput = (await runGitCommand(["diff", "--name-status", "-M"], cwd, timeoutMs)).trim(); + for (const line of workingTreeOutput.split("\n").filter(Boolean)) { + const parsed = parseNameStatusLine(line); + if (!parsed || fileMap.has(parsed.path)) continue; + fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); + } + } catch { + // working tree diff failed + } + + const results: WorktreeDetailedFile[] = []; + for (const [filePath, { statusCode, oldPath }] of fileMap.entries()) { + if (!filePath) continue; + + let status: "added" | "modified" | "deleted" | "renamed" = "modified"; + if (statusCode.startsWith("A")) status = "added"; + else if (statusCode.startsWith("D")) status = "deleted"; + else if (statusCode.startsWith("R")) status = "renamed"; + + let patch = ""; + try { + patch = diffBase + ? await runGitCommand(["diff", diffBase, "--", filePath], cwd, timeoutMs) + : await runGitCommand(["diff", "HEAD", "--", filePath], cwd, timeoutMs); + } catch { + // ignore individual file errors + } + + const { additions, deletions } = countPatchLines(patch); + results.push(oldPath ? { path: filePath, status, additions, deletions, patch, oldPath } : { path: filePath, status, additions, deletions, patch }); + } + + return results; +} + +/** + * Aggregate a workspace task's changed files across ALL acquired sub-repo + * worktrees. A workspace task has no singular `task.worktree`/`task.branch` + * (those are null by design); its per-repo state lives in + * `task.workspaceWorktrees`. Each sub-repo's diff is computed in its own live + * worktree (in-progress/in-review) or, when that worktree is gone (done tasks), + * from its landed range in the sub-repo root. Every file path is prefixed with + * the sub-repo key (e.g. `openvide/src/foo.ts`) so the Changes tab shows which + * sub-repo each file belongs to. A missing/unreadable sub-repo is skipped + * best-effort rather than failing the whole response. + */ +async function computeWorkspaceTaskFiles( + task: { + id: string; + baseBranch?: string; + workspaceWorktrees?: Record; + }, + rootDir: string, + timeoutMs: number, +): Promise { + const worktrees = task.workspaceWorktrees ?? {}; + const all: WorktreeDetailedFile[] = []; + + // Deterministic, repo-sorted order so the aggregated list is stable. + for (const repoRel of Object.keys(worktrees).sort()) { + const entry = worktrees[repoRel]; + if (!entry) continue; + + let repoFiles: WorktreeDetailedFile[] = []; + + // Prefer the live sub-repo worktree (in-progress / in-review). + let worktreeUsable = false; + if (entry.worktreePath) { + try { + await access(entry.worktreePath); + worktreeUsable = true; + } catch { + worktreeUsable = false; + } + } + if (worktreeUsable) { + try { + repoFiles = await computeWorktreeDetailedFiles( + // Per-repo base: use the sub-repo's own captured fork point, with the + // workspace task's baseBranch stripped so resolveDiffBase uses the + // per-repo baseCommitSha rather than a shared workspace branch. + { id: task.id, baseBranch: undefined, baseCommitSha: entry.baseCommitSha }, + entry.worktreePath, + timeoutMs, + ); + } catch { + repoFiles = []; + } + } + + // Fallback: landed range in the sub-repo root (a done task whose per-repo + // worktree was already cleaned up). Each sub-repo lands independently with + // its own baseCommitSha → landedSha. + if (repoFiles.length === 0 && entry.baseCommitSha && entry.landedSha) { + const repoRootDir = join(rootDir, repoRel); + try { + const rangeFiles = await collectDoneRangeFiles(`${entry.baseCommitSha}..${entry.landedSha}`, repoRootDir); + repoFiles = rangeFiles.map((file) => ({ + path: file.path, + status: file.status, + additions: file.additions, + deletions: file.deletions, + patch: file.patch, + })); + } catch { + repoFiles = []; + } + } + + for (const file of repoFiles) { + all.push({ + ...file, + path: `${repoRel}/${file.path}`, + oldPath: file.oldPath ? `${repoRel}/${file.oldPath}` : undefined, + }); + } + } + + return all; +} + function extractCommitShaCandidate(event: { target?: unknown; metadata?: unknown; payload?: unknown; newValue?: unknown }): string | undefined { if (typeof event.target === "string" && event.target.trim()) { return event.target.trim(); @@ -730,6 +914,30 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute return; } + // Workspace tasks have no singular worktree/branch; their changes live in + // per-sub-repo worktrees. Aggregate across them (repo-prefixed paths) and + // short-circuit before the single-repo logic, which would diff the non-git + // workspace root and return empty. + if (isWorkspaceTask(task)) { + const workspaceFiles = await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), 10000); + const files = workspaceFiles.map((file) => ({ + path: file.path, + status: file.status === "renamed" ? "modified" : file.status, + additions: file.additions, + deletions: file.deletions, + patch: file.patch, + })); + res.json({ + files, + stats: { + filesChanged: files.length, + additions: files.reduce((sum, file) => sum + file.additions, 0), + deletions: files.reduce((sum, file) => sum + file.deletions, 0), + }, + }); + return; + } + if (task.column === "done") { const mergeShaForBaseBoundary = await resolveDoneTaskMergeSha(task, scopedStore, { includeBaseCommitSha: true }); const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore); @@ -906,85 +1114,18 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute } const cwd = resolvedWorktree; - const diffBase = await resolveDiffBase(task, cwd, "HEAD", undefined, { enableDisplayRecovery: true }); - - // Only count files actually changed by the task: committed (base..HEAD) - // + staged + unstaged. Untracked files are intentionally excluded — at - // review time they're almost always build artifacts/cache/logs that - // weren't in .gitignore, not real task changes. - const fileMap = new Map(); - - if (diffBase) { - try { - const committedOutput = (await runGitCommand(["diff", "--name-status", "-M", `${diffBase}..HEAD`], cwd, 10000)).trim(); - for (const line of committedOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed) continue; - fileMap.set(parsed.path, parsed.statusCode); - } - } catch { - // committed diff failed - } - } - - await restrictActiveCommittedFilesToOwnTask(fileMap, { - taskId: task.id, - diffBase, - worktreePath: cwd, - runGit: (args) => runGitCommand(args, cwd, 10000), - }); - - try { - const stagedOutput = (await runGitCommand(["diff", "--cached", "--name-status", "-M"], cwd, 10000)).trim(); - for (const line of stagedOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed || fileMap.has(parsed.path)) continue; - fileMap.set(parsed.path, parsed.statusCode); - } - } catch { - // staged diff failed - } - - try { - const workingTreeOutput = (await runGitCommand(["diff", "--name-status", "-M"], cwd, 10000)).trim(); - for (const line of workingTreeOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed || fileMap.has(parsed.path)) continue; - fileMap.set(parsed.path, parsed.statusCode); - } - } catch { - // working tree diff failed - } - - const files: Array<{ - path: string; - status: "added" | "modified" | "deleted"; - additions: number; - deletions: number; - patch: string; - }> = []; - - for (const [filePath, statusCode] of fileMap) { - if (!filePath) continue; - - let status: "added" | "modified" | "deleted"; - if (statusCode.startsWith("A")) status = "added"; - else if (statusCode.startsWith("D")) status = "deleted"; - else status = "modified"; - - let patch = ""; - try { - patch = diffBase - ? await runGitCommand(["diff", diffBase, "--", filePath], cwd, 10000) - : await runGitCommand(["diff", "HEAD", "--", filePath], cwd, 10000); - } catch { - // ignore individual file errors - } - - const { additions, deletions } = countPatchLines(patch); - - files.push({ path: filePath, status, additions, deletions, patch }); - } + // Single-repo detailed diff (committed base..HEAD + staged + unstaged), + // shared with the per-sub-repo workspace aggregation. Renames fold to + // "modified" here (the /diff shape has no "renamed" status), matching the + // previous inline behaviour. + const detailed = await computeWorktreeDetailedFiles(task, cwd, 10000); + const files = detailed.map((file) => ({ + path: file.path, + status: file.status === "renamed" ? ("modified" as const) : file.status, + additions: file.additions, + deletions: file.deletions, + patch: file.patch, + })); const stats = { filesChanged: files.length, @@ -1010,6 +1151,18 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute return; } + // Workspace tasks aggregate per-sub-repo patches (repo-prefixed paths); + // short-circuit before the single-repo logic that diffs the non-git root. + if (isWorkspaceTask(task)) { + const workspaceFiles = (await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), 5000)) + .filter((file) => file.patch) + .map((file) => (file.oldPath + ? { path: file.path, status: file.status, diff: file.patch, oldPath: file.oldPath } + : { path: file.path, status: file.status, diff: file.patch })); + res.json(workspaceFiles); + return; + } + if (task.column === "done") { const mergeShaForBaseBoundary = await resolveDoneTaskMergeSha(task, scopedStore, { includeBaseCommitSha: true }); const resolvedMergeSha = await resolveDoneTaskMergeSha(task, scopedStore); @@ -1153,83 +1306,17 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute } const cwd = worktree; - const diffBase = await resolveDiffBase(task, cwd, "HEAD", undefined, { enableDisplayRecovery: true }); - // Only files actually changed by the task: committed + staged + unstaged. - // Untracked files (build artifacts, cache, logs) are intentionally - // excluded so the count matches "ACTUAL files changed by the task". - const fileMap = new Map(); - - if (diffBase) { - try { - const committedOutput = (await runGitCommand(["diff", "--name-status", "-M", `${diffBase}..HEAD`], cwd, 5000)).trim(); - for (const line of committedOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed) continue; - fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); - } - } catch { - // continue with working-tree-only changes - } - } - - await restrictActiveCommittedFilesToOwnTask(fileMap, { - taskId: task.id, - diffBase, - worktreePath: cwd, - runGit: (args) => runGitCommand(args, cwd, 5000), - }); - - try { - const stagedOutput = (await runGitCommand(["diff", "--cached", "--name-status", "-M"], cwd, 5000)).trim(); - for (const line of stagedOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed || fileMap.has(parsed.path)) continue; - fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); - } - } catch { - // ignore staged diff failures - } - - try { - const workingTreeOutput = (await runGitCommand(["diff", "--name-status", "-M"], cwd, 5000)).trim(); - for (const line of workingTreeOutput.split("\n").filter(Boolean)) { - const parsed = parseNameStatusLine(line); - if (!parsed || fileMap.has(parsed.path)) continue; - fileMap.set(parsed.path, { statusCode: parsed.statusCode, oldPath: parsed.oldPath }); - } - } catch { - // ignore unstaged diff failures - } - - const files = []; - - for (const [filePath, { statusCode, oldPath }] of fileMap.entries()) { - let status: "added" | "modified" | "deleted" | "renamed" = "modified"; - - if (statusCode.startsWith("A")) { - status = "added"; - } else if (statusCode.startsWith("D")) { - status = "deleted"; - } else if (statusCode.startsWith("R")) { - status = "renamed"; - } - - let diff = ""; - try { - diff = diffBase - ? await runGitCommand(["diff", diffBase, "--", filePath], cwd, 5000) - : await runGitCommand(["diff", "HEAD", "--", filePath], cwd, 5000); - } catch { - diff = ""; - } - - if (!diff) { - continue; - } - - files.push(oldPath ? { path: filePath, status, diff, oldPath } : { path: filePath, status, diff }); - } + // Single-repo per-file patches (committed base..HEAD + staged + unstaged), + // shared with the per-sub-repo workspace aggregation. Files with an empty + // patch (e.g. pure renames with no content change) are dropped, matching + // the previous inline behaviour. + const detailed = await computeWorktreeDetailedFiles(task, cwd, 5000); + const files = detailed + .filter((file) => file.patch) + .map((file) => (file.oldPath + ? { path: file.path, status: file.status, diff: file.patch, oldPath: file.oldPath } + : { path: file.path, status: file.status, diff: file.patch })); fileDiffsCache.set(task.id, { files, From e6e096645aa2672141346ff88a17f97537b1ccbf Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 24 Jun 2026 19:05:11 -0700 Subject: [PATCH 3/3] fix(workspace): address code-review findings on the workspace diff + Git Manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the multi-agent /ce-code-review of PR #1749 (no P0/P1 correctness bugs; these are perf, race-hardening, and convention fixes): - P1 (perf/reliability): the workspace diff ran git subprocesses serially per sub-repo AND per file — an N×M explosion with no aggregate cap. Add a bounded mapWithConcurrency helper (order-preserving) and parallelize the per-file patch loop (cap 8) and the per-sub-repo loop (cap 4). Deleted files still fetch their patch (skipping it would drop deletes from /file-diffs and zero /diff stats). - P2 (frontend race): GitManagerModal's workspace-detection could be clobbered by a previous project's in-flight fetch on a rapid projectId switch / close-reopen. Add a detectionGenerationRef guard — only the latest detection run may mutate state; the effect cleanup bumps the generation to abandon superseded runs. - P2 (DRY): reuse the existing parseStatusCode instead of re-inlining the status-code mapping. - P2 (convention): FNXC-tag the new functions/branches per CLAUDE.md. - P3: extract DIFF_TIMEOUT_MS/FILE_DIFFS_TIMEOUT_MS constants, drop a dead catch-assignment, note the done-fallback oldPath limitation. Tests: order-preservation after parallelization; rapid-project-switch generation guard (a stale workspace verdict must not suppress a new project's real error). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../app/components/GitManagerModal.tsx | 24 +++- .../app/components/TaskChangesTab.tsx | 1 + .../__tests__/GitManagerModal.test.tsx | 24 ++++ .../__tests__/routes-diff-workspace.test.ts | 36 ++++++ .../routes/register-session-diff-routes.ts | 120 +++++++++++++----- 5 files changed, 168 insertions(+), 37 deletions(-) diff --git a/packages/dashboard/app/components/GitManagerModal.tsx b/packages/dashboard/app/components/GitManagerModal.tsx index 3be5667be1..1771c5a851 100644 --- a/packages/dashboard/app/components/GitManagerModal.tsx +++ b/packages/dashboard/app/components/GitManagerModal.tsx @@ -285,6 +285,16 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj // common non-workspace-OK path (where the first fetch already succeeded). const suppressedRootRaceRef = useRef(false); const [detectionResolved, setDetectionResolved] = useState(false); + /* + FNXC:Workspace 2026-06-25-09:40 (detection generation guard): + A rapid projectId switch (or close→reopen) can leave a previous project's fetchWorkspaceRepos + promise in flight. When it resolves it must NOT overwrite the CURRENT project's detection verdict — + doing so could suppress a real error for the new project or mis-fire the re-surface effect. Each + detection run is stamped with a monotonically increasing generation; only the latest run is allowed + to mutate detection state, and the effect cleanup bumps the generation so a superseded/unmounted run + is abandoned. + */ + const detectionGenerationRef = useRef(0); // ── Changes state const [fileChanges, setFileChanges] = useState([]); @@ -950,11 +960,15 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj selectedRepo in the effect deps, preserving the projectId-keyed intent. */ useEffect(() => { - // Reset detection on project switch so a stale verdict can't suppress a real error. + // Reset detection on project switch so a stale verdict can't suppress a real error. The + // generation guard (see ref note above) makes a superseded in-flight resolution a no-op. + const gen = ++detectionGenerationRef.current; workspaceDetectionRef.current = { resolved: false, isWorkspace: false }; + suppressedRootRaceRef.current = false; setDetectionResolved(false); fetchWorkspaceRepos(projectId) .then((result) => { + if (gen !== detectionGenerationRef.current) return; const repos = result.repos; workspaceDetectionRef.current = { resolved: true, isWorkspace: repos.length > 0 }; setWorkspaceRepos(repos); @@ -963,11 +977,17 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj ); }) .catch(() => { + if (gen !== detectionGenerationRef.current) return; workspaceDetectionRef.current = { resolved: true, isWorkspace: false }; setWorkspaceRepos([]); setSelectedRepo(null); }) - .finally(() => setDetectionResolved(true)); + .finally(() => { + if (gen !== detectionGenerationRef.current) return; + setDetectionResolved(true); + }); + // Bump the generation on cleanup so an unmounted/superseded run's late resolution is abandoned. + return () => { detectionGenerationRef.current++; }; }, [projectId]); // keyed on projectId; selectedRepo is revalidated via the functional updater // FNXC:Workspace 2026-06-25-00:10: once detection settles, re-surface a suppressed root-race error diff --git a/packages/dashboard/app/components/TaskChangesTab.tsx b/packages/dashboard/app/components/TaskChangesTab.tsx index 1ac2815139..582d3d67fb 100644 --- a/packages/dashboard/app/components/TaskChangesTab.tsx +++ b/packages/dashboard/app/components/TaskChangesTab.tsx @@ -20,6 +20,7 @@ interface TaskChangesTabProps { column?: ColumnId; mergeDetails?: MergeDetails; /** + * FNXC:Workspace 2026-06-25-09:40: * True for a workspace (multi-repo) task. Such a task has no singular * `worktree`/`branch` — its changes live in per-sub-repo worktrees, which the * backend `/tasks/:id/diff` now aggregates (repo-prefixed paths). Used to skip diff --git a/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx b/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx index 32f2859119..e00773951a 100644 --- a/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/GitManagerModal.test.tsx @@ -323,6 +323,30 @@ describe("GitManagerModal", () => { }); }); + it("does not let a stale workspace project's late detection suppress a real error after a rapid project switch", async () => { + // FNXC:Workspace 2026-06-25-09:40 (generation guard): switch from workspace project A (whose + // fetchWorkspaceRepos resolves LATE) to broken non-workspace project B before A resolves. A's late + // "workspace" verdict must be abandoned (generation guard) so it can't suppress B's real error. + let resolveA: (v: { repos: string[] }) => void = () => {}; + const aPromise = new Promise<{ repos: string[] }>((r) => { resolveA = r; }); + (fetchWorkspaceRepos as any).mockImplementation((pid: string) => + pid === "projA" ? aPromise : Promise.resolve({ repos: [] })); + (fetchGitStatus as any).mockRejectedValue(new Error("Not a git repository")); + + const { rerender } = render( + , + ); + // Switch to B before A's detection resolves. + rerender(); + // A resolves late as a workspace — must be ignored for the now-current project B. + resolveA({ repos: ["openvide"] }); + + // B is a genuinely broken non-workspace repo → its error must still surface. + await waitFor(() => { + expect(mockAddToast).toHaveBeenCalledWith(expect.stringMatching(/not a git repository/i), "error"); + }); + }); + // ── Basic Rendering ───────────────────────────────────────── it("renders nothing when not open", () => { diff --git a/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts b/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts index d1fa0d2c47..15ac0b266b 100644 --- a/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts +++ b/packages/dashboard/src/__tests__/routes-diff-workspace.test.ts @@ -103,6 +103,42 @@ describe("workspace task diff aggregation", () => { expect(res.body.stats).toEqual({ filesChanged: 2, additions: 3, deletions: 1 }); }); + it("preserves deterministic repo-sorted order across the concurrent (parallelized) aggregation", async () => { + // FNXC:WorkspaceDiff 2026-06-25-09:40: sub-repos are now diffed concurrently; the output must + // still be sorted by repo key regardless of which sub-repo's git calls finish first. Three repos + // inserted out of order, with the first-sorted repo deliberately given the slowest git response. + const task = workspaceTask(); + (task as any).workspaceWorktrees = { + zulu: { worktreePath: "/wt/zulu", branch: "fusion/mult-002", baseCommitSha: "baseZ" }, + alpha: { worktreePath: "/wt/alpha", branch: "fusion/mult-002", baseCommitSha: "baseA" }, + mike: { worktreePath: "/wt/mike", branch: "fusion/mult-002", baseCommitSha: "baseM" }, + }; + const resp: Record> = { + "/wt/alpha": { "diff --name-status -M baseA..HEAD": "A\ta.ts", "diff --cached --name-status -M": "", "diff --name-status -M": "", "diff baseA -- a.ts": "+x\n" }, + "/wt/mike": { "diff --name-status -M baseM..HEAD": "A\tm.ts", "diff --cached --name-status -M": "", "diff --name-status -M": "", "diff baseM -- m.ts": "+y\n" }, + "/wt/zulu": { "diff --name-status -M baseZ..HEAD": "A\tz.ts", "diff --cached --name-status -M": "", "diff --name-status -M": "", "diff baseZ -- z.ts": "+w\n" }, + }; + runGitCommandMock.mockImplementation(async (gitArgs: string[], cwd?: string) => { + const key = gitArgs.join(" "); + const repo = (cwd && resp[cwd]) || {}; + if (key in repo) { + // Make the first-sorted repo (alpha) resolve LAST to prove order is by key, not completion. + if (cwd === "/wt/alpha") await new Promise((r) => setTimeout(r, 5)); + return repo[key] ?? ""; + } + throw new Error(`Unexpected git command [${cwd}]: ${key}`); + }); + + const store = new MockStore(); + store.addTask(task); + const app = createServer(store as any); + const { get } = await import("../test-request.js"); + const res = await get(app, "/api/tasks/MULT-002/diff"); + + expect(res.status).toBe(200); + expect(res.body.files.map((f: any) => f.path)).toEqual(["alpha/a.ts", "mike/m.ts", "zulu/z.ts"]); + }); + it("/file-diffs returns repo-prefixed per-file patches", async () => { const store = new MockStore(); store.addTask(workspaceTask()); diff --git a/packages/dashboard/src/routes/register-session-diff-routes.ts b/packages/dashboard/src/routes/register-session-diff-routes.ts index 3b848688ab..5ddc7785cc 100644 --- a/packages/dashboard/src/routes/register-session-diff-routes.ts +++ b/packages/dashboard/src/routes/register-session-diff-routes.ts @@ -383,6 +383,38 @@ interface WorktreeDetailedFile { oldPath?: string; } +/* +FNXC:WorkspaceDiff 2026-06-25-09:40: +Per-call git timeouts for the task-diff endpoints. /diff allows a longer budget than /file-diffs +because the former drives the primary Changes view; both are named so the difference is visible at a +glance and the literals are not duplicated across call sites. +*/ +const DIFF_TIMEOUT_MS = 10_000; +const FILE_DIFFS_TIMEOUT_MS = 5_000; + +/* +FNXC:WorkspaceDiff 2026-06-25-09:40: +Bounded-concurrency mapper. A workspace task fans the diff out across N sub-repos × M files; running +those git subprocesses strictly serially makes the Changes tab block for a long time on large +multi-repo tasks (each per-file `git diff` is an independent subprocess). Run them concurrently with a +cap so we get parallel wall-clock without spawning an unbounded herd of git processes. Output order is +preserved (results indexed by input position) so the aggregated diff stays deterministic. +*/ +async function mapWithConcurrency(items: T[], limit: number, fn: (item: T, index: number) => Promise): Promise { + const results = new Array(items.length); + let cursor = 0; + const workerCount = Math.max(1, Math.min(limit, items.length)); + const workers = Array.from({ length: workerCount }, async () => { + for (;;) { + const index = cursor++; + if (index >= items.length) return; + results[index] = await fn(items[index]!, index); + } + }); + await Promise.all(workers); + return results; +} + /** * Build the per-file detailed diff for a SINGLE worktree: committed * (diffBase..HEAD) + staged + unstaged, with the committed set scoped to the @@ -445,14 +477,18 @@ async function computeWorktreeDetailedFiles( // working tree diff failed } - const results: WorktreeDetailedFile[] = []; - for (const [filePath, { statusCode, oldPath }] of fileMap.entries()) { - if (!filePath) continue; - - let status: "added" | "modified" | "deleted" | "renamed" = "modified"; - if (statusCode.startsWith("A")) status = "added"; - else if (statusCode.startsWith("D")) status = "deleted"; - else if (statusCode.startsWith("R")) status = "renamed"; + /* + FNXC:WorkspaceDiff 2026-06-25-09:40: + The per-file `git diff` patch fetch is the dominant cost (one subprocess per changed file). Run it + with bounded concurrency instead of a serial await loop — independent files do not depend on each + other, so this collapses M serial git spawns to ~M/limit wall-clock. We deliberately do NOT skip the + patch for deleted files: /file-diffs filters out empty-patch entries and the patch supplies the + additions/deletions counts, so a delete needs its real patch to stay visible and counted. Status + uses the shared parseStatusCode helper (single source of truth for the A/D/R/M mapping). + */ + const entries = Array.from(fileMap.entries()).filter(([filePath]) => Boolean(filePath)); + const results = await mapWithConcurrency(entries, 8, async ([filePath, { statusCode, oldPath }]) => { + const status = parseStatusCode(statusCode); let patch = ""; try { @@ -464,8 +500,10 @@ async function computeWorktreeDetailedFiles( } const { additions, deletions } = countPatchLines(patch); - results.push(oldPath ? { path: filePath, status, additions, deletions, patch, oldPath } : { path: filePath, status, additions, deletions, patch }); - } + return oldPath + ? { path: filePath, status, additions, deletions, patch, oldPath } + : { path: filePath, status, additions, deletions, patch }; + }); return results; } @@ -491,23 +529,30 @@ async function computeWorkspaceTaskFiles( timeoutMs: number, ): Promise { const worktrees = task.workspaceWorktrees ?? {}; - const all: WorktreeDetailedFile[] = []; - // Deterministic, repo-sorted order so the aggregated list is stable. - for (const repoRel of Object.keys(worktrees).sort()) { + /* + FNXC:WorkspaceDiff 2026-06-25-09:40: + Resolve each sub-repo's diff CONCURRENTLY (bounded) rather than awaiting them one at a time: every + sub-repo's git work is independent, so a serial loop made the aggregate cost N×(per-repo) and could + block the response for a long time on a many-repo task. Keys are sorted first and mapped by position, + so the aggregated output stays in deterministic repo-sorted order regardless of completion order. + */ + const repoRels = Object.keys(worktrees).sort(); + const perRepo = await mapWithConcurrency(repoRels, 4, async (repoRel) => { const entry = worktrees[repoRel]; - if (!entry) continue; + if (!entry) return [] as WorktreeDetailedFile[]; let repoFiles: WorktreeDetailedFile[] = []; - // Prefer the live sub-repo worktree (in-progress / in-review). + // Prefer the live sub-repo worktree (in-progress / in-review). The access() + // probe is an optimistic fast-path skip; the try/catch below is the real guard. let worktreeUsable = false; if (entry.worktreePath) { try { await access(entry.worktreePath); worktreeUsable = true; } catch { - worktreeUsable = false; + // worktree gone → fall through to the landed-range fallback } } if (worktreeUsable) { @@ -528,6 +573,9 @@ async function computeWorkspaceTaskFiles( // Fallback: landed range in the sub-repo root (a done task whose per-repo // worktree was already cleaned up). Each sub-repo lands independently with // its own baseCommitSha → landedSha. + // FNXC:WorkspaceDiff 2026-06-25-09:40: collectDoneRangeFiles returns AggregatedDoneTaskFile, which + // carries no oldPath, so a renamed file's rename-SOURCE is unavailable on this done fallback (the + // file still shows under its new path). The live-worktree path above does preserve oldPath. if (repoFiles.length === 0 && entry.baseCommitSha && entry.landedSha) { const repoRootDir = join(rootDir, repoRel); try { @@ -544,16 +592,15 @@ async function computeWorkspaceTaskFiles( } } - for (const file of repoFiles) { - all.push({ - ...file, - path: `${repoRel}/${file.path}`, - oldPath: file.oldPath ? `${repoRel}/${file.oldPath}` : undefined, - }); - } - } + // Prefix every path with the sub-repo key so the Changes tab shows which repo each file is in. + return repoFiles.map((file) => ({ + ...file, + path: `${repoRel}/${file.path}`, + oldPath: file.oldPath ? `${repoRel}/${file.oldPath}` : undefined, + })); + }); - return all; + return perRepo.flat(); } function extractCommitShaCandidate(event: { target?: unknown; metadata?: unknown; payload?: unknown; newValue?: unknown }): string | undefined { @@ -914,12 +961,13 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute return; } - // Workspace tasks have no singular worktree/branch; their changes live in - // per-sub-repo worktrees. Aggregate across them (repo-prefixed paths) and - // short-circuit before the single-repo logic, which would diff the non-git - // workspace root and return empty. + // FNXC:WorkspaceDiff 2026-06-25-09:40: + // Workspace tasks have no singular worktree/branch; their changes live in per-sub-repo + // worktrees. Aggregate across them (repo-prefixed paths) and short-circuit BEFORE the single-repo + // logic, which would diff the non-git workspace root and return empty. renamed→modified is folded + // to match the /diff contract (which has no 'renamed' status; /file-diffs keeps it). if (isWorkspaceTask(task)) { - const workspaceFiles = await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), 10000); + const workspaceFiles = await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), DIFF_TIMEOUT_MS); const files = workspaceFiles.map((file) => ({ path: file.path, status: file.status === "renamed" ? "modified" : file.status, @@ -1118,7 +1166,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute // shared with the per-sub-repo workspace aggregation. Renames fold to // "modified" here (the /diff shape has no "renamed" status), matching the // previous inline behaviour. - const detailed = await computeWorktreeDetailedFiles(task, cwd, 10000); + const detailed = await computeWorktreeDetailedFiles(task, cwd, DIFF_TIMEOUT_MS); const files = detailed.map((file) => ({ path: file.path, status: file.status === "renamed" ? ("modified" as const) : file.status, @@ -1151,10 +1199,12 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute return; } - // Workspace tasks aggregate per-sub-repo patches (repo-prefixed paths); - // short-circuit before the single-repo logic that diffs the non-git root. + // FNXC:WorkspaceDiff 2026-06-25-09:40: + // Workspace tasks aggregate per-sub-repo patches (repo-prefixed paths); short-circuit before the + // single-repo logic that diffs the non-git root. Unlike /diff, /file-diffs preserves the + // 'renamed' status + oldPath. Empty-patch entries are dropped (parity with the single-repo path). if (isWorkspaceTask(task)) { - const workspaceFiles = (await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), 5000)) + const workspaceFiles = (await computeWorkspaceTaskFiles(task, scopedStore.getRootDir(), FILE_DIFFS_TIMEOUT_MS)) .filter((file) => file.patch) .map((file) => (file.oldPath ? { path: file.path, status: file.status, diff: file.patch, oldPath: file.oldPath } @@ -1311,7 +1361,7 @@ export function registerSessionDiffRoutes(router: Router, deps: SessionDiffRoute // shared with the per-sub-repo workspace aggregation. Files with an empty // patch (e.g. pure renames with no content change) are dropped, matching // the previous inline behaviour. - const detailed = await computeWorktreeDetailedFiles(task, cwd, 5000); + const detailed = await computeWorktreeDetailedFiles(task, cwd, FILE_DIFFS_TIMEOUT_MS); const files = detailed .filter((file) => file.patch) .map((file) => (file.oldPath