Skip to content

fix: use package license for skills#2048

Open
maxchang3 wants to merge 1 commit intonpmx-dev:mainfrom
mcontrib:fix-skills-license-fallback-2043
Open

fix: use package license for skills#2048
maxchang3 wants to merge 1 commit intonpmx-dev:mainfrom
mcontrib:fix-skills-license-fallback-2043

Conversation

@maxchang3
Copy link
Contributor

@maxchang3 maxchang3 commented Mar 12, 2026

🔗 Linked issue

resolves #2043

🧭 Context

Currently, the logic for retrieving a skill’s license only checks the frontmatter.

As a result, when a skill does not specify a license there, the system displays a misleading "No license specified" warning, even if the parent package already defines a valid license.

📚 Description

This PR improves license handling in skills.ts by retrieving and passing the package license as a fallback when a skill does not specify its own license.


(A related question: if a skill's license differs from the package's license, should we explicitly surface this (or even treat it as a warning)?)

@vercel
Copy link

vercel bot commented Mar 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 12, 2026 2:13pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 12, 2026 2:13pm
npmx-lunaria Ignored Ignored Mar 12, 2026 2:13pm

Request Review

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

The validateSkill function in server/utils/skills.ts was modified to accept an optional packageLicense parameter. The fetchSkillsList function now retrieves the package license from npm for the specified version. When constructing a SkillListItem, the license field prioritises frontmatter.license, then falls back to packageLicense. License-based warnings are only triggered when neither source provides a license. In-code comments were added to document the fallback logic.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code changes directly address issue #2043 by implementing package license fallback logic in validateSkill and fetchSkillsList, preventing misleading 'No license specified' warnings.
Out of Scope Changes check ✅ Passed All changes in server/utils/skills.ts are directly related to resolving license fallback handling for skills, with no unrelated modifications present.
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the problem (license warnings despite package-level licenses) and the solution (fallback to package license).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan for PR comments
  • Generate coding plan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

License detection fails for nitro skills

1 participant