Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tasksync-chat/src/constants/remoteConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ export const HUMAN_DELAY_MIN_UPPER = 30; // maximum allowed for "min delay" inpu
export const HUMAN_DELAY_MAX_LOWER = 2; // minimum allowed for "max delay" input
export const HUMAN_DELAY_MAX_UPPER = 60; // maximum allowed for "max delay" input

// Superseded ask_user directive — returned as normal tool result (not CancellationError)
// so the ToolCallingLoop keeps running. Instructs the LLM to re-ask the same question.
export const ASKUSER_SUPERSEDED_MESSAGE =
"[CANCELLED: This ask_user request was superseded internally. When you call ask_user again, re-ask the exact same question — do not rephrase or summarize differently.]";

// Shared askUser prompt fragments (used by both local and remote session starts)
export const ASKUSER_VISIBILITY_TEXT =
"The user can ONLY see messages you send via the #askUser tool — your normal chat responses are invisible to them.";
Expand Down
108 changes: 92 additions & 16 deletions tasksync-chat/src/tools.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { AUTO_APPEND_DEFAULT_TEXT } from "./constants/remoteConstants";
import {
ASKUSER_SUPERSEDED_MESSAGE,
AUTO_APPEND_DEFAULT_TEXT,
} from "./constants/remoteConstants";
import { buildFinalResponse } from "./tools";

const {
Expand Down Expand Up @@ -290,40 +293,51 @@ beforeEach(() => {
});

/**
* Verify that cancelled ask_user results stop the LM turn instead of becoming fake replies.
* Verify that cancelled ask_user results are returned as normal text
* (NOT CancellationError) so the LLM can see the message and call ask_user again.
* CancellationError kills the ToolCallingLoop — only real token cancellation should throw it.
*/
describe("askUser cancellation handling", () => {
/**
* A cancelled provider result must propagate as a cancellation error.
* A superseded provider result must return as normal text, not throw.
*/
it("throws CancellationError when waitForUserResponse returns a cancelled result", async () => {
it("returns cancelled message as normal result when waitForUserResponse returns a cancelled result", async () => {
const { askUser } = await import("./tools");
const provider = {
waitForUserResponse: vi.fn().mockResolvedValue({
value: "[Session reset by user]",
value: ASKUSER_SUPERSEDED_MESSAGE,
attachments: [],
queue: false,
cancelled: true,
}),
};

await expect(
askUser({ question: "Reset?" }, provider as any, createToken() as any),
).rejects.toBeInstanceOf(MockCancellationError);
const result = await askUser(
{ question: "Reset?" },
provider as any,
createToken() as any,
);
expect(result.response).toBe(ASKUSER_SUPERSEDED_MESSAGE);
expect(result.attachments).toEqual([]);
expect(result.queue).toBe(false);
});

/**
* The registered LM tool must rethrow cancellation so Copilot stops the old turn cleanly.
* The registered LM tool must return a LanguageModelToolResult (not throw)
* for superseded requests so the LLM sees the message.
*/
it("rethrows CancellationError from the registered tool invoke handler", async () => {
it("returns LanguageModelToolResult for superseded requests from the registered tool invoke handler", async () => {
const { registerTools } = await import("./tools");
const provider = {
waitForUserResponse: vi.fn().mockResolvedValue({
value: "[Session reset by user]",
value: ASKUSER_SUPERSEDED_MESSAGE,
attachments: [],
queue: false,
cancelled: true,
}),
_autoAppendEnabled: false,
_autoAppendText: "",
_alwaysAppendReminder: false,
};
const context = { subscriptions: [] as unknown[] };

Expand All @@ -332,12 +346,74 @@ describe("askUser cancellation handling", () => {
const toolDefinition = registerToolMock.mock.calls[0]?.[1];
expect(toolDefinition).toBeTruthy();

const result = await toolDefinition.invoke(
{ input: { question: "Reset?" } },
createToken() as any,
);
// Should return a result, not throw
expect(result).toBeTruthy();
expect(result.parts).toBeDefined();
expect(result.parts.length).toBe(1);
// The response text should contain the cancelled message
const textPart = result.parts[0];
const parsed = JSON.parse(textPart.value);
expect(parsed.response).toBe(ASKUSER_SUPERSEDED_MESSAGE);
expect(showErrorMessageMock).not.toHaveBeenCalled();
});

/**
* Real token cancellation (CancellationToken fires) must still throw CancellationError.
*/
it("throws CancellationError when token is already cancelled before starting", async () => {
const { askUser } = await import("./tools");
const provider = {
waitForUserResponse: vi.fn(),
};

const cancelledToken = {
isCancellationRequested: true,
onCancellationRequested: vi.fn(() => ({
dispose: vi.fn(),
})),
};

await expect(
toolDefinition.invoke(
{ input: { question: "Reset?" } },
createToken() as any,
),
askUser({ question: "Test?" }, provider as any, cancelledToken as any),
).rejects.toBeInstanceOf(MockCancellationError);
expect(showErrorMessageMock).not.toHaveBeenCalled();
});

/**
* Mid-flight cancellation: token fires while waitForUserResponse is still pending.
* The createCancellationPromise race must cause askUser to reject with CancellationError.
*/
it("throws CancellationError when token fires mid-flight during waitForUserResponse", async () => {
const { askUser } = await import("./tools");

// waitForUserResponse never resolves — simulates the user hasn't responded yet
const provider = {
waitForUserResponse: vi.fn(() => new Promise<never>(() => {})),
};

// Capture the onCancellationRequested callback so we can fire it manually
let cancelCallback: (() => void) | undefined;
const token = {
isCancellationRequested: false,
onCancellationRequested: vi.fn((cb: () => void) => {
cancelCallback = cb;
return { dispose: vi.fn() };
}),
};

const promise = askUser(
{ question: "Pending?" },
provider as any,
token as any,
);

// Fire the cancellation callback to simulate the Stop button
expect(cancelCallback).toBeDefined();
cancelCallback!();

await expect(promise).rejects.toBeInstanceOf(MockCancellationError);
});
});
14 changes: 12 additions & 2 deletions tasksync-chat/src/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,23 @@ export async function askUser(
cancellation.promise,
]);

// Handle case where request was superseded by another call
// Handle case where request was superseded by another call.
// IMPORTANT: Do NOT throw CancellationError here — that kills the entire
// ToolCallingLoop in VS Code's Copilot Chat (buildPrompt2 checks isCancelled
// on tool results and throws CancellationError, _runLoop catches it and breaks).
// Instead, return the cancelled message as a normal result so the LLM sees it
// and can call ask_user again. Real user cancellation (Stop button) is handled
// by the CancellationToken/createCancellationPromise path which correctly throws.
if (result.cancelled) {
debugLog(
"[TaskSync] askUser — superseded/cancelled, response:",
result.value.slice(0, 80),
);
throw new vscode.CancellationError();
return {
response: result.value,
attachments: [],
queue: result.queue,
};
}
debugLog(
"[TaskSync] askUser — user responded:",
Expand Down
3 changes: 2 additions & 1 deletion tasksync-chat/src/webview/toolCallHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
import * as vscode from "vscode";
import {
ASKUSER_SUPERSEDED_MESSAGE,
CONFIG_SECTION,
DEFAULT_MAX_CONSECUTIVE_AUTO_RESPONSES,
} from "../constants/remoteConstants";
Expand Down Expand Up @@ -44,7 +45,7 @@ export function cancelSupersededPendingRequest(p: P): void {
const oldResolve = p._pendingRequests.get(oldToolCallId);
if (oldResolve) {
oldResolve({
value: "[CANCELLED: New request superseded this one]",
value: ASKUSER_SUPERSEDED_MESSAGE,
queue: hasQueuedItems(p),
attachments: [],
cancelled: true,
Expand Down
Loading