atlas-iac/services/hermes/scripts/cli_lane_goal.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

499 lines
21 KiB
Python

#!/usr/bin/env python3
"""Role-aware completion checks for durable external Kanban workers.
Completion is judged against the action the card assigned, not against the
health of whatever the card asked the worker to look at.
* Implementation cards keep the fail-closed local goal judge: a report is
accepted only when the model agrees every explicit requirement finished.
* Review, audit, and diagnostic cards deliver a *verdict*. A completed report
carrying a truthful SHIP or BLOCK verdict with evidence finalizes the card
even when the verdict says the reviewed artifact must not ship. Findings are
the deliverable, not a task blocker, so the card is never resumed with an
instruction to repair an implementation the reviewer may not touch.
Three deliberate boundaries keep that regime honest. First, ``role`` belongs to
the caller that holds the card: ``unfinished_result_reason`` cannot classify a
card it was never handed, so every caller holding the objective MUST pass
``role``; the role-blind default is the replay contract below, never a weaker
classifier that could short-circuit the role-aware gate.
Second, the verdict contract binds in single-shot mode exactly as in the goal
loop: a review card's deliverable *is* its verdict, so the lane fails closed
rather than record a verdictless review as done. Two single-shot outcomes change
against main d8f2d818, deliberately - a completed review whose prose calls the
*reviewed* artifact pending now completes instead of being discarded, and one
carrying no usable verdict now blocks instead of completing.
Third, a journalled terminal record stays valid across an upgrade. PR15 re-runs
this gate on replay without a card, so ``role=None`` keeps the unfinished-work
heuristic - that integrity check still works - and skips it only for a report
that satisfies the whole review contract and changed no files. That is precisely
the set the role-aware lane accepts as a review, so a record accepted under one
version re-validates under every other instead of being quarantined into a
re-dispatch of an already-accepted task.
The task role comes from explicit card metadata (``Hermes-Task-Role: review``)
whenever the card author supplies it, read from real newlines and from the
literal ``\\n`` escapes the board stores in single-line bodies. Pre-contract
cards fall back to a narrow inference needing a read-only scope, a requested
SHIP/BLOCK verdict, no requested mutation deliverable, and a report that changed
no files. A card whose deliverable is a findings list rather than a verdict
deliberately stays on the implementation regime: the verdict *is* the review
contract's gate, so inferring a review role for a card that never asked for one
could only fail closed. A report that changed files never resolves to the review
role, even when the card declares it.
"""
from __future__ import annotations
import json
import os
import re
import urllib.error
import urllib.request
from typing import Any, Callable
try: # The lane runs inside the agent image, which owns the canonical redactor.
from agent.redact import redact_sensitive_text as _canonical_redact
except Exception: # pragma: no cover - only when the agent runtime is absent
_canonical_redact = None
GOAL_JUDGE_URL = os.environ.get(
"HERMES_GOAL_JUDGE_URL",
"http://hermes-model-gate.hermes.svc.cluster.local:11434/v1/chat/completions",
)
GOAL_JUDGE_MODEL = os.environ.get(
"HERMES_GOAL_JUDGE_MODEL",
"qwen2.5:14b-instruct-q4_0",
)
GOAL_JUDGE_TIMEOUT_SECONDS = max(
30,
min(180, int(os.environ.get("HERMES_GOAL_JUDGE_TIMEOUT_SECONDS", "120"))),
)
RESULT_STATUSES = frozenset({"completed", "blocked", "incomplete"})
REVIEW_ROLE = "review"
IMPLEMENTATION_ROLE = "implementation"
TASK_ROLES = frozenset({REVIEW_ROLE, IMPLEMENTATION_ROLE})
ROLE_ALIASES = {
"audit": REVIEW_ROLE,
"assessment": REVIEW_ROLE,
"diagnosis": REVIEW_ROLE,
"diagnostic": REVIEW_ROLE,
"inspection": REVIEW_ROLE,
"review": REVIEW_ROLE,
"reviewer": REVIEW_ROLE,
"verdict": REVIEW_ROLE,
"build": IMPLEMENTATION_ROLE,
"change": IMPLEMENTATION_ROLE,
"implement": IMPLEMENTATION_ROLE,
"implementation": IMPLEMENTATION_ROLE,
"repair": IMPLEMENTATION_ROLE,
}
JUDGE_REASON_LIMIT = 600
MIN_REVIEW_SUMMARY_CHARS = 80
READ_ONLY_GUARD = (
"the assigned action is a read-only review: report the verdict and evidence, "
"and do not modify the reviewed implementation"
)
# The runner appends its own turn/rejection evidence to the judge objective.
# It is emitted under this heading so ``card_scope`` drops it with every other
# non-card section instead of letting a quoted worker sentence reassign a role.
CONTROLLER_EVIDENCE_HEADING = "## Hermes goal-controller evidence"
# ``hermes_cli.kanban_db.build_worker_context`` (Hermes runtime 0.18.2) renders
# the worker context as H2 sections and emits exactly CARD_SECTIONS followed by
# HISTORY_SECTIONS; ``test_hermes_cli_review_context.py`` pins that contract
# against the installed runtime. The cut below allow-lists the two card
# sections, so an upstream rename fails closed - the card just ends early.
CARD_SECTIONS = ("Body", "Attachments")
HISTORY_SECTIONS = (
"Prior attempts on this task",
"Parent task results",
"Recent work by",
"Comment thread",
)
GOAL_JUDGE_SCHEMA: dict[str, Any] = {
"type": "object",
"additionalProperties": False,
"required": ["verdict", "reason"],
"properties": {
"verdict": {"type": "string", "enum": ["complete", "continue"]},
"reason": {"type": "string"},
},
}
UNFINISHED_EVIDENCE = re.compile(
r"(?:\bin[ -]?progress\b|\bstill\s+(?:active|pending|running)\b|"
r"\bremains?\s+(?:active|pending|running|unfinished)\b|"
r"\b(?:tests?|checks?|build|validation|verification|commit|push)\b"
r"\s+(?:is|are|remains?|still|currently|has|have)\s+"
r"(?:pending|running|unfinished|not\s+(?:yet\s+)?(?:done|finished|run|complete))\b|"
r"\b(?:pending|running|unfinished)\s+"
r"\b(?:tests?|checks?|build|validation|verification|commit|push)\b)",
re.IGNORECASE | re.DOTALL,
)
HISTORY_SECTION = re.compile(
r"^##\s+(?!(?:Body|Attachments)\b)"
r"|^Authoritative Hermes goal-controller evidence\b",
re.IGNORECASE | re.MULTILINE,
)
# Explicit, machine-readable card metadata. This is the supported contract;
# every pattern below it only keeps pre-contract cards working.
ROLE_DIRECTIVE = re.compile(
r"^[\s>#*\-]*(?:hermes[\s_-]*)?task[\s_-]*role\s*[:=]\s*([A-Za-z_-]+)\s*$",
re.IGNORECASE | re.MULTILINE,
)
OUTPUT_DIRECTIVE = re.compile(
r"^[\s>#*\-]*(?:hermes[\s_-]*)?expected[\s_-]*output\s*[:=]\s*([A-Za-z_-]+)\s*$",
re.IGNORECASE | re.MULTILINE,
)
READ_ONLY_SCOPE = re.compile(
r"(?:\b(?:do|does|must|should|shall|may|will)\s+not\s+(?:\w+\s+){0,3}?"
r"(?:edit|modify|change|alter|patch|rewrite|implement|repair|author)\b"
r"|\bdon'?t\s+(?:\w+\s+){0,3}?(?:edit|modify|change|alter|patch)\b"
r"|\bwithout\s+(?:editing|modifying|changing|altering|patching)\b"
r"|\bno\s+(?:\w+\s+){0,2}?(?:edits?|changes?|modifications?)\b"
r"|\bmake\s+no\s+(?:code|file|source)?\s*changes\b"
r"|\bpristine\b|\bread[\s-]?only\b)",
re.IGNORECASE,
)
# A SHIP/BLOCK-shaped deliverable, not merely a mention of the word "verdict":
# cards that audit some other component's verdict machinery must not be read as
# owing one themselves.
VERDICT_DELIVERABLE = re.compile(
r"(?:\bship\b[^.\n]{0,120}?\bblock\b|\bblock\b[^.\n]{0,120}?\bship\b"
r"|\bship\s*/\s*not-?ship\b|\bverdict\s+of\b"
r"|\b(?:return|record|report|give|provide|deliver|state|produce|emit)\b"
r"(?:\s+[-\w']+){0,6}?\s+verdict\b)",
re.IGNORECASE,
)
MUTATION_DELIVERABLE = re.compile(
r"(?:\bpushed\s+(?:sha|commit|head|branch|revision)\b"
r"|\bpush\s+(?:\w+\s+){0,3}?(?:branch|commit|change\w*|fix\w*)\b"
r"|\bcommit\s+(?:\w+\s+){0,4}?and\s+push\b|\bamend\b"
r"|\bopen\s+(?:a|an|one|the)\s+(?:new\s+)?(?:draft\s+)?(?:pr\b|pull\s+request)"
r"|\bdraft\s+pr\s+only\b)",
re.IGNORECASE,
)
VERDICT_DIRECTIVE = re.compile(
r"\b(?:final\s+|strict\s+|overall\s+)?(?:verdict|recommendation|decision)\s*"
r"(?:is\s*)?[:=—-]\s*[*_`\s]*(ship|block)\b",
re.IGNORECASE,
)
VERDICT_TOKEN = re.compile(r"(?<![A-Za-z])(SHIP|BLOCK)(?![A-Za-z])")
CONTROL_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]")
# Fallback for environments without the agent runtime (repo-only checkouts).
SECRET_LIKE = re.compile(
r"(?:gh[pousr]_[A-Za-z0-9]{16,}|xox[abprs]-[A-Za-z0-9-]{10,}"
r"|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16}"
r"|eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,})"
)
# Two shapes the canonical redactor deliberately passes through but this lane
# handles: ``scheme://user:secret@host`` clone URLs (upstream leaves web
# userinfo alone so round-trip workflow URLs keep working) and an opaque
# credential named in prose. Both are anchored on surrounding syntax so a 40-hex
# git SHA - indistinguishable from a Gitea PAT alone - survives as evidence.
URL_USERINFO = re.compile(r"(?<=://)([^/\s:@]{1,64}):([^/\s@]{1,256})@")
CREDENTIAL_CONTEXT = re.compile(
r"((?:pat|token|secret|password|passwd|credential|api[_-]?key|authorization)"
r"[\"'\s:=]+(?:bearer\s+|token\s+)?)([A-Za-z0-9_.\-]{20,})",
re.IGNORECASE,
)
def _bounded(value: str, limit: int) -> str:
"""Retain both ends of evidence while bounding local-judge context."""
if len(value) <= limit:
return value
marker = "\n...[goal evidence compacted]...\n"
remaining = max(0, limit - len(marker))
head = remaining // 2
return f"{value[:head]}{marker}{value[-(remaining - head):]}"
def redact(value: str) -> str:
"""Redact with the runtime's canonical helper plus this lane's own gaps."""
text = SECRET_LIKE.sub("[redacted]", value)
text = URL_USERINFO.sub(r"\1:[redacted]@", text)
text = CREDENTIAL_CONTEXT.sub(r"\1[redacted]", text)
return _canonical_redact(text, force=True) if _canonical_redact else text
def sanitize_reason(value: Any, limit: int = JUDGE_REASON_LIMIT) -> str:
"""Return one bounded, single-line, secret-free judge reason."""
text = redact(str(value or ""))
text = " ".join(CONTROL_CHARACTERS.sub(" ", text).split())
if not text:
return "local judge supplied no reason"
return text if len(text) <= limit else f"{text[: limit - 3].rstrip()}..."
def _strings(value: Any) -> list[str]:
"""Return the non-empty string entries of a reported evidence list."""
if not isinstance(value, list):
return []
return [str(item).strip() for item in value if str(item).strip()]
def card_scope(objective: str) -> str:
"""Return the card the worker was assigned, without appended run history.
Literal ``\\n`` escapes are expanded first: the board stores most bodies on
a single physical line, and every pattern here is line-anchored or depends
on word boundaries the escape sequence would otherwise destroy. Expanding
before the cut is monotone - it can only move the first history heading
earlier - so it cannot smuggle appended history into the card. The
normalization is for analysis only; the judge objective is never rewritten.
"""
text = str(objective or "").replace("\\r\\n", "\n").replace("\\n", "\n")
match = HISTORY_SECTION.search(text)
return text[: match.start()] if match else text
def _declared_role(objective: str) -> str | None:
"""Return the role a card declared through explicit worker metadata."""
for pattern in (ROLE_DIRECTIVE, OUTPUT_DIRECTIVE):
roles = {
ROLE_ALIASES[value]
for value in (
match.group(1).strip().lower() for match in pattern.finditer(objective)
)
if value in ROLE_ALIASES
}
if len(roles) == 1:
return roles.pop()
if roles:
# A card that declares two different roles has no usable contract.
# Hold it to the stricter implementation regime rather than letting
# the more permissive review path win an ambiguous declaration.
return IMPLEMENTATION_ROLE
return None
def task_role(objective: str, result: dict[str, Any] | None = None) -> tuple[str, str]:
"""Resolve the assigned task role and how that resolution was reached."""
text = card_scope(objective)
changed = _strings((result or {}).get("changed_files"))
declared = _declared_role(text)
if declared == REVIEW_ROLE and changed:
# Mutation evidence outranks a declaration: a card cannot label itself
# read-only and then self-certify a report that edited the tree.
return IMPLEMENTATION_ROLE, "conflict"
if declared is not None:
return declared, "directive"
if (
not changed
and READ_ONLY_SCOPE.search(text)
and VERDICT_DELIVERABLE.search(text)
and not MUTATION_DELIVERABLE.search(text)
):
return REVIEW_ROLE, "inferred"
return IMPLEMENTATION_ROLE, "default"
def review_verdict(result: dict[str, Any]) -> tuple[str | None, str | None]:
"""Return the declared SHIP/BLOCK verdict, or why it cannot be trusted."""
declared: set[str] = set()
field = result.get("verdict")
if field is not None:
value = str(field).strip().upper()
if value not in {"SHIP", "BLOCK"}:
return None, f"review verdict field is not SHIP or BLOCK: {value[:40]!r}"
declared.add(value)
summary = str(result.get("summary") or "")
declared.update(match.upper() for match in VERDICT_DIRECTIVE.findall(summary))
if len(declared) > 1:
return None, "review report declares both SHIP and BLOCK verdicts"
if declared:
return declared.pop(), None
tokens = {match.upper() for match in VERDICT_TOKEN.findall(summary)}
if len(tokens) > 1:
return None, "review summary names both SHIP and BLOCK without a verdict line"
if tokens:
return tokens.pop(), None
return None, "review report declares no explicit SHIP or BLOCK verdict"
def review_completion_problem(result: dict[str, Any]) -> str | None:
"""Fail closed on a review report that does not carry a usable verdict."""
verdict, problem = review_verdict(result)
if problem or verdict is None:
return problem or "review report declares no explicit SHIP or BLOCK verdict"
summary = str(result.get("summary") or "").strip()
if len(summary) < MIN_REVIEW_SUMMARY_CHARS:
return f"{verdict} verdict carries no reviewable rationale in its summary"
findings = _strings(result.get("findings"))
if verdict == "BLOCK" and not findings:
return "BLOCK verdict reports no findings to justify it"
if not findings and not _strings(result.get("tests_run")) and not _strings(
result.get("artifacts")
):
return f"{verdict} verdict reports no findings, tests, or artifacts as evidence"
return None
def _completion_problem(result: dict[str, Any], role: str | None) -> str | None:
"""Return the unguarded reason this report is not a finished deliverable."""
status = str(result.get("status") or "")
if status == "incomplete":
summary = str(result.get("summary") or "").strip()
return summary or "worker explicitly reported incomplete work"
if status != "completed":
return None
blockers = result.get("blockers")
if isinstance(blockers, list) and any(str(item).strip() for item in blockers):
return "worker reported blockers while claiming completion"
if role == REVIEW_ROLE:
# A review's prose describes the reviewed artifact, so the unfinished
# work heuristic below would read the artifact's state as the review's
# own. The verdict contract is the deterministic gate instead.
return review_completion_problem(result)
if (
role is None
and not _strings(result.get("changed_files"))
and review_completion_problem(result) is None
):
# A replay caller holds no card. This is exactly the shape the
# role-aware lane accepts as a review, so honouring it here keeps a
# journalled record valid without weakening the heuristic for anything
# that is not already a complete review deliverable.
return None
tests = result.get("tests_run")
evidence = [str(result.get("summary") or "").strip()]
if isinstance(tests, list):
evidence.extend(str(item) for item in tests)
match = UNFINISHED_EVIDENCE.search("\n".join(evidence))
if match:
return f"completion evidence says work is unfinished: {match.group(0).strip()}"
return None
def unfinished_result_reason(
result: dict[str, Any],
*,
role: str | None = None,
) -> str | None:
"""Reject self-contradictory completion reports without model inference.
``role`` must be supplied by any caller holding the card; see the module
docstring for the role-blind replay contract a caller without one gets.
"""
problem = _completion_problem(result, role)
if problem is None:
return None
if role == REVIEW_ROLE:
# Every reason a reviewer may ever read carries the guard, so a resumed
# review is never told to repair the implementation it must not touch.
problem = f"{READ_ONLY_GUARD}; {problem}"
return sanitize_reason(problem)
def _completion_claimed(result: dict[str, Any]) -> bool:
"""Report whether the worker actually claimed a finished task."""
return str(result.get("status") or "") == "completed"
def _judge_payload(objective: str, result: dict[str, Any], role: str) -> dict[str, Any]:
"""Build the fail-closed local judge request for an implementation card."""
return {
"model": GOAL_JUDGE_MODEL,
"messages": [
{
"role": "system",
"content": (
"You are a fail-closed completion judge for an engineering task. "
"Compare the objective and acceptance criteria with the worker report. "
"Return complete only when the report gives concrete evidence that every "
"explicit requirement finished. Return continue when any work, test, command, "
"commit, push, review, or verification is pending, in progress, omitted, or "
"inconclusive. Defects the worker reports about some other artifact are "
"findings, not unfinished work on this task. Do not trust the report status "
"field."
),
},
{
"role": "user",
"content": json.dumps(
{
"task_role": role,
"objective": _bounded(objective, 6000),
"worker_report": _bounded(
json.dumps(result, default=str), 4000
),
},
separators=(",", ":"),
),
},
],
"stream": False,
"temperature": 0,
"max_tokens": 160,
"response_format": {
"type": "json_schema",
"json_schema": {
"name": "goal_completion_verdict",
"strict": True,
"schema": GOAL_JUDGE_SCHEMA,
},
},
}
def judge_goal_completion(
objective: str,
result: dict[str, Any],
*,
url: str = GOAL_JUDGE_URL,
open_request: Callable[..., Any] = urllib.request.urlopen,
) -> tuple[bool, str]:
"""Judge one worker report against the action its card actually assigned."""
role, source = task_role(objective, result)
reason = unfinished_result_reason(result, role=role)
if reason is None and not _completion_claimed(result):
reason = (
f"worker reported status {str(result.get('status') or 'unknown')!r} "
"rather than a finished task"
)
if role == REVIEW_ROLE:
reason = f"{READ_ONLY_GUARD}; {reason}"
if reason:
return False, sanitize_reason(reason)
if role == REVIEW_ROLE:
# The requested deliverable is present and self-consistent. A verdict
# that the reviewed artifact must not ship is a finished review, so the
# card finalizes instead of resuming a worker that may not repair it.
verdict, _ = review_verdict(result)
findings = len(_strings(result.get("findings")))
return True, sanitize_reason(
f"review deliverable complete: {verdict} verdict with {findings} finding(s); "
f"task role {role} resolved by {source}"
)
request = urllib.request.Request(
url,
data=json.dumps(_judge_payload(objective, result, role)).encode("utf-8"),
headers={"Content-Type": "application/json"},
method="POST",
)
try:
with open_request(request, timeout=GOAL_JUDGE_TIMEOUT_SECONDS) as response:
document = json.load(response)
content = document["choices"][0]["message"]["content"]
verdict = json.loads(content)
if verdict.get("verdict") not in {"complete", "continue"}:
raise ValueError("local judge returned an invalid verdict")
reason = sanitize_reason(verdict.get("reason"))
return verdict.get("verdict") == "complete", reason
except (
AttributeError,
IndexError,
KeyError,
OSError,
TypeError,
ValueError,
json.JSONDecodeError,
urllib.error.URLError,
) as error:
return False, sanitize_reason(
f"local completion judge unavailable: {type(error).__name__}: {error}"
)