fix(session-tabs): mark re-sent held closes as replays for hosts that accept it (#31951)

A held close re-sent after reconnect went out as reason 'user', so a host's
trace could not tell automatic replays from clicks (#31449). Hosts now accept
an optional 'replay' reason, advertised by session-tabs.close-replay-reason.v1,
and treat it as the user close it re-sends. Clients mark replays only for hosts
that advertise it; older hosts keep receiving 'user'.
This commit is contained in:
Neil
2026-10-22 03:20:15 -07:00
committed by GitHub
parent d8aeb1f984
commit aeb96c580a
6 changed files with 73 additions and 7 deletions
@@ -56,7 +56,7 @@ export const SESSION_TAB_CLOSE_METHODS = [
{
reason: 'user',
// Why explicit reason only: a reasonless legacy close never carried a confirmation.
...(params.force && params.reason === 'user' ? { force: true } : {}),
...(params.force && params.reason !== undefined ? { force: true } : {}),
...(context.pairedDeviceId ? { clientNavigationId: context.pairedDeviceId } : {})
}
)
@@ -124,6 +124,13 @@ describe.each([false, true])('ORCA_LAYOUT_OWNER_IS_MAIN=%s', (layoutOwnerIsMain)
}
})
it('closes a pinned tab on a replayed confirmed close, which a newer client marks as replay', async () => {
const { f, close } = await pinnedServerTab(layoutOwnerIsMain)
const reply = await close({ reason: 'replay', force: true })
expect(reply.ok).toBe(true)
expect(f.hasTab()).toBe(false)
})
it('ignores force on a reasonless legacy close, which never carried a confirmation', async () => {
const { f, close } = await pinnedServerTab(layoutOwnerIsMain)
const reply = await close({ reason: undefined, force: true }, [])
@@ -19,7 +19,10 @@ import {
import type * as RuntimeRpcClient from './runtime-rpc-client'
import type * as WebRuntimeSessionEnvironment from './web-runtime-session-environment'
import { ENVIRONMENT_ID, WORKTREE_ID, makeSnapshot } from './web-runtime-session-test-harness'
import { SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY } from '../../../shared/session-tabs-runtime-capabilities'
import {
SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY,
SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY
} from '../../../shared/session-tabs-runtime-capabilities'
import { stubRuntimeEnvironmentBridge } from './runtime-environment-bridge-fixture'
const mocks = vi.hoisted(() => ({
@@ -234,7 +237,7 @@ describe('a confirmed close of a pinned server tab', () => {
expect(closeCalls(runtimeCall)).toEqual([
expect.objectContaining({ force: true }),
expect.objectContaining({ tabId: HOST_TAB, reason: 'user', force: true })
expect.objectContaining({ tabId: HOST_TAB, reason: 'replay', force: true })
])
})
@@ -268,7 +271,7 @@ describe('a confirmed close of a pinned server tab', () => {
mocks.isWebRuntimeSessionActive.mockReturnValue(true)
await replayWebSessionTabCloses(ENVIRONMENT_ID, due)
expect(closeCalls(runtimeCall)).toEqual([
expect.objectContaining({ tabId: HOST_TAB, reason: 'user', force: true })
expect.objectContaining({ tabId: HOST_TAB, reason: 'replay', force: true })
])
})
@@ -287,4 +290,37 @@ describe('a confirmed close of a pinned server tab', () => {
takeDueWebSessionCloseReplays(INTENTS, ENVIRONMENT_ID, WORKTREE_ID, new Set([HOST_TAB]))
).toEqual([expect.objectContaining({ force: true })])
})
it.each([
[true, 'replay'],
[false, 'user']
])(
'marks a re-sent held close as a replay only for a host that accepts it (%s)',
async (hostAcceptsReplay, reason) => {
mocks.runtimeEnvironmentSupportsCapability.mockImplementation(
async (_environmentId: string, capability: string) =>
capability !== SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY || hostAcceptsReplay
)
const runtimeCall = vi
.fn()
.mockRejectedValueOnce(new Error('socket hang up'))
.mockResolvedValueOnce({ id: 'c', ok: true, result: { closed: true } })
.mockResolvedValue({ id: 'list', ok: true, result: makeSnapshot() })
stubRuntimeEnvironmentBridge({ call: runtimeCall })
await expect(closeTab()).resolves.toBe('failed')
const due = takeDueWebSessionCloseReplays(
INTENTS,
ENVIRONMENT_ID,
WORKTREE_ID,
new Set([HOST_TAB])
)
await replayWebSessionTabCloses(ENVIRONMENT_ID, due)
expect(closeCalls(runtimeCall)).toEqual([
expect.objectContaining({ reason: 'user' }),
expect.objectContaining({ tabId: HOST_TAB, reason })
])
}
)
})
@@ -34,7 +34,10 @@ import {
} from './web-runtime-session-environment'
import { refreshWebRuntimeSessionTabsSnapshot } from './web-runtime-session-snapshot'
import { runtimeEnvironmentSupportsCapability } from './runtime-rpc-client'
import { SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY } from '../../../shared/session-tabs-runtime-capabilities'
import {
SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY,
SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY
} from '../../../shared/session-tabs-runtime-capabilities'
export async function activateWebRuntimeSessionTab(args: {
worktreeId: string
@@ -89,6 +92,8 @@ export async function closeWebRuntimeSessionTab(args: {
expectedPairingRevision?: number
/** The user confirmed closing a pinned tab; sent only to hosts that honour it. */
force?: boolean
/** A held close re-sent, not a fresh click; marked so only to hosts that accept the mark. */
replay?: true
}): Promise<WebRuntimeSessionTabCloseOutcome> {
return callWebRuntimeSessionTabMethod('session.tabs.close', args)
}
@@ -109,6 +114,7 @@ export async function replayWebSessionTabCloses(
tabId: replay.hostTabIds[0],
environmentId,
reason: 'user',
replay: true,
...(replay.owner.pairingRevision !== undefined
? { expectedPairingRevision: replay.owner.pairingRevision }
: {}),
@@ -155,6 +161,7 @@ async function callWebRuntimeSessionTabMethod(
expectedCurrentLocalTabId?: string | null
expectedPairingRevision?: number
force?: boolean
replay?: true
}
): Promise<WebRuntimeSessionTabCloseOutcome> {
const environmentId = resolveTabEnvironmentId(args.environmentId)
@@ -232,6 +239,15 @@ async function callWebRuntimeSessionTabMethod(
environmentId,
SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY
))
// Only a user close carries `replay`. The mark is diagnostic, so an unanswered probe sends 'user'.
const closeReason =
args.replay === true &&
(await runtimeEnvironmentSupportsCapability(
environmentId,
SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY
).catch(() => false))
? 'replay'
: args.reason
const response = await callEnvironment({
// Why: old hosts cannot route this additive method, so a generation
// cutover fails closed before their destructive legacy close handler.
@@ -257,7 +273,7 @@ async function callWebRuntimeSessionTabMethod(
terminal: args.terminalHandle
}
: isClose
? { reason: args.reason, ...(force ? { force: true } : {}) }
? { reason: closeReason, ...(force ? { force: true } : {}) }
: {})
},
timeoutMs: WEB_SESSION_TAB_RPC_TIMEOUT_MS
@@ -35,7 +35,9 @@ export const ActivateTab = WorktreeTabSelector.extend({
export const CloseTab = ActivateTab.extend({
// Why: optional preserves authenticated legacy user closes; lifecycle intent
// uses the additive evidence-bearing method instead.
reason: z.literal('user').optional(),
// 'replay' is a held user close re-sent after reconnect; only sent to hosts advertising
// SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY.
reason: z.enum(['user', 'replay']).optional(),
// Why: the user confirmed closing a pinned tab. Older hosts strip it and keep refusing, so
// clients send it only to hosts advertising SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY.
force: z.literal(true).optional()
@@ -20,6 +20,10 @@ export const SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY = 'session-tabs.close-i
// Why: an older host drops session.tabs.close's `force` and refuses a pinned tab, so a client sends
// a confirmed pinned close's force only when advertised. Not behind a build switch: both builds honour it.
export const SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY = 'session-tabs.close-force.v1' as const
// Why: an older host's close schema accepts only reason 'user', so a client re-sending a held close
// marks it 'replay' only where advertised; elsewhere it stays 'user', exactly as before.
export const SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY =
'session-tabs.close-replay-reason.v1' as const
export const SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY =
'session-tabs.authoritative-inventory.v1' as const
// Why: a client advertising this retains every terminal retirement proof it receives until the
@@ -39,6 +43,7 @@ export const SESSION_TABS_RUNTIME_CAPABILITIES = [
QODER_OWNED_TERMINAL_CREATE_CAPABILITY,
SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY,
SESSION_TAB_CLOSE_FORCE_RUNTIME_CAPABILITY,
SESSION_TAB_CLOSE_REPLAY_REASON_RUNTIME_CAPABILITY,
SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY,
SESSION_TABS_SPLIT_GROUP_PLACEMENT_RUNTIME_CAPABILITY,
SESSION_TABS_CHAT_FORK_CLI_RUNTIME_CAPABILITY,