✨ Add /ai-coding-config doctor diagnostic subcommand

[TechNickAI]

Summary

• Adds a doctor subcommand to /ai-coding-config via a new <doctor-mode> section in the command file
• Runs a ✅/⚠️/❌ diagnostic battery grouped by category: plugin install state, symlinks, hook scripts, settings JSON, marketplace JSON consistency, version drift, skill frontmatter
• Context-aware: detects source-repo vs user-project and runs applicable checks
• Auto-fixes hook permissions (with confirmation); all other issues report with suggested fix commands

Design Decisions

Extension vs new command: Added as a subcommand to the existing /ai-coding-config command rather than creating a standalone file. Users invoke it as /ai-coding-config doctor, matching the familiar pattern from /ai-coding-config update. The XML mode section pattern already established in the command handles routing cleanly.

Read-only by default: Doctor only auto-fixes hook permissions (safe, reversible, no content change). Everything else gets a diagnosis and a fix suggestion — the user runs the fix explicitly. This prevents doctor from accidentally doing destructive things in a broken state.

Context-aware checks: Source-repo checks (symlinks, marketplace JSON parity) are skipped in user-project context where they'd always fail. User-project checks (plugin cache, settings enablement) are emphasized instead.

Skill frontmatter validation includes composition fields: Since #52 landed, doctor also verifies next-skill targets resolve — first live consumer of the new frontmatter schema.

Complexity

balanced

Validation

• Pre-commit markdown frontmatter validation passed
• Manually verified the doctor mode section is syntactically valid and follows existing command patterns

Fixes #53

[claude]

Code Review: /ai-coding-config doctor

Overall this is a solid addition — the design decisions (read-only by default, context-aware, progressive output) are well-considered and the XML block pattern fits naturally. A few things worth addressing:

---

Bug / Inconsistency: auto-fix example script name

The <auto-fix> section uses verify-deliverables.sh in the confirmation prompt example:

"⚠️ verify-deliverables.sh is not executable. Fix now? [y/N]"
If yes: chmod +x plugins/core/hooks/verify-deliverables.sh

But the <output-format> example shows todo-persist.sh as the non-executable script. These two code blocks should use the same script name to avoid confusing the LLM about which pattern to follow.

---

Inconsistency: bash commands contradict the file's own best practices

The command file has an explicit "Shell and Tool Best Practices" section stating:

Prefer native tools over bash for file inspection. … Never change working directory with cd. Use absolute paths for all file operations.

Yet the new <checks> section includes several bash examples that violate this:

