From 1f5a551579989c29152c4bfd429d125ecd6dcac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 17 Jun 2026 22:50:07 +0100 Subject: [PATCH] ci: clarify opencode review completion (#1420) --- .github/workflows/opencode-code-review.yml | 121 ++++++++++++++++++++- Makefile | 8 +- scripts/opencode-code-review.test.sh | 41 +++++++ scripts/pr-state | 42 ++++++- scripts/pr-state.test.sh | 56 +++++++++- 5 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 scripts/opencode-code-review.test.sh diff --git a/.github/workflows/opencode-code-review.yml b/.github/workflows/opencode-code-review.yml index 8976a97b21..5bd73b15d7 100644 --- a/.github/workflows/opencode-code-review.yml +++ b/.github/workflows/opencode-code-review.yml @@ -18,6 +18,9 @@ concurrency: group: opencode-review-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true +env: + OPENCODE_MODEL: opencode-go/minimax-m3 + jobs: opencode-review-same-repo: if: > @@ -154,7 +157,7 @@ jobs: "$HOME/.opencode/bin/opencode" run \ --agent github-pr-review \ - --model opencode-go/minimax-m3 \ + --model "$OPENCODE_MODEL" \ --title "PR #${{ github.event.pull_request.number }} OpenCode review" \ --file /tmp/opencode-review-guidelines.md \ --file /tmp/opencode-review.patch \ @@ -195,6 +198,63 @@ jobs: 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): @@ -419,7 +479,7 @@ jobs: "$HOME/.opencode/bin/opencode" run \ --agent github-pr-review \ - --model opencode-go/minimax-m3 \ + --model "$OPENCODE_MODEL" \ --title "PR #${{ github.event.pull_request.number }} OpenCode review" \ --file /tmp/opencode-review-guidelines.md \ --file /tmp/opencode-review.patch \ @@ -460,6 +520,63 @@ jobs: 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): diff --git a/Makefile b/Makefile index 05315453ab..a20ef8cdb8 100644 --- a/Makefile +++ b/Makefile @@ -331,7 +331,7 @@ install-web: # .PHONY: test -test: test-backend test-web test-cli +test: test-backend test-web test-cli test-scripts @printf "\n$(GREEN)$(BOLD)✓ All tests complete!$(RESET)\n" # Curated Windows-clean test run. Mirrors the test-windows job in @@ -374,6 +374,12 @@ test-cli: @printf "$(CYAN)Running CLI tests...$(RESET)\n" @cd $(APPS_DIR) && $(PNPM) --filter kandev test +.PHONY: test-scripts +test-scripts: + @printf "$(CYAN)Running script tests...$(RESET)\n" + @bash scripts/pr-state.test.sh + @bash scripts/opencode-code-review.test.sh + .PHONY: test-e2e test-e2e: build-backend build-web @printf "$(CYAN)Running E2E tests (headless, parallel)...$(RESET)\n" diff --git a/scripts/opencode-code-review.test.sh b/scripts/opencode-code-review.test.sh new file mode 100644 index 0000000000..b3924aece4 --- /dev/null +++ b/scripts/opencode-code-review.test.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +WORKFLOW="$ROOT_DIR/.github/workflows/opencode-code-review.yml" + +pass() { + printf 'ok - %s\n' "$1" +} + +fail() { + printf 'not ok - %s\n' "$1" >&2 + exit 1 +} + +count_occurrences() { + local pattern=$1 + local output + output="$(rg --fixed-strings --count-matches "$pattern" "$WORKFLOW" || true)" + if [[ -z "$output" ]]; then + printf '0\n' + return + fi + if [[ "$output" =~ ^[0-9]+$ ]]; then + printf '%s\n' "$output" + return + fi + 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" +fi +pass "OpenCode no-suggestions comments include the model in both workflow paths" + +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" +fi +pass "OpenCode no-suggestions comment lookup paginates in both workflow paths" diff --git a/scripts/pr-state b/scripts/pr-state index 6c1b6325e4..fcb5b7bdfa 100755 --- a/scripts/pr-state +++ b/scripts/pr-state @@ -117,7 +117,23 @@ extract_checks_json() { ((.state // "") | lower_or_null) as $state | if $state == "pending" or $state == "expected" then null else $state end end; - $checks_in | map({ + def preferred_check_order: + if .conclusion == "success" then 0 + elif .status != "completed" then 1 + elif ( + .conclusion != null + and .conclusion != "neutral" + and .conclusion != "skipped" + and .conclusion != "cancelled" + ) then 2 + elif .conclusion == "neutral" then 3 + elif .conclusion == "skipped" then 4 + elif .conclusion == "cancelled" then 5 + else 6 + end; + $checks_in + | to_entries + | map(.value + {_index: .key} | { type: (.["__typename"] // null), name: (.name // .context // "unknown"), workflow: (.workflowName // .workflow // .checkSuite.workflowRun.workflow.name // null), @@ -126,8 +142,18 @@ extract_checks_json() { details_url: (.detailsUrl // null), target_url: (.targetUrl // null), run_id: ((.detailsUrl // null) | extract("/actions/runs/(?[0-9]+)"; "id")), - job_id: ((.detailsUrl // null) | extract("/job/(?[0-9]+)"; "id")) + job_id: ((.detailsUrl // null) | extract("/job/(?[0-9]+)"; "id")), + _index }) + | sort_by([(.workflow // ""), .name]) + | group_by([(.workflow // ""), .name]) + | map({ + first_index: (map(._index) | min), + items: sort_by(preferred_check_order, ._index) + }) + | sort_by(.first_index) + | map(.items[]) + | map(del(._index)) ' } @@ -186,6 +212,14 @@ extract_review_threads_json() { summarize_state_json() { jq -c ' + def effective_checks: + .checks + | to_entries + | map(.value + {_index: .key}) + | sort_by([(.workflow // ""), .name]) + | group_by([(.workflow // ""), .name]) + | map(sort_by(._index) | .[0] | del(._index)); + def bad_conclusion: .conclusion != null and .conclusion != "success" @@ -207,12 +241,12 @@ summarize_state_json() { pr, since, failed_checks: [ - .checks[] + effective_checks[] | select(terminal_failure_status and bad_conclusion) | {name, workflow, status, conclusion, run_id, job_id, details_url, target_url} ], pending_checks: [ - .checks[] + effective_checks[] | select(pending_status and .conclusion == null) | {name, workflow, status, run_id, job_id, details_url, target_url} ], diff --git a/scripts/pr-state.test.sh b/scripts/pr-state.test.sh index ae5f3d0226..00471a34aa 100644 --- a/scripts/pr-state.test.sh +++ b/scripts/pr-state.test.sh @@ -94,6 +94,13 @@ if [[ "$1" == "pr" && "$2" == "view" && "$4" == "--json" ]]; then } ], "statusCheckRollup": [ + { + "__typename": "CheckRun", + "name": "web lint", + "status": "COMPLETED", + "conclusion": "CANCELLED", + "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000000/job/55150000000" + }, { "__typename": "CheckRun", "name": "web lint", @@ -122,6 +129,38 @@ if [[ "$1" == "pr" && "$2" == "view" && "$4" == "--json" ]]; then "conclusion": "", "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000003/job/55150000003" }, + { + "__typename": "CheckRun", + "name": "opencode-review-same-repo", + "workflowName": "OpenCode Code Review", + "status": "COMPLETED", + "conclusion": "SKIPPED", + "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000004/job/55150000004" + }, + { + "__typename": "CheckRun", + "name": "opencode-review-same-repo", + "workflowName": "OpenCode Code Review", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000005/job/55150000005" + }, + { + "__typename": "CheckRun", + "name": "opencode-review-fork", + "workflowName": "OpenCode Code Review", + "status": "COMPLETED", + "conclusion": "SKIPPED", + "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000006/job/55150000006" + }, + { + "__typename": "CheckRun", + "name": "opencode-review-fork", + "workflowName": "OpenCode Code Review", + "status": "COMPLETED", + "conclusion": "FAILURE", + "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000007/job/55150000007" + }, { "__typename": "StatusContext", "context": "external pending", @@ -319,10 +358,12 @@ test_snapshot_happy_path() { assert_jq "pr number" '.pr.number == 123' "$json" assert_jq "branch" '.pr.branch == "feat/pr-state"' "$json" assert_jq "since timestamp" '.since.committed_at == "2026-06-01T12:00:00Z"' "$json" - assert_jq "checks count" '.checks | length == 4' "$json" + assert_jq "checks count" '.checks | length == 9' "$json" + assert_jq "duplicate cancelled check prefers success first" '[.checks[] | select(.name == "web lint")][0] | .conclusion == "success" and .run_id == "27340000001"' "$json" assert_jq "failed check preserved" '.checks[] | select(.name == "e2e") | .conclusion == "failure"' "$json" assert_jq "check run id" '.checks[] | select(.name == "e2e") | .run_id == "27340000002"' "$json" assert_jq "nested workflow name" '.checks[] | select(.name == "e2e") | .workflow == "CI"' "$json" + assert_jq "duplicate check prefers non-skipped first" '[.checks[] | select(.name == "opencode-review-same-repo")][0] | .conclusion == "success" and .run_id == "27340000005"' "$json" assert_jq "pending status context conclusion normalized" '.checks[] | select(.name == "external pending") | .status == "pending" and .conclusion == null' "$json" assert_jq "threads count" '.review_threads | length == 1' "$json" assert_jq "total unresolved count includes historical unresolved thread" '.unresolved_review_thread_count == 2' "$json" @@ -347,7 +388,7 @@ test_partial_failure_records_error_but_keeps_other_data() { json="$(<"$tmp/out.json")" assert_jq "reviews empty on failure" '.reviews == []' "$json" - assert_jq "checks still present" '.checks | length == 4' "$json" + assert_jq "checks still present" '.checks | length == 9' "$json" assert_jq "new issue comments still present" '.issue_comments | length == 1' "$json" assert_jq "partial failure recorded" '.errors | length == 1' "$json" assert_jq "partial failure source" '.errors[0].source == "reviews"' "$json" @@ -378,7 +419,7 @@ test_repo_failure_skips_review_threads() { GH_FAIL_REPO=1 GH_NO_PR_URL=1 PATH="$tmp/bin:$PATH" "$SCRIPT" 123 >"$tmp/out.json" json="$(<"$tmp/out.json")" - assert_jq "checks still present on repo failure" '.checks | length == 4' "$json" + assert_jq "checks still present on repo failure" '.checks | length == 9' "$json" assert_jq "review threads empty on repo failure" '.review_threads == []' "$json" assert_jq "unresolved count unknown on repo failure" '.unresolved_review_thread_count == null' "$json" assert_jq "repo failure recorded" '.errors[] | select(.source == "repo") | .message == "gh repo view failed"' "$json" @@ -395,7 +436,7 @@ test_graphql_failure_records_error_but_keeps_other_data() { GH_FAIL_GRAPHQL=1 PATH="$tmp/bin:$PATH" "$SCRIPT" 123 >"$tmp/out.json" json="$(<"$tmp/out.json")" - assert_jq "checks still present on graphql failure" '.checks | length == 4' "$json" + assert_jq "checks still present on graphql failure" '.checks | length == 9' "$json" assert_jq "reviews still present on graphql failure" '.reviews | length == 2' "$json" assert_jq "review threads empty on graphql failure" '.review_threads == []' "$json" assert_jq "unresolved count unknown on graphql failure" '.unresolved_review_thread_count == null' "$json" @@ -452,8 +493,11 @@ test_summary_mode_returns_compact_fixup_state() { assert_jq "summary keeps pr" '.pr.number == 123' "$json" assert_jq "summary keeps since" '.since.committed_at == "2026-06-01T12:00:00Z"' "$json" - assert_jq "summary failed check count" '.failed_checks | length == 1' "$json" - assert_jq "summary failed check" '.failed_checks[0] | .name == "e2e" and .conclusion == "failure" and .run_id == "27340000002"' "$json" + assert_jq "summary failed check count" '.failed_checks | length == 2' "$json" + assert_jq "summary failed check" '.failed_checks[] | select(.name == "e2e") | .conclusion == "failure" and .run_id == "27340000002"' "$json" + assert_jq "summary reports failed duplicate over skipped" '.failed_checks[] | select(.name == "opencode-review-fork") | .conclusion == "failure" and .run_id == "27340000007"' "$json" + assert_jq "summary ignores cancelled duplicate check" '[.failed_checks[] | select(.name == "web lint")] | length == 0' "$json" + assert_jq "summary dedupes skipped duplicate check" '[.failed_checks[], .pending_checks[] | select(.name == "opencode-review-same-repo")] | length == 0' "$json" assert_jq "summary pending check count" '.pending_checks | length == 2' "$json" assert_jq "summary pending check" '.pending_checks[0] | .name == "claude-review" and .status == "in_progress" and .run_id == "27340000003"' "$json" assert_jq "summary pending status context keeps target url" '.pending_checks[] | select(.name == "external pending") | .status == "pending" and .details_url == null and .target_url == "https://ci.example.test/build/1"' "$json"