Skip to content

Add skill diff command (#46)#66

Merged
devrimcavusoglu merged 2 commits intomainfrom
feature/skill-diff
Mar 17, 2026
Merged

Add skill diff command (#46)#66
devrimcavusoglu merged 2 commits intomainfrom
feature/skill-diff

Conversation

@devrimcavusoglu
Copy link
Copy Markdown
Owner

Summary

  • Add skern skill diff <name> --platform <platform> to compare a registry skill against its installed copy on a platform
  • Add skern skill diff <name-a> <name-b> to compare two registry skills by name
  • Compare metadata fields (name, description, version, author, tags, allowed-tools) and body content
  • Support --json, --scope, and --platform flags

Test plan

  • 12 table-driven tests covering both modes (registry-vs-registry, registry-vs-platform)
  • Tests for identical skills, different metadata, different body, text output, not-found, missing flags, --platform all rejection, default scope
  • make build passes
  • make test passes (all packages)
  • make lint — 0 issues

Closes #46

🤖 Generated with Claude Code

Compare two registry skills or a registry skill against its installed
copy on a platform. Supports --json, --scope, and --platform flags.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@devrimcavusoglu devrimcavusoglu left a comment

Choose a reason for hiding this comment

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

Code Review

Overall this is a solid, well-tested PR that follows project conventions. All checks pass locally (make test, make lint). A few items to consider:

Issues

  1. Missing modified-by comparisoncompareSkills covers all Skill fields except Metadata.ModifiedBy. This is the append-only provenance list and a likely source of drift between registry and platform copies. Either add it to the comparison or document its intentional omission.

  2. --scope default inconsistency between modes — In two-arg mode, empty --scope searches project-first then user (via resolveSkill). In one-arg (platform) mode, it hardcodes "user". This is pragmatic but subtly inconsistent. The Long description should at minimum note the default.

  3. Missing skill.ValidateName call — Other commands (skill install, skill remove) validate names upfront for clearer error messages. This PR skips that step and relies on downstream lookup failures.

  4. Long description slightly inaccurate — Says --scope is required for one-arg mode, but it actually defaults to "user". Also --scope applies in two-arg mode too. Suggest:

    With one argument, compares a registry skill against its installed copy on a platform
    (requires --platform; --scope defaults to "user").
    
    With two arguments, compares two registry skills by name
    (--scope filters to a specific scope; omit to search both).
    

Test coverage

12 tests with good coverage of both modes, JSON/text output, and error paths. Nice-to-have additions: --scope project for platform mode, invalid --scope value, two-arg mode with explicit --scope.

None of the above are blocking — approve once addressed or acknowledged.

…ing tests

- Compare modified-by field in skill diff for drift detection
- Validate skill names upfront with skill.ValidateName
- Fix Long description to accurately document --scope defaults
- Add tests: project scope, invalid scope, explicit scope, invalid name, modified-by diff

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@devrimcavusoglu
Copy link
Copy Markdown
Owner Author

Pushed fixes addressing the review findings:

  • modified-by comparisoncompareSkills now diffs the modified-by provenance list, formatted as "name (type/platform @ date)" entries
  • Name validation — Added upfront skill.ValidateName for all args, matching skill install/skill remove convention
  • Long description — Updated to accurately document --scope defaults per mode
  • 5 new testsProjectScope, InvalidScope, ExplicitScope (two-arg), InvalidName, DifferentModifiedBy

All checks pass locally (make build, make test, make lint — 0 issues). Total test count: 17.

@devrimcavusoglu devrimcavusoglu merged commit 113f29d into main Mar 17, 2026
4 checks passed
@devrimcavusoglu devrimcavusoglu deleted the feature/skill-diff branch March 17, 2026 14:22
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.

Add skill diff command

1 participant