|
| 1 | +# -*- coding: utf-8 -*- |
| 2 | +"""Hard acceptance gate for autonomous (oneshot) runs. |
| 3 | +
|
| 4 | +The system prompt already asks the model to verify its work before reporting |
| 5 | +completion, and the model already ignores that. Aider's ``--auto-test`` and |
| 6 | +Agentless' three-stage filter make verification a property of the loop rather |
| 7 | +than a request: a non-zero exit means the answer is not accepted yet. |
| 8 | +OpenHands shipping the advice without the gate is the counterexample. |
| 9 | +
|
| 10 | +Only oneshot runs are gated. Interactively the user *is* the acceptance |
| 11 | +criterion and is reading the output, so spending minutes on a test suite |
| 12 | +before every reply costs more than the near-miss it prevents. |
| 13 | +""" |
| 14 | + |
| 15 | +from __future__ import annotations |
| 16 | + |
| 17 | +import asyncio |
| 18 | +import os |
| 19 | +import shlex |
| 20 | +import sys |
| 21 | +from dataclasses import dataclass |
| 22 | +from pathlib import Path |
| 23 | + |
| 24 | +from dashscope.acli.utils.text import markdown_fence, truncate_head_tail |
| 25 | + |
| 26 | +# Quoted verbatim into the next prompt, so: large enough to contain the |
| 27 | +# failing test id and its assertion, small enough to read. Tail-biased, |
| 28 | +# because that is where pytest and cargo put the verdict. |
| 29 | +_OUTPUT_CHARS = 3000 |
| 30 | + |
| 31 | +DEFAULT_RETRIES = 1 |
| 32 | +DEFAULT_TIMEOUT = 600 |
| 33 | + |
| 34 | +EXPLICIT = "explicit" |
| 35 | +DISCOVERED = "discovered" |
| 36 | + |
| 37 | +# Exit statuses meaning the check never reached the code under test, so its |
| 38 | +# output describes the environment rather than the change. 127 is the shell's |
| 39 | +# command-not-found; 2/3/4/5 are pytest's interrupted (also what a collection |
| 40 | +# error such as a missing dependency reports), internal-error, usage-error and |
| 41 | +# nothing-collected. pytest's 1 is deliberately absent: tests ran and one |
| 42 | +# failed, which is the verdict this whole module exists to act on. |
| 43 | +_NOT_FOUND = 127 |
| 44 | +_PYTEST_NEVER_RAN = frozenset({2, 3, 4, 5}) |
| 45 | + |
| 46 | + |
| 47 | +def _never_ran(run: AcceptanceRun) -> bool: |
| 48 | + """True when ``run``'s exit status shows the check could not execute.""" |
| 49 | + if run.exit_code == _NOT_FOUND: |
| 50 | + return True |
| 51 | + return "pytest" in run.command and run.exit_code in _PYTEST_NEVER_RAN |
| 52 | + |
| 53 | + |
| 54 | +def _env_int(name: str, default: int) -> int: |
| 55 | + try: |
| 56 | + return int(os.environ.get(name, "") or default) |
| 57 | + except ValueError: |
| 58 | + return default |
| 59 | + |
| 60 | + |
| 61 | +def _env_flag(name: str, default: bool) -> bool: |
| 62 | + raw = os.environ.get(name) |
| 63 | + if raw is None or not raw.strip(): |
| 64 | + return default |
| 65 | + return raw.strip().lower() not in ("0", "false", "no", "off") |
| 66 | + |
| 67 | + |
| 68 | +def _read_head(path: Path, limit: int = 65536) -> str: |
| 69 | + try: |
| 70 | + with path.open("r", encoding="utf-8", errors="replace") as fh: |
| 71 | + return fh.read(limit) |
| 72 | + except OSError: |
| 73 | + return "" |
| 74 | + |
| 75 | + |
| 76 | +def _is_pytest_project(root: Path) -> bool: |
| 77 | + if (root / "pytest.ini").is_file(): |
| 78 | + return True |
| 79 | + for name, marker in ( |
| 80 | + ("pyproject.toml", "[tool.pytest"), |
| 81 | + ("setup.cfg", "[tool:pytest]"), |
| 82 | + ("tox.ini", "[pytest]"), |
| 83 | + ): |
| 84 | + path = root / name |
| 85 | + if path.is_file() and marker in _read_head(path): |
| 86 | + return True |
| 87 | + # Most Python repos never declare pytest config at all. A tests/ directory |
| 88 | + # beside a project file is pytest's default layout, and is what bare |
| 89 | + # `pytest` collects. |
| 90 | + return (root / "tests").is_dir() and (root / "pyproject.toml").is_file() |
| 91 | + |
| 92 | + |
| 93 | +def discover_acceptance_command( |
| 94 | + cwd: str | os.PathLike[str] | None = None, |
| 95 | +) -> str | None: |
| 96 | + """The conventional test entry point for the project at ``cwd``, if any. |
| 97 | +
|
| 98 | + The allowlist is deliberately short. Every entry runs code from the |
| 99 | + repository — ``conftest.py`` at collection time, ``build.rs``, ``TestMain`` |
| 100 | + — so "conservative" here means "a standard test entry point for an |
| 101 | + ecosystem whose manifest is sitting right here", not "harmless". |
| 102 | + ``npm test`` and ``make test`` are excluded because their bodies are |
| 103 | + arbitrary user scripts with no conventional meaning; declaring them via |
| 104 | + ``ACLI_ACCEPTANCE_CMD`` is how you ask for those. |
| 105 | + """ |
| 106 | + root = Path(cwd or os.getcwd()) |
| 107 | + if not root.is_dir(): |
| 108 | + return None |
| 109 | + if _is_pytest_project(root): |
| 110 | + # This interpreter, not a bare `pytest`: the gate must run against the |
| 111 | + # same environment the agent's own run_command calls use. |
| 112 | + return f"{shlex.quote(sys.executable)} -m pytest -q" |
| 113 | + if (root / "Cargo.toml").is_file(): |
| 114 | + return "cargo test --quiet" |
| 115 | + if (root / "go.mod").is_file(): |
| 116 | + return "go test ./..." |
| 117 | + return None |
| 118 | + |
| 119 | + |
| 120 | +@dataclass(frozen=True) |
| 121 | +class AcceptanceRun: |
| 122 | + """One execution of the acceptance check.""" |
| 123 | + |
| 124 | + command: str |
| 125 | + source: str |
| 126 | + ok: bool |
| 127 | + exit_code: int | None |
| 128 | + output: str |
| 129 | + |
| 130 | + |
| 131 | +@dataclass(frozen=True) |
| 132 | +class GateVerdict: |
| 133 | + """What the loop should do about a check that did not pass. |
| 134 | +
|
| 135 | + At most one of ``rejection`` and ``note`` is non-empty. ``rejection`` |
| 136 | + withholds the answer and sends the model back with the raw output; |
| 137 | + ``note`` is appended to the final answer when sending it back would not |
| 138 | + help. |
| 139 | + """ |
| 140 | + |
| 141 | + run: AcceptanceRun |
| 142 | + rejection: str = "" |
| 143 | + note: str = "" |
| 144 | + |
| 145 | + |
| 146 | +class AcceptanceGate: |
| 147 | + """Runs the acceptance check and decides whether the answer stands. |
| 148 | +
|
| 149 | + Two runs per turn at most: a baseline captured before the first mutation, |
| 150 | + and the check at the loop's exit. The baseline is what turns "the suite is |
| 151 | + red" into "you made it red" — Agentless reproduces the issue on the |
| 152 | + unpatched repository for the same reason, and AlphaCodium's test anchors |
| 153 | + are the same obligation stated positively. |
| 154 | + """ |
| 155 | + |
| 156 | + def __init__( |
| 157 | + self, |
| 158 | + command: str = "", |
| 159 | + source: str = EXPLICIT, |
| 160 | + retries: int = DEFAULT_RETRIES, |
| 161 | + timeout: int = DEFAULT_TIMEOUT, |
| 162 | + ): |
| 163 | + self.command = command |
| 164 | + self.source = source |
| 165 | + self.retries = retries |
| 166 | + self.timeout = timeout |
| 167 | + self.baseline: AcceptanceRun | None = None |
| 168 | + self.attempts = 0 |
| 169 | + self._baseline_task: asyncio.Task | None = None |
| 170 | + |
| 171 | + @classmethod |
| 172 | + def from_env(cls, oneshot: bool) -> AcceptanceGate: |
| 173 | + """Build the gate from the ``ACLI_ACCEPTANCE*`` environment variables. |
| 174 | +
|
| 175 | + An explicit command wins over discovery. Setting ``ACLI_ACCEPTANCE=0`` |
| 176 | + turns the gate off, which is what a benchmark measuring raw model |
| 177 | + output wants. |
| 178 | + """ |
| 179 | + if not (oneshot and _env_flag("ACLI_ACCEPTANCE", True)): |
| 180 | + return cls() |
| 181 | + command = os.environ.get("ACLI_ACCEPTANCE_CMD", "").strip() |
| 182 | + source = EXPLICIT |
| 183 | + if not command: |
| 184 | + command = discover_acceptance_command() or "" |
| 185 | + source = DISCOVERED |
| 186 | + return cls( |
| 187 | + command=command, |
| 188 | + source=source, |
| 189 | + retries=_env_int("ACLI_ACCEPTANCE_RETRIES", DEFAULT_RETRIES), |
| 190 | + timeout=_env_int("ACLI_ACCEPTANCE_TIMEOUT", DEFAULT_TIMEOUT), |
| 191 | + ) |
| 192 | + |
| 193 | + @property |
| 194 | + def active(self) -> bool: |
| 195 | + """An empty command is the off switch.""" |
| 196 | + return bool(self.command) |
| 197 | + |
| 198 | + def reset(self) -> None: |
| 199 | + """Drop the previous turn's anchor and retry budget. |
| 200 | +
|
| 201 | + The baseline has to come from the workspace as *this* turn found it, |
| 202 | + and the retry budget is per-turn so one slow turn cannot spend the |
| 203 | + next one's. An in-flight capture is cancelled for the same reason: |
| 204 | + left running, it would write the previous turn's verdict into this |
| 205 | + turn's ``baseline``. |
| 206 | + """ |
| 207 | + if self._baseline_task is not None and not self._baseline_task.done(): |
| 208 | + self._baseline_task.cancel() |
| 209 | + self._baseline_task = None |
| 210 | + self.baseline = None |
| 211 | + self.attempts = 0 |
| 212 | + |
| 213 | + async def ensure_baseline(self) -> None: |
| 214 | + """Capture the pre-change verdict, once per turn. |
| 215 | +
|
| 216 | + Called before the first mutating tool call rather than at turn start: |
| 217 | + a read-only or pure-answer turn never pays for a test suite, and the |
| 218 | + baseline still comes from a workspace the agent has not touched. |
| 219 | +
|
| 220 | + Concurrent callers share one task rather than checking a flag. The |
| 221 | + flag form lets the second mutating call in a parallel batch proceed |
| 222 | + while the baseline is still running, so the "pre-change" verdict would |
| 223 | + be captured against a workspace that had already changed. |
| 224 | + """ |
| 225 | + if not self.active: |
| 226 | + return |
| 227 | + if self._baseline_task is None: |
| 228 | + self._baseline_task = asyncio.ensure_future(self._capture()) |
| 229 | + await self._baseline_task |
| 230 | + |
| 231 | + async def _capture(self) -> None: |
| 232 | + self.baseline = await self._run() |
| 233 | + |
| 234 | + async def check(self) -> GateVerdict | None: |
| 235 | + """Run the check at the loop's exit and rule on the answer. |
| 236 | +
|
| 237 | + ``None`` means "not applicable" — gate off, no command found, or |
| 238 | + nothing was mutated this turn, in which case there is no work to |
| 239 | + accept. |
| 240 | + """ |
| 241 | + if not self.active or self.baseline is None: |
| 242 | + return None |
| 243 | + run = await self._run() |
| 244 | + self.attempts += 1 |
| 245 | + if run.ok: |
| 246 | + return GateVerdict(run) |
| 247 | + if _never_ran(run) and self._baseline_never_ran(): |
| 248 | + return GateVerdict(run, note=self._unrunnable_note(run)) |
| 249 | + if self._unchanged_since_baseline(run): |
| 250 | + return GateVerdict(run, note=self._insensitive_note(run)) |
| 251 | + if self.attempts > self.retries: |
| 252 | + return GateVerdict(run, note=self._unverified_note(run)) |
| 253 | + return GateVerdict(run, rejection=self._rejection(run)) |
| 254 | + |
| 255 | + def _baseline_never_ran(self) -> bool: |
| 256 | + """True when the pre-change check could not execute either. |
| 257 | +
|
| 258 | + This is what separates a broken environment from a broken change. The |
| 259 | + baseline is captured before the agent touches anything, so a check that |
| 260 | + already could not run then was not made unrunnable by the change. |
| 261 | + Requiring it also keeps the sharp edge: a *green* baseline proves the |
| 262 | + check does run here, so a collection error the agent introduced stays |
| 263 | + on the rejection path instead of being excused. |
| 264 | + """ |
| 265 | + base = self.baseline |
| 266 | + return bool(base and not base.ok and _never_ran(base)) |
| 267 | + |
| 268 | + def _unchanged_since_baseline(self, run: AcceptanceRun) -> bool: |
| 269 | + """True when the check came out byte-identical to a failing baseline. |
| 270 | +
|
| 271 | + The catch-all behind ``_baseline_never_ran``, for a check whose exit |
| 272 | + statuses this module does not know — cargo, go, an immediate timeout — |
| 273 | + where reproducing the baseline exactly is the only evidence available |
| 274 | + that the break is environmental. Sending the model back to fix that |
| 275 | + burns the turn on a problem it neither created nor can solve, so the |
| 276 | + gate reports it instead of rejecting. |
| 277 | +
|
| 278 | + Byte-identical is the point: a suite that genuinely fails prints |
| 279 | + timings, so real failures differ between runs and still get rejected. |
| 280 | + Only a stable, environment-level break reproduces exactly. |
| 281 | + """ |
| 282 | + base = self.baseline |
| 283 | + return bool( |
| 284 | + base |
| 285 | + and not base.ok |
| 286 | + and base.exit_code == run.exit_code |
| 287 | + and base.output == run.output, |
| 288 | + ) |
| 289 | + |
| 290 | + async def _run(self) -> AcceptanceRun: |
| 291 | + # run_command rather than a raw subprocess: it carries the |
| 292 | + # blocked-pattern checks, the optional OS sandbox, the output caps, and |
| 293 | + # the exit-code trailer that the rest of the loop already classifies. |
| 294 | + from dashscope.acli.audit import get_audit_logger |
| 295 | + from dashscope.acli.executor import classify_tool_text |
| 296 | + from dashscope.acli.tools.shell import run_command |
| 297 | + |
| 298 | + get_audit_logger().log_tool_call( |
| 299 | + "acceptance_gate", |
| 300 | + {"command": self.command}, |
| 301 | + decision="allowed", |
| 302 | + reason=f"source: {self.source}", |
| 303 | + ) |
| 304 | + text = await run_command(command=self.command, timeout=self.timeout) |
| 305 | + outcome = classify_tool_text(text, {"command": self.command}) |
| 306 | + return AcceptanceRun( |
| 307 | + command=self.command, |
| 308 | + source=self.source, |
| 309 | + ok=outcome.ok, |
| 310 | + exit_code=outcome.exit_code, |
| 311 | + output=outcome.text, |
| 312 | + ) |
| 313 | + |
| 314 | + def _rejection(self, run: AcceptanceRun) -> str: |
| 315 | + quoted = truncate_head_tail(run.output, _OUTPUT_CHARS, ratio=0.25) |
| 316 | + fence = markdown_fence(quoted) |
| 317 | + if self.baseline and self.baseline.ok: |
| 318 | + verdict = ( |
| 319 | + "a REGRESSION: it passed before anything in this turn " |
| 320 | + "changed, so the change broke it" |
| 321 | + ) |
| 322 | + else: |
| 323 | + verdict = "still failing" |
| 324 | + return ( |
| 325 | + "\n## ⛔ Not accepted: the verification check is " |
| 326 | + f"{verdict}\n" |
| 327 | + f"`{run.command}` exited {run.exit_code}.\n" |
| 328 | + f"{fence}\n{quoted}\n{fence}\n" |
| 329 | + "Your answer is being withheld, not discarded: fix what this " |
| 330 | + "output shows and the turn ends by itself. Work from the output " |
| 331 | + "above — do not restate your plan, and do not declare the check " |
| 332 | + "wrong unless this output says so.\n" |
| 333 | + ) |
| 334 | + |
| 335 | + def _unverified_note(self, run: AcceptanceRun) -> str: |
| 336 | + return ( |
| 337 | + f"\n[Unverified: `{run.command}` exited {run.exit_code} and the " |
| 338 | + f"retry budget ({self.retries}) is spent, so the answer below is " |
| 339 | + "reported as-is rather than withheld.]\n" |
| 340 | + ) |
| 341 | + |
| 342 | + def _unrunnable_note(self, run: AcceptanceRun) -> str: |
| 343 | + return ( |
| 344 | + f"\n[Unverified: `{run.command}` exited {run.exit_code} before " |
| 345 | + "and after these changes, a status that means it never reached " |
| 346 | + "the tests. The check does not run in this environment, so it " |
| 347 | + "says nothing about the answer below.]\n" |
| 348 | + ) |
| 349 | + |
| 350 | + def _insensitive_note(self, run: AcceptanceRun) -> str: |
| 351 | + return ( |
| 352 | + f"\n[Unverified: `{run.command}` failed identically before and " |
| 353 | + "after these changes, so it says nothing about them — most likely " |
| 354 | + "the check itself does not run in this environment.]\n" |
| 355 | + ) |
0 commit comments