Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/skill-review/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "skill-review",
"description": "Quick AI reviewer for Agent Skills: checks structure, workflow, references, and links. Clear report without jargon, with a summary from an exhausted data scientist.",
"version": "1.0.0",
"version": "1.1.0",
"author": {
"name": "mindbox.cloud"
}
Expand Down
29 changes: 29 additions & 0 deletions plugins/skill-review/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
# Changelog — skill-review

## [1.1.0] — 2026-09-17

### Changed

- The review no longer depends on a client-specific tool. The preflight used to stop the
review when `TodoWrite` or `Task` was missing, so the skill could not start at all in a
client that does not offer them. File read is the only hard requirement now.
- The review plan is always a markdown checklist in the chat. Both execution modes now ship a
ready-made skeleton to copy into the response and check off as the review proceeds — the
pattern Anthropic's skill authoring guide recommends. It works in any client and names no tool.
- The review plan gained an explicit completion gate: no final report while plan items are open.
- Single-pass mode can now also be activated by the absence of a sub-agent tool, not only by
volume: it gets a reading strategy for a large skill in one context and an obligation to
record the missing delegation in `Review Limitations`.
- WF12 now asks for a planning mechanism that survives a client without a given tool.
WF13 is worded in terms of open plan items instead of one tool's status names.

### Added

- Antipattern Bingo #21 "Client tool lock-in" (stage 3, WF12 + WF15, owner Workflow) — the skill
names a tool of one client and describes no way to work without it. WF15 is the second anchor
so the row still fires on a skill too short for WF12 to apply.

### Fixed

- With logging on, the results folder is now created right after the user confirms its path,
so single-pass mode has somewhere to write `report.md`. Previously the folder was created
only on the sub-agent branch.

## [1.0.0] — 2026-05-26

### Added
Expand Down
65 changes: 41 additions & 24 deletions plugins/skill-review/skills/skill-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ description: >
user asks to create a new skill, to review code,
user asks to review a prompt that is not a skill.
metadata:
version: 1.6.0
version: 1.7.0
---

# Skill Review Novice — Autonomous Instruction for the Chat Agent
Expand All @@ -25,23 +25,26 @@ metadata:

## Preconditions

1. Before Step 0, check `Task`, `TodoWrite`, and file access for the skill. If anything is missing — stop and report what is lacking.
1. Before Step 0, check that you can **read the files** of the skill under review. If read access is unavailable — stop and report what is lacking.
2. If the user selects logging **ON** in Step 0, verify file write capability. If write is unavailable — stop and suggest disabling logging.

> Do not gate the review on the presence of a named client tool. Planning always works (see `## Mandatory Review Plan`); a missing sub-agent tool only changes the execution mode (see `## Execution Mode Selection`).

---

## CRITICAL — Mandatory Rules

- **Questions to the user — only in Step 0**. Mandatory question: context of use + logging. Ask for log path confirmation **only if** the user enabled logging.
- **A TODO plan is MANDATORY** from the very start of the review.
- The TODO plan must cover the full review scope. For every 3–4 check items there should be a separate TODO item. Do not create one giant TODO for the entire review.
- Update statuses as the review progresses: `pending` → `in_progress` → `completed`.
- **A review plan is MANDATORY** from the very start of the review — as a markdown checklist in the chat.
- The plan must cover the full review scope. For every 3–4 check items there should be a separate plan item. Do not create one giant item for the entire review.
- Check items off (`[ ]` → `[x]`) as they close and repost the updated checklist at every phase boundary.
- **Do not produce the final report while any plan item is still open.**
- If the folder is not a valid skill directory (no `SKILL.md`) or contains more than one skill — see `## Troubleshooting`, problem 1. Do not abort the check silently.
- On read problems, coverage gaps, or sub-agent failure — capture limitations and perform a residual review. On write problems with `logs=on` — notify the user and stop the review.
- In sub-agent mode (`>= 500` lines), the orchestrator must delegate checklist checks to sub-agents via `Task`. Running a full review in the main context instead of launching sub-agents is a workflow violation. Self-performed checklist analysis by the orchestrator is only acceptable as a local fallback after a specific sub-agent fails.
- In sub-agent mode (`>= 500` lines), the orchestrator must delegate checklist checks to sub-agents. Running a full review in the main context instead of launching sub-agents is a workflow violation — unless the client provides no sub-agent tool at all (see `## Execution Mode Selection`). Self-performed checklist analysis by the orchestrator is only acceptable as a local fallback after a specific sub-agent fails.
- **Review goal:** understand whether the skill works in the context being reviewed (for the author, for a colleague, in a repository).
- **Novice does not compute a maturity stage.** Scope is determined by the user's choice. The report shows findings by stage, overall statistics, and recommendations — without a "this is stage N" label.
- **Language:** conduct the entire review (report, logs, TODO, sub-agent briefs) in the language the user started the conversation in.
- **Language:** conduct the entire review (report, logs, review plan, sub-agent briefs) in the language the user started the conversation in.
- **Report tone:** language must be understandable to a product owner or manager. Avoid technical checklist jargon. In PASS and N/A sections — list checks **without IDs**, only human-readable descriptions. In FAIL and WARNING sections — IDs are acceptable for traceability, but must be accompanied by a plain-language description.

