fix(terminals): terminal IDs, agent rows and dashboard rows follow the owner (#27243)

* fix(terminals): terminal IDs, agent rows and dashboard rows follow the owner

Copy Terminal ID on a paired-server pane copies the owning server's handle
instead of asking this desktop's runtime, which has no record of that pane.
Sidebar agent rows and dashboard reveals tell the workspace's owning server
which tab was opened, as a tab-strip click does. A paired-server dashboard card
with no preview says the session is remote instead of claiming its pane closed.

* fix(terminals): owner activation keeps the picked pane and yields to later navigation

The row and dashboard reveal now send the exact split pane to the owner and
record it in the focus intent, guarded by the tab visible at the click, so a
later navigation away is never overridden when the tab arrives.

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
OrcaWin
2026-10-10 05:28:40 -07:00
committed by GitHub
co-authored by m4air
parent 4dfc74e5e6
commit e41b8e17f0
14 changed files with 323 additions and 5 deletions
@@ -15,4 +15,11 @@ describe('terminalPreviewUnavailableMessage', () => {
expect(fromPtyId).not.toMatch(/pane has closed/)
expect(terminalPreviewUnavailableMessage({ hostKind: 'ssh' })).toBe(fromPtyId)
})
it('reports a paired-server card as remote instead of claiming its pane closed (#25688)', () => {
const remote = terminalPreviewUnavailableMessage({ hostKind: 'ssh' })
expect(terminalPreviewUnavailableMessage({ hostKind: 'remote' })).toBe(remote)
expect(terminalPreviewUnavailableMessage({ ptyId: 'remote:env-1@@term_1' })).toBe(remote)
expect(terminalPreviewUnavailableMessage({ ptyId: 'remote:term_1' })).toBe(remote)
})
})
@@ -1,12 +1,14 @@
import { translate } from '@/i18n/i18n'
import type { DashboardCardHostKind } from '../../../../shared/dashboard-snapshot'
import { parseAppSshPtyId } from '../../../../shared/ssh-pty-id'
import { isRemoteRuntimePtyId } from '../../../../shared/remote-runtime-pty-id'
/**
* A missing buffer snapshot only proves the pane exited when the client could have
* observed it. `SshPtyProvider` reports no authoritative buffer snapshot and the relay
* exposes no snapshot RPC, so for a remote pty the absence is loss of contact —
* `unverifiable`, never `exited`. See docs/reference/ssh-execution-boundary.md.
* `unverifiable`, never `exited`. A paired server's pane is likewise held by that server, not by
* this client. See docs/reference/ssh-execution-boundary.md.
*/
export function terminalPreviewUnavailableMessage(source: {
ptyId?: string | null
@@ -14,6 +16,8 @@ export function terminalPreviewUnavailableMessage(source: {
}): string {
const isRemote =
source.hostKind === 'ssh' ||
source.hostKind === 'remote' ||
isRemoteRuntimePtyId(source.ptyId) ||
(typeof source.ptyId === 'string' && parseAppSshPtyId(source.ptyId) !== null)
return isRemote
? translate(
@@ -0,0 +1,57 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
activateAndRevealWorkspace: vi.fn(),
activateTabAndFocusPane: vi.fn(),
activateTerminalTabOnOwner: vi.fn()
}))
vi.mock('@/lib/worktree-activation', () => ({
activateAndRevealWorkspace: mocks.activateAndRevealWorkspace
}))
vi.mock('@/lib/activate-tab-and-focus-pane', () => ({
activateTabAndFocusPane: mocks.activateTabAndFocusPane
}))
vi.mock('@/lib/terminal-tab-owner-activation', () => ({
activateTerminalTabOnOwner: mocks.activateTerminalTabOnOwner
}))
import { revealDashboardAgent } from './reveal-dashboard-agent'
const args = {
repoId: 'repo-1',
worktreeId: 'wt-1',
executionHostId: 'runtime:env-1' as const,
tabId: 'web-terminal-host-tab',
leafId: '11111111-1111-4111-8111-111111111111'
}
describe('revealDashboardAgent', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it("focuses the card's own tab on the workspace's owner", () => {
mocks.activateAndRevealWorkspace.mockReturnValue(true)
expect(revealDashboardAgent(args)).toBe(true)
expect(mocks.activateTerminalTabOnOwner).toHaveBeenCalledWith('wt-1', args.tabId, args.leafId)
expect(mocks.activateTabAndFocusPane).toHaveBeenCalledWith(args.tabId, args.leafId, {
flashFocusedPane: true
})
// Why: the owner call captures the visible tab, so it must follow the local activation.
expect(mocks.activateTabAndFocusPane.mock.invocationCallOrder[0]).toBeLessThan(
mocks.activateTerminalTabOnOwner.mock.invocationCallOrder[0]
)
})
it('touches no tab when the workspace cannot be revealed', () => {
mocks.activateAndRevealWorkspace.mockReturnValue(false)
expect(revealDashboardAgent(args)).toBe(false)
expect(mocks.activateTerminalTabOnOwner).not.toHaveBeenCalled()
expect(mocks.activateTabAndFocusPane).not.toHaveBeenCalled()
})
})
@@ -1,5 +1,6 @@
import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane'
import { activateAndRevealWorkspace } from '@/lib/worktree-activation'
import { activateTerminalTabOnOwner } from '@/lib/terminal-tab-owner-activation'
import type { DashboardRevealAgentArgs } from '../../../../shared/dashboard-snapshot'
/**
@@ -19,5 +20,6 @@ export function revealDashboardAgent(args: DashboardRevealAgentArgs): boolean {
return false
}
activateTabAndFocusPane(args.tabId, args.leafId, { flashFocusedPane: true })
activateTerminalTabOnOwner(args.worktreeId, args.tabId, args.leafId)
return true
}
@@ -98,6 +98,10 @@ const activationMocks = vi.hoisted(() => ({
activateTabAndFocusPane: vi.fn()
}))
const ownerActivationMocks = vi.hoisted(() => ({
activateTerminalTabOnOwner: vi.fn()
}))
const staleAgentRowMocks = vi.hoisted(() => ({
dismissStaleAgentRowByKey: vi.fn()
}))
@@ -123,6 +127,10 @@ vi.mock('@/lib/activate-tab-and-focus-pane', () => ({
activateTabAndFocusPane: activationMocks.activateTabAndFocusPane
}))
vi.mock('@/lib/terminal-tab-owner-activation', () => ({
activateTerminalTabOnOwner: ownerActivationMocks.activateTerminalTabOnOwner
}))
vi.mock('../terminal-pane/stale-agent-row', () => ({
dismissStaleAgentRowByKey: staleAgentRowMocks.dismissStaleAgentRowByKey
}))
@@ -203,6 +211,54 @@ describe('WorktreeCardAgents activation', () => {
expect(staleAgentRowMocks.dismissStaleAgentRowByKey).not.toHaveBeenCalled()
})
it('tells the owning server which tab a row opens (#12739, #22196)', async () => {
mockAgentActivityDisplayMode = 'full'
const tabId = 'web-terminal-host-tab'
const paneKey = makePaneKey(tabId, LEAF_A)
mockAgents = [
mockAgent({ paneKey, tabId, agentType: 'codex', prompt: 'Remote', worktreeId: 'wt-1' })
]
mockTabsByWorktree = { 'wt-1': [{ id: tabId }] }
mockAgentStatusByPaneKey = { [paneKey]: { worktreeId: 'wt-1' } }
const { default: WorktreeCardAgents } = await import('./WorktreeCardAgents')
renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
capturedRowActivations[0].onActivate(tabId, paneKey)
expect(ownerActivationMocks.activateTerminalTabOnOwner).toHaveBeenCalledWith(
'wt-1',
tabId,
LEAF_A
)
expect(activationMocks.activateTabAndFocusPane).toHaveBeenCalledWith(
tabId,
LEAF_A,
expect.anything()
)
})
it('asks the owner to focus a live row whose tab has not reached this window yet', async () => {
mockAgentActivityDisplayMode = 'full'
const tabId = 'web-terminal-unhydrated'
const paneKey = makePaneKey(tabId, LEAF_A)
mockAgents = [
mockAgent({ paneKey, tabId, agentType: 'codex', prompt: 'Remote', worktreeId: 'wt-1' })
]
mockAgentStatusByPaneKey = { [paneKey]: { worktreeId: 'wt-1' } }
const { default: WorktreeCardAgents } = await import('./WorktreeCardAgents')
renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
capturedRowActivations[0].onActivate(tabId, paneKey)
expect(ownerActivationMocks.activateTerminalTabOnOwner).toHaveBeenCalledWith(
'wt-1',
tabId,
LEAF_A
)
expect(activationMocks.activateTabAndFocusPane).not.toHaveBeenCalled()
expect(staleAgentRowMocks.dismissStaleAgentRowByKey).not.toHaveBeenCalled()
})
it('reveals the worktree and focuses an automation worker row hydrated during reveal', async () => {
mockAgentActivityDisplayMode = 'full'
const tabId = 'worker-tab'
@@ -3,6 +3,7 @@ import { useShallow } from 'zustand/react/shallow'
import { useAppStore } from '@/store'
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane'
import { activateTerminalTabOnOwner } from '@/lib/terminal-tab-owner-activation'
import DashboardAgentRow from '@/components/dashboard/DashboardAgentRow'
import { useNow } from '@/hooks/use-now'
import { deriveRunningAgentSendTargets } from '@/lib/running-agent-targets'
@@ -178,10 +179,12 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({
flashFocusedPane: true,
scrollToBottomIfOutputSinceLastView: true
})
activateTerminalTabOnOwner(worktreeId, tabId, parsed.leafId)
} else if (!activateStructuredAgentSessionTab({ worktreeId, tabId })) {
const liveEntry = useAppStore.getState().agentStatusByPaneKey[paneKey]
if (liveEntry?.worktreeId === worktreeId) {
// Why: orchestration worker status can be worktree-attributed before the renderer knows its tab; keep the live row instead of dismissing as stale.
activateTerminalTabOnOwner(worktreeId, tabId, parsed.leafId)
return
}
dismissStaleAgentRowByKey(paneKey)
@@ -1,5 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { copyTerminalHandleForPane } from './terminal-handle-copy'
import { toRemoteRuntimePtyId } from '../../../../shared/remote-runtime-pty-id'
const LEAF_ID = '11111111-1111-4111-8111-111111111111'
@@ -58,4 +59,45 @@ describe('copyTerminalHandleForPane', () => {
expect(writeClipboardText).not.toHaveBeenCalled()
})
it('copies the owning server handle for a paired-server pane without asking this runtime', async () => {
// Why: this desktop's runtime has no record of a server-owned pane (#11664, #15553).
const callRuntime = vi.fn().mockResolvedValue({
id: 'req-1',
ok: false,
error: { code: 'terminal_not_found', message: 'terminal not found' }
})
const writeClipboardText = vi.fn().mockResolvedValue(undefined)
await expect(
copyTerminalHandleForPane({
tabId: 'tab-1',
leafId: LEAF_ID,
ptyId: toRemoteRuntimePtyId('term_on_server', 'env-1'),
callRuntime,
writeClipboardText
})
).resolves.toBe('term_on_server')
expect(callRuntime).not.toHaveBeenCalled()
expect(writeClipboardText).toHaveBeenCalledWith('term_on_server')
})
it('refuses an unreadable paired-server pty instead of asking this runtime', async () => {
const callRuntime = vi.fn()
const writeClipboardText = vi.fn()
await expect(
copyTerminalHandleForPane({
tabId: 'tab-1',
leafId: LEAF_ID,
ptyId: 'remote:',
callRuntime,
writeClipboardText
})
).rejects.toThrow('Terminal ID unavailable')
expect(callRuntime).not.toHaveBeenCalled()
expect(writeClipboardText).not.toHaveBeenCalled()
})
})
@@ -1,9 +1,15 @@
import type { RuntimeRpcResponse } from '../../../../shared/runtime-rpc-envelope'
import { makePaneKey } from '../../../../shared/stable-pane-id'
import {
isRemoteRuntimePtyId,
parseRemoteRuntimePtyId
} from '../../../../shared/remote-runtime-pty-id'
type CopyTerminalHandleDeps = {
tabId: string
leafId: string
/** The pane's bound PTY; a paired-server PTY id already names the owner's handle. */
ptyId?: string | null
callRuntime: (request: {
method: 'terminal.resolvePane'
params: { paneKey: string }
@@ -14,9 +20,20 @@ type CopyTerminalHandleDeps = {
export async function copyTerminalHandleForPane({
tabId,
leafId,
ptyId,
callRuntime,
writeClipboardText
}: CopyTerminalHandleDeps): Promise<string> {
// Why: this desktop's runtime does not own a paired server's pane; the owner's handle is what
// its CLI addresses, and the PTY binding already carries it.
if (ptyId && isRemoteRuntimePtyId(ptyId)) {
const ownerHandle = parseRemoteRuntimePtyId(ptyId)?.handle
if (!ownerHandle) {
throw new Error('Terminal ID unavailable')
}
await writeClipboardText(ownerHandle)
return ownerHandle
}
const paneKey = makePaneKey(tabId, leafId)
const response = await callRuntime({
method: 'terminal.resolvePane',
@@ -55,7 +55,8 @@ export const copyTerminalPaneMenuPaneId = async (
export const copyTerminalPaneMenuTerminalId = async (
pane: ManagedPane | null,
tabId: string
tabId: string,
ptyId: string | null
): Promise<void> => {
if (!pane) {
return
@@ -64,6 +65,7 @@ export const copyTerminalPaneMenuTerminalId = async (
await copyTerminalHandleForPane({
tabId,
leafId: pane.leafId,
ptyId,
callRuntime: window.api.runtime.call,
writeClipboardText: window.api.ui.writeTerminalClipboardText
})
@@ -174,8 +174,11 @@ export function useTerminalPaneContextMenu({
const onCopyPaneId = async (): Promise<void> =>
copyTerminalPaneMenuPaneId(resolveMenuPane(), tabId)
const onCopyTerminalId = async (): Promise<void> =>
copyTerminalPaneMenuTerminalId(resolveMenuPane(), tabId)
const onCopyTerminalId = async (): Promise<void> => {
const pane = resolveMenuPane()
const ptyId = pane ? (paneTransportsRef.current?.get(pane.id)?.getPtyId() ?? null) : null
return copyTerminalPaneMenuTerminalId(pane, tabId, ptyId)
}
const onCopyAgentSessionId = async (): Promise<void> => {
const pane = resolveMenuPane()
@@ -0,0 +1,54 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
activateWebRuntimeSessionTab: vi.fn(),
getRuntimeEnvironmentIdForWorktree: vi.fn(),
resolveWebSessionVisibleTabId: vi.fn()
}))
vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } }))
vi.mock('@/lib/worktree-runtime-owner', () => ({
getRuntimeEnvironmentIdForWorktree: mocks.getRuntimeEnvironmentIdForWorktree
}))
vi.mock('@/runtime/web-runtime-session', () => ({
activateWebRuntimeSessionTab: mocks.activateWebRuntimeSessionTab,
isWebRuntimeSessionActive: (environmentId: string | null | undefined) =>
Boolean(environmentId?.trim())
}))
vi.mock('@/runtime/web-session-focus-intent', () => ({
resolveWebSessionVisibleTabId: mocks.resolveWebSessionVisibleTabId
}))
import { activateTerminalTabOnOwner } from './terminal-tab-owner-activation'
describe('activateTerminalTabOnOwner', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it('tells the paired server that owns the workspace, not the focused one', () => {
mocks.getRuntimeEnvironmentIdForWorktree.mockReturnValue('env-owner')
mocks.resolveWebSessionVisibleTabId.mockReturnValue('unified-visible')
activateTerminalTabOnOwner('wt-1', 'web-terminal-host-tab', 'leaf-b')
expect(mocks.getRuntimeEnvironmentIdForWorktree).toHaveBeenCalledWith({}, 'wt-1')
expect(mocks.activateWebRuntimeSessionTab).toHaveBeenCalledWith({
worktreeId: 'wt-1',
tabId: 'web-terminal-host-tab',
environmentId: 'env-owner',
leafId: 'leaf-b',
// Why: a later navigation away from this tab must cancel the deferred focus.
expectedCurrentLocalTabId: 'unified-visible'
})
})
it('makes no owner round trip for a workspace this client runs', () => {
mocks.getRuntimeEnvironmentIdForWorktree.mockReturnValue(null)
activateTerminalTabOnOwner('wt-local', 'tab-1', null)
expect(mocks.activateWebRuntimeSessionTab).not.toHaveBeenCalled()
})
})
@@ -0,0 +1,33 @@
import { useAppStore } from '@/store'
import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner'
import {
activateWebRuntimeSessionTab,
isWebRuntimeSessionActive
} from '@/runtime/web-runtime-session'
import { resolveWebSessionVisibleTabId } from '@/runtime/web-session-focus-intent'
/**
* Tells the paired server that owns `worktreeId` which tab and pane this client revealed, as a
* tab-strip click does: the owner stores it as this client's selection and wakes a parked pane,
* and a tab that has not reached this renderer yet is focused when it arrives.
* Call after the local activation; navigating away before the tab arrives cancels that focus.
* A workspace this client runs itself (local or SSH) needs no owner round trip.
*/
export function activateTerminalTabOnOwner(
worktreeId: string,
tabId: string,
leafId: string | null
): void {
const state = useAppStore.getState()
const environmentId = getRuntimeEnvironmentIdForWorktree(state, worktreeId)
if (!environmentId || !isWebRuntimeSessionActive(environmentId)) {
return
}
void activateWebRuntimeSessionTab({
worktreeId,
tabId,
environmentId,
leafId,
expectedCurrentLocalTabId: resolveWebSessionVisibleTabId(state, worktreeId)
})
}
@@ -182,6 +182,31 @@ describe('web runtime session tab actions', () => {
expect(mocks.applyWebSessionTabsSnapshot).toHaveBeenCalled()
})
it('carries the picked split pane and the visible tab guard to the owner and focus intent', async () => {
mocks.resolveHostSessionTabIdForWebSessionTab.mockReturnValue('host-terminal')
const runtimeCall = vi.fn().mockResolvedValue({ id: 'activate', ok: true, result: {} })
vi.stubGlobal('window', { api: { runtimeEnvironments: { call: runtimeCall } } })
await activateWebRuntimeSessionTab({
worktreeId: WORKTREE_ID,
tabId: 'local-terminal',
leafId: 'leaf-b',
expectedCurrentLocalTabId: 'unified-local-terminal'
})
expect(runtimeCall).toHaveBeenCalledWith(
expect.objectContaining({
method: 'session.tabs.activate',
params: expect.objectContaining({ tabId: 'host-terminal', leafId: 'leaf-b' })
})
)
expect(peekWebSessionFocusIntent({ environmentId: ENVIRONMENT_ID }, WORKTREE_ID)).toEqual({
hostTabId: 'host-terminal',
leafId: 'leaf-b',
expectedCurrentLocalTabId: 'unified-local-terminal'
})
})
it('supersedes browser focus intent when a terminal is activated next', async () => {
mocks.resolveHostSessionTabIdForWebSessionTab.mockImplementation(
(_state, args: { tabId: string }) =>
@@ -28,6 +28,10 @@ export async function activateWebRuntimeSessionTab(args: {
worktreeId: string
tabId: string
environmentId?: string | null
/** The split pane the user picked; without it the host and focus intent pick the first. */
leafId?: string | null
/** The local tab visible at the gesture; later navigation away cancels the deferred focus. */
expectedCurrentLocalTabId?: string | null
}): Promise<boolean> {
return (await callWebRuntimeSessionTabMethod('session.tabs.activate', args)) === 'applied'
}
@@ -60,6 +64,8 @@ async function callWebRuntimeSessionTabMethod(
reason?: RuntimeSessionTabCloseReason
publicationEpoch?: string | null
terminalHandle?: string | null
leafId?: string | null
expectedCurrentLocalTabId?: string | null
}
): Promise<WebRuntimeSessionTabCloseOutcome> {
const environmentId =
@@ -110,7 +116,13 @@ async function callWebRuntimeSessionTabMethod(
recordWebSessionCloseIntent(intentOwner, args.worktreeId, hostTabId, Date.now())
} else {
activationHostTabId = hostTabId
recordWebSessionFocusIntent(intentOwner, args.worktreeId, hostTabId)
recordWebSessionFocusIntent(
intentOwner,
args.worktreeId,
hostTabId,
args.leafId ?? undefined,
args.expectedCurrentLocalTabId
)
}
const response = await callEnvironment({
// Why: old hosts cannot route this additive method, so a generation
@@ -121,6 +133,7 @@ async function callWebRuntimeSessionTabMethod(
tabId: hostTabId,
...(method === 'session.tabs.activate'
? {
...(args.leafId ? { leafId: args.leafId } : {}),
// Why: the additive navigation target protects new hosts while notifyClients:false protects old hosts.
notifyClients: false,
navigation: 'caller' as const,