From 852a867fd5c59f5845ca1eaf0291138ebc113660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Fri, 25 Sep 2026 15:54:32 +0100 Subject: [PATCH] fix(tasks): refresh top bar after workflow migration (#3927) --- .../task/task-page-content-helpers.test.ts | 12 +- .../task/task-page-content-helpers.ts | 71 ++++- .../task/task-page-content-workflow.test.ts | 263 ++++++++++++++++++ .../task/task-page-content.test.tsx | 83 +++++- .../web/components/task/task-page-content.tsx | 66 ++--- apps/web/components/task/task-page-inner.tsx | 11 +- .../e2e/tests/task/change-workflow.spec.ts | 66 ++++- .../tests/task/mobile-change-workflow.spec.ts | 21 ++ .../kanban/use-all-workflow-snapshots.test.ts | 100 ++++++- .../kanban/use-all-workflow-snapshots.ts | 82 +++++- .../kanban/use-workflow-steps-by-id.test.ts | 36 +++ .../kanban/use-workflow-steps-by-id.ts | 6 +- apps/web/lib/state/slices/kanban/types.ts | 2 + .../workflow-migration-topbar-refresh/plan.md | 160 +++++++++++ .../task-01-open-task-workflow-placement.md | 140 ++++++++++ .../tasks/system-design/change-workflow.md | 32 +++ 16 files changed, 1081 insertions(+), 70 deletions(-) create mode 100644 apps/web/components/task/task-page-content-workflow.test.ts create mode 100644 docs/plans/workflow-migration-topbar-refresh/plan.md create mode 100644 docs/plans/workflow-migration-topbar-refresh/task-01-open-task-workflow-placement.md 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 7b3322a83d..b0fe5696a8 100644 --- a/apps/web/components/task/task-page-content-helpers.test.ts +++ b/apps/web/components/task/task-page-content-helpers.test.ts @@ -22,6 +22,7 @@ import { type KanbanTask = KanbanState["tasks"][number]; const ARCHIVED_AT = "2026-07-19T00:00:00Z"; +const FRESH_KANBAN_UPDATED_AT = "2026-07-20T00:00:00Z"; function makeArchivedTaskDetails(overrides: Partial = {}): Task { return { @@ -561,7 +562,10 @@ describe("resolveEffectiveTask archived state", () => { const enabled = resolveEffectiveTask( base, null, - makeKanbanTask({ metadata: { port_forwarding_enabled: true } }), + makeKanbanTask({ + metadata: { port_forwarding_enabled: true }, + updatedAt: FRESH_KANBAN_UPDATED_AT, + }), "task-1", ); expect(enabled?.metadata).toEqual({ port_forwarding_enabled: true }); @@ -601,7 +605,11 @@ describe("resolveEffectiveTask archived state", () => { it("prefers live kanban title/state while preserving base-only fields", () => { const taskDetails = makeArchivedTaskDetails({ archived_at: null }); - const kanbanTask = makeKanbanTask({ title: "Live title", state: "IN_PROGRESS" }); + const kanbanTask = makeKanbanTask({ + title: "Live title", + state: "IN_PROGRESS", + updatedAt: FRESH_KANBAN_UPDATED_AT, + }); const resolved = resolveEffectiveTask(taskDetails, null, kanbanTask, "task-1"); diff --git a/apps/web/components/task/task-page-content-helpers.ts b/apps/web/components/task/task-page-content-helpers.ts index 1331a21c74..e8d88fdf51 100644 --- a/apps/web/components/task/task-page-content-helpers.ts +++ b/apps/web/components/task/task-page-content-helpers.ts @@ -10,6 +10,7 @@ import type { KanbanState } from "@/lib/state/slices"; import { issueFieldsFromMetadata } from "@/lib/metadata-utils"; import { repositorySlug } from "@/lib/repository-slug"; import { remoteRepositoryBrowserUrl } from "@/lib/utils/remote-repository-browser-url"; +import { parseTurnTimestamp } from "@/lib/state/slices/session/turn-actions"; import type { TaskActionsMenuBoardRow } from "@/hooks/use-task-actions-menu"; import { useAppStore } from "@/components/state-provider"; import { findTaskInSnapshots } from "@/lib/kanban/find-task"; @@ -141,29 +142,77 @@ export function resolveEffectiveTask( return null; } +export function resolveLatestTaskProjection( + taskId: string | null, + activeTasks: KanbanState["tasks"], + snapshots: Record, +): KanbanState["tasks"][number] | null { + if (!taskId) return null; + const candidates = [activeTasks, ...Object.values(snapshots).map((snapshot) => snapshot.tasks)]; + let latestTask: KanbanState["tasks"][number] | null = null; + let latestTimestamp: bigint | null = null; + + for (const tasks of candidates) { + for (const task of tasks) { + if (task.id !== taskId) continue; + const updatedAt = parseTurnTimestamp(task.updatedAt ?? undefined); + if ( + !latestTask || + (updatedAt !== null && (latestTimestamp === null || updatedAt > latestTimestamp)) + ) { + latestTask = task; + latestTimestamp = updatedAt; + } + } + } + + return latestTask; +} + +export function resolveWorkflowCurrentStepId( + sessionStepId: string | null, + taskStepId: string | null, + workflowStepIds: readonly string[], +): string | null { + // The resolved task step is freshest across sources. Only use the session + // step as a fallback when it belongs to this workflow. + if (taskStepId) return taskStepId; + if (sessionStepId && workflowStepIds.includes(sessionStepId)) return sessionStepId; + return null; +} + +function hasNewerKanbanState( + baseTask: Task, + kanbanTimestamp: bigint, + baseTimestamp: bigint | null, +): boolean { + return Boolean(baseTask.archived_at && baseTimestamp !== null && kanbanTimestamp > baseTimestamp); +} + export function mergeBaseWithKanban( baseTask: Task, kanbanTask: KanbanState["tasks"][number] | null, ): Task { if (!kanbanTask) return baseTask; - const kanbanUpdatedAt = Date.parse(kanbanTask.updatedAt ?? ""); - const baseUpdatedAt = Date.parse(baseTask.updated_at ?? ""); - const hasNewerKanbanState = - Boolean(baseTask.archived_at) && - Number.isFinite(kanbanUpdatedAt) && - Number.isFinite(baseUpdatedAt) && - kanbanUpdatedAt > baseUpdatedAt; + const kanbanTimestamp = parseTurnTimestamp(kanbanTask.updatedAt ?? undefined); + const baseTimestamp = parseTurnTimestamp(baseTask.updated_at); + if (kanbanTimestamp === null || (baseTimestamp !== null && kanbanTimestamp < baseTimestamp)) { + return baseTask; + } + const hasCompletePlacement = Boolean(kanbanTask.workflowId && kanbanTask.workflowStepId); return { ...baseTask, title: kanbanTask.title ?? baseTask.title, description: kanbanTask.description ?? baseTask.description, - workflow_step_id: - (kanbanTask.workflowStepId as string | undefined) ?? baseTask.workflow_step_id, + workflow_id: hasCompletePlacement ? toWorkflowId(kanbanTask.workflowId) : baseTask.workflow_id, + workflow_step_id: hasCompletePlacement ? kanbanTask.workflowStepId : baseTask.workflow_step_id, position: kanbanTask.position ?? baseTask.position, state: (kanbanTask.state as Task["state"] | undefined) ?? baseTask.state, repositories: baseTask.repositories, metadata: kanbanTask.metadata !== undefined ? kanbanTask.metadata : baseTask.metadata, - archived_at: hasNewerKanbanState ? null : baseTask.archived_at, + archived_at: hasNewerKanbanState(baseTask, kanbanTimestamp, baseTimestamp) + ? null + : baseTask.archived_at, }; } @@ -176,7 +225,7 @@ export function buildTaskFromKanban(kanbanTask: KanbanState["tasks"][number]): T position: kanbanTask.position, state: kanbanTask.state ?? "CREATED", workspace_id: toWorkspaceId(""), - workflow_id: toWorkflowId(""), + workflow_id: toWorkflowId(kanbanTask.workflowId ?? ""), priority: kanbanTask.priority ?? "medium", repositories: [], created_at: "", diff --git a/apps/web/components/task/task-page-content-workflow.test.ts b/apps/web/components/task/task-page-content-workflow.test.ts new file mode 100644 index 0000000000..58f951e870 --- /dev/null +++ b/apps/web/components/task/task-page-content-workflow.test.ts @@ -0,0 +1,263 @@ +import { describe, expect, it } from "vitest"; +import { repositoryId, taskId, workflowId, type Task } from "@/lib/types/http"; +import type { KanbanState } from "@/lib/state/slices"; +import { + buildTaskFromKanban, + resolveEffectiveTask, + resolveLatestTaskProjection, + resolveWorkflowCurrentStepId, +} from "./task-page-content-helpers"; + +type KanbanTask = KanbanState["tasks"][number]; + +const TASK_ID = "task-1"; +const SOURCE_WORKFLOW = "workflow-source"; +const DESTINATION_WORKFLOW = "workflow-destination"; +const SOURCE_STEP = "step-source"; +const DESTINATION_STEP = "step-destination"; +const SOURCE_UPDATED_AT = "2026-07-18T00:00:00Z"; +const DESTINATION_UPDATED_AT = "2026-07-19T00:00:00Z"; + +function makeTaskDetails(overrides: Partial = {}): Task { + return { + id: taskId(TASK_ID), + title: "Workflow migration task", + description: "Task details", + workflow_step_id: SOURCE_STEP, + position: 0, + state: "TODO", + workspace_id: "workspace-1", + workflow_id: workflowId(SOURCE_WORKFLOW), + priority: "medium", + repositories: [], + created_at: "", + updated_at: SOURCE_UPDATED_AT, + ...overrides, + } as Task; +} + +function makeKanbanTask(overrides: Partial = {}): KanbanTask { + return { + id: TASK_ID, + title: "Workflow migration task", + workflowId: SOURCE_WORKFLOW, + workflowStepId: SOURCE_STEP, + position: 0, + state: "TODO", + ...overrides, + } as KanbanTask; +} + +// eslint-disable-next-line max-lines-per-function -- placement cases share fixtures and freshness assertions. +describe("task workflow placement", () => { + // @covers AC-TASKS-CHANGE-WORKFLOW-001.8 + it("applies a newer destination placement and retains task-only details", () => { + const repositories: Task["repositories"] = [ + { + id: "task-repo-1", + task_id: taskId(TASK_ID), + repository_id: repositoryId("repo-1"), + base_branch: "main", + position: 0, + created_at: "", + updated_at: "", + }, + ]; + const details = makeTaskDetails({ repositories }); + const destination = makeKanbanTask({ + workflowId: DESTINATION_WORKFLOW, + workflowStepId: DESTINATION_STEP, + updatedAt: DESTINATION_UPDATED_AT, + }); + + const resolved = resolveEffectiveTask(details, null, destination, TASK_ID); + + expect(resolved).toMatchObject({ + workflow_id: DESTINATION_WORKFLOW, + workflow_step_id: DESTINATION_STEP, + repositories, + }); + }); + + // @covers AC-TASKS-CHANGE-WORKFLOW-001.8 + it("rejects an older source placement as one workflow and step pair", () => { + const details = makeTaskDetails({ + workflow_id: workflowId(DESTINATION_WORKFLOW), + workflow_step_id: DESTINATION_STEP, + updated_at: DESTINATION_UPDATED_AT, + }); + const staleSource = makeKanbanTask({ updatedAt: SOURCE_UPDATED_AT }); + + const resolved = resolveEffectiveTask(details, null, staleSource, TASK_ID); + + expect(resolved).toMatchObject({ + workflow_id: DESTINATION_WORKFLOW, + workflow_step_id: DESTINATION_STEP, + }); + }); + + // @covers AC-TASKS-CHANGE-WORKFLOW-001.8 + it("preserves the known placement when a live row omits its destination step", () => { + const details = makeTaskDetails(); + const partial = makeKanbanTask({ + workflowId: DESTINATION_WORKFLOW, + workflowStepId: "", + }); + + const resolved = resolveEffectiveTask(details, null, partial, TASK_ID); + + expect(resolved).toMatchObject({ + workflow_id: SOURCE_WORKFLOW, + workflow_step_id: SOURCE_STEP, + }); + }); + + it("does not merge an older projection's freshness-sensitive details", () => { + const details = makeTaskDetails({ + title: "Fresh title", + description: "Fresh description", + state: "IN_PROGRESS", + position: 9, + metadata: { source: "http" }, + workflow_id: workflowId(DESTINATION_WORKFLOW), + workflow_step_id: DESTINATION_STEP, + updated_at: DESTINATION_UPDATED_AT, + }); + const staleProjection = makeKanbanTask({ + title: "Stale title", + description: "Stale description", + state: "TODO", + position: 1, + metadata: { source: "cache" }, + workflowId: SOURCE_WORKFLOW, + workflowStepId: SOURCE_STEP, + updatedAt: SOURCE_UPDATED_AT, + }); + + const resolved = resolveEffectiveTask(details, null, staleProjection, TASK_ID); + + expect(resolved).toMatchObject({ + title: "Fresh title", + description: "Fresh description", + state: "IN_PROGRESS", + position: 9, + metadata: { source: "http" }, + workflow_id: DESTINATION_WORKFLOW, + workflow_step_id: DESTINATION_STEP, + }); + }); + + it.each([ + ["missing", undefined], + ["malformed", "not-a-timestamp"], + ["date-normalized malformed", "2026-02-30T00:00:00Z"], + ])("rejects a %s projection timestamp for placement", (_label, updatedAt) => { + const details = makeTaskDetails({ + workflow_id: workflowId(DESTINATION_WORKFLOW), + workflow_step_id: DESTINATION_STEP, + updated_at: SOURCE_UPDATED_AT, + }); + const untrustedProjection = makeKanbanTask({ + workflowId: SOURCE_WORKFLOW, + workflowStepId: SOURCE_STEP, + updatedAt, + }); + + const resolved = resolveEffectiveTask(details, null, untrustedProjection, TASK_ID); + + expect(resolved).toMatchObject({ + workflow_id: DESTINATION_WORKFLOW, + workflow_step_id: DESTINATION_STEP, + }); + }); +}); + +describe("task workflow projection selection", () => { + // @covers AC-TASKS-CHANGE-WORKFLOW-001.8 + it("selects the newer destination row when an older source snapshot remains", () => { + const source = makeKanbanTask({ updatedAt: SOURCE_UPDATED_AT }); + const destination = makeKanbanTask({ + workflowId: DESTINATION_WORKFLOW, + workflowStepId: DESTINATION_STEP, + updatedAt: DESTINATION_UPDATED_AT, + }); + + const selected = resolveLatestTaskProjection(TASK_ID, [source], { + [SOURCE_WORKFLOW]: { tasks: [source] }, + [DESTINATION_WORKFLOW]: { tasks: [destination] }, + }); + + expect(selected).toBe(destination); + }); + + it("keeps a nanosecond-newer active row over an older cached snapshot", () => { + const freshDestination = makeKanbanTask({ + workflowId: DESTINATION_WORKFLOW, + workflowStepId: DESTINATION_STEP, + updatedAt: "2026-07-19T00:00:00.123456790Z", + }); + const staleSource = makeKanbanTask({ + updatedAt: "2026-07-19T00:00:00.123456789Z", + }); + + const selected = resolveLatestTaskProjection(TASK_ID, [freshDestination], { + [SOURCE_WORKFLOW]: { tasks: [staleSource] }, + }); + + expect(selected).toBe(freshDestination); + }); + + it("ignores a date-normalized malformed cache timestamp", () => { + const validTask = makeKanbanTask({ + workflowId: DESTINATION_WORKFLOW, + workflowStepId: DESTINATION_STEP, + updatedAt: "2026-02-28T00:00:00Z", + }); + const malformedTask = makeKanbanTask({ + updatedAt: "2026-02-30T00:00:00Z", + }); + + const selected = resolveLatestTaskProjection(TASK_ID, [validTask], { + [SOURCE_WORKFLOW]: { tasks: [malformedTask] }, + }); + + expect(selected).toBe(validTask); + }); + + // @covers AC-TASKS-CHANGE-WORKFLOW-001.8 + it("uses a task step when the session step belongs to the source workflow", () => { + expect( + resolveWorkflowCurrentStepId(SOURCE_STEP, DESTINATION_STEP, [DESTINATION_STEP, "step-next"]), + ).toBe(DESTINATION_STEP); + }); + + it("uses the fresh task-detail step over an older cached step in the same workflow", () => { + const details = makeTaskDetails({ + workflow_id: workflowId(SOURCE_WORKFLOW), + workflow_step_id: DESTINATION_STEP, + updated_at: DESTINATION_UPDATED_AT, + }); + const staleCachedTask = makeKanbanTask({ + workflowId: SOURCE_WORKFLOW, + workflowStepId: SOURCE_STEP, + updatedAt: SOURCE_UPDATED_AT, + }); + const resolvedTask = resolveEffectiveTask(details, null, staleCachedTask, TASK_ID); + + expect( + resolveWorkflowCurrentStepId( + staleCachedTask.workflowStepId, + resolvedTask?.workflow_step_id ?? null, + [SOURCE_STEP, DESTINATION_STEP], + ), + ).toBe(DESTINATION_STEP); + }); + + it("includes the workflow ID when building a task from a projection", () => { + const task = buildTaskFromKanban( + makeKanbanTask({ workflowId: DESTINATION_WORKFLOW, workflowStepId: DESTINATION_STEP }), + ); + + expect(task.workflow_id).toBe(DESTINATION_WORKFLOW); + }); +}); diff --git a/apps/web/components/task/task-page-content.test.tsx b/apps/web/components/task/task-page-content.test.tsx index db58217a95..a40775343b 100644 --- a/apps/web/components/task/task-page-content.test.tsx +++ b/apps/web/components/task/task-page-content.test.tsx @@ -1,16 +1,27 @@ -import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; -import { useEffect } from "react"; -import { afterEach, describe, expect, it } from "vitest"; +import { createElement, type ReactNode, useEffect } from "react"; +import { act, cleanup, render, renderHook, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { StateProvider, useAppStoreApi } from "@/components/state-provider"; -import { TaskLoadErrorState } from "./task-page-content"; +import * as api from "@/lib/api"; +import { taskId, workflowId, workspaceId, type Task } from "@/lib/types/http"; +import { TaskLoadErrorState, useTaskDetails } from "./task-page-content"; import { TaskRemovalBoundary } from "./task-removal-boundary"; -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); const TASK_A = "task-a"; const TASK_B = "task-b"; const REMOVAL_STATUS_TEST_ID = "task-removal-status"; +function createStateWrapper(initialState: unknown) { + return function StateTestWrapper({ children }: { children: ReactNode }) { + return createElement(StateProvider, { initialState: initialState as never, children }); + }; +} + function renderErrorState(activeId: string | null) { render( @@ -37,6 +48,68 @@ describe("TaskLoadErrorState", () => { }); }); +describe("useTaskDetails reconnect refresh", () => { + it("reloads task placement after the websocket reconnects", async () => { + const initialTask = { + id: taskId(TASK_A), + title: "Workflow migration task", + description: "Task details", + workflow_id: workflowId("workflow-source"), + workflow_step_id: "step-source", + position: 0, + state: "TODO", + workspace_id: workspaceId("workspace-1"), + priority: "medium", + repositories: [], + created_at: "2026-07-18T00:00:00Z", + updated_at: "2026-07-18T00:00:00Z", + } as Task; + const movedTask = { + ...initialTask, + workflow_id: workflowId("workflow-destination"), + workflow_step_id: "step-analysis", + updated_at: "2026-07-19T00:00:00Z", + }; + const fetchTask = vi.spyOn(api, "fetchTask").mockResolvedValue(movedTask); + const wrapper = createStateWrapper({ + tasks: { activeTaskId: TASK_A }, + connection: { status: "disconnected" }, + kanban: { + tasks: [ + { + id: TASK_A, + title: initialTask.title, + description: initialTask.description, + workflowId: "workflow-source", + workflowStepId: "step-source", + position: initialTask.position, + state: initialTask.state, + updatedAt: initialTask.updated_at, + }, + ], + } as never, + }); + const { result } = renderHook( + () => ({ + details: useTaskDetails(TASK_A, initialTask), + store: useAppStoreApi(), + }), + { wrapper }, + ); + + expect(fetchTask).not.toHaveBeenCalled(); + act(() => result.current.store.getState().setConnectionStatus("connected")); + + await waitFor(() => expect(fetchTask).toHaveBeenCalledWith(TASK_A, { cache: "no-store" })); + await waitFor(() => { + expect(result.current.details.task).toMatchObject({ + workflow_id: "workflow-destination", + workflow_step_id: "step-analysis", + }); + }); + }); +}); + function StartRemoval({ removalTaskId = "task-1", activeTaskId, diff --git a/apps/web/components/task/task-page-content.tsx b/apps/web/components/task/task-page-content.tsx index 1ac80ad644..41835c4cb8 100644 --- a/apps/web/components/task/task-page-content.tsx +++ b/apps/web/components/task/task-page-content.tsx @@ -7,7 +7,6 @@ import Link from "@/components/routing/app-link"; import { useTranslation } from "react-i18next"; import type { Repository, RepositoryScript, Task } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; -import type { KanbanState } from "@/lib/state/slices"; import { useRepositories } from "@/hooks/domains/workspace/use-repositories"; import { useSessionAgent } from "@/hooks/domains/session/use-session-agent"; import { useSessionResumption } from "@/hooks/domains/session/use-session-resumption"; @@ -18,7 +17,8 @@ import { useEnsureTaskSession } from "@/hooks/domains/session/use-ensure-task-se import { useExternalVcsFileLinkHydration } from "@/hooks/domains/workspace/use-external-vcs-file-link"; import { fetchTask } from "@/lib/api"; import { linkToTaskOverview } from "@/lib/links"; -import { useTasks } from "@/hooks/use-tasks"; +import { useWorkflowSnapshotById } from "@/hooks/domains/kanban/use-all-workflow-snapshots"; +import { useWorkflowStepsById } from "@/hooks/domains/kanban/use-workflow-steps-by-id"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { useFeature } from "@/hooks/domains/features/use-feature"; import { useTaskCanvasesStateForTask } from "@/hooks/domains/task/use-task-canvases"; @@ -29,6 +29,7 @@ import { buildArchivedValue, hasResolvedTaskDetails, resolveEffectiveTask, + resolveLatestTaskProjection, resolveTaskContentState, syncActiveTaskSession, } from "@/components/task/task-page-content-helpers"; @@ -48,23 +49,16 @@ type TaskPageContentProps = { officeTaskHref?: string | null; }; -export function useWorkflowStepsMapped() { - const kanbanSteps = useAppStore((state) => state.kanban.steps); - return useMemo( - () => - kanbanSteps.map((s) => ({ - id: s.id, - name: s.title, - color: s.color, - position: s.position, - events: s.events, - allow_manual_move: s.allow_manual_move, - prompt: s.prompt, - is_start_step: s.is_start_step, - agent_profile_id: s.agent_profile_id, - })), - [kanbanSteps], - ); +export function useWorkflowStepsMapped(workflowId: string | null | undefined) { + return useWorkflowStepsById(workflowId); +} + +function useTaskWorkflowSnapshot(task: Task | null) { + useWorkflowSnapshotById(task?.workspace_id ?? null, task?.workflow_id ?? null); +} + +function getTaskArchivedState(task: Task | null): boolean | null { + return task ? task.archived_at != null : null; } export function useSessionPanelState(effectiveSessionId: string | null | undefined) { @@ -83,8 +77,10 @@ export function useSessionPanelState(effectiveSessionId: string | null | undefin const sessionWorkflowStepId = useAppStore((state) => { const taskId = state.tasks.activeTaskId; if (!taskId) return null; - const task = state.kanban.tasks.find((t: { id: string }) => t.id === taskId); - return (task?.workflowStepId as string) ?? null; + return ( + resolveLatestTaskProjection(taskId, state.kanban.tasks, state.kanbanMulti.snapshots) + ?.workflowStepId ?? null + ); }); const previewOpen = useAppStore((state) => effectiveSessionId ? (state.previewPanel.openBySessionId[effectiveSessionId] ?? false) : false, @@ -187,18 +183,16 @@ export function TaskLoadErrorState() { ); } -function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { +export function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { const [taskDetails, setTaskDetails] = useState(initialTask); const [taskLoadError, setTaskLoadError] = useState(null); const activeTaskIdRef = useRef(activeTaskId); - const kanbanTask = useAppStore((state) => - activeTaskId - ? (state.kanban.tasks.find( - (item: KanbanState["tasks"][number]) => item.id === activeTaskId, - ) ?? null) - : null, - ); + const connectionStatus = useAppStore((state) => state.connection.status); + const previousConnectionStatus = useRef(connectionStatus); const effectiveTaskId = activeTaskId ?? initialTask?.id ?? null; + const kanbanTask = useAppStore((state) => + resolveLatestTaskProjection(effectiveTaskId, state.kanban.tasks, state.kanbanMulti.snapshots), + ); const task = useMemo( () => resolveEffectiveTask(taskDetails, initialTask, kanbanTask, effectiveTaskId), [taskDetails, initialTask, kanbanTask, effectiveTaskId], @@ -208,8 +202,6 @@ function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { taskDetailsId: taskDetails?.id ?? null, initialTaskId: initialTask?.id ?? null, }); - useTasks(task?.workflow_id ?? null); - useEffect(() => { activeTaskIdRef.current = activeTaskId; }, [activeTaskId]); @@ -238,6 +230,13 @@ function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { void loadTaskDetails(); }, [activeTaskId, taskDetails?.id, loadTaskDetails]); + useEffect(() => { + const reconnected = + previousConnectionStatus.current !== "connected" && connectionStatus === "connected"; + previousConnectionStatus.current = connectionStatus; + if (reconnected) void loadTaskDetails(); + }, [connectionStatus, loadTaskDetails]); + useForegroundRefresh(loadTaskDetails, Boolean(activeTaskId), activeTaskId); const onTaskUnarchived = useCallback( @@ -360,16 +359,17 @@ function TaskPageContentLive({ onTaskUnarchived, refreshTask, } = useTaskPageData(initialTask, initialTaskId, sessionId, initialRepositories); + useTaskWorkflowSnapshot(task); const taskCanvasesState = useTaskCanvasesStateForTask(task, canvasesEnabled); useExternalVcsFileLinkHydration(task, repositories); - const workflowSteps = useWorkflowStepsMapped(); + const workflowSteps = useWorkflowStepsMapped(task?.workflow_id); const sessionPanel = useSessionPanelState(effectiveSessionId); const agentctlStatus = useSessionAgentctl(effectiveSessionId); const resumption = useSessionResumption( task?.id ?? null, effectiveSessionId, - task ? task.archived_at != null : null, + getTaskArchivedState(task), { onTaskArchiveConflict: refreshTask }, ); const merged = useMergedAgentState(agent, resumption, sessionPanel, effectiveSessionId, task); diff --git a/apps/web/components/task/task-page-inner.tsx b/apps/web/components/task/task-page-inner.tsx index c185d2ff93..c201eb5d5f 100644 --- a/apps/web/components/task/task-page-inner.tsx +++ b/apps/web/components/task/task-page-inner.tsx @@ -35,6 +35,7 @@ import { buildDebugEntries, buildArchivedValue, resolveTaskProps, + resolveWorkflowCurrentStepId, useTaskActionsMenuBoardRow, selectWorkspaceRepositories, } from "@/components/task/task-page-content-helpers"; @@ -105,12 +106,12 @@ function resolveRemoteExecutor(status?: RemoteExecutorStatus | null) { }; } -// Prefer the session-level step (delivered direct via session.state_changed) over the task-level step (routed through the hub broadcast and slightly stale). function resolveCurrentStepId( sessionStepId: string | null, taskStepId: string | null, + workflowStepIds: readonly string[], ): string | null { - return sessionStepId || taskStepId || null; + return resolveWorkflowCurrentStepId(sessionStepId, taskStepId, workflowStepIds); } function buildTaskTopBarProps(params: { @@ -137,7 +138,11 @@ function buildTaskTopBarProps(params: { showDebugOverlay, onToggleDebugOverlay, workflowSteps, - currentStepId: resolveCurrentStepId(params.sessionWorkflowStepId, taskProps.workflowStepId), + currentStepId: resolveCurrentStepId( + params.sessionWorkflowStepId, + taskProps.workflowStepId, + workflowSteps.map((step) => step.id), + ), workflowId: taskProps.workflowId, taskState: params.task?.state ?? null, workspaceId: taskProps.workspaceId, diff --git a/apps/web/e2e/tests/task/change-workflow.spec.ts b/apps/web/e2e/tests/task/change-workflow.spec.ts index 07d270acc7..2a48df87fe 100644 --- a/apps/web/e2e/tests/task/change-workflow.spec.ts +++ b/apps/web/e2e/tests/task/change-workflow.spec.ts @@ -9,7 +9,38 @@ import { } from "./task-workflow-agent-overrides-helpers"; test.describe("Change workflow", () => { - test("maps a task profile for a later destination step and preserves its context", async ({ + test("updates an open task page after a move from another client", async ({ + testPage, + apiClient, + seedData, + }) => { + const destination = await apiClient.createWorkflow( + seedData.workspaceId, + "External move target", + ); + const analysis = await apiClient.createWorkflowStep(destination.id, "Analysis", 0); + await apiClient.createWorkflowStep(destination.id, "Implement", 1); + const task = await apiClient.createTask(seedData.workspaceId, "External workflow move task", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + + await testPage.goto(`/t/${task.id}`); + await expect(testPage.getByTestId("task-topbar")).toBeVisible(); + const taskUrl = testPage.url(); + + await apiClient.moveTask(task.id, destination.id, analysis.id); + + await expect(testPage).toHaveURL(taskUrl); + const stepper = testPage.getByTestId("workflow-stepper"); + await expect(stepper.getByTestId("workflow-step-Analysis")).toHaveAttribute( + "aria-current", + "step", + ); + await expect(stepper.getByTestId("workflow-step-Implement")).toBeVisible(); + }); + + test("updates the open task stepper after changing workflow and preserves its context", async ({ testPage, apiClient, seedData, @@ -39,6 +70,13 @@ test.describe("Change workflow", () => { fixture.profileA.id, ); const before = await apiClient.getTask(task.id); + await testPage.goto(`/t/${task.id}`); + await expect(testPage.getByTestId("task-topbar")).toBeVisible(); + const taskUrl = testPage.url(); + await testPage.getByTestId("task-topbar-actions-menu").click(); + await expect(testPage.getByRole("menuitem", { name: "Change workflow..." })).toBeVisible(); + await testPage.getByRole("menuitem", { name: "Change workflow..." }).click(); + const previewChanges: Array> = []; testPage.on("request", (request) => { if (request.url().includes(`/api/v1/tasks/${task.id}/move-preview`)) { @@ -49,12 +87,6 @@ test.describe("Change workflow", () => { previewChanges.push(body.workflow_change as unknown as Record); } }); - const kanban = new KanbanPage(testPage); - await kanban.goto(); - await kanban.openTaskActionsMenu(task.id); - - await expect(testPage.getByRole("menuitem", { name: "Change workflow..." })).toBeVisible(); - await kanban.contextChangeWorkflow().click(); const changeWorkflow = new ChangeWorkflowPage(testPage); await expect(changeWorkflow.desktopDialog).toBeVisible(); await changeWorkflow.chooseWorkflow(fixture.workflow.id); @@ -95,6 +127,26 @@ test.describe("Change workflow", () => { await expect(changeWorkflow.desktopDialog).toBeHidden(); await waitForWorkflowStep(apiClient, task.id, fixture.prStep.id); + await expect(testPage).toHaveURL(taskUrl); + const stepper = testPage.getByTestId("workflow-stepper"); + await expect(stepper.getByTestId("workflow-step-Analysis")).toBeVisible(); + await expect(stepper.getByTestId("workflow-step-Implement")).toBeVisible(); + await expect(stepper.getByTestId("workflow-step-Review")).toBeVisible(); + await expect(stepper.getByTestId("workflow-step-PR")).toHaveAttribute("aria-current", "step"); + if (prCapture.capturing) { + await testPage.evaluate(async () => { + await Promise.all( + document + .getAnimations() + .filter((animation) => animation.playState === "running") + .map((animation) => animation.finished.catch(() => undefined)), + ); + }); + await prCapture.screenshot("desktop-task-destination-stepper", { + caption: + "Open task page showing the destination workflow and current step after migration.", + }); + } const destinationSessionId = await waitForNewWorkflowProfileSession( apiClient, task.id, diff --git a/apps/web/e2e/tests/task/mobile-change-workflow.spec.ts b/apps/web/e2e/tests/task/mobile-change-workflow.spec.ts index b141cb3331..caceb55fb3 100644 --- a/apps/web/e2e/tests/task/mobile-change-workflow.spec.ts +++ b/apps/web/e2e/tests/task/mobile-change-workflow.spec.ts @@ -127,6 +127,27 @@ test("changes workflow from phone task actions with task-local agent routing", a originalSessionId, ]); await waitForWorkflowMoveLifecycle(apiClient, task.id); + await expect(actions.column(task.id)).toBeVisible(); + await actions.open(task.id); + await actions.nested("Move to"); + const currentDestinationStep = testPage.getByTestId(`task-context-step-${fixture.prStep.id}`); + await expect(currentDestinationStep).toBeVisible(); + await expect(currentDestinationStep).toBeDisabled(); + if (prCapture.capturing) { + await testPage.evaluate(async () => { + await Promise.all( + document + .getAnimations() + .filter((animation) => animation.playState === "running") + .map((animation) => animation.finished.catch(() => undefined)), + ); + }); + await prCapture.screenshot("phone-task-destination-step", { + caption: + "Phone task actions showing the task in its destination workflow step after migration.", + }); + } + const changed = await apiClient.getTask(task.id); expect(changed).toMatchObject({ id: before.id, diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts index 8257618922..5ab1b931c6 100644 --- a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts @@ -7,18 +7,23 @@ const mockSetKanbanMultiLoading = vi.fn(); const mockSetWorkflowSnapshot = vi.fn(); const mockSetWorkspaceSnapshotRead = vi.fn(); const mockFetchWorkflowSnapshot = vi.fn(); +const mockHydrate = vi.fn(); type Workflow = { id: string; workspaceId: string; name: string }; +const NEW_WORKFLOW_ID = "workflow-new"; +const NEW_WORKFLOW_NAME = "Created later"; type MockState = { connection: { status: string }; workspaces: { activeId: string | null }; workspaceContextGeneration: number; workflows: { items: Workflow[] }; + kanban: { workflowId: string | null; steps: unknown[]; tasks: unknown[] }; kanbanMulti: { snapshots: Record; isLoading: boolean }; clearKanbanMulti: typeof mockClearKanbanMulti; setKanbanMultiLoading: typeof mockSetKanbanMultiLoading; setWorkflowSnapshot: typeof mockSetWorkflowSnapshot; setWorkspaceSnapshotRead: typeof mockSetWorkspaceSnapshotRead; + hydrate: typeof mockHydrate; }; let mockState: MockState = { @@ -26,11 +31,13 @@ let mockState: MockState = { workspaces: { activeId: "ws-A" }, workspaceContextGeneration: 0, workflows: { items: [] }, + kanban: { workflowId: null, steps: [], tasks: [] }, kanbanMulti: { snapshots: {}, isLoading: false }, clearKanbanMulti: mockClearKanbanMulti, setKanbanMultiLoading: mockSetKanbanMultiLoading, setWorkflowSnapshot: mockSetWorkflowSnapshot, setWorkspaceSnapshotRead: mockSetWorkspaceSnapshotRead, + hydrate: mockHydrate, }; vi.mock("@/components/state-provider", () => ({ @@ -42,7 +49,7 @@ vi.mock("@/lib/api", () => ({ fetchWorkflowSnapshot: (...args: unknown[]) => mockFetchWorkflowSnapshot(...args), })); -import { useAllWorkflowSnapshots } from "./use-all-workflow-snapshots"; +import { useAllWorkflowSnapshots, useWorkflowSnapshotById } from "./use-all-workflow-snapshots"; function resetMocks(workflows: Workflow[] = []) { vi.clearAllMocks(); @@ -52,11 +59,13 @@ function resetMocks(workflows: Workflow[] = []) { workspaces: { activeId: workflows[0]?.workspaceId ?? null }, workspaceContextGeneration: 0, workflows: { items: workflows }, + kanban: { workflowId: null, steps: [], tasks: [] }, kanbanMulti: { snapshots: {}, isLoading: false }, clearKanbanMulti: mockClearKanbanMulti, setKanbanMultiLoading: mockSetKanbanMultiLoading, setWorkflowSnapshot: mockSetWorkflowSnapshot, setWorkspaceSnapshotRead: mockSetWorkspaceSnapshotRead, + hydrate: mockHydrate, }; } @@ -152,6 +161,95 @@ describe("useAllWorkflowSnapshots — workspace scoping", () => { }); }); +describe("useWorkflowSnapshotById", () => { + beforeEach(() => { + resetMocks([{ id: "workflow-catalog", workspaceId: "ws-A", name: "Catalog workflow" }]); + }); + + it("loads steps for a workflow missing from the catalog without changing board selection", async () => { + mockState.kanban = { workflowId: "workflow-active", steps: [], tasks: [] }; + mockFetchWorkflowSnapshot.mockResolvedValueOnce({ + workflow: { id: NEW_WORKFLOW_ID, name: NEW_WORKFLOW_NAME }, + steps: [{ id: "analysis", name: "Analysis", position: 0 }], + tasks: [], + }); + + renderHook(() => useWorkflowSnapshotById("ws-A", NEW_WORKFLOW_ID)); + + await waitFor(() => + expect(mockFetchWorkflowSnapshot).toHaveBeenCalledWith(NEW_WORKFLOW_ID, { + cache: "no-store", + }), + ); + await waitFor(() => + expect(mockSetWorkflowSnapshot).toHaveBeenCalledWith( + NEW_WORKFLOW_ID, + expect.objectContaining({ + workflowId: NEW_WORKFLOW_ID, + workflowName: NEW_WORKFLOW_NAME, + steps: [expect.objectContaining({ id: "analysis", title: "Analysis" })], + }), + ), + ); + + expect(mockState.kanban.workflowId).toBe("workflow-active"); + expect(mockHydrate).not.toHaveBeenCalled(); + expect(mockFetchWorkflowSnapshot).toHaveBeenCalledTimes(1); + }); + + it("retries a failed placeholder fetch after the task page remounts", async () => { + mockState.kanbanMulti.snapshots[NEW_WORKFLOW_ID] = { + workflowId: NEW_WORKFLOW_ID, + workflowName: NEW_WORKFLOW_NAME, + steps: [], + tasks: [], + isPlaceholder: true, + }; + mockSetWorkflowSnapshot.mockImplementationOnce((id: string, snapshot: unknown) => { + mockState.kanbanMulti.snapshots[id] = snapshot; + }); + mockFetchWorkflowSnapshot + .mockRejectedValueOnce(new Error("temporary fetch failure")) + .mockResolvedValueOnce({ + workflow: { id: NEW_WORKFLOW_ID, name: NEW_WORKFLOW_NAME }, + steps: [{ id: "analysis", name: "Analysis", position: 0 }], + tasks: [], + }); + + const firstMount = renderHook(() => useWorkflowSnapshotById("ws-A", NEW_WORKFLOW_ID)); + try { + await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(mockSetWorkflowSnapshot).toHaveBeenCalledWith( + NEW_WORKFLOW_ID, + expect.objectContaining({ isPlaceholder: false, fetchFailed: true }), + ), + ); + firstMount.unmount(); + + const secondMount = renderHook(() => useWorkflowSnapshotById("ws-A", NEW_WORKFLOW_ID)); + try { + await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(mockSetWorkflowSnapshot).toHaveBeenCalledWith( + NEW_WORKFLOW_ID, + expect.objectContaining({ + workflowId: NEW_WORKFLOW_ID, + steps: [expect.objectContaining({ id: "analysis", title: "Analysis" })], + }), + ), + ); + } finally { + secondMount.unmount(); + } + } finally { + firstMount.unmount(); + mockFetchWorkflowSnapshot.mockReset(); + mockFetchWorkflowSnapshot.mockResolvedValue({ steps: [], tasks: [] }); + } + }); +}); + describe("useAllWorkflowSnapshots — fetch guards", () => { beforeEach(() => { resetMocks([{ id: "wf-A", workspaceId: "ws-A", name: "A" }]); diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts index fa8c689f82..b1537f1397 100644 --- a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts @@ -24,7 +24,7 @@ type Workflow = { id: string; name: string }; type WorkspaceContextRequest = { workspaceId: string; generation: number }; function isBootHydratedSnapshot(snapshot: WorkflowSnapshotData | undefined): boolean { - return !!snapshot && snapshot.isPlaceholder !== true; + return !!snapshot && snapshot.isPlaceholder !== true && snapshot.fetchFailed !== true; } function hasNewerLivePlacement(existing: KanbanTask, fetchStart: KanbanTask | undefined): boolean { @@ -273,7 +273,7 @@ async function fetchAndWriteSnapshot( const workflowSnapshot = { workflowId: wf.id, - workflowName: wf.name, + workflowName: snapshot.workflow?.name ?? wf.name, steps, tasks, }; @@ -303,11 +303,15 @@ async function fetchAndWriteSnapshot( } markFailed(wf.id, err); // A failed fetch must not leave a placeholder permanently "unknown": that - // would block the final-step ensure forever. Transition to an explicit - // known-but-empty state so the ensure proceeds ungated (safe default). + // would block the final-step ensure forever. Keep it known-but-empty for + // now, and mark it retryable for a later task-page mount. const current = store.getState().kanbanMulti.snapshots[wf.id]; if (current?.isPlaceholder) { - store.getState().setWorkflowSnapshot(wf.id, { ...current, isPlaceholder: false }); + store.getState().setWorkflowSnapshot(wf.id, { + ...current, + isPlaceholder: false, + fetchFailed: true, + }); } } } @@ -321,6 +325,74 @@ function mapSnapshotTask(task: Task, stepIds: Set): KanbanTask | null { return toKanbanTask(task); } +export function useWorkflowSnapshotById( + workspaceId: string | null, + workflowId: string | null | undefined, +) { + const store = useAppStoreApi(); + const connectionStatus = useAppStore((state) => state.connection.status); + const workspaceContextGeneration = useAppStore((state) => state.workspaceContextGeneration); + const workspaceContextRetryVersion = useAppStore( + (state) => state.workspaceContextRead?.retryVersion ?? 0, + ); + const [refreshNonce, setRefreshNonce] = useState(0); + const fetchGenRef = useRef(0); + const lastRefreshNonceRef = useRef(refreshNonce); + const lastConnectionStatusRef = useRef(connectionStatus); + const lastContextGenerationRef = useRef(workspaceContextGeneration); + const lastRecoveryRetryVersionRef = useRef(workspaceContextRetryVersion); + const refresh = useCallback(() => setRefreshNonce((current) => current + 1), []); + + useForegroundRefresh(refresh, Boolean(workspaceId && workflowId), `${workspaceId}:${workflowId}`); + + useEffect(() => { + if (!workspaceId || !workflowId) return; + + const forceRefresh = + refreshNonce !== lastRefreshNonceRef.current || + connectionStatus !== lastConnectionStatusRef.current || + workspaceContextGeneration !== lastContextGenerationRef.current || + workspaceContextRetryVersion !== lastRecoveryRetryVersionRef.current; + lastRefreshNonceRef.current = refreshNonce; + lastConnectionStatusRef.current = connectionStatus; + lastContextGenerationRef.current = workspaceContextGeneration; + lastRecoveryRetryVersionRef.current = workspaceContextRetryVersion; + + const state = store.getState(); + const currentSnapshot = state.kanbanMulti.snapshots[workflowId]; + const activeWorkflowHasSteps = + state.kanban.workflowId === workflowId && state.kanban.steps.length > 0; + if (!forceRefresh && (isBootHydratedSnapshot(currentSnapshot) || activeWorkflowHasSteps)) { + return; + } + + const myGen = fetchGenRef.current + 1; + fetchGenRef.current = myGen; + const request = { workspaceId, generation: state.workspaceContextGeneration }; + void fetchAndWriteSnapshot( + { id: workflowId, name: currentSnapshot?.workflowName ?? workflowId }, + store, + fetchGenRef, + myGen, + request, + () => {}, + () => {}, + ); + + return () => { + if (fetchGenRef.current === myGen) fetchGenRef.current += 1; + }; + }, [ + connectionStatus, + refreshNonce, + store, + workspaceContextGeneration, + workspaceContextRetryVersion, + workflowId, + workspaceId, + ]); +} + // eslint-disable-next-line max-lines-per-function -- one hook owns snapshot refresh and generation cleanup export function useAllWorkflowSnapshots(workspaceId: string | null) { const store = useAppStoreApi(); diff --git a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts index 6aa72e628b..895aec58fa 100644 --- a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts +++ b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts @@ -68,6 +68,42 @@ describe("useWorkflowStepsById", () => { expect(result.current[0]).toMatchObject({ id: "x", name: "Triage" }); }); + it("updates ordered steps when a placeholder destination snapshot loads", () => { + mockState = { + kanban: { + workflowId: ACTIVE_WORKFLOW_ID, + steps: [{ id: "source", title: "Review", color: "#111", position: 0 }], + }, + kanbanMulti: { snapshots: { "workflow-other": { steps: [] } } }, + }; + const { result, rerender } = renderHook( + ({ workflowId }: { workflowId: string }) => useWorkflowStepsById(workflowId), + { initialProps: { workflowId: "workflow-other" } }, + ); + expect(result.current).toEqual([]); + + mockState = { + kanban: { + workflowId: ACTIVE_WORKFLOW_ID, + steps: [{ id: "source", title: "Review", color: "#111", position: 0 }], + }, + kanbanMulti: { + snapshots: { + "workflow-other": { + steps: [ + { id: "later", title: "Implement", color: "#222", position: 1 }, + { id: "current", title: "Analysis", color: "#333", position: 0 }, + ], + }, + }, + }, + }; + rerender({ workflowId: "workflow-other" }); + + expect(result.current.map((step) => step.id)).toEqual(["current", "later"]); + expect(result.current.map((step) => step.name)).toEqual(["Analysis", "Implement"]); + }); + it("returns an empty list when neither the active workflow nor a snapshot resolves it", () => { mockState = { kanban: { workflowId: ACTIVE_WORKFLOW_ID, steps: [] }, diff --git a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts index fada33f979..42f9838b7d 100644 --- a/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts +++ b/apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts @@ -26,9 +26,9 @@ function mapStep(step: StoreStep): WorkflowStepperStep { * Resolves the ordered step list for a workflow that is not necessarily the * board's active workflow: the previewed task's own workflow, per * `plugin-context-api.ts`'s rule. `kanban.steps` covers the active workflow; - * `kanbanMulti.snapshots` (populated by `useAllWorkflowSnapshots` for every - * workflow in the workspace) covers every other one, including a task on a - * workflow the board is not currently filtered to. + * `kanbanMulti.snapshots` covers other workflows. The workspace-wide board + * uses `useAllWorkflowSnapshots`, while task pages fetch only their own + * workflow through `useWorkflowSnapshotById`. */ export function useWorkflowStepsById(workflowId: string | null | undefined): WorkflowStepperStep[] { const activeWorkflowId = useAppStore((state) => state.kanban.workflowId); diff --git a/apps/web/lib/state/slices/kanban/types.ts b/apps/web/lib/state/slices/kanban/types.ts index 07aab65589..53d1a83f84 100644 --- a/apps/web/lib/state/slices/kanban/types.ts +++ b/apps/web/lib/state/slices/kanban/types.ts @@ -219,6 +219,8 @@ export type WorkflowSnapshotData = { steps: KanbanState["steps"]; tasks: KanbanState["tasks"]; isPlaceholder?: boolean; + /** A known-empty failed fetch is retryable after a task-page remount. */ + fetchFailed?: boolean; }; export type KanbanMultiState = { diff --git a/docs/plans/workflow-migration-topbar-refresh/plan.md b/docs/plans/workflow-migration-topbar-refresh/plan.md new file mode 100644 index 0000000000..ca56f56815 --- /dev/null +++ b/docs/plans/workflow-migration-topbar-refresh/plan.md @@ -0,0 +1,160 @@ +--- +created: 2026-09-25 +status: complete +requirements: + - REQ-TASKS-CHANGE-WORKFLOW-001 +system_design: + - ../../specs/tasks/system-design/change-workflow.md +legacy_specs: [] +--- + +# Workflow migration top bar refresh + +## Overview + +An open task moved from Kanban/Review to Feature/Analysis can continue to show +Kanban/Review in its desktop top bar until a browser reload. Make the open task +follow the destination workflow event and load that workflow's steps. The +existing AC-TASKS-CHANGE-WORKFLOW-001.8 already requires this behavior, so this +package repairs its implementation without adding another product requirement. + +## Root cause and reproduction + +The `task.updated` handler moves a task projection into +`kanbanMulti.snapshots[destination]`. The task page must choose the freshest row +across active and cached workflows, while a newer HTTP task detail remains +authoritative over an older cached projection. A move can also be missed while +the WebSocket is disconnected, so reconnect must refresh task details before +the page can discover the destination workflow and its steps. + +The smallest regression is a task detail page open on Kanban/Review: migrate +that task to Feature/Analysis through the existing Change workflow form, keep +the page open, and observe that the stepper retains Kanban/Review even after the +backend task reports Feature/Analysis. Existing change-workflow E2E checks the +backend result but does not assert the open page's stepper before reload. + +## Scope + +### In scope + +- Reconcile the open task's workflow ID and step ID from its live destination + projection while preserving full task detail fields. +- Load ordered steps for the task's resolved workflow and render its current + step in the desktop top bar without changing the board workflow selection. +- Cover a destination with no cached step definitions, delayed HTTP snapshots, + a stale source-board step, and another client's move while the task page is open. +- Preserve the existing phone migration action and task placement behavior. + +### Out of scope + +- Backend move semantics, workflow definitions, agent choices, and session + routing. +- A new phone stepper or a change to the workflow-change form layout. + +## Technical approach + +In `task-page-content.tsx`, resolve the live task from the active workflow or +its owning `kanbanMulti` snapshot by task ID. Compare projection timestamps +with the shared strict RFC3339Nano parser, and do not merge stale or invalid +projections over newer HTTP task details. Keep the resolved task step +authoritative for the top bar, and use a session step only as a fallback when +it belongs to the current workflow. Refresh task details after a WebSocket +reconnect so a move missed while offline is recovered. + +Have the task page request/consume step definitions for the resolved task +workflow directly by ID, even if it is absent from `workflows.items`. Fetch only +the task's workflow instead of loading all workspace snapshots from the task +page. Use the existing workflow-aware step selector or equivalent shared +mapping instead of the unscoped `kanban.steps`. In `task-page-inner.tsx`, use +the freshness-resolved task step first and use a current-workflow cache step +only when task details omit the step. Do not change the selected workflow of a +separate board tab. + +## ASCII UI preview + +### UI-01: Desktop open task after migration + +Current observed state after backend success, before reload: + +```text +Task: Investigate binary size increase [Backlog] - [In Progress] - [Review*] - [Done] + (old Kanban steps remain) +``` + +Required state on the same open page: + +```text +Task: Investigate binary size increase [Backlog] - [Analysis*] - [Implement] - [Review] - [PR] + (Feature steps; Analysis is current) +``` + +`*` marks the current step. Step names are illustrative; the destination +workflow supplies the actual ordered labels. The desktop top bar keeps its +existing layout and controls. AC-TASKS-CHANGE-WORKFLOW-001.8 owns the change. + +### UI-02: Phone task action and destination placement + +```text +Task actions > Change workflow... > select Feature / Analysis > Change workflow +Task remains open; task lists and subsequent actions use Feature / Analysis. +``` + +The phone task page has no desktop stepper. Its action entry, form, and layout +remain as shipped; the shared task projection changes underneath them. Existing +phone E2E checks the form, and the targeted extension checks visible destination +placement without reloading. No new copy or control geometry is required. + +## Tests + +| Acceptance | Evidence | +| ------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| AC-TASKS-CHANGE-WORKFLOW-001.8 | `task-page-content-workflow.test.ts`: newer destination placement wins; older and partial projections cannot restore source placement; stale fields cannot overwrite newer HTTP details; invalid timestamps, nanosecond ordering, and same-workflow current-step selection are covered. | +| AC-TASKS-CHANGE-WORKFLOW-001.8 | `task-page-content.test.tsx`: task details refresh after WebSocket reconnection and retain the destination step over a stale cached row. | +| AC-TASKS-CHANGE-WORKFLOW-001.8 | `use-workflow-steps-by-id.test.ts`: a placeholder destination snapshot updates to its ordered workflow steps when they load. | +| AC-TASKS-CHANGE-WORKFLOW-001.8 | `use-all-workflow-snapshots.test.ts`: an uncatalogued task workflow is fetched by ID without changing board selection; a failed placeholder fetch retries after remount. | + +## E2E tests + +| File / project | Scenario | +| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `tests/task/change-workflow.spec.ts` / `chromium` | Open task page, migrate from its action, assert destination step names and current marker before any page reload; verify backend placement and unchanged task URL. | +| `tests/task/change-workflow.spec.ts` / `chromium` | Move a task through the API while its task page remains open; assert the receiving page switches to destination steps and current marker without navigation. | +| `tests/task/mobile-change-workflow.spec.ts` / `mobile-chrome` | Complete the existing phone flow and assert the destination workflow/step in the visible task surface before reload. | + +## Work orders + +- [x] [Task 01: Reconcile open task workflow placement](task-01-open-task-workflow-placement.md) (done) + +One focused frontend slice covers the live projection, step definitions, and +desktop/phone regression evidence. Implementation was completed sequentially in +this session. + +## Verification results + +- Focused unit tests: 7 files, 108 tests passed. +- PR fixup regression tests: 4 files, 97 tests passed, covering strict + projection freshness, task-detail refresh on reconnect, retry after a failed + placeholder fetch, and valid timestamps on live projection fixtures. +- TypeScript typecheck passed. +- Scoped ESLint passed with no warnings. +- Production Vite build passed. Vite reported existing chunk-size, deprecated + option, and ineffective dynamic import warnings. +- PR screenshot recapture passed: desktop Chromium 4 tests and phone mobile- + chrome 1 test. The phone test's first capture attempt hit a tap-stability + timeout and passed on retry. +- Desktop Chromium change-workflow E2E: 4 tests passed, including an external + move event received by an open task page. +- Phone mobile-chrome change-workflow E2E: 1 test passed. +- `CAPTURE_PR_ASSETS=1 pnpm e2e:run --host --no-build --project chromium e2e/tests/task/change-workflow.spec.ts`: 4 passed; captured the destination stepper on the open task page. +- `CAPTURE_PR_ASSETS=1 pnpm e2e:run --host --no-build --project mobile-chrome e2e/tests/task/mobile-change-workflow.spec.ts`: 1 passed; captured the destination step in phone task actions. +- Specification catalog and specification lint passed. +- `git diff --check` passed. + +## Risks + +- HTTP snapshot responses can race a newer live move; accepting the old + response would briefly restore the old workflow. +- A destination placeholder has a task row but no step definitions until its + workflow snapshot loads. +- The source-board step fallback can outlive the task's source placement during + event reconciliation. diff --git a/docs/plans/workflow-migration-topbar-refresh/task-01-open-task-workflow-placement.md b/docs/plans/workflow-migration-topbar-refresh/task-01-open-task-workflow-placement.md new file mode 100644 index 0000000000..31f0330338 --- /dev/null +++ b/docs/plans/workflow-migration-topbar-refresh/task-01-open-task-workflow-placement.md @@ -0,0 +1,140 @@ +--- +id: "01-open-task-workflow-placement" +title: "Reconcile open task workflow placement" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-CHANGE-WORKFLOW-001 +acceptance_criteria: + - AC-TASKS-CHANGE-WORKFLOW-001.8 +system_design: + - ../../specs/tasks/system-design/change-workflow.md +--- + +# Task 01: Reconcile open task workflow placement + +## Summary + +Make an open task page use the destination task projection and step definitions +after a cross-workflow move. Keep the task page open and update its desktop +stepper as soon as the live move is observed. + +## In scope + +- Add a failing regression for workflow ID and step ID reconciliation after the + task leaves `kanban.tasks` and enters a destination snapshot. +- Resolve the task's live placement and ordered workflow steps without allowing + stale HTTP data or the source-board step fallback to restore source labels. +- Fetch only the resolved task workflow snapshot by ID, even when its workflow + is absent from the global workflow catalog. +- Extend desktop and phone change-workflow E2E to assert visible destination + placement before reload, including a move received from another client. + +## Out of scope + +- Backend changes, form redesign, new mobile stepper, and unrelated cache + refactoring. + +## Acceptance + +- After a committed move, an open desktop task shows destination workflow + steps with the destination step current; the URL and board selection stay put. +- An uncached destination loads its steps, and a stale snapshot or source-board step + does not restore source placement. +- The phone flow remains usable and shows the task in its destination before + reload. + +## ASCII UI preview + +### UI-01: Desktop open task after migration + +```text +Before: Task title [Backlog] - [In Progress] - [Review*] - [Done] +After: Task title [Backlog] - [Analysis*] - [Implement] - [Review] - [PR] +``` + +`*` is the current step. Actual names come from the selected workflow. See +[the full preview](plan.md#ascii-ui-preview). AC-TASKS-CHANGE-WORKFLOW-001.8. + +### UI-02: Phone task action and destination placement + +```text +Task actions > Change workflow... > Feature / Analysis > Change workflow +Task list and actions reflect Feature / Analysis without reload. +``` + +The existing phone composition remains. See +[the full preview](plan.md#ascii-ui-preview). AC-TASKS-CHANGE-WORKFLOW-001.8. + +## Verification + +Run from the repository root, without overlapping E2E suites: + +```bash +(cd apps/web && pnpm test -- components/task/task-page-content-helpers.test.ts components/task/task-page-content-workflow.test.ts components/task/task-page-content.test.tsx hooks/domains/kanban/use-workflow-steps-by-id.test.ts hooks/domains/kanban/use-all-workflow-snapshots.test.ts hooks/domains/kanban/use-all-workflow-snapshots-inflight.test.ts hooks/domains/kanban/use-all-workflow-snapshots.signal-gated.test.ts) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm run build:vite) +(cd apps/web && pnpm exec eslint components/task/task-page-content-helpers.ts components/task/task-page-content-workflow.test.ts components/task/task-page-content.tsx components/task/task-page-inner.tsx hooks/domains/kanban/use-workflow-steps-by-id.ts hooks/domains/kanban/use-workflow-steps-by-id.test.ts hooks/domains/kanban/use-all-workflow-snapshots.ts hooks/domains/kanban/use-all-workflow-snapshots.test.ts e2e/tests/task/change-workflow.spec.ts e2e/tests/task/mobile-change-workflow.spec.ts) +(cd apps/web && pnpm e2e:run --project chromium tests/task/change-workflow.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-change-workflow.spec.ts) +``` + +Run scoped lint on changed frontend files if the implementation modifies linted +source outside the already covered test paths. + +## Files changed + +- `apps/web/components/task/task-page-content-helpers.ts` +- `apps/web/components/task/task-page-content-workflow.test.ts` +- `apps/web/components/task/task-page-content.tsx` +- `apps/web/components/task/task-page-inner.tsx` +- `apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts` +- `apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts` +- `apps/web/hooks/domains/kanban/use-workflow-steps-by-id.ts` +- `apps/web/hooks/domains/kanban/use-workflow-steps-by-id.test.ts` +- `apps/web/e2e/tests/task/change-workflow.spec.ts` +- `apps/web/e2e/tests/task/mobile-change-workflow.spec.ts` +- `docs/specs/tasks/system-design/change-workflow.md` + +## Dependencies + +None. + +## Risks + +- Snapshot freshness and placeholder step loading need separate assertions so + a fast local E2E does not mask either path. + +## Parallelism + +`sequential` + +## Inputs + +- [Change workflow requirement](../../specs/tasks/requirements/change-workflow.md), + especially AC-TASKS-CHANGE-WORKFLOW-001.8. +- [Open task workflow projection](../../specs/tasks/system-design/change-workflow.md#open-task-workflow-projection). +- Existing task page helpers, task WebSocket handler, and change-workflow E2E. + +## Results + +- Focused unit tests: 7 files, 108 tests passed. +- PR fixup regression tests: 4 files, 97 tests passed, covering strict + projection freshness, task-detail refresh on reconnect, retry after a failed + placeholder fetch, and valid timestamps on live projection fixtures. +- `pnpm run typecheck` passed. +- `pnpm run build:vite` passed. Vite reported existing chunk-size, + deprecated option, and ineffective dynamic import warnings. +- Scoped ESLint passed with no warnings. +- Desktop Chromium E2E: 4 tests passed, including an external move event + received by an open task page. +- Phone mobile-chrome E2E: 1 test passed. +- PR screenshot recapture passed: desktop Chromium 4 tests and phone mobile- + chrome 1 test. The phone test's first capture attempt hit a tap-stability + timeout and passed on retry. +- `CAPTURE_PR_ASSETS=1 pnpm e2e:run --host --no-build --project chromium e2e/tests/task/change-workflow.spec.ts`: 4 passed; captured the destination stepper. +- `CAPTURE_PR_ASSETS=1 pnpm e2e:run --host --no-build --project mobile-chrome e2e/tests/task/mobile-change-workflow.spec.ts`: 1 passed; captured the destination step action. +- Specification catalog and specification lint passed. +- `git diff --check` passed. diff --git a/docs/specs/tasks/system-design/change-workflow.md b/docs/specs/tasks/system-design/change-workflow.md index 882ac55c47..571bede7ab 100644 --- a/docs/specs/tasks/system-design/change-workflow.md +++ b/docs/specs/tasks/system-design/change-workflow.md @@ -110,6 +110,38 @@ After commit, existing task events carry the complete task projection. Routing, future queued promotion, and restart load the new map. Never implement this as a move followed by an independent override update. +## Open task workflow projection + +The open task page must keep workflow membership, current step, and step +definitions in one destination-aware projection. `task.updated` already removes +a cross-workflow task from the source `kanban` cache and inserts it into the +destination `kanbanMulti` snapshot, creating a placeholder when necessary. The +page currently merges local task details with only `kanban.tasks`, and that +merge does not update `workflow_id`. After removal from the source cache, the +local details therefore retain the source workflow and the top bar continues +to read the source `kanban.steps` until a full page reload. + +Resolve the open task's live placement from its task ID across the active and +multi-workflow snapshots. Use the owning snapshot's workflow ID together with +the task's step ID. A live placement newer than local task details supersedes +both fields; a partial task update that omits placement must preserve the known +fields. Do not use a source workflow row after the destination event has removed +it. Keep full task details for fields absent from board projections. A request +started before a newer live move must not restore the older placement. + +The task page fetches the resolved workflow snapshot by ID, including when the +workflow is absent from the global workflow catalog and its destination +snapshot is only a placeholder. Fetch only the task's workflow; do not load all +workspace workflows solely for this page. The desktop stepper uses that +workflow's ordered steps and the freshness-resolved task step as current. Use a +cached projection step only when task details do not contain a step and the +cached step belongs to the resolved workflow. A newer task-detail response +remains authoritative over an older cached row, including for a move within the +same workflow. Keep the board's selected workflow unchanged. Reconnect and +foreground refresh converge the same projection after missed events. The phone +task action and list surfaces reuse the task placement state without a new +mobile stepper. + ## Routing and compatibility Use existing task-aware fixed-profile resolution. Initial and earlier-step