Skip to content

fix(tool-executor): set tool_name on sequential per-iteration interrupt skip messages - #29100

Closed
EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/sequential-interrupt-tool-name
Closed

fix(tool-executor): set tool_name on sequential per-iteration interrupt skip messages#29100
EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/sequential-interrupt-tool-name

Conversation

@EloquentBrush0x

Copy link
Copy Markdown
Contributor

Summary

execute_tool_calls_sequential checks _interrupt_requested at the top of each loop iteration before starting the next tool. When triggered, it builds skip messages manually — without tool_name. PR #28914 fixed every other interrupt path in the file to use make_tool_result_message, but this per-iteration block was missed.

Before (manual dict, tool_name absent):

skip_msg = {
    "role": "tool",
    "name": skipped_name,
    "content": f"[Tool execution cancelled — {skipped_name} was skipped due to user interrupt]",
    "tool_call_id": skipped_tc.id,
}
messages.append(skip_msg)

After (symmetric with every other interrupt path):

messages.append(make_tool_result_message(
    skipped_name,
    f"[Tool execution cancelled — {skipped_name} was skipped due to user interrupt]",
    skipped_tc.id,
))

Coverage of all interrupt skip paths in tool_executor.py after this fix:

Path Fixed in
execute_tool_calls_concurrent pre-flight #28914
execute_tool_calls_sequential end-of-loop #28914
execute_tool_calls_sequential per-iteration ← this PR

Test plan

  • New regression test TestPreToolCheck::test_sequential_per_iteration_interrupt_sets_tool_name asserts tool_name == name on all skip messages produced by the per-iteration check
  • All existing interrupt tests pass (tests/tools/test_interrupt.py — 7 passed)
  • Existing test_tool_name_db_persistence.py still passes (1 passed)
  • make_tool_result_message was already imported; no new imports needed
  • Zero remaining manual {"role": "tool", ...} constructions in tool_executor.py

…pt skip messages

execute_tool_calls_sequential checked _interrupt_requested at the top of
each loop iteration and built skip messages manually — without tool_name.
The sibling concurrent path and the end-of-loop sequential path were both
already converted to make_tool_result_message by NousResearch#28914, but this per-
iteration block was missed.

Result: interrupted sequential tool calls produced blank tool_name entries
in the session DB, breaking the invariant introduced by NousResearch#28914.

Fix: replace the manual dict with make_tool_result_message, symmetric with
every other interrupt skip path in the same file.

Adds a regression test that asserts tool_name == name on all skip messages
produced by the per-iteration interrupt check.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 20, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. I verified the premise against current main, and this still looks like a real gap.

The top-of-loop sequential interrupt path in agent/tool_executor.py:782 still manually constructs the skipped tool result without tool_name. The helper this PR switches to, make_tool_result_message, sets both name and tool_name at agent/tool_dispatch_helpers.py:337, matching the sibling interrupt paths already using it at agent/tool_executor.py:253 and agent/tool_executor.py:1396.

I do not see blockers in the implementation or the regression test. The change is narrow, fits the existing helper pattern introduced by #28914, and does not add new core surface or cache-sensitive behavior.

This is an automated hermes-sweeper review.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression report and test coverage.

This is an automated hermes-sweeper review. Current main already provides the requested guarantee:

  • agent/tool_executor.py:1036 builds the sequential per-iteration interrupt skip result with make_tool_result_message(...).
  • agent/tool_dispatch_helpers.py:389-395 sets both name and tool_name on that message.
  • tests/run_agent/test_tool_name_db_persistence.py:28-45 already verifies helper-created tool results persist tool_name through the SessionDB flush path.
  • The current implementation arrived with d682f320b35a13084371a541a835e1d988c982b8; the PR now conflicts with main.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants