Skip to content

fix(contract): define one authoritative SKILL.md contract (#583)#626

Open
AlvinStanescu wants to merge 2 commits intomainfrom
fix/canonical-skill-contract-583
Open

fix(contract): define one authoritative SKILL.md contract (#583)#626
AlvinStanescu wants to merge 2 commits intomainfrom
fix/canonical-skill-contract-583

Conversation

@AlvinStanescu
Copy link
Copy Markdown
Collaborator

Closes #583.

Problem

Issue #583 documented contract drift between root guidance, actual skills, and validators:

  • CLAUDE.md required description to include TRIGGER/DO NOT TRIGGER clauses, while .claude/rules/skill-structure.md and CONTRIBUTING.md explicitly forbid them. Reality: 0 of 20 skills use that form.
  • when_to_use is used in 5 real skills (uipath-rpa, uipath-interact, uipath-planner, uipath-feedback, uipath-tasks) but is undocumented in CONTRIBUTING.md.
  • CLAUDE.md and .claude/rules/skill-structure.md both said "no cross-skill references", yet every description contains redirects to sibling skills (and uipath-planner exists specifically to route between them).
  • validate-skill-descriptions.sh only checked description length — no name-folder match, no required-field check.
  • Strict rules like "MUST start with brand identity" and "no [PREVIEW] prefix" don't match reality (e.g., uipath-review uses [PREVIEW]; 7 skills lead with action verbs instead of "UiPath…").

Approach

Ground truth = current skill structure. No skills were modified. All four changed files now agree with what skills actually look like.

Audit (20 skills)

Field Coverage
name 20/20 (required)
description 20/20 (required)
allowed-tools 14/20 (optional)
when_to_use 5/20 (optional)
user-invocable 5/20 (optional, always true)
Nested metadata: 0/20 ✓
TRIGGER: / DO NOT TRIGGER: clauses 0/20
redirects in description ~20/20

Changes

  • .claude/rules/skill-structure.md — rewrite as the single source of truth. Required/Optional/Style tables. New ## Cross-Skill Boundaries section: runtime independence required; redirects are required, not violations.
  • CLAUDE.md — drop the TRIGGER/DO NOT TRIGGER requirement. Point at the canonical files. Restate the cross-skill rule correctly.
  • CONTRIBUTING.md — new top-level Canonical SKILL.md Contract section enumerating Required, Optional, Style, Cross-skill boundaries, and What the validator enforces. when_to_use added to the optional fields table.
  • hooks/validate-skill-descriptions.sh — extend with name present, name matches folder, description present, plus a non-fatal warning when combined description + when_to_use > 1,500 chars (Claude Code truncates at 1,536). Filename kept for compatibility with the pre-commit hook and .github/workflows/validate-skills.yml.

Validation

  • bash hooks/validate-skill-descriptions.sh skills/uipath-*/SKILL.md → all 20 skills pass
  • Failure paths exercised against synthetic fixtures:
    • Name/folder mismatch → fails
    • Missing description → fails
    • 1025-char description → fails (existing behavior preserved)
    • 1600-char combined → warns + exits 0

Test plan

  • CI: Validate Skills workflow passes against this branch
  • CI: pre-commit hook (if installed locally via scripts/setup-hooks.sh) passes
  • Manual: render CONTRIBUTING.md on GitHub, verify the new Canonical SKILL.md Contract anchor and table-of-contents entry
  • Manual: confirm CLAUDE.md links to .claude/rules/skill-structure.md and CONTRIBUTING.md#canonical-skillmd-contract resolve

🤖 Generated with Claude Code

AlvinStanescu and others added 2 commits May 7, 2026 07:54
Document how to develop long-running or experimental skills in this
public repository: feature-branch flow off main, public-repo discipline
(no secrets, customer-readable commits), experimental callout in
SKILL.md body, and rules on when to merge to main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve drift between root guidance, real skill structure, and validators.
Establishes a single canonical SKILL.md contract that matches how skills
are actually written today, and tightens the validator to enforce the
required portion of that contract.

Changes
-------
- .claude/rules/skill-structure.md: rewrite the frontmatter section as the
  single source of truth. Document `when_to_use`, `allowed-tools`, and
  `user-invocable` as supported optional fields (all already used in
  real skills). Soften the "MUST start with brand identity" / "no [PREVIEW]
  prefix" rules to SHOULD-style guidance, since real skills diverge here
  (uipath-review uses [PREVIEW]; uipath-diagnostics, llm-config, feedback,
  maestro-flow, rpa-legacy lead with action verbs). Replace the misleading
  "no references to other skills" rule with explicit Cross-Skill
  Boundaries: runtime independence is required, but `→` redirects in
  description are required for sibling disambiguation, not violations.

- CLAUDE.md: drop the contradictory "must include TRIGGER/DO NOT TRIGGER"
  requirement (0/20 skills use it; .claude/rules already forbids it).
  Point at .claude/rules/skill-structure.md and the new CONTRIBUTING.md
  section as the authoritative contract. Clarify cross-skill rule.

- CONTRIBUTING.md: add a top-level "Canonical SKILL.md Contract" section
  (Required / Optional / Style guidance / Cross-skill boundaries / What
  the validator enforces). Add `when_to_use` to the optional fields table
  in "Adding a New Skill". Update Key Principles to reflect runtime
  independence vs `→` disambiguation.

- hooks/validate-skill-descriptions.sh: extend beyond description-length
  checks. Now also enforces (errors): `name` present, `name` matches
  parent folder, `description` present. Adds a non-fatal warning when
  `description + when_to_use` exceeds 1,500 chars (Claude Code truncates
  at 1,536). All 20 existing skills pass; tested name-mismatch,
  missing-field, and oversized-description failure paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Claude finished @AlvinStanescu's task in 3m 14s —— View job


PR Review: fix(contract): define one authoritative SKILL.md contract (#583)

  • Read review criteria and project conventions
  • Read existing PR conversation
  • Review full diff and all changed files
  • Verify relative links resolve
  • Check for cross-skill references, secrets, consistency
  • Post review

Summary

This PR resolves contract drift between CLAUDE.md, .claude/rules/skill-structure.md, CONTRIBUTING.md, and the validator script by grounding all four in the reality of how the 20 existing skills actually look. It removes the TRIGGER when: / DO NOT TRIGGER when: mandate that zero skills followed, documents when_to_use as an optional field, correctly characterizes redirects as required disambiguation (not cross-skill violations), and extends the validator with name-presence, name-folder-match, and description-presence checks.

Change-by-Change Review

1. .claude/rules/skill-structure.mdHigh

The rewrite is well-structured: Required/Optional/Style tables are clear and the new "Cross-Skill Boundaries" section correctly models what skills actually do.

Issue: Contradictory source-of-truth claim. This file (committed line: "This section is the single source of truth for the SKILL.md frontmatter contract. CLAUDE.md and CONTRIBUTING.md defer to this file.") directly contradicts CONTRIBUTING.md line 83 ("This section is the single source of truth for the SKILL.md frontmatter contract in this repo. Both CLAUDE.md and .claude/rules/skill-structure.md defer to this section."). Each file claims authority and says the other defers. CLAUDE.md sides with skill-structure.md ("The authoritative SKILL.md frontmatter contract lives in .claude/rules/skill-structure.md"). This is exactly the kind of drift the PR aims to fix — one of these claims must be removed so there's a single, unambiguous authority.

Fix: Pick one file as the canonical source. Given that .claude/rules/ files are auto-loaded into the agent's context (making them the most operationally authoritative for AI agents), skill-structure.md is the natural choice. Change CONTRIBUTING.md line 83 to say it mirrors skill-structure.md (not "is the single source of truth"), or vice versa. Fix this →

2. CLAUDE.mdOK

Clean and concise. Correctly positions itself as a "brief overview only" and links to both skill-structure.md (as the authority) and CONTRIBUTING.md (as the human-readable mirror). The TRIGGER/DO NOT TRIGGER mandate is removed. Cross-skill language is fixed. The old "Ensure description field has both TRIGGER and DO NOT TRIGGER conditions" is replaced with a validator command — good improvement.

3. CONTRIBUTING.mdMedium

The new "Canonical SKILL.md Contract" section is thorough and well-organized. The "Developing Experimental Skills" section is unrelated to issue #583 but is a reasonable addition. Two issues:

Issue 1: Source-of-truth contradiction (covered above under skill-structure.md).

Issue 2: "below" reference is wrong. Line 167 says "the [Canonical SKILL.md Contract](#canonical-skillmd-contract) section below" but the Canonical section is above "Adding a New Skill" in the document (lines 81–129 vs. line 131+). The anchor link works regardless, but the word "below" is factually incorrect.

Fix: Change "below" to "above" at CONTRIBUTING.md:167. Fix this →

4. hooks/validate-skill-descriptions.shMedium

The new extract_field function and expanded validation are correct and well-structured. The path-guarding case pattern is a nice defensive addition. Two observations:

Issue 1: Multi-line / folded YAML descriptions are silently ignored. The extract_field awk function only reads the line matching ^field: . If a description used YAML block scalar syntax (> or |), it would extract only the > or | character, not the actual content. Currently all 20 skills use single-line descriptions, so this doesn't break anything today, but it's a silent failure mode for future contributors.

Severity: Low — no current skill is affected, but a future contributor using description: > would pass validation with an effectively empty description.

Issue 2: Unquoted $FILES in for loop. Line 57: for file in $FILES — if $FILES were empty or contained glob-sensitive characters, this could misbehave. In practice, skill paths won't trigger this, but "$FILES" (or using an array) would be more robust.

Severity: Low — theoretical, not practical given the path patterns.

What's Missing

  • .claude/rules/skill-review.md not updated. This file (loaded into agent context for reviews) still says description "starts with the brand/domain identity … NOT a metadata tag like [PREVIEW]" as a structural check (line 23), and lists [PREVIEW]-prefixed descriptions as a red flag (line 29). The PR demoted this to style guidance (SHOULD, not MUST) — uipath-review actively uses [PREVIEW] in its description and is compliant under the new contract. Without updating skill-review.md, automated reviewers will flag compliant skills as having structural issues.
  • .claude/rules/pr-review.md not updated. Line 14 still says "No cross-skill references (skill is fully self-contained)" — the old absolute language that doesn't distinguish runtime imports from disambiguation pointers. This could cause reviewers to incorrectly flag redirects.
  • CONTRIBUTING.md line 388 (Quality Checklist): "No references to other skills (skills must be self-contained)" — this contradicts the new "Cross-skill boundaries" section in the same file that says redirects are required, not violations.

Area Ratings

Area Status Notes
Frontmatter OK Contract is well-defined with Required/Optional/Style tiers
E2E Tests N/A Documentation-only PR, no skill changes
Skill Body OK No skills were modified
References & Assets OK No reference files changed
Repo Hygiene Issue Source-of-truth contradiction between two files; adjacent rule files (skill-review.md, pr-review.md) not updated to match

Issues for Manual Review

  • Verify that CI (Validate Skills workflow) passes — I was unable to execute the validator script directly, but the logic appears correct based on code review.
  • Confirm the [source](https://code.claude.com/docs/en/skills.md) link in skill-structure.md and CONTRIBUTING.md still resolves to the correct Claude Code docs page.
  • The "Developing Experimental Skills" section in CONTRIBUTING.md is new content unrelated to issue Define and enforce one authoritative SKILL contract #583 — confirm this was intentional scope.

Conclusion

The PR's core goal — aligning documentation with reality — is well-executed. The Required/Optional/Style tiering, the nuanced cross-skill boundaries model, and the expanded validator are all solid improvements. However, the contradictory source-of-truth claims between skill-structure.md and CONTRIBUTING.md reintroduce the exact kind of drift this PR aims to eliminate. That should be resolved before merge. The adjacent rule files (skill-review.md, pr-review.md) and the Quality Checklist in CONTRIBUTING.md also still carry old language that conflicts with the new contract.

Recommendation: Request changes — fix the source-of-truth contradiction and update the adjacent rule files to match the new contract.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define and enforce one authoritative SKILL contract

1 participant