test: make marketplace symlink tests compatible with Windows#97439
test: make marketplace symlink tests compatible with Windows#97439aniruddhaadak80 wants to merge 2 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 4:15 AM ET / 08:15 UTC. Summary PR surface: Tests +27. Total +27 across 1 file. Reproducibility: yes. Current main shows the two marketplace symlink tests are skipped on every Windows host via Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the test-local capability probe and typed symlink fixture, then require a PR-body evidence update with redacted Windows run output before merge. Do we have a high-confidence way to reproduce the issue? Yes. Current main shows the two marketplace symlink tests are skipped on every Windows host via Is this the best way to solve the issue? Yes, for the code shape. After the second commit, the fixture uses the same platform-specific directory link type as the probe and matches existing sibling test patterns; the remaining gap is proof, not another code change. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 07bf384a8b3f. Label changesLabel justifications:
Evidence reviewedPR surface: Tests +27. Total +27 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
|
This pull request has been automatically marked as stale due to inactivity. |
��Closes #95534
What Problem This Solves
Fixes an issue where several marketplace plugin symlink boundary tests were unconditionally skipped on Windows environments, even if the environment supports creating directory symlinks (like with Developer Mode enabled).
Why This Change Was Made
Replaces the hardcoded Windows skips with a dynamic capability check (canCreateDirectorySymlinks) and explicitly passes directorySymlinkType to s.symlink when creating the test fixture, so that it creates a junction on Windows and matches the canCreateDirectorySymlinks probe logic.
User Impact
No user-visible product impact. This improves test portability and preserves regression coverage on capable Windows environments.
Evidence
ode scripts/run-vitest.mjs src/plugins/marketplace.test.ts locally and it exited successfully without errors.
@clawsweeper re-review