diff --git a/agent/tool_guardrails.py b/agent/tool_guardrails.py index 1c4951733e..b432c21d9f 100644 --- a/agent/tool_guardrails.py +++ b/agent/tool_guardrails.py @@ -100,6 +100,49 @@ IDENTICAL_RESULT_STUB_MIN_CHARS = 512 _RESULT_STUB_ARGS_PREVIEW_CHARS = 120 +# Tools whose "failure" is a normal, informative outcome of legitimate work: +# a red test run, a grep with no matches, a failing build during a fix loop, a +# page that times out. Hard stops never fire on these from failure counts of +# DIFFERENT commands (same_tool_failure) — only an exact-args replay with NO +# intervening change, or an identical-result streak, can halt them. +FAILURE_TOLERANT_TOOL_NAMES = frozenset( + { + "terminal", + "execute_code", + "process_manage", + "process", + "browser_navigate", + "web_extract", + } +) + +# A landed mutation between two attempts means the retry is a NEW experiment +# (edit -> re-run) rather than a replay. A successful call to one of these +# marks progress for every failing signature still being counted this turn. +PROGRESS_RESET_TOOL_NAMES = frozenset( + { + "write_file", + "patch", + "terminal", + "execute_code", + "browser_click", + "browser_type", + "browser_press", + "browser_navigate", + "process_manage", + "process", + "delegate_task", + "send_message", + "cronjob", + "cronjob_manage", + "todo", + "todo_list", + "memory", + "skill_manage", + } +) + + def is_stall_guard_repeatable(tool_name: str) -> bool: """Whether a tool is exempt from the identical-call loop notice.""" if tool_name in STALL_GUARD_REPEATABLE_TOOLS: @@ -238,12 +281,21 @@ class LoopCapConfig: _INTERACTIVE_PLATFORMS = frozenset({"cli", "tui", "desktop", "acp"}) +# Platforms that are not chat gateways but whose work is a bounded, supervised +# task loop: a subagent inherits its parent's budget and is stopped by the +# parent; api_server runs have a live client holding the request. Both do +# real edit -> re-run work, so they keep the interactive (warn-only) default. +_SUPERVISED_TASK_PLATFORMS = frozenset({"subagent", "api_server"}) + def _is_non_interactive_platform(platform: str | None) -> bool: """Return true for gateway/cron sessions where tool loops are unattended.""" if not isinstance(platform, str) or not platform.strip(): return False - return platform.strip().lower() not in _INTERACTIVE_PLATFORMS + key = platform.strip().lower() + if key in _INTERACTIVE_PLATFORMS or key in _SUPERVISED_TASK_PLATFORMS: + return False + return True @dataclass(frozen=True) @@ -368,6 +420,8 @@ class ToolCallGuardrailController: def reset_for_turn(self) -> None: self._exact_failure_counts: dict[ToolCallSignature, int] = {} self._same_tool_failure_counts: dict[str, int] = {} + # signature -> a mutating call succeeded since its last failure + self._progress_since_failure: dict[ToolCallSignature, bool] = {} self._no_progress: dict[ToolCallSignature, tuple[str, int]] = {} self._halt_decision: ToolGuardrailDecision | None = None # Identical-call loop-breaker state (agent.stall_guards): tracks the @@ -417,6 +471,10 @@ class ToolCallGuardrailController: return ToolGuardrailDecision(tool_name=tool_name, signature=signature) exact_count = self._exact_failure_counts.get(signature, 0) + if self._progress_since_failure.get(signature): + # Something landed since this call last failed — let it run; the + # streak restarts in after_call if it fails again. + exact_count = 0 if exact_count >= self.config.exact_failure_block_after: decision = ToolGuardrailDecision( action="block", @@ -469,6 +527,12 @@ class ToolCallGuardrailController: failed, _ = classify_tool_failure(tool_name, result) if failed: + # An identical failing call is only a REPLAY if nothing landed in + # between. If any mutating call succeeded since the previous + # identical failure (edit -> re-run pytest, click -> re-snapshot), + # the retry is a new experiment: restart the exact-args streak. + if self._progress_since_failure.pop(signature, False): + self._exact_failure_counts.pop(signature, None) exact_count = self._exact_failure_counts.get(signature, 0) + 1 self._exact_failure_counts[signature] = exact_count self._no_progress.pop(signature, None) @@ -476,7 +540,17 @@ class ToolCallGuardrailController: same_count = self._same_tool_failure_counts.get(tool_name, 0) + 1 self._same_tool_failure_counts[tool_name] = same_count - if self.config.hard_stop_enabled and same_count >= self.config.same_tool_failure_halt_after: + # same_tool_failure counts DIFFERENT args on one tool. For tools + # whose non-zero exit is ordinary work output (terminal, + # execute_code, pollers) a run of distinct red commands is + # diagnosis, not a loop — warn, never halt. The exact-args replay + # path still applies to them. + same_tool_halt_eligible = tool_name not in FAILURE_TOLERANT_TOOL_NAMES + if ( + self.config.hard_stop_enabled + and same_tool_halt_eligible + and same_count >= self.config.same_tool_failure_halt_after + ): decision = ToolGuardrailDecision( action="halt", code="same_tool_failure_halt", @@ -520,6 +594,16 @@ class ToolCallGuardrailController: self._exact_failure_counts.pop(signature, None) self._same_tool_failure_counts.pop(tool_name, None) + # A successful mutation is progress for every failing signature still + # being counted this turn: the next identical retry runs against + # changed state, so it is a fresh attempt rather than a replay. Pure + # loops never mutate anything between attempts, so the replay detector + # keeps its teeth. + if tool_name in PROGRESS_RESET_TOOL_NAMES or file_mutation_result_landed(tool_name, result): + for sig in list(self._exact_failure_counts): + self._progress_since_failure[sig] = True + self._same_tool_failure_counts.clear() + if not self._is_idempotent(tool_name): self._no_progress.pop(signature, None) return ToolGuardrailDecision(tool_name=tool_name, signature=signature) diff --git a/tests/agent/test_tool_guardrails.py b/tests/agent/test_tool_guardrails.py index 70d07829f4..63ae5debd3 100644 --- a/tests/agent/test_tool_guardrails.py +++ b/tests/agent/test_tool_guardrails.py @@ -290,3 +290,89 @@ def test_web_search_cap_blocks_after_limit_regardless_of_hard_stop(): + + +# ── Legitimate flows must survive hard stops (Teknium, Sep 2026) ──────────── +# Hard stops default ON for unattended platforms. These pin the flows that +# must NEVER be cut off there: edit -> re-run loops, diagnostic sweeps of +# distinct red commands, and browser retry-after-action — while the pure +# replay (same call, nothing changed between attempts) is still stopped. + +_HARD = lambda: ToolCallGuardrailController( # noqa: E731 + ToolCallGuardrailConfig(hard_stop_enabled=True) +) +_PYTEST = {"command": "pytest tests/test_x.py -q"} +_RED = '{"output": "1 failed", "exit_code": 1}' + + +def _run_red(c, args=_PYTEST): + assert c.before_call("terminal", args).allows_execution + return c.after_call("terminal", args, _RED, failed=True) + + +def test_fix_retest_loop_is_never_hard_stopped(): + c = _HARD() + for i in range(12): + d = _run_red(c) + assert not d.should_halt, f"halted on red run {i + 1}" + # the model edits between runs — a landed mutation is progress + c.after_call("patch", {"path": "x.py", "old_string": "a", "new_string": f"b{i}"}, + '{"success": true, "diff": "..."}', failed=False) + assert c.halt_decision is None + assert c.before_call("terminal", _PYTEST).allows_execution + + +def test_pure_replay_with_no_intervening_change_is_still_blocked(): + c = _HARD() + for _ in range(5): + _run_red(c) + d = c.before_call("terminal", _PYTEST) + assert d.action == "block" and d.code == "repeated_exact_failure_block" + + +def test_intervening_mutation_resets_the_replay_streak_only_once(): + # 4 reds, one edit, then 4 reds with NO edit: the second run of 4 is a + # fresh streak, and the 5th unchanged retry after it is blocked. + c = _HARD() + for _ in range(4): + _run_red(c) + c.after_call("write_file", {"path": "x.py", "content": "y"}, '{"bytes_written": 1}', failed=False) + for _ in range(5): + assert c.before_call("terminal", _PYTEST).allows_execution + c.after_call("terminal", _PYTEST, _RED, failed=True) + assert c.before_call("terminal", _PYTEST).action == "block" + + +def test_distinct_failing_terminal_commands_warn_but_never_halt(): + # A diagnostic sweep: grep with no matches, missing binaries, red builds. + c = _HARD() + for i in range(12): + args = {"command": f"grep -q needle{i} haystack.txt"} + d = c.after_call("terminal", args, _RED, failed=True) + assert not d.should_halt, f"same_tool halt on distinct command #{i + 1}" + assert c.halt_decision is None + # ...while a non-tolerant tool failing 8 distinct ways still halts. + c2 = _HARD() + last = None + for i in range(8): + last = c2.after_call("send_message", {"to": f"u{i}"}, '{"error": "no route"}', failed=True) + assert last.should_halt and last.code == "same_tool_failure_halt" + + +def test_browser_retry_after_action_is_not_a_replay(): + c = _HARD() + nav = {"url": "https://example.test/app"} + for _ in range(8): + assert c.before_call("browser_navigate", nav).allows_execution + c.after_call("browser_navigate", nav, '{"error": "timeout"}', failed=True) + c.after_call("browser_click", {"selector": "#retry"}, '{"ok": true}', failed=False) + assert c.halt_decision is None + + +def test_supervised_task_platforms_keep_warning_only_default(): + for platform in ("subagent", "api_server", "cli"): + cfg = ToolCallGuardrailConfig.from_mapping({}, platform=platform) + assert cfg.hard_stop_enabled is False, platform + for platform in ("telegram", "discord", "cron", "kanban"): + cfg = ToolCallGuardrailConfig.from_mapping({}, platform=platform) + assert cfg.hard_stop_enabled is True, platform diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 763297796e..95c0a1c030 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -1787,7 +1787,13 @@ tool_loop_guardrails: max_subagents: 50 # max subagents spawned per turn (0 = unlimited) ``` -`hard_stop_enabled` explicitly enables hard stops on every platform. When it remains `false`, `non_interactive_hard_stop_enabled` still enables them for unattended gateway/cron-style platforms while preserving warning-only behavior for CLI, TUI, Desktop, and ACP. Set `non_interactive_hard_stop_enabled: false` to opt an unattended deployment out. See also [Docker / unattended deployments](docker.md). +`hard_stop_enabled` explicitly enables hard stops on every platform. When it remains `false`, `non_interactive_hard_stop_enabled` still enables them for unattended gateway/cron-style platforms while preserving warning-only behavior for CLI, TUI, Desktop, ACP, subagents, and `api_server` runs (supervised task loops with a live parent or client). Set `non_interactive_hard_stop_enabled: false` to opt an unattended deployment out. See also [Docker / unattended deployments](docker.md). + +Hard stops are designed to catch **replays** — the same call, unchanged, with nothing happening in between — not legitimate iteration: + +- **Edit → re-run is never a loop.** Any successful mutating call (`write_file`, `patch`, a green `terminal`/`execute_code`, a browser action, a job/message/cron mutation) marks progress for every failing call still being counted. The next identical retry (re-running a red test after a fix, re-snapshotting after a click) starts a fresh streak instead of accumulating toward a block. +- **Distinct red commands are diagnosis, not a loop.** For tools whose non-zero exit is ordinary output (`terminal`, `execute_code`, process pollers, `browser_navigate`, `web_extract`) the `same_tool_failure` threshold only warns and never halts. Only an exact-args replay with no intervening change, or an identical-result streak, can stop them. +- **A halt ends the turn, not the session.** The agent replies with which guardrail fired and why; replying "continue" resumes with fresh per-turn counters. ### Per-turn runaway-loop caps