Skip to content

Extract Marketplace logic from KiloProvider#10850

Merged
imanolmzd-svg merged 3 commits into
mainfrom
imanol/extract-marketplace
Jun 3, 2026
Merged

Extract Marketplace logic from KiloProvider#10850
imanolmzd-svg merged 3 commits into
mainfrom
imanol/extract-marketplace

Conversation

@imanolmzd-svg

@imanolmzd-svg imanolmzd-svg commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Part of #8666

Why

KiloProvider is growing in size and responsibilities, this refactor removes Marketplace domain logic into its own view and provider.
There's still marketplace related code on KiloProvider that is required for the Settings view.

Implementation

  • Marketplace now runs as a standalone editor panel with its own MarketplacePanelProvider, SolidJS entry point, webview bundle, and lightweight session-status context.
  • Panel lifecycle is separate from SettingsEditorProvider: opening, revealing, restoring after restart, project-directory resolution, and webview serialization are handled independently.
  • Marketplace webview messages are handled by MarketplacePanelProvider instead of KiloProvider, including data fetching, installation, removal, banner dismissal, external links, telemetry, connection state, and session-status updates.
  • Shared Marketplace operations live in services/marketplace/actions.ts, so the standalone panel and sidebar Settings removal flow reuse the same cache invalidation and legacy MCP cleanup behavior.
  • KiloProvider no longer owns a MarketplaceService, loads Marketplace data, or processes Marketplace webview messages. It only keeps the narrow Settings integration needed to remove configured agents and MCP servers, then refresh the cached agent and config data.

Testing

  • Open Marketplace from the sidebar and confirm it opens as a separate editor tab without changing the current chat.
  • Install and remove an agent at project scope; confirm the sidebar agent list updates after removal.
  • Install and remove an MCP server at project scope; confirm the sidebar config updates immediately.
  • Install and remove an MCP server at global scope; confirm the global entry disappears.
  • Remove an MCP from sidebar Settings; confirm both project and global copies are removed intentionally.
  • Open a multi-root workspace with no active editor; confirm project-scope installation is unavailable while global installation still works.
  • Restart VS Code with Marketplace open; confirm the Marketplace tab restores correctly.
  • Run bun run test:unit, bun run typecheck, bun run lint, and bun run knip from packages/kilo-vscode/.

@imanolmzd-svg imanolmzd-svg force-pushed the imanol/extract-marketplace branch from cd97675 to 389f9ae Compare June 3, 2026 11:10

@marius-kilocode marius-kilocode left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much better, architecture wise

Comment thread packages/kilo-vscode/src/MarketplacePanelProvider.ts Outdated
Comment thread packages/kilo-vscode/src/MarketplacePanelProvider.ts Outdated
@marius-kilocode

Copy link
Copy Markdown
Collaborator

My bot found this regression:

Confirmed Regression: Global Skills Look Uninstalled In the new standalone provider:

const project = this.project ?? undefined
const data = await fetchMarketplaceData(this.marketplaceCtx, project, project)
packages/kilo-vscode/src/MarketplacePanelProvider.ts:201

The second argument is the directory used to ask the CLI for installed skills:

const skills = dir ? await fetchSkills(ctx, dir) : undefined
packages/kilo-vscode/src/services/marketplace/actions.ts:32

When Marketplace intentionally enters global-only mode, project is undefined. This happens in an ambiguous multi-root workspace with no active editor, and can also happen without an open workspace. As a result, the CLI skill request is skipped entirely.

That matters because global skill installation detection depends on the CLI skill list:

...this.skillEntries(skills, workspace, false),
packages/kilo-vscode/src/services/marketplace/detection.ts:36

The practical effect:

Open a multi-root workspace with no active editor.
Open Marketplace.
Install a skill globally.
The install succeeds.
Marketplace refreshes, but the same skill still appears uninstalled.
Clicking Install again fails with “Skill already installed.”
The fix should be small:

const data = await fetchMarketplaceData(this.marketplaceCtx, project, this.directory())
This keeps project-scope detection disabled while still allowing the CLI to report global skills.

Retained-Panel Risk There is also a race around changing directories while reusing the existing panel:

if (this.panel) {
this.setProjectDirectory(project)
this.panel.reveal(vscode.ViewColumn.One)
return
}

Replace swallowed catch block with console.warn for session status
sync failures, and pass `this.directory()` instead of `project` as the
second positional argument to `fetchMarketplaceData`.
@imanolmzd-svg

Copy link
Copy Markdown
Contributor Author

Addressed comments

@imanolmzd-svg imanolmzd-svg marked this pull request as ready for review June 3, 2026 21:06
@imanolmzd-svg imanolmzd-svg merged commit b89d4ea into main Jun 3, 2026
22 of 24 checks passed
@imanolmzd-svg imanolmzd-svg deleted the imanol/extract-marketplace branch June 3, 2026 21:07
@kilo-code-bot

kilo-code-bot Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 2 Issues Found | Recommendation: Informational (PR already merged)

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
packages/kilo-vscode/src/MarketplacePanelProvider.ts 83–86 dispose() double-calls cleanup(): panel?.dispose() fires onDidDispose → cleanup() synchronously, then line 85 calls cleanup() again. Benign (idempotent) but inconsistent with KiloClawProvider which calls cleanup() before panel?.dispose(). Consider: this.cleanup(); this.panel?.dispose(); this.marketplace.dispose() to match the established pattern.

SUGGESTION

File Line Issue
.changeset/ N/A No changeset for a user-visible behavioral change. The marketplace now opens as a standalone editor tab instead of a sidebar view — this is a noticeable change for users. A minor changeset entry (e.g. "Open Marketplace in a dedicated editor panel instead of the sidebar view") would capture this for release notes.
Positive Observations
  • The regression flagged in review (global skills showing as uninstalled when project was undefined) was correctly fixed: fetchData() now passes this.directory() (line 202) rather than project, so the CLI still receives a directory for global skill detection.
  • The retained-panel workspace-directory race is handled correctly: setProjectDirectory posts workspaceDirectoryChanged, which reactively triggers createEffect(() => { server.workspaceDirectory(); fetchData() }) in MarketplaceView.tsx — no stale install badges.
  • Clean separation of concerns: actions.ts centralizes cache invalidation and legacy MCP cleanup so both the standalone panel and the sidebar removal adapter reuse identical logic.
  • MarketplaceItemRef = Pick<MarketplaceItem, 'id' | 'type'> is a good narrowing that avoids full-item coupling in removal flows.
  • Test coverage is solid: arch tests guard against regressions, and the legacy MCP cleanup tests cover the scoped-removal edge cases.
Files Reviewed (16 files)
  • packages/kilo-vscode/src/MarketplacePanelProvider.ts - 1 issue (double-cleanup in dispose)
  • packages/kilo-vscode/src/services/marketplace/actions.ts
  • packages/kilo-vscode/src/kilo-provider/remove-config-item.ts
  • packages/kilo-vscode/src/services/marketplace/types.ts
  • packages/kilo-vscode/src/services/marketplace/installer.ts
  • packages/kilo-vscode/src/extension.ts
  • packages/kilo-vscode/src/SettingsEditorProvider.ts
  • packages/kilo-vscode/webview-ui/marketplace/MarketplaceApp.tsx
  • packages/kilo-vscode/webview-ui/marketplace/index.tsx
  • packages/kilo-vscode/webview-ui/src/context/marketplace-session.tsx
  • packages/kilo-vscode/webview-ui/src/context/language-bridge.tsx
  • packages/kilo-vscode/webview-ui/src/App.tsx
  • packages/kilo-vscode/tests/unit/marketplace-actions.test.ts
  • packages/kilo-vscode/tests/unit/marketplace-panel-arch.test.ts
  • packages/kilo-vscode/tests/unit/project-directory.test.ts
  • packages/kilo-vscode/tests/unit/remove-config-item.test.ts

Fix these issues in Kilo Cloud


Reviewed by claude-4.6-sonnet-20260217 · 2,252,825 tokens

Review guidance: REVIEW.md from base branch main

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