From 2560f194eec7afda8395f52e57778a21f3d7c698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 18 Jun 2026 19:57:40 +0100 Subject: [PATCH] ci: harden opencode pr review workflow (#1434) --- .github/workflows/opencode-code-review.yml | 353 +++++--------------- scripts/opencode-code-review | 371 +++++++++++++++++++++ scripts/opencode-code-review.test.py | 238 +++++++++++++ scripts/opencode-code-review.test.sh | 48 ++- 4 files changed, 733 insertions(+), 277 deletions(-) create mode 100755 scripts/opencode-code-review create mode 100755 scripts/opencode-code-review.test.py diff --git a/.github/workflows/opencode-code-review.yml b/.github/workflows/opencode-code-review.yml index 5bd73b15d7..4afcfc560f 100644 --- a/.github/workflows/opencode-code-review.yml +++ b/.github/workflows/opencode-code-review.yml @@ -51,6 +51,10 @@ jobs: run: | set -euo pipefail + REVIEW_DIR=.opencode-review + rm -rf "$REVIEW_DIR" + mkdir -p "$REVIEW_DIR" + if [[ "$ACTION" == "synchronize" ]] && [[ -n "${BEFORE:-}" ]] && [[ "$BEFORE" != "0000000000000000000000000000000000000000" ]] && @@ -62,14 +66,14 @@ jobs: MODE="full-pr" fi - git diff --find-renames --find-copies --unified=80 "$RANGE" > /tmp/opencode-review.patch - git diff --name-only "$RANGE" > /tmp/opencode-review-files.txt + git diff --find-renames --find-copies --unified=80 "$RANGE" > "$REVIEW_DIR/review.patch" + git diff --name-only "$RANGE" > "$REVIEW_DIR/files.txt" { echo "Review mode: $MODE" echo "Diff range: $RANGE" echo echo "Changed files:" - sed 's/^/- /' /tmp/opencode-review-files.txt + sed 's/^/- /' "$REVIEW_DIR/files.txt" echo echo "Repository review guidelines from base commit:" echo @@ -78,7 +82,7 @@ jobs: echo "Code review skill from base commit:" echo git show "$BASE_SHA:.agents/skills/code-review/SKILL.md" 2>/dev/null | sed -n '1,220p' || echo "(code-review skill not present in base commit)" - } > /tmp/opencode-review-guidelines.md + } > "$REVIEW_DIR/guidelines.md" { echo "mode=$MODE" @@ -110,6 +114,9 @@ jobs: run: | set -euo pipefail + REVIEW_DIR=.opencode-review + mkdir -p "$REVIEW_DIR" + rm -rf .opencode mkdir -p .opencode/agents cat > .opencode/agents/github-pr-review.md <<'EOF' --- @@ -155,15 +162,28 @@ jobs: - Use an empty array when there are no high-confidence findings. EOF + set +e "$HOME/.opencode/bin/opencode" run \ --agent github-pr-review \ --model "$OPENCODE_MODEL" \ --title "PR #${{ github.event.pull_request.number }} OpenCode review" \ - --file /tmp/opencode-review-guidelines.md \ - --file /tmp/opencode-review.patch \ + --file "$REVIEW_DIR/guidelines.md" \ + --file "$REVIEW_DIR/review.patch" \ -- \ "Review PR #${{ github.event.pull_request.number }} in ${REVIEW_MODE} mode. The attached patch is the review scope. Read repository files only when needed to understand architecture around the patched files. Return only the requested JSON findings block; a trusted workflow step will publish inline comments." \ - > /tmp/opencode-review-output.txt + > "$REVIEW_DIR/opencode.stdout" \ + 2> "$REVIEW_DIR/opencode.stderr" + opencode_status=$? + set -e + + cp "$REVIEW_DIR/opencode.stdout" "$REVIEW_DIR/output.txt" + printf '%s\n' "$opencode_status" > "$REVIEW_DIR/opencode.status" + { + echo "## OpenCode command" + echo + echo "OpenCode exit status: \`$opencode_status\`" + echo "Output files are preserved under \`$REVIEW_DIR/\`." + } >> "$GITHUB_STEP_SUMMARY" - name: Post OpenCode findings env: @@ -171,137 +191,25 @@ jobs: GITHUB_REPOSITORY: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + OPENCODE_MODEL: ${{ env.OPENCODE_MODEL }} run: | set -euo pipefail - python3 <<'PY' - import json - import os - import re - import subprocess - from pathlib import Path + python3 scripts/opencode-code-review post-findings \ + --output .opencode-review/output.txt \ + --files .opencode-review/files.txt \ + --opencode-status .opencode-review/opencode.status \ + --stdout .opencode-review/opencode.stdout \ + --stderr .opencode-review/opencode.stderr \ + --summary "$GITHUB_STEP_SUMMARY" - raw = Path("/tmp/opencode-review-output.txt").read_text() - changed_paths = set(Path("/tmp/opencode-review-files.txt").read_text().splitlines()) - match = re.search(r"\s*(\[.*?\])\s*", raw, re.S) - if not match: - print("OpenCode did not return a valid findings block; skipping comments.") - raise SystemExit(0) - - try: - findings = json.loads(match.group(1)) - except json.JSONDecodeError as exc: - print(f"OpenCode returned invalid findings JSON: {exc}") - raise SystemExit(0) - - if not isinstance(findings, list): - print("OpenCode findings payload was not a JSON array; skipping comments.") - raise SystemExit(0) - - if not findings: - marker = "" - short_sha = os.environ["HEAD_SHA"][:12] - model = os.environ["OPENCODE_MODEL"] - body = ( - f"{marker}\n" - "**OpenCode review complete**\n\n" - f"OpenCode model: `{model}`\n\n" - f"No suggestions found for commit `{short_sha}`." - ) - - comments_result = subprocess.run( - [ - "gh", "api", "--paginate", - f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['PR_NUMBER']}/comments?per_page=100", - ], - text=True, - capture_output=True, - check=False, - ) - comment_id = None - if comments_result.returncode == 0: - try: - comments = json.loads(comments_result.stdout) - except json.JSONDecodeError as exc: - print(f"Could not parse existing PR comments: {exc}") - comments = [] - for comment in reversed(comments): - if marker in (comment.get("body") or ""): - comment_id = comment.get("id") - break - else: - print(f"Could not list existing PR comments: {comments_result.stderr.strip()}") - - method = "PATCH" if comment_id else "POST" - endpoint = ( - f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/comments/{comment_id}" - if comment_id - else f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['PR_NUMBER']}/comments" - ) - result = subprocess.run( - [ - "gh", "api", "-X", method, - endpoint, - "-f", f"body={body}", - ], - text=True, - capture_output=True, - check=False, - ) - if result.returncode == 0: - action = "Updated" if comment_id else "Posted" - print(f"{action} OpenCode no-suggestions comment.") - else: - print(f"Skipped OpenCode no-suggestions comment: {result.stderr.strip()}") - raise SystemExit(0) - - posted = 0 - for finding in findings[:20]: - if not isinstance(finding, dict): - continue - path = finding.get("path") - line = finding.get("line") - title = finding.get("title") - body = finding.get("body") - if not isinstance(path, str) or not isinstance(line, int): - continue - if not isinstance(title, str) or not isinstance(body, str): - continue - if path not in changed_paths: - print(f"Skipped OpenCode finding outside diff scope: {path}:{line}") - continue - comment = f"**OpenCode: {title.strip()}**\n\n{body.strip()}"[:4000] - result = subprocess.run( - [ - "gh", "api", "-X", "POST", - f"repos/{os.environ['GITHUB_REPOSITORY']}/pulls/{os.environ['PR_NUMBER']}/comments", - "-f", f"body={comment}", - "-f", f"commit_id={os.environ['HEAD_SHA']}", - "-f", f"path={path}", - "-F", f"line={line}", - "-f", "side=RIGHT", - ], - text=True, - capture_output=True, - check=False, - ) - if result.returncode == 0: - posted += 1 - else: - print(f"Skipped OpenCode finding for {path}:{line}: {result.stderr.strip()}") - - print(f"Posted {posted} OpenCode inline comments.") - PY - - - name: Summarize OpenCode review - run: | - set -euo pipefail - - { - echo "## OpenCode review" - echo - sed -n '1,180p' /tmp/opencode-review-output.txt - } >> "$GITHUB_STEP_SUMMARY" + - name: Upload OpenCode review artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: opencode-review-same-repo-${{ github.event.pull_request.number }} + path: .opencode-review/ + if-no-files-found: ignore # Strip safe-to-review label on new pushes to fork PRs. # Together with the `github.event.action != 'synchronize'` guard on the @@ -371,6 +279,9 @@ jobs: run: | set -euo pipefail + REVIEW_DIR=.opencode-review + rm -rf "$REVIEW_DIR" + mkdir -p "$REVIEW_DIR" git fetch --no-tags --prune --depth=1 origin "$BASE_SHA" if [[ "$ACTION" == "synchronize" ]] && @@ -384,14 +295,14 @@ jobs: MODE="full-pr" fi - git diff --find-renames --find-copies --unified=80 "$RANGE" > /tmp/opencode-review.patch - git diff --name-only "$RANGE" > /tmp/opencode-review-files.txt + git diff --find-renames --find-copies --unified=80 "$RANGE" > "$REVIEW_DIR/review.patch" + git diff --name-only "$RANGE" > "$REVIEW_DIR/files.txt" { echo "Review mode: $MODE" echo "Diff range: $RANGE" echo echo "Changed files:" - sed 's/^/- /' /tmp/opencode-review-files.txt + sed 's/^/- /' "$REVIEW_DIR/files.txt" echo echo "Repository review guidelines from base commit:" echo @@ -400,7 +311,7 @@ jobs: echo "Code review skill from base commit:" echo git show "$BASE_SHA:.agents/skills/code-review/SKILL.md" 2>/dev/null | sed -n '1,220p' || echo "(code-review skill not present in base commit)" - } > /tmp/opencode-review-guidelines.md + } > "$REVIEW_DIR/guidelines.md" { echo "mode=$MODE" @@ -432,6 +343,9 @@ jobs: run: | set -euo pipefail + REVIEW_DIR=.opencode-review + mkdir -p "$REVIEW_DIR" + rm -rf .opencode mkdir -p .opencode/agents cat > .opencode/agents/github-pr-review.md <<'EOF' --- @@ -477,15 +391,28 @@ jobs: - Use an empty array when there are no high-confidence findings. EOF + set +e "$HOME/.opencode/bin/opencode" run \ --agent github-pr-review \ --model "$OPENCODE_MODEL" \ --title "PR #${{ github.event.pull_request.number }} OpenCode review" \ - --file /tmp/opencode-review-guidelines.md \ - --file /tmp/opencode-review.patch \ + --file "$REVIEW_DIR/guidelines.md" \ + --file "$REVIEW_DIR/review.patch" \ -- \ "Review PR #${{ github.event.pull_request.number }} in ${REVIEW_MODE} mode. The attached patch is the review scope. Read repository files only when needed to understand architecture around the patched files. Return only the requested JSON findings block; a trusted workflow step will publish inline comments." \ - > /tmp/opencode-review-output.txt + > "$REVIEW_DIR/opencode.stdout" \ + 2> "$REVIEW_DIR/opencode.stderr" + opencode_status=$? + set -e + + cp "$REVIEW_DIR/opencode.stdout" "$REVIEW_DIR/output.txt" + printf '%s\n' "$opencode_status" > "$REVIEW_DIR/opencode.status" + { + echo "## OpenCode command" + echo + echo "OpenCode exit status: \`$opencode_status\`" + echo "Output files are preserved under \`$REVIEW_DIR/\`." + } >> "$GITHUB_STEP_SUMMARY" - name: Post OpenCode findings env: @@ -493,134 +420,24 @@ jobs: GITHUB_REPOSITORY: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + OPENCODE_MODEL: ${{ env.OPENCODE_MODEL }} run: | set -euo pipefail - python3 <<'PY' - import json - import os - import re - import subprocess - from pathlib import Path + git show "$BASE_SHA:scripts/opencode-code-review" > .opencode-review/opencode-code-review + python3 .opencode-review/opencode-code-review post-findings \ + --output .opencode-review/output.txt \ + --files .opencode-review/files.txt \ + --opencode-status .opencode-review/opencode.status \ + --stdout .opencode-review/opencode.stdout \ + --stderr .opencode-review/opencode.stderr \ + --summary "$GITHUB_STEP_SUMMARY" - raw = Path("/tmp/opencode-review-output.txt").read_text() - changed_paths = set(Path("/tmp/opencode-review-files.txt").read_text().splitlines()) - match = re.search(r"\s*(\[.*?\])\s*", raw, re.S) - if not match: - print("OpenCode did not return a valid findings block; skipping comments.") - raise SystemExit(0) - - try: - findings = json.loads(match.group(1)) - except json.JSONDecodeError as exc: - print(f"OpenCode returned invalid findings JSON: {exc}") - raise SystemExit(0) - - if not isinstance(findings, list): - print("OpenCode findings payload was not a JSON array; skipping comments.") - raise SystemExit(0) - - if not findings: - marker = "" - short_sha = os.environ["HEAD_SHA"][:12] - model = os.environ["OPENCODE_MODEL"] - body = ( - f"{marker}\n" - "**OpenCode review complete**\n\n" - f"OpenCode model: `{model}`\n\n" - f"No suggestions found for commit `{short_sha}`." - ) - - comments_result = subprocess.run( - [ - "gh", "api", "--paginate", - f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['PR_NUMBER']}/comments?per_page=100", - ], - text=True, - capture_output=True, - check=False, - ) - comment_id = None - if comments_result.returncode == 0: - try: - comments = json.loads(comments_result.stdout) - except json.JSONDecodeError as exc: - print(f"Could not parse existing PR comments: {exc}") - comments = [] - for comment in reversed(comments): - if marker in (comment.get("body") or ""): - comment_id = comment.get("id") - break - else: - print(f"Could not list existing PR comments: {comments_result.stderr.strip()}") - - method = "PATCH" if comment_id else "POST" - endpoint = ( - f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/comments/{comment_id}" - if comment_id - else f"repos/{os.environ['GITHUB_REPOSITORY']}/issues/{os.environ['PR_NUMBER']}/comments" - ) - result = subprocess.run( - [ - "gh", "api", "-X", method, - endpoint, - "-f", f"body={body}", - ], - text=True, - capture_output=True, - check=False, - ) - if result.returncode == 0: - action = "Updated" if comment_id else "Posted" - print(f"{action} OpenCode no-suggestions comment.") - else: - print(f"Skipped OpenCode no-suggestions comment: {result.stderr.strip()}") - raise SystemExit(0) - - posted = 0 - for finding in findings[:20]: - if not isinstance(finding, dict): - continue - path = finding.get("path") - line = finding.get("line") - title = finding.get("title") - body = finding.get("body") - if not isinstance(path, str) or not isinstance(line, int): - continue - if not isinstance(title, str) or not isinstance(body, str): - continue - if path not in changed_paths: - print(f"Skipped OpenCode finding outside diff scope: {path}:{line}") - continue - comment = f"**OpenCode: {title.strip()}**\n\n{body.strip()}"[:4000] - result = subprocess.run( - [ - "gh", "api", "-X", "POST", - f"repos/{os.environ['GITHUB_REPOSITORY']}/pulls/{os.environ['PR_NUMBER']}/comments", - "-f", f"body={comment}", - "-f", f"commit_id={os.environ['HEAD_SHA']}", - "-f", f"path={path}", - "-F", f"line={line}", - "-f", "side=RIGHT", - ], - text=True, - capture_output=True, - check=False, - ) - if result.returncode == 0: - posted += 1 - else: - print(f"Skipped OpenCode finding for {path}:{line}: {result.stderr.strip()}") - - print(f"Posted {posted} OpenCode inline comments.") - PY - - - name: Summarize OpenCode review - run: | - set -euo pipefail - - { - echo "## OpenCode review" - echo - sed -n '1,180p' /tmp/opencode-review-output.txt - } >> "$GITHUB_STEP_SUMMARY" + - name: Upload OpenCode review artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: opencode-review-fork-${{ github.event.pull_request.number }} + path: .opencode-review/ + if-no-files-found: ignore diff --git a/scripts/opencode-code-review b/scripts/opencode-code-review new file mode 100755 index 0000000000..7777c5cbd8 --- /dev/null +++ b/scripts/opencode-code-review @@ -0,0 +1,371 @@ +#!/usr/bin/env python3 +import argparse +import json +import os +import re +import subprocess +import sys +from pathlib import Path +from typing import Any + + +MAX_INLINE_FINDINGS = 20 +MAX_COMMENT_BODY = 60000 +NO_FINDINGS_MARKER = "" +DIAGNOSTIC_MARKER = "" +FALLBACK_MARKER = "" + + +def required_env(name: str) -> str: + value = os.environ.get(name) + if not value: + raise RuntimeError(f"{name} is required") + return value + + +def gh_api(args: list[str]) -> subprocess.CompletedProcess[str]: + gh_bin = os.environ.get("GH_BIN", "gh") + return subprocess.run( + [gh_bin, "api", *args], + text=True, + capture_output=True, + check=False, + ) + + +def list_issue_comments() -> list[dict[str, Any]]: + repository = required_env("GITHUB_REPOSITORY") + pr_number = required_env("PR_NUMBER") + result = gh_api(["--paginate", f"repos/{repository}/issues/{pr_number}/comments?per_page=100"]) + if result.returncode != 0: + print(f"Could not list existing PR comments: {result.stderr.strip()}", file=sys.stderr) + return [] + try: + comments = json.loads(result.stdout) + except json.JSONDecodeError as exc: + print(f"Could not parse existing PR comments: {exc}", file=sys.stderr) + return [] + if isinstance(comments, list): + return [comment for comment in comments if isinstance(comment, dict)] + return [] + + +def upsert_issue_comment(marker: str, body: str) -> bool: + repository = required_env("GITHUB_REPOSITORY") + pr_number = required_env("PR_NUMBER") + comment_id = None + for comment in reversed(list_issue_comments()): + if marker in (comment.get("body") or ""): + comment_id = comment.get("id") + break + + method = "PATCH" if comment_id else "POST" + endpoint = ( + f"repos/{repository}/issues/comments/{comment_id}" + if comment_id + else f"repos/{repository}/issues/{pr_number}/comments" + ) + result = gh_api(["-X", method, endpoint, "-f", f"body={body}"]) + if result.returncode == 0: + action = "Updated" if comment_id else "Posted" + print(f"{action} OpenCode comment for marker {marker}.") + return True + print(f"Could not post OpenCode comment for marker {marker}: {result.stderr.strip()}", file=sys.stderr) + return False + + +def append_summary(path: Path, title: str, lines: list[str]) -> None: + with path.open("a", encoding="utf-8") as handle: + handle.write(f"## {title}\n\n") + for line in lines: + handle.write(f"{line}\n") + handle.write("\n") + + +def snippet(label: str, text: str, *, max_chars: int = 12000) -> list[str]: + trimmed = text.strip() + if len(trimmed) > max_chars: + trimmed = trimmed[:max_chars] + "\n... truncated ..." + if not trimmed: + trimmed = "(empty)" + return [f"### {label}", "", "```text", trimmed, "```", ""] + + +def read_status(path: Path) -> int: + try: + return int(path.read_text(encoding="utf-8").strip()) + except (OSError, ValueError): + return 1 + + +def parse_findings(raw: str) -> tuple[list[Any] | None, str | None]: + match = re.search(r"\s*(.*?)\s*", raw, re.S) + if not match: + return None, "No parseable findings block was found." + try: + payload = json.loads(match.group(1)) + except json.JSONDecodeError as exc: + return None, f"Findings block contained invalid JSON: {exc}" + if not isinstance(payload, list): + return None, "Findings block JSON was not an array." + return payload, None + + +def diagnostic_body(reason: str, status: int, stdout: str, stderr: str) -> str: + short_sha = required_env("HEAD_SHA")[:12] + model = required_env("OPENCODE_MODEL") + lines = [ + DIAGNOSTIC_MARKER, + "**OpenCode review diagnostic**", + "", + "OpenCode did not produce parseable findings, so the review check failed instead of silently skipping comments.", + "", + f"OpenCode model: `{model}`", + f"Commit: `{short_sha}`", + f"OpenCode exit status: `{status}`", + f"Reason: {reason}", + ] + stderr_text = stderr.strip() + stdout_text = stdout.strip() + if stderr_text: + lines.extend(["", "Recent stderr:", "```text", stderr_text[-1800:], "```"]) + elif stdout_text: + lines.extend(["", "Recent stdout:", "```text", stdout_text[-1800:], "```"]) + return "\n".join(lines) + + +def no_findings_body() -> str: + short_sha = required_env("HEAD_SHA")[:12] + model = required_env("OPENCODE_MODEL") + return ( + f"{NO_FINDINGS_MARKER}\n" + "**OpenCode review complete**\n\n" + f"OpenCode model: `{model}`\n\n" + f"No suggestions found for commit `{short_sha}`." + ) + + +def fallback_body(rejected: list[dict[str, Any]], overflow: list[dict[str, Any]]) -> tuple[str, int, int]: + model = required_env("OPENCODE_MODEL") + short_sha = required_env("HEAD_SHA")[:12] + body = "\n".join( + [ + FALLBACK_MARKER, + "**OpenCode findings that could not be posted inline**", + "", + f"OpenCode model: `{model}`", + f"Commit: `{short_sha}`", + "", + "Some findings could not be posted inline, so they are preserved here.", + ] + ) + included = 0 + omitted = 0 + + def append_section(title: str, findings: list[dict[str, Any]]) -> None: + nonlocal body, included, omitted + if not findings: + return + heading = f"\n\n## {title}" + if len(body) + len(heading) > MAX_COMMENT_BODY: + omitted += len(findings) + return + body += heading + + for finding in findings: + section = "\n\n".join( + [ + "", + f"### {finding['path']}:{finding['line']} - {finding['title']}", + str(finding["body"]).strip(), + ] + ) + if len(body) + len(section) > MAX_COMMENT_BODY: + omitted += 1 + continue + body += section + included += 1 + + append_section("GitHub rejected inline placement", rejected) + append_section("Additional findings beyond inline comment limit", overflow) + + if omitted: + notice = f"\n\n_Additional fallback findings omitted from this comment due to GitHub body size limits: {omitted}._" + if len(body) + len(notice) <= MAX_COMMENT_BODY: + body += notice + + return body, included, omitted + + +def normalize_finding(value: Any, changed_paths: set[str]) -> dict[str, Any] | None: + if not isinstance(value, dict): + return None + path = value.get("path") + line = value.get("line") + title = value.get("title") + body = value.get("body") + if not isinstance(path, str) or not isinstance(line, int): + return None + if not isinstance(title, str) or not isinstance(body, str): + return None + if path not in changed_paths: + print(f"Skipped OpenCode finding outside diff scope: {path}:{line}") + return None + return { + "path": path, + "line": line, + "title": title.strip(), + "body": body.strip(), + } + + +def post_inline_finding(finding: dict[str, Any]) -> tuple[bool, str]: + repository = required_env("GITHUB_REPOSITORY") + pr_number = required_env("PR_NUMBER") + head_sha = required_env("HEAD_SHA") + comment = f"**OpenCode: {finding['title']}**\n\n{finding['body']}"[:4000] + result = gh_api( + [ + "-X", + "POST", + f"repos/{repository}/pulls/{pr_number}/comments", + "-f", + f"body={comment}", + "-f", + f"commit_id={head_sha}", + "-f", + f"path={finding['path']}", + "-F", + f"line={finding['line']}", + "-f", + "side=RIGHT", + ] + ) + if result.returncode == 0: + return True, "" + return False, result.stderr.strip() + + +def handle_diagnostic(args: argparse.Namespace, reason: str, status: int, stdout: str, stderr: str, raw: str) -> int: + append_summary( + args.summary, + "OpenCode review diagnostic", + [ + f"OpenCode status: `{status}`", + f"Reason: {reason}", + "", + *snippet("OpenCode stdout", stdout), + *snippet("OpenCode stderr", stderr), + *snippet("OpenCode findings output", raw), + ], + ) + upsert_issue_comment(DIAGNOSTIC_MARKER, diagnostic_body(reason, status, stdout, stderr)) + print(f"OpenCode did not produce parseable findings: {reason}", file=sys.stderr) + return 1 + + +def post_findings(args: argparse.Namespace) -> int: + raw = args.output.read_text(encoding="utf-8") + stdout = args.stdout.read_text(encoding="utf-8") if args.stdout.exists() else raw + stderr = args.stderr.read_text(encoding="utf-8") if args.stderr.exists() else "" + status = read_status(args.opencode_status) + + if status != 0: + return handle_diagnostic(args, f"OpenCode exited with status {status}.", status, stdout, stderr, raw) + + findings, error = parse_findings(raw) + if error is not None or findings is None: + return handle_diagnostic(args, error or "Unknown parser error.", status, stdout, stderr, raw) + + if not findings: + posted_no_findings = upsert_issue_comment(NO_FINDINGS_MARKER, no_findings_body()) + append_summary( + args.summary, + "OpenCode review", + [ + "OpenCode returned a valid empty findings array.", + "", + *snippet("OpenCode stdout", stdout), + *snippet("OpenCode stderr", stderr), + ], + ) + return 0 if posted_no_findings else 1 + + changed_paths = set(args.files.read_text(encoding="utf-8").splitlines()) + posted = 0 + rejected = [] + valid_findings = [ + finding + for finding in (normalize_finding(item, changed_paths) for item in findings) + if finding is not None + ] + inline_findings = valid_findings[:MAX_INLINE_FINDINGS] + overflow_findings = valid_findings[MAX_INLINE_FINDINGS:] + + for finding in inline_findings: + ok, error_message = post_inline_finding(finding) + if ok: + posted += 1 + else: + print( + f"GitHub rejected OpenCode inline finding for {finding['path']}:{finding['line']}: {error_message}", + file=sys.stderr, + ) + rejected.append(finding) + + if overflow_findings: + print(f"Preserving {len(overflow_findings)} OpenCode findings beyond inline limit in fallback comment.") + + posted_fallback = True + fallback_included = 0 + fallback_omitted = 0 + if rejected or overflow_findings: + fallback_comment, fallback_included, fallback_omitted = fallback_body(rejected, overflow_findings) + posted_fallback = upsert_issue_comment(FALLBACK_MARKER, fallback_comment) + + append_summary( + args.summary, + "OpenCode review", + [ + f"Parsed findings: `{len(findings)}`", + f"Inline comments posted: `{posted}`", + f"Inline comments rejected: `{len(rejected)}`", + f"Findings beyond inline limit: `{len(overflow_findings)}`", + f"Fallback findings included in comment: `{fallback_included}`", + f"Fallback findings omitted from comment: `{fallback_omitted}`", + "", + *snippet("OpenCode stdout", stdout), + *snippet("OpenCode stderr", stderr), + ], + ) + print(f"Posted {posted} OpenCode inline comments; preserved {fallback_included} fallback findings.") + return 0 if posted_fallback else 1 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="OpenCode PR review workflow helpers") + subparsers = parser.add_subparsers(dest="command", required=True) + + post = subparsers.add_parser("post-findings") + post.add_argument("--output", type=Path, required=True) + post.add_argument("--files", type=Path, required=True) + post.add_argument("--opencode-status", type=Path, required=True) + post.add_argument("--stdout", type=Path, required=True) + post.add_argument("--stderr", type=Path, required=True) + post.add_argument("--summary", type=Path, required=True) + post.set_defaults(func=post_findings) + return parser + + +def main() -> int: + parser = build_parser() + args = parser.parse_args() + try: + return args.func(args) + except RuntimeError as exc: + print(str(exc), file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/opencode-code-review.test.py b/scripts/opencode-code-review.test.py new file mode 100755 index 0000000000..fd00b3a82e --- /dev/null +++ b/scripts/opencode-code-review.test.py @@ -0,0 +1,238 @@ +#!/usr/bin/env python3 +import json +import os +import re +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / "scripts" / "opencode-code-review" + + +class OpenCodeReviewScriptTest(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.workdir = Path(self.tmp.name) + self.calls_path = self.workdir / "gh-calls.jsonl" + self.output_path = self.workdir / "output.txt" + self.files_path = self.workdir / "files.txt" + self.status_path = self.workdir / "status.txt" + self.stdout_path = self.workdir / "stdout.txt" + self.stderr_path = self.workdir / "stderr.txt" + self.summary_path = self.workdir / "summary.md" + self.files_path.write_text("src/app.ts\n", encoding="utf-8") + self.status_path.write_text("0\n", encoding="utf-8") + self.stdout_path.write_text("stdout line\n", encoding="utf-8") + self.stderr_path.write_text("stderr line\n", encoding="utf-8") + self.summary_path.write_text("", encoding="utf-8") + self.fake_gh = self.write_fake_gh() + + def write_fake_gh(self) -> Path: + fake_gh = self.workdir / "gh" + fake_gh.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import os + import sys + from pathlib import Path + + calls_path = Path(os.environ["GH_CALLS"]) + args = sys.argv[1:] + calls_path.write_text( + calls_path.read_text() + json.dumps(args) + "\\n" + if calls_path.exists() + else json.dumps(args) + "\\n" + ) + + if "comments?per_page=100" in " ".join(args): + print("[]") + raise SystemExit(0) + + joined = " ".join(args) + if "/pulls/" in joined and os.environ.get("INLINE_FAIL") == "1": + print("line is not part of the diff", file=sys.stderr) + raise SystemExit(1) + if "/issues/" in joined and os.environ.get("ISSUE_COMMENT_FAIL") == "1": + print("issue comments unavailable", file=sys.stderr) + raise SystemExit(1) + + print("{}") + """ + ), + encoding="utf-8", + ) + fake_gh.chmod(0o755) + return fake_gh + + def run_script( + self, + *, + output: str, + inline_fail: bool = False, + issue_comment_fail: bool = False, + ) -> subprocess.CompletedProcess[str]: + self.output_path.write_text(output, encoding="utf-8") + env = { + **os.environ, + "GH_BIN": str(self.fake_gh), + "GH_CALLS": str(self.calls_path), + "GITHUB_REPOSITORY": "kdlbs/kandev", + "PR_NUMBER": "42", + "HEAD_SHA": "0123456789abcdef", + "OPENCODE_MODEL": "opencode-go/minimax-m3", + } + if inline_fail: + env["INLINE_FAIL"] = "1" + if issue_comment_fail: + env["ISSUE_COMMENT_FAIL"] = "1" + return subprocess.run( + [ + sys.executable, + str(SCRIPT), + "post-findings", + "--output", + str(self.output_path), + "--files", + str(self.files_path), + "--opencode-status", + str(self.status_path), + "--stdout", + str(self.stdout_path), + "--stderr", + str(self.stderr_path), + "--summary", + str(self.summary_path), + ], + text=True, + capture_output=True, + env=env, + check=False, + ) + + def read_calls(self) -> list[list[str]]: + if not self.calls_path.exists(): + return [] + return [json.loads(line) for line in self.calls_path.read_text().splitlines()] + + def bodies(self) -> list[str]: + bodies = [] + for call in self.read_calls(): + for index, arg in enumerate(call): + if arg == "-f" and index + 1 < len(call) and call[index + 1].startswith("body="): + bodies.append(call[index + 1][len("body=") :]) + return bodies + + def test_missing_findings_block_fails_and_posts_diagnostic(self) -> None: + result = self.run_script(output="OpenCode refused to read external_directory (/tmp/*)\n") + + self.assertNotEqual(result.returncode, 0) + self.assertIn("did not produce parseable findings", result.stderr) + self.assertTrue( + any("" in body for body in self.bodies()), + "expected a stable diagnostic PR comment", + ) + self.assertIn("No parseable findings block", self.summary_path.read_text()) + + def test_invalid_findings_json_fails_and_posts_diagnostic(self) -> None: + result = self.run_script(output="{}\n") + + self.assertNotEqual(result.returncode, 0) + self.assertIn("did not produce parseable findings", result.stderr) + self.assertTrue(any("" in body for body in self.bodies())) + self.assertIn("not an array", self.summary_path.read_text()) + + def test_valid_empty_array_posts_no_findings_comment(self) -> None: + result = self.run_script(output="[]\n") + + self.assertEqual(result.returncode, 0, result.stderr) + bodies = self.bodies() + self.assertTrue(any("" in body for body in bodies)) + self.assertFalse(any("" in body for body in bodies)) + + def test_inline_failures_are_posted_as_one_fallback_comment(self) -> None: + result = self.run_script( + output=textwrap.dedent( + """\ + + [{"path":"src/app.ts","line":99,"title":"Bad line","body":"This line moved."}] + + """ + ), + inline_fail=True, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + bodies = self.bodies() + self.assertTrue(any("" in body for body in bodies)) + self.assertTrue(any("src/app.ts:99" in body for body in bodies)) + + def test_inline_findings_beyond_limit_are_preserved_in_fallback_comment(self) -> None: + findings = [ + {"path": "src/app.ts", "line": index + 1, "title": f"Finding {index + 1}", "body": "body"} + for index in range(30) + ] + + result = self.run_script( + output=f"{json.dumps(findings)}\n", + inline_fail=True, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + bodies = self.bodies() + self.assertTrue(any("" in body for body in bodies)) + self.assertTrue(any("src/app.ts:1" in body for body in bodies)) + self.assertTrue(any("src/app.ts:30" in body for body in bodies)) + self.assertTrue(any("## GitHub rejected inline placement" in body for body in bodies)) + self.assertTrue(any("## Additional findings beyond inline comment limit" in body for body in bodies)) + summary = self.summary_path.read_text() + self.assertIn("Inline comments rejected: `20`", summary) + self.assertIn("Findings beyond inline limit: `10`", summary) + self.assertIn("Fallback findings included in comment: `30`", summary) + self.assertIn("Fallback findings omitted from comment: `0`", summary) + + def test_fallback_comment_reports_findings_omitted_by_body_limit(self) -> None: + findings = [ + {"path": "src/app.ts", "line": index + 1, "title": f"Finding {index + 1}", "body": "x" * 4000} + for index in range(30) + ] + + result = self.run_script( + output=f"{json.dumps(findings)}\n", + inline_fail=True, + ) + + self.assertEqual(result.returncode, 0, result.stderr) + bodies = self.bodies() + self.assertTrue(any("Additional fallback findings omitted" in body for body in bodies)) + summary = self.summary_path.read_text() + self.assertRegex(summary, r"Fallback findings included in comment: `1[0-9]`") + omitted_match = re.search(r"Fallback findings omitted from comment: `([1-9][0-9]*)`", summary) + self.assertIsNotNone(omitted_match, summary) + + def test_fallback_comment_failure_fails_the_step(self) -> None: + result = self.run_script( + output=textwrap.dedent( + """\ + + [{"path":"src/app.ts","line":99,"title":"Bad line","body":"This line moved."}] + + """ + ), + inline_fail=True, + issue_comment_fail=True, + ) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("Could not post OpenCode comment", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/opencode-code-review.test.sh b/scripts/opencode-code-review.test.sh index b3924aece4..bf523a6e7d 100644 --- a/scripts/opencode-code-review.test.sh +++ b/scripts/opencode-code-review.test.sh @@ -16,7 +16,7 @@ fail() { count_occurrences() { local pattern=$1 local output - output="$(rg --fixed-strings --count-matches "$pattern" "$WORKFLOW" || true)" + output="$(rg --fixed-strings --count-matches -- "$pattern" "$WORKFLOW" || true)" if [[ -z "$output" ]]; then printf '0\n' return @@ -28,14 +28,44 @@ count_occurrences() { awk -F: '{ print $2 }' <<<"$output" } -model_reference_count="$(count_occurrences 'OpenCode model: `{model}`')" -if [[ "$model_reference_count" != "2" ]]; then - fail "OpenCode no-suggestions comments include the model in both workflow paths" +tmp_review_reference_count="$(count_occurrences '/tmp/opencode-review')" +if [[ "$tmp_review_reference_count" != "0" ]]; then + fail "OpenCode review artifacts are not written under /tmp" fi -pass "OpenCode no-suggestions comments include the model in both workflow paths" +pass "OpenCode review artifacts are not written under /tmp" -paginated_comment_lookup_count="$(count_occurrences '"gh", "api", "--paginate",')" -if [[ "$paginated_comment_lookup_count" != "2" ]]; then - fail "OpenCode no-suggestions comment lookup paginates in both workflow paths" +relative_guidelines_count="$(count_occurrences '--file "$REVIEW_DIR/guidelines.md"')" +if [[ "$relative_guidelines_count" != "2" ]]; then + fail "OpenCode guidelines are passed as relative workspace files in both workflow paths" fi -pass "OpenCode no-suggestions comment lookup paginates in both workflow paths" +pass "OpenCode guidelines are passed as relative workspace files in both workflow paths" + +relative_patch_count="$(count_occurrences '--file "$REVIEW_DIR/review.patch"')" +if [[ "$relative_patch_count" != "2" ]]; then + fail "OpenCode patches are passed as relative workspace files in both workflow paths" +fi +pass "OpenCode patches are passed as relative workspace files in both workflow paths" + +recreate_config_count="$(count_occurrences 'rm -rf .opencode')" +if [[ "$recreate_config_count" != "2" ]]; then + fail "OpenCode project config is recreated before writing the review agent" +fi +pass "OpenCode project config is recreated before writing the review agent" + +explicit_model_env_count="$(count_occurrences 'OPENCODE_MODEL: ${{ env.OPENCODE_MODEL }}')" +if [[ "$explicit_model_env_count" != "2" ]]; then + fail "OpenCode posting steps explicitly pass OPENCODE_MODEL" +fi +pass "OpenCode posting steps explicitly pass OPENCODE_MODEL" + +trusted_fork_script_count="$(count_occurrences 'git show "$BASE_SHA:scripts/opencode-code-review" > .opencode-review/opencode-code-review')" +if [[ "$trusted_fork_script_count" != "1" ]]; then + fail "Fork review executes the parser script from the trusted base commit" +fi +pass "Fork review executes the parser script from the trusted base commit" + +artifact_upload_count="$(count_occurrences 'uses: actions/upload-artifact@v4')" +if [[ "$artifact_upload_count" != "2" ]]; then + fail "OpenCode review artifacts are uploaded from both workflow paths" +fi +pass "OpenCode review artifacts are uploaded from both workflow paths"