atlas-iac/testing/tests/test_hermes_cli_review_roles.py
Hermes Agent 360d8aad8f hermes: close the review-role gaps found reviewing PR 22
Repairs the blockers from the independent review of the previous head. The
role-aware verdict contract was correct but reachable only through one call
site and only on cards written with real newlines, so most real review cards
never used it.

* The role-blind call is no longer a weaker classifier that can reject before
  the role-aware judge runs. Without a card there is no defensible
  role-dependent judgement, so `unfinished_result_reason()` applies only the
  card-independent checks. That closes the short-circuit at every call site,
  including the one PR15 moves to `cli_lane_execution`, and it makes a
  journalled terminal record accepted under one version of these semantics
  re-validate under any other instead of being quarantined into a re-dispatch
  of an already-accepted task.
* The lane resolves the role from the card and passes it, and records it in
  the Kanban metadata. The verdict contract binds only where the lane can buy
  another turn: in single-shot mode a rejection discards the worker's real
  result, so review cards keep the relaxation without gaining any rejection
  single-shot mode did not already have.
* Card scope expands the literal \n escapes the board stores in one-line
  bodies, so explicit `Hermes-Task-Role` / `Hermes-Expected-Output` directives
  are honoured on the 6 of 78 live cards that carry no real newline, and the
  read-only, verdict and mutation heuristics stop being cut apart by them.
* Card scope now ends at the first non-card H2 and at the runner's controller
  evidence, which is emitted under its own heading. Goal-controller rejection
  history can no longer sit inside the card, and an upstream heading rename
  fails closed instead of admitting history into role resolution.
* The inference recognises the SHIP/BLOCK-shaped deliverables real cards
  actually use: 21 of 78 live cards resolve to review, up from 10, with no
  implementation card misclassified. Cards asking for a findings list rather
  than a verdict deliberately stay on the model judge, since the verdict is
  the review contract's only gate.
* A declared review role no longer outranks mutation evidence: a report that
  changed files falls back to the implementation regime.
* Judge reasons go through the agent runtime's canonical redactor, extended
  for the two shapes it deliberately passes through and this lane handles -
  `scheme://user:secret@host` and a credential named in prose - while a 40-hex
  commit SHA survives as evidence.

Regressions cover the recovered t_dbdcd739 incident, a verbatim snapshot of
every live card on three boards with a hand-labelled expected role, the
upgrade and single-shot properties against the previous gate, the upstream
context-heading contract, and the end-to-end `execute_claim` shape that used
to burn every goal turn.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 17:27:11 +00:00

160 lines
4.9 KiB
Python

"""Role-aware lane regressions for the goal-completion incident.
The instrumented single-claim harness lives in
``test_hermes_cli_review_lane`` so both lane suites drive the same fake board
and the same ``execute_claim`` entry point.
"""
from __future__ import annotations
from test_hermes_cli_review_lane import (
BLOCK_REVIEW,
IMPLEMENTATION_CARD,
REVIEW_CARD,
_Lane,
_no_judge,
_result,
_task,
execution,
goal,
lanes,
)
BARE_TOKEN_REVIEW = dict(
BLOCK_REVIEW,
summary=(
"Independent read-only review of PR #18 concludes BLOCK: the coordinator "
"drops its lease on the first tick and deployment verification is pending "
"on two of three nodes, so the change is unfit to ship."
),
)
def test_a_bare_token_review_with_artifact_prose_finalizes_on_turn_one(
tmp_path, monkeypatch
):
"""The end-to-end shape that burned every goal turn before the role reached
``unfinished_result_reason``: a bare uppercase verdict token whose prose
describes the *reviewed* deployment as pending."""
monkeypatch.setattr(goal.urllib.request, "urlopen", _no_judge)
lane = _Lane(
tmp_path,
monkeypatch,
card=REVIEW_CARD,
task=_task(goal_max_turns=3),
reports=[_result(**BARE_TOKEN_REVIEW)],
)
execution.execute_claim("titan-iac", "t_card")
action, kwargs = lane.terminal
assert action == "complete"
assert lane.rejections() == []
assert len(lane.prompts) == 1
assert kwargs["metadata"]["task_role"] == goal.REVIEW_ROLE
assert kwargs["metadata"]["task_role_source"] == "inferred"
assert "BLOCK verdict with 1 finding(s)" in kwargs["metadata"]["goal_judge_reason"]
def test_single_shot_review_completes_instead_of_discarding_its_verdict(
tmp_path, monkeypatch
):
monkeypatch.setattr(goal.urllib.request, "urlopen", _no_judge)
lane = _Lane(
tmp_path,
monkeypatch,
card=REVIEW_CARD,
task=_task(goal_mode=False, goal_max_turns=1),
reports=[_result(**BARE_TOKEN_REVIEW)],
)
execution.execute_claim("titan-iac", "t_card")
action, kwargs = lane.terminal
assert action == "complete"
assert kwargs["metadata"]["task_role"] == goal.REVIEW_ROLE
assert "goal_judge_reason" not in kwargs["metadata"]
def test_single_shot_review_without_a_verdict_fails_closed(tmp_path, monkeypatch):
"""The documented single-shot change: a verdictless review is not finished."""
monkeypatch.setattr(goal.urllib.request, "urlopen", _no_judge)
lane = _Lane(
tmp_path,
monkeypatch,
card=REVIEW_CARD,
task=_task(goal_mode=False, goal_max_turns=1),
reports=[_result(summary="Review complete; residual risks are listed.")],
)
execution.execute_claim("titan-iac", "t_card")
action, kwargs = lane.terminal
assert action == "block"
assert kwargs["reason"].startswith(goal.READ_ONLY_GUARD)
assert "declares no explicit SHIP or BLOCK verdict" in kwargs["reason"]
def test_single_shot_implementation_still_blocks_on_unfinished_evidence(
tmp_path, monkeypatch
):
lane = _Lane(
tmp_path,
monkeypatch,
card=IMPLEMENTATION_CARD,
task=_task(goal_mode=False, goal_max_turns=1),
reports=[
lanes.ProcessResult(
0,
"turn one",
{
**BLOCK_REVIEW,
"summary": "The broad rerun remains active before the push.",
"changed_files": ["src/a.py"],
},
False,
)
],
)
execution.execute_claim("titan-iac", "t_card")
action, kwargs = lane.terminal
assert action == "block"
assert "completion evidence says work is unfinished" in kwargs["reason"]
assert goal.READ_ONLY_GUARD not in kwargs["reason"]
def test_goal_controller_evidence_stays_outside_the_card(tmp_path, monkeypatch):
contexts: list[str] = []
def judge(objective, *_args, **_kwargs):
contexts.append(objective)
return (len(contexts) > 1, "ok" if len(contexts) > 1 else "push the branch")
monkeypatch.setattr(goal, "judge_goal_completion", judge)
implementation = {
**BLOCK_REVIEW,
"summary": "Focused tests passed and the branch was pushed.",
"changed_files": ["src/a.py"],
"findings": [],
}
lane = _Lane(
tmp_path,
monkeypatch,
card=IMPLEMENTATION_CARD,
task=_task(goal_max_turns=2),
reports=[
lanes.ProcessResult(0, "one", dict(implementation), False),
lanes.ProcessResult(0, "two", dict(implementation), False),
],
)
execution.execute_claim("titan-iac", "t_card")
assert lane.terminal[0] == "complete"
assert goal.CONTROLLER_EVIDENCE_HEADING in contexts[1]
assert "push the branch" in contexts[1]
assert "push the branch" not in goal.card_scope(contexts[1])