# Relative paths (violates "use absolute paths")
ls -la .claude/commands .claude/agents .claude/skills
ls -la plugins/core/hooks/*.sh
ls -la rules

# Uses cat (prefer Read tool)
cat .claude/settings.json 2>/dev/null | python3 -m json.tool > /dev/null && echo "valid"

The symlink checks should instruct using Bash with $(pwd)-anchored paths. The JSON validation is better expressed as: "Read .claude/settings.json with the Read tool and verify it parses as valid JSON." Using python3 -m json.tool also introduces a dependency — Python 3 may not be available in all environments.

---

Ambiguity: next-skill resolution algorithm

The frontmatter check says:

If next-skill declared: ✅ that skill name exists in the skills directory or .claude/commands/

It's unclear whether "skill name" means the directory name (fast, no reads required) or the name field in each SKILL.md (accurate but requires reading every file). Since skills can theoretically have a directory name differ from their name field, this is worth making explicit. Suggesting: check directory names first (fast path), fall back to scanning name fields only if directory lookup fails.

---

Minor: next-skill target scope is underspecified for user-project context

In source-repo context, the skill path is plugins/core/skills/. In user-project context, it's ~/.claude/plugins/cache/ai-coding-config/skills/. The next-skill resolution should use the same context-aware path that's already established for the skill frontmatter scan — the current wording doesn't explicitly tie these together.

---

Positive observations

• Semver bump (4.1.1 → 4.2.0) is correct for a new feature
• The output format example is tight and scannable — exactly what a doctor output should look like
• The 9.21.0 version in the example matches the actual marketplace.json version, not a made-up placeholder
• Validating next-skill targets is a nice first consumer of the composition frontmatter from Upgrade skill frontmatter for declarative composition (pipelines, handoffs, model hints) #52
• Three hooks (session-multiplexing.sh, todo-persist.sh, verify-deliverables.sh) are correctly reflected
• The "report as you go" instruction in <objective> prevents a long silent pause before output

---

Suggested priority before merge:

1. Fix the verify-deliverables.sh / todo-persist.sh inconsistency (one example should be used consistently)
2. Update the bash examples in <checks> to use native tools or absolute paths to match the file's own guidelines

The rest are polish / clarifications that could ship as a follow-up.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0bbcac1240

ℹ️ 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".

[chatgpt-codex-connector]

Use user-project hook paths when running doctor outside source repo

The new doctor flow advertises context-aware behavior, but the hook checks always read plugins/core/hooks/..., which only exists in the source repository layout. In a normal user project (where plugins/core/ is absent by definition in this same section), this will incorrectly report missing hooks and can’t validate the installed plugin hooks at all, producing false failures for one of the core diagnostic categories.

Useful? React with 👍 / 👎.

[TechNickAI]

Fixed in fa53a64. Hook script file checks are now scoped to source-repo context only. In user-project context, the section instructs checking hooks registration in settings instead of looking for script files in plugin cache.

[chatgpt-codex-connector]

Validate global Claude settings instead of only local file

This check claims to verify hooks registration in “project or global settings,” but the command only parses .claude/settings.json in the current repo. If that file is absent (common when only ~/.claude/settings.json exists), piping empty input to python3 -m json.tool yields a parse error, so doctor reports a false JSON failure and never checks the actual global settings file.

Useful? React with 👍 / 👎.

[TechNickAI]

Fixed in fa53a64. The settings check now reads .claude/settings.json if it exists, falls back to ~/.claude/settings.json for projects using only global settings. Both cases report valid/warning/error states based on JSON validity and hook registration.

[TechNickAI]

Fixed all three items (5ecf04d):

• auto-fix example: changed to todo-persist.sh to match the output-format example
• bash relative paths: replaced ls -la examples with $(pwd)-anchored absolute paths and Glob/Read tool instructions — consistent with the file's own best practices
• next-skill resolution: clarified the algorithm (directory name first as fast path, fall back to scanning name fields, also checks .claude/commands/)

Declined: next-skill scope underspecified for user-project context — the context-detection section at the top already establishes which skill path to use; no additional wording needed.

[claude]

Code Review: ✨ Add /ai-coding-config doctor diagnostic subcommand

Overall this is a well-designed feature. The subcommand architecture, read-only-by-default philosophy, context detection, and incremental output guidance are all solid choices. The second commit shows good responsiveness to multi-review feedback. That said, there are two real bugs that will affect users and a few smaller items worth addressing.

---

Must Fix

Hook Scripts check uses hardcoded source-repo paths in user-project context

The ### Hook Scripts section references plugins/core/hooks/hooks.json and plugins/core/hooks/*.sh. These paths only exist in the source repo. Users who installed via the plugin marketplace (the majority) will hit false ❌ failures on every hook check.

The <context-detection> section correctly identifies this distinction and gates symlink/marketplace-parity checks behind source-repo detection — but the hook check doesn't get the same treatment. It needs a branch:

• Source repo: check plugins/core/hooks/*.sh
• User project: check ~/.claude/plugins/cache/ai-coding-config/hooks/*.sh (or whatever the actual cache path is)

The auto-fix chmod +x suggestion also points to a source-repo path and will fail silently in user-project context.

---

Should Fix

Settings JSON validation only checks .claude/settings.json, not ~/.claude/settings.json

The check claims to verify "project or global settings" but only reads the project-level file. A user with no .claude/settings.json but a valid ~/.claude/settings.json (global-only config) will see a false ❌ Invalid JSON. The check should inspect both files and be explicit about which one it found.

next-skill resolution logic is underspecified

The skill frontmatter check says to verify next-skill targets "exist in the skills directory or .claude/commands/". But skills are identified by their name field in frontmatter, not their directory or file name. An LLM resolving this without explicit guidance may match against directory names, which will produce false positives/negatives. Should say: match the next-skill value against the name: field in each SKILL.md's frontmatter.

---

Nice to Have

Plugin cache subdirectory layout assumption

The plugin install check describes looking for "agents, commands, and skills subdirectories" in the cache. One of the fix-up commits noted the cache uses flat .md files without subdirectories — but this description in the install check wasn't updated. Worth verifying against the actual Claude Code cache layout and correcting the wording.

Explicit doctor routing in the preamble

The update subcommand has an explicit entry-point description in the preamble routing section. doctor relies on its XML section header for LLM discovery. Matching the update pattern with an explicit mention would be cleaner and more predictable.

---

Future / Out of Scope

Issue #53 listed "MCP servers referenced by skills not actually installed" as a target. The new requires: [mcp:name] schema from #52 enables this check — but it's not included here. Makes sense as a follow-up since the schema itself just landed.

---

Positive Notes

• Subcommand-not-new-file approach keeps the command surface clean and follows the established update pattern
• chmod +x auto-fix is appropriately scoped (hook scripts only, user-confirmed) — good judgment call
• Failure-first summary ordering (❌ → ⚠️ → ✅) is good UX
• Concrete annotated example output is excellent LLM instruction design — anchors what "good output" looks like far better than abstract descriptions
• version bump to 4.2.0 is appropriate for a new subcommand feature

The hook path branching fix is the highest-priority item — it will produce noisy false failures for most users in the field.

[claude]

Code Review

Clean implementation overall. The <doctor-mode> section follows the established XML-mode pattern from <update-mode> and the design rationale is solid. A few specific things worth addressing:

Issues

Hook registration check is incomplete

The ### Hook Scripts section validates that settings.json is valid JSON but never checks whether the hooks declared in hooks.json are actually registered in settings. A user could have all three hook scripts present and executable but hooks still silently not running because they were never wired into settings.json. This is the most likely real-world failure mode. The check should read hooks.json, extract the command paths, then verify those commands appear under the hooks key in settings.json (or the global ~/.claude/settings.json).

stability: experimental shown as ⚠️ without a defined check condition

In the output format example:

⚠️ mcp-debug — triggers field present, stability: experimental

But the <checks> section never defines stability: experimental as a warning condition. It's just metadata. This will confuse users who see a warning on a healthy skill. Either remove it from the example or add a corresponding check definition (e.g., "⚠️ experimental skills noted for awareness").

todo-persist.sh hardcoded in <auto-fix> example

"⚠️ todo-persist.sh is not executable. Fix now? [y/N]"
If yes: chmod +x plugins/core/hooks/todo-persist.sh

This uses a specific filename rather than a generic <hook-name>. If hooks are renamed or new ones added, this example becomes misleading. Use a placeholder like <hook>.sh to make it clear this is parameterized behavior.

Minor Notes

• The plugins/core/.claude-plugin/plugin.json path is real (verified in the repo) — the marketplace consistency check will work correctly.
• The next-skill fast-path by directory name is smart — directory name matches skill name in this repo's structure.
• The Glob pattern plugins/core/skills/*/SKILL.md correctly matches the subdirectory structure.
• Semver bump 4.1.1 → 4.2.0 is appropriate for a new subcommand.

