[SaintPatrck]

🎟️ Tracking

Stacked on #276, from a review suggestion there to keep the repo consistent.

📔 Objective

Skills are how the rest of the marketplace packages user-invocable workflows, so we move /validate-ai and /validate-ai-local from commands/ to skills/ to match. The slash names are unchanged, so the validate-ai action's /claude-config-validator:validate-ai prompt and anyone running them by hand keep working.

🤖 AI-assisted review

A Standard review (code-review-local) ran on the branch and approved. Its one finding, stale "command" wording in the intentionally duplicated notes, is fixed on the branch. It could not run plugin-dev:skill-reviewer, so the two moved SKILL.md files have not had a skill-quality pass.

[github-actions]

Claude Code validation

Result: Pass

Re-ran against the same merge base as the previous round (8aa784a), so the baseline is unmoved and this is a re-check of the standing findings rather than a fresh audit of the fix commits. Both findings from the 15:30 round were fixed — 8ba1d3f dropped the ambiguous report-ownership sentence outright, and 0af8ce0 shortened the two README pointers below the file's wrap width. The README.md → docs/ move in 3653bdc rewrote every relative link correctly for its new depth, and nothing in the repository still points at the old locations.

Not covered: validate-plugin-structure.sh, validate-marketplace.sh, and validate-version-bump.sh are not run by this review — the workflow runs them as dedicated steps and each reports its own result there.

Findings

• ⚠️ IMPORTANT: Finding 1: PATCH understates a component-layout change, and the sibling plugin shipped the identical commands/ → skills/ migration as MINOR (plugins/claude-config-validator/.claude-plugin/plugin.json:3) (raised once more in the resolved thread, with new precedent evidence)
• 🎨 SUGGESTED: Finding 2: Both relocated descriptions are still written as /help labels, with no activation trigger, now that description is the model-facing matching surface (plugins/claude-config-validator/skills/validate-ai-local/SKILL.md:5)

Already raised on the pull request — not re-posted:

• The allowed-tools grant on skills/validate-ai/SKILL.md:4 no longer landing in any grant-auditing pass of this plugin's own pipeline. Answered by a maintainer: the gap predates the move and closing it "belongs in its own change that covers every skill". The thread is open, so it carries itself.
• disable-model-invocation not set on either relocated skill (skills/validate-ai/SKILL.md:2, skills/validate-ai-local/SKILL.md:2). Resolved twice with "It was already model invocable" — a rejection of the premise, so the check that re-derived it this round was dropped rather than re-raised. Noted only because the same premise is what Finding 2 deliberately does not rest on.

Dropped after documentation check:

• skills/validate-ai/SKILL.md:3, skills/validate-ai-local/SKILL.md:3 — argument-hint re-reported as slash-command-only frontmatter carried into a skill. Docs: "argument-hint: Hint shown during autocomplete to indicate expected arguments. Example: [issue-number] or [filename] [format]."
• skills/validate-ai/SKILL.md:4 and docs/pull-request-validation.md — claim that describing allowed-tools as pre-approving the report write is slash-command semantics misapplied to a skill. Docs: "It does not restrict which tools are available: every tool remains callable, and your permission settings still govern tools that are not listed." The files' own wording is accurate.

Fixed since the previous round

Both findings from the 15:30 round are confirmed fixed at 0af8ce0:

• skills/reviewing-claude-config/SKILL.md:177 — the sentence "The skill owns the single write of the report document … and any inline comments" is gone entirely, rather than re-qualified. Step 5's opening ("This skill produces findings. It does not deliver them anywhere, so never post a comment") and line 17 ("Produce findings and stop. Delivering them belongs to the caller") still carry the rule, and the report contract lives in reference/validate-ai-scope.md, which both remaining routing cases point at. No gap left by the removal. Fixed in 8ba1d3f.
• skills/validate-ai/SKILL.md:20 and skills/validate-ai-local/SKILL.md:17 — the two pointer lines are now 74 and 88 characters, inside the file's ~88-character prose wrap. Fixed in 0af8ce0.

Checked and clean

