From e4a01185797050c3ebfb8e70df98b2c10815d8ac Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 24 Jun 2026 18:09:39 -0700 Subject: [PATCH] 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", () => {