Skip to content

Commit 0addf7e

Browse files
AVADSA25Mikarina13claude
authored
fix(agent-runner): loop-breaker for stuck repeat-loops + web_fetch raw-HTML bug (#229)
* fix(agent-runner): loop-breaker for stuck repeat-loops + web_fetch returns raw HTML Live incident: a Project (research 5 AI startups, draft outreach emails) burned its ENTIRE 80-step budget on checkpoint 1 without ever finishing it. Root cause was two compounding bugs, both fixed: 1. web_fetch returned response.text verbatim for ANY html content-type — raw markup ("<!DOCTYPE html><html lang=...") is unusable for an LLM trying to extract facts from a page. The agent kept re-fetching the same URLs hoping for different content and got identical garbage every time. Fixed with a stdlib-only HTML-to-text extractor (no new dependency — bs4 is installed in this venv but not a pinned requirement, so depending on it would be a CI risk): strips script/style/head/comments, converts block tags to newlines, unescapes entities, and caps output at 12k chars so a single fetch can't blow a step's context budget. Verified against the actual URL from the incident (legible text instead of raw markup). 2. Even with readable results, nothing detected that the agent was making the same call over and over. Qwen's own "return checkpoint_done when satisfied" self-check never caught it — after 80 identical-ish steps it was still issuing near-duplicate web_search/web_fetch calls. Added a deterministic loop-breaker: NoProgressDetected fires when the same (skill, result) signature repeats _REPEAT_THRESHOLD=3 times within a checkpoint, pausing early with an honest, specific reason ("'web_fetch' returned the same result 3x... without new information") instead of grinding through the rest of the budget. Doesn't depend on the model noticing its own loop — this is deterministic, seeded from prior history on resume too so a crash-restart can't reset the counter and re-earn 3 more free repeats. Two existing tests (test_execute_checkpoint_step_budget_exhausted, test_run_agent_step_budget_exhausted_pauses_not_blocks) were mocking a "repeat the same result forever" scenario to test pure budget exhaustion — now correctly caught earlier by the loop-breaker instead. Fixed both to use distinct per-step results so they test what they say they test; added two new tests proving the loop-breaker fires within ~3 repeats (not 60) and does NOT false-positive on genuine step-by-step progress. Verified: full test suite (2469 passed, 0 failed, 79 skipped/env-gated), ruff clean, ran the fixed web_fetch against the real incident URL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: regenerate D-1 skill manifest for web_fetch.py change Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Mickael Farina <farina.mickael@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e876b99 commit 0addf7e

5 files changed

Lines changed: 192 additions & 6 deletions

File tree

codec_agent_runner.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,36 @@ class StepBudgetExhausted(Exception):
727727
"""Per-checkpoint step budget cap reached without checkpoint_done."""
728728

729729

730+
class NoProgressDetected(Exception):
731+
"""The last several actions returned near-identical results — the agent is
732+
stuck in a repeat loop, not making progress. Raised well before the step
733+
budget exhausts so the human sees an early, honest pause instead of the
734+
agent silently grinding through its whole budget on the same dead end.
735+
736+
2026-07: a live Project run (research 5 AI startups) re-fetched the same
737+
URL 15+ times and re-issued the same search 10+ times, burning all 80
738+
steps of checkpoint 1 without ever finishing it — Qwen's own
739+
"return checkpoint_done" self-check never caught it. This is a
740+
deterministic backstop that doesn't depend on the model noticing its own
741+
loop."""
742+
743+
744+
_REPEAT_THRESHOLD = 3 # this many near-identical results anywhere in the
745+
# checkpoint so far => treat as a stuck loop, not luck
746+
747+
748+
def _result_signature(skill: str, result: str) -> str:
749+
"""Cheap normalized signature for repeat-detection: same skill + a
750+
whitespace-collapsed prefix of the result. Two fetches of the same URL (or
751+
two searches surfacing the same top result) collapse to the same
752+
signature even when the LLM phrases the task text slightly differently
753+
each time — task-text similarity is NOT a reliable signal (the model
754+
reliably varies its phrasing while making the exact same call), but
755+
getting back the same information twice is unambiguous."""
756+
norm = re.sub(r"\s+", " ", (result or "")).strip()[:300]
757+
return f"{skill}::{norm}"
758+
759+
730760
def _build_correction_nudge(pv: "PermissionViolation",
731761
action: Action,
732762
agent_grants: Dict[str, Any],
@@ -872,6 +902,15 @@ def _execute_checkpoint(plan_dict: Dict[str, Any],
872902
# (seeded from state.json on resume by _run_agent). Mutated in place + persisted.
873903
if executed_destructive is None:
874904
executed_destructive = []
905+
# Loop-breaker: seed the repeat-signature counter from any PRE-EXISTING
906+
# history (resume-after-crash case) so a checkpoint that was ALREADY
907+
# looping before a restart doesn't get a fresh 3-strike allowance.
908+
_sig_counts: Dict[str, int] = {}
909+
for h in history:
910+
if h.get("_skill_correction_nudge") or h.get("_resume_skipped"):
911+
continue
912+
sig = _result_signature(h.get("skill", ""), h.get("result", ""))
913+
_sig_counts[sig] = _sig_counts.get(sig, 0) + 1
875914
cp_id = str(checkpoint.get("id", ""))
876915
# Floor to DEFAULT so plans with tiny LLM-generated budgets (e.g. 5 or 10)
877916
# don't exhaust before Qwen can finish real work.
@@ -985,6 +1024,19 @@ def _next_action():
9851024
})
9861025
_persist_checkpoint_progress(agent_id, cp_id, history, executed_destructive)
9871026

1027+
# Loop-breaker: this exact (skill, result) pair has now come back
1028+
# _REPEAT_THRESHOLD times in this checkpoint — repeating it isn't
1029+
# teaching the agent anything new. Stop here instead of grinding
1030+
# through the rest of the step budget on the same dead end.
1031+
sig = _result_signature(action.skill, result)
1032+
_sig_counts[sig] = _sig_counts.get(sig, 0) + 1
1033+
if _sig_counts[sig] >= _REPEAT_THRESHOLD:
1034+
raise NoProgressDetected(
1035+
f"'{action.skill}' returned the same result {_sig_counts[sig]}x in "
1036+
f"checkpoint {cp_id!r} without new information (last task: "
1037+
f"{action.task[:120]!r})"
1038+
)
1039+
9881040
raise StepBudgetExhausted(f"step_budget {budget} exhausted in checkpoint {checkpoint.get('id')}")
9891041

9901042

@@ -1296,6 +1348,29 @@ def _run_agent(agent_id: str, cid: Optional[str] = None) -> None:
12961348
else:
12971349
log.info("[%s] pause not announced — status superseded externally", agent_id)
12981350
return
1351+
except NoProgressDetected as e:
1352+
# Deterministic loop-breaker fired before the step budget was
1353+
# anywhere near exhausted — the agent is stuck, not still
1354+
# working. Pause early with an honest, specific reason instead
1355+
# of grinding through the rest of the budget for nothing.
1356+
if _atomic_set_status(agent_id, "paused", reason=f"no_progress_detected:{e}"):
1357+
_audit(AGENT_PAUSED,
1358+
message=f"paused: no progress ({e})",
1359+
correlation_id=cid, outcome="warning", level="warning",
1360+
extra={"agent_id": agent_id, "checkpoint_id": cp.id,
1361+
"reason": "no_progress_detected"})
1362+
post_message(agent_id=agent_id, type="agent_blocked",
1363+
title="Paused: not making progress",
1364+
body=f"{e}. Resume to let me try a different approach, "
1365+
f"or abort if this isn't worth pursuing further.",
1366+
actions=[
1367+
{"label": "Resume", "endpoint": f"/api/agents/{agent_id}/resume"},
1368+
{"label": "Abort", "endpoint": f"/api/agents/{agent_id}/abort"},
1369+
],
1370+
correlation_id=cid)
1371+
else:
1372+
log.info("[%s] no-progress pause not announced — status superseded externally", agent_id)
1373+
return
12991374

13001375
# Checkpoint complete: atomic state save (resume guarantee).
13011376
# B-5: load-modify-save (not overwrite) — advance the cursor and CLEAR the

skills/.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
"tts_say.py": "bbc8828d73f0fa4ebb935919ffd0e64f706362bf715fe86851046e4527f62c92",
8787
"volume.py": "cbd20b36ee7d4010a4a4d13829b2d7e96e5123bd112ccd5a17fe18ab85c18429",
8888
"weather.py": "d33a479119a23a26a485df230440a7e4e1beb04bb0bb68bd2a7444e3504a6b2e",
89-
"web_fetch.py": "cd6e2b7565bc3f486faeabdf6d338f5fd052d6cde0e5c8d7625b3b630f987859",
89+
"web_fetch.py": "3a07aed6b21ddfae096ae9e94c4a27048b653167cad3102781ea2cc44954ae38",
9090
"web_search.py": "49023eef091cbedb7e099ad7a545e02eed68676f5c0de0e168625d51914d6462"
9191
}
9292
}

skills/web_fetch.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,39 @@
44
SKILL_TRIGGERS = ["fetch", "get url"]
55
SKILL_MCP_EXPOSE = True
66

7+
import html as _html
78
import re
89
from urllib.parse import urljoin
910

1011
import requests
1112

13+
_MAX_CHARS = 12_000 # keep a fetch from blowing an agent step's context budget
14+
15+
16+
def _html_to_text(raw: str) -> str:
17+
"""Strip an HTML document down to its readable text. Stdlib-only (no bs4
18+
dependency risk) — good enough to make a page's actual content legible to
19+
the LLM instead of raw markup, which is unusable for extracting facts.
20+
21+
2026-07: web_fetch previously returned response.text verbatim for ANY
22+
HTML page — a Project agent researching AI startup launches re-fetched
23+
the same page 15+ times because it kept receiving
24+
'<!DOCTYPE html><html lang="en">...' and could never extract a founder
25+
name from it, burning its entire step budget (80 steps) in the loop
26+
before ever finishing checkpoint 1."""
27+
# Drop non-content blocks entirely — their text isn't page content.
28+
text = re.sub(r"(?is)<(script|style|noscript|svg|head)\b.*?</\1>", " ", raw)
29+
text = re.sub(r"(?is)<!--.*?-->", " ", text)
30+
# Block-level tags become newlines so paragraphs/list items stay separable.
31+
text = re.sub(r"(?i)<(br|/p|/div|/li|/h[1-6]|/tr)\s*/?>", "\n", text)
32+
text = re.sub(r"(?s)<[^>]+>", " ", text) # strip remaining tags
33+
text = _html.unescape(text)
34+
text = re.sub(r"[ \t]+", " ", text)
35+
text = re.sub(r"\n\s*\n+", "\n", text).strip()
36+
if len(text) > _MAX_CHARS:
37+
text = text[:_MAX_CHARS] + f"\n...[truncated, {len(raw)} raw chars]"
38+
return text
39+
1240

