fix(workspace): suppress spurious "Not a git repository" toast in Git Manager
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) <noreply@anthropic.com>
This commit is contained in:
@@ -267,6 +267,24 @@ export function GitManagerModal({ isOpen, onClose, tasks: _tasks, addToast, proj
|
||||
const [workspaceRepos, setWorkspaceRepos] = useState<string[]>([]);
|
||||
const [selectedRepo, setSelectedRepo] = useState<string | null>(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<GitFileChange[]>([]);
|
||||
@@ -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;
|
||||
|
||||
@@ -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(<GitManagerModal isOpen={true} onClose={vi.fn()} tasks={mockTasks} addToast={mockAddToast} />);
|
||||
|
||||
// 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(<GitManagerModal isOpen={true} onClose={vi.fn()} tasks={mockTasks} addToast={mockAddToast} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockAddToast).toHaveBeenCalledWith(expect.stringMatching(/not a git repository/i), "error");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Basic Rendering ─────────────────────────────────────────
|
||||
|
||||
it("renders nothing when not open", () => {
|
||||
|
||||
Reference in New Issue
Block a user