diff --git a/docs/reference/agents.md b/docs/reference/agents.md index 904d826..ddf65fc 100644 --- a/docs/reference/agents.md +++ b/docs/reference/agents.md @@ -378,6 +378,15 @@ rather than a turn to retry, and not an `Unrecoverable`. That last one for the r not caught: a `while True` that swallowed a failure no other try could come out differently on would go round on the same failure until somebody stopped it. +What it covers is the **raising** rather than the failing. The turn still closes on a +[`failed`](#watching-a-turn-as-it-happens) carrying what went wrong, and a run nothing is +watching is told the same sentence on stderr — so a loop that went round on a turn it was +handed nothing for is still a loop somebody can see went round. That matters most for the agent +that never once worked: a turn that failed opens no session, so the +[epic](/user/tracing#what-a-run-writes-down) does not name it either, and a reviewer whose CLI +was never signed in would otherwise read afterwards exactly like a reviewer that had nothing to +say. + ## Sessions ```python diff --git a/docs/user/stopping.md b/docs/user/stopping.md index a4ce014..c17ada1 100644 --- a/docs/user/stopping.md +++ b/docs/user/stopping.md @@ -64,6 +64,10 @@ wrote](/weaver/writing-a-flow#make-the-loop-survive-a-bad-turn), which has to le agent(task, suppress=True) # a turn that failed answers ""; the loop goes round again ``` +The turn still says it failed — a [`failed`](/reference/agents#watching-a-turn-as-it-happens) +event, and the same sentence on stderr where nothing is watching the run. Suppressing a turn +keeps the loop going; it does not make the failure quiet. + It deliberately does not catch `Stopped`. A loop that carried on past a stop would never end: ```python diff --git a/docs/user/troubleshooting.md b/docs/user/troubleshooting.md index 482c682..eef9b2a 100644 --- a/docs/user/troubleshooting.md +++ b/docs/user/troubleshooting.md @@ -262,6 +262,20 @@ Nothing matched. In order of likelihood: `hmz trace collect --all` or `--session `. 5. **The time window excludes it.** Drop `--start`/`--end`. +### One of my agents is not in the trace at all + +A trace is gathered by session id, and a session is opened by a turn that **landed**. An agent +whose every turn failed — a CLI that was never signed in, an account whose quota went overnight +— opened nothing, so the run's record names no session for it and a trace of that run has +nothing of it to collect. Two agents declared and one of them in the trace is that, rather than +a trace that lost one. + +Where it is said is the run as it happened: a failed turn closes on a +[`failed`](/reference/agents#watching-a-turn-as-it-happens) carrying what the CLI said about it, +shown in the interface and put on stderr for a run nothing is watching. The flow cannot tell you +— a turn taken under `suppress=True`, which is every loop, is handed the same nothing whether it +failed or answered with nothing — so that event is the place to read it. + ### Two agents show up as one They ran at the same configuration, and nothing said they were two. `hmz trace collect` reads diff --git a/src/hmz/agents/base.py b/src/hmz/agents/base.py index 7a33417..6a1b8db 100644 --- a/src/hmz/agents/base.py +++ b/src/hmz/agents/base.py @@ -673,7 +673,10 @@ def __call__[T: BaseModel]( prompt: The input prompt for this turn. suppress: Whether a turn that fails answers with nothing instead of raising. A flow is a loop, and a loop that catches its own turns is `try` around every line of - it; this is the `|| true` that flowbench writes beside each one. + it; this is the `|| true` that flowbench writes beside each one. What it covers + is the raising rather than the failing: the turn still closes on a `failed`, so + a loop that carried on is a loop whoever is watching it can see carrying on -- + an agent that failed every round of a week is not one that read quietly. schema: The shape to answer in, as the pydantic model a flow reads the answer as, or None to take what the agent says as it says it. A turn asked for one answers with that model rather than with text, so a flow that needs a decision reads a field @@ -840,6 +843,12 @@ def _turning( Yields: What the agent said, in the order it said it. + + Raises: + subprocess.CalledProcessError: If the turn failed, which is said as a `failed` + before it is raised: a turn closes on one or the other whichever way it went, + so that a turn that failed is one thing a watcher can see rather than the + absence of a thing. """ if self._agent._stopped: raise Stopped(f"{self._agent.id} was stopped") @@ -862,6 +871,12 @@ def _turning( if submitted.adds: prompt = f"{prompt}\n\n{submitted.adds}" self._heard(Event(kind="begins", text=prompt)) + # Whether the turn has already closed on a `failed` of the backend's own. One reading + # a protocol is told which request failed and in what words, and says so as the event; + # one run as a command line has its exit status and whatever it wrote on the way out, + # which arrives here as the exception instead. The turn closes the same way on either, + # and not twice on the one that came both ways. + closed = False try: if submitted.refused: # The turn does not run, and what the hook said instead is what it answers @@ -884,6 +899,7 @@ def _turning( # sent on has not answered. answered = event continue + closed = closed or event.kind == "failed" self._heard(event) if event.kind == "tool": named, _, about = event.text.partition(" ") @@ -912,6 +928,22 @@ def _turning( yield answered return prompt, again = stopping.because, again + 1 + except subprocess.CalledProcessError as failed: + # A turn that failed closes on `failed`, the way one that landed closes on + # `result`. Between `begins` and `ends` there was otherwise nothing at all, which + # is what a turn that answered with nothing looks like too -- so whatever is + # watching the agent was shown a round that failed and a round with nothing to + # say as the same thing, and `suppress` hands the flow the same nothing for both. + # Which is how a whole agent goes unnoticed: a turn that failed is never asked + # for a session id, so the run's own record names no session for it either, and a + # reviewer whose CLI was never signed in reads afterwards exactly like a reviewer + # that agreed. A run nothing is watching has the reason on stderr already; this + # is the same reason for the interface, the status column and anything else hung + # on the agent. Said once: a backend that closed the turn on a `failed` of its + # own is not made to say it twice. + if not closed: + self._heard(Event(kind="failed", text=str(failed))) + raise finally: self._heard(Event(kind="ends", text="")) diff --git a/tests/agents/test_agents.py b/tests/agents/test_agents.py index addbe57..a4893fa 100644 --- a/tests/agents/test_agents.py +++ b/tests/agents/test_agents.py @@ -27,15 +27,20 @@ ClaudeCodeAgent, ClaudeCodeAgentConfig, CommandSessionBase, + Event, + Failed, Question, Stopped, ) from hmz.machines import AnchoredConfig -from tests.stubs import HereAnchor, ShellAgent +from tests.stubs import HereAnchor, ShellAgent, ShellSession if TYPE_CHECKING: + from collections.abc import Iterator from pathlib import Path + from pydantic import BaseModel + CODEX_ID = "019fa62b-d9e1-7b73-be84-bd70260e1cf6" CONFIG = AgentConfig(model="m", effort="high") @@ -751,6 +756,80 @@ def test_a_suppressed_failure_is_quiet_on_the_answer_but_not_on_the_reason( assert "returned non-zero exit status" in said.err +def test_a_turn_that_failed_closes_on_a_failed_rather_than_on_nothing() -> None: + """A turn ends on one event whichever way it went, so that a watcher can see it end. + + `begins` and then `ends` with nothing between them was a turn that failed and a turn that + answered with nothing alike, and `suppress` -- which every loop is written with -- hands a + flow the same nothing for both. So an agent that never once worked read afterwards as an + agent that had nothing to say, and nothing anywhere said otherwise. + """ + agent = ShellAgent(CONFIG) + heard: list[tuple[str, str]] = [] + agent.watch(lambda _agent, _session, event: heard.append((event.kind, event.text))) + + with pytest.raises(subprocess.CalledProcessError): + agent.new()("echo boom >&2; exit 3") + + # In place of the `result` a turn that landed ends on, and inside the same bracket. + kinds = [kind for kind, _text in heard] + assert kinds[0] == "begins" + assert kinds[-2:] == ["failed", "ends"] + assert "result" not in kinds + failed = [text for kind, text in heard if kind == "failed"] + assert ( + "boom" in failed[0] + ) # carrying what the CLI said about it, not only that it did + + +def test_a_suppressed_failure_is_still_said_to_whoever_is_watching() -> None: + """`suppress` covers the raising rather than the failing. + + The reason reaches stderr on a run nothing is watching, which is the other test above. + This is the same reason for a run something *is* watching: the interface, the status + column, and anything else hung on the agent read the turns rather than stderr, and a + round that failed has to be one of the things they are shown. + """ + agent = ShellAgent(CONFIG, name="reviewer") + heard: list[tuple[str, str]] = [] + agent.watch(lambda _agent, _session, event: heard.append((event.kind, event.text))) + + assert agent.new()("echo boom >&2; exit 3", suppress=True) == "" + + assert [kind for kind, _text in heard].count("failed") == 1 + + +def test_a_turn_that_said_it_failed_is_not_made_to_say_it_twice() -> None: + """A backend reading a protocol is told which request failed, and says so itself. + + It raises after saying it, the way every other failed turn does -- so the two would be one + failure written down as two, which is a run that reads as twice as broken as it was. + """ + + class _SaysItFailed(ShellSession): + def _stream( + self, prompt: str, *, schema: type[BaseModel] | None = None + ) -> Iterator[Event]: + yield Event(kind="failed", text="the account is not signed in") + raise Failed(1, ["stand-in"], "", "the account is not signed in") + + class _SaysItFailedAgent(ShellAgent): + def new(self, cwd: str | os.PathLike[str] | None = None) -> _SaysItFailed: + return _SaysItFailed(self, cwd) + + agent = _SaysItFailedAgent(CONFIG) + heard: list[tuple[str, str]] = [] + agent.watch(lambda _agent, _session, event: heard.append((event.kind, event.text))) + + assert agent.new()("whatever", suppress=True) == "" + + assert [kind for kind, _text in heard].count("failed") == 1 + assert heard[1] == ( + "failed", + "the account is not signed in", + ) # the backend's own words + + def test_calling_the_agent_is_a_session_it_keeps_nothing_of(clis: _FakeCLIs) -> None: """Which is the shape a ralph loop is made of, said without reaching through a session.""" agent = ClaudeCodeAgent(CONFIG) diff --git a/tests/agents/test_shapes.py b/tests/agents/test_shapes.py index f2a6173..48872d3 100644 --- a/tests/agents/test_shapes.py +++ b/tests/agents/test_shapes.py @@ -112,6 +112,23 @@ def test_a_turn_that_failed_answers_with_nothing_rather_than_an_empty_model() -> assert agent.new()("how did it go?", schema=Verdict, suppress=True) is None +def test_a_reviewer_that_never_ran_is_not_a_reviewer_that_agreed() -> None: + """The rlar shape, where None is the answer to two questions at once. + + A flow reads `done` off a shape and goes round again on anything else, so a reviewer whose + CLI is not signed in and a reviewer that had nothing to add are the same None -- and the + loop that is held to the reviewer's judgement rather than to a budget runs on. Which of + the two it was is the turn's to say, and it says it here. + """ + agent = _SaysAgent(None) + heard: list[tuple[str, str]] = [] + agent.watch(lambda _agent, _session, event: heard.append((event.kind, event.text))) + + assert agent.new()("how did it go?", schema=Verdict, suppress=True) is None + + assert [kind for kind, _text in heard].count("failed") == 1 + + def test_a_turn_asked_for_nothing_in_particular_is_asked_for_nothing() -> None: agent = _SaysAgent("looks fine") assert agent.new()("how did it go?") == "looks fine"