From 9664e13f076cd422e92c8d688e3199cb0e2f799f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Fri, 8 May 2026 21:23:48 +0100 Subject: [PATCH] fix(web): keep "All Workflows" filter on task nav and workflow.created (#854) --- .../task/dockview-desktop-layout.tsx | 3 +- .../task/dockview-header-actions.tsx | 3 +- apps/web/components/task/task-top-bar.tsx | 1 + .../e2e/tests/kanban/workflow-filter.spec.ts | 40 +++++++++++++++++- apps/web/lib/kanban/resolve-workflow.ts | 3 +- apps/web/lib/ssr/session-page-state.ts | 5 ++- .../lib/state/slices/kanban/kanban-slice.ts | 11 ++--- apps/web/lib/ws/handlers/workflows.test.ts | 41 +++++++++++++++++++ apps/web/lib/ws/handlers/workflows.ts | 5 +-- 9 files changed, 96 insertions(+), 16 deletions(-) diff --git a/apps/web/components/task/dockview-desktop-layout.tsx b/apps/web/components/task/dockview-desktop-layout.tsx index e116ee5db7..82b22cf29c 100644 --- a/apps/web/components/task/dockview-desktop-layout.tsx +++ b/apps/web/components/task/dockview-desktop-layout.tsx @@ -198,7 +198,8 @@ const tabComponents: Record state.workspaces.activeId); - const workflowId = useAppStore((state) => state.workflows.activeId); + // Read kanban.workflowId (task snapshot), not workflows.activeId (homepage filter), to preserve "All Workflows" across task navigation. + const workflowId = useAppStore((state) => state.kanban.workflowId); const workspaceName = useAppStore((state) => { const ws = state.workspaces.items.find((w: { id: string }) => w.id === workspaceId); return ws?.name ?? "Workspace"; diff --git a/apps/web/components/task/dockview-header-actions.tsx b/apps/web/components/task/dockview-header-actions.tsx index 6e111eab2a..f8840b218f 100644 --- a/apps/web/components/task/dockview-header-actions.tsx +++ b/apps/web/components/task/dockview-header-actions.tsx @@ -251,8 +251,9 @@ export function RightHeaderActions(props: IDockviewHeaderActionsProps) { function SidebarRightActions() { const workspaceId = useAppStore((state) => state.workspaces.activeId); - const workflowId = useAppStore((state) => state.workflows.activeId); const kanban = useAppStore((state) => state.kanban); + // Use kanban.workflowId (task context) not workflows.activeId so "All Workflows" isn't clobbered when viewing a task. + const workflowId = kanban.workflowId; const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); const activeTaskTitle = useAppStore((state) => { const id = state.tasks.activeTaskId; diff --git a/apps/web/components/task/task-top-bar.tsx b/apps/web/components/task/task-top-bar.tsx index edb9e1a476..ab24e7aafe 100644 --- a/apps/web/components/task/task-top-bar.tsx +++ b/apps/web/components/task/task-top-bar.tsx @@ -199,6 +199,7 @@ function TopBarLeft({ diff --git a/apps/web/e2e/tests/kanban/workflow-filter.spec.ts b/apps/web/e2e/tests/kanban/workflow-filter.spec.ts index db74ca63fe..b53d3c0e05 100644 --- a/apps/web/e2e/tests/kanban/workflow-filter.spec.ts +++ b/apps/web/e2e/tests/kanban/workflow-filter.spec.ts @@ -33,6 +33,7 @@ async function selectWorkflowFilter(page: Page, optionLabel: string): Promise { let workflowBId: string | null = null; + let betaTaskId: string | null = null; // Pull `testPage` so its fixture (which runs `e2eReset` and resets user // settings) is set up before this hook seeds workflows/tasks — otherwise @@ -48,10 +49,11 @@ test.describe("Kanban workflow filter", () => { workflow_id: seedData.workflowId, workflow_step_id: seedData.startStepId, }); - await apiClient.createTask(seedData.workspaceId, BETA_TASK, { + const beta = await apiClient.createTask(seedData.workspaceId, BETA_TASK, { workflow_id: workflowB.id, workflow_step_id: startB.id, }); + betaTaskId = beta.id; }); test.afterEach(async ({ apiClient, seedData }) => { @@ -59,6 +61,7 @@ test.describe("Kanban workflow filter", () => { await apiClient.deleteWorkflow(workflowBId).catch(() => {}); workflowBId = null; } + betaTaskId = null; await apiClient.saveUserSettings({ workspace_id: seedData.workspaceId, workflow_filter_id: seedData.workflowId, @@ -118,4 +121,39 @@ test.describe("Kanban workflow filter", () => { timeout: TASK_VISIBLE_TIMEOUT, }); }); + + // Regression: SSR wrote workflows.activeId from the task's workflow_id, clobbering the "All Workflows" filter on return. Pins the cross-page flow. + test("'All Workflows' selection survives navigating into a task and back", async ({ + testPage, + }) => { + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + await selectWorkflowFilter(testPage, "All Workflows"); + + await expect(kanban.taskCardByTitle(ALPHA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + await expect(kanban.taskCardByTitle(BETA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + + // Visit a task that belongs to Workflow B — the SSR build path that used + // to poison the global filter. + if (!betaTaskId) throw new Error("beta task was not seeded"); + await testPage.goto(`/t/${betaTaskId}`); + await expect(testPage).toHaveURL(new RegExp(`/t/${betaTaskId}`)); + + // Breadcrumb = client-side nav: goto("/") re-runs SSR and re-resolves activeId, masking the bug. + await testPage.getByTestId("task-breadcrumb-home").click(); + await expect(testPage).toHaveURL(/\/$|\?/); + await expect(kanban.board).toBeVisible(); + + await expect(kanban.taskCardByTitle(ALPHA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + await expect(kanban.taskCardByTitle(BETA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + }); }); diff --git a/apps/web/lib/kanban/resolve-workflow.ts b/apps/web/lib/kanban/resolve-workflow.ts index 27d5d2affd..464dca7ead 100644 --- a/apps/web/lib/kanban/resolve-workflow.ts +++ b/apps/web/lib/kanban/resolve-workflow.ts @@ -30,5 +30,6 @@ export function resolveDesiredWorkflowId({ ) { return settingsWorkflowId; } - return visibleWorkflows.length === 1 ? visibleWorkflows[0].id : null; + if (visibleWorkflows.length === 1) return visibleWorkflows[0].id; + return null; } diff --git a/apps/web/lib/ssr/session-page-state.ts b/apps/web/lib/ssr/session-page-state.ts index 5bafcfb4da..c59bc9675f 100644 --- a/apps/web/lib/ssr/session-page-state.ts +++ b/apps/web/lib/ssr/session-page-state.ts @@ -22,6 +22,7 @@ import type { import type { Terminal } from "@/hooks/domains/session/use-terminals"; import { snapshotToState, taskToState } from "@/lib/ssr/mapper"; import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; +import type { AppState } from "@/lib/state/store"; function buildWorktreeState(allSessions: TaskSession[]) { const sessionsWithWorktrees = allSessions.filter((s) => s.worktree_id); @@ -104,6 +105,7 @@ function buildResourceState(p: BuildSessionPageStateParams) { })), activeId: task.workspace_id, }, + // Don't write activeId — null means "All Workflows"; task context lives in kanban.workflowId. workflows: { items: workflows.map((w) => ({ id: w.id, @@ -111,8 +113,7 @@ function buildResourceState(p: BuildSessionPageStateParams) { name: w.name, hidden: w.hidden, })), - activeId: task.workflow_id, - }, + } as Partial["workflows"], repositories: { itemsByWorkspaceId: { [task.workspace_id]: repositories }, loadingByWorkspaceId: { [task.workspace_id]: false }, diff --git a/apps/web/lib/state/slices/kanban/kanban-slice.ts b/apps/web/lib/state/slices/kanban/kanban-slice.ts index 8a3d594e52..5c4adfd741 100644 --- a/apps/web/lib/state/slices/kanban/kanban-slice.ts +++ b/apps/web/lib/state/slices/kanban/kanban-slice.ts @@ -13,16 +13,13 @@ export const createKanbanSlice: StateCreator< [["zustand/immer", never]], [], KanbanSlice -> = (set, get) => ({ +> = (set) => ({ ...defaultKanbanState, - setActiveWorkflow: (workflowId) => { - if (get().workflows.activeId === workflowId) { - return; - } + setActiveWorkflow: (workflowId) => set((draft) => { + if (draft.workflows.activeId === workflowId) return; draft.workflows.activeId = workflowId; - }); - }, + }), setWorkflows: (workflows) => set((draft) => { draft.workflows.items = workflows; diff --git a/apps/web/lib/ws/handlers/workflows.test.ts b/apps/web/lib/ws/handlers/workflows.test.ts index 340d54d2ea..0188693ef5 100644 --- a/apps/web/lib/ws/handlers/workflows.test.ts +++ b/apps/web/lib/ws/handlers/workflows.test.ts @@ -35,6 +35,47 @@ function updatedMessage(payload: WorkflowPayload): BackendMessageMap["workflow.u }; } +function createdMessage(payload: WorkflowPayload): BackendMessageMap["workflow.created"] { + return { + id: "msg-1", + type: "notification", + action: "workflow.created", + payload, + timestamp: "2026-01-01T00:00:00Z", + }; +} + +describe("workflow.created handler — preserves user filter", () => { + it("does not promote a new workflow when activeId is null ('All Workflows')", () => { + const store = makeStore( + [{ id: "wf-1", workspaceId: "ws-1", name: "Existing", hidden: false }], + null, + ); + const handlers = registerWorkflowsHandlers(store); + + handlers["workflow.created"]?.( + createdMessage({ id: "wf-2", workspace_id: "ws-1", name: "Brand New" }), + ); + + expect(store.getState().workflows.activeId).toBeNull(); + expect(store.getState().workflows.items.map((i) => i.id)).toEqual(["wf-2", "wf-1"]); + }); + + it("leaves an existing activeId untouched when a new workflow appears", () => { + const store = makeStore( + [{ id: "wf-1", workspaceId: "ws-1", name: "Existing", hidden: false }], + "wf-1", + ); + const handlers = registerWorkflowsHandlers(store); + + handlers["workflow.created"]?.( + createdMessage({ id: "wf-2", workspace_id: "ws-1", name: "Brand New" }), + ); + + expect(store.getState().workflows.activeId).toBe("wf-1"); + }); +}); + describe("workflow.updated handler — hidden flag reconciles activeId", () => { it("clears activeId to next visible workflow when active becomes hidden", () => { const store = makeStore( diff --git a/apps/web/lib/ws/handlers/workflows.ts b/apps/web/lib/ws/handlers/workflows.ts index 71c98c3c81..ead4e004f8 100644 --- a/apps/web/lib/ws/handlers/workflows.ts +++ b/apps/web/lib/ws/handlers/workflows.ts @@ -23,6 +23,7 @@ function applyWorkflowCreated(state: AppState, payload: WorkflowPayload): AppSta if (state.workspaces.activeId !== payload.workspace_id) return state; if (state.workflows.items.some((item) => item.id === payload.id)) return state; const isHidden = Boolean(payload.hidden); + // Never use `??` here: null is a valid "All Workflows" selection, not a missing value. return { ...state, workflows: { @@ -35,9 +36,7 @@ function applyWorkflowCreated(state: AppState, payload: WorkflowPayload): AppSta }, ...state.workflows.items, ], - // Hidden workflows must never be promoted to the active selection; - // they are system-only and would surface in the workflow picker. - activeId: state.workflows.activeId ?? (isHidden ? null : payload.id), + activeId: state.workflows.activeId, }, }; }