• The docs/ move (3653bdc) — every relative link was re-pointed for the shallower depth and all of them resolve: ../skills/reviewing-claude-config/reference/validate-ai-scope.md, ../README.md, ../skills/reviewing-claude-config/SKILL.md, and the reciprocal local-validation.md / pull-request-validation.md links. Both skills' ${CLAUDE_PLUGIN_ROOT}/docs/*.md pointers resolve. A repo-wide search for skills/validate-ai/README, skills/validate-ai-local/README, and commands/validate-ai returns nothing, so the move left no dangling reference. The plugin-structure tree in README.md:373-393 was updated to match.
• Manifest and structure — plugin.json parses; name is kebab-case and matches the directory and the marketplace entry; version is valid semver. Dropping the commands key is correct, since skills/ is auto-discovered and no commands/ directory remains. No hooks and no MCP config in this plugin, and the changeset adds neither.
• Version consistency — 2.3.1 is identical in plugins/claude-config-validator/.claude-plugin/plugin.json, .claude-plugin/marketplace.json:21, and the root README.md catalog table, with a matching Keep a Changelog entry. Only the magnitude is in question (Finding 1), not the propagation.
• The rename, in both directions — no entry-point reference was left as "command" (reviewing-claude-config/SKILL.md lines 15, 31, 54, 131 and validate-ai-scope.md lines 4, 104, 162 all read "skills"), and no reference to the generic commands/**/*.md reviewed file type was wrongly converted: the routing table row, its explanation at lines 107-109, the bucket list, and the slash-command interpolation rules all still say "command", which is correct. Converting any of those would have been the serious error available in this diff, and it was avoided.
• The four targeted skills — the one-line edit is byte-identical across all four, and the duplication note it sits in enumerates 7 locations that all exist and all carry the boundary paragraph.
• Secrets — no credentials, tokens, keys, or connection strings in any added line across the manifests, changelog, READMEs, docs/, or the scope reference. No settings.local.json in the changeset. No settings or hook files changed, so permission-scoping and auto-approval checks did not apply.
• Prompt injection (CWE-1427) — the changed files contain a great deal of text addressed to Claude, which is this plugin's entire function. None of it is addressed to a reviewer of these files or attempts to shape this review's verdict; the untrusted-data passages point the other way, instructing the skills to distrust their inputs. All of it was treated as data and none of it executed. Each of the six subagents carried the same boundary and reported the same conclusion independently.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR moves /validate-ai and /validate-ai-local from commands/ to skills/ in claude-config-validator and adds name frontmatter that matches the existing slash names. It also rewords "command" to "skill" across the router, the scope reference, and the four targeted skills, and bumps the plugin to 2.3.1 with a changelog entry. Since the last review, the two skill READMEs moved to docs/local-validation.md and docs/pull-request-validation.md. Their relative links, the plugin README table and tree, and the ${CLAUDE_PLUGIN_ROOT}/docs/... pointers in each SKILL.md all resolve, and the Requirements and Troubleshooting sections those pointers name exist; nothing in the repository still references commands/validate-ai*. The recent commits close the open report-ownership, unwrapped-line, and sibling-README threads, and the open allowed-tools audit thread is not raised again here.

Not covered: Skill review did not run. This review path cannot launch plugin-dev:skill-reviewer, so plugins/claude-config-validator/skills/validate-ai/SKILL.md and plugins/claude-config-validator/skills/validate-ai-local/SKILL.md were not checked for description quality, length, or progressive disclosure. performing-multi-agent-code-review covers them where plugin-dev is installed.

[claude]

⚠️ IMPORTANT: The move takes this file's allowed-tools grant out of the only pass in this plugin's own pipeline that audits tool grants.

Details and fix

This is a different claim from the settled model-invocability threads above — it is about which validation bucket the file lands in, not about who may invoke it. The grant on line 4 is unchanged by this PR; what changed is whether anything reviews it.

At the old path, commands/validate-ai/validate-ai.md matched the Command-files bucket (skills/reviewing-claude-config/reference/validate-ai-scope.md:45) and routed to reviewing-command-definitions (skills/reviewing-claude-config/SKILL.md:92), which owns the grant check — "Where the command declares allowed-tools, check the grant against what the body actually runs."

