Skip to content

Conversation

@saksharthakkar
Copy link
Contributor

@saksharthakkar saksharthakkar commented Dec 29, 2025

Summary

Fixes escalation tool that was broken due to missing tool_call_id when returning Command.

Problem

  • Escalation tool required runtime: ToolRuntime parameter to get tool_call_id
  • Nothing was injecting ToolRuntime into the tool
  • Result: TypeError: escalation_tool_fn() missing 1 required positional argument: 'runtime'

Solution

Use wrapper pattern consistent with other tools:

  1. Tool returns dict (like process_tool, context_tool, etc.)

    • escalation_tool_fn returns {"action": ..., "output": ..., "escalation_action": ...}
    • Graph-agnostic, no knowledge of Commands or Messages
  2. Wrapper handles graph integration

    • escalation_wrapper receives (tool, call, state) where call["id"] is the tool_call_id
    • Converts result to Command when termination needed
    • Uses ToolWrapperMixin to attach wrapper to tool
  3. Simplified tool_node.py

    • Removed ToolRuntime injection logic that was never fully implemented
    • Clean separation: tools return data, wrappers handle graph concerns

Why escalation is different from other tools

  • Other tools (process, context, integration) always continue - just return data
  • Escalation can END the graph based on user action - needs to return Command
  • Wrapper pattern elegantly handles this: tool returns data, wrapper decides Command vs dict

Changes

  • escalation_tool.py: Returns dict, wrapper converts to Command
  • tool_node.py: Removed unused ToolRuntime injection code
  • tool_factory.py: Fixed return type
  • test_tool_node.py: Cleaned up imports
  • pyproject.toml: Version bump to 0.2.5

Test plan

  • pytest tests/agent/tools/test_tool_node.py - all pass
  • ruff check . - no errors
  • mypy - no new errors

@saksharthakkar saksharthakkar changed the title feat: add ToolRuntime injection for interruptible tools fix: escalation tool missing runtime parameter Jan 3, 2026
@saksharthakkar saksharthakkar force-pushed the feat/tool-runtime-injection branch 7 times, most recently from 19495a5 to 7129020 Compare January 3, 2026 00:31
@saksharthakkar saksharthakkar marked this pull request as ready for review January 3, 2026 00:39
@saksharthakkar saksharthakkar force-pushed the feat/tool-runtime-injection branch from 7129020 to 4869ffe Compare January 3, 2026 00:43
@smflorentino smflorentino self-requested a review January 3, 2026 01:44
Copy link

@Chibionos Chibionos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with a design comment, @saksharthakkar can't the StructuredTool be improved ? rather than creating a new class ?

@saksharthakkar saksharthakkar force-pushed the feat/tool-runtime-injection branch from 4869ffe to 55997f5 Compare January 4, 2026 02:49
The escalation tool was failing with:
  TypeError: escalation_tool_fn() missing 1 required positional argument: 'runtime'

Root cause: Tool expected `runtime: ToolRuntime` param but nothing provided it.

Solution: Use wrapper pattern instead of injection
- Tool returns graph-agnostic EscalationResult dataclass
- Wrapper converts result to Command using call["id"] (tool_call_id)
- Remove ToolRuntime injection code from tool_node.py

This follows reviewer feedback: tools should be graph-agnostic,
wrappers handle graph integration.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@saksharthakkar saksharthakkar force-pushed the feat/tool-runtime-injection branch from 55997f5 to e207324 Compare January 4, 2026 02:50
@saksharthakkar saksharthakkar dismissed andreitava-uip’s stale review January 4, 2026 03:45

Addressed feedback: refactored to wrapper pattern - tool now returns dict (graph-agnostic), wrapper handles Command.

@saksharthakkar saksharthakkar merged commit 3567f60 into main Jan 4, 2026
60 of 61 checks passed
@saksharthakkar saksharthakkar deleted the feat/tool-runtime-injection branch January 4, 2026 03:45
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.

4 participants