Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions src/agent_harness/adoption.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
)

Expand Down Expand Up @@ -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}"
)
),
)
)
Expand Down
42 changes: 41 additions & 1 deletion src/agent_harness/work.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
54 changes: 53 additions & 1 deletion tests/test_adoption.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
84 changes: 84 additions & 0 deletions tests/test_work.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Loading