fix: use Path.name for cross-platform directory extraction #52
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The validation logic used `path.rsplit("/", 1)[-1]` which only splits on
forward slashes. On Windows, paths use backslashes, causing the full
path to be compared against frontmatter.name instead of just the
directory name.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
gotalab
force-pushed
the
fix/windows-path-validation
branch
from
f442b25 to
1ecc905
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a cross-platform compatibility issue in the skillport add command by replacing Unix-specific path parsing with a platform-agnostic approach using pathlib.Path.
Key Changes:
- Replaced
path.rsplit("/", 1)[-1]withPath(path).nameto extract directory names in a cross-platform manner - Leverages the already-imported
Pathclass frompathlib(line 6)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lines = skill.get("lines", 0) | ||
| path = skill.get("path", "") | ||
| dir_name = path.rsplit("/", 1)[-1] if path else "" | ||
| dir_name = Path(path).name if path else "" |
There was a problem hiding this comment.
The existing test suite has comprehensive coverage for the validate_skill_record function, but lacks test cases for Windows-style paths. Consider adding a test case that validates the name-directory matching works correctly with Windows paths like 'C:\skills\test-skill' to ensure the cross-platform fix is tested.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
skillport addcommandpath.rsplit("/", 1)[-1]withPath(path).namefor cross-platform compatibilityRelated: #51
Test plan
🤖 Generated with Claude Code