Skip to content

Add dedicated load-skills test coverage#502

Merged
jahooma merged 4 commits intomainfrom
aether/add-skills-loading-tests-128e
Apr 15, 2026
Merged

Add dedicated load-skills test coverage#502
jahooma merged 4 commits intomainfrom
aether/add-skills-loading-tests-128e

Conversation

@aether-agent
Copy link
Copy Markdown
Contributor

@aether-agent aether-agent bot commented Apr 15, 2026

Summary

  • add a dedicated load-skills test suite that exercises all four default search roots
  • cover precedence rules across global/project and .claude/.agents directories
  • validate malformed/mismatched/invalid skill definitions and names are skipped with verbose logging

Testing

  • bun run --cwd sdk test src/__tests__/load-skills.test.ts
  • bun run --cwd sdk typecheck

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 15, 2026

Greptile Summary

This PR adds a dedicated test suite (sdk/src/__tests__/load-skills.test.ts) for the existing loadSkills function in sdk/src/skills/load-skills.ts. The four new tests cover: discovering skills from all four default search roots, the two-dimensional precedence order (project > global and .agents > .claude), an intermediate precedence comparison, and rejection of invalid/malformed skill definitions with verbose logging.

Key changes:

  • New writeSkill helper creates on-disk skill fixtures (dir + SKILL.md) to test the full filesystem discovery path
  • Validates that Object.assign-based later-override precedence (~/.claude < ~/.agents < {cwd}/.claude < {cwd}/.agents) is correctly respected
  • Verifies that six classes of invalid inputs are skipped: missing SKILL.md, malformed YAML frontmatter, name/directory mismatch, oversized name, uppercase directory name, and underscore directory name

Confidence Score: 5/5

Test-only PR with no production code changes — safe to merge.

No logic changes are introduced; the file is purely additive test coverage. All four tests correctly reflect the implementation behaviour. The P2 comment about the malformed-YAML fixture is a robustness suggestion, not a correctness bug.

No files require special attention.

Important Files Changed

Filename Overview
sdk/src/tests/load-skills.test.ts New test file — well-structured, covers all four search roots and full precedence matrix; minor reliability concern in the malformed-frontmatter fixture and the skillsPath option path is not exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["loadSkills({ cwd, verbose })"] --> B{skillsPath provided?}
    B -- yes --> C[Single custom dir]
    B -- no --> D[getDefaultSkillsDirs]
    D --> E["~/.claude/skills (priority 1)"]
    D --> F["~/.agents/skills (priority 2)"]
    D --> G["{cwd}/.claude/skills (priority 3)"]
    D --> H["{cwd}/.agents/skills (priority 4)"]
    C --> I[discoverSkillsFromDirectory]
    E & F & G & H --> I
    I --> J{isValidSkillName?}
    J -- no --> K["console.warn (verbose)"]
    J -- yes --> L{SKILL.md exists?}
    L -- no --> M[silent skip]
    L -- yes --> N[parseFrontmatter]
    N -- null --> O["console.error: Invalid frontmatter"]
    N -- ok --> P[SkillFrontmatterSchema.safeParse]
    P -- fail --> Q["console.error: Invalid skill frontmatter"]
    P -- ok --> R{name matches dirName?}
    R -- no --> S["console.error: Name mismatch"]
    R -- yes --> T[Add to SkillsMap]
    T --> U[Object.assign merge]
    U --> V[Return SkillsMap]
Loading

Reviews (1): Last reviewed commit: "Add load skills test suite" | Re-trigger Greptile

Comment thread sdk/src/__tests__/load-skills.test.ts
Comment thread sdk/src/__tests__/load-skills.test.ts
jahooma and others added 3 commits April 15, 2026 11:43
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@jahooma jahooma merged commit ae49d36 into main Apr 15, 2026
34 checks passed
@jahooma jahooma deleted the aether/add-skills-loading-tests-128e branch April 15, 2026 19:04
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.

1 participant