---
Expand Down Expand Up @@ -103,16 +106,28 @@ The orchestrator captures **two distinct entities**:

---

## Mandatory TODO Plan
## Mandatory Review Plan

Create a preliminary plan immediately after the user's response, before the first check.

Create a preliminary TODO immediately after the user's response and refine it after collecting the manifest and choosing the mode. Break the review into blocks so that one TODO covers approximately 3–4 checks, not the entire document. Update statuses as work proceeds.
**Copy this checklist into your response and check items off as you complete them:**

**Example of a good breakdown:**
```text
- Check folder structure, SKILL.md presence, directory validity
- Check frontmatter: name, trigger phrases, safety
Review plan:
- [ ] Collect the manifest, count the total volume, choose the execution mode
- [ ] Structure checks (ST01–ST16)
- [ ] Workflow checks (WF01–WF29)
- [ ] References checks (RF01–RF15)
- [ ] Link checks (LK01–LK07)
- [ ] Lifecycle checks (LC01–LC05)
- [ ] Antipattern Bingo
- [ ] Final report
```

Refine it once the manifest is collected and the mode is chosen: drop the groups that lie outside the selected scope, and split any group covering more than 3–4 checks into separate items. Repost the updated checklist at every phase boundary.

**Completion gate:** do not produce the final report while any plan item is still open. An item is closed either by a verdict or by an explicit entry in `Review Limitations`.

---

## Orchestrator Algorithm
Expand All @@ -124,33 +139,33 @@ Create a preliminary TODO immediately after the user's response and refine it af
3a. Check file write
3b. Get timestamp from command line (date +%Y%m%d-%H%M or equivalent)
3c. Show user the full path to the results folder, await confirmation
4. Create the preliminary TODO plan for the review
3d. Create the confirmed results folder: {skill-name}-review-{YYYYMMDD-HHMM}/
4. Create the preliminary review plan (see "Mandatory Review Plan")
5. Collect the manifest of the skill under review: file list, sizes, line counts, frontmatter,
presence of references/, scripts/, assets/. The goal of this step is routing and passing to
sub-agents; do not perform checklist content analysis at this step.
6. Capture unreadable / damaged files and read limitations, if any
7. Count the total lines of all readable files in the skill under review
8. Choose the mode based on the threshold (see "Execution Mode Selection")
9. IF SINGLE-PASS (< 500 lines):
9a. Refine the TODO plan for compact single-pass review
9a. Refine the review plan for compact single-pass review
9b. Read references/instruction-singlepass.md
9c. Execute the entire review sequentially in the main context
9d. If logging ON — write report.md to the confirmed folder
10. IF SUB-AGENT (>= 500 lines):
10a. Refine the TODO plan for sub-agent review
10b. If logging ON — create the results folder: {skill-name}-review-{YYYYMMDD-HHMM}/
10c. Using the "scope → sub-agents" matrix, determine which sub-agents to launch
10d. Launch the required sub-agents; pass scope and checklist mapping to each.
Each selected sub-agent is launched as a separate Task call.
10a. Refine the review plan for sub-agent review
10b. Using the "scope → sub-agents" matrix, determine which sub-agents to launch
10c. Launch the required sub-agents; pass scope and checklist mapping to each.
Each selected sub-agent is launched as a separate sub-agent call.
Do not combine multiple sub-agents in one prompt.
Parallel launch of multiple separate Task calls is allowed.
10e. Collect condensed summaries (and temp_log_path if direct log write to the output folder failed)
10f. Run the verification gate:
Parallel launch of multiple separate sub-agent calls is allowed.
10d. Collect condensed summaries (and temp_log_path if direct log write to the output folder failed)
10e. Run the verification gate:
- Count total FAIL / WARNING across all summaries.
- Verify that every FAIL / WARNING from every summary entered the working findings list.
- Log sub-agents not launched due to scope: "[sub-agent] — not launched: all checks outside selected scope".
- Capture cross-signals between summaries if they require a note in the final report.
10g. If logging ON — move fallback logs to the output folder
10f. If logging ON — move fallback logs to the output folder
11. Fill in "Antipattern Bingo" per references/antipattern-bingo.md. This file may be read early
to distribute Bingo assignments, but fill the final table only after receiving summaries.
12. After receiving summaries, generate the final report with findings grouped by stage
Expand Down Expand Up @@ -212,6 +227,8 @@ After collecting the manifest, count the total number of lines **in all files of
| **< 500 lines** | **Single-pass** | Read `references/instruction-singlepass.md` and run the entire review in the main context, without sub-agents |
| **>= 500 lines** | **Sub-agent** | Launch sub-agents per the scope matrix (logic below) |

