Skip to content

fix(vscode): harden marketplace skill installs#10806

Merged
marius-kilocode merged 3 commits into
mainfrom
orderly-dinosaur
Jun 2, 2026
Merged

fix(vscode): harden marketplace skill installs#10806
marius-kilocode merged 3 commits into
mainfrom
orderly-dinosaur

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Marketplace project skill installation can surface raw filesystem errors instead of a stable Marketplace result. The shipped Skills Marketplace came from #7122, scoped from the original Marketplace implementation in #6882. Its installer kept timestamp-derived temporary paths and assumed that every project-scope request still had a workspace directory by the time it reached the filesystem layer.

Two paths reproduce the problem:

  1. Open Marketplace for a project, then let the project context become unavailable or send a stale project-scope install request. Installing a skill reaches path.join() without a workspace directory and throws a raw path argument error.
  2. Start two installs of the same project skill at nearly the same time, such as a repeated install action before the first request settles. Both requests can reuse the same millisecond-based tarball and staging paths. One request can rename or clean up files while the other still uses them, producing an intermittent filename, extraction, or rename error. Retrying later usually works because the timestamp changes.

This change makes project-scope Marketplace operations reject missing workspace context before touching paths, gives every skill install collision-safe staging and tarball paths, and reports the normal already-installed result when concurrent installs race for the final destination. It also tightens filesystem-backed Marketplace ids so unsafe path aliases cannot target an unintended skill or agent location.

@marius-kilocode marius-kilocode enabled auto-merge June 2, 2026 08:19
Comment thread packages/kilo-vscode/tests/unit/marketplace-installer.test.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

The new commit (e6898387) stabilizes the shell-cancellation test in packages/opencode/test/session/prompt.test.ts. The fragile yield* Effect.sleep(50) race window is replaced with a deterministic waitFor("shell busy", ...) poll that waits until the session is actually in "busy" state before issuing the cancel. The SessionStatus.Service yield is hoisted above the fork so it is available at the right scope. This pattern is already used by at least four other tests in the same file (lines 1515, 1556, 1681, 1751), so it is consistent with the existing codebase. No bugs, resource leaks, or logic issues introduced.

All three commits in this PR are clean:

  • ed3e1ac — hardens marketplace skill installs (original fix)
  • 3f8afe83 — removes obsolete Date.now monkey-patch from test (addressed previous suggestion)
  • e6898387 — stabilizes the shell-cancellation wait (this incremental review)
Files Reviewed (4 files)
  • packages/kilo-vscode/src/services/marketplace/installer.ts - clean
  • packages/kilo-vscode/tests/unit/marketplace-installer.test.ts - clean (previous suggestion resolved)
  • .changeset/calm-marketplace-skills-install.md - clean
  • packages/opencode/test/session/prompt.test.ts - clean

Reviewed by claude-4.6-sonnet-20260217 · 256,629 tokens

Review guidance: REVIEW.md from base branch main

@marius-kilocode marius-kilocode merged commit 89a8141 into main Jun 2, 2026
18 of 19 checks passed
@marius-kilocode marius-kilocode deleted the orderly-dinosaur branch June 2, 2026 17:15
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.

2 participants