fix(tasks): tie a receiver task's answer collection to its cancellation
`tasks/cancel` marked the task cancelled and rejected its payload, but left whatever was collecting the answer running — the native pending-request entry, and now an app-rendered elicitation's renderer and bridge. A modal therefore outlived the task it belonged to, and an answer arriving afterwards re-settled it, overwriting `cancelled` with `completed`: a task the server had been told it cancelled. Each receiver task now owns an `AbortController`, aborted by `tasks/cancel` and by session teardown. The task-augmented elicitation branch passes its signal to the app attempt and wires it to the native queue entry, so both answer routes are torn down the same way. Both settle helpers return early on a terminal status, so a late answer through either route cannot resurrect the task. The overwrite predates the app path — a native answer after a cancel did the same thing — so the guard fixes both. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
d90c51a04c
commit
d97c9bd391
2 changed files with 66 additions and 1 deletions
|
|
@ -482,6 +482,41 @@ describe("app-rendered elicitation routing (#1854)", () => {
|
|||
await client.disconnect();
|
||||
});
|
||||
|
||||
it("tears the app down on tasks/cancel and keeps the task cancelled", async () => {
|
||||
// `tasks/cancel` is the server changing its mind. Without an abort the
|
||||
// modal stayed live, and a later answer overwrote `cancelled` with
|
||||
// `completed` — a task the server was told it had cancelled.
|
||||
const transport = new ElicitTransport();
|
||||
let aborted = false;
|
||||
let settle: ((r: ElicitResult) => void) | undefined;
|
||||
const client = await connectWithTasks(
|
||||
transport,
|
||||
(request) =>
|
||||
new Promise<ElicitResult>((resolve) => {
|
||||
settle = resolve;
|
||||
request.signal.addEventListener("abort", () => (aborted = true));
|
||||
}),
|
||||
);
|
||||
|
||||
const created = (await transport.elicit(52, taskParams())) as {
|
||||
result?: { task?: { taskId?: string } };
|
||||
};
|
||||
const taskId = created.result?.task?.taskId;
|
||||
expect(taskId).toBeDefined();
|
||||
await vi.waitFor(() => expect(settle).toBeDefined());
|
||||
|
||||
await transport.sendRequest(53, "tasks/cancel", { taskId });
|
||||
expect(aborted).toBe(true);
|
||||
expect(await taskStatuses(transport, 54)).toContain("cancelled");
|
||||
|
||||
// An answer that lands after the cancel must not resurrect the task.
|
||||
settle?.({ action: "accept", content: { choice: "too-late" } });
|
||||
await vi.waitFor(async () =>
|
||||
expect(await taskStatuses(transport, 55)).toEqual(["cancelled"]),
|
||||
);
|
||||
await client.disconnect();
|
||||
});
|
||||
|
||||
it("falls back to the native queue when the app cannot answer", async () => {
|
||||
const transport = new ElicitTransport();
|
||||
const client = await connectWithTasks(transport, async () => {
|
||||
|
|
|
|||
|
|
@ -239,6 +239,14 @@ interface ReceiverTaskRecord {
|
|||
resolvePayload: (payload: ClientResult) => void;
|
||||
rejectPayload: (reason?: unknown) => void;
|
||||
cleanupTimeoutId?: ReturnType<typeof setTimeout>;
|
||||
/**
|
||||
* Aborted when the task reaches a terminal state some way other than the
|
||||
* user answering — a `tasks/cancel`, or session teardown. Whatever is
|
||||
* collecting the answer (the native pending-request entry, or an app-rendered
|
||||
* elicitation and its bridge) is torn down from this, so a cancelled task
|
||||
* cannot leave a modal on screen waiting for an answer nothing will read.
|
||||
*/
|
||||
abort: AbortController;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1222,6 +1230,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
payloadPromise,
|
||||
resolvePayload,
|
||||
rejectPayload,
|
||||
abort: new AbortController(),
|
||||
};
|
||||
record.cleanupTimeoutId = setTimeout(() => {
|
||||
record.cleanupTimeoutId = undefined;
|
||||
|
|
@ -1298,6 +1307,9 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
};
|
||||
record.task = updatedTask;
|
||||
record.rejectPayload(new Error("Task cancelled"));
|
||||
// Stop collecting an answer nobody will read: drops the native pending
|
||||
// entry and tears down an app-rendered elicitation's renderer.
|
||||
record.abort.abort();
|
||||
if (record.cleanupTimeoutId != null) {
|
||||
clearTimeout(record.cleanupTimeoutId);
|
||||
record.cleanupTimeoutId = undefined;
|
||||
|
|
@ -1440,6 +1452,11 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
// an app-rendered answer completes the task exactly as a native one
|
||||
// does.
|
||||
const completeTask = (result: ElicitResult) => {
|
||||
// A cancelled (or otherwise terminal) task must not be re-settled:
|
||||
// an answer that arrives after `tasks/cancel` would otherwise
|
||||
// overwrite `cancelled` with `completed`.
|
||||
if (InspectorClient.isTerminalTaskStatus(record.task.status))
|
||||
return;
|
||||
record.resolvePayload(result);
|
||||
const updated: Task = {
|
||||
...record.task,
|
||||
|
|
@ -1450,6 +1467,8 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
this.upsertReceiverTask(updated);
|
||||
};
|
||||
const failTask = (error: Error) => {
|
||||
if (InspectorClient.isTerminalTaskStatus(record.task.status))
|
||||
return;
|
||||
record.rejectPayload(error);
|
||||
const updated: Task = {
|
||||
...record.task,
|
||||
|
|
@ -1471,7 +1490,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
// it natively.
|
||||
let appResult: ElicitResult | null;
|
||||
try {
|
||||
appResult = await this.tryAppElicitation(request);
|
||||
appResult = await this.tryAppElicitation(
|
||||
request,
|
||||
record.abort.signal,
|
||||
);
|
||||
} catch (error) {
|
||||
failTask(
|
||||
error instanceof Error ? error : new Error(String(error)),
|
||||
|
|
@ -1489,6 +1511,11 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
failTask,
|
||||
);
|
||||
this.addPendingElicitation(elicitationRequest);
|
||||
// A `tasks/cancel` (or teardown) drops the queued entry, so the
|
||||
// modal does not outlive the task it belongs to.
|
||||
this.wirePendingAbort(record.abort.signal, () =>
|
||||
this.removePendingElicitation(elicitationRequest.id),
|
||||
);
|
||||
})();
|
||||
// Task-augmented (2025-11-25) response — see the sampling handler
|
||||
// above. Reply with a `CreateTaskResult` (`{ task }`), routed around
|
||||
|
|
@ -1620,6 +1647,9 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
if (record.cleanupTimeoutId != null) {
|
||||
clearTimeout(record.cleanupTimeoutId);
|
||||
}
|
||||
// Same reason as `cancelReceiverTask`: the session that owns whatever is
|
||||
// collecting the answer is ending.
|
||||
record.abort.abort();
|
||||
}
|
||||
this.receiverTaskRecords.clear();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue