feat(FN-1456): add explicit abandon flow for planning and subtask AI sessions
- Add abandon action to useBackgroundSessions hook for deterministic session cleanup - Make background dismissal deterministic instead of relying on 7-day TTL expiration - Update PlanningModeModal to include abandon button in header actions - Add comprehensive tests for abandon behavior and session lifecycle - Update dashboard guide with session lifecycle documentation
This commit is contained in:
@@ -207,6 +207,7 @@ describe("PlanningModeModal", () => {
|
||||
mockAcquireSessionLock.mockResolvedValue({ acquired: true, currentHolder: null });
|
||||
mockReleaseSessionLock.mockResolvedValue(undefined);
|
||||
mockForceAcquireSessionLock.mockResolvedValue(undefined);
|
||||
mockCancelPlanning.mockResolvedValue(undefined);
|
||||
|
||||
// Default: simulate receiving a question after a brief delay
|
||||
mockConnectPlanningStream.mockImplementation((_sessionId: string, _projectId: string | undefined, handlers: any) => {
|
||||
@@ -1530,7 +1531,7 @@ describe("PlanningModeModal", () => {
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes active question session without canceling server session", async () => {
|
||||
it("closes active question session and abandons server session", async () => {
|
||||
const confirmSpy = vi.spyOn(window, "confirm");
|
||||
|
||||
render(
|
||||
@@ -1554,11 +1555,12 @@ describe("PlanningModeModal", () => {
|
||||
fireEvent.click(screen.getByLabelText("Close"));
|
||||
|
||||
expect(confirmSpy).not.toHaveBeenCalled();
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
// Closing an active session should abandon it on the server
|
||||
expect(mockCancelPlanning).toHaveBeenCalledTimes(1);
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes summary view without canceling server session", async () => {
|
||||
it("closes summary view and abandons server session", async () => {
|
||||
const confirmSpy = vi.spyOn(window, "confirm");
|
||||
|
||||
mockConnectPlanningStream.mockImplementationOnce((_sessionId: string, _projectId: string | undefined, handlers: any) => {
|
||||
@@ -1593,11 +1595,12 @@ describe("PlanningModeModal", () => {
|
||||
fireEvent.click(screen.getByLabelText("Close"));
|
||||
|
||||
expect(confirmSpy).not.toHaveBeenCalled();
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
// Closing an active session should abandon it on the server
|
||||
expect(mockCancelPlanning).toHaveBeenCalledTimes(1);
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes via overlay without canceling server session", async () => {
|
||||
it("closes via overlay and abandons server session", async () => {
|
||||
const confirmSpy = vi.spyOn(window, "confirm");
|
||||
|
||||
const { container } = render(
|
||||
@@ -1623,11 +1626,12 @@ describe("PlanningModeModal", () => {
|
||||
fireEvent.click(overlay!);
|
||||
|
||||
expect(confirmSpy).not.toHaveBeenCalled();
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
// Closing an active session should abandon it on the server
|
||||
expect(mockCancelPlanning).toHaveBeenCalledTimes(1);
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes during loading state without canceling server session", async () => {
|
||||
it("closes during loading state and abandons server session", async () => {
|
||||
const confirmSpy = vi.spyOn(window, "confirm");
|
||||
|
||||
mockConnectPlanningStream.mockImplementationOnce(() => ({
|
||||
@@ -1656,7 +1660,8 @@ describe("PlanningModeModal", () => {
|
||||
fireEvent.click(screen.getByLabelText("Close"));
|
||||
|
||||
expect(confirmSpy).not.toHaveBeenCalled();
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
// Closing an active session should abandon it on the server
|
||||
expect(mockCancelPlanning).toHaveBeenCalledTimes(1);
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -1693,7 +1698,7 @@ describe("PlanningModeModal", () => {
|
||||
expect(mockOnClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("disconnects SSE stream on close", async () => {
|
||||
it("disconnects SSE stream and abandons session on close", async () => {
|
||||
const closeSpy = vi.fn();
|
||||
|
||||
mockConnectPlanningStream.mockImplementationOnce(() => ({
|
||||
@@ -1722,7 +1727,8 @@ describe("PlanningModeModal", () => {
|
||||
fireEvent.click(screen.getByLabelText("Close"));
|
||||
|
||||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
// Closing an active session should abandon it on the server
|
||||
expect(mockCancelPlanning).toHaveBeenCalledTimes(1);
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
startPlanningBreakdown,
|
||||
createTasksFromPlanning,
|
||||
fetchModels,
|
||||
cancelPlanning,
|
||||
type PlanningSession,
|
||||
type SubtaskItem,
|
||||
type ModelInfo,
|
||||
@@ -467,6 +468,17 @@ export function PlanningModeModal({ isOpen, onClose, onTaskCreated, onTasksCreat
|
||||
}, [onClose]);
|
||||
|
||||
const handleCancel = useCallback(() => {
|
||||
// Determine the active session ID to abandon
|
||||
let activeSessionId: string | null = null;
|
||||
if (view.type === "question" || view.type === "summary" || view.type === "error") {
|
||||
activeSessionId = view.session.sessionId;
|
||||
} else if (view.type === "breakdown") {
|
||||
activeSessionId = view.sessionId;
|
||||
} else if (view.type === "loading") {
|
||||
// During loading, the session ID is stored in the ref
|
||||
activeSessionId = currentSessionIdRef.current;
|
||||
}
|
||||
|
||||
// Save to localStorage BEFORE any cleanup (preserve for re-entry)
|
||||
if (initialPlan) {
|
||||
savePlanningDescription(initialPlan, projectId);
|
||||
@@ -476,6 +488,13 @@ export function PlanningModeModal({ isOpen, onClose, onTaskCreated, onTasksCreat
|
||||
streamConnectionRef.current?.close();
|
||||
streamConnectionRef.current = null;
|
||||
|
||||
// Explicitly abandon the session on the server to prevent zombie sessions
|
||||
if (activeSessionId) {
|
||||
void cancelPlanning(activeSessionId, projectId, sessionTabId).catch(() => {
|
||||
// Best-effort: cancellation failures should not block UI reset
|
||||
});
|
||||
}
|
||||
|
||||
setInitialPlan("");
|
||||
setView({ type: "initial" });
|
||||
setError(null);
|
||||
@@ -490,7 +509,7 @@ export function PlanningModeModal({ isOpen, onClose, onTaskCreated, onTasksCreat
|
||||
currentSessionIdRef.current = null;
|
||||
setLockSessionId(null);
|
||||
onClose();
|
||||
}, [initialPlan, onClose, projectId]);
|
||||
}, [initialPlan, onClose, projectId, sessionTabId, view]);
|
||||
|
||||
// Handle escape key to close
|
||||
useEffect(() => {
|
||||
|
||||
@@ -16,10 +16,14 @@ import { MockEventSource } from "../../../vitest.setup";
|
||||
vi.mock("../../api", () => ({
|
||||
fetchAiSessions: vi.fn(),
|
||||
deleteAiSession: vi.fn(),
|
||||
cancelPlanning: vi.fn(),
|
||||
cancelSubtaskBreakdown: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockFetchAiSessions = vi.mocked(apiModule.fetchAiSessions);
|
||||
const mockDeleteAiSession = vi.mocked(apiModule.deleteAiSession);
|
||||
const mockCancelPlanning = vi.mocked(apiModule.cancelPlanning);
|
||||
const mockCancelSubtaskBreakdown = vi.mocked(apiModule.cancelSubtaskBreakdown);
|
||||
|
||||
function makeSession(overrides: Partial<apiModule.AiSessionSummary> & Pick<apiModule.AiSessionSummary, "id">): apiModule.AiSessionSummary {
|
||||
return {
|
||||
@@ -40,6 +44,8 @@ describe("useBackgroundSessions", () => {
|
||||
__destroyAiSessionSyncStoreForTests();
|
||||
mockFetchAiSessions.mockResolvedValue([]);
|
||||
mockDeleteAiSession.mockResolvedValue(undefined);
|
||||
mockCancelPlanning.mockResolvedValue(undefined);
|
||||
mockCancelSubtaskBreakdown.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -152,8 +158,8 @@ describe("useBackgroundSessions", () => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["dismiss-me"]);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.dismissSession("dismiss-me");
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("dismiss-me");
|
||||
});
|
||||
|
||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("dismiss-me");
|
||||
@@ -162,6 +168,64 @@ describe("useBackgroundSessions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("dismissSession calls cancelPlanning for planning sessions", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "planning-session", status: "generating", type: "planning" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["planning-session"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("planning-session");
|
||||
});
|
||||
|
||||
expect(mockCancelPlanning).toHaveBeenCalledWith("planning-session", undefined, expect.any(String));
|
||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("planning-session");
|
||||
});
|
||||
|
||||
it("dismissSession calls cancelSubtaskBreakdown for subtask sessions", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "subtask-session", status: "generating", type: "subtask" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["subtask-session"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("subtask-session");
|
||||
});
|
||||
|
||||
expect(mockCancelSubtaskBreakdown).toHaveBeenCalledWith("subtask-session", undefined, expect.any(String));
|
||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("subtask-session");
|
||||
});
|
||||
|
||||
it("dismissSession does not call cancel for mission_interview sessions", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "interview-session", status: "generating", type: "mission_interview" }),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useBackgroundSessions());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.sessions.map((session) => session.id)).toEqual(["interview-session"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.dismissSession("interview-session");
|
||||
});
|
||||
|
||||
expect(mockCancelPlanning).not.toHaveBeenCalled();
|
||||
expect(mockCancelSubtaskBreakdown).not.toHaveBeenCalled();
|
||||
expect(mockDeleteAiSession).toHaveBeenCalledWith("interview-session");
|
||||
});
|
||||
|
||||
it("returns accurate generating/needsInput counts and planningSessions filter", async () => {
|
||||
mockFetchAiSessions.mockResolvedValueOnce([
|
||||
makeSession({ id: "count-generating", status: "generating", type: "planning" }),
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
import { useState, useEffect, useCallback, useMemo, useRef } from "react";
|
||||
import { fetchAiSessions, deleteAiSession, type AiSessionSummary } from "../api";
|
||||
import {
|
||||
fetchAiSessions,
|
||||
deleteAiSession,
|
||||
cancelPlanning,
|
||||
cancelSubtaskBreakdown,
|
||||
type AiSessionSummary,
|
||||
} from "../api";
|
||||
import { useAiSessionSync } from "./useAiSessionSync";
|
||||
import { getSessionTabId } from "../utils/getSessionTabId";
|
||||
|
||||
interface UseBackgroundSessionsResult {
|
||||
sessions: AiSessionSummary[];
|
||||
@@ -202,11 +209,57 @@ export function useBackgroundSessions(projectId?: string): UseBackgroundSessions
|
||||
};
|
||||
}, [broadcastCompleted, broadcastUpdate, projectId]);
|
||||
|
||||
const dismissSession = useCallback((id: string) => {
|
||||
deleteAiSession(id).catch(() => {});
|
||||
const dismissSession = useCallback(async (id: string) => {
|
||||
// Find the session to determine its type
|
||||
const session = sessions.find((s) => s.id === id);
|
||||
const sessionType = session?.type;
|
||||
const sessionTabId = getSessionTabId();
|
||||
|
||||
// Cancel the session based on its type to ensure proper cleanup
|
||||
// Pass tabId for lock-aware cancellation - if locked by another tab, the API returns 409
|
||||
let cancelFailed = false;
|
||||
let lockConflict = false;
|
||||
|
||||
if (sessionType === "planning") {
|
||||
try {
|
||||
await cancelPlanning(id, projectId, sessionTabId);
|
||||
} catch (err: unknown) {
|
||||
cancelFailed = true;
|
||||
// Check if this was a lock conflict (409)
|
||||
if (err instanceof Error && err.message.includes("locked")) {
|
||||
lockConflict = true;
|
||||
console.warn(`[useBackgroundSessions] Cannot dismiss planning session ${id}: locked by another tab`);
|
||||
}
|
||||
}
|
||||
} else if (sessionType === "subtask") {
|
||||
try {
|
||||
await cancelSubtaskBreakdown(id, projectId, sessionTabId);
|
||||
} catch (err: unknown) {
|
||||
cancelFailed = true;
|
||||
if (err instanceof Error && err.message.includes("locked")) {
|
||||
lockConflict = true;
|
||||
console.warn(`[useBackgroundSessions] Cannot dismiss subtask session ${id}: locked by another tab`);
|
||||
}
|
||||
}
|
||||
}
|
||||
// For other session types (mission_interview, etc.), just delete without cancellation
|
||||
|
||||
// Only proceed with deletion if cancellation succeeded or wasn't needed
|
||||
if (cancelFailed && !lockConflict) {
|
||||
// Non-lock cancellation failure: still try to delete
|
||||
console.warn(`[useBackgroundSessions] Cancellation failed for session ${id}, attempting delete anyway`);
|
||||
}
|
||||
|
||||
// Delete the session and update local state
|
||||
try {
|
||||
await deleteAiSession(id);
|
||||
} catch {
|
||||
// Best-effort: deletion failures should not block local state update
|
||||
}
|
||||
|
||||
setSessions((prev) => prev.filter((s) => s.id !== id));
|
||||
sessionTimestampsRef.current.delete(id);
|
||||
}, []);
|
||||
}, [projectId, sessions]);
|
||||
|
||||
const active = useMemo(
|
||||
() => sessions.filter((session) => shouldIncludeSession(session)),
|
||||
|
||||
Reference in New Issue
Block a user