[FIX] CancelledError immunity — _cancellation_shield decorator + run_skill inner handler#3390
Merged
Trecek merged 11 commits intoMay 31, 2026
Conversation
Trecek
force-pushed
the
asyncio-cancellederror-from-mcp-transport-teardown-drops-ses/3373
branch
from
7335655 to
9bd95b7
Compare
…ed phase) - RetryReason.CANCELLED: transport-induced asyncio.CancelledError variant - SkillResult.cancelled(): retriable factory (needs_retry=True, subtype='cancelled') - fleet/state_types._ABANDON_REASONS: include CANCELLED (non-resumable teardown) - test_retry_reason_values: add CANCELLED to exhaustiveness set - test_retry_reason_has_cancelled_variant: new assertion - test_skill_result_cancelled_factory: verify cancelled() factory fields - test_retry_reason_value_count_is_16: updated from 15 - _ROUTING_EXCLUDED: add CANCELLED (transport event; not orchestrator-routed) - _has_cancellation_shield: AST helper in tests/arch/_helpers.py - test_all_mcp_tool_handlers_have_cancellation_shield: structural enforcement (fails until Phase 2) - test_run_skill_returns_structured_result_on_cancelled_error: gap test (fails until Phase 2) - test_dispatch_food_truck_returns_structured_result_on_cancelled_error: gap test (fails until Phase 2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…skill inner handler (green phase) - _cancellation_shield: decorator catches asyncio.CancelledError at MCP tool boundary, returns schema-aware structured JSON (fleet_error, run_cmd, run_python, generic) - run_skill: inner except asyncio.CancelledError returns SkillResult.cancelled() with resolved_command/effective_order_id provenance - Applied @_cancellation_shield to all 52 @mcp.tool() handlers across 18 tool files - Updated subpackage file count budget (server/tools: 22 → 23) - Updated _LEGACY_JSON_WRITES line numbers for decorator insertion drift Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ror test Strengthens test_dispatch_food_truck_returns_structured_result_on_cancelled_error to verify the fleet_error envelope identifies a CancelledError specifically, not just any failure envelope. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… trim docstring duplication, fix run_python description - Wrap logger.warning() inside shield in try/except Exception to prevent skipped return if logger raises - Trim decorator docstring to usage-specific lines (removes duplication with module docstring) - Give run_python its own description line instead of "same as run_cmd" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion without logger call The reviewer's suggestion to wrap logger.warning() in try/except Exception violates ARCH-003 (DS-001: broad except without logger call). Reclassified as REJECT with category arch_violation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lScope Wraps the logger.warning call in the inner CancelledError handler with anyio.CancelScope(shield=True), consistent with _cancellation_shield.py pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…or test Adds a human-readable message check so a regression producing blank or misleading result text would be caught. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The standalone assertion (RetryReason.CANCELLED == "cancelled") is already covered by test_retry_reason_values which asserts CANCELLED in the full set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dler Fixes test_all_mcp_tool_handlers_have_cancellation_shield arch test — lock_ingredients was the only @mcp.tool() handler missing the decorator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Trecek
force-pushed
the
asyncio-cancellederror-from-mcp-transport-teardown-drops-ses/3373
branch
from
9bd95b7 to
b73f40c
Compare
Trecek
deleted the
asyncio-cancellederror-from-mcp-transport-teardown-drops-ses/3373
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
asyncio.CancelledErrorfrom MCP transport teardown (SIGTERM/SIGINT/SIGHUP) propagates through MCP tool handlers (run_skill,dispatch_food_truck,record_gate_dispatch) without producing a structured SkillResult. The retry FSM is never consulted, no SkillResult is returned, and pipeline orchestrators proceed with stale artifacts. Part A adds: aSkillResult.cancelled()factory, theCANCELLEDretry reason enum variant, a_cancellation_shielddecorator for all MCP tool handlers, an explicit innerexcept asyncio.CancelledErrorhandler inrun_skill, and tests that prove the gap and verify the fix. Part B (separate) covers recipe schema routing.Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260530-193133-478133/.autoskillit/temp/rectify/rectify_cancelled_error_skill_result_gap_2026-05-30_193500.mdCloses #3373
🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown