ReviewOS

also looking at this

stacks/buddy

fix!: make strategies and labels behave as documented

#1399
Merged glennmichael123 wants to merge fix/strategy-and-label-semantics into main
7 files +187 -27
9 days ago

Three defects that shared a cause: a config key was declared, validated and documented, and then not read by the code that needed it.

Update strategy meant two different things

filterUpdatesByStrategy in src/buddy.ts and strategyAllows in src/rules/engine.ts each carried their own switch, and they disagreed:

strategyglobal filter (before)rule-levelnow
alleverythingeverythingeverything
majormajor onlymajor onlymajor only
minormajor + minorminor + patchminor + patch
patcheverythingpatch onlypatch only

So the same word filtered differently depending on where you wrote it, and patch — the setting that sounds safest — was the one admitting breaking changes. strategyAllows is now exported and both paths resolve through it, so they cannot drift apart again.

[!WARNING] This is a breaking behaviour change. A repository on strategy: 'patch' was silently receiving majors and will now receive only patches. Move to all to keep the old behaviour.

Worth deciding on a version bump before this merges: ci.yml runs pantry publish:commit on every green push to main, so a merge is immediately consumable.

pullRequest.labels never reached a pull request

Declared, validated and documented — and read by nothing on the live path. generatePRLabels (src/buddy.ts:2203) does read it and has zero callers; the live path calls prGenerator.generateLabels(group), which does not.

Rule labels had a second, quieter bug: they were unioned only when a PR was created, so refreshing an existing PR dropped them.

A single labelsFor now unions the automatic set, pullRequest.labels, and rule effects — and all five call sites go through it.

Smaller corrections

  • @buddy resolve and @buddy plan removed from HELP_TEXT. Both were advertised there with no registered handler, so the bot offered them and then replied "I don't know how to resolve yet."
  • src/setup.ts wrote "Automatically checks every minute" into the job summary of every generated workflow. It runs on the daily cleanup tick; rebase requests arrive on the PR-edited event. Fixed in the template and in this repo's committed buddy.yml.

Verification

  • 1885 pass / 0 fail (+9 new tests), lint clean, typecheck clean
  • New tests pin the strategy semantics and assert the global and rule-level paths agree — which is what would have caught this originally
  • Five new tests cover the label union, including the pullRequest.labels case that would have failed before

Part 1 of 3. Followed by docs/correct-unsupported-claims, then ci/check-docs-against-source.

🤖 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.