From 46b036d7df325dd1546a7285d1726a7c1df50cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sun, 14 Jun 2026 14:47:40 +0100 Subject: [PATCH] fix: summarize current PR state (#1362) --- .agents/skills/pr-fixup/SKILL.md | 28 +++- scripts/pr-state | 211 ++++++++++++++++++++++++++++--- scripts/pr-state.test.sh | 177 +++++++++++++++++++++----- 3 files changed, 362 insertions(+), 54 deletions(-) diff --git a/.agents/skills/pr-fixup/SKILL.md b/.agents/skills/pr-fixup/SKILL.md index 4d19b05669..524987e4fd 100644 --- a/.agents/skills/pr-fixup/SKILL.md +++ b/.agents/skills/pr-fixup/SKILL.md @@ -60,7 +60,7 @@ If available, invoke the `pr-poller` subagent with the PR number (or let it reso - `bots.` — `done` / `rate_limited` / `pending` / `timeout`. Anything in `done` or `rate_limited` has had its chance; treat the rest as missing data, not a blocker. - `unresolved_review_threads` and `issue_comments_from_bots` — drive steps 3-4. If both are 0 and `ci_failed` is empty, skip to step 5 (still run verify + push if you have fixes from earlier). -**E2E CI outlasts the poller.** The pr-poller caps at ~20 minutes. The E2E matrix (10 shards × 2 projects) often runs longer. If the report shows `ci_pending` with only E2E/lint jobs and `ci_failed` is empty, re-invoke pr-poller once those jobs finish — do not spin a manual `gh pr checks` loop in the parent. If the cap hits with E2E still pending, report "CI in progress" to the user instead of blocking. +**E2E CI outlasts the poller.** The pr-poller caps at ~20 minutes. The E2E matrix (10 shards × 2 projects) often runs longer. GitHub can expand E2E matrix jobs late; if pending checks briefly drop near zero and then jump when E2E shards appear, keep treating that as normal pending CI unless a shard reports failure. If the report shows `ci_pending` with only E2E/lint jobs and `ci_failed` is empty, re-invoke pr-poller once those jobs finish — do not spin a manual `gh pr checks` loop in the parent. If the cap hits with E2E still pending, report "CI in progress" to the user instead of blocking, and include the exact pending shard names from `ci_pending`. **Do not fetch poll output yourself** — that is what burns context. The report is the only thing that enters your context. @@ -69,12 +69,30 @@ If available, invoke the `pr-poller` subagent with the PR number (or let it reso If delegated polling is unavailable, gather the same information directly without streaming long logs into context: ```bash -scripts/pr-state +scripts/pr-state --summary ``` -Poll at a 30s cadence with a **20 min cap**. Stop early if any required check fails. If the cap hits and only E2E shards are still pending with no failures, report "CI in progress" instead of continuing to watch indefinitely. Do not run `gh pr checks --watch` in the main session unless the runtime can keep the watcher isolated and automatically clean it up. If you do use `gh pr checks --watch`, keep watching until the command exits; GitHub can expand matrix jobs after an initial aggregate "Build" check passes, so the first green build/lint/test rows are not necessarily terminal. +Prefer `scripts/pr-state --summary ` for direct polling and `scripts/pr-resolve list ` for review-thread state over raw `gh pr checks`. `gh pr checks` only reports CI/status checks; review bots can open unresolved threads after CI is green, and a checks-only poll will miss that blocker. -Summarize the direct-command result from `scripts/pr-state` using its raw snapshot fields: `.checks`, `.review_threads`, `.unresolved_review_thread_count`, `.reviews`, `.issue_comments`, and `.errors`. Derive `ci_failed`, `ci_pending`, and which bot comments are actionable in the parent from those raw arrays. If `.errors` is non-empty, treat the affected fields as unknown and report the data-gathering failure instead of reconstructing them ad hoc. When deriving non-green checks, filter to actionable failures/pending checks; check/status records with both `status=success` and `conclusion=success` are green even if they appear in a mixed raw array. +By default, `scripts/pr-state` returns comments, reviews, and review threads created after the latest PR head commit only. This is intentional for fixup passes: it keeps old bot summaries and already-addressed historical threads out of the working set. Use `scripts/pr-state --summary --all ` only when you need to audit the full PR history. + +The summary output contains: + +- `failed_checks` — actionable non-green checks with `name`, `workflow`, `status`, `conclusion`, `run_id`, `job_id`, `details_url`, and `target_url` +- `pending_checks` — still-running or queued checks with `name`, `workflow`, `status`, `run_id`, `job_id`, `details_url`, and `target_url` +- `unresolved_review_thread_count` — total unresolved thread count on the PR, including older unresolved threads outside the current-head filter +- `filtered_review_thread_count` and `unresolved_threads` — compact current-head inline review state to triage in this fixup pass +- `errors` — data-gathering failures; treat affected fields as unknown instead of reconstructing them ad hoc + +Poll at a 30s cadence with a **20 min cap**. Stop early if any required check fails. If the cap hits and only E2E shards are still pending with no failures or unresolved comments, report "CI in progress" instead of continuing to watch indefinitely, and include the exact pending shard names from `pending_checks`. Do not run `gh pr checks --watch` in the main session unless the runtime can keep the watcher isolated and automatically clean it up. If you do use `gh pr checks --watch`, keep watching until the command exits; GitHub can expand matrix jobs after an initial aggregate "Build" check passes, so the first green build/lint/test rows are not necessarily terminal. + +If a user interrupts a long manual poll loop (`sleep`, `gh pr checks`, or `scripts/pr-state`), check for leftover polling processes before switching tasks and terminate only the processes you started. + +Use raw mode only when debugging an odd GitHub state: + +```bash +scripts/pr-state +``` Mark task 1 as completed. @@ -334,6 +352,8 @@ Mark task 6 as completed. **Expect new threads after every push.** CodeRabbit, Greptile, Claude, and cubic often re-review the latest commit and open fresh inline threads even when earlier ones were resolved. On cross-cutting changes (backend event payloads + frontend WS handlers + E2E), plan for 2–3 fixup rounds. After each push, always run `scripts/pr-resolve list ` before declaring done — do not rely on the prior round's zero count. +**Stop when green unless the next comment is clearly worth another cycle.** Tiny review-only commits restart the full CI and bot-review stack. Once the PR is green with no unresolved threads, avoid nonessential cleanup that would trigger another round unless the comment is blocking, clearly valid, or requested by the user. + ### 7. Summary Mark task 7 as in_progress. diff --git a/scripts/pr-state b/scripts/pr-state index 54b51e5237..72a86026e0 100755 --- a/scripts/pr-state +++ b/scripts/pr-state @@ -2,27 +2,72 @@ # pr-state — emit a raw JSON snapshot of PR CI and review state. # # Usage: -# scripts/pr-state +# scripts/pr-state [--summary] [--all] # -# Output: +# Raw output: # JSON object with: # - pr: { number, branch } +# - since: { head_oid, committed_at } or null # - checks: [{ type, name, workflow, status, conclusion, details_url, target_url, run_id, job_id }] -# - review_threads: [{ thread_id, is_resolved, path, comment_id, author, body_preview }] +# - review_threads: [{ thread_id, is_resolved, path, comment_id, comment_created_at, author, body_preview }] # - unresolved_review_thread_count: +# - filtered_review_thread_count: # - reviews: [{ author, state, submitted_at, html_url }] # - issue_comments: [{ author, body, created_at, url }] # - errors: [{ source, message }] # +# Summary output (--summary): +# JSON object with: +# - pr: { number, branch } +# - since: { head_oid, committed_at } or null +# - failed_checks: [{ name, workflow, status, conclusion, run_id, job_id, details_url, target_url }] +# - pending_checks: [{ name, workflow, status, run_id, job_id, details_url, target_url }] +# - unresolved_review_thread_count: +# - filtered_review_thread_count: +# - unresolved_threads: [{ thread_id, comment_id, author, path, body_preview }] +# - errors: [{ source, message }] +# +# By default, comments/reviews/review threads are limited to items created after +# the PR head commit. Checks are already scoped by GitHub to the current head. +# Pass --all to include historical comments and reviews. +# # Requires: gh (authenticated), jq. set -euo pipefail usage() { - sed -n '2,15p' "$0" | sed 's|^# \{0,1\}||' + sed -n '2,/^# Requires:/p' "$0" | sed 's|^# \{0,1\}||' exit "${1:-1}" } +INCLUDE_ALL=0 +SUMMARY=0 +while [[ $# -gt 0 ]]; do + case "$1" in + --all) + INCLUDE_ALL=1 + shift + ;; + --summary) + SUMMARY=1 + shift + ;; + -h | --help) + usage 0 + ;; + --) + shift + break + ;; + -*) + usage 1 + ;; + *) + break + ;; + esac +done + [ $# -eq 1 ] || usage PR=$1 @@ -32,7 +77,9 @@ checks_json='[]' review_threads_json='[]' reviews_json='[]' issue_comments_json='[]' +since_json='null' unresolved_review_thread_count=null +filtered_review_thread_count=null add_error() { local source=$1 @@ -54,12 +101,19 @@ extract_checks_json() { if . == null or . == "" then null else ascii_downcase end; def extract($re; $field): if . == null then null else ((capture($re)? | .[$field]) // null) end; + def normalized_conclusion: + if has("conclusion") then + (.conclusion | lower_or_null) + else + ((.state // "") | lower_or_null) as $state + | if $state == "pending" or $state == "expected" then null else $state end + end; $checks_in | map({ type: (.["__typename"] // null), name: (.name // .context // "unknown"), workflow: (.workflowName // .workflow // .checkSuite.workflowRun.workflow.name // null), status: ((.status // .state // "unknown") | ascii_downcase), - conclusion: ((.conclusion // .state // "") | lower_or_null), + conclusion: normalized_conclusion, details_url: (.detailsUrl // null), target_url: (.targetUrl // null), run_id: ((.detailsUrl // null) | extract("/actions/runs/(?[0-9]+)"; "id")), @@ -70,8 +124,11 @@ extract_checks_json() { extract_issue_comments_json() { local comments_json=$1 - jq -cn --argjson comments "$comments_json" ' - $comments | map({ + local since=$2 + jq -cn --argjson comments "$comments_json" --arg since "$since" ' + $comments + | map(select($since == "" or ((.createdAt // .created_at // "") > $since))) + | map({ author: (.author.login // "unknown"), body: (.body // ""), created_at: (.createdAt // .created_at // null), @@ -82,8 +139,11 @@ extract_issue_comments_json() { extract_reviews_json() { local reviews_api_json=$1 - jq -cn --argjson reviews "$reviews_api_json" ' - $reviews | map({ + local since=$2 + jq -cn --argjson reviews "$reviews_api_json" --arg since "$since" ' + $reviews + | map(select($since == "" or ((.submitted_at // "") > $since))) + | map({ author: (.user.login // "unknown"), state: ((.state // "unknown") | ascii_downcase), submitted_at: (.submitted_at // null), @@ -94,18 +154,100 @@ extract_reviews_json() { extract_review_threads_json() { local threads_nodes_json=$1 - jq -cn --argjson threads "$threads_nodes_json" ' - $threads | map({ + local since=$2 + jq -cn --argjson threads "$threads_nodes_json" --arg since "$since" ' + def relevant_comments: + if $since == "" then + (.comments.nodes // []) + else + [(.comments.nodes // [])[] | select((.createdAt // "") > $since)] + end; + $threads + | map(. as $thread | (relevant_comments) as $comments | select($comments | length > 0) | { thread_id: .id, is_resolved: (.isResolved // false), path: (.path // ""), - comment_id: (.comments.nodes[0].databaseId // null), - author: (.comments.nodes[0].author.login // "unknown"), - body_preview: ((.comments.nodes[0].body // "") | gsub("\n"; " ") | .[0:120]) + comment_id: ($comments[0].databaseId // null), + comment_created_at: ($comments[0].createdAt // null), + author: ($comments[0].author.login // "unknown"), + body_preview: (($comments[0].body // "") | gsub("\n"; " ") | .[0:120]) }) ' } +summarize_state_json() { + jq -c ' + def bad_conclusion: + .conclusion != null + and .conclusion != "success" + and .conclusion != "skipped" + and .conclusion != "neutral"; + + def terminal_failure_status: + .status == "completed" + or .status == "failure" + or .status == "error"; + + def pending_status: + .status != "completed" + and .status != "success" + and .status != "failure" + and .status != "error"; + + { + pr, + since, + failed_checks: [ + .checks[] + | select(terminal_failure_status and bad_conclusion) + | {name, workflow, status, conclusion, run_id, job_id, details_url, target_url} + ], + pending_checks: [ + .checks[] + | select(pending_status and .conclusion == null) + | {name, workflow, status, run_id, job_id, details_url, target_url} + ], + unresolved_review_thread_count, + filtered_review_thread_count, + unresolved_threads: [ + .review_threads[] + | select(.is_resolved == false) + | {thread_id, comment_id, author, path, body_preview} + ], + errors + } + ' +} + +fetch_head_commit_json() { + local owner=$1 + local repo=$2 + local pr_number=$3 + + gh_json api graphql \ + -f query=' + query($owner: String!, $repo: String!, $pr: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pr) { + headRefOid + commits(last: 1) { + nodes { + commit { + oid + committedDate + } + } + } + } + } + }' \ + -f owner="$owner" -f repo="$repo" -F pr="$pr_number" | + jq -c '{ + head_oid: (.data.repository.pullRequest.headRefOid // .data.repository.pullRequest.commits.nodes[0].commit.oid // null), + committed_at: (.data.repository.pullRequest.commits.nodes[0].commit.committedDate // null) + }' +} + fetch_review_threads_nodes_json() { local owner=$1 local repo=$2 @@ -134,9 +276,10 @@ fetch_review_threads_nodes_json() { id isResolved path - comments(first: 1) { + comments(last: 100) { nodes { databaseId + createdAt author { login } body } @@ -192,7 +335,6 @@ if pr_view_json="$(gh_json pr view "$PR" --json number,headRefName,statusCheckRo comments_json="$(jq -c '.comments // []' <<<"$pr_view_json")" status_json="$(jq -c '.statusCheckRollup // []' <<<"$pr_view_json")" checks_json="$(extract_checks_json "$status_json")" - issue_comments_json="$(extract_issue_comments_json "$comments_json")" else if [[ "$PR" =~ ^[0-9]+$ ]]; then pr_json="$(jq -cn --argjson number "$PR" '{number: $number, branch: "unknown"}')" @@ -202,10 +344,26 @@ else add_error "pr_view" "gh pr view failed" fi +since='' +if [[ "$INCLUDE_ALL" != "1" && -n "$repo_owner" && -n "$repo_name" && -n "$resolved_pr_number" ]]; then + if since_json="$(fetch_head_commit_json "$repo_owner" "$repo_name" "$resolved_pr_number")"; then + since="$(jq -r '.committed_at // ""' <<<"$since_json")" + if [[ -z "$since" ]]; then + add_error "since" "head commit timestamp unavailable; including historical comments" + since_json='null' + fi + else + add_error "since" "gh api graphql head commit failed; including historical comments" + since_json='null' + fi +fi + +issue_comments_json="$(extract_issue_comments_json "$comments_json" "$since")" + if [[ -n "$repo_owner" && -n "$repo_name" && -n "$resolved_pr_number" ]]; then if reviews_pages_json="$(gh_json api --paginate -X GET "repos/$repo_owner/$repo_name/pulls/$resolved_pr_number/reviews")"; then reviews_api_json="$(jq -cs 'add // []' <<<"$reviews_pages_json")" - reviews_json="$(extract_reviews_json "$reviews_api_json")" + reviews_json="$(extract_reviews_json "$reviews_api_json" "$since")" else add_error "reviews" "gh api pulls reviews failed" fi @@ -215,8 +373,9 @@ fi if [[ -n "$repo_owner" && -n "$repo_name" && -n "$resolved_pr_number" ]]; then if threads_nodes_json="$(fetch_review_threads_nodes_json "$repo_owner" "$repo_name" "$resolved_pr_number")"; then - review_threads_json="$(extract_review_threads_json "$threads_nodes_json")" - unresolved_review_thread_count="$(jq -r '[.[] | select(.is_resolved == false)] | length' <<<"$review_threads_json")" + review_threads_json="$(extract_review_threads_json "$threads_nodes_json" "$since")" + unresolved_review_thread_count="$(jq -r '[.[] | select(.isResolved == false)] | length' <<<"$threads_nodes_json")" + filtered_review_thread_count="$(jq -r 'length' <<<"$review_threads_json")" else add_error "review_threads" "gh api graphql reviewThreads failed" fi @@ -226,20 +385,30 @@ elif [[ -z "$resolved_pr_number" ]]; then add_error "review_threads" "skipped: resolved PR identity unavailable" fi -jq -cn \ +state_json="$(jq -cn \ --argjson pr "$pr_json" \ + --argjson since "$since_json" \ --argjson checks "$checks_json" \ --argjson review_threads "$review_threads_json" \ --argjson reviews "$reviews_json" \ --argjson issue_comments "$issue_comments_json" \ --argjson unresolved_review_thread_count "$unresolved_review_thread_count" \ + --argjson filtered_review_thread_count "$filtered_review_thread_count" \ --argjson errors "$errors" \ '{ pr: $pr, + since: $since, checks: $checks, review_threads: $review_threads, unresolved_review_thread_count: $unresolved_review_thread_count, + filtered_review_thread_count: $filtered_review_thread_count, reviews: $reviews, issue_comments: $issue_comments, errors: $errors - }' + }')" + +if [[ "$SUMMARY" == "1" ]]; then + summarize_state_json <<<"$state_json" +else + printf '%s\n' "$state_json" +fi diff --git a/scripts/pr-state.test.sh b/scripts/pr-state.test.sh index 2fd70fbe13..ae5f3d0226 100644 --- a/scripts/pr-state.test.sh +++ b/scripts/pr-state.test.sh @@ -83,11 +83,14 @@ if [[ "$1" == "pr" && "$2" == "view" && "$4" == "--json" ]]; then "comments": [ { "author": { "login": "coderabbitai" }, - "body": "" + "body": "", + "createdAt": "2026-06-01T10:00:00Z" }, { "author": { "login": "github-actions" }, - "body": "**Claude finished review**\\n| Blocker | 1 |\\n| Suggestion | 2 |\\n**Verdict:** Ready with suggestions" + "body": "**Claude finished review**\\n| Blocker | 1 |\\n| Suggestion | 2 |\\n**Verdict:** Ready with suggestions", + "createdAt": "2026-06-01T13:00:00Z", + "url": "https://github.com/kdlbs/kandev/pull/123#issuecomment-2" } ], "statusCheckRollup": [ @@ -118,6 +121,12 @@ if [[ "$1" == "pr" && "$2" == "view" && "$4" == "--json" ]]; then "status": "IN_PROGRESS", "conclusion": "", "detailsUrl": "https://github.com/kdlbs/kandev/actions/runs/27340000003/job/55150000003" + }, + { + "__typename": "StatusContext", + "context": "external pending", + "state": "PENDING", + "targetUrl": "https://ci.example.test/build/1" } ] } @@ -127,15 +136,46 @@ fi if [[ "$1" == "api" && "$2" == "--paginate" && "$3" == "-X" && "$4" == "GET" && "$5" == "repos/kdlbs/kandev/pulls/123/reviews" ]]; then printf '%s\n' '[ - { "user": { "login": "greptile-apps[bot]" } } + { + "user": { "login": "greptile-apps[bot]" }, + "state": "COMMENTED", + "submitted_at": "2026-06-01T10:30:00Z" + } ]' printf '%s\n' '[ - { "user": { "login": "cubic-dev-ai[bot]" } } + { + "user": { "login": "cubic-dev-ai[bot]" }, + "state": "CHANGES_REQUESTED", + "submitted_at": "2026-06-01T13:30:00Z" + } ]' exit 0 fi if [[ "$1" == "api" && "$2" == "graphql" ]]; then + if [[ "$*" == *"headRefOid"* ]]; then + printf '%s\n' '{ + "data": { + "repository": { + "pullRequest": { + "headRefOid": "abc123", + "commits": { + "nodes": [ + { + "commit": { + "oid": "abc123", + "committedDate": "2026-06-01T12:00:00Z" + } + } + ] + } + } + } + } + }' + exit 0 + fi + if [[ "${GH_GRAPHQL_TWO_PAGES:-0}" == "1" && "$*" != *"cursor=CURSOR1"* ]]; then printf '%s\n' '{ "data": { @@ -155,6 +195,7 @@ if [[ "$1" == "api" && "$2" == "graphql" ]]; then "nodes": [ { "databaseId": 111, + "createdAt": "2026-06-01T13:00:00Z", "author": { "login": "greptile-apps[bot]" }, "body": "Please rename this helper" } @@ -189,6 +230,7 @@ if [[ "$1" == "api" && "$2" == "graphql" ]]; then "nodes": [ { "databaseId": 222, + "createdAt": "2026-06-01T14:00:00Z", "author": { "login": "cubic-dev-ai[bot]" }, "body": "resolved" } @@ -220,24 +262,32 @@ if [[ "$1" == "api" && "$2" == "graphql" ]]; then "path": "apps/web/file.ts", "comments": { "nodes": [ - { - "databaseId": 111, - "author": { "login": "greptile-apps[bot]" }, - "body": "Please rename this helper" - } - ] - } + { + "databaseId": 111, + "createdAt": "2026-06-01T10:00:00Z", + "author": { "login": "greptile-apps[bot]" }, + "body": "Please rename this helper" + }, + { + "databaseId": 112, + "createdAt": "2026-06-01T13:00:00Z", + "author": { "login": "greptile-apps[bot]" }, + "body": "Please rename this helper after the latest commit" + } + ] + } }, { "id": "PRRT_2", - "isResolved": true, + "isResolved": false, "path": "apps/web/other.ts", "comments": { "nodes": [ - { - "databaseId": 222, - "author": { "login": "cubic-dev-ai[bot]" }, - "body": "resolved" + { + "databaseId": 222, + "createdAt": "2026-06-01T11:00:00Z", + "author": { "login": "cubic-dev-ai[bot]" }, + "body": "resolved" } ] } @@ -268,17 +318,20 @@ test_snapshot_happy_path() { assert_jq "pr number" '.pr.number == 123' "$json" assert_jq "branch" '.pr.branch == "feat/pr-state"' "$json" - assert_jq "checks count" '.checks | length == 3' "$json" + assert_jq "since timestamp" '.since.committed_at == "2026-06-01T12:00:00Z"' "$json" + assert_jq "checks count" '.checks | length == 4' "$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 "threads count" '.review_threads | length == 2' "$json" - assert_jq "unresolved count" '.unresolved_review_thread_count == 1' "$json" - assert_jq "thread comment id" '.review_threads[] | select(.thread_id == "PRRT_1") | .comment_id == 111' "$json" - assert_jq "thread resolved field" '.review_threads[] | select(.thread_id == "PRRT_2") | .is_resolved == true' "$json" - assert_jq "reviews count" '.reviews | length == 2' "$json" - assert_jq "review author" '.reviews[] | select(.author == "greptile-apps[bot]") | .author == "greptile-apps[bot]"' "$json" - assert_jq "issue comments count" '.issue_comments | length == 2' "$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" + assert_jq "filtered thread count" '.filtered_review_thread_count == 1' "$json" + assert_jq "thread comment id" '.review_threads[] | select(.thread_id == "PRRT_1") | .comment_id == 112' "$json" + assert_jq "thread comment timestamp" '.review_threads[] | select(.thread_id == "PRRT_1") | .comment_created_at == "2026-06-01T13:00:00Z"' "$json" + assert_jq "reviews count" '.reviews | length == 1' "$json" + assert_jq "review author" '.reviews[] | select(.author == "cubic-dev-ai[bot]") | .author == "cubic-dev-ai[bot]"' "$json" + assert_jq "issue comments count" '.issue_comments | length == 1' "$json" assert_jq "issue comment author" '.issue_comments[] | select(.author == "github-actions") | .body | contains("Verdict")' "$json" assert_jq "no errors" '.errors == []' "$json" pass "snapshot happy path" @@ -294,8 +347,8 @@ 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 == 3' "$json" - assert_jq "issue comments still present" '.issue_comments | length == 2' "$json" + assert_jq "checks still present" '.checks | length == 4' "$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" pass "partial failure records error but keeps other data" @@ -325,7 +378,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 == 3' "$json" + assert_jq "checks still present on repo failure" '.checks | length == 4' "$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" @@ -342,11 +395,14 @@ 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 == 3' "$json" + assert_jq "checks still present on graphql failure" '.checks | length == 4' "$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" - assert_jq "graphql failure recorded" '.errors[] | select(.source == "review_threads") | .message == "gh api graphql reviewThreads failed"' "$json" + assert_jq "graphql failure records both errors" '.errors | length == 2' "$json" + assert_jq "graphql failure records since error" '.errors[] | select(.source == "since") | .message == "gh api graphql head commit failed; including historical comments"' "$json" + assert_jq "graphql failure records review_threads error" '.errors[] | select(.source == "review_threads") | .message == "gh api graphql reviewThreads failed"' "$json" + assert_jq "since fallback includes all historical comments" '.issue_comments | length == 2' "$json" pass "graphql failure records error but keeps other data" } @@ -363,13 +419,76 @@ test_graphql_pagination_collects_all_threads() { assert_jq "paginated first thread" '.review_threads[] | select(.thread_id == "PRRT_1") | .is_resolved == false' "$json" assert_jq "paginated second thread" '.review_threads[] | select(.thread_id == "PRRT_2") | .is_resolved == true' "$json" assert_jq "paginated unresolved count" '.unresolved_review_thread_count == 1' "$json" + assert_jq "paginated filtered thread count" '.filtered_review_thread_count == 2' "$json" assert_jq "paginated no errors" '.errors == []' "$json" pass "graphql pagination collects all threads" } +test_all_flag_includes_historical_comments_and_reviews() { + local tmp + make_tmp_dir tmp + make_mock_gh "$tmp/bin" + + local json + PATH="$tmp/bin:$PATH" "$SCRIPT" --all 123 >"$tmp/out.json" + json="$(<"$tmp/out.json")" + + assert_jq "since omitted in all mode" '.since == null' "$json" + assert_jq "all issue comments present" '.issue_comments | length == 2' "$json" + assert_jq "all reviews present" '.reviews | length == 2' "$json" + assert_jq "all review threads present" '.review_threads | length == 2' "$json" + assert_jq "all mode keeps historical thread comment" '.review_threads[] | select(.thread_id == "PRRT_1") | .comment_id == 111' "$json" + pass "--all includes historical comments and reviews" +} + +test_summary_mode_returns_compact_fixup_state() { + local tmp + make_tmp_dir tmp + make_mock_gh "$tmp/bin" + + local json + PATH="$tmp/bin:$PATH" "$SCRIPT" --summary 123 >"$tmp/out.json" + json="$(<"$tmp/out.json")" + + 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 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" + assert_jq "summary unresolved count" '.unresolved_review_thread_count == 2' "$json" + assert_jq "summary filtered thread count" '.filtered_review_thread_count == 1' "$json" + assert_jq "summary unresolved threads" '.unresolved_threads | length == 1' "$json" + assert_jq "summary unresolved thread fields" '.unresolved_threads[0] | .thread_id == "PRRT_1" and .comment_id == 112 and .author == "greptile-apps[bot]"' "$json" + assert_jq "summary omits raw arrays" 'has("checks") | not' "$json" + assert_jq "summary no errors" '.errors == []' "$json" + pass "--summary returns compact fixup state" +} + +test_summary_all_flag_includes_historical_unresolved_threads() { + local tmp + make_tmp_dir tmp + make_mock_gh "$tmp/bin" + + local json + PATH="$tmp/bin:$PATH" "$SCRIPT" --summary --all 123 >"$tmp/out.json" + json="$(<"$tmp/out.json")" + + assert_jq "summary all since omitted" '.since == null' "$json" + assert_jq "summary all unresolved count" '.unresolved_review_thread_count == 2' "$json" + assert_jq "summary all filtered thread count" '.filtered_review_thread_count == 2' "$json" + assert_jq "summary all keeps historical first comment" '.unresolved_threads[] | select(.thread_id == "PRRT_1") | .comment_id == 111' "$json" + assert_jq "summary all unresolved threads count" '.unresolved_threads | length == 2' "$json" + pass "--summary --all includes historical unresolved thread comments" +} + test_snapshot_happy_path test_partial_failure_records_error_but_keeps_other_data test_pr_view_failure_with_non_numeric_ref_keeps_schema test_repo_failure_skips_review_threads test_graphql_failure_records_error_but_keeps_other_data test_graphql_pagination_collects_all_threads +test_all_flag_includes_historical_comments_and_reviews +test_summary_mode_returns_compact_fixup_state +test_summary_all_flag_includes_historical_unresolved_threads