Questions

1. For user-project context, the plugin cache check looks at ~/.claude/plugins/cache/ai-coding-config/ — is this the actual path Claude Code uses, or is it illustrative? If it's guessed, a failing check here would be confusing.

2. Should the skill frontmatter check in user-project context scan the flat .md files in the plugin cache (even partially)? Right now there's no skill validation for users who installed via marketplace, which is the majority of users.

Overall the feature is solid and fills a real gap. The hook registration gap is the most actionable thing before merge.

[claude]

Code Review: /ai-coding-config doctor

The design is solid and the PR description is thorough. The existing review already caught the most actionable issues (script-name inconsistency, bash vs native tools, next-skill scope). Here are a few additional observations that weren't covered:

---

Overlap with the existing <marketplace-doctor> section

<update-mode> already contains a <marketplace-doctor> subsection that checks the same three things as the new <doctor-mode> Plugin Install State group:

• known_marketplaces.json entry
• Plugin cache presence
• Plugin enablement in ~/.claude/settings.json

Having two sections define these checks independently creates a maintenance surface: if one is updated (e.g., a new settings key name), the other will silently diverge. Consider either:

• Factoring the shared checks into a named reference block that both sections call out, or
• Adding a comment noting that these checks intentionally mirror <marketplace-doctor> so they stay in sync.

