From 07de7a4b24aa8f773adab8747b637d8b127659fa Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Mon, 22 Jun 2026 17:51:07 -0400 Subject: [PATCH] =?UTF-8?q?feat(agent-team):=20P3-flip=20Phase=201=20?= =?UTF-8?q?=E2=80=94=20gate-weakening=20detector=20(=C2=A74.5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A diff that ADDS a lint/type/coverage/security suppression (noqa, type: ignore, pragma: no cover, nosec, nosemgrep), a test skip/xfail, or a hook bypass (--no-verify) could make CI pass falsely. The pure-code gate now flags these via gate_weakening_violations() and BLOCKs in evaluate_ci_gate as a top-priority trust violation (step 1b, alongside the denylist) — regardless of the authenticated CI conclusion. A build cannot pass itself by disabling its own checks; flagged diffs escalate to a human. Only ADDED lines are inspected (removing a suppression is fine). 1015 tests pass, ruff clean. --- agent-team/agent_team/ci_gate.py | 73 ++++++++++++++++++++++++++++++++ agent-team/tests/test_ci_gate.py | 69 ++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/agent-team/agent_team/ci_gate.py b/agent-team/agent_team/ci_gate.py index f6aa8b1..d789be7 100644 --- a/agent-team/agent_team/ci_gate.py +++ b/agent-team/agent_team/ci_gate.py @@ -71,6 +71,7 @@ __all__ = [ "denylist_violations", "diff_touched_paths", "evaluate_ci_gate", + "gate_weakening_violations", "verify_diff_hash", ] @@ -368,6 +369,63 @@ def _within_scope(path: str, scope: Sequence[str]) -> bool: return False +# §4.5 gate-weakening: substrings whose appearance on an ADDED diff line means the +# change is suppressing a quality/security gate (a self-passing move). Matched +# case-insensitively against added-line content. +_GATE_WEAKENING_MARKERS: tuple[tuple[str, str], ...] = ( + ("noqa", "ruff/flake8 lint suppression (noqa)"), + ("type: ignore", "type-check suppression (type: ignore)"), + ("type:ignore", "type-check suppression (type:ignore)"), + ("pragma: no cover", "coverage suppression (pragma: no cover)"), + ("pragma: no-cover", "coverage suppression (pragma: no-cover)"), + ("nosec", "bandit security suppression (nosec)"), + ("nosemgrep", "semgrep suppression (nosemgrep)"), + ("--no-verify", "git/pre-commit hook bypass (--no-verify)"), +) + +# Test skip/xfail decorators and calls (carry args, so regex not substring). +_TEST_SKIP_RE: re.Pattern[str] = re.compile( + r"@(?:pytest\.mark\.(?:skip|xfail)|unittest\.skip\w*)\b" + r"|\bpytest\.(?:skip|xfail)\s*\(" + r"|\.skipTest\s*\(" +) + + +def _added_diff_lines(unified_diff: str) -> list[str]: + """Return the content of lines ADDED by a unified diff (excluding ``+++`` headers).""" + if not isinstance(unified_diff, str): + raise CiGateError( + f"unified_diff must be str, got {type(unified_diff).__name__}" + ) + return [ + line[1:] + for line in unified_diff.splitlines() + if line.startswith("+") and not line.startswith("+++") + ] + + +def gate_weakening_violations(unified_diff: str) -> list[str]: + """Return reasons the diff WEAKENS a quality/security gate (empty if none). + + A diff that ADDS a lint/type/coverage/security suppression, a test + skip/xfail, or a hook bypass could make CI pass falsely — so the pure-code + gate flags it even when the authenticated CI conclusion is ``success`` + (§4.5). Such diffs are escalated to a human, never auto-built. Only ADDED + lines are inspected (removing a suppression is fine); matching is + case-insensitive on the marker text. + """ + violations: list[str] = [] + for content in _added_diff_lines(unified_diff): + low = content.lower() + snippet = content.strip()[:120] + for marker, desc in _GATE_WEAKENING_MARKERS: + if marker in low: + violations.append(f"gate-weakening: added {desc}: {snippet!r}") + if _TEST_SKIP_RE.search(content): + violations.append(f"gate-weakening: added a test skip/xfail: {snippet!r}") + return violations + + def verify_diff_hash( candidate_diff: str, *, @@ -464,6 +522,21 @@ def evaluate_ci_gate( ci_conclusion=None, ) + # (1b) Gate-weakening (§4.5): a diff that suppresses a lint/type/coverage/ + # security check or skips tests could make CI pass falsely -> BLOCK regardless + # of the authenticated conclusion. A build cannot pass itself by disabling the + # checks; these escalate to a human. + weakening = gate_weakening_violations(candidate_diff) + if weakening: + reasons.extend(weakening) + return GateResult( + decision=GateDecision.BLOCK, + reasons=reasons, + run_id=expected_run_id, + diff_hash=ledger_hash, + ci_conclusion=None, + ) + # (2) Diff-hash integrity: bind the decision to the exact bytes the ledger # recorded (and, if present, what CI verified before applying). ci_verified_hash = ( diff --git a/agent-team/tests/test_ci_gate.py b/agent-team/tests/test_ci_gate.py index 41f8e00..f6f0ec9 100644 --- a/agent-team/tests/test_ci_gate.py +++ b/agent-team/tests/test_ci_gate.py @@ -11,6 +11,7 @@ from agent_team.ci_gate import ( denylist_violations, diff_touched_paths, evaluate_ci_gate, + gate_weakening_violations, verify_diff_hash, ) from agent_team.state_store import compute_content_hash @@ -411,3 +412,71 @@ def test_gate_result_carries_provenance() -> None: assert result.run_id == "run-7" assert result.diff_hash == h assert result.reasons # always records why + + +# --------------------------------------------------------------------------- # +# §4.5 gate-weakening detector +# --------------------------------------------------------------------------- # + + +def _diff_adding(added_line: str, path: str = "src/mod.py") -> str: + """A unified diff that ADDS one line (plus an unchanged context line).""" + return ( + f"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n" + f"@@ -1 +1,2 @@\n unchanged\n+{added_line}\n" + ) + + +@pytest.mark.parametrize( + "added_line", + [ + "x = 1 # noqa", + "y = 2 # noqa: E501", + "z = bad() # type: ignore", + "z = bad() # type:ignore[arg-type]", + "def f(): # pragma: no cover", + "def f(): # pragma: no-cover", + "subprocess.run(cmd) # nosec", + "eval(x) # nosemgrep", + " git commit --no-verify", + "@pytest.mark.skip(reason='flaky')", + "@pytest.mark.xfail", + " pytest.skip('todo')", + " self.skipTest('later')", + ], +) +def test_gate_weakening_added_line_flagged(added_line: str) -> None: + violations = gate_weakening_violations(_diff_adding(added_line)) + assert violations, f"expected {added_line!r} to be flagged as gate-weakening" + + +def test_gate_weakening_clean_diff_has_no_violations() -> None: + assert gate_weakening_violations(_diff_for("src/foo.py")) == [] + + +def test_gate_weakening_only_inspects_added_lines() -> None: + # A diff that REMOVES a suppression (line starts with '-') must not be flagged. + removing = ( + "diff --git a/m.py b/m.py\n--- a/m.py\n+++ b/m.py\n" + "@@ -1,2 +1 @@\n-x = 1 # noqa\n unchanged\n" + ) + assert gate_weakening_violations(removing) == [] + + +def test_gate_weakening_rejects_non_str() -> None: + with pytest.raises(CiGateError): + gate_weakening_violations(None) # type: ignore[arg-type] + + +def test_gate_blocks_weakening_diff_even_if_ci_success() -> None: + # A diff that adds a coverage suppression BLOCKs even with a green CI run. + diff = _diff_adding("def f(): # pragma: no cover") + result = evaluate_ci_gate( + candidate_diff=diff, + ledger_hash=_ledger_hash(diff), + ci_result=_good_ci("run-w", diff, "success"), + expected_run_id="run-w", + ) + assert result.decision is GateDecision.BLOCK + assert result.blocked is True + assert any("gate-weakening" in r for r in result.reasons)