mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
fix(jetbrains): harden editor context and fix tool-view placement
Address review feedback on the automatic editor-context PR: - Skip malformed .kilocodeignore/.gitignore globs instead of throwing PatternSyntaxException that broke every prompt send. - Add KiloIgnoreCache so the compiled matcher is cached per root and invalidated via a VFS listener, keeping the blocking (remote cwm) read off the prompt-send path after the first prompt. - Gather editor context only on the prompt path so slash commands and client actions no longer pay its cost or hit its failure modes. - Guard Path.of(file.path) against cross-OS invalid filenames, matching rootDir(). - Keep the local Auto-Include Editor Context toggle interactive regardless of backend readiness; it is a per-IDE preference, not CLI config. - Fix a replaced tool view (e.g. completed question) jumping above the prompt bubble on user messages.
This commit is contained in:
+13
-11
@@ -683,17 +683,6 @@ class SessionUi(
|
||||
|
||||
private fun sendPrompt(text: String, files: List<PromptPartDto>) {
|
||||
if (text.isBlank() && files.isEmpty()) return
|
||||
val editor = EditorContextGatherer.gather(project, workspace.directory)
|
||||
val allFiles = files + listOfNotNull(editor.selection)
|
||||
val parts = buildList {
|
||||
text.takeIf { it.isNotBlank() }?.let { add(PromptPartDto(type = "text", text = it)) }
|
||||
addAll(allFiles)
|
||||
}
|
||||
LOG.debug {
|
||||
val agent = controller.model.agent ?: "none"
|
||||
val model = controller.model.model ?: "none"
|
||||
"${ChatLogSummary.prompt(PromptDto(parts = parts, editorContext = editor.context))} agent=$agent model=$model ready=${controller.ready}"
|
||||
}
|
||||
prompt.clear()
|
||||
val follow = scroll.atBottom()
|
||||
val action = completion.clientAction(text)
|
||||
@@ -708,6 +697,19 @@ class SessionUi(
|
||||
scroll.followBottom(follow)
|
||||
return
|
||||
}
|
||||
// Only the prompt path uses editor context; gather after the command branches so slash
|
||||
// commands and client actions don't pay the editor-context cost or hit its failure modes.
|
||||
val editor = EditorContextGatherer.gather(project, workspace.directory)
|
||||
val allFiles = files + listOfNotNull(editor.selection)
|
||||
LOG.debug {
|
||||
val parts = buildList {
|
||||
text.takeIf { it.isNotBlank() }?.let { add(PromptPartDto(type = "text", text = it)) }
|
||||
addAll(allFiles)
|
||||
}
|
||||
val agent = controller.model.agent ?: "none"
|
||||
val model = controller.model.model ?: "none"
|
||||
"${ChatLogSummary.prompt(PromptDto(parts = parts, editorContext = editor.context))} agent=$agent model=$model ready=${controller.ready}"
|
||||
}
|
||||
controller.prompt(text, allFiles, editor.context)
|
||||
scroll.followBottom(follow)
|
||||
}
|
||||
|
||||
+5
-2
@@ -6,6 +6,7 @@ import ai.kilocode.log.KiloLog
|
||||
import ai.kilocode.rpc.dto.EditorContextDto
|
||||
import ai.kilocode.rpc.dto.PromptPartDto
|
||||
import com.intellij.codeWithMe.ClientId
|
||||
import com.intellij.openapi.components.service
|
||||
import com.intellij.openapi.editor.Editor
|
||||
import com.intellij.openapi.fileEditor.FileDocumentManager
|
||||
import com.intellij.openapi.fileEditor.FileEditorManager
|
||||
@@ -49,7 +50,7 @@ internal object EditorContextGatherer {
|
||||
val openFiles = manager.openFilesWithRemotes
|
||||
val editor = manager.selectedTextEditorWithRemotes.firstOrNull()
|
||||
val activeFile = editor?.let { file(it) } ?: lastOpen(project, openFiles) ?: openFiles.firstOrNull()
|
||||
val ignore = KiloIgnore.load(rootDir(listOfNotNull(activeFile) + openFiles, base))
|
||||
val ignore = project.service<KiloIgnoreCache>().matcher(rootDir(listOfNotNull(activeFile) + openFiles, base))
|
||||
|
||||
fun keep(file: VirtualFile?): String? = rel(file, base)?.takeUnless { ignore.ignored(it) }
|
||||
|
||||
@@ -129,7 +130,9 @@ internal object EditorContextGatherer {
|
||||
|
||||
private fun local(file: VirtualFile, root: Path): Path? {
|
||||
if (file.fileSystem.protocol == KiloVirtualFileSystem.PROTOCOL) return null
|
||||
val path = Path.of(file.path).toAbsolutePath().normalize()
|
||||
// A host filename that is invalid on the client OS (e.g. `?`/`*` from a Linux host on
|
||||
// a Windows frontend) throws InvalidPathException; drop the file instead of failing the send.
|
||||
val path = runCatching { Path.of(file.path).toAbsolutePath().normalize() }.getOrNull() ?: return null
|
||||
if (!path.startsWith(root)) return null
|
||||
return path
|
||||
}
|
||||
|
||||
+6
-3
@@ -36,8 +36,8 @@ internal class KiloIgnore private constructor(private val rules: List<Rule>) {
|
||||
companion object {
|
||||
val EMPTY = KiloIgnore(emptyList())
|
||||
|
||||
private const val KILO = ".kilocodeignore"
|
||||
private const val GIT = ".gitignore"
|
||||
const val KILO = ".kilocodeignore"
|
||||
const val GIT = ".gitignore"
|
||||
private val SENSITIVE = listOf(".env", ".env.*")
|
||||
|
||||
/**
|
||||
@@ -80,7 +80,10 @@ internal class KiloIgnore private constructor(private val rules: List<Rule>) {
|
||||
val anchored = leading || line.contains('/')
|
||||
val prefix = if (anchored) "" else "(?:.*/)?"
|
||||
val suffix = if (dirOnly) "/.*" else "(?:/.*)?"
|
||||
return Rule(Regex("^$prefix${glob(line)}$suffix$"), negate)
|
||||
// A malformed character class (e.g. `[]`, `[z-a]`) yields an invalid Java regex.
|
||||
// Skip the bad rule instead of letting PatternSyntaxException break every prompt send.
|
||||
val regex = runCatching { Regex("^$prefix${glob(line)}$suffix$") }.getOrNull() ?: return null
|
||||
return Rule(regex, negate)
|
||||
}
|
||||
|
||||
private fun glob(glob: String): String {
|
||||
|
||||
+46
@@ -0,0 +1,46 @@
|
||||
package ai.kilocode.client.session.context
|
||||
|
||||
import com.intellij.openapi.Disposable
|
||||
import com.intellij.openapi.application.ApplicationManager
|
||||
import com.intellij.openapi.components.Service
|
||||
import com.intellij.openapi.vfs.VirtualFile
|
||||
import com.intellij.openapi.vfs.VirtualFileManager
|
||||
import com.intellij.openapi.vfs.newvfs.BulkFileListener
|
||||
import com.intellij.openapi.vfs.newvfs.events.VFileEvent
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
|
||||
/**
|
||||
* Caches the compiled [KiloIgnore] per workspace-root directory so editor-context
|
||||
* gathering does not re-read and re-compile the ignore files on every prompt.
|
||||
*
|
||||
* The compiled matcher is reused until a `.kilocodeignore` or `.gitignore` change
|
||||
* invalidates it via a VFS listener. This keeps the blocking VFS read (a remote `cwm`
|
||||
* round-trip in split mode) off the prompt-send path after the first prompt, instead of
|
||||
* repeating it for every message the user sends.
|
||||
*/
|
||||
@Service(Service.Level.PROJECT)
|
||||
internal class KiloIgnoreCache : Disposable {
|
||||
private val cache = ConcurrentHashMap<String, KiloIgnore>()
|
||||
|
||||
init {
|
||||
ApplicationManager.getApplication().messageBus.connect(this)
|
||||
.subscribe(VirtualFileManager.VFS_CHANGES, object : BulkFileListener {
|
||||
override fun after(events: List<VFileEvent>) {
|
||||
if (events.any { relevant(it) }) cache.clear()
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/** Returns the cached matcher for [root], compiling and caching it on first use. */
|
||||
fun matcher(root: VirtualFile?): KiloIgnore {
|
||||
if (root == null) return KiloIgnore.EMPTY
|
||||
return cache.getOrPut(root.url) { KiloIgnore.load(root) }
|
||||
}
|
||||
|
||||
override fun dispose() = cache.clear()
|
||||
|
||||
private fun relevant(event: VFileEvent): Boolean {
|
||||
val name = event.path.substringAfterLast('/')
|
||||
return name == KiloIgnore.KILO || name == KiloIgnore.GIT
|
||||
}
|
||||
}
|
||||
+5
-1
@@ -254,7 +254,11 @@ class MessageView(
|
||||
|
||||
@RequiresEdt
|
||||
private fun replacePart(content: Content, existing: PartView) {
|
||||
val at = components.indexOfFirst { it === existing || it === wrap }.takeIf { it >= 0 } ?: componentCount
|
||||
// A replaced tool view is a direct child, so re-insert at its own slot. Only fall back to
|
||||
// the prompt wrap's index when the replaced view is nested inside it, otherwise the wrap's
|
||||
// lower index would push the replacement above the prompt bubble on user messages.
|
||||
val at = (if (existing.parent !== this) components.indexOf(wrap) else components.indexOfFirst { it === existing })
|
||||
.takeIf { it >= 0 } ?: componentCount
|
||||
parts.remove(content.id)
|
||||
aliases.values.removeAll { it == content.id }
|
||||
sources.keys.removeAll { it !in aliases }
|
||||
|
||||
+7
-1
@@ -135,6 +135,9 @@ internal class ContextSettingsContent(
|
||||
private val update: (ContextDraft.() -> ContextDraft) -> Unit,
|
||||
) : BaseContentPanel() {
|
||||
private val auto = SettingsToggle { value -> update { copy(auto = value) } }
|
||||
// Editor-context auto-include is a local per-IDE preference in PropertiesComponent (like
|
||||
// autoApprove), applied immediately on toggle rather than through the CLI-backed draft/apply/
|
||||
// reset flow used by the other rows. It stays interactive even when the backend isn't READY.
|
||||
private val editor = SettingsToggle(KiloPluginSettings.getAutoEditorContext()) { value ->
|
||||
KiloPluginSettings.setAutoEditorContext(value)
|
||||
}
|
||||
@@ -183,11 +186,14 @@ internal class ContextSettingsContent(
|
||||
@RequiresEdt
|
||||
fun sync(draft: ContextDraft, enabled: Boolean) {
|
||||
auto.isSelected = draft.auto
|
||||
// Local preference: reflects PropertiesComponent and stays enabled regardless of the
|
||||
// CLI-backed [enabled] gating that applies to the draft-driven rows below.
|
||||
editor.isSelected = KiloPluginSettings.getAutoEditorContext()
|
||||
editor.isEnabled = true
|
||||
prune.isSelected = draft.prune
|
||||
threshold.sync(draft.threshold)
|
||||
patterns.sync(draft.ignore)
|
||||
listOf(auto, editor, prune, threshold, patterns).forEach { it.isEnabled = enabled }
|
||||
listOf(auto, prune, threshold, patterns).forEach { it.isEnabled = enabled }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+34
@@ -0,0 +1,34 @@
|
||||
package ai.kilocode.client.session.context
|
||||
|
||||
import com.intellij.openapi.application.ApplicationManager
|
||||
import com.intellij.openapi.components.service
|
||||
import com.intellij.openapi.vfs.VfsUtil
|
||||
import com.intellij.testFramework.fixtures.BasePlatformTestCase
|
||||
import com.intellij.util.ui.UIUtil
|
||||
|
||||
class KiloIgnoreCacheTest : BasePlatformTestCase() {
|
||||
fun `test matcher caches until ignore file changes`() {
|
||||
val file = myFixture.addFileToProject(".kilocodeignore", "ignored/\n").virtualFile
|
||||
val root = file.parent
|
||||
val cache = project.service<KiloIgnoreCache>()
|
||||
|
||||
val first = cache.matcher(root)
|
||||
assertTrue(first.ignored("ignored/Secret.kt"))
|
||||
assertFalse(first.ignored("src/App.kt"))
|
||||
assertSame(first, cache.matcher(root))
|
||||
|
||||
ApplicationManager.getApplication().runWriteAction {
|
||||
VfsUtil.saveText(file, "src/\n")
|
||||
}
|
||||
UIUtil.dispatchAllInvocationEvents()
|
||||
|
||||
val second = cache.matcher(root)
|
||||
assertNotSame(first, second)
|
||||
assertFalse(second.ignored("ignored/Secret.kt"))
|
||||
assertTrue(second.ignored("src/App.kt"))
|
||||
}
|
||||
|
||||
fun `test null root allows everything`() {
|
||||
assertSame(KiloIgnore.EMPTY, project.service<KiloIgnoreCache>().matcher(null))
|
||||
}
|
||||
}
|
||||
+6
@@ -86,4 +86,10 @@ class KiloIgnoreTest : TestCase() {
|
||||
val ignore = KiloIgnore.of("node_modules/")
|
||||
assertTrue(ignore.ignored("a\\node_modules\\pkg.js"))
|
||||
}
|
||||
|
||||
fun `test malformed char class is skipped without throwing`() {
|
||||
val ignore = KiloIgnore.of("[z-a]\n[]\n[!]\n*.log")
|
||||
assertTrue(ignore.ignored("debug.log"))
|
||||
assertFalse(ignore.ignored("src/App.kt"))
|
||||
}
|
||||
}
|
||||
|
||||
+48
@@ -0,0 +1,48 @@
|
||||
package ai.kilocode.client.session.views
|
||||
|
||||
import ai.kilocode.client.session.model.Message
|
||||
import ai.kilocode.client.session.model.Text
|
||||
import ai.kilocode.client.session.model.Tool
|
||||
import ai.kilocode.client.session.model.ToolExecState
|
||||
import ai.kilocode.client.session.model.ToolKind
|
||||
import ai.kilocode.client.session.views.question.QuestionResultView
|
||||
import ai.kilocode.rpc.dto.MessageDto
|
||||
import ai.kilocode.rpc.dto.MessageTimeDto
|
||||
import com.intellij.testFramework.fixtures.BasePlatformTestCase
|
||||
import javax.swing.SwingUtilities
|
||||
|
||||
class MessageViewTest : BasePlatformTestCase() {
|
||||
// A user message can carry both a prompt bubble (wrapped, lower component index) and a tool
|
||||
// view added after it. Replacing that tool (e.g. a completed question) must reuse the tool's
|
||||
// own slot, not the prompt wrap's lower index, or the replacement jumps above the bubble.
|
||||
fun `test replacing a tool view keeps it below the prompt bubble`() {
|
||||
val msg = Message(MessageDto("m1", "ses", "user", MessageTimeDto(0.0)))
|
||||
val view = MessageView(msg, openFile = { _, _ -> })
|
||||
|
||||
val text = Text("p1").also { it.content.append("do the thing") }
|
||||
msg.parts["p1"] = text
|
||||
view.upsertPart(text)
|
||||
|
||||
val tool = Tool("t1", "question", ToolKind.GENERIC).also {
|
||||
it.state = ToolExecState.RUNNING
|
||||
it.input = mapOf("questions" to """[{"question":"Proceed?"}]""")
|
||||
}
|
||||
msg.parts["t1"] = tool
|
||||
view.upsertPart(tool)
|
||||
|
||||
tool.state = ToolExecState.COMPLETED
|
||||
tool.metadata = mapOf("answers" to """[["Yes"]]""")
|
||||
view.upsertPart(tool)
|
||||
|
||||
val result = view.part("t1")
|
||||
val prompt = view.part("p1")
|
||||
assertNotNull(result)
|
||||
assertNotNull(prompt)
|
||||
assertTrue(result is QuestionResultView)
|
||||
val children = view.components.toList()
|
||||
val wrapIndex = children.indexOfFirst { SwingUtilities.isDescendingFrom(prompt, it) }
|
||||
val resultIndex = children.indexOf(result)
|
||||
assertTrue("prompt bubble is a direct child", wrapIndex >= 0)
|
||||
assertTrue("question result stays below the prompt bubble", resultIndex > wrapIndex)
|
||||
}
|
||||
}
|
||||
+31
-2
@@ -2,6 +2,8 @@ package ai.kilocode.client.settings.context
|
||||
|
||||
import ai.kilocode.client.app.KiloAppService
|
||||
import ai.kilocode.client.app.KiloWorkspaceService
|
||||
import ai.kilocode.client.plugin.KiloPluginSettings
|
||||
import ai.kilocode.client.settings.base.SettingsRow
|
||||
import ai.kilocode.client.settings.base.SettingsToggle
|
||||
import ai.kilocode.client.ui.HoverIcon
|
||||
import ai.kilocode.client.testing.FakeAppRpcApi
|
||||
@@ -69,6 +71,7 @@ class ContextSettingsUiTest : BasePlatformTestCase() {
|
||||
ui = null
|
||||
uiScope.cancel()
|
||||
appScope.cancel()
|
||||
KiloPluginSettings.unsetAutoEditorContext()
|
||||
} finally {
|
||||
super.tearDown()
|
||||
}
|
||||
@@ -253,21 +256,39 @@ class ContextSettingsUiTest : BasePlatformTestCase() {
|
||||
}
|
||||
}
|
||||
|
||||
fun `test controls are disabled during pending save`() {
|
||||
fun `test controls are disabled during pending save except local editor toggle`() {
|
||||
val panel = requireUi()
|
||||
rpc.configUpdateGate = CompletableDeferred()
|
||||
|
||||
edt {
|
||||
threshold(panel).text = "80"
|
||||
panel.applyDraft()
|
||||
assertTrue(components(panel).filterIsInstance<SettingsToggle>().all { !it.isEnabled })
|
||||
val editor = editorToggle(panel)
|
||||
val cli = components(panel).filterIsInstance<SettingsToggle>().filter { it !== editor }
|
||||
assertTrue(cli.all { !it.isEnabled })
|
||||
assertFalse(threshold(panel).isEnabled)
|
||||
assertTrue(editor.isEnabled)
|
||||
}
|
||||
|
||||
rpc.configUpdateGate?.complete(Unit)
|
||||
flushUntil { rpc.configPatches.isNotEmpty() }
|
||||
}
|
||||
|
||||
fun `test editor context toggle persists immediately without a config patch`() {
|
||||
val panel = requireUi()
|
||||
assertTrue(KiloPluginSettings.getAutoEditorContext())
|
||||
|
||||
edt {
|
||||
val editor = editorToggle(panel)
|
||||
assertTrue(editor.isEnabled)
|
||||
editor.doClick()
|
||||
}
|
||||
|
||||
assertFalse(KiloPluginSettings.getAutoEditorContext())
|
||||
edt { UIUtil.dispatchAllInvocationEvents() }
|
||||
assertTrue(rpc.configPatches.isEmpty())
|
||||
}
|
||||
|
||||
private fun requireUi(): ContextSettingsUi = requireNotNull(ui)
|
||||
|
||||
private fun threshold(panel: ContextSettingsUi): JBTextField = components(panel)
|
||||
@@ -284,6 +305,14 @@ class ContextSettingsUiTest : BasePlatformTestCase() {
|
||||
.filterIsInstance<HoverIcon>()
|
||||
.single { it.toolTipText == tip }
|
||||
|
||||
private fun editorToggle(panel: ContextSettingsUi): SettingsToggle {
|
||||
val label = components(panel).filterIsInstance<JLabel>()
|
||||
.first { it.text == "Auto-Include Editor Context" }
|
||||
var row: Container? = label.parent
|
||||
while (row != null && row !is SettingsRow) row = row.parent
|
||||
return components(requireNotNull(row)).filterIsInstance<SettingsToggle>().single()
|
||||
}
|
||||
|
||||
private fun <T> edt(block: () -> T): T {
|
||||
var result: T? = null
|
||||
ApplicationManager.getApplication().invokeAndWait { result = block() }
|
||||
|
||||
Reference in New Issue
Block a user