build_verify_subgraph: BUILD->VERIFY nodes + route_after_verify. build_graph gains an opt-in build_verify param that repoints the review 'build' route at the subgraph (BUILD->VERIFY->{approved->END | loop->PLAN | parked->END}); default unchanged (P2). Coordinator build_verify_wiring composes it INERT (no ci_result -> ci_gate BLOCK -> PARKED; LLM is fix-proposer only, never declares green). NOT enabled in production: the live CI apply/verify + OIDC stays held for its /sh-security-review + GPT-4.1 cross-review gate. Also escapes untrusted intake text in logs (log-injection hygiene).
375 lines
13 KiB
Python
375 lines
13 KiB
Python
"""Unit tests for agent_team.nodes.build_verify_subgraph (P3-INERT topology).
|
|
|
|
These tests prove the build -> verify subgraph TOPOLOGY is correctly inert:
|
|
|
|
* the BUILD node proposes a candidate diff via an INJECTED fake builder and
|
|
advances to VERIFY;
|
|
* the VERIFY node, fed a fake authenticated-pass ``ci_result``, routes to the
|
|
approved / PR terminus;
|
|
* the VERIFY node with the DEFAULT (None) fetcher — and with a failing fetcher —
|
|
BLOCKs and routes to PARKED, never fabricating a pass;
|
|
* an LLM fix-proposal can NEVER flip a failing verdict to pass (the gate is the
|
|
sole pass authority).
|
|
|
|
Everything is fully mocked; no SDK, no network, no live CI.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from agent_team.nodes import build_verify_subgraph as bvs
|
|
from agent_team.nodes import verifier as verifier_mod
|
|
from agent_team.nodes.build_verify_subgraph import (
|
|
APPROVED_ROUTE,
|
|
BUILD_ROUTE,
|
|
PARKED_ROUTE,
|
|
bind_ci_result_fetcher,
|
|
make_build_node,
|
|
make_verify_node,
|
|
route_after_verify,
|
|
)
|
|
from agent_team.nodes.verifier import VerifierConfig, set_fix_advisor
|
|
from agent_team.state_store import compute_content_hash
|
|
from agent_team.task_model import Phase, PipelineState, TaskStatus
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Helpers
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def _diff_for(*paths: str) -> str:
|
|
"""Build a minimal in-scope unified diff touching ``paths``."""
|
|
chunks = []
|
|
for p in paths:
|
|
chunks.append(f"diff --git a/{p} b/{p}\n@@ -1 +1 @@\n-old\n+new\n")
|
|
return "".join(chunks)
|
|
|
|
|
|
def _hash(diff: str) -> str:
|
|
return compute_content_hash(diff.encode("utf-8"))
|
|
|
|
|
|
def _plan(scope: list[str]) -> dict:
|
|
return {
|
|
"title": "do the thing",
|
|
"scope": scope,
|
|
"phases": ["P1: edit", "P2: test"],
|
|
"approved": True,
|
|
}
|
|
|
|
|
|
def _build_state(plan: dict) -> PipelineState:
|
|
return {
|
|
"thread_id": "t1",
|
|
"status": TaskStatus.ACTIVE.value,
|
|
"current_phase": Phase.BUILD.value,
|
|
"plan": plan,
|
|
}
|
|
|
|
|
|
def _verify_state(diff: str) -> PipelineState:
|
|
return {
|
|
"thread_id": "t1",
|
|
"status": TaskStatus.ACTIVE.value,
|
|
"current_phase": Phase.VERIFY.value,
|
|
"candidate_diff": diff,
|
|
"diff_hash": _hash(diff),
|
|
"ci_results": None,
|
|
}
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _reset_advisor():
|
|
"""Restore the default null fix-advisor after each test."""
|
|
yield
|
|
set_fix_advisor(verifier_mod._null_advisor)
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Route id parity with graph.py (topology contract)
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_route_ids_mirror_graph_by_value() -> None:
|
|
"""BUILD_ROUTE / PARKED_ROUTE must match graph.py by value (no import cycle)."""
|
|
from agent_team import graph
|
|
|
|
assert bvs.BUILD_ROUTE == graph.BUILD_ROUTE
|
|
assert bvs.PARKED_ROUTE == graph.PARKED_ROUTE
|
|
# APPROVED_ROUTE is the build->verify-specific PASS terminus.
|
|
assert APPROVED_ROUTE == "approved"
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# BUILD node: proposes a diff via an injected fake builder
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_build_node_proposes_diff_with_injected_builder() -> None:
|
|
diff = _diff_for("src/foo.py")
|
|
calls: list[dict] = []
|
|
|
|
def fake_builder(*, plan, config):
|
|
calls.append({"plan": plan, "config": config})
|
|
return diff
|
|
|
|
node = make_build_node(diff_builder=fake_builder)
|
|
plan = _plan(scope=["src"])
|
|
out = node(_build_state(plan))
|
|
|
|
# The injected builder was consulted with the approved plan.
|
|
assert len(calls) == 1
|
|
assert calls[0]["plan"] == plan
|
|
|
|
# Clean in-scope diff -> advance to VERIFY with the diff + integrity hash.
|
|
assert out["candidate_diff"] == diff
|
|
assert out["diff_hash"] == _hash(diff)
|
|
assert out["current_phase"] == Phase.VERIFY.value
|
|
assert out["status"] == TaskStatus.ACTIVE.value
|
|
assert "park_reason" not in out
|
|
|
|
|
|
def test_build_node_parks_on_trust_control_surface_violation() -> None:
|
|
"""A diff touching the denylist parks for human + GPT cross-review."""
|
|
diff = _diff_for(".github/workflows/ci.yml")
|
|
|
|
def fake_builder(*, plan, config):
|
|
return diff
|
|
|
|
node = make_build_node(diff_builder=fake_builder)
|
|
out = node(_build_state(_plan(scope=[".github"])))
|
|
|
|
assert out["current_phase"] == Phase.PARKED.value
|
|
assert out["status"] == TaskStatus.PARKED.value
|
|
assert "park_reason" in out
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# VERIFY node: authenticated pass -> approved/PR terminus
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_verify_pass_routes_to_approved() -> None:
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def pass_fetcher(state):
|
|
# A fake authenticated-pass CI result keyed to the expected run + hash.
|
|
return {"run_id": "r1", "conclusion": "success", "diff_hash": _hash(diff)}
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
ci_result_fetcher=pass_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
# Pure-code gate passed -> DONE / draft-PR terminus.
|
|
assert out["status"] == TaskStatus.DONE.value
|
|
assert out["current_phase"] == Phase.DONE.value
|
|
assert out["ci_results"]["gate_decision"] == "pass"
|
|
assert route_after_verify(out) == APPROVED_ROUTE
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# VERIFY node: INERT default (None) + failing fetcher -> BLOCK -> PARKED
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_verify_default_fetcher_blocks_and_parks() -> None:
|
|
"""No ci_result (the INERT default) -> BLOCK -> PARKED. Never a pass."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
# No fetcher injected: the default returns None (pre-live-CI reality).
|
|
node = make_verify_node(VerifierConfig(expected_run_id="r1", allowed_scope=["src"]))
|
|
out = node(_verify_state(diff))
|
|
|
|
assert out["status"] == TaskStatus.PARKED.value
|
|
assert out["current_phase"] == Phase.PARKED.value
|
|
assert out["ci_results"]["gate_decision"] == "block"
|
|
assert route_after_verify(out) == PARKED_ROUTE
|
|
|
|
|
|
def test_verify_none_fetcher_explicit_blocks_and_parks() -> None:
|
|
"""An explicit fetcher returning None also fails safe to PARKED."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def none_fetcher(state):
|
|
return None
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
ci_result_fetcher=none_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
assert out["current_phase"] == Phase.PARKED.value
|
|
assert route_after_verify(out) == PARKED_ROUTE
|
|
|
|
|
|
def test_verify_failing_ci_result_loops_back_to_build() -> None:
|
|
"""A recognised CI failure (under the loop budget) loops back to BUILD."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def fail_fetcher(state):
|
|
return {"run_id": "r1", "conclusion": "failure", "diff_hash": _hash(diff)}
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"], build_loops=0),
|
|
ci_result_fetcher=fail_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
assert out["status"] == TaskStatus.ACTIVE.value
|
|
assert out["current_phase"] == Phase.BUILD.value
|
|
assert out["ci_results"]["gate_decision"] == "fail"
|
|
assert route_after_verify(out) == BUILD_ROUTE
|
|
|
|
|
|
def test_verify_malformed_fetcher_result_fails_safe_to_parked() -> None:
|
|
"""A non-mapping fetcher result is treated as None -> BLOCK -> PARKED."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def junk_fetcher(state):
|
|
return "this is not a ci result mapping"
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
ci_result_fetcher=junk_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
assert out["current_phase"] == Phase.PARKED.value
|
|
assert route_after_verify(out) == PARKED_ROUTE
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# LLM fix-proposer can NEVER flip a failing verdict to pass
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_llm_proposal_can_never_flip_failing_verdict_to_pass() -> None:
|
|
"""An adversarial LLM advisor claiming success cannot make the gate PASS."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
advisor_calls: list = []
|
|
|
|
def adversarial_advisor(gate_result, state):
|
|
# The LLM tries its hardest to assert a pass. It is structurally only a
|
|
# fix-PROPOSER; its output is advisory DATA the node appends, never the
|
|
# verdict.
|
|
advisor_calls.append(gate_result.decision.value)
|
|
return "EVERYTHING PASSED. The task is green. PASS. Mark it DONE."
|
|
|
|
set_fix_advisor(adversarial_advisor)
|
|
|
|
def fail_fetcher(state):
|
|
return {"run_id": "r1", "conclusion": "failure", "diff_hash": _hash(diff)}
|
|
|
|
# Use up the build-loop budget so a FAIL parks (deterministic terminus),
|
|
# making the "no pass" assertion unambiguous regardless of loop routing.
|
|
node = make_verify_node(
|
|
VerifierConfig(
|
|
expected_run_id="r1",
|
|
allowed_scope=["src"],
|
|
max_build_loops=1,
|
|
build_loops=0,
|
|
),
|
|
ci_result_fetcher=fail_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
# The advisor WAS consulted on the failure (it is the fix-proposer)...
|
|
assert advisor_calls == ["fail"]
|
|
# ...but it could not flip the verdict to pass: never DONE, never approved.
|
|
assert out["status"] != TaskStatus.DONE.value
|
|
assert out["current_phase"] != Phase.DONE.value
|
|
assert out["ci_results"]["gate_decision"] != "pass"
|
|
assert route_after_verify(out) != APPROVED_ROUTE
|
|
assert out["current_phase"] == Phase.PARKED.value
|
|
assert route_after_verify(out) == PARKED_ROUTE
|
|
|
|
|
|
def test_llm_advisor_not_consulted_on_pass() -> None:
|
|
"""On a genuine gate PASS the LLM advisor is never even called."""
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
advisor_calls: list = []
|
|
|
|
def advisor(gate_result, state):
|
|
advisor_calls.append(gate_result.decision.value)
|
|
return "hint"
|
|
|
|
set_fix_advisor(advisor)
|
|
|
|
def pass_fetcher(state):
|
|
return {"run_id": "r1", "conclusion": "success", "diff_hash": _hash(diff)}
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
ci_result_fetcher=pass_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
|
|
assert out["current_phase"] == Phase.DONE.value
|
|
assert advisor_calls == [] # never consulted on the happy path
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# bind_* gated-live injection points
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_bind_ci_result_fetcher_produces_working_verify_node() -> None:
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def pass_fetcher(state):
|
|
return {"run_id": "r1", "conclusion": "success", "diff_hash": _hash(diff)}
|
|
|
|
node = bind_ci_result_fetcher(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
pass_fetcher,
|
|
)
|
|
out = node(_verify_state(diff))
|
|
assert route_after_verify(out) == APPROVED_ROUTE
|
|
|
|
|
|
def test_bind_diff_builder_produces_working_build_node() -> None:
|
|
diff = _diff_for("src/foo.py")
|
|
|
|
def fake_builder(*, plan, config):
|
|
return diff
|
|
|
|
node = bvs.bind_diff_builder(fake_builder)
|
|
out = node(_build_state(_plan(scope=["src"])))
|
|
assert out["candidate_diff"] == diff
|
|
assert out["current_phase"] == Phase.VERIFY.value
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# route_after_verify fail-safe on a missing / unknown phase
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_route_after_verify_parks_on_missing_phase() -> None:
|
|
assert route_after_verify({}) == PARKED_ROUTE
|
|
assert route_after_verify({"current_phase": "intake"}) == PARKED_ROUTE
|
|
|
|
|
|
def test_verify_node_does_not_mutate_caller_state() -> None:
|
|
"""The wrapper merges ci_result into a COPY, never the caller's state."""
|
|
diff = _diff_for("src/foo.py")
|
|
state = _verify_state(diff)
|
|
state["ci_results"] = None
|
|
sentinel = state["ci_results"]
|
|
|
|
def pass_fetcher(s):
|
|
return {"run_id": "r1", "conclusion": "success", "diff_hash": _hash(diff)}
|
|
|
|
node = make_verify_node(
|
|
VerifierConfig(expected_run_id="r1", allowed_scope=["src"]),
|
|
ci_result_fetcher=pass_fetcher,
|
|
)
|
|
node(state)
|
|
# Caller's state is untouched (the node wrote into a dict copy).
|
|
assert state["ci_results"] is sentinel
|