feat(marketplace): implement marketplace feature for VS Code extension#6882
feat(marketplace): implement marketplace feature for VS Code extension#6882markijbema wants to merge 42 commits into
Conversation
|
(not ready but i want bot review) |
|
Fixes #6067 |
| async install(item: MarketplaceItem, options: InstallMarketplaceItemOptions, workspace?: string): Promise<InstallResult> { | ||
| const scope = options.target ?? "project" | ||
| if (item.type === "mode") return this.installMode(item, scope, workspace) | ||
| if (item.type === "mcp") return this.installMcp(item, options) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)No additional issues outside diff hunks. Files Reviewed (12 files)
Reviewed by gpt-5.4-20260305 · 2,383,727 tokens |
4917c79 to
c92db33
Compare
|
|
||
| if (result.success && result.filePath) { | ||
| const line = result.line ?? 1 | ||
| await vscode.window.showTextDocument(vscode.Uri.file(result.filePath), { |
There was a problem hiding this comment.
WARNING: Editor-opening failures can mask a successful install
At this point the filesystem install has already completed, but a showTextDocument() rejection will bubble out of MarketplaceService.install() and prevent marketplaceInstallResult from being posted back to the webview. That leaves the UI treating a completed install as a failed or hung request. Wrapping the editor-opening step as best-effort keeps the returned result aligned with the actual install outcome.
| try { | ||
| const entries = await fs.readdir(dir, { withFileTypes: true }) | ||
| for (const entry of entries) { | ||
| if (!entry.isDirectory()) continue |
There was a problem hiding this comment.
WARNING: Symlinked skills are skipped during install detection
Kilo's skill discovery already supports .kilocode/skills/<name> symlinks, but readdir(..., { withFileTypes: true }) reports those entries as symbolic links, not directories. This guard skips them, so the marketplace metadata marks linked skills as uninstalled and can offer duplicate install/remove actions.
| if (!entry.isDirectory()) continue | |
| if (!entry.isDirectory() && !entry.isSymbolicLink()) continue |
| const item = removeItem() | ||
| if (!item) return | ||
| const scope = removeScope() | ||
| setPendingRemove({ item, scope }) |
There was a problem hiding this comment.
WARNING: Concurrent removals overwrite each other's tracking
pendingRemove only stores one in-flight item. Because the UI keeps the other Remove buttons enabled after confirmation, confirming removal B before removal A finishes replaces A here; when A's result comes back, the handler will attribute it to B and then clear the only pending slot, so B's later response loses its context entirely. Track removals by slug/request id, or block additional removals until the current one completes.
| vscode.postMessage({ type: "fetchMarketplaceData" }) | ||
|
|
||
| const unsubscribe = vscode.onMessage((msg: ExtensionMessage) => { | ||
| if (msg.type === "marketplaceData") { |
There was a problem hiding this comment.
WARNING: Stale marketplace responses can overwrite the current workspace
fetchMarketplaceData is sent on mount and again whenever workspaceDirectory changes, but marketplaceData has no workspace or request identifier. If a slower response from workspace A arrives after the user has already switched to workspace B, this handler will still replace items and marketplaceInstalledMetadata with A's state, so the panel can show install status for the wrong project.
| } | ||
|
|
||
| openPanel(view: PanelView): void { | ||
| const projectDirectory = this.getProjectDirectory() |
There was a problem hiding this comment.
WARNING: Project scope goes stale after the panel opens
This only snapshots the active workspace when openPanel() runs. If the user leaves the marketplace panel open and switches to a different workspace folder, later project-scoped installs/removals still use the old directory until the panel is reopened, so actions can be applied to the wrong repo in a multi-root workspace.
| } | ||
| // kilocode_change start — marketplace IPC handlers | ||
| case "fetchMarketplaceData": { | ||
| const workspace = this.getProjectDirectory(this.currentSession?.id) |
There was a problem hiding this comment.
why? this should just be the dir of the currently opened workspace and not be depnedent on teh session i think?
| > | ||
| <button | ||
| class="marketplace-remove-btn" | ||
| onClick={() => props.onRemove(props.item, installed() as "project" | "global")} |
There was a problem hiding this comment.
WARNING: Remove always targets the project copy when both scopes are installed
isInstalled() collapses the install state down to a single scope and prefers "project". If the same marketplace item is installed both globally and in the project, clicking Remove here can never target the global copy first, so the uninstall appears to fail because the card stays installed after the project entry is deleted.
| } | ||
|
|
||
| private getProjectDirectory(sessionId?: string): string | undefined { | ||
| return resolveProjectDirectory(this.projectDirectory, () => this.getWorkspaceDirectory(sessionId)) |
There was a problem hiding this comment.
WARNING: Standalone panel overrides are still lost after a session load
getProjectDirectory() establishes the new override/null contract for standalone panels, but loadMessages() still emits workspaceDirectoryChanged with getWorkspaceDirectory(sessionID) later in this file. Because the webview server context treats that message as authoritative, loading or switching sessions can overwrite the panel's explicit project selection and re-enable project-scoped installs for the wrong workspace.
| const text = await fetchWithRetry(`${BASE_URL}/skills`) | ||
| const parsed = parseResponse(text) as { items?: unknown[] } | ||
| const items = (parsed.items ?? []) as RawSkill[] | ||
| const result = items.map(transformSkill) |
There was a problem hiding this comment.
WARNING: A single malformed skill record drops the entire skills catalog
transformSkill() assumes every marketplace item has string id and category values. If the API returns one partial/bad row, items.map(transformSkill) will throw on kebabToTitleCase(undefined), and fetchAll() then replaces the whole skills response with []. Validating or skipping invalid entries here would keep the rest of the marketplace visible during a partial rollout or bad publish.
8b954cc to
9edd69f
Compare
| } | ||
|
|
||
| globalSkillsDir() { | ||
| return path.join(os.homedir(), ".kilocode", "skills") |
There was a problem hiding this comment.
WARNING: Global marketplace skills are stored in the wrong layer
~/.kilocode/skills is the generic user-global skills directory, while the CLI and migration docs treat <VS Code globalStorage>/skills as the location for marketplace-installed skills. Writing marketplace installs here collapses those two layers, so precedence changes and the install no longer follows the actual host editor storage path passed in KILO_VSCODE_STORAGE_PATH.
| const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "kilo-marketplace-installer-")) | ||
| dirs.push(workspace) | ||
|
|
||
| const paths = new MarketplacePaths(path.join(workspace, "storage")) |
There was a problem hiding this comment.
WARNING: This test instantiates MarketplacePaths with an argument the class no longer accepts. TypeScript will flag Expected 0 arguments, but got 1, so the new test suite will not compile as written.
| const paths = new MarketplacePaths(path.join(workspace, "storage")) | |
| const paths = new MarketplacePaths() |
|
|
||
| describe("MarketplaceService.install", () => { | ||
| it("does not open the installed file in the editor", async () => { | ||
| const service = new MarketplaceService("/tmp/kilo-marketplace-service") |
There was a problem hiding this comment.
WARNING: MarketplaceService also has a zero-argument constructor now, so this test currently fails type-checking before it can exercise the install path.
| const service = new MarketplaceService("/tmp/kilo-marketplace-service") | |
| const service = new MarketplaceService() |
3d8bc61 to
3a637fc
Compare
|
|
||
| const text = await fetchWithRetry(`${BASE_URL}/modes`) | ||
| const parsed = parseResponse(text) as { items?: unknown[] } | ||
| const items = (parsed.items ?? []) as Array<Record<string, unknown>> |
There was a problem hiding this comment.
WARNING: Null marketplace payloads crash this fetch path
parseResponse() can return null for an empty document or a literal null, but this line dereferences parsed.items immediately. That throws before fetchAll() can turn the failure into a user-visible errors[] entry, so a transient bad marketplace response breaks the whole load instead of degrading gracefully. The same object guard is needed in fetchMcps() and fetchSkills() as well.
|
|
||
| const unsubscribe = vscode.onMessage((msg: ExtensionMessage) => { | ||
| if (msg.type !== "marketplaceInstallResult") return | ||
| if (msg.slug !== item.id) return |
There was a problem hiding this comment.
WARNING: Install results are not correlated to a specific request
This listener accepts any marketplaceInstallResult for the same item.id, but the response payload only carries slug. If the modal is closed and reopened for the same item before an earlier request finishes, a delayed result from the old install will be applied to the new modal and reported with the current scope() on line 48. That can show the wrong success/failure state and emit follow-up telemetry/refreshes for the wrong target.
087b33d to
c66b5f4
Compare
| edit: "edit", | ||
| browser: "bash", | ||
| command: "bash", | ||
| mcp: "mcp", |
There was a problem hiding this comment.
WARNING: Marketplace modes with an mcp group lose skill access
Config.Permission and the skill tool use the skill permission key, not mcp. With this mapping, a mode that declares groups: ["mcp"] is written as { mcp: "allow" }, which the runtime does not honor, so the installed agent still prompts/blocks when it tries to use skills. ALL_PERMISSIONS is also keyed to mcp, so skill never receives the intended explicit allow/deny here.
| export class MarketplacePaths { | ||
| /** Project-scope config file: <workspace>/.kilo/kilo.json */ | ||
| configPath(scope: "project" | "global", workspace?: string): string { | ||
| if (scope === "project") return path.join(workspace!, ".kilo", "kilo.json") |
There was a problem hiding this comment.
WARNING: Project-scoped marketplace paths can throw without a resolved workspace
Standalone panels intentionally allow projectDirectory to be absent in ambiguous multi-root cases. If a project-scope install/remove reaches this helper with workspace === undefined, path.join(workspace!, ...) throws ERR_INVALID_ARG_TYPE instead of falling back to global scope or returning a clean validation error.
…h project and global
…emove, cancel, scope
… migrator paths
The CLI's KilocodePaths.vscodeGlobalStorage() hardcoded the 'Code' directory,
which doesn't match VS Code Insiders ('Code - Insiders'), VSCodium, or Cursor.
This caused the MCP and modes migrators to look in the wrong directory, so
marketplace-installed MCPs and modes never appeared in settings.
Pass the actual globalStorageUri from the extension via KILO_VSCODE_STORAGE_PATH
env var, and use it when available in the CLI.
…ode extension dir Global marketplace items (MCPs, modes) were written to the VS Code extension's globalStorageUri, which varies by VS Code variant (Insiders, VSCodium, Cursor). The CLI migrators couldn't find them because they guessed the standard Code/ path. Now global MCPs write to ~/.kilocode/mcp.json and global modes to ~/.kilocodemodes, matching where the CLI already looks. Added ~/.kilocode/mcp.json to the MCP migrator's read paths. Skills already used ~/.kilocode/skills/ and were unaffected.
…ead of legacy files
The marketplace installer was writing MCPs and modes in the legacy Kilocode
format to separate files (mcp.json, .kilocodemodes), relying on CLI migrators
to bridge them into the config. This broke because:
- The CLI binary must be rebuilt to pick up migrator path changes
- The legacy format needed conversion that could silently fail
- VS Code variant paths (Insiders, VSCodium) didn't match
Now the marketplace writes directly to kilo.json in the native opencode config
format that the CLI reads without any migration:
- Project scope: <workspace>/.kilo/kilo.json
- Global scope: ~/.config/kilo/kilo.json
MCPs are written as {type, command: [...], environment} and modes as
{mode: 'primary', prompt, permission} — the exact format Config.get() returns.
Also updated the webview McpConfig type and MCP tab rendering to handle both
the opencode array command format and the legacy string+args format.
…issing skills mock
…onents Replace raw HTML elements and 596 lines of custom CSS with kilo-ui components used throughout the rest of the extension webview: - Tabs (tab bar), Dialog (install modal, remove dialog) - Button/IconButton (all buttons), TextField (search inputs) - Select (status filter), Card (item cards, error banners) - Tag (badges, type labels), RadioGroup (skill categories) - Icon (success checkmark) Modals now use useDialog() context pattern matching the existing CloudImportDialog/FeedbackDialog implementations. marketplace.css reduced from 596 to 230 lines (layout-only residuals).
…kilo/skills/ The CLI scans both .kilocode/skills/ and .kilo/skills/ at project and global scope when loading skills. The marketplace detection only checked .kilo/skills/, missing skills installed in .kilocode/skills/. Add allSkillsDirs() to MarketplacePaths that returns both directories per scope, and use it in InstallationDetector.detect() so the installed status accurately reflects what the CLI sees.
The CLI's GET /skill endpoint (Skill.all()) does a comprehensive scan of all skill directories: .kilocode/, .kilo/, .claude/, .agents/, .opencode/, config paths, and skill URLs. The marketplace was duplicating this with its own filesystem scan that only checked .kilo/skills/. Now KiloProvider fetches skills from the CLI via the SDK and passes them to the marketplace detector, which uses the location path to infer scope. Falls back to filesystem scanning when the CLI client is not available.
Drop the fallback filesystem scan and allSkillsDirs() — the CLI data is always available when the marketplace is usable.
…matching - transformSkill() now sets name to the title-cased display name instead of the raw slug, so dialogs/toasts/telemetry show 'Agent Md Refactor' not 'agent-md-refactor' - Workspace prefix check now appends path.sep before startsWith to prevent /repo-app matching /repo-app2/...
These markers are not needed in packages/kilo-vscode/ since the entire package is Kilo-specific. The check-kilocode-change CI check rejects them.
…all/remove popups
…etplace with settings - Route marketplace mode removal through CLI removeAgent endpoint instead of direct filesystem manipulation, leveraging the new Agent.remove() backend - Extend Agent.remove() to also handle agents stored in JSON config files (e.g. marketplace-installed modes in kilo.json) - Refresh marketplace installed metadata when agents/skills change externally (e.g. mode removed via Settings tab)
2b1ff64 to
96eb500
Compare
| const scope = message.mpInstallOptions?.target ?? "project" | ||
| const result = | ||
| message.mpItem.type === "mode" | ||
| ? await this.removeModeViaCli(message.mpItem) |
There was a problem hiding this comment.
CRITICAL: Mode removal ignores the selected scope
removeInstalledMarketplaceItem computes scope, but the mode branch drops it and calls removeModeViaCli() with only the item payload. The CLI removeAgent path is unscoped and Agent.remove() scans both project and global config sources, so clicking the project/global remove button here can delete the mode from the wrong location (or from both).
| const filepath = path.join(dir, file) | ||
| const raw = await readFile(filepath, "utf-8").catch(() => null) | ||
| if (!raw) continue | ||
| const parsed = JSON.parse(raw) as Record<string, unknown> |
There was a problem hiding this comment.
WARNING: Supported .jsonc configs will fail on removal
This loop explicitly scans kilo.jsonc and opencode.jsonc, but JSON.parse(raw) rejects valid JSONC comments and trailing commas. In any project using the CLI's supported JSONC config format, removing a marketplace-installed mode throws before the agent entry can be deleted.
Implements the marketplace feature for the VS Code extension with the Skills tab fully functional and MCP Servers/Modes tabs showing 'to be implemented' placeholders. Based on #6882 but scoped to only the Skills marketplace tab: - Backend: API client, installation detection, installer, paths - IPC: fetchMarketplaceData, installMarketplaceItem, removeInstalledMarketplaceItem - Webview: MarketplaceView with 3-tab UI, SkillsMarketplace, ItemCard, InstallModal, RemoveDialog - Marketplace opens as editor panel via SettingsEditorProvider - Project directory resolution for multi-root workspace support - yaml dependency for mode content parsing
Fixes #6067
Marketplace Feature Implementation
Implements the full marketplace feature for the Kilo VS Code extension, allowing users to browse, install, and remove MCP Servers, Modes, and Skills from the Kilo marketplace catalog.
What's Included
Extension-Side Services
IPC Protocol
Webview UI
Telemetry
Not Yet Implemented
Spec Reference