diff --git a/docs/USAGE.md b/docs/USAGE.md index df7ba24..c4dc7eb 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1289,7 +1289,37 @@ can branch on it: `checks_failed`, `check_escalated`, `check_transient`, `review_rejected`, `patch_rejected`, `no_target`, `worker_error`, `provider_exhausted`, `budget_exhausted`, `dependency_invalidated`, `agent_timeout`, `claim_lost`, `item_wall_clock`, `item_spend`, -`hold_expired`, `context_unavailable`. +`hold_expired`, `context_unavailable`, `item_impossible`. + +### 6a.0 When the agent says the item cannot be done + +An agent in session mode is told, in its prompt, that if the item is ambiguous, +contradicts the code, or depends on something absent, it should write its +reasoning to `.harness-refusal.md` and change nothing else. + +That is a correct outcome, and the harness records it as one: + +```json +{"state": "blocked", "disposition": "escalated", + "reason_kind": "item_impossible", "attempts": 0, + "last_error": "The item asks for a snippet timestamp, but every route to one + is forbidden by its own criteria: …"} +``` + +**It costs no attempt.** What is wrong is the brief, and no number of retries +rewrites a brief — so the item waits for you rather than spending its budget +proving the same point three more times. It appears wherever your deployment +surfaces `escalated`, which is the set of dispositions meaning *a person, not a +timer, is what this is waiting on*. + +The note itself never reaches a commit, a diff or a reviewer: it is read and +deleted before the worktree is inspected. An agent that leaves the note *and* +makes real changes has not refused, and is judged on the changes as usual. + +A session that ends with a clean tree and **no** note escalates too, with +`reason_kind: no_target` and the session id in `last_error` — an agent that +did nothing and an agent that could not say why both want a human, and the +session is where the explanation is. ### 6a.1 When the target does not fit in the prompt diff --git a/src/agent_harness/outcomes.py b/src/agent_harness/outcomes.py index 98e2a8f..3ebd70a 100644 --- a/src/agent_harness/outcomes.py +++ b/src/agent_harness/outcomes.py @@ -206,6 +206,12 @@ def as_dict(self) -> dict[str, Any]: #: nothing — the file is the size it is — so it needs a person to raise the #: budget or split the file, which is why it escalates. CONTEXT_UNAVAILABLE = "context_unavailable" +#: The agent read the repository, concluded the item cannot be done as written, +#: and said why. Kept apart from `NO_TARGET`, which is the agent finding nothing +#: to change: here it found the target and the *brief* is what does not work. +#: Retrying is pointless — the brief is the brief — so this needs a person to +#: rewrite or withdraw the item, which is why it escalates rather than failing. +ITEM_IMPOSSIBLE = "item_impossible" REASON_KINDS = ( CHECKS_FAILED, @@ -224,6 +230,7 @@ def as_dict(self) -> dict[str, Any]: ITEM_SPEND, HOLD_EXPIRED, CONTEXT_UNAVAILABLE, + ITEM_IMPOSSIBLE, ) diff --git a/src/agent_harness/session_executor.py b/src/agent_harness/session_executor.py index 745007b..b9aa090 100644 --- a/src/agent_harness/session_executor.py +++ b/src/agent_harness/session_executor.py @@ -49,11 +49,14 @@ from .model_client import CapExhausted, ModelClient, RequestRefused, RetryExhausted from .outcomes import ( AGENT_TIMEOUT, + BLOCKED, BUDGET_EXHAUSTED, CLAIM_LOST, COMPLETED, CRASHED, DEPENDENCY_INVALIDATED, + ESCALATED, + ITEM_IMPOSSIBLE, NO_TARGET, PROVIDER_EXHAUSTED, REFUSED, @@ -126,11 +129,54 @@ - Do not commit; the harness commits what you leave in the working tree. - Do not push, and do not open a pull request. - If the item cannot be done as written — it is ambiguous, contradicts the - code, or depends on something absent — stop and say so plainly. Saying - "this cannot be done as specified" is a correct outcome; inventing a way - around it is not. + code, or depends on something absent — **write your reasoning to + `{refusal_file}` in this directory and make no other change.** Say which + part of the item cannot be met and what you found that rules it out, citing + the files you read. Saying "this cannot be done as specified" is a correct + outcome; inventing a way around it is not. + + Write that file only when you are refusing the whole item. If you can do + what the item asks, do it and leave the file absent. """ +#: Where a refusing agent leaves its reasoning. +#: +#: The rule above used to end at "stop and say so plainly", which told the +#: agent to explain itself **to a terminal nobody reads**. The explanation went +#: into the session scrollback, the harness saw only a clean worktree, and the +#: item was recorded `refused / no_target — "the agent made no changes"`: an +#: ordinary failure, costing an attempt, invisible to anyone looking for work +#: that needs them (#174). +#: +#: A file, inside the tree, because that is the one channel the agent certainly +#: has — it is there to edit files. Read and deleted before the tree is +#: inspected, so it cannot itself be mistaken for a change or reach a commit. +REFUSAL_FILE = ".harness-refusal.md" + +#: How much of a refusal reaches `last_error`. Generous next to the other +#: limits here, because this is the one text a person is being asked to act on +#: and there is no second copy of it once the worktree is gone. +REFUSAL_LIMIT = 4000 + + +def _take_refusal(tree: Path) -> str: + """The agent's refusal, removed from the tree as it is read. + + Removed rather than ignored so that the file cannot be committed, cannot + show up in a diff a reviewer reads, and cannot make a genuinely empty + attempt look like a change. Returns an empty string if no note was left, + which is the ordinary case. + """ + note = tree / REFUSAL_FILE + try: + text = note.read_text(errors="replace").strip() + except OSError: + return "" + finally: + note.unlink(missing_ok=True) + return text[:REFUSAL_LIMIT] if text else "" + + #: The review rubric lives with the headless executor and is imported, not #: copied. It used to be copied, and every correction made from measurement #: landed in one of the two: the session reviewer was never told that a diff is @@ -499,6 +545,7 @@ def _execute(self, record: WorkRecord) -> Outcome: brief=record.brief, checks_description=self._describe_checks(), prior=self._prior_failure(record), + refusal_file=REFUSAL_FILE, ) ) @@ -561,11 +608,33 @@ def _execute(self, record: WorkRecord) -> Outcome: # was impossible leaves a clean tree, and that is a real answer, # not a failure to paper over. prompt_file.unlink(missing_ok=True) + # Read and remove before the tree is inspected: a refusal note is + # the agent's answer *about* the item, never a change to it, and + # leaving it would make a refusing agent look like a working one. + refusal = _take_refusal(tree) diff = run_git(tree, "diff", "HEAD") if not diff.strip() and not run_git(tree, "status", "--porcelain").strip(): - outcome.reason = "the agent made no changes" - self._emit(record, "no_changes", session_id=session.id) - outcome.stop = Stop(REFUSED, NO_TARGET, detail=outcome.reason) + # Nobody has judged this item and nobody can: the agent is + # telling us the brief is wrong, or it left without saying + # anything at all. Both need a person to read them, and + # neither is worth spending an attempt to discover twice. + outcome.reason = refusal or ( + "the agent made no changes and left no reason; " + f"read session {session.id} before retrying" + ) + self._emit( + record, + "refused_as_impossible" if refusal else "no_changes", + detail=outcome.reason, + session_id=session.id, + ) + outcome.stop = Stop( + ESCALATED, + ITEM_IMPOSSIBLE if refusal else NO_TARGET, + detail=outcome.reason, + state=BLOCKED, + consumes_attempt=False, + ) return outcome outcome.stages.append("changes") diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py index 7294a11..2e09091 100644 --- a/tests/test_reviewer.py +++ b/tests/test_reviewer.py @@ -140,7 +140,11 @@ def test_a_session_retry_is_told_why_the_last_attempt_was_refused(tmp_path: Any) type — a specific, actionable criticism — and the retry would have been sent the identical brief with no mention of it. """ - from agent_harness.session_executor import PROMPT_TEMPLATE, SessionExecutor + from agent_harness.session_executor import ( + PROMPT_TEMPLATE, + REFUSAL_FILE, + SessionExecutor, + ) from agent_harness.work import WorkQueue, WorkRecord queue = WorkQueue(str(tmp_path / "w.sqlite")) @@ -157,6 +161,7 @@ def test_a_session_retry_is_told_why_the_last_attempt_was_refused(tmp_path: Any) brief=refused.brief, checks_description="none", prior=executor._prior_failure(refused), + refusal_file=REFUSAL_FILE, ) assert "widens the repository trait" in prompt @@ -168,6 +173,7 @@ def test_a_session_retry_is_told_why_the_last_attempt_was_refused(tmp_path: Any) brief="b", checks_description="none", prior=executor._prior_failure(WorkRecord(item_id="R3", title="t", brief="b")), + refusal_file=REFUSAL_FILE, ) assert "What happened last time" not in fresh diff --git a/tests/test_session_executor.py b/tests/test_session_executor.py index e967d8c..4414c42 100644 --- a/tests/test_session_executor.py +++ b/tests/test_session_executor.py @@ -18,7 +18,19 @@ from agent_harness import providers as P from agent_harness.executor import Checks, is_disk_exhaustion from agent_harness.model_client import ModelClient, Response, RetryExhausted, Route -from agent_harness.session_executor import AgentSpec, SessionExecutor +from agent_harness.outcomes import ( + BLOCKED, + ESCALATED, + ITEM_IMPOSSIBLE, + NEEDS_A_PERSON, + NO_TARGET, +) +from agent_harness.session_executor import ( + REFUSAL_FILE, + REFUSAL_LIMIT, + AgentSpec, + SessionExecutor, +) from agent_harness.session_host import IDLE, RUNNING, WAITING, Session from agent_harness.work import DONE, FAILED, WorkQueue, WorkRecord from conftest import make_queue @@ -698,3 +710,137 @@ def slow_agent(cwd: Path) -> None: assert outcome.state == DONE, outcome.reason record = queue.get("W1") assert record is not None and record.state == DONE + + +# ------------------------------------------------ refusing an impossible item + + +def refuse(reason: str) -> Callable[[Path], None]: + """An agent that reads the item, decides it cannot be done, and says why.""" + + def agent(tree: Path) -> None: + (tree / REFUSAL_FILE).write_text(reason) + + return agent + + +def test_a_reasoned_refusal_escalates_and_carries_the_reason(repo: Path, tmp_path: Path) -> None: + reason = "calc.py has no Decimal support, and the item requires exact money arithmetic." + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse(reason))) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.disposition == ESCALATED + assert outcome.stop.reason_kind == ITEM_IMPOSSIBLE + assert outcome.stop.state == BLOCKED + assert reason in outcome.stop.detail + + +def test_a_refusal_does_not_cost_the_item_an_attempt(repo: Path, tmp_path: Path) -> None: + """The brief is what is wrong. Spending tries to rediscover that is waste.""" + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse("the item contradicts itself"))) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.consumes_attempt is False + item = queue.get("W1") + assert item is not None + assert item.attempts == 0 + + +def test_a_refusal_is_something_a_person_is_asked_to_look_at(repo: Path, tmp_path: Path) -> None: + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse("no such module exists"))) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.disposition in NEEDS_A_PERSON + + +def test_the_refusal_note_is_never_committed_or_reviewed(repo: Path, tmp_path: Path) -> None: + """It is the agent's answer about the item, not a change to the repository.""" + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse("cannot be done"))) + add_item(queue) + + executor.run_once() + + # The branch exists — it is cut before the agent starts — but nothing was + # ever committed onto it, so the note reached no history and no reviewer. + assert git(repo, "log", "--oneline", "main..harness/w1").strip() == "" + assert not (tmp_path / "trees" / "W1" / REFUSAL_FILE).exists() + + +def test_a_refusal_alongside_real_changes_is_treated_as_the_work( + repo: Path, tmp_path: Path +) -> None: + """A tree with edits in it is not a refusal, whatever else the agent wrote.""" + + def hedging_agent(tree: Path) -> None: + add_multiply(tree) + (tree / REFUSAL_FILE).write_text("I was not sure about this one.") + + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=hedging_agent)) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.state == DONE, outcome.reason + # The note must not reach the branch, or a reviewer reads the agent's + # hedging as part of the diff it is judging. + assert REFUSAL_FILE not in git(repo, "show", "--stat", "harness/w1") + + +def test_a_silent_empty_attempt_also_needs_a_person_and_names_the_session( + repo: Path, tmp_path: Path +) -> None: + """Ambiguous on purpose: 'impossible' and 'did nothing' look identical here. + + Both want a human, so both escalate; the reason is what tells them apart, + and when there is no reason the operator is pointed at the session that + holds one. + """ + executor, queue = build(repo, tmp_path, FakeDevEnv()) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.disposition == ESCALATED + assert outcome.stop.reason_kind == NO_TARGET + assert "sess-1" in outcome.stop.detail + + +def test_a_long_refusal_is_kept_but_bounded(repo: Path, tmp_path: Path) -> None: + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse("x" * 20_000))) + add_item(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert len(outcome.stop.detail) == REFUSAL_LIMIT + + +def test_the_agent_is_told_where_to_put_a_refusal(repo: Path, tmp_path: Path) -> None: + """A rule to explain yourself is worthless if it names no place to do it.""" + seen: list[str] = [] + + def read_prompt(tree: Path) -> None: + seen.append((tree / ".harness-prompt.md").read_text()) + add_multiply(tree) + + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=read_prompt)) + add_item(queue) + executor.run_once() + + assert REFUSAL_FILE in seen[0]