buddy review <pr> reimplemented the pull request review that runReviewForPR already did, and the two had drifted apart in exactly the ways a second copy always does.
What each path did
buddy review <pr> | @buddy review | |
|---|---|---|
| Analyzers | ❌ skipped | ✅ |
| Learnings | ❌ never loaded | ✅ |
| Guidelines | ✅ | ✅ |
| Analyzer findings shown to the model | ❌ merged after | ✅ passed in |
The analyzer skip was right there in the condition:
const analysis = config.analysis?.enabled === false || prNumber
? { findings: [], ran: [], skipped: [] }
: await runAnalyzers({ ... })Why that fell the wrong way round
The generated workflow runs:
run: bunx @buddysh/buddy review ${{ github.event.pull_request.number }} --auto --verboseSo the automatic review every repository gets on every pull request was the path missing secret scanning, workflow auditing, and every learning @buddy remember had recorded.
@buddy remember worked, wrote its learning, opened a PR for it — and the review that ran on the next push never read it. The feature only ever applied for someone who asked for a review by hand.
The fix
The command's pull request branch now delegates, the way fix-ci was moved onto runFixCi in #1413. runReviewForPR gains profile and dryRun so the flags the CLI advertises still mean something, and bin/cli.ts loses ~100 lines.
The local path is untouched — it has no provider, no pull request to post to, and its own output formats.
Testability was the root cause
runReviewForPR now takes an injectable ai, for the same reason attemptFix has one: an orchestration that builds its own client can't be exercised without a key, and this one drifted from its second caller for exactly that long.
The tests use it to assert the two things that were actually missing — not that a file was read, but that:
- a recorded learning reaches the model's prompt
- a planted credential is found by static analysis on a pull request with no AI configured at all
Deliberately unchanged
A local review still runs without repository guidelines. It has no provider to read the base branch through, and reading them from the working tree would be a different thing: guidelines are trusted context, and the working tree is whatever the person running the command has just edited.
Tests
2025 → 2034.
Lint ✓ · typecheck ✓ · check:docs ✓ · 2034 pass / 0 fail
🤖 Generated with Claude Code