Repository navigation
Conversation
Adds an ACP stdio client and chat runtime adapter behind SUPERSET_EXPERIMENTAL_ACP_CHAT so existing Mastra chat remains the default. The adapter launches a configurable ACP command, initializes protocol v1, creates an ACP session, streams session/update message and tool state into the existing chat snapshot shape, handles cancellation, and bridges ACP permission requests to the existing approval UI.\n\nDefault ACP command is 'omp acp'; override with SUPERSET_ACP_COMMAND and SUPERSET_ACP_ARGS.\n\nVerification:\n- bun test packages/host-service/src/runtime/chat/acp-chat-runtime.test.ts packages/host-service/test/integration/chat.integration.test.ts\n- bun run --cwd packages/host-service typecheck\n- bun run lint
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new ACP (agent control plane) chat backend. A ChangesACP Chat Runtime
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant ChatRuntimeManager
participant AcpChatRuntime
participant StdioAcpAgentClient
participant AgentProcess
Client->>ChatRuntimeManager: sendMessage(payload)
ChatRuntimeManager->>AcpChatRuntime: sendMessage(payload)
AcpChatRuntime->>StdioAcpAgentClient: prompt(sessionId, acpBlocks)
StdioAcpAgentClient->>AgentProcess: session/prompt (stdin)
AgentProcess-->>StdioAcpAgentClient: session/update notifications (stdout)
StdioAcpAgentClient->>AcpChatRuntime: onUpdate → appendAcpUpdateToDisplayState
AgentProcess->>StdioAcpAgentClient: session/request_permission (stdout)
StdioAcpAgentClient->>AcpChatRuntime: onPermissionRequest → pendingApproval set
Client->>ChatRuntimeManager: respondToApproval(decision)
ChatRuntimeManager->>AcpChatRuntime: respondToApproval(decision)
AcpChatRuntime->>AcpChatRuntime: selectPermissionOption → resolve waiter
StdioAcpAgentClient-->>AgentProcess: permission result (stdin)
AgentProcess-->>StdioAcpAgentClient: stopReason response (stdout)
StdioAcpAgentClient-->>AcpChatRuntime: prompt resolves
AcpChatRuntime->>AcpChatRuntime: finishCurrentAssistantMessage
AcpChatRuntime-->>ChatRuntimeManager: stopReason
ChatRuntimeManager-->>Client: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Ready to review this PR? Stage has broken it down into 7 individual chapters for you: 
Chapters generated by Stage for commit 92e5a38 on Jun 28, 2026 12:36am UTC. |
Contributor
Greptile SummaryThis PR adds an experimental ACP chat runtime behind an environment flag. The main changes are:
Confidence Score: 4/5The ACP runtime path has lifecycle bugs that can corrupt streamed chat state, hang stopped prompts, and leak runtimes.
packages/host-service/src/runtime/chat/acp-chat-runtime.ts, packages/host-service/src/runtime/chat/chat.ts, packages/host-service/src/runtime/chat/acp-protocol.ts
|
| Filename | Overview |
|---|---|
| packages/host-service/src/runtime/chat/acp-chat-runtime.ts | Adds the ACP session runtime, including prompt sending, cancellation, approval handling, and chat state storage. |
| packages/host-service/src/runtime/chat/stdio-acp-client.ts | Adds the stdio JSON-RPC client used to start the ACP process and route requests, responses, notifications, and permission requests. |
| packages/host-service/src/runtime/chat/acp-protocol.ts | Adds conversion between Superset chat payloads/state and ACP prompt/update shapes. |
| packages/host-service/src/runtime/chat/chat.ts | Updates ChatRuntimeManager to keep Mastra as the default path and switch to ACP when the experimental flag is enabled. |
| packages/host-service/src/runtime/chat/acp-types.ts | Adds shared ACP chat runtime types for messages, display state, approvals, questions, and plans. |
| packages/host-service/src/runtime/chat/acp-chat-runtime.test.ts | Adds tests for sequential ACP streaming and permission approval behavior. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Chat UI
participant M as ChatRuntimeManager
participant R as AcpChatRuntime
participant C as StdioAcpAgentClient
participant A as ACP agent
UI->>M: sendMessage(sessionId)
M->>R: sendMessage(payload)
R->>C: session/prompt
C->>A: JSON-RPC prompt
A-->>C: session/update
C-->>R: onUpdate(update)
R->>R: append to displayState
UI->>M: stop(sessionId)
M->>R: stop()
R->>C: session/cancel
R->>R: finish as cancelled
A-->>C: prompt response may be late or absent
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as Chat UI
participant M as ChatRuntimeManager
participant R as AcpChatRuntime
participant C as StdioAcpAgentClient
participant A as ACP agent
UI->>M: sendMessage(sessionId)
M->>R: sendMessage(payload)
R->>C: session/prompt
C->>A: JSON-RPC prompt
A-->>C: session/update
C-->>R: onUpdate(update)
R->>R: append to displayState
UI->>M: stop(sessionId)
M->>R: stop()
R->>C: session/cancel
R->>R: finish as cancelled
A-->>C: prompt response may be late or absent
Comments Outside Diff (1)
-
packages/host-service/src/runtime/chat/acp-protocol.ts, line 641-647 (link)Numeric Tool IDs Disconnect Results
tool_call_updateignores anytoolCallIdthat is not already a string. If an ACP agent sends numeric tool IDs, the earliertool_callpath creates a random ID, but the update path drops the numeric ID entirely, so the tool result is never attached and the tool can stay visible as running.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/host-service/src/runtime/chat/acp-protocol.ts Line: 641-647 Comment: **Numeric Tool IDs Disconnect Results** `tool_call_update` ignores any `toolCallId` that is not already a string. If an ACP agent sends numeric tool IDs, the earlier `tool_call` path creates a random ID, but the update path drops the numeric ID entirely, so the tool result is never attached and the tool can stay visible as running. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 5
packages/host-service/src/runtime/chat/acp-chat-runtime.ts:92-112
**Concurrent Prompts Share State**
When two `sendMessage` mutations reach the same ACP session before the first prompt resolves, both prompts append updates into the same `displayState.currentMessage`. The UI can show one assistant message with mixed chunks or tool calls from two user turns, and whichever prompt finishes first can finalize the shared message for the wrong turn.
### Issue 2 of 5
packages/host-service/src/runtime/chat/acp-chat-runtime.ts:134-143
**Cancelled Prompt Still Pending**
`stop()` sends `session/cancel` and marks the local turn cancelled, but it does not settle the pending `session/prompt` request. If the ACP agent stops streaming without replying to the original prompt request, the `sendMessage` mutation can hang indefinitely; if it replies later, the old prompt completion can run after the UI already recorded a cancelled turn.
### Issue 3 of 5
packages/host-service/src/runtime/chat/chat.ts:375-381
**Disposal Uses Current Flag**
`disposeRuntime` re-reads `SUPERSET_EXPERIMENTAL_ACP_CHAT` instead of disposing the runtime that actually owns the session. If the flag changes during the process lifetime, a Mastra session can be looked up in the ACP map, or an ACP session in the Mastra map, causing the method to return without aborting the real runtime or killing the ACP child process.
### Issue 4 of 5
packages/host-service/src/runtime/chat/acp-protocol.ts:225-233
**Plans Never Become Approvals**
ACP `plan` updates are converted into plain assistant text, while `pendingPlanApproval` remains `null` for the whole ACP display state. An agent that sends a plan requiring approval will show the plan text but no approve or reject controls, so the plan workflow cannot be completed from the existing chat UI.
### Issue 5 of 5
packages/host-service/src/runtime/chat/acp-protocol.ts:641-647
**Numeric Tool IDs Disconnect Results**
`tool_call_update` ignores any `toolCallId` that is not already a string. If an ACP agent sends numeric tool IDs, the earlier `tool_call` path creates a random ID, but the update path drops the numeric ID entirely, so the tool result is never attached and the tool can stay visible as running.
Reviews (1): Last reviewed commit: "feat(chat): add experimental ACP runtime" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/host-service/src/runtime/chat/acp-chat-runtime.test.ts`:
- Around line 16-52: Ensure each AcpChatRuntime instance is always cleaned up by
moving runtime.dispose() into a finally block in the affected tests. In the
AcpChatRuntime test cases that call initialize(), sendMessage(), listMessages(),
and getDisplayState(), wrap the assertions in try/finally so dispose() runs even
if an assertion fails, preventing leaked stdio child processes and test
interference.
- Around line 98-109: The waitFor helper currently fails after a fixed number of
event-loop turns instead of a real elapsed-time limit, which can make the
acp-chat-runtime permission test flaky on slower machines. Update waitFor to
measure wall-clock time with an explicit timeout budget and keep polling until
either predicate() passes or that duration elapses; use the existing
nextEventLoopTurn helper for yielding, and keep the timeout error in waitFor
descriptive so it still identifies the condition that timed out.
In `@packages/host-service/src/runtime/chat/acp-chat-runtime.ts`:
- Around line 57-66: The ACP error path in ACPChatRuntime’s onError currently
finalizes the turn but leaves pendingPermission alive, so clear or cancel that
JSON-RPC permission waiter before calling finishCurrentAssistantMessage. Update
the error handling in acp-chat-runtime’s onError flow (and the related
terminal-failure path around the same ACP prompt handling) to explicitly
resolve/reject and null out pendingPermission whenever the prompt fails,
especially when displayState.isRunning is true.
- Around line 233-246: Update the approval resolution logic in
acp-chat-runtime’s option-matching flow so always_allow_category does not fall
back to allow_once or generic “allow” labels. In the preferredKinds and
labelNeedles handling used to select a chat approval option, keep
always_allow_category restricted to explicit permanent/always choices only, and
if no such option exists, return an error or failure instead of silently
treating it as a one-time approval. Use the decision-based matching around the
existing option selection helpers to preserve the semantics of
always_allow_category.
- Around line 201-206: The permission waiter in ACPChatRuntime is being
overwritten when a new request arrives, which leaves the previous Promise
unresolved. Update the pending-permission flow in ACPChatRuntime so it checks
for an existing this.pendingPermission before assigning a new waiter; if one is
already active, reject or return the new request, or enqueue it, but do not
replace the current resolver. Use the existing pendingPermission, resolve, and
toolCallId handling to keep only one active permission request at a time.
In `@packages/host-service/src/runtime/chat/acp-protocol.ts`:
- Around line 199-221: The tool_call_update handling in acp-protocol should
preserve the original tool input instead of overwriting it with an empty object
when progress updates omit content. Update the logic in the tool update branch
to reuse the existing entry from state.activeTools or the original args/input
for the same toolCallId, and only replace input when update.content is actually
present. Keep the behavior in ensureCurrentAssistantMessage, findToolName, and
state.activeTools consistent so non-terminal updates do not erase the renderer’s
call context.
In `@packages/host-service/src/runtime/chat/chat.ts`:
- Around line 562-568: The ACP control-only handlers in chat.ts are still
creating runtimes via getOrCreateAcpRuntime, which can spawn a new acp process
for stale stop/response actions. Update restartFromMessage, stop,
respondToApproval, respondToQuestion, and respondToPlan to use an
existing-runtime lookup path instead of creating one, and keep
restartFromMessage throwing the unsupported restart error without initializing
ACP. Use the chat runtime methods around shouldUseAcpChat(),
getOrCreateAcpRuntime, and the ACP approval/question/plan handlers as the places
to refactor.
In `@packages/host-service/src/runtime/chat/stdio-acp-client.ts`:
- Around line 173-183: The request flow in stdio-acp-client’s request method can
leave entries stuck in pending forever when the agent stops responding. Add a
per-request timeout in request() that rejects the promise and removes the
pending entry if no response arrives in time, and make sure the timeout is
cleared when a reply or write failure resolves the request. Apply the fix
through the request/pending handling used by initialize(), newSession(), and
prompt().
- Around line 124-132: The shutdown logic in the stdio ACP client uses
child.killed to decide whether to escalate, but that flag is set as soon as
SIGTERM is sent and does not confirm the process exited. Update the kill path in
the client shutdown flow to track actual exit state separately via the existing
child.once("exit") handling (or an explicit exited flag), and use that state to
decide whether to send the SIGKILL fallback. Keep the timeout/promise flow
intact, but only suppress escalation after the child has really exited.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b365ab39-41e0-46e1-9359-7d7308e05a27
📒 Files selected for processing (6)
packages/host-service/src/runtime/chat/acp-chat-runtime.test.tspackages/host-service/src/runtime/chat/acp-chat-runtime.tspackages/host-service/src/runtime/chat/acp-protocol.tspackages/host-service/src/runtime/chat/acp-types.tspackages/host-service/src/runtime/chat/chat.tspackages/host-service/src/runtime/chat/stdio-acp-client.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found across 6 files
Architecture diagram
sequenceDiagram
participant UI as Chat UI (Renderer)
participant TRPC as tRPC Router
participant Manager as ChatRuntimeManager
participant Mastra as MastraRuntime (unchanged)
participant ACP as AcpChatRuntime (NEW)
participant Client as StdioAcpAgentClient (NEW)
participant Agent as ACP Agent Process (NEW)
participant Protocol as acp-protocol (NEW)
Note over UI,Agent: Experimental ACP Chat Flow (only when flag enabled)
UI->>TRPC: sendMessage(sessionId, payload)
TRPC->>Manager: sendMessage(sessionId, payload)
alt SUPERSET_EXPERIMENTAL_ACP_CHAT=1
Manager->>ACP: route to AcpChatRuntime
Note over ACP,Agent: NEW: Session creation & initialization
ACP->>Client: initialize()
Client->>Agent: spawn(child_process, stdio)
Agent-->>Client: stderr/sidestream
Client->>Agent: JSON-RPC: initialize(protocolVersion, clientCapabilities)
Agent-->>Client: {result: protocolVersion, agentCapabilities}
Client-->>ACP: initialized
ACP->>Client: newSession(cwd)
Client->>Agent: JSON-RPC: session/new(cwd, mcpServers)
Agent-->>Client: {sessionId: "acp-session-1"}
Client-->>ACP: sessionId
Note over ACP,Protocol: NEW: sendMessage with streaming
ACP->>Client: prompt(sessionId, promptBlocks)
Client->>Agent: JSON-RPC: session/prompt(sessionId, prompt)
Agent-->>Client: session/update (agent_message_chunk) "Hello "
Agent-->>Client: session/update (agent_thought_chunk) "...thinking..."
Agent-->>Client: session/update (tool_call) id="tool-1", title="read", input={path:"README.md"}
Agent-->>Client: session/update (tool_call_update) id="tool-1", status="completed"
Client-->>ACP: update notification
Note over ACP,Protocol: appendAcpUpdateToDisplayState()
Protocol-->>ACP: returns updated ChatDisplayState
opt ACP requests permission
Agent-->>Client: session/request_permission (toolCall, options)
Client-->>ACP: onPermissionRequest
ACP-->>UI: pendingApproval set in display state
UI->>ACP: respondToApproval({decision: "approve"})
ACP-->>Agent: resolve permission (selected option)
end
Agent-->>Client: session/prompt result {stopReason: "end_turn"}
Client-->>ACP: finished
ACP->>ACP: finishCurrentAssistantMessage()
ACP-->>TRPC: {stopReason: "end_turn"}
opt User stops
UI->>TRPC: stop(sessionId)
TRPC->>ACP: stop()
ACP->>Client: cancel(sessionId)
Client->>Agent: JSON-RPC: session/cancel
ACP->>ACP: finishCurrentAssistantMessage(stopReason: "cancelled")
end
opt ACP process error or exit
Agent--xClient: child process exits unexpectedly
Client-->>ACP: onError(error)
ACP->>ACP: finishCurrentAssistantMessage(stopReason: "error", errorMessage)
end
else default (Mastra)
Manager->>Mastra: route to existing MastraRuntime
Note over Mastra: Unchanged flow (not detailed)
end
TRPC-->>UI: result (shape unchanged)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
werkamsus
marked this pull request as draft
June 27, 2026 23:52
Adds the ACP chat runtime toggle to Desktop Settings > Experimental, persists it in local SQLite settings, and restarts active host services so the host-service env picks it up.\n\nAddresses PR review feedback by serializing ACP prompts, settling prompts on stop, avoiding runtime creation for control-only actions, disposing the runtime owner independent of current flags, clearing pending permission waiters on terminal failures, preserving tool inputs, supporting numeric tool ids, adding request timeouts, and hardening child process cleanup.\n\nUpdates the PR body to the repo template and keeps PR ParadevAI#5376 in draft.\n\nVerification:\n- bun test packages/host-service/src/runtime/chat/acp-chat-runtime.test.ts packages/host-service/src/runtime/chat/acp-protocol.test.ts packages/host-service/test/integration/chat.integration.test.ts\n- bun run --cwd packages/host-service typecheck\n- bun run --cwd packages/local-db typecheck\n- bun run lint
Contributor
Author
|
Follow-up pushed in 5574c90 and PR is now draft. I moved the ACP runtime flag into Settings → Experimental, updated the PR body to the repo template, and addressed the valid review findings: prompt serialization/cancel settling, control-only runtime lookup, flag-independent disposal, permission cleanup/guarding, always-allow matching, numeric tool IDs, tool input preservation, request timeouts, child exit cleanup, and test disposal. I did not turn ACP plan updates into pending plan approvals because ACP plan updates are informational protocol updates with no approval response path; faking that would leave the agent uninformed. |
Ensure LocalHostServiceProvider starts a host-service child for the session active organization even when the synced organization collection has not populated yet. This keeps local workspace creation unblocked after app startup.
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds an experimental ACP-backed chat runtime for Superset workspace chat while keeping the existing Mastra chat runtime as the default.
This PR is intentionally additive:
SUPERSET_EXPERIMENTAL_ACP_CHATremains the host-service runtime flag.omp acp; advanced users can still override withSUPERSET_ACP_COMMANDandSUPERSET_ACP_ARGS.Research notes:
initialize,session/new,session/prompt,session/cancel, andsession/updatenotifications.omp acpas the stdio entrypoint.Related Issues
Related to #3350
Related to #2019
Related to #4628
Related to #5326
Type of Change
Testing
Ran:
bun test packages/host-service/src/runtime/chat/acp-chat-runtime.test.ts packages/host-service/src/runtime/chat/acp-protocol.test.ts packages/host-service/test/integration/chat.integration.test.ts bun run --cwd packages/host-service typecheck bun run lintAlso ran:
Screenshots (if applicable)
Not included yet. The PR is draft while the ACP setting and review feedback fixes are being finalized.
Additional Notes
This PR is draft for now.
Follow-up commit addresses review feedback from CodeRabbit/Greptile/Cubic:
stop()always_allow_categorysemantics