feat(runtime): add auto-stop for max iterations in non-interactive mode#2208
feat(runtime): add auto-stop for max iterations in non-interactive mode#2208tdabasinskas wants to merge 1 commit intodocker:mainfrom
Conversation
When max iterations are reached in non-interactive mode (e.g., MCP server), the runtime now automatically stops execution instead of blocking indefinitely waiting for user input. This prevents the system from hanging when `ToolsApproved` is true and provides a clear assistant message explaining why execution was stopped.
|
/review |
|
|
||
| // In non-interactive mode (e.g. MCP server), auto-stop instead of | ||
| // blocking forever waiting for user input. | ||
| if sess.ToolsApproved { |
There was a problem hiding this comment.
🔴 HIGH SEVERITY: Auto-stop logic incorrectly triggers in interactive sessions with tools approved
The auto-stop logic checks only sess.ToolsApproved to determine non-interactive mode, but this flag is set in two distinct scenarios:
- MCP Server Mode (pkg/mcp/server.go:145):
ToolsApprovedis set totruefor non-interactive MCP tool calls - Interactive TUI Mode (pkg/cli/runner.go:119):
ToolsApprovedis set totruewhen users select "ConfirmationApproveSession" to approve all tools
The Problem:
In interactive TUI sessions where users have approved tools, this auto-stop will prevent them from manually resuming after max iterations. The stream exits with return, making it impossible for users to continue the conversation interactively as they would expect in TUI mode.
Recommendation:
The condition should distinguish between truly non-interactive contexts (MCP server, API mode) and interactive TUI sessions with tools approved. Consider:
- Adding a dedicated
sess.NonInteractiveflag, or - Checking
sess.ToolsApproved && !r.isInteractiveMode(), or - Using a different session property that specifically indicates MCP/API server mode
There was a problem hiding this comment.
OK, sounds valid. This flag indeed can also be set mid-session when a user selects "approve all tools" in the TUI, not just in non-interactive modes.
However, I think the practical impact is limited:
-
In the CLI/TUI, the
MaxIterationsReachedEventis emitted before this early return. The outer runner receives it and would attempt rt.Resume(), but the loop has already exited. The result is the stream closes cleanly - the user just can't extend iterations. This is a behavior change but not a hang or crash. -
The current behavior without this patch is that MCP/A2A/eval sessions block forever on
r.resumeChansince there's no consumer to callrt.Resume(). I believe this is the actual bug.
I guess to properly distinguish non-interactive contexts, we'd need something like a sess.NonInteractive runtime option set only in MCP server, A2A, and evaluation modes.
@derekmisler, what are your thoughts on this? Should I update the code to introduce sess.NonInteractive and use it instead?
When max iterations are reached in non-interactive mode (e.g., MCP server), the runtime now automatically stops execution instead of blocking indefinitely waiting for user input. This prevents the system from hanging when
ToolsApprovedis true and provides a clear assistant message explaining why execution was stopped.