Repository navigation
fix(create): avoid following target symlinks - #2418
Open
leslieeilsel wants to merge 4 commits into
Open
leslieeilsel wants to merge 4 commits into
leslieeilsel wants to merge 4 commits into
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
leslieeilsel
marked this pull request as ready for review
August 12, 2026 04:33
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de4880171
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+108
to
+109
| const stats = fs.lstatSync(projectDirFullPath, { throwIfNoEntry: false }); | ||
| if (!stats || (stats.isDirectory() && isEmpty(projectDirFullPath))) { |
There was a problem hiding this comment.
When the requested target ends in a separator (for example, vp create vite:application --directory linked-project/), path.normalize preserves that separator and POSIX lstatSync("linked-project/") dereferences the final symlink. The link is therefore classified as a real directory, and after the user confirms the ordinary overwrite prompt, clearTargetPath traverses the link and deletes files in its destination—the data-loss case this change is intended to prevent. Strip trailing separators (while preserving filesystem roots) before every classification and cleanup operation, and cover this input in the symlink regression tests.
Useful? React with 👍 / 👎.
leslieeilsel
force-pushed
the
agent/one-line-challenge
branch
from
August 25, 2026 10:03
2085563 to
568252c
Compare
fengmk2
pushed a commit
that referenced
this pull request
Oct 15, 2026
<!-- claude-projects-attribution: {"github_login":"Brooooooklyn"} -->
_Requested by **Brooklyn** · [project
thread](https://claude.ai/code/project/chan_018s9RxRdtH424j2EhXdzV3N?thread=cmsg_018s9RxRdtH424j2EhXdzV3NQxbeR8Pt7qaqSQwoq9rEFo)_
<!-- routine:prune-tests:duplicate-shell-and-helper-tests -->
Before: `vp_global_cli` and `vp_pm_cli` carried Rust unit tests that
could not fail when the product broke: an `assert!(true)` in
`create.rs`, four `env use` shell-detection tests that repeat
`commands::shell` tests for code `env use` only imports, and three tests
of test-only helpers. `upgrade_check.rs` had 20 hand-unrolled lookup
tests.
After: those 8 tests are gone and the 20 lookup tests are 2 table-driven
tests that keep every row and comment, with the failing row named in the
assertion message. Only test code changes: every hunk is inside a
`#[cfg(test)]` module or the `#[cfg(test)]`-only
`resolution::test_utils` module.
How: each candidate was defended by mutating product code with the most
realistic regression only it would catch, running the crate's tests, and
reverting. No deleted test failed alone on any mutation.
### Totals
| | Before (origin/main c04bcf6) | After |
|---|---|---|
| Files touched | – | 5 |
| `vp_global_cli` cases collected (Linux) | 502 (+2 ignored) | 479 (+2
ignored) |
| `vp_pm_cli` cases collected | 923 (+2 ignored) | 920 (+2 ignored) |
- Cases deleted: **8** of 1,425 collected in the two touched crates.
Repo-wide: about 2,432 `#[test]`/`#[tokio::test]` attributes in tracked
Rust (static count) and 1,492 static `it`/`test` calls in 101 JS spec
files (not collected; Vitest needs a full `packages/core` build).
- Cases folded: **20 into 2**, every row and comment kept.
Runner-collected total: −26.
- Net lines: **−159** (+42 / −201).
- Runtime saved: about 0.03 s (the 8 deleted cases each report 0.000 s;
the 20 folded cases summed 0.028 s against 0.011 s for the 2
replacements).
- Test-binary time on this shared machine: `vp_global_cli` 3.13 s → 3.32
s, `vp_pm_cli` 27.81 s → 29.06 s (noise).
### Per file
1. **`crates/vp_global_cli/src/commands/create.rs` ›
`test_create_command_module_exists`**
- Class: tautology (`assert!(true)`).
- Best defense: M1 (delegate `"new"` instead of `"create"`) passes all
502 tests, so this test cannot fail. The whole `mod tests` goes.
- Coverage gap (pre-existing, unchanged by this PR): no unit test covers
`vp create` delegation. It may be covered by the PTY snapshot suite,
which was not run.
2. **`crates/vp_global_cli/src/commands/env/use.rs` ›
`test_detect_shell_vp_shell_{powershell,fish,nu}`,
`test_detect_shell_posix_default`**
- Class: duplicates of stronger tests. `detect_shell` /
`Shell::from_str` live in `commands/shell.rs`; `env use` only imports
them.
- Still covered: `fish` is byte-identical to
`commands::shell::tests::test_detect_shell_vp_shell_fish`; `nu` to
`test_detect_shell_vp_shell_explicit`; lowercase `powershell` by
`test_shell_from_str` and `test_detect_shell_vp_shell_case_insensitive`;
`posix_default` by `test_detect_shell_defaults_without_vp_shell`.
- Best defenses M2–M5: each failed the nominee together with at least
one `commands::shell` test.
- The `use vp_shared::env_vars;` import is now `#[cfg(windows)]`, since
only the three Windows-only tests in the module still use it.
3. **`crates/vp_pm_cli/src/resolution/test_utils.rs` ›
`parses_args_without_a_program_name`,
`parses_subcommands_without_a_program_name`**
- Class: test tooling testing test tooling.
- Best defense: M7 fails 122 tests and M8 fails 17; every resolution
parser test exercises these helpers.
4. **`crates/vp_pm_cli/src/resolution/commands/list.rs` ›
`list_args_helper_keeps_pattern`** (and its test-only `list_args`
helper, which had no other caller)
- Class: test tooling testing test tooling; it only checked struct
construction.
- Best defenses: M9 fails only
`test_list_with_pattern_and_pass_through_args`; M10 fails
`test_parser_accepts_short_dev_and_global_flags` and
`cli::tests::projects_managed_global_commands`. The nominee passes both.
### Folds (`crates/vp_global_cli/src/upgrade_check.rs`)
- The 9 `is_newer_version_*` tests → `is_newer_version_compares_semver`
(12 rows, original comments kept, one comment added for the `0.0.0`
dev-build row).
- The 11 `should_run_for_*` / `should_not_run_for_*` tests →
`should_run_for_command_skips_excluded_and_machine_readable_commands`
(11 rows).
- M11, M12 and M13 still fail the folded tables.
### Kept after defense — do not re-nominate
None: no nominee survived its defense.
### Case counts
| file | before | after | delta | expected delta |
|---|---|---|---|---|
| vp_global_cli `commands/create.rs` | 1 | 0 | −1 | −1 |
| vp_global_cli `commands/env/use.rs` (Linux) | 16 | 12 | −4 | −4 |
| vp_global_cli `upgrade_check.rs` | 36 | 18 | −18 | −18 (20 folded into
2) |
| vp_pm_cli `resolution/commands/list.rs` | 45 | 44 | −1 | −1 |
| vp_pm_cli `resolution/test_utils.rs` | 2 | 0 | −2 | −2 |
| **Crates total** | 1,425 | 1,399 | −26 | −26 |
**0 new failures** (0 failed and 4 ignored, before and after).
### Mutations
Each was temporary and reverted (`git status` clean afterwards).
1. M1 `create.rs:15`: delegate `"new"` instead of `"create"`. Failed:
none.
2. M2 `shell.rs:30`: `"fish"` → `"fishshell"`. Failed: `use::…_fish`,
`shell::test_shell_from_str`, `shell::…_fish`.
3. M3 `shell.rs:31`: `"nu" | "nushell"` → `"nushell"`. Failed:
`use::…_nu`, `shell::…_explicit`, `shell::test_shell_from_str`.
4. M4 `shell.rs:32`: drop `"powershell"`. Failed: `use::…_powershell`,
`shell::…_case_insensitive`, `shell::test_shell_from_str`.
5. M5 `shell.rs:54`: default always `Cmd`. Failed:
`use::…_posix_default`, `shell::…_defaults_without_vp_shell`,
`shell::…_invalid_vp_shell_falls_back_to_default`.
6. M6 `shell.rs:49`: upper-case the value. Failed: none (equivalent
mutation; `from_str` lowercases).
7. M7 `test_utils.rs:70`: `test_argv` no longer prepends a program name.
Failed: 122 tests, including both nominees.
8. M8 `test_utils.rs:57`: `parse_subcommand` gets raw args. Failed: 17
tests (`config`, `dist_tag`, `owner`, `stage`, `token`, plus the
nominee).
9. M9 `list.rs:78-80`: pnpm drops the pattern. Failed:
`test_list_with_pattern_and_pass_through_args` only.
10. M10 `list.rs:11`: `pattern` becomes `#[arg(long)]`. Failed:
`test_parser_accepts_short_dev_and_global_flags`,
`cli::tests::projects_managed_global_commands`.
11. M11 `upgrade_check.rs:170` (after the fold): `>` → `>=`. Failed:
`is_newer_version_compares_semver`.
12. M12 `upgrade_check.rs:338` (after the fold): `Upgrade` removed from
the excluded commands. Failed:
`should_run_for_command_skips_excluded_and_machine_readable_commands`.
13. M13 `upgrade_check.rs:165` (after the fold): the `0.0.0` early
return removed. Failed: `is_newer_version_compares_semver`.
### What ran and what did not
- Ran, at origin/main and on this branch: `cargo test -p vp_global_cli`
and `cargo test -p vp_pm_cli` with `--report-time`; `cargo clippy -p
vp_global_cli -p vp_pm_cli --all-targets --all-features -- --deny
warnings` with the `just lint` allow-list; `cargo fmt --all -- --check`;
`git diff --check`. All pass.
- Not run: the full workspace `just test` / `just lint` / `just check`;
Windows targets (none installed here, so the `#[cfg(windows)]` import
gate was reviewed but not compiled; Windows CI will compile it); the
JS/Vitest suite; the PTY snapshot suite.
### Excluded pull requests
55 open PRs touch 737 files between them; none of the 5 files here is
among them. #3055 #3052 #3051 #3049 #3047 #3046 #3044 #3040 #3038 #3037
#3020 #3018 #3014 #3012 #3009 #3008 #3000 #2986 #2981 #2966 #2936 #2922
#2900 #2863 #2862 #2810 #2792 #2740 #2721 #2699 #2695 #2574 #2557 #2552
#2549 #2543 #2542 #2541 #2483 #2475 #2463 #2462 #2461 #2456 #2428 #2418
#2357 #2267 #2125 #1958 #1783 #1782 #1614 #1517 #1180.
### Age check (cutoff: 30 days, 2026-09-14)
First-added dates from `git log -S`, followed across the `vite_*` →
`vp_*` crate rename.
| Test | First added |
|---|---|
| `test_create_command_module_exists` | 2026-02-10 (a29ac83, #570) |
| `test_detect_shell_posix_default` | 2026-02-07 (194c0dc, #524) |
| `test_detect_shell_vp_shell_{powershell,fish,nu}` | 2026-05-24
(53ae721, #1658) |
| `test_utils` tests and `list_args_helper_keeps_pattern` | 2026-08-03
(c1574dd, #2140) |
| The 20 folded `upgrade_check` tests | 2026-03-30 (db75675) |
None was added for a bug or incident, and none is in a permission,
security or entitlement suite.
### Checks
| Check | Before | After |
|---|---|---|
| Tests `vp_global_cli` | 502 pass | 479 pass |
| Tests `vp_pm_cli` | 923 pass | 920 pass |
| Clippy (deny warnings) | pass | pass |
| rustfmt | – | pass |
| `git diff --check` | – | clean |
To revert: `git revert 687935d`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf
---
_Generated by [Claude
Code](https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf)_
Co-authored-by: Claude <noreply@anthropic.com>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #2419
Summary
lstatSyncinstead of following the final symbolic link.gitRoot cause
The availability check used
existsSyncfollowed byreaddirSync, so a directory symbolic link was treated as the directory it referenced. The cleanup loop then removed child paths through that link.existsSyncalso treated dangling links as missing, while existing regular files reached directory-only operations.Implementation
The target is now inspected with:
This keeps the final filesystem entry distinct from its destination:
The cleanup function performs a fresh
lstatSyncimmediately before mutation, so it also handles a target that disappears while the prompt is open.Validation
vitest run packages/cli/src/create/__tests__: 13 files, 185 tests passedtsc -p packages/cli/tsconfig.json --noEmitgit diff --checkRegression coverage includes real directories, links to empty and populated directories, dangling links, regular files, linked-target preservation, and the existing
.gitpreservation behavior.