diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 14c2b1f3..9a96ce70 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -263,6 +263,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ onReadResource: (uri: string) => void; onSetLogLevel: (level: string) => void; onCancelTask: (taskId: string) => void; + onCancelToolCall: () => void; onClearCompletedTasks: () => void; onRefreshTasks: () => void; onServerSettings: (id: string) => void; @@ -357,6 +358,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ call-as-task + @@ -756,6 +758,87 @@ describe("App task wiring", () => { ); }); + it("cancels the underlying task when a task-augmented tool call is cancelled", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + const client = clientInstances[0] as unknown as { + cancelRequestorTask: ReturnType; + }; + + // callToolStream surfaces the created task's id via `toolCallTaskUpdated` + // mid-call; App stashes it so Cancel knows which task to cancel (#1455). + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("toolCallTaskUpdated", { + detail: { taskId: "task-42", task: { taskId: "task-42" } }, + }), + ); + }); + + await user.click(screen.getByText("cancel-tool-call")); + + expect(client.cancelRequestorTask).toHaveBeenCalledWith("task-42"); + }); + + it("does not re-cancel on a rapid second Cancel click", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + const client = clientInstances[0] as unknown as { + cancelRequestorTask: ReturnType; + }; + + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("toolCallTaskUpdated", { + detail: { taskId: "task-42", task: { taskId: "task-42" } }, + }), + ); + }); + + // Two clicks before the call resolves must cancel only once — the second + // finds the ref already cleared, avoiding a spurious cancel of a terminal + // task. + await user.click(screen.getByText("cancel-tool-call")); + await user.click(screen.getByText("cancel-tool-call")); + + expect(client.cancelRequestorTask).toHaveBeenCalledTimes(1); + }); + + it("does not cancel a task when an ordinary tool call is cancelled", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + const client = clientInstances[0] as unknown as { + cancelRequestorTask: ReturnType; + }; + + // A stale task id from an earlier task call... + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("toolCallTaskUpdated", { + detail: { taskId: "old-task", task: { taskId: "old-task" } }, + }), + ); + }); + // ...is cleared when a new ordinary call starts, so its Cancel is a no-op. + await user.click(screen.getByText("call")); + await waitFor(() => + expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"), + ); + + await user.click(screen.getByText("cancel-tool-call")); + + expect(client.cancelRequestorTask).not.toHaveBeenCalled(); + }); + it("shows a URL-elicitation toast when a tool call fails with a no-list -32042", async () => { const user = userEvent.setup(); renderWithMantine(); @@ -941,6 +1024,57 @@ describe("App task wiring", () => { ), ); }); + + it("shows a 'Task cancelled' toast when a task is cancelled", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + // No live status toast for this task — cancellation shows a fresh one. + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("taskCancelled", { detail: { taskId: "task-1" } }), + ); + }); + + expect(notificationsMock.show).toHaveBeenCalledWith( + expect.objectContaining({ title: "Task cancelled", color: "gray" }), + ); + }); + + it("converts a running task's live toast into the cancellation toast", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + // A running task has an open "Task working" toast... + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("taskStatusChange", { + detail: { + taskId: "task-1", + task: { status: "working", statusMessage: "Interpreting" }, + }, + }), + ); + }); + const liveId = notificationsMock.show.mock.calls[0][0].id; + notificationsMock.show.mockClear(); + + // ...which the cancel replaces in place (update), not a stacked toast. + act(() => { + clientInstances[0].dispatchEvent( + new CustomEvent("taskCancelled", { detail: { taskId: "task-1" } }), + ); + }); + + expect(notificationsMock.show).not.toHaveBeenCalled(); + expect(notificationsMock.update).toHaveBeenCalledWith( + expect.objectContaining({ id: liveId, title: "Task cancelled" }), + ); + }); }); // Live-apply roots on settings-dialog close: the App diffs the final draft diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 78db6474..21571276 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -347,6 +347,10 @@ function formatErrorDetails(err: unknown): string { // progress stops (i.e. the call finished or went quiet). const PROGRESS_TOAST_AUTOCLOSE_MS = 5000; +// A task cancellation is a one-shot confirmation (unlike the live status toast, +// which stays open while the task runs), so it auto-dismisses after a moment. +const TASK_CANCELLED_TOAST_AUTOCLOSE_MS = 5000; + // Stable toast id for a progress stream. Notifications keyed by this id are // replaced (not stacked) so a chatty server updates one toast per stream // rather than flooding the corner. The injected `progressToken` correlates a @@ -585,6 +589,15 @@ function App() { // `notifications/tasks/status` tick replaces the existing toast. const taskToastIdsRef = useRef>(new Set()); + // The taskId of the in-flight task-augmented tool call, captured from the + // `toolCallTaskUpdated` event `callToolStream` dispatches. Lets the Tool + // detail panel's Cancel button cancel the underlying task on the server + // (#1455) instead of no-op'ing. Reset at the start of every call, so an + // ordinary (non-task) call leaves it undefined and its Cancel doesn't fire a + // stray task cancellation. A ref (not state) because it's only read at the + // moment Cancel is clicked and must not trigger re-renders. + const activeToolCallTaskIdRef = useRef(undefined); + // Per-task progress, keyed by taskId. Sourced from the core `requestorTaskProgress` // event (emitted by callToolStream, which owns the taskId), fed to the Tasks // screen so `TaskCard`'s progress bar renders for active tasks. Pruned on @@ -805,6 +818,30 @@ function App() { }; }, [inspectorClient]); + // Capture the in-flight task-augmented tool call's taskId so the detail + // panel's Cancel button can cancel the task on the server (#1455). The id + // only becomes known mid-call, when `callToolStream` dispatches + // `toolCallTaskUpdated`, so we stash the latest into the ref the cancel + // handler reads. `onCallTool` clears it at the start of each call. + useEffect(() => { + if (!inspectorClient) return; + const onToolCallTaskUpdated = ( + event: TypedEventGeneric, + ) => { + activeToolCallTaskIdRef.current = event.detail.taskId; + }; + inspectorClient.addEventListener( + "toolCallTaskUpdated", + onToolCallTaskUpdated, + ); + return () => { + inspectorClient.removeEventListener( + "toolCallTaskUpdated", + onToolCallTaskUpdated, + ); + }; + }, [inspectorClient]); + // Surface live task status as per-task toasts — the v2 replacement for v1/v1.5's // inline "Task status: … Polling…" line under the Tool Result (#1422, consistent // with #1414). Subscribes to `taskStatusChange` (server `notifications/tasks/status`) @@ -862,11 +899,47 @@ function App() { ) => { handleTaskUpdate(event.detail.taskId, event.detail.task); }; + // A cancel goes out as `taskCancelled` (dispatched by cancelRequestorTask), + // not as a status notification, so it would otherwise leave the running + // task's live "Task " toast hanging with no confirmation. Replace + // that toast (or show a fresh one) with a short "Task cancelled" toast, and + // prune the now-dead progress entry. Covers both cancel paths — the Tasks + // screen and the Tool detail panel — since both route through + // cancelRequestorTask (#1455). + const onTaskCancelled = ( + event: TypedEventGeneric, + ) => { + const { taskId } = event.detail; + setProgressByTaskId((prev) => { + if (!(taskId in prev)) return prev; + const next = { ...prev }; + delete next[taskId]; + return next; + }); + const id = taskToastId(taskId); + const toast = { + id, + title: "Task cancelled", + message: "The task was cancelled.", + color: taskToastColor("cancelled"), + autoClose: TASK_CANCELLED_TOAST_AUTOCLOSE_MS, + }; + if (liveToastIds.has(id)) { + // Convert the open status toast into the auto-closing confirmation; drop + // it from the live set so a trailing "cancelled" status tick (if the + // server sends one) doesn't re-show it. + notifications.update(toast); + liveToastIds.delete(id); + } else { + notifications.show(toast); + } + }; inspectorClient.addEventListener("taskStatusChange", onTaskStatusChange); inspectorClient.addEventListener( "requestorTaskUpdated", onRequestorTaskUpdated, ); + inspectorClient.addEventListener("taskCancelled", onTaskCancelled); return () => { inspectorClient.removeEventListener( "taskStatusChange", @@ -876,6 +949,7 @@ function App() { "requestorTaskUpdated", onRequestorTaskUpdated, ); + inspectorClient.removeEventListener("taskCancelled", onTaskCancelled); // Hide any still-visible task toasts on client swap so they don't linger // into the next session, then drop the bookkeeping (mirrors the progress- // toast teardown above). @@ -1378,6 +1452,10 @@ function App() { const asTask = serverSupportsTaskToolCalls && (runAsTask || tool.execution?.taskSupport === "required"); + // Drop any prior call's task id before starting; a task-augmented call + // repopulates it via the `toolCallTaskUpdated` listener below, an ordinary + // call leaves it cleared (#1455). + activeToolCallTaskIdRef.current = undefined; setToolCallState({ status: "pending" }); try { // ToolsScreen types the args as `Record` (it accepts @@ -1673,6 +1751,22 @@ function App() { [inspectorClient], ); + // Cancel the in-flight tool call. A task-augmented call (run-as-task) has a + // server-side task, so cancel that via the tasks API (#1455) — the cancelled + // status then flows back through the managed task store and toasts, the same + // as cancelling from the Tasks screen. An ordinary call has no task (and no + // request-abort channel yet), so there is nothing to cancel server-side. + const onCancelToolCall = useCallback(() => { + const taskId = activeToolCallTaskIdRef.current; + if (taskId) { + // Clear the ref before the call resolves so a rapid second Cancel click + // doesn't re-cancel the now-terminating task (which would surface a + // spurious "Failed to cancel task" toast). + activeToolCallTaskIdRef.current = undefined; + void onCancelTask(taskId); + } + }, [onCancelTask]); + const onClearCompletedTasks = useCallback(() => { clearCompletedTasks(); }, [clearCompletedTasks]); @@ -2184,6 +2278,7 @@ function App() { onCallTool={(name, args, runAsTask) => { void onCallTool(name, args, runAsTask); }} + onCancelToolCall={onCancelToolCall} onClearToolResult={onClearToolResult} onReadResourceContents={onReadResourceContents} onRefreshTools={onRefreshTools} diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 05e635d1..5a078242 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -3956,6 +3956,21 @@ describe("InspectorClient", () => { const taskId = taskCreatedDetail.taskId; expect(taskId).toBeDefined(); + // Collect the optimistic task updates the stream emits. The cancellation + // ends the stream with a generic error; the terminal update must report + // "cancelled" (not "failed") so the UI lands on the true state without + // waiting for a refresh (#1455). + const taskUpdates: TaskWithOptionalCreatedAt[] = []; + const onRequestorTaskUpdated = ( + e: CustomEvent<{ taskId: string; task: TaskWithOptionalCreatedAt }>, + ) => { + taskUpdates.push(e.detail.task); + }; + client!.addEventListener( + "requestorTaskUpdated", + onRequestorTaskUpdated as EventListener, + ); + const cancelledPromise = waitForEvent<{ taskId: string }>( client, "taskCancelled", @@ -3967,6 +3982,10 @@ describe("InspectorClient", () => { cancelledPromise, taskPromise, ]); + client!.removeEventListener( + "requestorTaskUpdated", + onRequestorTaskUpdated as EventListener, + ); expect(cancelledResult.status).toBe("fulfilled"); const cancelledDetail = ( cancelledResult as PromiseFulfilledResult<{ taskId: string }> @@ -3974,6 +3993,12 @@ describe("InspectorClient", () => { expect(cancelledDetail.taskId).toBe(taskId); expect(taskResult.status).toBe("rejected"); + // The terminal optimistic update is "cancelled", never "failed". + const terminalUpdate = taskUpdates.find((t) => + ["completed", "failed", "cancelled"].includes(t.status), + ); + expect(terminalUpdate?.status).toBe("cancelled"); + const task = await client!.getRequestorTask(taskId); expect(task.status).toBe("cancelled"); }); diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 0243761e..c543b1b1 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -191,6 +191,14 @@ export class InspectorClient extends InspectorClientEventTarget { }; // Resource subscriptions private subscribedResources: Set = new Set(); + // Task ids the user explicitly cancelled. A cancel makes the in-flight + // `callToolStream` reject with a generic -32603 error, which the stream's + // error path would otherwise report as a *failed* task — flashing "failed" + // in the UI until a refresh fetches the server's true "cancelled" state. + // Recording the id lets that path label the terminal task "cancelled" + // instead, so it lands in the right state immediately (#1455). Cleared on + // disconnect. + private cancelledTaskIds: Set = new Set(); // Receiver tasks (server-initiated: server sends createMessage/elicit with params.task, server polls us) private receiverTasks: boolean; private receiverTaskTtlMs: number | (() => number); @@ -1040,6 +1048,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.pendingElicitations = []; // Clear resource subscriptions on disconnect this.subscribedResources.clear(); + this.cancelledTaskIds.clear(); // Clear receiver tasks: stop TTL timers and drop records for (const record of this.receiverTaskRecords.values()) { if (record.cleanupTimeoutId != null) { @@ -1182,6 +1191,10 @@ export class InspectorClient extends InspectorClientEventTarget { if (!this.client) { throw new Error("Client is not connected"); } + // Mark before awaiting: cancelling unblocks the in-flight callToolStream, + // whose error message may arrive before this resolves — the stream's error + // path reads this set to label the task "cancelled" rather than "failed". + this.cancelledTaskIds.add(taskId); await this.client.experimental.tasks.cancelTask( taskId, this.getRequestOptions(), @@ -1826,21 +1839,33 @@ export class InspectorClient extends InspectorClientEventTarget { message.error.message || "Task execution failed"; error = new Error(errorMessage); if (taskId) { - const failedTask: TaskWithOptionalCreatedAt = { + // A user-cancelled task surfaces here as a generic error; report + // it as "cancelled" (not "failed") so the UI lands on the true + // terminal state immediately, matching what a refresh would show + // (#1455). + const cancelled = this.cancelledTaskIds.has(taskId); + // Consume the marker — task ids are single-use, so this keeps the + // set from growing across a long session of cancellations (the + // disconnect-clear stays the backstop for cancels whose task + // completed before the cancel landed and never hit this path). + this.cancelledTaskIds.delete(taskId); + const terminalTask: TaskWithOptionalCreatedAt = { taskId, ttl: null, - status: "failed", - statusMessage: errorMessage, + status: cancelled ? "cancelled" : "failed", + statusMessage: cancelled + ? "Client cancelled task execution." + : errorMessage, lastUpdatedAt: new Date().toISOString(), }; this.dispatchTypedEvent("toolCallTaskUpdated", { taskId, - task: failedTask, + task: terminalTask, error: message.error, }); this.dispatchTypedEvent("requestorTaskUpdated", { taskId, - task: failedTask, + task: terminalTask, error: message.error, }); }