Skip to content

fix(cli): move this desktop on terminal switch to a paired desktop - #31527

Open
Velm14 wants to merge 8 commits into
stablyai:mainfrom
Velm14:fix/paired-desktop-terminal-switch
Open

Velm14 wants to merge 8 commits into
stablyai:mainfrom
Velm14:fix/paired-desktop-terminal-switch

Conversation

@Velm14

@Velm14 Velm14 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

ELI5

orca terminal switch --environment <name> should show that terminal in the Orca window on the machine where you ran the command. When the other machine was also running the Orca desktop app, its window jumped to the terminal and yours stayed where it was.

What Changed

Why

#28023 already said "the desktop on the CLI's own machine is the one to move"; it just decided that from the host's answer, which is the wrong signal once the host has a window. Asking the local desktop first is the direct form of the same rule, and it also stops the other machine's window from jumping for whoever sits at it. caller and host were added together in #9664, so any server that accepts host accepts caller — no mixed-version risk.

Linked Issue

Fixes #31073

Visual Proof

N/A — CLI routing only; the window behaviour is the same reveal #28023 added, now also reached for paired desktops.

Testing

  • src/cli/handlers/terminal-switch-paired-reveal.test.ts now covers: the local desktop moves and the host gets navigation: 'caller' for both a windowless server and a paired desktop (both cases fail on main); the host navigates itself when no local desktop shows the environment; local terminals still go straight to the host.

  • pnpm test src/cli/handlers src/cli/paired-terminal-desktop-reveal.test.ts — 55 files, 595 tests pass.

  • tc:cli, oxlint, oxfmt and check:code-quality:changed pass.

  • I couldn't run the two-desktop setup from the issue myself; @luisvae, if you have a minute, a run of the repro from [Bug]: terminal switch --environment doesn't move the local desktop when the paired environment is a desktop app (follow-up to #24688) #31073 on this branch would confirm it.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Written with Claude Code (Claude Opus 5.5).

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

🤖 Generated with Claude Code

terminal switch --environment revealed the terminal on this machine's
desktop only when the paired host answered navigated:false. A paired host
that is itself a desktop app navigates its own window and answers true, so
this desktop never moved and the other machine's window jumped instead.

The local desktop is now asked first; the paired host only moves its own
window when no desktop here shows that environment.

Fixes stablyai#31073

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
Walkthrough

For remote terminals, the handler attempts to reveal the paired terminal on the local desktop before calling terminal.focus. If the reveal succeeds, it uses caller navigation and marks the result as navigated. Otherwise, it uses host navigation. Local terminals continue to use host navigation. The reveal request now has a 12-second timeout. Tests cover these paths.

Severity of issue fixed: Medium

Merge Risk: Low · previous review e0dcb

Paired terminal switching can be slow when the remote host is resolving a worktree. Bound that preliminary request before merging, or accept the limited navigation delay.

Pre-merge checks: Passed 5

Check name Status Explanation
Description check Passed The description is complete and aligned with the template. It explains the user impact, implementation, rationale, linked issue #31073, testing, visual proof exception, scope, and checklist status.
Title check Passed The title clearly identifies the CLI terminal-switch change for paired desktops. It is concise and related to the primary change.
Linked Issues check Passed Issue #31073 requires a paired terminal switch to reveal the terminal in the CLI machine's desktop. terminalFocusHandler calls revealPairedTerminalOnLocalDesktop first for remote clients. When loc…
Out of Scope Changes check Passed The changes remain within issue #31073. The 12,000 ms timeout bounds the local desktop reveal and supports fallback behavior. The related tests and comments verify the routing. No unrelated change is …
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files.
Full details: Linked Issues check

Explanation

Issue #31073 requires a paired terminal switch to reveal the terminal in the CLI machine's desktop. terminalFocusHandler calls revealPairedTerminalOnLocalDesktop first for remote clients. When local reveal succeeds, it calls terminal.focus with navigation: 'caller' and reports navigated: true. When local reveal fails, it uses host navigation. Local terminals still use host navigation. Tests cover both host types, fallback behavior, and local terminals.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #31073. The 12,000 ms timeout bounds the local desktop reveal and supports fallback behavior. The related tests and comments verify the routing. No unrelated change is shown.

  • 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.

Share

@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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fa77de59-f886-494a-be23-846bdb218f33
📥 Commits

Reviewing files that changed from the base of the PR and between ad269f4 and 01ef467.

📒 Files selected for processing: 3
  • src/cli/handlers/terminal-switch-paired-reveal.test.ts
  • src/cli/handlers/terminal.ts
  • src/cli/paired-terminal-desktop-reveal.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.

A local desktop that accepted the reveal but never answered held the
fallback for the client's default request timeout. The probe now gives up
just after the desktop's own 10 s renderer wait.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@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.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a144582c-7535-4842-b44d-28d727b4699f
📥 Commits

Reviewing files that changed from the base of the PR and between 01ef467 and 95f4175.

📒 Files selected for processing: 2
  • src/cli/paired-terminal-desktop-reveal.test.ts
  • src/cli/paired-terminal-desktop-reveal.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Velm14 and others added 3 commits October 21, 2026 17:20
The runtime-source fence can probe status.get on the local runtime before
terminal.reveal, and that probe used the client's 60 s default.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@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.

Actionable comments posted: 1


ℹ️ 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: 55366747-e37f-42fc-b409-444e8b4e043a
📥 Commits

Reviewing files that changed from the base of the PR and between 8042bd4 and e0dcb3b.

📒 Files selected for processing: 2
  • src/cli/handlers/terminal-switch-paired-reveal.test.ts
  • src/cli/handlers/terminal.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/cli/handlers/terminal.ts Resolved
A cold worktree catalog can make terminal.show scan before it answers, and the host
fallback waits on it now that the local reveal runs first.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Upstream dropped paired-terminal-desktop-reveal.test.ts as low-value (stablyai#38642). Keep only the two cases
this PR adds: both reveal steps are bounded before the host fallback, and a slow host asks no desktop.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Velm14

Velm14 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@AmethystLiang ping on this since you're on #31073. Two more people ran into it this week (the comment on #31073 and #38927), so it'd be nice to get it in. The workflows need an approval to run since it's from a fork.

@AmethystLiang

Copy link
Copy Markdown
Contributor

Thank you @Velm14 . Look into it

Upstream moved the terminal CLI into src/cli/terminals and dropped the handlers test; keep our switch reveal coverage there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Velm14

Velm14 commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! Fwiw two more people hit the same thing on #31073 since, and worktree activate and file open --focus have the same root cause, happy to follow up on those after this one.

This branch has not been deployed

No deployments
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.

[Bug]: terminal switch --environment doesn't move the local desktop when the paired environment is a desktop app (follow-up to #24688)

2 participants