Skip to content

Fall back from a refused APPROVE only on 422 #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
william-gr merged 1 commit into from
Sep 13, 2026
Merged

Fall back from a refused APPROVE only on 422 #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ said, and is where a repo puts the paths it always wants eyes on.

Each run adds a review; GitHub reviews are appended, not edited, so a PR with
ten pushes carries ten of them, and the newest is the one that describes the
head commit. REQUEST_CHANGES and APPROVE change the PR's state; the two
head commit. REQUEST_CHANGES and APPROVE change the PR's state; the
COMMENT outcomes do not.

Nothing is submitted when the reviewer crashed, on purpose: a changes-requested
Expand Down
4 changes: 4 additions & 0 deletions review/submit-verdict.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@
'Nothing blocks and no human review is needed.',
]);
} catch (error) {
// Only a refusal falls back. A request that got no answer (the 15s
// timeout, a dropped connection) may have created the review anyway,
// and commenting then would dismiss the approval that landed.
if (!/HTTP 422/.test(error.message)) throw error;

Check notice on line 318 in review/submit-verdict.mjs

View workflow job for this annotation

GitHub Actions / claude-review / Automatic PR review 

LOW: The refusal test /HTTP 422/ is duplicated here and in approveHint (line 251), so widening the refusal set requires editing both; extract a shared isRefusal predicate.

@github-actions github-actions Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — "is this a refusal" is now the literal /HTTP 422/ in two places: here, and inside approveHint at line 251. Because line 320 is only reached once this test has already passed, approveHint's own guard is a no-op on this path — it still earns its keep at line 341, where the outer catch calls it on arbitrary errors.

The cost is that the two have to move together. If the refusal set ever widens — GitHub returning 403 for the Actions-approve setting, say — widening only this one gives a fallback COMMENT with no hint explaining it, and widening only approveHint prints the hint on a path that never falls back. A shared predicate says once what a refusal is:

const isRefusal = (error) => /HTTP 422/.test(error.message);

used by both approveHint and the guard here. Not blocking; the behaviour today is right.

console.log(`::warning::could not approve: ${escapeData(error.message)}`);
approveHint(error);
const own = await submit('COMMENT', [
Expand Down
Loading