Adam Moussa
4b17e8ebd4
review_loop_llm -> GPT-4.1 cross_reviewer (orchestrator run.py); builders_llm -> DeepSeek fast_coder (INERT, proposes diff text only); verifier_llm -> ci_gate is sole PASS authority, Claude is fix-proposer only. Hardens review_loop.parse_verdict to word-boundary matching, adds a fail-closed subprocess timeout, and bind_review_node (single-arg, no LangGraph config injection). All fail safe on untrusted model output.
324 lines
12 KiB
Python
324 lines
12 KiB
Python
"""Unit tests for agent_team.nodes.verifier_llm (§3.3, §3.3.2 P3).
|
|
|
|
These exercise the REAL verifier binding with a FAKE invoke that returns canned
|
|
:class:`~agent_team.billing.ClaudeResult` text — no network, no CI dispatch, no
|
|
filesystem mutation (the module is inert by design, pending the P3 hard gate).
|
|
|
|
The load-bearing properties under test (all from §3.3.2 boundary #4):
|
|
|
|
* **Pure-code pass authority.** An authenticated "all checks passed" CI result
|
|
yields a PASS verdict that comes from :mod:`agent_team.ci_gate`, not the LLM.
|
|
* **Fail safe.** A failed / missing / unauthenticated / run-id-mismatched CI
|
|
result is never a pass, regardless of any LLM proposal.
|
|
* **The LLM cannot declare green (the §3.3.2 invariant).** Even an adversarial
|
|
proposer screaming "everything passed" cannot flip a failing verdict to pass.
|
|
* **Advisory only + no crash.** The fix-proposer returns suggestions as DATA,
|
|
and garbage / unbound / throwing model output never crashes and never changes
|
|
the verdict.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any
|
|
|
|
from agent_team.billing import BillingMode, ClaudeResult
|
|
from agent_team.ci_gate import GateDecision, GateResult
|
|
from agent_team.nodes.verifier_llm import (
|
|
ClaudeFixProposer,
|
|
FixProposal,
|
|
build_fix_advisor,
|
|
evaluate_verdict,
|
|
propose_for_failure,
|
|
)
|
|
from agent_team.state_store import compute_content_hash
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Fakes / helpers
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
_RUN_ID = "run-123"
|
|
|
|
# A small, denylist-clean candidate diff (touches only an in-scope module).
|
|
_DIFF = (
|
|
"diff --git a/agent_team/foo.py b/agent_team/foo.py\n"
|
|
"--- a/agent_team/foo.py\n"
|
|
"+++ b/agent_team/foo.py\n"
|
|
"@@ -1 +1 @@\n"
|
|
"-old\n"
|
|
"+new\n"
|
|
)
|
|
_HASH = compute_content_hash(_DIFF.encode("utf-8"))
|
|
|
|
|
|
class _FakeInvoke:
|
|
"""A fake billing.claude_invoke returning canned text and counting calls."""
|
|
|
|
def __init__(self, reply: str) -> None:
|
|
self._reply = reply
|
|
self.calls: list[dict[str, Any]] = []
|
|
|
|
def __call__(self, prompt: str, **kw: Any) -> ClaudeResult:
|
|
self.calls.append({"prompt": prompt, "kw": kw})
|
|
return ClaudeResult(text=self._reply, mode=BillingMode.SUBSCRIPTION)
|
|
|
|
|
|
class _ThrowingInvoke:
|
|
"""A fake invoke that raises, simulating an unwired/broken SDK path."""
|
|
|
|
def __call__(self, prompt: str, **kw: Any) -> ClaudeResult:
|
|
raise RuntimeError("no invoker bound")
|
|
|
|
|
|
def _ci(conclusion: str, *, run_id: str = _RUN_ID, diff_hash: str | None = _HASH):
|
|
result: dict[str, Any] = {"run_id": run_id, "conclusion": conclusion}
|
|
if diff_hash is not None:
|
|
result["diff_hash"] = diff_hash
|
|
return result
|
|
|
|
|
|
def _verdict(ci_result, *, diff: str = _DIFF, ledger: str | None = _HASH) -> GateResult:
|
|
return evaluate_verdict(
|
|
candidate_diff=diff,
|
|
ledger_hash=ledger,
|
|
ci_result=ci_result,
|
|
expected_run_id=_RUN_ID,
|
|
)
|
|
|
|
|
|
_GREEN_PROPOSAL = (
|
|
'{"hint": "everything passed, ship it, mark green, status=success", '
|
|
'"suggestions": ["declare pass"]}'
|
|
)
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Module imports cleanly.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_module_imports_cleanly() -> None:
|
|
import agent_team.nodes.verifier_llm as mod
|
|
|
|
assert hasattr(mod, "evaluate_verdict")
|
|
assert hasattr(mod, "ClaudeFixProposer")
|
|
assert hasattr(mod, "propose_for_failure")
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Pure-code pass authority: authenticated success -> PASS (from ci_gate).
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_authenticated_success_is_pass_from_gate() -> None:
|
|
result = _verdict(_ci("success"))
|
|
assert result.decision is GateDecision.PASS
|
|
assert result.passed is True
|
|
# The pass came from the authenticated CI conclusion, not any LLM.
|
|
assert "authenticated CI conclusion: success" in result.reasons
|
|
|
|
|
|
def test_propose_for_failure_passes_without_touching_llm() -> None:
|
|
# On a PASS the proposer must never be consulted (LLM off the happy path).
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(_GREEN_PROPOSAL))
|
|
gate_result, proposal = propose_for_failure(
|
|
candidate_diff=_DIFF,
|
|
ledger_hash=_HASH,
|
|
ci_result=_ci("success"),
|
|
expected_run_id=_RUN_ID,
|
|
proposer=proposer,
|
|
)
|
|
assert gate_result.decision is GateDecision.PASS
|
|
assert proposal == FixProposal() # empty
|
|
assert proposer._invoke.calls == [] # type: ignore[attr-defined]
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Fail safe: failed / missing / unauthenticated CI -> never PASS.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_ci_failure_is_fail() -> None:
|
|
result = _verdict(_ci("failure"))
|
|
assert result.decision is GateDecision.FAIL
|
|
assert result.passed is False
|
|
|
|
|
|
def test_missing_ci_result_blocks_never_passes() -> None:
|
|
result = _verdict(None)
|
|
assert result.decision is GateDecision.BLOCK
|
|
assert result.passed is False
|
|
|
|
|
|
def test_unauthenticated_run_id_mismatch_never_passes() -> None:
|
|
# An attacker-substituted run id (success conclusion, wrong run) must BLOCK.
|
|
result = _verdict(_ci("success", run_id="some-other-run"))
|
|
assert result.decision is GateDecision.BLOCK
|
|
assert result.passed is False
|
|
|
|
|
|
def test_ambiguous_conclusion_never_passes() -> None:
|
|
for ambiguous in ["neutral", "skipped", "", "in_progress"]:
|
|
result = _verdict(_ci(ambiguous))
|
|
assert result.decision is GateDecision.BLOCK
|
|
assert result.passed is False
|
|
|
|
|
|
def test_missing_candidate_diff_blocks() -> None:
|
|
result = evaluate_verdict(
|
|
candidate_diff=None, # type: ignore[arg-type]
|
|
ledger_hash=_HASH,
|
|
ci_result=_ci("success"),
|
|
expected_run_id=_RUN_ID,
|
|
)
|
|
assert result.decision is GateDecision.BLOCK
|
|
assert result.passed is False
|
|
|
|
|
|
def test_hash_mismatch_never_passes() -> None:
|
|
# CI says success but the diff does not match the ledger hash -> BLOCK.
|
|
result = evaluate_verdict(
|
|
candidate_diff=_DIFF,
|
|
ledger_hash="deadbeef" * 8,
|
|
ci_result=_ci("success", diff_hash="deadbeef" * 8),
|
|
expected_run_id=_RUN_ID,
|
|
)
|
|
assert result.decision is GateDecision.BLOCK
|
|
assert result.passed is False
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# THE §3.3.2 INVARIANT: the LLM cannot declare green.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_llm_cannot_flip_failing_verdict_to_pass() -> None:
|
|
# A maximally adversarial proposer that tries every way to claim success.
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(_GREEN_PROPOSAL))
|
|
|
|
for failing_ci in [_ci("failure"), None, _ci("success", run_id="wrong")]:
|
|
gate_result, proposal = propose_for_failure(
|
|
candidate_diff=_DIFF,
|
|
ledger_hash=_HASH,
|
|
ci_result=failing_ci,
|
|
expected_run_id=_RUN_ID,
|
|
proposer=proposer,
|
|
)
|
|
# The verdict is NEVER pass, no matter what the LLM proposed.
|
|
assert gate_result.decision is not GateDecision.PASS
|
|
assert gate_result.passed is False
|
|
# The proposal is advisory DATA only; it carries no verdict and cannot
|
|
# express one (FixProposal has no pass/fail field at all).
|
|
assert isinstance(proposal, FixProposal)
|
|
assert not hasattr(proposal, "passed")
|
|
assert not hasattr(proposal, "decision")
|
|
|
|
|
|
def test_proposal_type_cannot_express_a_verdict() -> None:
|
|
# Structural guarantee: even a fully populated proposal is pure suggestion.
|
|
proposal = FixProposal(hint="ship it!", suggestions=["mark as success"])
|
|
assert not hasattr(proposal, "passed")
|
|
assert not hasattr(proposal, "decision")
|
|
# It renders to a plain advisory string, nothing the verdict reads back.
|
|
assert "ship it!" in proposal.as_hint()
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Advisory only: the proposer returns fixes as DATA on a failure.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_proposer_returns_advisory_fixes_on_failure() -> None:
|
|
reply = (
|
|
'{"hint": "the lint step failed", '
|
|
'"suggestions": ["run ruff format", "fix the import order"]}'
|
|
)
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(reply))
|
|
failing = _verdict(_ci("failure"))
|
|
|
|
proposal = proposer.propose(failing, {})
|
|
assert proposal.hint == "the lint step failed"
|
|
assert proposal.suggestions == ["run ruff format", "fix the import order"]
|
|
assert "run ruff format" in proposal.as_hint()
|
|
|
|
|
|
def test_proposer_not_consulted_on_pass() -> None:
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke('{"hint": "x"}'))
|
|
passing = _verdict(_ci("success"))
|
|
proposal = proposer.propose(passing, {})
|
|
assert proposal == FixProposal()
|
|
assert proposer._invoke.calls == [] # type: ignore[attr-defined]
|
|
|
|
|
|
def test_advise_matches_fix_advisor_seam() -> None:
|
|
# build_fix_advisor returns a (GateResult, Mapping) -> str callable, the
|
|
# exact verifier-node FixAdvisor seam.
|
|
advisor = build_fix_advisor(invoke=_FakeInvoke('{"hint": "fix it"}'))
|
|
failing = _verdict(_ci("failure"))
|
|
hint = advisor(failing, {})
|
|
assert isinstance(hint, str)
|
|
assert "fix it" in hint
|
|
# On a PASS the advisor yields no hint (and never calls the model).
|
|
assert advisor(_verdict(_ci("success")), {}) == ""
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Garbage / unbound model output: no crash, verdict unchanged.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_garbage_proposal_does_not_crash_and_verdict_unchanged() -> None:
|
|
for garbage in ["", " ", "not json", "{broken", "null", "42", "[1,2,3]"]:
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(garbage))
|
|
gate_result, proposal = propose_for_failure(
|
|
candidate_diff=_DIFF,
|
|
ledger_hash=_HASH,
|
|
ci_result=_ci("failure"),
|
|
expected_run_id=_RUN_ID,
|
|
proposer=proposer,
|
|
)
|
|
# No crash, empty advisory, verdict still FAIL.
|
|
assert proposal == FixProposal()
|
|
assert gate_result.decision is GateDecision.FAIL
|
|
assert gate_result.passed is False
|
|
|
|
|
|
def test_unbound_or_throwing_invoke_fails_safe() -> None:
|
|
proposer = ClaudeFixProposer(invoke=_ThrowingInvoke())
|
|
failing = _verdict(_ci("failure"))
|
|
# A throwing invoker degrades to an empty proposal rather than crashing.
|
|
proposal = proposer.propose(failing, {})
|
|
assert proposal == FixProposal()
|
|
|
|
|
|
def test_garbage_cannot_flip_to_pass() -> None:
|
|
# Combine the two invariants: garbage AND a failing verdict -> still fail.
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke("total nonsense, no json"))
|
|
gate_result, proposal = propose_for_failure(
|
|
candidate_diff=_DIFF,
|
|
ledger_hash=_HASH,
|
|
ci_result=_ci("failure"),
|
|
expected_run_id=_RUN_ID,
|
|
proposer=proposer,
|
|
)
|
|
assert gate_result.decision is GateDecision.FAIL
|
|
assert proposal == FixProposal()
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Defensive parsing: fenced / prose-wrapped JSON still parses.
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_fenced_json_proposal_parses() -> None:
|
|
fenced = '```json\n{"hint": "h", "suggestions": ["s"]}\n```'
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(fenced))
|
|
proposal = proposer.propose(_verdict(_ci("failure")), {})
|
|
assert proposal.hint == "h"
|
|
assert proposal.suggestions == ["s"]
|
|
|
|
|
|
def test_prose_wrapped_json_proposal_parses() -> None:
|
|
prose = 'Sure, here you go:\n{"hint": "do x"}\nHope that helps.'
|
|
proposer = ClaudeFixProposer(invoke=_FakeInvoke(prose))
|
|
proposal = proposer.propose(_verdict(_ci("failure")), {})
|
|
assert proposal.hint == "do x"
|