Skip to content

fix: normalize build context component paths#101

Closed
nyxst4ck wants to merge 1 commit into
NVIDIA:mainfrom
nyxst4ck:fix/posix-build-context-paths
Closed

fix: normalize build context component paths#101
nyxst4ck wants to merge 1 commit into
NVIDIA:mainfrom
nyxst4ck:fix/posix-build-context-paths

Conversation

@nyxst4ck

Copy link
Copy Markdown
Contributor

Summary

  • Normalize build context component paths with Path.as_posix() so Windows emits forward-slash relative paths.
  • Update test helper file-cache builders to mirror the same portable path convention.

Test Plan

  • python -m compileall -q src\skillspector\nodes\build_context.py tests\test_mcp_least_privilege.py tests\test_mcp_tool_poisoning.py tests\nodes\test_semantic_quality_policy.py tests\nodes\analyzers\test_semantic_security_discovery.py tests\nodes\analyzers\test_semantic_developer_intent.py
  • git diff --check

Note: full pytest collection was not run locally because this machine only has Python 3.11, while the project requires Python >=3.12.

Fixes #86

@rng1995 rng1995 left a comment

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.

Correct fix — Path.as_posix() yields forward-slash relative paths on every OS, which is what these dict keys and SARIF/URI locations need, and it's behavior-preserving on Linux/macOS. Approving.

Minor / optional (non-blocking): this appears to fully overlap with PR #87, which makes the identical change to the same lines in build_context.py and the same test helpers (and adds short explanatory comments). Only one of the two can merge cleanly. Maintainers may want to pick one and close the other to avoid a conflict — no change needed to the code itself.

@rng1995

rng1995 commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Thank you so much for this contribution, @nyxst4ck! 🙏

We're going to close this PR in favor of #87, which makes the identical Path.as_posix() change (replacing str(rel) with rel.as_posix() in build_context and the test helpers). We chose #87 because it also adds explanatory inline comments around the change and was validated against the full test suite, so we'll consolidate on that one to keep the history tidy.

The underlying issue (#86) is fully resolved by the merged change, so your fix and #87's are functionally equivalent — you both correctly identified and solved the same problem.

We really appreciate you taking the time to dig in and send a fix, and we'd love to see more contributions from you in the future. Thanks again! 🚀

@rng1995 rng1995 closed this Jun 22, 2026
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.

bug: build_context emits OS-native (backslash) component paths on Windows — non-portable SARIF + Windows test failures

2 participants