fix(mobile): stop a created tab from jumping when the host snapshot lands

Creating a tab from the mobile session strip painted the new tab at the end
of the strip and then visibly jumped it to a different slot a beat later.

The client asked the host to insert the tab after the active tab, but then
predicted a different placement for its own optimistic paint:

    afterTabId: activeSessionTabId ?? undefined   // host: splice(insertAfter + 1)
    ...
    return [...prev, { ...created, isActive: true }]   // client: append

Two independent placements that disagree, so the optimistic frame is wrong by
construction and the tab snaps to its real slot on the next published snapshot.

The disagreement dates to 57a70d2ac0 ("Fix mobile session tab authority"),
which introduced afterTabId and left the append in place. Before that the
client used terminal.create with no anchor, so both sides appended and agreed.

Rather than teach the client to re-derive the host's rule, both sides now call
one shared placeCreatedSessionTab, and the client captures a single afterTabId
for the request and the paint so they cannot drift apart again.

The host change is a pure refactor onto the shared helper; the 1260-test
runtime characterization suite is unchanged. The mobile route-parity hash pin
moves once because handleCreateTerminal's body changed - it is the only one of
the 12 extracted functions that differs.
This commit is contained in:
Merge Sim
2026-09-11 00:42:34 -07:00
parent e187c82678
commit d1502d1d53
6 changed files with 161 additions and 13 deletions
@@ -69,8 +69,11 @@ const HEAD_CALLBACK_IDENTITY_SHA256 =
const HEAD_CALLBACK_BODY_SHA256 = 'af7f3c62954250d4be7ee432ecd10dc2689792aad8230fed2d1d68bbc892d776'
const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13'
const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581'
// Moved once for the create-tab placement fix: handleCreateTerminal now paints the created tab
// after the anchor it sent the host instead of appending. Only that body changed; the other 11
// extracted functions are byte-identical.
const HEAD_NESTED_FUNCTION_SHA256 =
'fde6679349ab2b8c30c7e627841ff99bd1dd24441ee95323d0aa70230422ae24'
'90b88dfc168c664d87b240af14f6c4e19fa506a3d1005fcd9ef29df1befc810c'
const HEAD_NATIVE_REGISTRATION_SHA256 =
'cab85e4e4a3f43289ba93ddea9ccce57aea83e0bf14fd1620a965aad0c1cb49e'
const HEAD_NATIVE_REMOVAL_SHA256 =
@@ -262,3 +262,78 @@ describe('mobile + Codex tab creation routing', () => {
}
)
})
describe('optimistic placement of a created tab', () => {
let renderer: ReactTestRenderer | undefined
afterEach(() => renderer?.unmount())
async function createLegacyTerminal(scope: ReturnType<typeof createScope>) {
let actions: ReturnType<typeof useMobileSessionTerminalCreateActions> | undefined
function Harness() {
actions = useMobileSessionTerminalCreateActions(scope as never)
return null
}
await act(async () => {
renderer = create(createElement(Harness))
})
await act(async () => {
await actions?.handleCreateTerminal()
})
}
function tabIdsAfterCreate(
scope: ReturnType<typeof createScope>,
prior: { id: string }[]
): string[] {
const updater = (scope.setSessionTabs as ReturnType<typeof vi.fn>).mock.calls.at(-1)?.[0] as (
prev: { id: string }[]
) => { id: string }[]
return updater(prior).map((tab) => tab.id)
}
it('paints the created tab after the anchor it asked the host for, not at the end', async () => {
const scope = createScope(clientReturning(terminalCreateResponse()))
await createLegacyTerminal(scope)
expect(scope.setSessionTabs).toHaveBeenCalled()
// The request anchored on the active tab, so the paint must land in the same slot the host
// splices into; appending here is what made the tab jump on the next snapshot.
expect(tabIdsAfterCreate(scope, [{ id: 'existing-tab' }, { id: 'trailing-tab' }])).toEqual([
'existing-tab',
'terminal-tab-1',
'trailing-tab'
])
})
it('sends the same anchor it paints with', async () => {
const scope = createScope(clientReturning(terminalCreateResponse()))
await createLegacyTerminal(scope)
expect(scope.client.sendRequest).toHaveBeenCalledWith(
'session.tabs.createTerminal',
expect.objectContaining({ afterTabId: 'existing-tab' })
)
})
it('appends when the anchor is not in the client list, matching the host fallback', async () => {
const scope = createScope(clientReturning(terminalCreateResponse()))
await createLegacyTerminal(scope)
expect(tabIdsAfterCreate(scope, [{ id: 'unrelated-tab' }])).toEqual([
'unrelated-tab',
'terminal-tab-1'
])
})
it('leaves the list alone when the host snapshot already placed the tab', async () => {
const scope = createScope(clientReturning(terminalCreateResponse()))
await createLegacyTerminal(scope)
const prior = [{ id: 'existing-tab' }, { id: 'terminal-tab-1' }, { id: 'trailing-tab' }]
expect(tabIdsAfterCreate(scope, prior)).toEqual([
'existing-tab',
'terminal-tab-1',
'trailing-tab'
])
})
})
@@ -12,6 +12,7 @@ import type { Terminal, TerminalCreateResult } from './mobile-session-route-type
import type { MobileSessionAttachmentsModel } from './use-mobile-session-attachments'
import { isAgentSessionHandleProvider } from '../../../src/shared/agent-session-provider-handle'
import { createMobileStructuredAgentSession } from './mobile-structured-agent-session-launch'
import { placeCreatedSessionTab } from '../../../src/shared/session-tab-placement'
export function useMobileSessionTerminalCreateActions(scope: MobileSessionAttachmentsModel) {
const {
@@ -93,9 +94,12 @@ export function useMobileSessionTerminalCreateActions(scope: MobileSessionAttach
return
}
}
// Why: one anchor for both the request and the optimistic paint below; when they disagreed the
// new tab painted at the end and jumped to its real slot on the next host snapshot.
const afterTabId = activeSessionTabId ?? undefined
const response = await client.sendRequest('session.tabs.createTerminal', {
worktree: `id:${worktreeId}`,
afterTabId: activeSessionTabId ?? undefined,
afterTabId,
clientMutationId,
...(options?.startupCommand ? { command: options.startupCommand } : {}),
...(options?.startupCommandDelivery
@@ -123,7 +127,7 @@ export function useMobileSessionTerminalCreateActions(scope: MobileSessionAttach
if (prev.some((tab) => tab.id === created.id)) {
return prev
}
return [...prev, { ...created, isActive: true }]
return placeCreatedSessionTab(prev, { ...created, isActive: true }, afterTabId)
})
if (typeof created.terminal === 'string') {
const createdHandle = created.terminal
@@ -10,6 +10,7 @@ import type {
} from '../../shared/runtime-types'
import { randomUUID } from 'node:crypto'
import { parsePaneKey } from '../../shared/stable-pane-id'
import { placeCreatedSessionTab } from '../../shared/session-tab-placement'
import {
buildHeadlessMobileSessionTabGroups,
buildMaterializedHeadlessParentLayout,
@@ -103,21 +104,17 @@ export class OrcaRuntimeWithCreateRuntimeOwnedMobileSessionTerminal extends Orca
parentLayout,
isActive: activate
}
const tabs = (existing?.tabs ?? [])
.filter((candidate) => candidate.id !== tab.id)
.map((candidate) => ({
const tabs = placeCreatedSessionTab(
(existing?.tabs ?? []).map((candidate) => ({
...candidate,
...(candidate.type === 'terminal' && candidate.parentTabId === parentTabId
? { parentLayout }
: {}),
isActive: activate ? false : candidate.isActive
}))
const insertAfter = afterTabId ? tabs.findIndex((candidate) => candidate.id === afterTabId) : -1
if (insertAfter >= 0) {
tabs.splice(insertAfter + 1, 0, tab)
} else {
tabs.push(tab)
}
})),
tab,
afterTabId
)
const next: RuntimeMobileSessionTabsSnapshot = {
worktree: worktreeId,
// Why: a fresh epoch retires the current publisher, so clients drop its later tab updates.
+47
View File
@@ -0,0 +1,47 @@
import { describe, expect, it } from 'vitest'
import { placeCreatedSessionTab } from './session-tab-placement'
const created = { id: 'new' }
describe('placeCreatedSessionTab', () => {
it('inserts directly after the anchor', () => {
expect(
placeCreatedSessionTab([{ id: 'a' }, { id: 'b' }, { id: 'c' }], created, 'a').map((t) => t.id)
).toEqual(['a', 'new', 'b', 'c'])
})
it('appends when the anchor is the last tab', () => {
expect(
placeCreatedSessionTab([{ id: 'a' }, { id: 'b' }], created, 'b').map((t) => t.id)
).toEqual(['a', 'b', 'new'])
})
it('appends when the anchor is absent', () => {
expect(
placeCreatedSessionTab([{ id: 'a' }, { id: 'b' }], created, 'missing').map((t) => t.id)
).toEqual(['a', 'b', 'new'])
})
it('appends when no anchor is given', () => {
for (const anchor of [undefined, null, '']) {
expect(placeCreatedSessionTab([{ id: 'a' }], created, anchor).map((t) => t.id)).toEqual([
'a',
'new'
])
}
})
it('re-places a tab that is already in the list instead of duplicating it', () => {
expect(
placeCreatedSessionTab([{ id: 'a' }, { id: 'new' }, { id: 'b' }], created, 'b').map(
(t) => t.id
)
).toEqual(['a', 'b', 'new'])
})
it('does not mutate the input list', () => {
const tabs = [{ id: 'a' }, { id: 'b' }]
placeCreatedSessionTab(tabs, created, 'a')
expect(tabs.map((t) => t.id)).toEqual(['a', 'b'])
})
})
+22
View File
@@ -0,0 +1,22 @@
/**
* Where a newly created session tab lands in a tab list.
*
* Shared because a client that optimistically paints a created tab and the host that publishes
* the authoritative snapshot must agree. When they disagreed — the client appending while the
* host spliced after `afterTabId` — the new tab painted at the end and then visibly jumped to its
* real slot as soon as the host frame landed.
*/
export function placeCreatedSessionTab<T extends { id: string }>(
tabs: readonly T[],
created: T,
afterTabId: string | null | undefined
): T[] {
const next = tabs.filter((tab) => tab.id !== created.id)
const anchor = afterTabId ? next.findIndex((tab) => tab.id === afterTabId) : -1
if (anchor < 0) {
next.push(created)
return next
}
next.splice(anchor + 1, 0, created)
return next
}