dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(web): refresh commits panel snapshot on mount (#1232)

Browse Source 
* fix(web): fetch commits snapshot on mount even when prefilled by live events

The useSessionCommits hook gated its initial snapshot fetch on
`commits === undefined`. When a `commit_created` WS event populated the
list before mount (session already running when the panel opens, or
stream replay on reconnect) and carried zero stats, the bad data stuck —
the authoritative `git log --shortstat` snapshot never ran to overwrite
it, leaving older commits stranded with `+0 -0` in the Commits panel.

Anchor the initial-fetch gate to `sessionId` via a ref so the snapshot
fires once per session regardless of how `commits` got populated.

* fix(web): reset fetchedSessionRef when sessionId goes null

Without resetting the ref in the !sessionId early-return path, a hook
that stays mounted while sessionId cycles through null (and commits get
cleared via clearSessionCommits) would not refetch when the same
sessionId is reselected, because fetchedSessionRef.current would still
equal it. Now the ref clears on null so reselect always re-fetches.

Addresses Greptile P2 on #1232.

* fix(web): reset fetchedSessionRef during disconnect teardown

Move the `!sessionId` early-return ref clear before the connection
gate. Without this, a session teardown that runs while WS is
disconnected leaves `fetchedSessionRef.current` pinned to the old
sessionId, so reconnecting to the same session never triggers the
authoritative snapshot fetch.

Addresses cubic P2 on #1232.

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
2026-06-01 21:52:55 +01:00
committed by GitHub
parent 140e828ead
commit a814379cf6
2 changed files with 157 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/hooks/domains/session/use-session-commits.test.ts
+126 -21
View File
@@ -154,16 +154,105 @@ describe("useSessionCommits", () => {
});
});
// Helper: seed store with one existing commit and a deferred request so we
// can observe the store mid-refetch (stale-while-revalidate test).
// Lives in its own describe so the outer block stays under the 100-line
// max-lines-per-function limit.
describe("useSessionCommits — authoritative snapshot on mount", () => {
beforeEach(() => {
vi.clearAllMocks();
setStore();
});
afterEach(() => {
cleanup();
});
it("fetches a snapshot on mount even when commits were prefilled by live events", async () => {
// Regression: a `commit_created` WS event can populate the commits list
// before the hook mounts (e.g. session already running when the panel
// opens). If the live event carried stale/zero stats — possible during
// stream reconnect / replay — the panel displays them forever unless
// the authoritative `git log --shortstat` snapshot also runs. The
// pre-existing `commits === undefined` gate skipped that fetch.
storeState.sessionCommits = {
byEnvironmentId: {
"sess-1": [
{
commit_sha: "older",
commit_message: "feat: x",
parent_sha: "base",
files_changed: 0,
insertions: 0,
deletions: 0,
},
],
},
loading: {},
refetchTrigger: {},
};
mockRequest.mockResolvedValueOnce({
commits: [
{
commit_sha: "older",
commit_message: "feat: x",
parent_sha: "base",
files_changed: 60,
insertions: 3443,
deletions: 227,
},
],
ready: true,
});
renderHook(() => useSessionCommits("sess-1"));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
await waitFor(() => {
expect(mockSetSessionCommits).toHaveBeenCalledWith(
"sess-1",
[
expect.objectContaining({
commit_sha: "older",
files_changed: 60,
insertions: 3443,
deletions: 227,
}),
],
undefined,
);
});
});
it("re-fetches after sessionId cycles null → same id (post-teardown reselect)", async () => {
// Greptile P2: if the hook stays mounted while sessionId goes null and the
// store's commits are cleared via clearSessionCommits, then when the same
// sessionId returns the ref still equals it — so no fetch fires and the
// panel stays blank. Clearing the ref in the early-return path fixes it.
mockRequest.mockResolvedValue({ commits: [{ commit_sha: "a" }], ready: true });
const { rerender } = renderHook(({ id }) => useSessionCommits(id), {
initialProps: { id: "sess-1" as string | null },
});
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
rerender({ id: null });
rerender({ id: "sess-1" });
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2));
});
});
// Helper: seed store with one existing commit, resolve the mount-time
// snapshot fetch with that same data, and return a deferred resolver for
// the trigger-bump refetch so the test can observe the store mid-refetch.
async function seedAndDeferRefetch(sessionId: string) {
const seeded = [{ commit_sha: "old", insertions: 1, deletions: 0 }];
storeState.sessionCommits = {
byEnvironmentId: {
[sessionId]: [{ commit_sha: "old", insertions: 1, deletions: 0 }],
},
byEnvironmentId: { [sessionId]: seeded },
loading: {},
refetchTrigger: { [sessionId]: 0 },
};
// The mount-time snapshot fetch resolves immediately with the seeded
// data (no-op write), so we can isolate the trigger-bump refetch below.
mockRequest.mockResolvedValueOnce({ commits: seeded, ready: true });
let resolveRequest!: (value: unknown) => void;
mockRequest.mockReturnValueOnce(
new Promise((resolve) => {
@@ -194,12 +283,13 @@ describe("useSessionCommits — stale-while-revalidate on trigger bump", () => {
it("refetches when refetchTrigger bumps without nulling the visible list", async () => {
const resolveRequest = await seedAndDeferRefetch("sess-1");
const { rerender } = renderHook(() => useSessionCommits("sess-1"));
expect(mockRequest).not.toHaveBeenCalled();
// The mount-time snapshot fetch fires once with the seeded data.
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
bumpTrigger("sess-1", 1);
rerender();
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2));
// During mid-refetch the store must still hold the OLD commits.
const midRefetch = (storeState.sessionCommits as { byEnvironmentId: Record<string, unknown> })
.byEnvironmentId;
@@ -217,23 +307,26 @@ describe("useSessionCommits — stale-while-revalidate on trigger bump", () => {
// After a `git reset`, the refetch legitimately returns []. Without
// `allowEmpty: true`, the default guard in `setSessionCommits` would
// silently drop that response and the panel would keep showing stale data.
const seeded = [
{ commit_sha: "a", insertions: 0, deletions: 0 },
{ commit_sha: "b", insertions: 0, deletions: 0 },
];
storeState.sessionCommits = {
byEnvironmentId: {
"sess-1": [
{ commit_sha: "a", insertions: 0, deletions: 0 },
{ commit_sha: "b", insertions: 0, deletions: 0 },
],
},
byEnvironmentId: { "sess-1": seeded },
loading: {},
refetchTrigger: { "sess-1": 0 },
};
// Mount-time snapshot fetch returns the seeded data (no-op write).
mockRequest.mockResolvedValueOnce({ commits: seeded, ready: true });
// Trigger-bump fetch returns the authoritative empty list.
mockRequest.mockResolvedValueOnce({ commits: [], ready: true });
const { rerender } = renderHook(() => useSessionCommits("sess-1"));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
bumpTrigger("sess-1", 1);
rerender();
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2));
await waitFor(() => {
expect(mockSetSessionCommits).toHaveBeenCalledWith("sess-1", [], { allowEmpty: true });
});
@@ -245,11 +338,15 @@ describe("useSessionCommits — stale-while-revalidate on trigger bump", () => {
});
it("drops a stale response when a newer fetch already started", async () => {
const seeded = [{ commit_sha: "initial" }];
storeState.sessionCommits = {
byEnvironmentId: { "sess-1": [{ commit_sha: "initial" }] },
byEnvironmentId: { "sess-1": seeded },
loading: {},
refetchTrigger: { "sess-1": 0 },
};
// Mount-time snapshot fetch — resolves immediately with the seeded data
// so the stale-vs-fresh race below only involves the trigger-bump fetches.
mockRequest.mockResolvedValueOnce({ commits: seeded, ready: true });
let resolveFirst!: (value: unknown) => void;
let resolveSecond!: (value: unknown) => void;
mockRequest
@@ -265,14 +362,15 @@ describe("useSessionCommits — stale-while-revalidate on trigger bump", () => {
);
const { rerender } = renderHook(() => useSessionCommits("sess-1"));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
bumpTrigger("sess-1", 1);
rerender();
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2));
bumpTrigger("sess-1", 2);
rerender();
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(2));
await waitFor(() => expect(mockRequest).toHaveBeenCalledTimes(3));
resolveFirst({ commits: [{ commit_sha: "stale" }], ready: true });
resolveSecond({ commits: [{ commit_sha: "fresh" }], ready: true });
@@ -283,9 +381,16 @@ describe("useSessionCommits — stale-while-revalidate on trigger bump", () => {
expect(after["sess-1"]).toEqual([{ commit_sha: "fresh" }]);
});
const winningCalls = mockSetSessionCommits.mock.calls.filter(
([, commits]) => Array.isArray(commits) && commits.length > 0,
);
expect(winningCalls).toHaveLength(1);
// The mount-fetch write (seeded data) and the fresh write should land;
// the stale write must be dropped by the request-version guard.
const writtenSHAs = mockSetSessionCommits.mock.calls
.map(([, commits]) =>
Array.isArray(commits) && commits.length > 0
? (commits[0] as { commit_sha: string }).commit_sha
: null,
)
.filter((sha): sha is string => sha !== null);
expect(writtenSHAs).not.toContain("stale");
expect(writtenSHAs).toContain("fresh");
});
});
apps/web/hooks/domains/session/use-session-commits.ts
+31 -12
View File
@@ -46,6 +46,15 @@ export function useSessionCommits(sessionId: string | null) {
// still drives an initial refetch — otherwise prevRef would equal the live
// value and `triggerBumped` would silently be false on first render.
const prevRefetchTriggerRef = useRef<number>(REFETCH_TRIGGER_INIT);
// Tracks which sessionId we've already run an authoritative snapshot fetch
// for. Without this, the initial-fetch gate fires only when `commits` is
// undefined — but commits can be populated by a live `commit_created`
// event that arrived before mount, and a replayed/raced event can carry
// zero stats. The bad stats then stick because the snapshot (which would
// overwrite with correct stats from `git log --shortstat`) never runs.
// Anchoring to sessionId guarantees a snapshot per session regardless of
// how `commits` got populated.
const fetchedSessionRef = useRef<string | null>(null);
// Retry timer for the not-ready case — agentctl recovers asynchronously
// after a backend restart, so the first fetch may land before the workspace
// execution has been ensured. Without a retry the store would be stuck on
@@ -125,33 +134,43 @@ export function useSessionCommits(sessionId: string | null) {
);
// Fetch commits when:
// 1. commits is undefined (initial load — clearSessionCommits is still
// called by callers that genuinely want to drop the list, e.g. session
// teardown).
// 2. the refetch trigger was bumped (commits_reset / branch_switched).
// 1. The hook mounts (or sessionId changes) — runs an authoritative
// snapshot once per session so live `commit_created` events that
// populated the store with stale/zero stats get overwritten by the
// real `git log --shortstat` data.
// 2. The refetch trigger was bumped (commits_reset / branch_switched).
//
// The trigger path replaces the old "clear then refetch from undefined"
// pattern: it keeps the previous commits in the store so the Changes panel
// doesn't flicker through its empty state while the refetch is in flight
// (stale-while-revalidate, matching how useCumulativeDiff works).
// The trigger path keeps the previous commits in the store while the
// refetch is in flight (stale-while-revalidate, matching how
// useCumulativeDiff works).
useEffect(() => {
// !sessionId clears the ref BEFORE the connection check so a teardown
// during a disconnect still resets the gate — otherwise the ref would
// retain the old id, and a same-id reselect after reconnect would skip
// the snapshot fetch ("session teardown during disconnect can still
// block the next snapshot fetch for the same session").
if (!sessionId) {
fetchedSessionRef.current = null;
return;
}
if (connectionStatus !== "connected") return;
if (!sessionId) return;
const needsInitialFetch = commits === undefined;
const triggerBumped = refetchTrigger !== prevRefetchTriggerRef.current;
const sessionChanged = fetchedSessionRef.current !== sessionId;
if (triggerBumped) {
// Trigger-bump path: the backend already mutated state (reset / branch
// switch). An empty response is authoritative — bypass the default
// anti-race guard so the panel reflects the actual post-reset state.
fetchCommits({ allowEmpty: true });
} else if (needsInitialFetch) {
fetchedSessionRef.current = sessionId;
} else if (sessionChanged) {
fetchCommits();
fetchedSessionRef.current = sessionId;
}
prevRefetchTriggerRef.current = refetchTrigger;
}, [sessionId, commits, refetchTrigger, fetchCommits, connectionStatus]);
}, [sessionId, refetchTrigger, fetchCommits, connectionStatus]);
// Cancel any in-flight retry on unmount, when the session changes, or when
// the WS disconnects — a retry firing against a disconnected client would