1341
def _get_validating_redirects(url: str, max_redirects: int = 5):
1442
"""SSRF-safe GET (Fix #7 H1 + re-audit N3). Validates the URL, then follows
@@ -46,8 +74,11 @@ def run(task: str, context: str = "") -> str:
4674
response.raise_for_status()
4775

4876
content_type = response.headers.get("content-type", "")
77+
if "html" in content_type:
78+
return _html_to_text(response.text)
4979
if "text" in content_type or "json" in content_type:
50-
return response.text
80+
body = response.text
81+
return body[:_MAX_CHARS] + (f"\n...[truncated, {len(body)} raw chars]" if len(body) > _MAX_CHARS else "")
5182
else:
5283
return f"web_fetch failed: unsupported content type {content_type}"
5384

tests/test_agent_runner.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,14 +492,23 @@ def test_execute_checkpoint_destructive_rejection_raises(monkeypatch, temp_codec
492492

493493

494494
def test_execute_checkpoint_step_budget_exhausted(monkeypatch, temp_codec_dir):
495-
"""Step budget cap reached → StepBudgetExhausted."""
495+
"""Step budget cap reached → StepBudgetExhausted.
496+
497+
Each step must return GENUINELY DISTINCT results (a counter, not a fixed
498+
"r") — otherwise this exercises the 2026-07 loop-breaker (NoProgressDetected,
499+
tested separately in test_runner_loop.py) instead of real budget exhaustion,
500+
since identical results repeated 3x now correctly pause early."""
496501
import codec_agent_runner as car
497502

498503
# Always return a skill call (never checkpoint_done)
499504
monkeypatch.setattr(car, "_qwen_next_action", lambda *a, **k:
500505
car.Action(skill="weather", task="loop",
501506
is_destructive=False, network_call=False, touches_path=False))
502-
monkeypatch.setattr(car, "_run_skill", MagicMock(return_value="r"))
507+
_n = {"i": 0}
508+
def _distinct_result(*a, **k):
509+
_n["i"] += 1
510+
return f"result #{_n['i']}"
511+
monkeypatch.setattr(car, "_run_skill", _distinct_result)
503512

504513
grants = {"skills": ["weather"], "read_paths": [], "write_paths": [],
505514
"network_domains": []}
@@ -941,7 +950,10 @@ def test_post_api_agents_404_for_unknown_id(temp_codec_dir):
941950

942951
def test_run_agent_step_budget_exhausted_pauses_not_blocks(monkeypatch, temp_codec_dir):
943952
"""Real budget hit (not destructive_consent_timeout) → status=paused
944-
with reason=step_budget_exhausted (was: blocked_on_permission)."""
953+
with reason=step_budget_exhausted (was: blocked_on_permission).
954+
955+
Distinct per-step results (see test_execute_checkpoint_step_budget_exhausted
956+
above) so this tests real exhaustion, not the loop-breaker."""
945957
import codec_agent_runner as car
946958
import codec_agent_plan as cap
947959
_setup_approved_agent(temp_codec_dir, monkeypatch, num_checkpoints=1)
@@ -950,7 +962,11 @@ def test_run_agent_step_budget_exhausted_pauses_not_blocks(monkeypatch, temp_cod
950962
monkeypatch.setattr(car, "_qwen_next_action", lambda *a, **k:
951963
car.Action(skill="weather", task="loop", kind="skill_call",
952964
is_destructive=False, network_call=False, touches_path=False))
953-
monkeypatch.setattr(car, "_run_skill", MagicMock(return_value="r"))
965+
_n = {"i": 0}
966+
def _distinct_result(*a, **k):
967+
_n["i"] += 1
968+
return f"result #{_n['i']}"
969+
monkeypatch.setattr(car, "_run_skill", _distinct_result)
954970

955971
car._run_agent("test_agent")
956972

tests/test_runner_loop.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,70 @@ def fake_next(plan_dict, checkpoint, history, *a, **k):
109109
f"qwen calls must be capped at ~budget (4)+1, counting corrections; got {calls['n']} (B-14)"
110110

111111

112+
# ── Loop-breaker (2026-07): detect a stuck repeat-loop well before budget ──────
113+
def test_no_progress_detected_on_repeated_identical_results(monkeypatch, temp_codec_dir):
114+
"""Reproduces the live failure: a research checkpoint kept re-fetching the
115+
same URL and re-issuing the same search, getting back identical results,
116+
and burned its ENTIRE 80-step budget without ever finishing. The
117+
loop-breaker must raise NoProgressDetected after _REPEAT_THRESHOLD
118+
identical results — long before a large budget would exhaust — so the
119+
agent pauses early with an honest reason instead of grinding pointlessly."""
120+
grants = {"skills": ["web_fetch"], "read_paths": [], "write_paths": [],
121+
"network_domains": ["*"]}
122+
gg = {"schema": 1, "version": 0, "skills": [], "read_paths": [],
123+
"write_paths": [], "network_domains": []}
124+
# A large budget — if the loop-breaker didn't exist, this would grind for
125+
# 60 steps before StepBudgetExhausted, exactly like the real incident.
126+
cp = {"id": "cp0", "title": "research", "description": "d",
127+
"expected_output": "o", "step_budget": 60}
128+
129+
call_n = {"n": 0}
130+
def fake_next(plan_dict, checkpoint, history, *a, **k):
131+
call_n["n"] += 1
132+
# The model keeps varying its phrasing (as it did live) but keeps
133+
# calling the same skill against the same effective target.
134+
return car.Action(skill="web_fetch", task=f"Fetch attempt #{call_n['n']} of the page",
135+
kind="skill_call")
136+
monkeypatch.setattr(car, "_qwen_next_action", fake_next)
137+
# Every fetch returns the EXACT same raw content — no new information.
138+
monkeypatch.setattr(car, "_run_skill",
139+
MagicMock(return_value="<!DOCTYPE html>...same page every time..."))
140+
141+
with pytest.raises(car.NoProgressDetected):
142+
car._execute_checkpoint(plan_dict={"goals": ["g"]}, checkpoint=cp,
143+
agent_grants=grants, global_grants=gg, agent_id="test_agent")
144+
assert call_n["n"] <= car._REPEAT_THRESHOLD + 1, (
145+
f"loop-breaker must fire within ~{car._REPEAT_THRESHOLD} repeats, not grind "
146+
f"toward the 60-step budget; got {call_n['n']} calls"
147+
)
148+
149+
150+
def test_no_progress_not_triggered_by_genuinely_different_results(monkeypatch, temp_codec_dir):
151+
"""Sanity check: normal progress (each step yields new information) must
152+
NOT trip the loop-breaker — only true repeats should."""
153+
grants = {"skills": ["web_fetch"], "read_paths": [], "write_paths": [],
154+
"network_domains": ["*"]}
155+
gg = {"schema": 1, "version": 0, "skills": [], "read_paths": [],
156+
"write_paths": [], "network_domains": []}
157+
cp = {"id": "cp0", "title": "research", "description": "d",
158+
"expected_output": "o", "step_budget": 5}
159+
160+
step = {"n": 0}
161+
def fake_next(plan_dict, checkpoint, history, *a, **k):
162+
step["n"] += 1
163+
if step["n"] > 4:
164+
return car.Action(skill="", task="", kind="checkpoint_done")
165+
return car.Action(skill="web_fetch", task=f"Fetch source #{step['n']}", kind="skill_call")
166+
monkeypatch.setattr(car, "_qwen_next_action", fake_next)
167+
# Each call returns DIFFERENT content — genuine progress, not a loop.
168+
monkeypatch.setattr(car, "_run_skill",
169+
lambda skill, task, agent_id: f"unique content for step {step['n']}")
170+
171+
history = car._execute_checkpoint(plan_dict={"goals": ["g"]}, checkpoint=cp,
172+
agent_grants=grants, global_grants=gg, agent_id="test_agent")
173+
assert len(history) == 4 # all 4 distinct fetches ran, then checkpoint_done
174+
175+
112176
def test_extend_budget_caps_cumulative(monkeypatch, temp_codec_dir):
113177
"""extend_budget cannot push a checkpoint's override above MAX_CHECKPOINT_STEP_BUDGET."""
114178
import routes.agents as ra

0 commit comments

Comments
 (0)