Skip to content

fix(esc): override cancelled tool_result with REJECT_MESSAGE in production path - #150

Merged
ericleepi314 merged 1 commit into
mainfrom
fix/esc-cancel-reject-message-production-path
May 16, 2026
Merged

fix(esc): override cancelled tool_result with REJECT_MESSAGE in production path#150
ericleepi314 merged 1 commit into
mainfrom
fix/esc-cancel-reject-message-production-path

Conversation

@ericleepi314

Copy link
Copy Markdown
Collaborator

Summary

  • When ESC fires during a Bash command, the production tool-dispatch path was passing the bash tool's <error>Command was aborted before completion</error> payload through to the model. On the resume turn, the model read this as a generic failure and retried the command instead of honouring the cancel.
  • The TS reference at StreamingToolExecutor.ts:153-205 overrides the tool_result with REJECT_MESSAGE when the abort reason is user_interrupted, but the Python production REPL bypasses StreamingToolExecutor (dispatches via _run_tools_partitioned_dispatch_single_tool), so that override never fired in production.
  • This PR adds the override at four sites in _dispatch_single_tool (pre-tool gate, post-tool override, AbortError catch, late-abort tail) — all funneling through two helpers (_is_user_cancelled_abort, _build_user_cancelled_result). The sibling_error reason is carved out so the streaming-executor's parallel-tool cascade isn't mislabelled as a user rejection. The except AbortError branch re-gates on the same user-cancel check so future tools repurposing AbortError for their own internal cancellation aren't silently relabelled.

Reproduction (before the fix)

User runs a long Bash command, presses ESC, then types please resume. The model sees the bash tool_result content as <error>Command was aborted before completion</error> and treats it as a transient failure — it retries the command instead of honouring the user's cancel.

After the fix

The tool_result for the ESC-cancelled call now contains REJECT_MESSAGE ("The user doesn't want to proceed with this tool use. The tool use was rejected (eg. if it was a file edit, the new_string was NOT written to the file). STOP what you are doing and wait for the user to tell you how to proceed."). On the resume turn the model sees an unambiguous "user rejected" signal in the conversation history.

Test plan

  • New regression file tests/test_esc_reject_message_dispatch.py — 13 tests covering each override site, the sibling_error carve-out, normal completion, and the defensive AbortError-without-signal-aborted case
  • Wider sweep: 217/217 tests pass across test_esc_reject_message_dispatch + test_esc_cancel_propagation + test_abort_controller* + test_streaming_executor_interruptible + test_tool_execution_integration + test_fast_path_dispatch + test_tool_result_budget + test_query_loop + test_query_engine + test_query_error_recovery + test_query_hook_stopped + test_query_terminal + test_streaming_query_loop + test_bash_parser + test_bash_security
  • Critic review: APPROVE after one revision round (tightened the except AbortError gate and reworked the sibling_error test to exercise a real failure payload)

TS parity

Mirrors typescript/src/services/tools/StreamingToolExecutor.ts:153-205 (createSyntheticErrorMessage for user_interrupted) and :278-292/:332-345 (the initial-abort branch + per-iteration abort check).

Divergence noted in _is_user_cancelled_abort docstring: TS distinguishes 'interrupt' (mid-stream submit) from 'user_interrupted' (ESC) via a per-tool interruptBehavior() === 'cancel' gate. Python today emits neither 'interrupt' nor any per-tool interrupt_behavior, so the collapsed check is sound. Any future 'interrupt' wire-up must land the per-tool gate first.

Follow-ups (out of scope)

  • Bash timeout produces the same <error>Command was aborted before completion</error> payload but with signal.aborted == False, so the post-tool override doesn't fire. The model may still retry on timeout — pre-existing behavior.
  • Subagent contexts spawned mid-turn hold their own ToolContext snapshot; if ESC trips before reset_abort_controller, a delayed wake-up could see the previously-aborted controller and trigger REJECT_MESSAGE spuriously.

🤖 Generated with Claude Code

…ction path

When ESC fires during a Bash command, the production tool-dispatch path
(`_dispatch_single_tool` in `src/query/query.py`) was passing the bash
tool's `<error>Command was aborted before completion</error>` payload
through to the model. On the resume turn, the model read this as a
generic failure and retried the bash command instead of honouring the
user's cancel.

The TS reference at `StreamingToolExecutor.ts:153-205` overrides the
tool_result with REJECT_MESSAGE when the abort reason is
`user_interrupted`, but the Python production REPL bypasses
`StreamingToolExecutor` (it dispatches via `_run_tools_partitioned` →
`_dispatch_single_tool`), so the override never fired in production.

Add the override at four sites in `_dispatch_single_tool` — pre-tool
gate, post-tool override, `AbortError` catch, and a late-abort tail in
the generic exception handler — all funneling through two helpers
(`_is_user_cancelled_abort`, `_build_user_cancelled_result`). The
`sibling_error` reason is carved out so the streaming-executor's
parallel-tool cascade doesn't get mislabelled as a user rejection.

The `except AbortError` branch re-gates on the same user-cancel check
so future tools that repurpose `AbortError` for their own internal
cancellation aren't silently relabelled as "user rejected".

Pinned by 13 tests in `tests/test_esc_reject_message_dispatch.py`
covering each override site, the `sibling_error` carve-out, normal
completion, and the defensive `AbortError`-without-signal-aborted
case. 217/217 tests pass across the abort/ESC/query/bash domain.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ericleepi314
ericleepi314 merged commit 5fdbe79 into main May 16, 2026
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
…l-reject-message-production-path

fix(esc): override cancelled tool_result with REJECT_MESSAGE in production path
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
After the REJECT_MESSAGE override fix in PR agentforce314#150, a critic flagged a
defensive concern: when the user presses ESC and then types resume,
the engine's reset_abort_controller swaps in a fresh controller. If a
subagent spawned in the previous turn held a stale (aborted) parent
controller snapshot, _dispatch_single_tool would emit REJECT_MESSAGE
for every tool the subagent ran — spurious user-rejected results.

Verification confirmed the existing Python code already mirrors TS at
typescript/src/tools/AgentTool/runAgent.ts:533-541:

* Async agents get a fresh, unlinked AbortController at
  src/agent/run_agent.py:281-284.
* Sync agents share params.parent_context.abort_controller at line 287
  (current reference, mutated in place by engine.reset_abort_controller
  so the field reflects the new controller without snapshot staleness).
* Engine.reset_abort_controller mutates the controller on the SAME
  tool_context object, so any holder of the context reference sees the
  new controller automatically.

This is pure regression-test coverage — no production-code change — to
lock down the invariants so a future refactor that switches the async
path to a child-linked controller (which WOULD propagate parent abort
and re-introduce the bug) is caught immediately.

The 4 tests cover:

1. async path: explicit override → child controller IS the fresh
   instance, NOT a child-linked wrapper; parent abort does NOT
   propagate to child.
2. sync path: shared controller → child.abort_controller IS the
   parent's controller (same object); parent ESC reaches child for free.
3. End-to-end: parent ESC fires, parent controller swapped for a fresh
   one (simulating engine.reset_abort_controller), async subagent's
   controller remains untouched throughout.
4. Default fallback: no override, no share → child-linked branch fires
   (parent-abort propagates down one-way; child-abort does NOT
   propagate up).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
- Codebase stats: 894 files / 177,428 lines (up from 167,034 on
  2026-05-14; ~+10.4k lines in two days).
- New news entries:
  * 2026-05-16 Image-handling parity (agentforce314#149/agentforce314#154/agentforce314#155/agentforce314#156) — Read tool
    TS image pipeline; @image.png mention fix; image tool_result list
    shape preserved; OpenAI-compat image/document block translation;
    validate_images_for_api promoted into BaseProvider._prepare_messages;
    BOM-aware text decoding.
  * 2026-05-16 Subagent + Bash reliability — custom subagent discovery
    from .claude/agents/ (agentforce314#151); Bash timeout vs ESC-abort distinction
    (agentforce314#152); async-subagent AbortController isolation tests (agentforce314#153);
    REJECT_MESSAGE on cancelled production path (agentforce314#150).
  * 2026-05-15 to 2026-05-16 ESC cancellation hardening across providers
    (agentforce314#144agentforce314#148) — mid-stream cancel under ~50ms for every provider;
    shared StreamAbortGuard; LiteLLM worker-thread iteration fix.
- Core Systems table:
  * Multi-Provider description extends to call out Anthropic→OpenAI
    image/document block translation.
  * New "Cancellation / Abort" row (✅) capturing the recent sweep.
  * New "Image Handling" row (✅) capturing Tier C parity.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant