mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(guardrails): no exact-replay warning on a throttled retry after earlier failures
Review follow-up: when the same call had already failed twice for an ordinary reason, a later rate-limited result kept the old exact-args count and still drew the "stop retrying unchanged" warning. A throttled result now never gets that warning; the per-tool cap still bounds it.
This commit is contained in:
1 parent
455e0a7402
commit
7d0fb393b7
2 files changed
+24
-16
No files matched your search
@@ -393,10 +393,12 @@ class ToolCallGuardrailController:
|
||||
if self._progress_since_failure.pop(signature, False):
|
||||
self._exact_failure_counts.pop(signature, None)
|
||||
# A throttled call retried after its cooldown is the correct move, not a replay:
|
||||
# keep it out of the exact-args streak (whose warning says "stop retrying unchanged").
|
||||
# keep it out of the exact-args streak and never give it that streak's "stop retrying
|
||||
# unchanged" warning, even after earlier ordinary failures with the same args.
|
||||
# It still counts per tool below, so same_tool_failure_halt bounds a dead endpoint.
|
||||
throttled = is_rate_limited(result)
|
||||
exact_count = self._exact_failure_counts.get(signature, 0)
|
||||
if not is_rate_limited(result):
|
||||
if not throttled:
|
||||
exact_count = self._exact_failure_counts[signature] = exact_count + 1
|
||||
same_count = self._same_tool_failure_counts[tool_name] = self._same_tool_failure_counts.get(tool_name, 0) + 1
|
||||
self._no_progress.pop(signature, None)
|
||||
@@ -414,7 +416,7 @@ class ToolCallGuardrailController:
|
||||
and same_count >= self.config.same_tool_failure_halt_after
|
||||
):
|
||||
return self._decide("halt", "same_tool_failure_halt", tool_name, same_count, signature)
|
||||
if warnings and exact_count >= self.config.exact_failure_warn_after:
|
||||
if warnings and not throttled and exact_count >= self.config.exact_failure_warn_after:
|
||||
return self._decide("warn", "repeated_exact_failure_warning", tool_name, exact_count, signature)
|
||||
if warnings and same_count >= self.config.same_tool_failure_warn_after:
|
||||
return self._decide(
|
||||
|
||||
@@ -482,21 +482,27 @@ def test_execute_code_replay_streak_notice_fires_on_warn_only_desktop_config():
|
||||
assert controller.halt_decision is None, "warn-only surfaces must not halt"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("result,second_attempt", [
|
||||
(json.dumps({"error": "Slack API error: ratelimited. Retry after 1 seconds."}), "allow"),
|
||||
(json.dumps({"ok": False, "error": "busy", "retry_after": 30}), "allow"),
|
||||
(json.dumps({"error": "channel_not_found"}), "warn"), # control: a real replay still warns
|
||||
@pytest.mark.parametrize("prior_ordinary_failures", [0, 2])
|
||||
@pytest.mark.parametrize("result,throttled", [
|
||||
(json.dumps({"error": "Slack API error: ratelimited. Retry after 1 seconds."}), True),
|
||||
(json.dumps({"ok": False, "error": "busy", "retry_after": 30}), True),
|
||||
(json.dumps({"error": "channel_not_found"}), False), # control: a real replay still warns
|
||||
])
|
||||
def test_rate_limited_retry_is_not_a_replay_but_stays_bounded(result, second_attempt):
|
||||
def test_rate_limited_retry_is_not_a_replay_but_stays_bounded(result, throttled, prior_ordinary_failures):
|
||||
"""Retrying a throttled read after its cooldown must not draw the "stop retrying unchanged"
|
||||
warning, yet an endpoint that never recovers still halts unattended runs on the per-tool cap."""
|
||||
warning (even after earlier ordinary failures with the same args), yet an endpoint that never
|
||||
recovers still halts unattended runs on the per-tool cap."""
|
||||
controller = ToolCallGuardrailController(ToolCallGuardrailConfig.from_mapping({}, platform="webhook"))
|
||||
args = {"channel": "C123"}
|
||||
actions = []
|
||||
tool, args = "mcp_slack_history", {"channel": "C123"}
|
||||
for _ in range(prior_ordinary_failures):
|
||||
controller.after_call(tool, args, json.dumps({"error": "upstream timeout"}))
|
||||
controller.after_call(tool, args, result)
|
||||
retry = controller.after_call(tool, args, result)
|
||||
assert (retry.code == "repeated_exact_failure_warning") is not throttled, retry
|
||||
for _ in range(10):
|
||||
if not controller.before_call("mcp_slack_history", args).allows_execution:
|
||||
actions.append("block")
|
||||
if not controller.before_call(tool, args).allows_execution:
|
||||
break
|
||||
actions.append(controller.after_call("mcp_slack_history", args, result).action)
|
||||
assert actions[1] == second_attempt
|
||||
assert actions[-1] in {"halt", "block"}, actions
|
||||
if controller.after_call(tool, args, result).action == "halt":
|
||||
break
|
||||
else:
|
||||
raise AssertionError("a permanently failing endpoint must still halt or block")
|
||||
Reference in new issue
Block a user