AI tools: 22 Task bridges gone, 17 uncancellable tool calls fixed, the ThreadPool hop made drainable by rbuergi · Pull Request #2537 · Systemorph/MeshWeaver

rbuergi · 2026-08-27T20:12:59Z

Maintainer instruction: "get rid of any task. tasks are evil." — for src/MeshWeaver.AI/.

src/MeshWeaver.AI/ held 29 sites in the Task/blocking family. 22 converted, 1 dead async method deleted, 9 kept — each of the nine named below with why it is a genuine external boundary. Classified by reading what each site does, never by grep shape.

1. The real defect — 17 agent tools could not be cancelled at all

Every tool on MeshPlugin (13: get, search, create, update, patch, edit_content, delete, move, copy, get_diagnostics, recycle, navigate_to, run_tests) and every tool on LspPlugin (4) ended in .FirstAsync().ToTask() — the overload taking no CancellationToken — and declared no token parameter. Nothing could end the wait: not Stop, not the round timeout, not IoPool.Drain().

This is the #1956 defect class, fixed for VersionPlugin / SkillTool / PlanStorageTool / CollaborationPlugin and left standing on these seventeen. The round runs as a leaf on the bounded IoPoolNames.Ai pool holding one gate permit for its whole duration; Drain() — the join teardown performs before disposing the service scope and unloading collectible node ALCs — cancels the pool token and re-acquires every permit. A round parked in an uncancellable tool call never reaches the code that would notice, so Drain sits out its full 30 s budget and teardown proceeds over live code. The LSP four are the sharpest case: a speculative Roslyn compile over a NodeType's whole source set is holding the very ALC teardown is about to unload.

All seventeen now take the round's token (AIFunctionFactory binds it and keeps it out of the JSON schema, so it costs the model nothing) and go through the folder's one shared bridge, ToolTask.Bridge. That also supplies two terminals these got wrong: a fault escaped as an exception where the model needed an answer, and an empty completion reached FirstAsync as "Sequence contains no elements".

2. `AgentFilesPlugin` (5) — the terminal the `.Catch` could not see

These did observe the token and did handle faults fluently — but the .Catch sat upstream of FirstAsync, so an empty completion still surfaced as InvalidOperationException. Reachable, not theoretical: MeshNodeStreamCache completes its per-path subjects on eviction and on dispose, which is exactly what a thread whose hub is going away does.

3. The #2488 question, answered: `ToolTask.Bridge` stops waiting, and requests the teardown

You asked me to establish whether the bridge actually stops the work or only stops waiting for it. It stops waiting and requests teardown — it cannot promise the work has stopped, and the file claimed it could.

Bridge disposes before it settles, which is correct and stays. But Rx disposal is synchronous only for a chain that is synchronous to dispose, and every one of these pipelines hops a scheduler — and Rx runs a sequence's unsubscribe on the scheduler it subscribed on. So pending.Dispose() returns once the unsubscribe is scheduled. The repo already knew half of this: AgentToolCancellationTest waits for the disposal rather than reading it, and says why. ToolTask's own summary asserted the stronger claim. Corrected — in the summary and at Settle.

What can promise it is the pool, so the mechanism now matches the claim. The seven bare .SubscribeOn(TaskPoolScheduler.Default) hops (VersionPlugin ×4, CollaborationPlugin ×2, DelegationTool ×1) go through new ToolTask.Pooled → IIoPool.SubscribeThroughPool on the mesh-scoped AgentStore pool — which documents itself as precisely "the tracked, drainable replacement for a bare .SubscribeOn(TaskPoolScheduler.Default), which the drain cannot reach". Both leave the caller's scheduler, which is all those comments ever asked for; only one is visible to Drain.

AgentStore and never Ai: the call already sits inside a round holding an Ai permit, and re-entering the same bounded pool from inside a permit it holds is the classic nested-gate deadlock (that is why the pool exists). With no IoPoolRegistry the fallback is IoPool.Unbounded, whose SubscribeThroughPool is .SubscribeOn(TaskPoolScheduler.Default) — so a mesh without pools behaves exactly as before.

4. `DelegationTool.Settle` had the ordering defect `ToolTask` documents

It read if (set()) pending.Dispose(); — the exact shape ToolTask's comment names as the defect it removed. tcs is RunContinuationsAsynchronously, so TrySet* schedules the round's continuation concurrently with the rest of the callback: on cancellation the round could observe the cancel, release its Ai permit and let teardown through while this call still held a live subscription to the sub-thread's node stream. Now dispose-then-set.

5. Deleted

AgentChatClient.CreateAgentsAsync — 55 lines, private, no callers, marked [Obsolete("… deadlocks in Orleans")]. CreateAgentsSync is the live path.

Kept (9) — every one an external boundary we do not own

Site	Why
`MeshNodeAgentFileStore` ×7	Overrides of MAF's abstract `AgentFileStore` (`WriteAsync`, `ReadAsync`, `FileExistsAsync`, `DeleteAsync`, `CreateDirectoryAsync`, `ListChildrenAsync`, `SearchAsync`). Signatures are Microsoft's. One line each over a fully reactive body, token observed.
`MeshAgentSkillsSource` ×1	Override of MAF's `AgentSkillsSource.GetSkillsAsync`.
`ToolTask` ×1 `TaskCompletionSource`	The ONE shared bridge to `AIFunctionFactory`'s `Task<string>` — now used by every tool in the folder.

Verified already-correct and left alone (read, not grepped): ThreadExecution's aiPool.Invoke(async poolCt => …), ImageGenerator, ProviderModelLister, ClaudeConnectStrategy — all IIoPool leaf bodies, the one sanctioned async boundary; AgentChatClient's remaining async methods, which run inside that Ai-pool leaf's enumeration rather than on a hub scheduler; ChatClientAgentFactory.InvokeCoreAsync, MAF's AIFunction override; and HandoffTool / ThreadMessageTool / InboxTool, synchronous string-returning tools with no wait to cancel.

Findings that are NOT fixed here

IChatClientFactory.CreateAgentAsync now has no production caller — its only one was the dead method deleted here. Removing it from the interface deletes ~40 Task-returning test doubles across four test projects plus the plugins repo: a separate mechanical PR, not this one.
DelegationTool's TaskCompletionSource is the last hand-rolled bridge in the folder, and is not a drop-in for ToolTask.Bridge: it fans in three independent settle sources (delegation-lifecycle event, sub-thread terminal state, chunk aggregate) where one must be subscribed for its side effect — it launches the sub-thread — while not owning the answer. Expressible as a Merge, but that is a behaviour-carrying refactor with its own test surface. The ordering defect it actually had is fixed here.

Tests

New MeshToolCancellationTest: four park→cancel→dispose tests driving the LSP tools through their AIFunction surface against an injected language service that accepts the request and never answers; a live-token positive control (so "everything throws cancelled" cannot pass); an already-cancelled-round test; and a ratchet asserting every awaiting tool on these five plugins binds a CancellationToken — keyed off the return type, so the synchronous tools need no exemption list.

Non-vacuity measured, not asserted. With the token deliberately disconnected the four park tests fail 4/4 with Expected a OperationCanceledException …, but found TimeoutException — the predicted failure mode exactly. With it wired: 7/7 in 5 s.

MeshWeaver.AI.Test — 1479/1479 (3 pre-existing skips)
MeshWeaver.Threading.Test — 139/139
MeshWeaver.Documentation.Test — 92/92
dotnet build -c Release -warnaserror — 0 warnings, 0 errors

What's New entry included (Category: Fix — Stop now actually stops a running tool call).

Refs #2488, #1956.

🤖 Generated with Claude Code

…e ThreadPool hop made drainable `src/MeshWeaver.AI/` had 29 sites in the Task/blocking family. 22 are converted, 1 dead async method is deleted, and the 9 that remain are each an external boundary we do not own, named individually below. ## The real defect: 17 agent tools could not be cancelled at all Every tool on `MeshPlugin` (13 — get, search, create, update, patch, edit_content, delete, move, copy, get_diagnostics, recycle, navigate_to, run_tests) and every tool on `LspPlugin` (4) ended in `.FirstAsync().ToTask()` — the overload that takes NO CancellationToken — and declared no token parameter, so nothing could end the wait. That is the #1956 defect class, fixed for VersionPlugin / SkillTool / PlanStorageTool / CollaborationPlugin and left standing on these seventeen. The round runs as a leaf on the bounded `IoPoolNames.Ai` pool holding one gate permit for its whole duration, and `IoPool.Drain()` — the join teardown performs before disposing the service scope and unloading collectible node ALCs — cancels the pool token and re-acquires every permit. A round parked in an uncancellable tool call never reaches the code that would notice: Drain sits out its full 30 s budget and teardown proceeds over live code. It also made Stop a lie. The LSP four are the sharpest case — a speculative Roslyn compile over a NodeType's whole source set is holding the very ALC teardown is about to unload. All seventeen now take the round's token (AIFunctionFactory binds it and keeps it out of the JSON schema) and go through `ToolTask.Bridge`, which settles on all four terminals. Two of those were previously wrong here: a fault escaped as an exception where the model needed an answer, and an EMPTY completion reached `FirstAsync` as "Sequence contains no elements". ## AgentFilesPlugin (5): the terminal `.Catch` could not see These observed the token and handled faults fluently, but the `.Catch` sat UPSTREAM of `FirstAsync`, so an empty completion still surfaced as InvalidOperationException — and that terminal is reachable, not theoretical: MeshNodeStreamCache completes its per-path subjects on eviction and on dispose, which is what a thread whose hub is going away does. ## The #2488 question: does ToolTask.Bridge STOP the work, or stop WAITING for it? It stops waiting, and REQUESTS the teardown — it cannot promise the work has stopped, and the file claimed it could. `Bridge` disposes before it settles, which is right and stays; but Rx disposal is synchronous only for a chain that is synchronous to dispose, and every one of these pipelines hops a scheduler. Rx runs a sequence's UNSUBSCRIBE on the scheduler it subscribed on, so `pending.Dispose()` returns once the unsubscribe is SCHEDULED. AgentToolCancellationTest already knew this (it waits for the disposal rather than reading it); ToolTask's own summary asserted the stronger claim. Corrected. What can promise it is the pool, so the mechanism changed to match the claim. Seven bare `.SubscribeOn(TaskPoolScheduler.Default)` hops (VersionPlugin 4, CollaborationPlugin 2, DelegationTool 1) now go through `ToolTask.Pooled` → `IIoPool.SubscribeThroughPool` on the mesh-scoped `AgentStore` pool, which documents itself as exactly "the tracked, drainable replacement for a bare .SubscribeOn(TaskPoolScheduler.Default), which the drain cannot reach". Both leave the caller's scheduler, which is all those comments ever asked for; only one is visible to Drain. AgentStore and never Ai, because the call already sits inside a round holding an Ai permit — nesting the same bounded pool is the classic gate deadlock. With no IoPoolRegistry the fallback is `IoPool.Unbounded`, whose SubscribeThroughPool IS `.SubscribeOn(TaskPoolScheduler.Default)` — so a mesh without pools behaves as before. ## DelegationTool.Settle had the ordering defect ToolTask documents It read `if (set()) pending.Dispose();` — the shape ToolTask's own comment names as the defect. `tcs` is RunContinuationsAsynchronously, so TrySet* schedules the round's continuation concurrently with the rest of the callback: on cancellation the round could observe the cancel, release its Ai permit and let teardown through while this call still held a live subscription to the sub-thread's node stream. Now dispose-then-set, as in ToolTask. ## Deleted `AgentChatClient.CreateAgentsAsync` — 55 lines, private, no callers, and marked `[Obsolete("… deadlocks in Orleans")]`. Its duplicate `CreateAgentsSync` is the live path. ## Kept, and why each is a genuine external boundary - `MeshNodeAgentFileStore` ×7 — overrides of Microsoft Agent Framework's abstract `AgentFileStore` (WriteAsync/ReadAsync/FileExistsAsync/DeleteAsync/CreateDirectoryAsync/ ListChildrenAsync/SearchAsync). The signatures are MAF's; each is one line over a fully reactive body, with the token observed. - `MeshAgentSkillsSource` ×1 — override of MAF's `AgentSkillsSource.GetSkillsAsync`. - `ToolTask` ×1 TaskCompletionSource — the ONE shared bridge to AIFunctionFactory's `Task<string>`, which every tool in the folder now uses. Verified reactive already, by reading what they do rather than by grep: `ThreadExecution`'s `aiPool.Invoke(async poolCt => …)`, `ImageGenerator`, `ProviderModelLister` and `ClaudeConnectStrategy` (all IIoPool leaf bodies — the one sanctioned async boundary), AgentChatClient's remaining async methods (they run INSIDE that Ai-pool leaf, not on a hub scheduler), `ChatClientAgentFactory.InvokeCoreAsync` (MAF's `AIFunction` override), and HandoffTool / ThreadMessageTool / InboxTool (synchronous string-returning tools — no wait to cancel). ## Tests `MeshToolCancellationTest`: four park→cancel→dispose tests driving the LSP tools through their AIFunction surface against an injected language service that accepts and never answers; a live-token positive control; an already-cancelled-round test; and a ratchet asserting every awaiting tool on these five plugins binds a CancellationToken. Non-vacuity MEASURED, not asserted: with the token disconnected the four park tests fail 4/4 with `Expected a OperationCanceledException …, but found TimeoutException`; with it wired they pass 7/7 in 5 s. MeshWeaver.AI.Test 1479/1479, MeshWeaver.Threading.Test 139/139, MeshWeaver.Documentation.Test 92/92. Release + -warnaserror clean, 0 warnings. Refs #2488, #1956. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The two new failure terminals put an arbitrary exception message inside a hand-written JSON literal, so a quote or backslash in it would emit invalid JSON to a tool whose whole contract is a JSON object. JsonSerializer instead. (The pre-existing literals beside it stay: they are constant or a repo-relative path.) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR removes a large set of uncancellable Task bridges in src/MeshWeaver.AI/ by routing agent tools through a single, cancellation-aware reactive→Task<string> adapter (ToolTask.Bridge) and by replacing raw ThreadPool hops with a drainable IIoPool hop (ToolTask.Pooled). This aligns tool execution with IoPool.Drain() teardown guarantees so “Stop” actually stops work and teardown doesn’t proceed over live code.

