Repository navigation
Conversation
Six operations interfaces the mobile screens will be handed instead of an RpcClient: HostSessionOperations, HostTaskOperations, HostWorkspaceOperations, HostFileOperations, HostAccountOperations and DeviceOperations. Each host domain groups its per-concern contracts as namespaces so a screen takes one prop while every sub-adapter keeps its own file, and each has one native factory built over the socket RpcClient. No screen is wired yet, so this carries no behavior change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
The Tasks screens and the new-workspace composer now take an operations adapter instead of an RpcClient. Every RPC envelope check, response cast and payload literal that lived in a hook body moved into the matching native-host-task-* adapter, and the composer's search and create requests moved into HostWorkspaceCreationOperations. Behavior is unchanged apart from four adapter-owned differences noted in the PR body. `worktree.create` from a task item stays inline: the creation adapter builds the composer's params and adds name-collision retry, which is a different request, not the same one behind a seam. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…rations The session hooks take HostSessionOperations, the file explorer takes HostFileExplorerOperations, and the accounts route takes HostAccountOperations. Terminal, tab, markdown, file, quick-command, browser and native-chat requests now live in their native-host-session-* adapters. Four session RPCs stay inline because no adapter method means the same thing: terminal close, note creation's two calls, and the buffered terminal submit that needs the raw response to classify a partial write. Worker-terminal takeover reporting is re-issued at the call sites the adapter does not report from, so the send-site ratchet still covers all of them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Pins the screens this PR routed at zero inline `sendRequest`, pins the four calls deliberately left inline at their exact count, and caps the total screen-side call sites so the remaining debt can only shrink. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
An adapter body has to be the inline call it replaced, so each place where a carried adapter answered differently from main is corrected here: - Linear: per-method timeouts and error strings as main sent them, no result check on selectWorkspace or updateIssue, "Sub-issue not found" and "Failed to create sub-issue" restored. - browser.tabCreate: the host's error message, main's 30s budget, and a tolerated missing page id. - terminal.clearBuffer: an ok:false reply keeps main's success toast. - Quick commands: only the load retries a logical cutover, and an empty error message falls back to main's per-operation copy. - Project rows: the pull-request mutations no longer require a repository slug; a row without one forwards prRepo: null, as main did. - worktree.listRetiredNames: a refused read reads as an empty registry again. - repo.hooks: the create sheet keeps its state on a refused read, while task create still fails on one. The paced-Escape route ref moves from render into a layout effect so the changed-code React Doctor gate passes; a timer cannot observe the ref between commit and layout, so the guard's timing is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Two refusals the old inline calls dropped are now reported, because dropping them is a bug rather than behavior worth preserving: - linear.selectWorkspace: a refused switch reaches the picker's error instead of leaving it showing a workspace the host never selected. - worktree.listRetiredNames: a refused read fails rather than reading as an empty registry, so the create sheet keeps its previous answer instead of offering a name that is already spent. This is the policy src/shared/worktree/retired-name-cache.ts documents. Every other adapter still answers exactly as the call it replaced did. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
Too many files changed for review (170 files, 100 file limit). Bypass the limit by tagging |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
This run reviewed the full refactor across 203 files: the six host-operation adapter groups (session, task, workspace, file, account, device) and their native/default factories, the ~45 screens and hooks re-routed off inline sendRequest, three deliberate behavior exceptions, three carried-branch bug fixes, and the new screen-RPC ratchet.
- Adapter interfaces + native factories — one
Host<Domain>Operationsper domain with anativeHost<Domain>Operations(client)factory;defaultHost<Domain>Operationsdelegates to native (a later PR hands a different client for hybrid/WebView). - Screen re-routing — task/session/file/workspace/account screens now call e.g.
taskOperations.linear.listTeams(); envelope checks and casts move behind the seam. Adapters mostly delegate to pre-existing helpers (subscribeMobileTerminalSafely,sendMobileNativeChatMessageWithOutcome,fetchMergePR, …), keeping parity risk low. - Deliberate exceptions + bug fixes —
worktree.listRetiredNamesandlinear.selectWorkspacenow surface refusals; PR label/assignee edits route to the issue endpoint; worker-terminal takeover reporting restored at three send sites;resolvePrBase/resolveMrBaseparams corrected. - Ratchet —
screen-rpc-ratchet.test.tspins 45 routed screens at zero inlinesendRequest(, pins four deliberate inline calls by count, and caps the screen-side total at 112.
ℹ️ The ratchet only closes the sendRequest seam, not subscribe
The screen-RPC ratchet counts sendRequest( occurrences and nothing else, so the "screen never sees the RPC" goal is only enforced for request/response calls. Several session/worktree screens still subscribe to host streams inline — use-mobile-session-tabs-reconciliation.ts, use-mobile-structured-agent-state.ts (agentSession.subscribe), use-live-worktree-name.ts, and host-worktree-refresh.ts — and a regrown inline client.subscribe(...) would pass the ratchet unnoticed. For the follow-on hybrid PR that hands the adapters a non-socket client, these inline subscriptions are the same seam leak sendRequest had.
Technical details
# Ratchet covers sendRequest only
## Affected sites
- mobile/src/transport/screen-rpc-ratchet.test.ts — `countCalls` matches `/sendRequest\(/g` only.
## Required outcome
- Decide whether inline `client.subscribe(...)` in screens is in scope for the "never sees the RPC" goal; if so, extend the ratchet (or the adapter surface) to cover `subscribe` before the hybrid PR relies on the seam being closed.
## Open questions for the human
- Are the remaining inline `subscribe` calls (tab-strip reconciliation, agent-session state, live worktree name, worktree refresh) meant to be routed in a later PR, or are they out of scope because they read streams rather than issue requests?DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
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:
📝 WalkthroughWalkthroughThe PR routes mobile account, workspace, file, session, and task interactions through typed host-operation adapters. Native adapters centralize RPC mapping, response validation, fallbacks, caching, and timeouts. Screens and hooks now consume operation namespaces instead of raw RPC clients. Tests cover adapter contracts, legacy fallbacks, operation caching, hydration phases, parity counts, and remaining inline RPC calls. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to The mobile adapter refactor preserves most routing behavior, but several failure paths can report success or surface unexpected errors when host requests are refused or malformed. These cases can leave task, review, browser, and workspace state misleading, so the remaining issues should be addressed before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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: 6
🧹 Nitpick comments (4)
mobile/src/session/native-host-session-quick-command-operations.ts (1)
38-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake
loadWithCutoverRetryabort-aware.
signalis checked only afterclient.sendRequestsettles. The resolvedRpcClient.sendRequestcontract acceptstimeoutMs,budgetSpansConnect, andfailWhenDisconnected, but notAbortSignal. The request tracker otherwise waits for a response, connection close, cutover, or its 30-second timeout. Therefore, an aborted snapshot can remain pending until the timeout. Add abort support to the RPC transport and passsignalthrough the request tracker.mobile/src/session/host-session-native-chat-operations.ts (1)
39-44: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRemove the redundant
onErrorparameter.
MobileRelayRpcStreams.failsends an error frame through the subscription listener, anduse-mobile-native-chat-sessionhandles that frame. Transcript failures are not silent. The requiredonErrorcallback is unused; remove it from the contract and call.mobile/src/tasks/github-project-host-routing-source.test.ts (1)
35-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the host assertion request-scoped and type-aware.
The 700-character slice can cross into another RPC. The bare
payloadalternative can then satisfy the assertion without proving the current request carries a host. ThelistAccessiblerequest also uses{ host }shorthand, and several other requests pass payload types that declarehost.Bound the slice to the next
github.project.*literal. Match{ host }, explicithost:fields, and the known host-bearing payload types. Do not usehostPayload(; that helper does not exist.mobile/src/tasks/native-host-task-linear-operations.ts (1)
31-37: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHandle refused
linear.updateIssuemutations.
linear.updateIssuereturns{ ok: false, error }when Linear rejects the update. The localrequesthelper returns that result without throwing. Wrap the result withassertMutationsoupdateStaterejects instead of reporting success.🛠️ Proposed change
async updateState(target, stateId) { - await request(client, 'linear.updateIssue', { - id: target.issueId, - workspaceId: target.workspaceId, - updates: { stateId } - }) + assertMutation( + await request(client, 'linear.updateIssue', { + id: target.issueId, + workspaceId: target.workspaceId, + updates: { stateId } + }), + 'Failed to update issue state' + ) },
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: b152222a-100a-44e4-aab9-fc347979a91a
📒 Files selected for processing (203)
mobile/app/h/[hostId]/accounts.tsxmobile/src/accounts-route-reset-credit.test.tsmobile/src/accounts/default-host-account-operations.tsmobile/src/accounts/host-account-operations.tsmobile/src/accounts/native-host-account-operations.tsmobile/src/components/NewWorktreeModal.tsxmobile/src/components/NewWorktreeModalDrawers.tsxmobile/src/components/SmartWorkspaceSourceDrawer.tsxmobile/src/components/use-new-workspace-execution-target.tsmobile/src/components/use-new-workspace-repositories.tsmobile/src/components/use-new-workspace-runtime-context.tsmobile/src/components/use-new-workspace-setup-script.tsmobile/src/device/default-device-operations.tsmobile/src/device/device-operations.tsmobile/src/device/native-device-operations.tsmobile/src/files/MobileFileExplorerPanel.tsxmobile/src/files/default-host-file-explorer-operations.tsmobile/src/files/default-host-file-operations.tsmobile/src/files/default-host-file-preview-operations.tsmobile/src/files/host-file-explorer-operations.tsmobile/src/files/host-file-operations.tsmobile/src/files/host-file-preview-operations.tsmobile/src/files/native-host-file-explorer-operations.tsmobile/src/files/native-host-file-operations.tsmobile/src/files/native-host-file-preview-operations.tsmobile/src/session/MobileSessionSheets.tsxmobile/src/session/QuickCommandsSheet.tsxmobile/src/session/default-host-session-browser-operations.tsmobile/src/session/default-host-session-file-operations.tsmobile/src/session/default-host-session-markdown-operations.tsmobile/src/session/default-host-session-native-chat-operations.tsmobile/src/session/default-host-session-operations.tsmobile/src/session/default-host-session-quick-command-operations.tsmobile/src/session/default-host-session-tab-operations.tsmobile/src/session/default-host-session-terminal-file-operations.tsmobile/src/session/default-host-session-terminal-operations.tsmobile/src/session/github-pr-mutations.tsmobile/src/session/host-session-browser-operations.tsmobile/src/session/host-session-file-operations.tsmobile/src/session/host-session-markdown-operations.tsmobile/src/session/host-session-native-chat-operations.tsmobile/src/session/host-session-operations.tsmobile/src/session/host-session-quick-command-operations.tsmobile/src/session/host-session-runtime-capabilities.tsmobile/src/session/host-session-tab-operations.tsmobile/src/session/host-session-terminal-file-operations.tsmobile/src/session/host-session-terminal-operations.tsmobile/src/session/mobile-file-tap-open.test.tsmobile/src/session/mobile-file-tap-open.tsmobile/src/session/mobile-native-chat-open-file.test.tsmobile/src/session/mobile-native-chat-open-file.tsmobile/src/session/mobile-native-chat-operation-target.tsmobile/src/session/mobile-native-chat-send.tsmobile/src/session/mobile-session-route-parity.test.tsmobile/src/session/mobile-worker-takeover-send-sites.test.tsmobile/src/session/native-host-session-browser-operations.tsmobile/src/session/native-host-session-file-operations.tsmobile/src/session/native-host-session-markdown-operations.tsmobile/src/session/native-host-session-native-chat-operations.test.tsmobile/src/session/native-host-session-native-chat-operations.tsmobile/src/session/native-host-session-operations.tsmobile/src/session/native-host-session-quick-command-operations.tsmobile/src/session/native-host-session-tab-operations.test.tsmobile/src/session/native-host-session-tab-operations.tsmobile/src/session/native-host-session-terminal-file-operations.tsmobile/src/session/native-host-session-terminal-operations.test.tsmobile/src/session/native-host-session-terminal-operations.tsmobile/src/session/use-mobile-file-tap-handlers.test.tsmobile/src/session/use-mobile-file-tap-handlers.tsmobile/src/session/use-mobile-native-chat-controller.tsmobile/src/session/use-mobile-native-chat-file-search.test.tsmobile/src/session/use-mobile-native-chat-file-search.tsmobile/src/session/use-mobile-native-chat-readability.test.tsmobile/src/session/use-mobile-native-chat-readability.tsmobile/src/session/use-mobile-native-chat-session-lane.tsmobile/src/session/use-mobile-native-chat-session.test.tsmobile/src/session/use-mobile-native-chat-session.tsmobile/src/session/use-mobile-native-chat-stop.test.tsmobile/src/session/use-mobile-native-chat-stop.tsmobile/src/session/use-mobile-native-chat-terminal-response-lane.tsmobile/src/session/use-mobile-session-close-actions.tsmobile/src/session/use-mobile-session-content-create-actions.tsmobile/src/session/use-mobile-session-document-readers.tsmobile/src/session/use-mobile-session-feedback-capabilities.tsmobile/src/session/use-mobile-session-file-actions.tsmobile/src/session/use-mobile-session-markdown-actions.tsmobile/src/session/use-mobile-session-native-chat-dictation.tsmobile/src/session/use-mobile-session-screen-state.tsmobile/src/session/use-mobile-session-terminal-input.tsmobile/src/session/use-mobile-session-terminal-runtime.tsmobile/src/session/use-mobile-session-terminal-send-actions.tsmobile/src/session/use-mobile-session-terminal-stream-display.tsmobile/src/session/use-quick-commands.test.tsmobile/src/session/use-quick-commands.tsmobile/src/tasks/composer-source-base-resolve.tsmobile/src/tasks/default-host-task-detail-operations.tsmobile/src/tasks/default-host-task-item-file-operations.tsmobile/src/tasks/default-host-task-item-mutation-operations.tsmobile/src/tasks/default-host-task-item-review-operations.tsmobile/src/tasks/default-host-task-linear-operations.tsmobile/src/tasks/default-host-task-list-operations.tsmobile/src/tasks/default-host-task-operations.tsmobile/src/tasks/default-host-task-preference-operations.tsmobile/src/tasks/default-host-task-project-file-operations.tsmobile/src/tasks/default-host-task-project-mutation-operations.tsmobile/src/tasks/default-host-task-project-read-operations.tsmobile/src/tasks/default-host-task-provider-write-operations.tsmobile/src/tasks/default-host-task-read-operations.tsmobile/src/tasks/github-project-host-routing-source.test.tsmobile/src/tasks/github-task-detail-projection.tsmobile/src/tasks/host-task-detail-operations.tsmobile/src/tasks/host-task-item-file-operations.tsmobile/src/tasks/host-task-item-mutation-operations.tsmobile/src/tasks/host-task-item-review-operations.tsmobile/src/tasks/host-task-linear-operations.tsmobile/src/tasks/host-task-list-operations.tsmobile/src/tasks/host-task-operations.tsmobile/src/tasks/host-task-preference-operations.tsmobile/src/tasks/host-task-project-file-operations.tsmobile/src/tasks/host-task-project-mutation-operations.tsmobile/src/tasks/host-task-project-payloads.tsmobile/src/tasks/host-task-project-read-operations.tsmobile/src/tasks/host-task-provider-payloads.tsmobile/src/tasks/host-task-provider-write-operations.tsmobile/src/tasks/host-task-read-operations.tsmobile/src/tasks/host-task-runtime-payloads.tsmobile/src/tasks/mobile-tasks-filter-pickers.tsxmobile/src/tasks/mobile-tasks-mutation-targets.tsmobile/src/tasks/mobile-tasks-project-row-targets.tsmobile/src/tasks/mobile-tasks-refactor-parity.test.tsmobile/src/tasks/native-host-task-detail-operations.tsmobile/src/tasks/native-host-task-item-file-operations.tsmobile/src/tasks/native-host-task-item-mutation-operations.tsmobile/src/tasks/native-host-task-item-review-operations.tsmobile/src/tasks/native-host-task-linear-operations.tsmobile/src/tasks/native-host-task-list-operations.tsmobile/src/tasks/native-host-task-operation-contracts.test.tsmobile/src/tasks/native-host-task-operations.tsmobile/src/tasks/native-host-task-preference-operations.tsmobile/src/tasks/native-host-task-project-file-operations.tsmobile/src/tasks/native-host-task-project-mutation-operations.tsmobile/src/tasks/native-host-task-project-read-operations.tsmobile/src/tasks/native-host-task-provider-write-operations.tsmobile/src/tasks/native-host-task-read-operations.tsmobile/src/tasks/native-host-task-review-comment-addressing.test.tsmobile/src/tasks/setup-hook-trust.tsmobile/src/tasks/smart-source-fan-out.test.tsmobile/src/tasks/smart-source-fan-out.tsmobile/src/tasks/smart-source-paste-concurrency.test.tsmobile/src/tasks/smart-source-paste-intent.test.tsmobile/src/tasks/smart-source-paste-intent.tsmobile/src/tasks/smart-source-search-requests.tsmobile/src/tasks/source-workspace-create.tsmobile/src/tasks/use-mobile-tasks-client-settings-actions.tsxmobile/src/tasks/use-mobile-tasks-github-check-file-actions.tsxmobile/src/tasks/use-mobile-tasks-github-reply-merge-actions.tsxmobile/src/tasks/use-mobile-tasks-gitlab-github-status-actions.tsxmobile/src/tasks/use-mobile-tasks-item-detail-loading.tsxmobile/src/tasks/use-mobile-tasks-item-detail-metadata-effects.tsxmobile/src/tasks/use-mobile-tasks-linear-item-actions.tsxmobile/src/tasks/use-mobile-tasks-list-and-detail-effects.tsxmobile/src/tasks/use-mobile-tasks-project-detail-loading.tsxmobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsxmobile/src/tasks/use-mobile-tasks-project-loading-actions.tsxmobile/src/tasks/use-mobile-tasks-project-metadata-actions.tsxmobile/src/tasks/use-mobile-tasks-project-metadata-loading.tsxmobile/src/tasks/use-mobile-tasks-project-repository-resolution.tsxmobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsxmobile/src/tasks/use-mobile-tasks-project-thread-reply-actions.tsxmobile/src/tasks/use-mobile-tasks-project-workspace-comment-actions.tsxmobile/src/tasks/use-mobile-tasks-provider-load-actions.tsxmobile/src/tasks/use-mobile-tasks-route-and-item-state.tsxmobile/src/tasks/use-mobile-tasks-runtime-hydration.tsxmobile/src/tasks/use-mobile-tasks-task-create-actions.tsxmobile/src/tasks/use-mobile-tasks-task-list-loading.tsxmobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsxmobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsxmobile/src/tasks/use-mobile-tasks-workspace-source-effects.tsxmobile/src/tasks/use-mobile-tasks-workspace-sparse-actions.tsxmobile/src/tasks/use-mobile-tasks-workspace-ssh-state.tsxmobile/src/tasks/use-smart-workspace-source.tsmobile/src/tasks/worktree-create-capability.tsmobile/src/tasks/worktree-create-retry.tsmobile/src/terminal/terminal-input-connection-gate.test.tsmobile/src/transport/rpc-client.tsmobile/src/transport/screen-rpc-ratchet.test.tsmobile/src/worktree/default-host-workspace-catalog-operations.tsmobile/src/worktree/default-host-workspace-creation-operations.tsmobile/src/worktree/default-host-workspace-operations.tsmobile/src/worktree/host-workspace-catalog-operations.tsmobile/src/worktree/host-workspace-creation-operations.tsmobile/src/worktree/host-workspace-host-context.tsmobile/src/worktree/host-workspace-operations.tsmobile/src/worktree/native-host-workspace-catalog-operations.test.tsmobile/src/worktree/native-host-workspace-catalog-operations.tsmobile/src/worktree/native-host-workspace-creation-operations.test.tsmobile/src/worktree/native-host-workspace-creation-operations.tsmobile/src/worktree/native-host-workspace-creation-read-operations.tsmobile/src/worktree/native-host-workspace-creation-source-operations.tsmobile/src/worktree/native-host-workspace-operations.tsmobile/src/worktree/rpc-workspace-creation-operations.tsmobile/src/worktree/use-retired-worktree-names.test.tsxmobile/src/worktree/use-retired-worktree-names.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| const sourceTabId = | ||
| terminalHandle ?? activation.sourceTerminalHandle ?? activation.sourceSessionTabId ?? '' | ||
| // Why: resolution opts into sibling-workspace hits; this caller honors resolved.workspaceId. | ||
| const resolved = await options.operations.resolveTerminalPath({ | ||
| workspaceId: options.worktreeId, | ||
| tabId: sourceTabId, | ||
| terminalHandle, | ||
| pathText: options.pathText, | ||
| cwd, | ||
| nativeChatContext: options.nativeChatContext ?? null, | ||
| line: options.line, | ||
| column: options.column | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Check every implementation of resolveTerminalPath for tabId usage.
rg -n -C 12 'resolveTerminalPath' --glob 'mobile/src/session/*terminal-file-operations*.ts'
rg -n 'tabId' --glob 'mobile/src/session/*terminal-file-operations*.ts'Repository: stablyai/orca
Length of output: 4712
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- repository conventions and learnings ---'
# No repository-knowledge capability is needed for this narrow contract check.
printf '%s\n' '--- mobile caller and request contract ---'
sed -n '55,100p' mobile/src/session/mobile-file-tap-open.ts
sed -n '20,45p' mobile/src/session/host-session-terminal-file-operations.ts
printf '%s\n' '--- native adapter ---'
sed -n '12,32p' mobile/src/session/native-host-session-terminal-file-operations.ts
printf '%s\n' '--- all files.resolveTerminalPath bindings and tabId references ---'
rg -n -C 8 "files\.resolveTerminalPath|resolveTerminalPath|tabId" mobile src --glob '*.ts' --glob '*.tsx' | head -n 240Repository: stablyai/orca
Length of output: 20560
Forward tabId in the native adapter. openMobileFileTapAsync supplies sourceTabId, but nativeHostSessionTerminalFileOperations.resolveTerminalPath omits request.tabId from files.resolveTerminalPath. A structured chat tab without a terminal handle therefore reaches the resolver without its session-tab anchor.
| { worktree: `id:${workspaceId}`, relativePath }, | ||
| { timeoutMs: 15_000 } | ||
| ) | ||
| if (!response.ok || !(response.result as RuntimeFileOpenResult).opened) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the result shape before reading opened.
If a host/version-skew response has ok: true with a null or undefined result, this access throws a TypeError instead of the adapter's file_open_failed error. Use a null-safe check.
🐛 Proposed fix
- if (!response.ok || !(response.result as RuntimeFileOpenResult).opened) {
+ if (!response.ok || (response.result as RuntimeFileOpenResult | null)?.opened !== true) {
throw new Error('file_open_failed')
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!response.ok || !(response.result as RuntimeFileOpenResult).opened) { | |
| if (!response.ok || (response.result as RuntimeFileOpenResult | null)?.opened !== true) { |
| { | ||
| repo: `id:${target.repoId}`, | ||
| prNumber: target.number, | ||
| updates: { title: updates.title, body: updates.body } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Route GitHub pull-request label and assignee changes through github.updateIssue.
When updateMetadata receives a GitHub pull-request target, this branch sends only title and body to github.updatePR. That RPC accepts only those fields. Send addLabels, removeLabels, addAssignees, and removeAssignees through github.updateIssue. Coordinate both requests when one update contains title/body and label or assignee changes.
| const result = response.result as { | ||
| ok?: boolean | ||
| error?: string | ||
| comment?: DetailComment | ||
| } | ||
| if (result.ok === false) { | ||
| throw new Error(result.error ?? fallback) | ||
| } | ||
| return result.comment |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Default nullable mutation results before reading ok.
If a review-mutation RPC returns { ok: true, result: null }, mutationComment and assertMutation can throw a TypeError when they read result.ok. The shared RPC contract accepts this success shape. Default the result to an empty object in both helpers:
🐛 Proposed fix: default the result body
- const result = response.result as {
+ const result = (response.result ?? {}) as {
ok?: boolean
error?: string
comment?: DetailComment
}- const result = response.result as { ok?: boolean; error?: string }
+ const result = (response.result ?? {}) as { ok?: boolean; error?: string }| async function bootstrap(client: RpcRequestSender): Promise<HostTaskBootstrap> { | ||
| const statusResponse = await client.sendRequest('status.get') | ||
| requireSuccess(statusResponse) | ||
| const status = statusResponse.result as { capabilities?: string[] } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the RPC response contract and the relevant command fixtures/handlers.
rg -n -C 5 'export type Rpc(Response|Success)|result:' mobile/src/transport
rg -n -C 5 "sendRequest\\('(status.get|repo.list|github.createIssue|gitlab.createIssue)" mobile/src/tasks
rg -n -C 5 "(status\\.get|repo\\.list|github\\.createIssue|gitlab\\.createIssue)" \
mobile src --glob '*test*' --glob '*.ts' --glob '*.tsx'Repository: stablyai/orca
Length of output: 50370
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions
Length of output: 29495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- changed task files ---'
sed -n '1,140p' mobile/src/tasks/native-host-task-read-operations.ts
sed -n '1,120p' mobile/src/tasks/native-host-task-provider-write-operations.ts
printf '%s\n' '--- direct imports and RPC definitions ---'
rg -n -C 4 '^(import|export).*?(sendRequest|RuntimeRpc|RpcResponse|NativeHost)|sendRequest|interface .*Response|type .*Response' \
mobile/src/tasks mobile/src/transport mobile/src --glob '*.ts' --glob '*.tsx' \
| head -n 500
printf '%s\n' '--- exact task call sites and nearby error handling ---'
rg -n -C 8 'status\.get|repo\.list|github\.createIssue|gitlab\.createIssue' \
mobile/src/tasks --glob '*.ts' --glob '*.tsx' | head -n 500Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- native-host-task-read-operations.ts ---'
cat -n mobile/src/tasks/native-host-task-read-operations.ts | sed -n '1,110p'
printf '%s\n' '--- native-host-task-provider-write-operations.ts ---'
cat -n mobile/src/tasks/native-host-task-provider-write-operations.ts | sed -n '1,90p'
printf '%s\n' '--- symbols imported by these files ---'
head -n 35 mobile/src/tasks/native-host-task-read-operations.ts
head -n 35 mobile/src/tasks/native-host-task-provider-write-operations.ts
rg -n -C 6 'function sendRequest|const sendRequest|sendRequest<' mobile/src --glob '*.ts' --glob '*.tsx' | head -n 300
printf '%s\n' '--- response type declarations ---'
rg -n -C 8 'RuntimeRpcResponse|RpcResponse|NativeHost.*Response|result\?:|result:' mobile/src shared/src --glob '*.ts' --glob '*.tsx' | head -n 500Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository knowledge files ---'
if command -v find >/dev/null; then
find /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4 -maxdepth 2 -type f -print 2>/dev/null | head -n 80
fi
printf '%s\n' '--- exact initial-log matches for transport and task files ---'
rg -n -C 8 'native-host-task-read-operations|native-host-task-provider-write-operations|export (type|interface) .*Rpc|RuntimeRpcResponse|sendRequest' \
/tmp/coderabbit-shell-logs/shell-output-7srvV7 | head -n 500Repository: stablyai/orca
Length of output: 43616
Validate RPC result shapes before field access.
RpcSuccess.result is unknown; requireSuccess and response.ok only validate the envelope. These casts perform no runtime validation. A null or undefined result can cause a TypeError, while other wrong shapes can return invalid values at status.capabilities, result.repos, or the issue fields. Add narrow runtime decoders for the object and expected arrays and fields, and throw a controlled task-operation error.
📍 Affects 2 files
mobile/src/tasks/native-host-task-read-operations.ts#L24-L24(this comment)mobile/src/tasks/native-host-task-read-operations.ts#L59-L59mobile/src/tasks/native-host-task-provider-write-operations.ts#L20-L20
- Clear terminal consumes the boolean the adapter returns, so a clear that never reached the host no longer toasts success. - A refused `@`-autocomplete search returns null instead of an empty list, so the failure is not cached and the next keystroke retries. - The Linear workspace picker reloads the host's context on both outcomes, and `selectWorkspace` detects a refusal by the id the host echoes back, since it answers a refusal with its unchanged status and no `ok` field. - `read.loadLinearContext` splits into `linearStatus` and `linearTeams`, so the screen commits the workspace list and the resolved selection before it asks for teams, as it did before the seam existed. - Resolving a review thread reports the host's message for a transport failure and the caller's wording only for a refusal, in both the item and the Projects lane. - The RPC ratchet matches the identifier rather than `sendRequest(`, counts client subscriptions, walks every source directory and all of `mobile/app`, classifies adapters by path, and pins each remaining site by file. Verified against bind, bracket access, a space before the paren, an aliased client, a helper in another directory and an adapter-shaped new file. - Deletes the adapter families nothing constructs (file preview, workspace catalog and the workspace group), the dead device wiring, and the three methods that shadowed a live call with different behavior. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
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 (3)
mobile/src/session/native-host-session-terminal-file-operations.ts (1)
42-42: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
response.resultbefore reading.opened.RpcSuccess.resultisunknown, and the reachable file-tap path callsopenWorktreeFile. Iffiles.openreturnsok: truewithresult: null, the cast does not add a runtime check, so this expression throwsTypeErrorinstead ofError('file_open_failed'). Check for a non-null result first and use the existing failure error when it is absent oropenedis false.mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx (1)
165-168: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAwait
listOperations.countGitHubinside thistryblock.countGitHubreturns a promise, so the currentreturncatches only synchronous errors. A rejected count propagates throughmapWithConcurrency, and the total is not updated. Usereturn awaitso the catch returns0for that repository.mobile/src/tasks/native-host-task-item-review-operations.ts (1)
126-134: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard successful mutation results before reading
ok.RpcSuccess.resultisunknown, butmutationCommentandassertMutationdereferenceresult.okwithout checking fornullorundefined. A successful comment, reviewer, or merge mutation can therefore throwTypeErrorinstead of its operation-specific fallback. Guard the result object in both helpers. KeepresolveThread's existing non-truecheck.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: c0b3e8d6-32cc-4c60-92a9-3552372150c7
📒 Files selected for processing (30)
mobile/src/device/device-operations.tsmobile/src/device/native-device-operations.tsmobile/src/session/host-session-native-chat-operations.tsmobile/src/session/host-session-tab-operations.tsmobile/src/session/host-session-terminal-operations.tsmobile/src/session/mobile-session-route-parity.test.tsmobile/src/session/native-host-session-browser-operations.tsmobile/src/session/native-host-session-native-chat-operations.tsmobile/src/session/native-host-session-tab-operations.test.tsmobile/src/session/native-host-session-tab-operations.tsmobile/src/session/native-host-session-terminal-operations.test.tsmobile/src/session/native-host-session-terminal-operations.tsmobile/src/session/use-mobile-native-chat-file-search.test.tsmobile/src/session/use-mobile-native-chat-file-search.tsmobile/src/session/use-mobile-session-terminal-input.tsmobile/src/tasks/host-task-read-operations.tsmobile/src/tasks/host-task-runtime-payloads.tsmobile/src/tasks/linear-status-projection.tsmobile/src/tasks/mobile-tasks-filter-pickers.tsxmobile/src/tasks/mobile-tasks-refactor-parity.test.tsmobile/src/tasks/native-host-task-item-review-operations.tsmobile/src/tasks/native-host-task-linear-operations.tsmobile/src/tasks/native-host-task-operation-contracts.test.tsmobile/src/tasks/native-host-task-project-mutation-operations.tsmobile/src/tasks/native-host-task-read-operations.tsmobile/src/tasks/use-mobile-tasks-project-detail-loading.tsxmobile/src/tasks/use-mobile-tasks-provider-load-actions.tsxmobile/src/tasks/use-mobile-tasks-route-and-item-state.tsxmobile/src/transport/screen-rpc-ratchet.test.tsmobile/src/worktree/native-host-workspace-creation-read-operations.ts
💤 Files with no reviewable changes (6)
- mobile/src/tasks/use-mobile-tasks-route-and-item-state.tsx
- mobile/src/device/native-device-operations.ts
- mobile/src/session/native-host-session-tab-operations.ts
- mobile/src/session/host-session-terminal-operations.ts
- mobile/src/session/host-session-tab-operations.ts
- mobile/src/session/native-host-session-terminal-operations.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| // No move/down/up fallback here: the live caller wraps that sequence in a try/catch | ||
| // because pointer commands race page navigation, and a throwing copy of it would surface | ||
| // those races as errors the user never saw. | ||
| await client.sendRequest( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Check how callers of the browser click operation handle success and failure.
rg -nP -C6 '\.click\s*\(' --type=ts --type=tsx mobile/src | rg -n -C6 'browser|click' | head -80
rg -nP -C8 'browser\.mouseClick' mobile/srcRepository: stablyai/orca
Length of output: 3268
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- adapter operations ---'
sed -n '1,130p' mobile/src/session/native-host-session-browser-operations.ts
printf '%s\n' '--- requestResult and client contract ---'
rg -n -C8 'function requestResult|const requestResult|requestResult\s*=|sendRequest\s*\(' mobile/src/session mobile/src/browser --glob '*.ts'
printf '%s\n' '--- direct caller ---'
sed -n '90,145p' mobile/src/browser/use-mobile-browser-commands.tsRepository: stablyai/orca
Length of output: 50370
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings
Length of output: 21485
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- RpcClient declaration and implementation ---'
rg -n -C12 'export (interface|type) RpcClient|interface RpcClient|class .*RpcClient|sendRequest\(' mobile/src/transport mobile/src --glob 'rpc-client.ts' --glob '*.ts' | head -220
printf '%s\n' '--- browser operation interface and native adapter wiring ---'
rg -n -C10 'HostSessionBrowserOperations|nativeHostSessionBrowserOperations|operations\.click|browserOperations' mobile/src --glob '*.ts' --glob '*.tsx' | head -260
printf '%s\n' '--- click helper contract ---'
sed -n '100,165p' mobile/src/browser/use-mobile-browser-commands.tsRepository: stablyai/orca
Length of output: 30953
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- browser operation consumers ---'
rg -n -C8 '\.browser\.click|browser\.click|browser:\s*.*operations|const .*browser.*operations|HostSessionBrowserOperations' mobile --glob '*.ts' --glob '*.tsx' | head -260
printf '%s\n' '--- request helper and response handling ---'
sed -n '1,85p' mobile/src/browser/use-mobile-browser-request.ts
rg -n -C5 'function assertRpcOk|const assertRpcOk|assertRpcOk' mobile/src/browser/use-mobile-browser-commands.tsRepository: stablyai/orca
Length of output: 13184
Surface refused browser.mouseClick responses. click awaits client.sendRequest directly and ignores ok: false, so a refused click resolves successfully. Use requestResult for this request while keeping the navigation-race fallback omitted.
| async updateState(target, stateId) { | ||
| await request(client, 'linear.updateIssue', { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate the linear.updateIssue result before resolving updateState. The host returns { ok: false, error } when the Linear mutation fails, but the request helper checks only the outer RPC envelope. The reachable caller updates the local item after updateState resolves, so it can display an unapplied state. Apply assertMutation to this result as addComment does.
| setSelectedLinearWorkspaceId(workspaceId) | ||
|
|
||
| const teamsResponse = await client.sendRequest('linear.listTeams', { | ||
| workspaceId: workspaceId ?? undefined | ||
| }) | ||
| if (!isSuccess(teamsResponse)) { | ||
| throw new Error(teamsResponse.error.message) | ||
| } | ||
| const teams = teamsResponse.result as LinearTeam[] | ||
| const teams = await taskOperations.read.linearTeams(workspaceId) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear stale Linear team state before loading the new workspace.
If linearTeams(workspaceId) rejects, the new workspace is already committed, but linearTeams and selectedLinearTeamIds retain their previous values. The team picker can then show teams from the previous workspace, and task filtering can use stale team IDs. Clear both before the awaited request.
Proposed fix
setLinearWorkspaces(status.workspaces)
setSelectedLinearWorkspaceId(workspaceId)
+setLinearTeams([])
+setSelectedLinearTeamIds(new Set())
const teams = await taskOperations.read.linearTeams(workspaceId)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setSelectedLinearWorkspaceId(workspaceId) | |
| const teamsResponse = await client.sendRequest('linear.listTeams', { | |
| workspaceId: workspaceId ?? undefined | |
| }) | |
| if (!isSuccess(teamsResponse)) { | |
| throw new Error(teamsResponse.error.message) | |
| } | |
| const teams = teamsResponse.result as LinearTeam[] | |
| const teams = await taskOperations.read.linearTeams(workspaceId) | |
| setSelectedLinearWorkspaceId(workspaceId) | |
| setLinearTeams([]) | |
| setSelectedLinearTeamIds(new Set()) | |
| const teams = await taskOperations.read.linearTeams(workspaceId) |
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
This run reviewed the single follow-on commit (1b86a68, "close the seven defects the adapter review found") on top of the prior Pullfrog review.
- Removed unreferenced adapters — dropped the file-preview and workspace-catalog adapter families and narrowed
DeviceOperationstorandomNonce, consistent with the "only live consumers" rule. - Removed three dead session methods —
createBlank,createAgent, andsendQueryReplyare gone from the session adapters and their interfaces. - Fixed file-search refusal caching —
searchFilesnow returnsstring[] | null, so a refusedfiles.searchPathsis no longer cached as an empty autocomplete answer (with a test that a refused search retries on the next keystroke). - Split
loadLinearContext—linearStatusandlinearTeamsnow commit workspaces before the team read, so a failed team read no longer blanks the picker. - Hardened
linear.selectWorkspace— refusal is detected by the echoedselectedWorkspaceIdrather thanassertMutation. - Fixed
handleClearTerminal— removed a double-clear and keyed the toast off the host'sokresult. - Rewrote the screen-RPC ratchet — now also matches inline
subscribe(several alias/bracket/stream shapes) and pins remaining inline RPC sites by method name, addressing the prior review'ssendRequest-only concern.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Returning the promise from inside the try let a rejected count escape the per-repo catch, reject the whole batch and reach the caller, which fires this without a handler. The inline call it replaced awaited and reported zero. The new test drives the real batching shape and fails if the await is removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Only back, forward and reload have a consumer; the browser pane still issues its own RPCs. subscribe, navigate, scroll, click, insertText, keypress and dialog go, with the event and frame-comparison types that existed only for them. click's fallback had also lost main's move/down/up sequence and cited a caller that does not exist here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
The native chat subscribe declared an onError the adapter never invoked, since the transport has none: a stream error arrives as an error frame through the listener, which is where the caller already handles it. The hook's second callback was unreachable, so it goes with the parameter. The terminal adapter's acknowledge was a no-op with no caller here or on main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
resolveTerminalPath took a tabId the native adapter never sent, and main's wire had none either, so the caller's sourceTabId computation fed nothing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the four commits pushed since the prior Pullfrog review (1b86a68): three dead-surface removals that close out the open threads, and one per-repo count robustness fix.
- Dropped the seven dead browser adapter methods —
HostSessionBrowserOperationsnow carries onlyback/forward/reload, its three live consumers; the uncalledsubscribe/navigate/scroll/click/insertText/keypress/dialogmethods and their screencast types are gone. - Removed the native-chat
onErrorcallback —subscribeno longer declares anonErrorparam the transport could never fire; the interface now documents that stream errors arrive aserrorframes throughonEvent. - Removed the no-op terminal
acknowledge— dropped fromHostSessionTerminalOperationsand its native implementation (nothing called it). - Dropped the resolve
tabIdfield —HostSessionTerminalFileResolveRequest.tabIdand thesourceTabIdderivation inmobile-file-tap-open.tswere removed, since the native adapter never sent it on the wire. - Awaited per-repo GitHub counts —
countGitHubItemsnowawaitscountGitHubinside its per-repotryso a rejected count is read as zero instead of rejecting the whole batch, with a test that pins the propagation.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
… phases read.bootstrap() folded status.get and the settings fan-out behind one await, so the supported commit ran only after all five requests succeeded. A post-connect timeout on any of the four left tasksSupportState at 'unknown', which the list surface renders as a bare spinner, and the effect deps do not change on that failure so it never retried. The probe is now read.tasksSupported() and the fan-out is read.bootstrap(), with the caller committing supported and clearing the error between them, as it did before the seam existed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
- The Project host oracle no longer accepts a bare `payload` identifier, which every call site satisfies by construction. Inline payloads must show a host or a slug builder; the four typed-payload reads are named explicitly and their payload types are pinned to carry the host. - `prRepo` is pinned per mutation and bounded to that adapter method, so dropping it from one of six fails instead of needing all six. `github.addPRReviewComment` is pinned again, with the row repository. - The smart-source fan-out fake now delegates to the shipped search functions over a scripted transport, so the repoId stamping under test is the real one rather than the fake's copy. - The retired-name read's method and `id:` prefix are asserted again, in the adapter test, since the hook now takes a callback. - The session adapters join the route parity family, bringing the runtime strings and identity fields that moved out of the hooks back into scope. Each restored assertion was verified to fail under the mutation it guards. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the two follow-on commits since the prior Pullfrog review (cac6346): one restoring main's observable behavior and one hardening the extracted adapter tests.
- Split
tasksSupported()out ofbootstrap()— the Tasks hydration hook now commits itssupportedstate before the settings/ui/preflight/linear fan-out, so a post-connect transport failure renders behind an error banner instead of a permanent spinner (pinned byuse-mobile-tasks-hydration-phases.test.tsx). - Restored the 30 s project-mutation timeouts — resolve-thread, review-reply, conversation-comment, and request-reviewers now pass a timeout through the shared
github-pr-mutationsfetchers via a newoptionsparam, with rerun/merge keeping their 60 s budget. - Made Linear comment reads fail on transport, not refuse —
optionalCommentstolerates only a refusal envelope, so a timed-out comment read again fails the detail load. - Reverted five parity drifts — the
github.listWorkItemstimeout, the clear-terminal toast on refusal, the SSH badge on a null-state answer, four error strings, and the legacy file-inventory cache reset (resetFileSearchCache). - Removed remaining dead surface — the
devicemodule,host-workspace-host-context.ts, uncalled tab/account methods, andnativeChat.openFile. - Restored four weakened test assertions — the Project host oracle, per-method
prRepo, smart-source fan-out stamping, and four unpinned RPC names, each verified to fail under mutation.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
The per-call slice was still a fixed 700 characters, so deleting host from updateComment passed on deleteComment's host sitting inside the window. It now ends at the enclosing method, matching the prRepo loop, and accepts the shorthand property form that github.project.listAccessible uses. Verified: dropping `host: target.host` from updateComment fails the oracle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
- HostSessionNativeChatOperations sendMessage, respond and attachImage. Every shipped send path calls sendMobileNativeChatMessageWithOutcome or typeMobileNativeChatCommandWithOutcome directly, as on main. - HostWorkspaceCreationOperations createBlankWorkspace and createWorkspaceFromSource. use-new-workspace-create-submit imports createBlankWorkspace from the source module instead. With those two gone the native creation factory only spread the RPC operations and RpcWorkspaceCreationOperations only excluded them, so both collapse. The arg and result types that existed solely for the five methods go with them, as does the dead SessionTabsResult re-export. Each name was grepped against non-test, non-adapter callers before and after. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the two commits pushed since the prior Pullfrog review (0aaa70e): a test-hardening commit and a dead-surface removal commit.
- Dropped the five dead adapter methods — native-chat
sendMessage/respond/attachImageand workspace-creationcreateBlankWorkspace/createWorkspaceFromSourceare gone. I confirmed the send paths callsendMobileNativeChatMessageWithOutcomeand the image-upload hooks directly, and the create paths importblank-workspace-create.ts/source-workspace-create.tsdirectly, so these adapter methods had no live caller. - Removed the dead
SessionTabsResultre-export — every consumer already imports it frommobile-session-route-types.ts. - Bounded the Project host-routing oracle — the test now slices to the enclosing adapter method (
\n },) and matches\bhost\s*[:,}], so a neighbour'shostcan no longer satisfy the assertion. - Re-froze the session-route parity digest — runtime strings 593 → 592, matching the removed method bodies.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 82153d6d-c543-4188-a961-c8caf329fdb8
📒 Files selected for processing (8)
mobile/src/session/host-session-native-chat-operations.tsmobile/src/session/host-session-tab-operations.tsmobile/src/session/mobile-session-route-parity.test.tsmobile/src/session/native-host-session-native-chat-operations.tsmobile/src/tasks/github-project-host-routing-source.test.tsmobile/src/worktree/host-workspace-creation-operations.tsmobile/src/worktree/native-host-workspace-creation-operations.tsmobile/src/worktree/rpc-workspace-creation-operations.ts
💤 Files with no reviewable changes (3)
- mobile/src/session/host-session-native-chat-operations.ts
- mobile/src/session/native-host-session-native-chat-operations.ts
- mobile/src/session/host-session-tab-operations.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
… behind The tab adapter's capabilities() method was removed as uncalled; this module existed only to serve it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…ports Each was a one-line pass-through to its native factory. The five domain-level defaults are the only ones the screens construct. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the two commits pushed since the prior Pullfrog review (5cbbadf): a mechanical dead-surface census that removes 21 files nothing imports and prunes the ratchet's adapter list to match.
- Dropped the dead capability projection —
host-session-runtime-capabilities.ts(andprojectHostSessionRuntimeCapabilities) existed only to serve the tab adapter'scapabilities()method, which was already removed as uncalled; no references remain anywhere inmobile/. - Dropped 20 one-line per-namespace default factories — each
defaultHost<X>Operationswas a pass-through to itsnativeHost<X>Operationswith zero importers. The five domain-level defaults (session/task/account/file-explorer/workspace-creation) are the only ones screens construct. - Pruned the ratchet adapter list —
screen-rpc-ratchet.test.ts'sADAPTER_FILESset dropped the 20 removed paths, keeping the "pins every remaining inline RPC site" assertion consistent with the deleted files.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the single follow-on commit (753ec13, "bound the Project host oracle to a real method boundary") pushed since the prior Pullfrog review (892c6fc).
- Extracted
enclosingAdapterMethod— a helper that slices the adapter source from an offset to the next 4‑space‑indent method-closing brace (\n {4}\},?\n) and throws if none exists, so ahost/prRepo/repoPayloadassertion can no longer leak into a neighbour method or the rest of the file when the boundary is absent. - Replaced the three
indexOf('\n },')+ EOF‑fallback slices — thehost-qualification, per-methodprRepo, and per-methodrepoPayloadassertions now use the helper instead of falling back toslice(offset, undefined). - Corrected the last‑method boundary — the comma‑optional regex (
},?) now stops at a file's final method closing with}rather than},, which the previous code could only handle by silently slicing to end‑of‑file.
Verified mobile/src/tasks/github-project-host-routing-source.test.ts passes (4/4), and confirmed the regex matches the actual 4‑space method indentation in all three adapter source files it inspects.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
main had no clearInputFirst. The flag prefixed a clear byte onto the body of a terminal.send, which is the shape the same file documents as observed broken in the field: the burst reached the agent as literal control characters with the draft still parked. No production caller set it. mobile-native-chat-send.ts now has no diff against main. The assertions that existed only to prove nobody set the flag are gone; the case that pins verbatim text stays and fails if a prefix comes back. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
main checked only response.ok, pruned the tab, wrote a tombstone and let the host's republished snapshot restore it if the close was refused. The adapter had mapped a refusal to an outcome the caller skipped every state commit for, leaving the tab on screen, and had added a `closed !== true` precondition main never had, so a host answering ok without that field would strand the tab. close() returns a boolean again and the caller prunes on it. The test that pinned the refusal projection is replaced by two that pin main's contract: ok with an empty result still reports acceptance, a transport refusal does not. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
… identity again
Stop routed through the shared chat write, which refuses to start under a 2s
residual budget. The call it replaced tried on anything above zero and could be
accepted, so Stop now has its own send: main's params, main's
{ timeoutMs, budgetSpansConnect: true }, no enter field, the same worker
takeover report on accept and the same unknown/rejected classification.
The parity oracle also follows the two adapters the route's identity fields
moved into, since the visitor only expands bare-identifier calls and could
never reach operations.terminal.sendInput. Identity fields are 15 rather than
main's 14: the seam turned two hook-side deviceToken fields into three
adapter-side client:{id} builders. Dropping the binding in the adapter now
fails the oracle.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…umer sparseCheckout was added to the composer create args and forwarded into the worktree.create params; its only caller never passes it. warning was added to WorktreeCreateResult and read off the response; nothing reads it, and the one screen that shows a create warning is on the unchanged inline path with its own copy. worktree-create-retry.ts and source-workspace-create.ts are now byte-identical to main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…r retry main handled `!response.ok` after the retry helper returned. Throwing inside the try made a refusal replayable, because the cutover predicate matches any Error whose message is exactly the migration string, so a host refusal carrying that text was retried five times. The helper returns the response again and the caller checks it. The new test fails under the old shape with six attempts instead of one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Each was confirmed to have zero non-test readers before and after:
- HostSessionTerminalOperations.subscribe, its adapter body and its test. The
live stream still calls subscribeMobileTerminalSafely directly, as on main,
so the method existed only to be tested.
- pasteImages and releaseImages, declared on the native-chat interface and
never implemented or called.
- explicitQuickCommandScope, exported with no callers.
- The quick-command snapshot's totalCount and repoId, which nothing read, and
with them the workspaceId argument that existed only to compute repoId. The
hook takes { operations, enabled } again, as main took { client, enabled }.
- The web-artifact result variant, line and column on the resolve request,
rename's third parameter and the directory variant's truncated. The
legacy-list variant's truncated stays: the panel reads it.
Deleting the terminal subscribe also brings the parity oracle's identity
fields to 14, matching main exactly.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…actions Each had gained a non-null target requirement that demanded a number and a recognised item type where main's guard did not, so a row main would have sent for now returns silently or reports an error. File-viewed and the comment reply gate on main's conditions and build the never-null identity target after them. Metadata update, issue type and comment add gate on the slug and the number, as main did, through the slug target, which does not read the item type. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
- listRepositories dereferences result.repos, so a repo.list answer without it
throws into the caller's catch as it did before.
- projectResult requires an explicit ok:true again; a result that omits the
flag is not a confirmed success.
- readRuntimeSettings returns null for a settings-less answer, so the create
sheet keeps what it already had instead of committing {}. The task-create
path keeps its own {} default, which is what it had.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…to its call The project target carried a `slug` field the type does not declare and omitted owner/repo/host, so repoPayload read them as undefined and the oracle only ever exercised the prRepo:null path. It now uses the real shape and asserts the row repository reaches the wire; blanking prRepo fails it. The connection gate asserts TERMINAL_INPUT_SEND_OPTIONS inside sendInput rather than anywhere in the counted set; removing it from that call fails the gate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…e comments The file explorer's reconnect ternary called the same function on both sides, and its comment claimed the null branch mattered. Retry now calls forceReconnect directly, as main did, and `reconnect` leaves the explorer operations interface, where a UI action did not belong. Comments corrected: the ratchet's regex doc no longer claims aliasing cannot hide a subscribe, and names the two shapes it misses; the deliberate-inline comment says those four are in the routed screens, not the whole remaining surface; and the chat stop's layout effect names the gate that requires it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
This run reviewed the ten commits pushed since the prior Pullfrog review (753ec13): a parity-restoration pass that reverts the adapters to main's observable behavior and deletes the seam surface the extraction had left unreferenced.
- File-explorer Retry routed through the transport — the disconnected-state revive path now calls
forceReconnectdirectly instead of hanging off anoperations.reconnect(), soHostFileExplorerOperationsno longer carries areconnectmethod anddefaultHostFileExplorerOperationstakes only the client. - Quick-command seam slimmed and the envelope check moved — the
workspaceId/repoId/totalCountthreading is gone fromsnapshot/mutate(the sheet derivesrepoIdfrom route helpers itself), andokis checked outside the cutover retry. - Tab close prunes on
okalone —close()returns the transport'sokinstead of theHostSessionTabCloseResultunion, matchingmainwhich never inspected the result body. - Stop uses its own send —
sendStopEscapesends a bare Escape (noenter, no 2 s floor) on whatever budget remains, and the device-identity binding is re-pinned via the newADAPTER_IDENTITY_SOURCESparity walk. - Project-row guards restored to
main— the four rows re-gate onprojectRowSlugTarget/projectRowIdentityTarget, andprojectResultnow requires an explicitok: true,listRepositoriesdereferencesreposdirectly, andreadRuntimeSettingsreturnsnullfor a settings-less answer. - Dead surface removed — the Ctrl+U
clearInputFirstsend path, the native-chatpasteImages/releaseImagesoptions, the terminalsubscribeadapter method, and two create fields with no producer or consumer are gone.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
… leftovers The refused-body case had no test after the round-5 revert; reintroducing the defect passed the suite. Also removes the stream types stranded when the terminal subscribe adapter was deleted, and a truncated field the directory result type no longer declares. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the single follow-on commit (bcba08b, "pin that a refused tab close still prunes, and drop two leftovers") pushed since the prior Pullfrog review (59e5e6e).
- Dropped the dead
truncatedfield —nativeHostFileExplorerOperationsno longer emitstruncated: falseon thedirectoryresult, closing the prior review's open nit;truncatedonly exists on thelegacy-listvariant. - Removed the stranded terminal stream types —
HostSessionTerminalData/HostSessionTerminalStreamEvent/HostSessionTerminalSubscribeArgsand theTerminalOscLinkRangeimport are gone fromhost-session-terminal-operations.ts, the surplus left behind when thesubscribeadapter was deleted; I confirmed no references remain repo-wide. - Added a refused-close regression pin —
native-host-session-tab-operations.test.tsnow assertsclose()resolvestrueonok: trueeven when the result body reportsrefused: true, so reintroducing result-body inspection fails the suite.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the ten commits pushed since the prior Pullfrog review (bcba08b): the three parity regressions a Codex whole-diff read had surfaced, the type/fixture cleanups that landed alongside them, and the fix-forward commits that repaired a parallel-worker overwrite in the middle of the range.
- Legacy file-inventory generation guard —
searchFilesinnative-host-session-native-chat-operations.tsnow bumps a per-workspace generation onresetFileSearchCacheand drops an in-flightfiles.listcompletion whose generation no longer matches, so a slow inventory started before leaving a workspace can no longer repopulate that workspace's cache afterward; the in-flight cleanup is identity-guarded. - Project mutation result acceptance —
projectMutationgained arequireOkparameter:addCommentrequires a truthyok, the other mutations checkresult.ok === falseon a dereferenced result (a null/undefined result throws before any optimistic commit), andresolveReviewThreadrequires a literaltrueresult. Error wording splits bare-string (github.*) vs object (github.project.*) failures consistently withgithub-pr-mutations.ts. - Linear detail settlement order —
loadLinearawaits both raw requests viaPromise.alland checks the issue envelope before comments, so a comments transport rejection again wins over an issue refusal while a refused comments envelope still yields an empty thread. - Projection/list types tightened — the top-level fallback fields the detail projection accepted beyond
itemare gone, list results are typed as the rawOmit<…, 'repoId' | 'repoName'>itemsmainused (compensating casts removed), and the phantomcursorfield and duplicateNewWorkspaceRepositorytype were dropped in favor ofMobileWorkspaceRepo. - Fixture/type cleanup — mock envelopes now carry
id/_meta, and the obsolete clipboard/haptics mocks were removed from the accounts reset-credit test.
Each of the three parity fixes carries a dedicated test that fails with the fix reverted.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Closing as superseded. The step-4 to step-7 RPC-foundation series landed every screen call as a fixed operation descriptor with a checked reply (session #21089, tasks #21169 and #21246, notifications/terminal/transport #21176, files/dictation/host-screen/agent-history #21269), with the recorded golden corpus and the CI pin guard as the oracle. The adapter routing this PR proposed is now built on those descriptors; the hybrid re-split of #13386 continues on top of the foundation rather than on this branch. |
Summary
Mobile screens on
maincall Desktop RPC inline (client.sendRequest('linear.listTeams'), envelope check and cast in the screen). This PR moves every such call in the task, session, file, workspace and account screens behind an operations adapter the screen is handed, so a screen callstaskOperations.linear.listTeams()and never sees the RPC. The native app hands each adapter a socketRpcClient. No hybrid or WebView code is included; a later PR hands the same adapters a different client.This is PR 1 of the split of #13386. The adapter bodies are carried from that branch and regrouped from 31 interfaces into six.
Shape
HostSessionOperationsHostTaskOperationsHostWorkspaceCreationOperationsHostAccountOperationsHostFileExplorerOperationsDeviceOperationsOne
nativeHost<Domain>Operations(client)factory and onedefaultHost<Domain>Operationsper domain. Sub-adapters keep their own files. Screen hooks take one prop per domain instead of one per concern. Only adapters with a live consumer are included; the file-preview and workspace-catalog families and three dead methods were removed after review so no unreferenced adapter can drift from the inline call it shadows.Behavior versus main
Matched exactly, including 82 user-facing fallback strings and 69 per-method timeouts swept individually. Three deliberate exceptions, each commented at the site:
worktree.listRetiredNames: a refused read holds the previous registry instead of blanking it, per the contract insrc/shared/worktree/retired-name-cache.ts.linear.selectWorkspace: a refused switch surfaces "Failed to select workspace" instead of leaving the picker showing a workspace the host never switched to.error.messagenames the operation instead of rendering the literal word "undefined".Three bugs in the source branch's adapters were fixed rather than carried: PR label/assignee edits routed to
github.updatePR(which drops them), worker-terminal takeover reporting deleted at three send sites, andresolvePrBase/resolveMrBaseforwarding params main never sent.Left inline on purpose
Four calls with no adapter method meaning the same thing, pinned by count in the ratchet:
terminal.closein session close actions,files.createFile+files.openin content create, the buffered terminal submit, andworktree.createfrom a task item. 112 inline calls remain elsewhere in mobile (diff review, AI vault, dictation, browser commands, PR sidebar); the ratchet caps that number so later PRs can only lower it.Ratchet
mobile/src/transport/screen-rpc-ratchet.test.tswalks every directory undermobile/srcand all ofmobile/app, matchessendRequestandclient.subscribe(by word boundary (so aliasing, bracket access and spacing cannot bypass it), pins every file with a remaining inline call by exact count, and keeps the four deliberate calls pinned.Verification
pnpm --dir mobile typecheckcleanpnpm --dir mobile test525 files, 4264 passedpnpm --dir mobile lint,oxfmt --checkcleanpnpm run check:code-quality:changedandpnpm run check:react-doctor:changed0 findingsSecond adversarial review follow-ups (0aaa70e)
read.tasksSupported()runs first and the hook commitssupportedbeforeread.bootstrap()fans out to settings/ui/preflight/linear. A transport failure in the fan-out again renders the screen behind an error banner instead of a permanent spinner. Hook test pins the ordering.github.listWorkItemstimeout, clear-terminal toast on refusal, SSH badge on a null state answer, four error strings, legacy file-inventory cache reset.host-workspace-host-context.ts, five uncalled tab methods, three uncalled account methods, the uncallednativeChat.openFilebacking, and thedevicemodule whose only consumer was one of those account methods.prRepo, smart-source fan-out stamping, four unpinned RPC names), each verified to fail under mutation.Third adversarial review follow-ups (5cbbadf)
hostcould previously satisfy it); verified by deletingupdateComment's host, which now fails the test.sendMessage/respond/attachImage(send paths call the source module directly, as on main) and workspace-creationcreateBlankWorkspace/createWorkspaceFromSource, plus a dead type re-export.Mechanical census (892c6fc)
Run by the lead on the final head, not by review judgment:
@ts-ignore, noas anyoutside tests, no config or lockfile changeFourth review follow-ups (59e5e6e)
Reverted to main: refused tab close prunes on
okalone with no result-body precondition; the Stop escape sends on any remaining budget (no 2 s floor); quick-command envelope check sits outside the cutover retry; the four Project-row guards use main's conditions; the read adapters dereference as main did instead of defaulting. Deleted: a Ctrl+U-prefix send option this PR had added with no caller, two create fields with no producer or consumer, the terminalsubscribeadapter method (the hook still subscribes directly, as on main), and the remaining interface members with no reader. Three files are now byte-identical to main. The session-route parity oracle pins 14 identity bindings again by following adapter property calls.Fourth disclosed change: in
use-mobile-native-chat-stop.tsthe active-route ref is written in auseLayoutEffectinstead of during render, becausecheck:react-doctor:changedrejects ref writes in render. Under a discarded render the ref holds the last committed route rather than the last rendered one.Sixth review follow-ups (different model)
A Codex (gpt-6-astra) whole-diff read of bcba08b found three parity regressions the five earlier reviews had passed. All fixed on this head; each has a test that fails with the fix reverted.
@file-search hook bumped a generation on workspace reset and ignored afiles.listcompletion from an older generation; the adapter had dropped that guard, so a slow inventory started before leaving a workspace could repopulate that workspace's cache after returning. Restored per workspace, with main's identity-guarded in-flight cleanup.result.ok; every other Project site didresult.ok === falseon a dereferenced result, so a null result threw before any optimistic commit. The adapter's shared reader had accepted null as success. Restored per method with a Project-specific reader;sendGithubPrMutationingithub-pr-mutations.tsis unchanged.Promise.all, so a comments transport failure no longer won over an issue refusal. Restored.Also from that review: removed the top-level fallback fields the detail projection accepted beyond
item; list results are typed as the rawOmit<…,'repoId'|'repoName'>items main used, with the compensating casts gone; three stale test fixtures corrected; the obsolete clipboard/haptics mocks and two history comments removed; the phantomcursorfield and a duplicate repository type removed in favor ofMobileWorkspaceRepo.Not changed, disclosed: the operation-target types carry unused placeholder fields; the five default factories and the terminal-response-lane wrapper stay; request-only leaf adapters are inconsistently typed as
RpcClientvsRpcRequestSender; the ratchet is a lexical count, not a semantic census. The extraction covers the session, task, workspace-creation, account and file-explorer domains; 112 inline calls remain elsewhere in mobile and are pinned by count.Round-6 history note: commits 1ec6db4 through 76c8fc3 were produced by a worker whose parallel sub-agents overwrote each other (one left a duplicate declaration); be4a2da through 3d2c11e repair them fix-forward. No history was rewritten.