Overview
Perform a deep, systematic review of supplementary agent artifacts — Agent
Skills, hooks, AGENTS.md/CLAUDE.md files, scripts, MCP configurations, or
workflow definitions. Surfaces design flaws, unclosed loops, incorrect
references, stale assumptions, side effects, security risks, portability issues,
and unanswered design questions that structural validators miss.
Quick Reference
| Severity |
Meaning |
Blocks Pass? |
| CRITICAL |
Breaks functionality, security risk, or data loss |
Yes |
| WARNING |
Degrades quality, reliability, or maintainability |
No |
| SUGGESTION |
Improvement opportunity or best practice deviation |
No |
| Category |
ID |
Checks |
Applies To |
| Metadata & Registration |
M |
7 |
Skills |
| Structure & Format |
S |
6 |
Skills, AGENTS.md/CLAUDE.md |
| Instruction Quality |
C |
6 |
All |
| Dependencies & Assumptions |
D |
6 |
All |
| Reliability & Staleness |
R |
5 |
All |
| Side Effects |
E |
6 |
All |
| Security |
X |
5 |
All |
| Context Gathering |
G |
5 |
Skills, Hooks |
| Hooks-Specific |
H |
9 |
Hooks only |
Pass criteria: Zero CRITICAL issues.
Workflow
Phase 1: Input Detection & Scope
Determine what is being reviewed and read all files.
- If
$ARGUMENTS is provided, treat as path to the artifact
- If no argument, ask the user what to review and where it lives
- Detect artifact type from file structure:
- Skill: Directory containing
SKILL.md — also read references/, scripts/, agents/, orchestrators/, templates/
- Hook: Hook config block in
settings.json — also read any scripts it references
- AGENTS.md / CLAUDE.md: instruction files — check for imports or linked references and read those too
- Script: Standalone Python script — check PEP 723 metadata, dependencies
- MCP Config: MCP server configuration — check tool references, permissions
- Composite: Multiple types (e.g., a skill that also installs hooks)
- Read ALL files in scope before proceeding. Do not review from memory or assumptions.
Phase 2: Audience & Invocation Analysis
- Who is the audience? Human users? Host agent auto-invocation? Both? Delegated agents?
- How is it invoked?
/slash-command? Auto-discovered by description? Event-triggered hook?
- Is registration correct? Frontmatter must match intended invocation:
- Both human and model → default host-discoverable metadata
- Human-only → use the host's supported human-invocation flag or adapter metadata
- Machine-only → use the host's supported model-invocation flag or adapter metadata
- Is discovery viable? Will a host agent's description-matching find this when relevant? Are trigger keywords present? Is the key use case early in the description?
Phase 3: Systematic Review
Run through each applicable check category. Classify every finding by severity and check ID. See references/check-catalog.md for detailed check descriptions.
Metadata & Registration (M001-M007)
| Check |
What to verify |
| M001 |
Frontmatter YAML is valid and parseable |
| M002 |
Name: kebab-case, ≤64 chars, matches directory name |
| M003 |
Description: trigger conditions present, specific, ≤1024 chars, key use case front-loaded within first 250 chars |
| M004 |
Invocation mode flags match intended audience |
| M005 |
Context mode appropriate — fork for isolated work, default for session-persistent guidance |
| M006 |
allowed-tools restrictions (if present) are complete and correct for the task |
| M007 |
The artifact will actually be discovered and invoked when needed |
Structure & Format (S001-S006)
| Check |
What to verify |
| S001 |
Required sections present: Overview, Quick Reference, Main Content, Common Mistakes |
| S002 |
SKILL.md ≤500 lines; detailed material in references/ (progressive disclosure) |
| S003 |
Quick Reference is scannable and genuinely useful, not decorative |
| S004 |
Common Mistakes contains realistic anti-patterns with actionable ❌/✅ fixes |
| S005 |
Supporting files organized correctly (references/, scripts/, assets/) |
| S006 |
Code blocks have proper language tags; tool names are correct (rg not grep) |
Instruction Quality (C001-C006)
| Check |
What to verify |
| C001 |
Instructions are clear, precise, and unambiguous — a human could follow them |
| C002 |
No conflicting or contradictory directives between sections |
| C003 |
Ambiguous scope identified — words like "primarily", "generally" collapse to "always" or "never" in practice |
| C004 |
Writing style: imperative form, third person ("the agent"), no specific AI names |
| C005 |
Degrees of freedom match the task — high for creative, low for critical/fragile operations |
| C006 |
Instruction volume within effective budget — compliance degrades past ~150-200 directives |
Dependencies & Assumptions (D001-D006)
| Check |
What to verify |
| D001 |
All referenced file paths exist; resolve portable resources relative to the active SKILL.md, not hardcoded absolute paths |
| D002 |
All referenced tools/MCP servers exist and are available; MCP names fully qualified (Server:tool) |
| D003 |
Scripts are Python (not bash), use PEP 723 inline metadata, dependencies declared or stdlib-only |
| D004 |
No implicit OS/shell/tool assumptions (brew, apt, npm globals, osascript) |
| D005 |
Pre-existing file/config/state requirements are documented and checked before use |
| D006 |
Portability: can this be shared or moved to another machine without modification? |
Reliability & Staleness (R001-R005)
| Check |
What to verify |
| R001 |
No stale references — file paths, tool schemas, API endpoints all currently valid |
| R002 |
No time-sensitive content — hardcoded dates, version numbers, temporal conditional logic |
| R003 |
Instructions work across model capability tiers without vendor/model-specific assumptions |
| R004 |
Critical content is front-loaded within first ~100 lines (compaction preserves first 5,000 tokens) |
| R005 |
Assumption chains identified — if an early assumption breaks, does downstream logic cascade-fail? |
Side Effects (E001-E006)
| Check |
What to verify |
| E001 |
File mutations: creates, modifies, or deletes files? Intentional and documented? |
| E002 |
Git state: stages, commits, pushes, or modifies branches? Could interfere with user's working state? |
| E003 |
External calls: network requests, API calls, webhooks? Reversible? |
| E004 |
Context pollution: injects large content into context window? Cleans up after itself? |
| E005 |
Permission escalation: requests broader tool permissions than the task requires? |
| E006 |
Hook conflicts: could conflict with other hooks on the same event? (Parallel updatedInput rewrites are non-deterministic) |
Security (X001-X005)
| Check |
What to verify |
| X001 |
No credentials embedded — API keys, passwords, tokens, connection strings |
| X002 |
Injection surface — user input or external content interpolated into prompts without sanitization? |
| X003 |
Excessive agency — tools beyond scope, permissions beyond need, autonomy without human oversight? |
| X004 |
System prompt leakage — could instructions be extracted via crafted input? |
| X005 |
Destructive operations gated behind confirmation — guardrails against rm -rf, force push, etc. |
Context Gathering (G001-G005)
| Check |
What to verify |
| G001 |
All required context is identified — what info does the artifact need to function? |
| G002 |
Acquisition strategy defined — user interaction? File reads? Tool calls? Env vars? |
| G003 |
Missing context handled gracefully — fails clearly rather than proceeding on assumptions |
| G004 |
User interaction quality — structured clarification asks few questions with clear options when the host supports it |
| G005 |
Feedback loop closed — the artifact can validate its own output or the user can verify results |
Hooks-Specific (H001-H009)
| Check |
What to verify |
| H001 |
Correct hook event for the intended trigger (26 events available) |
| H002 |
Matcher is specific enough — broad PermissionRequest matchers auto-approve everything |
| H003 |
Exit codes correct — 0 (proceed), 2 (block), other (non-blocking error) |
| H004 |
No infinite loop risk — Stop hooks must check stop_hook_active; PreToolUse must not self-trigger |
| H005 |
Shell profile safe — ~/.zshrc/~/.bashrc echo statements won't corrupt JSON I/O |
| H006 |
Parallel-safe — no conflicting updatedInput rewrites across multiple hooks on same event |
| H007 |
JSON I/O compliant — input via stdin, output via stdout, fields ≤10,000 chars |
| H008 |
Non-blocking (HARDLINE) — hook returns in sub-~50ms; registered script is pure event dispatch (read stdin → fork → exit 0) with ZERO business logic on the critical path; all parsing/I/O/jq/LLM/network detached to a backgrounded child. If removing timeout: would make it block, it's blocking |
| H009 |
Detached child's stdio severed from the harness pipe (</dev/null >/dev/null 2>&1 on the backgrounded subshell). A bare & inherits fd 1, keeps the pipe open, and stalls the harness to the timeout even though "backgrounded" — the classic non-fix |
Phase 4: Design Questions Audit
Surface unanswered questions from the design process:
- Unhappy path: What happens when files are missing, tools fail, permissions are denied, or input is unexpected?
- Wrong context: What happens when invoked in the wrong project, wrong branch, or without a git repo?
- Post-compaction: Is critical content in the first 5,000 tokens? Will this skill degrade after compaction?
- Composition: How does this interact with other skills/hooks? Conflicts? Redundancy? Ordering?
- Blast radius: If this malfunctions — is damage limited to context, or does it mutate files/push code?
- Feedback loop: Can the output be verified as correct? By the artifact itself? By the user?
- Assumption durability: What assumptions could change? APIs, file structures, team conventions, tool availability?
- Maintenance: Who maintains this? What triggers an update? How will staleness be detected?
Phase 5: Report
## Review: [artifact-name] ([artifact-type])
### Verdict: PASS | FAIL (N critical issues)
### Summary
[2-3 sentences: what this does, overall quality, key concern]
### Findings
#### Critical
- **[ID]**: [description] — [specific location/line]
#### Warnings
- **[ID]**: [description] — [specific location/line]
#### Suggestions
- **[ID]**: [description] — [specific location/line]
### Unanswered Design Questions
[Numbered list from Phase 4 that lack clear answers]
### Positive Aspects
[What the artifact does well]
### Recommendations
[Prioritized, specific, actionable fixes]
Common Mistakes
❌ Reviewing from memory instead of reading files
The artifact may have changed. Always read all files fresh in Phase 1.
✅ Read the complete artifact and all supporting files before starting review.
❌ Checking only structure, ignoring semantics
Perfect frontmatter and section headers can hide contradictory instructions or nonexistent tool references.
✅ Validate that instructions are correct, consistent, and grounded in current reality.
❌ Missing transitive dependencies
A skill references a script that imports a module that requires an MCP server. Each link is a break point.
✅ Trace the full dependency graph, not just first-level references.
❌ Ignoring compaction behavior
Skills get truncated to 5,000 tokens after compaction. Critical instructions buried at line 400 disappear.
✅ Verify critical content is front-loaded within the first ~100 lines.
❌ Treating warning clusters as individual issues
A single warning is fine. Five warnings in the same category signals systemic design rot.
✅ Look for patterns across warnings — clusters indicate deeper problems.
❌ Skipping the unhappy path
The artifact works perfectly with perfect input. What about missing files, tool failures, or unexpected user input?
✅ Explicitly probe failure modes and edge cases in Phase 4.
Notes
- This review complements
pdm-skill-create validation. That validates structure; this validates design quality, correctness, and reliability.
- For hook reviews, behavior differs by host and version. Check the adapter docs
for the exact event schema.
- Skills using isolated/delegated context must provide enough actionable context
for the delegated agent to produce useful output — guidelines alone without a
task prompt return nothing.
- Findings must be specific and actionable. "Description could be better" is useless. "Description missing trigger keywords — add 'review this PR', 'code review' to match natural user phrasing" is useful.
- When reviewing composites (e.g., a plugin with skills + hooks + scripts), review each component individually, then review their interactions as a system.
1---2name: util-review3description: Review Agent Skills, hooks, AGENTS.md/CLAUDE.md files, scripts, MCP configs, or workflow configurations for design flaws, unclosed loops, stale references, side effects, security risks, and portability issues. Use when: 'review this skill', 'audit this hook', '/util-review', 'check this config', or when evaluating any supplementary agent artifact for quality and correctness.4---56## Overview78Perform a deep, systematic review of supplementary agent artifacts — Agent9Skills, hooks, AGENTS.md/CLAUDE.md files, scripts, MCP configurations, or10workflow definitions. Surfaces design flaws, unclosed loops, incorrect11references, stale assumptions, side effects, security risks, portability issues,12and unanswered design questions that structural validators miss.1314## Quick Reference1516| Severity | Meaning | Blocks Pass? |17|----------|---------|--------------|18| **CRITICAL** | Breaks functionality, security risk, or data loss | Yes |19| **WARNING** | Degrades quality, reliability, or maintainability | No |20| **SUGGESTION** | Improvement opportunity or best practice deviation | No |2122| Category | ID | Checks | Applies To |23|----------|----|--------|------------|24| Metadata & Registration | M | 7 | Skills |25| Structure & Format | S | 6 | Skills, AGENTS.md/CLAUDE.md |26| Instruction Quality | C | 6 | All |27| Dependencies & Assumptions | D | 6 | All |28| Reliability & Staleness | R | 5 | All |29| Side Effects | E | 6 | All |30| Security | X | 5 | All |31| Context Gathering | G | 5 | Skills, Hooks |32| Hooks-Specific | H | 9 | Hooks only |3334**Pass criteria:** Zero CRITICAL issues.3536## Workflow3738### Phase 1: Input Detection & Scope3940Determine what is being reviewed and read all files.41421. If `$ARGUMENTS` is provided, treat as path to the artifact432. If no argument, ask the user what to review and where it lives443. Detect artifact type from file structure:45 - **Skill**: Directory containing `SKILL.md` — also read `references/`, `scripts/`, `agents/`, `orchestrators/`, `templates/`46 - **Hook**: Hook config block in `settings.json` — also read any scripts it references47 - **AGENTS.md / CLAUDE.md**: instruction files — check for imports or linked references and read those too48 - **Script**: Standalone Python script — check PEP 723 metadata, dependencies49 - **MCP Config**: MCP server configuration — check tool references, permissions50 - **Composite**: Multiple types (e.g., a skill that also installs hooks)514. **Read ALL files in scope before proceeding.** Do not review from memory or assumptions.5253### Phase 2: Audience & Invocation Analysis54551. **Who is the audience?** Human users? Host agent auto-invocation? Both? Delegated agents?562. **How is it invoked?** `/slash-command`? Auto-discovered by description? Event-triggered hook?573. **Is registration correct?** Frontmatter must match intended invocation:58 - Both human and model → default host-discoverable metadata59 - Human-only → use the host's supported human-invocation flag or adapter metadata60 - Machine-only → use the host's supported model-invocation flag or adapter metadata614. **Is discovery viable?** Will a host agent's description-matching find this when relevant? Are trigger keywords present? Is the key use case early in the description?6263### Phase 3: Systematic Review6465Run through each applicable check category. Classify every finding by severity and check ID. See `references/check-catalog.md` for detailed check descriptions.6667#### Metadata & Registration (M001-M007)6869| Check | What to verify |70|-------|---------------|71| M001 | Frontmatter YAML is valid and parseable |72| M002 | Name: kebab-case, ≤64 chars, matches directory name |73| M003 | Description: trigger conditions present, specific, ≤1024 chars, key use case front-loaded within first 250 chars |74| M004 | Invocation mode flags match intended audience |75| M005 | Context mode appropriate — `fork` for isolated work, default for session-persistent guidance |76| M006 | `allowed-tools` restrictions (if present) are complete and correct for the task |77| M007 | The artifact will actually be discovered and invoked when needed |7879#### Structure & Format (S001-S006)8081| Check | What to verify |82|-------|---------------|83| S001 | Required sections present: Overview, Quick Reference, Main Content, Common Mistakes |84| S002 | SKILL.md ≤500 lines; detailed material in `references/` (progressive disclosure) |85| S003 | Quick Reference is scannable and genuinely useful, not decorative |86| S004 | Common Mistakes contains realistic anti-patterns with actionable ❌/✅ fixes |87| S005 | Supporting files organized correctly (`references/`, `scripts/`, `assets/`) |88| S006 | Code blocks have proper language tags; tool names are correct (`rg` not `grep`) |8990#### Instruction Quality (C001-C006)9192| Check | What to verify |93|-------|---------------|94| C001 | Instructions are clear, precise, and unambiguous — a human could follow them |95| C002 | No conflicting or contradictory directives between sections |96| C003 | Ambiguous scope identified — words like "primarily", "generally" collapse to "always" or "never" in practice |97| C004 | Writing style: imperative form, third person ("the agent"), no specific AI names |98| C005 | Degrees of freedom match the task — high for creative, low for critical/fragile operations |99| C006 | Instruction volume within effective budget — compliance degrades past ~150-200 directives |100101#### Dependencies & Assumptions (D001-D006)102103| Check | What to verify |104|-------|---------------|105| D001 | All referenced file paths exist; resolve portable resources relative to the active `SKILL.md`, not hardcoded absolute paths |106| D002 | All referenced tools/MCP servers exist and are available; MCP names fully qualified (`Server:tool`) |107| D003 | Scripts are Python (not bash), use PEP 723 inline metadata, dependencies declared or stdlib-only |108| D004 | No implicit OS/shell/tool assumptions (`brew`, `apt`, `npm` globals, `osascript`) |109| D005 | Pre-existing file/config/state requirements are documented and checked before use |110| D006 | Portability: can this be shared or moved to another machine without modification? |111112#### Reliability & Staleness (R001-R005)113114| Check | What to verify |115|-------|---------------|116| R001 | No stale references — file paths, tool schemas, API endpoints all currently valid |117| R002 | No time-sensitive content — hardcoded dates, version numbers, temporal conditional logic |118| R003 | Instructions work across model capability tiers without vendor/model-specific assumptions |119| R004 | Critical content is front-loaded within first ~100 lines (compaction preserves first 5,000 tokens) |120| R005 | Assumption chains identified — if an early assumption breaks, does downstream logic cascade-fail? |121122#### Side Effects (E001-E006)123124| Check | What to verify |125|-------|---------------|126| E001 | File mutations: creates, modifies, or deletes files? Intentional and documented? |127| E002 | Git state: stages, commits, pushes, or modifies branches? Could interfere with user's working state? |128| E003 | External calls: network requests, API calls, webhooks? Reversible? |129| E004 | Context pollution: injects large content into context window? Cleans up after itself? |130| E005 | Permission escalation: requests broader tool permissions than the task requires? |131| E006 | Hook conflicts: could conflict with other hooks on the same event? (Parallel `updatedInput` rewrites are non-deterministic) |132133#### Security (X001-X005)134135| Check | What to verify |136|-------|---------------|137| X001 | No credentials embedded — API keys, passwords, tokens, connection strings |138| X002 | Injection surface — user input or external content interpolated into prompts without sanitization? |139| X003 | Excessive agency — tools beyond scope, permissions beyond need, autonomy without human oversight? |140| X004 | System prompt leakage — could instructions be extracted via crafted input? |141| X005 | Destructive operations gated behind confirmation — guardrails against `rm -rf`, force push, etc. |142143#### Context Gathering (G001-G005)144145| Check | What to verify |146|-------|---------------|147| G001 | All required context is identified — what info does the artifact need to function? |148| G002 | Acquisition strategy defined — user interaction? File reads? Tool calls? Env vars? |149| G003 | Missing context handled gracefully — fails clearly rather than proceeding on assumptions |150| G004 | User interaction quality — structured clarification asks few questions with clear options when the host supports it |151| G005 | Feedback loop closed — the artifact can validate its own output or the user can verify results |152153#### Hooks-Specific (H001-H009)154155| Check | What to verify |156|-------|---------------|157| H001 | Correct hook event for the intended trigger (26 events available) |158| H002 | Matcher is specific enough — broad PermissionRequest matchers auto-approve everything |159| H003 | Exit codes correct — 0 (proceed), 2 (block), other (non-blocking error) |160| H004 | No infinite loop risk — Stop hooks must check `stop_hook_active`; PreToolUse must not self-trigger |161| H005 | Shell profile safe — `~/.zshrc`/`~/.bashrc` echo statements won't corrupt JSON I/O |162| H006 | Parallel-safe — no conflicting `updatedInput` rewrites across multiple hooks on same event |163| H007 | JSON I/O compliant — input via stdin, output via stdout, fields ≤10,000 chars |164| H008 | Non-blocking (HARDLINE) — hook returns in sub-~50ms; registered script is pure event dispatch (read stdin → fork → `exit 0`) with ZERO business logic on the critical path; all parsing/I/O/jq/LLM/network detached to a backgrounded child. If removing `timeout:` would make it block, it's blocking |165| H009 | Detached child's stdio severed from the harness pipe (`</dev/null >/dev/null 2>&1` on the backgrounded subshell). A bare `&` inherits fd 1, keeps the pipe open, and stalls the harness to the timeout even though "backgrounded" — the classic non-fix |166167### Phase 4: Design Questions Audit168169Surface unanswered questions from the design process:1701711. **Unhappy path**: What happens when files are missing, tools fail, permissions are denied, or input is unexpected?1722. **Wrong context**: What happens when invoked in the wrong project, wrong branch, or without a git repo?1733. **Post-compaction**: Is critical content in the first 5,000 tokens? Will this skill degrade after compaction?1744. **Composition**: How does this interact with other skills/hooks? Conflicts? Redundancy? Ordering?1755. **Blast radius**: If this malfunctions — is damage limited to context, or does it mutate files/push code?1766. **Feedback loop**: Can the output be verified as correct? By the artifact itself? By the user?1777. **Assumption durability**: What assumptions could change? APIs, file structures, team conventions, tool availability?1788. **Maintenance**: Who maintains this? What triggers an update? How will staleness be detected?179180### Phase 5: Report181182```183## Review: [artifact-name] ([artifact-type])184185### Verdict: PASS | FAIL (N critical issues)186187### Summary188[2-3 sentences: what this does, overall quality, key concern]189190### Findings191192#### Critical193- **[ID]**: [description] — [specific location/line]194195#### Warnings196- **[ID]**: [description] — [specific location/line]197198#### Suggestions199- **[ID]**: [description] — [specific location/line]200201### Unanswered Design Questions202[Numbered list from Phase 4 that lack clear answers]203204### Positive Aspects205[What the artifact does well]206207### Recommendations208[Prioritized, specific, actionable fixes]209```210211## Common Mistakes212213❌ **Reviewing from memory instead of reading files**214The artifact may have changed. Always read all files fresh in Phase 1.215✅ Read the complete artifact and all supporting files before starting review.216217❌ **Checking only structure, ignoring semantics**218Perfect frontmatter and section headers can hide contradictory instructions or nonexistent tool references.219✅ Validate that instructions are correct, consistent, and grounded in current reality.220221❌ **Missing transitive dependencies**222A skill references a script that imports a module that requires an MCP server. Each link is a break point.223✅ Trace the full dependency graph, not just first-level references.224225❌ **Ignoring compaction behavior**226Skills get truncated to 5,000 tokens after compaction. Critical instructions buried at line 400 disappear.227✅ Verify critical content is front-loaded within the first ~100 lines.228229❌ **Treating warning clusters as individual issues**230A single warning is fine. Five warnings in the same category signals systemic design rot.231✅ Look for patterns across warnings — clusters indicate deeper problems.232233❌ **Skipping the unhappy path**234The artifact works perfectly with perfect input. What about missing files, tool failures, or unexpected user input?235✅ Explicitly probe failure modes and edge cases in Phase 4.236237## Notes238239- This review complements `pdm-skill-create` validation. That validates **structure**; this validates **design quality, correctness, and reliability**.240- For hook reviews, behavior differs by host and version. Check the adapter docs241 for the exact event schema.242- Skills using isolated/delegated context must provide enough actionable context243 for the delegated agent to produce useful output — guidelines alone without a244 task prompt return nothing.245- Findings must be specific and actionable. "Description could be better" is useless. "Description missing trigger keywords — add 'review this PR', 'code review' to match natural user phrasing" is useful.246- When reviewing composites (e.g., a plugin with skills + hooks + scripts), review each component individually, then review their interactions as a system.
mnox