fix(web): aggregate changes count across all repos to stop flicker (#986)
Addresses #980 — multi-repo workspaces showed a flickering / stale Changes count because the source hook returned only the last-arriving repo's status. `useSessionChangesCount` now sums file counts across the per-repo `byEnvironmentRepo` map plus commits and is wired into the three call sites; single-repo behaviour is unchanged.
This commit is contained in:
@@ -10,7 +10,7 @@ import {
|
||||
} from "@kandev/ui/context-menu";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
|
||||
import { useSessionCommits } from "@/hooks/domains/session/use-session-commits";
|
||||
import { useSessionChangesCount } from "@/hooks/domains/session/use-session-changes-count";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { cn } from "@kandev/ui/lib/utils";
|
||||
import { useTabMaximizeOnDoubleClick } from "./use-tab-maximize";
|
||||
@@ -40,9 +40,7 @@ export function ChangesTab(props: IDockviewPanelHeaderProps) {
|
||||
|
||||
const activeSessionId = useAppStore((s) => s.tasks.activeSessionId);
|
||||
const gitStatus = useSessionGitStatus(activeSessionId);
|
||||
const { commits } = useSessionCommits(activeSessionId ?? null);
|
||||
const fileCount = gitStatus?.files ? Object.keys(gitStatus.files).length : 0;
|
||||
const totalCount = fileCount + commits.length;
|
||||
const totalCount = useSessionChangesCount(activeSessionId ?? null);
|
||||
|
||||
// gitStatus is undefined until the first WS git-status event arrives,
|
||||
// which marks the end of the initial data load for this session.
|
||||
|
||||
@@ -22,8 +22,7 @@ import { useFileEditors } from "@/hooks/use-file-editors";
|
||||
import { useLspFileOpener } from "@/hooks/use-lsp-file-opener";
|
||||
import { useEditorKeybinds } from "@/hooks/use-editor-keybinds";
|
||||
import { usePlanPanelAutoOpen } from "@/hooks/use-plan-panel-auto-open";
|
||||
import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
|
||||
import { useSessionCommits } from "@/hooks/domains/session/use-session-commits";
|
||||
import { useSessionChangesCount } from "@/hooks/domains/session/use-session-changes-count";
|
||||
import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id";
|
||||
import { useActiveTaskHasRepos } from "@/hooks/domains/kanban/use-active-task-has-repos";
|
||||
|
||||
@@ -313,10 +312,7 @@ function ChangesContent({ panelId }: { panelId: string }) {
|
||||
// Dynamic title with file count — use environment-stable sessionId so the
|
||||
// tab title doesn't re-fetch on same-environment session tab switches.
|
||||
const activeSessionId = useEnvironmentSessionId();
|
||||
const gitStatus = useSessionGitStatus(activeSessionId);
|
||||
const { commits } = useSessionCommits(activeSessionId);
|
||||
const fileCount = gitStatus?.files ? Object.keys(gitStatus.files).length : 0;
|
||||
const totalCount = fileCount + commits.length;
|
||||
const totalCount = useSessionChangesCount(activeSessionId);
|
||||
|
||||
// Repo-less tasks have no git changes ever — auto-close the panel so users
|
||||
// don't see a permanently empty Changes tab. Gate on a confirmed `false`:
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { cleanup, renderHook } from "@testing-library/react";
|
||||
|
||||
vi.mock("@/lib/ws/connection", () => ({
|
||||
getWebSocketClient: () => null,
|
||||
}));
|
||||
|
||||
let storeState: Record<string, unknown> = {};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: Record<string, unknown>) => unknown) => selector(storeState),
|
||||
}));
|
||||
|
||||
import { useSessionChangesCount } from "./use-session-changes-count";
|
||||
|
||||
type FileEntry = { path: string; status: "modified"; staged: boolean };
|
||||
type StatusEntry = {
|
||||
branch: string;
|
||||
files: Record<string, FileEntry>;
|
||||
timestamp: string;
|
||||
repository_name?: string;
|
||||
};
|
||||
|
||||
function setStore(opts: {
|
||||
envBySession?: Record<string, string>;
|
||||
byEnvironmentRepo?: Record<string, Record<string, StatusEntry>>;
|
||||
commitsByEnvironmentId?: Record<string, unknown[]>;
|
||||
}) {
|
||||
storeState = {
|
||||
environmentIdBySessionId: opts.envBySession ?? {},
|
||||
gitStatus: {
|
||||
byEnvironmentId: {} as Record<string, StatusEntry>,
|
||||
byEnvironmentRepo: opts.byEnvironmentRepo ?? {},
|
||||
},
|
||||
sessionCommits: {
|
||||
byEnvironmentId: opts.commitsByEnvironmentId ?? {},
|
||||
loading: {} as Record<string, boolean>,
|
||||
},
|
||||
connection: { status: "disconnected" },
|
||||
setSessionCommits: vi.fn(),
|
||||
setSessionCommitsLoading: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
function file(path: string): FileEntry {
|
||||
return { path, status: "modified", staged: false };
|
||||
}
|
||||
|
||||
function status(files: string[], repository_name?: string): StatusEntry {
|
||||
const map: Record<string, FileEntry> = {};
|
||||
for (const p of files) map[p] = file(p);
|
||||
return {
|
||||
branch: "feature",
|
||||
files: map,
|
||||
timestamp: "t",
|
||||
repository_name,
|
||||
};
|
||||
}
|
||||
|
||||
describe("useSessionChangesCount", () => {
|
||||
beforeEach(() => {
|
||||
setStore({});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it("returns 0 when the session has no gitStatus and no commits yet", () => {
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-new"));
|
||||
expect(result.current).toBe(0);
|
||||
});
|
||||
|
||||
it("returns 0 for null session id", () => {
|
||||
const { result } = renderHook(() => useSessionChangesCount(null));
|
||||
expect(result.current).toBe(0);
|
||||
});
|
||||
|
||||
it("counts files from a single-repo workspace via the empty repo key", () => {
|
||||
setStore({
|
||||
envBySession: { "sess-1": "env-1" },
|
||||
byEnvironmentRepo: { "env-1": { "": status(["a.ts", "b.ts"]) } },
|
||||
});
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-1"));
|
||||
expect(result.current).toBe(2);
|
||||
});
|
||||
|
||||
it("sums files across every repo in a multi-repo workspace", () => {
|
||||
// Reproduces the reported bug shape: useSessionGitStatus alone would
|
||||
// report only the last-arriving repo's files (1), masking changes in
|
||||
// sibling repos. The aggregated count must show the true total.
|
||||
setStore({
|
||||
envBySession: { "sess-1": "env-1" },
|
||||
byEnvironmentRepo: {
|
||||
"env-1": {
|
||||
frontend: status(["app.tsx", "page.tsx"], "frontend"),
|
||||
backend: status(["server.go"], "backend"),
|
||||
},
|
||||
},
|
||||
});
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-1"));
|
||||
expect(result.current).toBe(3);
|
||||
});
|
||||
|
||||
it("includes commits in the total count", () => {
|
||||
setStore({
|
||||
envBySession: { "sess-1": "env-1" },
|
||||
byEnvironmentRepo: { "env-1": { "": status(["a.ts"]) } },
|
||||
commitsByEnvironmentId: { "env-1": [{ commit_sha: "x" }, { commit_sha: "y" }] },
|
||||
});
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-1"));
|
||||
expect(result.current).toBe(3);
|
||||
});
|
||||
|
||||
it("falls back to sessionId when no environment mapping is registered yet", () => {
|
||||
setStore({
|
||||
byEnvironmentRepo: { "sess-pending": { "": status(["only.ts"]) } },
|
||||
});
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-pending"));
|
||||
expect(result.current).toBe(1);
|
||||
});
|
||||
|
||||
it("does not leak stale data from a different session's environment", () => {
|
||||
// The bug: a brand-new task whose env has no gitStatus yet must not pick
|
||||
// up another task's count just because their env IDs happen to share the
|
||||
// map. The selector keys strictly by envKey resolved from sessionId.
|
||||
setStore({
|
||||
envBySession: { "sess-old": "env-old", "sess-new": "env-new" },
|
||||
byEnvironmentRepo: { "env-old": { "": status(["leak.ts", "leak2.ts"]) } },
|
||||
commitsByEnvironmentId: { "env-old": [{ commit_sha: "old" }] },
|
||||
});
|
||||
const { result } = renderHook(() => useSessionChangesCount("sess-new"));
|
||||
expect(result.current).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,27 @@
|
||||
import { useMemo } from "react";
|
||||
import { useSessionGitStatusByRepo } from "./use-session-git-status";
|
||||
import { useSessionCommits } from "./use-session-commits";
|
||||
|
||||
/**
|
||||
* Total count of uncommitted file changes + commits for a session, summed
|
||||
* across every repo in multi-repo workspaces.
|
||||
*
|
||||
* `useSessionGitStatus` exposes only the single status that arrived last
|
||||
* (documented "last write wins" behaviour), so any badge or title sourced
|
||||
* directly from it loses changes from sibling repos as new updates land — the
|
||||
* count flickers as repos take turns overwriting each other, and a stale
|
||||
* value can persist after the *current* session's actual changes arrive on
|
||||
* a different repo's status. Read this hook instead whenever the UI needs a
|
||||
* single number representing the session's total changes.
|
||||
*/
|
||||
export function useSessionChangesCount(sessionId: string | null): number {
|
||||
const statusByRepo = useSessionGitStatusByRepo(sessionId);
|
||||
const { commits } = useSessionCommits(sessionId);
|
||||
return useMemo(() => {
|
||||
let fileCount = 0;
|
||||
for (const { status } of statusByRepo) {
|
||||
if (status?.files) fileCount += Object.keys(status.files).length;
|
||||
}
|
||||
return fileCount + commits.length;
|
||||
}, [statusByRepo, commits.length]);
|
||||
}
|
||||
@@ -2,8 +2,7 @@
|
||||
|
||||
import { useState, useCallback, useMemo } from "react";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
|
||||
import { useSessionCommits } from "@/hooks/domains/session/use-session-commits";
|
||||
import { useSessionChangesCount } from "@/hooks/domains/session/use-session-changes-count";
|
||||
import { getPlanLastSeen } from "@/lib/local-storage";
|
||||
import { executeApprove } from "@/lib/services/session-approve";
|
||||
import type { OpenFileTab } from "@/lib/types/backend";
|
||||
@@ -74,15 +73,10 @@ export function useSessionLayoutState(options: UseSessionLayoutStateOptions = {}
|
||||
const { openFileRequest, handleOpenFile, handleFileOpenHandled } = useOpenFileRequestState();
|
||||
|
||||
// --- Git status for badges ---
|
||||
const gitStatus = useSessionGitStatus(effectiveSessionId);
|
||||
const { commits } = useSessionCommits(effectiveSessionId);
|
||||
|
||||
const uncommittedCount = useMemo(() => {
|
||||
if (!gitStatus?.files) return 0;
|
||||
return Object.keys(gitStatus.files).length;
|
||||
}, [gitStatus]);
|
||||
|
||||
const totalChangesCount = uncommittedCount + commits.length;
|
||||
// `useSessionChangesCount` reads the per-repo statuses so the badge count
|
||||
// stays correct in multi-repo workspaces and doesn't flicker as sibling
|
||||
// repos overwrite the legacy single-status map.
|
||||
const totalChangesCount = useSessionChangesCount(effectiveSessionId);
|
||||
|
||||
// --- Mobile session state (computed before plan badge to use in badge logic) ---
|
||||
const activePanelBySessionId = useAppStore((state) => state.mobileSession.activePanelBySessionId);
|
||||
@@ -158,9 +152,6 @@ export function useSessionLayoutState(options: UseSessionLayoutStateOptions = {}
|
||||
handleFileOpenHandled,
|
||||
|
||||
// Git status
|
||||
gitStatus,
|
||||
commits,
|
||||
uncommittedCount,
|
||||
totalChangesCount,
|
||||
|
||||
// Plan
|
||||
|
||||
Reference in New Issue
Block a user