Changes:

Introduces ToolTask.Pooled (AgentStore pool) and updates AI plugins/tools to use ToolTask.Bridge with the round CancellationToken instead of .FirstAsync().ToTask().
Updates Mesh and LSP tool surfaces to accept/bind CancellationToken, fixing previously-uncancellable calls.
Adds/extends cancellation-focused tests and a What's New entry documenting the user-visible behavioral fix.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/MeshWeaver.AI.Test/MeshToolCancellationTest.cs	Adds new cancellation/parking tests for MeshPlugin + LspPlugin tools, including a ratchet ensuring awaiting tools bind `CancellationToken`.
test/MeshWeaver.AI.Test/AgentToolCancellationTest.cs	Updates cancellation/disposal commentary to reflect pooled subscribe/unsubscribe behavior via `ToolTask.Pooled`.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-27-stop-actually-stops-tool-calls.md	Documents the “Stop now actually stops tool calls” fix and related behavioral improvements.
src/MeshWeaver.AI/Plugins/VersionPlugin.cs	Replaces raw `SubscribeOn(TaskPoolScheduler.Default)` hops with `ToolTask.Pooled` + `ToolTask.Bridge` and ensures cancellation is observed.
src/MeshWeaver.AI/Plugins/ToolTask.cs	Centralizes the reactive→`Task<string>` bridge, adds drainable pooling helper (`Pooled`) and strengthens cancellation/terminal handling.
src/MeshWeaver.AI/Plugins/LspPlugin.cs	Adds `CancellationToken` to LSP tools and routes all tool calls through the shared `ToolTask.Bridge` adapter.
src/MeshWeaver.AI/Plugins/DelegationTool.cs	Fixes settle ordering (dispose-before-settle) and routes subscribe off caller scheduler via pool when available.
src/MeshWeaver.AI/Plugins/CollaborationPlugin.cs	Switches read-side subscribe hop to `ToolTask.Pooled` and uses `ToolTask.Bridge` for complete terminal + cancellation behavior.
src/MeshWeaver.AI/Plugins/AgentFilesPlugin.cs	Replaces per-tool `.Catch(...).FirstAsync().ToTask(ct)` with `ToolTask.Bridge` to correctly handle empty completion + cancellation.
src/MeshWeaver.AI/MeshPlugin.cs	Adds `CancellationToken` to all mesh tools and routes them through a shared `ToolTask.Bridge` adapter; updates `RunTests` to observe cancellation.
src/MeshWeaver.AI/AgentChatClient.cs	Deletes an obsolete, uncancellable async agent-creation path with no callers.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-27T20:30:15Z