Skip to content

A label is a second way to ask either bot for a review - #7

Open
henokteixeira wants to merge 2 commits into
mainfrom
henok/eng-613-a-label-is-a-second-way-into-a-bot-review
Open

A label is a second way to ask either bot for a review#7
henokteixeira wants to merge 2 commits into
mainfrom
henok/eng-613-a-label-is-a-second-way-into-a-bot-review

Conversation

@henokteixeira

Copy link
Copy Markdown
Contributor

The fact, measured

Both reusable workflows guard their only job with:

if: >-
  github.event.action == 'review_requested' &&
  github.event.requested_reviewer.login == inputs.reviewer_login &&
  ...

On shemaobt/facilitator-desk that condition can never be met. Every pull request there is opened by @henokteixeira, and GitHub refuses a review request aimed at a pull request's own author422 Review cannot be requested from pull request author. So Henokinho has never reviewed anything on that repository.

The runs are not missing, which is the part worth knowing: #66, #67, #68 and #69 each have a Henokinho Review run, fired by the review request aimed at @joaocarvoli, that finishes skipped in one or two seconds on the login test. From the outside it looks configured and working.

The change

A label_name input, and a second arm on the job's if:

if: >-
  (
  (github.event.action == 'review_requested' && github.event.requested_reviewer.login == inputs.reviewer_login)
  ||
  (github.event.action == 'labeled' && github.event.label.name == inputs.label_name)
  )
  && github.event.pull_request.draft == false
  && !endsWith(github.event.pull_request.user.login, '[bot]')

Additive, and the existing way in is untouched. The review request is still the org's own shape and is what works the day a pull request comes from somebody else; the login test is scoped to its own arm because a labeled event carries no requested_reviewer at all. Draft and bot-authored pull requests are excluded on either way in, as before.

No caller changes behaviour by this merge. A caller reaches the label arm only by putting labeled into its own on: pull_request: types:. Every caller that has not still dispatches on review_requested alone, whatever label_name defaults to. The default is the bot's own name rather than an empty string, so that a caller that does opt in and forgets to name a label does not get a run that starts, skips and looks identical to the guard working.

Joãozinho takes the same change. The two files are deliberately one shape copied rather than a shared abstraction, and fixing one of them is exactly how that copy starts to diverge. I read both before touching either: they differ in bot_login / verdict_mode / max_turns, none of which touches this.

Four sentences that would have gone stale

Each said the bot runs only on a review request, and each is rewritten here rather than left behind the diff:

  • the if comment in both workflows — including the note that a team request leaves requested_reviewer null, which is still true and is now scoped to the arm it belongs to;
  • the prompt in both workflows, which told the bot why it was running;
  • README.md, which is the one-paragraph description of how either bot fires;
  • henokinho/review-style.md, which is binding spec the bot reads.

The YAML detail that nearly shipped wrong

The readable version of that if — inner clauses indented to show the nesting — parses, lints clean, and is not one expression. >- folds only across lines at the same indentation; a more-indented line keeps its newline, so what would reach GitHub is a multi-line string. Caught by parsing the file and counting lines in the result, not by reading it. Every line of the expression now sits at one indentation, and the reason is written beside it.

What holds this

  • actionlint clean on henokinho-review.yml. joaozinho-review.yml reports one SC2129 style warning that is pre-existing — same single finding on main, only the line number moves — in a run: block this PR does not touch.
  • The folded if was read back out of the parsed YAML and is a single line in both files.
  • henokinho/tests/test-check-bot-login.sh — 1 behaviour, 0 failed. henokinho/tests/test-collect-prior-passes.sh — 7 behaviours, 0 failed.
  • Nothing here proves a run. The caller side is shemaobt/facilitator-desk#69, whose trigger already carries labeled; that pull request is where labelling produces a real review, and it cannot until this merges.

Both reusable workflows fired only on a review request aimed at their
configured human. On a repository where that human opens the pull requests
himself, that condition can never be met: GitHub answers a review request
aimed at a pull request's own author with `422 Review cannot be requested
from pull request author`. facilitator-desk is such a repository, and
Henokinho has never once reviewed anything there — every run since the
workflow landed finished `skipped` in under two seconds.

A `label_name` input is added and the job's `if` gains a second arm for
`labeled`. The review-request arm is untouched, and so is every caller: a
caller reaches the new arm only by putting `labeled` into its own
`on: pull_request: types:`, so one that has not behaves exactly as before.
Draft and bot-authored pull requests are still excluded on either way in.

Joãozinho takes the same change in the same commit. The two workflows are
deliberately a copy of one shape rather than a shared abstraction, and
fixing one of them is how that copy starts to diverge.

Four sentences said the bot runs *only* on a review request — the two `if`
comments, the two prompts, the README and Henokinho's own spec. All are
rewritten here rather than left to go stale silently.
@linear-code

linear-code Bot commented Aug 28, 2026

Copy link
Copy Markdown

ENG-613

Nothing gates the default here against the guard each opted-in caller writes
in its own job-level `if`. That guard is deliberate — without it an unrelated
label spends an invocation of this workflow just to be turned away, which grows
with a repository's label list and is invisible, because being turned away is
the outcome either way. Two copies with nothing between them stay in step only
by naming each other.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant