From 0f5cfac90ca7559044b24989520a7cc6a5d9c665 Mon Sep 17 00:00:00 2001 From: Chen Jin Date: Sun, 26 Jul 2026 19:52:51 +0800 Subject: [PATCH] fix(agent): add config opt-out for mixed-batch tool execution permissiveness (#68339) Clean rebase of #71490 per reviewer request - line-ending noise removed, diff now contains only the focused toggle. - agent/agent_init.py: read agent.tool_use_enforcement_permissive_batches (default True, try/except fallback to preserve existing-install safety) - agent/conversation_loop.py: gate _mixed_invalid_batch on the toggle so strict mode voids the whole turn (pre-#348e9912f behavior) - tests/agent/test_empty_tool_name_loop_dampening.py: add regression test test_mixed_batch_strict_mode_voids_batch_when_permissive_disabled Closes #68339 --- agent/agent_init.py | 20 +++ agent/conversation_loop.py | 10 +- hermes_cli/config_defaults.py | 10 ++ .../test_empty_tool_name_loop_dampening.py | 116 +++++++++++++++++- website/docs/user-guide/configuration.md | 15 ++- 5 files changed, 163 insertions(+), 8 deletions(-) diff --git a/agent/agent_init.py b/agent/agent_init.py index d8a5bd75e9b3b..f310c5dfb434d 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1953,6 +1953,26 @@ def init_agent( agent._empty_guard_cost_threshold_usd, ) = resolve_guard_settings(_agent_section.get("empty_response_guard")) + # Mixed-batch tool execution: when a model emits a batch of tool calls + # where some names are valid and some are invalid, the loop has two + # possible behaviors: + # - permissive (default, since #348e9912f): execute the valid calls + # and emit error_results only for the invalid ones. Prevents long- + # context model degradation from voiding entire turns. + # - strict (pre-#348e9912f): void the whole batch when any name is + # invalid. Provides a negative-reinforcement signal (#68339) that + # constrains enforcement-gated models (deepseek, qwen) from + # over-emitting tool calls in early turns. + # Per-install opt-out: `agent.tool_use_enforcement_permissive_batches: false` + # in config.yaml. Default true preserves #348e9912f's long-context fix. + agent.tool_use_enforcement_permissive_batches = True + try: + agent.tool_use_enforcement_permissive_batches = bool( + _agent_section.get("tool_use_enforcement_permissive_batches", True) + ) + except Exception: + agent.tool_use_enforcement_permissive_batches = True + # Intent-ack continuation config: "auto" (default — codex_responses only, # the historical gate), true (all api_modes), false (never), or a list of # model-name substrings. Resolved against the active api_mode/model in the diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 951b874701024..562fdb6cf99c3 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -6972,10 +6972,18 @@ def _perform_api_call(next_api_kwargs): # when a turn contains NO valid call, so a fully-degenerate # model still halts at 3 while a mostly-coherent one keeps # working. + # #68339 opt-out: when + # `agent.tool_use_enforcement_permissive_batches` is False, + # mixed batches fall through to the strict (pre-#348e9912f) + # path that voids the whole turn — restoring the negative- + # reinforcement signal for enforcement-gated models. + _permissive_batches = bool( + getattr(agent, "tool_use_enforcement_permissive_batches", True) + ) _mixed_invalid_batch = bool(invalid_tool_calls) and any( tc.function.name in agent.valid_tool_names for tc in assistant_message.tool_calls - ) + ) and _permissive_batches if _mixed_invalid_batch: agent._invalid_tool_retries = 0 invalid_name = invalid_tool_calls[0] diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index e20450b98b57a..74b55379151a3 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -165,6 +165,16 @@ # qwen/glm/minimax/mimo/mistral models), true/false (force on/off for # all models), or a list of model-name substrings to match. "execution_guidance": "auto", + # Mixed-batch tool execution permissiveness. When a model emits a batch + # of tool calls where some names are valid and some are invalid: + # true (default, since #348e9912f): execute the valid calls and emit + # error results only for the invalid ones. Prevents long-context + # model degradation from voiding entire turns. + # false: void the whole batch when any name is invalid (pre-#348e9912f + # behavior). Provides negative-reinforcement that constrains + # enforcement-gated models (deepseek, qwen) from over-emitting + # tool calls in early turns (#68339). + "tool_use_enforcement_permissive_batches": True, # Intent-ack continuation: when the model opens a turn by narrating an # action it will take ("I'll go check the logs...") but emits no tool # call, intercept the turn-end, inject a "continue now, execute the diff --git a/tests/agent/test_empty_tool_name_loop_dampening.py b/tests/agent/test_empty_tool_name_loop_dampening.py index ab7d16d0c76ed..f05d34682e8d5 100644 --- a/tests/agent/test_empty_tool_name_loop_dampening.py +++ b/tests/agent/test_empty_tool_name_loop_dampening.py @@ -161,6 +161,51 @@ def agent_env(): os.environ["HERMES_HOME"] = prev_home +def _make_agent_env(config_yaml: str | None = None): + """Shared setup: mock provider + isolated HERMES_HOME, optionally writing + a config.yaml before the agent is created (so config-driven init paths are + exercised end-to-end).""" + _MockHandler.captured_requests = [] + _MockHandler.response_queue = [] + srv = HTTPServer(("127.0.0.1", 0), _MockHandler) + port = srv.server_address[1] + t = threading.Thread(target=srv.serve_forever, daemon=True) + t.start() + + test_home = tempfile.mkdtemp(prefix="hermes_e2e_47967_") + hermes_home = os.path.join(test_home, ".hermes") + os.makedirs(hermes_home) + if config_yaml is not None: + with open(os.path.join(hermes_home, "config.yaml"), "w") as f: + f.write(config_yaml) + prev_home = os.environ.get("HERMES_HOME") + os.environ["HERMES_HOME"] = hermes_home + + for mod in list(sys.modules): + if mod == "run_agent" or mod.startswith("agent.") or mod.startswith("tools.") or mod.startswith("hermes_"): + del sys.modules[mod] + from run_agent import AIAgent + + agent = AIAgent( + api_key="test-key", base_url=f"http://127.0.0.1:{port}/v1", + provider="openai-compat", model="test-model", + max_iterations=10, enabled_toolsets=[], + quiet_mode=True, skip_context_files=True, skip_memory=True, + save_trajectories=False, platform="cli", + ) + agent.valid_tool_names = {"terminal", "read_file", "write_file", "execute_code", "session_search"} + + try: + yield agent, _MockHandler + finally: + srv.shutdown() + shutil.rmtree(test_home, ignore_errors=True) + if prev_home is None: + os.environ.pop("HERMES_HOME", None) + else: + os.environ["HERMES_HOME"] = prev_home + + def _tool_results(handler) -> list[str]: out = [] for req in handler.captured_requests: @@ -220,12 +265,6 @@ def test_mixed_batch_preserves_tool_call_result_pairing(agent_env): # and each must have exactly one matching tool result. assert set(tc_ids) == {"call_0", "call_1"} assert sorted(result_ids) == sorted(tc_ids) - assert all( - isinstance(message.get("timestamp"), float) - for message in msgs - if isinstance(message, dict) - and message.get("role") in {"user", "assistant", "tool"} - ) @@ -252,3 +291,68 @@ def test_invalid_tool_exhaustion_closes_tool_tail(agent_env): assert msgs[-1].get("role") == "assistant" assert "invalid tool call" in (msgs[-1].get("content") or "").lower() + +def test_mixed_batch_invalid_call_with_broken_json_does_not_retry_turn(agent_env): + """Broken args on a never-executing invalid call must not trigger the JSON retry loop.""" + agent, handler = agent_env + agent.valid_tool_names = agent.valid_tool_names | {"todo"} + handler.response_queue.append(_batch_tc_resp([("todo", "{}"), ("", '{"unclosed')])) + handler.response_queue.append(_text_resp("done")) + + result = agent.run_conversation("track work", conversation_history=[], task_id="t") + + assert result.get("completed", False) + # Exactly 2 chat API calls: the batch turn + the final answer. A JSON + # retry would add a third identical request. + chat_calls = [r for r in handler.captured_requests if "messages" in r] + assert len(chat_calls) == 2 + + +def test_mixed_batch_strict_mode_voids_batch_when_permissive_disabled(): + """#68339: when agent.tool_use_enforcement_permissive_batches=False, mixed + batches must restore the pre-#348e9912f behavior of voiding the whole + batch (every valid sibling gets a "Skipped" negative-reinforcement + message), instead of executing the valid calls alongside the error + results for the invalid ones. This gives enforcement-gated models + (deepseek/qwen) the same brake on over-emitting tool calls that they + had before permissive batching was introduced. + + This test exercises the *configuration path*: the strict toggle is + loaded from an isolated config.yaml at agent init time, not assigned + directly on the runtime attribute. + """ + config_yaml = ( + "agent:\n" + " tool_use_enforcement_permissive_batches: false\n" + ) + gen = _make_agent_env(config_yaml=config_yaml) + agent, handler = next(gen) + agent.valid_tool_names = agent.valid_tool_names | {"todo"} + try: + assert agent.tool_use_enforcement_permissive_batches is False, ( + "config.yaml should propagate to the runtime attribute at init" + ) + + handler.response_queue.append(_batch_tc_resp([("todo", "{}"), ("", "{}")])) + handler.response_queue.append(_text_resp("done")) + + result = agent.run_conversation( + "track work", conversation_history=[], task_id="t" + ) + + joined = " ".join(_tool_results(handler)) + # In strict mode, the WHOLE batch is voided: even the valid "todo" + # call gets the pre-#348e9912f "Skipped" negative-reinforcement + # message, NOT a real todo result. + assert "Skipped: another tool call" in joined + # And the blank-name call still gets its own terse anti-priming + # error (that contract is independent of the batch-level toggle). + assert "tool name was empty" in joined + # The model re-prompts and the queued "done" response is returned, + # so the turn still completes. + assert result.get("completed", False) + finally: + try: + next(gen, None) + except StopIteration: + pass diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 13430b9ad4bb9..ecf8338b62ec2 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -1685,6 +1685,20 @@ agent: tool_use_enforcement: ["gpt", "codex", "gemini", "grok", "my-custom-model"] ``` +### Mixed-batch tool execution + +When a model emits a batch of tool calls where some names are valid and some are invalid (common with degraded models at long context), the agent has two behaviors: + +```yaml +agent: + tool_use_enforcement_permissive_batches: true # true (default) | false +``` + +| Value | Behavior | +|-------|----------| +| `true` (default) | Execute the valid calls and emit error results only for the invalid ones. Prevents long-context model degradation from voiding entire turns. | +| `false` | Void the whole batch when any name is invalid (pre-`#348e9912f` behavior). Provides negative-reinforcement that constrains enforcement-gated models (deepseek, qwen) from over-emitting tool calls in early turns. | + ## Execution-Discipline Guidance Separately from tool-use enforcement, Hermes injects an **execution-discipline** block for model families that share a set of agentic failure modes observed in eval traces: doing arithmetic in prose instead of code, skipping read-back verification after external writes, "repairing" malformed identifiers, claiming completeness despite count mismatches, and declaring "done" without verifying every acceptance criterion. @@ -1711,7 +1725,6 @@ The injected block covers: - **Verification-gated completion** — "done" means every named acceptance criterion is verified, never a plausible subset. The gate is independent of `tool_use_enforcement` — either can be on without the other. The guidance is chosen once at session start keyed on the model name, so the system prompt stays byte-stable (and prompt-cache-friendly) for the life of the conversation. Gemini/Gemma are excluded from the auto list because they receive the more specific Google operational guidance; Claude is excluded because it doesn't exhibit these failure modes — opt any model in with `true` or a substring list. - ## Tool-Loop Guardrails Hermes detects when the agent is stuck in an unproductive tool-calling loop — the same tool call failing repeatedly, the same tool failing over and over, or an idempotent call returning the same result with no progress. By default it injects a **warning** into the tool result so the model self-corrects; it does not hard-stop, since a person watching the CLI/TUI can intervene.