> If the client provides no sub-agent tool, run single-pass regardless of volume: read `references/instruction-singlepass.md`, execute the checks sequentially in the main context, and record in `Review Limitations` that the review ran without delegation.

---

## "Scope → Sub-agents" Matrix (sub-agent mode only)
Expand Down Expand Up @@ -285,7 +302,7 @@ Findings counters are maintained **in total across all checked stages**: total F
> - Antipatterns above the current scope — mark `NOT_CHECKED`.
> - After reading, work strictly according to the checklist, scope, base rules, and bingo file.

3. **Logging (if enabled in Step 0):** The orchestrator creates the folder `{skill-name}-review-{YYYYMMDD-HHMM}/`. The sub-agent first tries to write `log-{subagent}.md` directly to the output folder. If that fails — writes the log to a temporary file and returns `temp_log_path`, and the orchestrator moves such a log to the output folder. Do not request additional permissions from the user. **If logging is off:** sub-agents return only a condensed summary. No files or folders are created. File write capability is checked before the review starts.
3. **Logging (if enabled in Step 0):** The orchestrator has already created the folder `{skill-name}-review-{YYYYMMDD-HHMM}/` in Step 0. The sub-agent first tries to write `log-{subagent}.md` directly to the output folder. If that fails — writes the log to a temporary file and returns `temp_log_path`, and the orchestrator moves such a log to the output folder. Do not request additional permissions from the user. **If logging is off:** sub-agents return only a condensed summary. No files or folders are created. File write capability is checked before the review starts.
4. **Sub-agents return condensed summaries** to the orchestrator. A summary is the distillation of the review; when log fallback applies, `temp_log_path` is added.

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,15 @@ For **each** antipattern, choose one of four verdicts:
| 18 | **Lifecycle hygiene gap (rot risk)** | 4 | LC03 | **Lifecycle** |
| 19 | **Silent chain failures** | 2 | WF11 | **Workflow** |
| 20 | **Monolithic reference dump** | 3 | RF06, RF12 | **References** |
| 21 | **Client tool lock-in** | 3 | WF12, WF15 | **Workflow** |

---

## Notes on Specific Antipatterns

- **#15 Schema drift risk** vs **#13 Mirroring MCP schema**: both use WF22, but differently. #13 is a structural fact (a schema copy lives in SKILL.md). #15 is a lifecycle risk (the schema version is not pinned, drift is not tracked).
- **#16 Context overfitting** vs **#12 Hardcoded paths**: #12 is a concrete mechanical signal (absolute paths). #16 is a broader pattern (coupling to OS, permissions, environment, implicit requirements). If the only signal is hardcoded paths, do not duplicate the verdict.
- **#21 Client tool lock-in** vs **#16 Context overfitting**: #21 is a mechanical signal — the skill names a tool of one client (for planning, delegation, or anything else) and describes no way to work without it. `CRITICAL` if a precondition stops the skill over the missing tool; `MINOR` if the tool is merely assumed. Both use WF15, so split by signal: a named client tool goes to #21, everything else (OS, permissions, packages, paths) to #16. Do not duplicate the verdict.
- **#16 in novice mode:** checked **partially** — only by mechanical portability signals (WF15: preconditions, OS, permissions, packages). Deep overfitting analysis (IN09–IN14: hidden assumptions, self-sufficiency, implicit knowledge) is only available in skill-review-nightmare with a full Intern walkthrough.

