diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts index f8639dd6e8..da3588acd5 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts @@ -23,6 +23,7 @@ import { type WorkflowSnapshot, } from "@/lib/types/http"; import type { KanbanState } from "@/lib/state/slices"; +import { resolvePreferredSessionId } from "../task-select-helpers"; // Map workflow snapshot to kanban state on workspace switch. function mapSnapshotToKanban(snapshot: WorkflowSnapshot, newWorkflowId: string) { @@ -453,10 +454,16 @@ export function useSheetActions(workspaceId: string | null, onOpenChange: (open: const handleSelectTask = useCallback( (taskId: string) => { - const kanbanTasks = store.getState().kanban.tasks; - const task = kanbanTasks.find((t) => t.id === taskId); + const state = store.getState(); + const task = state.kanban.tasks.find((t) => t.id === taskId); if (task?.primarySessionId) { - setActiveSession(taskId, task.primarySessionId); + const targetSessionId = resolvePreferredSessionId( + taskId, + task.primarySessionId, + state.tasks.lastSessionByTaskId, + state.environmentIdBySessionId, + ); + setActiveSession(taskId, targetSessionId); loadTaskSessionsForTask(taskId); replaceTaskUrl(taskId); onOpenChange(false); diff --git a/apps/web/components/task/task-select-helpers.test.ts b/apps/web/components/task/task-select-helpers.test.ts index 59a3aa9a8e..cfdafdb4a3 100644 --- a/apps/web/components/task/task-select-helpers.test.ts +++ b/apps/web/components/task/task-select-helpers.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { prepareAndSwitchTask, buildSwitchToSession } from "./task-select-helpers"; +import { + prepareAndSwitchTask, + buildSwitchToSession, + selectTaskWithLayout, +} from "./task-select-helpers"; vi.mock("@/lib/services/session-launch-service", () => ({ launchSession: vi.fn(), @@ -163,3 +167,120 @@ describe("buildSwitchToSession", () => { expect(releaseLayoutToDefault).not.toHaveBeenCalled(); }); }); + +/** + * Regression for "switching tasks loses the user's last-selected session": + * + * 1. Task A has sessions [primary, gpt]; user clicks the gpt tab. + * 2. User clicks Task B in the sidebar. + * 3. User clicks Task A in the sidebar — expected the gpt tab still active. + * + * Before the fix, `selectTaskWithLayout` always switched to `primarySessionId`, + * so step 3 set activeSessionId back to "primary". The dockview slow-path then + * closed the gpt panel (it didn't match activeSessionId), and the surviving + * sibling tab (Diff) auto-promoted to active. + * + * The fix tracks the user's last-selected session per task in + * `tasks.lastSessionByTaskId` and prefers it over `primarySessionId` on + * re-entry, as long as the session still has a known environment. + */ +describe("selectTaskWithLayout — last-selected session preference", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + function makeKanbanStore(args: { + activeSessionId: string | null; + envIds: Record; + lastSessionByTaskId?: Record; + }): StoreApi { + const state = { + tasks: { + activeSessionId: args.activeSessionId, + lastSessionByTaskId: args.lastSessionByTaskId ?? {}, + }, + taskPRs: { byTaskId: {} as Record }, + environmentIdBySessionId: args.envIds, + }; + return { + getState: () => state as unknown as AppState, + setState: vi.fn(), + subscribe: vi.fn(), + } as unknown as StoreApi; + } + + it("prefers the user's last-selected session over primarySessionId on re-entry", () => { + const TASK_ID = "task-A"; + const PRIMARY = "sess-primary"; + const LAST = "sess-gpt"; + const store = makeKanbanStore({ + activeSessionId: "sess-other-task", + envIds: { + "sess-other-task": "env-B", + [PRIMARY]: "env-A", + [LAST]: "env-A", + }, + lastSessionByTaskId: { [TASK_ID]: LAST }, + }); + const switchToSession = vi.fn(); + + selectTaskWithLayout({ + taskId: TASK_ID, + task: { primarySessionId: PRIMARY }, + store, + switchToSession, + loadTaskSessionsForTask: vi.fn(async () => []), + setActiveTask: vi.fn(), + setPreparingTaskId: vi.fn(), + }); + + expect(switchToSession).toHaveBeenCalledWith(TASK_ID, LAST, "sess-other-task"); + }); + + it("falls back to primarySessionId when the remembered session has no env mapping", () => { + const TASK_ID = "task-A"; + const PRIMARY = "sess-primary"; + const LAST = "sess-stale"; + const store = makeKanbanStore({ + activeSessionId: null, + envIds: { [PRIMARY]: "env-A" }, + lastSessionByTaskId: { [TASK_ID]: LAST }, + }); + const switchToSession = vi.fn(); + + selectTaskWithLayout({ + taskId: TASK_ID, + task: { primarySessionId: PRIMARY }, + store, + switchToSession, + loadTaskSessionsForTask: vi.fn(async () => []), + setActiveTask: vi.fn(), + setPreparingTaskId: vi.fn(), + }); + + expect(switchToSession).toHaveBeenCalledWith(TASK_ID, PRIMARY, null); + }); + + it("uses primarySessionId when no last-selected session is recorded for the task", () => { + const TASK_ID = "task-A"; + const PRIMARY = "sess-primary"; + const store = makeKanbanStore({ + activeSessionId: null, + envIds: { [PRIMARY]: "env-A" }, + lastSessionByTaskId: {}, + }); + const switchToSession = vi.fn(); + + selectTaskWithLayout({ + taskId: TASK_ID, + task: { primarySessionId: PRIMARY }, + store, + switchToSession, + loadTaskSessionsForTask: vi.fn(async () => []), + setActiveTask: vi.fn(), + setPreparingTaskId: vi.fn(), + }); + + expect(switchToSession).toHaveBeenCalledWith(TASK_ID, PRIMARY, null); + }); +}); diff --git a/apps/web/components/task/task-select-helpers.ts b/apps/web/components/task/task-select-helpers.ts index f4d42e4b22..9b73f44de7 100644 --- a/apps/web/components/task/task-select-helpers.ts +++ b/apps/web/components/task/task-select-helpers.ts @@ -35,6 +35,28 @@ export function resolveLoadedSessionId( ); } +/** + * Pick the session to re-open when the user navigates back to a task. + * + * Prefers the user's last-selected session (tracked per task in + * `lastSessionByTaskId`) over `primarySessionId`, so opening a non-primary + * tab then bouncing through another task does not silently snap the user + * back to primary. Falls back to `primarySessionId` when the remembered + * session is unknown / missing an env mapping (e.g. it was deleted). + */ +export function resolvePreferredSessionId( + taskId: string, + primarySessionId: string, + lastSessionByTaskId: Record, + environmentIdBySessionId: Record, +): string { + const last = lastSessionByTaskId[taskId]; + if (last && environmentIdBySessionId[last]) { + return last; + } + return primarySessionId; +} + export function buildSwitchToSession( store: StoreApi, setActiveSession: (taskId: string, sessionId: string) => void, @@ -113,18 +135,24 @@ export function selectTaskWithLayout(params: { setPreparingTaskId: (id: string | null) => void; }): void { const { taskId, task, store, switchToSession, loadTaskSessionsForTask } = params; - const oldSessionId = store.getState().tasks.activeSessionId; + const state = store.getState(); + const oldSessionId = state.tasks.activeSessionId; if (task?.primarySessionId) { - const primarySessionId = task.primarySessionId; - const hasEnvId = !!store.getState().environmentIdBySessionId[primarySessionId]; + const targetSessionId = resolvePreferredSessionId( + taskId, + task.primarySessionId, + state.tasks.lastSessionByTaskId, + state.environmentIdBySessionId, + ); + const hasEnvId = !!state.environmentIdBySessionId[targetSessionId]; if (hasEnvId) { - switchToSession(taskId, primarySessionId, oldSessionId); + switchToSession(taskId, targetSessionId, oldSessionId); loadTaskSessionsForTask(taskId); replaceTaskUrl(taskId); return; } loadTaskSessionsForTask(taskId).then((sessions) => { - switchToSession(taskId, resolveLoadedSessionId(sessions, primarySessionId), oldSessionId); + switchToSession(taskId, resolveLoadedSessionId(sessions, targetSessionId), oldSessionId); replaceTaskUrl(taskId); }); return; diff --git a/apps/web/lib/ssr/mapper.ts b/apps/web/lib/ssr/mapper.ts index 78047ca32c..081f036208 100644 --- a/apps/web/lib/ssr/mapper.ts +++ b/apps/web/lib/ssr/mapper.ts @@ -89,6 +89,7 @@ export function taskToState( activeTaskId: task.id, activeSessionId: resolvedSessionId, pinnedSessionId: null, + lastSessionByTaskId: resolvedSessionId ? { [task.id]: resolvedSessionId } : {}, }, messages: resolvedSessionId && messages diff --git a/apps/web/lib/state/slices/kanban/kanban-slice.ts b/apps/web/lib/state/slices/kanban/kanban-slice.ts index 5c4adfd741..b836eface3 100644 --- a/apps/web/lib/state/slices/kanban/kanban-slice.ts +++ b/apps/web/lib/state/slices/kanban/kanban-slice.ts @@ -5,7 +5,12 @@ export const defaultKanbanState: KanbanSliceState = { kanban: { workflowId: null, steps: [], tasks: [] }, kanbanMulti: { snapshots: {}, isLoading: false }, workflows: { items: [], activeId: null }, - tasks: { activeTaskId: null, activeSessionId: null, pinnedSessionId: null }, + tasks: { + activeTaskId: null, + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, }; export const createKanbanSlice: StateCreator< @@ -49,12 +54,14 @@ export const createKanbanSlice: StateCreator< draft.tasks.activeSessionId = sessionId; // User-initiated selection: pin so WS auto-replace handoff respects it. draft.tasks.pinnedSessionId = sessionId; + draft.tasks.lastSessionByTaskId[taskId] = sessionId; }), setActiveSessionAuto: (taskId, sessionId) => set((draft) => { draft.tasks.activeTaskId = taskId; draft.tasks.activeSessionId = sessionId; // Auto-driven (WS) selection: don't touch pinnedSessionId. + draft.tasks.lastSessionByTaskId[taskId] = sessionId; }), clearActiveSession: () => set((draft) => { diff --git a/apps/web/lib/state/slices/kanban/types.ts b/apps/web/lib/state/slices/kanban/types.ts index ef7b9a81ba..ba170c9157 100644 --- a/apps/web/lib/state/slices/kanban/types.ts +++ b/apps/web/lib/state/slices/kanban/types.ts @@ -107,6 +107,11 @@ export type TaskState = { // pinnedSessionId alone — and skip auto-replace when the terminating session // matches the pin (the user wants to stay even though the workflow moved on). pinnedSessionId: string | null; + // lastSessionByTaskId remembers the most-recent active session for each task. + // Unlike pinnedSessionId (single global slot, cleared on task change), this + // map survives task switches so navigating back to a task can restore the + // user's last-selected session instead of always jumping to primary. + lastSessionByTaskId: Record; }; export type KanbanSliceState = { diff --git a/apps/web/lib/ws/handlers/agent-session-pure.test.ts b/apps/web/lib/ws/handlers/agent-session-pure.test.ts new file mode 100644 index 0000000000..f8dde95bff --- /dev/null +++ b/apps/web/lib/ws/handlers/agent-session-pure.test.ts @@ -0,0 +1,155 @@ +import { describe, it, expect } from "vitest"; +import { + isTerminalSessionState, + pickReplacementSessionId, + shouldAdoptNewSession, +} from "./agent-session"; +import type { AppState } from "@/lib/state/store"; +import type { TaskSessionState } from "@/lib/types/http"; + +function makeAppState(partial: Partial): AppState { + return { + tasks: { + activeTaskId: null, + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + taskSessions: { items: {} }, + taskSessionsByTask: { itemsByTaskId: {} }, + ...partial, + } as unknown as AppState; +} + +describe("isTerminalSessionState", () => { + it.each<[TaskSessionState | undefined, boolean]>([ + ["COMPLETED", true], + ["FAILED", true], + ["CANCELLED", true], + ["RUNNING", false], + ["STARTING", false], + ["CREATED", false], + ["WAITING_FOR_INPUT", false], + [undefined, false], + ])("returns %o → %s", (input, expected) => { + expect(isTerminalSessionState(input)).toBe(expected); + }); +}); + +describe("shouldAdoptNewSession", () => { + it("adopts when there is no active session for the task", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + }); + expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); + }); + + it("adopts when active session belongs to a different task", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-other", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + taskSessions: { + items: { "s-other": { id: "s-other", task_id: "t-2", state: "RUNNING" } }, + } as unknown as AppState["taskSessions"], + }); + expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); + }); + + it("adopts when active session is already terminal", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + taskSessions: { + items: { "s-old": { id: "s-old", task_id: "t-1", state: "COMPLETED" } }, + } as unknown as AppState["taskSessions"], + }); + expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); + }); + + it("does NOT adopt while the current active session is still running", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + taskSessions: { + items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, + } as unknown as AppState["taskSessions"], + }); + expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(false); + }); + + it("does NOT adopt when the event is for a non-active task", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + }); + expect(shouldAdoptNewSession(state, "t-2", "STARTING")).toBe(false); + }); + + it("does NOT adopt terminal state events", () => { + const state = makeAppState({ + tasks: { + activeTaskId: "t-1", + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, + }); + expect(shouldAdoptNewSession(state, "t-1", "COMPLETED")).toBe(false); + }); +}); + +describe("pickReplacementSessionId", () => { + it("returns the newest non-terminal session in the per-task list", () => { + const state = makeAppState({ + taskSessionsByTask: { + itemsByTaskId: { + "t-1": [ + { id: "s-1", task_id: "t-1", state: "COMPLETED", started_at: "", updated_at: "" }, + { id: "s-2", task_id: "t-1", state: "RUNNING", started_at: "", updated_at: "" }, + { id: "s-3", task_id: "t-1", state: "CANCELLED", started_at: "", updated_at: "" }, + ], + }, + } as unknown as AppState["taskSessionsByTask"], + }); + expect(pickReplacementSessionId(state, "t-1")).toBe("s-2"); + }); + + it("returns null when all sessions are terminal", () => { + const state = makeAppState({ + taskSessionsByTask: { + itemsByTaskId: { + "t-1": [ + { id: "s-1", task_id: "t-1", state: "COMPLETED", started_at: "", updated_at: "" }, + { id: "s-2", task_id: "t-1", state: "FAILED", started_at: "", updated_at: "" }, + ], + }, + } as unknown as AppState["taskSessionsByTask"], + }); + expect(pickReplacementSessionId(state, "t-1")).toBeNull(); + }); + + it("returns null when the task has no sessions tracked", () => { + expect(pickReplacementSessionId(makeAppState({}), "t-missing")).toBeNull(); + }); +}); diff --git a/apps/web/lib/ws/handlers/agent-session.test.ts b/apps/web/lib/ws/handlers/agent-session.test.ts index f869add3cd..ab4b09dfae 100644 --- a/apps/web/lib/ws/handlers/agent-session.test.ts +++ b/apps/web/lib/ws/handlers/agent-session.test.ts @@ -1,17 +1,16 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { - isTerminalSessionState, - pickReplacementSessionId, - registerTaskSessionHandlers, - shouldAdoptNewSession, -} from "./agent-session"; +import { registerTaskSessionHandlers } from "./agent-session"; import type { StoreApi } from "zustand"; import type { AppState } from "@/lib/state/store"; -import type { TaskSessionState } from "@/lib/types/http"; function makeStore(overrides: Record = {}) { const state: Record = { - tasks: { activeTaskId: null, activeSessionId: null, pinnedSessionId: null }, + tasks: { + activeTaskId: null, + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessions: { items: {} }, taskSessionsByTask: { itemsByTaskId: {} }, setTaskSession: vi.fn(), @@ -132,118 +131,6 @@ describe("session.state_changed handler", () => { }); }); -function makeAppState(partial: Partial): AppState { - return { - tasks: { activeTaskId: null, activeSessionId: null, pinnedSessionId: null }, - taskSessions: { items: {} }, - taskSessionsByTask: { itemsByTaskId: {} }, - ...partial, - } as unknown as AppState; -} - -describe("isTerminalSessionState", () => { - it.each<[TaskSessionState | undefined, boolean]>([ - ["COMPLETED", true], - ["FAILED", true], - ["CANCELLED", true], - ["RUNNING", false], - ["STARTING", false], - ["CREATED", false], - ["WAITING_FOR_INPUT", false], - [undefined, false], - ])("returns %o → %s", (input, expected) => { - expect(isTerminalSessionState(input)).toBe(expected); - }); -}); - -describe("shouldAdoptNewSession", () => { - it("adopts when there is no active session for the task", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: null, pinnedSessionId: null }, - }); - expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); - }); - - it("adopts when active session belongs to a different task", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-other", pinnedSessionId: null }, - taskSessions: { - items: { "s-other": { id: "s-other", task_id: "t-2", state: "RUNNING" } }, - } as unknown as AppState["taskSessions"], - }); - expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); - }); - - it("adopts when active session is already terminal", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, - taskSessions: { - items: { "s-old": { id: "s-old", task_id: "t-1", state: "COMPLETED" } }, - } as unknown as AppState["taskSessions"], - }); - expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(true); - }); - - it("does NOT adopt while the current active session is still running", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, - taskSessions: { - items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, - } as unknown as AppState["taskSessions"], - }); - expect(shouldAdoptNewSession(state, "t-1", "STARTING")).toBe(false); - }); - - it("does NOT adopt when the event is for a non-active task", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: null, pinnedSessionId: null }, - }); - expect(shouldAdoptNewSession(state, "t-2", "STARTING")).toBe(false); - }); - - it("does NOT adopt terminal state events", () => { - const state = makeAppState({ - tasks: { activeTaskId: "t-1", activeSessionId: null, pinnedSessionId: null }, - }); - expect(shouldAdoptNewSession(state, "t-1", "COMPLETED")).toBe(false); - }); -}); - -describe("pickReplacementSessionId", () => { - it("returns the newest non-terminal session in the per-task list", () => { - const state = makeAppState({ - taskSessionsByTask: { - itemsByTaskId: { - "t-1": [ - { id: "s-1", task_id: "t-1", state: "COMPLETED", started_at: "", updated_at: "" }, - { id: "s-2", task_id: "t-1", state: "RUNNING", started_at: "", updated_at: "" }, - { id: "s-3", task_id: "t-1", state: "CANCELLED", started_at: "", updated_at: "" }, - ], - }, - } as unknown as AppState["taskSessionsByTask"], - }); - expect(pickReplacementSessionId(state, "t-1")).toBe("s-2"); - }); - - it("returns null when all sessions are terminal", () => { - const state = makeAppState({ - taskSessionsByTask: { - itemsByTaskId: { - "t-1": [ - { id: "s-1", task_id: "t-1", state: "COMPLETED", started_at: "", updated_at: "" }, - { id: "s-2", task_id: "t-1", state: "FAILED", started_at: "", updated_at: "" }, - ], - }, - } as unknown as AppState["taskSessionsByTask"], - }); - expect(pickReplacementSessionId(state, "t-1")).toBeNull(); - }); - - it("returns null when the task has no sessions tracked", () => { - expect(pickReplacementSessionId(makeAppState({}), "t-missing")).toBeNull(); - }); -}); - describe("session.state_changed → active session switching", () => { beforeEach(() => { vi.clearAllMocks(); @@ -251,7 +138,12 @@ describe("session.state_changed → active session switching", () => { it("adopts a newly-created session for the active task", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: null, pinnedSessionId: null }, + tasks: { + activeTaskId: "t-1", + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, }); const handler = registerTaskSessionHandlers(store)[STATE_CHANGED_EVENT]!; @@ -268,7 +160,12 @@ describe("session.state_changed → active session switching", () => { it("does not adopt a new session for a task that is not active", () => { const store = makeStore({ - tasks: { activeTaskId: "other-task", activeSessionId: null, pinnedSessionId: null }, + tasks: { + activeTaskId: "other-task", + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, }); const handler = registerTaskSessionHandlers(store)[STATE_CHANGED_EVENT]!; @@ -284,7 +181,12 @@ describe("session.state_changed → active session switching", () => { it("does not adopt while the current active session is still running", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, }, @@ -310,7 +212,12 @@ describe("session.state_changed → active session switching", () => { // this path too. it("does not yank a pinned session on reverse event ordering (old COMPLETED, then new STARTING)", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: "s-old" }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: "s-old", + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "COMPLETED" } }, }, @@ -335,7 +242,12 @@ describe("session.state_changed → active session handoff on terminal", () => { it("hands off when the current active session transitions to terminal", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, }, @@ -367,7 +279,12 @@ describe("session.state_changed → active session handoff on terminal", () => { // to the same session that just became terminal. it("does not hand off when the only candidate is the terminating session itself", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, }, @@ -393,7 +310,12 @@ describe("session.state_changed → active session handoff on terminal", () => { it("does not hand off when all other sessions for the task are terminal", () => { const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, }, @@ -428,7 +350,12 @@ describe("session.state_changed → respects user-pinned session", () => { // User manually clicked s-old, so pinnedSessionId === "s-old". // When s-old terminates we should respect the pin and stay on it. const store = makeStore({ - tasks: { activeTaskId: "t-1", activeSessionId: "s-old", pinnedSessionId: "s-old" }, + tasks: { + activeTaskId: "t-1", + activeSessionId: "s-old", + pinnedSessionId: "s-old", + lastSessionByTaskId: {}, + }, taskSessions: { items: { "s-old": { id: "s-old", task_id: "t-1", state: "RUNNING" } }, }, diff --git a/apps/web/lib/ws/handlers/tasks.test.ts b/apps/web/lib/ws/handlers/tasks.test.ts index 257627834b..866200c9c0 100644 --- a/apps/web/lib/ws/handlers/tasks.test.ts +++ b/apps/web/lib/ws/handlers/tasks.test.ts @@ -8,6 +8,8 @@ vi.mock("@/lib/recent-tasks", () => ({ removeRecentTask: vi.fn(), })); +const SESS_OTHER = "sess-other"; + type Listener = (state: AppState) => void; /** @@ -19,7 +21,12 @@ function makeStore(initial: Partial = {}) { let state = { kanban: { workflowId: "wf1", steps: [], tasks: [] }, kanbanMulti: { snapshots: {}, isLoading: false }, - tasks: { activeTaskId: null, activeSessionId: null, pinnedSessionId: null }, + tasks: { + activeTaskId: null, + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, taskSessionsByTask: { itemsByTaskId: {}, loadedByTaskId: {}, loadingByTaskId: {} }, environmentIdBySessionId: {}, setActiveSession: vi.fn((taskId: string, sessionId: string | null) => { @@ -29,6 +36,9 @@ function makeStore(initial: Partial = {}) { activeTaskId: taskId, activeSessionId: sessionId, pinnedSessionId: sessionId, + lastSessionByTaskId: sessionId + ? { ...state.tasks.lastSessionByTaskId, [taskId]: sessionId } + : state.tasks.lastSessionByTaskId, }, }; }), @@ -111,7 +121,12 @@ describe("task.updated primary-session focus follow", () => { steps: [], tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], } as unknown as AppState["kanban"], - tasks: { activeTaskId: "t1", activeSessionId: "sess-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t1", + activeSessionId: "sess-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, setActiveSessionAuto, }); @@ -130,7 +145,12 @@ describe("task.updated primary-session focus follow", () => { steps: [], tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], } as unknown as AppState["kanban"], - tasks: { activeTaskId: "t1", activeSessionId: "sess-other", pinnedSessionId: "sess-other" }, + tasks: { + activeTaskId: "t1", + activeSessionId: SESS_OTHER, + pinnedSessionId: SESS_OTHER, + lastSessionByTaskId: {}, + }, setActiveSessionAuto, }); @@ -147,7 +167,12 @@ describe("task.updated primary-session focus follow", () => { steps: [], tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], } as unknown as AppState["kanban"], - tasks: { activeTaskId: "t2", activeSessionId: "sess-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t2", + activeSessionId: "sess-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, setActiveSessionAuto, }); @@ -164,7 +189,12 @@ describe("task.updated primary-session focus follow", () => { steps: [], tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], } as unknown as AppState["kanban"], - tasks: { activeTaskId: "t1", activeSessionId: "sess-old", pinnedSessionId: null }, + tasks: { + activeTaskId: "t1", + activeSessionId: "sess-old", + pinnedSessionId: null, + lastSessionByTaskId: {}, + }, setActiveSessionAuto, }); @@ -173,11 +203,21 @@ describe("task.updated primary-session focus follow", () => { expect(setActiveSessionAuto).not.toHaveBeenCalled(); }); +}); + +// Regression: even when the user happens to be sitting on the previous +// primary, an explicit pin on it must override primary-follow-focus — +// otherwise a workflow profile switch silently yanks them off the session +// they deliberately clicked into. +describe("task.updated primary-session focus follow (pinning)", () => { + let store: ReturnType; + let setActiveSessionAuto: ReturnType; + + beforeEach(() => { + setActiveSessionAuto = vi.fn(); + vi.mocked(removeRecentTask).mockClear(); + }); - // Regression: even when the user happens to be sitting on the previous - // primary, an explicit pin on it must override primary-follow-focus — - // otherwise a workflow profile switch silently yanks them off the session - // they deliberately clicked into. it("does NOT follow focus when the user has pinned the previous primary", () => { store = makeStore({ kanban: { @@ -185,7 +225,12 @@ describe("task.updated primary-session focus follow", () => { steps: [], tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], } as unknown as AppState["kanban"], - tasks: { activeTaskId: "t1", activeSessionId: "sess-old", pinnedSessionId: "sess-old" }, + tasks: { + activeTaskId: "t1", + activeSessionId: "sess-old", + pinnedSessionId: "sess-old", + lastSessionByTaskId: {}, + }, setActiveSessionAuto, }); @@ -250,4 +295,28 @@ describe("task.deleted cleanup", () => { expect(removeRecentTask).toHaveBeenCalledTimes(1); expect(removeRecentTask).toHaveBeenCalledWith("t1"); }); + + it("removes the deleted task from lastSessionByTaskId", () => { + const store = makeStore({ + kanban: { + workflowId: "wf1", + steps: [], + tasks: [{ id: "t1", primarySessionId: "sess-old", workflowId: "wf1" }], + } as unknown as AppState["kanban"], + tasks: { + activeTaskId: null, + activeSessionId: null, + pinnedSessionId: null, + lastSessionByTaskId: { t1: "sess-old", t2: SESS_OTHER }, + }, + environmentIdBySessionId: {}, + }); + + const handlers = registerTasksHandlers(store); + handlers["task.deleted"]!(makeDeletedMessage({ task_id: "t1", workflow_id: "wf1" })); + + const state = store.getState(); + expect(state.tasks.lastSessionByTaskId).not.toHaveProperty("t1"); + expect(state.tasks.lastSessionByTaskId).toHaveProperty("t2", SESS_OTHER); + }); }); diff --git a/apps/web/lib/ws/handlers/tasks.ts b/apps/web/lib/ws/handlers/tasks.ts index a0e76d56c6..5a0931407b 100644 --- a/apps/web/lib/ws/handlers/tasks.ts +++ b/apps/web/lib/ws/handlers/tasks.ts @@ -197,6 +197,10 @@ export function registerTasksHandlers(store: StoreApi): WsHandlers { if (isActive) { next = { ...next, tasks: { ...next.tasks, activeTaskId: null, activeSessionId: null } }; } + if (next.tasks.lastSessionByTaskId[deletedId]) { + const { [deletedId]: _, ...rest } = next.tasks.lastSessionByTaskId; + next = { ...next, tasks: { ...next.tasks, lastSessionByTaskId: rest } }; + } return next; }); },