fix(sv): caching bugs

[sacrosanctic]

Closes #1312

Description

Does not address the concurrency issue.

Checklist

• Update snapshots (if applicable)
• Add a changeset (if applicable)
• Allow maintainers to edit this PR
• I care about what I'm doing, no matter the tool I use (Notepad, Sublime, VSCode, AI...)

Summary by CodeRabbit

• Bug Fixes
  • Improved add-on installation so local and npm packages use a consistent destination.
  • Existing destinations are now replaced cleanly before installation.
  • Local packages continue to use symlinks, with a fallback for Windows permission errors.
  • npm packages are downloaded and unpacked correctly before being added.
  • Prevented stale local links from affecting package installation.

[pkg-svelte-dev]

Install the latest version of sv from c567baf:

pnx https://pkg.svelte.dev/sv/c/c567baffe3354f27e796d48483cb2a4c891469cc create

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/cli/pr/1313

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

[changeset-bot]

🦋 Changeset detected

Latest commit: c567baf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
sv	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7bfafaa2-fa24-4c62-a060-fe6e2edcde69

📥 Commits

Reviewing files that changed from the base of the PR and between 08b3fc3 and e03fddb.

📒 Files selected for processing (2)

• packages/sv/src/core/fetch-packages.ts
• packages/sv/src/core/tests/fetch-packages.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The addon downloader now resolves node_modules from the package manifest, removes stale destinations, and installs local and npm addons through one path. Tests cover downloading over an existing symlink without modifying its target.

Changes

Addon installation

Layer / File(s)	Summary
Resolve and install addons packages/sv/src/core/fetch-packages.ts	The downloader derives its installation root from the nearest package manifest, removes existing destinations, preserves local symlink creation with a Windows copy fallback, and unpacks npm packages into the shared destination.
Validate downloaded addon replacement packages/sv/src/core/tests/fetch-packages.ts	Tests mock a compressed package response and verify that extraction replaces a directory symlink without modifying the symlink target.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to e03fd

The change replaces installed add-ons by deleting the current version before the new version is ready. A failed or concurrent installation can leave an add-on missing, incomplete, or stale, which can break commands or execute an older version; this should be addressed or explicitly accepted before merge.

Suggested reviewers: adriangonz97

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The changes address the incorrect package path, stale destination, stale symlink, and local/npm package collision described in issue #1312. The PR objectives explicitly state that concurrent-run handl…	Implement synchronization or per-run isolated destinations to prevent concurrent runs from deleting or overwriting each other's add-on data, or update the linked issue scope to exclude concurrency handling.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the package and the primary caching-bug fix.
Out of Scope Changes check	✅ Passed	The changes are limited to add-on package resolution, installation, cleanup, and related tests. These changes directly support the caching objectives in issue #1312.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.

Full details: Linked Issues check

Explanation

The changes address the incorrect package path, stale destination, stale symlink, and local/npm package collision described in issue #1312. The PR objectives explicitly state that concurrent-run handling is not addressed, so full compliance with the issue's concurrency concern cannot be confirmed.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[jycouet]

Merci :)