fix: keep the low-tier promises — bulk-update label, event-name validation, workflow personalisation - #1434
Merged
Merged
Conversation
…ation, workflow personalisation Four declared-but-never-wired findings from the sweep's low tier: - generateLabels now applies the documented bulk-update label to PRs carrying five or more updates; the docs promised it under Dynamic Labeling before any code read it. - validateConfig gains validateNotifications: EVENT_NAMES was exported 'for config validation' and no validator imported it, so a mistyped notification event silently subscribed a sink to nothing. - generate-workflows honours workflows.enabled: false (explicit false only, the dashboard's reading). - workflows.custom[].assignees was validated and dropped — the template did not accept the field, and reviewers/labels were accepted and then discarded. The generated workflow now passes per-workflow overrides to ./buddy update as --reviewers/--assignees/--labels flags (new on the update command), and stops baking global pullRequest values into YAML, where they would shadow every later config edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… existsSync Under CI's file order, test/dependency-file-parser.test.ts's node:fs module mock leaks into cli-surface, and existsSync returned undefined. Bun's native glob cannot be reached by a node:fs mock, and asserting the directory holds exactly buddy.config.ts is the stronger claim anyway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
glennmichael123
force-pushed
the
fix/low-tier-wiring
branch
from
August 31, 2026 11:39
97b5e81 to
d04bc1b
Compare
glennmichael123
added a commit
that referenced
this pull request
Aug 31, 2026
…sted modules (#1436) * fix(review): read what publishing reported, honour reviewSuggestions, and un-strand two flags Four review-side findings, the last of the sweep's low tier: - reviewPullRequest now reads the ReviewSubmissionResult every provider already returned: a review that failed to post says so instead of claiming success (and records no state, so the next run does not skip a review nobody can see), and inline comments the platform dropped are counted in the status and warned about. - prepareReview/renderFinding take the provider's reviewSuggestions capability: Bitbucket declares it false and was receiving literal suggestion fences promising a one-click apply it does not have; it now gets a plain code fence labelled as a suggested replacement. - buddy review --auto passes skipIfReviewed, so the workflow that fires on every edited event stops re-reviewing the same head commit; a person at a terminal still always gets their review. - buddy review --light --fix applies suggestions: the light branch returned before the fix block, so the flag worked everywhere except the one mode fast enough for a pre-commit hook. The review-parity stub answered createReview with { id: 1 } — encoding the defect that the result was ignored; it now returns what the interface declares. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(test): defuse the suite's three landmines - The node:fs module mock in dependency-file-parser leaked into every file that ran after it: mock.module is process-global and permanent, so once afterEach reset the mocks, existsSync returned undefined suite-wide — which is exactly how #1434's decline test failed on CI while passing alone. The mock now delegates to the real fs except while this suite has armed it. - preload.ts gains a repair-only cwd net: a suite that chdirs into a temp directory and deletes it leaves process.cwd() dead, and every later filesystem-touching test fails with an unrelated ENOENT. A live cwd is never moved. - test/composer-constraint-updates.test.ts was a 0-byte file that ran and asserted nothing — coverage in every listing, a test of nothing. It now holds the suite its name promises: caret, tilde, exact and compound constraints keep their shape, require-dev updates, and an unknown package leaves the file untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: cover the dossier's two untested modules, and say bun install is not optional - src/ai/providers/anthropic.ts was the one AI provider without a direct test (google and openai have them) — and it is the default when several keys are present. The SDK is faked at the module seam so complete() runs its real mapping: block collection, the stop-reason table (refusal preserved — it arrives as a successful response with empty content), usage with cache reads, schema-constrained JSON parsing, request passthrough, and the lazily built, reused client. - src/utils/lock-file.ts: 188 lines, publicly re-exported, no direct test. The pure parts are asserted directly; regenerateLockFile is exercised through its non-fatal contract — a machine without the package manager gets a result object naming the failure, not an unhandled rejection (which channel reports it is platform-dependent: node fires 'error', Bun closes with a negative code). - README: bun install is a hard prerequisite on a fresh clone (bun test otherwise fails with phantom module errors), and the declared git hooks do not install themselves — bunx bun-git-hooks activates them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ai): assert the anthropic mapping at the fetch layer, not through a second module mock The new anthropic-provider file owned its own mock.module of the SDK — process-global, so under CI's file order it clobbered ai-provider's fetch-stubbed anthropic tests, which started receiving the fake's canned response. The same landmine class this branch exists to defuse. The genuinely new cases move into ai-provider.test.ts through the real SDK: the remaining stop reasons (stop_sequence, max_tokens, unknown), schema-constrained JSON parsing and its malformed-JSON failure, and the wire shape (system, tools mapped to input_schema, output_config, max_tokens default) captured from the request body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four declared-but-never-wired findings from the sweep's
lowtier, all the same defect class this run has been retiring: things typed, validated and documented that no code ever read.bulk-update label
docs/features/pull-requests.mdpromisesbulk-update: For PRs with 5+ package updates under Dynamic Labeling. Nothing applied it — a filter watching for the label saw every grouped PR slip past.generateLabelsnow adds it at five or more updates.Notification event names are validated
EVENT_NAMESwas exported with the comment "for config validation and subscription lists" — and no validator imported it. A mistypedevents: ['pr.opened']built the sink, read the credential, and filtered every real event out: notifications configured, nothing ever delivered.validateConfiggainsvalidateNotifications, checkingslack/discord/webhooks[]— event names againstEVENT_NAMES, env-var names non-empty, webhookurlpresent and http(s).workflows.enabledDeclared and documented;
generate-workflowsgenerated the full set regardless. It now declines on explicitfalseonly — the same reading the dashboard uses, so a repository that never wrote aworkflowsblock keeps the command.workflows.custom[].assignees— and the personalisation that was already theaterThe field was declared and validated, and
generateCustomWorkflowdidn't even accept it. Worse:reviewersandlabelswere accepted — and then dropped bygenerateWorkflow, which never rendered them into the YAML. The generated workflow runs./buddy updateagainst whateverbuddy.config.tsthe checkout carries, so a per-workflow override can only travel through the command line:buddy updategains--reviewers/--assignees/--labels(comma-separated; fold intopullRequestonly when given, so an unsetpullRequestblock stays unset).pullRequestvalues are deliberately not baked into the YAML any more (scheduled workflows used to pass them into the void): the runtime reads them from config on every run, and freezing today's values into a workflow file would shadow every later config edit.Tests
bun test: 2165 pass / 0 fail. New:test/custom-workflows.test.ts(flags rendered on both invocations, no flags when no overrides, shell-quote hardening, globals not frozen); extensions totest/pr-labels.test.ts(5 ⇒ labelled, 4 ⇒ not),test/config-validation.test.ts(valid block passes; mistyped event, missing url, bad webhook events all reported),test/cli-surface.test.ts(decline honoured, no.githubwritten).Remaining
lowitems (review --light --fix,reviewSuggestions,skipIfReviewed) wait on #1431, which owns the review files.🤖 Generated with Claude Code