-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Feat/add toggle for edit rules skills #9506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c947e2a
1d0bf5d
03e7cb5
171d193
3148282
4f558fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -806,6 +806,9 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper | |
| case "updateConfig": | ||
| await this.handleUpdateConfig(message.config) | ||
| break | ||
| case "editSkill": | ||
| await this.handleEditSkill(message.originalName, message.name, message.content) | ||
| break | ||
| case "setLanguage": | ||
| await vscode.workspace | ||
| .getConfiguration("kilo-code.new") | ||
|
|
@@ -2292,6 +2295,60 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Handle skill editing request from the webview. | ||
| * Writes the updated skill content to the SKILL.md file. | ||
| */ | ||
| private async handleEditSkill(originalName: string, name: string, content: string): Promise<void> { | ||
| if (!this.client || this.connectionState !== "connected") { | ||
| this.postMessage({ | ||
| type: "skillEditResult", | ||
| success: false, | ||
| error: "Not connected to CLI backend", | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| const dir = this.getWorkspaceDirectory() | ||
|
|
||
| // Get the skill location from the CLI | ||
| const { data: skills } = await this.client.app.skills( | ||
| { directory: dir }, | ||
| { throwOnError: true }, | ||
| ) | ||
|
|
||
| const skill = skills.find((s) => s.name === originalName) | ||
| if (!skill) { | ||
| this.postMessage({ | ||
| type: "skillEditResult", | ||
| success: false, | ||
| error: `Skill "${originalName}" not found`, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const skillUri = vscode.Uri.file(skill.location) | ||
| const skillContent = Buffer.from(content, "utf8") | ||
|
|
||
| // Write the updated skill content | ||
| await vscode.workspace.fs.writeFile(skillUri, skillContent) | ||
|
|
||
|
Comment on lines
+2331
to
+2336
|
||
| // Send success message | ||
| this.postMessage({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Skill cache is left stale after a direct file write This handler writes
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed: Added explicit documentation of skill cache invalidation flow. After successful skill edit:
|
||
| type: "skillEditResult", | ||
| success: true, | ||
| }) | ||
| } catch (error) { | ||
| console.error("[Kilo New] KiloProvider: Failed to edit skill:", error) | ||
| this.postMessage({ | ||
| type: "skillEditResult", | ||
| success: false, | ||
| error: getErrorMessage(error) || "Failed to save skill", | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Ensure a session exists, creating one if needed. Returns the resolved | ||
| * session ID and workspace directory, or undefined when the client is | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import ModeEditView from "./ModeEditView" | |
| import ModeCreateView from "./ModeCreateView" | ||
| import McpEditView from "./McpEditView" | ||
| import WorkflowsTab from "./agent-behaviour/WorkflowsTab" | ||
| import { SkillEditModal } from "./SkillEditModal" | ||
| import { parseImport, MAX_IMPORT_SIZE } from "./mode-io" | ||
| import type { ImportError } from "./mode-io" | ||
|
|
||
|
|
@@ -74,6 +75,9 @@ const AgentBehaviourTab: Component = () => { | |
| // MCP view state | ||
| const [editingMcp, setEditingMcp] = createSignal<string>("") | ||
|
|
||
| // Skill edit state | ||
| const [editingSkill, setEditingSkill] = createSignal<SkillInfo | null>(null) | ||
|
|
||
| // Fetch skills whenever the skills subtab becomes active | ||
| createEffect(() => { | ||
| if (activeSubtab() === "skills") { | ||
|
|
@@ -170,6 +174,36 @@ const AgentBehaviourTab: Component = () => { | |
| updateConfig({ skills: { ...config().skills, urls: current } }) | ||
| } | ||
|
|
||
| const disabledSkills = () => config().skills?.disabled ?? [] | ||
|
|
||
| const toggleSkillEnabled = (skillName: string) => { | ||
| const disabled = [...disabledSkills()] | ||
| const index = disabled.indexOf(skillName) | ||
| if (index >= 0) { | ||
| disabled.splice(index, 1) | ||
| } else { | ||
| disabled.push(skillName) | ||
| } | ||
| updateConfig({ skills: { ...config().skills, disabled } }) | ||
| } | ||
|
|
||
| const isSkillDisabled = (skillName: string) => disabledSkills().includes(skillName) | ||
|
|
||
| const disabledRulePatterns = () => config().permission?.disabled ?? [] | ||
|
|
||
| const toggleRuleEnabled = (pattern: string) => { | ||
| const disabled = [...disabledRulePatterns()] | ||
| const index = disabled.indexOf(pattern) | ||
| if (index >= 0) { | ||
| disabled.splice(index, 1) | ||
| } else { | ||
| disabled.push(pattern) | ||
| } | ||
| updateConfig({ permission: { ...config().permission, disabled } }) | ||
| } | ||
|
Comment on lines
+192
to
+203
|
||
|
|
||
| const isRuleDisabled = (pattern: string) => disabledRulePatterns().includes(pattern) | ||
|
|
||
| const confirmRemoveSkill = (skill: SkillInfo) => { | ||
| dialog.show(() => ( | ||
| <Dialog title={language.t("settings.agentBehaviour.removeSkill.title")} fit> | ||
|
|
@@ -821,26 +855,45 @@ const AgentBehaviourTab: Component = () => { | |
| "justify-content": "space-between", | ||
| padding: "8px 0", | ||
| "border-bottom": index() < session.skills().length - 1 ? "1px solid var(--border-weak-base)" : "none", | ||
| opacity: isSkillDisabled(skill.name) ? "0.6" : "1", | ||
| "text-decoration": isSkillDisabled(skill.name) ? "line-through" : "none", | ||
| }} | ||
| > | ||
| <div style={{ flex: 1, "min-width": 0 }}> | ||
| <div data-slot="settings-row-label-title" style={{ "margin-bottom": "0" }}> | ||
| {skill.name} | ||
| </div> | ||
| <div | ||
| data-slot="settings-row-label-subtitle" | ||
| style={{ | ||
| "margin-top": "4px", | ||
| "font-family": "var(--vscode-editor-font-family, monospace)", | ||
| }} | ||
| > | ||
| <div>{skill.description}</div> | ||
| {skill.location !== "builtin" && <div>{skill.location}</div>} | ||
| <div style={{ flex: 1, "min-width": 0, display: "flex", "align-items": "center", gap: "8px" }}> | ||
| <Switch | ||
| checked={!isSkillDisabled(skill.name)} | ||
| onChange={() => toggleSkillEnabled(skill.name)} | ||
| hideLabel | ||
| /> | ||
| <div> | ||
| <div data-slot="settings-row-label-title" style={{ "margin-bottom": "0" }}> | ||
| {skill.name} | ||
| </div> | ||
| <div | ||
| data-slot="settings-row-label-subtitle" | ||
| style={{ | ||
| "margin-top": "4px", | ||
| "font-family": "var(--vscode-editor-font-family, monospace)", | ||
| }} | ||
| > | ||
| <div>{skill.description}</div> | ||
| {skill.location !== "builtin" && <div>{skill.location}</div>} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {skill.location !== "builtin" && ( | ||
| <IconButton size="small" variant="ghost" icon="close" onClick={() => confirmRemoveSkill(skill)} /> | ||
| )} | ||
| <div style={{ display: "flex", "align-items": "center", gap: "4px" }}> | ||
| {skill.location !== "builtin" && ( | ||
| <IconButton | ||
| size="small" | ||
| variant="ghost" | ||
| icon="pencil-line" | ||
| onClick={() => setEditingSkill(skill)} | ||
| /> | ||
| )} | ||
| {skill.location !== "builtin" && ( | ||
| <IconButton size="small" variant="ghost" icon="close" onClick={() => confirmRemoveSkill(skill)} /> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </For> | ||
|
|
@@ -1043,6 +1096,91 @@ const AgentBehaviourTab: Component = () => { | |
| </For> | ||
| </Card> | ||
|
|
||
| {/* Rules Toggle Section */} | ||
| <h4 style={{ "margin-top": "16px", "margin-bottom": "8px" }}> | ||
| {language.t("settings.agentBehaviour.rules.title") || "Permission Rules"} | ||
| </h4> | ||
| <Card style={{ "margin-bottom": "16px" }}> | ||
| <div | ||
| style={{ | ||
| "padding-bottom": "8px", | ||
| "border-bottom": "1px solid var(--border-weak-base)", | ||
| }} | ||
| > | ||
| <div | ||
| style={{ | ||
| "font-size": "12px", | ||
| color: "var(--text-weak-base, var(--vscode-descriptionForeground))", | ||
| "margin-top": "2px", | ||
| }} | ||
| > | ||
| {language.t("settings.agentBehaviour.rules.toggleDescription") || | ||
| "Enable or disable permission rules without deleting them"} | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Display discovered rules */} | ||
| <Show | ||
| when={ | ||
| config().permission && | ||
| Object.keys(config().permission).length > 0 && | ||
| Object.keys(config().permission).some((k) => k !== "disabled" && k !== "__originalKeys") | ||
| } | ||
| fallback={ | ||
| <div | ||
| style={{ | ||
| padding: "8px 0", | ||
| "font-size": "12px", | ||
| color: "var(--text-weak-base, var(--vscode-descriptionForeground))", | ||
| }} | ||
| > | ||
| {language.t("settings.agentBehaviour.rules.noRules") || "No permission rules configured"} | ||
| </div> | ||
| } | ||
| > | ||
| <For | ||
| each={Object.keys(config().permission ?? {}).filter( | ||
| (k) => k !== "disabled" && k !== "__originalKeys", | ||
| )} | ||
| > | ||
| {(permission, index) => { | ||
| const allKeys = Object.keys(config().permission ?? {}).filter( | ||
| (k) => k !== "disabled" && k !== "__originalKeys", | ||
| ) | ||
| return ( | ||
| <div | ||
| style={{ | ||
| display: "flex", | ||
| "align-items": "center", | ||
| "justify-content": "space-between", | ||
| padding: "6px 0", | ||
| "border-bottom": index() < allKeys.length - 1 ? "1px solid var(--border-weak-base)" : "none", | ||
| }} | ||
| > | ||
| <div style={{ display: "flex", "align-items": "center", gap: "8px", flex: 1 }}> | ||
| <div | ||
| style={{ | ||
| "font-family": "var(--vscode-editor-font-family, monospace)", | ||
| "font-size": "12px", | ||
| }} | ||
| > | ||
| {permission} | ||
| </div> | ||
| </div> | ||
| <div style={{ display: "flex", "align-items": "center", gap: "4px" }}> | ||
| <Switch | ||
| checked={!isRuleDisabled(permission)} | ||
| onChange={() => toggleRuleEnabled(permission)} | ||
| hideLabel | ||
| /> | ||
| </div> | ||
| </div> | ||
| ) | ||
| }} | ||
| </For> | ||
| </Show> | ||
| </Card> | ||
|
|
||
| {/* Claude Code compatibility */} | ||
| <h4 style={{ "margin-top": "16px", "margin-bottom": "8px" }}> | ||
| {language.t("settings.agentBehaviour.claudeCompat.heading")} | ||
|
|
@@ -1140,6 +1278,19 @@ const AgentBehaviourTab: Component = () => { | |
|
|
||
| {/* Subtab content */} | ||
| {renderSubtabContent()} | ||
|
|
||
| {/* Skill Edit Modal */} | ||
| <Show when={editingSkill()}> | ||
| {(skill) => ( | ||
| <SkillEditModal | ||
| skill={skill()} | ||
| onClose={() => setEditingSkill(null)} | ||
| onSave={(updated) => { | ||
| session.refreshSkills() | ||
| }} | ||
| /> | ||
| )} | ||
| </Show> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleEditSkillaccepts anameparameter but never uses it, while the UI allows editing the skill name. This will lead to a confusing UX where renames appear to succeed but are not persisted. Either disable name editing in the modal or implement renaming (including updating frontmatter and/or folder naming rules).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed: Disabled skill name editing in the modal to prevent confusing UX. The
namefield is now read-only since skill renaming requires folder/file renames which are not yet implemented. The backend no longer attempts to update the frontmatter name. Renaming can be added as a future enhancement.