Skip to content

Fix restored terminals rendering blank on mobile#8768

Merged
Jinwoo-H merged 3 commits into
mainfrom
Jinwoo-H/terminals-not-working-mobile
Jul 14, 2026
Merged

Fix restored terminals rendering blank on mobile#8768
Jinwoo-H merged 3 commits into
mainfrom
Jinwoo-H/terminals-not-working-mobile

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • restore terminal output on mobile for daemon PTYs that survived a desktop app relaunch
  • preserve mobile-only navigation introduced by Prevent mobile worktree selection from focusing desktop #6461 without focusing or mounting the matching desktop workspace
  • use the provider-owned buffer snapshot only after both the main headless model and a mounted renderer snapshot are unavailable

Root cause

#6461 intentionally changed mobile worktree/tab selection to avoid navigating the desktop (notifyClients: false). That exposed an existing state gap for restored terminals: main could discover that a daemon PTY was alive and mark its tab ready, while neither a main-process headless model nor a mounted desktop renderer serializer existed. Mobile subscription therefore emitted an empty initial snapshot until a desktop reveal mounted the pane.

This is independent of the cloud relay path and reproduces over a direct local-only connection.

Fix

The serialization order is now:

  1. main-process headless snapshot
  2. registered desktop renderer snapshot
  3. provider-owned snapshot by PTY ID

The local daemon already retains an authoritative buffer for surviving PTYs, so the final fallback can return their history without mounting or focusing desktop UI. Providers that do not implement buffer snapshots, including unsupported native/SSH paths, return null and retain the previous behavior.

Validation

  • pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts src/main/runtime/mobile-subscribe-integration.test.ts src/main/runtime/rpc/terminal-subscribe-buffer.test.ts — 1,025 passed, 2 skipped
  • pnpm run typecheck:node
  • pnpm exec oxlint src/main/ipc/pty.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.ts src/main/runtime/orca-runtime.test.ts
  • pnpm exec electron-vite build --mode e2e
  • real iPhone 17 Pro simulator, fullscreen Electron, direct LAN/local-only pairing, relay disabled

The simulator flow created nine terminals with unique markers, moved desktop to another workspace, fully quit/relaunched Electron while daemon PTYs survived, and opened the background workspace on mobile. No terminal tab was touched before evidence.

Before: the active restored terminal remained blank after five seconds.

Before: restored terminal blank on mobile

After: the active restored terminal displayed its marker within five seconds while all eight marked desktop panes remained unmounted. The mobile stream emitted a 381-byte, one-chunk initial snapshot.

After: restored terminal output visible immediately

Made with Orca 🐋

Jinwoo-H and others added 2 commits July 14, 2026 17:24
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Provider reattach results now include reset or continued output sequence metadata, and IPC spawn paths synchronize runtime PTY sequencing with it. Runtime serialization can use provider-owned buffer snapshots when renderer serialization is unavailable or provider state is preferred. Headless hydration, title handling, sequence rebasing, and alternate-screen tracking were updated for restored PTYs. Tests cover sequencing races, provider snapshot fallback, streaming, cold restore, and alternate-screen state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers summary, root cause, fix, and testing, but omits the required AI Review Report, Security Audit, and Notes sections. Add the missing AI Review Report, Security Audit, and Notes sections, and move the screenshots into a dedicated Screenshots section.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main user-visible fix and is concise and specific.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Co-authored-by: Orca <help@stably.ai>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/shared/terminal-kitty-keyboard-mode-tracker.test.ts (1)

63-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Verify that alternate-screen observation remains sticky.

src/main/runtime/orca-runtime.ts uses this flag to determine whether a switch was ever observed. Assert it remains true after 1049l; otherwise a regression could pass this test while skipping provider snapshot reconciliation.

Proposed test addition
    tracker.scan('\x1b[?1049l')
+    expect(tracker.hasObservedAlternateScreenSwitch).toBe(true)
    expect(tracker.isAlternateScreen).toBe(false)
src/main/runtime/orca-runtime.test.ts (1)

7407-7613: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the repeated provider-buffer controller mock into a helper.

The same { write, kill, getForegroundProcess, serializeProviderBuffer, hasRendererSerializer: () => false } shape is re-declared inline across six tests. A small factory would cut duplication and keep future controller-contract changes to one place.

♻️ Suggested helper
+function createProviderBufferOnlyController(
+  serializeProviderBuffer: RuntimePtyController['serializeProviderBuffer']
+): RuntimePtyController {
+  return {
+    write: () => true,
+    kill: () => true,
+    getForegroundProcess: async () => null,
+    serializeProviderBuffer,
+    hasRendererSerializer: () => false
+  }
+}

Then each test becomes runtime.setPtyController(createProviderBufferOnlyController(serializeProviderBuffer)).


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ebd83fba-c015-47fa-b1da-08d8c0f58f6b

📥 Commits

Reviewing files that changed from the base of the PR and between 49303fe and 02de3c5.

📒 Files selected for processing (10)
  • src/main/daemon/daemon-pty-adapter.test.ts
  • src/main/daemon/daemon-pty-adapter.ts
  • src/main/ipc/pty.test.ts
  • src/main/ipc/pty.ts
  • src/main/providers/types.ts
  • src/main/runtime/orca-runtime.test.ts
  • src/main/runtime/orca-runtime.ts
  • src/main/runtime/rpc/terminal-provider-snapshot-sequence.test.ts
  • src/shared/terminal-kitty-keyboard-mode-tracker.test.ts
  • src/shared/terminal-kitty-keyboard-mode-tracker.ts

@Jinwoo-H Jinwoo-H merged commit 515bf2d into main Jul 14, 2026
6 checks passed
@Jinwoo-H Jinwoo-H mentioned this pull request Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant