An escalation lands in blocked, and does not read as a failed run

events.jsonl

@@ -0,0 +1,5 @@
+{"ts": 1785899243.5450313, "kind": "work", "worker": "mydevenv2-dev:3107904", "item_id": "R7", "issue": null, "outcome": "started", "error_class": null, "detail": null, "session_id": null, "session_url": null, "project_id": "rdpapp"}
+{"ts": 1785899243.6182284, "kind": "work", "worker": "mydevenv2-dev:3107904", "item_id": "R7", "issue": null, "outcome": "agent_started", "error_class": null, "detail": "affa8f68-0507-446c-9281-34280bd77af3", "session_id": "affa8f68-0507-446c-9281-34280bd77af3", "session_url": "http://localhost:8910/t/affa8f68-0507-446c-9281-34280bd77af3", "project_id": "rdpapp"}
+{"ts": 1785899353.6382027, "kind": "work", "worker": "mydevenv2-dev:3107904", "item_id": "R7", "issue": null, "outcome": "agent_finished", "error_class": null, "detail": null, "session_id": "affa8f68-0507-446c-9281-34280bd77af3", "session_url": null, "project_id": "rdpapp"}
+{"ts": 1785899353.6437557, "kind": "work", "worker": "mydevenv2-dev:3107904", "item_id": "R7", "issue": null, "outcome": "refused_as_impossible", "error_class": null, "detail": "# Refusal: the support-bundle tests cannot build a bundle without a live Postgres\n\nThe item states its own stopping condition:\n\n> If the existing tests cannot build a bundle without a live Postgres, say so\n> and stop rather than introducing a mock \u2014 that is a correct outcome and a\n> different item.\n\nThat condition holds. Nothing else in the item is ambiguous; this one fact\nrules out delivering a test that \"passes now\" under the stated check.\n\n## What I found\n\n**The only support-bundle assertions live inside an ignored Postgres test.**\nEvery existing reference to the bundle in `crates/rdpapp-gateway/src/main.rs`\nis inside `postgres_auth_csrf_roles_and_connection_persistence`\n(`crates/rdpapp-gateway/src/main.rs:15195`), which is declared:\n\n```rust\n#[tokio::test]\n#[ignore = \"requires RDPAPP_TEST_DATABASE_URL\"]\nasync fn postgres_auth_csrf_roles_and_connection_persistence() {\n    let database_url = env::var(\"RDPAPP_TEST_DATABASE_URL\")\n        .expect(\"set RDPAPP_TEST_DATABASE_URL to a disposable Postgres database\");\n```\n\nIts own doc comment says the ignore is deliberate: \"Keeping this ignored in the\nordinary unit suite makes `cargo test` independent of a developer's local\nDocker setup.\" The bundle assertions run at\n`crates/rdpapp-gateway/src/main.rs:15841`-`15876`, including the existing\n`assert!(!support_bundle_text.contains(\"rotated-secret\"))` \u2014 the closest\nneighbour to what this item asks for.\n\n**The fixture is a real database, not an in-memory one.** That test builds\n`Repository::new(pool, test_vault())` from a `PgPoolOptions::connect(...)`,\nmigrates, `TRUNCATE`s, and calls `app_with_repository(...)`\n(`crates/rdpapp-gateway/src/main.rs:11996`\u2026 declared at\n`crates/rdpapp-gateway/src/main.rs:10996`). `just test-integration` (justfile\nline 25) exists precisely to bring up `docker compose ... postgres` for it.\n\n**There is no non-Postgres route to a serialised bundle.**\n`download_support_bundle` (`crates/rdpapp-gateway/src/main.rs:6915`) obtains its\ndata through `repository(&state)?`\n(`crates/rdpapp-gateway/src/main.rs:6252`), which returns the concrete\n`Repository` struct (`crates/rdpapp-gateway/src/main.rs:95`) or a\n`503 repository_unavailable` when `state.repository` is `None`. `Repository` is\na struct over a `PgPool`, not a trait, so it cannot be substituted. The snippet\nrows the bundle maps from come from\n`Repository::snippets` (`crates/rdpapp-gateway/src/main.rs:2807`), a direct\n`sqlx::query(\"SELECT id,name,text,revision FROM snippets ...\")` against\n`self.pool`. Creating a workspace snippet with a sentinel `text` and then\nserialising a bundle for that workspace \u2014 exactly what the item asks \u2014 therefore\nrequires a live Postgres.\n\n## Why this collides with the item as written\n\n- Scope point 2 requires the test to **pass now**. The judging check is\n  `cargo test -p rdpapp-models -p rdpapp-sessions -p rdpapp-gateway`, which does\n  not run `#[ignore]`d tests and does not set `RDPAPP_TEST_DATABASE_URL`. A test\n  written against the neighbouring fixture would be skipped, not passed.\n- Scope point 4 requires reusing the neighbours' fixture rather than\n  introducing a new one, and point 3 forbids any production change. So the two\n  routes that would make the test run under the stated command are both closed:\n  a fake/in-memory repository is the mock the item rules out, and making\n  `Repository` substitutable is a production change.\n\n## What the different item would be\n\nAdding an `#[ignore = \"requires RDPAPP_TEST_DATABASE_URL\"]` sentinel test beside\n`postgres_auth_csrf_roles_and_connection_persistence`, guarding the bundle under\n`just test-integration` rather than under `cargo test`. That is a real guard and\nit is not a mock, but it does not satisfy \"passes now\" against the check this\nitem is judged by, so I have left it undone rather than quietly redefining the\nscope. Deciding whether the integration suite is the right home for this guard\nis the call the follow-up item should make.\n\nNo other change is left in the working tree.", "session_id": "affa8f68-0507-446c-9281-34280bd77af3", "session_url": null, "project_id": "rdpapp"}
+{"ts": 1785899353.654138, "kind": "work", "worker": "mydevenv2-dev:3107904", "item_id": "R7", "issue": null, "outcome": "worktree_removed", "error_class": null, "detail": "reclaimed 13094912 bytes", "session_id": null, "session_url": null, "project_id": "rdpapp", "worktree_bytes": 13094912}

src/agent_harness/__main__.py

@@ -25,6 +25,7 @@
 import shutil
 import sys
 import time
+from collections.abc import Sequence
 from pathlib import Path
 from typing import Any
 
@@ -777,13 +778,50 @@ def live_routes() -> dict[str, Chain]:
     if not outcomes:
         print("nothing to do")
         return 0
+    for line in run_summary(outcomes):
+        print(line)
+    return 1 if any(_is_failure(o) for o in outcomes) else 0
+
+
+def _is_failure(outcome: Any) -> bool:
+    """Did this go wrong, as opposed to needing a person?
+
+    An escalation is not a failure. Nothing malfunctioned, no attempt was
+    wasted, and the item is waiting on a decision only a human can make — so
+    it must not colour the exit status, or a queue full of well-formed
+    questions reads to CI as a broken run.
+    """
+    from .outcomes import NEEDS_A_PERSON
+
+    if outcome.stop is not None and outcome.stop.disposition in NEEDS_A_PERSON:
+        return False
+    return not outcome.ok
+
+
+def run_summary(outcomes: Sequence[Any]) -> list[str]:
+    """The lines a finished run prints, as data so they can be tested.
+
+    `FAIL` and `ok` were the whole vocabulary, so an item that escalated —
+    nothing went wrong, a person is needed — was announced as a failure.
+    Measured on rdpapp R7: an agent correctly refused an impossible item, with
+    citations and no attempt spent, and the run's last word on it was `FAIL R7`.
+    """
+    from .outcomes import NEEDS_A_PERSON
+
+    lines = []
+    waiting = [o for o in outcomes if o.stop and o.stop.disposition in NEEDS_A_PERSON]
     for outcome in outcomes:
-        mark = "ok " if outcome.ok else "FAIL"
+        mark = "YOU" if outcome in waiting else ("ok " if outcome.ok else "FAIL")
         detail = outcome.pr_url or outcome.reason[:100]
-        print(f"  {mark} {outcome.item_id}: {' -> '.join(outcome.stages)}  {detail}")
-    failed = [o for o in outcomes if not o.ok]
-    print(f"{len(outcomes) - len(failed)}/{len(outcomes)} items completed")
-    return 1 if failed else 0
+        lines.append(f"  {mark} {outcome.item_id}: {' -> '.join(outcome.stages)}  {detail}")
+    done = [o for o in outcomes if o.ok]
+    lines.append(f"{len(done)}/{len(outcomes)} items completed")
+    if waiting:
+        lines.append(
+            f"{len(waiting)} waiting on you, not on a retry: "
+            + ", ".join(o.item_id for o in waiting)
+        )
+    return lines
 
 
 def _assessor(args: argparse.Namespace) -> Any:

src/agent_harness/session_executor.py

@@ -373,6 +373,16 @@ def run_once(self) -> Outcome | None:
                 partial.stop = stop
                 return partial
             return Outcome(record.item_id, FAILED, reason=str(exc), stop=stop)
