fix(admission): a cancelled caller keeps its slot until its worker ends (#728, slice 1) - #885

Summary

safe_handler held the per-tool admission slot with async with admit(tool_name) around await asyncio.to_thread(...). Cancelling the awaiting task exits that block and releases the slot, but asyncio.to_thread cannot stop the thread: the worker keeps running. A new request then takes the freed slot and starts one worker more than the declared budget — the probe of the issue (budget 4, four cancelled callers, five active workers).

Root cause: the slot was owned by the caller, not by the work. This PR adds run_admitted (mcp_server/handlers/admission.py): it acquires the slot, starts the worker, releases the slot from the worker's done callback, and the caller waits through asyncio.shield. A cancelled caller still gets CancelledError at once, but the slot stays counted until the worker ends. A worker whose caller was cancelled logs its outcome when it ends (<tool> worker completed|raised <Exc>|was cancelled after its caller was cancelled); reading exception() there is also what retrieves it, so no "exception never retrieved" is left behind. safe_handler uses run_admitted; the duration histogram is now observed on the worker thread (_timed_handler_run), so an abandoned run is still measured.

Refs #728 (slice 1 — the remaining acceptance items are listed below, so this PR does not close the issue).

Type of change

• Bug fix (non-breaking change which fixes an issue)

Test plan

• New tests_py/server/test_safe_handler_cancellation.py: the issue's probe through the real safe_handler (detect_gaps, budget 4, four callers cancelled while their handlers run, then a fifth request). Without the fix it fails: after the cancellations the semaphore is [unlocked, value:4] (checked locally by stashing mcp_server/). With the fix the slot stays locked, the fifth waits, and the peak of active workers is 4. Also asserts the duration and ok counter labels.
• New tests_py/handlers/test_admission_cancellation.py (5 tests): budget 1 held by an abandoned worker until it ends; result and error reach a waiting caller and release the slot; the three abandoned outcomes are logged with their exact message.
• New tests_py/server/test_safe_handler_error_path.py (3 tests): the error path safe_handler kept (error log with and without a tool name, error counter, a failing error counter logged at debug). Added because ADR-1122 mutates the whole changed function.
• Orderings use threading.Event/Semaphore; a 5 s guard only bounds waits a broken implementation would never satisfy, so a mutant fails instead of hanging. No verdict depends on elapsed time.
• GITHUB_BASE_REF=main UV_NO_SYNC=1 python scripts/ci_mutation_diff.py (mutmut 3.8.1): 2 file(s), 0 failed, 0 untested; 35/35 mutants of run_admitted/_release_slot/_log_abandoned and 78/78 of safe_handler/_timed_handler_run killed. The 4 survivors of _classify_error are outside the changed functions.
• pytest tests_py/server tests_py/handlers tests_py/observability + every other unit test file importing tool_error_handler/admission: 1853 passed, 82 skipped (PostgreSQL not reachable here).
• python scripts/check_craftsmanship.py: OK. ruff check / ruff format --check (0.17.3, from uv.lock) clean on the five touched files.

Audit notes

• Behaviour change, deliberate: cortex_tool_duration_seconds now measures the handler on its worker thread. Before, it measured the to_thread await, which also counted the wait for a free executor thread and stopped at caller cancellation.
• If the event loop itself cancels the worker task (shutdown), the slot is released while the thread may still run; the loop is going away, and the log line says was cancelled.
• admit is kept (tests and a with-block API); production tool calls no longer use it.
• Not verified: real MCP cancellation/disconnect (item 4 below); the full suite on PostgreSQL; torch could not be installed through the proxy, so the dev env was installed from uv.lock without the torch stack.

Remaining slices for #728

1. Define and report in-flight mutation status for work completing after client abandonment (beyond the log line): e.g. a counter or a receipt the client can query.
2. Exercise real MCP cancellation/disconnect with isolated stores and verify side effects and teardown.
3. A documented cooperative cancellation boundary for handlers that can stop early.

Coding-standards compliance

• Layer direction preserved (tool_error_handler → handlers.admission, as before).
• No file over 300 lines, no function over 40; test constants carry # source:.

Generated by Claude Code