feat(vscode): add workflow management and slash command support#6260
feat(vscode): add workflow management and slash command support#6260Githubguy132010 wants to merge 23 commits into
Conversation
Implement full workflow support in the VS Code extension:
- View, create, edit, and delete workflows in Settings > Agent Behaviour > Workflows
- Type '/' in the chat input to see a filterable dropdown of all available commands
- Slash commands route to POST /session/{id}/command for proper backend execution
- New files: commands context, useSlashCommand hook, slash command CSS
- i18n strings added for all 16 locales
Closes Kilo-Org#6069
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No New Issues Found | Recommendation: Address remaining open comments, then merge OverviewThis PR adds slash command support (workflows, skills, MCP prompts) to the VS Code extension, including:
The PR has gone through multiple review iterations and many previously flagged issues have been fixed in subsequent commits (path traversal validation, delete confirmation, error handling, optimistic messages, Previously Flagged Issues — Now Fixed ✅
Remaining Open Items from Previous ReviewsSee inline comments for details. Key remaining items:
Most remaining items are edge cases or minor improvements rather than blocking issues. Files Reviewed (24 files)
|
Add validateWorkflowName() guard to all three workflow file handlers. Rejects names containing path separators or directory traversal patterns (e.g. '../../etc/something') with an early return and error message.
…lify slash input - Enforce strict alphanumeric/hyphen/underscore allowlist in validateWorkflowName - Use write-exclusive flag (wx) in createWorkflowFile to prevent silent overwrite - Remove redundant dead code in useSlashCommand onInput
Show a native VS Code modal warning before deleting a workflow file to prevent accidental data loss from misclicks.
Catch cloud session import failures and emit `cloudSessionImportFailed` with a useful error message instead of failing silently. Also only return a project workflow path when the file exists, so missing project-scoped workflows correctly fall back to other resolution paths.
Propagate command-loading errors to the webview and surface them to users, while keeping command requests retried with bounded exponential backoff. Relax local slash-command validation to format checks so command execution is not blocked by backend command-list fetch issues. Improve cloud import failure handling by emitting a dedicated failure message, cleaning up optimistic preview messages, and only clearing the active session when the failed import matches the current preview context.
Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com>
- useSlashCommand.ts: guard ArrowDown against empty filtered list to prevent Math.min producing -1 index - commands.tsx: re-request commands on backend reconnect (ready/connected) so slow startup doesn't permanently miss command loading; refactor retry logic into startRetry() helper - session.tsx: create optimistic message in sendCommand even when no session exists yet, using a temporary pending session key that is transferred to the real session ID in handleSessionCreated
| // chat without waiting for the server. When there is no session yet, use a | ||
| // temporary pending key; handleSessionCreated will transfer the messages once | ||
| // the real session ID is known. | ||
| const targetSid = sid ?? `pending:${crypto.randomUUID()}` |
There was a problem hiding this comment.
WARNING: Race condition when sendCommand is called twice before the first session is created
On the first call, sid is undefined, so a pending:* key is generated and set as currentSessionID (line 935). On a second call (before sessionCreated arrives), currentSessionID() returns the pending:* string, so sid is now truthy. The code skips the pending-key branch and sends sessionID: sid (line 956) — which is the pending:uuid string — to the backend. The backend will fail because pending:uuid is not a valid session ID.
While the UI's isBusy() guard in handleSend makes this unlikely in normal usage, it's still possible if the session status SSE event hasn't arrived yet or if sendCommand is called programmatically.
Consider either:
- Checking if
sidstarts withpending:and treating it as no-session (sendsessionID: undefined) - Queuing commands while a pending session creation is in flight
| case "error": | ||
| // Only clear loading if the error is for the current session | ||
| // (or has no sessionID for backwards compatibility) | ||
| clearOptimisticMessages(message.sessionID ?? currentSessionID()) // kilocode_change |
There was a problem hiding this comment.
WARNING: pendingSessionKey is not reset on the error path
When an error arrives, clearOptimisticMessages removes the optimistic messages from the pending key, but setPendingSessionKey(null) is never called. If a subsequent unrelated sessionCreated event arrives (e.g., from a different flow), handleSessionCreated (line 458) will check pendingSessionKey(), find the stale key, and attempt to transfer messages from it — which will be an empty array but still triggers unnecessary store updates.
Consider adding setPendingSessionKey(null) in the error handler when the cleared session matches the pending key.
| if (message.type === "commandsLoaded") { | ||
| setCommands(message.commands) | ||
| setError(message.error) | ||
| setLoading(false) |
There was a problem hiding this comment.
SUGGESTION: clearTimeouts() should be called when commandsLoaded is received
When the commands load successfully, setLoading(false) stops future retries from sending new requests (via the if (!loading()) return guard in tick), but the pending setTimeout callbacks remain allocated. They'll fire, check loading(), and return — but in the meantime they hold references and send one extra requestCommands message each before the guard kicks in.
Adding clearTimeouts() here would cancel pending retries immediately:
if (message.type === "commandsLoaded") {
clearTimeouts()
setCommands(message.commands)
...
}- commands.tsx: call clearTimeouts() immediately when commandsLoaded is received to cancel pending retry callbacks - session.tsx: fix race condition where a second sendCommand call before session creation would send a pending:* key as sessionID to the backend; reuse the existing pending key and pass undefined to the backend - session.tsx: reset pendingSessionKey on the error path to prevent stale state from causing unnecessary store updates on the next sessionCreated
| if (pendingKey) { | ||
| const pendingMessages = store.messages[pendingKey] ?? [] | ||
| if (pendingMessages.length > 0) { | ||
| setStore("messages", session.id, pendingMessages) |
There was a problem hiding this comment.
SUGGESTION: Memory leak — stale pending messages are never cleaned up from the store
When messages are transferred from pendingKey to session.id, the old store.messages[pendingKey] entry (keyed by e.g. pending:uuid) is never deleted. The store.parts entries for the optimistic message IDs also remain under the old key. Over time (e.g., if the user sends many commands that each create a new pending key), these orphaned entries accumulate.
Consider deleting the stale entries after transfer:
if (pendingKey) {
const pendingMessages = store.messages[pendingKey] ?? []
if (pendingMessages.length > 0) {
setStore("messages", session.id, pendingMessages)
}
// Clean up stale pending entries
setStore("messages", pendingKey, undefined)
setPendingSessionKey(null)
}| }) | ||
| } catch (err) { | ||
| console.error("[Kilo New] Failed to execute command after cloud import:", err) | ||
| this.postMessage({ |
There was a problem hiding this comment.
WARNING: Misleading error type — cloudSessionImportFailed sent after a successful import
At this point, the cloud session has already been successfully imported and cloudSessionImported was sent to the webview (line 1492). If only the command execution fails, sending cloudSessionImportFailed is semantically incorrect. The webview handler for this message type:
- Shows a toast saying "Failed to import cloud session" — misleading since the import succeeded
- Calls
clearOptimisticMessages(key)using the cloud key, which won't match the real session ID - May call
setCurrentSessionID(undefined), discarding the successfully imported session
The sessionID field (line 1510) partially mitigates issue #3, but the toast message and optimistic message cleanup are still wrong. Consider posting a regular error message with the session ID instead:
this.postMessage({
type: "error",
message: err instanceof Error ? err.message : "Failed to execute command after import",
sessionID: session.id,
})- KiloProvider.ts: after a successful cloud session import, if executeCommand fails send a regular error message instead of cloudSessionImportFailed so the webview keeps the imported session rather than discarding it - session.tsx: clean up stale pending session messages from the store after transferring them to the real session ID to prevent a memory leak
| this.postMessage({ | ||
| type: "error", | ||
| message: session ? `[sessionID:${session}] ${base}` : base, | ||
| }) |
There was a problem hiding this comment.
WARNING: handleSendCommand error response is missing the sessionID property, unlike handleImportAndSendCommand (line 1512)
The webview's error handler in session.tsx (line 399–403) checks message.sessionID to decide whether to clear loading state and reset pendingSessionKey. Without sessionID on this error message, the !message.sessionID fallback always evaluates to true, which means:
- Loading state is cleared even if the error is for a different session than the one currently displayed.
pendingSessionKeyis always reset, even when the error belongs to a background session.
This is inconsistent with handleImportAndSendCommand at line 1509–1513 which correctly includes sessionID: session.id.
| }) | |
| this.postMessage({ | |
| type: "error", | |
| message: session ? `[sessionID:${session}] ${base}` : base, | |
| sessionID: session, | |
| }) |
Without the sessionID property on the error message, the webview error handler always matched the current session (falling back to currentSessionID()), potentially clearing loading state for the wrong session when multiple sessions are active.
|
Redoing this PR tomorrow because bugs keep coming in |
1 participant
Fixes #6069
Context
Implements full workflow support in the VS Code extension, addressing #6069. Previously, the Workflows tab in Settings showed only a "Not yet implemented" placeholder. The backend already supported workflows (exposed as commands via
GET /command), but the extension had no UI to view, manage, or invoke them.Implementation
The implementation follows the established Extension ↔ Webview Feature Pattern documented in
packages/kilo-vscode/AGENTS.md.Workflow Viewing & Management (Settings > Agent Behaviour > Workflows)
CommandInfotype,listCommands()andexecuteCommand()methods toHttpClient, pluscommandsLoaded/requestCommandsmessage typescontext/commands.tsx): New SolidJS context provider that fetches commands from the CLI backend with retry logic, wired into bothApp.tsxandAgentManagerApp.tsxprovider hierarchies.mdworkflow files in.kilocode/workflows/handleCreateWorkflowFilecreates the directory + file with a template and opens it in the editor;handleOpenWorkflowFileopens an existing file;handleDeleteWorkflowFileremoves the file — all refresh the command list afterwardSlash Command Support (Chat Input)
useSlashCommandhook (new file): Detects "/" at the start of chat input, filters the full command list (workflows, skills, MCP prompts) as the user types, supports keyboard navigation (ArrowUp/Down, Enter/Tab, Escape)@file mention dropdown. Each item shows the command name, description, and a source badge (skill/mcp). Selecting fills the input with/{name}for argument entrysession.sendCommand()→POST /session/{id}/commandinstead of the normal message endpoint, matching how the desktop app handles slash commandsi18n
All new strings translated across 16 locales (ar, br, bs, da, de, en, es, fr, ja, ko, no, pl, ru, th, zh, zht).
Screenshots
How to Test
Workflow management:
.mdfile should open in the editorSlash commands:
/{name}— type arguments and press EnterGet in Touch
thomas07374