fix: sidebar task delete/archive redirects to next task or home (#351)
This commit is contained in:
@@ -304,27 +304,35 @@ function useSheetActions(workspaceId: string | null, onOpenChange: (open: boolea
|
||||
|
||||
const handleArchiveTask = useCallback(
|
||||
async (taskId: string) => {
|
||||
// Capture active state before the async API call — the WS "task.updated"
|
||||
// handler removes the archived task from kanbans before removeTaskFromBoard runs.
|
||||
const { activeTaskId: wasActiveTaskId, activeSessionId: wasActiveSessionId } =
|
||||
store.getState().tasks;
|
||||
try {
|
||||
await archiveTaskById(taskId);
|
||||
await removeTaskFromBoard(taskId);
|
||||
await removeTaskFromBoard(taskId, { wasActiveTaskId, wasActiveSessionId });
|
||||
} catch (error) {
|
||||
console.error("Failed to archive task:", error);
|
||||
}
|
||||
},
|
||||
[archiveTaskById, removeTaskFromBoard],
|
||||
[archiveTaskById, removeTaskFromBoard, store],
|
||||
);
|
||||
|
||||
const handleDeleteTask = useCallback(
|
||||
async (taskId: string) => {
|
||||
setDeletingTaskId(taskId);
|
||||
// Capture active state before the async API call — the WS "task.deleted"
|
||||
// handler may clear activeTaskId/activeSessionId before removeTaskFromBoard runs.
|
||||
const { activeTaskId: wasActiveTaskId, activeSessionId: wasActiveSessionId } =
|
||||
store.getState().tasks;
|
||||
try {
|
||||
await deleteTaskById(taskId);
|
||||
await removeTaskFromBoard(taskId);
|
||||
await removeTaskFromBoard(taskId, { wasActiveTaskId, wasActiveSessionId });
|
||||
} finally {
|
||||
setDeletingTaskId(null);
|
||||
}
|
||||
},
|
||||
[deleteTaskById, removeTaskFromBoard],
|
||||
[deleteTaskById, removeTaskFromBoard, store],
|
||||
);
|
||||
|
||||
const { handleWorkspaceChange, handleTaskCreated } = useWorkspaceAndTaskCreatedActions({
|
||||
|
||||
@@ -271,27 +271,35 @@ function useSidebarActions(store: StoreApi) {
|
||||
|
||||
const handleArchiveTask = useCallback(
|
||||
async (taskId: string) => {
|
||||
// Capture active state before the async API call — the WS "task.updated"
|
||||
// handler removes the archived task from kanbans before removeTaskFromBoard runs.
|
||||
const { activeTaskId: wasActiveTaskId, activeSessionId: wasActiveSessionId } =
|
||||
store.getState().tasks;
|
||||
try {
|
||||
await archiveTaskById(taskId);
|
||||
await removeTaskFromBoard(taskId);
|
||||
await removeTaskFromBoard(taskId, { wasActiveTaskId, wasActiveSessionId });
|
||||
} catch (error) {
|
||||
console.error("Failed to archive task:", error);
|
||||
}
|
||||
},
|
||||
[archiveTaskById, removeTaskFromBoard],
|
||||
[archiveTaskById, removeTaskFromBoard, store],
|
||||
);
|
||||
|
||||
const handleDeleteTask = useCallback(
|
||||
async (taskId: string) => {
|
||||
setDeletingTaskId(taskId);
|
||||
// Capture active state before the async API call — the WS "task.deleted"
|
||||
// handler may clear activeTaskId/activeSessionId before removeTaskFromBoard runs.
|
||||
const { activeTaskId: wasActiveTaskId, activeSessionId: wasActiveSessionId } =
|
||||
store.getState().tasks;
|
||||
try {
|
||||
await deleteTaskById(taskId);
|
||||
await removeTaskFromBoard(taskId);
|
||||
await removeTaskFromBoard(taskId, { wasActiveTaskId, wasActiveSessionId });
|
||||
} finally {
|
||||
setDeletingTaskId(null);
|
||||
}
|
||||
},
|
||||
[deleteTaskById, removeTaskFromBoard],
|
||||
[deleteTaskById, removeTaskFromBoard, store],
|
||||
);
|
||||
|
||||
const [renamingTask, setRenamingTask] = useState<{ id: string; title: string } | null>(null);
|
||||
|
||||
@@ -68,6 +68,28 @@ export class SessionPage {
|
||||
return this.chat.getByText("Plan mode", { exact: true });
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a task via the sidebar context menu.
|
||||
* Hovers to reveal the menu trigger, opens it, and clicks "Delete".
|
||||
*/
|
||||
async deleteTaskInSidebar(title: string): Promise<void> {
|
||||
const taskRow = this.sidebar.locator('[role="button"]').filter({ hasText: title });
|
||||
await taskRow.hover();
|
||||
await taskRow.getByRole("button", { name: "Task actions" }).click();
|
||||
await this.page.getByRole("menuitem", { name: "Delete" }).click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Archive a task via the sidebar context menu.
|
||||
* Hovers to reveal the menu trigger, opens it, and clicks "Archive".
|
||||
*/
|
||||
async archiveTaskInSidebar(title: string): Promise<void> {
|
||||
const taskRow = this.sidebar.locator('[role="button"]').filter({ hasText: title });
|
||||
await taskRow.hover();
|
||||
await taskRow.getByRole("button", { name: "Task actions" }).click();
|
||||
await this.page.getByRole("menuitem", { name: "Archive" }).click();
|
||||
}
|
||||
|
||||
stepperStep(name: string): Locator {
|
||||
return this.page.getByTestId(`workflow-step-${name}`);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { test, expect } from "../fixtures/test-base";
|
||||
import { KanbanPage } from "../pages/kanban-page";
|
||||
import { SessionPage } from "../pages/session-page";
|
||||
|
||||
test.describe("Archive task redirect", () => {
|
||||
/**
|
||||
* Verifies that archiving a task from the sidebar correctly redirects:
|
||||
* - When other tasks remain: switches to the next available task's session
|
||||
* and the chat panel shows that task's agent conversation.
|
||||
* - When no tasks remain: redirects to the home page.
|
||||
*/
|
||||
test("archiving active task redirects to remaining task, archiving last task redirects home", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(90_000);
|
||||
|
||||
// Use distinct descriptions so we can verify the chat panel switches content.
|
||||
await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Archive Task A",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: 'e2e:message("alpha archive response")',
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
|
||||
await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Archive Task B",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: 'e2e:message("beta archive response")',
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
|
||||
// --- Navigate to kanban and wait for both task cards ---
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
|
||||
const cardA = kanban.taskCardByTitle("Archive Task A");
|
||||
const cardB = kanban.taskCardByTitle("Archive Task B");
|
||||
await expect(cardA).toBeVisible({ timeout: 30_000 });
|
||||
await expect(cardB).toBeVisible({ timeout: 30_000 });
|
||||
|
||||
// Click task A to open its session detail page
|
||||
await cardA.click();
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
|
||||
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
|
||||
// Verify we're viewing Task A's session — the chat shows its agent response.
|
||||
// Use .last() because the response text also appears inside the user message
|
||||
// prompt text (e2e:message("alpha archive response")).
|
||||
await expect(session.chat.getByText("alpha archive response").last()).toBeVisible({
|
||||
timeout: 30_000,
|
||||
});
|
||||
|
||||
// Both tasks should be visible in the sidebar
|
||||
await expect(session.taskInSidebar("Archive Task A")).toBeVisible({ timeout: 15_000 });
|
||||
await expect(session.taskInSidebar("Archive Task B")).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Record the current URL so we can verify it changes after archival
|
||||
const urlBeforeArchive = testPage.url();
|
||||
|
||||
// --- Archive the active task (A) — should switch to task B ---
|
||||
await session.archiveTaskInSidebar("Archive Task A");
|
||||
|
||||
// Task A should disappear from sidebar
|
||||
await expect(session.taskInSidebar("Archive Task A")).not.toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Task B should still be visible in sidebar
|
||||
await expect(session.taskInSidebar("Archive Task B")).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// The chat panel should now show Task B's session content
|
||||
await expect(session.chat.getByText("beta archive response").last()).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
|
||||
// The URL should have changed to Task B's session (still a /s/ route, but different ID)
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 10_000 });
|
||||
expect(testPage.url()).not.toBe(urlBeforeArchive);
|
||||
|
||||
// --- Archive the last remaining task (B) — should redirect home ---
|
||||
await session.archiveTaskInSidebar("Archive Task B");
|
||||
|
||||
// Should redirect to the home page
|
||||
await expect(testPage).not.toHaveURL(/\/s\//, { timeout: 15_000 });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,104 @@
|
||||
import { test, expect } from "../fixtures/test-base";
|
||||
import { KanbanPage } from "../pages/kanban-page";
|
||||
import { SessionPage } from "../pages/session-page";
|
||||
|
||||
test.describe("Delete task redirect", () => {
|
||||
/**
|
||||
* Verifies that deleting a task from the sidebar correctly redirects:
|
||||
* - When other tasks remain: switches to the next available task's session
|
||||
* and the chat panel shows that task's agent conversation.
|
||||
* - When no tasks remain: redirects to the home page.
|
||||
*
|
||||
* This tests the fix for a race condition where the WS "task.deleted"
|
||||
* handler would clear activeTaskId before removeTaskFromBoard could check
|
||||
* it, causing the user to stay on a stale/deleted task page.
|
||||
*/
|
||||
test("deleting active task redirects to remaining task, deleting last task redirects home", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(90_000);
|
||||
|
||||
// Use distinct descriptions so we can verify the chat panel switches content.
|
||||
// The mock agent echoes e2e:message() text back, giving each task a unique
|
||||
// response we can assert on.
|
||||
await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Delete Task A",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: 'e2e:message("alpha session response")',
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
|
||||
await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Delete Task B",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: 'e2e:message("beta session response")',
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
|
||||
// --- Navigate to kanban and wait for both task cards ---
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
|
||||
const cardA = kanban.taskCardByTitle("Delete Task A");
|
||||
const cardB = kanban.taskCardByTitle("Delete Task B");
|
||||
await expect(cardA).toBeVisible({ timeout: 30_000 });
|
||||
await expect(cardB).toBeVisible({ timeout: 30_000 });
|
||||
|
||||
// Click task A to open its session detail page
|
||||
await cardA.click();
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
|
||||
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
|
||||
// Verify we're viewing Task A's session — the chat shows its agent response.
|
||||
// Use .last() because "alpha session response" also appears inside the user
|
||||
// message prompt text (e2e:message("alpha session response")).
|
||||
await expect(session.chat.getByText("alpha session response").last()).toBeVisible({
|
||||
timeout: 30_000,
|
||||
});
|
||||
|
||||
// Both tasks should be visible in the sidebar
|
||||
await expect(session.taskInSidebar("Delete Task A")).toBeVisible({ timeout: 15_000 });
|
||||
await expect(session.taskInSidebar("Delete Task B")).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Record the current URL so we can verify it changes after deletion
|
||||
const urlBeforeDelete = testPage.url();
|
||||
|
||||
// --- Delete the active task (A) — should switch to task B ---
|
||||
await session.deleteTaskInSidebar("Delete Task A");
|
||||
|
||||
// Task A should disappear from sidebar
|
||||
await expect(session.taskInSidebar("Delete Task A")).not.toBeVisible({ timeout: 15_000 });
|
||||
|
||||
// Task B should still be visible in sidebar
|
||||
await expect(session.taskInSidebar("Delete Task B")).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// The chat panel should now show Task B's session content
|
||||
await expect(session.chat.getByText("beta session response").last()).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
|
||||
// The URL should have changed to Task B's session (still a /s/ route, but different ID)
|
||||
await expect(testPage).toHaveURL(/\/s\//, { timeout: 10_000 });
|
||||
expect(testPage.url()).not.toBe(urlBeforeDelete);
|
||||
|
||||
// --- Delete the last remaining task (B) — should redirect home ---
|
||||
await session.deleteTaskInSidebar("Delete Task B");
|
||||
|
||||
// Should redirect to the home page
|
||||
await expect(testPage).not.toHaveURL(/\/s\//, { timeout: 15_000 });
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,17 @@ type TaskRemovalOptions = {
|
||||
useLayoutSwitch?: boolean;
|
||||
};
|
||||
|
||||
type RemoveFromBoardOptions = {
|
||||
/**
|
||||
* The active task ID captured **before** the async delete API call.
|
||||
* Avoids a race with the WS "task.deleted" handler that may clear
|
||||
* activeTaskId before removeTaskFromBoard runs.
|
||||
*/
|
||||
wasActiveTaskId?: string | null;
|
||||
/** The active session ID captured before the async delete API call. */
|
||||
wasActiveSessionId?: string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* Hook that provides shared logic for removing a task from the kanban board
|
||||
* (after archive or delete) and switching to the next available task.
|
||||
@@ -100,9 +111,13 @@ export function useTaskRemoval({ store, useLayoutSwitch = false }: TaskRemovalOp
|
||||
/**
|
||||
* Remove a task from the kanban board state (both single and multi snapshots)
|
||||
* and switch to the next available task if the removed task was active.
|
||||
*
|
||||
* Pass `opts.wasActiveTaskId` / `opts.wasActiveSessionId` when calling after
|
||||
* an async API call (e.g. deleteTaskById) — the WS "task.deleted" handler may
|
||||
* clear activeTaskId before this function runs.
|
||||
*/
|
||||
const removeTaskFromBoard = useCallback(
|
||||
async (taskId: string) => {
|
||||
async (taskId: string, opts?: RemoveFromBoardOptions) => {
|
||||
removeTaskFromSnapshots(taskId);
|
||||
|
||||
// Collect remaining tasks across snapshots
|
||||
@@ -114,11 +129,20 @@ export function useTaskRemoval({ store, useLayoutSwitch = false }: TaskRemovalOp
|
||||
allRemainingTasks.push(...store.getState().kanban.tasks);
|
||||
}
|
||||
|
||||
// If removed task was active, switch to another task or go home
|
||||
const currentActiveTaskId = store.getState().tasks.activeTaskId;
|
||||
if (currentActiveTaskId !== taskId) return;
|
||||
// Use the caller-provided active task ID (captured before the async API
|
||||
// call) to avoid the race with the WS handler that may have already
|
||||
// cleared it. Fall back to the current store value for callers that
|
||||
// don't provide it (e.g. archive, which doesn't go through the API).
|
||||
const activeTaskId =
|
||||
opts?.wasActiveTaskId !== undefined
|
||||
? opts.wasActiveTaskId
|
||||
: store.getState().tasks.activeTaskId;
|
||||
if (activeTaskId !== taskId) return;
|
||||
|
||||
const oldSessionId = store.getState().tasks.activeSessionId;
|
||||
const oldSessionId =
|
||||
opts?.wasActiveSessionId !== undefined
|
||||
? opts.wasActiveSessionId
|
||||
: store.getState().tasks.activeSessionId;
|
||||
if (allRemainingTasks.length > 0) {
|
||||
await switchToNextTask(allRemainingTasks[0], oldSessionId);
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user