Skip to content

fix: find skills in category folders and require Node.js 22 - #5

Open
PaulBoye-py wants to merge 2 commits into
mainfrom
fix/skill-paths-node-22
Open

PaulBoye-py wants to merge 2 commits into
mainfrom
fix/skill-paths-node-22

Conversation

@PaulBoye-py

Copy link
Copy Markdown
Collaborator

Summary

Fixes the CLI so it installs the current skills from the Cloudinary skills repo again, and raises the Node.js requirement to 22 for next-cloudinary v7.

Problem

The CLI clones the skills repo when it creates a project and copies cloudinary-next, cloudinary-docs and cloudinary-transformations into it. It falls back to copies bundled in the CLI only if the clone fails.

On 18 September the skills repo moved its skills into category folders (skills/frameworks/…, skills/platform/…). The CLI only looks in skills/<name>, so it no longer finds any of the three. It then silently uses the bundled copies, even though the clone worked. Two of the bundled copies (docs and transformations) are already out of date.

Changes

  • findSkillSourceDir also looks in each category folder under skills/, and only accepts a folder that contains a SKILL.md.
  • If the clone works but a skill still isn't found, the CLI now prints a warning before using the bundled copy, so a future layout change doesn't go unnoticed.
  • engines.node raised from >=18.0.0 to >=22.0.0, and the README requirement changed from "current LTS" to "Node.js 22 or later". The template now uses next-cloudinary v7 (chore: upgrade next-cloudinary to v7 create-cloudinary-next-template#1), which requires Node.js 22.
  • Refreshed the bundled cloudinary-docs and cloudinary-transformations copies from the skills repo.

The bundled cloudinary-next copy is unchanged (it matches the skills repo's current version). The v7 update to that skill is in cloudinary-devs/skills#28. With this fix, the CLI picks it up from the skills repo as soon as it's merged; the bundled copy can be refreshed afterwards.

Testing

node cli.js --headless --cloudName demo --projectName … --aiTools claude --aiTools cursor --installDeps false:

  • Before: all three installed skills matched the bundled copies; docs and transformations differed from the skills repo.
  • After: all six installed copies (.claude/skills and .agents/skills) are identical to the skills repo's main. The MCP config files are still written.

Not changed

The release workflow still uses Node.js 20, which is end-of-life. It's left for a separate change, since it can only be tested by running a release.

@PaulBoye-py
PaulBoye-py requested a review from strausr November 4, 2026 05:39
Comment thread package.json
"license": "MIT",
"engines": {
"node": ">=18.0.0"
"node": ">=22.0.0"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

engines doesn't enforce the Node.js version by default but is only a warning, or nothing at all, unless they've turned on engine-strict.
We can have the CLI check process.versions.node and stop or warn with a clear message

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @strausr, and a great point too! The CLI now checks process.versions.node at startup and stops with a clear message on anything below 22, before any prompts or files. I went with stopping rather than a warning, since the template needs Node 22 for next-cloudinary v7, but happy to switch it to a warning if you prefer.

Comment thread cli.js Outdated
}

if (!sourceDir) {
if (remoteInstallAvailable) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This sits inside for (const targetRoot of targetRoots). If the user picks Claude plus Cursor/Copilot/Generic, there are two target folders (.claude/skills and .agents/skills), so every missing skill gets the same warning twice.

Instead we can work out sourceDir for each skill once, before the targetRoots loop, and warn there.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's a good catch, thank you. Skill sources are now resolved once before the target-folder loop, so each missing skill is reported once. I tested it with Claude plus Cursor against a skills repo with one skill removed: two warnings before, one now, with identical installed files.

Comment thread package.json
@@ -20,7 +20,7 @@
"author": "Cloudinary, Inc.",
"license": "MIT",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to update lockfle to match version

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated, thank you! The lockfile's root version and engines now match package.json. It drifts because each release bumps the version in both files, but the release commit only includes package.json and CHANGELOG.md (all four beta release commits show this). Would you like me to add package-lock.json to the @semantic-release/git assets in a separate PR, so it stays in sync after each release?

…kill

npm only warns about engines unless engine-strict is set, so the CLI now stops with a clear message on Node.js below 22. Skill sources are resolved once before copying, so a missing skill is reported once even with several target folders. Syncs package-lock.json with package.json (version and engines).
@PaulBoye-py
PaulBoye-py requested a review from strausr November 8, 2026 11:02
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.

2 participants