---

mcp-debug example sends an unspecified signal

The <output-format> example shows:

⚠️ mcp-debug — triggers field present, stability: experimental

But the <checks> section never mentions that stability: experimental should produce a warning. The LLM will infer from the example that it should flag experimental skills, even though the spec doesn't document this behavior. Either:

• Remove stability: experimental from the example output (just show the ✅/⚠️ patterns you've specified), or
• Add an explicit rule: ⚠️ stability: experimental → note as informational

Right now the example is teaching behavior that isn't in the spec.

---

Missing file case for known_marketplaces.json

The Plugin Install State check says "Read ~/.claude/plugins/known_marketplaces.json with the Read tool" but doesn't handle the case where the file doesn't exist (fresh Claude Code install, never used the plugin marketplace). Add an explicit fallback:

• ❌ known_marketplaces.json not found → marketplace was never initialized; suggest running /plugin marketplace add ...

Without this, the Read tool will error and the LLM may handle it unpredictably.

---

No unrecognized-argument fallback

The three modes (setup-mode, update-mode, doctor-mode) rely on the LLM to route based on the argument, but none of the mode sections specify what to do for an unrecognized argument (e.g., /ai-coding-config diagnostics or a typo). Consider adding a one-liner: "If the argument doesn't match update or doctor, enter setup mode and mention the available subcommands."

---

Positive observations not already noted

• The <context-detection> block reporting its conclusion at the top of output is excellent — users will immediately know if the wrong context was detected.
• Scoping skill frontmatter checks to source-repo only (with the reason stated) is exactly the right call; it prevents meaningless failures in user-project runs.
• Limiting auto-fix to chmod only is the right level of conservatism for a diagnostic command.

Overall this is ready to merge with the script-name inconsistency fixed. The rest can be addressed as follow-up polish.

[TechNickAI]

Fixed all four in 8201b12:

• Marketplace overlap: added a sync comment to Plugin Install State noting it mirrors <marketplace-doctor> in update-mode
• known_marketplaces.json missing file: added explicit ❌ case for file not found vs. file-exists-but-entry-missing
• stability: experimental undocumented: added the rule to Skill Frontmatter checks so the spec matches the example
• Unrecognized-argument fallback: added a Mode Routing section at the top-level before all mode blocks (correct placement — the LLM reads this before deciding which mode to enter)

[claude]

Code Review — /ai-coding-config doctor

Overall this is well-designed. The subcommand pattern, read-only-by-default principle, and progressive output are all good calls. A few things worth addressing:

---

Bug: Icon inconsistency between spec and example output

The <checks> section (Skill Frontmatter) says:

If stability: experimental declared: ℹ️ note as informational

But the <output-format> example shows:

⚠️ mcp-debug — triggers field present, stability: experimental

⚠️ and ℹ️ have different semantics in the output format (warning vs info). An LLM reading this will encounter conflicting instructions and could emit either. The example should match the spec — change ⚠️ to ℹ️ for the mcp-debug line.

---

Minor: Hardcoded script name in <auto-fix> could mislead the LLM

"⚠️ todo-persist.sh is not executable. Fix now? [y/N]"
If yes: `chmod +x plugins/core/hooks/todo-persist.sh`

The example uses a specific filename. LLMs pattern-match on examples, and there's a small but real risk it substitutes todo-persist.sh literally instead of the actual failing hook name. A safer example:

"⚠️ <hook-name>.sh is not executable. Fix now? [y/N]"
If yes: `chmod +x plugins/core/hooks/<hook-name>.sh`

---

Design smell: Duplicated logic with <marketplace-doctor>

The sync comment at line 561 acknowledges this:

These checks mirror the ones in <marketplace-doctor> (update-mode). If either section is updated, keep both in sync.

This is the right thing to note, but it's a maintenance trap. Consider having <doctor-mode> explicitly reference <marketplace-doctor> with an instruction like "run the same plugin install state checks as in <marketplace-doctor>" — the LLM can read the sibling section rather than requiring manual sync.

---

Minor: Plugin enablement warning is underspecific

⚠️ Not listed → plugin may be disabled; run `/plugin` to check status

/plugin with no args probably shows a list but the user doesn't know what to do next. A more actionable suggestion: /plugin enable ai-coding-config (or whatever the correct subcommand is). If the exact syntax isn't stable, at least explain what to look for in the output.

---

Gap: No agent frontmatter check

Skill frontmatter is validated but agent frontmatter is not. If agents have required fields (name, description, color per the CLAUDE.md color scheme), a broken agent file would be invisible to doctor. Worth adding or explicitly calling out as out of scope.

---

Good things worth keeping

• Read-only by default with a single safe auto-fix (chmod) is the right design. The [y/N] default-no is the correct safer choice.
• Progressive output (print headers as checks complete) is a good UX call for a potentially slow sequential scan.
• Context detection (source repo vs user project) prevents the common failure mode of always-failing checks in user projects.
• Single summary line per skill rather than per-field keeps the output scannable.
• The version bump (4.1.1 → 4.2.0) is correctly semver for a new feature.

---

The icon inconsistency is the only thing that could cause incorrect output at runtime. Everything else is polish. Happy to see this land once that's addressed.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8201b12. Configure here.}

[cursor]

Example uses ⚠️ but spec says ℹ️ for experimental

Medium Severity

The check specification at line 651 explicitly says stability: experimental gets an ℹ️ and is "not a problem, just surfaced for awareness." However, the example output at line 704 renders mcp-debug with a ⚠️ warning icon instead. This contradiction means the AI agent may treat experimental skills as warnings (needing action) rather than informational notes, and the ⚠️ also inflates the summary warning count at line 708.

Additional Locations (1)

• plugins/core/commands/ai-coding-config.md#L650-L652

 

^{Reviewed by Cursor Bugbot for commit 8201b12. Configure here.}

[TechNickAI]

Good catch — fixed in PR #59. Changed ⚠️ to ℹ️ in the example output to match the spec.

[TechNickAI]

Great review! Here's how each item landed:

Fixed in PR #59:

• ✅ Icon inconsistency (⚠️ → ℹ️ for experimental stability) — the only runtime-impacting bug
• ✅ Hardcoded todo-persist.sh → generalized to <hook-name>.sh placeholder

Declined:

• 🚫 Plugin enablement warning underspecific — the correct subcommand is context-dependent; adding a potentially-wrong specific command could be worse than the current generic guidance
• 🚫 Duplicated logic with <marketplace-doctor> — tracked in issue refactor(doctor): reduce duplication between doctor-mode and marketplace-doctor checks #60 as an architectural improvement; out of scope for this sweep

GitHub issue created:

• 📌 refactor(doctor): reduce duplication between doctor-mode and marketplace-doctor checks #60: Reduce duplication between doctor-mode and marketplace-doctor + agent frontmatter validation gap