fix: honor remote terminal insertion in the calling client (#18995)

* fix: settle remote terminal insertion in the calling client

* refactor: share one anchor insertion path for local and remote terminals

Extract the created-tab-after-anchor reorder that the local terminal IPC
bridge already carried into insertUnifiedTabAfterAnchor, and settle the
remote placement through it instead of a second copy.

Also repairs two anchor-resolution gaps in the settlement:
- keep an exact unified tab id (legacy leaf-keyed anchors, browser and
  editor tabs) instead of collapsing every anchor to a terminal parent,
  which could mint a `web-terminal-<browser tab>` id that matches nothing
- fall back to the anchor's own group when the requested group was closed
  while the mirrored tab was still in flight
This commit is contained in:
Neil
2026-09-06 18:29:25 -07:00
committed by GitHub
parent b497f15b53
commit 0d973c1505
5 changed files with 147 additions and 36 deletions
@@ -3,6 +3,7 @@ import { getConnectionIdFromState } from '@/lib/connection-context'
import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode'
import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability'
import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route'
import { insertUnifiedTabAfterAnchor } from '@/lib/unified-tab-anchor-insertion'
import { translate } from '@/i18n/i18n'
import { useAppStore } from '../../store'
import {
@@ -83,30 +84,11 @@ export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void {
requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] })
}
if (data.afterTabId) {
const createdUnifiedTab = useAppStore
const createdUnifiedTabId = useAppStore
.getState()
.unifiedTabsByWorktree[worktreeId]?.find((item) => item.entityId === tab.id)
const anchorUnifiedTab = useAppStore
.getState()
.unifiedTabsByWorktree[worktreeId]?.find((item) => item.id === data.afterTabId)
if (
createdUnifiedTab &&
anchorUnifiedTab &&
createdUnifiedTab.groupId === anchorUnifiedTab.groupId
) {
const group = useAppStore
.getState()
.groupsByWorktree[worktreeId]?.find((item) => item.id === createdUnifiedTab.groupId)
const order = (group?.tabOrder ?? []).filter((id) => id !== createdUnifiedTab.id)
const anchorIndex = order.indexOf(anchorUnifiedTab.id)
order.splice(
anchorIndex === -1 ? order.length : anchorIndex + 1,
0,
createdUnifiedTab.id
)
useAppStore.getState().reorderUnifiedTabs(createdUnifiedTab.groupId, order, {
recordInteraction: false
})
.unifiedTabsByWorktree[worktreeId]?.find((item) => item.entityId === tab.id)?.id
if (createdUnifiedTabId) {
insertUnifiedTabAfterAnchor(worktreeId, createdUnifiedTabId, data.afterTabId)
}
}
if (shouldActivate) {
@@ -0,0 +1,22 @@
import { useAppStore } from '../store'
/** Move `tabId` to sit immediately after `anchorTabId`; no-op unless both share a group. */
export function insertUnifiedTabAfterAnchor(
worktreeId: string,
tabId: string,
anchorTabId: string
): void {
if (tabId === anchorTabId) {
return
}
const state = useAppStore.getState()
const group = (state.groupsByWorktree[worktreeId] ?? []).find(
(candidate) => candidate.tabOrder.includes(tabId) && candidate.tabOrder.includes(anchorTabId)
)
if (!group) {
return
}
const order = group.tabOrder.filter((id) => id !== tabId)
order.splice(order.indexOf(anchorTabId) + 1, 0, tabId)
state.reorderUnifiedTabs(group.id, order, { recordInteraction: false })
}
@@ -131,6 +131,71 @@ describe('createWebRuntimeSessionTerminal', () => {
])
})
it.each([
{
agent: 'codex' as const,
predecessor: 'web-terminal-host-tab-1',
afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1'
},
{
agent: undefined,
predecessor: 'web-terminal-host-tab-1',
afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1'
},
{ agent: undefined, predecessor: 'local-browser-tab', afterTabId: 'local-browser-tab' },
{
agent: undefined,
predecessor: 'web-terminal-host-tab-1%3A%3Aleaf-1',
afterTabId: 'web-terminal-host-tab-1%3A%3Aleaf-1'
}
])(
'settles after $afterTabId for $agent creation without activating it',
async ({ agent, predecessor, afterTabId }) => {
const successor = 'web-terminal-host-tab-3'
const created = 'web-terminal-host-tab-2'
const reorderUnifiedTabs = vi.fn()
mocks.getState.mockReturnValue({
...mocks.getState(),
unifiedTabsByWorktree: {
[WORKTREE_ID]: [predecessor, successor, created].map((id) => ({
id,
groupId: 'client-group'
}))
},
groupsByWorktree: {
[WORKTREE_ID]: [{ id: 'client-group', tabOrder: [predecessor, successor, created] }]
},
reorderUnifiedTabs,
moveUnifiedTabToGroup: mocks.moveUnifiedTabToGroup
})
const runtimeCall = vi.fn(async (request: { method: string }) => ({
id: request.method,
ok: true,
result:
request.method === 'session.tabs.createTerminal'
? { tab: { id: 'host-tab-2::leaf-2' }, publicationEpoch: 'epoch-1', snapshotVersion: 2 }
: makeSnapshot()
}))
vi.stubGlobal('window', { api: { runtimeEnvironments: { call: runtimeCall } } })
await expect(
createWebRuntimeSessionTerminal({
worktreeId: WORKTREE_ID,
afterTabId,
agent,
activate: false
})
).resolves.toEqual({ status: 'created' })
expect(reorderUnifiedTabs).toHaveBeenCalledExactlyOnceWith(
'client-group',
[predecessor, created, successor],
{ recordInteraction: false }
)
expect(mocks.moveUnifiedTabToGroup).not.toHaveBeenCalled()
}
)
it('can create a terminal without selecting the target worktree', async () => {
const setStateResults: unknown[] = []
mocks.setState.mockImplementation((updater: (state: unknown) => unknown) => {
@@ -248,20 +248,26 @@ export async function createWebRuntimeSessionTerminalResult(
// tab to THIS new terminal, instead of sticky-keeping the prior tab.
recordWebSessionFocusIntent(intentOwner, args.worktreeId, createdTabId, createdLeafId)
}
const placementTabId =
createdTabId && (args.targetGroupId || args.afterTabId) ? createdTabId : undefined
await refreshWebRuntimeSessionTabsSnapshot(environmentId, args.worktreeId, {
expectedEnvironmentPairingRevision: intentOwner.pairingRevision,
// Why: the publication can beat the RPC response; replay it once after caller intent exists.
acceptCurrentSnapshot:
Boolean(createdTabId) && (args.activate !== false || Boolean(args.targetGroupId)),
Boolean(createdTabId) && (args.activate !== false || Boolean(placementTabId)),
// Why: a placement record needs a post-create list; a deduped in-flight one can predate it.
...(args.targetGroupId && createdTabId ? { afterCurrentInFlight: true } : {})
...(placementTabId ? { afterCurrentInFlight: true } : {})
})
if (args.targetGroupId && createdTabId) {
if (placementTabId) {
await settleWebRuntimeTerminalPlacement(
environmentId,
args.worktreeId,
webTerminalPlacementParentTabId(createdTabId),
{ groupId: args.targetGroupId, activate: args.activate !== false }
webTerminalPlacementParentTabId(placementTabId),
{
groupId: args.targetGroupId,
afterTabId: args.afterTabId,
activate: args.activate !== false
}
)
}
return {
@@ -1,13 +1,31 @@
import { insertUnifiedTabAfterAnchor } from '../lib/unified-tab-anchor-insertion'
import { useAppStore } from '../store'
import { forgetWebSessionTerminalPlacement } from './web-session-terminal-placement'
import { toWebTerminalSurfaceTabId } from './web-terminal-surface-id'
import {
forgetWebSessionTerminalPlacement,
webTerminalPlacementParentTabId
} from './web-session-terminal-placement'
import {
isWebTerminalSurfaceTabId,
toHostSessionTabId,
toWebTerminalSurfaceTabId
} from './web-terminal-surface-id'
/** Snapshots key mirrored terminals by the parent tab, so an unknown `parent::leaf` anchor resolves to its parent. */
function anchorUnifiedTabId(worktreeId: string, afterTabId: string): string {
const known = (useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? []).some(
(tab) => tab.id === afterTabId
)
return known || !isWebTerminalSurfaceTabId(afterTabId)
? afterTabId
: toWebTerminalSurfaceTabId(webTerminalPlacementParentTabId(toHostSessionTabId(afterTabId)))
}
/** Settle the placement once the mirrored tab exists (bounded poll), then consume the record. */
export async function settleWebRuntimeTerminalPlacement(
environmentId: string,
worktreeId: string,
hostTabId: string,
placement: { groupId: string; activate: boolean }
placement: { groupId?: string; afterTabId?: string; activate: boolean }
): Promise<void> {
const unifiedTabId = toWebTerminalSurfaceTabId(hostTabId)
const findTab = () =>
@@ -20,18 +38,36 @@ export async function settleWebRuntimeTerminalPlacement(
await new Promise((resolve) => setTimeout(resolve, 250))
}
const tab = findTab()
if (!tab) {
return
}
const anchorId = placement.afterTabId
? anchorUnifiedTabId(worktreeId, placement.afterTabId)
: undefined
const state = useAppStore.getState()
const targetGroupExists = (state.groupsByWorktree[worktreeId] ?? []).some(
(group) => group.id === placement.groupId
)
if (tab && targetGroupExists && tab.groupId !== placement.groupId) {
const groups = state.groupsByWorktree[worktreeId] ?? []
// Why: the requested group can be closed while the mirrored tab is still in flight; the
// anchor's own group still expresses where the caller asked for this terminal.
const targetGroup =
groups.find((group) => group.id === placement.groupId) ??
(anchorId === undefined
? undefined
: groups.find((group) => group.tabOrder.includes(anchorId)))
if (!targetGroup) {
return
}
if (tab.groupId !== targetGroup.id) {
// Why: a snapshot can adopt the tab before the record exists (the publication races the
// RPC response); repair through the same client-owned move a user drag takes.
state.moveUnifiedTabToGroup(unifiedTabId, placement.groupId, {
state.moveUnifiedTabToGroup(unifiedTabId, targetGroup.id, {
activate: placement.activate,
recordInteraction: false
})
}
if (anchorId) {
// The create caller owns this insertion; subsequent host snapshots preserve client order.
insertUnifiedTabAfterAnchor(worktreeId, unifiedTabId, anchorId)
}
} finally {
forgetWebSessionTerminalPlacement({ environmentId, worktreeId, hostTabId })
}