+        # A `Stop` that names a state means it, and it is the last word: the
+        # checks path used to copy `stop.state` across by hand and every other
+        # path forgot to, so an item that escalated with `state=BLOCKED` was
+        # still released as `failed`. Measured on rdpapp R7 — correctly refused,
+        # correctly `escalated / item_impossible` with no attempt spent, and
+        # sitting in `failed` where nothing looking for work that needs a
+        # person would ever find it. An empty `state` still means the caller
+        # already chose, which is every path older than this taxonomy.
+        if outcome.stop is not None and outcome.stop.state:
+            outcome.state = outcome.stop.state
         self.queue.release(
             record.item_id,
             outcome.state,

tests/test_cli_roles.py

@@ -317,3 +317,49 @@ def test_the_cli_agent_default_is_the_executor_s() -> None:
         __import__("agent_harness.session_executor", fromlist=["x"])
     ), "and that command must still grant edit permission"
     assert "--permission-mode" in " ".join(DEFAULT_AGENT_COMMAND)
+
+
+# ------------------------------------- an escalation is not a failed run
+
+
+def _outcome(item_id: str, *, ok: bool, disposition: str = "", reason: str = "") -> Any:
+    from agent_harness.executor import Outcome
+    from agent_harness.outcomes import Stop
+
+    out = Outcome(item_id, "done" if ok else "blocked", reason=reason)
+    if disposition:
+        out.stop = Stop(disposition, detail=reason)
+    return out
+
+
+def test_an_escalated_item_is_marked_for_a_person_not_as_a_failure() -> None:
+    """Measured on rdpapp R7: a correct refusal was announced as `FAIL R7`."""
+    from agent_harness.__main__ import run_summary
+    from agent_harness.outcomes import ESCALATED
+
+    lines = run_summary(
+        [_outcome("R7", ok=False, disposition=ESCALATED, reason="needs a live Postgres")]
+    )
+
+    assert lines[0].startswith("  YOU R7:")
+    assert "FAIL" not in "\n".join(lines)
+    assert "waiting on you, not on a retry: R7" in lines[-1]
+
+
+def test_a_real_failure_still_reads_as_one() -> None:
+    from agent_harness.__main__ import run_summary
+    from agent_harness.outcomes import REFUSED
+
+    lines = run_summary([_outcome("R2", ok=False, disposition=REFUSED, reason="review rejected")])
+
+    assert lines[0].startswith("  FAIL R2:")
+    assert "waiting on you" not in "\n".join(lines)
+
+
+def test_an_escalation_does_not_colour_the_exit_status() -> None:
+    """A queue of well-formed questions is not a broken run."""
+    from agent_harness.__main__ import _is_failure
+    from agent_harness.outcomes import ESCALATED, REFUSED
+
+    assert _is_failure(_outcome("R7", ok=False, disposition=ESCALATED)) is False
+    assert _is_failure(_outcome("R2", ok=False, disposition=REFUSED)) is True

tests/test_session_executor.py

@@ -330,13 +330,18 @@ def test_a_nonzero_exit_fails_the_item_without_reviewing(repo: Path, tmp_path: P
 
 def test_an_agent_that_changed_nothing_is_reported_as_such(repo: Path, tmp_path: Path) -> None:
     """A CLI agent that decided the task was impossible leaves a clean tree.
-    That is a real answer, not a failure to paper over."""
+    That is a real answer, not a failure to paper over.
+
+    This test used to assert `FAILED`, which is what the docstring above spent
+    two lines saying it was not. It is `BLOCKED` now: a real answer nobody can
+    act on without a person is what `blocked` is for.
+    """
     devenv = FakeDevEnv(agent=None)
     executor, queue = build(repo, tmp_path, devenv)
     add_item(queue)
     outcome = executor.run_once()
     assert outcome is not None
-    assert outcome.state == FAILED
+    assert outcome.state == BLOCKED
     assert "no changes" in outcome.reason
 
 
@@ -844,3 +849,23 @@ def read_prompt(tree: Path) -> None:
     executor.run_once()
 
     assert REFUSAL_FILE in seen[0]
+
+
+def test_a_refusal_lands_in_blocked_not_failed(repo: Path, tmp_path: Path) -> None:
+    """Measured on rdpapp R7, and the reason the Stop carries a state at all.
+
+    Disposition, reason kind and attempt count were all correct and the item
+    still sat in `failed`, because only the checks path copied `stop.state`
+    across. An escalation filed under `failed` is invisible to exactly the
+    person it was raised for.
+    """
+    executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse("needs a live database")))
+    add_item(queue)
+
+    outcome = executor.run_once()
+
+    assert outcome is not None
+    assert outcome.state == BLOCKED
+    item = queue.get("W1")
+    assert item is not None
+    assert item.state == BLOCKED