fix(marketplace): harden git plugin install and cleanup - #14492
Conversation
Expand `~/` repository paths in cloneUrl instead of turning them into an https URL. Remove the staging directory when a clone fails so no `.tmp-*` folder is left in the cache. Keep backslashes in POSIX paths during identity normalization and convert them only for Windows-style paths, so a legal POSIX filename is not rewritten. Delete the cloned plugin cache on uninstall when no remaining scope still installs the plugin, and keep it when another scope still uses it. Stop describing every plugin as an npm plugin in the install dialog, and document plugin install and git-source publishing in the marketplace and plugins docs.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The follow-up commit removes the shared-clone cache cleanup on uninstall, resolving both prior findings, and simplifies Files Reviewed (4 files)
Previous Review Summary (commit 43f5d55)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 43f5d55)Status: 2 Issues Found | Recommendation: Address before merge Overview
The git-source hardening looks correct overall: Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (26 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
The clone cache is keyed by identity and ref under a shared global cache, so it is shared across projects and worktrees. The removal-time check only inspected the caller's directory tree, so removing a plugin in one worktree could delete a clone another worktree still installs, forcing a re-clone or failing offline. Drop the cache deletion and the specs it needed. Uninstall leaves the shared clone in place.
What Problem This Solves
Follow-up fixes for git-hosted Marketplace plugins (#14485):
~/repository spec passed validation but was cloned ashttps://~/..., so it always failed..tmp-*staging directory in the plugin cache.Why This Change Was Made
cloneUrlnow expands~/to the home directory before it falls through to the shorthandhttps://branch.cloneIntowraps the clone and checkout in atry/catchthat removes the staging directory and rethrows.normalizeRepoconverts backslashes only for Windows-style paths (drive letter or UNC). POSIX paths are left unchanged.enand all 19 locales.A shared-clone cache deletion on uninstall was drafted and dropped: the cache lives under a shared global cache keyed by identity and ref, so it is shared across projects and worktrees. Checking only the caller's directory tree could delete a clone another worktree still installs. The clone cache is intentionally left in place on uninstall.
User Impact
~/git plugins now install.Evidence
packages/opencode:bun test ./test/kilocode/plugin-git-source.test.tsplus the marketplace suite (marketplace-plugin,marketplace-installer,marketplace-api,marketplace-plugin-http) -> 43 pass, 281 assertions. New cases cover~/expansion, staging cleanup on a failed clone, POSIX backslash identity, Windows and file URL identity, and git vs npm/local identity collision.bun run typecheckinpackages/opencode-> pass.packages/kilo-i18ntypecheck and the i18n keys guard -> pass.script/check-md-table-padding.ts-> pass. Scopedoxlinton the changed files -> 0 errors.