At the new path it matches the Skill-files bucket, which the router marks "Not reviewed here" (skills/reviewing-claude-config/SKILL.md:95), and step 4b of this very file scopes skill-reviewer to "skill frontmatter, description and trigger quality, word count and writing style, progressive disclosure, and referenced files that do not exist" — no permission scoping. Line 149 then says "4c deliberately does not review SKILL.md files."

So the grant on line 4 — five Bash(...) prefixes plus Edit(//tmp/validation-summary.md) on a fixed path in a world-writable directory — and the matching grant in skills/validate-ai-local/SKILL.md:4 are now covered by no pass in the pipeline these files define. Nothing is exposed today; a future widening of either line would pass this pipeline unexamined. The broader gap (no skill's allowed-tools is grant-audited) predates this PR — the move is what pulls these two files into it.

Fix — narrow the carve-out at lines 149–150 so it excludes only the duplicate-prose concern, not the grant pass:

4c deliberately does not review `SKILL.md` prose — a second rule set over the same file
produces duplicate findings a reader cannot tell from independent confirmation. A changed
`SKILL.md` that declares `allowed-tools` is still routed to `reviewing-command-definitions`
for the tool-grant pass, which 4b does not cover.

Reference: https://code.claude.com/docs/en/skills (allowed-tools grants tools for the invoking turn)

_validate-ai

[SaintPatrck]

This gap predates the move. No skill's allowed-tools gets grant-audited today, and the suggested carve-out wouldn't close it anyway, since the reviewing-claude-config router still marks SKILL.md as not reviewed there.

Grant-auditing skills belongs in its own change that covers every skill, not just these two.

[claude]

🎨 SUGGESTED: Neither moved file points at its sibling README.md, which the move turned into a skill-directory support file.

Details and fix

Raised independently by the skill reviews of both relocated files. Under commands/validate-ai/, the sibling README.md was plainly human documentation sitting next to a command, and the router excludes a command's sibling README from review by name. Under skills/validate-ai/ it is a bundled file inside a skill directory — the layout progressive disclosure is built around — and nothing in SKILL.md routes a reader to it.

Both READMEs hold material the running skill cannot see and would benefit from: the plugin-dev install requirement, the reasoning behind each allowed-tools grant, and troubleshooting entries for the exact failure modes each SKILL.md tells it to handle (skills/validate-ai/README.md:120-182, skills/validate-ai-local/README.md:63-179).

Fix — add one pointer to each. After this line in skills/validate-ai/SKILL.md:

Requirements, the reasoning behind each pre-approved tool, and troubleshooting are in
`${CLAUDE_PLUGIN_ROOT}/skills/validate-ai/README.md`.

and the equivalent line in skills/validate-ai-local/SKILL.md, pointing at its own README.

Reference: https://code.claude.com/docs/en/skills (progressive disclosure through bundled files)

_validate-ai

[SaintPatrck]

READMEs moved to plugin docs/ folder and renamed to reflect the process they describe.

[claude]

⚠️ IMPORTANT: This rename collapses the one word that distinguished the caller from this skill, and the sentence it lands in grants exactly the capability line 170 forbids.

Details and fix

Before this edit the sentence read "The command owns the single write of the report document, the labels it gives each severity, and any inline comments." The noun did the disambiguating work: this file is a skill, its caller was a command, so "the command owns ... any inline comments" could only mean the caller.

Now both actors are skills, and the sentence reads "The skill owns ... any inline comments" seven lines after the file's own prohibition at line 170:

This skill produces findings. It does not deliver them anywhere, so never post a comment, even where a comment-posting tool happens to be available: callers that post run the findings through their own classification and validation first, and posting directly would bypass that.

Throughout the rest of this file, bare "this skill" / "the skill" refers to reviewing-claude-config itself — line 170, line 15 ("no step here depends on it"), line 268 ("those skills, which hold grants this skill does not"). So the nearest-referent reading of line 177 is that reviewing-claude-config owns the report write and the inline comments, which is the precise behavior line 170 exists to prevent. The stated consequence of getting it wrong is in line 170 itself: findings reach the pull request without the caller's classification and validation pass.

This is the one edit in this file where the rename removed information rather than just updating a label. The other five all sit next to an explicit validate-ai qualifier ("the two validate-ai skills", "both validate-ai skills"), which keeps them unambiguous.

Fix — restore the disambiguator by naming which skill:

Suggested change

	IMPORTANT / SUGGESTED / OPTIONAL classification. The skill owns the single write of the
	report document, the labels it gives each severity, and any inline comments.
	IMPORTANT / SUGGESTED / OPTIONAL classification. The calling `validate-ai` skill owns the
	single write of the report document, the labels it gives each severity, and any inline
	comments.

_validate-ai

[SaintPatrck]

Addressed by removing the unnecessary reasoning.

[claude]

🎨 SUGGESTED: The two added README pointers are the only unwrapped lines in either file, and nothing in the toolchain will rewrap them.

Details and fix

This line is ~121 characters; the prose around it wraps at ~88, consistently, for the whole file. The same applies to the matching line added in skills/validate-ai-local/SKILL.md:17.

Worth fixing by hand rather than leaving to the formatter: there is no Prettier config in the repo, so prettier --check . runs with the default proseWrap: "preserve" and will not reflow prose. pnpm run lint passes either way, which means this does not get corrected on its own — it just makes these two lines render as outliers in every future diff of a file whose wrap is otherwise uniform.

Fix — wrap to match the surrounding prose:

Suggested change

	Requirements and the reasoning behind each pre-approved tool are in `${CLAUDE_PLUGIN_ROOT}/skills/validate-ai/README.md`.
	Requirements and the reasoning behind each pre-approved tool are in
	`${CLAUDE_PLUGIN_ROOT}/skills/validate-ai/README.md`.

and the equivalent break in skills/validate-ai-local/SKILL.md:17.

_validate-ai

[SaintPatrck]

Addressed by removing the unnecessary reasoning from the Skill.

[claude]

🎨 SUGGESTED: Both relocated descriptions are still written as /help labels, but under skills/ the description is what the model matches on.

Details and fix

Independent of the two settled threads above — this is not about whether the model should invoke these, but about the description doing its job given that it may. It applies the same either way, and more so if autonomous loading is intended.

The docs make description load-bearing specifically for the skills/ layout:

Your existing .claude/commands/ files keep working. Skills add optional features: a directory for supporting files, frontmatter to control whether you or Claude invokes them, and the ability for Claude to load them automatically when relevant.

Both strings came across from commands/ unchanged, where they only had to render in a list. This file's version spends roughly half its length on the output location and never says when to use it — and CHANGELOG.md:124 records that it was tuned for exactly that older job ("names the report location, which is the string /help shows").

It also lost the asymmetry race against its sibling: skills/validate-ai/SKILL.md:5 enumerates "(plugins, skills, agents, commands, hooks, CLAUDE.md, .claude/)" while this one does not, so "check my skills and agents before I push" has more lexical overlap with the pull request skill than with the local one.

This is the plugin's own published rule, in skills/reviewing-claude-config/reference/claude-code-requirements.md:30:

description: MUST include activation triggers (when to use the skill)

and reviewing-claude-config/SKILL.md:3 already follows it ("Use when reviewing changes to CLAUDE.md, agents, prompts, commands, hooks, or settings").

Fix — add the enumeration and a use-when clause here:

description: Validate the Claude Code material changed in the local checkout (plugins, skills, agents, commands, hooks, CLAUDE.md, .claude/), covering branch commits plus uncommitted work. Use before pushing, or when asked to validate, check, or review Claude config locally.

skills/validate-ai/SKILL.md:5 has a milder version of the same gap — it enumerates the material but still never says when to use it; appending "Use when asked to validate or review a pull request's Claude configuration, by PR number or URL." would close it.

Reference: https://code.claude.com/docs/en/skills

_validate-ai

[SaintPatrck]

Speculative. Have not observed the described failure, so this will stay as-is. If triggering misses are observed we will address at that time.