fix(FN-2534): prevent dismissed background sessions from resurfacing
- Add dismissal tombstone tracking in useBackgroundSessions to ignore stale refresh, sync, and SSE updates for dismissed session IDs - Treat terminal session states consistently and broadcast completion on dismiss so local/session-sync state remains converged - Keep lock-conflicted planning cancellations visible instead of falsely dismissing the session when another tab owns the lock - Expand useBackgroundSessions regression tests to cover stale sync/SSE resurrection, refresh behavior, lock-conflict handling, and newer-authoritative restore paths
This commit is contained in:
@@ -9,6 +9,7 @@ import { useBackgroundSessions } from "../useBackgroundSessions";
|
||||
import {
|
||||
__destroyAiSessionSyncStoreForTests,
|
||||
__resetAiSessionSyncStoreForTests,
|
||||
useAiSessionSync,
|
||||
} from "../useAiSessionSync";
|
||||
import * as apiModule from "../../api";
|
||||
import { MockEventSource } from "../../../vitest.setup";
|
||||
@@ -297,6 +298,189 @@ describe("useBackgroundSessions", () => {
|
||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("planning-session");
|
||||
});
|
||||
|
||||
it("does not dismiss planning session when cancellation is lock-conflicted", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "planning-locked", status: "generating", type: "planning" }),
|
||||
]);
|
||||
mockCancelPlanning.mockRejectedValueOnce(new Error("locked by another tab"));
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["planning-locked"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("planning-locked");
|
||||
});
|
||||
|
||||
expect(mockDeleteAiSession).not.toHaveBeenCalled();
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["planning-locked"]);
|
||||
expect(result.current.generating).toBe(1);
|
||||
});
|
||||
|
||||
it("keeps a dismissed planning session hidden when stale sync update arrives", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "dismiss-sync", status: "generating", type: "planning" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
const { result: syncResult } = renderHook(() => useAiSessionSync());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-sync"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("dismiss-sync");
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
syncResult.current.broadcastUpdate({
|
||||
sessionId: "dismiss-sync",
|
||||
status: "generating",
|
||||
needsInput: false,
|
||||
type: "planning",
|
||||
title: "Dismiss Sync",
|
||||
updatedAt: "1970-01-01T00:00:01.000Z",
|
||||
timestamp: 1,
|
||||
});
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
expect(result.current.planningSessions).toEqual([]);
|
||||
expect(result.current.generating).toBe(0);
|
||||
expect(result.current.needsInput).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a dismissed planning session hidden when stale SSE update arrives", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "dismiss-sse", status: "generating", type: "planning" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(MockEventSource.instances.length).toBeGreaterThan(0);
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-sse"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("dismiss-sse");
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
});
|
||||
|
||||
const eventSource = MockEventSource.instances[0]!;
|
||||
act(() => {
|
||||
eventSource._emit(
|
||||
"ai_session:updated",
|
||||
makeSession({
|
||||
id: "dismiss-sse",
|
||||
type: "planning",
|
||||
status: "generating",
|
||||
title: "Dismiss SSE",
|
||||
updatedAt: "1970-01-01T00:00:01.000Z",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
expect(result.current.planningSessions).toEqual([]);
|
||||
expect(result.current.generating).toBe(0);
|
||||
expect(result.current.needsInput).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it("allows a newer authoritative SSE update to restore a dismissed session", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "dismiss-restore", status: "generating", type: "planning" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(MockEventSource.instances.length).toBeGreaterThan(0);
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-restore"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("dismiss-restore");
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
});
|
||||
|
||||
const eventSource = MockEventSource.instances[0]!;
|
||||
act(() => {
|
||||
eventSource._emit(
|
||||
"ai_session:updated",
|
||||
makeSession({
|
||||
id: "dismiss-restore",
|
||||
type: "planning",
|
||||
status: "awaiting_input",
|
||||
title: "Dismiss Restore",
|
||||
updatedAt: new Date(Date.now() + 60_000).toISOString(),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-restore"]);
|
||||
expect(result.current.planningSessions.map((session) => session.id)).toEqual(["dismiss-restore"]);
|
||||
expect(result.current.generating).toBe(0);
|
||||
expect(result.current.needsInput).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("refresh keeps dismissed sessions hidden when server returns stale data", async () => {
|
||||
const staleSession = makeSession({
|
||||
id: "dismiss-refresh",
|
||||
status: "generating",
|
||||
type: "planning",
|
||||
updatedAt: "2026-04-08T00:00:01.000Z",
|
||||
});
|
||||
|
||||
mockFetchAiSessions.mockResolvedValueOnce([staleSession]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-refresh"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("dismiss-refresh");
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
});
|
||||
|
||||
mockFetchAiSessions.mockResolvedValueOnce([staleSession]);
|
||||
|
||||
act(() => {
|
||||
result.current.refresh();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions).toEqual([]);
|
||||
expect(result.current.planningSessions).toEqual([]);
|
||||
expect(result.current.generating).toBe(0);
|
||||
expect(result.current.needsInput).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it("dismissSession calls cancelSubtaskBreakdown for subtask sessions", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "subtask-session", status: "generating", type: "subtask" }),
|
||||
|
||||
@@ -31,9 +31,16 @@ function shouldIncludeSession(session: AiSessionSummary): boolean {
|
||||
return session.status === "generating" || session.status === "awaiting_input";
|
||||
}
|
||||
|
||||
function isTerminalStatus(
|
||||
status: AiSessionSummary["status"],
|
||||
): status is Extract<AiSessionSummary["status"], "complete" | "error"> {
|
||||
return status === "complete" || status === "error";
|
||||
}
|
||||
|
||||
export function useBackgroundSessions(projectId?: string): UseBackgroundSessionsResult {
|
||||
const [sessions, setSessions] = useState<AiSessionSummary[]>([]);
|
||||
const sessionTimestampsRef = useRef<Map<string, number>>(new Map());
|
||||
const dismissedSessionTimestampsRef = useRef<Map<string, number>>(new Map());
|
||||
|
||||
const {
|
||||
sessions: syncedSessions,
|
||||
@@ -45,12 +52,29 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
const refresh = useCallback(() => {
|
||||
fetchAiSessions(projectId)
|
||||
.then((fetched) => {
|
||||
const filtered = fetched.filter((session) => {
|
||||
const fetchedTimestamp = parseTimestamp(session.updatedAt);
|
||||
const dismissedTimestamp = dismissedSessionTimestampsRef.current.get(session.id);
|
||||
if (dismissedTimestamp === undefined) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Treat explicit dismissals as terminal tombstones unless the server has
|
||||
// emitted a genuinely newer state for the same session id.
|
||||
if (fetchedTimestamp <= dismissedTimestamp) {
|
||||
return false;
|
||||
}
|
||||
|
||||
dismissedSessionTimestampsRef.current.delete(session.id);
|
||||
return true;
|
||||
});
|
||||
|
||||
const nextTimestampMap = new Map<string, number>();
|
||||
for (const session of fetched) {
|
||||
for (const session of filtered) {
|
||||
nextTimestampMap.set(session.id, parseTimestamp(session.updatedAt));
|
||||
}
|
||||
sessionTimestampsRef.current = nextTimestampMap;
|
||||
setSessions(fetched);
|
||||
setSessions(filtered);
|
||||
})
|
||||
.catch((err) => {
|
||||
console.warn("[useBackgroundSessions] Failed to fetch AI sessions:", err);
|
||||
@@ -84,6 +108,27 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
continue;
|
||||
}
|
||||
|
||||
const dismissedTimestamp = dismissedSessionTimestampsRef.current.get(syncState.sessionId);
|
||||
if (dismissedTimestamp !== undefined && incomingTimestamp <= dismissedTimestamp) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
dismissedTimestamp !== undefined &&
|
||||
incomingTimestamp > dismissedTimestamp &&
|
||||
!isTerminalStatus(syncState.status)
|
||||
) {
|
||||
dismissedSessionTimestampsRef.current.delete(syncState.sessionId);
|
||||
}
|
||||
|
||||
if (isTerminalStatus(syncState.status)) {
|
||||
if (nextById.delete(syncState.sessionId)) {
|
||||
changed = true;
|
||||
}
|
||||
sessionTimestampsRef.current.set(syncState.sessionId, incomingTimestamp);
|
||||
continue;
|
||||
}
|
||||
|
||||
const existing = nextById.get(syncState.sessionId);
|
||||
const type = syncState.type ?? existing?.type;
|
||||
const title = syncState.title ?? existing?.title;
|
||||
@@ -145,9 +190,22 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
return prev;
|
||||
}
|
||||
|
||||
const dismissedTimestamp = dismissedSessionTimestampsRef.current.get(updated.id);
|
||||
if (dismissedTimestamp !== undefined && eventTimestamp <= dismissedTimestamp) {
|
||||
return prev;
|
||||
}
|
||||
|
||||
if (
|
||||
dismissedTimestamp !== undefined &&
|
||||
eventTimestamp > dismissedTimestamp &&
|
||||
!isTerminalStatus(updated.status)
|
||||
) {
|
||||
dismissedSessionTimestampsRef.current.delete(updated.id);
|
||||
}
|
||||
|
||||
sessionTimestampsRef.current.set(updated.id, eventTimestamp);
|
||||
|
||||
if (updated.status === "complete" || updated.status === "error") {
|
||||
if (isTerminalStatus(updated.status)) {
|
||||
return prev.filter((session) => session.id !== updated.id);
|
||||
}
|
||||
|
||||
@@ -177,7 +235,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
timestamp: eventTimestamp,
|
||||
});
|
||||
|
||||
if (updated.status === "complete" || updated.status === "error") {
|
||||
if (isTerminalStatus(updated.status)) {
|
||||
broadcastCompleted({
|
||||
sessionId: updated.id,
|
||||
status: updated.status,
|
||||
@@ -194,6 +252,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
const id = JSON.parse(e.data) as string;
|
||||
setSessions((prev) => prev.filter((s) => s.id !== id));
|
||||
sessionTimestampsRef.current.delete(id);
|
||||
dismissedSessionTimestampsRef.current.delete(id);
|
||||
} catch {
|
||||
// ignore malformed payload
|
||||
}
|
||||
@@ -251,12 +310,29 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
}
|
||||
}
|
||||
|
||||
// If another tab owns the lock, local dismiss must not pretend success.
|
||||
if (lockConflict) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only proceed with deletion if cancellation succeeded or wasn't needed
|
||||
if (cancelFailed && !lockConflict) {
|
||||
if (cancelFailed) {
|
||||
// Non-lock cancellation failure: still try to delete
|
||||
console.warn(`[useBackgroundSessions] Cancellation failed for session ${id}, attempting delete anyway`);
|
||||
}
|
||||
|
||||
const dismissalTimestamp = Date.now();
|
||||
|
||||
// Root cause: local removal without dismissal tombstone let stale sync/SSE
|
||||
// snapshots resurrect the same session. Record terminal dismissal semantics
|
||||
// before async deletion so delayed updates cannot re-materialize it.
|
||||
dismissedSessionTimestampsRef.current.set(id, dismissalTimestamp);
|
||||
sessionTimestampsRef.current.set(
|
||||
id,
|
||||
Math.max(sessionTimestampsRef.current.get(id) ?? 0, dismissalTimestamp),
|
||||
);
|
||||
broadcastCompleted({ sessionId: id, status: "complete", timestamp: dismissalTimestamp });
|
||||
|
||||
// Delete the session and update local state
|
||||
try {
|
||||
await deleteAiSession(id);
|
||||
@@ -265,8 +341,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
}
|
||||
|
||||
setSessions((prev) => prev.filter((s) => s.id !== id));
|
||||
sessionTimestampsRef.current.delete(id);
|
||||
}, [projectId, sessions]);
|
||||
}, [broadcastCompleted, projectId, sessions]);
|
||||
|
||||
const active = useMemo(
|
||||
() => sessions.filter((session) => shouldIncludeSession(session)),
|
||||
|
||||
Reference in New Issue
Block a user