Coverage for src/keel/runner.py: 100%
70 statements
« prev ^ index » next coverage.py v7.16.2, created at 2026-10-02 20:26 +0000
1"""Thin I/O: execute shell-command gates (build / lint / command extensions).
3This is the only place keel shells out for gates. It is deliberately thin and
4**fail-soft**: a timeout or a missing binary becomes a failed :class:`CommandResult`
5rather than an exception. The subprocess call is injectable (``_run``) so the gate
6runner is fully unit-testable offline; agentic gates are dispatched elsewhere.
7"""
9from __future__ import annotations
11import re
12import subprocess # nosec B404
13from dataclasses import dataclass
14from typing import TYPE_CHECKING
16from .findings import Finding
17from .model import DEFAULT_GATE_TIMEOUT_S
19if TYPE_CHECKING: # pragma: no cover
20 from .gates import GateSpec
22# Re-exported, not re-declared. #876 asked for the two aliases to agree and #896
23# achieved that by copying the definition here byte for byte — which reverting
24# left the whole suite green, because nothing compared them (#931). One
25# definition cannot drift from itself.
26#
27# `gates` is the lower-level module: it owns `GateSpec`, and it imports nothing
28# from here, so this edge is one-way and creates no cycle.
29# `command_unset` and `unconfigured_finding` ride the same import and are used here,
30# not re-exported.
31from .gates import GateRunner, command_unset, unconfigured_finding # noqa: E402
33__all__ = ["GateRunner", "CommandResult", "run_argv", "command_gate_runner"]
35_ON_FAIL_SEVERITY = {"block": "major", "suggest": "minor", "warn": "nit"}
37#: reviewdog-style errorformat: ``path:line[:col]: message`` (first hit wins).
38#: A single multiline ``search`` replaces a per-``splitlines`` loop: ``^`` is
39#: anchored to each line by ``re.MULTILINE``, the path classes exclude ``\n`` so a
40#: match can never span lines, and the trailing ``(?:[:\s]|$)`` accepts the line
41#: number at end-of-line.
42_LOCATION_RE = re.compile(
43 r"^[ \t]*(?P<path>[^\s\n:][^:\n]*?):(?P<line>\d+)(?::\d+)?(?:[:\s]|$)",
44 re.MULTILINE,
45)
48def first_location(text: str) -> tuple[str | None, int | None]:
49 """Extract the first ``path:line`` location from tool output (``(None, None)`` if none)."""
50 m = _LOCATION_RE.search(text)
51 return (m.group("path"), int(m.group("line"))) if m else (None, None)
54@dataclass(frozen=True)
55class CommandResult:
56 ok: bool
57 code: int
58 #: ``stdout + stderr``, concatenated. Kept for the diagnostic uses that genuinely
59 #: want both (a failing gate's message, an output tail). **Do not parse structured
60 #: data out of this** — a command that writes progress or warnings to stderr while
61 #: exiting 0 (git's ``warning: refname … is ambiguous``, ai-jury's ``[jury] …``
62 #: logs) leaves the real payload glued to noise. Parse :attr:`stdout` instead.
63 output: str
64 #: True when the wall-clock timeout killed the command (exit 124). A timeout is
65 #: still a failure — ``ok`` stays False — but it carries no pass/fail verdict, so
66 #: callers can label it distinctly instead of reporting it as a broken test.
67 timed_out: bool = False
68 #: Captured standard output alone. This is what parsers must read: a tool's
69 #: machine-readable result goes here, never contaminated by stderr diagnostics.
70 stdout: str = ""
71 #: Captured standard error alone.
72 stderr: str = ""
73 #: True when the command could **not be started** — the ``OSError`` path, which for a
74 #: delegate almost always means "that binary is not installed". Distinct from exit
75 #: 127, which a command that *did* run can also return: `run_argv` reports both as
76 #: code 127, so classifying on the code alone cannot tell "no such binary" from "the
77 #: tool ran and said 127". Every caller that needs the difference reads this flag.
78 spawn_failed: bool = False
81def _result(proc) -> CommandResult:
82 out = _decoded(proc.stdout)
83 err = _decoded(proc.stderr)
84 return CommandResult(proc.returncode == 0, proc.returncode, out + err, stdout=out, stderr=err)
87def run_command(
88 cmd: str,
89 *,
90 cwd: str | None = None,
91 timeout: int = DEFAULT_GATE_TIMEOUT_S,
92 env: dict[str, str] | None = None,
93 _run=subprocess.run,
94) -> CommandResult:
95 """Run ``cmd`` in a shell, capturing output. Fail-soft on timeout/OS error.
97 ``env`` replaces the child's environment when given (``None`` inherits it).
98 """
99 try:
100 # Intentional shell boundary: cmd must come only from operator-controlled
101 # project config or extension YAML, never from PR content or agent output.
102 proc = _run(
103 cmd,
104 shell=True,
105 cwd=cwd,
106 capture_output=True,
107 text=True,
108 # **UTF-8, not the platform default.** `text=True` alone decodes with
109 # `locale.getencoding()`, which on Windows is the ANSI code page: cp1252
110 # leaves 0x81/8D/8F/90/9D undefined, so `git ls-tree -z` on a repository
111 # holding a Cyrillic filename (`Ё` is D0 81) raised UnicodeDecodeError out
112 # of the subprocess call — past `run_argv`'s own `TimeoutExpired`/`OSError`
113 # guards, turning every fail-soft reader into a traceback. `surrogateescape`
114 # also round-trips undecodable bytes back out unchanged, which the landing
115 # needs: the names it reads from `ls-tree` are written straight back to
116 # `mktree`.
117 encoding="utf-8",
118 errors="surrogateescape",
119 timeout=timeout,
120 stdin=subprocess.DEVNULL,
121 env=env,
122 ) # nosec B604
123 except subprocess.TimeoutExpired:
124 return CommandResult(False, 124, f"timed out after {timeout}s", timed_out=True)
125 except OSError as exc:
126 return CommandResult(False, 127, str(exc), stderr=str(exc), spawn_failed=True)
127 return _result(proc)
130def run_argv(
131 argv: list[str],
132 *,
133 cwd: str | None = None,
134 timeout: int = 120,
135 stdin_text: str | None = None,
136 env: dict[str, str] | None = None,
137 keep_line_endings: bool = False,
138 _run=subprocess.run,
139) -> CommandResult:
140 """Run an argv list (no shell). Fail-soft on timeout/OS error. Used by git/gh wrappers.
142 ``keep_line_endings`` reads the output as bytes and decodes it here, the same way
143 (UTF-8, ``surrogateescape``), because text mode's universal newlines turn every
144 ``\\r\\n`` into ``\\n``: a diff of a CRLF file then no longer matches its own lines.
146 ``env`` replaces the child's environment when given (``None`` inherits it) — for a
147 caller that must keep a variable such as ``GIT_DIFF_OPTS`` away from the child.
149 ``stdin_text`` feeds the child on standard input instead of closing it. Every delegate
150 CLI keel dispatches to takes its prompt that way (:mod:`keel.delegate`): a prompt
151 carries the diff and the brief, and an argv is world-readable in ``ps`` for the life of
152 the process. The default stays ``DEVNULL`` — a gate left waiting for input in an
153 unattended run is a hang, not a prompt.
154 """
155 try:
156 proc = _run(
157 argv,
158 cwd=cwd,
159 capture_output=True,
160 text=not keep_line_endings,
161 # **UTF-8, not the platform default.** `text=True` alone decodes with
162 # `locale.getencoding()`, which on Windows is the ANSI code page: cp1252
163 # leaves 0x81/8D/8F/90/9D undefined, so `git ls-tree -z` on a repository
164 # holding a Cyrillic filename (`Ё` is D0 81) raised UnicodeDecodeError out
165 # of the subprocess call — past `run_argv`'s own `TimeoutExpired`/`OSError`
166 # guards, turning every fail-soft reader into a traceback. `surrogateescape`
167 # also round-trips undecodable bytes back out unchanged, which the landing
168 # needs: the names it reads from `ls-tree` are written straight back to
169 # `mktree`.
170 encoding=None if keep_line_endings else "utf-8",
171 errors=None if keep_line_endings else "surrogateescape",
172 timeout=timeout,
173 env=env,
174 input=(
175 stdin_text.encode("utf-8")
176 if keep_line_endings and stdin_text is not None
177 else stdin_text
178 ),
179 # Written out rather than assembled into a **kwargs dict: #879's sweep in
180 # tests/test_missing_pins.py reads every spawn site's keywords out of the
181 # AST, and a site that hides `stdin` behind a splat is a site the rule
182 # cannot see. `subprocess.run` accepts `stdin=None` beside `input` and
183 # substitutes a PIPE itself.
184 stdin=None if stdin_text is not None else subprocess.DEVNULL,
185 )
186 except subprocess.TimeoutExpired:
187 return CommandResult(False, 124, f"timed out after {timeout}s", timed_out=True)
188 except OSError as exc:
189 return CommandResult(False, 127, str(exc), stderr=str(exc), spawn_failed=True)
190 return _result(proc)
193def _decoded(data: bytes | str | None) -> str:
194 """Captured output as text: bytes (``keep_line_endings``) are decoded as text mode
195 would, minus its newline translation."""
196 if isinstance(data, bytes):
197 return data.decode("utf-8", "surrogateescape")
198 return data or ""
201def _tail(text: str, n: int = 20) -> str:
202 return "\n".join(text.strip().splitlines()[-n:])
205def command_gate_runner(
206 repo_root: str | None = None,
207 *,
208 timeout: int = DEFAULT_GATE_TIMEOUT_S,
209 _run=subprocess.run,
210) -> GateRunner:
211 """A :data:`keel.gates.GateRunner` that executes ``command`` gates via the shell.
213 Non-command gates (agentic / builtin like ``jury``) are not executed here — in
214 command-only mode they pass as no-ops; the agent-dispatch layer runs those.
216 ``timeout`` is the fallback wall-clock limit for a gate that carries none of its
217 own; a :attr:`~keel.gates.GateSpec.timeout` resolved by
218 :func:`~keel.gates.plan_gates` always wins. A gate killed by that limit is
219 reported as a **timeout** rather than a failure: it still blocks (``ok`` is
220 False and the severity is unchanged), but the message says the command never
221 produced a verdict instead of implying a test broke.
222 """
224 def runner(spec: GateSpec) -> tuple[bool, list[Finding], bool, bool]:
225 if spec.kind != "command":
226 # Not executed here — the agent-dispatch layer runs agentic gates. Flagged
227 # `not_run` so this can never be recorded as "ran and passed"; `ok` stays
228 # True so a soft gate does not spuriously fail a command-only run.
229 return True, [], False, True
230 if command_unset(spec):
231 # A command gate with nothing to run (an unset `knobs.build_gate_cmd`, #1328,
232 # or a blank one, #1364 — `sh -c ' '` exits 0) is ours and it fails, naming
233 # the knob. "not_run" would read as "record a result for this gate" rather
234 # than "configure it".
235 return False, [unconfigured_finding(spec)], False, False
236 limit = timeout if spec.timeout is None else spec.timeout
237 result = run_command(spec.run, cwd=repo_root, timeout=limit, _run=_run)
238 if result.ok:
239 return True, [], False, False
240 severity = _ON_FAIL_SEVERITY[spec.on_fail]
241 if result.timed_out:
242 # No pass/fail verdict exists — do not dress the kill up as a test result.
243 message = (
244 f"{spec.id} timed out after {limit}s (exit {result.code}); "
245 "the command produced no pass/fail result. Raise the limit via "
246 "knobs.gate_timeout_s (or this gate's timeout:) if it legitimately "
247 "needs longer — a genuinely hanging command is still a defect."
248 )
249 return False, [Finding(severity, message, spec.id)], True, False
250 message = f"{spec.id} failed (exit {result.code})"
251 tail = _tail(result.output)
252 if tail:
253 message += f": {tail}"
254 path, line = first_location(result.output)
255 return (
256 False,
257 [
258 Finding(
259 severity,
260 message,
261 spec.id,
262 path=path,
263 line=line,
264 anchorable=path is not None and line is not None,
265 )
266 ],
267 False,
268 False,
269 )
271 return runner