fix: budget gate and loop-breaker refusals exit without a traceback (Gitea #147)
Files changed: - CHANGES.md - VERSION - tools/chemenu/cli.py - tools/chemenu/commands/run_budget.py - tools/chemenu/tests/test_run_budget.py
This commit is contained in:
1 parent
8f61209b6b
commit
bc314e5c8c
5 files changed
+133
-17
No files matched your search
+20
-8
@@ -266,13 +266,13 @@ _typer_click_core.Command.format_help = _contract_format_help
|
||||
|
||||
|
||||
def main() -> None:
|
||||
# Iteration Budget Gate / Loop-Breaker (see the tooling contract's
|
||||
# "Iteration and Cost Limits"): recorded and enforced here, once per
|
||||
# process, before Typer dispatches to any subcommand - so it covers every
|
||||
# command uniformly and cannot be bypassed by the calling agent skipping a
|
||||
# step. Help output is never counted: discovering a command's options is
|
||||
# not iteration on the wiki, and charging for it would discourage exactly
|
||||
# the behavior the skills ask for.
|
||||
# Iteration Budget Gate / Loop-Breaker (see instructions/gates.md
|
||||
# "Iteration Budget Gate and loop-breaker"): recorded and enforced here,
|
||||
# once per process, before Typer dispatches to any subcommand - so it
|
||||
# covers every command uniformly and cannot be bypassed by the calling
|
||||
# agent skipping a step. Help output is never counted: discovering a
|
||||
# command's options is not iteration on the wiki, and charging for it
|
||||
# would discourage exactly the behavior the skills ask for.
|
||||
#
|
||||
# Tracing sits at the same point for the same reason - one place that no
|
||||
# command can route around. It is not the same set, though: the budget
|
||||
@@ -284,7 +284,19 @@ def main() -> None:
|
||||
override = "--override-budget" in argv
|
||||
filtered = [a for a in argv if a != "--override-budget"]
|
||||
command = filtered[0] if filtered else ""
|
||||
charged = run_budget.record_and_check(command, filtered[1:], override)
|
||||
# A gate refusal leaves `record_and_check` through `_util.fail()`,
|
||||
# which raises `typer.Exit` (Gitea #147). That is fine inside Typer's
|
||||
# own dispatch - Click catches it - but this call runs *before*
|
||||
# `app()` ever starts, so nothing catches it here: left alone, the
|
||||
# process would exit 1 correctly but print a Python traceback right
|
||||
# after the `ERROR` line, which AGENTS.md § Gates asks a session to
|
||||
# show a human and stop on. A traceback reads as a crash, not a gate,
|
||||
# and invites exactly the retry the message forbids (see #52).
|
||||
try:
|
||||
charged = run_budget.record_and_check(command, filtered[1:], override)
|
||||
except typer.Exit as exc:
|
||||
code = exc.exit_code
|
||||
sys.exit(code if isinstance(code, int) else (0 if code is None else 1))
|
||||
sys.argv = [sys.argv[0], *filtered]
|
||||
_run_traced(command, filtered[1:], charged)
|
||||
return
|
||||
|
||||
@@ -33,7 +33,7 @@ from chemenu.session import session_id as _shared_session_id
|
||||
from chemenu.session import session_id_source as _shared_session_id_source
|
||||
from chemenu.telemetry import emit
|
||||
|
||||
app = typer.Typer(help="Session iteration/cost budget gate (see the tooling contract's 'Iteration and Cost Limits').")
|
||||
app = typer.Typer(help="Session iteration/cost budget gate (see instructions/gates.md 'Iteration Budget Gate and loop-breaker').")
|
||||
|
||||
STATE_DIR = config.ROOT / "tools" / ".wikitool_session"
|
||||
STATE_FILE = STATE_DIR / "budget.json"
|
||||
@@ -206,8 +206,8 @@ def loop_breaker_message(call_signature: str, loop_window: int) -> str:
|
||||
return (
|
||||
f"Loop-Breaker: the last {loop_window} wikitool calls in this session were "
|
||||
f"identical ('{call_signature}'). This usually means the agent is stuck retrying "
|
||||
"the same failing operation instead of changing approach - per the tooling contract's "
|
||||
"Tool Error Contracts, that is exactly the case for stopping and escalating rather than "
|
||||
"the same failing operation instead of changing approach - per AGENTS.md's "
|
||||
"Tool error contract, that is exactly the case for stopping and escalating rather than "
|
||||
"retrying again. Stop, explain the situation to the user, and get explicit direction "
|
||||
"before continuing. Only re-run with --override-budget once the user has confirmed "
|
||||
"the repeat is intentional - never add it on the agent's own initiative."
|
||||
@@ -217,7 +217,8 @@ def loop_breaker_message(call_signature: str, loop_window: int) -> str:
|
||||
def call_limit_message(count: int, call_limit: int) -> str:
|
||||
return (
|
||||
f"Iteration Budget Gate: this session has made {count} wikitool calls, exceeding the "
|
||||
f"limit of {call_limit}. Per the tooling contract's 'Iteration and Cost Limits' section, a "
|
||||
f"limit of {call_limit}. Per instructions/gates.md's 'Iteration Budget Gate and "
|
||||
"loop-breaker' section, a "
|
||||
"single task should typically need roughly 5-15 calls (simple) or 20-35 (complex multi-tool "
|
||||
"workflow like wiki-ingest/wiki-lint). This far past that band is a documented sign of poor "
|
||||
"task decomposition or a stuck loop. Stop, summarize progress and the blocker to the "
|
||||
@@ -304,8 +305,8 @@ def refund() -> None:
|
||||
"""Give the current session its last charged slot back.
|
||||
|
||||
Called when the command declined instead of acting: a rejected argument,
|
||||
or a read-only check reporting findings (`_util.fail`, exit 1). The
|
||||
tooling contract answers a rejected argument with "fix it and retry once",
|
||||
or a read-only check reporting findings (`_util.fail`, exit 1). AGENTS.md's
|
||||
Tool error contract answers a rejected argument with "fix it and retry once",
|
||||
so charging for the rejection makes the prescribed response cost two slots
|
||||
for one operation - and the budget exists to bound iteration on the wiki,
|
||||
which a call that changed nothing did not do.
|
||||
|
||||
@@ -47,11 +47,23 @@ def test_call_limit_trips_past_threshold():
|
||||
|
||||
def test_call_limit_message_mentions_contract_and_override():
|
||||
message = run_budget.call_limit_message(61, 60)
|
||||
assert "Iteration and Cost Limits" in message
|
||||
assert "Iteration Budget Gate and loop-breaker" in message
|
||||
assert "--override-budget" in message
|
||||
assert "61" in message and "60" in message
|
||||
|
||||
|
||||
def test_message_sections_exist_in_the_documents_they_cite():
|
||||
"""Both refusal messages point a reader at a section by name (Gitea
|
||||
#147: they used to point at "the tooling contract's 'Iteration and Cost
|
||||
Limits'" and "...Tool Error Contracts", neither of which exists). Pin
|
||||
that the cited headings are real, so the two drift apart again only if
|
||||
this test is touched too."""
|
||||
agents_md = (config._PACKAGE_ROOT / "AGENTS.md").read_text(encoding="utf-8")
|
||||
gates_md = (config._PACKAGE_ROOT / "instructions" / "gates.md").read_text(encoding="utf-8")
|
||||
assert "## Tool error contract" in agents_md
|
||||
assert "## Iteration Budget Gate and loop-breaker" in gates_md
|
||||
|
||||
|
||||
def test_record_and_check_reports_whether_it_charged():
|
||||
assert run_budget.record_and_check("new", ["entity"], override=False) is True
|
||||
assert run_budget.record_and_check("search", ["anything"], override=False) is False
|
||||
@@ -348,3 +360,76 @@ def test_the_loop_breaker_trips_across_separate_shells(tmp_path, monkeypatch):
|
||||
result = _spawn_call(tmp_path, monkeypatch, command="xref", args=args)
|
||||
assert result.returncode != 0
|
||||
assert "Loop-Breaker" in result.stdout
|
||||
|
||||
|
||||
# --- a refusal exits without a traceback (Gitea #147) ---
|
||||
#
|
||||
# The two tests above call `run_budget.record_and_check` directly, so they
|
||||
# never exercise `cli.main()` - the place `_util.fail()`'s `typer.Exit` used
|
||||
# to go uncaught, since it is raised *before* `app()` starts and nothing
|
||||
# outside `app()` catches it. These drive the real entry point
|
||||
# (`python -m chemenu.cli`, not a `-c` snippet) so the whole `main()` codepath
|
||||
# runs, and check the property the fix promises: exit 1, no traceback on
|
||||
# either stream, `gate.refused` recorded, no `wikitool.call` for the refused
|
||||
# invocation, and the budget counter left exactly where it was.
|
||||
|
||||
def _spawn_cli_call(tmp_path, trace_dir, monkeypatch, *, session_id, args):
|
||||
monkeypatch.setenv("CHEMENU_ROOT", str(tmp_path))
|
||||
monkeypatch.setenv("WIKI_TRACE_DIR", str(trace_dir))
|
||||
monkeypatch.setenv("WIKITOOL_SESSION_ID", session_id)
|
||||
monkeypatch.delenv("CLAUDE_CODE_SESSION_ID", raising=False)
|
||||
return subprocess.run(
|
||||
[sys.executable, "-m", "chemenu.cli", *args],
|
||||
cwd=config._PACKAGE_ROOT / "tools",
|
||||
capture_output=True, text=True,
|
||||
)
|
||||
|
||||
|
||||
def _trace_events(trace_dir, session_id):
|
||||
path = trace_dir / session_id / "trace.jsonl"
|
||||
return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line]
|
||||
|
||||
|
||||
def test_loop_breaker_refusal_has_no_traceback(tmp_path, monkeypatch):
|
||||
session_id = "test-147-loop"
|
||||
trace_dir = tmp_path / "trace"
|
||||
args = ["xref", "add", "--a", "X", "--b", "Y"]
|
||||
for _ in range(run_budget.DEFAULT_LOOP_WINDOW):
|
||||
_spawn_cli_call(tmp_path, trace_dir, monkeypatch, session_id=session_id, args=args)
|
||||
|
||||
result = _spawn_cli_call(tmp_path, trace_dir, monkeypatch, session_id=session_id, args=args)
|
||||
assert result.returncode == 1
|
||||
assert result.stdout.startswith("ERROR Loop-Breaker")
|
||||
assert "Traceback" not in result.stdout
|
||||
assert "Traceback" not in result.stderr
|
||||
|
||||
events = _trace_events(trace_dir, session_id)
|
||||
assert [e["event"] for e in events[-1:]] == ["gate.refused"]
|
||||
assert events[-1]["attrs"]["gate"] == "loop-breaker"
|
||||
assert sum(e["event"] == "wikitool.call" for e in events) == run_budget.DEFAULT_LOOP_WINDOW
|
||||
|
||||
state = json.loads((tmp_path / "tools" / ".wikitool_session" / "budget.json").read_text())
|
||||
assert state[session_id]["count"] == run_budget.DEFAULT_LOOP_WINDOW
|
||||
|
||||
|
||||
def test_iteration_budget_refusal_has_no_traceback(tmp_path, monkeypatch):
|
||||
session_id = "test-147-limit"
|
||||
trace_dir = tmp_path / "trace"
|
||||
for i in range(run_budget.DEFAULT_CALL_LIMIT):
|
||||
_spawn_cli_call(tmp_path, trace_dir, monkeypatch, session_id=session_id, args=["lint", f"--pass-{i}"])
|
||||
|
||||
result = _spawn_cli_call(
|
||||
tmp_path, trace_dir, monkeypatch, session_id=session_id, args=["lint", "--one-too-many"]
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert result.stdout.startswith("ERROR Iteration Budget Gate")
|
||||
assert "Traceback" not in result.stdout
|
||||
assert "Traceback" not in result.stderr
|
||||
|
||||
events = _trace_events(trace_dir, session_id)
|
||||
assert [e["event"] for e in events[-1:]] == ["gate.refused"]
|
||||
assert events[-1]["attrs"]["gate"] == "iteration-budget"
|
||||
assert sum(e["event"] == "wikitool.call" for e in events) == run_budget.DEFAULT_CALL_LIMIT
|
||||
|
||||
state = json.loads((tmp_path / "tools" / ".wikitool_session" / "budget.json").read_text())
|
||||
assert state[session_id]["count"] == run_budget.DEFAULT_CALL_LIMIT
|
||||
Reference in new issue
Block a user