diff --git a/apps/web/components/task/task-page-content-helpers.test.ts b/apps/web/components/task/task-page-content-helpers.test.ts index 03a87d08a4..f90811f74b 100644 --- a/apps/web/components/task/task-page-content-helpers.test.ts +++ b/apps/web/components/task/task-page-content-helpers.test.ts @@ -198,14 +198,17 @@ describe("syncActiveTaskSession", () => { const setActiveSessionAuto = vi.fn(); const setActiveTask = vi.fn(); - syncActiveTaskSession({ + const applied = syncActiveTaskSession({ initialTaskId: "task-1", fallbackTaskId: null, initialSessionId: "session-1", + activeTaskId: null, + previousRouteTaskId: undefined, setActiveSessionAuto, setActiveTask, }); + expect(applied).toBe(true); expect(setActiveSessionAuto).toHaveBeenCalledWith("task-1", "session-1"); expect(setActiveTask).not.toHaveBeenCalled(); }); @@ -214,17 +217,76 @@ describe("syncActiveTaskSession", () => { const setActiveSessionAuto = vi.fn(); const setActiveTask = vi.fn(); - syncActiveTaskSession({ + const applied = syncActiveTaskSession({ initialTaskId: "task-1", fallbackTaskId: null, initialSessionId: null, + activeTaskId: null, + previousRouteTaskId: undefined, setActiveSessionAuto, setActiveTask, }); + expect(applied).toBe(true); expect(setActiveTask).toHaveBeenCalledWith("task-1"); expect(setActiveSessionAuto).not.toHaveBeenCalled(); }); + + it("applies a changed route over the previous active task", () => { + const setActiveSessionAuto = vi.fn(); + const setActiveTask = vi.fn(); + + const applied = syncActiveTaskSession({ + initialTaskId: "task-2", + fallbackTaskId: null, + initialSessionId: "session-2", + activeTaskId: "task-1", + previousRouteTaskId: "task-1", + setActiveSessionAuto, + setActiveTask, + }); + + expect(applied).toBe(true); + expect(setActiveSessionAuto).toHaveBeenCalledWith("task-2", "session-2"); + expect(setActiveTask).not.toHaveBeenCalled(); + }); + + it("adopts a session that arrives for the current route", () => { + const setActiveSessionAuto = vi.fn(); + const setActiveTask = vi.fn(); + + const applied = syncActiveTaskSession({ + initialTaskId: "task-1", + fallbackTaskId: null, + initialSessionId: "session-1", + activeTaskId: "task-1", + previousRouteTaskId: "task-1", + setActiveSessionAuto, + setActiveTask, + }); + + expect(applied).toBe(true); + expect(setActiveSessionAuto).toHaveBeenCalledWith("task-1", "session-1"); + expect(setActiveTask).not.toHaveBeenCalled(); + }); + + it("does not restore an unchanged route over an in-place sibling selection", () => { + const setActiveSessionAuto = vi.fn(); + const setActiveTask = vi.fn(); + const applied = syncActiveTaskSession({ + initialTaskId: "missing-task", + fallbackTaskId: null, + initialSessionId: "sibling-session", + activeTaskId: "sibling-task", + previousRouteTaskId: "missing-task", + setActiveSessionAuto, + setActiveTask, + }); + + expect(applied).toBe(false); + expect(setActiveSessionAuto).not.toHaveBeenCalled(); + expect(setActiveTask).not.toHaveBeenCalled(); + }); }); describe("resolveEffectiveTask archived state", () => { diff --git a/apps/web/components/task/task-page-content-helpers.ts b/apps/web/components/task/task-page-content-helpers.ts index 6754b70cff..2b05d8d52d 100644 --- a/apps/web/components/task/task-page-content-helpers.ts +++ b/apps/web/components/task/task-page-content-helpers.ts @@ -201,13 +201,18 @@ export function syncActiveTaskSession(params: { initialTaskId: string | undefined; fallbackTaskId: string | null | undefined; initialSessionId: string | null; + activeTaskId: string | null; + previousRouteTaskId: string | null | undefined; setActiveSessionAuto: (taskId: string, sessionId: string) => void; setActiveTask: (taskId: string) => void; -}) { +}): boolean { const taskId = params.initialTaskId ?? params.fallbackTaskId; - if (!taskId) return; + if (!taskId) return false; + const routeChanged = params.previousRouteTaskId !== taskId; + if (!routeChanged && params.activeTaskId !== taskId) return false; if (params.initialSessionId) params.setActiveSessionAuto(taskId, params.initialSessionId); else params.setActiveTask(taskId); + return true; } export function resolveTaskIds(task: Task | null) { diff --git a/apps/web/components/task/task-page-content.tsx b/apps/web/components/task/task-page-content.tsx index e1966d3291..ae93294cf6 100644 --- a/apps/web/components/task/task-page-content.tsx +++ b/apps/web/components/task/task-page-content.tsx @@ -260,6 +260,12 @@ function useTaskPageData( const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); const setActiveSessionAuto = useAppStore((state) => state.setActiveSessionAuto); const setActiveTask = useAppStore((state) => state.setActiveTask); + const previousRouteTaskId = useRef(undefined); + // Route synchronization must see the current selection when route data or + // delayed session hydration changes, but a sidebar selection must not + // retrigger it and overwrite the session chosen by task selection. + const activeTaskIdForSyncRef = useRef(activeTaskId); + activeTaskIdForSyncRef.current = activeTaskId; // Validate that activeSessionId belongs to activeTaskId to prevent showing // messages from an unrelated session when navigating to a task without sessions. @@ -282,13 +288,16 @@ function useTaskPageData( const effectiveSessionId = validatedActiveSessionId ?? initialSessionId; useEffect(() => { - syncActiveTaskSession({ + const applied = syncActiveTaskSession({ initialTaskId: initialTask?.id, fallbackTaskId, initialSessionId, + activeTaskId: activeTaskIdForSyncRef.current, + previousRouteTaskId: previousRouteTaskId.current, setActiveSessionAuto, setActiveTask, }); + if (applied) previousRouteTaskId.current = initialTask?.id ?? fallbackTaskId; }, [initialTask?.id, fallbackTaskId, initialSessionId, setActiveSessionAuto, setActiveTask]); const { repositories } = useRepositories(task?.workspace_id ?? null, Boolean(task?.workspace_id)); diff --git a/apps/web/e2e/tests/task/task-loading-state.spec.ts b/apps/web/e2e/tests/task/task-loading-state.spec.ts index b4ac723d64..4663bc9d86 100644 --- a/apps/web/e2e/tests/task/task-loading-state.spec.ts +++ b/apps/web/e2e/tests/task/task-loading-state.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import { SidebarTasksPage } from "../../pages/sidebar-tasks-page"; +import { SessionPage } from "../../pages/session-page"; import { openBlockedTaskLoadingState } from "./task-loading-state-helpers"; test.describe("Task loading state", () => { @@ -47,4 +48,131 @@ test.describe("Task loading state", () => { await expect(testPage).toHaveURL(new RegExp(`/t/${sibling.id}$`)); await expect(testPage.getByTestId("task-load-error-state")).toBeHidden({ timeout: 10_000 }); }); + + test("keeps a session-backed sibling active", async ({ testPage, apiClient, seedData }) => { + const sibling = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Missing route session sibling", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + if (!sibling.session_id) throw new Error("session-backed sibling has no session"); + + await testPage.goto(`/t/missing-task-route-session?workspaceId=${seedData.workspaceId}`); + await expect(testPage.getByTestId("task-load-error-state")).toBeVisible({ timeout: 10_000 }); + + const sidebar = new SidebarTasksPage(testPage); + await expect(sidebar.row(sibling.id)).toBeVisible({ timeout: 10_000 }); + await sidebar.plainClick(sibling.id); + + await expect(testPage).toHaveURL(new RegExp(`/t/${sibling.id}$`)); + await expect(testPage.getByTestId("task-load-error-state")).toBeHidden({ timeout: 10_000 }); + await expect(testPage.getByTestId("dockview-task-layout")).toBeVisible({ timeout: 10_000 }); + await expect(sidebar.row(sibling.id)).toHaveAttribute("data-active", "true", { + timeout: 10_000, + }); + }); + + test("restores the last selected session after an A to B to A switch", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + + const taskA = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Session route sync task A", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + const taskB = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Session route sync task B", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + if (!taskA.session_id || !taskB.session_id) { + throw new Error("session-backed route sync tasks have no primary session"); + } + + await testPage.goto(`/t/${taskA.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + await session.openNewSessionDialog(); + await expect(session.newSessionDialog()).toBeVisible({ timeout: 5_000 }); + await session.newSessionPromptInput().fill("/e2e:simple-message"); + await session.newSessionStartButton().click(); + await expect(session.newSessionDialog()).not.toBeVisible({ timeout: 10_000 }); + + let secondarySessionId: string | undefined; + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(taskA.id); + secondarySessionId = sessions.find((item) => item.id !== taskA.session_id)?.id; + return secondarySessionId ?? null; + }, + { timeout: 30_000, message: "Waiting for task A secondary session" }, + ) + .toBeTruthy(); + + await expect(session.sessionTabBySessionId(secondarySessionId!)).toBeVisible({ + timeout: 15_000, + }); + await session.sessionTabBySessionId(secondarySessionId!).click(); + await expect + .poll(async () => + testPage.evaluate( + () => + ( + window as unknown as { + __KANDEV_E2E_STORE__?: { getState: () => { tasks: { activeSessionId: string } } }; + } + ).__KANDEV_E2E_STORE__?.getState().tasks.activeSessionId, + ), + ) + .toBe(secondarySessionId); + + await session.clickTaskInSidebar("Session route sync task B"); + await expect(testPage).toHaveURL(new RegExp(`/t/${taskB.id}$`), { timeout: 15_000 }); + await session.waitForLoad(); + await expect(session.sessionTabBySessionId(taskB.session_id)).toBeVisible({ + timeout: 15_000, + }); + + await session.clickTaskInSidebar("Session route sync task A"); + await expect(testPage).toHaveURL(new RegExp(`/t/${taskA.id}$`), { timeout: 15_000 }); + await session.waitForLoad(); + await expect + .poll(async () => + testPage.evaluate( + () => + ( + window as unknown as { + __KANDEV_E2E_STORE__?: { getState: () => { tasks: { activeSessionId: string } } }; + } + ).__KANDEV_E2E_STORE__?.getState().tasks.activeSessionId, + ), + ) + .toBe(secondarySessionId); + }); }); diff --git a/docs/plans/missing-task-session-navigation/plan.md b/docs/plans/missing-task-session-navigation/plan.md new file mode 100644 index 0000000000..2c126ff01e --- /dev/null +++ b/docs/plans/missing-task-session-navigation/plan.md @@ -0,0 +1,125 @@ +--- +spec: docs/specs/missing-task-route-recovery/spec.md +created: 2026-08-12 +status: complete +--- + +# Implementation Plan: Missing Task Session Navigation + +## Overview + +Guard route initialization so that stale route props cannot replace a later sidebar selection. Add unit coverage for the guard and a session-backed browser regression. + +## Root cause + +The missing route keeps its task ID in `TaskPageContent` after an in-place sidebar switch. The sidebar updates the store and URL without a route remount. + +A loaded sibling session then changes `agent.taskSessionId`. This change runs the route synchronization effect again. The effect pairs the sibling session with the missing route ID and restores the error state. + +The current browser test creates a sibling without a session. That setup does not enter the failing fast path. + +## Frontend + +### Route-to-store synchronization + +- Extend `syncActiveTaskSession` with `activeTaskId` and `previousRouteTaskId` inputs. +- Change its return type to `boolean`. The value reports whether the function applied the route state. + +```typescript +export function syncActiveTaskSession(params: { + initialTaskId: string | undefined; + fallbackTaskId: string | null | undefined; + initialSessionId: string | null; + activeTaskId: string | null; + previousRouteTaskId: string | null | undefined; + setActiveSessionAuto: (taskId: string, sessionId: string) => void; + setActiveTask: (taskId: string) => void; +}): boolean; +``` + +- Return `true` for the first route initialization and for a changed route task ID. +- Return `true` when the active store task still matches the unchanged route. This rule permits delayed session adoption for the current route. +- Return `false` when the route ID is unchanged and the active store task differs. This rule preserves an in-place sidebar selection. +- Track the prior route task ID with `useRef(undefined)` in `useTaskPageData`. +- Update the ref only when `syncActiveTaskSession` returns `true`. +- Read the latest active task ID through a ref, but do not include it as a + reactive effect dependency. Sidebar task selection owns preferred-session + restoration and must not be followed by route synchronization that reapplies + the route's primary session. + +### URL and Dockview boundaries + +- Keep `replaceTaskUrl` as a non-navigating URL update. +- Keep the existing Dockview environment switch and session selection paths. +- Do not add a route remount or a hard refresh. Both changes can discard the fast workbench transition. + +### Mobile design contract + +- The change only guards shared task and session state. It does not change layout, controls, touch behavior, or scrolling. +- The phone error state keeps the existing task-overview action. +- The existing phone scenario continues to cover recovery from an unavailable route. +- No new phone browser test is necessary for this desktop-sidebar path. + +## Tests + +- **What:** The first route initialization can select its route task. +- **File:** `apps/web/components/task/task-page-content-helpers.test.ts`. +- **How:** Call `syncActiveTaskSession` with no prior route task ID. + +- **What:** A changed route task ID can replace a stale active task. +- **File:** `apps/web/components/task/task-page-content-helpers.test.ts`. +- **How:** Call `syncActiveTaskSession` with different prior and current route task IDs. + +- **What:** The current route can adopt a session that becomes available later. +- **File:** `apps/web/components/task/task-page-content-helpers.test.ts`. +- **How:** Use the same route and active task ID. + +- **What:** An unchanged stale route cannot replace an in-place sibling selection. +- **File:** `apps/web/components/task/task-page-content-helpers.test.ts`. +- **How:** Use the same route ID and a different active store task ID. This assertion must fail before the production change. + +## E2E Tests + +- **Scenario:** Given a missing task route and a sibling with a loaded primary session, selecting the sibling keeps its workbench active. +- **File:** `apps/web/e2e/tests/task/task-loading-state.spec.ts`. +- **Setup:** Create the sibling with `createTaskWithAgent` and the existing mock-agent profile. +- **What to validate:** The URL selects the sibling. The error state disappears. The Dockview workbench appears. The sibling row stays active after session updates. +- Keep the existing no-session sibling scenario. It covers the prepare-session path. +- **Scenario:** Given task A has a primary and a non-primary session, switching + A → B → A preserves the selected non-primary session. +- **What to validate:** The route returns to A while the active-session store + value remains the non-primary session. + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [Task 01: Guard missing-route task synchronization](task-01-guard-route-task-sync.md) + +The task is sequential. The unit guard and browser regression describe one state transition. + +## Risks and out of scope + +- The guard must still let a real route change replace the active task. +- The guard must still let the current route adopt a session that arrives later. +- A ref must distinguish the first route synchronization from an unchanged missing route. +- Changes to `replaceTaskUrl`, Dockview layouts, backend task lookup, and archive behavior are out of scope. +- The repair adds no API, WebSocket, persistence, permission, or user-interface contract. + +## Verification Results + +- RED browser run: the focused session-backed sibling E2E reproduced the visible + task error state after sibling selection. +- RED unit run: the stale-route case failed because `syncActiveTaskSession` returned + `undefined` and re-applied stale route state. +- GREEN unit run: the focused helper suite passed, 22 tests. +- GREEN browser run: the focused production-build E2E passed, 1 test. +- `pnpm run typecheck` passed. +- Targeted Prettier check passed. +- Targeted ESLint passed. +- Review fixup RED E2E: the A → B → A secondary-session regression returned + task A's primary session after returning from task B. +- Review fixup GREEN E2E: the same production-build regression passed after + removing the active-task-only effect trigger and reading the current active + task from a ref. +- Full task-loading-state production-build E2E passed, 4 tests. diff --git a/docs/plans/missing-task-session-navigation/task-01-guard-route-task-sync.md b/docs/plans/missing-task-session-navigation/task-01-guard-route-task-sync.md new file mode 100644 index 0000000000..5b2b887c5c --- /dev/null +++ b/docs/plans/missing-task-session-navigation/task-01-guard-route-task-sync.md @@ -0,0 +1,104 @@ +--- +id: "01-guard-route-task-sync" +title: "Guard missing-route task synchronization" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/missing-task-route-recovery/spec.md" +--- + +# Task 01: Guard Missing-Route Task Synchronization + +Prevent an unchanged missing route from replacing a later session-backed sidebar selection. + +## Acceptance + +- Route initialization and real route changes still select the route task and its available session. +- An unchanged stale route cannot pair its task ID with a session from the active sibling task. +- The session-backed browser regression keeps the sibling URL, active row, and Dockview workbench after session updates. +- Returning from another task preserves a deliberately selected non-primary session. + +## TDD sequence + +1. Add the session-backed browser scenario. Run it before production changes and record the expected error-state failure. +2. Add a stale-route unit case to `syncActiveTaskSession`. Record the expected setter assertion failure. +3. Extend the helper inputs and add the minimum effect guard that passes the unit cases. +4. Run the focused unit cases and web typecheck. +5. Rebuild the production web assets through the managed E2E command. Run the browser scenario again. +6. Run the A to B to A secondary-session regression before closing review fixup. + +## Verification + +If `apps/node_modules` is absent, run this command first: + +```bash +cd apps && pnpm install --frozen-lockfile +``` + +Run these commands in order: + +```bash +cd apps/web && pnpm e2e:run tests/task/task-loading-state.spec.ts -- --grep "keeps a session-backed sibling active" +cd apps && pnpm --filter @kandev/web exec vitest run components/task/task-page-content-helpers.test.ts +cd apps/web && pnpm run typecheck +cd apps && pnpm exec prettier --check web/components/task/task-page-content.tsx web/components/task/task-page-content-helpers.ts web/components/task/task-page-content-helpers.test.ts web/e2e/tests/task/task-loading-state.spec.ts +cd apps && pnpm --filter @kandev/web exec eslint components/task/task-page-content.tsx components/task/task-page-content-helpers.ts components/task/task-page-content-helpers.test.ts e2e/tests/task/task-loading-state.spec.ts +cd apps/web && pnpm e2e:run tests/task/task-loading-state.spec.ts -- --grep "keeps a session-backed sibling active" +``` + +The first browser command is the RED run. The final browser command is the GREEN run and must rebuild the production assets. + +## Files likely touched + +- `apps/web/components/task/task-page-content.tsx` +- `apps/web/components/task/task-page-content-helpers.ts` +- `apps/web/components/task/task-page-content-helpers.test.ts` +- `apps/web/e2e/tests/task/task-loading-state.spec.ts` +- `docs/plans/missing-task-session-navigation/plan.md` +- `docs/plans/missing-task-session-navigation/task-01-guard-route-task-sync.md` + +## Dependencies + +None. + +## Parallelism + +`sequential`. The unit guard and browser regression cover the same state transition. + +## Inputs + +- The session-backed regression scenario in `docs/specs/missing-task-route-recovery/spec.md`. +- The confirmed production-build failure in the current task conversation. +- `syncActiveTaskSession` and `useTaskPageData` as the current route synchronization path. +- `selectTaskWithLayout` and `replaceTaskUrl` as the current in-place switch path. +- The existing no-session test in `apps/web/e2e/tests/task/task-loading-state.spec.ts`. + +## Risks + +- An over-strict guard can block a real route change. +- An over-strict guard can block delayed session adoption for the current route. +- A URL-navigation change can remount the workbench and expand the repair scope. + +## Output contract + +Report the RED and GREEN results. Report all changed files and exact command outcomes. Update this task and `plan.md` in the same conversation. + +## Results + +- RED browser run reproduced the task-load error after selecting a + session-backed sibling from the missing route. +- RED unit run failed because the helper returned `undefined` for the stale + route case. +- GREEN unit run passed 22 focused tests. +- GREEN production-build E2E passed the session-backed sibling scenario. +- Web typecheck, targeted Prettier, and targeted ESLint passed. +- No new mobile test was needed because the repair changes shared route/store + state only. The existing mobile missing-route recovery scenario is unchanged. +- Review fixup RED production-build E2E returned task A's primary session after + an A to B to A switch. +- Review fixup GREEN production-build E2E preserved task A's selected + non-primary session. +- The synchronization effect now reads the current active task from a ref and + is not triggered by active-task changes alone. +- Full task-loading-state production-build E2E passed, 4 tests. diff --git a/docs/specs/missing-task-route-recovery/spec.md b/docs/specs/missing-task-route-recovery/spec.md index 52b3503fdd..0e94e5c0f6 100644 --- a/docs/specs/missing-task-route-recovery/spec.md +++ b/docs/specs/missing-task-route-recovery/spec.md @@ -29,6 +29,11 @@ the desktop sidebar is hidden. - When the resolved workspace contains other visible tasks, the desktop task sidebar shows them and its rows remain usable navigation targets. The invalid route ID is not synthesized as a sidebar task. +- When a visible sibling has a loaded session, selecting its sidebar row keeps + that sibling active after later session and WebSocket updates. +- When a task has multiple sessions, selecting a non-primary session, visiting + another task, and returning to the first task restores the selected session + instead of reapplying the route's primary session. - The unavailable state includes a visible action back to the task overview for the resolved workspace. The action is keyboard accessible, touch reachable, and usable when no workspace can be resolved. @@ -46,6 +51,12 @@ the desktop sidebar is hidden. the user cold-loads `/t/`, **THEN** the unavailable state appears, the desktop sidebar lists the other task instead of **No tasks yet**, and selecting that row opens the valid task. +- **GIVEN** a visible sibling with a loaded primary session, **WHEN** the user + selects it from a missing task route, **THEN** the sibling route and workbench + stay active after session hydration and WebSocket updates. +- **GIVEN** task A has a selected non-primary session, **WHEN** the user + switches to task B and returns to task A, **THEN** task A keeps that selected + session active. - **GIVEN** the active-workspace cookie and user settings identify different authorized workspaces, **WHEN** a missing task route is cold-loaded, **THEN** fallback task data comes from the cookie-selected workspace according to the @@ -81,6 +92,7 @@ the desktop sidebar is hidden. - Changing Office task-detail not-found behavior under `/office/tasks/:id`. - Retrying backend failures beyond the existing task-detail client retry. -## Implementation plan +## Implementation plans - [Missing task route recovery](../../plans/missing-task-route-recovery/plan.md) +- [Missing task session navigation repair](../../plans/missing-task-session-navigation/plan.md)