Skip to content
Merged
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
300 changes: 300 additions & 0 deletions tools/skill-validator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,300 @@
# Skill Validator — Inference-Based

An LLM-readable validation prompt for skills following the Agent Skills open standard.

## How to Use

1. You are given a **skill directory path** to validate.
2. Read every file in the skill directory recursively.
3. Apply every rule in the catalog below to every applicable file.
4. Produce a findings report using the report template at the end.

If no findings are generated, the skill passes validation.

---

## Definitions

- **Skill directory**: the folder containing `SKILL.md` and all supporting files.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incomplete definition of "Skill directory."

Line 18 defines a skill directory as "the folder containing SKILL.md and all supporting files." But learnings show that skills also require bmad-skill-manifest.yaml for registration. The definition should be:

Skill directory: the folder containing SKILL.md, bmad-skill-manifest.yaml, and all supporting files (workflow.md, steps/, templates, etc.).

This aligns the definition with the actual skill architecture and sets up the missing manifest validation rules.

Based on learnings: "The skill self-registers via its own bmad-skill-manifest.yaml in its directory."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` at line 18, Update the "Skill directory" definition
to explicitly require bmad-skill-manifest.yaml in addition to SKILL.md and
supporting files: change the text for the "Skill directory" entry to read that
the folder contains SKILL.md, bmad-skill-manifest.yaml, and all supporting files
(e.g., workflow.md, steps/, templates/), so the documentation and downstream
validation rules reflect that the skill self-registers via its own manifest.

- **Internal reference**: a file path from one file in the skill to another file in the same skill.
- **External reference**: a file path from a skill file to a file outside the skill directory.
- **Originating file**: the file that contains the reference (path resolution is relative to this file's location).
- **Config variable**: a name-value pair whose value comes from the project config file (e.g., `planning_artifacts`, `implementation_artifacts`, `communication_language`).
- **Runtime variable**: a name-value pair whose value is set during workflow execution (e.g., `spec_file`, `date`, `status`).
- **Intra-skill path variable**: a frontmatter variable whose value is a path to another file within the same skill — this is an anti-pattern.

Comment on lines +16 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical omission: No validation of bmad-skill-manifest.yaml.

The Definitions section (and the entire rule catalog) never mentions bmad-skill-manifest.yaml, yet learnings confirm this file is required for skill self-registration in the BMAD-METHOD architecture. A skill directory containing only SKILL.md without the manifest cannot be discovered by the installation pipeline.

Add:

  • Definition for "Skill manifest file"
  • Rule SKILL-06: Manifest file must exist
  • Rule SKILL-07: Manifest must contain required fields (canonicalId, etc.)
  • Rule SKILL-08: canonicalId must follow naming convention (based on learnings about bmad- prefix)

Based on learnings: "The skill self-registers via its own bmad-skill-manifest.yaml in its directory" and "canonicalId uniqueness across bmad-skill-manifest.yaml files is considered enforced by the naming convention (all IDs carry the bmad- prefix)."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 16 - 25, Add a new definition "Skill
manifest file" describing bmad-skill-manifest.yaml as the file in the skill
directory used for self-registration, and add three rules: SKILL-06 requiring
bmad-skill-manifest.yaml to exist in every skill directory; SKILL-07 validating
the manifest contains required fields (at minimum canonicalId, name, version,
entryPoint, and any registry metadata) and fail if missing; and SKILL-08
enforcing canonicalId naming/uniqueness rules (must start with the "bmad-"
prefix and follow the project's canonical ID pattern to avoid collisions).
Ensure the new definition and rules are placed alongside the existing
Definitions and rule catalog entries and reference "bmad-skill-manifest.yaml",
"SKILL-06", "SKILL-07", "SKILL-08", and the field name "canonicalId" so they are
discoverable and machine-validateable.

---

## Rule Catalog
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule catalog defines 22 rule IDs (SKILL/WF/PATH/STEP/SEQ/REF); consider stating the total in-file (and keeping any external references in sync) to avoid confusion if other docs mention a different count.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.


### SKILL-01 — SKILL.md Must Exist

- **Severity:** CRITICAL
- **Applies to:** skill directory
- **Rule:** The skill directory must contain a file named `SKILL.md` (exact case).
- **Detection:** Check for the file's existence.
- **Fix:** Create `SKILL.md` as the skill entrypoint.

### SKILL-02 — SKILL.md Must Have `name` in Frontmatter

- **Severity:** CRITICAL
- **Applies to:** `SKILL.md`
- **Rule:** The YAML frontmatter must contain a `name` field.
- **Detection:** Parse the `---` delimited frontmatter block and check for `name:`.
- **Fix:** Add `name: <skill-name>` to the frontmatter.

### SKILL-03 — SKILL.md Must Have `description` in Frontmatter

- **Severity:** CRITICAL
- **Applies to:** `SKILL.md`
- **Rule:** The YAML frontmatter must contain a `description` field.
- **Detection:** Parse the `---` delimited frontmatter block and check for `description:`.
- **Fix:** Add `description: '<what it does and when to use it>'` to the frontmatter.

### SKILL-04 — `name` Format

- **Severity:** HIGH
- **Applies to:** `SKILL.md`
- **Rule:** The `name` value must use only lowercase letters, numbers, and hyphens. Max 64 characters. Must not contain "anthropic" or "claude".
- **Detection:** Regex test: `^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$`. String search for forbidden substrings.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex ^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$ enforces a minimum length of 2 characters, but SKILL-04 only specifies a max length; if 1-character names are valid, this would incorrectly fail them.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

- **Fix:** Rename to comply with the format.
Comment on lines +54 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

SKILL-04 regex forbids single-character skill names.

The regex ^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$ requires start character + middle (0-62 chars) + end character, forcing a minimum of 2 characters. Single-character skill names (e.g., x, q) are rejected, yet the rule text only specifies "Max 64 characters" without documenting this minimum-length constraint.

If single-character names should be forbidden, update the rule text to state "2–64 characters." Otherwise, fix the regex:

-^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$
+^[a-z0-9]([a-z0-9-]{0,62}[a-z0-9])?$
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 54 - 60, SKILL-04's regex currently
requires two characters and thus forbids single-character names; update the
regex in the SKILL-04 rule from `^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$` to allow a
single-character name while preserving the 1–64 length and "no leading/trailing
hyphen" constraints, e.g. `^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$`;
alternatively, if single-character names should be disallowed, change the rule
text to explicitly state "2–64 characters" instead of only "Max 64 characters."


### SKILL-05 — `name` Must Match Directory Name

- **Severity:** HIGH
- **Applies to:** `SKILL.md`
- **Rule:** The `name` value in SKILL.md frontmatter must exactly match the skill directory name. The directory name is the canonical identifier used by installers, manifests, and `skill:` references throughout the project.
- **Detection:** Compare the `name:` frontmatter value against the basename of the skill directory (i.e., the immediate parent directory of `SKILL.md`).
- **Fix:** Change the `name:` value to match the directory name, or rename the directory to match — prefer changing `name:` unless other references depend on the current value.

### SKILL-06 — `description` Quality

- **Severity:** MEDIUM
- **Applies to:** `SKILL.md`
- **Rule:** The `description` must state both what the skill does AND when to use it. Max 1024 characters.
- **Detection:** Check length. Look for trigger phrases like "Use when" or "Use if" — their absence suggests the description only says _what_ but not _when_.
- **Fix:** Append a "Use when..." clause to the description.

---

### WF-01 — workflow.md Must NOT Have `name` in Frontmatter

- **Severity:** HIGH
- **Applies to:** `workflow.md` (if it exists)
- **Rule:** The `name` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `name:`.
- **Detection:** Parse frontmatter and check for `name:` key.
- **Fix:** Remove the `name:` line from workflow.md frontmatter.

### WF-02 — workflow.md Must NOT Have `description` in Frontmatter

- **Severity:** HIGH
- **Applies to:** `workflow.md` (if it exists)
- **Rule:** The `description` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `description:`.
- **Detection:** Parse frontmatter and check for `description:` key.
- **Fix:** Remove the `description:` line from workflow.md frontmatter.

### WF-03 — workflow.md Frontmatter Variables Must Be Config or Runtime Only

- **Severity:** HIGH
- **Applies to:** `workflow.md` frontmatter
- **Rule:** Every variable defined in workflow.md frontmatter must be either:
- A config variable (value references `{project-root}` or a config-derived variable like `{planning_artifacts}`)
- A runtime variable (value is empty, a placeholder, or set during execution)
- A legitimate external path expression

It must NOT be a path to a file within the skill directory.
- **Detection:** For each frontmatter variable, check if its value resolves to a file inside the skill (e.g., starts with `./`, `{installed_path}`, or is a bare relative path to a sibling file). If so, it is an intra-skill path variable.
- **Fix:** Remove the variable. Use a hardcoded relative path inline where the file is referenced.

Comment on lines +80 to +108
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing rule: workflow.md existence validation.

WF-01, WF-02, and WF-03 all apply to workflow.md (lines 75, 83, 91), yet no rule validates that this file exists in the skill directory. Learnings confirm that workflow.md is the "canonical entry file for verbatim skill directories."

If a skill is missing workflow.md, the WF rules will silently pass (no file to check), but the skill will be broken. Add:

WF-00 — workflow.md Must Exist

  • Severity: CRITICAL
  • Applies to: skill directory
  • Rule: The skill directory must contain workflow.md as the canonical entry file.
  • Detection: Check for file existence.
  • Fix: Create workflow.md or verify the skill structure.

Based on learnings: "workflow.md is always the canonical entry file for verbatim skill directories."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 72 - 100, Add a new validation rule
WF-00 that ensures workflow.md exists in the skill directory: define WF-00 —
"workflow.md Must Exist" with Severity: CRITICAL, Applies to: skill directory,
Rule: the skill directory must contain workflow.md as the canonical entry file,
Detection: check for the presence of workflow.md (file existence), and Fix:
instruct to create workflow.md or correct the skill structure; update the
validator registry alongside WF-01, WF-02, and WF-03 so WF rules fail when
workflow.md is missing.

Comment on lines +96 to +108
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

WF-03 detection logic is vague and partially unexecutable.

The rule requires distinguishing config variables from runtime variables from intra-skill path variables, but the detection guidance conflates static syntax analysis with runtime semantics:

  • "value is empty, a placeholder, or set during execution" — how does static analysis determine what gets "set during execution"?
  • "resolves to a file inside the skill" — requires evaluating relative paths, but variable values can be template expressions (e.g., {planning_artifacts}/{spec_file}) whose components are unknowable at validation time.

Either:

  1. Provide concrete syntax patterns for each category (e.g., "starts with {project-root} or {planning_artifacts} → config; empty string or null → runtime; starts with ./ or ../ → forbidden intra-skill"), or
  2. Mark this as a heuristic rule that produces findings for manual review rather than definitive violations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 88 - 100, WF-03's detection is
ambiguous; update the WF-03 rule text in tools/skill-validator.md (the "WF-03 —
workflow.md Frontmatter Variables Must Be Config or Runtime Only" section) to
use concrete syntactic patterns: treat values that start with "{project-root}"
or "{planning_artifacts}" (or other documented config placeholders) as config
variables; treat empty, "null", or a single placeholder token like "{...}" as
runtime variables; treat any value that is a relative path starting with "./",
"../", or a bare filename (no braces) as a forbidden intra-skill path; and if
you prefer not to enforce these statically, mark WF-03 explicitly as a
heuristic/manual-review rule and document that template expressions (e.g.,
"{planning_artifacts}/{spec_file}") are ambiguous and will only generate a
flagged finding for reviewer inspection.

---

### PATH-01 — Internal References Must Be Relative From Originating File

- **Severity:** CRITICAL
- **Applies to:** all files in the skill
- **Rule:** Any reference from one file in the skill to another file in the same skill must be a relative path resolved from the directory of the originating file. Use `./` prefix for siblings or children, `../` for parent traversal. Bare relative filenames in markdown links (e.g., `[text](sibling.md)`) are also acceptable.
- **Detection:** Scan for file path references (in markdown links, frontmatter values, inline backtick paths, and prose instructions like "Read fully and follow"). Verify each internal reference uses relative notation (`./`, `../`, or bare filename). Always resolve the path from the originating file's directory — a reference to `./steps/step-01.md` from a file already inside `steps/` would resolve to `steps/steps/step-01.md`, which is wrong.
- **Examples:**
- CORRECT: `./steps/step-01-init.md` (from workflow.md at skill root to a step)
- CORRECT: `./template.md` (from workflow.md to a sibling)
- CORRECT: `../template.md` (from steps/step-01.md to a skill-root file)
- CORRECT: `[workflow.md](workflow.md)` (markdown link to sibling — bare relative)
- CORRECT: `./step-02-plan.md` (from steps/step-01.md to a sibling step)
- WRONG: `./steps/step-02-plan.md` (from a file already inside steps/ — resolves to steps/steps/)
- WRONG: `{installed_path}/template.md`
- WRONG: `{project-root}/.claude/skills/my-skill/template.md`
- WRONG: `/Users/someone/.claude/skills/my-skill/steps/step-01.md`
- WRONG: `~/.claude/skills/my-skill/file.md`

Comment on lines +111 to +128
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

PATH-01 assumes universal resolution but learnings show skill-type-specific conventions.

The rule states "Always resolve the path from the originating file's directory" (line 108), yet learnings document two distinct conventions:

  1. Converted-workflow skills (e.g., bmad-create-ux-design): ./step-02.md inside steps/step-01.md resolves to steps/step-02.md (relative to the step file's own directory).
  2. bmad-quick-dev-new-preview-style skills: ./ resolves relative to the skill root, so step files need ../ to reference skill-root siblings.

The validator provides no mechanism to detect which convention applies. Without skill-type classification, PATH-01 will produce false positives on one category or the other.

Either:

  • Add a skill-type detection rule (e.g., presence of domain-steps/ vs steps/ vs manifest metadata) and apply convention-specific path rules, or
  • Document that this validator assumes the converted-workflow convention and may flag bmad-quick-dev-new-preview patterns as violations.

Based on learnings: "converted-workflow native skills use a different ./ path resolution convention from bmad-quick-dev-new-preview-style skills."

🧰 Tools
🪛 LanguageTool

[uncategorized] ~107-~107: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...t traversal. Bare relative filenames in markdown links (e.g., [text](sibling.md)) are ...

(MARKDOWN_NNP)


[uncategorized] ~108-~108: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...on:** Scan for file path references (in markdown links, frontmatter values, inline backt...

(MARKDOWN_NNP)


[uncategorized] ~113-~113: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... CORRECT: [workflow.md](workflow.md) (markdown link to sibling — bare relative) - CO...

(MARKDOWN_NNP)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 103 - 120, PATH-01 currently assumes a
single "./" resolution convention and will produce false positives for skills
using the bmad-quick-dev-new-preview convention; update the validator logic that
enforces PATH-01 to first detect skill type (e.g., check for markers like
presence of domain-steps/, steps/, or a manifest field such as
skill-type/manifest metadata) and apply the appropriate resolution rule per
type, or alternatively document in PATH-01 that the rule assumes the
converted-workflow convention; specifically modify the PATH-01 enforcement
branch in the skill path validation module to (1) implement a detector that
examines repository layout/manifest, (2) choose the resolution strategy
(step-file-relative vs skill-root-relative) accordingly, and (3) run path
normalization and validation against that chosen strategy so
bmad-quick-dev-new-preview-style skills are not incorrectly flagged.

### PATH-02 — No `installed_path` Variable

- **Severity:** HIGH
- **Applies to:** all files in the skill
- **Rule:** The `installed_path` variable is an anti-pattern from the pre-skill workflow era. It must not be defined in any frontmatter, and `{installed_path}` must not appear anywhere in any file.
- **Detection:** Search all files for:
- Frontmatter key `installed_path:`
- String `{installed_path}` anywhere in content
- Markdown/prose assigning `installed_path` (e.g., `` `installed_path` = `.` ``)
- **Fix:** Remove all `installed_path` definitions. Replace every `{installed_path}/path` with `./path` (relative from the file that contains the reference). If the reference is in a step file and points to a skill-root file, use `../path` instead.

Comment on lines +129 to +139
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

PATH-02 detection risks false positives on educational prose.

Line 129 instructs: "String {installed_path} anywhere in content." This will flag explanatory text like:

"Never use {installed_path} — it is deprecated."

or

"Legacy workflows used {installed_path}, but skills must use relative paths."

Add context-aware detection:

  • Exempt prose that negates or warns against installed_path usage (e.g., preceded by "don't", "never", "avoid", "deprecated").
  • Or refine the rule to state that educational mentions in comments/prose are acceptable if they explicitly discourage usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 121 - 131, The PATH-02 rule currently
flags any occurrence of `{installed_path}` or `installed_path:` even when used
in explanatory prose; update the detection in tools/skill-validator.md so
PATH-02 only triggers on actual definitions or actionable uses by requiring: (1)
frontmatter key `installed_path:` or backticked/inline occurrences that are part
of a path expression (e.g., `{installed_path}/` or `` `installed_path` =` ``),
and (2) ignore prose mentions that are explicitly negating or warning (preceded
by words like "don't", "never", "avoid", "deprecated", "legacy", or phrases like
"must not"), implemented via regex/negative-context checks or an exception list
for phrases; keep the existing Fix instructions for real usages.

