docs(rules): fix stale 5-plugin claims and make plugin inventory drift-resistant - #557
Conversation
.claude/rules/plugin-docs.md is loaded into every session's context, so its "## The 5 Plugins" section was actively steering plugin work away from claude-setup, skill-management, and spec-workflow. The repo has had 8 plugins for some time; the root CLAUDE.md already said 8, so the two contradicted each other. Rather than just correcting 5 to 8 (which drifts again on the next addition), both files now name packages/plugins/ and .claude-plugin/marketplace.json as the source of truth, give an enumeration command, and state that the enumerated output wins over the inline snapshot. - plugin-docs.md: rewrite "The 5 Plugins" as "The Plugins" with a source-of-truth block, an enumeration command, a one-line parity check between the directories and marketplace.json, and a dated snapshot that readers are told to repair on mismatch - plugin-docs.md: "Each of the 5 plugins has its own section" -> "Each plugin" - CLAUDE.md: same de-hardcoding for the inventory bullet and the same "Each of the 5 plugins" phrasing in the Notion sync section Left .plan/DEV-218-implementation.md alone: its "5 plugins" is a point-in-time justification in a completed migration plan, and rewriting it would make it wrong about the state it describes. No files under packages/plugins/ changed, so no plugin version bump applies.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🤖 Claude Code Review
Review: Documentation parity check for plugin inventoryThis is a docs-only PR that replaces hardcoded plugin-count prose ("the 5 plugins", "8 plugins") with a "source of truth = enumerate the directories + Verification performedInventory matches (8 plugins). Both parity commands run and exit silently against the current tree:
The Assessment
No blocking issues. Safe to merge. 💡 Want a fresh review? Add a comment containing |
📚 Documentation Check ✅Verdict: Passed This PR only touches documentation files (.claude/rules/plugin-docs.md, CLAUDE.md) fixing stale "5 plugins" references — no files under packages/plugins/ were modified, so no plugin version bump is required. The content changes are accurate and verified against the live repository state. SummaryThis PR fixes stale "5 plugins" claims in two documentation files and replaces hardcoded plugin counts/lists with drift-resistant guidance (enumerate via Verification performed:
Plugin version bump check: No files under Assessment: The changes are self-consistent, verified accurate against live repo state, and actually improve documentation maintainability by pointing at an enumeration command as source of truth instead of a hardcoded count that will drift again. No missing updates identified. ✨ No Documentation Updates NeededAll documentation appears to be up to date with the code changes. 🤖 Generated by Claude Documentation Validator | Mode: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review verdict: APPROVE
👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.
This formal review submission is for the verdict only.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ebf1cc538
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```bash | ||
| diff <(find packages/plugins -mindepth 1 -maxdepth 1 -type d -exec basename {} \; | sort) \ | ||
| <(jq -r '.plugins[].name' .claude-plugin/marketplace.json | sort) |
There was a problem hiding this comment.
When a plugin is added or renamed and its marketplace entry retains a copied or stale source, this command still succeeds as long as the entry's name matches the directory. The marketplace validator in .github/actions/validate-plugins/action.yml resolves and validates .plugins[].source, so this can falsely certify parity while the marketplace loads another plugin directory and the new plugin remains unreferenced. Compare the normalized source paths or their basenames against the directories, in addition to checking names.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, this is real. Fixed in b16f43e.
Verified the mechanism before changing anything: .github/actions/validate-plugins/action.yml lines 64-71 resolve .plugins[i].source into PLUGIN_PATH and hand that path to scripts/validate-plugin.cjs. name is only used for log output, so source is the field that actually decides which directory loads. Comparing names alone checks the label, not the pointer.
Reproduced the false pass against a scratch fixture (new plugin foo, entry copy-pasted so source still points at skill-management):
=== CURRENT check (names vs dirs) ===
>>> PASSES (falsely certifies parity)
=== PROPOSED check (sources vs dirs) ===
2c2
< packages/plugins/foo
---
> packages/plugins/skill-management
>>> CATCHES IT
Replaced the single check with two:
# 1. Every source resolves to a real plugin directory, and every directory is referenced once.
diff <(find packages/plugins -mindepth 1 -maxdepth 1 -type d | sort) \
<(jq -r '.plugins[].source | sub("^\\./"; "")' .claude-plugin/marketplace.json | sort)
# 2. Every entry's name agrees with its own source directory.
jq -r '.plugins[] | select((.source | sub("^\\./packages/plugins/"; "")) != .name)
| "MISMATCH: name=\(.name) source=\(.source)"' .claude-plugin/marketplace.jsonCheck 1 catches the duplicate-source and dangling-source cases; check 2 catches the name/source disagreement with a direct message. Both are silent against the current marketplace.json (all 8 entries agree), and I ran them verbatim as extracted from the rendered markdown to confirm the jq escapes survive prettier.
There was a problem hiding this comment.
Independently verified this is fully addressed in b16f43e. I traced the mechanism in .github/actions/validate-plugins/action.yml: PLUGIN_SOURCE is resolved into PLUGIN_PATH (lines 64-71) and that path is what's handed to scripts/validate-plugin.cjs (line 83); PLUGIN_NAME is only used for echo/::notice:: output. So source is indeed the field that decides which directory loads, and the original concern is correct.
Ran both replacement checks verbatim against the current tree — check 1 (sources vs dirs) and check 2 (name vs source basename) both exit 0 with no output, and all 8 entries agree. The duplicate/stale/dangling-source cases the original comment described would now be caught by check 1. Leaving unresolved since there's an active discussion on the thread.
The parity check compared .plugins[].name against the directory names. That passes even when an entry's `source` is stale or copy-pasted, because `name` is not the field that determines what loads. .github/actions/validate-plugins/action.yml:64-71 resolves .plugins[i].source into PLUGIN_PATH and validates that path; `name` is only used for log output. So an entry named `foo` whose source still points at `./packages/plugins/ skill-management` makes the validator check skill-management twice while `foo` goes unreferenced - and the old check certified that as parity. Reproduced against a scratch fixture: names-vs-dirs passed, sources-vs-dirs caught it. Replaced with two checks: (1) resolved source paths against the directories on disk, (2) each entry's name against its own source basename. Both verified silent against the current marketplace.json, and executed verbatim as extracted from the rendered file to confirm the jq escapes survive prettier.
wkoutre
changed the title
Superseded by new review after PR update
There was a problem hiding this comment.
📋 Review verdict: APPROVE
👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.
This formal review submission is for the verdict only.
1 participant
Problem
.claude/rules/plugin-docs.mdis a project rules file, so it loads into every session's context. Its## The 5 Pluginssection listed only 5 of the repo's 8 plugin directories, omittingclaude-setup,skill-management, andspec-workflow. Any agent following that rule would skip those three when doing plugin work.It also contradicted the root
CLAUDE.md, which already said 8.Verified actual inventory (8, and directories match
marketplace.jsonexactly):Approach
Correcting
5to8would drift again on the next plugin addition, so both files now derive the inventory instead of asserting it:packages/plugins/directories + thepluginsarray in.claude-plugin/marketplace.json.find/basenamerather thanls -1, which emits.and..under a commonlsalias).diffof the directory list againstmarketplace.jsonnames. Any output means the inventory is inconsistent.Same de-hardcoding applied to the
CLAUDE.mdinventory bullet.Changes
.claude/rules/plugin-docs.md## The 5 Plugins->## The Pluginswith source-of-truth block, enumeration command, parity check, dated snapshot of all 8.claude/rules/plugin-docs.mdCLAUDE.mdCLAUDE.mdGrep sweep
Swept the repo for other hardcoded plugin-count claims. One remaining hit, deliberately left:
.plan/DEV-218-implementation.md:17— "the 27 commands ... are covered by the 5 plugins". This is a point-in-time justification inside a completed migration plan. Rewriting it to 8 would make it wrong about the state it describes.(
packages/plugins/claude-setup/skills/setup-repository/references/boris-best-practices.md:21matches "The 5 Pillars" — unrelated false positive.)Verification
bunx nx format:write --uncommitted— cleanbunx markdownlint-cli2 --fix "**/*.md"— 196 files, 0 errorsmarketplace.jsonagreepackages/plugins/, so no plugin version bump applies