fix(web): tab session bugs — primary star drift + tab close re-creation (#800)
* fix(web): preserve primarySessionId across kanban.update broadcasts The kanban.update WS handler rebuilt kanban.tasks without primarySessionId / primarySessionState, wiping values set by task.updated. After any board refresh the primary star vanished and selectTaskWithLayout fell into the slower async fallback path. Move task building inside setState so existing values are preserved in both single and multi snapshots. * fix(web): prevent deleted session panel from being re-created handleDelete called removeTaskSession before containerApi.removePanel, which let useAutoSessionTab re-run with effectiveSessionId still pointing at the deleted session and re-create its panel. The setupChatPanelSafetyNet had a matching gap — it re-added a panel for activeSessionId without checking the session still existed. Switch the active session (or clear it) before removeTaskSession so effectiveSessionId updates first. Add guards to useAutoSessionTab and the safety net so neither creates a panel for a session that is no longer in the store. * test(web): add session tab management regression suite Cover the three bugs fixed in this branch end-to-end: deleting a non-active session keeps it gone, deleting the active session switches focus to the remainder, deleting then immediately switching tasks does not resurrect the tab, and the primary star survives both a kanban.update broadcast (triggered via apiClient.moveTask) and a task-switch round trip. * fix(web): hand off deletion to newest remaining session, document multi-snapshot fallback - session-tab.tsx: sort remaining sessions by started_at DESC and pick the newest, instead of relying on the unspecified store ordering. - kanban.ts: comment the multi-snapshot primarySessionId fallback so the double lookup does not look like redundancy. Both per Claude review feedback on #800. * fix(web): preserve explicit-null primary, abort delete on WS failure - kanban.ts: switch the multi-snapshot fallback from `??` to an explicit `=== undefined` check. An explicitly-cleared null primary must NOT be replaced by a stale snapshot value, only an absent key. - session-tab.tsx: wsAction now returns a boolean; handleDelete bails out before mutating the store / dockview when the backend session.delete request fails. Prevents client/server desync from a transient WS failure. - kanban.test.ts: add regression covering the null-vs-undefined merge. Per CodeRabbit and Greptile review on #800. * style(web): drop redundant .slice() in session deletion sort chain filter() already returns a fresh array, so the subsequent sort() does not mutate shared store state. Per Greptile review on #800.
This commit is contained in:
@@ -59,6 +59,13 @@ export function setupChatPanelSafetyNet(
|
||||
// If all sessions were deleted, leave the layout empty — the user
|
||||
// can create a new session via the "+" menu.
|
||||
if (!activeSessionId) return;
|
||||
// Don't recreate a panel for a session that no longer exists in the
|
||||
// store — this guards against handleDelete racing with the safety net.
|
||||
const activeTaskId = appStore.getState().tasks.activeTaskId;
|
||||
const knownSessions = activeTaskId
|
||||
? (appStore.getState().taskSessionsByTask.itemsByTaskId[activeTaskId] ?? [])
|
||||
: [];
|
||||
if (!knownSessions.some((s) => s.id === activeSessionId)) return;
|
||||
api.addPanel({
|
||||
id: `session:${activeSessionId}`,
|
||||
component: "chat",
|
||||
@@ -285,6 +292,12 @@ export function useAutoSessionTab(effectiveSessionId: string | null) {
|
||||
|
||||
if (!effectiveSessionId) return;
|
||||
|
||||
// Don't create a panel for a session that no longer exists in the store —
|
||||
// guards against a race where removeTaskSession fires before the active
|
||||
// session is switched, which would cause the deleted session's panel to
|
||||
// be re-created by ensureSessionPanel.
|
||||
if (!currentSessionIds.includes(effectiveSessionId)) return;
|
||||
|
||||
// Remove the generic "chat" placeholder as soon as a real session is
|
||||
// active — per-session tabs replace it. Skip in maximized state to
|
||||
// preserve the saved maximize layout.
|
||||
|
||||
@@ -21,7 +21,7 @@ import {
|
||||
AlertDialogHeader,
|
||||
AlertDialogTitle,
|
||||
} from "@kandev/ui/alert-dialog";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
import type { TaskSessionState } from "@/lib/types/http";
|
||||
@@ -99,18 +99,26 @@ function useSessionTabActions(
|
||||
) {
|
||||
const { toast, updateToast } = useToast();
|
||||
const removeTaskSession = useAppStore((state) => state.removeTaskSession);
|
||||
const appStoreApi = useAppStoreApi();
|
||||
|
||||
const wsAction = useCallback(
|
||||
async (action: string, label: string, payload: Record<string, unknown>, timeout = 15000) => {
|
||||
async (
|
||||
action: string,
|
||||
label: string,
|
||||
payload: Record<string, unknown>,
|
||||
timeout = 15000,
|
||||
): Promise<boolean> => {
|
||||
const client = getWebSocketClient();
|
||||
if (!client) return;
|
||||
if (!client) return false;
|
||||
const toastId = toast({ title: `${label}...`, variant: "loading" });
|
||||
try {
|
||||
await client.request(action, payload, timeout);
|
||||
updateToast(toastId, { title: `${label} successful`, variant: "success" });
|
||||
return true;
|
||||
} catch (error) {
|
||||
const msg = error instanceof Error ? error.message : "Unknown error";
|
||||
updateToast(toastId, { title: `${label} failed`, description: msg, variant: "error" });
|
||||
return false;
|
||||
}
|
||||
},
|
||||
[toast, updateToast],
|
||||
@@ -138,11 +146,32 @@ function useSessionTabActions(
|
||||
);
|
||||
const handleDelete = useCallback(async () => {
|
||||
if (!sessionId || !taskId) return;
|
||||
await wsAction("session.delete", "Deleting session", { session_id: sessionId });
|
||||
// Only mutate local state once the backend confirms the delete — otherwise
|
||||
// a transient WS failure leaves the UI desynced from the server.
|
||||
const ok = await wsAction("session.delete", "Deleting session", { session_id: sessionId });
|
||||
if (!ok) return;
|
||||
|
||||
// Switch the active session BEFORE removing from the store so
|
||||
// useAutoSessionTab doesn't re-create this panel with the deleted session's ID.
|
||||
// Hand off to the most-recently-started remaining session — store ordering
|
||||
// is unspecified, so sort explicitly rather than rely on array position.
|
||||
const state = appStoreApi.getState();
|
||||
if (state.tasks.activeSessionId === sessionId) {
|
||||
const sessions = state.taskSessionsByTask.itemsByTaskId[taskId] ?? [];
|
||||
const remaining = sessions
|
||||
.filter((s) => s.id !== sessionId)
|
||||
.sort((a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime());
|
||||
if (remaining.length > 0) {
|
||||
state.setActiveSessionAuto(taskId, remaining[0].id);
|
||||
} else {
|
||||
state.clearActiveSession();
|
||||
}
|
||||
}
|
||||
|
||||
removeTaskSession(taskId, sessionId);
|
||||
const panel = containerApi.getPanel(api.id);
|
||||
if (panel) containerApi.removePanel(panel);
|
||||
}, [sessionId, taskId, wsAction, removeTaskSession, api.id, containerApi]);
|
||||
}, [sessionId, taskId, wsAction, removeTaskSession, api.id, containerApi, appStoreApi]);
|
||||
const handleCloseOthers = useCallback(() => {
|
||||
const toClose = api.group.panels.filter((p) => p.id !== api.id);
|
||||
for (const panel of toClose) containerApi.removePanel(panel);
|
||||
|
||||
@@ -0,0 +1,340 @@
|
||||
import { expect } from "@playwright/test";
|
||||
import type { Page } from "@playwright/test";
|
||||
import { test } from "../../fixtures/test-base";
|
||||
import type { SeedData } from "../../fixtures/test-base";
|
||||
import type { ApiClient } from "../../helpers/api-client";
|
||||
import { KanbanPage } from "../../pages/kanban-page";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
|
||||
const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"];
|
||||
|
||||
/**
|
||||
* Regression suite for tab session management bugs:
|
||||
* - Bug A: kanban.update wiped primarySessionId, dropping the primary star.
|
||||
* - Bug B: handleDelete fired removeTaskSession before removePanel, so
|
||||
* useAutoSessionTab re-created the deleted session's panel.
|
||||
* - Bug C: setupChatPanelSafetyNet recreated panels for sessions that no
|
||||
* longer existed in the store.
|
||||
*/
|
||||
|
||||
type SetupResult = {
|
||||
task: { id: string };
|
||||
session: SessionPage;
|
||||
session1Id: string;
|
||||
session2Id: string;
|
||||
};
|
||||
|
||||
async function createTaskWithTwoSessions(
|
||||
testPage: Page,
|
||||
apiClient: ApiClient,
|
||||
seedData: SeedData,
|
||||
title: string,
|
||||
): Promise<SetupResult> {
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
title,
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
|
||||
// First session must reach a done state before we open the second.
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
return DONE_STATES.includes(sessions[0]?.state ?? "");
|
||||
},
|
||||
{ timeout: 30_000, message: "Waiting for first session to finish" },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
const card = kanban.taskCardByTitle(title);
|
||||
await expect(card).toBeVisible({ timeout: 10_000 });
|
||||
await card.click();
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.chat.getByText("simple mock response", { exact: false })).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
|
||||
// Launch the second session via the new-session dialog.
|
||||
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 });
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
return sessions.filter((s) => DONE_STATES.includes(s.state)).length;
|
||||
},
|
||||
{ timeout: 60_000, message: "Waiting for both sessions to finish" },
|
||||
)
|
||||
.toBe(2);
|
||||
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
const sorted = sessions.sort(
|
||||
(a, b) => new Date(a.started_at).getTime() - new Date(b.started_at).getTime(),
|
||||
);
|
||||
return { task, session, session1Id: sorted[0].id, session2Id: sorted[1].id };
|
||||
}
|
||||
|
||||
function starInTab(session: SessionPage, sessionId: string) {
|
||||
return session.sessionTabBySessionId(sessionId).locator(".tabler-icon-star").first();
|
||||
}
|
||||
|
||||
test.describe("Session tab management — close behavior", () => {
|
||||
test("deleting a non-active session removes its tab and stays gone", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
|
||||
const { task, session, session1Id, session2Id } = await createTaskWithTwoSessions(
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
"Tab Stays Gone",
|
||||
);
|
||||
|
||||
// Make session #2 active so #1 is the non-active deletion target.
|
||||
await session.sessionTabBySessionId(session2Id).click();
|
||||
|
||||
await session.sessionTabBySessionId(session1Id).click({ button: "right" });
|
||||
await session.contextMenuItem("Delete").click();
|
||||
const dialog = session.alertDialog();
|
||||
await expect(dialog).toBeVisible({ timeout: 5_000 });
|
||||
await dialog.getByRole("button", { name: "Delete" }).click();
|
||||
|
||||
// Tab disappears…
|
||||
await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// …and stays gone — useAutoSessionTab must not recreate it.
|
||||
await testPage.waitForTimeout(800);
|
||||
await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible();
|
||||
await expect(session.sessionTabBySessionId(session2Id)).toBeVisible();
|
||||
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
expect(sessions).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("deleting the active session switches focus to the remaining session", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
|
||||
const { task, session, session1Id, session2Id } = await createTaskWithTwoSessions(
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
"Delete Active Session",
|
||||
);
|
||||
|
||||
// Make session #1 active so the deletion target IS the active one.
|
||||
await session.sessionTabBySessionId(session1Id).click();
|
||||
|
||||
await session.sessionTabBySessionId(session1Id).click({ button: "right" });
|
||||
await session.contextMenuItem("Delete").click();
|
||||
await expect(session.alertDialog()).toBeVisible({ timeout: 5_000 });
|
||||
await session.alertDialog().getByRole("button", { name: "Delete" }).click();
|
||||
|
||||
await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible({ timeout: 15_000 });
|
||||
await expect(session.sessionTabBySessionId(session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
|
||||
// URL must not have switched to a different task.
|
||||
await expect(testPage).toHaveURL(new RegExp(`/t/${task.id}`));
|
||||
|
||||
// Backend must reflect the deletion — exactly one session remains.
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
expect(sessions.map((s) => s.id)).toEqual([session2Id]);
|
||||
});
|
||||
|
||||
test("deleting then immediately switching tasks does not resurrect the tab", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
|
||||
const {
|
||||
task: task1,
|
||||
session,
|
||||
session1Id,
|
||||
session2Id,
|
||||
} = await createTaskWithTwoSessions(testPage, apiClient, seedData, "Delete Then Switch A");
|
||||
|
||||
// Second task to switch into mid-delete.
|
||||
const task2 = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Delete Then Switch B",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task2.id);
|
||||
return DONE_STATES.includes(sessions[0]?.state ?? "");
|
||||
},
|
||||
{ timeout: 30_000, message: "Waiting for task2 session" },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
await session.sessionTabBySessionId(session1Id).click({ button: "right" });
|
||||
await session.contextMenuItem("Delete").click();
|
||||
await expect(session.alertDialog()).toBeVisible({ timeout: 5_000 });
|
||||
await session.alertDialog().getByRole("button", { name: "Delete" }).click();
|
||||
|
||||
// Wait for backend to confirm deletion (don't wait for tab to disappear).
|
||||
await expect
|
||||
.poll(async () => (await apiClient.listTaskSessions(task1.id)).sessions.length, {
|
||||
timeout: 10_000,
|
||||
})
|
||||
.toBe(1);
|
||||
|
||||
// Switch to task2 then back to task1 — the deleted tab must not return.
|
||||
await session.clickTaskInSidebar("Delete Then Switch B");
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await session.waitForLoad();
|
||||
|
||||
await session.clickTaskInSidebar("Delete Then Switch A");
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await session.waitForLoad();
|
||||
|
||||
await expect(session.sessionTabBySessionId(session2Id)).toBeVisible({ timeout: 10_000 });
|
||||
await testPage.waitForTimeout(800);
|
||||
await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("Session tab management — primary session persistence", () => {
|
||||
test("primary star survives a kanban.update broadcast", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
|
||||
const { task, session, session1Id, session2Id } = await createTaskWithTwoSessions(
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
"Primary Through Kanban Update",
|
||||
);
|
||||
|
||||
// Session #1 is auto-primary; promote session #2 via the context menu so we
|
||||
// can verify the new primary survives the next kanban.update broadcast.
|
||||
await session.sessionTabBySessionId(session2Id).click({ button: "right" });
|
||||
await expect(session.contextMenuItem("Set as Primary")).toBeVisible({ timeout: 5_000 });
|
||||
await session.contextMenuItem("Set as Primary").click();
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task.id);
|
||||
return sessions.find((s) => s.id === session2Id)?.is_primary ?? false;
|
||||
},
|
||||
{ timeout: 10_000, message: "Waiting for primary set in backend" },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
await expect(starInTab(session, session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
await expect(starInTab(session, session1Id)).not.toBeVisible({ timeout: 5_000 });
|
||||
|
||||
// Trigger kanban.update by moving the task to a non-start step.
|
||||
const otherStep = seedData.steps.find((s) => s.id !== seedData.startStepId);
|
||||
if (!otherStep) throw new Error("Workflow needs at least 2 steps to trigger kanban.update");
|
||||
await apiClient.moveTask(task.id, seedData.workflowId, otherStep.id);
|
||||
|
||||
// Give the WS broadcast time to land.
|
||||
await testPage.waitForTimeout(500);
|
||||
|
||||
// Star must still be on session #2 (would jump back to #1 before the kanban.ts fix).
|
||||
await expect(starInTab(session, session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
await expect(starInTab(session, session1Id)).not.toBeVisible();
|
||||
});
|
||||
|
||||
test("primary star survives switching tasks and returning", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
|
||||
const {
|
||||
task: task1,
|
||||
session,
|
||||
session1Id,
|
||||
session2Id,
|
||||
} = await createTaskWithTwoSessions(testPage, apiClient, seedData, "Primary Round Trip A");
|
||||
|
||||
const task2 = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Primary Round Trip B",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task2.id);
|
||||
return DONE_STATES.includes(sessions[0]?.state ?? "");
|
||||
},
|
||||
{ timeout: 30_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
// Session #1 is auto-primary; promote session #2 (matches the user's bug
|
||||
// scenario: setting the second session as primary).
|
||||
await session.sessionTabBySessionId(session2Id).click({ button: "right" });
|
||||
await session.contextMenuItem("Set as Primary").click();
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { sessions } = await apiClient.listTaskSessions(task1.id);
|
||||
return sessions.find((s) => s.id === session2Id)?.is_primary ?? false;
|
||||
},
|
||||
{ timeout: 10_000 },
|
||||
)
|
||||
.toBe(true);
|
||||
await expect(starInTab(session, session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
|
||||
// A → B → A round trip.
|
||||
await session.clickTaskInSidebar("Primary Round Trip B");
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await session.waitForLoad();
|
||||
|
||||
await session.clickTaskInSidebar("Primary Round Trip A");
|
||||
await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 });
|
||||
await session.waitForLoad();
|
||||
|
||||
await expect(session.sessionTabBySessionId(session1Id)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.sessionTabBySessionId(session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
await expect(starInTab(session, session2Id)).toBeVisible({ timeout: 5_000 });
|
||||
await expect(starInTab(session, session1Id)).not.toBeVisible();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,220 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { StoreApi } from "zustand";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import { registerKanbanHandlers } from "./kanban";
|
||||
|
||||
function makeStore(initial: Partial<AppState> = {}) {
|
||||
let state = {
|
||||
kanban: { workflowId: null, steps: [], tasks: [] },
|
||||
kanbanMulti: { snapshots: {}, isLoading: false },
|
||||
...initial,
|
||||
} as unknown as AppState;
|
||||
|
||||
return {
|
||||
getState: () => state,
|
||||
setState: (updater: AppState | ((s: AppState) => AppState)) => {
|
||||
state =
|
||||
typeof updater === "function" ? (updater as (s: AppState) => AppState)(state) : updater;
|
||||
},
|
||||
subscribe: () => () => {},
|
||||
destroy: () => {},
|
||||
getInitialState: () => state,
|
||||
} as unknown as StoreApi<AppState>;
|
||||
}
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
function makeUpdateMessage(workflowId: string, tasks: unknown[], steps: unknown[] = []): any {
|
||||
return {
|
||||
id: "msg-1",
|
||||
type: "notification",
|
||||
action: "kanban.update",
|
||||
payload: { workflowId, tasks, steps },
|
||||
};
|
||||
}
|
||||
|
||||
describe("kanban.update handler — primarySessionId preservation", () => {
|
||||
it("preserves primarySessionId from existing tasks", () => {
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: "wf1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
position: 0,
|
||||
primarySessionId: "sess-primary",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [
|
||||
{ id: "t1", workflowStepId: "s1", title: "T1 updated", position: 0, state: "IN_PROGRESS" },
|
||||
]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === "t1");
|
||||
expect(task?.primarySessionId).toBe("sess-primary");
|
||||
expect(task?.title).toBe("T1 updated");
|
||||
});
|
||||
|
||||
it("preserves primarySessionState from existing tasks", () => {
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: "wf1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
position: 0,
|
||||
primarySessionId: "sess-primary",
|
||||
primarySessionState: "RUNNING",
|
||||
},
|
||||
],
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [{ id: "t1", workflowStepId: "s1", title: "T1", position: 0 }]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === "t1");
|
||||
expect(task?.primarySessionState).toBe("RUNNING");
|
||||
});
|
||||
|
||||
it("new tasks start with undefined primarySessionId", () => {
|
||||
const store = makeStore({
|
||||
kanban: { workflowId: "wf1", steps: [], tasks: [] },
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [
|
||||
{ id: "t-new", workflowStepId: "s1", title: "New Task", position: 0 },
|
||||
]),
|
||||
);
|
||||
|
||||
const task = store.getState().kanban.tasks.find((t) => t.id === "t-new");
|
||||
expect(task).toBeDefined();
|
||||
expect(task?.primarySessionId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — explicit-null primary preservation", () => {
|
||||
it("does not restore stale snapshot value when primarySessionId is explicitly cleared", () => {
|
||||
// Repro for the multi-snapshot null-preservation bug: when task.updated
|
||||
// clears primarySessionId to null in kanban.tasks, the multi-snapshot must
|
||||
// accept the null rather than fall back to a stale value.
|
||||
const store = makeStore({
|
||||
kanban: {
|
||||
workflowId: "wf1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
position: 0,
|
||||
primarySessionId: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
kanbanMulti: {
|
||||
isLoading: false,
|
||||
snapshots: {
|
||||
wf1: {
|
||||
workflowId: "wf1",
|
||||
workflowName: "WF1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
position: 0,
|
||||
primarySessionId: "stale-session",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [{ id: "t1", workflowStepId: "s1", title: "T1", position: 0 }]),
|
||||
);
|
||||
|
||||
const snapshot = store.getState().kanbanMulti.snapshots["wf1"];
|
||||
const task = snapshot?.tasks.find((t) => t.id === "t1");
|
||||
expect(task?.primarySessionId).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — multi-snapshot primary lookup", () => {
|
||||
it("preserves primarySessionId in kanbanMulti snapshot", () => {
|
||||
const store = makeStore({
|
||||
kanban: { workflowId: "wf1", steps: [], tasks: [] },
|
||||
kanbanMulti: {
|
||||
isLoading: false,
|
||||
snapshots: {
|
||||
wf1: {
|
||||
workflowId: "wf1",
|
||||
workflowName: "WF1",
|
||||
steps: [],
|
||||
tasks: [
|
||||
{
|
||||
id: "t1",
|
||||
workflowStepId: "s1",
|
||||
title: "T1",
|
||||
position: 0,
|
||||
primarySessionId: "sess-multi-primary",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [{ id: "t1", workflowStepId: "s1", title: "T1", position: 0 }]),
|
||||
);
|
||||
|
||||
const snapshot = store.getState().kanbanMulti.snapshots["wf1"];
|
||||
const task = snapshot?.tasks.find((t) => t.id === "t1");
|
||||
expect(task?.primarySessionId).toBe("sess-multi-primary");
|
||||
});
|
||||
});
|
||||
|
||||
describe("kanban.update handler — task filtering", () => {
|
||||
it("skips ephemeral tasks", () => {
|
||||
const store = makeStore({
|
||||
kanban: { workflowId: "wf1", steps: [], tasks: [] },
|
||||
} as Partial<AppState>);
|
||||
|
||||
const handler = registerKanbanHandlers(store)["kanban.update"]!;
|
||||
handler(
|
||||
makeUpdateMessage("wf1", [
|
||||
{ id: "t1", workflowStepId: "s1", title: "Real", position: 0 },
|
||||
{
|
||||
id: "t-ephemeral",
|
||||
workflowStepId: "s1",
|
||||
title: "Ephemeral",
|
||||
position: 1,
|
||||
is_ephemeral: true,
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
expect(store.getState().kanban.tasks).toHaveLength(1);
|
||||
expect(store.getState().kanban.tasks[0].id).toBe("t1");
|
||||
});
|
||||
});
|
||||
@@ -20,21 +20,31 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
show_in_command_panel: step.show_in_command_panel,
|
||||
agent_profile_id: step.agent_profile_id,
|
||||
}));
|
||||
const tasks: KanbanTask[] = message.payload.tasks
|
||||
// Filter out ephemeral tasks (e.g., quick chat)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.filter((task: any) => !task.is_ephemeral)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.map((task: any) => ({
|
||||
id: task.id,
|
||||
workflowStepId: task.workflowStepId,
|
||||
title: task.title,
|
||||
description: task.description,
|
||||
position: task.position ?? 0,
|
||||
state: task.state,
|
||||
}));
|
||||
|
||||
store.setState((state) => {
|
||||
// kanban.update doesn't carry primarySessionId / primarySessionState —
|
||||
// those are set by task.updated WS events. Build tasks inside setState
|
||||
// so we can read existing values and preserve them.
|
||||
const existingById = new Map(state.kanban.tasks.map((t) => [t.id, t]));
|
||||
const tasks: KanbanTask[] = message.payload.tasks
|
||||
// Filter out ephemeral tasks (e.g., quick chat)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.filter((task: any) => !task.is_ephemeral)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
.map((task: any) => {
|
||||
const existing = existingById.get(task.id);
|
||||
return {
|
||||
id: task.id,
|
||||
workflowStepId: task.workflowStepId,
|
||||
title: task.title,
|
||||
description: task.description,
|
||||
position: task.position ?? 0,
|
||||
state: task.state,
|
||||
primarySessionId: existing?.primarySessionId,
|
||||
primarySessionState: existing?.primarySessionState,
|
||||
};
|
||||
});
|
||||
|
||||
const next = {
|
||||
...state,
|
||||
kanban: { workflowId, steps, tasks },
|
||||
@@ -43,13 +53,30 @@ export function registerKanbanHandlers(store: StoreApi<AppState>): WsHandlers {
|
||||
// Also update multi-workflow snapshots if this workflow is tracked
|
||||
const snapshot = state.kanbanMulti.snapshots[workflowId];
|
||||
if (snapshot) {
|
||||
const existingMultiById = new Map(snapshot.tasks.map((t) => [t.id, t]));
|
||||
const multiTasks = tasks.map((t) => {
|
||||
const fallback = existingMultiById.get(t.id);
|
||||
// Fall back to the multi-snapshot's own value only when the main
|
||||
// kanban lookup returned `undefined` (task absent from kanban.tasks).
|
||||
// An explicit `null` means the primary was intentionally cleared
|
||||
// and must NOT be replaced by a stale snapshot value.
|
||||
return {
|
||||
...t,
|
||||
primarySessionId:
|
||||
t.primarySessionId === undefined ? fallback?.primarySessionId : t.primarySessionId,
|
||||
primarySessionState:
|
||||
t.primarySessionState === undefined
|
||||
? fallback?.primarySessionState
|
||||
: t.primarySessionState,
|
||||
};
|
||||
});
|
||||
return {
|
||||
...next,
|
||||
kanbanMulti: {
|
||||
...next.kanbanMulti,
|
||||
snapshots: {
|
||||
...next.kanbanMulti.snapshots,
|
||||
[workflowId]: { ...snapshot, steps, tasks },
|
||||
[workflowId]: { ...snapshot, steps, tasks: multiTasks },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user