[cloclo] Fix locked error check: require 403 status AND locked message#15980
Merged
[cloclo] Fix locked error check: require 403 status AND locked message#15980
Conversation
This fixes the locked resource error check logic to properly require BOTH a 403 status code AND a locked message before suppressing the error. The previous logic was incorrectly simplified to ignore any error with a locked message, regardless of status code. This could mask legitimate errors that mention "locked" but aren't actually resource-locked errors. Changes: - Updated error check from `(is403Error && hasLockedMessage) || (!is403Error && hasLockedMessage)` to `is403Error && hasLockedMessage` - Updated comments to clarify that BOTH conditions are required - Updated tests to verify non-403 errors with locked messages are not suppressed Files modified: - actions/setup/js/add_reaction.cjs - actions/setup/js/add_reaction.test.cjs - actions/setup/js/add_reaction_and_edit_comment.cjs - actions/setup/js/add_reaction_and_edit_comment.test.cjs Fixes #15977 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
pelikhan
approved these changes
Feb 15, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes overly-broad “locked resource” error suppression in the reaction scripts so that errors are only ignored when they are both a 403 and contain a locked-related message—preventing non-403 “locked” failures from being silently swallowed.
Changes:
- Tighten the locked-resource suppression condition in both reaction handlers to
is403Error && hasLockedMessage. - Update unit tests to assert that non-403 errors mentioning “locked” correctly fail instead of being suppressed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| actions/setup/js/add_reaction.cjs | Require both 403 status and locked message before suppressing the error. |
| actions/setup/js/add_reaction_and_edit_comment.cjs | Same tightened suppression logic for the reaction+comment flow. |
| actions/setup/js/add_reaction.test.cjs | Add coverage ensuring non-403 “locked” errors fail. |
| actions/setup/js/add_reaction_and_edit_comment.test.cjs | Add coverage ensuring non-403 “locked” errors fail in the combined flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
10 tasks
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
This PR fixes the locked resource error check logic to properly require BOTH a 403 status code AND a locked message before suppressing the error.
Problem
The previous logic in PR #15977 was incorrectly simplified:
This simplifies to just
hasLockedMessage, which means errors like:Would all be silently suppressed, potentially masking real issues.
Solution
The correct logic requires BOTH conditions:
This ensures we only suppress GitHub API 403 errors specifically related to locked conversations/issues/PRs.
Changes Made
Code Changes
Test Changes
Testing
✅ All tests passing:
add_reaction.test.cjs: 27 tests passedadd_reaction_and_edit_comment.test.cjs: 19 tests passedTest Coverage
Why This Matters
Proper error handling is critical. We should only suppress errors we know are expected and safe to ignore. Suppressing all errors mentioning "locked" regardless of status code could hide:
Related
Fixes #15977Based on feedback from @pelikhan
🤖 Generated with [Claude Code]((claude.com/redacted)