diff --git a/src/agent_harness/adoption.py b/src/agent_harness/adoption.py index 48fdc76..b94d028 100644 --- a/src/agent_harness/adoption.py +++ b/src/agent_harness/adoption.py @@ -25,8 +25,17 @@ without citations, a verification that failed — biases towards `not_started`. **Prior harness attempts are evidence, not authority.** An item the queue has -already failed keeps its attempts, its error and its event history. Adoption -reports the prior failure so a human can decide; it never rewrites it. +already failed keeps its attempts and its event history. Adoption reports the +prior failure so a human can decide; it never overrules it. + +**Except when the question changed.** A refresh that **rewrites an item's +brief** returns it to `pending` from `failed` or `blocked`, and clears the +error with it. The attempt that failed was made against wording that no longer +exists, so keeping the verdict records a decision about a question nobody is +asking. This is the second half of the loop that lets an agent refuse an +impossible item: without it, a human who rewrites the item in response is told +"nothing to do" (#178). Nothing else moves state — a changed title, label or +dependency leaves it exactly where it was. """ from __future__ import annotations @@ -44,7 +53,7 @@ from .github import MARKER, GitHub from .outcomes import ESCALATE, RETRY from .plan import ParsedPlan, WorkItem -from .work import CLAIMED, DONE, PENDING, Project, WorkQueue, WorkRecord +from .work import CLAIMED, DONE, PENDING, Project, WorkQueue, WorkRecord, revives #: The lifecycle of one adoption (proposal §5.1). `rejected` and `revise` are #: the two ways out of `proposed` that do not mutate anything. @@ -183,6 +192,9 @@ class AdoptionItem: #: What the queue already says about this item, if anything. Adoption #: reports it rather than overwriting it. queue_state: str | None = None + #: The brief the queue is holding, so the report can tell a re-sync that + #: changes nothing from one that rewrites the item. + queue_brief: str | None = None proposed_state: str = PENDING evidence: list[Evidence] = field(default_factory=list) candidates: list[ExternalCandidate] = field(default_factory=list) @@ -582,6 +594,7 @@ def _inspect_item( brief=item.brief(), depends_on=list(item.depends_on), queue_state=existing.state if existing else None, + queue_brief=existing.brief if existing else None, candidates=list(candidates), ) @@ -700,8 +713,19 @@ def _add_mutations(self, project_id: str, result: AdoptionItem) -> None: ProposedMutation( kind="refresh queue row", target=f"item {result.item_id} in project {project_id}", + # The report used to say `R7 -> pending` in its heading and + # `state stays failed` in this line, which is the report + # contradicting itself two lines apart. Say what will + # happen: a rewritten brief revives a stalled item, and + # nothing else moves it. detail=( - f"update title, brief and dependencies; state stays {result.queue_state}" + "update title, brief and dependencies; " + + ( + f"the brief changed, so state returns to pending " + f"from {result.queue_state}" + if revives(result.queue_state or "", result.queue_brief, result.brief) + else f"state stays {result.queue_state}" + ) ), ) ) diff --git a/src/agent_harness/work.py b/src/agent_harness/work.py index aae03c3..5a0b28a 100644 --- a/src/agent_harness/work.py +++ b/src/agent_harness/work.py @@ -318,6 +318,28 @@ def _run(self) -> None: return +#: States a rewritten brief rescues an item from. `done` is absent on purpose — +#: editing the description of finished work does not un-finish it — and so are +#: `claimed` and `pending`, which are not stuck. +STALLED = frozenset({FAILED, BLOCKED}) + + +def revives(state: str, was: str | None, now: str | None) -> bool: + """Does this refresh un-stick a stalled item? + + Only a changed **brief** does. Re-syncing an unchanged plan must leave + every state exactly where it was, and fixing a typo in a title or adding a + label is not an answer to whatever stopped the item. + + A changed brief is different in the one way that matters: the attempt that + failed was made against wording that no longer exists. Leaving it `failed` + records a verdict on a question nobody is asking any more — and it is + silent, so a person who rewrites an item in response to an agent refusing + it as impossible (#174) watches the next run say "nothing to do" (#178). + """ + return state in STALLED and (was or "").strip() != (now or "").strip() + + def _process_alive(pid: int) -> bool: """Whether a pid on this host is still running. @@ -782,7 +804,7 @@ def add( ) for record in records: existing = conn.execute( - "SELECT state FROM work WHERE project_id = ? AND item_id = ?", + "SELECT state, brief FROM work WHERE project_id = ? AND item_id = ?", (project_id, record.item_id), ).fetchone() if existing is None: @@ -815,6 +837,24 @@ def add( record.item_id, ), ) + if revives(existing["state"], existing["brief"], record.brief): + # The verdict that stopped this item was reached against + # wording that no longer exists, so it is no longer a + # verdict on anything. Clearing `last_error` with it: the + # next attempt is told what the previous one was refused + # for, and that refusal is about a question nobody is + # asking any more. + conn.execute( + "UPDATE work SET state = ?, last_error = NULL " + "WHERE project_id = ? AND item_id = ?", + (PENDING, project_id, record.item_id), + ) + log.info( + "%s/%s: brief rewritten, returning it to pending from %s", + project_id, + record.item_id, + existing["state"], + ) self.graph.set_edges( project_id, record.item_id, diff --git a/tests/test_adoption.py b/tests/test_adoption.py index 1d5f6a1..4e03b2d 100644 --- a/tests/test_adoption.py +++ b/tests/test_adoption.py @@ -589,7 +589,19 @@ def test_prior_harness_attempts_are_evidence_and_history_is_retained( ) -> None: audit = AuditStore(tmp_path / "audit.sqlite") sink = event_sink(audit, source="stage-c") - queue.add([WorkRecord(item_id="T5", title="Never started")], project_id="existing") + # Seeded with the brief the plan carries, so this is a re-sync that changes + # nothing. A refresh that *rewrites* the brief deliberately does move the + # state, which the test below this one covers. + queue.add( + [ + WorkRecord( + item_id="T5", + title="Never started", + brief="Never started\n\nNothing anywhere refers to this.", + ) + ], + project_id="existing", + ) queue.set_control("running", project_id="existing") claimed = queue.claim("worker-1", project_id="existing") assert claimed is not None and claimed.item_id == "T5" @@ -892,3 +904,43 @@ def test_adopt_cli_dry_run_leaves_no_trace(tmp_path: Path, repo: Path, capsys: A stored = WorkQueue(str(db)) assert stored.items() == [] assert stored.get_setting("adoption:existing") is None + + +def test_a_rewritten_brief_revives_a_failed_item_and_the_report_says_so( + tmp_path: Path, queue: WorkQueue, repo: Path +) -> None: + """#178: the half of the refusal loop that acts on the human's answer. + + The report used to say `T5 -> pending` in its heading and `state stays + failed` in the mutation line two lines below, and the second one was true. + A human who rewrote an item in response to an agent refusing it as + impossible was then told "nothing to do". + """ + queue.add( + [WorkRecord(item_id="T5", title="Never started", brief="the original wording")], + project_id="existing", + ) + queue.set_control("running", project_id="existing") + queue.claim("worker-1", project_id="existing") + queue.release( + "T5", FAILED, error="cannot be done as specified", owner="worker-1", project_id="existing" + ) + + adopter = adoption(queue, repo) + report = adopter.inspect("existing", parse_plan(PLAN)) + + t5 = by_id(report)["T5"] + refresh = next(m for m in t5.mutations if m.kind == "refresh queue row") + assert "state returns to pending" in refresh.detail + assert "state stays" not in refresh.detail + + adopter.approve("existing", approved_drops=[]) + adopter.reconcile("existing") + + record = queue.get("T5", project_id="existing") + assert record is not None + assert record.state == PENDING + assert not record.last_error + # The attempt itself is history and stays history. What changed is the + # question, not the fact that it was once attempted. + assert record.attempts == 1 diff --git a/tests/test_work.py b/tests/test_work.py index 457d621..bd5feb3 100644 --- a/tests/test_work.py +++ b/tests/test_work.py @@ -8,9 +8,11 @@ import pytest from agent_harness.graph import ResolverOutcome +from agent_harness.outcomes import BLOCKED from agent_harness.work import ( CLAIMED, DONE, + FAILED, PENDING, WorkQueue, WorkRecord, @@ -329,3 +331,85 @@ def test_settings_are_shared_across_processes(tmp_path: Path) -> None: def test_an_unset_setting_is_none_not_an_error(queue: WorkQueue) -> None: assert queue.get_setting("nope") is None + + +# ------------------------------- a rewritten brief revives a stalled item + + +def _stalled(tmp_path: Path, state: str, *, brief: str = "the brief") -> WorkQueue: + """An item the queue has stopped, ready for a plan re-sync to land on.""" + queue = WorkQueue(str(tmp_path / "w.sqlite")) + queue.add([WorkRecord(item_id="R7", title="t", brief=brief)]) + queue.set_control("running") + queue.claim(owner="w") + queue.release("R7", state, owner="w", error="cannot be done as specified") + return queue + + +def test_a_rewritten_brief_returns_a_blocked_item_to_pending(tmp_path: Path) -> None: + """The second half of the refusal loop (#178). + + An agent refuses an item as impossible, a human rewrites it in response, + and until this the rewrite was inert: the brief updated, the item stayed + put, and the next run said "nothing to do". + """ + queue = _stalled(tmp_path, BLOCKED) + + queue.add([WorkRecord(item_id="R7", title="t", brief="an achievable thing instead")]) + + item = queue.get("R7") + assert item is not None + assert item.state == PENDING + assert item.brief == "an achievable thing instead" + # Feeding the old refusal to the next attempt would be telling it not to + # repeat a fault it can no longer commit. + assert not item.last_error + + +def test_a_rewritten_brief_also_revives_a_failed_item(tmp_path: Path) -> None: + queue = _stalled(tmp_path, FAILED) + + queue.add([WorkRecord(item_id="R7", title="t", brief="something else entirely")]) + + item = queue.get("R7") + assert item is not None + assert item.state == PENDING + + +def test_an_unchanged_brief_leaves_a_failed_item_where_it_was(tmp_path: Path) -> None: + """A routine re-sync must not un-fail work, and a better title is not an answer.""" + queue = _stalled(tmp_path, FAILED) + + queue.add([WorkRecord(item_id="R7", title="a better title", brief="the brief")]) + + item = queue.get("R7") + assert item is not None + assert item.state == FAILED + assert item.title == "a better title" + assert item.last_error == "cannot be done as specified" + + +def test_whitespace_alone_is_not_a_rewrite(tmp_path: Path) -> None: + """Re-parsing a plan must not revive an item because a newline moved.""" + queue = _stalled(tmp_path, FAILED) + + queue.add([WorkRecord(item_id="R7", title="t", brief=" the brief\n")]) + + item = queue.get("R7") + assert item is not None + assert item.state == FAILED + + +def test_rewriting_the_brief_of_finished_work_does_not_unfinish_it(tmp_path: Path) -> None: + """`done` is not stalled, and editing a description is not a reason to redo it.""" + queue = WorkQueue(str(tmp_path / "w.sqlite")) + queue.add([WorkRecord(item_id="R1", title="t", brief="the brief")]) + queue.set_control("running") + queue.claim(owner="w") + queue.release("R1", DONE, owner="w") + + queue.add([WorkRecord(item_id="R1", title="t", brief="a completely different brief")]) + + item = queue.get("R1") + assert item is not None + assert item.state == DONE