Repository navigation
feat(mobile): live ACP agent sessions end to end (host harness, mobile UI, desktop gate) - #5580
Merged
saddlepaddle merged 14 commits intoJul 11, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an ACP live-session system spanning shared protocol contracts, host runtime and streaming APIs, relay transport, desktop enablement, and mobile session browsing/chat UI. It also adds deterministic integration coverage, database settings support, presentation updates, and implementation plans. ChangesACP live sessions
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Mobile as Mobile ACP UI
participant HostClient as Host client transport
participant HostService as Host service
participant Manager as AcpSessionManager
participant Adapter as claude-agent-acp
Mobile->>HostClient: Create or load session
HostClient->>HostService: Call acpSessions procedure
HostService->>Manager: Create, query, or prompt session
Manager->>Adapter: Send ACP request
Adapter-->>Manager: Emit session updates
Manager-->>HostService: Journal and stream envelopes
HostService-->>HostClient: WebSocket session updates
HostClient-->>Mobile: Fold timeline and render session
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 16 individual chapters for you: 
Chapters generated by Stage for commit cec18ef on Jul 11, 2026 7:18pm UTC. |
Contributor
Greptile SummaryThis PR adds a standalone comparison page for the mobile ACP and Claude SDK harnesses.
Confidence Score: 4/5The standalone page is mergeable after a small browser-compatibility cleanup.
apps/mobile/plans/acp-vs-claude-sdk.html
|
| Filename | Overview |
|---|---|
| apps/mobile/plans/acp-vs-claude-sdk.html | Adds a self-contained interactive protocol comparison page; the inline script has one browser compatibility issue. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/mobile/plans/acp-vs-claude-sdk.html:1796
**Lookbehind Breaks Page Script**
When this local page is opened in a browser that does not support regex lookbehind, the `(?<!...)` in the inline highlighter causes a parse-time script error. That stops the whole script from running, so the ACP/Both/SDK filter buttons and Differences mode do not work.
Reviews (1): Last reviewed commit: "docs(mobile): add ACP vs Claude SDK prot..." | Re-trigger Greptile
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Architecture diagram
sequenceDiagram
participant Browser as Browser (User)
participant Page as acp-vs-claude-sdk.html
participant ACP as ACP Harness
participant SDK as Claude SDK Harness
participant Journal as Ring Journal (in-memory)
participant Transcript as Transcript File (disk)
participant ChildProc as ACP Child Process
Note over Browser,ChildProc: Comparison Page Loading & Interactive Filtering
Browser->>Page: Open self-contained HTML
Page->>Page: Initialize shared envelope skeleton
Page->>Page: Generate 46-hop message waterfall
alt ACP Lane Selected
Page->>ACP: Load ACP-specific frames (6 kinds)
ACP->>ChildProc: JSON-RPC over stdio
ChildProc-->>ACP: Semantic stream chunks
ACP->>Journal: Store frames (ephemeral)
Note over ACP,Journal: D1: Host restart loses history
ACP-->>Page: Render ACP hop data
Page->>Page: Show semantic frame kinds (D2)
Page->>Page: Client taps optionId for permissions (D3)
Page->>Page: Host mints permission cards (D4)
Page->>Page: Child process crash isolation (D5)
Page->>Page: Show plain content blocks & mid-turn sends (D6)
Page->>Page: Show stopReason → state frame only (D7)
else SDK Lane Selected
Page->>SDK: Load SDK-specific frames (9 kinds)
SDK->>Transcript: Read/write persistent JSONL
Note over SDK,Transcript: D1: Survives restarts, retry() starts new epoch
Transcript-->>SDK: Full transcript history
SDK-->>Page: Render SDK hop data
Page->>Page: Show raw deltas & authoritative replace (D2)
Page->>Page: Client constructs PermissionResult (D3)
Page->>Page: Client parses AskUserQuestion (D4)
Page->>Page: In-process SDK, no crash isolation (D5)
Page->>Page: Show SDKUserMessage envelope, idle-only (D6)
Page->>Page: Show journaled result with cost/usage (D7)
else Both Lane Selected
Page->>ACP: Load ACP frames
Page->>SDK: Load SDK frames
Page->>Page: Side-by-side comparison
alt Differences-Only Mode
Page->>Page: Filter to D1-D7 dimensions only
end
Page->>Page: Highlight ACP strength (crash isolation, simpler)
Page->>Page: Highlight SDK strength (fidelity, durability)
end
Note over Page: Shared skeleton functions identically
Page->>Page: Envelope {seq, sessionId, ts, frame}
Page->>Page: Host-only gapless seq from 1
Page->>Page: In-memory ring journal (cap 5,000)
Page->>Page: WS replay (since, latest] with client dedupe
Page->>Page: Full-snapshot state frames
Page->>Page: Parked-promise permissions
Page->>Page: Admission-ack sends
Page->>Page: Pure client-side fold
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (str) return '<span class="' + (colon ? 'k' : 's') + '">' + str + '</span>' + (colon || ''); | ||
| if (kw) return '<span class="n">' + kw + '</span>'; | ||
| if (num) return '<span class="n">' + num + '</span>'; | ||
| if (verb !== undefined) return nl + '<span class="m">' + verb + '</span>'; |
Contributor
There was a problem hiding this comment.
When this local page is opened in a browser that does not support regex lookbehind, the (?<!...) in the inline highlighter causes a parse-time script error. That stops the whole script from running, so the ACP/Both/SDK filter buttons and Differences mode do not work.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/plans/acp-vs-claude-sdk.html
Line: 1796
Comment:
**Lookbehind Breaks Page Script**
When this local page is opened in a browser that does not support regex lookbehind, the `(?<!...)` in the inline highlighter causes a parse-time script error. That stops the whole script from running, so the ACP/Both/SDK filter buttons and Differences mode do not work.
How can I resolve this? If you propose a fix, please make it concise.…e UI, desktop gate) Everything needed to start and drive a coding-agent session on a desktop host from the mobile app, over the existing relay: - packages/host-service: AcpSessionManager runtime (claude-agent-acp adapter children over JSON-RPC/stdio), per-session journal ring buffer with gapless seq + resync contract, WS update stream, tRPC surface. Gated off by default; `list` stays ungated and answers `enabled: false` as a zero-extra-request capability probe. - packages/session-protocol: shared envelope/state/fold contracts, ACP type re-exports, subscribeToSession client, React hooks. - packages/host-client: transport + ACP client extracted from apps/mobile/lib/host (SuperJSON tRPC calls, 401 retry-once, stream URLs). - apps/mobile: sessions list + live session thread (timeline, tool calls, plan view, permission cards incl. multi-select, composer), gated on the host capability signal. - apps/desktop: off-by-default "live agent sessions" security setting next to the relay toggle (local-db migration 0042); coordinator injects SUPERSET_ACP_SESSIONS=1 into host children and restarts them on change; Anthropic env vars scrubbed at the adapter spawn site so agent children use the user's own login.
bra1nDump
changed the title
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/mobile/components/ai-elements/tool.tsx (1)
119-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStatus information no longer reaches assistive technology.
The removed
statusLabels/statusIconsbadge conveyed textual status ("Running", "Failed", etc.). Now, running state is signaled only by an opacity pulse (imperceptible to screen readers) and error state by a bareXIconwith noaccessibilityLabel. Screen-reader users can no longer distinguish running/completed/failed tool calls from this header.🔧 Proposed fix — add an accessible status label
<View className="flex-1 flex-row items-center gap-2"> <Icon as={WrenchIcon} className="size-4 text-muted-foreground" /> <Animated.View className="shrink" style={titleStyle}> <Text className="font-medium text-sm">{title ?? derivedName}</Text> </Animated.View> {/* No status chrome — only a failure leaves a subtle faded mark. */} {state === "output-error" ? ( - <Icon as={XIcon} className="size-3.5 text-destructive/70" /> + <Icon + as={XIcon} + className="size-3.5 text-destructive/70" + accessibilityLabel="Tool call failed" + /> ) : null} </View>Consider also setting
accessibilityLabel/accessibilityValueon the outerView(orCollapsibleTrigger) reflectingisRunning/error state so the pulse's meaning isn't lost entirely.🤖 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/mobile/components/ai-elements/tool.tsx` around lines 119 - 155, Add an accessible status announcement to the tool header in the CollapsibleTrigger/titleStyle section, preserving textual distinctions for running, completed, and error states. Derive a status label/value from isRunning and state (including output-error), expose it through the outer CollapsibleTrigger or wrapping View via accessibilityLabel/accessibilityValue, and give the error XIcon an explicit accessibility label or hide it when redundant.
🧹 Nitpick comments (6)
plans/session-harness-claude-agent-sdk.md (1)
142-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifiers to fenced code blocks.
Two fenced code blocks (the directory structure at line 142 and the API listing at line 246) lack language specifiers, triggering markdownlint MD040 warnings. Adding
textas the language tag resolves this.♻️ Proposed fix
-``` +```text packages/session-protocol/ package.json # exports ".", "./client"-``` +```text sessions.list({ workspaceId?, cursor?, limit }) → { items: SessionScopedState[], nextCursor }Also applies to: 246-246
🤖 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 `@plans/session-harness-claude-agent-sdk.md` at line 142, Add the text language specifier to the two fenced code blocks in the document: the directory structure block near the session protocol listing and the API listing block near the sessions.list example. Change each opening fence to ```text while preserving the existing contents.Source: Linters/SAST tools
apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsx (1)
48-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHandle
already_resolvedto clear stale optimistic highlight.When
onRespondresolves with{ status: "already_resolved" },pendingOptionIdremains set to the user's tapped option, but the actual resolution may have selected a different option. The stream will eventually deliver the correctview.resolutionand self-correct, but there's a brief window where the wrong option is highlighted. Adding a.then()to resetpendingOptionIdwhen the result isalready_resolvedwould prevent this.♻️ Proposed fix
setPendingOptionId(option.optionId); onRespond(view.requestId, { outcome: "selected", optionId: option.optionId, - }).catch(() => setPendingOptionId(null)); + }) + .then((result) => { + if (result.status === "already_resolved") setPendingOptionId(null); + }) + .catch(() => setPendingOptionId(null));🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsx around lines 48 - 53, Add handling in the PermissionRequestView option onPress callback after onRespond: when the resolved result has status "already_resolved", clear pendingOptionId, while preserving the existing catch reset for rejected responses. Use the onRespond invocation and setPendingOptionId symbols to update the optimistic state immediately.plans/host-chat-session-permisstance.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFilename typo: "permisstance" → "persistence".
Consider renaming the file to
host-chat-session-persistence.mdfor discoverability and to avoid propagating the typo into future cross-references.#!/bin/bash # Find any references to this filename that would need updating on rename. rg -n 'host-chat-session-permisstance' -g '*.md'🤖 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 `@plans/host-chat-session-permisstance.md` at line 1, Rename the documentation file from “host-chat-session-permisstance.md” to “host-chat-session-persistence.md” and update every matching reference found by searching for the misspelled filename, including cross-references and links.apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/PermissionStack.tsx (1)
32-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFront card ordering trusts array order, not
requestedAt.
PendingPermissioncarries arequestedAttimestamp specifically for ordering, butfrontis derived from raw array position (visible[0]) rather than sorting by it. The docstring promises "the oldest request is the front card" — if the upstreampendingPermissionsarray is ever reordered (e.g., after astatesnapshot replace), the wrong card would surface as "front."🔧 Proposed defensive sort
- const visible = pending.filter((entry) => !answeredIds.has(entry.requestId)); + const visible = pending + .filter((entry) => !answeredIds.has(entry.requestId)) + .sort((a, b) => a.requestedAt - b.requestedAt); const front = visible[0];🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/PermissionStack.tsx around lines 32 - 33, Update the visible pending-permission selection in PermissionStack so entries are defensively sorted by requestedAt in ascending order after filtering answeredIds, then derive front from the sorted result; preserve the existing oldest-request-first behavior even when pendingPermissions is reordered.apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/components/PermissionCard/PermissionCard.tsx (1)
94-117: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSelection state not exposed to accessibility tree.
Multi-select options only convey "picked" via a checkmark icon and text color; add
accessibilityState={{ selected: picked.has(option.optionId) }}so VoiceOver/TalkBack users get a spoken confirmation when toggling.🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/components/PermissionCard/PermissionCard.tsx around lines 94 - 117, The multi-select state in the option Pressables is not exposed to assistive technologies. In the options map within PermissionCard, add accessibilityState with selected set to picked.has(option.optionId) on each Pressable so VoiceOver and TalkBack announce selection changes.packages/host-client/src/transport/transport.ts (1)
60-60: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider URL-encoding
routingKeyin URL construction.
routingKeyis interpolated directly into both the HTTP and WebSocket URLs withoutencodeURIComponent. If it ever contains characters like/,?, or#, the URL path could be malformed or enable path traversal. This is low-risk ifroutingKeyis always a safe identifier (e.g., UUID), but encoding it is a cheap defense-in-depth measure.Optional fix
- const base = `${relayUrl()}/hosts/${routingKey}/trpc/${procedure}`; + const base = `${relayUrl()}/hosts/${encodeURIComponent(routingKey)}/trpc/${procedure}`;- const base = `${wsBase}/hosts/${routingKey}/${path}`; + const base = `${wsBase}/hosts/${encodeURIComponent(routingKey)}/${path}`;Also applies to: 114-114
🤖 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 `@packages/host-client/src/transport/transport.ts` at line 60, URL-encode routingKey before interpolating it into the HTTP and WebSocket URLs. Update the URL construction in the transport logic, including the base URL expression and the corresponding WebSocket URL, to use encodeURIComponent(routingKey) so reserved characters cannot alter the path.
🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/ToolCallItemView.tsx:
- Around line 1-6: The shared ToolCallItemView currently imports iOS/tvOS-only
Swift UI components without an Android fallback. Split the sheet rendering by
platform in ToolCallItemView, using the existing Swift UI implementation only
for iOS/tvOS and an Android-compatible implementation or universal Expo UI
components for Android; ensure tool rows render correctly on both platforms.
In `@plans/host-integration-test.md`:
- Line 21: Add a language identifier to the unlabeled Markdown code fence in the
architecture section, using text or plaintext for the diagram so it satisfies
markdownlint MD040.
In `@plans/session-harness-acp.md`:
- Line 184: Add an explicit language identifier to the three fenced code blocks
in plans/session-harness-acp.md: mark the architecture, package-tree, and router
examples as text (or another appropriate language) to satisfy markdownlint
MD040.
- Line 407: Update the dependency entry for
`@agentclientprotocol/claude-agent-acp` to document 0.56.0 as the installed
exact-pinned version, and identify 0.58.1 as the pending upgrade rather than the
current dependency.
- Line 336: Update the “Model selection” capability-matrix entry to state that
set_config_option returns the refreshed configOptions catalog in the RPC
response, and remove the claim that it emits a config_option_update
notification, matching the response-authoritative behavior described elsewhere.
---
Outside diff comments:
In `@apps/mobile/components/ai-elements/tool.tsx`:
- Around line 119-155: Add an accessible status announcement to the tool header
in the CollapsibleTrigger/titleStyle section, preserving textual distinctions
for running, completed, and error states. Derive a status label/value from
isRunning and state (including output-error), expose it through the outer
CollapsibleTrigger or wrapping View via accessibilityLabel/accessibilityValue,
and give the error XIcon an explicit accessibility label or hide it when
redundant.
---
Nitpick comments:
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsx:
- Around line 48-53: Add handling in the PermissionRequestView option onPress
callback after onRespond: when the resolved result has status
"already_resolved", clear pendingOptionId, while preserving the existing catch
reset for rejected responses. Use the onRespond invocation and
setPendingOptionId symbols to update the optimistic state immediately.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/components/PermissionCard/PermissionCard.tsx:
- Around line 94-117: The multi-select state in the option Pressables is not
exposed to assistive technologies. In the options map within PermissionCard, add
accessibilityState with selected set to picked.has(option.optionId) on each
Pressable so VoiceOver and TalkBack announce selection changes.
In
`@apps/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/PermissionStack.tsx:
- Around line 32-33: Update the visible pending-permission selection in
PermissionStack so entries are defensively sorted by requestedAt in ascending
order after filtering answeredIds, then derive front from the sorted result;
preserve the existing oldest-request-first behavior even when pendingPermissions
is reordered.
In `@packages/host-client/src/transport/transport.ts`:
- Line 60: URL-encode routingKey before interpolating it into the HTTP and
WebSocket URLs. Update the URL construction in the transport logic, including
the base URL expression and the corresponding WebSocket URL, to use
encodeURIComponent(routingKey) so reserved characters cannot alter the path.
In `@plans/host-chat-session-permisstance.md`:
- Line 1: Rename the documentation file from “host-chat-session-permisstance.md”
to “host-chat-session-persistence.md” and update every matching reference found
by searching for the misspelled filename, including cross-references and links.
In `@plans/session-harness-claude-agent-sdk.md`:
- Line 142: Add the text language specifier to the two fenced code blocks in the
document: the directory structure block near the session protocol listing and
the API listing block near the sessions.list example. Change each opening fence
to ```text while preserving the existing contents.
🪄 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: 32baac1e-b232-435c-8d8b-e19970fafe45
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (92)
apps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/main/lib/host-service-coordinator.tsapps/desktop/src/renderer/routes/_authenticated/settings/security/components/SecuritySettings/SecuritySettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/shared/constants.tsapps/mobile/app/(authenticated)/workspace/[id]/chat/_layout.tsxapps/mobile/app/(authenticated)/workspace/[id]/chat/acp/[sessionId].tsxapps/mobile/components/ai-elements/conversation.tsxapps/mobile/components/ai-elements/tool.tsxapps/mobile/lib/host/client.tsapps/mobile/package.jsonapps/mobile/plans/acp-vs-claude-sdk.htmlapps/mobile/screens/(authenticated)/workspace/[id]/chat/ChatSessionsScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/[sessionId]/ChatThreadScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/AcpSessionThreadScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/SessionThread.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/Composer.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/PermissionStack.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/components/PermissionCard/PermissionCard.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/components/PermissionCard/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionStack/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/TimelineItemView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/MessageItemView/MessageItemView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/MessageItemView/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/PlanItemView/PlanItemView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/PlanItemView/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/ToolCallItemView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/chat/components/GlassHeaderTitle/GlassHeaderTitle.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/components/GlassHeaderTitle/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useHostRoutingKey/index.tsapps/mobile/screens/(authenticated)/workspace/[id]/hooks/useHostRoutingKey/useHostRoutingKey.tspackages/host-client/package.jsonpackages/host-client/src/acp/acp.tspackages/host-client/src/acp/index.tspackages/host-client/src/index.tspackages/host-client/src/transport/index.tspackages/host-client/src/transport/transport.test.tspackages/host-client/src/transport/transport.tspackages/host-client/tsconfig.jsonpackages/host-service/package.jsonpackages/host-service/src/app.tspackages/host-service/src/runtime/acp-sessions/acp-sessions.tspackages/host-service/src/runtime/acp-sessions/index.tspackages/host-service/src/runtime/acp-sessions/journal.test.tspackages/host-service/src/runtime/acp-sessions/journal.tspackages/host-service/src/runtime/acp-sessions/stream.test.tspackages/host-service/src/runtime/acp-sessions/stream.tspackages/host-service/src/trpc/router/acp-sessions/acp-sessions.tspackages/host-service/src/trpc/router/acp-sessions/index.tspackages/host-service/src/trpc/router/router.tspackages/host-service/src/types.tspackages/host-service/test/integration/acp-sessions-stream.integration.test.tspackages/host-service/test/integration/acp-sessions.integration.test.tspackages/local-db/drizzle/0042_add_acp_sessions_enabled_setting.sqlpackages/local-db/drizzle/meta/0042_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.tspackages/session-protocol/package.jsonpackages/session-protocol/src/acp.tspackages/session-protocol/src/api.test.tspackages/session-protocol/src/api.tspackages/session-protocol/src/client/index.tspackages/session-protocol/src/client/subscribeToSession/index.tspackages/session-protocol/src/client/subscribeToSession/subscribeToSession.test.tspackages/session-protocol/src/client/subscribeToSession/subscribeToSession.tspackages/session-protocol/src/envelope.tspackages/session-protocol/src/fold/fold.test.tspackages/session-protocol/src/fold/fold.tspackages/session-protocol/src/fold/index.tspackages/session-protocol/src/index.tspackages/session-protocol/src/react/index.tspackages/session-protocol/src/react/useAcpPermissions/index.tspackages/session-protocol/src/react/useAcpPermissions/useAcpPermissions.tspackages/session-protocol/src/react/useAcpSession/index.tspackages/session-protocol/src/react/useAcpSession/useAcpSession.tspackages/session-protocol/src/state.tspackages/session-protocol/tsconfig.jsonplans/host-chat-session-permisstance.mdplans/host-integration-test.mdplans/session-harness-acp.mdplans/session-harness-claude-agent-sdk.md
Comment on lines
+1
to
+6
| import { BottomSheet, Group, Host, RNHostView } from "@expo/ui/swift-ui"; | ||
| import { | ||
| background, | ||
| environment, | ||
| presentationDragIndicator, | ||
| } from "@expo/ui/swift-ui/modifiers"; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 '`@expo/ui/`(swift-ui|jetpack-compose)|Platform\.select|\.ios\.|\.android\.' apps/mobile --glob '*.{ts,tsx}'Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby usage sites
sed -n '1,240p' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/ToolCallItemView.tsx'
printf '\n--- call sites ---\n'
rg -n -C3 'ToolCallItemView' apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId] --glob '*.{ts,tsx}'
printf '\n--- platform gating nearby ---\n'
rg -n -C3 'Platform\.OS|Platform\.select|ios|android|web' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]' --glob '*.{ts,tsx}'
printf '\n--- package / dependency evidence ---\n'
rg -n '"`@expo/ui`"|swift-ui|jetpack-compose' apps/mobile/package.json apps/mobile/bun.lock package.json --glob '*.{json,jsonc,ts,tsx}'Repository: superset-sh/superset
Length of output: 7308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- ToolCallItemView definition ---\n'
sed -n '1,240p' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/ToolCallItemView.tsx'
printf '\n--- call sites ---\n'
rg -n -C2 'ToolCallItemView' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]' --glob '*.{ts,tsx}'
printf '\n--- platform gating in ACP chat tree ---\n'
rg -n -C2 'Platform\.OS|Platform\.select|ios|android|native' 'apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]' --glob '*.{ts,tsx}'
printf '\n--- package evidence for Expo UI ---\n'
rg -n '"`@expo/ui`"|swift-ui|jetpack-compose' apps/mobile/package.json package.json bun.lock --glob '*.{json,jsonc,yaml,yml,ts,tsx}'Repository: superset-sh/superset
Length of output: 37410
🌐 Web query:
Expo UI @expo/ui/swift-ui platform support iOS tvOS Android Jetpack Compose documentation
💡 Result:
Expo UI provides platform-specific and universal components to integrate native UI toolkits into React Native applications [1][2][3]. Platform-Specific Support: - @expo/ui/swift-ui: Designed for iOS and tvOS [4][3]. It enables the use of SwiftUI components within React Native by bridging them through a Host component [4]. - @expo/ui/jetpack-compose: Designed specifically for Android [5][3][6]. It allows for the use of Jetpack Compose components within React Native, also requiring a Host component for rendering [5][6]. Universal Support: - @expo/ui/universal: Provides a cross-platform API layer that enables developers to write a single component tree that runs across Android, iOS, and web [1]. This universal API automatically delegates to the appropriate native toolkit (@expo/ui/jetpack-compose on Android and @expo/ui/swift-ui on iOS) [1]. When to use each: - Use the universal components when you want a shared UI codebase that preserves a native look and feel on each platform [1]. - Use the platform-specific packages (@expo/ui/swift-ui or @expo/ui/jetpack-compose) directly when you require platform-specific controls, modifiers, or behaviors that are not covered by the universal API [1].
Citations:
- 1: https://docs.expo.dev/versions/latest/sdk/ui/universal/
- 2: https://docs.expo.dev/versions/latest/sdk/ui/
- 3: https://docs.expo.dev/versions/unversioned/sdk/ui
- 4: https://docs.expo.dev/versions/latest/sdk/ui/swift-ui/host/
- 5: https://docs.expo.dev/versions/latest/sdk/ui/jetpack-compose
- 6: https://docs.expo.dev/versions/latest/sdk/ui/jetpack-compose/host
Split this sheet by platform. @expo/ui/swift-ui is iOS/tvOS-only, and this shared ACP row has no Android fallback. Add an Android implementation or switch to the universal Expo UI path before enabling tool rows on Android.
🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/components/ToolCallItemView/ToolCallItemView.tsx
around lines 1 - 6, The shared ToolCallItemView currently imports iOS/tvOS-only
Swift UI components without an Android fallback. Split the sheet rendering by
platform in ToolCallItemView, using the existing Swift UI implementation only
for iOS/tvOS and an Android-compatible implementation or universal Expo UI
components for Android; ensure tool rows render correctly on both platforms.
Source: MCP tools
Contributor
There was a problem hiding this comment.
3 issues found across 92 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/runtime/acp-sessions/acp-sessions.ts">
<violation number="1" location="packages/host-service/src/runtime/acp-sessions/acp-sessions.ts:222">
P2: After an adapter exits, the session remains in `list()` because this filter does not exclude `runtime.dead`. That contradicts the live-session-list acceptance flow (which expects the dead session to remain addressable through `get()` but disappear from `list()`), so the enabled ACP integration test fails and mobile can render dead sessions as active-list rows. Filtering dead runtimes here while retaining them in the map would preserve transcript/error access through `get()` and `getMessages()`.</violation>
<violation number="2" location="packages/host-service/src/runtime/acp-sessions/acp-sessions.ts:231">
P3: Malformed list cursors are silently interpreted as sort positions, so a truncated or corrupted cursor can make pagination look exhausted and hide all sessions instead of surfacing a bad request. Validating the complete `<createdAt>:<sessionId>` shape before using it would make cursor failures recoverable and consistent with `getMessages`.</violation>
</file>
<file name="apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsx">
<violation number="1" location="apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsx:1">
P1: ACP tool detail rows currently render `@expo/ui/swift-ui` `BottomSheet` unconditionally. That package does not support Android, so opening tool details on Android can fail instead of showing the details sheet. A platform guard with an Android-safe fallback would keep the ACP thread usable across platforms.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,134 @@ | |||
| import { BottomSheet, Group, Host, RNHostView } from "@expo/ui/swift-ui"; | |||
Contributor
There was a problem hiding this comment.
P1: ACP tool detail rows currently render @expo/ui/swift-ui BottomSheet unconditionally. That package does not support Android, so opening tool details on Android can fail instead of showing the details sheet. A platform guard with an Android-safe fallback would keep the ACP thread usable across platforms.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsx, line 1:
<comment>ACP tool detail rows currently render `@expo/ui/swift-ui` `BottomSheet` unconditionally. That package does not support Android, so opening tool details on Android can fail instead of showing the details sheet. A platform guard with an Android-safe fallback would keep the ACP thread usable across platforms.</comment>
<file context>
@@ -0,0 +1,134 @@
+import { BottomSheet, Group, Host, RNHostView } from "@expo/ui/swift-ui";
+import {
+ background,
</file context>
| ); | ||
| let start = 0; | ||
| if (input.cursor) { | ||
| const separator = input.cursor.indexOf(":"); |
Contributor
There was a problem hiding this comment.
P3: Malformed list cursors are silently interpreted as sort positions, so a truncated or corrupted cursor can make pagination look exhausted and hide all sessions instead of surfacing a bad request. Validating the complete <createdAt>:<sessionId> shape before using it would make cursor failures recoverable and consistent with getMessages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/runtime/acp-sessions/acp-sessions.ts, line 231:
<comment>Malformed list cursors are silently interpreted as sort positions, so a truncated or corrupted cursor can make pagination look exhausted and hide all sessions instead of surfacing a bad request. Validating the complete `<createdAt>:<sessionId>` shape before using it would make cursor failures recoverable and consistent with `getMessages`.</comment>
<file context>
@@ -0,0 +1,1069 @@
+ );
+ let start = 0;
+ if (input.cursor) {
+ const separator = input.cursor.indexOf(":");
+ const createdAt = Number(input.cursor.slice(0, separator));
+ const sessionId = input.cursor.slice(separator + 1);
</file context>
Adds a scripted fake claude-agent-acp adapter (real ACP over stdio via the official SDK, no model/tokens/network) and wires AcpSessionManager's new adapterEntry option into the spawn site so tests can inject it. The new e2e suite runs in every `bun test` (~600ms) and covers: a 30-turn marathon with gapless WS streams and getMessages pagination folds, permission allow/deny with duplicate-answer staleness, single- and multi-select elicitations (answers riding the _meta extension), cancel mid-tool-call terminalization, adapter crash (dead-but-listed, readable transcript), host restart (session_not_found reset + re-create resync), and stale-cursor journal eviction resyncs.
ChatSessionsScreen goes back to its pre-ACP mastra-only form (byte-identical to main) instead of serving both session kinds — the mastra list is slated for removal once live sessions prove out, so mixing them bought complexity for a screen that's about to die. Live sessions get a forked AcpSessionsScreen at chat/acp/ with the create button, status rows, pull-to-refresh, and a disabled-host explainer. Entry is a static "Live" header link on the chat list: zero background requests for hosts with the feature off; the capability probe (list's enabled flag) only fires when the user navigates in.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsx:
- Around line 37-59: Add a dedicated loading state near the existing sessions
state and set it true before the initial fetch, then false in a finally block
within load, while preserving refresh behavior as appropriate. Update
ListEmptyComponent (and its loaded condition) to render a simple activity
indicator while the first load is pending, ensuring the screen is not blank
before load resolves.
🪄 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: ad644c7d-0f2c-4078-90a8-33d4c8e3015c
📒 Files selected for processing (4)
apps/mobile/app/(authenticated)/workspace/[id]/chat/_layout.tsxapps/mobile/app/(authenticated)/workspace/[id]/chat/acp/index.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/index.ts
The fake adapter's payloads are now copied verbatim from the vendored claude-agent-acp dist (buildAvailableModes, buildConfigOptions, describeAlwaysAllow's allow_always/allow/reject triple, askUserQuestionsToCreateRequest incl. the question_<n>_custom "Other" fields), so tests exercise exactly what real runs put on the wire. New manager-level e2e coverage: non-fatal prompt rejection + recovery, create idempotency and workspace-mismatch conflicts, setMode / setConfigOption round trips, cancel and adapter-death with pending permission cards, url/empty elicitation edge modes, sequential two-question forms with Skip, adapter-owned elicitation tool calls (exactly one terminal update), ANTHROPIC_* spawn scrubbing, concurrent turns not sweeping each other's open tools, dead-session graveyard eviction, list pagination/filtering, title clearing, and subscriber throw isolation. New router-layer integration test via createTestHost (acpSessions passthrough): feature gate closed-by-default with list feature-detect, domain-error → TRPC-code mapping (NOT_FOUND/CONFLICT/BAD_REQUEST/ PRECONDITION_FAILED), and a full create → prompt → poll → fold round trip over real tRPC. Note: the manager's boolean setConfigOption branch stays untested at the wire level on purpose — our initialize never declares session.configOptions.boolean, so the real adapter can only ever send the select fallback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test/integration/acp-sessions.router.integration.test.ts`:
- Line 88: Add cleanup for the temporary workspace created by mkdtempSync in the
test suite’s afterAll block, alongside host.dispose(). Use rmSync with recursive
and force options on workspaceDir, ensuring the directory is removed after
integration tests complete.
🪄 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: 909198d2-8584-42aa-9915-c9c36654eb80
📒 Files selected for processing (5)
packages/host-service/src/runtime/acp-sessions/stream.tspackages/host-service/test/fixtures/fake-acp-adapter.tspackages/host-service/test/helpers/createTestHost.tspackages/host-service/test/integration/acp-sessions.e2e.test.tspackages/host-service/test/integration/acp-sessions.router.integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/host-service/src/runtime/acp-sessions/stream.ts
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Bot-review triage for ParadevAI#5580 — the accepted set: - host-client: 401 retry now passes the force-refreshed token into the retried call instead of re-reading a possibly stale provider cache; transport test pins the cached→fresh header sequence. - session-protocol: subscribeToSession structurally validates incoming envelopes (resync instead of crash on non-envelope JSON) and survives a throwing WebSocket factory during reconnect (both under test); list-cursor input schema rejects malformed cursors as BAD_REQUEST (router integration test added); PermissionView carries multiSelect; useAcpSession only clears rendered state on actual sessionId change and warns instead of swallowing loadOlder failures; tsconfig drops the no-op outDir. - mobile: PermissionRequestView implements multi-select permissions (collect picks, Done submits via selectedOptionIds _meta) mirroring PermissionCard; AcpSessionsScreen shows a spinner on first load; TimelineItemView switch is exhaustiveness-checked; tool header and OptionPicker options get accessibility state/labels. - desktop: revert the ambient-ANTHROPIC env scrub in electron.vite.config — spawn-boundary scrubbing in AcpSessionManager (covered by the env-scrub e2e test) is the right layer; the desktop-wide deletion changed how the rest of the host works. - AGENTS.md: record that apps/mobile is iOS-only for the time being. - plans: sync host-integration-test.md with the shipped host-client extraction, fix session-harness-acp.md's config-option-update and adapter-version claims, mark host-chat-session-permisstance.md as a hypothetical fallback (ACP shipped instead), language-tag bare fences.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/session-protocol/src/react/useAcpSession/useAcpSession.ts (1)
177-207: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReset older-loading state when resync starts. If an in-flight
loadOldergets invalidated by a resync and that resync fails,loadingOlderRef/isLoadingOlderstay true until the next successful resync, which blocks further history loads and leaves the spinner stuck. Clear those flags at the start ofresync.🤖 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 `@packages/session-protocol/src/react/useAcpSession/useAcpSession.ts` around lines 177 - 207, Reset the older-history loading state at the beginning of the resync logic. In the resync function, clear loadingOlderRef.current and setIsLoadingOlder(false) before starting the resync request so an invalidated loadOlder cannot leave history loading blocked or the spinner stuck when resync fails.Source: Linters/SAST tools
🤖 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/mobile/screens/`(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsx:
- Around line 92-100: Update the option text styling in PermissionRequestView so
selected options remain highlighted after answering. Replace the current
answered/unanswered branching that uses picked with
selectedIds.has(option.optionId) for the primary, bold style, while retaining
muted styling for answered options not in selectedIds and the default style
otherwise.
---
Outside diff comments:
In `@packages/session-protocol/src/react/useAcpSession/useAcpSession.ts`:
- Around line 177-207: Reset the older-history loading state at the beginning of
the resync logic. In the resync function, clear loadingOlderRef.current and
setIsLoadingOlder(false) before starting the resync request so an invalidated
loadOlder cannot leave history loading blocked or the spinner stuck when resync
fails.
🪄 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: d2e5a07f-667a-47d5-a941-72736e803281
📒 Files selected for processing (19)
AGENTS.mdapps/mobile/components/ai-elements/tool.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/PermissionRequestView/PermissionRequestView.tsxapps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/TimelineItemView.tsxpackages/host-client/src/transport/transport.test.tspackages/host-client/src/transport/transport.tspackages/host-service/test/integration/acp-sessions.integration.test.tspackages/host-service/test/integration/acp-sessions.router.integration.test.tspackages/session-protocol/src/api.tspackages/session-protocol/src/client/subscribeToSession/subscribeToSession.test.tspackages/session-protocol/src/client/subscribeToSession/subscribeToSession.tspackages/session-protocol/src/fold/fold.tspackages/session-protocol/src/react/useAcpSession/useAcpSession.tspackages/session-protocol/tsconfig.jsonplans/host-chat-session-permisstance.mdplans/host-integration-test.mdplans/session-harness-acp.md
💤 Files with no reviewable changes (1)
- packages/session-protocol/tsconfig.json
✅ Files skipped from review due to trivial changes (3)
- plans/host-integration-test.md
- plans/session-harness-acp.md
- plans/host-chat-session-permisstance.md
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/TimelineItemView/TimelineItemView.tsx
- apps/mobile/components/ai-elements/tool.tsx
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/AcpSessionsScreen/AcpSessionsScreen.tsx
- packages/host-service/test/integration/acp-sessions.router.integration.test.ts
- packages/host-client/src/transport/transport.test.ts
- packages/host-service/test/integration/acp-sessions.integration.test.ts
- apps/mobile/screens/(authenticated)/workspace/[id]/chat/acp/[sessionId]/components/SessionThread/components/Composer/components/OptionPicker/OptionPicker.tsx
- packages/host-client/src/transport/transport.ts
- packages/session-protocol/src/api.ts
- packages/session-protocol/src/client/subscribeToSession/subscribeToSession.ts
- packages/session-protocol/src/fold/fold.ts
…parison # Conflicts: # apps/mobile/app/(authenticated)/workspace/[id]/chat/_layout.tsx
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…d of a security setting The pre-release ACP session harness no longer has a user-facing toggle: the host-service coordinator injects SUPERSET_ACP_SESSIONS=1 whenever the build is internal (semver prerelease like 0.0.53-canary, or unpackaged dev runs). Stable builds never mount the surface. Removes the settings procedures, security toggle UI, search entries, and local-db migration 0042; host-side gating via the env var is unchanged.
5 of 6 tasks
1 of 3 tasks
This was referenced Aug 4, 2026
This was referenced Aug 13, 2026
This branch had an error being deployed
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.
TL;DR
This adds an experimental way to start and control a coding-agent session running on a desktop host from the iOS app. The mobile app is a remote UI; the agent process, workspace access, permissions, and native session files stay on the desktop.
The feature is off by default behind Desktop → Settings → Security → Enable live agent sessions (experimental).
The detailed ACP-vs-direct-SDK walkthrough is in the standalone comparison page. This PR body is the shorter implementation guide for reviewers.
What changed
packages/host-servicepackages/host-client(new)packages/session-protocol(new)apps/mobileapps/desktopSUPERSET_ACP_SESSIONS=1. It does not render this session UI.The existing Mastra chat path is not rewritten by this PR; live ACP sessions have a separate mobile route while the feature is evaluated.
Protocol flow
Create and run a turn
The public Superset session id is different from the adapter's native session id.
A prompt is admission-acknowledged over HTTP; a potentially long-running turn does not keep a relay request open:
When the adapter blocks on a tool or
AskUserQuestion, the host parks the ACP request and streams a renderable card to mobile. The mobile response resolves that exact request id:Single-select, multi-select, and skipped question answers are supported. The adapter's paired free-text
*_customfields are intentionally ignored today, so the mobile flow is tap-only. Multi-select answers use ACP's_metaextension while preserving a normal firstoptionIdfor single-select consumers.Reconnect and restart
Each runtime has a gapless
seqjournal. A reconnect presents its last sequence; the host replays(since, latest]and then attaches the live subscriber without a handoff gap. Duplicates are ignored. A missing/evicted cursor produces a terminalreset, after which the client refetches state/history and opens a fresh stream URL/token.Current caveat: the numeric cursor does not yet include a journal-incarnation id. A new journal starts at 1 after host resurrection, so an overlapping pre-restart cursor is not guaranteed to be recognized as stale by number alone. The mobile hook's normal restart path refetches state/history before attaching; making cross-incarnation rejection intrinsic to every cursor is tracked as P0 follow-up.
Host SQLite stores only the durable binding needed to find the harness-owned session again:
After a host restart,
list/getreturn the row asstatus: "offline"without starting a process. The first live operation (getMessages, prompt, permission/config command, or stream attach) spawns a new adapter and calls:An in-flight turn and pending permission callbacks do not survive a host restart. Replayed tool calls that were left open are terminalized instead of appearing to run forever.
If the native Claude session file was deleted or is corrupt,
session/loadfails. The registry row remainsofflineand retryable; the stream emitsreset: "session_load_failed", the hook retries through tRPC, and the actual load error is shown in the thread's destructive error banner. The composer stays hidden until resurrection succeeds, and no replacement session is silently created.Security and feature gating
listrejects withPRECONDITION_FAILED.listremains authenticated but returns{ items: [], enabled: false }, so the sessions screen can feature-detect without an extra capability endpoint.ANTHROPIC_API_KEYandANTHROPIC_AUTH_TOKENare removed at the adapter spawn boundary, so a repo.envcannot override the user's Claude login.History, memory, and validation boundaries
The host journal is a fixed-capacity O(1) circular buffer (5,000 envelopes), and pre-runtime
session/loadreplay buffering is capped to the same window. This prevents lifetime-unbounded host memory, but the journal still serves both recent reconnect catch-up andgetMessagespagination. Because ACP exposes whole-transcript replay rather than paginated native history, history older than the retained window is not currently available through Superset.The React hook retains the pages the user loaded plus live envelopes for the current mount so it can refold the timeline. Separating disk-backed history from a much smaller catch-up ring, and bounding the live client overlay, is explicitly follow-up work rather than something this PR claims to have solved.
TypeScript is strict across the production path, but runtime validation is not complete. tRPC inputs are Zod-validated and the official ACP SDK validates its registered request/notification boundary; nested WebSocket payloads, generic relay outputs, adapter responses, and SQLite rows are not all fully parsed by canonical Zod schemas yet. The current truth and the hardening plan are documented in
packages/host-service/docs/acp-sessions.mdandplans/acp-session-follow-ups.md.Test coverage
Primary confidence: authenticated real-Claude runs
The
ACP_E2E=1suites are the important compatibility evidence. Nothing at the model/adapter boundary is mocked: they use this Mac's authenticated Claude CLI account, a real Sonnet model, the pinnedclaude-agent-acpexecutable, real ACP JSON-RPC over stdio,AcpSessionManager, and the real WebSocket route/client.The manager suite covers real initialize/create/prompt/fold behavior, a saved multi-agent Workflow,
AskUserQuestion, tool permissions, cancellation after permission, first-response-wins, parallel tool use, and adapter death. The stream suite runs the same real adapter/model through the actual WebSocket route and sync client, covering concurrent subscribers, a mid-turn disconnect/cursor reconnect with no gaps or duplicates, and eviction reset/re-attach.The Workflow assertion does not stop at Claude's asynchronous
async_launchedacknowledgement. It waits for the persisted run to reachcompleted, then verifies all five Sonnet agents, non-zero real token/tool usage, two parallel audits, and the final structured verdict. In the latest complete authenticated run, that Workflow finished in 36.1 seconds with 5 Sonnet agents, 77,981 tokens, and 14 tool calls.Run the full authenticated lane on a logged-in Mac whenever changing the ACP runtime, adapter/SDK, Workflow handling, permissions, questions, cancellation, stream/reconnect/sequencing, or resurrection:
These tests are skipped in ordinary CI only because CI does not have a Claude login and the runs spend real tokens. That is an infrastructure limitation, not a signal that this lane is optional; until an authenticated runner exists, the agent making a relevant change is expected to run it locally and report the real result.
Latest full authenticated result: 10/10 passed, 113 assertions, 72.1 seconds.
One current adapter behavior is pinned explicitly: Claude emits two parallel Bash tool uses together, but
claude-agent-acp0.56.0 exposes their permission callbacks to Superset one at a time. Superset still supports truly simultaneous pending permission requests; that state-machine case is exercised by the deterministic backup below.Belt and suspenders: deterministic regression breadth
The always-run deterministic ACP adapter is deliberately secondary. It speaks real ACP JSON-RPC over stdio through the official SDK and gives cheap breadth plus exact fault injection, but it uses no Claude model/account/network and cannot establish real adapter/model compatibility.
Covered scenarios include:
offlinelisting,session/loadtranscript replay, bounded replay, and deleted native-session failure;Session could not be resumedempty state, and the disabled composer;The host-client boundary suite is another belt-and-suspenders layer: it starts the real
createApphost and crosses actual auth, HTTP/SuperJSON/tRPC, WebSockets, child-process stdio, and on-disk SQLite through@superset/host-client; only the model/adapter behavior is deterministic. It is especially valuable for the question, abort, simultaneous-permission, reconnect, full host/app/server/DB rebuild, resume, and missing-native-transcript paths, but it does not replace the authenticated real-Claude lane.Current ACP evidence on this head: the authenticated Sonnet lane is 10/10 (113 assertions), and the focused deterministic ACP/runtime/host-client lane is 49/49 (553 assertions). Host-service typecheck and root lint also pass. The broader branch baseline previously passed Sherif, 34/34 Turbo typecheck tasks, 14/14 Turbo test tasks (including 835 host-service tests), release typecheck, 15/15 release tests, unified-version validation, and an iOS production Metro export (5,556 modules).
Suggested review order
packages/host-service/docs/acp-sessions.md— current behavior and known limits.packages/host-service/src/runtime/acp-sessions/acp-sessions.ts— lifecycle and ACP bridge.packages/host-service/src/runtime/acp-sessions/{persistence,journal,stream}.ts— durability and delivery.packages/session-protocol/src/{api,envelope,state}.tsandsrc/client/subscribeToSession— shared wire contract.packages/session-protocol/src/react/useAcpSession— resync/pagination state owner.apps/mobile/.../chat/acp/— iOS presentation and interaction layer.packages/host-service/test/integration/acp-sessions*.test.ts— executable behavior matrix.Remaining architecture and production-hardening work is centralized in
plans/acp-session-follow-ups.md; completed implementation history is archived underplans/done/.