diff --git a/apps/web/components/task/task-page-content.tsx b/apps/web/components/task/task-page-content.tsx index c8dfccdc73..6b68154e60 100644 --- a/apps/web/components/task/task-page-content.tsx +++ b/apps/web/components/task/task-page-content.tsx @@ -532,16 +532,24 @@ function useTaskPageData( initialRepositories: Repository[], ) { const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); - const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); const setActiveSession = useAppStore((state) => state.setActiveSession); const setActiveTask = useAppStore((state) => state.setActiveTask); + // Validate that activeSessionId belongs to activeTaskId to prevent showing + // messages from an unrelated session when navigating to a task without sessions. + const validatedActiveSessionId = useAppStore((state) => { + const sid = state.tasks.activeSessionId; + if (!sid || !activeTaskId) return null; + const session = state.taskSessions.items[sid]; + return session?.task_id === activeTaskId ? sid : null; + }); + const { task } = useTaskDetails(activeTaskId, initialTask); const agent = useSessionAgent(task); useAutoStartSession(task, agent.handleStartAgent); const initialSessionId = sessionId ?? agent.taskSessionId ?? null; - const effectiveSessionId = activeSessionId ?? initialSessionId; + const effectiveSessionId = validatedActiveSessionId ?? initialSessionId; useEffect(() => { syncActiveTaskSession({ diff --git a/apps/web/e2e/tests/task/session-isolation.spec.ts b/apps/web/e2e/tests/task/session-isolation.spec.ts new file mode 100644 index 0000000000..d798f5b3c0 --- /dev/null +++ b/apps/web/e2e/tests/task/session-isolation.spec.ts @@ -0,0 +1,134 @@ +import { test, expect } from "../../fixtures/test-base"; +import { SessionPage } from "../../pages/session-page"; + +/** Mock response text from the simple-message scenario */ +const SIMPLE_MOCK_RESPONSE = "This is a simple mock response for e2e testing."; + +/** + * Test that navigating to a task without sessions does NOT show messages + * from the previously viewed task's session. + * + * Reproduces bug: when activeSessionId holds an old session ID and the user + * navigates to a new task (created with start_agent=false), the chat panel + * was incorrectly showing messages from the old session. + */ +test.describe("Session isolation", () => { + test("navigating to session-less task does not show messages from previous task", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + + // 1. Create a task WITH an agent session that will have messages + const taskWithSession = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Task With Messages", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + // 2. Navigate to the task with session and wait for agent to complete + await testPage.goto(`/t/${taskWithSession.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + // 3. Verify there are messages in the chat (agent has responded) + const chatPanel = testPage.getByTestId("session-chat"); + await expect(chatPanel.getByText(SIMPLE_MOCK_RESPONSE)).toBeVisible({ timeout: 10_000 }); + + // 4. Create a task WITHOUT an agent session (start_agent=false) + const taskWithoutSession = await apiClient.createTask( + seedData.workspaceId, + "Task Without Session", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + // 5. Navigate to the session-less task directly via URL + await testPage.goto(`/t/${taskWithoutSession.id}`); + await session.waitForLoad(); + + // 6. The chat panel should NOT show the message from the previous task + // This is the core assertion - we're testing that messages don't leak + await expect(chatPanel.getByText(SIMPLE_MOCK_RESPONSE)).not.toBeVisible({ timeout: 5_000 }); + + // 7. Also verify the task title in the page matches the new task + // (ensuring we actually navigated to the correct task) + await expect(testPage.getByRole("link", { name: "Task Without Session" })).toBeVisible({ + timeout: 5_000, + }); + }); + + test("switching tasks via sidebar does not show messages from previous task", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + + // 1. Create first task with agent session + const taskA = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "First Task A", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + // 2. Create second task with a DIFFERENT agent session (different message) + const taskB = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Second Task B", + seedData.agentProfileId, + { + description: "/e2e:read-and-edit", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + // 3. Navigate to first task and wait for agent to complete + await testPage.goto(`/t/${taskA.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + // 4. Verify mock response message is visible for first task + const chatPanel = testPage.getByTestId("session-chat"); + await expect(chatPanel.getByText(SIMPLE_MOCK_RESPONSE)).toBeVisible({ timeout: 10_000 }); + + // 5. Click on second task in the sidebar to switch + await session.clickTaskInSidebar("Second Task B"); + + // 6. Wait for navigation to complete + await expect(testPage).toHaveURL((url) => url.pathname.includes(`/t/${taskB.id}`), { + timeout: 10_000, + }); + await session.waitForLoad(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + + // 7. The chat should NOT show messages from the first task's session + // (the "simple-message" text should not appear for task B which uses "read-and-edit") + await expect(chatPanel.getByText(SIMPLE_MOCK_RESPONSE)).not.toBeVisible({ timeout: 5_000 }); + + // 8. Verify we're on the correct task + await expect(testPage.getByRole("link", { name: "Second Task B" })).toBeVisible({ + timeout: 5_000, + }); + }); +}); diff --git a/apps/web/hooks/domains/session/use-session-state.test.ts b/apps/web/hooks/domains/session/use-session-state.test.ts new file mode 100644 index 0000000000..18079848ac --- /dev/null +++ b/apps/web/hooks/domains/session/use-session-state.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook } from "@testing-library/react"; +import type { TaskSession } from "@/lib/types/http"; + +// Mock state +let mockActiveTaskId: string | null = null; +let mockActiveSessionId: string | null = null; +let mockSessionItems: Record = {}; +let mockSession: TaskSession | null = null; +let mockTask: { id: string; description: string } | null = null; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ + tasks: { + activeTaskId: mockActiveTaskId, + activeSessionId: mockActiveSessionId, + }, + taskSessions: { + items: mockSessionItems, + }, + }), +})); + +vi.mock("@/hooks/domains/session/use-session", () => ({ + useSession: (id: string | null) => ({ session: id ? mockSession : null }), +})); + +vi.mock("@/hooks/use-task", () => ({ + useTask: (id: string | null) => (id ? mockTask : null), +})); + +import { useSessionState } from "./use-session-state"; + +const createMockSession = ( + id: string, + taskId: string, + state: TaskSession["state"] = "IDLE", +): TaskSession => + ({ + id, + task_id: taskId, + state, + error_message: "", + }) as TaskSession; + +describe("useSessionState", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockActiveTaskId = null; + mockActiveSessionId = null; + mockSessionItems = {}; + mockSession = null; + mockTask = null; + }); + + describe("resolvedSessionId", () => { + it("uses sessionId directly when provided", () => { + mockActiveTaskId = "task-1"; + mockActiveSessionId = "session-old"; + mockSessionItems = { "session-old": createMockSession("session-old", "task-old") }; + + const { result } = renderHook(() => useSessionState("session-explicit")); + + expect(result.current.resolvedSessionId).toBe("session-explicit"); + }); + + it("uses activeSessionId when sessionId is null and session belongs to active task", () => { + mockActiveTaskId = "task-1"; + mockActiveSessionId = "session-1"; + mockSessionItems = { "session-1": createMockSession("session-1", "task-1") }; + + const { result } = renderHook(() => useSessionState(null)); + + expect(result.current.resolvedSessionId).toBe("session-1"); + }); + + it("returns null when sessionId is null and activeSessionId belongs to different task", () => { + mockActiveTaskId = "task-2"; + mockActiveSessionId = "session-1"; + mockSessionItems = { "session-1": createMockSession("session-1", "task-1") }; + + const { result } = renderHook(() => useSessionState(null)); + + expect(result.current.resolvedSessionId).toBeNull(); + }); + + it("returns null when sessionId is null and session not yet in store", () => { + mockActiveTaskId = "task-1"; + mockActiveSessionId = "session-1"; + mockSessionItems = {}; // Session not loaded yet + + const { result } = renderHook(() => useSessionState(null)); + + expect(result.current.resolvedSessionId).toBeNull(); + }); + + it("returns null when sessionId is null and activeSessionId is null", () => { + mockActiveTaskId = "task-1"; + mockActiveSessionId = null; + mockSessionItems = {}; + + const { result } = renderHook(() => useSessionState(null)); + + expect(result.current.resolvedSessionId).toBeNull(); + }); + }); + + describe("session flags", () => { + it("sets isStarting when session state is STARTING", () => { + mockSession = createMockSession("session-1", "task-1", "STARTING"); + + const { result } = renderHook(() => useSessionState("session-1")); + + expect(result.current.isStarting).toBe(true); + expect(result.current.isWorking).toBe(true); + }); + + it("sets isAgentBusy when session state is RUNNING", () => { + mockSession = createMockSession("session-1", "task-1", "RUNNING"); + + const { result } = renderHook(() => useSessionState("session-1")); + + expect(result.current.isAgentBusy).toBe(true); + expect(result.current.isWorking).toBe(true); + }); + + it("sets isFailed when session state is FAILED", () => { + mockSession = createMockSession("session-1", "task-1", "FAILED"); + + const { result } = renderHook(() => useSessionState("session-1")); + + expect(result.current.isFailed).toBe(true); + }); + }); +}); diff --git a/apps/web/hooks/domains/session/use-session-state.ts b/apps/web/hooks/domains/session/use-session-state.ts index d434b8910b..80945b004e 100644 --- a/apps/web/hooks/domains/session/use-session-state.ts +++ b/apps/web/hooks/domains/session/use-session-state.ts @@ -13,8 +13,20 @@ function deriveSessionFlags(state: TaskSession["state"] | undefined, errorMessag } export function useSessionState(sessionId: string | null) { + const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); - const resolvedSessionId = sessionId ?? activeSessionId; + + // Validate that active session belongs to the active task before using it. + // This prevents showing messages from an unrelated session when navigating + // to a task that has no sessions yet (activeSessionId may still hold the + // old session from the previous task). + const activeSessionData = useAppStore((state) => + activeSessionId ? (state.taskSessions.items[activeSessionId] ?? null) : null, + ); + const validatedActiveSessionId = + activeSessionData && activeSessionData.task_id === activeTaskId ? activeSessionId : null; + + const resolvedSessionId = sessionId ?? validatedActiveSessionId; const { session } = useSession(resolvedSessionId); const task = useTask(session?.task_id ?? null);