From edb9098a70f2272cbd4a54a4db02369ae6041aa2 Mon Sep 17 00:00:00 2001 From: kirillk Date: Mon, 20 Jul 2026 12:53:52 -0400 Subject: [PATCH] fix(jetbrains): treat remote skills as read-only --- .../kilocode/backend/cli/KiloCliDataParser.kt | 8 ++- .../rpc/KiloAgentBehaviorRpcApiImpl.kt | 37 +++++++++- .../backend/rpc/KiloWorkspaceDtoMapper.kt | 1 + .../rpc/KiloAgentBehaviorRpcApiImplTest.kt | 18 +++++ .../settings/agents/SkillsConfigurable.kt | 16 +++-- .../settings/agents/SkillsSettingsUiTest.kt | 71 +++++++++++++------ .../kotlin/ai/kilocode/rpc/dto/SkillDto.kt | 1 + 7 files changed, 121 insertions(+), 31 deletions(-) diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/cli/KiloCliDataParser.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/cli/KiloCliDataParser.kt index 868a1c6306..73fd3058db 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/cli/KiloCliDataParser.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/cli/KiloCliDataParser.kt @@ -625,7 +625,13 @@ object KiloCliDataParser { val obj = item.obj() ?: return@mapNotNull null val name = obj.str("name") ?: return@mapNotNull null val location = obj.str("location") ?: return@mapNotNull null - SkillDto(name = name, description = obj.str("description"), location = location, content = obj.str("content")) + SkillDto( + name = name, + description = obj.str("description"), + location = location, + content = obj.str("content"), + editable = obj.bool("editable"), + ) } fun parseAgentBehaviorCommands(raw: String): List = diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImpl.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImpl.kt index 9d3e505608..1a85c74c37 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImpl.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImpl.kt @@ -23,6 +23,7 @@ import kotlinx.serialization.json.JsonPrimitive import okhttp3.MediaType.Companion.toMediaType import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody +import com.intellij.openapi.util.SystemInfo import java.net.URLEncoder import java.nio.file.Files import java.nio.file.InvalidPathException @@ -63,7 +64,10 @@ class KiloAgentBehaviorRpcApiImpl(private val backend: KiloBackendAppService? = override suspend fun skills(directory: String): List { val items = KiloCliDataParser.parseAgentBehaviorSkills(request(directory, "/skill", null)) - return items.map { item -> item.copy(content = skillContent(item) ?: item.content) } + return items.map { item -> + val editable = editable(item) + item.copy(content = skillContent(item) ?: item.content, editable = editable) + } } override suspend fun removeSkill(directory: String, location: String): Boolean = @@ -225,6 +229,35 @@ class KiloAgentBehaviorRpcApiImpl(private val backend: KiloBackendAppService? = }.getOrNull() } + private fun editable(skill: SkillDto): Boolean { + val raw = normalizeWorkspacePath(skill.location) ?: return false + val path = try { + Path.of(raw).normalize() + } catch (_: InvalidPathException) { + return false + } + if (!path.isAbsolute || !isSkillFile(path)) return false + if (urlCached(path)) return false + return true + } + + private fun urlCached(path: Path): Boolean { + val root = Path.of(cacheRoot(), "kilo", "skills").normalize() + if (path.startsWith(root)) return true + val parts = (0 until path.nameCount).map { path.getName(it).toString() } + return parts.windowed(3).any { it[1] == "kilo" && it[2] == "skills" && it[0] in cacheNames } + } + + private fun cacheRoot(): String { + val xdg = System.getenv("XDG_CACHE_HOME")?.takeIf { it.isNotBlank() } + if (xdg != null) return xdg + val home = System.getProperty("user.home") + if (SystemInfo.isMac) return Path.of(home, "Library", "Caches").toString() + if (SystemInfo.isWindows) return System.getenv("LOCALAPPDATA")?.takeIf { it.isNotBlank() } + ?: Path.of(home, "AppData", "Local").toString() + return Path.of(home, ".cache").toString() + } + private suspend fun patchConfig(path: String, body: String): Unit = withContext(Dispatchers.IO) { val http = app.http ?: throw IllegalStateException("Kilo HTTP client is unavailable") val url = "http://127.0.0.1:${app.port}$path" @@ -333,3 +366,5 @@ class KiloAgentBehaviorRpcApiImpl(private val backend: KiloBackendAppService? = val config: McpConfigDto?, ) } + +private val cacheNames = setOf(".cache", "cache", "Caches") diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceDtoMapper.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceDtoMapper.kt index 395ef9b4de..981ea1ad28 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceDtoMapper.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorkspaceDtoMapper.kt @@ -65,6 +65,7 @@ internal object KiloWorkspaceDtoMapper { description = s.description, location = s.location, content = s.content, + editable = false, ) private fun provider(p: ProviderInfo) = ProviderDto( diff --git a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImplTest.kt b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImplTest.kt index eaebbb1089..fad12e7cfb 100644 --- a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImplTest.kt +++ b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloAgentBehaviorRpcApiImplTest.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import java.nio.file.Files +import java.nio.file.Path import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertContains @@ -101,6 +102,8 @@ class KiloAgentBehaviorRpcApiImplTest { assertEquals(listOf("plan", "builtin"), skills.map { it.name }) assertEquals("Plan work", skills.single { it.name == "plan" }.description) assertEquals(content, skills.single { it.name == "plan" }.content) + assertEquals(true, skills.single { it.name == "plan" }.editable) + assertEquals(false, skills.single { it.name == "builtin" }.editable) assertTrue(rpc.removeSkill("/test project", file.toString())) assertEquals("{\"location\":\"$file\"}", mock.lastSkillRemoveBody) @@ -116,6 +119,21 @@ class KiloAgentBehaviorRpcApiImplTest { assertEquals(1, mock.requestCount("/instance/reload")) } + @Test + fun `url cached skills are read only`() = runBlocking { + val cache = Path.of(System.getProperty("user.home"), ".cache", "kilo", "skills", "remote") + val file = Files.createDirectories(cache).resolve("SKILL.md") + Files.writeString(file, "# Remote") + mock.skills = """[ + {"name":"remote","description":"Remote","location":"$file","content":"# Remote"} + ]""".trimIndent() + + val skill = rpc().skills("/test project").single() + + assertEquals(false, skill.editable) + assertEquals("# Remote", skill.content) + } + @Test fun `save skill supports configured markdown text and html files without reload`() = runBlocking { val dir = Files.createTempDirectory("kilo-skill-test") diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/agents/SkillsConfigurable.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/agents/SkillsConfigurable.kt index 807a0d0c59..164434f6de 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/agents/SkillsConfigurable.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/agents/SkillsConfigurable.kt @@ -235,31 +235,35 @@ internal class SkillsSettingsUi( OPEN_CELL, KiloBundle.message("settings.agentBehavior.skills.openInEditor"), primary = true, - ).takeUnless { builtin(skill.location) }, + ).takeIf { skill.editable }, SettingsListCell( EDIT_CELL, - KiloBundle.message(if (builtin(skill.location)) "common.open" else "settings.agentBehavior.edit"), - primary = builtin(skill.location), + KiloBundle.message(if (skill.editable) "settings.agentBehavior.edit" else "common.open"), + primary = !skill.editable, ), SettingsListCell( DELETE_CELL, KiloBundle.message("common.delete"), icon = AllIcons.Actions.GC, iconOnly = true, - ).takeUnless { builtin(skill.location) }, + ).takeIf { skill.editable }, ) } private fun edit(skill: SkillDto) { val current = skill.copy(content = content(skill)) - val dialog = edit(current, !builtin(skill.location)) + val dialog = edit(current, skill.editable) + if (!skill.editable) { + dialog.showAndGet() + return + } if (!dialog.showAndGet()) return state.update { copy(edited = edited + (skill.location to dialog.content())) } view.update(rows(), SettingsListSelection.Key(key(skill))) } private fun open(skill: SkillDto) { - if (builtin(skill.location)) return + if (!skill.editable) return showProgress(KiloBundle.message("settings.agentBehavior.skills.openInEditor.pending")) cs.launch { val opened = service().openFile(skill.location) diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/SkillsSettingsUiTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/SkillsSettingsUiTest.kt index 8e5736323e..fbde73c460 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/SkillsSettingsUiTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/SkillsSettingsUiTest.kt @@ -53,6 +53,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { private lateinit var appRpc: FakeAppRpcApi private lateinit var agentRpc: FakeAgentBehaviorRpcApi private lateinit var workspaceRpc: FakeWorkspaceRpcApi + private var shown = 0 override fun tearDown() { try { @@ -69,7 +70,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test loads skills with location note and builtins have no actions`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { val rows = rows(panel) @@ -90,6 +91,9 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { assertEquals(listOf("built-in"), builtin.badges.map { it.text }) assertEquals(listOf("edit"), builtin.cells.map { it.id }) assertEquals("Open", builtin.cells.single().label) + val remote = rows.single { it.key == REMOTE } + assertEquals(listOf("edit"), remote.cells.map { it.id }) + assertEquals("Open", remote.cells.single().label) assertEquals(listOf(DIR), agentRpc.skillCalls) true } @@ -97,7 +101,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test skills list is vertically scrolled without horizontal scrollbar`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { val pane = scrollFor(panel, skillsList(panel)) @@ -124,7 +128,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test skills list does not show description tooltips`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { val list = skillsList(panel) @@ -139,7 +143,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test renderer puts location on first line and description on preview line`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { val list = skillsList(panel) @@ -159,7 +163,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test double click stages skill content until apply`() { val panel = panel(edit = { _, _ -> FakeSkillDialog("# Saved") }) - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } doubleClick(skillsList(panel), panel, CUSTOM) @@ -172,7 +176,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test edited skill row keeps normal actions`() { val panel = panel(edit = { _, _ -> FakeSkillDialog("# Draft") }) - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } doubleClick(skillsList(panel), panel, CUSTOM) @@ -186,7 +190,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { seen += skill.content FakeSkillDialog(if (seen.size == 1) "# Draft" else "# Draft 2") }) - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } doubleClick(skillsList(panel), panel, CUSTOM) doubleClick(skillsList(panel), panel, CUSTOM) @@ -197,7 +201,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test open in editor action opens skill file`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } click(skillsList(panel), panel, CUSTOM, "open") @@ -206,6 +210,22 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { assertEquals(FakeWorkspaceRpcApi.Opened(CUSTOM, null, null), workspaceRpc.openedFiles.single()) } + fun `test read only skills open without staging edits or editor file open`() { + shown = 0 + val panel = panel(edit = { _, savable -> + assertFalse(savable) + FakeSkillDialog("# Ignored") { shown += 1 } + }) + flushUntil { rows(panel).size == 3 } + + click(skillsList(panel), panel, REMOTE, "edit") + + assertEquals(1, shown) + assertFalse(edt { panel.modified() }) + assertTrue(agentRpc.skillSaves.isEmpty()) + assertTrue(workspaceRpc.openedFiles.isEmpty()) + } + fun `test skill edit dialog shows content with fallback`() { edt { val content = SkillEditDialog(SkillDto("plan", "desc", CUSTOM, "# Plan\nUse steps"), true) @@ -240,7 +260,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test delete action stages skill removal until apply`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } TestDialogManager.setTestDialog(TestDialog.YES) click(skillsList(panel), panel, CUSTOM, "delete") @@ -254,7 +274,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test delete action requires confirmation`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } TestDialogManager.setTestDialog { Messages.NO } click(skillsList(panel), panel, CUSTOM, "delete") @@ -268,7 +288,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { var path = "/extra/skills" var url = "https://skills.test/index.json" val panel = panel(choose = { path }, input = { _, _ -> url }) - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { panel.sources.addPath(); true } edt { panel.sources.addUrl(); true } @@ -296,7 +316,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { val panel = panel(choose = { path }, input = { _, _ -> url }) appRpc.configUpdateReturnStale = true appRpc.afterConfig = { agentRpc.skills = agentRpc.skills + SkillDto("extra", "Extra skill", extra) } - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { panel.sources.addPath() @@ -316,7 +336,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test source reset discards staged changes`() { val path = "/extra/skills" val panel = panel(choose = { path }) - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { panel.sources.addPath(); true } @@ -325,14 +345,14 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { edt { panel.resetDraft(); true } assertTrue(appRpc.configPatches.isEmpty()) - assertEquals(listOf(CUSTOM, "builtin"), edt { rows(panel).map { it.key } }) + assertEquals(listOf(CUSTOM, "builtin", REMOTE), edt { rows(panel).map { it.key } }) assertFalse(edt { sourceRows(panel).any { it.key == "path:$path" } }) assertTrue(agentRpc.skillReloads.isEmpty()) } fun `test delete source writes skills config patch`() { val panel = panel() - flushUntil { rows(panel).size == 2 && sourceRows(panel).size == 2 } + flushUntil { rows(panel).size == 3 && sourceRows(panel).size == 2 } edt { sourceList(panel).selectedIndices = intArrayOf(0) @@ -353,7 +373,7 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { val panel = panel() appRpc.configUpdateReturnStale = true appRpc.afterConfig = { agentRpc.skills = agentRpc.skills.filterNot { it.location == CUSTOM } } - flushUntil { rows(panel).size == 2 && sourceRows(panel).size == 2 } + flushUntil { rows(panel).size == 3 && sourceRows(panel).size == 2 } edt { sourceList(panel).selectedIndices = intArrayOf(0) @@ -363,13 +383,13 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { } flushUntil { appRpc.configPatches.size == 1 && !edt { panel.modified() } } - assertEquals(listOf("builtin"), edt { rows(panel).map { it.key } }) + assertEquals(listOf("builtin", REMOTE), edt { rows(panel).map { it.key } }) assertEquals(listOf("url:https://skills.test/base.json"), edt { sourceRows(panel).map { it.key } }) } fun `test search filters skills by name`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } edt { components(panel).filterIsInstance().single().text = "think" @@ -382,13 +402,13 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { fun `test skills reload failure keeps existing rows`() { val panel = panel() - flushUntil { rows(panel).size == 2 } + flushUntil { rows(panel).size == 3 } agentRpc.skillsError = RuntimeException("timeout") edt { panel.reload(); true } flushUntil { edt { skillsList(panel).isEnabled } } - assertEquals(listOf(CUSTOM, "builtin"), edt { rows(panel).map { it.key } }) + assertEquals(listOf(CUSTOM, "builtin", REMOTE), edt { rows(panel).map { it.key } }) } fun `test skill editor file type follows location extension`() { @@ -426,8 +446,9 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { workspaceRpc = FakeWorkspaceRpcApi() agentRpc = FakeAgentBehaviorRpcApi().apply { skills = listOf( - SkillDto("plan", "Plan work", CUSTOM, "# Plan\nUse steps"), + SkillDto("plan", "Plan work", CUSTOM, "# Plan\nUse steps", editable = true), SkillDto("thinking", "Built in", "builtin", "Built in content"), + SkillDto("remote", "Remote skill", REMOTE, "# Remote skill"), ) } app = KiloAppService(cs, appRpc) @@ -550,10 +571,14 @@ class SkillsSettingsUiTest : BasePlatformTestCase() { private companion object { const val DIR = "/test" const val CUSTOM = "/home/test/.config/kilo/skill/plan/SKILL.md" + const val REMOTE = "/home/test/.cache/kilo/skills/remote/SKILL.md" } } -private class FakeSkillDialog(private val text: String) : SkillEditDialogHandle { - override fun showAndGet() = true +private class FakeSkillDialog(private val text: String, private val show: () -> Unit = {}) : SkillEditDialogHandle { + override fun showAndGet(): Boolean { + show() + return true + } override fun content() = text } diff --git a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/SkillDto.kt b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/SkillDto.kt index 386f43bc18..50dfee7adf 100644 --- a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/SkillDto.kt +++ b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/dto/SkillDto.kt @@ -8,4 +8,5 @@ data class SkillDto( val description: String? = null, val location: String, val content: String? = null, + val editable: Boolean = false, )