From aacb409fbf74a8add2c6b57ab9a2c4326f8a7376 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 12 Oct 2026 11:32:52 -0400 Subject: [PATCH] fix(tools): limit cancellation guard to pre-execution --- CHANGELOG.md | 2 + docs/CURRENT-STATE.md | 7 +- src/tools/catalog-cancellation.test.mjs | 117 +++++++++--------------- src/tools/catalog.js | 13 +-- 4 files changed, 52 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33ebae897..21b296fdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ - Cancelled tool calls refuse actions before execution and discard results that settle after cancellation. +- Cancelled catalog queries refuse execution when their signal is already aborted. + - Treat explicitly current, complete empty vessel coverage as a successful update, clearing obsolete contacts while retaining stale or incomplete snapshots. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 709613bad..648fa0584 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -97,10 +97,9 @@ lists at 25 rows by default. Voice offers the same queries next to its app actions: the session lists them, and the browser runs them through the same catalog, loaded on first use. See [tools and the MCP server](TOOLS.md). -Catalog calls check their cancellation signal before tool execution and after -the tool resolves, including nested calls. Cancellation discards late catalog -results without promising rollback of completed actions or invalidating correct -static-source caches. +Catalog calls check their cancellation signal immediately before executing a +tool. Already-cancelled queries do not start; cancellation after execution starts +continues to follow the existing tool and caller behavior. ## Cyber HUD — September 30, 2026 diff --git a/src/tools/catalog-cancellation.test.mjs b/src/tools/catalog-cancellation.test.mjs index a4a2b7c9d..0f666b262 100644 --- a/src/tools/catalog-cancellation.test.mjs +++ b/src/tools/catalog-cancellation.test.mjs @@ -1,78 +1,51 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { composeCatalog, defineTool } from './catalog.js'; -test('cancelled catalog calls cannot execute or return late results', async () => { - let calls = 0, - release, - block = false; - const action = defineTool({ - name: 'fixture_action', - kind: 'action', - title: 'Fixture', - description: 'Offline fixture', - inputSchema: { type: 'object' }, - run: async () => { - calls++; - return block - ? new Promise((r) => (release = r)) - : { summary: 'fixture', data: {} }; - }, - }); - const catalog = composeCatalog({ tools: [action] }); - const c = new AbortController(); - c.abort(); - await assert.rejects( - catalog.call('fixture_action', {}, { signal: c.signal }), - { name: 'AbortError' }, - ); - assert.equal(calls, 0); - block = true; - const running = new AbortController(); - const pending = catalog.call( - 'fixture_action', - {}, - { signal: running.signal }, - ); - while (!release) await Promise.resolve(); - running.abort(); - release({ summary: 'late', data: {} }); - await assert.rejects(pending, { name: 'AbortError' }); - assert.equal(calls, 1); -}); -test('cancellation during an awaited interceptor cannot publish its late result', async () => { - const controller = new AbortController(); - const reason = new Error('interceptor caller canceled'); - let calls = 0; - const action = defineTool({ - name: 'fixture_intercepted_action', - kind: 'action', - title: 'Fixture', - description: 'Offline fixture', - inputSchema: { type: 'object' }, - run: async () => { - calls++; - return { summary: 'completed action', data: {} }; - }, - }); - const catalog = composeCatalog({ - tools: [action], - interceptors: [ - async (call, next) => { - const result = await next(call); - await Promise.resolve(); - controller.abort(reason); - return result; +for (const intercept of [false, true]) { + test(`cancelled query refuses execution${intercept ? ' after an awaited interceptor' : ''}`, async () => { + let calls = 0; + const controller = new AbortController(); + const query = defineTool({ + name: 'fixture_query', + kind: 'query', + title: 'Fixture', + description: 'Offline fixture', + inputSchema: { type: 'object' }, + run: async () => { + calls++; + return { summary: 'fixture', data: {} }; }, - ], + }); + const catalog = composeCatalog({ + tools: [query], + interceptors: intercept + ? [ + async (call, next) => { + await Promise.resolve(); + controller.abort(); + return next(call); + }, + ] + : [], + }); + if (!intercept) controller.abort(); + await assert.rejects( + catalog.call('fixture_query', {}, { signal: controller.signal }), + { name: 'AbortError' }, + ); + assert.equal(calls, 0); + const fresh = composeCatalog({ tools: [query] }); + assert.equal( + ( + await fresh.call( + 'fixture_query', + {}, + { signal: new AbortController().signal }, + ) + ).summary, + 'fixture', + ); + assert.equal(calls, 1); }); - await assert.rejects( - catalog.call( - 'fixture_intercepted_action', - {}, - { signal: controller.signal }, - ), - (error) => error === reason, - ); - assert.equal(calls, 1, 'completed actions are not rolled back or rerun'); -}); +} diff --git a/src/tools/catalog.js b/src/tools/catalog.js index ea0e37460..d617a9f28 100644 --- a/src/tools/catalog.js +++ b/src/tools/catalog.js @@ -169,21 +169,13 @@ export function composeCatalog({ ); const index = new Map(available.map((tool) => [tool.name, tool])); - const invoke = async (name, args, signal, parent) => { - signal?.throwIfAborted(); + const invoke = (name, args, signal, parent) => { const tool = index.get(name); if (!tool) return Promise.reject( new ToolError('unsupported', `No tool named ${name} is available`), ); - const result = await chain({ - tool, - args, - signal, - ...(parent ? { parent } : {}), - }); - signal?.throwIfAborted(); - return result; + return chain({ tool, args, signal, ...(parent ? { parent } : {}) }); }; const execute = async ({ tool, args, signal }) => { signal?.throwIfAborted(); @@ -203,7 +195,6 @@ export function composeCatalog({ } catch (error) { throw fromSourceError(error); } - signal?.throwIfAborted(); if (!result || typeof result.summary !== 'string' || !result.data) throw new TypeError(`${tool.name} returned no summary or data`); if (