fix(dashboard): open remote sessions from every agent reveal path - #18403
Conversation
Three reveal paths called bare setActiveWorktree + activateTabAndFocusPane,
skipping setActiveView('terminal'), ensureWorktreeHasInitialTerminal and
resumeSleepingAgentSessionsForWorktree. A parked SSH workspace has no resident
tab until those run, so the reveal landed on a workspace with no terminal.
Route all three through the incumbent activateAndRevealWorkspace dispatcher
(which the sidebar and "Jump to workspace" already use, and which also handles
folder workspaces). The Activity row-click additionally early-returned when the
thread's tab was absent from tabsByWorktree/unifiedTabsByWorktree, which made a
cold-parked remote thread a silent no-op; residency is now probed after
activation, so a revived tab is focused and a genuinely retained thread still
activates its workspace instead of doing nothing.
Also stop asserting `exited` from an absence of local state: SshPtyProvider
reports no authoritative buffer snapshot and the relay has no snapshot RPC, so
a null preview snapshot for a remote pty is loss of contact. The preview and
the no-pty dialog branch now say the remote preview is unavailable rather than
claiming the pane closed. Adding the relay snapshot RPC stays out of scope --
it needs capability negotiation.
Fixes #16731
📝 WalkthroughWalkthroughWorkspace and dashboard agent reveal flows now activate the target workspace before focusing its tab or pane, including execution-host targeting and parked SSH workspace revival. Thread activation checks the post-activation tab state before focusing. Terminal preview fallbacks now distinguish unavailable remote SSH sessions from closed local panes through a shared message helper and new localization coverage. Tests cover activation ordering, missing workspaces, missing tabs, and terminal preview states. Merge Risk: 🔵 Low · up to Some remote terminal previews can still incorrectly say that a pane has closed when the SSH session is merely unavailable. This is a bounded user-facing status error and should be corrected by passing the remote host classification through the preview. 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Full details: Description checkExplanation The description explains the bug, implementation, scope, linked issue, and testing results. However, it omits the required Visual Proof section and checklist, and it does not provide the required before/after attachments or an applicable N/A explanation. Full details: Linked Issues checkExplanation The changes address the remote preview message and route all three reveal paths through workspace activation. However, linked issue Full details: Out of Scope Changes checkExplanation The production changes and tests remain focused on remote dashboard activation, Activity reveal behavior, and honest unavailable-preview messaging. No unrelated feature or remote-wire changes are included. Full details: Docstring CoverageExplanation Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 13 files. (1 skipped: 1 unsupported.)
✨ 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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: d4d01d3b-cba9-419e-8245-e38ff0fd655a
📒 Files selected for processing (14)
src/renderer/src/components/activity/activity-thread-actions.test.tssrc/renderer/src/components/activity/activity-thread-actions.tssrc/renderer/src/components/dashboard-popout/AgentTerminalDialog.test.tsxsrc/renderer/src/components/dashboard-popout/AgentTerminalDialog.tsxsrc/renderer/src/components/dashboard-popout/AgentTerminalPreview.test.tsxsrc/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsxsrc/renderer/src/components/dashboard-popout/terminal-preview-unavailable-message.test.tssrc/renderer/src/components/dashboard-popout/terminal-preview-unavailable-message.tssrc/renderer/src/components/dashboard/AgentDashboardDrawer.test.tsxsrc/renderer/src/components/dashboard/AgentDashboardDrawer.tsxsrc/renderer/src/components/dashboard/reveal-dashboard-agent.tssrc/renderer/src/components/dashboard/useDashboardPopoutBridge.test.tsxsrc/renderer/src/components/dashboard/useDashboardPopoutBridge.tssrc/renderer/src/i18n/locales/en.json
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.
| 'dashboardPopout.terminal.closed', | ||
| "No live terminal — this agent's pane has closed." | ||
| )} | ||
| {terminalPreviewUnavailableMessage({ ptyId })} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings
Length of output: 13825
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate symbols ---'
rg -n -C 5 "terminalPreviewUnavailableMessage|parseAppSshPtyId|AgentTerminalPreview|AgentTerminalDialog|hostKind" src/renderer/src/components/dashboard-popout src/renderer/src 2>/dev/null | head -n 260
printf '%s\n' '--- file outline ---'
ast-grep outline src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx --view compact
printf '%s\n' '--- related tests ---'
rg -n -C 4 "terminalPreviewUnavailableMessage|parseAppSshPtyId|AgentTerminalPreview|hostKind" --glob '*test*' --glob '*spec*' src 2>/dev/null | head -n 220Repository: stablyai/orca
Length of output: 25236
🏁 Script executed:
#!/bin/bash
set -e
preview='src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx'
dialog='src/renderer/src/components/dashboard-popout/AgentTerminalDialog.tsx'
printf '%s\n' '--- preview ---'
cat -n "$preview" | sed -n '1,120p;390,455p'
printf '%s\n' '--- dialog references ---'
rg -n -C 8 "AgentTerminalPreview|terminalPreviewUnavailableMessage|hostKind|parseAppSshPtyId" "$dialog"
printf '%s\n' '--- definitions and tests ---'
files=$(rg -l "terminalPreviewUnavailableMessage|parseAppSshPtyId" src)
printf '%s\n' "$files"
for file in $files; do
printf '\n--- %s ---\n' "$file"
rg -n -C 8 "terminalPreviewUnavailableMessage|parseAppSshPtyId" "$file"
doneRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- dialog ---'
cat -n src/renderer/src/components/dashboard-popout/AgentTerminalDialog.tsx | sed -n '1,105p'
printf '%s\n' '--- unavailable-message implementation ---'
cat -n src/renderer/src/components/dashboard-popout/terminal-preview-unavailable-message.ts
printf '%s\n' '--- unavailable-message tests ---'
cat -n src/renderer/src/components/dashboard-popout/terminal-preview-unavailable-message.test.ts
printf '%s\n' '--- PTY creation and snapshot path in preview ---'
rg -n -C 6 "ptyGone|snapshot|getMainBufferSnapshot|terminalPreview" src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsxRepository: stablyai/orca
Length of output: 16519
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- DashboardCard contract ---'
rg -n -C 10 "export type DashboardCard|interface DashboardCard|ptyId:|hostKind:" src/shared/dashboard-snapshot.ts
printf '%s\n' '--- dashboard card construction ---'
rg -n -C 8 "DashboardCard|hostKind|ptyId" src/renderer/src/components/dashboard src/renderer/src/components/dashboard-popout src/shared/dashboard-snapshot.ts | head -n 260
printf '%s\n' '--- SSH PTY ID contract ---'
cat -n src/shared/ssh-pty-id.ts | sed -n '1,75p'Repository: stablyai/orca
Length of output: 30088
Pass hostKind to AgentTerminalPreview.
When card.hostKind is 'ssh' and parseAppSshPtyId(card.ptyId) returns null, terminalPreviewUnavailableMessage renders the local dashboardPopout.terminal.closed message. Thread hostKind through AgentTerminalPreview and add a regression test.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Activity row-click —
activateThreadTargetnow routes throughactivateAndRevealWorkspaceand probes tab residency after activation instead of before, so a cold-parked remote thread revives and focuses instead of silently no-op'ing. A gone workspace still returns early via the=== falsecontract. - Dashboard reveal surfaces — the pop-out bridge and in-window drawer share one
revealDashboardAgentwrapper that goes through the workspace dispatcher (notactivateAndRevealWorktreedirectly), preserving the folder-workspace path-status gate. - Message honesty — new
terminalPreviewUnavailableMessagedistinguishes SSH (hostKind === 'ssh'or an SSH ptyId) from local, backed bySshPtyProvider.canProvideAuthoritativeBufferSnapshot === false. - Tests — the bug-pinning assertion is inverted (
setActiveWorktreemust not be called), a cold-parked revive is covered directly, and the gone-workspace no-op is pinned.
The change is renderer-only and adds no wire fields, matching the remote-wire compatibility constraint. Routing through the incumbent dispatcher is the right call: it reuses setActiveView('terminal'), resumeSleepingAgentSessionsForWorktree, and ensureWorktreeHasInitialTerminal, which is exactly what the bare setActiveWorktree calls were skipping. ensureWorktreeHasInitialTerminal only reseeds when renderableTabCount === 0, so structured-agent-session and live-tab workspaces aren't given spurious shells.
ℹ️ hostKind remote detection is SSH-specific
terminalPreviewUnavailableMessage keys "unobservable" off hostKind === 'ssh' / parseAppSshPtyId, while DashboardCardHostKind also enumerates remote (remote-runtime relay) and wsl. WSL is fine because the daemon snapshot is authoritative; whether the remote-runtime relay also lacks an authoritative preview snapshot is out of this PR's SSH scope and unchanged by it. Nothing to do here — just flagging the boundary so it isn't mistaken for a complete "remote host" fix.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
1 participant
Summary
Fixes #16731. Clicking an agent could not open a remote (SSH) session: the reveal landed on a workspace with no terminal, the Activity row-click was a silent no-op, and the preview claimed the pane had closed.
Three reveal paths called bare
setActiveWorktree+activateTabAndFocusPane, skippingsetActiveView('terminal'),ensureWorktreeHasInitialTerminalandresumeSleepingAgentSessionsForWorktree:useDashboardPopoutBridge.tsAgentDashboardDrawer.tsxactivity-thread-actions.ts(added by #18222)All three now route through the incumbent
activateAndRevealWorkspace— the same dispatcher the sidebar (WorktreeCardAgents,active-worktree-focus-after-delete,use-worktree-context-menu-commands) and the row's own "Jump to workspace" affordance already use. No new activation helper; the two dashboard surfaces share one smallrevealDashboardAgentwrapper. The workspace dispatcher (notactivateAndRevealWorktreedirectly) so a folder-workspace card keeps its path-status gate.The Activity early-return is removed. It probed
tabsByWorktree/unifiedTabsByWorktreebefore activating, so a cold-parked remote thread — the normal state of an SSH session that was never revived — produced a silent no-op click. Residency is now probed after activation, matchingWorktreeCardAgents.handleActivateAgentTab: a revived tab gets focused, and a genuinely retained thread whose tab never returns still activates its workspace instead of doing nothing.Message honesty.
SshPtyProvider.canProvideAuthoritativeBufferSnapshotreturnsfalseand the relay'sRecentPtyOutputBufferhas no snapshot RPC, soserializeTerminalBufferFromAvailableStatehas nothing to fall through to for SSH. A null snapshot for a remote pty is loss of contact, and perdocs/reference/ssh-execution-boundary.mdthat isunverifiable, neverexited. The preview and the no-pty dialog branch now render a host-aware string instead of asserting the pane closed. Adding the relay snapshot RPC is out of scope — a new stream opcode needs capability negotiation (docs/reference/remote-wire-compatibility.md).Renderer-only. Nothing crosses the remote wire:
hostKindis already onDashboardCard, and no snapshot field was added.Not fixed here (deliberate)
activate-tab-and-focus-pane.tsdispatchesFOCUS_TERMINAL_PANE_EVENTinside a singlerequestAnimationFrame, and its only consumer is a barewindow.addEventListener, so a pane mounting after the dispatch never sees it. All four surfaces share this. It is a real gap, but it is a focus flash, not the reported symptom — with activation routed correctly the workspace opens and the terminal is there. Building a TTL'd pending-focus registry for it speculatively is a larger change than the bug being fixed, so it is left alone and noted.Testing
pnpm tc— cleanpnpm test src(whole tree) — 69,614 passed. The only 2 failures are insrc/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts, which fails identically on a clean tree (verified by stashing) and is unrelated main-process transcript-watcher work.pnpm run check:code-quality:changed— 0 new findings (code quality, type-aware, React Doctor)oxlintover the three touched directories — clean, withmax-linesproven to fire there via a 320-line positive controlFailed-first: 12 of the 66 assertions in the touched suites fail against the pre-fix source. The important one is
activity-thread-actions.test.ts, which previously pinned the bug: it asserted the baresetActiveWorktree(thread.worktree.id, REMOTE_HOST)call for a remote host. That assertion is now inverted (setActiveWorktreemust not be called; the dispatcher must be), and a cold-parked remote thread whose tab is not resident until activation revives it is covered directly — the reported symptom and the early-return case.Notes
Supersedes #16732, which attempted this before the defect spread to Activity. That PR routes through
activateAndRevealWorktree(missing the folder-workspace dispatcher), does not touch the Activity surface or its early-return, does not update the test that pins the bug, and copies the new English string intoes/ja/ko/zh. Its one piece of unique work is the pending-focus registry described above. Recommend closing it in favour of this PR and tracking the focus registry separately if it proves load-bearing.