feat(idea): ACP client integration (stdio)#537
Conversation
Implements ACP client for the IDEA plugin and surfaces it under Remote -> ACP, streaming session updates into the existing timeline renderer. Refs: #535
📝 WalkthroughWalkthroughThis PR introduces ACP (Agent Client Protocol) integration into the IntelliJ IDEA plugin through build dependencies, extended settings for ACP configuration, new UI components and ViewModel for ACP lifecycle management, a mode selector for remote agent types, and refactoring of the main agent app to support multiple remote modes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User<br/>(IDEA UI)
participant IdeaAcpAgentContent as IdeaAcpAgentContent<br/>(UI Component)
participant IdeaAcpAgentViewModel as IdeaAcpAgentViewModel<br/>(ViewModel)
participant AcpProcess as Local ACP<br/>Process
participant Protocol as ACP Protocol<br/>(Client/StdioTransport)
User->>IdeaAcpAgentContent: Click Start
IdeaAcpAgentContent->>IdeaAcpAgentViewModel: connect(config)
IdeaAcpAgentViewModel->>IdeaAcpAgentViewModel: parseEnvLines() + splitArgs()
IdeaAcpAgentViewModel->>AcpProcess: ProcessBuilder.start()
activate AcpProcess
IdeaAcpAgentViewModel->>Protocol: StdioTransport(stdin, stdout)
IdeaAcpAgentViewModel->>Protocol: Protocol.initSession(ClientInfo)
Protocol->>AcpProcess: handshake + session init
AcpProcess-->>Protocol: session established
Protocol->>IdeaAcpAgentViewModel: ClientSessionOperations callback
IdeaAcpAgentViewModel->>IdeaAcpAgentContent: isConnected.emit(true)
User->>IdeaAcpAgentContent: Enter prompt + Send
IdeaAcpAgentContent->>IdeaAcpAgentViewModel: sendMessage(text)
IdeaAcpAgentViewModel->>Protocol: send ResourceRequest/message
Protocol->>AcpProcess: stdin stream
AcpProcess-->>Protocol: Agent Message + Plan + ToolCall
Protocol->>IdeaAcpAgentViewModel: handleMessage() callback
IdeaAcpAgentViewModel->>IdeaAcpAgentViewModel: renderer.render() (converts to timeline)
IdeaAcpAgentViewModel->>IdeaAcpAgentContent: renderer updates (StateFlow)
IdeaAcpAgentContent->>IdeaAcpAgentContent: IdeaTimelineContent recomposes
User->>IdeaAcpAgentContent: Click Stop
IdeaAcpAgentContent->>IdeaAcpAgentViewModel: disconnect()
IdeaAcpAgentViewModel->>Protocol: close()
IdeaAcpAgentViewModel->>AcpProcess: destroy()
deactivate AcpProcess
IdeaAcpAgentViewModel->>IdeaAcpAgentContent: isConnected.emit(false)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR integrates an ACP (Agent Client Protocol) client into the IntelliJ IDEA plugin as an alternative “Remote” backend, using JSON-RPC over stdio, and persists ACP command configuration in plugin settings.
Changes:
- Add
RemoteAgentModeselector UI and wire it into the existing Remote tab so users can switch between Server and ACP modes while sharing the same input area. - Introduce
IdeaAcpAgentViewModelplusIdeaAcpAgentContentto manage the ACP stdio process, client/session lifecycle, and rendersession/updatestreams into the existing timeline UI. - Extend
AutoDevSettingsStateandbuild.gradle.ktsto persist ACP command/args/env settings and to pull in the ACP +kotlinx-iodependencies.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteModeSelector.kt |
Adds a small pill-based RemoteAgentMode selector (Server/ACP) used to toggle between remote backends in the Remote tab. |
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentViewModel.kt |
Implements the ACP client integration: spawns the local ACP process over stdio, initializes the protocol and session, streams events into JewelRenderer, and defines minimal client operations with fs/terminal disabled. |
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentContent.kt |
Provides the ACP configuration and status UI (command/args/cwd/env, connection indicator, stderr tail) and hooks it to the shared timeline view. |
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt |
Wires ACP mode into the main toolwindow: creates and disposes the new ACP view model, adds the mode selector, switches the top content between server and ACP, and routes input/abort actions to the appropriate backend. |
mpp-idea/mpp-idea-core/src/main/kotlin/cc/unitmesh/devti/settings/AutoDevSettingsState.kt |
Adds persisted ACP configuration fields for command, args, and environment text. |
mpp-idea/build.gradle.kts |
Declares the ACP library dependency (with explicit kotlinx exclusions) and adds kotlinx-io-core to support the stdio transport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import kotlinx.serialization.json.JsonObject | ||
| import kotlinx.serialization.json.JsonPrimitive |
There was a problem hiding this comment.
These Json-related imports (JsonObject, JsonPrimitive) are not used in this file and can be removed to reduce noise and avoid unused-import warnings.
| import kotlinx.serialization.json.JsonObject | |
| import kotlinx.serialization.json.JsonPrimitive |
| class IdeaAcpAgentViewModel( | ||
| val project: Project, | ||
| private val coroutineScope: CoroutineScope, | ||
| ) : Disposable { | ||
| val renderer = JewelRenderer() | ||
|
|
||
| private val _isExecuting = MutableStateFlow(false) | ||
| val isExecuting: StateFlow<Boolean> = _isExecuting.asStateFlow() | ||
|
|
||
| private val _isConnected = MutableStateFlow(false) | ||
| val isConnected: StateFlow<Boolean> = _isConnected.asStateFlow() | ||
|
|
||
| private val _connectionError = MutableStateFlow<String?>(null) | ||
| val connectionError: StateFlow<String?> = _connectionError.asStateFlow() | ||
|
|
||
| private val _stderrTail = MutableStateFlow<List<String>>(emptyList()) | ||
| val stderrTail: StateFlow<List<String>> = _stderrTail.asStateFlow() | ||
|
|
||
| private var process: Process? = null | ||
| private var protocol: Protocol? = null | ||
| private var client: Client? = null | ||
| private var session: ClientSession? = null | ||
|
|
||
| private var stderrJob: Job? = null | ||
| private var connectJob: Job? = null | ||
| private var currentPromptJob: Job? = null | ||
|
|
||
| private val receivedAnyAgentChunk = AtomicBoolean(false) | ||
| private val inThoughtStream = AtomicBoolean(false) | ||
|
|
||
| fun loadConfigFromSettings(): AcpAgentConfig { |
There was a problem hiding this comment.
The new IdeaAcpAgentViewModel adds non-trivial logic for process management, ACP session handling, and helpers like splitArgs/parseEnvLines but currently has no dedicated tests, whereas the analogous remote agent view model is covered by tests (e.g. mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.kt). Adding unit tests for connection failure paths, prompt flow handling, and the argument/env parsing helpers would improve confidence and prevent regressions in this integration.
🤖 Augment PR SummarySummary: Adds ACP (Agent Client Protocol) client support to the IntelliJ IDEA plugin via JSON-RPC over stdio. Changes:
Technical Notes: Uses 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| pb.redirectError(ProcessBuilder.Redirect.PIPE) | ||
| pb.environment().putAll(env) | ||
|
|
||
| acpLogger.info("Starting ACP agent process: ${pb.command().joinToString(" ")} (cwd=$cwd)") |
There was a problem hiding this comment.
acpLogger.info("Starting ACP agent process: ...") logs the full command line, which may accidentally include tokens/keys passed via args; consider redacting or logging only the executable name. (Guideline: no_sensitive_logging)
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| fun cancelTask() { | ||
| currentPromptJob?.cancel(CancellationException("Cancelled by user")) | ||
| currentPromptJob = null | ||
| _isExecuting.value = false |
There was a problem hiding this comment.
cancelTask() sets _isExecuting to false immediately, which can let the UI start a new prompt while the previous ACP turn is still cancelling and potentially interleave session state.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
| withContext(Dispatchers.IO) { | ||
| try { | ||
| p.waitFor() |
There was a problem hiding this comment.
disconnectInternal() calls p.waitFor() without a timeout; if the spawned agent ignores termination, disconnect/toolwindow disposal could block indefinitely on an IO thread.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| buf.append(ch) | ||
| escape = false | ||
| } | ||
| ch == '\\' -> { |
There was a problem hiding this comment.
splitArgs() treats every \\ as an escape character, which can break common Windows paths/arguments (e.g., C:\\Program Files\\...) even when the user didn’t intend escaping.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentViewModel.kt`:
- Around line 434-447: Replace the best-effort p.destroy() close with a forcible
shutdown: call p.destroyForcibly() and then wait for termination with a bounded
wait (e.g., p.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) inside the
withContext(Dispatchers.IO) block to avoid indefinite blocking; keep the
null-check on process/p, handle/ignore exceptions as before, and log or handle
the timeout case if waitFor returns false so cleanup is reliable in
IdeaAcpAgentViewModel (references: the process variable, local p, and the
withContext(Dispatchers.IO) wait block).
- Around line 230-239: The code currently force-unwraps session inside
currentPromptJob's coroutine (session!!) which can race with disconnect();
capture the session into a local val (e.g., val activeSession = session) before
launching the coroutine and use that local (activeSession) when calling
session.prompt so the coroutine uses a stable non-null reference; ensure you
handle the case where the captured session is null by aborting the
launch/returning early to avoid NPEs and keep disconnect() semantics intact.
🧹 Nitpick comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentContent.kt (1)
41-55: Multiple flow collectors with same key.Multiple
IdeaLaunchedEffectblocks share the same key (viewModel.rendererorviewModel), which could lead to unexpected behavior if the effect is restarted. Consider combining related collectors or using distinct keys.♻️ Optional: Combine related collectors
- IdeaLaunchedEffect(viewModel.renderer) { - viewModel.renderer.timeline.collect { timeline = it } - } - IdeaLaunchedEffect(viewModel.renderer) { - viewModel.renderer.currentStreamingOutput.collect { streamingOutput = it } - } + IdeaLaunchedEffect(viewModel.renderer) { + launch { viewModel.renderer.timeline.collect { timeline = it } } + launch { viewModel.renderer.currentStreamingOutput.collect { streamingOutput = it } } + }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentViewModel.kt (1)
450-460: Frequent list allocation in stderr tail collection.Each line read creates a new list via
_stderrTail.value + line. For high-volume stderr output, this could cause memory pressure. Consider using a more efficient approach.♻️ Optional: Use a bounded queue for efficiency
- private fun readStderrTail(p: Process, maxLines: Int = 200) { + private fun readStderrTail(p: Process, maxLines: Int = 200) { + val buffer = ArrayDeque<String>(maxLines) try { BufferedReader(InputStreamReader(p.errorStream)).useLines { lines -> lines.forEach { line -> - _stderrTail.value = (_stderrTail.value + line).takeLast(maxLines) + if (buffer.size >= maxLines) buffer.removeFirst() + buffer.addLast(line) + _stderrTail.value = buffer.toList() } } } catch (e: Exception) { // Ignore } }
| currentPromptJob = coroutineScope.launch(Dispatchers.IO) { | ||
| try { | ||
| receivedAnyAgentChunk.set(false) | ||
| inThoughtStream.set(false) | ||
|
|
||
| val flow = session!!.prompt( | ||
| listOf(ContentBlock.Text(text, Annotations(), JsonNull)), | ||
| JsonNull | ||
| ) | ||
|
|
There was a problem hiding this comment.
Potential race condition with session access.
The session!! force unwrap on line 235 could cause an NPE if disconnect() is called concurrently between the null check on line 220 and the access on line 235. Consider capturing the session reference locally.
🛡️ Proposed fix to avoid race condition
fun sendMessage(text: String) {
if (_isExecuting.value) return
- if (!_isConnected.value || session == null) {
+ val currentSession = session
+ if (!_isConnected.value || currentSession == null) {
renderer.renderError("ACP agent is not connected. Please start it first.")
return
}
renderer.clearError()
renderer.addUserMessage(text)
_isExecuting.value = true
currentPromptJob?.cancel()
currentPromptJob = coroutineScope.launch(Dispatchers.IO) {
try {
receivedAnyAgentChunk.set(false)
inThoughtStream.set(false)
- val flow = session!!.prompt(
+ val flow = currentSession.prompt(
listOf(ContentBlock.Text(text, Annotations(), JsonNull)),
JsonNull
)🤖 Prompt for AI Agents
In
`@mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentViewModel.kt`
around lines 230 - 239, The code currently force-unwraps session inside
currentPromptJob's coroutine (session!!) which can race with disconnect();
capture the session into a local val (e.g., val activeSession = session) before
launching the coroutine and use that local (activeSession) when calling
session.prompt so the coroutine uses a stable non-null reference; ensure you
handle the case where the captured session is null by aborting the
launch/returning early to avoid NPEs and keep disconnect() semantics intact.
| val p = process | ||
| process = null | ||
| if (p != null) { | ||
| try { | ||
| p.destroy() | ||
| } catch (_: Exception) { | ||
| } | ||
| withContext(Dispatchers.IO) { | ||
| try { | ||
| p.waitFor() | ||
| } catch (_: Exception) { | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Prefer destroyForcibly() for process termination.
Using destroy() may not reliably terminate the child process on all platforms. Consider using destroyForcibly() followed by waitFor() with a timeout to ensure cleanup.
🛡️ Proposed fix for reliable process termination
val p = process
process = null
if (p != null) {
try {
- p.destroy()
+ p.destroyForcibly()
} catch (_: Exception) {
}
withContext(Dispatchers.IO) {
try {
- p.waitFor()
+ p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS)
} catch (_: Exception) {
}
}
}📝 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.
| val p = process | |
| process = null | |
| if (p != null) { | |
| try { | |
| p.destroy() | |
| } catch (_: Exception) { | |
| } | |
| withContext(Dispatchers.IO) { | |
| try { | |
| p.waitFor() | |
| } catch (_: Exception) { | |
| } | |
| } | |
| } | |
| val p = process | |
| process = null | |
| if (p != null) { | |
| try { | |
| p.destroyForcibly() | |
| } catch (_: Exception) { | |
| } | |
| withContext(Dispatchers.IO) { | |
| try { | |
| p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) | |
| } catch (_: Exception) { | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/acp/IdeaAcpAgentViewModel.kt`
around lines 434 - 447, Replace the best-effort p.destroy() close with a
forcible shutdown: call p.destroyForcibly() and then wait for termination with a
bounded wait (e.g., p.waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) inside the
withContext(Dispatchers.IO) block to avoid indefinite blocking; keep the
null-check on process/p, handle/ignore exceptions as before, and log or handle
the timeout case if waitFor returns false so cleanup is reliable in
IdeaAcpAgentViewModel (references: the process variable, local p, and the
withContext(Dispatchers.IO) wait block).
1 participant
Summary
session/updateinto the existing timeline renderer.command/args/envin IDEA settings.Notes
Test plan
Refs: #535
Summary by CodeRabbit