Skip to content

fix(acp): honor configured max turns - #84205

Open
desmond-rai wants to merge 1 commit into
NousResearch:mainfrom
desmond-rai:fix/acp-max-turns-config
Open

fix(acp): honor configured max turns#84205
desmond-rai wants to merge 1 commit into
NousResearch:mainfrom
desmond-rai:fix/acp-max-turns-config

Conversation

@desmond-rai

Copy link
Copy Markdown

Summary

  • propagate agent.max_turns into ACP-created AIAgent instances
  • retain root-level max_turns compatibility
  • use the documented 500-turn default when no value is configured
  • add regression coverage for ACP config propagation

Root cause

The ACP session manager loaded Hermes configuration but never passed max_iterations into AIAgent. Paseo and other ACP clients therefore silently used the constructor default instead of the user-configured value.

Verification

  • uv run --extra dev --extra acp pytest -q tests/acp_adapter (15 passed)
  • uv run --extra dev ruff check acp_adapter/session.py tests/acp_adapter/test_acp_commands.py
  • python3 -m compileall -q acp_adapter/session.py tests/acp_adapter/test_acp_commands.py
  • git diff --check

@desmond-rai
desmond-rai force-pushed the fix/acp-max-turns-config branch from 6797235 to 241b1f4 Compare August 12, 2026 02:08
@alt-glitch alt-glitch added type/bug Something isn't working comp/acp Agent Communication Protocol adapter area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P4 Best-effort: we will get to it when we get to it (no commitment) duplicate This issue or pull request already exists labels Aug 12, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #70592. Both propagate agent.max_turns into ACP-created AIAgent.max_iterations; #70592 is the earlier open implementation and validates the configured value before forwarding it.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

PR: fix(acp): honor configured max turns

  1. This overlaps with the earlier open fix(acp): honor configured max turns #70592, which propagates agent.max_turns into ACP-created AIAgent.max_iterations and additionally validates the configured value before forwarding it. Consider aligning with (or closing in favor of) that implementation so the two don't diverge in validation behavior.
  2. max_iterations = agent_cfg.get("max_turns") or config.get("max_turns") or 500 performs no type/range validation. A YAML value such as max_turns: "150" (string) or a negative/zero value flows straight into AIAgent.max_iterations, where the loop's api_call_count < self.max_iterations comparison would raise a TypeError on a string. Coerce with int(...) and clamp to a sane minimum.
  3. The or chain silently swallows an explicit 0 (falls through to the next source / 500). If 0 is meant to mean "no limit", document it; otherwise reject it explicitly so a user's intent is never silently overridden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/acp Agent Communication Protocol adapter duplicate This issue or pull request already exists P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants