Skip to content

feat: add ability to remove discovered skills - #7070

Merged
markijbema merged 10 commits into
mainfrom
mark/add-skill-removal
Mar 16, 2026
Merged

feat: add ability to remove discovered skills#7070
markijbema merged 10 commits into
mainfrom
mark/add-skill-removal

Conversation

@markijbema

@markijbema markijbema commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a remove button (X icon) to each discovered skill in the Agent Behaviour > Skills settings tab
  • Shows a confirmation dialog before deleting to prevent accidental removal
  • On confirm, deletes the skill's directory from disk via vscode.workspace.fs.delete and refreshes the skills list

Changes

  • messages.ts: New RemoveSkillMessage type added to WebviewMessage union
  • en.ts: i18n strings for the removal confirmation dialog
  • AgentBehaviourTab.tsx: Remove button + confirmation dialog on each discovered skill row
  • KiloProvider.ts: handleRemoveSkill handler that deletes the skill directory and re-fetches skills
CleanShot 2026-03-16 at 13 29 12@2x CleanShot 2026-03-16 at 13 29 41@2x

@markijbema
markijbema marked this pull request as ready for review March 16, 2026 11:20
.route("/telemetry", TelemetryRoutes()) // kilocode_change
.route("/commit-message", CommitMessageRoutes()) // kilocode_change
.route("/enhance-prompt", EnhancePromptRoutes()) // kilocode_change
.route("/kilocode", KilocodeRoutes()) // kilocode_change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer kilo, but that is taken by the gateway, and I didn't feel like renaming everything for this change

Comment thread packages/opencode/src/skill/skill.ts Outdated
export async function remove(location: string) {
const resolved = path.resolve(location)
const dir = path.dirname(resolved)
await rm(dir, { recursive: true, force: true })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CRITICAL: Unvalidated path can delete arbitrary directories

location comes straight from the client and this deletes path.dirname(location) recursively. Any caller can point it at an arbitrary file and remove its parent directory, not just a discovered skill folder. Please resolve the skill from the loaded registry (or otherwise validate the path) before deleting anything on disk.

@kilo-code-bot

kilo-code-bot Bot commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 2
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

CRITICAL

File Line Issue
packages/opencode/src/skill/skill.ts 222 Unvalidated path can delete arbitrary directories

WARNING

File Line Issue
packages/opencode/src/skill/skill.ts 226 URL-backed skills are removed only for the current instance and come back after restart
packages/kilo-vscode/src/KiloProvider.ts 1150 Failed deletes leave the webview in a stale optimistic state
Other Observations (not in diff)

No additional issues found outside the diff.

Files Reviewed (7 files)
  • packages/opencode/src/skill/skill.ts - 2 issues
  • packages/kilo-vscode/src/KiloProvider.ts - 1 issue
  • packages/opencode/src/server/routes/kilocode.ts - 0 issues
  • packages/opencode/src/server/server.ts - 0 issues
  • packages/kilo-vscode/webview-ui/src/context/session.tsx - 0 issues
  • packages/kilo-vscode/webview-ui/src/components/settings/AgentBehaviourTab.tsx - 0 issues
  • packages/kilo-vscode/webview-ui/src/types/messages.ts - 0 issues

Reviewed by gpt-5.4-20260305 · 726,534 tokens

Mark IJbema added 9 commits March 16, 2026 13:04
…ttings

Add a remove button (X icon) to each discovered skill in the skills
subtab with a confirmation dialog. On confirm, the skill's directory
is deleted from disk and the skills list is refreshed.
…oveSkill

Move discoveredSkills signal from AgentBehaviourTab into the session
context so any component can access session.skills(), session.refreshSkills(),
and session.removeSkill(location).

removeSkill optimistically removes the skill from the UI signal before
sending the message to the extension, so the screen updates immediately.
Add Skill.remove() that deletes the skill directory from disk and
mutates the cached Instance.state in-place so subsequent GET /skill
calls return the updated list.

Add DELETE /skill?location=... server endpoint, regenerate the SDK,
and update KiloProvider to call the SDK method instead of deleting
files directly from the extension.
Extract the inline DELETE /skill endpoint from server.ts into a
dedicated packages/opencode/src/server/routes/skill.ts file, reducing
kilo-specific changes in the shared server.ts and minimizing future
merge conflicts with upstream.

The route is mounted at /skill and uses the same lazy() pattern as
other kilo-specific route files (telemetry, enhance-prompt, etc.).
Rename routes/skill.ts to routes/kilocode.ts with KilocodeRoutes
export, mounted at /kilocode. This is the single home for all future
kilo-specific CLI endpoints, avoiding conflicts with the existing
/kilo gateway routes and keeping all kilo changes out of server.ts.

SDK: client.kilocode.removeSkill()
The DELETE /kilocode/skill endpoint now returns the updated skills
list (Skill.Info[]) instead of a boolean. The extension uses this
response directly to update the webview, eliminating the need for a
separate GET /skill round-trip that could return stale cached data.

Also normalize paths in Skill.remove() with path.resolve() to ensure
location strings match regardless of path format differences.
The DELETE endpoint was returning data that somehow rendered as empty
rows in the webview. Simplify the approach:

- Webview optimistically removes the skill from its signal immediately
- Extension fires DELETE to the backend (deletes from disk + cache)
- Extension invalidates cachedSkillsMessage so next tab visit fetches
  fresh data
- No second skillsLoaded message is sent after removal, avoiding the
  stale data overwrite that caused the UI corruption
…th query params

File paths as query parameters can fail due to URL encoding issues.
Switch to POST /kilocode/skill/remove with the location in the JSON
request body, which avoids encoding problems entirely.

Also log the SDK error response to aid debugging.
@markijbema
markijbema force-pushed the mark/add-skill-removal branch from 3874de6 to cfe4e8c Compare March 16, 2026 12:04
Comment thread packages/opencode/src/skill/skill.ts Outdated
Comment thread packages/kilo-vscode/src/KiloProvider.ts
@markijbema
markijbema merged commit 2a5808f into main Mar 16, 2026
12 of 13 checks passed
@markijbema
markijbema deleted the mark/add-skill-removal branch March 16, 2026 13:43
jliounis pushed a commit to jliounis/kilocode that referenced this pull request May 18, 2026
feat: add ability to remove discovered skills
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
feat: add ability to remove discovered skills
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