### PATH-03 — External References Must Use `{project-root}` or Config Variables

- **Severity:** HIGH
- **Applies to:** all files in the skill
- **Rule:** References to files outside the skill directory must use `{project-root}/...` or a config-derived variable path (e.g., `{planning_artifacts}/...`, `{implementation_artifacts}/...`).
- **Detection:** Identify file references that point outside the skill. Verify they start with `{project-root}` or a known config variable. Flag absolute paths, home-relative paths (`~/`), or bare paths that resolve outside the skill.
- **Fix:** Replace with `{project-root}/...` or the appropriate config variable.

### PATH-04 — No Intra-Skill Path Variables

- **Severity:** MEDIUM
- **Applies to:** all files (frontmatter AND body content)
- **Rule:** Variables must not store paths to files within the same skill. These paths should be hardcoded as relative paths inline where used. This applies to YAML frontmatter variables AND markdown body variable assignments (e.g., `` `template` = `./template.md` `` under a `### Paths` section).
- **Detection:** For each variable with a path-like value — whether defined in frontmatter or in body text — determine if the target is inside the skill directory. Indicators: value starts with `./`, `../`, `{installed_path}`, or is a bare filename of a file that exists in the skill. Exclude variables whose values are prefixed with a config variable like `{planning_artifacts}`, `{implementation_artifacts}`, `{project-root}`, or other config-derived paths — these are external references and are legitimate.
- **Fix:** Remove the variable. Replace each `{variable_name}` usage with the direct relative path.
- **Exception:** If a path variable is used in 4+ locations across multiple files and the path is non-trivial, a variable MAY be acceptable. Flag it as LOW instead and note the exception.

---

### STEP-01 — Step File Naming

- **Severity:** MEDIUM
- **Applies to:** files in `steps/` directory
- **Rule:** Step files must be named `step-NN-description.md` where NN is a zero-padded two-digit number. An optional single-letter variant suffix is allowed for branching steps (e.g., `step-01b-continue.md`).
- **Detection:** Regex: `^step-\d{2}[a-z]?-[a-z0-9-]+\.md$`
- **Fix:** Rename to match the pattern.
Comment on lines +159 to +165
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

STEP-01 allows branching variants but doesn't define branching rules.

The rule permits step-01a-continue.md and step-01b-alt.md (single-letter suffix for "branching steps") but provides zero guidance on:

  • How many branches are allowed per step number?
  • Must all branches be terminal, or can they rejoin?
  • Are there naming conventions for branch semantics (e.g., a = primary, b = fallback)?
  • How does STEP-03 (next-step reference) apply to branches?

Without these constraints, the validator cannot detect malformed branch structures (e.g., step-01c existing without step-01a/01b, or cyclic branch references).

Add STEP-01b rule defining branch structure constraints, or document that branching validation is out of scope.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 151 - 157, The STEP-01 rule currently
allows single-letter branching but lacks constraints; add a new STEP-01b rule
(or an explicit note that branching is out of scope) that specifies: allowed
branch letters (e.g., a..z) and max branches per step, required presence rules
(e.g., branch 'a' must exist before 'b'/'c'), whether branches are terminal or
may rejoin, and how STEP-03 (next-step references) resolves across branches
(e.g., next-step can reference step-02 or step-01b); update the validator to
check for missing sibling branches, orphan branches (e.g., step-01c without
01a/01b), and cyclic branch references, and reference the existing filename
regex ^step-\d{2}[a-z]?-[a-z0-9-]+\.md$ when describing naming/branch-letter
parsing.

