fix(workspace): address code-review findings on the workspace diff + Git Manager

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) <noreply@anthropic.com>

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<GitFileChange[]>([]);
@@ -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

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

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(
+      <GitManagerModal isOpen={true} onClose={vi.fn()} tasks={mockTasks} addToast={mockAddToast} projectId="projA" />,
+    );
+    // Switch to B before A's detection resolves.
+    rerender(<GitManagerModal isOpen={true} onClose={vi.fn()} tasks={mockTasks} addToast={mockAddToast} projectId="projB" />);
+    // 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", () => {

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<string, Record<string, string>> = {
+      "/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());

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<T, R>(items: T[], limit: number, fn: (item: T, index: number) => Promise<R>): Promise<R[]> {
+  const results = new Array<R>(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<WorktreeDetailedFile[]> {
   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