ReviewOS

also looking at this

stacks/buddy

fix(review): honour the ai.review settings that nothing read

#1406
Merged glennmichael123 wants to merge fix/honour-review-filters into main
7 files +282 -8
9 days ago

Five settings under ai.review were typed, validated by config-validation.ts, and documented — then read by nothing.

enabled              → only appeared in a workflow comment string
autoReview           → 0 read sites
drafts               → 0 read sites
ignoreTitleKeywords  → only in config-validation.ts
ignoreUsernames      → only in config-validation.ts

A repository that set ai.review.enabled: false still got a review on every pull request.

The generated workflow asserted the opposite. src/setup.ts wrote this into every buddy.yml:

# Skips itself when ai.review.enabled is false, so adding the trigger
# does not turn reviews on by itself.

That was never true of the command underneath it.

The fix

reviewSkipReason(config, pr, trigger) decides in one place whether a pull request should be reviewed. Both paths that post a review consult it before fetching a diff or contacting a model, so an ignored pull request costs one API call rather than a review's worth of tokens.

SettingSkips when
enabled: falseAlways — including an explicit @buddy review
autoReview: falseThe trigger is automatic; asking still works
draftsThe PR is a draft and this is not true
ignoreTitleKeywordsTitle contains one, case-insensitively as a substring
ignoreUsernamesAuthor is listed, compared case-insensitively

All but enabled apply only to an automatic trigger. Someone who types @buddy review on a draft has already answered the question the filter exists to ask. enabled is a statement about the repository, not one pull request, so it refuses an explicit request too.

buddy review takes --auto for this, and the generated workflow now passes it — which makes that comment true. Without the flag the command assumes a person at a terminal: a filter nobody asked for should not swallow a review somebody requested.

Why unset does not mean off

Review already requires both a workflow trigger and a provider key. Treating an unset flag as a refusal would have silently disabled review for every existing repository, so only an explicit false turns anything off.

The type doc claimed enabled defaults to false "until you opt in". That was never how the code behaved, and implementing it literally would have been a breaking change. It now describes the override it actually is.

[!NOTE] One behaviour does change: a draft pull request is no longer reviewed on an automatic trigger unless ai.review.drafts is true. That is what the setting has always documented, and @buddy review still works on drafts.

Tests

14 new, covering each setting and the trigger distinction — including two edge cases worth calling out:

  • a blank entry in ignoreTitleKeywords must not match every title
  • ignoreUsernames matches whole logins, so bob does not ignore bobby

Verification

  • 1910 pass / 0 fail (+14), lint clean, typecheck clean
  • check:docs green: 75 pages, 33 commands, 221 config keys
  • Docs site builds: 73 pages

🤖 Generated with Claude Code

https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.