Repository navigation
feat(mobile): structured native Codex chat - #18074
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThis change adds structured Codex agent sessions to mobile native chat. It adds new mobile session types, prompts, options, attachments, and send flows for agent-session tabs. It adds runtime capability negotiation after authentication. It gates structured session projection and restoration on the experimental native chat setting and negotiated capabilities. It also adds renderer tab sync fencing and retirement for structured sessions. CI now detects mobile dependency changes and installs mobile dependencies when required. Merge Risk: 🟡 Moderate · up to Structured mobile chat can fail to establish a relay connection promptly when capability negotiation is unanswered, and previously identified structured-chat behavior issues remain open. Resolve these issues before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides detailed scope, rationale, testing results, limitations, and review context. However, it omits required template sections and information, including a linked issue, explicit Testing checklist entries, AI Disclosure, Agent skill upstream boundary, Notes, and Checklist. The required visual proof is described but no attached before-and-after evidence is shown in the provided description. Resolution Add a valid issue reference under Linked Issue, complete or mark N/A for each required template section, include explicit Testing checklist results and platforms tested, provide before-and-after visual attachments or write N/A with a reason, and complete the Agent skill upstream boundary, Notes, and Checklist sections.
✨ 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: 4
🧹 Nitpick comments (2)
mobile/src/session/use-mobile-structured-agent-session.test.tsx (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMigrate this suite away from
react-test-renderer.This test calls
createandactfromreact-test-rendererwhile the mobile package uses React 19.2.8, where this API is deprecated and emits warnings. If React Native Testing Library is the supported harness, add and configure its v14+ setup before migrating this suite.mobile/src/session/use-mobile-native-chat-image-attachments.ts (1)
173-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
structuredNativeChatto thesendNativeChatdependency array.
sendNativeChatreadsstructuredNativeChatat Line 173 to choose between the structured send and the terminal paste path. The dependency array at Lines 313-326 omits it. TodayscopeKeychanges on the same tab switch that flipsstructuredNativeChat, so the callback is recreated and the stale value is not observable. That safety depends on two separately derived props staying coupled.List the value it reads so the routing decision cannot go stale if
scopeKeyderivation changes later.♻️ Proposed dependency addition
readSeededLaunchDraft, scopeKey, - sleep + sleep, + structuredNativeChat ]
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: d29aadc5-93ae-4bd1-9eeb-f23c1d463fe1
📒 Files selected for processing (89)
mobile/src/session/MobileNativeChatQuestion.tsxmobile/src/session/MobileSessionActiveContent.tsxmobile/src/session/MobileSessionHeader.tsxmobile/src/session/MobileSessionSheets.tsxmobile/src/session/mobile-file-tap-open.tsmobile/src/session/mobile-native-chat-controller-contract.tsmobile/src/session/mobile-native-chat-eligibility.test.tsmobile/src/session/mobile-native-chat-eligibility.tsmobile/src/session/mobile-native-chat-image-scope-state.tsmobile/src/session/mobile-native-chat-question.test.tsmobile/src/session/mobile-native-chat-question.tsmobile/src/session/mobile-session-route-parity.test.tsmobile/src/session/mobile-session-route-types.tsmobile/src/session/mobile-structured-agent-prompts.tsmobile/src/session/mobile-structured-agent-session-rpc.tsmobile/src/session/mobile-terminal-records.test.tsmobile/src/session/mobile-terminal-records.tsmobile/src/session/mobile-terminal-tab-agent.test.tsmobile/src/session/mobile-terminal-tab-agent.tsmobile/src/session/opened-mobile-session-tab.test.tsmobile/src/session/opened-mobile-session-tab.tsmobile/src/session/use-mobile-file-tap-handlers.test.tsmobile/src/session/use-mobile-file-tap-handlers.tsmobile/src/session/use-mobile-native-chat-active-resolution.tsmobile/src/session/use-mobile-native-chat-controller.test.tsmobile/src/session/use-mobile-native-chat-controller.tsmobile/src/session/use-mobile-native-chat-image-attachments.tsmobile/src/session/use-mobile-native-chat-image-upload.tsmobile/src/session/use-mobile-native-chat-session-option-controller.tsmobile/src/session/use-mobile-session-attachments.tsmobile/src/session/use-mobile-session-file-actions.tsmobile/src/session/use-mobile-session-image-attachments.test.tsxmobile/src/session/use-mobile-session-image-attachments.tsmobile/src/session/use-mobile-session-native-chat-dictation.tsmobile/src/session/use-mobile-session-screen-state.tsmobile/src/session/use-mobile-session-tab-action-targets.tsmobile/src/session/use-mobile-session-tab-switching.tsmobile/src/session/use-mobile-session-terminal-send-actions.tsmobile/src/session/use-mobile-structured-agent-options.tsmobile/src/session/use-mobile-structured-agent-session.test.tsxmobile/src/session/use-mobile-structured-agent-session.tsmobile/src/session/use-mobile-structured-agent-state.tsmobile/src/session/use-mobile-structured-native-chat-send-bridge.tsmobile/src/transport/direct-rpc-client.tsmobile/src/transport/mobile-relay-rpc-session.test.tsmobile/src/transport/mobile-relay-rpc-session.tsmobile/src/transport/mobile-runtime-client-capabilities.tsmobile/src/transport/rpc-client-capabilities.test.tsmobile/src/transport/rpc-client.test.tssrc/main/runtime/mobile-rpc-allowlist.test.tssrc/main/runtime/orca-runtime-close-structured-agent-session-tab.tssrc/main/runtime/orca-runtime-state-fields.tssrc/main/runtime/orca-runtime-structured-native-chat-settings.test.tssrc/main/runtime/orca-runtime-structured-session-restore.test.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher-stream-options.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/methods/client-ui.test.tssrc/main/runtime/rpc/methods/index.tssrc/main/runtime/rpc/methods/runtime-client-capabilities.test.tssrc/main/runtime/rpc/methods/runtime-client-capabilities.tssrc/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.tssrc/main/runtime/rpc/methods/session-tab-agent-status-projection.test.tssrc/main/runtime/rpc/methods/session-tab-agent-status-projection.tssrc/main/runtime/rpc/methods/session-tab-close-methods.tssrc/main/runtime/rpc/methods/session-tab-mutation-methods.tssrc/main/runtime/rpc/methods/session-tabs-inventory.tssrc/main/runtime/rpc/methods/session-tabs.test.tssrc/main/runtime/rpc/methods/session-tabs.tssrc/main/runtime/rpc/methods/structured-agent-session-gate.tssrc/main/runtime/rpc/methods/structured-agent-session-policy.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/rpc/methods/structured-session-tab-restore.tssrc/main/runtime/rpc/rpc-streaming-dispatcher.tssrc/main/runtime/runtime-client-settings.tssrc/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.tssrc/main/runtime/runtime-rpc/runtime-rpc-websocket-dispatch.tssrc/main/runtime/runtime-store-contract.tssrc/renderer/src/app-shell/use-app-startup-hydration.tssrc/renderer/src/app-startup-routing.test.tssrc/renderer/src/components/native-chat/structured-agent-session-message-projection.tssrc/renderer/src/components/native-chat/use-structured-agent-session-hold.tssrc/renderer/src/runtime/local-structured-session-tabs-sync.tssrc/renderer/src/store/slices/runtime-status-recheck.tssrc/shared/structured-agent-session-holder.tssrc/shared/structured-agent-session-message-projection.tssrc/shared/structured-agent-session-reducer.tstests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.tstests/e2e/cross-version-wire/versioned-agent-session-wire.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| attachments?: readonly { | ||
| id?: string | ||
| path: string | ||
| previewUri: string | ||
| }[] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 '\b(StructuredMobileAttachment|handleNativeChatSendWithOutcome)\b' mobile srcRepository: stablyai/orca
Length of output: 7191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- contract ---'
cat -n mobile/src/session/mobile-native-chat-controller-contract.ts | sed -n '1,110p'
printf '%s\n' '--- structured session imports and send contract ---'
cat -n mobile/src/session/use-mobile-structured-agent-session.ts | sed -n '1,175p'
printf '%s\n' '--- attachment type declarations and exports ---'
rg -n -C 4 '\bStructuredAgentSessionAttachment\b|\bStructuredMobileAttachment\b' mobile/src
printf '%s\n' '--- applicable repository conventions ---'
find /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4 -type f -name '*.md' -printRepository: stablyai/orca
Length of output: 19582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- shared attachment contract ---'
rg -n -C 8 '\bexport\s+(type|interface)\s+StructuredAgentSessionAttachment\b|\bStructuredAgentSessionAttachment\b' src/shared mobile/src/session
printf '%s\n' '--- mobile conventions ---'
cat /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions/mobile.md
printf '%s\n' '--- attachment consumers and controller assignment ---'
rg -n -C 5 'attachments|structuredNativeChat\.sendWithOutcome|handleNativeChatSendWithOutcome' mobile/src/session src/shared/structured-agent-session-outbox.tsRepository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- attachment aliases ---'
rg -n -C 3 'type (PendingNativeChatImage|StructuredNativeChatAttachment)|interface (PendingNativeChatImage|StructuredNativeChatAttachment)|StructuredNativeChatAttachment' mobile/src/session/use-mobile-native-chat-image-attachments.ts mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts
printf '%s\n' '--- controller implementation and structured bridge wiring ---'
cat -n mobile/src/session/use-mobile-native-chat-controller.ts | sed -n '215,245p;315,340p'
cat -n mobile/src/session/use-mobile-native-chat-image-attachments.ts | sed -n '1,75p;160,215p'Repository: stablyai/orca
Length of output: 14172
Define one shared mobile attachment type.
mobile-native-chat-controller-contract.ts, use-mobile-structured-agent-session.ts, and use-mobile-structured-native-chat-send-bridge.ts define equivalent { id?: string; path: string; previewUri: string } shapes. Move this mobile type to a shared module and use it in all three contracts. Keep StructuredAgentSessionAttachment as the outbox type because it intentionally omits id.
| handleNativeChatQuestionAnswer: activeChatStructured | ||
| ? structuredNativeChat.respondQuestion | ||
| : legacyHandleNativeChatQuestionAnswer, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear the held send error after an accepted structured question response.
structuredNativeChat.respondQuestion bypasses useNativeChatAcceptedAction. If a prior send failed, an accepted question answer does not call onSendResolved, so the stale failure banner remains visible. Wrap the structured question handler before returning it.
Proposed fix
+ const handleStructuredNativeChatQuestionAnswer = useNativeChatAcceptedAction(
+ structuredNativeChat.respondQuestion,
+ onSendResolved
+ )
+
handleNativeChatQuestionAnswer: activeChatStructured
- ? structuredNativeChat.respondQuestion
+ ? handleStructuredNativeChatQuestionAnswer
: legacyHandleNativeChatQuestionAnswer,📝 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.
| handleNativeChatQuestionAnswer: activeChatStructured | |
| ? structuredNativeChat.respondQuestion | |
| : legacyHandleNativeChatQuestionAnswer, | |
| const handleStructuredNativeChatQuestionAnswer = useNativeChatAcceptedAction( | |
| structuredNativeChat.respondQuestion, | |
| onSendResolved | |
| ) | |
| handleNativeChatQuestionAnswer: activeChatStructured | |
| ? handleStructuredNativeChatQuestionAnswer | |
| : legacyHandleNativeChatQuestionAnswer, |
| [mutate, optionState] | ||
| ) | ||
|
|
||
| const invokeStructuredOption = useCallback(async () => false, []) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Implement action-option dispatch.
invokeStructuredOption always returns false. optionSurface.invokeAction does not call it and only returns the current snapshot. An action option can never send its session mutation or report success. Forward the action ID to the structured-session mutation path and wire that handler into invokeAction.
Also applies to: 148-149
| ): Promise<MobileNativeChatSendOutcome> => { | ||
| const origin = captureSendOrigin(text.trimEnd()) | ||
| if (!origin) { | ||
| onSendError('Message not sent (disconnected)') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the new structured-send error messages.
Replace these new user-facing literals with intent-named localization IDs. This keeps structured-chat errors translatable.
Based on learnings: “For newly added user-facing strings in stablyai/orca TypeScript and TSX files, follow config/i18n-translation-source.md: use intent-named localization IDs with named interpolation placeholders.”
Also applies to: 73-73
Source: Learnings
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ Minor suggestions only — this is a large but well-scoped feature with strong test coverage, and I found no blocking issues.
Reviewed changes
- Structured session state + hold lifecycle — new
useMobileStructuredAgentState/useMobileStructuredAgentSessionhooks subscribe toagentSession.*over RPC, take a connection-scoped hold, cache up to 32 per-session transcripts keyed by source identity, and keep the transcript visible across reconnect/outage. - Prompt cards and options — approval/question journal items project to tappable cards whose responses carry opaque
structured-agent-prompt:tokens (itemId/revision/optionId); model/effort options flow throughagentSession.setOption. - Capability negotiation — mobile advertises
agent-session.structured.v1(plus hold capability) post-auth via the newruntime.clientCapabilities.updatemethod, gated to mobile scope and further gated on the host-ownedexperimentalStructuredNativeChatsetting. - Session-tab routing —
agent-sessiontabs are threaded through session-tab inventory, close, and mutation projection, all now keyed on both capability and the host setting.
The reconnect-retention behavior, prompt-token round-trip, and post-auth capability handshake are all verified by focused unit tests, and the cross-version wire tests cover the additive RPC method against an old host. The one thing I would tighten is the option-surface stubs in use-mobile-structured-agent-options.ts, noted inline.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
| [mutate, optionState] | ||
| ) | ||
|
|
||
| const invokeStructuredOption = useCallback(async () => false, []) |
There was a problem hiding this comment.
invokeStructuredOption is a hardcoded async () => false, and the optionSurface you build below stubs invokeAction/setOption (the latter returns the pre-commit optionSnapshot). Today this is unreachable because mobile structured snapshots are built with liveTransport: 'agent-session', so actionForApply never emits an action row. But the hook still advertises a working SessionOptionsSurface with invokeAction/setOption, so the moment Codex surfaces an agent-picker or toggle-command option the picker will render a row that silently no-ops. Either implement these two paths (or delete optionSurface from the return type if it is genuinely unused) so the advertised contract and the behavior agree.
There was a problem hiding this comment.
✅ No new issues found in the incremental commit.
Reviewed changes
- SSH termination result contract —
ssh:terminateSessionsnow returnsSshTerminateSessionsResult { terminated, unverifiable }instead ofvoid, with the verdict threaded throughSshApi,ssh-bridge, theweb-terminal-apistub, and the sharedssh-types. The handler counts successful/non-isSshPtyNotFoundErrorshutdowns asterminatedand unreachable (no-provider) expired leases asunverifiable, and the updated tests pin each branch (no-provider, empty, live-kill, tombstone, failure-preserves-lease).
The change is type-consistent across the preload and web surfaces and the verdict vocabulary matches docs/reference/ssh-execution-boundary.md. I have one scope note below, not a correctness concern.
ℹ️ Scope note — this commit is outside the PR's stated scope
The PR body states the scope is "Mobile structured native Codex chat lane only" with "no broad integration branch," but 6afbe28 ("align SSH termination bridge result contract") touches only the desktop SSH termination surface, which nothing in the structured-chat lane consumes. The new return value is not read by any caller yet (ssh-session-termination.ts and ssh-target-remove.ts both ignore it), so this reads as either an accidental inclusion or a precursor to a follow-up. Worth confirming intent before merging — split it out if it is unrelated, or note the follow-up consumer if one is planned.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
b47414b to
62aeea7
Compare
Native iOS Simulator QA —
|
62aeea7 to
4e6197a
Compare
…retention Fence local structured-session inventory and subscription responses with a sync generation so a toggle-off clear, reconnect restore, or retry cannot apply a mirror from a superseded instance. Bound mobile ambiguous operation-ID retention at 128 with unmount cleanup. Staged on the reconcile branch only: the sync module is now 312 lines and needs a real split before this can reach the PR head.
…sis mobile types The local structured session-tabs sync module outgrew the 300-line cap once it took on generation fencing, so split it along its real seams instead of raising the cap: the generation/cursor fence, snapshot projection, snapshot apply, inventory refresh, and the subscription loop. The original path stays as a barrel so no importer moves. Repoint the host-session-mirror settle census at the apply module, which owns two receipts now — the snapshot it mirrors in, and the toggle-off teardown that retracts what it published. The teardown receipt is named rather than anonymous so the pin says which direction it settles. The changed-code quality gate lints mobile files and resolves their types from mobile/node_modules, but mobile is a separate pnpm project that the root install never populates, so every mobile type degraded to an `error` type and the gate reported phantom findings. Install mobile dependencies in static analysis when the diff touches mobile, gated on a new classifier output.
The mobile capability update is an advisory whose result is discarded, yet an unanswered one was fatal while an explicit rejection was tolerated. A 5s timeout on the direct client force-closed the socket, and on the relay path it failed `confirmResume` before `connected` was ever published, so a consistently slow link redialled forever. Both paths now share one helper that settles every ambiguous outcome (timeout, mid-flight drop) like a rejection and rejects only when the frame never reached the wire — the one case nothing else recovers from, since the socket's own desync force-close is gated on already being connected. The generation guard still keeps a replaced session from connecting. Retained structured-session operation ids were capped at 128 with oldest-first eviction, but every retained id belongs to a send whose outcome is unknown, so eviction turned a user's retry into a second message on the host. Bound the map by expiry against the id's own embedded timestamp instead, mirroring the host's operation ledger, so no id is released while the host would still honour it. Also give the mobile CI install the root install's lockfile drift guard (mobile's lockfile carries patchedDependencies a silent rewrite would drop), gate mobile_dependencies on should_run, and key the pnpm store cache on both lockfiles.
|
Too many files changed for review (122 files, 100 file limit). Bypass the limit by tagging |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 32102bd9-2dce-48c3-83b6-3fc05037d377
📒 Files selected for processing (121)
.github/actions/install-node-dependencies/action.yml.github/workflows/pr.ymlconfig/scripts/pr-code-change-scope.mjsconfig/scripts/pr-code-change-scope.test.mjsmobile/src/session/MobileNativeChatQuestion.tsxmobile/src/session/MobileSessionActiveContent.tsxmobile/src/session/MobileSessionHeader.tsxmobile/src/session/MobileSessionSheets.tsxmobile/src/session/mobile-file-tap-open.tsmobile/src/session/mobile-native-chat-controller-contract.tsmobile/src/session/mobile-native-chat-eligibility.test.tsmobile/src/session/mobile-native-chat-eligibility.tsmobile/src/session/mobile-native-chat-image-scope-state.tsmobile/src/session/mobile-native-chat-question.test.tsmobile/src/session/mobile-native-chat-question.tsmobile/src/session/mobile-session-route-parity.test.tsmobile/src/session/mobile-session-route-types.tsmobile/src/session/mobile-structured-agent-prompts.tsmobile/src/session/mobile-structured-agent-session-launch.test.tsmobile/src/session/mobile-structured-agent-session-launch.tsmobile/src/session/mobile-structured-agent-session-rpc.tsmobile/src/session/mobile-structured-session-operation-retention.test.tsmobile/src/session/mobile-terminal-records.test.tsmobile/src/session/mobile-terminal-records.tsmobile/src/session/mobile-terminal-tab-agent.test.tsmobile/src/session/mobile-terminal-tab-agent.tsmobile/src/session/opened-mobile-session-tab.test.tsmobile/src/session/opened-mobile-session-tab.tsmobile/src/session/use-mobile-file-tap-handlers.test.tsmobile/src/session/use-mobile-file-tap-handlers.tsmobile/src/session/use-mobile-native-chat-active-resolution.tsmobile/src/session/use-mobile-native-chat-controller.test.tsmobile/src/session/use-mobile-native-chat-controller.tsmobile/src/session/use-mobile-native-chat-image-attachments.tsmobile/src/session/use-mobile-native-chat-image-upload.tsmobile/src/session/use-mobile-native-chat-session-option-controller.tsmobile/src/session/use-mobile-session-attachments.tsmobile/src/session/use-mobile-session-file-actions.tsmobile/src/session/use-mobile-session-image-attachments.test.tsxmobile/src/session/use-mobile-session-image-attachments.tsmobile/src/session/use-mobile-session-native-chat-dictation.tsmobile/src/session/use-mobile-session-screen-state.tsmobile/src/session/use-mobile-session-tab-action-targets.tsmobile/src/session/use-mobile-session-tab-switching.tsmobile/src/session/use-mobile-session-terminal-create-actions.test.tsmobile/src/session/use-mobile-session-terminal-create-actions.tsmobile/src/session/use-mobile-session-terminal-send-actions.tsmobile/src/session/use-mobile-structured-agent-options.tsmobile/src/session/use-mobile-structured-agent-session.test.tsxmobile/src/session/use-mobile-structured-agent-session.tsmobile/src/session/use-mobile-structured-agent-state.tsmobile/src/session/use-mobile-structured-native-chat-send-bridge.tsmobile/src/transport/cellular-connecting-label-stall.test.tsmobile/src/transport/direct-connection-log.tsmobile/src/transport/direct-rpc-client.tsmobile/src/transport/foreground-stale-dial-restart.test.tsmobile/src/transport/mobile-relay-rpc-session-liveness.test.tsmobile/src/transport/mobile-relay-rpc-session.test.tsmobile/src/transport/mobile-relay-rpc-session.tsmobile/src/transport/mobile-runtime-capability-negotiation.test.tsmobile/src/transport/mobile-runtime-capability-negotiation.tsmobile/src/transport/mobile-runtime-client-capabilities.tsmobile/src/transport/rpc-client-capabilities.test.tsmobile/src/transport/rpc-client-connect-wait-replay.test.tsmobile/src/transport/rpc-client-delivery-ambiguity.test.tsmobile/src/transport/rpc-client-request-deadline.test.tsmobile/src/transport/rpc-client-request-tracker.tsmobile/src/transport/rpc-client-runtime-events.test.tsmobile/src/transport/rpc-client-synthesized-close-diagnostics.test.tsmobile/src/transport/rpc-client-terminal-reconnect.test.tsmobile/src/transport/rpc-client-unauthorized-close.test.tsmobile/src/transport/rpc-client.test.tsmobile/src/transport/rpc-session-liveness-integration.test.tssrc/main/runtime/mobile-rpc-allowlist.test.tssrc/main/runtime/orca-runtime-close-structured-agent-session-tab.tssrc/main/runtime/orca-runtime-state-fields.tssrc/main/runtime/orca-runtime-structured-native-chat-settings.test.tssrc/main/runtime/orca-runtime-structured-session-restore.test.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher-stream-options.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/methods/client-ui.test.tssrc/main/runtime/rpc/methods/index.tssrc/main/runtime/rpc/methods/runtime-client-capabilities.test.tssrc/main/runtime/rpc/methods/runtime-client-capabilities.tssrc/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.tssrc/main/runtime/rpc/methods/session-tab-agent-status-projection.test.tssrc/main/runtime/rpc/methods/session-tab-agent-status-projection.tssrc/main/runtime/rpc/methods/session-tab-close-methods.tssrc/main/runtime/rpc/methods/session-tab-mutation-methods.tssrc/main/runtime/rpc/methods/session-tabs-inventory.tssrc/main/runtime/rpc/methods/session-tabs.test.tssrc/main/runtime/rpc/methods/session-tabs.tssrc/main/runtime/rpc/methods/structured-agent-session-gate.tssrc/main/runtime/rpc/methods/structured-agent-session-policy.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/rpc/methods/structured-agent-session.tssrc/main/runtime/rpc/methods/structured-session-tab-restore.tssrc/main/runtime/rpc/rpc-streaming-dispatcher.tssrc/main/runtime/runtime-client-settings.tssrc/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.tssrc/main/runtime/runtime-rpc/runtime-rpc-websocket-dispatch.tssrc/main/runtime/runtime-store-contract.tssrc/renderer/src/app-shell/use-app-startup-hydration.tssrc/renderer/src/app-startup-routing.test.tssrc/renderer/src/components/native-chat/structured-agent-session-message-projection.tssrc/renderer/src/components/native-chat/use-structured-agent-session-hold.tssrc/renderer/src/runtime/host-session-mirror-settle-census.test.tssrc/renderer/src/runtime/local-structured-session-tab-retirement.tssrc/renderer/src/runtime/local-structured-session-tabs-sync.test.tssrc/renderer/src/runtime/local-structured-session-tabs-sync.tssrc/renderer/src/runtime/local-structured-session-tabs-sync/inventory-generation-fence.tssrc/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.tssrc/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.tssrc/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-projection.tssrc/renderer/src/runtime/local-structured-session-tabs-sync/subscription.tssrc/shared/structured-agent-session-holder.tssrc/shared/structured-agent-session-message-projection.tssrc/shared/structured-agent-session-reducer.tstests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.tstests/e2e/cross-version-wire/versioned-agent-session-wire.ts
🚧 Files skipped from review as they are similar to previous changes (79)
- src/main/runtime/orca-runtime-structured-native-chat-settings.test.ts
- src/main/runtime/runtime-client-settings.ts
- mobile/src/session/mobile-session-route-parity.test.ts
- src/main/runtime/rpc/methods/client-ui.test.ts
- tests/e2e/cross-version-wire/versioned-agent-session-wire.ts
- src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts
- src/main/runtime/rpc/rpc-streaming-dispatcher.ts
- src/main/runtime/rpc/methods/index.ts
- mobile/src/session/mobile-session-route-types.ts
- src/main/runtime/runtime-rpc/runtime-rpc-websocket-dispatch.ts
- mobile/src/session/MobileSessionActiveContent.tsx
- mobile/src/session/use-mobile-session-screen-state.ts
- mobile/src/session/mobile-native-chat-eligibility.test.ts
- mobile/src/session/mobile-terminal-records.ts
- mobile/src/session/use-mobile-session-attachments.ts
- src/main/runtime/rpc/methods/session-tabs.test.ts
- src/main/runtime/rpc/methods/session-tab-mutation-methods.ts
- src/renderer/src/app-shell/use-app-startup-hydration.ts
- mobile/src/session/mobile-native-chat-question.test.ts
- mobile/src/session/use-mobile-native-chat-image-upload.ts
- mobile/src/session/mobile-terminal-tab-agent.test.ts
- src/main/runtime/rpc/core.ts
- mobile/src/session/use-mobile-file-tap-handlers.test.ts
- mobile/src/session/mobile-terminal-tab-agent.ts
- mobile/src/session/use-mobile-session-image-attachments.ts
- mobile/src/session/MobileSessionSheets.tsx
- src/shared/structured-agent-session-message-projection.ts
- mobile/src/session/mobile-terminal-records.test.ts
- src/main/runtime/rpc/methods/session-tab-agent-status-projection.ts
- mobile/src/session/use-mobile-file-tap-handlers.ts
- src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
- src/main/runtime/rpc/methods/runtime-client-capabilities.test.ts
- src/main/runtime/rpc/methods/structured-agent-session-gate.ts
- mobile/src/session/mobile-native-chat-image-scope-state.ts
- mobile/src/session/use-mobile-session-tab-action-targets.ts
- src/main/runtime/rpc/methods/structured-session-tab-restore.ts
- mobile/src/session/opened-mobile-session-tab.ts
- mobile/src/session/mobile-native-chat-eligibility.ts
- src/shared/structured-agent-session-holder.ts
- src/main/runtime/rpc/methods/structured-agent-session-policy.ts
- src/main/runtime/rpc/methods/session-tab-close-methods.ts
- mobile/src/session/MobileNativeChatQuestion.tsx
- mobile/src/session/use-mobile-session-image-attachments.test.tsx
- mobile/src/session/use-mobile-session-native-chat-dictation.ts
- mobile/src/session/mobile-structured-agent-prompts.ts
- mobile/src/session/mobile-native-chat-controller-contract.ts
- mobile/src/transport/mobile-runtime-client-capabilities.ts
- src/renderer/src/app-startup-routing.test.ts
- mobile/src/session/use-mobile-structured-agent-session.ts
- src/renderer/src/components/native-chat/use-structured-agent-session-hold.ts
- src/main/runtime/rpc/methods/runtime-client-capabilities.ts
- mobile/src/session/use-mobile-native-chat-controller.test.ts
- src/main/runtime/orca-runtime-structured-session-restore.test.ts
- src/main/runtime/rpc/methods/session-tab-agent-capability-mutations.test.ts
- src/main/runtime/orca-runtime-state-fields.ts
- mobile/src/session/use-mobile-session-file-actions.ts
- mobile/src/session/use-mobile-native-chat-active-resolution.ts
- mobile/src/session/use-mobile-native-chat-controller.ts
- src/main/runtime/rpc/methods/session-tab-agent-status-projection.test.ts
- src/shared/structured-agent-session-reducer.ts
- src/main/runtime/rpc/methods/session-tabs-inventory.ts
- src/main/runtime/rpc/dispatcher.ts
- mobile/src/session/use-mobile-session-terminal-send-actions.ts
- src/main/runtime/rpc/dispatcher-stream-options.ts
- src/main/runtime/runtime-store-contract.ts
- src/main/runtime/rpc/methods/session-tabs.ts
- tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts
- mobile/src/transport/mobile-relay-rpc-session.ts
- mobile/src/session/opened-mobile-session-tab.test.ts
- mobile/src/session/use-mobile-structured-agent-session.test.tsx
- mobile/src/session/use-mobile-structured-agent-state.ts
- mobile/src/session/mobile-native-chat-question.ts
- src/main/runtime/mobile-rpc-allowlist.test.ts
- mobile/src/session/use-mobile-session-tab-switching.ts
- mobile/src/session/MobileSessionHeader.tsx
- mobile/src/session/use-mobile-structured-agent-options.ts
- mobile/src/session/use-mobile-native-chat-session-option-controller.ts
- mobile/src/session/mobile-file-tap-open.ts
- mobile/src/session/use-mobile-native-chat-image-attachments.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| ) | ||
| const enabled = useAppStore((state) => state.settings?.experimentalStructuredNativeChat === true) | ||
| useEffect(() => { | ||
| if (!ready) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear disabled tabs before the readiness return.
If experimentalStructuredNativeChat becomes false while startup restoration is pending, the effect returns with ready === false. The pending restore can then apply structured-session tabs before readiness completes, so disabled tabs remain visible until ready changes. Move the disable branch before the readiness branch.
There was a problem hiding this comment.
ℹ️ Minor suggestions only — this pass came back clean at the current head; the P1/P2 fixes from earlier rounds are baked in, and the one remaining rough edge is a latent
optionSurfacecontract, noted inline.
Reviewed changes
- Post-auth capability handshake — mobile advertises
agent-session.structured.v1+ hold via the additiveruntime.clientCapabilities.update; direct and relay transports now share a settle helper that proceeds on any ambiguous outcome and fails only when the advisory never reached the wire. - Structured session state + hold lifecycle — new
useMobileStructuredAgentState/useMobileStructuredAgentSessionhooks subscribe toagentSession.*, take connection-scoped holds, cache up to 32 transcripts keyed by host/workspace/agent/session, and keep the transcript visible across reconnect (loadingnow spreads prior state). - Prompt cards + options — approval/question journal items project to tappable cards whose responses carry opaque
structured-agent-prompt:tokens (itemId/revision/optionId); model/effort flow throughagentSession.setOption. - Host policy + projection — mobile structured chat is gated on both the client capability and the host-owned
experimentalStructuredNativeChatsetting, threaded through session-tab inventory, close, mutation, and restore. - Operation-id retention — retained dedupe IDs are now bounded by the host expiry window instead of a 128-entry count, so an unconfirmed send's ID can't be evicted into a duplicate.
- Launch integrity —
createMobileStructuredCodexSessionprobescreateSupport, replays the durable create envelope once, and returnsunknown(never a legacy sibling) on unclassifiable outcomes. - Shared CI fix —
static analysisinstalls mobile deps only when mobile files change, so its type-aware pass stops emitting phantomerror-type findings.
No blocking issues. I traced the handleAuthenticated capability-settle/force-close paths, the structuredAgentSessionHolderId extraction, the structured-agent-session-policy restore gate against the previous capability-only gate, and the self-declared-capability trust boundary (client caps were already shape-validated, not allowlisted, at auth — the real gate is the host-owned setting, which is correctly not client-mutable).
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
| const setOption = useCallback( | ||
| async (id: string, value: SessionOptionValue) => { | ||
| await setStructuredOption(id, value) | ||
| return { snapshot: optionSnapshot } |
There was a problem hiding this comment.
optionSurface.setOption resolves to the pre-mutation optionSnapshot captured in this closure, and invokeAction/invokeStructuredOption are hardwired no-ops — so any future consumer of the returned SessionOptionsSurface reads a stale snapshot or silently does nothing. Nothing in the production path calls optionSurface.setOption/invokeAction today (the pickers drive setStructuredOption directly), which is why this is latent rather than observable. Minimally, return the post-commit snapshot from setStructuredOption (or drop optionSurface until an action-bearing option kind exists) instead of resolving to the stale optionSnapshot.
Resolves one conflict in mobile/src/transport/mobile-relay-rpc-session.test.ts, where both sides added tests at the same point: this branch's capability-handshake regressions (an older runtime rejecting negotiation, and a relay that never answers) and main's relay dial-stage tracking from the stage-aware dial bound. Both cover slow-link behavior in the same transport and both are kept.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
ℹ️ Merge is clean — one timing seam to reconcile at the relay/migration boundary, noted inline.
Reviewed changes
- Merged
main(d66386b) — brought the stage-aware relay dial bound (#18518) intoconnectMobileRelayRpcSession: the relay session now tracksRelayDialStage(opening → awaiting-hello → handshaking → confirming) and exposesRelayDialStageSource, sowaitForAuthenticatedre-arms a per-stage budget instead of a flat bound. The PR's capability settle and delivery-ambiguity logic, and the credential-version rejection test, all survive the merge intact.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
| resumeExpiresAt = result.resumeConfirmation.resumeExpiresAt | ||
| lastConnectedAt = Date.now() | ||
| // Why: an unanswered advisory must not keep a slow relay from ever reaching connected. | ||
| await settleMobileRuntimeCapabilities((method, params) => |
There was a problem hiding this comment.
settleMobileRuntimeCapabilities now runs inside the relay confirming dial stage, which waitForAuthenticated bounds at 35s (relay-dial-stage.ts sized it as 30s resume-confirmation + slack). The settle serializes a second, independently 30s-bound sendRpc onto that same stage, so a resume that confirms slowly under lock contention plus an unanswered advisory can keep confirming well past 35s and get its migration cut — the same silent hang-up c0fd2db removed from the primary path, still live in the replacement path.
Technical details
# Confirming dial-stage budget doesn't account for the capability settle
## Affected sites
- mobile/src/transport/mobile-relay-rpc-session.ts:176 — `dialStage.advance('confirming')` opens the stage enclosing both the resume-confirmation RPC and the settle
- mobile/src/transport/mobile-relay-rpc-session.ts:195 — `await settleMobileRuntimeCapabilities(...)` adds up to `requestTimeoutMs` (30s) of serialized latency to `confirming`
- mobile/src/transport/relay-dial-stage.ts:59 — `RELAY_DIAL_STAGE_BUDGET_MS.confirming = 35_000` (30s resume-confirmation + slack)
- mobile/src/transport/replacement-session-authentication.ts:66 — re-arms `relayDialStageBudgetMs(stage)` on each stage and rejects at the `confirming` budget; `stable-logical-rpc-client.ts:232` (`migrateTo`) then closes `nextSession` and retries
## Required outcome
- A relay whose resume-confirmation succeeds slowly but whose capability advisory is unanswered must not have its migration cut while it is still progressing toward `connected`. The `confirming` budget and the settle timing must agree.
## Suggested approach (optional)
- Enlarge the `confirming` budget to cover resume-confirmation + settle, or advance a distinct stage after the resume-confirmation so the migration bound re-arms across the settle instead of lumping both under one 35s window.
## Open questions for the human (optional)
- Is holding `confirming` open for the advisory deliberate? `settleMobileRuntimeCapabilities` already proceeds on any ambiguous outcome, so blocking `connected` on it adds up to 30s of latency for no routing gain in the common case.The merge composed two independently-sized changes — this branch's capability handshake settle and main's dial-stage tracking — pushing the relay session file to 304 lines against a 300 cap. Neither side broke it alone. Move the in-flight request registry (id generation, tracking, settlement, and reject-all with its delivery-ambiguity marking) into RelayPendingRequests, matching the existing collaborator pattern alongside RelayDialStageTracker and RpcSessionLivenessWatchdog. No behavior change.
Reconciles main's structured native Codex chat (#18074) and the stage-aware relay dial bound (#18518) with the hybrid operations layer: - native-chat controller keeps the operations/target/disconnect-retention seam and routes agent-session tabs through the structured hooks; the active-resolution and terminal-write hooks are extracted to stay under the line cap - image attachments keep the hosted attachImage/pasteImages path and add main's structured (paste-free) send - bare Codex launches take the structured path only on the native client; hosted adapter creates stay on the adapter - file taps resolve against the source session tab when a structured chat has no backing terminal - relay session advertises client capabilities after resume confirm - package downloader contract split out to break the import cycle the native code-quality audit now rejects Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile): finalize structured native Codex chat * fix(mobile): close structured chat lifecycle gaps * wip(mobile): fence stale structured inventory and bound operation-id retention Fence local structured-session inventory and subscription responses with a sync generation so a toggle-off clear, reconnect restore, or retry cannot apply a mirror from a superseded instance. Bound mobile ambiguous operation-ID retention at 128 with unmount cleanup. Staged on the reconcile branch only: the sync module is now 312 lines and needs a real split before this can reach the PR head. * fix(ci): split the structured session-tabs sync and give static analysis mobile types The local structured session-tabs sync module outgrew the 300-line cap once it took on generation fencing, so split it along its real seams instead of raising the cap: the generation/cursor fence, snapshot projection, snapshot apply, inventory refresh, and the subscription loop. The original path stays as a barrel so no importer moves. Repoint the host-session-mirror settle census at the apply module, which owns two receipts now — the snapshot it mirrors in, and the toggle-off teardown that retracts what it published. The teardown receipt is named rather than anonymous so the pin says which direction it settles. The changed-code quality gate lints mobile files and resolves their types from mobile/node_modules, but mobile is a separate pnpm project that the root install never populates, so every mobile type degraded to an `error` type and the gate reported phantom findings. Install mobile dependencies in static analysis when the diff touches mobile, gated on a new classifier output. * fix(mobile): let a slow capability handshake still reach connected The mobile capability update is an advisory whose result is discarded, yet an unanswered one was fatal while an explicit rejection was tolerated. A 5s timeout on the direct client force-closed the socket, and on the relay path it failed `confirmResume` before `connected` was ever published, so a consistently slow link redialled forever. Both paths now share one helper that settles every ambiguous outcome (timeout, mid-flight drop) like a rejection and rejects only when the frame never reached the wire — the one case nothing else recovers from, since the socket's own desync force-close is gated on already being connected. The generation guard still keeps a replaced session from connecting. Retained structured-session operation ids were capped at 128 with oldest-first eviction, but every retained id belongs to a send whose outcome is unknown, so eviction turned a user's retry into a second message on the host. Bound the map by expiry against the id's own embedded timestamp instead, mirroring the host's operation ledger, so no id is released while the host would still honour it. Also give the mobile CI install the root install's lockfile drift guard (mobile's lockfile carries patchedDependencies a silent rewrite would drop), gate mobile_dependencies on should_run, and key the pnpm store cache on both lockfiles. * refactor(mobile): extract the relay pending-request registry The merge composed two independently-sized changes — this branch's capability handshake settle and main's dial-stage tracking — pushing the relay session file to 304 lines against a 300 cap. Neither side broke it alone. Move the in-flight request registry (id generation, tracking, settlement, and reject-all with its delivery-ambiguity marking) into RelayPendingRequests, matching the existing collaborator pattern alongside RelayDialStageTracker and RpcSessionLivenessWatchdog. No behavior change. --------- Co-authored-by: Merge Sim <sim@local>
ELI5
Before this, using Codex on your phone meant looking at a terminal. The phone streamed the raw text of a command-line program and you typed into it. It worked, but it was a terminal pretending to be a chat — text reflowed oddly, there were no real messages, and anything the terminal lost was just gone.
This PR gives the phone a real chat. The phone now asks the desktop for an actual Codex session, and the desktop sends back structured messages instead of a stream of characters. You get proper message bubbles, streaming replies, an interrupt button, and a conversation that survives closing and reopening the app.
The hard part wasn't drawing the chat UI — it was making it honest about what it knows. A phone on a train loses signal mid-sentence. Most of the work here is about that: not showing a stale conversation from a session you already closed, not sending your message twice when the network hiccups, and not giving up on a connection just because the desktop was slow to answer. Those three problems are the bulk of the code and nearly all of the review.
Before / after
Scope
Mobile structured native Codex chat.
Also included, and deliberately called out because it is broader than the mobile lane: a fix to the shared PR CI config (
.github/workflows/pr.yml,.github/actions/install-node-dependencies/action.yml,config/scripts/pr-code-change-scope.mjs). Thestatic analysisjob lints changed mobile files and resolves their types frommobile/node_modules, but CI installed only the root workspace. Mobile is a separate pnpm project, so every mobile type degraded to anerrortype and the gate emitted phantom findings. The fix installs mobile dependencies in that job only when mobile files changed, gated on a new tested classifier output. This affects every PR, not just this one.Exact head
brennanb2025/codex-native-chat-mobilec0fd2dbd1efb7c6aa37a65d9fe6713d760e43d29What changed beyond the feature
src/renderer/src/runtime/local-structured-session-tabs-sync.tswas split into five cohesive modules (inventory-generation-fence,inventory-refresh,snapshot-apply,snapshot-projection,subscription) with the original path kept as a barrel. Nomax-linessuppression and no per-file cap bump.Review findings and fixes
Two independent review lanes at the exact head. No P0 or P1 outstanding.
connected. Direct and relay paths now share one settle helper that proceeds on any ambiguous outcome and fails only when the frame never reached the wire.Validation at this head
Git compatibilityfailure was a confirmed flake — it passed on rerun with no code change, and that job only runs here because touching the PR workflow forces the full job matrix.unsubscribeAllstorm a newly launched session still reached the mirror. Console clean.connected.Mixed-version behavior
No required client/host update order. Mobile probes
agentSession.createSupportbefore mutating; a host that does not implement it degrades to normal terminal chat. Anunknownresult deliberately does not create a legacy sibling session. Old clients cannot advertise the capability, and old hosts replyforbidden, which both transports tolerate.Known limitations
forbiddenevery time, exercising the explicit-rejection branch instead.mobile/(proven with a positive control); mobile's real type coverage istsc.clientKind === undefined) that no caller reaches today, and CI store-cache/gating nits.