Skip to content

fix(github): paginate listComments to find managed comment beyond first page#835

Merged
kitsuyui merged 2 commits into
mainfrom
fix/audit-listcomments-no-pagination-001
May 26, 2026
Merged

fix(github): paginate listComments to find managed comment beyond first page#835
kitsuyui merged 2 commits into
mainfrom
fix/audit-listcomments-no-pagination-001

Conversation

@kitsuyui
Copy link
Copy Markdown
Owner

Summary

  • getManagedComment previously called listComments once with per_page: 100, missing any managed comment that appeared after the first 100 entries
  • On PRs with many comments, this caused decideCommentAction to see null as the past comment and repeatedly create duplicate managed comments
  • Extract listAllComments to iterate through all pages, then search the complete result in getManagedComment

Validation

  • All 44 tests pass
  • biome check passes (each new function stays within the cognitive-complexity limit of 3)
  • madge --circular src/ reports no circular dependencies

Notes

The listAllComments helper stops fetching once a page returns fewer than 100 items, which signals the last page. getManagedComment itself is unchanged in shape — it still returns the first managed comment by the given user, or null.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

🎉 Happy commit!

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

gh-counter

PR gate

Removed Added +/-
TODO/FIXME 0 0 0

Repo dashboard

main (fd2ef70) #835 (8235eb2) +/-
TODO/FIXME 0 0 0

Reported by gh-counter

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

Code Metrics Report

main (fd2ef70) #835 (8235eb2) +/-
Coverage 99.1% 99.2% +0.0%
Code to Test Ratio 1:0.5 1:0.5 -0.1
Test Execution Time 1s 1s 0s
Details
  |                     | main (fd2ef70) | #835 (8235eb2) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          99.1% |          99.2% | +0.0% |
  |   Files             |              5 |              5 |     0 |
  |   Lines             |            122 |            131 |    +9 |
+ |   Covered           |            121 |            130 |    +9 |
- | Code to Test Ratio  |          1:0.5 |          1:0.5 |  -0.1 |
  |   Code              |           1598 |           1616 |   +18 |
+ |   Test              |            898 |            899 |    +1 |
  | Test Execution Time |             1s |             1s |    0s |

Code coverage of files in pull request scope (100.0% → 100.0%)

Files Coverage +/- Status
src/github.ts 100.0% 0.0% modified

Reported by octocov

…st page

Previously getManagedComment called listComments once with per_page=100,
missing any managed comment that appeared after the first 100 entries.
On PRs with many comments this caused repeated duplicate comment creation.

Extract listAllComments to fetch all pages iteratively, then search the
full result in getManagedComment. Each function stays within the
cognitive-complexity limit enforced by biome.
@kitsuyui kitsuyui force-pushed the fix/audit-listcomments-no-pagination-001 branch from 13766ec to 071dfc4 Compare May 25, 2026 13:55
The test job was failing on `git diff --exit-code -- dist` because dist/
was not regenerated after the src/github.ts pagination refactor in this PR.
Running `bun run build` produces three updated files (github.mjs, its map,
and github.d.mts.map). All 46 tests pass.
@github-actions
Copy link
Copy Markdown

gh-build-size

Target main #835 +/-
Total action artifacts 23,299,554 B 23,300,320 B +766 B
Runtime bundle 1,153,519 B 1,153,633 B +114 B
Source maps 17,130,010 B 17,130,662 B +652 B

Reported by gh-build-size

@kitsuyui kitsuyui merged commit 98984a1 into main May 26, 2026
5 checks passed
@kitsuyui kitsuyui deleted the fix/audit-listcomments-no-pagination-001 branch May 26, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant