Install the ai-config plugin by default in claude.yml and claude-code-review.yml (opt out via use-ai-config) - #321
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
claude.yml and claude-code-review.yml both gain plugin-marketplaces/ plugins workflow_call inputs, passed through to claude-code-action's own plugin_marketplaces/plugins mechanism. Empty by default and purely additive: claude-code-review.yml's built-in anthropics/claude-code.git marketplace and code-review@claude-code-plugins plugin are appended to, never replaced, via new extra-plugin-marketplaces/extra-plugins inputs on the run-claude-review-attempt composite action. This lets a consumer opt an @claude agent and/or reviewer into another plugin marketplace -- e.g. d-morrison/ai-config, which already ships a .claude-plugin/marketplace.json and is installable as ai-config@d-morrison from https://github.com/d-morrison/ai-config.git.
VerdictReady for merge |
|
Code review Reviewed the full diff (adds opt-in Verification performed:
One minor finding was posted inline: the Note: the PR description still reads "WIP --- opened up front to claim the issue; implementing now," but the diff itself is a complete, working implementation. Worth updating the description before merge so reviewers or mergers do not mistake it for an in-progress stub. No bugs, security issues, or blocking CLAUDE.md violations found. |
VerdictReady for merge (the minor documentation-clarity nit noted above is optional, at the author's discretion). |
|
Claude finished review — View run Review complete for d-morrison/gha#321. Process: dispatched 2 CLAUDE.md-compliance agents + 2 bug/security agents in parallel over the diff, independently fact-checked their claims against Findings: one minor, non-blocking documentation-clarity issue — the Posted the findings summary and a separate One housekeeping note: while working around a Bash-tool restriction on markdown headings mid-body, an early throwaway test comment ( VerdictReady for merge (pending the optional documentation-clarity fix above, at the author's discretion). |
|
Working on this --- paws off until I'm done. Following up on the review above: per user direction, extending this PR so the Generated by Claude Code |
Both claude.yml and claude-code-review.yml now install ai-config@d-morrison from d-morrison/ai-config unless the caller sets use-ai-config: false, so a consumer repo picks up the lab's shared skills, commands, and review conventions without listing them itself. The plugin-marketplaces / plugins inputs added earlier in this PR stay, now scoped to FURTHER marketplaces/plugins layered on top of the default rather than being the only way to get ai-config. Both remain empty by default and purely additive: claude-code-review.yml's built-in code-review plugin is never replaced. Also addresses the review's inline finding: "each marketplace name must match one listed in plugin-marketplaces" was ambiguous, since that input holds URLs rather than names. Reworded at all four sites to say the name comes from the marketplace's own .claude-plugin/marketplace.json. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBKiU7139xzvrmZV2YDG9r
2026-07-27 05:10 UTC |
Code reviewReviewed the current head commit ( Verification performed on the new diff:
No bugs, security issues, or CLAUDE.md violations found in the new diff. The default-on behavior change itself is the PR's stated intent (per d-morrison's own follow-up comment directing it), not a design concern to flag. VerdictReady for merge |
|
Claude finished review — View run Posted the review comment to the PR: #321 (comment) SummaryThe PR's current head commit ( Verified and clean:
No new bugs, security issues, or CLAUDE.md violations found. VerdictReady for merge |
Ready for mergeVerified at head
Which reviewer this rests on. The clean call is the review bot's alone. Copilot was requested at this head and declined at 02:14:55 UTC with "unable to review this pull request because the user who requested the review has reached their quota limit" — the third such refusal on this PR, after The one thing worth a human's eye before merging, since no reviewer can settle it: this flips behavior for every existing consumer of Not merging - leaving that call to you. Generated by Claude Code |
* ums: prefer a toggle over flipping a list input's default Encodes the design trap hit on Morrison-Lab/gha#321. When a list-valued extension point that shipped empty should later include a built-in entry by default, flipping that input's own default silently breaks every caller who uses the input for its original purpose: a caller-supplied value replaces the default rather than adding to it, so adding an entry of their own drops the built-in one, with nothing to error on. Adds a section to configurable-parameters.md covering the separate-toggle alternative, the doc-sync it implies for the list input's own description, and the matching review-side check for the inverse finding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBKiU7139xzvrmZV2YDG9r * Scope the toggle rule to named defaults; exempt varargs Per review: listing "a `...` of extras" among the exposed extension points was wrong, not merely loose. Varargs have no named default for a caller's value to displace, so they are append-only by construction and cannot hit this footgun at all. Drops `...` from the enumeration and states the scoping condition directly: the trap needs a named parameter carrying a displaceable default. Names varargs as the shape that already behaves the way the section argues for, so a reader can tell which of their own extension points qualify. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBKiU7139xzvrmZV2YDG9r --------- Co-authored-by: Claude <noreply@anthropic.com>
* ums: default new owner-repo capabilities on, opt out for others Learned on Morrison-Lab/gha#321 (closing #319): built the ai-config plugin-marketplace passthrough as opt-in-only, reasoning gha's multi-tenancy meant opt-in was the safer default. The user's actual intent was on-by-default for their own tooling with an opt-out for other consumers -- extended after I'd already reported the PR ready. * fix: remove em-dash from the new preferences.md bullet * fix: break the two-sentence line at its sentence boundary (semantic line breaks)
Closes #319
Both
claude.yml(the@claudeagent) andclaude-code-review.yml(the reviewer)now install the
d-morrison/ai-configplugin (
ai-config@d-morrison) by default, so a consumer repo picks up thelab's shared skills, commands, and review conventions without listing them itself.
Consumers opt out with
use-ai-config: false.What changed
use-ai-configtrueai-config@d-morrison. Set false to opt out.plugin-marketplaces''plugins''Everything is additive:
claude-code-review.yml's built-incode-review@claude-code-plugins(and itsanthropics/claude-code.gitmarketplace) is never replaced, only added to.
The
plugin-marketplaces/pluginsinputs were opt-in-only in this PR's firstcommit; they are unchanged in shape, just re-scoped to further sources on top of
the new default rather than being the only route to ai-config.
Wiring
claude.ymlcomposes the marketplace/plugin lists inline, gated on its ownboolean input.
claude-code-review.ymlpassesuse-ai-configthrough torun-claude-review-attempt, which owns the reviewer's composition, at bothcall sites (the initial attempt and the gha#185 stub-retry).
claude.ymlandrun-claude-review-attempt/action.ymlrather than shared: acomposite action cannot
uses:a sibling local action, since a relative pathresolves against the caller's checkout instead of the action's (gha#284), so
a shared composite would break for every consumer. Both sites carry a comment
pointing at the other.
Verification
d-morrison/ai-configis public ("private": falsevia the repos API), sothe anonymous clone the plugin install performs works in a consumer's CI. This
matters because upstream's
installPlugins()stops on the first install errorand throws, so an unreachable marketplace would fail the whole run.
d-morrison) and plugin name (ai-config) were read from.claude-plugin/marketplace.jsonin that repo, not assumed.claude-code-action(
be7b93b, v1.0.183):parseMarketplaces()andparsePlugins()inbase-action/src/install-plugins.tsboth trim and filter empty entries, so anunset slot in the block scalar adds nothing.
check-new-line-breaksreports no missing semantic breaks on the diff; noem-dashes or other non-ASCII punctuation in any added line.
Review finding addressed
The inline finding on the previous round was correct: "each marketplace name must
match one listed in
plugin-marketplaces" was ambiguous, since that input holdsURLs rather than names. Reworded at all four sites to state that the name comes
from the marketplace's own
.claude-plugin/marketplace.jsonand is not derivedfrom the URL.
Docs
README table (both rows),
website/workflows.qmd(both rows),website/reference/claude.qmdandclaude-code-review.qmd(Inputs tables plusexample blocks),
examples/claude.ymlandexamples/claude-code-review.yml(example blocks, now showing the opt-out), and two
changelog.d/fragments(
addedfor the inputs,changedfor the new default).