⚠️ Potential issue | 🟠 Major

Missing validation: duplicate step numbers.

STEP-01 validates naming format but doesn't prevent:

  • step-01.md and step-01-init.md (both numbered 01, one missing description)
  • step-02a-branch.md and step-02-main.md (number collision between branch and non-branch)

This causes ambiguity in sequential execution order. Add duplicate-number detection:

STEP-01b — No Duplicate Step Numbers

  • Severity: HIGH
  • Applies to: steps/ directory
  • Rule: Each step number (NN in step-NN*.md) must be unique unless differentiated by a variant suffix (a/b/c). A step cannot have both step-05.md and step-05a.md.
  • Detection: Extract all step numbers (with/without suffix); flag collisions.
  • Fix: Renumber conflicting steps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 151 - 157, Add a new validation rule
"STEP-01b — No Duplicate Step Numbers" that scans files in the steps/ directory,
extracts the two-digit step number (NN) from names matching the existing STEP-01
regex (`step-NN[variant]?-...`), and reports a HIGH-severity error when the same
NN appears in more than one file unless they are distinct variant suffixes
(e.g., allow 02a vs 02b but not 02 and 02a together); implement detection by
collecting NN and optional suffix for each filename, flagging collisions where
the same NN appears with both a suffix and a plain form or appears in multiple
non-variant files, and update the rule message to instruct renaming/renumbering
conflicting step files.


