Skip to content

🏗️🔧:make the queue say why it could not start - #896

Open
DerekNonGeneric wants to merge 1 commit into
mainfrom
infra/queue-explains-itself
Open

🏗️🔧:make the queue say why it could not start#896
DerekNonGeneric wants to merge 1 commit into
mainfrom
infra/queue-explains-itself

Conversation

@DerekNonGeneric

@DerekNonGeneric DerekNonGeneric commented Sep 4, 2026

Copy link
Copy Markdown
Member

The commit queue is silent in exactly the failure it is most likely to
produce first.

The gap

Both steps that report back to a pull request are gated on the app
token:

- name: Take the label back off
  if: always() && steps.token.outcome == 'success'

- name: Say why it did not land
  if: failure() && steps.token.outcome == 'success'

They have to be — reporting takes a token. But that means a token that
never minted produces a bare red Land check, no comment, and the
label still sitting on the pull request as though the queue had yet to
look at it.

That is the first thing a queue nobody has run before will do if
anything is wrong with its credentials, and the two causes — the app
not installed on the repository, and a private key that lost its
BEGIN/END lines on the way into a secret — are indistinguishable
from outside. Both are live unknowns here: I can confirm the secrets
reach the repo, but not that the app is installed, which needs
admin:org.

The fix

A workflow annotation needs no token and no permission, so it is the
one report that survives having no credentials at all. It names both
causes and says the label has to be taken off by hand — unlabeled is
not a trigger, so reapplying it would otherwise start nothing.

No permissions change. The job keeps contents: read; widening
GITHUB_TOKEN to issues: write just to post a failure comment is a
larger surface than this warrants on a pull_request_target workflow.

Also: a comment that was wrong

The note above those steps claimed they "need only the pull request
permission the app already has". They do not — labels and comments are
issue endpoints even when the number belongs to a pull request, so
they need the app's Issues: write. Per GitHub's permissions
reference
. Left uncorrected it is an hour spent wondering why a
queue that can merge cannot comment.

Summary by CodeRabbit

  • Documentation

    • Clarified that label and comment API operations require the app’s issue-writing permission.
  • Bug Fixes

    • Added clearer workflow error reporting when the GitHub App token cannot be created, including relevant label-state details.

Every step that reports back to a pull request is gated on the app
token, because reporting takes one. So a token that never minted
reports nothing at all: a bare red check, no comment, and the label
still sitting there as though the queue had yet to look.

That is the first failure a queue nobody has run before will produce,
and the two things that cause it -- an app not installed on the
repository, and a private key that lost its BEGIN and END lines on the
way into a secret -- are indistinguishable from the outside.

A workflow annotation needs no token and no permission, so it is the
one report that survives having no credentials at all. It names both
causes and says the label has to come off by hand, since `unlabeled`
is not a trigger and reapplying it would otherwise start nothing.

The note above the two reporting steps also claimed they need only the
app's pull request permission. They do not: labels and comments are
issue endpoints even when the number belongs to a pull request, so
they need `Issues: write`. Left uncorrected that is an hour spent
wondering why a queue that merges cannot comment.

Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is>
Assisted-by: Claude-Code:claude-opus-5
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 46111593-eded-4cd3-bd00-232b3fc95e64

📥 Commits

Reviewing files that changed from the base of the PR and between 90f87d8 and ac2e557.

📒 Files selected for processing (1)
  • .github/workflows/commit-queue.yml

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The commit queue workflow now documents Issues: write permission requirements and reports GitHub App token-minting failures with an unauthenticated workflow error annotation.

Changes

Commit queue workflow

Layer / File(s) Summary
Permission documentation and token failure reporting
.github/workflows/commit-queue.yml
The workflow documents Issues: write for label and comment API calls. It reports token-minting failures, explains that no work occurred, and states that the queue label remains applied.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ac2e5

The workflow now explains token-minting failures without credentials and documents the required Issues permission. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: the commit queue now explains why it could not start. The emojis add minor noise, but the title remains clear and relevant.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch infra/queue-explains-itself

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant