Coverage for src/ai_jury/github.py: 99%
260 statements
« prev ^ index » next coverage.py v7.16.1, created at 2026-09-30 06:29 +0000
1"""Thin GitHub helpers built on the `gh` CLI.
3Used to pull a PR diff in and to post the jury verdict back as a comment.
4Kept dependency-free; if `gh` is unavailable these raise a clear error.
5"""
7from __future__ import annotations
9import hashlib
10import json
11import re
12import shutil
13import subprocess
14import threading
16from .findings import strip_html_comments
17from .redaction import redact
19# Every `gh` invocation is bounded (#246): a stalled network call or an
20# interactive auth/2FA prompt would otherwise block `subprocess.run` forever and
21# hang the whole jury run with no per-call ceiling. On timeout we fail soft with
22# a clear, actionable error like any other gh failure.
23_GH_TIMEOUT_S = 90
25# Ceiling on `gh` stdout. A hostile/huge PR diff pulled via `--pr`/`--issue`
26# would otherwise be buffered whole by `subprocess.run`, OOMing the process
27# before the diff budget engages (security audit 2026-06-13 r3). We stream the
28# output and stop at the cap; stdout/stderr are drained on separate threads so a
29# full stderr pipe can't deadlock the stdout read.
30_GH_MAX_OUTPUT_BYTES = 64 * 1024 * 1024 # 64 MiB
32# Spawning `gh` can fail before the process exists, and the `shutil.which` guard that opens
33# each of the two callers below narrows that without closing it: the binary can be removed
34# or unmounted between the check and the spawn, and a machine already short of memory or
35# process slots refuses the fork outright (`ENOMEM`, `EAGAIN`) however present `gh` is.
36#
37# Every other failure in this module — missing CLI, timeout, non-zero exit, output over the
38# cap — leaves as a `RuntimeError`, and on the path that reads the diff `cli.py` catches
39# exactly that and prints `error: …` with exit 2 (the handler #836 added around
40# `_read_diff`). An `OSError` is not a `RuntimeError`, so it escaped that handler and
41# reached the user as a traceback. `TimeoutExpired` is the only `SubprocessError` these
42# calls raise and it is caught where it happens, so `OSError` is the whole remaining gap.
43#
44# The handler is specific to reading the diff: the post-review block that comments, posts
45# inline findings and applies labels has no such guard, so a `gh` failure there still
46# surfaces as a traceback — for every failure kind, not only this one. That is a separate
47# defect, tracked on its own rather than widened into this change.
48_GH_SPAWN_FAILED = "gh {label} could not be started: {detail}"
51def _gh(*args: str) -> str:
52 if shutil.which("gh") is None:
53 raise RuntimeError("the GitHub CLI `gh` is not installed or not on PATH")
54 label = redact(" ".join(args))[0]
55 try:
56 proc = subprocess.Popen(["gh", *args], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
57 except OSError as exc:
58 raise RuntimeError(
59 _GH_SPAWN_FAILED.format(label=label, detail=redact(str(exc))[0])
60 ) from None
61 holder: dict[str, bytes] = {}
63 def _drain(stream, key: str) -> None:
64 # read(N+1) is bounded: at most N+1 bytes, never the whole stream if it
65 # is larger.
66 holder[key] = stream.read(_GH_MAX_OUTPUT_BYTES + 1)
68 t_out = threading.Thread(target=_drain, args=(proc.stdout, "out"), daemon=True)
69 t_err = threading.Thread(target=_drain, args=(proc.stderr, "err"), daemon=True)
70 t_out.start()
71 t_err.start()
72 t_out.join(_GH_TIMEOUT_S)
73 if t_out.is_alive():
74 proc.kill()
75 raise RuntimeError(f"gh {label} timed out after {_GH_TIMEOUT_S}s")
76 out = holder.get("out", b"")
77 if len(out) > _GH_MAX_OUTPUT_BYTES:
78 proc.kill()
79 raise RuntimeError(f"gh {label} output exceeds the {_GH_MAX_OUTPUT_BYTES}-byte limit")
80 try:
81 proc.wait(timeout=_GH_TIMEOUT_S)
82 except subprocess.TimeoutExpired:
83 proc.kill()
84 raise RuntimeError(f"gh {label} timed out after {_GH_TIMEOUT_S}s") from None
85 t_err.join(_GH_TIMEOUT_S)
86 if proc.returncode != 0:
87 err = holder.get("err", b"").decode("utf-8", "replace").strip()
88 out_err = holder.get("out", b"").decode("utf-8", "replace").strip()
89 safe_err = redact(err or out_err)[0]
90 raise RuntimeError(f"gh {label} failed: {safe_err}")
91 return out.decode("utf-8", "replace")
94def pr_diff(pr: str, repo: str | None = None) -> str:
95 args = ["pr", "diff"]
96 if repo:
97 args += ["--repo", repo]
98 args += ["--", str(pr)]
99 return _gh(*args)
102def pr_context(pr: str, repo: str | None = None) -> str:
103 """Return 'title\\n\\nbody' for a PR, best-effort."""
104 args = ["pr", "view", "--json", "title,body", "--jq", '.title + "\\n\\n" + (.body // "")']
105 if repo:
106 args += ["--repo", repo]
107 args += ["--", str(pr)]
108 try:
109 return _gh(*args).strip()
110 except RuntimeError:
111 return ""
114def post_pr_comment(pr: str, body: str, repo: str | None = None) -> None:
115 args = ["pr", "comment", "--body", body]
116 if repo:
117 args += ["--repo", repo]
118 args += ["--", str(pr)]
119 _gh(*args)
122def issue_body(number: str, repo: str | None = None) -> str:
123 """Return a reviewable text rendering of a GitHub issue, best-effort.
125 Formats the issue as ``"# <title>\\n\\n_labels: a, b_\\n\\n<body>"`` so the
126 reviewer sees the title, labels, and description as one prose block. Mirrors
127 :func:`pr_context`'s error handling: any ``gh`` failure degrades to a minimal
128 string (the bare number) rather than crashing the run.
129 """
130 jq_expr = (
131 '"# " + .title + "\\n\\n_labels: " '
132 '+ ((.labels | map(.name)) | join(", ")) + "_\\n\\n" + (.body // "")'
133 )
134 args = [
135 "issue",
136 "view",
137 "--json",
138 "title,body,labels",
139 "--jq",
140 jq_expr,
141 ]
142 if repo:
143 args += ["--repo", repo]
144 args += ["--", str(number)]
145 try:
146 return _gh(*args).strip()
147 except RuntimeError:
148 return f"# issue #{number}"
151def post_issue_comment(number: str, body: str, repo: str | None = None) -> None:
152 """Post a comment on a plain GitHub issue.
154 A separate function from :func:`post_pr_comment` because ``gh pr comment``
155 only works for pull requests; ``gh issue comment`` is the issue-side command.
156 """
157 args = ["issue", "comment", "--body", body]
158 if repo:
159 args += ["--repo", repo]
160 args += ["--", str(number)]
161 _gh(*args)
164def pr_head_sha(pr: str, repo: str | None = None) -> str:
165 """Return the current head commit SHA of a PR (best-effort, '' on failure)."""
166 args = ["pr", "view", "--json", "headRefOid", "--jq", ".headRefOid"]
167 if repo:
168 args += ["--repo", repo]
169 args += ["--", str(pr)]
170 try:
171 return _gh(*args).strip()
172 except RuntimeError:
173 return ""
176def pr_comment_bodies(pr: str, repo: str | None = None) -> list[str]:
177 """Return bodies of a PR's issue comments from TRUSTED authors only.
179 Used by incremental mode (issue #9) to find the jury's prior reviewed-SHA
180 marker. The marker is security-sensitive: a forged ``arc-reviewed-sha``
181 marker would let an attacker narrow the reviewed range and skip malicious
182 commits (audit 2026-06-13 r4/M-1). So we only return comments authored by a
183 repo OWNER/MEMBER/COLLABORATOR — an external fork-PR author (CONTRIBUTOR /
184 FIRST_TIME_CONTRIBUTOR / NONE) cannot inject a trusted marker. (Run the jury
185 under such an identity for incremental mode; otherwise it safely falls back
186 to a full review.) Network errors degrade to an empty list.
187 """
188 jq = (
189 '.comments[] | select(.authorAssociation=="OWNER" or '
190 '.authorAssociation=="MEMBER" or .authorAssociation=="COLLABORATOR") | .body'
191 )
192 args = ["pr", "view", "--json", "comments", "--jq", jq]
193 if repo:
194 args += ["--repo", repo]
195 args += ["--", str(pr)]
196 try:
197 out = _gh(*args)
198 except RuntimeError:
199 return []
200 return out.splitlines()
203def compare_diff(base: str, head: str, repo: str | None = None) -> str:
204 """Return the unified diff between two SHAs via the compare API (issue #9).
206 Uses the ``application/vnd.github.v3.diff`` media type so the response is a
207 ready-to-review unified diff. Returns '' on failure so callers can fall back.
208 """
209 resolved = _resolve_repo(repo)
210 if not resolved:
211 return ""
212 try:
213 return _gh(
214 "api",
215 "-H",
216 "Accept: application/vnd.github.v3.diff",
217 "--",
218 f"repos/{resolved}/compare/{base}...{head}",
219 )
220 except RuntimeError:
221 return ""
224def build_label_args(pr: str, labels, repo: str | None = None) -> list[str]:
225 """Build the ``gh pr edit`` arg vector for applying labels (pure).
227 Returns ``[]`` when there are no labels (nothing to do). Kept pure and
228 network-free so the arg construction can be unit-tested without invoking
229 ``gh`` or hitting GitHub.
230 """
231 clean = [str(label) for label in (labels or []) if str(label).strip()]
232 if not clean:
233 return []
234 args = ["pr", "edit"]
235 for label in clean:
236 args += ["--add-label", label]
237 if repo:
238 args += ["--repo", repo]
239 args += ["--", str(pr)]
240 return args
243def apply_labels(pr: str, labels, repo: str | None = None) -> list[str]:
244 """Best-effort: apply ``labels`` to ``pr`` via ``gh pr edit --add-label``.
246 Only called when labeling is explicitly enabled (CLI ``--label``); it never
247 runs by default. No-op (returns ``[]``) when there are no labels. Returns the
248 ``gh`` arg vector that was invoked so callers can log it.
249 """
250 args = build_label_args(pr, labels, repo)
251 if not args:
252 return args
253 _gh(*args)
254 return args
257# Marker prefix identifying comments authored by the jury (enables dedup).
258INLINE_MARKER = "<!-- arc-inline -->"
260# Hidden per-finding signature marker, embedded in the comment body so that
261# re-runs can match an existing comment back to the finding that produced it.
262# Two distinct findings on the same (path, line) get distinct signatures and
263# therefore do NOT collapse into one another during dedup.
264_SIG_MARKER_RE = re.compile(r"<!-- arc-sig:([0-9a-f]+) -->")
267def _finding_signature(finding) -> str:
268 """Return a short, stable hash identifying a finding.
270 Derived from the normalized ``severity`` and ``claim`` (lowercased and
271 stripped) so the same finding yields the same signature across runs, while
272 a different severity or claim yields a different one.
273 """
274 sev = (getattr(finding, "severity", "") or "").strip().lower()
275 claim = (getattr(finding, "claim", "") or "").strip().lower()
276 raw = f"{sev}|{claim}"
277 # A deduplication key, not a security boundary — said so, for FIPS-mode Pythons and for
278 # the linters that cannot tell the two apart (#813). The digest is the same either way.
279 return hashlib.sha1(raw.encode("utf-8"), usedforsecurity=False).hexdigest()[:12]
282def _sig_marker(signature: str) -> str:
283 return f"<!-- arc-sig:{signature} -->"
286def _sig_from_body(body: str | None) -> str:
287 """Extract the embedded finding signature from a comment body ('' if none)."""
288 if not body:
289 return ""
290 match = _SIG_MARKER_RE.search(body)
291 return match.group(1) if match else ""
294def _comment_body(finding) -> str:
295 sev = getattr(finding, "severity", "info")
296 # Strip HTML comments so a finding can't forge the hidden inline markers
297 # (``<!-- arc-inline -->`` / ``<!-- arc-sig:… -->``) and perturb dedup
298 # (audit 2026-06-13 r3/N-3).
299 claim = strip_html_comments(getattr(finding, "claim", "") or "")
300 fix = strip_html_comments(getattr(finding, "suggested_fix", "") or "")
301 text = f"[{sev}] {claim}"
302 if fix:
303 text += f" — {fix}"
304 # The signature marker is hidden (HTML comment); the visible body for humans
305 # is unchanged.
306 sig = _sig_marker(_finding_signature(finding))
307 return f"{INLINE_MARKER}{sig}\n{text}"
310def _review_body(n: int) -> str:
311 """Top-level review body. GitHub's create-review API requires a non-empty
312 ``body`` when ``event`` is COMMENT (omitting it can 422) — issue #122."""
313 return f"{INLINE_MARKER}\n🏛️ AI Jury — {n} inline finding(s)."
316def build_inline_payload(findings) -> list[dict]:
317 """Build the inline review-comment array for the GitHub reviews API.
319 Pure: one comment per finding that has BOTH a file and a line. Findings
320 without a file or line are skipped (they cannot be anchored inline).
321 """
322 payload: list[dict] = []
323 for f in findings or []:
324 path = getattr(f, "file", None)
325 line = getattr(f, "line", None)
326 if not path or line is None:
327 continue
328 payload.append(
329 {
330 "path": str(path),
331 "line": int(line),
332 "side": "RIGHT",
333 "body": _comment_body(f),
334 }
335 )
336 return payload
339def _resolve_repo(repo: str | None) -> str:
340 if repo:
341 return repo
342 try:
343 out = _gh("repo", "view", "--json", "nameWithOwner")
344 return json.loads(out).get("nameWithOwner", "")
345 except (RuntimeError, json.JSONDecodeError, RecursionError):
346 return ""
349def _existing_inline_keys(pr: str, repo: str) -> set:
350 """Return ``(path, line, signature)`` keys for existing jury comments.
352 Best-effort. ``line`` falls back to ``original_line`` when GitHub reports a
353 null ``line`` (e.g. for outdated comments). ``signature`` is parsed back out
354 of the comment body so distinct findings on the same line are tracked
355 independently.
356 """
357 keys: set = set()
358 try:
359 out = _gh("api", "--paginate", "--", f"repos/{repo}/pulls/{pr}/comments")
360 data = json.loads(out)
361 except (RuntimeError, json.JSONDecodeError, RecursionError):
362 return keys
363 if not isinstance(data, list):
364 return keys
365 for c in data:
366 if not isinstance(c, dict):
367 continue
368 body = c.get("body", "") or ""
369 if INLINE_MARKER not in body:
370 continue
371 line = c.get("line")
372 if line is None:
373 line = c.get("original_line")
374 keys.add((c.get("path"), line, _sig_from_body(body)))
375 return keys
378def _gh_with_input(args: list[str], stdin_data: str) -> str:
379 if shutil.which("gh") is None:
380 raise RuntimeError("the GitHub CLI `gh` is not installed or not on PATH")
381 try:
382 proc = subprocess.run(
383 ["gh", *args],
384 input=stdin_data,
385 capture_output=True,
386 text=True,
387 timeout=_GH_TIMEOUT_S,
388 )
389 except subprocess.TimeoutExpired:
390 raise RuntimeError(
391 f"gh {redact(' '.join(args))[0]} timed out after {_GH_TIMEOUT_S}s"
392 ) from None
393 except OSError as exc:
394 raise RuntimeError(
395 _GH_SPAWN_FAILED.format(label=redact(" ".join(args))[0], detail=redact(str(exc))[0])
396 ) from None
397 if proc.returncode != 0:
398 err = proc.stderr.strip()
399 out_err = proc.stdout.strip()
400 safe_err = redact(err or out_err)[0]
401 raise RuntimeError(f"gh {redact(' '.join(args))[0]} failed: {safe_err}")
402 return proc.stdout
405def post_inline_comments(
406 pr: str,
407 findings,
408 repo: str | None = None,
409 dry_run: bool = False,
410) -> dict:
411 """Post inline review comments as a single PR review.
413 Best-effort dedup: skips comments whose ``(path, line, finding-signature)``
414 already has a jury inline comment. Keying on the signature means two
415 distinct findings on the same line are both posted. When ``dry_run`` is True
416 the payload is printed and returned without any network call. Returns the
417 review payload (would-be) posted.
418 """
419 comments = build_inline_payload(findings)
421 if dry_run:
422 payload = {"event": "COMMENT", "body": _review_body(len(comments)), "comments": comments}
423 print(redact(json.dumps(payload, indent=2))[0])
424 return payload
426 resolved = _resolve_repo(repo)
427 existing = _existing_inline_keys(pr, resolved) if resolved else set()
428 deduped = [
429 c for c in comments if (c["path"], c["line"], _sig_from_body(c["body"])) not in existing
430 ]
432 payload = {"event": "COMMENT", "body": _review_body(len(deduped)), "comments": deduped}
433 if not deduped:
434 return payload
436 _gh_with_input(
437 ["api", "--method", "POST", "--input", "-", "--", f"repos/{resolved}/pulls/{pr}/reviews"],
438 json.dumps(payload),
439 )
440 return payload
443# Hidden marker identifying the jury's single sticky progress comment (issue #125).
444PROGRESS_MARKER = "<!-- arc-progress -->"
447def render_progress_body(stages: list[str], *, done: bool = False, final: str | None = None) -> str:
448 """Render the sticky progress-comment body (pure, issue #125).
450 ``stages`` is the ordered list of milestones reached. When ``done`` and a
451 ``final`` report is given, the comment becomes the verdict (with the marker
452 kept so the same comment is reused on a re-run).
453 """
454 if done and final is not None:
455 return f"{PROGRESS_MARKER}\n{final}"
456 header = "🏛️ **AI Jury** — review complete." if done else "🏛️ **AI Jury** — review in progress…"
457 lines = [PROGRESS_MARKER, header, ""]
458 for s in stages:
459 lines.append(f"- {s}")
460 if not done:
461 lines.append("\n_Updating live; the verdict will replace this when done._")
462 return "\n".join(lines)
465def _create_issue_comment(pr: str, body: str, repo: str) -> int | None:
466 """Create a PR/issue comment, returning its numeric id (or None)."""
467 try:
468 out = _gh_with_input(
469 ["api", "--method", "POST", "--input", "-", "--", f"repos/{repo}/issues/{pr}/comments"],
470 json.dumps({"body": body}),
471 )
472 return json.loads(out).get("id")
473 except (RuntimeError, json.JSONDecodeError, RecursionError):
474 return None
477def _edit_issue_comment(comment_id: int, body: str, repo: str) -> bool:
478 try:
479 _gh_with_input(
480 [
481 "api",
482 "--method",
483 "PATCH",
484 "--input",
485 "-",
486 "--",
487 f"repos/{repo}/issues/comments/{comment_id}",
488 ],
489 json.dumps({"body": body}),
490 )
491 return True
492 except RuntimeError:
493 return False
496class ProgressReporter:
497 """Maintains ONE sticky PR comment, updated as the run advances (issue #125).
499 Best-effort and resilient: a resolve/create/edit failure is swallowed so a
500 GitHub hiccup never crashes the review. The first ``update`` creates the
501 comment; subsequent updates edit it in place; ``finish`` turns it into the
502 final verdict.
503 """
505 def __init__(self, pr: str, repo: str | None = None):
506 self.pr = str(pr)
507 self.repo = _resolve_repo(repo)
508 self.comment_id: int | None = None
509 self.stages: list[str] = []
511 def _push(self, body: str) -> None:
512 if not self.repo:
513 return
514 if self.comment_id is None:
515 self.comment_id = _create_issue_comment(self.pr, body, self.repo)
516 else:
517 _edit_issue_comment(self.comment_id, body, self.repo)
519 def update(self, milestone: str) -> None:
520 self.stages.append(milestone)
521 self._push(render_progress_body(self.stages, done=False))
523 def finish(self, final_report: str) -> None:
524 self._push(render_progress_body(self.stages, done=True, final=final_report))