fix(intent): use stable node_modules paths for skill references#94
fix(intent): use stable node_modules paths for skill references#94KyleAMathews wants to merge 5 commits intomainfrom
Conversation
Skill paths previously stored absolute filesystem paths that included package-manager-internal directories with version numbers (e.g. .pnpm/@scope+pkg@1.2.3/node_modules/...), breaking whenever packages were updated. Now uses stable node_modules/<name>/... paths for direct deps, falls back to project-relative paths for transitive deps, and keeps absolute paths for global packages. Also updates the install prompt to guide agents on handling transitive dep paths, extracts a toPosixPath utility, fixes macOS /var symlink issue in workspace-patterns tests, and guards against empty package names producing malformed paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR normalizes discovered skill paths to stable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/intent/src/scanner.ts`:
- Around line 441-456: The code risks creating double-slash paths when name is
empty; add a guard checking name before treating it as a node_modules package:
only compute hasStableSymlink and apply the node_modules/${name}/${relFromPkg}
branch when name is non-empty (e.g., wrap the existsSync call and the
hasStableSymlink true branch in an if (name) check) so that skills' paths fall
back to the relative(projectRoot, skill.path) behavior when name is falsy;
update references to hasStableSymlink and the loop in scanner.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76f80779-89dd-4f8b-8225-ac5af061e9c7
📒 Files selected for processing (7)
.changeset/stable-skill-paths.mdpackages/intent/src/commands/install.tspackages/intent/src/library-scanner.tspackages/intent/src/scanner.tspackages/intent/src/utils.tspackages/intent/tests/library-scanner.test.tspackages/intent/tests/workspace-patterns.test.ts
…zation Matches the existing guard in library-scanner.ts. Without this, join(projectRoot, 'node_modules', '') resolves to node_modules/ which exists, causing hasStableSymlink to be true and producing paths like node_modules//skills/... Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Skill paths stored in
SkillEntry.pathused absolute filesystem paths that included package-manager-internal directories with version numbers (e.g.node_modules/.bun/@tanstack+router-core@1.168.3/node_modules/...). These paths broke whenever packages were updated, requiring users to re-runnpx @tanstack/intent@latest install.Root Cause
discoverSkills()stored the raw absolute path returned by filesystem traversal. In pnpm and bun, the resolved package directory lives inside.pnpm/or.bun/with the version baked into the path. These package managers create stable symlinks atnode_modules/<name>, but the scanner never used them.Approach
After
discoverSkills()returns intryRegister(), normalize each skill path:node_modules/<name>): usenode_modules/<name>/skills/...— stable across version bumpsThe install prompt is updated to tell agents not to embed versioned paths for transitive deps, and instead use
npx @tanstack/intent@latest list | grep <skill-name>to resolve at runtime.Also extracts a
toPosixPath()utility for the repeated.split(sep).join('/')pattern, fixes a macOS/var→/private/varsymlink issue inworkspace-patterns.test.ts, and guards against empty package names producingnode_modules//skills/...paths.Key Invariants
skill.pathnever contains.pnpm/,.bun/, or version numbers for local packages with a top-level symlinkskill.pathis always a relative path for local packages, absolute for globalNon-goals
catchblocks in scanner utils (out of scope)Verification
Files Changed
src/scanner.tsdiscoverSkills()— checks for stable symlink, falls back to project-relativesrc/library-scanner.tssrc/commands/install.tssrc/utils.tstoPosixPath()helper extracted from inline patterntests/library-scanner.test.tstests/workspace-patterns.test.tsrealpathSyncfix for macOS temp dir symlinks🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
Documentation
Tests