Summary
Code review of #5640 surfaced five pre-existing defects in the three scripts under .github/scripts. None is a regression from the ESM conversion, and all sit in lines that PR did not change, so they are collected here for a separate fix.
post_release.js
fetchIssues calls issues.listForRepo without pagination or per_page, so at most 30 closed pending-release issues are relabelled per release. PRs carrying the label count against the same page. There are 24 such items today, so a release that closes a handful more starts silently leaving issues on pending-release.
updateLabels calls fetchIssues without passing core, so the catch in fetchIssues dereferences undefined. An API error therefore surfaces as TypeError: Cannot read properties of undefined (reading 'setFailed') and the real cause is hidden.
updateLabels iterates with issues.forEach(async …), discarding the promises. The exported handler resolves before any setLabels call completes, and a failure is only reported through the process-level unhandled-rejection hook, twice, with non-deterministic log order.
release_patch_package_json.js
- The manifest is read from the argv-derived
packageJsonPath but written back with writeFileSync('package.json', …), which is cwd-relative. Every prepack caller passes ., so this is latent, but running node .github/scripts/release_patch_package_json.js packages/logger from the repo root would overwrite the root package.json with the logger manifest. The usage string invites exactly that call.
report_e2e_sweep.js
findIncident fetches one page of 100 open issues and PRs and matches the incident title client-side. Once open items exceed 100 and the incident issue is older than the newest 100, it is not found and a duplicate incident issue is created on every failed sweep.
Why is this needed?
Items 1 and 3 affect release labelling silently: issues can stay on pending-release after a release with the workflow reporting success. Item 2 makes any failure in that path harder to diagnose. Items 4 and 5 are latent today but are one plausible call or one busy month away from corrupting the root manifest or spamming incident issues.
Which area does this relate to?
Automation
Solution
post_release.js: fetch with github.paginate(github.rest.issues.listForRepo, { …, per_page: 100 }); pass core into fetchIssues; replace forEach(async …) with await Promise.all(issues.map(async …)) or allSettled so failures are awaited and reported once.
release_patch_package_json.js: write to packageJsonPath instead of 'package.json'.
report_e2e_sweep.js: paginate the open-issue lookup, or narrow it with a dedicated label or a title search, so the incident issue is found regardless of position.
Fixes can be verified with mocked Octokit clients as the review did; none needs a live workflow run except the release labelling path, which runs on the next release.
Acknowledgment
Future readers
Please react with 👍 and your use case to help us understand customer demand.
Summary
Code review of #5640 surfaced five pre-existing defects in the three scripts under
.github/scripts. None is a regression from the ESM conversion, and all sit in lines that PR did not change, so they are collected here for a separate fix.post_release.jsfetchIssuescallsissues.listForRepowithout pagination orper_page, so at most 30 closedpending-releaseissues are relabelled per release. PRs carrying the label count against the same page. There are 24 such items today, so a release that closes a handful more starts silently leaving issues onpending-release.updateLabelscallsfetchIssueswithout passingcore, so thecatchinfetchIssuesdereferencesundefined. An API error therefore surfaces asTypeError: Cannot read properties of undefined (reading 'setFailed')and the real cause is hidden.updateLabelsiterates withissues.forEach(async …), discarding the promises. The exported handler resolves before anysetLabelscall completes, and a failure is only reported through the process-level unhandled-rejection hook, twice, with non-deterministic log order.release_patch_package_json.jspackageJsonPathbut written back withwriteFileSync('package.json', …), which is cwd-relative. Everyprepackcaller passes., so this is latent, but runningnode .github/scripts/release_patch_package_json.js packages/loggerfrom the repo root would overwrite the rootpackage.jsonwith the logger manifest. The usage string invites exactly that call.report_e2e_sweep.jsfindIncidentfetches one page of 100 open issues and PRs and matches the incident title client-side. Once open items exceed 100 and the incident issue is older than the newest 100, it is not found and a duplicate incident issue is created on every failed sweep.Why is this needed?
Items 1 and 3 affect release labelling silently: issues can stay on
pending-releaseafter a release with the workflow reporting success. Item 2 makes any failure in that path harder to diagnose. Items 4 and 5 are latent today but are one plausible call or one busy month away from corrupting the root manifest or spamming incident issues.Which area does this relate to?
Automation
Solution
post_release.js: fetch withgithub.paginate(github.rest.issues.listForRepo, { …, per_page: 100 }); passcoreintofetchIssues; replaceforEach(async …)withawait Promise.all(issues.map(async …))orallSettledso failures are awaited and reported once.release_patch_package_json.js: write topackageJsonPathinstead of'package.json'.report_e2e_sweep.js: paginate the open-issue lookup, or narrow it with a dedicated label or a title search, so the incident issue is found regardless of position.Fixes can be verified with mocked Octokit clients as the review did; none needs a live workflow run except the release labelling path, which runs on the next release.
Acknowledgment
Future readers
Please react with 👍 and your use case to help us understand customer demand.