Skip to content

fix(no-response): rework GitHub API usage#5

Merged
tcely merged 6 commits intomainfrom
tcely-fix-pagination
Mar 17, 2026
Merged

fix(no-response): rework GitHub API usage#5
tcely merged 6 commits intomainfrom
tcely-fix-pagination

Conversation

@tcely
Copy link
Copy Markdown
Owner

@tcely tcely commented Mar 16, 2026

No description provided.

@tcely tcely self-assigned this Mar 16, 2026
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Fix pagination and async handling in no-response action

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Fix pagination in API requests using octokit.paginate() method
• Correct state_reason from 'inactivity' to 'not_planned' for issue closure
• Add missing await keywords for async operations
• Improve type safety and code consistency with spread operators
• Optimize label checking with some() instead of map().includes()
Diagram
flowchart LR
  A["API Requests"] -->|"Use octokit.paginate()"| B["Proper Pagination"]
  C["Async Operations"] -->|"Add await keywords"| D["Correct Execution"]
  E["Label Checking"] -->|"Use some() method"| F["Better Performance"]
  G["Issue Closure"] -->|"Update state_reason"| H["Correct Status"]
Loading

Grey Divider

File Changes

1. src/no-response.ts 🐞 Bug fix +35/-52

Fix pagination and async handling in API requests

• Removed unused RequestInterface import from @octokit/types
• Fixed pagination in findLastLabeledEvent() and getCloseableIssues() using octokit.paginate()
 method
• Added missing await keywords for async API calls in close(), removeLabels(), and
 onComment() methods
• Changed issue closure state_reason from 'inactivity' to 'not_planned'
• Updated LabeledEvent interface: created_at type from number to string, made label
 property optional
• Refactored API call parameters to use spread operator with issue object for consistency
• Optimized hasResponseRequiredLabel() to use some() instead of map().includes()
• Improved error handling in ensureLabelExists() with proper await and simplified catch clause
• Simplified conditional logic in removeLabels() and onComment() methods
• Minor performance improvement in since() method using Date.now() instead of `new
 Date().getTime()`

src/no-response.ts


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Mar 16, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Sequential issue closing🐞 Bug ➹ Performance
Description
sweep() now awaits close() inside the loop, forcing one issue to be closed completely before
starting the next, which increases total runtime proportional to the number of issues. Since close()
performs multiple GitHub API calls, this can significantly slow scheduled runs on busy repositories.
Code

src/no-response.ts[R51-53]

   for (const issue of issues) {
-      this.close({ issue_number: issue.number, ...this.config.repo })
+      await this.close({ issue_number: issue.number, ...this.config.repo })
   }
Evidence
The sweep loop performs awaited closes serially, and close() itself issues network calls (comment +
update), so each issue adds full round-trip latency before the next begins.

src/no-response.ts[41-54]
src/no-response.ts[134-144]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`sweep()` closes issues strictly sequentially by awaiting `close()` inside a `for ... of` loop. Since `close()` performs multiple GitHub API calls per issue, this unnecessarily increases total runtime for repositories with more than a handful of closeable issues.
### Issue Context
This change was introduced by adding `await` in the loop. We still want to await completion, but with controlled parallelism to reduce wall-clock time while avoiding rate-limit spikes.
### Fix Focus Areas
- src/no-response.ts[41-54]
- src/no-response.ts[134-144]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Commented-out API calls🐞 Bug ✓ Correctness
Description
removeLabels() and unmark() include commented-out awaited Octokit calls directly above the live
calls, leaving dead code in a production path. This increases maintenance risk by obscuring which
call shape is intended and invites accidental reintroduction of outdated code.
Code

src/no-response.ts[R78-88]

     if (plainLabels.includes(responseRequiredLabel)) {
+        //await this.octokit.rest.issues.removeLabel({ ...issue, name: responseRequiredLabel })
       await this.octokit.rest.issues.removeLabel({
-          owner,
-          repo,
-          issue_number: number,
+          ...issue,
         name: responseRequiredLabel
       })
     }

-      if (plainLabels.includes(optionalFollowUpLabel)) {
+      if (optionalFollowUpLabel && plainLabels.includes(optionalFollowUpLabel)) {
+        //await this.octokit.rest.issues.removeLabel({ ...issue, name: optionalFollowUpLabel })
       await this.octokit.rest.issues.removeLabel({
-          owner,
-          repo,
-          issue_number: number,
Evidence
There are multiple instances of commented-out //await this.octokit... lines adjacent to the actual
awaited calls, indicating refactor leftovers rather than intentional documentation.

src/no-response.ts[78-92]
src/no-response.ts[110-126]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
There are multiple commented-out `//await this.octokit...` lines next to the active code paths. These are dead code artifacts and reduce clarity.
### Issue Context
The active calls already use the preferred `{...issue, ...}` parameter style, so the commented-out variants provide no runtime value.
### Fix Focus Areas
- src/no-response.ts[78-92]
- src/no-response.ts[110-126]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Verbose paginated debug log🐞 Bug ✓ Correctness
Description
getCloseableIssues() now logs JSON.stringify(issues) where issues contains the fully paginated
search results array, which can produce extremely large debug output. This makes logs harder to use
and increases noise when debug logging is enabled.
Code

src/no-response.ts[R173-180]

+    const issues = await this.octokit.paginate(
+      this.octokit.rest.search.issuesAndPullRequests,
+      { q, sort: 'updated', order: 'asc', per_page: 100 }
+    )

   core.debug(`Issues to check for closing:`)
   core.debug(JSON.stringify(issues, null, 2))
Evidence
After switching to octokit.paginate, issues represents the complete aggregated result set; the
code then stringifies and logs it in full.

src/no-response.ts[173-180]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getCloseableIssues()` logs the entire paginated issues array via `JSON.stringify(issues, null, 2)`. With pagination enabled, this can be very large and reduces the usefulness of debug logs.
### Issue Context
The code already logs per-issue details later in the filter; the initial full dump is usually redundant.
### Fix Focus Areas
- src/no-response.ts[173-180]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@tcely tcely merged commit 259aea4 into main Mar 17, 2026
1 check passed
@tcely tcely deleted the tcely-fix-pagination branch March 17, 2026 02:17
@tcely tcely changed the title fix(no-response): request pagination fix(no-response): rework GitHub API usage Mar 17, 2026
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