Merged
Conversation
- Import and use getErrorMessage() from error_helpers.cjs instead of inline error instanceof Error checks (3 occurrences removed) - Add `/** @type {Record<string, number>} */` annotation for runsPerEvent - Fix GITHUB_WORKFLOW_REF env var not being cleaned up in test beforeEach - Add 5 new test cases: issue_comment/discussion_comment/pull_request event handling, stack trace logging, env var cleanup verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pelikhan
approved these changes
Mar 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Cleans up check_rate_limit.cjs error handling by standardizing error-message extraction, and improves the check_rate_limit test suite by fixing an environment-variable pollution issue and adding coverage for default event behavior and stack-trace logging.
Changes:
- Use shared
getErrorMessagehelper incheck_rate_limit.cjsand add a JSDoc type forrunsPerEvent. - Fix test isolation by clearing
GITHUB_WORKFLOW_REFinbeforeEach. - Add tests for default programmatic vs non-programmatic events and for stack trace logging on errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| actions/setup/js/check_rate_limit.cjs | Replaces inline error-to-message logic with getErrorMessage and adds typing for runsPerEvent. |
| actions/setup/js/check_rate_limit.test.cjs | Prevents env var leakage across tests and adds new cases covering default event behavior + stack trace logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Mar 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cleaned
check_rate_limit.cjs(github-script context) as part of the jsweep JavaScript unbloat initiative.Changes
check_rate_limit.cjsgetErrorMessagefromerror_helpers.cjsand use it throughout, replacing 3 instances of inlineerror instanceof Error ? error.message : String(error)pattern/**@type{Record(string, number)} */forrunsPerEventvariable to improve type safetycheck_rate_limit.test.cjsdelete process.env.GITHUB_WORKFLOW_REFtobeforeEachcleanup (was missing, could cause test interference)issue_commentevents are rate-limited by default (programmatic event)discussion_commentevents are rate-limited by default (programmatic event)pull_requestevents are NOT rate-limited by default (non-programmatic)Context
Validation ✅
npm run format:cjs✓npm run lint:cjs✓npm run typecheck✓check_rate_limittests pass ✓ (pre-existing unrelated failures inassign_issue,mcp_handler_go,frontmatter_hash_github_api,safe_outputs_tools_loaderare not caused by these changes)Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.