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 {
|
import {
|
||||||
__destroyAiSessionSyncStoreForTests,
|
__destroyAiSessionSyncStoreForTests,
|
||||||
__resetAiSessionSyncStoreForTests,
|
__resetAiSessionSyncStoreForTests,
|
||||||
|
useAiSessionSync,
|
||||||
} from "../useAiSessionSync";
|
} from "../useAiSessionSync";
|
||||||
import * as apiModule from "../../api";
|
import * as apiModule from "../../api";
|
||||||
import { MockEventSource } from "../../../vitest.setup";
|
import { MockEventSource } from "../../../vitest.setup";
|
||||||
@@ -297,6 +298,189 @@ describe("useBackgroundSessions", () => {
|
|||||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("planning-session");
|
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 () => {
|
it("dismissSession calls cancelSubtaskBreakdown for subtask sessions", async () => {
|
||||||
mockFetchAiSessions.mockResolvedValueOnce([
|
mockFetchAiSessions.mockResolvedValueOnce([
|
||||||
makeSession({ id: "subtask-session", status: "generating", type: "subtask" }),
|
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";
|
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 {
|
export function useBackgroundSessions(projectId?: string): UseBackgroundSessionsResult {
|
||||||
const [sessions, setSessions] = useState<AiSessionSummary[]>([]);
|
const [sessions, setSessions] = useState<AiSessionSummary[]>([]);
|
||||||
const sessionTimestampsRef = useRef<Map<string, number>>(new Map());
|
const sessionTimestampsRef = useRef<Map<string, number>>(new Map());
|
||||||
|
const dismissedSessionTimestampsRef = useRef<Map<string, number>>(new Map());
|
||||||
|
|
||||||
const {
|
const {
|
||||||
sessions: syncedSessions,
|
sessions: syncedSessions,
|
||||||
@@ -45,12 +52,29 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
const refresh = useCallback(() => {
|
const refresh = useCallback(() => {
|
||||||
fetchAiSessions(projectId)
|
fetchAiSessions(projectId)
|
||||||
.then((fetched) => {
|
.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>();
|
const nextTimestampMap = new Map<string, number>();
|
||||||
for (const session of fetched) {
|
for (const session of filtered) {
|
||||||
nextTimestampMap.set(session.id, parseTimestamp(session.updatedAt));
|
nextTimestampMap.set(session.id, parseTimestamp(session.updatedAt));
|
||||||
}
|
}
|
||||||
sessionTimestampsRef.current = nextTimestampMap;
|
sessionTimestampsRef.current = nextTimestampMap;
|
||||||
setSessions(fetched);
|
setSessions(filtered);
|
||||||
})
|
})
|
||||||
.catch((err) => {
|
.catch((err) => {
|
||||||
console.warn("[useBackgroundSessions] Failed to fetch AI sessions:", err);
|
console.warn("[useBackgroundSessions] Failed to fetch AI sessions:", err);
|
||||||
@@ -84,6 +108,27 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
continue;
|
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 existing = nextById.get(syncState.sessionId);
|
||||||
const type = syncState.type ?? existing?.type;
|
const type = syncState.type ?? existing?.type;
|
||||||
const title = syncState.title ?? existing?.title;
|
const title = syncState.title ?? existing?.title;
|
||||||
@@ -145,9 +190,22 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
return prev;
|
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);
|
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);
|
return prev.filter((session) => session.id !== updated.id);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -177,7 +235,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
timestamp: eventTimestamp,
|
timestamp: eventTimestamp,
|
||||||
});
|
});
|
||||||
|
|
||||||
if (updated.status === "complete" || updated.status === "error") {
|
if (isTerminalStatus(updated.status)) {
|
||||||
broadcastCompleted({
|
broadcastCompleted({
|
||||||
sessionId: updated.id,
|
sessionId: updated.id,
|
||||||
status: updated.status,
|
status: updated.status,
|
||||||
@@ -194,6 +252,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
const id = JSON.parse(e.data) as string;
|
const id = JSON.parse(e.data) as string;
|
||||||
setSessions((prev) => prev.filter((s) => s.id !== id));
|
setSessions((prev) => prev.filter((s) => s.id !== id));
|
||||||
sessionTimestampsRef.current.delete(id);
|
sessionTimestampsRef.current.delete(id);
|
||||||
|
dismissedSessionTimestampsRef.current.delete(id);
|
||||||
} catch {
|
} catch {
|
||||||
// ignore malformed payload
|
// 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
|
// Only proceed with deletion if cancellation succeeded or wasn't needed
|
||||||
if (cancelFailed && !lockConflict) {
|
if (cancelFailed) {
|
||||||
// Non-lock cancellation failure: still try to delete
|
// Non-lock cancellation failure: still try to delete
|
||||||
console.warn(`[useBackgroundSessions] Cancellation failed for session ${id}, attempting delete anyway`);
|
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
|
// Delete the session and update local state
|
||||||
try {
|
try {
|
||||||
await deleteAiSession(id);
|
await deleteAiSession(id);
|
||||||
@@ -265,8 +341,7 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
|||||||
}
|
}
|
||||||
|
|
||||||
setSessions((prev) => prev.filter((s) => s.id !== id));
|
setSessions((prev) => prev.filter((s) => s.id !== id));
|
||||||
sessionTimestampsRef.current.delete(id);
|
}, [broadcastCompleted, projectId, sessions]);
|
||||||
}, [projectId, sessions]);
|
|
||||||
|
|
||||||
const active = useMemo(
|
const active = useMemo(
|
||||||
() => sessions.filter((session) => shouldIncludeSession(session)),
|
() => sessions.filter((session) => shouldIncludeSession(session)),
|
||||||
|
|||||||
Reference in New Issue
Block a user