Repository navigation
fix(mobile): recover terminal input lease on attach and preserve picked file names - #32987
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 19 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes both primary changes: terminal input lease recovery and preservation of picked file names. Linked Issues check Issue #32985requires terminal subscription and input-lease recovery after Terminal-view stream teardown, reconnect, and an unready lease on a connected terminal tab. The changes add stream-revision r…Out of Scope Changes check The changes stay within issue #32985. Subscription recovery, lease-gate changes, filename propagation and sanitization, RPC contract updates, and related tests directly support terminal attachment rec…Description check The pull request description is complete and aligned with the template. It includes the ELI5 summary, detailed changes, rationale, linked issue, visual proof, testing details, AI disclosure, upstream-… Full details: Linked Issues check
-
Explanation
Issue
#32985requires terminal subscription and input-lease recovery after Terminal-view stream teardown, reconnect, and an unready lease on a connected terminal tab. The changes add stream-revision recovery, bounded resubscription, reconnect subscription, and forced lease recovery. Issue#32985also requires any-file selection and sanitized filename preservation. The changes use*/*, forwardfileName, and preserve extensions in host temporary files. The listed tests cover these recovery and filename paths.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#32985. Subscription recovery, lease-gate changes, filename propagation and sanitization, RPC contract updates, and related tests directly support terminal attachment recovery or filename preservation. No unrelated product change is demonstrated.
Full details: Description check
-
Explanation
The pull request description is complete and aligned with the template. It includes the ELI5 summary, detailed changes, rationale, linked issue, visual proof, testing details, AI disclosure, upstream-boundary checkbox, and checklist. The optional Review and Notes headings are not included, but the relevant considerations are covered elsewhere and this does not prevent review.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
-
⚙️ Run configuration
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a32681c5-2b33-48bc-ac34-b73576310955
📒 Files selected for processing: 19
-
mobile/src/platform/media-picker-contract.tsmobile/src/platform/media-picker.web.tsmobile/src/session/mobile-clipboard-image.tsmobile/src/session/mobile-image-attachment.test.tsmobile/src/session/mobile-image-attachment.tsmobile/src/session/mobile-image-source-picker.test.tsmobile/src/session/mobile-image-source-picker.tsmobile/src/session/use-mobile-attachment-input-lease-gate.test.tsmobile/src/session/use-mobile-attachment-input-lease-gate.tsmobile/src/session/use-mobile-native-chat-input-lease.tsmobile/src/session/use-mobile-native-chat-terminal-stream.test.tsmobile/src/session/use-mobile-native-chat-terminal-stream.tsmobile/src/session/use-mobile-session-attachments.tsmobile/src/session/use-mobile-session-startup.tsmobile/src/session/use-mobile-session-terminal-subscription-foundation.tssrc/main/runtime/rpc/methods/clipboard.tssrc/main/window/clipboard-image-temp-file.test.tssrc/main/window/clipboard-image-temp-file.tssrc/shared/rpc-contract/clipboard-params.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
-
…ve extensionless names
…le-terminal-file-attach-lease-and-name # Conflicts: # mobile/src/platform/media-picker.web.ts # mobile/src/session/native-chat/use-mobile-native-chat-terminal-stream.ts
…m path The teardown signal alone restores Terminal view after a reconnect, so drop the attach-gate recovery callback, the startup resubscribe and the synchronous lease ref write. Uncovered missing streams now go through the same rearm budget as covered chat instead of a second copy of it.
Reuse the chat attachment name sanitizer and give the paste its own temp folder, so the name fits the filesystem limit without new byte math. The name rides only on clipboard.startImageUpload; hosts without it never get here, and older hosts drop the unknown key and keep the .png name.
…d-terminal rearms Transcript readers (Grok history, the Pasted image label) recognize an uploaded image by its orca-paste-*.png name, so only non-image picks carry their own name. Rendered terminals now spend the rearm budget too, so they also get its refill when terminal.list drops and reports the handle again.
…ed at once A synchronous refusal ends the stream before subscribe returns, so the ref check read it as gated and the rearm retried on every render. subscribeToTerminal now reports whether it sent, and the rearm charges on that.
Read the reconciled handle from a ref so list callbacks keep their identity across tab switches, name pasted images with the shared helper, and drop conditional spreads and a dead resume guard.
…cters Agents that take raw image paths get the saved path unescaped, so a picked $(id).txt would expand if the paste reached a shell. Only letters, marks, digits, space, dot, underscore and hyphen survive; also closes PowerShell $() expansion inside the Windows double-quote form.
…le-terminal-file-attach-lease-and-name
…e stubs Image uploads send no fileName key at all, so recorded and older-host startImageUpload requests stay byte-identical; test stubs of subscribeToTerminal now return the sent flag.
ELI5
Long-pressing the mobile terminal attach button to upload any file no longer fails with
Attach failed (reconnecting)after the app returns from the system file picker or a brief background freeze, and uploaded files now keep their original filename and extension (e.g..txt,.pdf) instead of always being saved as.png.What Changed
nativeChatInputLeaseReadyRefwas cleared on disconnect (false), and when the previousterminal.subscribestream ended (unsubscribeTerminal),useMobileSessionTerminalSubscriptionFoundationonly bumpedcoveredStreamRevisionwhenshowNativeChatRef.currentwastrue. In Terminal view (showNativeChat === false), neithercoveredStreamRevisionnorleaseReadychanged, andensureSessionTabs()skipped unchanged tab snapshots on reconnect—leaving the terminal permanently unsubscribed and causinguseMobileAttachmentInputLeaseGateto time out withAttach failed (reconnecting).saveClipboardImageBufferAsTempFilehardcoded the host temp filename toorca-paste-${Date.now()}-${randomUUID()}.png, renaming any non-image file picked via long-press (pickImage('files')) to.png.useMobileSessionTerminalSubscriptionFoundationbumpscoveredStreamRevisionwhenever the active handle's stream is torn down without a rendered lease drop, regardless ofshowNativeChatRef.current, anduseMobileNativeChatTerminalStreamapplies the same bounded 3-attempt + 5s healthy-stream proof to uncovered terminal resubscribes.useMobileSessionStartupresubscribes an unsubscribed active terminal on reconnect afterensureSessionTabs(),useMobileAttachmentInputLeaseGateactively triggers lease recovery when waiting on a connected terminal tab, anduseMobileNativeChatInputLeaseupdatesreadyRef.currentsynchronously insidereplace(next).media-picker.web.ts,mobile-image-source-picker.ts(widened totype: '*/*'),mobile-image-attachment.ts,mobile-clipboard-image.ts,clipboard-params.ts,clipboard.ts, andclipboard-image-temp-file.tsthread and sanitizefileNameso host temp files preserve their original extension.Why
Users attach non-image files (logs, text files, docs) to terminal TUI agents on mobile via long-press on the attach button, and opening the OS file picker routinely backgrounds the app long enough to cycle the WebSocket or stream subscription. Recovering the terminal subscription/lease automatically and preserving the file's original extension makes mobile terminal file attachment reliable.
Linked Issue
Fixes #32985
Visual Proof
N/A - Verified on Android hardware (
com.stably.orca.mobile) and covered by unit tests acrossuse-mobile-attachment-input-lease-gate.test.ts,use-mobile-native-chat-terminal-stream.test.ts,mobile-image-attachment.test.ts,mobile-image-source-picker.test.ts, andclipboard-image-temp-file.test.ts.Testing
mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts: Verifies immediate and 500ms forced terminal input lease recovery.mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts: Verifies bounded resubscribe for uncovered terminal view onstreamRevisionbump.mobile/src/session/mobile-image-attachment.test.ts,mobile-image-source-picker.test.ts, &src/main/window/clipboard-image-temp-file.test.ts: Verifiestype: '*/*',fileNameforwarding, and sanitized filename/extension preservation on host temp files.AI Disclosure
Assisted by Google DeepMind Antigravity agentic coding assistant for root-cause investigation, implementation, and testing.
Agent skill upstream boundary
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Maintainer changes
Thanks for the careful report and fix. I merged current
maininto the branch and kept your commits; my changes are in the commits on top.Reconnect while the picker is open (Terminal view)
unsubscribeTerminalnow signals a stream teardown in Terminal view too, not only while native chat covers the terminal.terminal.listdrops and reports the handle again), the same one covered chat uses.subscribeToTerminalnow returns whether it actually sent a subscribe, so a subscribe the bridge refuses immediately (for example at its stream cap) spends an attempt instead of retrying on every render.File names on the host
clipboard.startImageUploadcarries the optionalfileName. Older hosts ignore the unknown key and keep the.pngname; the single-frame fallback only runs against hosts too old to know the field, so it doesn't send it.sanitizeAgentSessionAttachmentName(leaf only, Windows reserved names, UTF-8 byte bound), keeps only letters, digits, space,.,_and-so the typed path can't expand in a shell ($(id).txtbecomes__id_.txt), and saves the file as<temp>/orca-paste-<uuid>/<name>. A flatorca-paste-…-notes.txtwould read as an Orca long-text paste in native chat, and could exceed 255 bytes.orca-paste-*.pngname (the sharedagentSessionPastedImageName), since Grok history and the "Pasted image" label recognize pasted images by that name. Only non-image picks carry their own name. The web shell reuses the chat file picker's naming (nameMedia, or a name from the MIME type on older shells).Visual proof (iOS simulator, Terminal view, long-press Attach → Files →
notes.txt, plain shell terminal, dev desktop on a scratch profile)main5a4dd30)Before, the text file lands as
orca-paste-<ms>-<uuid>.png, with the text inside it. After, it lands asorca-paste-<uuid>/notes.txt. The after capture is from f74ec01, one commit before the shell-safe name rule, which leavesnotes.txtunchanged. The reconnect fix isn't practical to stage on a device;use-mobile-session-terminal-resubscribe-snapshot.test.tsxdrives the real foundation, subscription and rearm hooks through a stream that ends after the lease was already dropped. It fails onmain(no second subscribe) and passes here.Testing: focused mobile suites (stream/rearm, resubscribe snapshot, subscription, web and native pickers, attach, clipboard upload, lease gate) and host suites (
clipboard-image-temp-file,clipboardRPC,agent-session-attachments), pluscheck:code-quality:changedandcheck:react-doctor:changed.Left out on purpose: Windows paths are still wrapped in double quotes by the shared
shellEscapePath, which desktop drag-and-drop also uses. That's tracked separately in #28783; picked names here no longer contain$or%either way.