Skip to content

review: honour Gerald .github/NOTIFIED in the reviewer #264

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
jeresig merged 4 commits into from
Jul 23, 2026
Merged

review: honour Gerald .github/NOTIFIED in the reviewer #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6bae6b
[jeresig/review-workflow-notified-144eb8] review: honour Gerald .gith…
jeresig Jul 16, 2026
7d22998
[jeresig/review-workflow-notified-144eb8] review: address NOTIFIED re…
jeresig Jul 20, 2026
4a56602
[jeresig/review-workflow-notified-144eb8] review: polish NOTIFIED per…
jeresig Jul 21, 2026
56c407f
[jeresig/review-workflow-notified-144eb8] review: close NOTIFIED test…
jeresig Jul 23, 2026
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
5 changes: 5 additions & 0 deletions .changeset/review-notified.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"review": minor
---

Honour Gerald `.github/NOTIFIED` in the reviewer. New `lib/notified.ts` parses the file's `[ON PULL REQUEST]` notify rules (ignoring the documentation preamble above the `----…ignored!----` marker and the `[ON PUSH WITHOUT PULL REQUEST]` section) and matches them against the PR: path globs against the changed-file list, quoted `"/body/flags"` diff regexes against each file's staged unified diff. It groups the matches by `@mention`, carries each rule's label, and renders a ready-to-insert `### Notified` block. On approval, review.md Step 7 runs the CLI (`REVIEW_REPO_ROOT=$GITHUB_WORKSPACE npx -y tsx workflows/review/lib/notified.ts`) and pastes that block into the Review Guidance comment, so a matched person/team is pinged and told the label and which changed files matched them; the notification set feeds the comment's post trigger and its `risksPatternsKey` idempotency signature so a repost re-pings only when the set changes. The matching is a self-contained, practical subset of Gerald's micromatch (`**`, `*`, `?`, `{a,b,c}`, `[…]`, `(a|b)`, `?(…) *(…) +(…) @(…)`) anchored at the repo root, so the lib stays dependency-free (runs under `npx tsx` with no install); it is not a faithful micromatch reimplementation (wildcards match dotfiles, `!(…)` is unsupported — documented in the README). An unsupported glob construct degrades to no match; a malformed rule (bad regex body, unterminated quote) is dropped and surfaces a `Note:` on the PR — never a crashed review. The base-branch copy of `.github/NOTIFIED` is read (like `.github/REVIEWERS`), so a PR cannot inject notify rules that fire before it merges. Delivery of the pings still depends on the workflow's gh-aw `mentions:` allow-list (repository collaborators are allowed by default); the README documents how to widen it for teams. No existing review semantics change; a repo with no `.github/NOTIFIED` gets no section.
42 changes: 42 additions & 0 deletions workflows/review/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,48 @@ and are skipped; routing degrades to fewer lenses, never to a crashed review.
stays the model-facing prose about file *contents*; team ownership stays in
`.github/REVIEWERS`, unchanged.

### The `.github/NOTIFIED` file (optional)

If the repo has a Gerald [`.github/NOTIFIED`](https://khanacademy.atlassian.net/wiki/spaces/FRONTEND/pages/598278672/Gerald+Documentation)
file, the reviewer honours its **notify** rules (distinct from `REVIEWERS`
*reviewer* ownership): **on approval** it adds a `### Notified` section to the Review
Guidance comment that `@`-mentions each matched person/team, telling them the rule
label and which changed files matched. `lib/notified.ts` parses it and does the
matching deterministically (review.md Step 7 runs the CLI and pastes its rendered
block); no file means no section, so it costs nothing where it is absent.

**Delivery is approval-time, not on-touch.** The pings ride in the (approval-only)
Review Guidance comment, so — unlike Gerald, which notifies on every push — a
watcher is pinged when the reviewer approves, and a PR held at REQUEST_CHANGES or
merged before the AI verdict lands never pings them. This is intentional: the
notification piggybacks on the one comment the reviewer already posts, and firing
only on a clean approval keeps the ping meaningful and idempotent (below). Where a
repo still runs Gerald itself, Gerald's own on-touch NOTIFIED pings continue
independently — expect both until Gerald's NOTIFIED is retired for that repo.
Comment thread
jeresig marked this conversation as resolved.

Only the `[ON PULL REQUEST]` section applies (everything above the
`----Everything above this line…----` marker and the `[ON PUSH WITHOUT PULL
REQUEST]` section are ignored). Each rule is `[label:] <pattern> @user @Org/team`,
where `<pattern>` is either a **path glob** (matched against changed file paths) or
a **quoted diff regex** `"/body/flags"` (matched against each file's unified diff,
so a rule can fire on *added content*). The base-branch copy is read (like
`REVIEWERS`), so a PR cannot add notify rules that take effect before it merges.
The glob dialect is a **practical subset** of Gerald's micromatch, not a faithful
reimplementation (`**`, `*`, `?`, `{a,b,c}`, `[…]`, `(a|b)`, `?(…) *(…) +(…) @(…)`),
anchored at the repo root. Two known divergences: wildcards here match dotfiles
(micromatch defaults `dot:false`) and `!(…)` negation is unsupported — a rule that
relies on those may match a slightly different set than Gerald. An unsupported glob
construct matches nothing rather than crashing the review; a malformed rule (bad
regex body, unterminated quote) is dropped and adds a `Note:` to the PR review.

Because the notified `@mentions` ride in the Review Guidance comment (an
Comment thread
jeresig marked this conversation as resolved.
`add-comment` safe output), gh-aw's mention sanitizer governs whether they
actually ping: repository collaborators are allow-listed by default
(`mentions.allow-team-members`), and to let arbitrary teams/users through, widen
it with a `mentions:` block (`allowed`, `allowed-teams`; `allowed-teams` needs a
token with `read:org`). Without that, a non-collaborator or team mention is
rendered but neutralised (shown, not pinged).

### Re-review modes (the runs-per-PR cost lever)

The workflow reviews every push, so a PR's lifetime cost is runs-per-PR times
Expand Down
Loading
Loading