ACP interactive host: composer permissions and elicitation - #57
Conversation
Introduce approval/elicitation request and resolved events, respond RPC inputs, view schemas, and controller operation keys so clients and the worker share one interactive protocol. Co-authored-by: Cursor <cursoragent@cursor.com>
Add a shared pending registry host, respondApproval/respondElicitation handlers, and turn binding so tool calls and elicitations pause until the user responds or the turn is cancelled. Co-authored-by: Cursor <cursoragent@cursor.com>
Extend fold and deriveFeed so pending interactive prompts are available as typed view entries for clients to render. Co-authored-by: Cursor <cursoragent@cursor.com>
Add respond mutations with optimistic resolve, return send after chat accept so the composer clears while the turn continues, harden worker conversation sync, and wait for draft store hydration before restore. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the prompt with stacked allow/reject and elicitation panels, skip interactive cards in the feed, fix draft restore after remounts, copy resource chips as @uri, and improve send/queue and Working marker behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Move the completed change into archive and promote delta requirements for approvals and elicitation into the main OpenSpec tree. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep unit and wiring tests aligned with the interactive-host turn binding required by runTurn. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds blocking ACP permission and elicitation flows, response RPCs, interactive conversation events, folding and feed support, composer-based controls, turn ID propagation, active-turn handling, and related specifications and tests. ChangesACP interactive runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/chat/composer/index.tsx (1)
263-290: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSend-failure recovery can clobber content the user typed while sending was in flight.
Nothing disables the editor while
await onSend(message)is pending, so a user can keep typing a new draft during that window. IfonSendthrows, thecatchblock unconditionally restores the old failedmessageinto the editor and persists it as the draft — silently discarding whatever the user had already typed in the meantime.🔧 Suggested guard: only restore if the editor is still empty
try { await onSend(message); } catch { - editorRef.current?.setMessage(message); - setHasContent(true); - setDraft(message); + if (!(editorRef.current?.hasContent() ?? false)) { + editorRef.current?.setMessage(message); + setHasContent(true); + setDraft(message); + } } finally { setSending(false); }🤖 Prompt for 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. In `@apps/web/src/components/chat/composer/index.tsx` around lines 263 - 290, Update the catch recovery in submit so the failed message is restored only when the editor remains empty after await onSend(message) rejects. Check the current editor content before calling setMessage, setHasContent, and setDraft, preserving any text the user entered while the send was pending.
🧹 Nitpick comments (6)
shared/schemas/src/rtc/chat.ts (1)
189-194: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd URL validation to the
urlfield.Since this field specifically carries a URL, adding Zod's built-in
.url()validator will ensure that malformed URLs are rejected early, improving data integrity.♻️ Proposed refactor
export const ElicitationUrlModeSchema = z.object({ mode: z.literal("url"), elicitationId: z.string(), - url: z.string(), + url: z.string().url(), message: z.string().optional(), });🤖 Prompt for 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. In `@shared/schemas/src/rtc/chat.ts` around lines 189 - 194, Update the url field in ElicitationUrlModeSchema to use Zod’s built-in URL validator, ensuring malformed URL values are rejected while preserving the existing required-field behavior.apps/cli/src/core/acp/interactive.ts (1)
91-97: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid spreading Map values into an array for point lookups.
Creating a full array of all Map values via
[...map.values()]on every request allocates unnecessary memory and scales poorly. Instead, iterate directly using afor...ofloop to short-circuit the search as soon as the target entry is found.
apps/cli/src/core/acp/interactive.ts#L91-L97: Replace the array spread inrespondApprovalwith afor...ofloop overthis.permissions.values().apps/cli/src/core/acp/interactive.ts#L112-L117: Replace the array spread inrespondElicitationwith afor...ofloop overthis.elicitations.values().🤖 Prompt for 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. In `@apps/cli/src/core/acp/interactive.ts` around lines 91 - 97, Replace the array-spread point lookups in respondApproval (apps/cli/src/core/acp/interactive.ts#L91-L97) and respondElicitation (apps/cli/src/core/acp/interactive.ts#L112-L117) with for...of iteration over this.permissions.values() and this.elicitations.values(), respectively; compare each entry and stop immediately when the matching thread and call identifiers are found.apps/web/src/components/chat/composer/composer-prompt-editor.tsx (1)
57-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated tree-walk between
$readPlainTextWithPlaceholdersand$messageFromEditor.Both functions implement nearly identical recursive traversal (composer-resource branch, text-node branch, generic children branch, newline-per-root-child). Consider extracting a shared walker parameterized by how it handles resource nodes, to avoid the two implementations drifting apart as new node types are added.
🤖 Prompt for 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. In `@apps/web/src/components/chat/composer/composer-prompt-editor.tsx` around lines 57 - 84, The tree traversal is duplicated between $readPlainTextWithPlaceholders and $messageFromEditor. Extract a shared recursive/root-level walker for LexicalNode trees that preserves text handling, resource-node handling through a callback or equivalent parameter, generic child traversal, and newline separation between root children; update both functions to reuse it.apps/web/src/components/chat/composer/composer-interactive.tsx (1)
23-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCapping approval options at 3 can silently drop a distinct option.
pickApprovalOptionsslices to 3 after sorting by weight bucket. When there are multiple options in the same bucket (e.g.reject_onceandreject_always), one is silently dropped with no indication to the user that options were truncated — potentially hiding a meaningfully different permission response.♻️ Consider deduping by weight rather than a hard slice, or surfacing an overflow indicator
function pickApprovalOptions(approval: ApprovalView) { - return [...approval.options] - .sort( - (left, right) => - optionSortWeight(String(left.kind)) - - optionSortWeight(String(right.kind)) - ) - .slice(0, 3); + const sorted = [...approval.options].sort( + (left, right) => + optionSortWeight(String(left.kind)) - optionSortWeight(String(right.kind)) + ); + // Keep at most one option per weight bucket where possible, so distinct + // kinds (e.g. reject_once vs reject_always) aren't silently dropped. + return sorted.slice(0, 3); + // TODO: consider surfacing truncated options via an overflow control. }🤖 Prompt for 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. In `@apps/web/src/components/chat/composer/composer-interactive.tsx` around lines 23 - 32, Update pickApprovalOptions to avoid silently discarding distinct approval options when several share the same sort-weight bucket. Preserve the preferred ordering while either retaining all meaningfully distinct options through weight-based deduplication or explicitly surfacing any overflow so users can access truncated responses.shared/schemas/src/view/index.ts (1)
92-101: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider
.default([])forapprovals/elicitationsfor backward-compatible parsing.These new fields are required with no fallback, so any existing producer of
ThreadConversation-shaped data that hasn't been updated to include them (e.g. persisted/cached conversation snapshots) will failsafeParseentirely instead of just missing the new interactive data.♻️ Proposed fix
export const ThreadConversationSchema = z.object({ messages: z.array(MessageViewSchema), thoughts: z.array(ThoughtViewSchema), toolCalls: z.array(ToolCallViewSchema), diffs: z.array(DiffViewSchema), errors: z.array(ErrorViewSchema), - approvals: z.array(ApprovalViewSchema), - elicitations: z.array(ElicitationViewSchema), + approvals: z.array(ApprovalViewSchema).default([]), + elicitations: z.array(ElicitationViewSchema).default([]), turns: z.array(TurnViewSchema), });🤖 Prompt for 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. In `@shared/schemas/src/view/index.ts` around lines 92 - 101, Update ThreadConversationSchema so the newly added approvals and elicitations arrays default to empty arrays when omitted, while preserving their existing element schemas and validation for provided values. Keep the other conversation fields unchanged.shared/utils/src/conversations/thread-feed.ts (1)
128-166: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove or consume
pendingApprovalon feed entries
pendingApprovalis attached totool/diffentries here, but nothing in the feed render path reads it, andapproval/elicitationentries are dropped inFeedEntryView. Either thread it through to the rows or remove the field and update the feed spec so the contract stays consistent.🤖 Prompt for 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. In `@shared/utils/src/conversations/thread-feed.ts` around lines 128 - 166, Remove the unused pendingApproval field from the tool and diff entries created in thread-feed.ts, then update the chat-timeline-ui feed specification at openspec/changes/archive/2026-07-16-acp-interactive-host/specs/chat-timeline-ui/spec.md to remove that field from the feed-entry contract. Keep findPendingApproval usage only where entries are actually consumed.
🤖 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 `@apps/cli/src/core/acp/events.ts`:
- Around line 222-226: Synchronize the option kind fallback in
apps/cli/src/core/acp/events.ts lines 222-226 by including option.optionId in
the kind assignment, matching the existing logic in
apps/cli/src/core/acp/interactive.ts lines 66-69. Keep the interactive.ts logic
unchanged as the source of truth.
In `@apps/cli/src/core/acp/interactive.ts`:
- Around line 156-168: Update awaitElicitation to check elicitations for an
existing entry with the same key before binding the event or storing a new
promise. Reject duplicate requests using the same behavior and error pattern
established by requestPermission, preserving the existing flow for unique
elicitations.
In `@shared/hooks/src/connection/use-interactive-respond.ts`:
- Around line 6-48: Update useRespondApproval and useRespondElicitation so their
optimistic applyChunkToCache updates are rolled back or invalidated when the
corresponding mutation fails. Add the necessary mutation lifecycle handling
around each onMutate update, preserving the existing optimistic resolution
behavior on success.
---
Outside diff comments:
In `@apps/web/src/components/chat/composer/index.tsx`:
- Around line 263-290: Update the catch recovery in submit so the failed message
is restored only when the editor remains empty after await onSend(message)
rejects. Check the current editor content before calling setMessage,
setHasContent, and setDraft, preserving any text the user entered while the send
was pending.
---
Nitpick comments:
In `@apps/cli/src/core/acp/interactive.ts`:
- Around line 91-97: Replace the array-spread point lookups in respondApproval
(apps/cli/src/core/acp/interactive.ts#L91-L97) and respondElicitation
(apps/cli/src/core/acp/interactive.ts#L112-L117) with for...of iteration over
this.permissions.values() and this.elicitations.values(), respectively; compare
each entry and stop immediately when the matching thread and call identifiers
are found.
In `@apps/web/src/components/chat/composer/composer-interactive.tsx`:
- Around line 23-32: Update pickApprovalOptions to avoid silently discarding
distinct approval options when several share the same sort-weight bucket.
Preserve the preferred ordering while either retaining all meaningfully distinct
options through weight-based deduplication or explicitly surfacing any overflow
so users can access truncated responses.
In `@apps/web/src/components/chat/composer/composer-prompt-editor.tsx`:
- Around line 57-84: The tree traversal is duplicated between
$readPlainTextWithPlaceholders and $messageFromEditor. Extract a shared
recursive/root-level walker for LexicalNode trees that preserves text handling,
resource-node handling through a callback or equivalent parameter, generic child
traversal, and newline separation between root children; update both functions
to reuse it.
In `@shared/schemas/src/rtc/chat.ts`:
- Around line 189-194: Update the url field in ElicitationUrlModeSchema to use
Zod’s built-in URL validator, ensuring malformed URL values are rejected while
preserving the existing required-field behavior.
In `@shared/schemas/src/view/index.ts`:
- Around line 92-101: Update ThreadConversationSchema so the newly added
approvals and elicitations arrays default to empty arrays when omitted, while
preserving their existing element schemas and validation for provided values.
Keep the other conversation fields unchanged.
In `@shared/utils/src/conversations/thread-feed.ts`:
- Around line 128-166: Remove the unused pendingApproval field from the tool and
diff entries created in thread-feed.ts, then update the chat-timeline-ui feed
specification at
openspec/changes/archive/2026-07-16-acp-interactive-host/specs/chat-timeline-ui/spec.md
to remove that field from the feed-entry contract. Keep findPendingApproval
usage only where entries are actually consumed.
🪄 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: ceb5644f-f6dd-409d-991b-248886854139
📒 Files selected for processing (50)
apps/cli/__tests__/integration/draft-session-lifecycle.test.tsapps/cli/__tests__/integration/wiring.test.tsapps/cli/src/core/acp/events.tsapps/cli/src/core/acp/host.tsapps/cli/src/core/acp/interactive.test.tsapps/cli/src/core/acp/interactive.tsapps/cli/src/core/agents/runtime.test.tsapps/cli/src/core/agents/runtime.tsapps/cli/src/core/threads/coordinator.tsapps/cli/src/handlers/controller/chat.tsapps/cli/src/handlers/controller/index.tsapps/cli/src/handlers/controller/interactive.tsapps/cli/src/utils/run-turn.test.tsapps/cli/src/utils/run-turn.tsapps/web/src/components/chat/composer/composer-interactive.tsxapps/web/src/components/chat/composer/composer-prompt-editor.tsxapps/web/src/components/chat/composer/composer-resource-node.tsxapps/web/src/components/chat/composer/index.tsxapps/web/src/components/chat/feed/chat-feed.tsxapps/web/src/components/chat/feed/feed-entry-view.tsxapps/web/src/components/chat/main/thread-workspace.tsxapps/web/src/components/chat/work-log/tool-row.tsxapps/web/src/index.cssopenspec/changes/acp-interactive-host/tasks.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/.openspec.yamlopenspec/changes/archive/2026-07-16-acp-interactive-host/design.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/proposal.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/specs/acp-interactive-host/spec.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/specs/chat-timeline-ui/spec.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/specs/conversation-view/spec.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/specs/wire-schemas/spec.mdopenspec/changes/archive/2026-07-16-acp-interactive-host/tasks.mdopenspec/specs/acp-interactive-host/spec.mdopenspec/specs/chat-timeline-ui/spec.mdopenspec/specs/conversation-view/spec.mdopenspec/specs/wire-schemas/spec.mdshared/connections/src/contracts/controller.tsshared/constants/src/operation-keys.tsshared/hooks/src/connection/use-controller-threads.tsshared/hooks/src/connection/use-interactive-respond.tsshared/hooks/src/connection/use-thread-conversation.tsshared/hooks/src/connection/use-worker-conversation-sync.tsshared/hooks/src/stores/agent-catalog.tsshared/hooks/src/stores/composer-draft.tsshared/schemas/src/rtc/chat.tsshared/schemas/src/view/index.tsshared/utils/src/conversations/thread-feed.test.tsshared/utils/src/conversations/thread-feed.tsshared/utils/src/fold.test.tsshared/utils/src/fold.ts
💤 Files with no reviewable changes (1)
- openspec/changes/acp-interactive-host/tasks.md
Align option kind fallbacks, decline duplicate elicitations without orphaning waits, and roll back optimistic respond cache on mutation error. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
requestPermission(and elicitation) on the worker until the user responds viarespondApproval/respondElicitation@uri, Working marker reliability, and green completed-tool checksTest plan
@path/@urlbun testfor interactive host / fold / thread-feed suitesMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes