diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 11c4de7..001fdc9 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -65,13 +65,46 @@ jobs: PR_NUMBER=${{ github.event.pull_request.number }} REPO=${{ github.repository }} - CYCLE=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews" \ - --paginate | jq -s '[.[][] | select(.user.type == "Bot" and (.state == "CHANGES_REQUESTED" or .state == "APPROVED"))] | length') + # Count distinct commits already reviewed, never review state: the org ruleset + # sets dismiss_stale_reviews_on_push, so a push flips a prior APPROVED to + # DISMISSED and a state filter stops matching it. Inline comments each create + # their own COMMENTED review sharing the round's commit_id, so unique commit_id + # == round count, +/-1 when a push lands mid-round and splits it across two SHAs. + # Coupled to the reviewer's login: if that ever changes the count silently drops + # to 0 and every round looks like the first, hence the warning below. + CYCLE_JQ='[.[][] | select(.user.login == "claude[bot]") | .commit_id] | unique | length' + # Only consulted when CYCLE is 0; see the warning below. Kept in its own variable + # so tests/review-cycle-test.sh can assert it against the fixtures. + DRIFT_JQ='any(.[][]; .user.type == "Bot")' + # Never fail the review over the cycle number; degrade to 1, but say so. gh + # writes its error body to stdout, so an unguarded pipe into jq aborts the step + # under `bash -e` and skips the failure-notification step below. + if ! REVIEWS=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/reviews" --paginate); then + echo "::warning::Could not read prior reviews; treating this as review cycle 1." + REVIEWS='' + fi + CYCLE=$(printf '%s' "$REVIEWS" | jq -s "$CYCLE_JQ" 2>/dev/null) || CYCLE='' + if [ -z "$CYCLE" ]; then + echo "::warning::Could not parse prior reviews; treating this as review cycle 1." + CYCLE=0 + elif [ "$CYCLE" -eq 0 ] && printf '%s' "$REVIEWS" \ + | jq -e -s "$DRIFT_JQ" >/dev/null 2>&1; then + # claude[bot] is the only bot that submits reviews across the org (598 of 598 + # sampled), so bot reviews that the login filter did not count mean the + # reviewer's identity moved and the counter has silently pinned at 1. + echo "::warning::Bot reviews exist but none matched the reviewer login; the review cycle counter is stale." + fi echo "review_cycle=$((CYCLE + 1))" >> $GITHUB_OUTPUT - THREADS=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/comments" \ - --paginate | jq -s -r ' - add | sort_by(.created_at) | + # Same guard as the counter above: unguarded `gh api | jq` aborts the step, and a + # failure here *skips* the review step, so the notify step's failure check never + # fires and the PR gets no review and no explanation. + if ! COMMENTS=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/comments" --paginate); then + echo "::warning::Could not read prior review comments; reviewing without them." + COMMENTS='' + fi + THREADS=$(printf '%s' "$COMMENTS" | jq -s -r ' + (add // []) | sort_by(.created_at) | if length == 0 then "No prior review comments." else .[] | "---", @@ -82,7 +115,7 @@ jobs: "", .body end - ') + ') || THREADS='No prior review comments.' DELIMITER="REVIEW_CONTEXT_$(openssl rand -hex 16)" { diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..c77e53d --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,20 @@ +name: Tests + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6.0.2 + with: + fetch-depth: 1 + + - name: Review cycle counter + run: tests/review-cycle-test.sh diff --git a/tests/fixtures/reviews-approve-with-nits.json b/tests/fixtures/reviews-approve-with-nits.json new file mode 100644 index 0000000..48beb35 --- /dev/null +++ b/tests/fixtures/reviews-approve-with-nits.json @@ -0,0 +1,352 @@ +[ + { + "id": 4803070490, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "COMMENTED", + "submitted_at": "2026-07-29T00:55:42Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803070659, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "COMMENTED", + "submitted_at": "2026-07-29T00:55:45Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803071124, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "COMMENTED", + "submitted_at": "2026-07-29T00:55:54Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803071286, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "COMMENTED", + "submitted_at": "2026-07-29T00:55:57Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803071631, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "COMMENTED", + "submitted_at": "2026-07-29T00:56:03Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803071837, + "commit_id": "28027b1414cfd97666d91523815287373f2b7392", + "state": "DISMISSED", + "submitted_at": "2026-07-29T00:56:07Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803172821, + "commit_id": "172678856137ab53d98d6f4f3ec64d3b487c51a5", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:21:16Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803173638, + "commit_id": "172678856137ab53d98d6f4f3ec64d3b487c51a5", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:21:23Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803174365, + "commit_id": "172678856137ab53d98d6f4f3ec64d3b487c51a5", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:21:31Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803174637, + "commit_id": "172678856137ab53d98d6f4f3ec64d3b487c51a5", + "state": "DISMISSED", + "submitted_at": "2026-07-29T01:21:35Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803287072, + "commit_id": "2a9f29992bc41238cc515fe3ded706e659ca63b0", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:46:43Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803287878, + "commit_id": "2a9f29992bc41238cc515fe3ded706e659ca63b0", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:46:53Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803288508, + "commit_id": "2a9f29992bc41238cc515fe3ded706e659ca63b0", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:47:02Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803289241, + "commit_id": "2a9f29992bc41238cc515fe3ded706e659ca63b0", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:47:11Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803289574, + "commit_id": "2a9f29992bc41238cc515fe3ded706e659ca63b0", + "state": "DISMISSED", + "submitted_at": "2026-07-29T01:47:15Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803335452, + "commit_id": "67dad03d0ee86f6d7c1cc06845b3b005166252d9", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:59:06Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803335626, + "commit_id": "67dad03d0ee86f6d7c1cc06845b3b005166252d9", + "state": "COMMENTED", + "submitted_at": "2026-07-29T01:59:09Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803335837, + "commit_id": "67dad03d0ee86f6d7c1cc06845b3b005166252d9", + "state": "DISMISSED", + "submitted_at": "2026-07-29T01:59:13Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803472086, + "commit_id": "c66e3592047e43f65db110b3c5cf959145eb928f", + "state": "COMMENTED", + "submitted_at": "2026-07-29T02:28:39Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803472879, + "commit_id": "c66e3592047e43f65db110b3c5cf959145eb928f", + "state": "COMMENTED", + "submitted_at": "2026-07-29T02:28:48Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803473129, + "commit_id": "c66e3592047e43f65db110b3c5cf959145eb928f", + "state": "COMMENTED", + "submitted_at": "2026-07-29T02:28:52Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4803473496, + "commit_id": "c66e3592047e43f65db110b3c5cf959145eb928f", + "state": "DISMISSED", + "submitted_at": "2026-07-29T02:28:58Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810541861, + "commit_id": "ee53bdd8320e59270eba423d31d513dfae80a7c8", + "state": "COMMENTED", + "submitted_at": "2026-07-29T16:18:19Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810543705, + "commit_id": "ee53bdd8320e59270eba423d31d513dfae80a7c8", + "state": "COMMENTED", + "submitted_at": "2026-07-29T16:18:30Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810545119, + "commit_id": "ee53bdd8320e59270eba423d31d513dfae80a7c8", + "state": "DISMISSED", + "submitted_at": "2026-07-29T16:18:38Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810648343, + "commit_id": "239056bc5f535a6c55dfe494d54102a5c5bb3b90", + "state": "COMMENTED", + "submitted_at": "2026-07-29T16:29:54Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810650459, + "commit_id": "239056bc5f535a6c55dfe494d54102a5c5bb3b90", + "state": "COMMENTED", + "submitted_at": "2026-07-29T16:30:06Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4810651071, + "commit_id": "239056bc5f535a6c55dfe494d54102a5c5bb3b90", + "state": "DISMISSED", + "submitted_at": "2026-07-29T16:30:10Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811705495, + "commit_id": "3f3ba8d1962bb2b650e0394525be88a676101748", + "state": "COMMENTED", + "submitted_at": "2026-07-29T18:32:04Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811707279, + "commit_id": "3f3ba8d1962bb2b650e0394525be88a676101748", + "state": "COMMENTED", + "submitted_at": "2026-07-29T18:32:16Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811708296, + "commit_id": "3f3ba8d1962bb2b650e0394525be88a676101748", + "state": "DISMISSED", + "submitted_at": "2026-07-29T18:32:23Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811789242, + "commit_id": "fab04bd24df816278c5ac3aa38935f47ecac9b03", + "state": "COMMENTED", + "submitted_at": "2026-07-29T18:41:37Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811790302, + "commit_id": "fab04bd24df816278c5ac3aa38935f47ecac9b03", + "state": "COMMENTED", + "submitted_at": "2026-07-29T18:41:45Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811791546, + "commit_id": "fab04bd24df816278c5ac3aa38935f47ecac9b03", + "state": "COMMENTED", + "submitted_at": "2026-07-29T18:41:56Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4811792773, + "commit_id": "fab04bd24df816278c5ac3aa38935f47ecac9b03", + "state": "APPROVED", + "submitted_at": "2026-07-29T18:42:06Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + } +] diff --git a/tests/fixtures/reviews-first-review.json b/tests/fixtures/reviews-first-review.json new file mode 100644 index 0000000..fe51488 --- /dev/null +++ b/tests/fixtures/reviews-first-review.json @@ -0,0 +1 @@ +[] diff --git a/tests/fixtures/reviews-foreign-reviewer.json b/tests/fixtures/reviews-foreign-reviewer.json new file mode 100644 index 0000000..2e7b381 --- /dev/null +++ b/tests/fixtures/reviews-foreign-reviewer.json @@ -0,0 +1,62 @@ +[ + { + "id": 1, + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "state": "COMMENTED", + "submitted_at": "2026-07-01T00:00:00Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + }, + { + "id": 2, + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "state": "CHANGES_REQUESTED", + "submitted_at": "2026-07-01T00:00:05Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + }, + { + "id": 3, + "commit_id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "state": "DISMISSED", + "submitted_at": "2026-07-01T01:00:00Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + }, + { + "id": 4, + "commit_id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "state": "APPROVED", + "submitted_at": "2026-07-01T01:05:00Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + }, + { + "id": 5, + "commit_id": "cccccccccccccccccccccccccccccccccccccccc", + "state": "APPROVED", + "submitted_at": "2026-07-01T02:00:00Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + }, + { + "id": 6, + "commit_id": "dddddddddddddddddddddddddddddddddddddddd", + "state": "APPROVED", + "submitted_at": "2026-07-01T03:00:00Z", + "user": { + "login": "hotdata-automation[bot]", + "type": "Bot" + } + } +] diff --git a/tests/fixtures/reviews-mixed-bots.json b/tests/fixtures/reviews-mixed-bots.json new file mode 100644 index 0000000..980f83b --- /dev/null +++ b/tests/fixtures/reviews-mixed-bots.json @@ -0,0 +1,44 @@ +[ + { + "id": 1, + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "state": "COMMENTED", + "submitted_at": "2026-07-01T00:00:00Z", + "user": { "login": "claude[bot]", "type": "Bot" } + }, + { + "id": 2, + "commit_id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "state": "CHANGES_REQUESTED", + "submitted_at": "2026-07-01T00:00:05Z", + "user": { "login": "claude[bot]", "type": "Bot" } + }, + { + "id": 3, + "commit_id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "state": "DISMISSED", + "submitted_at": "2026-07-01T01:00:00Z", + "user": { "login": "claude[bot]", "type": "Bot" } + }, + { + "id": 4, + "commit_id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "state": "APPROVED", + "submitted_at": "2026-07-01T01:05:00Z", + "user": { "login": "hotdata-automation[bot]", "type": "Bot" } + }, + { + "id": 5, + "commit_id": "cccccccccccccccccccccccccccccccccccccccc", + "state": "APPROVED", + "submitted_at": "2026-07-01T02:00:00Z", + "user": { "login": "aikido-autofix[bot]", "type": "Bot" } + }, + { + "id": 6, + "commit_id": "dddddddddddddddddddddddddddddddddddddddd", + "state": "APPROVED", + "submitted_at": "2026-07-01T03:00:00Z", + "user": { "login": "zfarrell", "type": "User" } + } +] diff --git a/tests/fixtures/reviews-paginated.json b/tests/fixtures/reviews-paginated.json new file mode 100644 index 0000000..f7a482a --- /dev/null +++ b/tests/fixtures/reviews-paginated.json @@ -0,0 +1,2 @@ +[{"id":1,"commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","state":"COMMENTED","submitted_at":"2026-07-01T00:00:00Z","user":{"login":"claude[bot]","type":"Bot"}},{"id":2,"commit_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","state":"CHANGES_REQUESTED","submitted_at":"2026-07-01T00:00:05Z","user":{"login":"claude[bot]","type":"Bot"}},{"id":3,"commit_id":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","state":"DISMISSED","submitted_at":"2026-07-01T01:00:00Z","user":{"login":"claude[bot]","type":"Bot"}}] +[{"id":4,"commit_id":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","state":"APPROVED","submitted_at":"2026-07-01T01:05:00Z","user":{"login":"hotdata-automation[bot]","type":"Bot"}},{"id":5,"commit_id":"cccccccccccccccccccccccccccccccccccccccc","state":"APPROVED","submitted_at":"2026-07-01T02:00:00Z","user":{"login":"aikido-autofix[bot]","type":"Bot"}},{"id":6,"commit_id":"dddddddddddddddddddddddddddddddddddddddd","state":"APPROVED","submitted_at":"2026-07-01T03:00:00Z","user":{"login":"zfarrell","type":"User"}}] diff --git a/tests/fixtures/reviews-straddled-round.json b/tests/fixtures/reviews-straddled-round.json new file mode 100644 index 0000000..fc75dcc --- /dev/null +++ b/tests/fixtures/reviews-straddled-round.json @@ -0,0 +1,222 @@ +[ + { + "id": 4790491756, + "commit_id": "a89f19e81690a9091b320594f0f5a2dd1a79ed4b", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:00:03Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790492307, + "commit_id": "a89f19e81690a9091b320594f0f5a2dd1a79ed4b", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:00:08Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790493522, + "commit_id": "a89f19e81690a9091b320594f0f5a2dd1a79ed4b", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:00:17Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790494429, + "commit_id": "a89f19e81690a9091b320594f0f5a2dd1a79ed4b", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:00:25Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790495081, + "commit_id": "a89f19e81690a9091b320594f0f5a2dd1a79ed4b", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:00:30Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790495676, + "commit_id": "951c8afaddf5579bf8496df9d0c6ebd5e186d640", + "state": "APPROVED", + "submitted_at": "2026-07-27T19:00:35Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790560226, + "commit_id": "951c8afaddf5579bf8496df9d0c6ebd5e186d640", + "state": "DISMISSED", + "submitted_at": "2026-07-27T19:07:06Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790604991, + "commit_id": "d0f6dcab75199784b136cf89be2680980777efd7", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:11:49Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790606647, + "commit_id": "d0f6dcab75199784b136cf89be2680980777efd7", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:11:59Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790607676, + "commit_id": "d0f6dcab75199784b136cf89be2680980777efd7", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:12:05Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790608309, + "commit_id": "d0f6dcab75199784b136cf89be2680980777efd7", + "state": "DISMISSED", + "submitted_at": "2026-07-27T19:12:08Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790725357, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:24:14Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790726786, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:24:25Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790767497, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:29:36Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790768432, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:29:44Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790769530, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:29:53Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790770077, + "commit_id": "8099fef1793b2ac7531643cdeffc16b9e18d6eb1", + "state": "DISMISSED", + "submitted_at": "2026-07-27T19:29:58Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790978887, + "commit_id": "703e6f0f6139631dbab17fee47034fe05318cd35", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:56:17Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790980555, + "commit_id": "703e6f0f6139631dbab17fee47034fe05318cd35", + "state": "COMMENTED", + "submitted_at": "2026-07-27T19:56:31Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4790981470, + "commit_id": "703e6f0f6139631dbab17fee47034fe05318cd35", + "state": "DISMISSED", + "submitted_at": "2026-07-27T19:56:38Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4791181568, + "commit_id": "ffe5a7d50cccac18957cb592f4330d27473d10fb", + "state": "COMMENTED", + "submitted_at": "2026-07-27T20:22:28Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + }, + { + "id": 4791181903, + "commit_id": "ffe5a7d50cccac18957cb592f4330d27473d10fb", + "state": "APPROVED", + "submitted_at": "2026-07-27T20:22:32Z", + "user": { + "login": "claude[bot]", + "type": "Bot" + } + } +] diff --git a/tests/review-cycle-test.sh b/tests/review-cycle-test.sh new file mode 100755 index 0000000..5985362 --- /dev/null +++ b/tests/review-cycle-test.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# +# Guards the REVIEW CYCLE counter in claude-pr-review.yml against fixtures captured from +# the real reviews API. The jq program is extracted from the workflow rather than copied, +# so the test exercises the shipped expression. +# +# Regression: the counter used to filter on review state (CHANGES_REQUESTED/APPROVED). On +# a PR whose every round ends in approve-with-nits, dismiss_stale_reviews_on_push flips +# each of those approvals to DISMISSED, so the filter matched nothing and the counter read +# 1 forever, silently disabling the prompt's cycle-awareness ladder. CHANGES_REQUESTED +# survives a push, so request-changes PRs counted correctly and the bug stayed hidden. + +set -euo pipefail + +cd "$(dirname "$0")/.." + +WORKFLOW=.github/workflows/claude-pr-review.yml + +# extract_jq -- pull a single-quoted jq program out of the workflow +extract_jq() { + local name=$1 prog + prog=$(sed -n "s/^ *$name='\(.*\)'\$/\1/p" "$WORKFLOW") + if [ -z "$prog" ]; then + echo "FAIL: no $name='...' assignment found in $WORKFLOW" >&2 + exit 1 + fi + if [ "$(printf '%s\n' "$prog" | wc -l)" -ne 1 ]; then + echo "FAIL: more than one $name assignment in $WORKFLOW:" >&2 + printf '%s\n' "$prog" >&2 + exit 1 + fi + printf '%s' "$prog" +} + +CYCLE_JQ=$(extract_jq CYCLE_JQ) +DRIFT_JQ=$(extract_jq DRIFT_JQ) + +failures=0 + +# expect +expect() { + local fixture=$1 want=$2 desc=$3 + local count actual + count=$(jq -s "$CYCLE_JQ" "tests/fixtures/$fixture") + actual=$((count + 1)) + if [ "$actual" -eq "$want" ]; then + echo "ok $desc (cycle=$actual)" + else + echo "FAIL $desc: expected cycle $want, got $actual" + failures=$((failures + 1)) + fi +} + +# expect_drift +expect_drift() { + local fixture=$1 want=$2 desc=$3 actual=silent + if jq -e -s "$DRIFT_JQ" "tests/fixtures/$fixture" >/dev/null 2>&1; then + actual=fires + fi + if [ "$actual" = "$want" ]; then + echo "ok $desc ($actual)" + else + echo "FAIL $desc: expected $want, got $actual" + failures=$((failures + 1)) + fi +} + +# No prior reviews. +expect reviews-first-review.json 1 "opened PR is cycle 1" + +# monopoly#1560: nine review rounds, every one an approve-with-nits that the next push +# dismissed. The tenth review must know it is the tenth. +expect reviews-approve-with-nits.json 10 "nine dismissed approve-with-nits rounds count" + +# Rounds counted regardless of state; approvals by other bots and by humans do not count. +expect reviews-mixed-bots.json 3 "only claude[bot] rounds count, one per commit" + +# The counter is keyed on the reviewer's login. If claude-code-action ever posts under a +# different identity the count collapses to 0 and every round reads as cycle 1 again -- +# the original bug. The drift predicate is the runtime backstop for that; it is only +# consulted when the count is 0, so it has to separate "the login moved" from "genuine +# first review". +expect reviews-foreign-reviewer.json 1 "unknown reviewer login yields no rounds" +expect_drift reviews-foreign-reviewer.json fires "drift warning fires when the login moved" +expect_drift reviews-first-review.json silent "drift warning silent on a genuine cycle 1" + +# gh 2.93 merges --paginate pages into one array; older versions concatenate one array per +# page. `jq -s '.[][]'` must handle both, so keep a concatenated fixture. +expect reviews-paginated.json 3 "concatenated --paginate pages count once each" + +# Known +/-1: monopoly#1534 round 1 posted 5 inline comments against a89f19e8, then its +# approve landed 5s later against 951c8afa because a push arrived mid-review, so 5 real +# rounds count as 6. Harmless against a 5-step ladder; asserted so a future change to the +# counter has to acknowledge this case rather than shift it silently. +expect reviews-straddled-round.json 7 "push landing mid-round splits it in two" + +if [ "$failures" -ne 0 ]; then + echo "$failures test(s) failed" + exit 1 +fi +echo "all tests passed"