* fix(web): send a cancellation notification when cancelling an ordinary tool call (#1458) The Tool detail panel's Cancel button was a silent no-op for ordinary (non-task) tool calls: App.onCancelToolCall found no captured taskId and did nothing, leaving the button looking actionable while the call ran on. Wire request cancellation through the MCP cancellation flow. callTool now tracks the in-flight call with an AbortController whose signal is threaded into the SDK request options; aborting it makes the SDK send a notifications/cancelled for that request and reject the pending call. cancelToolCall() performs the abort. callTool surfaces the cancellation as a ToolCallCancelledError (without recording a failed call), which App clears as a clean cancel plus a short confirmation toast. Task-augmented calls are unchanged: they have a server-side task and are still cancelled via cancelRequestorTask (#1457). Closes #1458 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): address review — distinguish disconnect from cancel; tidy callTool params Review follow-ups on #1463: - Disconnect no longer surfaces as "Tool call cancelled". The catch in callToolWithRetries now throws ToolCallCancelledError only when the abort reason matches the deliberate user-cancel reason (TOOL_CALL_CANCELLED_REASON); a disconnect aborts the same controller with a different reason and falls through to the normal error path, so no misleading toast. - Note the activeToolCallAbortController slot is shared (last-writer-wins) across all callTool callers. - Fold callToolWithRetries/attemptToolCall's long positional parameter list into a single ToolCallRequest object; move the URL-elicitation loop state into callToolWithRetries. Adds an integration test that a disconnect mid-call rejects as an ordinary error (not ToolCallCancelledError), using a slow send_progress call over HTTP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d90cc4c34b
commit
3220d6239a
1 changed files with 309 additions and 26 deletions
|
|
@ -1,6 +1,7 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js";
|
||||
import { UrlElicitationLoopError } from "@inspector/core/mcp/urlElicitation.js";
|
||||
import { ToolCallCancelledError } from "@inspector/core/mcp/toolCallCancelledError.js";
|
||||
import {
|
||||
renderWithMantine,
|
||||
screen,
|
||||
|
|
@ -51,6 +52,7 @@ vi.mock("@inspector/core/mcp/index.js", () => {
|
|||
.fn()
|
||||
.mockResolvedValue({ success: true, result: { acts: [] } });
|
||||
cancelRequestorTask = vi.fn().mockResolvedValue(undefined);
|
||||
cancelToolCall = vi.fn().mockReturnValue(true);
|
||||
getPrompt = vi.fn().mockResolvedValue({ result: { messages: [] } });
|
||||
readResource = vi
|
||||
.fn()
|
||||
|
|
@ -810,7 +812,7 @@ describe("App task wiring", () => {
|
|||
expect(client.cancelRequestorTask).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not cancel a task when an ordinary tool call is cancelled", async () => {
|
||||
it("aborts the request (not a task) when an ordinary tool call is cancelled", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(<App />);
|
||||
await user.click(screen.getByText("connect"));
|
||||
|
|
@ -818,6 +820,7 @@ describe("App task wiring", () => {
|
|||
|
||||
const client = clientInstances[0] as unknown as {
|
||||
cancelRequestorTask: ReturnType<typeof vi.fn>;
|
||||
cancelToolCall: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
// A stale task id from an earlier task call...
|
||||
|
|
@ -828,7 +831,8 @@ describe("App task wiring", () => {
|
|||
}),
|
||||
);
|
||||
});
|
||||
// ...is cleared when a new ordinary call starts, so its Cancel is a no-op.
|
||||
// ...is cleared when a new ordinary call starts, so Cancel routes to the
|
||||
// request-abort path (notifications/cancelled), not the task API (#1458).
|
||||
await user.click(screen.getByText("call"));
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"),
|
||||
|
|
@ -836,9 +840,36 @@ describe("App task wiring", () => {
|
|||
|
||||
await user.click(screen.getByText("cancel-tool-call"));
|
||||
|
||||
expect(client.cancelToolCall).toHaveBeenCalledTimes(1);
|
||||
expect(client.cancelRequestorTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears the executing state and toasts when a cancelled call rejects", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(<App />);
|
||||
await user.click(screen.getByText("connect"));
|
||||
await waitFor(() => expect(clientInstances).toHaveLength(1));
|
||||
|
||||
// The aborted call rejects with ToolCallCancelledError (the SDK already sent
|
||||
// notifications/cancelled). App treats that as a clean cancel, not a failure.
|
||||
(
|
||||
clientInstances[0] as unknown as {
|
||||
callTool: ReturnType<typeof vi.fn>;
|
||||
}
|
||||
).callTool.mockRejectedValueOnce(new ToolCallCancelledError("get_acts"));
|
||||
|
||||
await user.click(screen.getByText("call"));
|
||||
|
||||
// The result panel returns to idle (no error state)...
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId("tool-status")).toHaveTextContent("none"),
|
||||
);
|
||||
// ...and a confirmation toast acknowledges the cancellation.
|
||||
expect(notificationsMock.show).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ title: "Tool call cancelled" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("shows a URL-elicitation toast when a tool call fails with a no-list -32042", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(<App />);
|
||||
|
|
|
|||
Loading…
Reference in a new issue