---
Expand Down Expand Up @@ -95,3 +97,4 @@ For **each** antipattern, choose one of four verdicts:
| 18 | Lifecycle hygiene gap (rot risk) | 4 | [NONE / MINOR / CRITICAL / NOT_CHECKED] | [if MINOR: 3–4 words, otherwise `—`] |
| 19 | Silent chain failures | 2 | [NONE / MINOR / CRITICAL / NOT_CHECKED] | [if MINOR: 3–4 words, otherwise `—`] |
| 20 | Monolithic reference dump | 3 | [NONE / MINOR / CRITICAL / NOT_CHECKED] | [if MINOR: 3–4 words, otherwise `—`] |
| 21 | Client tool lock-in | 3 | [NONE / MINOR / CRITICAL / NOT_CHECKED] | [if MINOR: 3–4 words, otherwise `—`] |
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@

*Context:* Without checkpoints, the agent continues the workflow on a silent step failure — this is "silent chain failures". A good skill does not just list steps — it sets conditions: what must be true before proceeding.

**WF12** If the workflow is longer than **4 steps** — do the instructions explicitly declare a **planning tool**, task list, or external planning artifact?
**WF12** If the workflow is longer than **4 steps** — do the instructions explicitly declare a **planning mechanism** — a task list, a checklist in the response, or an external planning artifact — that does not depend on a tool being present in a particular client?

*Context:* In long sessions, the agent easily loses its plan and starts jumping between tasks. Anthropic recommends structured note-taking / agentic memory: an explicit task list that maintains state between tool calls.
*Context:* In long sessions, the agent easily loses its plan and starts jumping between tasks. Anthropic recommends structured note-taking / agentic memory: an explicit task list that maintains state between tool calls. Tool availability differs between clients and model versions, so a plan that exists only inside one named tool disappears where that tool is not offered — and a precondition that stops the skill over a missing tool is worse than having no plan at all.

**WF13** If planning is used, is there an **enforcement gate**: completion is not allowed while there are `pending` or `in_progress` tasks?
**WF13** If planning is used, is there an **enforcement gate**: completion is not allowed while plan items remain open?

*Context:* The most effective way to make planning mandatory is to prohibit completion with unclosed tasks. Otherwise the TODO list remains decorative and does not prevent context loss.
*Context:* The most effective way to make planning mandatory is to prohibit completion with unclosed tasks. Otherwise the plan remains decorative and does not prevent context loss. The gate must be worded in terms of open plan items, not in terms of the status names of one specific tool.

**WF14** Are critical rules, prohibitions, and stop conditions at the beginning of the skill or under explicit `CRITICAL` headers, not buried in the middle of a long text?

Expand Down Expand Up @@ -149,7 +149,7 @@

> **Stage: 2** | Axis: personal use (if workflow is multi-step with multiple tools / co-skills)

**WF27** If the workflow passes through several tools / co-skills, is there an external planning artifact or TODO list that survives the handoff between steps?
**WF27** If the workflow passes through several tools / co-skills, is there an external planning artifact or task list that survives the handoff between steps?

*Context:* In a multi-skill environment, task state is most often lost precisely at transitions between steps and tools. An external planning artifact maintains dependent steps, execution status, and blockers that would otherwise dissolve into the thread history.

Expand All @@ -173,7 +173,7 @@
| Context-refresh: re-reading the plan before each new phase | WARNING |
| Strategy for context overflow (compaction, sub-agent, respawn) | INFO (if < 10 steps), WARNING (if >= 10) |
| Checkpoints with state capture | WARNING (for critical phases) |
| TODO list as attention management (updated during the work) | INFO |
| Task list as attention management (updated during the work) | INFO |

**Scale:**
- **PASS:** there is an explicit context management strategy (files, checkpoints, refresh)
Expand Down
Loading
Loading