### STEP-02 — Step Must Have a Goal Section

- **Severity:** HIGH
- **Applies to:** step files
- **Rule:** Each step must clearly state its goal. Look for a heading like `## YOUR TASK`, `## STEP GOAL`, `## INSTRUCTIONS`, `## INITIALIZATION`, `## EXECUTION`, `# Step N:`, or a frontmatter `goal:` field.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STEP-02 detection treats headings like ## INITIALIZATION/## EXECUTION as evidence of a goal, but those sections can exist without stating an explicit goal, so the validator may miss goal-less step files.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

- **Detection:** Scan for goal-indicating headings (including `# Step N: Title` as a top-level heading that names the step's purpose) or frontmatter.
- **Fix:** Add a clear goal section.

### STEP-03 — Step Must Reference Next Step

- **Severity:** MEDIUM
- **Applies to:** step files (except the final step)
- **Rule:** Each non-terminal step must contain a reference to the next step file for sequential execution.
- **Detection:** Look for `## NEXT` section or inline reference to a next step file. Remember to resolve the reference from the originating file's directory (PATH-01 applies here too).
- **Fix:** Add a `## NEXT` section with the relative path to the next step.
- **Note:** A terminal step is one that has no next-step reference and either contains completion/finalization language or is the highest-numbered step. If a workflow branches, there may be multiple terminal steps.

Comment on lines +175 to +183
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

STEP-03 terminal-step detection is circular and unexecutable.

The rule says: "A terminal step is one that has no next-step reference" (line 174). But STEP-03 is the rule that requires next-step references. This creates a logical circle:

  • To apply the rule, you must identify non-terminal steps.
  • To identify non-terminal steps, you must know which steps are terminal.
  • To know which steps are terminal, you check for missing next-step references.
  • But that's what the rule is trying to validate.

The note attempts to resolve this ("contains completion/finalization language or is the highest-numbered step") but introduces new ambiguities:

  • What phrases constitute "finalization language"?
  • Highest-numbered step assumes linear sequencing, but line 156 allows branching.

Provide deterministic terminal-step detection logic:

  • "A step is terminal if it is the highest-numbered step in its branch AND contains no ## NEXT section."
  • Or require explicit terminal-step marking (e.g., frontmatter terminal: true).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 167 - 175, The STEP-03 rule's
terminal-step detection is ambiguous and circular; change the rule to require
deterministic detection: declare that a step is terminal only if it both
contains no "## NEXT" section and is either the highest-numbered step within its
branch (determine branch by shared workflow ID or directory) OR the file
includes an explicit terminal marker (e.g., frontmatter key terminal: true).
Update the STEP-03 description to state these two alternative criteria (no "##
NEXT" + highest-numbered-in-branch OR explicit terminal: true) and clarify how
to resolve branch membership (by workflow frontmatter or directory naming) so
automated validators can unambiguously determine terminal vs non-terminal steps.

### STEP-04 — Halt Before Menu

- **Severity:** HIGH
- **Applies to:** step files
- **Rule:** Any step that presents a user menu (e.g., `[C] Continue`, `[A] Approve`, `[S] Split`) must explicitly HALT and wait for user response before proceeding.
- **Detection:** Find menu patterns (bracketed letter options). Check that text within the same section (under the same heading) includes "HALT", "wait", "stop", "FORBIDDEN to proceed", or equivalent.
- **Fix:** Add an explicit HALT instruction before or after the menu.

### STEP-05 — No Forward Loading

- **Severity:** HIGH
- **Applies to:** step files
- **Rule:** A step must not load or read future step files until the current step is complete. Just-in-time loading only.
- **Detection:** Look for instructions to read multiple step files simultaneously, or unconditional references to step files with higher numbers than the current step. Exempt locations: `## NEXT` sections, navigation/dispatch sections that list valid resumption targets, and conditional routing branches.
- **Fix:** Remove premature step loading. Ensure only the current step is active.

Comment on lines +192 to +199
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

STEP-05 exemption for "dispatch sections" is undefined.

Line 189 exempts "navigation/dispatch sections that list valid resumption targets" from the forward-loading prohibition, but provides no detection criteria. What syntactic or semantic patterns identify a dispatch section vs prohibited forward loading?

Without clear markers (e.g., a ## DISPATCH or ## NAVIGATION heading), the exemption is unenforceable — every multi-step reference could be argued as "navigation."

Specify:

  • Heading patterns that mark dispatch sections (e.g., ## NAVIGATION, ## RESUME FROM, ## ROUTING), or
  • Structural indicators (e.g., bulleted list of [X] Resume at step-NN options under a menu), or
  • Remove the exemption and require dispatch logic to reference steps just-in-time.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~189-~189: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ection:** Look for instructions to read multiple step files simultaneously, or unconditional ...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 184 - 191, The STEP-05 rule exemption
for "navigation/dispatch sections" needs concrete detection criteria: update
STEP-05 to list explicit heading patterns (e.g., `## NAVIGATION`, `## DISPATCH`,
`## RESUME FROM`, `## ROUTING`) and/or structural indicators (e.g., a bulleted
or numbered menu of resumption targets like "- Resume at step-NN" or checkboxes
"[ ] Resume at step-NN") that will be treated as allowed dispatch sections;
alternatively state that if no such heading/structure exists the exemption does
not apply (or remove the exemption entirely). Modify the rule text in STEP-05 to
include these exact heading examples and the structural patterns so validators
can reliably identify dispatch sections versus prohibited forward-loading
references.

### STEP-06 — Step File Frontmatter: No `name` or `description`

- **Severity:** MEDIUM
- **Applies to:** step files
- **Rule:** Step files should not have `name:` or `description:` in their YAML frontmatter. These are metadata noise — the step's purpose is conveyed by its goal section and filename.
- **Detection:** Parse step file frontmatter for `name:` or `description:` keys.
- **Fix:** Remove `name:` and `description:` from step file frontmatter.

### STEP-07 — Step Count

- **Severity:** LOW
- **Applies to:** workflow as a whole
- **Rule:** A sharded workflow should have between 2 and 10 step files. More than 10 risks LLM context degradation.
- **Detection:** Count files matching `step-*.md` in the `steps/` directory.
- **Fix:** Consider consolidating steps if over 10.

Comment on lines +159 to +215
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing validation: step files must reside in steps/ directory.

STEP-01 through STEP-07 all reference "files in steps/ directory" or "the steps/ directory" (lines 154, 205), but no rule enforces that:

  1. A steps/ directory exists when step files are present.
  2. All step-*.md files are located inside steps/, not scattered elsewhere.
  3. No step files exist outside steps/ (e.g., step-01-rouge.md at skill root).

Add:
STEP-00 — Step Files Must Be In steps/ Directory

  • Severity: HIGH
  • Applies to: skill directory
  • Rule: All files matching step-*.md must be located in the steps/ subdirectory. No step files may exist at skill root or other locations.
  • Detection: Search for step-*.md files; verify all are under steps/.
  • Fix: Move misplaced step files into steps/.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~189-~189: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ection:** Look for instructions to read multiple step files simultaneously, or unconditional ...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 151 - 207, Add a new HIGH-severity
rule "STEP-00 — Step Files Must Be In steps/ Directory" before STEP-01 that
applies to the skill directory and enforces that all files matching step-*.md
live under a steps/ subdirectory; specify the Rule ("All files matching
`step-*.md` must be located in the `steps/` subdirectory. No step files may
exist at skill root or other locations."), Detection (search for files matching
`step-*.md` and verify their path is under `steps/`), and Fix ("Move misplaced
step files into `steps/`"), and ensure documentation references in
STEP-01..STEP-07 reflect that STEP-00 validates presence of the steps/ directory
and rejects step files outside it.

---

### SEQ-01 — No Skip Instructions

- **Severity:** HIGH
- **Applies to:** all files
- **Rule:** No file should instruct the agent to skip steps or optimize step order. Sequential execution is mandatory.
- **Detection:** Scan for phrases like "skip to step", "jump to step", "skip ahead", "optimize the order", "you may skip". Exclude negation context (e.g., "do NOT skip steps", "NEVER skip") — these are enforcement instructions, not skip instructions.
- **Exception:** Conditional routing (e.g., "if X, go to step N; otherwise step M") is valid workflow branching, not skipping.

Comment on lines +218 to +225
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

SEQ-01 skip vs conditional-routing distinction is underspecified.

The rule forbids "skip to step" but exempts "conditional routing." The detection guidance doesn't explain how to distinguish:

  • Skip (forbidden): "If you already have X, skip to step 5."
  • Route (allowed): "If X exists, go to step-05a; otherwise step-05b."

Both use similar syntax ("if X, go to step N"). The validator needs concrete differentiation logic:

  • Route = exhaustive branching (if/else covering all cases, each branch leading to a different next step)
  • Skip = optional fast-path that bypasses intermediate steps in a linear sequence

Without this, the rule will either under-flag (miss skips disguised as routes) or over-flag (mark valid branching as violations).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 210 - 217, Update the SEQ-01 rule to
explicitly differentiate "skip" from "conditional routing": treat a conditional
as allowed routing only when it is exhaustive and mutually exclusive (e.g., an
if/else or switch where each branch maps to different subsequent step labels and
covers all logical cases); treat it as a forbidden skip when a single
conditional directs to a later step without alternative branches (no
else/alternate labels), or when it contains fast-path language like "skip to
step" or "skip ahead" that bypasses intermediate sequential steps. Modify the
detection logic that scans for phrases (the existing SEQ-01 detection list) to
(1) require presence of explicit else/alternate branches or step-label pairs for
allowed routing, (2) flag single-branch conditionals that reference a later step
number and do not mention alternatives as skips, and (3) keep negation handling
(e.g., "do NOT skip") as currently excluded; update examples and tests for
SEQ-01 to cover exhaustive branching vs single-branch skip cases.

### SEQ-02 — No Time Estimates

- **Severity:** LOW
- **Applies to:** all files
- **Rule:** Workflow files should not include time estimates. AI execution speed varies too much for estimates to be meaningful.
- **Detection:** Scan for patterns like "takes X minutes", "~N min", "estimated time", "ETA".
- **Fix:** Remove time estimates.

---

### REF-01 — Variable References Must Be Defined

- **Severity:** HIGH
- **Applies to:** all files
- **Rule:** Every `{variable_name}` reference in any file (body text, frontmatter values, inline instructions) must resolve to a defined source. Valid sources are:
1. A frontmatter variable in the same file
2. A frontmatter variable in the skill's `workflow.md` (workflow-level variables are available to all steps)
3. A known config variable from the project config (e.g., `project-root`, `planning_artifacts`, `implementation_artifacts`, `communication_language`)
4. A known runtime variable set during execution (e.g., `date`, `status`, `project_name`, user-provided input variables)
- **Detection:** Collect all `{...}` tokens in the file. For each, check whether it is defined in the file's own frontmatter, in `workflow.md` frontmatter, or is a recognized config/runtime variable. Flag any token that cannot be traced to a source. Use the config variable list from the project's `config.yaml` as the reference for recognized config variables. Runtime variables are those explicitly described as user-provided or set during execution in the workflow instructions.
- **Exceptions:**
- Double-curly `{{variable}}` — these are template placeholders intended to survive into generated output (e.g., `{{project_name}}` in a template file). Do not flag these.
- Variables inside fenced code blocks that are clearly illustrative examples.
- **Fix:** Either define the variable in the appropriate frontmatter, or replace the reference with a literal value. If the variable is a config variable that was misspelled, correct the spelling.

### REF-02 — File References Must Resolve

- **Severity:** HIGH
- **Applies to:** all files
- **Rule:** All file path references within the skill (markdown links, backtick paths, frontmatter values) should point to files that plausibly exist.
- **Detection:** For internal references, verify the target file exists in the skill directory. For external references using config variables, verify the path structure is plausible (you cannot resolve config variables, but you can check that the path after the variable looks reasonable — e.g., `{planning_artifacts}/*.md` is plausible, `{planning_artifacts}/../../etc/passwd` is not).
- **Fix:** Correct the path or remove the dead reference.

---

## Report Template

When reporting findings, use this format:

```markdown
# Skill Validation Report: {skill-name}

**Directory:** {path}
**Date:** {date}
**Files scanned:** {count}

## Summary

| Severity | Count |
|----------|-------|
| CRITICAL | N |
| HIGH | N |
| MEDIUM | N |
| LOW | N |

## Findings

### {RULE-ID} — {Rule Title}

- **Severity:** {severity}
- **File:** `{relative-path-within-skill}`
- **Line:** {line number or range, if identifiable}
- **Detail:** {what was found}
- **Fix:** {specific fix for this instance}

---

(repeat for each finding, grouped by rule ID)

## Passed Rules

(list rule IDs that produced no findings)
```
Comment on lines +261 to +298
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Report template formatting is underspecified.

The template provides structure but leaves key formatting decisions ambiguous:

  1. Findings sort order (line 270): "grouped by rule ID" — but should rule groups be ordered by severity (CRITICAL first) or alphabetically (PATH-01, REF-01, SEQ-01, SKILL-01...)?

  2. Multiple instances per rule: If PATH-01 fires on 5 different files, does the report show:

    • One ### PATH-01 section with 5 bulleted findings, or
    • Five separate ### PATH-01 sections?
  3. Line range format: "Line: {line number or range, if identifiable}" — what syntax? 42, 42-48, L42:L48, or lines 42-48?

  4. Zero-count severity rows: Should the summary table include | MEDIUM | 0 | if no MEDIUM findings exist, or omit zero rows?

Add a Formatting Conventions subsection specifying:

  • Sort: severity desc, then rule ID asc
  • Multiple instances: combine under one rule heading as bulleted list
  • Line range: 42 (single) or 42-48 (range)
  • Summary table: include all severities, show 0 for empty categories
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/skill-validator.md` around lines 238 - 275, Add a "Formatting
Conventions" subsection to the "Report Template" (near the "Findings" section)
that codifies the ambiguous points: state that rule groups are sorted by
severity (CRITICAL, HIGH, MEDIUM, LOW) descending and then by rule ID ascending;
state that multiple instances of the same rule (e.g., PATH-01) are combined
under one heading (single "### {RULE-ID} — {Rule Title}" with a bulleted list of
findings); specify line notation as either a single number "42" or a range
"42-48" (no prefixes like "L" or "lines"); and require the Summary table to
always include all severities with zero counts shown (e.g., "| MEDIUM | 0 |").


If zero findings: report "All {N} rules passed. No findings." and list all passed rule IDs.
Loading