ci: clarify opencode review completion (#1420)
This commit is contained in:
@@ -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 = "<!-- opencode-review:no-findings -->"
|
||||
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 = "<!-- opencode-review:no-findings -->"
|
||||
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):
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
+38
-4
@@ -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/(?<id>[0-9]+)"; "id")),
|
||||
job_id: ((.detailsUrl // null) | extract("/job/(?<id>[0-9]+)"; "id"))
|
||||
job_id: ((.detailsUrl // null) | extract("/job/(?<id>[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}
|
||||
],
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user