feat: Introduced an extension system to support multiple AI assistants.#38
Conversation
… the new naming convention
qdaxb
force-pushed
the
20250808_support_multiple
branch
from
3bc8514 to
1c34596
Compare
|
Caution Review failedThe pull request is closed. WalkthroughAdds a pluggable extension-provider system to the JetBrains plugin with provider selection/switching, VSIX install/upload support, dynamic toolbar/right-click/menu actions, persistent per-extension configuration, provider-driven extension startup, many new services/UI components, docs, tests, and assorted logging/packaging tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant IDE as JetBrains IDE
participant Plugin as WecoderPluginService
participant ExtCfg as ExtensionConfigurationManager
participant ExtMgr as ExtensionManager
participant Provider as ExtensionProvider
participant Host as ExtensionHostManager
User->>IDE: Open project
IDE->>Plugin: onProjectOpened
Plugin->>ExtCfg: initialize()
ExtCfg-->>Plugin: return currentExtensionId?
Plugin->>ExtMgr: initialize(configuredExtensionId)
ExtMgr->>ExtMgr: register providers (Roo, Cline)
ExtMgr->>Provider: initialize(project)
Plugin->>Host: start(extension from Provider.getConfiguration(project))
Host-->>Plugin: ready/initialized
sequenceDiagram
autonumber
actor User
participant UI as ExtensionSwitcherDialog
participant Switcher as ExtensionSwitcher
participant ExtMgr as ExtensionManager
participant Host as ExtensionHostManager
participant BtnMgr as DynamicButtonManager
User->>UI: Confirm switch to targetId
UI->>Switcher: switchExtension(targetId, forceRestart=true)
Switcher->>ExtMgr: setCurrentProvider(targetId)
ExtMgr-->>Switcher: provider updated
Switcher->>Host: stop current extension
Switcher->>Host: start extension with new provider config
Host-->>Switcher: restarted
Switcher->>BtnMgr: setCurrentExtension(targetId)
Switcher-->>UI: success/failure
sequenceDiagram
autonumber
participant UI as VsixUploadDialog
participant Vsix as VsixManager
participant FS as Filesystem
participant PluginRes as Plugin Resources
UI->>Vsix: installVsix(file, extensionId)
Vsix->>FS: extract ZIP to temp
Vsix->>FS: move extracted content to ~/.run-vs-agent/{extensionId}
Vsix->>PluginRes: locate plugin themes dir
PluginRes-->>Vsix: themes path?
Vsix->>FS: copy themes into extensionDir/src/integrations/theme/default-themes
Vsix-->>UI: return success/failure
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
jetbrains_plugin/build.gradle.kts (1)
62-127: Debug mode "none" contradicts docs and still executes heavy packaging logicPer the header comments (Lines 38-42), "none" should be lightweight (no platform.zip, no VSCode runtime resources). The current else-branch (non-"idea") executes for both "release" and "none", requiring prodDep.txt, plugin dir, and copying extension_host artifacts. This likely breaks CI and local lightweight runs.
Refactor the conditional to explicitly separate "release" and "none" branches.
Apply this diff to restrict heavy copying to release only and make "none" truly lightweight:
- } else { + } else if (ext.get("debugMode") == "release") { val vscodePluginDir = File("./plugins/${ext.get("vscodePlugin")}") if (!vscodePluginDir.exists()) { throw IllegalStateException("missing plugin dir") } val list = mutableListOf<String>() val depfile = File("prodDep.txt") if (!depfile.exists()) { throw IllegalStateException("missing prodDep.txt") } depfile.readLines().let { it.forEach { line -> list.add(line.substringAfterLast("node_modules/") + "/**") } } from("../extension_host/dist") { into("${intellij.pluginName.get()}/runtime/") } from("../extension_host/package.json") { into("${intellij.pluginName.get()}/runtime/") } // First copy extension_host node_modules from("../extension_host/node_modules") { into("${intellij.pluginName.get()}/node_modules/") list.forEach { include(it) } } from("${vscodePluginDir.path}/extension") { into("${intellij.pluginName.get()}/${ext.get("vscodePlugin")}") } from("src/main/resources/themes/") { into("${intellij.pluginName.get()}/${ext.get("vscodePlugin")}/integrations/theme/default-themes/") } // The platform.zip file required for release mode is associated with the code in ../base/vscode, currently using version 1.100.0. If upgrading this code later // Need to modify the vscodeVersion value in gradle.properties, then execute the task named genPlatform, which will generate a new platform.zip file for submission // To support new architectures, modify according to the logic in genPlatform.gradle script - if (ext.get("debugMode") == "release") { + if (ext.get("debugMode") == "release") { // Check if platform.zip file exists and is larger than 1MB, otherwise throw exception val platformZip = File("platform.zip") if (platformZip.exists() && platformZip.length() >= 1024 * 1024) { // Extract platform.zip to the platform subdirectory under the project build directory val platformDir = File("${project.buildDir}/platform") platformDir.mkdirs() copy { from(zipTree(platformZip)) into(platformDir) } } else { throw IllegalStateException("platform.zip file does not exist or is smaller than 1MB. This file is supported through git lfs and needs to be obtained through git lfs") } from(File(project.buildDir, "platform/platform.txt")) { into("${intellij.pluginName.get()}/") } // Copy platform node_modules last to ensure it takes precedence over extension_host node_modules from(File(project.buildDir, "platform/node_modules")) { into("${intellij.pluginName.get()}/node_modules") } } doLast { File("${destinationDir}/${intellij.pluginName.get()}/${ext.get("vscodePlugin")}/.env").createNewFile() } - } + } else { + // debugMode == "none" — lightweight mode: copy only core themes + from("src/main/resources/themes/") { + into("${intellij.pluginName.get()}/${ext.get("vscodePlugin")}/integrations/theme/default-themes/") + } + }jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/terminal/WeCoderTerminalCustomizer.kt (2)
103-108: Do not log all environment variables (potential secret leakage)Logging the full env map can expose tokens and credentials from the IDE environment. Restrict or remove.
Apply this diff to log a safe subset only at debug level:
- logger.info("🌍 Environment variables: ${envs.entries.joinToString("\n")}") + if (logger.isDebugEnabled) { + val safeKeys = envs.keys.filter { it.startsWith("VSCODE_") || it in setOf("ZDOTDIR","USER_ZDOTDIR","ORIGINAL_BASH_RCFILE","ORIGINAL_POWERSHELL_SCRIPT") } + logger.debug("🌍 Environment variables (safe subset): ${safeKeys.joinToString(", ")}") + }
70-79: Close InputStreams to avoid resource leaks when copying integration filesinputStream is not closed. Wrap it in use to ensure proper closure.
Apply this diff:
- val inputStream = javaClass.classLoader.getResourceAsStream("$sourceDir/$fileName") - if (inputStream != null) { + val inputStream = javaClass.classLoader.getResourceAsStream("$sourceDir/$fileName") + if (inputStream != null) { val targetFile = File("$targetDir/$fileName") - targetFile.outputStream().use { outputStream -> - inputStream.copyTo(outputStream) - } + inputStream.use { input -> + targetFile.outputStream().use { output -> + input.copyTo(output) + } + } // Set executable permission targetFile.setExecutable(true, true) logger.info("✅ Successfully copied $shellType file: $fileName")jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/ipc/proxy/RPCProtocol.kt (3)
131-131: Eliminate race conditions on lastMessageId by using AtomicInteger
lastMessageIdis incremented inremoteCallwithout synchronization and can be called concurrently, leading to duplicate or out-of-order IDs. Replace with AtomicInteger and update usage:- private var lastMessageId = 0 + private val lastMessageId = java.util.concurrent.atomic.AtomicInteger(0)And in remoteCall (already shown in a previous comment), use:
val req = lastMessageId.incrementAndGet()
744-781: Don’t treat Double as compatible with Boolean parameters
isMethodCompatiblecurrently treatsDoubleas compatible withBooleanwhich can route calls to the wrong overload and fail at invocation. Limit numeric widening to numeric targets only.- arg is Double && (paramJavaClass == Int::class.java || - paramJavaClass == Long::class.java || - paramJavaClass == Float::class.java || - paramJavaClass == Short::class.java || - paramJavaClass == Byte::class.java || - paramJavaClass == Boolean::class.java) -> true + arg is Double && ( + paramJavaClass == Int::class.java || + paramJavaClass == Long::class.java || + paramJavaClass == Float::class.java || + paramJavaClass == Short::class.java || + paramJavaClass == Byte::class.java + ) -> true
413-468: Fix cancellation-token handling, secure logging, and streamline promise cancellation• Update serialization and outgoing logging to use
effectiveArgs(without the trailing cancellation token)- val serializedRequestArguments = MessageIO.serializeRequestArguments(args.toList(), uriReplacer) + val serializedRequestArguments = MessageIO.serializeRequestArguments(effectiveArgs.toList(), uriReplacer) … - logger?.logOutgoing( - msg.size, - req, - RequestInitiator.LocalSide, - "request: ${getStringIdentifierForProxy(rpcId)}.$methodName(", - args - ) + logger?.logOutgoing( + msg.size, + req, + RequestInitiator.LocalSide, + "request: ${getStringIdentifierForProxy(rpcId)}.$methodName(", + effectiveArgs + )• Defer and demote initial log to DEBUG once the atomic request ID is available
- LOG.info("remoteCall: $rpcId.$methodName.${lastMessageId+1}") + // defer info-level logging until after we generate req; guard under DEBUG• Switch
lastMessageIdto anAtomicIntegerfor thread-safe increments and log under DEBUG-import kotlin.jvm.Volatile -private var lastMessageId = 0 +import java.util.concurrent.atomic.AtomicInteger +private val lastMessageId = AtomicInteger(0) … - val req = ++lastMessageId + val req = lastMessageId.incrementAndGet() + if (LOG.isDebugEnabled) { + LOG.debug("remoteCall: $rpcId.$methodName.$req") + }• Remove the unused
deferredpromise and wire cancellation directly toresult- val result = LazyPromise() - - // Use LazyPromise to implement Promise functionality - val deferred = LazyPromise() - - // Create Disposable object for cleanup on cancel - val disposable = Disposable { - if (!deferred.isCompleted) { - deferred.cancel() - } - } + val result = LazyPromise() + // Cancel the same promise on disposal + val disposable = Disposable { + if (!result.isCompleted) { + result.cancel() + } + }These changes ensure the cancellation token is excluded from the payload, sensitive arguments aren’t logged at INFO, IDs are generated safely in concurrent contexts, and we don’t maintain an unused promise.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionProcessManager.kt (2)
131-141: UDS mode passes null CLI args; build args conditionallyWhen
isUdsis true, you only setVSCODE_EXTHOST_IPC_HOOKbut still append--vscode-socket-*args using null values, resulting in--vscode-socket-port=nulletc. Build CLI args per mode:- val commandArgs = mutableListOf( - nodePath, - "--experimental-global-webcrypto", - "--no-deprecation", -// "--trace-uncaught", - extensionPath, - "--vscode-socket-port=${envVars["VSCODE_EXTHOST_SOCKET_PORT"]}", - "--vscode-socket-host=${envVars["VSCODE_EXTHOST_SOCKET_HOST"]}", - "--vscode-will-send-socket=${envVars["VSCODE_EXTHOST_WILL_SEND_SOCKET"]}" - ) + val commandArgs = mutableListOf( + nodePath, + "--experimental-global-webcrypto", + "--no-deprecation", +// "--trace-uncaught", + extensionPath + ).apply { + if (isUds) { + // If your extension host expects a flag for UDS, add it here; otherwise, omit socket flags entirely. + // Example (adjust as needed): + // add("--vscode-ipc-hook=${envVars["VSCODE_EXTHOST_IPC_HOOK"]}") + } else { + add("--vscode-socket-port=${envVars["VSCODE_EXTHOST_SOCKET_PORT"]}") + add("--vscode-socket-host=${envVars["VSCODE_EXTHOST_SOCKET_HOST"]}") + add("--vscode-will-send-socket=${envVars["VSCODE_EXTHOST_WILL_SEND_SOCKET"]}") + } + }
161-167: Do not log raw environment variable values; mask sensitive dataDumping all env var values at info can leak API keys, tokens, and credentials. Mask sensitive keys and/or only log keys:
- LOG.info("Environment variables:") - envVars.forEach { (key, value) -> - LOG.info(" $key = $value") - } + LOG.info("Environment variables:") + val sensitiveKeyHints = listOf("KEY", "TOKEN", "SECRET", "PASSWORD", "AUTH", "COOKIE") + envVars.forEach { (key, value) -> + val mask = sensitiveKeyHints.any { key.contains(it, ignoreCase = true) } + val display = if (mask) "********" else value.take(256) + LOG.info(" $key = $display") + }jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/plugin/WecoderPlugin.kt (3)
200-206: Compile-time bug: string interpolation references a type, not a variable
$DEBUG_MODErefers to the enum type, not the current value. This won’t compile. You want$DEBUG_TYPE.Apply:
- Logger.getInstance(WecoderPluginService::class.java).info("Read debug mode from config file: $DEBUG_MODE") + Logger.getInstance(WecoderPluginService::class.java).info("Read debug mode from config file: $DEBUG_TYPE") } else { - Logger.getInstance(WecoderPluginService::class.java).warn("Cannot load config file, use default debug mode: $DEBUG_MODE") + Logger.getInstance(WecoderPluginService::class.java).warn("Cannot load config file, use default debug mode: $DEBUG_TYPE") } } catch (e: Exception) { - Logger.getInstance(WecoderPluginService::class.java).warn("Error reading config file, use default debug mode: $DEBUG_MODE", e) + Logger.getInstance(WecoderPluginService::class.java).warn("Error reading config file, use default debug mode: $DEBUG_TYPE", e) }
357-361: Process leak in non-ALL debug modesYou start the extension process for all modes except
ALL, but only stop it whenDEBUG_TYPE == NONE. InIDEAorRELEASEmodes, this leaks the process on cleanup.Apply:
- // Stop extension process, only needed in non-debug mode - if (DEBUG_TYPE == com.sina.weibo.agent.plugin.DEBUG_MODE.NONE) { + // Stop extension process in all modes except ALL (ALL connects to external debug host) + if (DEBUG_TYPE != com.sina.weibo.agent.plugin.DEBUG_MODE.ALL) { processManager.stop() }
101-104: Guard disposer registration against null service
project.getService(WebViewManager::class.java)may return null. Registering a null disposable will throw. You already use a safe call when disposing in the service—mirror that here.Apply:
- val webViewManager = project.getService(WebViewManager::class.java) - Disposer.register(project, webViewManager) + val webViewManager = project.getService(WebViewManager::class.java) + if (webViewManager != null) { + Disposer.register(project, webViewManager) + } else { + LOG.warn("WebViewManager service not found; skipping disposer registration") + }
| private fun registerExtensionProviders() { | ||
| // 创建 Roo Code 扩展提供者实例 | ||
| val rooProvider = com.sina.weibo.agent.extensions.roo.RooExtensionProvider() | ||
| registerExtensionProvider(rooProvider) | ||
|
|
||
| // 创建 Cline AI 扩展提供者实例 | ||
| val clineProvider = com.sina.weibo.agent.extensions.cline.ClineExtensionProvider() | ||
| registerExtensionProvider(clineProvider) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Incorrect package paths in examples (plugin. is missing).*
In the code snippets, providers are referenced as:
- com.sina.weibo.agent.extensions.roo.RooExtensionProvider
- com.sina.weibo.agent.extensions.cline.ClineExtensionProvider
In the codebase they live under:
- com.sina.weibo.agent.extensions.plugin.roo.RooExtensionProvider
- com.sina.weibo.agent.extensions.plugin.cline.ClineExtensionProvider
Please correct the package names to avoid confusion for readers.
Apply this minimal diff in the snippet:
- val rooProvider = com.sina.weibo.agent.extensions.roo.RooExtensionProvider()
+ val rooProvider = com.sina.weibo.agent.extensions.plugin.roo.RooExtensionProvider()
...
- val clineProvider = com.sina.weibo.agent.extensions.cline.ClineExtensionProvider()
+ val clineProvider = com.sina.weibo.agent.extensions.plugin.cline.ClineExtensionProvider()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun registerExtensionProviders() { | |
| // 创建 Roo Code 扩展提供者实例 | |
| val rooProvider = com.sina.weibo.agent.extensions.roo.RooExtensionProvider() | |
| registerExtensionProvider(rooProvider) | |
| // 创建 Cline AI 扩展提供者实例 | |
| val clineProvider = com.sina.weibo.agent.extensions.cline.ClineExtensionProvider() | |
| registerExtensionProvider(clineProvider) | |
| } | |
| ``` | |
| private fun registerExtensionProviders() { | |
| // 创建 Roo Code 扩展提供者实例 | |
| val rooProvider = com.sina.weibo.agent.extensions.plugin.roo.RooExtensionProvider() | |
| registerExtensionProvider(rooProvider) | |
| // 创建 Cline AI 扩展提供者实例 | |
| val clineProvider = com.sina.weibo.agent.extensions.plugin.cline.ClineExtensionProvider() | |
| registerExtensionProvider(clineProvider) | |
| } |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In jetbrains_plugin/Architecture.md around lines 10 to 19, the example
instantiates extension providers using incorrect package paths (missing the
.plugin segment); update the fully-qualified class names to
com.sina.weibo.agent.extensions.plugin.roo.RooExtensionProvider and
com.sina.weibo.agent.extensions.plugin.cline.ClineExtensionProvider in the
snippet so the documented imports match the actual code locations.
| // 优先选择 cline 作为默认提供者 | ||
| val clineProvider = availableProviders.find { it.getExtensionId() == "cline" } | ||
| if (clineProvider != null) { | ||
| currentProvider = clineProvider | ||
| LOG.info("Set default extension provider: roo-code (preferred)") | ||
| } else { | ||
| // 回退到第一个可用提供者 | ||
| currentProvider = availableProviders.first() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Default provider selection doc and log mismatch.
The snippet prefers "cline" but logs say “roo-code (preferred)”. Align the explanation and logs to the actual selection: “cline (preferred)”.
- LOG.info("Set default extension provider: roo-code (preferred)")
+ LOG.info("Set default extension provider: cline (preferred)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 优先选择 cline 作为默认提供者 | |
| val clineProvider = availableProviders.find { it.getExtensionId() == "cline" } | |
| if (clineProvider != null) { | |
| currentProvider = clineProvider | |
| LOG.info("Set default extension provider: roo-code (preferred)") | |
| } else { | |
| // 回退到第一个可用提供者 | |
| currentProvider = availableProviders.first() | |
| } | |
| } | |
| } | |
| // 优先选择 cline 作为默认提供者 | |
| val clineProvider = availableProviders.find { it.getExtensionId() == "cline" } | |
| if (clineProvider != null) { | |
| currentProvider = clineProvider | |
| LOG.info("Set default extension provider: cline (preferred)") | |
| } else { | |
| // 回退到第一个可用提供者 | |
| currentProvider = availableProviders.first() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In jetbrains_plugin/Architecture.md around lines 33 to 43, the code prefers the
"cline" provider but the log message incorrectly states "roo-code (preferred)";
update the log and any explanatory text to reference "cline (preferred)" so the
documentation and runtime log match the actual default selection, ensuring the
message and description consistently say "cline (preferred)" wherever that
selection is mentioned.
| override fun initialize(project: Project) { | ||
| // 获取并初始化 cline 扩展配置 | ||
| val extensionConfig = com.sina.weibo.agent.extensions.roo.ExtensionConfiguration.getInstance(project) | ||
| extensionConfig.initialize() | ||
|
|
||
| // 初始化扩展管理器工厂 | ||
| try { | ||
| val extensionManagerFactory = com.sina.weibo.agent.extensions.roo.ExtensionManagerFactory.getInstance(project) | ||
| extensionManagerFactory.initialize() | ||
| } catch (e: Exception) { | ||
| // 如果 ExtensionManagerFactory 不可用，继续而不使用它 | ||
| // 这允许 cline 独立工作 | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Misleading references to Roo classes within Cline section.
Under “ClineExtensionProvider.initialize() 详细实现”, it references:
- com.sina.weibo.agent.extensions.roo.ExtensionConfiguration
- com.sina.weibo.agent.extensions.roo.ExtensionManagerFactory
These look like Roo-specific packages. Please update to the correct classes used by Cline or generalize the example to the shared config/manager used in the actual code.
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Lines 45-80 of Architecture.md ==="
sed -n '45,80p' jetbrains_plugin/Architecture.md
echo ""
echo "=== Searching for ExtensionConfiguration class definitions ==="
rg -n "class ExtensionConfiguration" -g "*.java" -g "*.kt"
echo ""
echo "=== Listing all com.sina.weibo.agent.extensions packages ==="
rg -n "package com.sina.weibo.agent.extensions" -g "*.java" -g "*.kt"Length of output: 5095
Fix incorrect package references in ClineExtensionProvider.initialize()
The docs currently reference Roo-specific classes under .extensions.roo, but Cline uses the shared config/manager types. Please update the snippet in jetbrains_plugin/Architecture.md (lines 52–66) as follows:
@@ ClineExtensionProvider.initialize() 详细实现
- override fun initialize(project: Project) {
- // 获取并初始化 cline 扩展配置
- val extensionConfig = com.sina.weibo.agent.extensions.roo.ExtensionConfiguration.getInstance(project)
+ override fun initialize(project: Project) {
+ // 获取并初始化 cline 扩展配置
+ val extensionConfig = com.sina.weibo.agent.extensions.config.ExtensionConfiguration.getInstance(project)
extensionConfig.initialize()
// 初始化扩展管理器工厂
try {
- val extensionManagerFactory = com.sina.weibo.agent.extensions.roo.ExtensionManagerFactory.getInstance(project)
+ val extensionManagerFactory = com.sina.weibo.agent.extensions.core.ExtensionManagerFactory.getInstance(project)
extensionManagerFactory.initialize()
} catch (e: Exception) {
// 如果 ExtensionManagerFactory 不可用，继续而不使用它
// 这允许 cline 独立工作
}
}– Update package paths to:
• com.sina.weibo.agent.extensions.config.ExtensionConfiguration
• com.sina.weibo.agent.extensions.core.ExtensionManagerFactory
jetbrains_plugin/Architecture.md:52–66
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| override fun initialize(project: Project) { | |
| // 获取并初始化 cline 扩展配置 | |
| val extensionConfig = com.sina.weibo.agent.extensions.roo.ExtensionConfiguration.getInstance(project) | |
| extensionConfig.initialize() | |
| // 初始化扩展管理器工厂 | |
| try { | |
| val extensionManagerFactory = com.sina.weibo.agent.extensions.roo.ExtensionManagerFactory.getInstance(project) | |
| extensionManagerFactory.initialize() | |
| } catch (e: Exception) { | |
| // 如果 ExtensionManagerFactory 不可用，继续而不使用它 | |
| // 这允许 cline 独立工作 | |
| } | |
| } | |
| ``` | |
| override fun initialize(project: Project) { | |
| // 获取并初始化 cline 扩展配置 | |
| val extensionConfig = com.sina.weibo.agent.extensions.config.ExtensionConfiguration.getInstance(project) | |
| extensionConfig.initialize() | |
| // 初始化扩展管理器工厂 | |
| try { | |
| val extensionManagerFactory = com.sina.weibo.agent.extensions.core.ExtensionManagerFactory.getInstance(project) | |
| extensionManagerFactory.initialize() | |
| } catch (e: Exception) { | |
| // 如果 ExtensionManagerFactory 不可用，继续而不使用它 | |
| // 这允许 cline 独立工作 | |
| } | |
| } |
🤖 Prompt for AI Agents
In jetbrains_plugin/Architecture.md around lines 52 to 66, the code snippet
references Roo-specific packages (.extensions.roo); update the fully-qualified
class references to use the shared config/manager types: replace
com.sina.weibo.agent.extensions.roo.ExtensionConfiguration with
com.sina.weibo.agent.extensions.config.ExtensionConfiguration and replace
com.sina.weibo.agent.extensions.roo.ExtensionManagerFactory with
com.sina.weibo.agent.extensions.core.ExtensionManagerFactory, keeping the rest
of the snippet unchanged and preserving the try/catch behavior.
| fun startInitialize() { | ||
| try { | ||
| logger.info("Starting to initialize plugin environment") | ||
| runBlocking { | ||
| // 获取 ExtHostConfiguration 代理 | ||
| val extHostConfiguration = rpcProtocol.getProxy(ServiceProxyRegistry.ExtHostContext.ExtHostConfiguration) | ||
|
|
||
| // 发送空配置模型 | ||
| logger.info("Sending configuration information to extension process") | ||
| val themeName = if (ThemeManager.getInstance().isDarkThemeForce()) "Default Dark Modern" else "Default Light Modern" | ||
|
|
||
| // 创建空配置模型 | ||
| val emptyMap = mapOf("contents" to emptyMap<String, Any>(), "keys" to emptyList<String>(), "overrides" to emptyList<String>()) | ||
| val emptyConfigModel = mapOf( | ||
| "defaults" to mapOf( | ||
| "contents" to mapOf("workbench" to mapOf("colorTheme" to themeName)), | ||
| "keys" to emptyList<String>(), | ||
| "overrides" to emptyList<String>() | ||
| ), | ||
| "policy" to emptyMap, | ||
| "application" to emptyMap, | ||
| "userLocal" to emptyMap, | ||
| "userRemote" to emptyMap, | ||
| "workspace" to emptyMap, | ||
| "folders" to emptyList<Any>(), | ||
| "configurationScopes" to emptyList<Any>() | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Code sample compiles incorrectly as written (shadowing emptyMap and typos).
- Defining
val emptyMap = ...shadows the standardemptyMap()function and is confusing. - The later fields set
policy/application/userLocaletc. toemptyMap(variable), but the value type differs.
Consider revising the snippet to avoid shadowing and use properly typed emptyMap():
val emptySection = mapOf(
"contents" to emptyMap<String, Any>(),
"keys" to emptyList<String>(),
"overrides" to emptyList<String>()
)
val emptyConfigModel = mapOf(
"defaults" to mapOf(
"contents" to mapOf("workbench" to mapOf("colorTheme" to themeName)),
"keys" to emptyList<String>(),
"overrides" to emptyList<String>()
),
"policy" to emptyMap<String, Any>(),
"application" to emptyMap<String, Any>(),
"userLocal" to emptyMap<String, Any>(),
"userRemote" to emptyMap<String, Any>(),
"workspace" to emptyMap<String, Any>(),
"folders" to emptyList<Any>(),
"configurationScopes" to emptyList<Any>()
)🤖 Prompt for AI Agents
In jetbrains_plugin/Architecture.md around lines 685 to 712, the snippet shadows
the standard emptyMap() by declaring val emptyMap and then assigns variables
like "policy"/"application"/"userLocal" to that incorrectly typed identifier;
replace the custom emptyMap variable with a differently named emptySection (or
remove it), and use properly typed Kotlin emptyMap<String, Any>() (or
emptyList<T>() for lists) for the "policy", "application", "userLocal",
"userRemote", and "workspace" fields so types match and no standard function is
shadowed.
| import com.intellij.openapi.project.Project | ||
| import com.sina.weibo.agent.extensions.ExtensionProvider | ||
| import com.sina.weibo.agent.extensions.ExtensionConfiguration as BaseExtensionConfiguration | ||
|
|
||
| class YourExtensionProvider : ExtensionProvider { | ||
| override fun getExtensionId(): String = "your-extension" | ||
| override fun getDisplayName(): String = "Your Extension" | ||
| override fun getDescription(): String = "Your extension description" |
There was a problem hiding this comment.
Incorrect imports and type names in sample: align with actual packages and API
The code uses ExtensionProvider under extensions.config, and the configuration contract is named ExtensionMetadata in this PR (not ExtensionConfiguration). Fixing these prevents copy-paste build errors.
Apply:
-import com.sina.weibo.agent.extensions.ExtensionProvider
-import com.sina.weibo.agent.extensions.ExtensionConfiguration as BaseExtensionConfiguration
+import com.sina.weibo.agent.extensions.config.ExtensionProvider
+import com.sina.weibo.agent.extensions.config.ExtensionMetadata
@@
-class YourExtensionProvider : ExtensionProvider {
+class YourExtensionProvider : ExtensionProvider {
@@
- override fun getConfiguration(project: Project): BaseExtensionConfiguration {
- return object : BaseExtensionConfiguration {
+ override fun getConfiguration(project: Project): ExtensionMetadata {
+ return object : ExtensionMetadata {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In jetbrains_plugin/EXTENSION_DECOUPLING.md around lines 56 to 63, the sample
imports and type names are incorrect and will cause build errors; change the
import for ExtensionProvider to the actual package path used in this PR
(extensions.config) and replace references to
ExtensionConfiguration/BaseExtensionConfiguration with the correct contract name
ExtensionMetadata, updating import statements and the class signature
accordingly so the sample matches the real API.
| val requestPath = request?.url?.decodeURLPart()?.replace("http://localhost:","")?.substringAfter("/")?.substringBefore("?") | ||
| logger.info("Extracted request path: $requestPath") | ||
|
|
||
| requestPath?.let { | ||
| val filePath = if (requestPath.isEmpty()) { | ||
| "$resourceBasePath/index.html" | ||
| } else { | ||
| "$resourceBasePath/$requestPath" | ||
| } | ||
| logger.info("Constructed file path: $filePath") | ||
|
|
||
| file = File(filePath) | ||
| logger.info("File object created: $file") | ||
|
|
||
| if (file!!.exists() && file!!.isFile) { | ||
| try { | ||
| fileContent = file!!.readBytes() | ||
| logger.info("File content loaded successfully, size: ${fileContent?.size} bytes") | ||
| } catch (e: Exception) { | ||
| logger.warn("cannot get fileContent,e:${e}") | ||
| logger.warn("Cannot get file content, error: ${e}") | ||
| file = null | ||
| fileContent = null | ||
| } | ||
| } else { | ||
| logger.warn("File does not exist or is not a file: exists=${file?.exists()}, isFile=${file?.isFile}") | ||
| file = null | ||
| fileContent = null | ||
| } | ||
| logger.info("init LocalCefResHandle,filePath:${filePath},file:${file},exists:${file?.exists()}") | ||
| logger.info("Final state: file=$file, exists=${file?.exists()}, content size=${fileContent?.size}") | ||
| } | ||
| logger.info("=== LocalCefResHandle INIT END ===") |
There was a problem hiding this comment.
Path traversal risk when mapping request URL to filesystem
requestPath is concatenated into a path without normalization or containment checks. Attackers (or even malformed URLs) could use ../ to escape resourceBasePath.
Apply this hardened resolution and validation:
- val requestPath = request?.url?.decodeURLPart()?.replace("http://localhost:","")?.substringAfter("/")?.substringBefore("?")
- logger.info("Extracted request path: $requestPath")
-
- requestPath?.let {
- val filePath = if (requestPath.isEmpty()) {
- "$resourceBasePath/index.html"
- } else {
- "$resourceBasePath/$requestPath"
- }
- logger.info("Constructed file path: $filePath")
-
- file = File(filePath)
+ val requestPath = request?.url
+ ?.let { runCatching { java.net.URI(it) }.getOrNull() }
+ ?.path
+ ?.removePrefix("/")
+ ?.trim()
+ ?: ""
+ logger.info("Extracted request path: $requestPath")
+
+ // Resolve to canonical paths and prevent directory traversal
+ val baseDir = File(resourceBasePath).canonicalFile
+ val candidate = File(baseDir, if (requestPath.isEmpty()) "index.html" else requestPath).canonicalFile
+
+ if (!candidate.path.startsWith(baseDir.path + File.separator)) {
+ logger.warn("Blocked path traversal attempt: $candidate outside $baseDir")
+ file = null
+ fileContent = null
+ } else {
+ logger.info("Constructed file path: ${candidate.path}")
+ file = candidate
logger.info("File object created: $file")
if (file!!.exists() && file!!.isFile) {
try {
fileContent = file!!.readBytes()
logger.info("File content loaded successfully, size: ${fileContent?.size} bytes")
} catch (e: Exception) {
logger.warn("Cannot get file content, error: ${e}")
file = null
fileContent = null
}
} else {
logger.warn("File does not exist or is not a file: exists=${file?.exists()}, isFile=${file?.isFile}")
file = null
fileContent = null
}
logger.info("Final state: file=$file, exists=${file?.exists()}, content size=${fileContent?.size}")
- }
+ }🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/webview/LocalResHandler.kt
around lines 41 to 71, the code currently concatenates requestPath into a
filesystem path allowing path traversal; normalize and validate the requested
path before accessing the file by: decode the URL safely, reject or canonicalize
any path that contains .. or backslashes, resolve the candidate file as
File(resourceBasePath).resolve(requestPath).canonicalFile and compare that
canonical path startsWith the canonical resourceBasePath (reject if not),
enforce a max path length and only allow expected characters if applicable, and
only then read the file; on validation failure return null/error and log the
sanitized requestPath (do not log full paths or sensitive details).
| <action id="RunVSAgent.extensionSwitcher" | ||
| icon="AllIcons.Actions.Refresh" | ||
| class="com.sina.weibo.agent.extensions.ui.actions.ExtensionSwitcherAction" | ||
| text="Switch Extension Provider" | ||
| description="Switch to a different extension provider"> | ||
| <override-text place="GoToAction" text="Switch Extension" /> | ||
| <keyboard-shortcut keymap="$default" first-keystroke="ctrl alt S" /> | ||
| </action> |
There was a problem hiding this comment.
Default shortcut conflicts with IDE Settings (Ctrl+Alt+S).
JetBrains IDEs commonly use Ctrl+Alt+S for Settings. Binding your action to the same chord will cause a UX conflict.
Recommend removing the default shortcut or choosing a non-conflicting one (e.g., Ctrl+Alt+E or none by default):
- <keyboard-shortcut keymap="$default" first-keystroke="ctrl alt S" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <action id="RunVSAgent.extensionSwitcher" | |
| icon="AllIcons.Actions.Refresh" | |
| class="com.sina.weibo.agent.extensions.ui.actions.ExtensionSwitcherAction" | |
| text="Switch Extension Provider" | |
| description="Switch to a different extension provider"> | |
| <override-text place="GoToAction" text="Switch Extension" /> | |
| <keyboard-shortcut keymap="$default" first-keystroke="ctrl alt S" /> | |
| </action> | |
| <action id="RunVSAgent.extensionSwitcher" | |
| icon="AllIcons.Actions.Refresh" | |
| class="com.sina.weibo.agent.extensions.ui.actions.ExtensionSwitcherAction" | |
| text="Switch Extension Provider" | |
| description="Switch to a different extension provider"> | |
| <override-text place="GoToAction" text="Switch Extension" /> | |
| </action> |
🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/resources/META-INF/plugin.xml around lines 123 to
130, the action RunVSAgent.extensionSwitcher is bound to the IDE Settings
shortcut Ctrl+Alt+S which conflicts with JetBrains defaults; remove the
<keyboard-shortcut ... first-keystroke="ctrl alt S" /> entry or replace it with
a non-conflicting default (for example change to "ctrl alt E" or omit the
keyboard-shortcut element entirely) so the plugin no longer overrides the IDE
Settings hotkey.
| fun testExtensionAvailability() { | ||
| // Test extension availability checking | ||
| val rooProvider = extensionManager.getProvider("roo-code") | ||
|
|
||
| assertNotNull("Roo provider should exist", rooProvider) | ||
|
|
||
| // Test availability (this will depend on whether extension files exist) | ||
| // In test environment, extensions may not be available due to missing files | ||
| // This is expected behavior, so we just verify the method works | ||
| assertNotNull("Roo provider availability check should work", rooProvider?.isAvailable(project)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Availability test assertion is ineffective
isAvailable() returns a primitive boolean; assertNotNull will always pass. Instead, verify that it doesn’t throw and is a boolean result.
Apply this diff:
fun testExtensionAvailability() {
// Test extension availability checking
val rooProvider = extensionManager.getProvider("roo-code")
assertNotNull("Roo provider should exist", rooProvider)
// Test availability (this will depend on whether extension files exist)
// In test environment, extensions may not be available due to missing files
// This is expected behavior, so we just verify the method works
- assertNotNull("Roo provider availability check should work", rooProvider?.isAvailable(project))
+ try {
+ rooProvider!!.isAvailable(project) // just ensure it doesn't throw
+ } catch (e: Exception) {
+ fail("isAvailable should not throw: ${e.message}")
+ }
}🤖 Prompt for AI Agents
In
jetbrains_plugin/src/test/kotlin/com/sina/weibo/agent/extensions/ExtensionDecouplingTest.kt
around lines 66 to 76, the test uses assertNotNull against
rooProvider?.isAvailable(project) which is ineffective because isAvailable()
returns a primitive boolean; replace that assertion with a safe call that
ensures the method doesn't throw and captures its boolean result: use
assertDoesNotThrow to call rooProvider!!.isAvailable(project), assign the result
to a local val (e.g. available), and then assert the result is a boolean by
asserting available == true || available == false (or simply keep the captured
value if you only need to ensure no exception was thrown).
| log_error "JSON configuration file not found: $json_file" | ||
| return 1 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add validation for undefined logging functions.
The script uses logging functions (log_error, log_debug, log_warn) and command_exists without defining them or checking if they're available. This could cause the script to fail if these functions aren't sourced from elsewhere.
🏁 Script executed:
#!/bin/bash
# Check if logging functions are defined elsewhere in the codebase
echo "Searching for log_error function definition:"
rg -n "^[[:space:]]*(function[[:space:]]+)?log_error[[:space:]]*\(\)" --type sh
echo -e "\nSearching for log_debug function definition:"
rg -n "^[[:space:]]*(function[[:space:]]+)?log_debug[[:space:]]*\(\)" --type sh
echo -e "\nSearching for log_warn function definition:"
rg -n "^[[:space:]]*(function[[:space:]]+)?log_warn[[:space:]]*\(\)" --type sh
echo -e "\nSearching for command_exists function definition:"
rg -n "^[[:space:]]*(function[[:space:]]+)?command_exists[[:space:]]*\(\)" --type shLength of output: 913
🏁 Script executed:
#!/bin/bash
# Check for sourcing of common.sh in json_parser.sh
echo "Searching for any reference to common.sh in json_parser.sh:"
rg -n "common\.sh" -C2 scripts/lib/json_parser.sh || echo "No references to common.sh found"
echo -e "\nSearching for source or dot-inclusion statements in json_parser.sh:"
rg -n "^\s*(source|\.)\s" scripts/lib/json_parser.sh || echo "No source/. statements found"
echo -e "\nDisplaying the first 30 lines of json_parser.sh for context:"
sed -n '1,30p' scripts/lib/json_parser.shLength of output: 1649
scripts/lib/json_parser.sh: Ensure logging utilities are available
The JSON parser calls log_error, log_debug, log_warn and command_exists but never defines or sources them. Without those helpers, the script will error out when run in isolation.
• At the top of scripts/lib/json_parser.sh, import your common library before any function definitions, for example:
# Load shared helpers (logging, command_exists, etc.)
source "$(dirname "${BASH_SOURCE[0]}")/common.sh"• Alternatively (or in addition), add a quick sanity check after sourcing to fail early if any helper is still missing:
for fn in log_error log_debug log_warn command_exists; do
if ! declare -f "$fn" >/dev/null; then
echo "ERROR: Required function '$fn' not found. Did you source common.sh?" >&2
return 1
fi
doneThis guarantees that your parser never calls undefined functions and makes its dependencies explicit.
🤖 Prompt for AI Agents
In scripts/lib/json_parser.sh around lines 16 to 17, the script calls logging
and helper functions (log_error, log_debug, log_warn, command_exists) that are
not defined or sourced; source the shared helper library at the top of the file
(e.g. source "$(dirname "${BASH_SOURCE[0]}")/common.sh") before any function
definitions, then add a short sanity check that verifies each required function
is declared (log_error, log_debug, log_warn, command_exists) and fail early with
a clear error if any are missing.
|
|
||
| # Parse individual extension data string | ||
| parse_extension_data() { | ||
| local extension_data="$1" |
There was a problem hiding this comment.
Fix array/string type mismatch in bash parsing.
The variable extension_data is initialized as an array on line 63 but reassigned as a string on line 117. This causes shellcheck warnings and could lead to unexpected behavior.
Remove the unused array initialization and declaration:
local in_extensions=false
local current_extension=""
-local extension_data=()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local extension_data="$1" | |
| local in_extensions=false | |
| local current_extension="" |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 117-117: Variable was used as an array but is now assigned a string.
(SC2178)
🤖 Prompt for AI Agents
In scripts/lib/json_parser.sh around lines 63 and 117, remove the earlier
declaration/initialization that defines extension_data as an array (the unused
array initialization at ~line 63) so that extension_data is consistently a
string; keep the local extension_data="$1" assignment at line 117 (or explicitly
declare local extension_data at its use site) and delete the array-specific
declaration to resolve the type mismatch and shellcheck warnings.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (5)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt (1)
58-68: Don’t swallow exceptions when checking plugin resources; add loggingSame concern previously raised: exceptions are ignored, hurting diagnosability.
Apply this diff in the catch and add a logger:
@@ -import com.intellij.openapi.project.Project +import com.intellij.openapi.project.Project +import com.intellij.openapi.diagnostic.Logger @@ class RooExtensionProvider : ExtensionProvider { + private val LOG = Logger.getInstance(RooExtensionProvider::class.java) @@ } catch (e: Exception) { - // Ignore exceptions when checking plugin resources + LOG.warn("Failed to resolve plugin resource path for ${config.codeDir}", e) }jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (4)
266-279: Mark current provider by ID, not by instance identityInstance equality is brittle (different instances representing the same provider break highlighting). Compare IDs.
Apply this diff:
availableProviders.forEach { provider -> val resourceStatus = checkExtensionResourceStatus(provider, project) val item = ExtensionItem( id = provider.getExtensionId(), displayName = provider.getDisplayName(), description = provider.getDescription(), isAvailable = provider.isAvailable(project), - isCurrent = provider == currentProvider, + isCurrent = currentProvider?.getExtensionId() == provider.getExtensionId(), resourceStatus = resourceStatus ) extensionItems.add(item) }
152-157: Avoid double south button bars; use DialogWrapper’s south panel instead of adding a custom bottom panelDialogWrapper already renders a south button bar. Adding your own panel causes duplicated controls. Return your custom bottom panel via createSouthPanel() and stop adding it in createCenterPanel().
Apply this diff to stop adding the bottom panel here:
panel.add(createTopPanel(), BorderLayout.NORTH) panel.add(listScrollPane, BorderLayout.WEST) panel.add(rightPanel, BorderLayout.CENTER) - panel.add(createBottomPanel(), BorderLayout.SOUTH) return panelAdd this method to let DialogWrapper use your custom south panel (place anywhere in the class body):
override fun createSouthPanel(): JComponent? = createBottomPanel()
294-301: Escape provider text before putting into HTML labeldisplayName/description may contain markup; embed safely to avoid broken rendering or injection.
Apply this diff:
- descriptionLabel.text = "<html><b>${item.displayName}</b><br><br>${item.description}</html>" + val html = com.intellij.openapi.util.text.HtmlBuilder() + .bold(com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(item.displayName)) + .br().br() + .append(com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(item.description)) + .wrapWithHtmlBody() + .toString() + descriptionLabel.text = html
561-569: Persist selection only after a successful switchWriting config before switching can leave config inconsistent if the switch fails. Defer persistence until success.
Apply this diff:
- if (result == Messages.YES) { - // Update configuration - configManager.setCurrentExtensionId(targetExtensionId) - // Note: Auto-switch functionality not implemented yet - // configManager.setAutoSwitchEnabled(autoSwitchCheckBox.isSelected) - - // Perform switch - performExtensionSwitch(targetExtensionId) - } + if (result == Messages.YES) { + // Perform switch (persist config after success) + performExtensionSwitch(targetExtensionId) + }And persist on success (see next comment’s diff in performExtensionSwitch).
🧹 Nitpick comments (21)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/cline/ClineExtensionProvider.kt (1)
35-41: Don't swallow exceptions during factory initialization; add loggingSwallowing exceptions makes diagnosing startup issues hard. Log a warning and keep the current fallback behavior.
Apply this diff within the catch block:
try { val extensionManagerFactory = ExtensionManagerFactory.getInstance(project) extensionManagerFactory.initialize() } catch (e: Exception) { - // If ExtensionManagerFactory is not available, continue without it - // This allows cline to work independently + LOG.warn("Failed to initialize ExtensionManagerFactory; Cline will proceed without it", e) }Additionally, add a logger import and field:
@@ -import com.intellij.openapi.project.Project +import com.intellij.openapi.project.Project +import com.intellij.openapi.diagnostic.Logger @@ class ClineExtensionProvider : ExtensionProvider { + private val LOG = Logger.getInstance(ClineExtensionProvider::class.java)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt (2)
33-36: Be defensive: wrap ExtensionManagerFactory initialization and logCline’s provider tolerates factory initialization failures; Roo should too to avoid startup breaks in edge environments.
Apply this diff (assumes LOG introduced in a nearby change as suggested):
- // Initialize extension manager factory - val extensionManagerFactory = ExtensionManagerFactory.getInstance(project) - extensionManagerFactory.initialize() + // Initialize extension manager factory + try { + val extensionManagerFactory = ExtensionManagerFactory.getInstance(project) + extensionManagerFactory.initialize() + } catch (e: Exception) { + LOG.warn("Failed to initialize ExtensionManagerFactory; Roo will proceed without it", e) + }
75-78: Nit: remove unnecessary semicolonKotlin doesn’t require it; keeping style consistent improves readability.
- val config = extensionConfig.getConfig(ExtensionType.ROO_CODE); + val config = extensionConfig.getConfig(ExtensionType.ROO_CODE)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt (4)
90-98: Provider acquisition and early abort is reasonableGracefully aborting when no provider is available prevents partial initialization. Consider logging available provider IDs (if any) for diagnostics in the global manager—but this is optional.
130-130: Include extension id in the startup log for traceabilityHelps correlate subsequent activation logs and RPC activity.
- LOG.info("ExtensionHostManager started successfully with extension: ${currentExtensionProvider!!.getExtensionId()}") + LOG.info("ExtensionHostManager started successfully with extension: id=$extensionIdentifier provider=${currentExtensionProvider!!.getExtensionId()}")
236-244: Activation logs should include the actual extension id (not only provider id)This improves diagnostics when multiple extensions/providers are in play.
- if (error != null) { - LOG.error("Failed to activate extension: ${currentProvider.getExtensionId()}", error) - } else { - LOG.info("Extension activated successfully: ${currentProvider.getExtensionId()}") - } + if (error != null) { + LOG.error("Failed to activate extension: id=$extensionId provider=${currentProvider.getExtensionId()}", error) + } else { + LOG.info("Extension activated successfully: id=$extensionId provider=${currentProvider.getExtensionId()}") + }
102-118: Provider-scoped storage directories in initData (global/workspace) should align with the active providerEven after adding multi-provider path resolution, initData still hard-codes storage roots under “.roo-cline”. For multiple providers, use provider-scoped directories to avoid collisions and to keep data isolated. This change occurs outside the edited lines here, but is key to the PR’s objective.
Example adjustment (outside this hunk), sketching the idea:
// helper private fun providerStorageHome(subdir: String): String { val providerId = currentExtensionProvider?.getExtensionId() ?: "default" return Paths.get(System.getProperty("user.home"), ".run-vs-agent", providerId, subdir).toString() } // in createInitData() environment map "globalStorageHome" to uriFromPath(providerStorageHome("globalStorage")), "workspaceStorageHome" to uriFromPath(providerStorageHome("workspaceStorage")),If you prefer a different base (or to keep backward compatibility), we can add a legacy fallback strategy similar to getExtensionPath().
Would you like me to submit a follow-up patch adjusting createInitData accordingly?Also applies to: 257-318
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (5)
114-132: Tooltips: use HTML instead of newlines and escape dynamic contentSwing tooltips ignore newlines. Build an HTML tooltip and escape all dynamic values (name, paths).
Apply this diff:
- val tooltipText = buildString { - append("Extension: ${value.displayName}\n") - append("Status: ${value.resourceStatus.statusText}\n") - if (value.resourceStatus.projectResourceExists) { - append("Local Path: ${value.resourceStatus.projectResourcePath}\n") - } - if (value.resourceStatus.vsixResourceExists) { - append("VSIX Path: ${value.resourceStatus.vsixResourcePath}\n") - } - if (value.resourceStatus.pluginResourceExists) { - append("Plugin Path: ${value.resourceStatus.pluginResourcePath}\n") - } - if (!value.resourceStatus.projectResourceExists && !value.resourceStatus.vsixResourceExists && !value.resourceStatus.pluginResourceExists) { - append("⚠️ Warning: No resource files found!") - } - } - label.toolTipText = tooltipText + val esc = com.intellij.openapi.util.text.StringUtil::escapeXmlEntities + val rs = value.resourceStatus + val tooltipHtml = buildString { + append("<html>") + append("<b>Extension:</b> ${esc(value.displayName)}<br>") + append("<b>Status:</b> ${esc(rs.statusText)}<br>") + if (rs.projectResourceExists) { + append("<b>Local Path:</b> ${esc(rs.projectResourcePath ?: "")}<br>") + } + if (rs.vsixResourceExists) { + append("<b>VSIX Path:</b> ${esc(rs.vsixResourcePath ?: "")}<br>") + } + if (rs.pluginResourceExists) { + append("<b>Plugin Path:</b> ${esc(rs.pluginResourcePath ?: "")}<br>") + } + if (!rs.projectResourceExists && !rs.vsixResourceExists && !rs.pluginResourceExists) { + append("<font color='red'>⚠️ Warning: No resource files found!</font>") + } + append("</html>") + } + label.toolTipText = tooltipHtml
421-438: Escape file paths and text in resource status HTMLPaths/status texts are injected into HTML without escaping; use escapeXmlEntities to avoid broken rendering for unusual paths.
Apply this diff:
- val statusHtml = buildString { + val statusHtml = buildString { append("<html><b>Resource Status:</b><br>") - append("${resourceStatus.statusIcon} ${resourceStatus.statusText}<br><br>") + append("${resourceStatus.statusIcon} ${com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(resourceStatus.statusText)}<br><br>") if (resourceStatus.projectResourceExists) { append("<b>Local Path:</b><br>") - append("<code>${resourceStatus.projectResourcePath}</code><br><br>") + val p = resourceStatus.projectResourcePath ?: "" + append("<code>${com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(p)}</code><br><br>") } if (resourceStatus.vsixResourceExists) { append("<b>VSIX Installation:</b><br>") - append("<code>${resourceStatus.vsixResourcePath}</code><br><br>") + val p = resourceStatus.vsixResourcePath ?: "" + append("<code>${com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(p)}</code><br><br>") } if (resourceStatus.pluginResourceExists) { append("<b>Plugin Path:</b><br>") - append("<code>${resourceStatus.pluginResourcePath}</code><br>") + val p = resourceStatus.pluginResourcePath ?: "" + append("<code>${com.intellij.openapi.util.text.StringUtil.escapeXmlEntities(p)}</code><br>") }
409-411: Don’t swallow exceptions silently; log at least at debugStatic analysis flagged this (SwallowedException). Add a logger entry to aid diagnostics when resource probing fails.
Apply this diff:
- } catch (e: Exception) { - // Ignore exceptions when checking plugin resources - } + } catch (e: Exception) { + LOG.debug("Error while checking plugin resources for ${extensionConfig.getCodeDir()}", e) + }Add a logger field and import:
@@ -import javax.swing.* +import javax.swing.* +import com.intellij.openapi.diagnostic.Logger @@ class ExtensionSwitcherDialog(private val project: Project) : DialogWrapper(project) { - + private val LOG = Logger.getInstance(ExtensionSwitcherDialog::class.java)
489-501: Button copy is misleading; it doesn’t actually auto-upload VSIX“Switch & Upload VSIX” suggests an automatic upload that isn’t implemented. Use neutral wording to avoid confusion.
Apply this diff:
- } else { - switchButton.text = "Switch & Upload VSIX" - switchButton.toolTipText = "Switch to ${selectedItem.displayName} and upload VSIX file for resources" - } + } else { + switchButton.text = "Switch Extension" + switchButton.toolTipText = "Switch to ${selectedItem.displayName}. You may need to upload a VSIX afterward." + }
511-520: Allow cancel during switching to call cancelSwitching()Currently cancel is disabled while switching, but doCancelAction supports cancellation. Keep Cancel enabled to let the user abort.
Apply this diff:
- if (extensionSwitcher.isSwitching()) { - switchButton.text = "Switching..." - switchButton.isEnabled = false - cancelButton.isEnabled = false - } else { + if (extensionSwitcher.isSwitching()) { + switchButton.text = "Switching..." + switchButton.isEnabled = false + cancelButton.isEnabled = true + } else { switchButton.text = "Switch Extension" updateSwitchButton() cancelButton.isEnabled = true }Also applies to: 609-626
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionSwitcherAction.kt (2)
39-60: Visibility vs. messaging: hiding makes the “No Extensions Available” text mootYou set isEnabledAndVisible = false and then set presentation.text. If hidden, the text won’t be seen. Prefer to keep visible but disabled with a clear label.
Apply this diff:
- if (availableProviders.isEmpty()) { - presentation.isEnabledAndVisible = false - presentation.text = "No Extensions Available" - return - } + if (availableProviders.isEmpty()) { + presentation.isVisible = true + presentation.isEnabled = false + presentation.text = "No Extensions Available" + presentation.description = "No extension providers are currently available" + return + }
19-19: Consider extending DumbAwareAction to work during indexingUsing DumbAwareAction allows the action to remain available when the IDE is in dumb mode (indexing).
Apply this diff:
-package com.sina.weibo.agent.extensions.ui.actions +package com.sina.weibo.agent.extensions.ui.actions @@ -import com.intellij.openapi.actionSystem.AnAction +import com.intellij.openapi.actionSystem.AnAction +import com.intellij.openapi.project.DumbAware @@ -class ExtensionSwitcherAction : AnAction() { +class ExtensionSwitcherAction : AnAction(), DumbAware {jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt (5)
23-38: Make getInstance a true singleton and build base path portablygetInstance() returns a new VsixManager each call. Also, building paths via string concatenation is non-portable. Use a lazy singleton and Paths.get, and ensure the base directory exists.
Apply this diff:
companion object { - private val LOG = Logger.getInstance(VsixManager::class.java) - private const val BASE_DIR = ".run-vs-agent" - - /** - * Get VSIX manager instance - */ - fun getInstance(): VsixManager = VsixManager() - - /** - * Get base directory for VSIX installations - */ - fun getBaseDirectory(): String { - val homeDir = System.getProperty("user.home") - return "$homeDir/$BASE_DIR" - } + private val LOG = Logger.getInstance(VsixManager::class.java) + private const val BASE_DIR = ".run-vs-agent" + private val INSTANCE: VsixManager by lazy { VsixManager() } + + /** + * Get VSIX manager instance + */ + fun getInstance(): VsixManager = INSTANCE + + /** + * Get base directory for VSIX installations + */ + fun getBaseDirectory(): String { + val homeDir = System.getProperty("user.home") + val basePath = Paths.get(homeDir, BASE_DIR) + try { + Files.createDirectories(basePath) + } catch (_: Exception) { + // best-effort; downstream calls also create as needed + } + return basePath.toString() + } }
291-295: Theme copy may fail on existing files; overwrite explicitlyFiles.copy without REPLACE_EXISTING throws if a file already exists. This can cause the entire copy loop to abort and log a warning. Overwrite to keep themes up to date.
Apply this diff:
- val targetFile = integrationsThemeDir.resolve(themeFile.name).toFile() - Files.copy(themeFile.toPath(), targetFile.toPath()) - copiedCount++ - LOG.debug("Copied theme file: ${themeFile.name} to integrations directory") + val targetPath = integrationsThemeDir.resolve(themeFile.name) + // Overwrite to ensure themes are current + Files.copy(themeFile.toPath(), targetPath, java.nio.file.StandardCopyOption.REPLACE_EXISTING) + copiedCount++ + LOG.debug("Copied theme file: ${themeFile.name} to integrations directory")
245-259: Improve robustness of moveDirectory across filesystemsFiles.move can fail across filesystems; add REPLACE_EXISTING and a copy+delete fallback.
Apply this diff:
if (file.isDirectory) { moveDirectory(file, targetFile) } else { - Files.move(file.toPath(), targetFile.toPath()) + try { + Files.move( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + } catch (e: Exception) { + Files.copy( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + file.delete() + } }
377-386: Prefer codeDir under VSIX root when presentCallers likely expect a path to codeDir. Check for vsixPath/codeDir first, then fall back to the VSIX root.
Apply this diff:
- val vsixPath = getVsixInstallationPath(extensionId) - if (vsixPath != null) { - // Since we now extract extension contents directly to the target directory, - // we don't need to check for an 'extension' subdirectory - if (File(vsixPath).exists()) { - LOG.debug("Found extension resources in VSIX installation: $vsixPath") - return vsixPath - } - } + val vsixPath = getVsixInstallationPath(extensionId) + if (vsixPath != null) { + // Prefer codeDir relative to VSIX root; else fall back to root. + val candidate = Paths.get(vsixPath, codeDir).toFile() + if (candidate.exists()) { + LOG.debug("Found extension resources in VSIX installation: ${candidate.path}") + return candidate.path + } + if (File(vsixPath).exists()) { + LOG.debug("Found extension resources in VSIX installation root: $vsixPath") + return vsixPath + } + }Please confirm how getExtensionResourcePath is consumed. If callers join codeDir themselves for VSIX, this change may be redundant.
343-350: Use recursive walk for deletes to reduce failuresThe current deleteDirectory ignores failures and may leave residual files. Walking the tree in reverse order is more reliable.
Apply this diff:
- private fun deleteDirectory(dir: File) { - if (dir.isDirectory) { - dir.listFiles()?.forEach { file -> - deleteDirectory(file) - } - } - dir.delete() - } + private fun deleteDirectory(dir: File) { + val root = dir.toPath() + if (!Files.exists(root)) return + Files.walk(root) + .sorted(java.util.Comparator.reverseOrder()) + .forEach { path -> + try { + Files.deleteIfExists(path) + } catch (_: Exception) { + // best-effort cleanup + } + } + }jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/VsixUploadDialog.kt (2)
40-43: Derive target directory from VsixManager to avoid path driftAvoid duplicating base-dir logic. Use the manager’s helper to keep paths consistent.
Apply this diff:
- private fun setupTargetDirectory() { - val homeDir = System.getProperty("user.home") - targetDirectory = "$homeDir/.run-vs-agent/$extensionId" - } + private fun setupTargetDirectory() { + targetDirectory = VsixManager.getInstance().getExtensionDirectory(extensionId) + }
73-79: Avoid hard-coded colors; use JBColor for theme-aware renderingRaw AWT colors don’t adapt to IDE themes. Prefer JBColor or named colors.
Apply this diff:
- statusLabel.foreground = if (hasExistingInstallation) { - java.awt.Color(255, 140, 0) // Orange for warning - } else { - java.awt.Color(0, 128, 0) // Green for success - } + statusLabel.foreground = if (hasExistingInstallation) { + com.intellij.ui.JBColor(java.awt.Color(255, 140, 0), java.awt.Color(255, 140, 0)) + } else { + com.intellij.ui.JBColor(java.awt.Color(0, 128, 0), java.awt.Color(0, 128, 0)) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
jetbrains_plugin/build.gradle.kts(4 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt(7 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/cline/ClineExtensionProvider.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/VsixUploadDialog.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionResourceStatusAction.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionSwitcherAction.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionResourceStatusAction.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- jetbrains_plugin/build.gradle.kts
🧰 Additional context used
🧬 Code Graph Analysis (2)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (2)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSelector.kt (1)
loadExtensions(35-45)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionSwitcher.kt (1)
performExtensionSwitch(143-174)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt (1)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt (1)
dispose(203-208)
🪛 detekt (1.23.8)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt
[warning] 66-66: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt
[warning] 409-409: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (10)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/cline/ClineExtensionProvider.kt (2)
44-48: Always-true availability: verify UX and action gatingReturning true unconditionally may surface Cline UI/actions when no VSIX is installed yet. If intentional, fine; otherwise consider gating by dev/test mode or a setting.
Would you like me to gate this with ApplicationManager.isInternal/isUnitTestMode or a plugin setting?
50-64: Configuration metadata mapping looks goodClean delegation to ExtensionConfiguration for CLINE. No issues spotted.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt (1)
70-73: Ensure consistentisAvailablefallback behaviorWe’ve identified that several extension providers always return
trueinisAvailable, deferring actual resource checks to later stages. Please confirm whether this “always-true” pattern is intentional for production or strictly for development/testing:• jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooExtensionProvider.kt (lines 38–45)
• jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/roo/RooCodeButtonProvider.kt (lines 29–32)
• jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/cline/ClineExtensionProvider.kt (lines 44–47)
• jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/plugin/cline/ClineButtonProvider.kt (lines 31–34)If this fallback is only for dev/test, consider gating or feature-flagging it consistently across all providers; otherwise, document this design decision in the extension guidelines.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt (4)
30-33: Alias import to avoid name clash looks goodUsing an alias for the global manager keeps local ExtensionManager unambiguous.
57-62: State tracked for current provider and extension id is appropriateThese fields enable accurate activation/logging flows. No issues.
224-224: LGTM: explicit IllegalState for provider is good defensive programmingThis ensures we fail fast if the provider state changes unexpectedly.
227-227: LGTM: RPCManager now has access to the ExtensionManagerThis aligns with the activation flow and extension descriptions used in initData.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (1)
170-177: Nice touch: legend explaining status iconsThe legend clarifies the visual status in the list and reduces confusion.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionSwitcherAction.kt (2)
21-37: LGTM: Guard against concurrent switches and show dialog otherwiseGood UX to avoid re-entrancy by checking isSwitching() and informing the user.
65-79: Adaptive presentation text/description looks goodNice differentiation by place (toolbar/menu) and handling of switching state with “Switching...” and disabled presentation.
Also applies to: 81-90
| // Get extension configuration | ||
| val extensionConfig = currentExtensionProvider!!.getConfiguration(project) | ||
|
|
||
| // Get extension path from configuration | ||
| val extensionPath = getExtensionPath(extensionConfig) | ||
|
|
||
| if (extensionPath != null && File(extensionPath).exists()) { | ||
| // Register extension using configuration | ||
| val extensionDesc = extensionManager!!.registerExtension(extensionPath, extensionConfig) | ||
| extensionIdentifier = extensionDesc.identifier.value | ||
| LOG.info("Registered extension: ${currentExtensionProvider!!.getExtensionId()}") | ||
| } else { | ||
| LOG.error("Extension path not found: $extensionPath") | ||
| dispose() | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make extension path resolution truly provider-driven; avoid double existence checks; improve registration logging
- Path resolution currently ignores provider-specific install locations and hard-codes “.run-vs-agent”. This undermines multi-provider support.
- getExtensionPath should return null when not found (not a default, non-existent path), and the call site should not repeat File.exists().
- Registration log should include both extensionId and providerId (and the resolved path) for easier diagnostics.
Apply this diff:
@@
- // Get extension configuration
- val extensionConfig = currentExtensionProvider!!.getConfiguration(project)
-
- // Get extension path from configuration
- val extensionPath = getExtensionPath(extensionConfig)
-
- if (extensionPath != null && File(extensionPath).exists()) {
- // Register extension using configuration
- val extensionDesc = extensionManager!!.registerExtension(extensionPath, extensionConfig)
- extensionIdentifier = extensionDesc.identifier.value
- LOG.info("Registered extension: ${currentExtensionProvider!!.getExtensionId()}")
- } else {
+ // Get extension configuration
+ val provider = currentExtensionProvider!!
+ val extensionConfig = provider.getConfiguration(project)
+
+ // Resolve extension path via provider-aware logic
+ val extensionPath = getExtensionPath(extensionConfig, provider)
+
+ if (extensionPath != null) {
+ // Register extension using configuration
+ val extensionDesc = extensionManager!!.registerExtension(extensionPath, extensionConfig)
+ extensionIdentifier = extensionDesc.identifier.value
+ LOG.info("Registered extension: id=$extensionIdentifier provider=${provider.getExtensionId()} path=$extensionPath")
+ } else {
LOG.error("Extension path not found: $extensionPath")
dispose()
return
}Follow-up (outside this hunk): see the getExtensionPath refactor below to accept the provider argument and resolve candidates robustly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt
around lines 102 to 118, make extension path resolution provider-driven and
avoid duplicate existence checks: call the refactored
getExtensionPath(currentExtensionProvider, extensionConfig) which returns null
when no valid candidate exists (do not substitute a default non-existent path),
remove the extra File(extensionPath).exists() check at the call site, and when
registration succeeds or fails log the provider id, resolved extension path and
extension id (e.g., include currentExtensionProvider.getProviderId(),
extensionPath, and extensionDesc.identifier.value) for diagnostics; if
getExtensionPath returns null, log the provider id and attempted candidates,
call dispose() and return. Ensure you do not hard-code “.run-vs-agent” here and
instead let the provider-driven function compute candidates (pass provider as
argument per the follow-up refactor).
| try { | ||
| // Build initialization data | ||
| val initData = createInitData() | ||
| LOG.info("handleReadyMessage createInitData: ${initData}") |
There was a problem hiding this comment.
Avoid logging the entire initData at INFO — potential PII leakage
initData includes absolute paths (user home), environment details, etc. Logging the whole object at INFO is risky and noisy. Reduce to DEBUG and log only non-sensitive metadata.
- LOG.info("handleReadyMessage createInitData: ${initData}")
+ LOG.debug("Init data prepared: keys=${initData.keys}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LOG.info("handleReadyMessage createInitData: ${initData}") | |
| LOG.debug("Init data prepared: keys=${initData.keys}") |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt
around line 202, the code logs the entire initData at INFO which can expose PII
(absolute paths, env). Change the log to DEBUG and replace the full object with
a small, explicit safe subset (e.g., agent version, platform/os name,
non-sensitive feature flags) or a masked summary; do not print home directories,
full env maps or absolute paths. Ensure the message reads clearly (e.g.,
"handleReadyMessage init summary: <safeFields>") and remove or redact any
sensitive fields before logging.
| /** | ||
| * Get extension path from configuration | ||
| */ | ||
| private fun getExtensionPath(extensionConfig: ExtensionMetadata): String? { | ||
| // First check project paths | ||
| val projectPath = project.basePath | ||
| val homeDir = System.getProperty("user.home") | ||
| if (projectPath != null) { | ||
| val possiblePaths = listOf( | ||
| "$homeDir/.run-vs-agent/${extensionConfig.getCodeDir()}" | ||
| ) | ||
|
|
||
| val foundPath = possiblePaths.find { File(it).exists() } | ||
| if (foundPath != null) { | ||
| return foundPath | ||
| } | ||
| } | ||
|
|
||
| // Then check plugin resources (for built-in extensions) | ||
| try { | ||
| val pluginResourcePath = com.sina.weibo.agent.util.PluginResourceUtil.getResourcePath( | ||
| com.sina.weibo.agent.util.PluginConstants.PLUGIN_ID, | ||
| extensionConfig.getCodeDir() | ||
| ) | ||
| if (pluginResourcePath != null && File(pluginResourcePath).exists()) { | ||
| return pluginResourcePath | ||
| } | ||
| } catch (e: Exception) { | ||
| LOG.warn("Failed to get plugin resource path for extension: ${extensionConfig.getCodeDir()}", e) | ||
| } | ||
|
|
||
| // For development/testing, return a default path | ||
| // This allows the extension to work even without the actual extension files | ||
| val defaultPath = projectPath?.let { "$it/${extensionConfig.getCodeDir()}" } ?: "/tmp/${extensionConfig.getCodeDir()}" | ||
| LOG.info("Using default extension path: $defaultPath") | ||
| return defaultPath | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor getExtensionPath to be provider-aware, robust, and non-misleading
Issues:
- Currently gated by project.basePath even though candidate user-level paths do not depend on it.
- Hard-coded .run-vs-agent path without provider namespace risks collisions and is not aligned with a provider-driven architecture.
- Uses fully qualified names for PluginResourceUtil/PluginConstants despite imports.
- Returns a default path that likely doesn’t exist, causing double-logging and confusion.
Refactor:
- Accept the provider, derive a list of candidates including provider-scoped user dir, legacy user dir, plugin resource path, and dev path under project root.
- Return the first existing path or null, and log clearly.
- /**
- * Get extension path from configuration
- */
- private fun getExtensionPath(extensionConfig: ExtensionMetadata): String? {
- // First check project paths
- val projectPath = project.basePath
- val homeDir = System.getProperty("user.home")
- if (projectPath != null) {
- val possiblePaths = listOf(
- "$homeDir/.run-vs-agent/${extensionConfig.getCodeDir()}"
- )
-
- val foundPath = possiblePaths.find { File(it).exists() }
- if (foundPath != null) {
- return foundPath
- }
- }
-
- // Then check plugin resources (for built-in extensions)
- try {
- val pluginResourcePath = com.sina.weibo.agent.util.PluginResourceUtil.getResourcePath(
- com.sina.weibo.agent.util.PluginConstants.PLUGIN_ID,
- extensionConfig.getCodeDir()
- )
- if (pluginResourcePath != null && File(pluginResourcePath).exists()) {
- return pluginResourcePath
- }
- } catch (e: Exception) {
- LOG.warn("Failed to get plugin resource path for extension: ${extensionConfig.getCodeDir()}", e)
- }
-
- // For development/testing, return a default path
- // This allows the extension to work even without the actual extension files
- val defaultPath = projectPath?.let { "$it/${extensionConfig.getCodeDir()}" } ?: "/tmp/${extensionConfig.getCodeDir()}"
- LOG.info("Using default extension path: $defaultPath")
- return defaultPath
- }
+ /**
+ * Resolve the extension code directory using provider-aware candidates.
+ * Returns the first existing path, or null if none exist.
+ */
+ private fun getExtensionPath(extensionConfig: ExtensionMetadata, provider: ExtensionProvider): String? {
+ val providerId = provider.getExtensionId()
+ val homeDir = System.getProperty("user.home")
+ val projectRoot = project.basePath
+
+ val candidates = buildList {
+ // Provider-scoped user installation dir
+ add(Paths.get(homeDir, ".run-vs-agent", providerId, extensionConfig.codeDir).toString())
+ // Legacy location for backward compatibility
+ add(Paths.get(homeDir, ".run-vs-agent", extensionConfig.codeDir).toString())
+ // Plugin-embedded resources (built-in extensions)
+ try {
+ PluginResourceUtil.getResourcePath(PluginConstants.PLUGIN_ID, extensionConfig.codeDir)?.let { add(it) }
+ } catch (e: Exception) {
+ LOG.warn("Failed to resolve plugin resource path for codeDir='${extensionConfig.codeDir}'", e)
+ }
+ // Dev fallback under project root
+ if (projectRoot != null) {
+ add(Paths.get(projectRoot, extensionConfig.codeDir).toString())
+ }
+ }
+
+ val found = candidates.firstOrNull { it.isNotBlank() && File(it).exists() }
+ return if (found != null) {
+ LOG.info("Resolved extension path: $found (provider=$providerId)")
+ found
+ } else {
+ LOG.error("Extension path not found for codeDir='${extensionConfig.codeDir}', provider='$providerId'. Candidates tried: ${candidates.joinToString()}")
+ null
+ }
+ }Note:
- Switched to property access
extensionConfig.codeDirfor Kotlin idioms. - Removed the misleading default non-existent path and duplicate existence checks.
- Removed fully qualified names for PluginResourceUtil/PluginConstants, using the existing imports instead.
Optional follow-up (outside this hunk): consider centralizing the base folder (".run-vs-agent") into a constant (e.g., PluginConstants) to avoid hard-coded strings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Get extension path from configuration | |
| */ | |
| private fun getExtensionPath(extensionConfig: ExtensionMetadata): String? { | |
| // First check project paths | |
| val projectPath = project.basePath | |
| val homeDir = System.getProperty("user.home") | |
| if (projectPath != null) { | |
| val possiblePaths = listOf( | |
| "$homeDir/.run-vs-agent/${extensionConfig.getCodeDir()}" | |
| ) | |
| val foundPath = possiblePaths.find { File(it).exists() } | |
| if (foundPath != null) { | |
| return foundPath | |
| } | |
| } | |
| // Then check plugin resources (for built-in extensions) | |
| try { | |
| val pluginResourcePath = com.sina.weibo.agent.util.PluginResourceUtil.getResourcePath( | |
| com.sina.weibo.agent.util.PluginConstants.PLUGIN_ID, | |
| extensionConfig.getCodeDir() | |
| ) | |
| if (pluginResourcePath != null && File(pluginResourcePath).exists()) { | |
| return pluginResourcePath | |
| } | |
| } catch (e: Exception) { | |
| LOG.warn("Failed to get plugin resource path for extension: ${extensionConfig.getCodeDir()}", e) | |
| } | |
| // For development/testing, return a default path | |
| // This allows the extension to work even without the actual extension files | |
| val defaultPath = projectPath?.let { "$it/${extensionConfig.getCodeDir()}" } ?: "/tmp/${extensionConfig.getCodeDir()}" | |
| LOG.info("Using default extension path: $defaultPath") | |
| return defaultPath | |
| } | |
| /** | |
| * Resolve the extension code directory using provider-aware candidates. | |
| * Returns the first existing path, or null if none exist. | |
| */ | |
| private fun getExtensionPath(extensionConfig: ExtensionMetadata, provider: ExtensionProvider): String? { | |
| val providerId = provider.getExtensionId() | |
| val homeDir = System.getProperty("user.home") | |
| val projectRoot = project.basePath | |
| val candidates = buildList { | |
| // Provider-scoped user installation dir | |
| add(Paths.get(homeDir, ".run-vs-agent", providerId, extensionConfig.codeDir).toString()) | |
| // Legacy location for backward compatibility | |
| add(Paths.get(homeDir, ".run-vs-agent", extensionConfig.codeDir).toString()) | |
| // Plugin-embedded resources (built-in extensions) | |
| try { | |
| PluginResourceUtil | |
| .getResourcePath(PluginConstants.PLUGIN_ID, extensionConfig.codeDir) | |
| ?.let { add(it) } | |
| } catch (e: Exception) { | |
| LOG.warn("Failed to resolve plugin resource path for codeDir='${extensionConfig.codeDir}'", e) | |
| } | |
| // Dev fallback under project root | |
| if (projectRoot != null) { | |
| add(Paths.get(projectRoot, extensionConfig.codeDir).toString()) | |
| } | |
| } | |
| val found = candidates.firstOrNull { it.isNotBlank() && File(it).exists() } | |
| return if (found != null) { | |
| LOG.info("Resolved extension path: $found (provider=$providerId)") | |
| found | |
| } else { | |
| LOG.error( | |
| "Extension path not found for codeDir='${extensionConfig.codeDir}', " + | |
| "provider='$providerId'. Candidates tried: ${candidates.joinToString()}" | |
| ) | |
| null | |
| } | |
| } |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt
around lines 377 to 414, refactor getExtensionPath to be provider-aware and
non-misleading: change the signature to accept a provider parameter, use
property access extensionConfig.codeDir, and build a candidate list in order —
provider-scoped user dir (e.g.
"$userHome/.run-vs-agent/$provider/${extensionConfig.codeDir}"), legacy user dir
("$userHome/.run-vs-agent/${extensionConfig.codeDir}"), plugin resource path via
PluginResourceUtil.getResourcePath(PluginConstants.PLUGIN_ID,
extensionConfig.codeDir) (use imports, not fully-qualified names), and finally a
dev path under project root ("$projectBase/${extensionConfig.codeDir}") — then
return the first path that exists or null (do not return a fabricated default
path), remove duplicate existence checks and the misleading default-path log,
and replace it with a clear debug/info log only when a path is found or a single
warn when none are found.
| ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis -> | ||
| var entry = zis.nextEntry | ||
| var extractedCount = 0 | ||
|
|
||
| while (entry != null) { | ||
| if (!entry.isDirectory) { | ||
| val entryPath = tempDir!!.toPath().resolve(entry.name) | ||
|
|
||
| // Create parent directories if needed | ||
| val parentDir = entryPath.parent | ||
| if (parentDir != null) { | ||
| Files.createDirectories(parentDir) | ||
| } | ||
|
|
||
| // Extract file | ||
| Files.copy(zis, entryPath) | ||
| extractedCount++ | ||
| } | ||
| entry = zis.nextEntry | ||
| } |
There was a problem hiding this comment.
Fix Zip Slip vulnerability during VSIX extraction
Zip entries are written to paths derived from entry.name without validating that they stay within the temp directory. A crafted VSIX could escape the extraction root and overwrite arbitrary files. Normalize and bound-check the resolved path; also handle directory entries explicitly.
Apply this diff to harden extraction:
- ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis ->
- var entry = zis.nextEntry
- var extractedCount = 0
-
- while (entry != null) {
- if (!entry.isDirectory) {
- val entryPath = tempDir!!.toPath().resolve(entry.name)
-
- // Create parent directories if needed
- val parentDir = entryPath.parent
- if (parentDir != null) {
- Files.createDirectories(parentDir)
- }
-
- // Extract file
- Files.copy(zis, entryPath)
- extractedCount++
- }
- entry = zis.nextEntry
- }
-
- LOG.debug("Extracted $extractedCount files to temporary directory")
- }
+ ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis ->
+ val tempPath = tempDir!!.toPath()
+ var entry = zis.nextEntry
+ var extractedCount = 0
+ while (entry != null) {
+ val resolvedPath = tempPath.resolve(entry.name).normalize()
+ // Prevent Zip Slip
+ if (!resolvedPath.startsWith(tempPath)) {
+ LOG.warn("Skipping suspicious zip entry outside temp dir: ${entry.name}")
+ entry = zis.nextEntry
+ continue
+ }
+ if (entry.isDirectory) {
+ Files.createDirectories(resolvedPath)
+ } else {
+ val parentDir = resolvedPath.parent
+ if (parentDir != null) {
+ Files.createDirectories(parentDir)
+ }
+ Files.copy(zis, resolvedPath)
+ extractedCount++
+ }
+ entry = zis.nextEntry
+ }
+ LOG.debug("Extracted $extractedCount files to temporary directory")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis -> | |
| var entry = zis.nextEntry | |
| var extractedCount = 0 | |
| while (entry != null) { | |
| if (!entry.isDirectory) { | |
| val entryPath = tempDir!!.toPath().resolve(entry.name) | |
| // Create parent directories if needed | |
| val parentDir = entryPath.parent | |
| if (parentDir != null) { | |
| Files.createDirectories(parentDir) | |
| } | |
| // Extract file | |
| Files.copy(zis, entryPath) | |
| extractedCount++ | |
| } | |
| entry = zis.nextEntry | |
| } | |
| ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis -> | |
| val tempPath = tempDir!!.toPath() | |
| var entry = zis.nextEntry | |
| var extractedCount = 0 | |
| while (entry != null) { | |
| val resolvedPath = tempPath.resolve(entry.name).normalize() | |
| // Prevent Zip Slip | |
| if (!resolvedPath.startsWith(tempPath)) { | |
| LOG.warn("Skipping suspicious zip entry outside temp dir: ${entry.name}") | |
| entry = zis.nextEntry | |
| continue | |
| } | |
| if (entry.isDirectory) { | |
| Files.createDirectories(resolvedPath) | |
| } else { | |
| val parentDir = resolvedPath.parent | |
| if (parentDir != null) { | |
| Files.createDirectories(parentDir) | |
| } | |
| Files.copy(zis, resolvedPath) | |
| extractedCount++ | |
| } | |
| entry = zis.nextEntry | |
| } | |
| LOG.debug("Extracted $extractedCount files to temporary directory") | |
| } |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt
around lines 154 to 173, the extraction loop writes ZipEntry names directly into
tempDir allowing Zip Slip; normalize and validate each resolved path before
writing: resolve entry.name against tempDir.toPath(), call .normalize(), then
check that the normalized path startsWith(tempDir.toPath()) (or isUnder) and
throw/skip if not; handle directory entries explicitly by creating directories
for entries with isDirectory or when entry name ends with separator, and only
call Files.copy for regular files after validation; ensure parent directories
are created and permissions/attributes are preserved as needed.
| package com.sina.weibo.agent.extensions.ui.actions | ||
|
|
||
| import com.intellij.openapi.actionSystem.AnAction | ||
| import com.intellij.openapi.actionSystem.AnActionEvent | ||
| import com.intellij.openapi.ui.Messages | ||
| import com.intellij.openapi.actionSystem.ActionPlaces | ||
| import com.sina.weibo.agent.extensions.core.ExtensionManager | ||
| import com.sina.weibo.agent.extensions.core.ExtensionSwitcher | ||
| import com.sina.weibo.agent.extensions.ui.ExtensionSwitcherDialog | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify action is registered and placed in UI groups in plugin.xml
Ensure users can access this action (toolbar, Tools menu, and optional shortcut).
Run this script to confirm plugin.xml wiring:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate plugin.xml
fd -t f plugin.xml | sed -n '1p' | while read -r P; do
echo "Checking: $P"
echo "---- Registration ----"
rg -n -C3 -P '<action[^>]+class="com\.sina\.weibo\.agent\.extensions\.ui\.actions\.ExtensionSwitcherAction".*?>.*?</action>' "$P" || true
echo "---- Add-to-group ----"
rg -n -C2 -P '<add-to-group[^>]+action-id="[^"]*extensionSwitcher[^"]*"' "$P" || true
echo "---- Keyboard shortcuts ----"
rg -n -C2 -P '<keyboard-shortcut[^>]+action-id="[^"]*extensionSwitcher[^"]*"' "$P" || true
doneLength of output: 865
Register ExtensionSwitcherAction in plugin.xml
The ExtensionSwitcherAction isn’t declared in your plugin descriptor, so it won’t appear in any menu or toolbar. Please update jetbrains_plugin/src/main/resources/META-INF/plugin.xml to include:
• An <action> entry referencing your class, e.g.:
<actions>
<action
id="extensionSwitcher"
class="com.sina.weibo.agent.extensions.ui.actions.ExtensionSwitcherAction"
text="Switch Extension…"
description="Open the extension switcher dialog" />• One or more <add-to-group> entries to place it in the UI, for example:
<add-to-group group-id="ToolsMenu" action-id="extensionSwitcher" anchor="last"/>
<add-to-group group-id="MainToolbar" action-id="extensionSwitcher" anchor="last"/>• (Optional) A <keyboard-shortcut> if you want a default shortcut:
<keyboard-shortcut action-id="extensionSwitcher" first-keystroke="ctrl shift E"/>
</actions>After adding these, your action will be accessible from the Tools menu, toolbar, and via the shortcut.
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/actions/ExtensionSwitcherAction.kt
around lines 5 to 14, the action class is not declared in the plugin descriptor
so it won't appear in menus or toolbars; update
jetbrains_plugin/src/main/resources/META-INF/plugin.xml to add an <action> entry
that references
class="com.sina.weibo.agent.extensions.ui.actions.ExtensionSwitcherAction" with
an id (e.g., extensionSwitcher), text and description, then add one or more
<add-to-group> entries (for example ToolsMenu and/or MainToolbar) referencing
that action-id with anchor positions so it appears in the UI, and optionally add
a <keyboard-shortcut> element with a default keystroke if you want a shortcut.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt (1)
154-177: Fix Zip Slip vulnerability during extractionZip entry names are written as-is under tempDir without normalization/bound checks. A malicious VSIX can escape the temp directory and overwrite arbitrary files. Normalize and ensure resolved paths stay within the extraction root; handle directory entries explicitly; use REPLACE_EXISTING to be robust.
- ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis -> - var entry = zis.nextEntry - var extractedCount = 0 - - while (entry != null) { - if (!entry.isDirectory) { - val entryPath = tempDir!!.toPath().resolve(entry.name) - - // Create parent directories if needed - val parentDir = entryPath.parent - if (parentDir != null) { - Files.createDirectories(parentDir) - } - - // Extract file - Files.copy(zis, entryPath) - extractedCount++ - } - entry = zis.nextEntry - } - - LOG.debug("Extracted $extractedCount files to temporary directory") - } + ZipInputStream(BufferedInputStream(FileInputStream(vsixFile))).use { zis -> + val tempPath = tempDir!!.toPath() + var entry = zis.nextEntry + var extractedCount = 0 + while (entry != null) { + val resolvedPath = tempPath.resolve(entry.name).normalize() + // Prevent Zip Slip + if (!resolvedPath.startsWith(tempPath)) { + LOG.warn("Skipping suspicious zip entry outside temp dir: ${entry.name}") + entry = zis.nextEntry + continue + } + if (entry.isDirectory) { + Files.createDirectories(resolvedPath) + } else { + val parentDir = resolvedPath.parent + if (parentDir != null) { + Files.createDirectories(parentDir) + } + Files.copy(zis, resolvedPath, java.nio.file.StandardCopyOption.REPLACE_EXISTING) + extractedCount++ + } + entry = zis.nextEntry + } + LOG.debug("Extracted $extractedCount files to temporary directory") + }
🧹 Nitpick comments (13)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/theme/ThemeManager.kt (2)
75-81: Log attempted theme paths when none is found (improves diagnosability).Today we only log the resourceRoot. It’s much easier to debug if we also log the exact paths we tried.
Apply this diff:
- if (themeResourceDir == null) { - logger.warn("Theme resource directory does not exist for resource root: $resourceRoot") - return - } + if (themeResourceDir == null) { + val tried = listOf( + Paths.get(resourceRoot, "src", "integrations", "theme", "default-themes"), + Paths.get(resourceRoot, "integrations", "theme", "default-themes") + ) + logger.warn("Theme resource directory not found. resourceRoot=$resourceRoot; tried=${tried.joinToString(", ") { it.toString() }}") + return + }
562-581: Ensure the resolved path is a directory and de-duplicate the resolution logic.The current logic treats any existing path (even a file) as valid. Prefer checking it’s a directory and simplify with a candidate list.
Apply this diff:
- fun getThemeResourceDir(resourceRoot: String): Path? { - // Try first path: src/integrations/theme/default-themes - var themeDir = Paths.get(resourceRoot, "src", "integrations", "theme", "default-themes") - - if (themeDir.notExists()) { - // Try second path: integrations/theme/default-themes - themeDir = Paths.get(resourceRoot, "integrations", "theme", "default-themes") - if (themeDir.notExists()) { - return null - } - } - - return themeDir - } + fun getThemeResourceDir(resourceRoot: String): Path? { + val candidates = listOf( + Paths.get(resourceRoot, "src", "integrations", "theme", "default-themes"), + Paths.get(resourceRoot, "integrations", "theme", "default-themes") + ) + return candidates.firstOrNull { java.nio.file.Files.isDirectory(it) }?.normalize() + }Additionally, unless you need external callers, consider restricting visibility to internal to avoid broad API surface.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/webview/WebViewManager.kt (2)
602-603: Constrain extremely tall media to avoid vertical overflow spikes.Keeping
height: autois correct for aspect ratio. Adding a max-height guard prevents single tall images/videos from consuming the entire viewport.Apply this diff:
- height: auto; /* keep aspect ratio and avoid vertical overflow */ - display: block; /* remove inline baseline gaps that can trigger overflow */ + height: auto; /* keep aspect ratio */ + max-height: 100vh; /* guard against extreme vertical overflow */ + display: block; /* remove inline baseline gaps that can trigger overflow */
664-666: Consider scoping global resets to reduce extension-style collisions.Universal
*and full-documenthtml, bodyresets can override extension-provided layouts. If feasible, scope resets under a container element (e.g., a root wrapper div) or tighten selectors to what the WebView actually controls.Would you like me to propose a scoped reset (e.g., insert a wrapper element and prefix these rules) so Roo/Cline pages won’t unexpectedly break when they bring their own CSS resets?
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt (9)
31-31: getInstance() is not a singleton; it returns a new instance each callIf you intended a singleton-like accessor, cache the instance in the companion object.
Apply this diff:
- fun getInstance(): VsixManager = VsixManager() + private val INSTANCE: VsixManager by lazy { VsixManager() } + fun getInstance(): VsixManager = INSTANCE
36-39: Use Path APIs for OS-safe path constructionMinor, but Paths.get improves portability and readability.
- return "$homeDir/$BASE_DIR" + return Paths.get(homeDir, BASE_DIR).toString()
49-55: Validate the VSIX file before extractionEarly guards reduce error noise and avoid surprising exceptions.
LOG.info("Installing VSIX for extension $extensionId to $targetDir") + // Basic input validation + if (!vsixFile.exists() || !vsixFile.isFile) { + LOG.error("Provided VSIX file does not exist or is not a file: ${vsixFile.absolutePath}") + return false + } + if (!vsixFile.name.endsWith(".vsix", ignoreCase = true)) { + LOG.warn("Installing a non-.vsix file: ${vsixFile.name}") + }
200-212: Make file moves robust across filesystems (fallback to copy+delete)Moving from a temp dir to the final target can cross devices and fail. Use REPLACE_EXISTING and fall back to copy+delete.
- } else { - // Move file - Files.move(file.toPath(), targetFile.toPath()) - } + } else { + // Move file (fallback to copy+delete if cross-device move fails) + try { + Files.move( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + } catch (e: Exception) { + Files.copy( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + Files.delete(file.toPath()) + } + }
246-261: Harden moveDirectory file moves (same cross-device fallback)Mirror the robust move behavior here as well.
- if (file.isDirectory) { - moveDirectory(file, targetFile) - } else { - Files.move(file.toPath(), targetFile.toPath()) - } + if (file.isDirectory) { + moveDirectory(file, targetFile) + } else { + try { + Files.move( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + } catch (e: Exception) { + Files.copy( + file.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + ) + Files.delete(file.toPath()) + } + }
280-284: Fix misleading log message when integrations theme dir is missingThis branch checks the integrations directory, but logs about plugin themes.
- if (integrationsThemeDir == null) { - LOG.warn("Plugin themes directory not found, skipping themes copy") + if (integrationsThemeDir == null) { + LOG.warn("Integrations theme directory not found, skipping themes copy") return }
298-299: Use REPLACE_EXISTING when copying theme filesPrevents failures on re-installations or when files already exist.
- Files.copy(themeFile.toPath(), targetFile.toPath()) + Files.copy( + themeFile.toPath(), + targetFile.toPath(), + java.nio.file.StandardCopyOption.REPLACE_EXISTING + )
349-356: Make deleteDirectory resilient (use NIO walk and deleteIfExists)Recursive File.delete() is brittle and swallows failures. Use Files.walk with reverse-order deletion and log problems.
- private fun deleteDirectory(dir: File) { - if (dir.isDirectory) { - dir.listFiles()?.forEach { file -> - deleteDirectory(file) - } - } - dir.delete() - } + private fun deleteDirectory(dir: File) { + if (!dir.exists()) return + val path = dir.toPath() + try { + java.nio.file.Files.walk(path) + .sorted(java.util.Comparator.reverseOrder()) + .forEach { p -> java.nio.file.Files.deleteIfExists(p) } + } catch (e: Exception) { + LOG.warn("Failed to delete directory: ${dir.absolutePath}", e) + } + }
146-241: Optional hardening: Guard against zip bombs (excessive entries/bytes)Consider limiting total extracted entries/bytes to mitigate decompression bombs. Track a running count and abort if thresholds are exceeded (e.g., >10k entries or >1 GB total).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt(7 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/VsixUploadDialog.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/theme/ThemeManager.kt(2 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/webview/WebViewManager.kt(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/VsixUploadDialog.kt
- jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/core/ExtensionHostManager.kt
🔇 Additional comments (1)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/webview/WebViewManager.kt (1)
595-597: LGTM: body overflow constraints reduce layout jank.Hiding horizontal scroll and allowing vertical scroll is reasonable for most webview content and prevents accidental x-overflow issues.
| fun getExtensionDirectory(extensionId: String): String { | ||
| return "${getBaseDirectory()}/$extensionId" | ||
| } |
There was a problem hiding this comment.
Sanitize extensionId and bound-check the resolved path to prevent path traversal
extensionId is directly interpolated into a filesystem path. A crafted extensionId containing path separators (e.g., ../) can escape the base directory. Sanitize and enforce that the normalized path stays under the base.
- fun getExtensionDirectory(extensionId: String): String {
- return "${getBaseDirectory()}/$extensionId"
- }
+ fun getExtensionDirectory(extensionId: String): String {
+ val basePath = Paths.get(getBaseDirectory())
+ // Allow only safe characters; replace the rest
+ val safeId = extensionId.replace(Regex("[^A-Za-z0-9._-]"), "_")
+ val resolved = basePath.resolve(safeId).normalize()
+ require(resolved.startsWith(basePath)) {
+ "Invalid extensionId: path traversal is not allowed"
+ }
+ return resolved.toString()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun getExtensionDirectory(extensionId: String): String { | |
| return "${getBaseDirectory()}/$extensionId" | |
| } | |
| fun getExtensionDirectory(extensionId: String): String { | |
| val basePath = Paths.get(getBaseDirectory()) | |
| // Allow only safe characters; replace the rest | |
| val safeId = extensionId.replace(Regex("[^A-Za-z0-9._-]"), "_") | |
| val resolved = basePath.resolve(safeId).normalize() | |
| require(resolved.startsWith(basePath)) { | |
| "Invalid extensionId: path traversal is not allowed" | |
| } | |
| return resolved.toString() | |
| } |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/VsixManager.kt
around lines 82 to 84, the method builds a path by string interpolation which
allows path traversal via extensionId (e.g., "../"). Change the implementation
to: resolve the extensionId against the base directory using Path APIs
(Paths.get(base).resolve(extensionId).normalize()), reject or throw if
extensionId is absolute, and after normalization verify the resulting path
startsWith the normalized base directory; if it does not, treat as invalid input
and fail. Also trim or strip any path separators from extensionId (or validate
against an allowed name pattern) before resolving to further harden against
malicious input.
| // Support both <script nonce="..."> and <script type="text/javascript" nonce="..."> formats | ||
| val mRst = """<script(?:\s+type="text/javascript")?\s+nonce="([A-Za-z0-9]{32})">""".toRegex().find(data.htmlContent) | ||
| val str = mRst?.value ?: "" |
There was a problem hiding this comment.
Fix nonce regex and avoid catastrophic empty-string replace when no match.
- The regex is too strict (order-dependent, limited character set, exact length 32). Real CSP nonces are typically base64 and can include +, /, = and variable lengths.
- When no match is found,
mRst?.value ?: ""yields an empty string; replacing""injects content across the entire HTML, corrupting it.
Harden the selector and guard on no-match.
Apply this diff:
- // Support both <script nonce="..."> and <script type="text/javascript" nonce="..."> formats
- val mRst = """<script(?:\s+type="text/javascript")?\s+nonce="([A-Za-z0-9]{32})">""".toRegex().find(data.htmlContent)
- val str = mRst?.value ?: ""
+ // Support <script ... nonce="..."> with arbitrary attribute order and nonce chars
+ val scriptTagRegex = """<script\b[^>]*\bnonce=(['"])([^'"]+)\1""".toRegex(RegexOption.IGNORE_CASE)
+ val match = scriptTagRegex.find(data.htmlContent)
+ if (match == null) {
+ logger.warn("No <script> tag with nonce found; skipping JS bridge injection to avoid corrupting HTML. Ensure template includes a nonce-bearing <script>.")
+ return
+ }
+ val str = match.value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Support both <script nonce="..."> and <script type="text/javascript" nonce="..."> formats | |
| val mRst = """<script(?:\s+type="text/javascript")?\s+nonce="([A-Za-z0-9]{32})">""".toRegex().find(data.htmlContent) | |
| val str = mRst?.value ?: "" | |
| // Support <script ... nonce="..."> with arbitrary attribute order and nonce chars | |
| val scriptTagRegex = """<script\b[^>]*\bnonce=(['"])([^'"]+)\1""".toRegex(RegexOption.IGNORE_CASE) | |
| val match = scriptTagRegex.find(data.htmlContent) | |
| if (match == null) { | |
| logger.warn("No <script> tag with nonce found; skipping JS bridge injection to avoid corrupting HTML. Ensure template includes a nonce-bearing <script>.") | |
| return | |
| } | |
| val str = match.value |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/webview/WebViewManager.kt
around lines 281 to 283, the nonce regex and replacement are fragile: update the
regex to match a nonce attribute more broadly (e.g. match <script ...
nonce="..." ...> or nonce='...' with the value allowing base64 characters
[A-Za-z0-9+/=:-] and variable length) and capture the actual nonce value in a
group; then avoid using mRst?.value ?: "" for replacements — explicitly check if
the match exists (if null, skip replacement and leave htmlContent unchanged) and
use the captured group value for any replacement or injection so you never
replace an empty string across the whole document.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (6)
217-221: Avoid duplicate south button bars: hide DialogWrapper’s default actionsYou're adding a custom bottom panel while DialogWrapper also renders its own OK/Cancel bar. This results in duplicated controls. Easiest fix: override createActions() to return no actions, keeping your custom bottom panel.
Add this method to the class (no other code changes needed):
override fun createActions(): Array<Action> = emptyArray()
781-789: Persist selected extension only after a successful switchConfiguration is persisted before the switch. If the switch fails, the config points to an extension that isn’t actually active. Move persistence to the success path.
Apply this diff:
- if (result == Messages.YES) { - // Update configuration - configManager.setCurrentExtensionId(targetExtensionId) - // Note: Auto-switch functionality not implemented yet - // configManager.setAutoSwitchEnabled(autoSwitchCheckBox.isSelected) - - // Perform switch - performExtensionSwitch(targetExtensionId) - } + if (result == Messages.YES) { + // Perform switch (persist configuration on success) + performExtensionSwitch(targetExtensionId) + }
800-823: Fix potential NPE in whenComplete and persist config on successCompletableFuture.whenComplete may pass a null result on exceptional completion. Using a non-null Boolean risks NPE. Also persist config here after success.
- extensionSwitcher.switchExtension(targetExtensionId, forceRestart = true) - .whenComplete { success: Boolean, throwable: Throwable? -> + extensionSwitcher.switchExtension(targetExtensionId, forceRestart = true) + .whenComplete { result: Boolean?, throwable: Throwable? -> SwingUtilities.invokeLater { - if (success) { + val success = throwable == null && (result == true) + if (success) { + // Persist selected extension after successful switch + configManager.setCurrentExtensionId(targetExtensionId) Messages.showInfoMessage( "Successfully switched to extension: $targetExtensionId", "Extension Switch Complete" ) close(OK_EXIT_CODE) } else { val errorMessage = throwable?.message ?: "Unknown error occurred" Messages.showErrorDialog( "Failed to switch extension: $errorMessage", "Extension Switch Failed" ) // Reset UI switchButton.text = "Switch Extension" updateSwitchButton() cancelButton.isEnabled = true } } }
455-462: Compare current provider by id, not instanceInstance equality is brittle; different instances of the same provider may exist. Compare extension ids instead.
- isCurrent = provider == currentProvider, + isCurrent = currentProvider?.getExtensionId() == provider.getExtensionId(),
154-206: Escape HTML in renderer and tooltip; build markup consistentlydisplayName and file paths are injected into HTML without escaping. Escape inputs to prevent broken rendering or injection; also always use HTML consistently.
setCellRenderer { _, value, _, isSelected, _ -> val label = JBLabel().apply { isOpaque = true background = if (isSelected) ColorTokens.listSelBg else ColorTokens.listBg foreground = if (isSelected) ColorTokens.listSelFg else ColorTokens.listFg border = Ui.insets(4, 6, 4, 6) } if (value is ExtensionItem) { val resourceIcon = when { value.resourceStatus.projectResourceExists -> "🟢" value.resourceStatus.vsixResourceExists -> "🔵" value.resourceStatus.pluginResourceExists -> "🟡" else -> "🔴" } - val displayText = if (value.isCurrent) - "<b>${value.displayName}</b> <i>(Current)</i>" - else value.displayName - - label.text = "<html><div style='padding:2px;'>$resourceIcon $displayText</div></html>" + val safeName = esc(value.displayName) + val displayText = if (value.isCurrent) + "<b>$safeName</b> <i>(Current)</i>" else safeName + label.text = "<html><div style='padding:2px;'>$resourceIcon $displayText</div></html>" // tooltip 里的 code 背景令牌化 - fun code(path: String?) = - "<code style='background:${toCss(ColorTokens.codeBg)};padding:2px 4px;border-radius:2px;'>$path</code>" + fun code(path: String?) = + "<code style='background:${toCss(ColorTokens.codeBg)};padding:2px 4px;border-radius:2px;'>${esc(path)}</code>" label.toolTipText = buildString { append("<html><div style='padding:5px;'>") - append("<b>Extension: ${value.displayName}</b><br><br>") + append("<b>Extension: ${esc(value.displayName)}</b><br><br>") append("<b>Status: ${value.resourceStatus.statusText}</b><br><br>") if (value.resourceStatus.projectResourceExists) { append("📁 <b>Local Path:</b><br>${code(value.resourceStatus.projectResourcePath)}<br><br>") } if (value.resourceStatus.vsixResourceExists) { append("📦 <b>VSIX Path:</b><br>${code(value.resourceStatus.vsixResourcePath)}<br><br>") } if (value.resourceStatus.pluginResourceExists) { append("🔧 <b>Plugin Path:</b><br>${code(value.resourceStatus.pluginResourcePath)}") }Add helper and import (outside this range):
+import com.intellij.openapi.util.text.StringUtil +import java.util.Locale- private fun toCss(c: java.awt.Color) = String.format("#%02x%02x%02x", c.red, c.green, c.blue) + private fun toCss(c: java.awt.Color) = String.format(Locale.ROOT, "#%02x%02x%02x", c.red, c.green, c.blue) + private fun esc(s: String?): String = StringUtil.escapeXmlEntities(s ?: "")
478-486: Escape displayName and description in Description panelProvider-supplied fields may contain markup; escape before embedding in HTML.
- descriptionLabel.text = """ + val safeName = esc(item.displayName) + val safeDesc = esc(item.description) + descriptionLabel.text = """ <html> <div style='margin:5px;'> - <div style='font-size:14px;font-weight:bold;margin-bottom:8px;'>${item.displayName}</div> - <div style='line-height:1.4;'>${item.description}</div> + <div style='font-size:14px;font-weight:bold;margin-bottom:8px;'>$safeName</div> + <div style='line-height:1.4;'>$safeDesc</div> </div> </html> """.trimIndent()
🧹 Nitpick comments (4)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (4)
731-740: Allow cancelling while switchingCancel is disabled during switching, but doCancelAction supports cancellation. Keep Cancel enabled to let users abort.
if (extensionSwitcher.isSwitching()) { switchButton.text = "Switching..." switchButton.isEnabled = false - cancelButton.isEnabled = false + cancelButton.isEnabled = true } else { switchButton.text = "Switch Extension" updateSwitchButton() cancelButton.isEnabled = true }
85-85: Locale-agnostic hex formattingUse Locale.ROOT with String.format to satisfy static analysis and avoid locale pitfalls (even if benign here).
-private fun toCss(c: java.awt.Color) = String.format("#%02x%02x%02x", c.red, c.green, c.blue) +private fun toCss(c: java.awt.Color) = String.format(java.util.Locale.ROOT, "#%02x%02x%02x", c.red, c.green, c.blue)Also import Locale if you prefer the short form:
+import java.util.Locale
771-779: Avoid showing ‘null’ when there’s no current extensionSmall UX polish: show “(none)” instead of null.
- val result = Messages.showYesNoDialog( - "Are you sure you want to switch from '$currentExtensionId' to '$targetExtensionId'?\n\n" + + val fromText = currentExtensionId ?: "(none)" + val result = Messages.showYesNoDialog( + "Are you sure you want to switch from '$fromText' to '$targetExtensionId'?\n\n" + "This will restart the extension process and may cause a brief interruption.", "Confirm Extension Switch", "Switch", "Cancel", Messages.getQuestionIcon() )
7-29: Add missing imports for escaping and LocaleTo support the HTML escaping helper and Locale usage suggested above.
import com.intellij.util.ui.UIUtil +import com.intellij.openapi.util.text.StringUtil +import java.util.Locale
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt (2)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSelector.kt (1)
loadExtensions(35-45)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionSwitcher.kt (1)
performExtensionSwitch(143-174)
🪛 detekt (1.23.8)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/ui/ExtensionSwitcherDialog.kt
[warning] 85-85: String.format("#%02x%02x%02x", c.red, c.green, c.blue) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/plugin/WecoderPlugin.kt (2)
298-306: Fix incorrect string interpolation: using type name instead of variable.
$DEBUG_MODErefers to the enum type, not the variable. This won’t compile. You meant$DEBUG_TYPE.- Logger.getInstance(WecoderPluginService::class.java).info("Read debug mode from config file: $DEBUG_MODE") + Logger.getInstance(WecoderPluginService::class.java).info("Read debug mode from config file: $DEBUG_TYPE") } else { - Logger.getInstance(WecoderPluginService::class.java).warn("Cannot load config file, use default debug mode: $DEBUG_MODE") + Logger.getInstance(WecoderPluginService::class.java).warn("Cannot load config file, use default debug mode: $DEBUG_TYPE") } } catch (e: Exception) { - Logger.getInstance(WecoderPluginService::class.java).warn("Error reading config file, use default debug mode: $DEBUG_MODE", e) + Logger.getInstance(WecoderPluginService::class.java).warn("Error reading config file, use default debug mode: $DEBUG_TYPE", e) }
261-262: Allow re-initialization by resetting initialization future.
initializationCompleteis a val and may be completed with false (e.g., invalid config), making subsequentwaitForInitialization()calls permanently return false. Make it a var and reset on each initialize attempt.- private val initializationComplete = CompletableFuture<Boolean>() + private var initializationComplete = CompletableFuture<Boolean>()And at the start of initialize():
fun initialize(project: Project) { // DEBUG_MODE is no longer set directly in code, now read from config file if (isInitialized) { LOG.info("WecoderPluginService already initialized") return } + // reset future for a fresh initialization cycle + if (initializationComplete.isDone) { + initializationComplete = CompletableFuture() + }Optionally, avoid completing with false on precondition failures; just return, so callers can attempt later without observing a terminal false. If you prefer to keep completing false, the reset above is still required.
Also applies to: 333-347
♻️ Duplicate comments (2)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt (2)
139-145: Fix inconsistent default provider preference and logging.The code searches for "cline" but uses a variable named
rooProviderand logs “roo-code (preferred)”, which is misleading.- // Prefer roo-code as default provider -// val rooProvider = availableProviders.find { it.getExtensionId() == "roo-code" } - val rooProvider = availableProviders.find { it.getExtensionId() == "cline" } - if (rooProvider != null) { - currentProvider = rooProvider - LOG.info("Set default extension provider: roo-code (preferred)") + // Prefer "cline" as default provider + val preferredProvider = availableProviders.find { it.getExtensionId() == "cline" } + if (preferredProvider != null) { + currentProvider = preferredProvider + LOG.info("Set default extension provider: ${preferredProvider.getExtensionId()} (preferred)")
165-174: Revert on initialization failure to avoid inconsistent state.If
initialize(project)throws,currentProviderremains switched while uninitialized.fun setCurrentProvider(extensionId: String): Boolean { val provider = extensionProviders[extensionId] if (provider != null && provider.isAvailable(project)) { val oldProvider = currentProvider currentProvider = provider - // Initialize new provider - provider.initialize(project) + // Initialize new provider (revert on failure) + try { + provider.initialize(project) + } catch (t: Throwable) { + currentProvider = oldProvider + LOG.warn("Failed to initialize provider: $extensionId, reverting to ${oldProvider?.getExtensionId()}", t) + return false + }
🧹 Nitpick comments (16)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/ui/RunVSAgentToolWindowFactory.kt (3)
44-47: Remove unused pluginService variable.
pluginServiceis created but not used here (startup handled later). Drop it to avoid confusion.- // Initialize plugin service - val pluginService = WecoderPlugin.getInstance(project) -// pluginService.initialize(project) + // Plugin service will be initialized after a valid selection/startup flow
440-445: Avoid repeated WebView lookup during removal.Look up the current WebView component once before iterating to reduce race potential and improve clarity.
- val components = contentPanel.components - for (component in components) { - if (component !== webViewManager.getLatestWebView()?.browser?.component) { - contentPanel.remove(component) - } - } + val currentWebViewComponent = webViewManager.getLatestWebView()?.browser?.component + val components = contentPanel.components + for (component in components) { + if (component !== currentWebViewComponent) { + contentPanel.remove(component) + } + }
497-520: Validate selected plugin ID before applying.The options include "custom" but no provider may exist for it. Pre-validate against registered providers to avoid leaving the system in an invalid state and a failing startup.
Suggested guard in the apply handler (pseudo):
val extensionManager = ExtensionManager.getInstance(project) if (!extensionManager.isRegistered(selectedPlugin)) { javax.swing.JOptionPane.showMessageDialog( contentPanel, "不支持的插件类型: $selectedPlugin", "无效选择", javax.swing.JOptionPane.ERROR_MESSAGE ) return@addActionListener } applyPluginSelection(selectedPlugin)Would you like me to wire this up against ExtensionManager’s registry API?
Also applies to: 535-540
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/plugin/WecoderPlugin.kt (2)
85-91: Increase or backoff the config load wait to avoid flakiness.Waiting up to 1 second (10 x 100ms) may be too short on slow disks/first run. Consider increasing retries, using exponential backoff, or using a completion signal from ExtensionConfigurationManager.
225-229: Enum comment mismatch and semantics.
RELEASEis documented as “Only IDEA plugin debug”, which contradicts its name. Clarify its semantics and ensure it’s handled in initialize (currently only ALL is special-cased).jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/config/ExtensionConfiguration.kt (1)
85-100: Consider memoizing default configs and avoiding mutable map writes in catch.Currently, failure to load writes defaults into the cache in the catch block. That’s fine, but you could precompute defaults for all types and keep them immutable. Minor style/readability improvement.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt (2)
127-133: Swallowed exception: log context for easier diagnosis.Catching generic Exception and returning the default hides problems. Log at least at debug/warn with the key to aid troubleshooting.
fun getConfigValue(key: String, defaultValue: String? = null): String? { return try { val properties = loadMainConfig() properties.getProperty(key, defaultValue) } catch (e: Exception) { - defaultValue + LOG.warn("Failed to read config value for key='$key', returning default", e) + defaultValue } }
181-183: Swallowed exception in listExtensionConfigFiles; add logging.Errors are silently ignored. Log at debug/warn to help diagnose filesystem issues.
- } catch (e: Exception) { - // Log error but don't throw - } + } catch (e: Exception) { + LOG.warn("Failed to list extension config files in ${PluginConstants.ConfigFiles.getUserConfigDir()}", e) + }jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt (4)
219-224: Use existing IO utilities and avoid manual stream management for verification.Replace manual reloading with the shared utility to ensure consistent encoding/behavior.
- if (configFile.exists()) { - val savedProperties = Properties() - savedProperties.load(configFile.inputStream()) + if (configFile.exists()) { + val savedProperties = ConfigFileUtils.loadMainConfig() val savedExtensionId = savedProperties.getProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY) logger.info("Configuration saved successfully. File content: ${PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY}=$savedExtensionId") } else {
244-253: Avoid reload loops; track last known mtime instead of a 5-second heuristic.Reloading based on “modified in the last 5 seconds” can miss changes or cause repeated reloads. Compare against a stored lastModified instead.
try { if (configFile.exists()) { val lastModified = configFile.lastModified() - // Simple change detection - could be enhanced with file watcher - if (lastModified > (System.currentTimeMillis() - 5000)) { // Check if modified in last 5 seconds + // Reload only when file mtime increases + if (lastModified > lastKnownConfigModified) { logger.info("Configuration file changed, reloading...") reloadConfiguration() + lastKnownConfigModified = lastModified } }Add this field near other state fields:
// Track last known config file modification time @Volatile private var lastKnownConfigModified: Long = 0L
83-83: Unify comment language (English) for consistency.Replace the inline Chinese comment with an English equivalent.
- val extensionId = currentExtensionId // 创建局部变量避免并发问题 + val extensionId = currentExtensionId // Create a local snapshot to avoid concurrency issues
100-101: Unify comment language (English) for consistency.- val extensionId = currentExtensionId // 创建局部变量避免并发问题 + val extensionId = currentExtensionId // Create a local snapshot to avoid concurrency issuesjetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt (4)
196-202: Remove duplicate DynamicButtonManager update.
buttonManager.setCurrentExtension(extensionId)is called twice; keep a single call.- // Update dynamic button manager - try { - val buttonManager = DynamicButtonManager.getInstance(project) - buttonManager.setCurrentExtension(extensionId) - } catch (e: Exception) { - LOG.warn("Failed to update button configuration", e) - }
216-217: Kotlin style: call companion method without explicitCompanion.If
getInstanceis in a companion object, preferExtensionSwitcher.getInstance(project).- val extensionSwitcher = ExtensionSwitcher.Companion.getInstance(project) + val extensionSwitcher = ExtensionSwitcher.getInstance(project)
84-86: Avoid duplicated logic: reuseisConfigurationValid()inisProperlyInitialized().fun isProperlyInitialized(): Boolean { - return currentProvider != null && currentProvider!!.isAvailable(project) + return isConfigurationValid() }Also applies to: 102-104
165-209: Optional: make provider switching thread-safe.Concurrent calls to
setCurrentProvidercan race. Annotate with@Synchronizedto serialize switches.- fun setCurrentProvider(extensionId: String): Boolean { + @Synchronized + fun setCurrentProvider(extensionId: String): Boolean {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/config/ExtensionConfiguration.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/plugin/WecoderPlugin.kt(8 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/ui/RunVSAgentToolWindowFactory.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt(1 hunks)jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/PluginConstants.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt
[warning] 131-131: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 181-181: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (4)
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/config/ExtensionConfiguration.kt (1)
44-57: LGTM: clear initialization and defaulting.Initialization loads all types, derives current type from main config, and falls back to defaults with sensible logging. API surface is cohesive.
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt (1)
329-337: listExtensionConfigFiles returns extension IDs, not filenames. Verified: the implementation callsgetExtensionIdFromFilename(file.name)to extract and return only the IDs. No changes required.jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt (2)
109-121: Provider registration looks good.Straightforward registration of Roo and Cline providers with a placeholder for custom providers. No issues spotted.
77-79: Remove redundant no-arg overload in ExtensionManagerThe default-arg declaration
fun initialize(configuredExtensionId: String? = null)already covers calls with no parameters. No direct usages of the explicit
fun initialize() { initialize(null) }overload were found.
• Delete lines 77–79 in
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionManager.kt:- fun initialize() { - initialize(null) - }
| try { | ||
| val properties = Properties() | ||
| properties.load(configFile.inputStream()) | ||
| append("Config file content:\n") | ||
| properties.stringPropertyNames().forEach { key -> | ||
| append(" $key = ${properties.getProperty(key)}\n") | ||
| } | ||
| } catch (e: Exception) { | ||
| append("Failed to read config file: ${e.message}\n") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking file descriptors and mask secrets in debug output.
- The InputStream from
configFile.inputStream()is not closed; this can leak FDs and cause file locks on Windows. getDebugInfo()prints raw values, potentially exposing secrets (e.g., api keys, tokens).
Apply this diff to close streams and mask sensitive keys:
- try {
- val properties = Properties()
- properties.load(configFile.inputStream())
- append("Config file content:\n")
- properties.stringPropertyNames().forEach { key ->
- append(" $key = ${properties.getProperty(key)}\n")
- }
- } catch (e: Exception) {
+ try {
+ val properties = Properties()
+ configFile.inputStream().use { properties.load(it) }
+ append("Config file content:\n")
+ properties.stringPropertyNames().forEach { key ->
+ val raw = properties.getProperty(key)
+ val masked = if (
+ key.contains("key", ignoreCase = true) ||
+ key.contains("token", ignoreCase = true) ||
+ key.contains("secret", ignoreCase = true) ||
+ key.contains("password", ignoreCase = true)
+ ) "****" else raw
+ append(" $key = $masked\n")
+ }
+ } catch (e: Exception) {
append("Failed to read config file: ${e.message}\n")
}🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt
around lines 128 to 137, the code opens configFile.inputStream() without closing
it (risking FD leaks/file locks) and prints raw property values (risking secret
leakage); change to use the Kotlin use{} pattern or try-with-resources to ensure
the InputStream is closed, and when iterating properties mask sensitive values
(e.g., if the key matches /(?i)(key|secret|token|password|api)/ replace most
characters with a fixed masked string like "****" or show only last 4 chars)
before appending to the debug output.
| private fun saveConfiguration() { | ||
| try { | ||
| val properties = Properties() | ||
| currentExtensionId?.let { properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, it) } | ||
|
|
||
| logger.info("Saving configuration to file: ${configFile.absolutePath}") | ||
| logger.info("Configuration content: ${PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY}=$currentExtensionId") | ||
|
|
||
| ConfigFileUtils.saveMainConfig(properties) |
There was a problem hiding this comment.
Prevent accidental config wipe; merge existing properties and guard null/blank extension ID.
Current logic saves an empty Properties when currentExtensionId is null, which can erase the main config. Also, it doesn't merge with existing properties.
Apply this diff:
private fun saveConfiguration() {
try {
- val properties = Properties()
- currentExtensionId?.let { properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, it) }
+ if (currentExtensionId.isNullOrBlank()) {
+ logger.warn("Current extension ID is null/blank; skipping save to avoid clearing main config")
+ return
+ }
+ val properties = if (ConfigFileUtils.mainConfigExists()) {
+ ConfigFileUtils.loadMainConfig()
+ } else {
+ Properties()
+ }
+ properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, currentExtensionId!!)
logger.info("Saving configuration to file: ${configFile.absolutePath}")
logger.info("Configuration content: ${PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY}=$currentExtensionId")
ConfigFileUtils.saveMainConfig(properties)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun saveConfiguration() { | |
| try { | |
| val properties = Properties() | |
| currentExtensionId?.let { properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, it) } | |
| logger.info("Saving configuration to file: ${configFile.absolutePath}") | |
| logger.info("Configuration content: ${PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY}=$currentExtensionId") | |
| ConfigFileUtils.saveMainConfig(properties) | |
| private fun saveConfiguration() { | |
| try { | |
| if (currentExtensionId.isNullOrBlank()) { | |
| logger.warn("Current extension ID is null/blank; skipping save to avoid clearing main config") | |
| return | |
| } | |
| val properties = if (ConfigFileUtils.mainConfigExists()) { | |
| ConfigFileUtils.loadMainConfig() | |
| } else { | |
| Properties() | |
| } | |
| properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, currentExtensionId!!) | |
| logger.info("Saving configuration to file: ${configFile.absolutePath}") | |
| logger.info("Configuration content: ${PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY}=$currentExtensionId") | |
| ConfigFileUtils.saveMainConfig(properties) |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt
around lines 208 to 216, the saveConfiguration() currently writes a fresh
Properties object (and can write an empty file) when currentExtensionId is null,
and it does not merge with any existing main config; change it to first load
existing properties from the main config (using ConfigFileUtils.loadMainConfig
or equivalent), only set PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY when
currentExtensionId is non-null and not blank, update/put that key into the
loaded properties, and then save the merged properties back with
ConfigFileUtils.saveMainConfig; ensure you skip saving entirely if no change is
needed to avoid accidental wipes.
| fun dispose() { | ||
| logger.info("Disposing extension configuration manager") | ||
| saveConfiguration() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not save invalid/empty configuration during dispose.
Persisting on dispose when currentExtensionId is null/blank risks wiping config. Guard the save.
fun dispose() {
logger.info("Disposing extension configuration manager")
- saveConfiguration()
+ if (isConfigurationLoaded && isConfigurationValid && !currentExtensionId.isNullOrBlank()) {
+ saveConfiguration()
+ } else {
+ logger.debug("Skipping save on dispose: configuration is not in a valid state")
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun dispose() { | |
| logger.info("Disposing extension configuration manager") | |
| saveConfiguration() | |
| } | |
| fun dispose() { | |
| logger.info("Disposing extension configuration manager") | |
| if (isConfigurationLoaded && isConfigurationValid && !currentExtensionId.isNullOrBlank()) { | |
| saveConfiguration() | |
| } else { | |
| logger.debug("Skipping save on dispose: configuration is not in a valid state") | |
| } | |
| } |
🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/extensions/core/ExtensionConfigurationManager.kt
around lines 364-367, the dispose() method always calls saveConfiguration()
which can persist an invalid/empty config when currentExtensionId is null or
blank; update dispose() to check that currentExtensionId (or equivalent
indicator of a valid configuration) is non-null and non-blank before calling
saveConfiguration(), skipping the save if the check fails so you don't overwrite
existing config with an empty/invalid one.
| val webViewManager = project.getService(WebViewManager::class.java) | ||
| Disposer.register(project, webViewManager) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Null-safety when registering WebViewManager with Disposer.
project.getService(WebViewManager::class.java) may return null; guard to avoid NPE.
- val webViewManager = project.getService(WebViewManager::class.java)
- Disposer.register(project, webViewManager)
+ project.getService(WebViewManager::class.java)?.let { Disposer.register(project, it) }
+ ?: LOG.warn("WebViewManager service not available; skipping disposer registration")🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/plugin/WecoderPlugin.kt
around lines 133 to 135, project.getService(WebViewManager::class.java) can
return null causing an NPE when passed to Disposer.register; guard the call by
checking for null before registering (e.g. retrieve the service into a local
variable and only call Disposer.register(project, service) if service != null,
or use a safe-call/let pattern) so you never pass a null reference to Disposer.
| private fun startConfigurationMonitoring() { | ||
| // Start background monitoring thread | ||
| Thread { | ||
| try { | ||
| while (!project.isDisposed) { | ||
| Thread.sleep(2000) // Check every 2 seconds | ||
|
|
||
| if (!project.isDisposed) { | ||
| // Don't update UI if plugin is starting or running | ||
| if (isPluginStarting || isPluginRunning) { | ||
| logger.debug("Plugin is starting or running, skipping UI update") | ||
| continue | ||
| } | ||
|
|
||
| // Only update UI if we're not in the middle of plugin startup | ||
| // Check if plugin is actually running before updating UI | ||
| val isPluginRunning = isPluginActuallyRunning() | ||
|
|
||
| // Only update UI if plugin is not running or if there's a significant change | ||
| if (!isPluginRunning) { | ||
| ApplicationManager.getApplication().invokeLater { | ||
| updateUIContent() | ||
| } | ||
| } else { | ||
| // Plugin is running, only update status labels, don't change main UI | ||
| ApplicationManager.getApplication().invokeLater { | ||
| updateConfigStatus(configStatusPanel.getComponent(0) as JLabel) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (e: InterruptedException) { | ||
| logger.info("Configuration monitoring interrupted") | ||
| } catch (e: Exception) { | ||
| logger.error("Error in configuration monitoring", e) | ||
| } | ||
| }.apply { | ||
| isDaemon = true | ||
| name = "RunVSAgent-ConfigMonitor-UI" | ||
| start() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent spawning multiple configuration-monitor threads.
startConfigurationMonitoring() is called in init and again after selection; each call spawns a new daemon thread. Guard with a single-thread check.
Add a field (outside this range) to the inner class:
// place near other fields
@Volatile private var monitorThread: Thread? = nullThen change the method:
private fun startConfigurationMonitoring() {
- // Start background monitoring thread
- Thread {
+ // Start background monitoring thread (single instance)
+ if (monitorThread?.isAlive == true) {
+ logger.debug("Config monitor already running, skip starting another")
+ return
+ }
+ monitorThread = Thread {
try {
while (!project.isDisposed) {
Thread.sleep(2000) // Check every 2 seconds
...
} catch (e: InterruptedException) {
logger.info("Configuration monitoring interrupted")
} catch (e: Exception) {
logger.error("Error in configuration monitoring", e)
}
- }.apply {
- isDaemon = true
- name = "RunVSAgent-ConfigMonitor-UI"
- start()
- }
+ }.apply {
+ isDaemon = true
+ name = "RunVSAgent-ConfigMonitor-UI"
+ start()
+ }
}🤖 Prompt for AI Agents
In
jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/ui/RunVSAgentToolWindowFactory.kt
around lines 346 to 387, the method startConfigurationMonitoring() currently
spawns a new daemon thread every time it is called; add a @Volatile private var
monitorThread: Thread? = null near the other fields and modify
startConfigurationMonitoring() to first check if monitorThread?.isAlive == true
and return early if so (or synchronize a short critical section) to prevent
creating multiple monitor threads, otherwise create the thread, assign it to
monitorThread before starting, and ensure the thread clears monitorThread (set
to null) when it exits; keep the existing daemon, name, and exception handling
behavior.
| if (!configDir.exists()) { | ||
| configDir.mkdirs() | ||
| println("Created configuration directory: ${configDir.absolutePath}") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid println in plugins; use IntelliJ Logger and handle mkdirs() result.
Use the platform logger instead of stdout. Also, check mkdirs()’s return value to detect failures explicitly.
Add logger:
// imports (top of file)
import com.intellij.openapi.diagnostic.Logger
// inside object
private val LOG = Logger.getInstance(ConfigFileUtils::class.java)Then:
- configDir.mkdirs()
- println("Created configuration directory: ${configDir.absolutePath}")
+ if (!configDir.mkdirs() && !configDir.exists()) {
+ throw IOException("Failed to create configuration directory: ${configDir.absolutePath}")
+ }
+ LOG.info("Created configuration directory: ${configDir.absolutePath}")🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt
around lines 23 to 26, replace the println usage and unguarded
configDir.mkdirs() with IntelliJ platform logging and explicit mkdirs() result
handling: add a private Logger instance for this class, call LOG.info(...)
instead of println when directory creation succeeds, and if mkdirs() returns
false (and the directory still does not exist) log an appropriate error via
LOG.error(...) and handle the failure (throw an IOException or return a clear
failure result) so directory-creation failures are detected and reported rather
than silently ignored.
| properties.load(configFile.inputStream()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Close streams with use{} to prevent leaks.
All file I/O should use use {} so streams are always closed.
- properties.load(configFile.inputStream())
+ configFile.inputStream().use { properties.load(it) }
...
- properties.store(configFile.outputStream(), comment)
+ configFile.outputStream().use { properties.store(it, comment) }
...
- properties.load(configFile.inputStream())
+ configFile.inputStream().use { properties.load(it) }
...
- properties.store(configFile.outputStream(), comment)
+ configFile.outputStream().use { properties.store(it, comment) }Also applies to: 57-58, 71-72, 88-89
🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt
around lines 40-41 (and similarly at 57-58, 71-72, 88-89), the code opens
InputStreams/Readers without using Kotlin's use{} idiom which can leak
resources; update each location to wrap the stream/reader creation and
consumption inside a use { ... } block so the stream is automatically closed
(e.g., open the file/inputStream inside use and perform properties.load or read
operations inside the lambda), replacing the current direct calls to
inputStream()/reader() with use-wrapped versions to ensure proper resource
cleanup.
| properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, "roo-code") | ||
| properties.setProperty("# Available extension types:", "") | ||
| properties.setProperty("# - roo-code: Roo Code extension", "") | ||
| properties.setProperty("# - cline: Cline AI extension", "") | ||
| properties.setProperty("# - custom: Custom extension", "") | ||
|
|
||
| saveMainConfig(properties, "RunVSAgent Extension Configuration - Default Template") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t create fake “comment keys” in Properties; write a proper comment header.
Setting properties with keys starting with “# …” persists them as actual keys, not comments. Keep the file clean and use the store() comment instead.
fun createDefaultMainConfig() {
val properties = Properties()
properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, "roo-code")
- properties.setProperty("# Available extension types:", "")
- properties.setProperty("# - roo-code: Roo Code extension", "")
- properties.setProperty("# - cline: Cline AI extension", "")
- properties.setProperty("# - custom: Custom extension", "")
-
- saveMainConfig(properties, "RunVSAgent Extension Configuration - Default Template")
+ val comment = "RunVSAgent Extension Configuration - Default Template. Supported types: roo-code, cline, custom."
+ saveMainConfig(properties, comment)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, "roo-code") | |
| properties.setProperty("# Available extension types:", "") | |
| properties.setProperty("# - roo-code: Roo Code extension", "") | |
| properties.setProperty("# - cline: Cline AI extension", "") | |
| properties.setProperty("# - custom: Custom extension", "") | |
| saveMainConfig(properties, "RunVSAgent Extension Configuration - Default Template") | |
| } | |
| fun createDefaultMainConfig() { | |
| val properties = Properties() | |
| properties.setProperty(PluginConstants.ConfigFiles.EXTENSION_TYPE_KEY, "roo-code") | |
| val comment = "RunVSAgent Extension Configuration - Default Template. Supported types: roo-code, cline, custom." | |
| saveMainConfig(properties, comment) | |
| } |
🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/ConfigFileUtils.kt
around lines 154-161, remove the properties.setProperty calls that use keys
beginning with “# …” (they create real properties instead of comments) and
instead provide a proper comment/header when saving the Properties (i.e., pass
the human-readable header to the Properties.store/saveMainConfig comment
parameter or otherwise write a header line via the save function). Ensure the
properties only contain real keys (like EXTENSION_TYPE_KEY) and that the
human-facing description is written as a comment during the save operation.
| fun getUserConfigDir(): String { | ||
| return System.getProperty("user.home") + "/.run-vs-agent" | ||
| } | ||
|
|
||
| /** | ||
| * Get main configuration file path in user home directory | ||
| */ | ||
| fun getMainConfigPath(): String { | ||
| return getUserConfigDir() + "/" + MAIN_CONFIG_FILE | ||
| } | ||
|
|
||
| /** | ||
| * Get extension configuration file path in user home directory | ||
| */ | ||
| fun getExtensionConfigPath(extensionId: String): String { | ||
| return getUserConfigDir() + "/" + EXTENSION_CONFIG_PREFIX + extensionId | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use platform-independent path building (avoid hardcoded "/").
String concatenation with "/" will break on Windows and is brittle. Build paths with Paths to ensure correct separators and readability.
Apply this diff to the affected methods:
+// add import:
+// import java.nio.file.Paths
+
- fun getUserConfigDir(): String {
- return System.getProperty("user.home") + "/.run-vs-agent"
- }
+ fun getUserConfigDir(): String {
+ return java.nio.file.Paths.get(System.getProperty("user.home"), ".run-vs-agent").toString()
+ }
...
- fun getMainConfigPath(): String {
- return getUserConfigDir() + "/" + MAIN_CONFIG_FILE
- }
+ fun getMainConfigPath(): String {
+ return java.nio.file.Paths.get(getUserConfigDir(), MAIN_CONFIG_FILE).toString()
+ }
...
- fun getExtensionConfigPath(extensionId: String): String {
- return getUserConfigDir() + "/" + EXTENSION_CONFIG_PREFIX + extensionId
- }
+ fun getExtensionConfigPath(extensionId: String): String {
+ return java.nio.file.Paths.get(getUserConfigDir(), EXTENSION_CONFIG_PREFIX + extensionId).toString()
+ }Additionally, consider sanitizing extensionId to avoid path traversal or illegal filename characters (e.g., allow [a-zA-Z0-9._-] only). I can add a small helper if you want.
🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/PluginConstants.kt
around lines 45 to 61, the methods build file paths by concatenating strings
with "/" which is platform-dependent and unsafe; change them to use
java.nio.file.Paths.get(...) or Path.resolve to construct user config dir, main
config path, and extension config path so the JVM will use the correct file
separators, and before using extensionId sanitize it (restrict to [a-zA-Z0-9._-]
or replace invalid chars) to prevent path traversal or invalid filenames.
| fun getExtensionIdFromFilename(filename: String): String? { | ||
| return if (filename.startsWith(EXTENSION_CONFIG_PREFIX)) { | ||
| filename.substring(EXTENSION_CONFIG_PREFIX.length) | ||
| } else null | ||
| } | ||
|
|
||
| /** | ||
| * Check if filename is an extension config file | ||
| */ | ||
| fun isExtensionConfigFile(filename: String): Boolean { | ||
| return filename.startsWith(EXTENSION_CONFIG_PREFIX) && filename != MAIN_CONFIG_FILE | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid empty extension IDs and simplify checks.
- getExtensionIdFromFilename returns an empty string when the filename is exactly the prefix; return null instead.
- isExtensionConfigFile redundantly checks
filename != MAIN_CONFIG_FILEeven though MAIN_CONFIG_FILE doesn’t start with the prefix. Also ensure the suffix is non-empty.
- fun getExtensionIdFromFilename(filename: String): String? {
- return if (filename.startsWith(EXTENSION_CONFIG_PREFIX)) {
- filename.substring(EXTENSION_CONFIG_PREFIX.length)
- } else null
- }
+ fun getExtensionIdFromFilename(filename: String): String? {
+ if (!filename.startsWith(EXTENSION_CONFIG_PREFIX)) return null
+ val id = filename.substring(EXTENSION_CONFIG_PREFIX.length)
+ return id.takeIf { it.isNotBlank() }
+ }
...
- fun isExtensionConfigFile(filename: String): Boolean {
- return filename.startsWith(EXTENSION_CONFIG_PREFIX) && filename != MAIN_CONFIG_FILE
- }
+ fun isExtensionConfigFile(filename: String): Boolean {
+ return filename.startsWith(EXTENSION_CONFIG_PREFIX)
+ && filename.length > EXTENSION_CONFIG_PREFIX.length
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun getExtensionIdFromFilename(filename: String): String? { | |
| return if (filename.startsWith(EXTENSION_CONFIG_PREFIX)) { | |
| filename.substring(EXTENSION_CONFIG_PREFIX.length) | |
| } else null | |
| } | |
| /** | |
| * Check if filename is an extension config file | |
| */ | |
| fun isExtensionConfigFile(filename: String): Boolean { | |
| return filename.startsWith(EXTENSION_CONFIG_PREFIX) && filename != MAIN_CONFIG_FILE | |
| } | |
| fun getExtensionIdFromFilename(filename: String): String? { | |
| if (!filename.startsWith(EXTENSION_CONFIG_PREFIX)) return null | |
| val id = filename.substring(EXTENSION_CONFIG_PREFIX.length) | |
| return id.takeIf { it.isNotBlank() } | |
| } | |
| /** | |
| * Check if filename is an extension config file | |
| */ | |
| fun isExtensionConfigFile(filename: String): Boolean { | |
| return filename.startsWith(EXTENSION_CONFIG_PREFIX) | |
| && filename.length > EXTENSION_CONFIG_PREFIX.length | |
| } |
🤖 Prompt for AI Agents
In jetbrains_plugin/src/main/kotlin/com/sina/weibo/agent/util/PluginConstants.kt
around lines 66 to 77, getExtensionIdFromFilename currently returns an empty
string when filename equals the prefix — change it to return null for that case
(only return a substring when filename.length > EXTENSION_CONFIG_PREFIX.length),
and simplify isExtensionConfigFile by removing the redundant check against
MAIN_CONFIG_FILE and instead verify filename startsWith EXTENSION_CONFIG_PREFIX
and has a non-empty suffix (e.g., filename.length >
EXTENSION_CONFIG_PREFIX.length or by reusing getExtensionIdFromFilename() and
checking for non-null).
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores
Tests