fix(desktop): compare preview/tour gate on one session identity class (#122062)
drive_preview (and tour) were refused with "The in-app browser only takes actions in the session the user is looking at." for a compressed-but-still- selected conversation: the backend stamps preview.act.request with the RUNTIME session id, which auto-compression rotates mid-conversation, while the pane keeps the durable/lineage id it navigated to — so `sessionId === activeSessionId` failed for the rest of the conversation even though desktop_preview open/read kept working. Both sides of the check now resolve to the same identity class before comparing: the request's runtime id maps to its stored id through the message stream's session state cache (rotation-aware), unknown ids pass through unchanged (they may already be stored ids), and a final lineage match lets a compression-rotated tip and its root read as one conversation — the same one-row lineage test session.info matching uses. Branch siblings that only share a root stay distinct, and a background tile session is still refused by the gate. The same matcher backs windowHostsSession, so the window hosting the conversation claims the request instead of leaving it unanswered when the ids differ only by rotation. Fixes #122062
This commit is contained in:
committed by
Austin Pickett
parent
83597d9b71
commit
d4720b4ea8
@@ -7,12 +7,15 @@ import { $sessionTiles } from '@/store/session-states'
|
||||
import { $toursEnabled } from '@/store/tours'
|
||||
import type { SessionInfo } from '@/types/hermes'
|
||||
|
||||
import { handleServerRequest, previewSessionRoute } from './server-requests'
|
||||
import { handleServerRequest, previewSessionRoute, requestNamesActiveSession } from './server-requests'
|
||||
import type { ServerRequestContext } from './server-requests'
|
||||
|
||||
vi.mock('@/lib/tour', () => ({ runTour: vi.fn(async () => ({ ok: true })) }))
|
||||
|
||||
const deps = {
|
||||
activeSessionIdRef: { current: null },
|
||||
sessionInterrupted: () => false,
|
||||
sessionStateByRuntimeIdRef: { current: new Map() },
|
||||
updateSessionState: (_sessionId, update) => update(createClientSessionState('stored-session')),
|
||||
upsertToolCall: () => undefined
|
||||
} as ServerRequestContext['deps']
|
||||
@@ -159,6 +162,93 @@ describe('tour request routing', () => {
|
||||
|
||||
expect(JSON.parse(respond.mock.calls[0][0].value)).toMatchObject({ success: false })
|
||||
})
|
||||
|
||||
it('runs the tour for a compression-rotated session driving the conversation on screen', async () => {
|
||||
// #122062: auto-compression rotates the runtime session id (and the stored
|
||||
// tip) while the pane keeps the durable id it navigated to. The request is
|
||||
// stamped with the rotated runtime id — the gate must still see that it
|
||||
// names the conversation on screen instead of refusing it.
|
||||
setSessions([{ id: 'stored-tip', _lineage_root_id: 'stored-root', _lineage_ids: ['stored-root'] } as SessionInfo])
|
||||
deps.sessionStateByRuntimeIdRef.current.set('runtime-2', createClientSessionState('stored-tip'))
|
||||
|
||||
try {
|
||||
const { handled, respond } = deliver('tour', { action: 'discover', session_id: 'runtime-2' }, 'stored-root')
|
||||
|
||||
expect(handled).toBe(true)
|
||||
await vi.waitFor(() => expect(respond).toHaveBeenCalledTimes(1))
|
||||
expect(JSON.parse(respond.mock.calls[0][0].value)).toMatchObject({ ok: true })
|
||||
} finally {
|
||||
deps.sessionStateByRuntimeIdRef.current.clear()
|
||||
setSessions([])
|
||||
}
|
||||
})
|
||||
|
||||
it('still refuses a scoped request naming another conversation', () => {
|
||||
// The window hosts the request's session as a tile (so the request is
|
||||
// routed here rather than left unanswered), but the pane shows a different
|
||||
// conversation — the gate must still refuse.
|
||||
setSessions([{ id: 'stored-tip', _lineage_root_id: 'stored-root' } as SessionInfo])
|
||||
deps.sessionStateByRuntimeIdRef.current.set('runtime-2', createClientSessionState('stored-tip'))
|
||||
$sessionTiles.set([{ runtimeId: 'runtime-2', storedSessionId: 'stored-tip' } as never])
|
||||
|
||||
try {
|
||||
const { respond } = deliver('tour', { action: 'discover', session_id: 'runtime-2' }, 'other-root')
|
||||
|
||||
expect(JSON.parse(respond.mock.calls[0][0].value)).toMatchObject({
|
||||
error: expect.stringContaining('the session the user is looking at')
|
||||
})
|
||||
} finally {
|
||||
$sessionTiles.set([])
|
||||
deps.sessionStateByRuntimeIdRef.current.clear()
|
||||
setSessions([])
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('session identity matching (runtime vs stored ids)', () => {
|
||||
afterEach(() => {
|
||||
setSessions([])
|
||||
})
|
||||
|
||||
it('matches a compression-rotated request id to the conversation the pane holds', () => {
|
||||
setSessions([{ id: 'stored-tip', _lineage_root_id: 'stored-root', _lineage_ids: ['stored-root'] } as SessionInfo])
|
||||
const bindings: Record<string, string> = { 'runtime-1': 'stored-root', 'runtime-2': 'stored-tip' }
|
||||
const storedIdForRuntimeId = (id: string) => bindings[id]
|
||||
|
||||
// The pane holds the durable/lineage id the user navigated to.
|
||||
expect(
|
||||
requestNamesActiveSession({ activeSessionId: 'stored-root', sessionId: 'runtime-2', storedIdForRuntimeId })
|
||||
).toBe(true)
|
||||
// The pane holds a stale runtime id while the request carries the rotated one.
|
||||
expect(
|
||||
requestNamesActiveSession({ activeSessionId: 'runtime-1', sessionId: 'runtime-2', storedIdForRuntimeId })
|
||||
).toBe(true)
|
||||
// Plain equality still short-circuits.
|
||||
expect(requestNamesActiveSession({ activeSessionId: 'runtime-2', sessionId: 'runtime-2' })).toBe(true)
|
||||
// A foreign conversation is still not the active one.
|
||||
expect(
|
||||
requestNamesActiveSession({
|
||||
activeSessionId: 'stored-root',
|
||||
sessionId: 'other-runtime',
|
||||
storedIdForRuntimeId: (id: string) => (id === 'other-runtime' ? 'other-stored' : undefined)
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('does not merge branch siblings that only share a lineage root', () => {
|
||||
setSessions([
|
||||
{ id: 'branch-a', _lineage_root_id: 'shared-root' } as SessionInfo,
|
||||
{ id: 'branch-b', _lineage_root_id: 'shared-root' } as SessionInfo
|
||||
])
|
||||
|
||||
expect(requestNamesActiveSession({ activeSessionId: 'branch-a', sessionId: 'branch-b' })).toBe(false)
|
||||
expect(requestNamesActiveSession({ activeSessionId: 'shared-root', sessionId: 'branch-b' })).toBe(true)
|
||||
})
|
||||
|
||||
it('stays false when either side is unscoped', () => {
|
||||
expect(requestNamesActiveSession({ activeSessionId: null, sessionId: 'runtime-2' })).toBe(false)
|
||||
expect(requestNamesActiveSession({ activeSessionId: 'stored-root', sessionId: '' })).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
// #75587: a blocking-input request still in flight when the session's runtime is
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
setVaultUnlockRequest
|
||||
} from '@/store/prompts'
|
||||
import { rememberServerRequest } from '@/store/server-requests'
|
||||
import { $sessions, sessionMatchesStoredId } from '@/store/session'
|
||||
import { $sessionTiles } from '@/store/session-states'
|
||||
import { requestScrollToBottom } from '@/store/thread-scroll'
|
||||
import { $toursEnabled } from '@/store/tours'
|
||||
@@ -53,7 +54,10 @@ const answerValue = (request: ScopedServerRequest, result: unknown) =>
|
||||
request.respond({ value: result ? JSON.stringify(result) : '' })
|
||||
|
||||
export interface ServerRequestContext {
|
||||
deps: Pick<GatewayEventDeps, 'activeSessionIdRef' | 'sessionInterrupted' | 'updateSessionState' | 'upsertToolCall'>
|
||||
deps: Pick<
|
||||
GatewayEventDeps,
|
||||
'activeSessionIdRef' | 'sessionInterrupted' | 'sessionStateByRuntimeIdRef' | 'updateSessionState' | 'upsertToolCall'
|
||||
>
|
||||
request: ScopedServerRequest
|
||||
/** The session the request names ('' when unscoped). */
|
||||
sessionId: string
|
||||
@@ -74,9 +78,57 @@ type PreviewSessionRoute = 'ignore' | 'retry' | 'run'
|
||||
*/
|
||||
const WINDOW_OWNED_REQUESTS = new Set(['preview.act', 'preview.read', 'terminal.read', 'window.read', 'tour'])
|
||||
|
||||
/**
|
||||
* Whether a request's `session_id` names the same conversation as the pane's
|
||||
* active session. The two sides are not always the same identity class: the
|
||||
* gateway stamps requests with the RUNTIME session id — which auto-compression
|
||||
* rotates mid-conversation — while the pane may hold the durable/lineage id it
|
||||
* navigated to, so plain equality refuses the very session on screen (#122062).
|
||||
* Compare through the stored id each side resolves to (an unknown id passes
|
||||
* through unchanged: it may already be a stored id), then through the lineage,
|
||||
* so a compression-rotated tip and its root still read as one conversation.
|
||||
* The lineage leg requires ONE session row to answer to both ids — branch
|
||||
* siblings share a root but are distinct conversations.
|
||||
*/
|
||||
export function requestNamesActiveSession({
|
||||
activeSessionId,
|
||||
sessionId,
|
||||
storedIdForRuntimeId = () => undefined
|
||||
}: {
|
||||
activeSessionId: null | string
|
||||
sessionId: string
|
||||
storedIdForRuntimeId?: (runtimeId: string) => string | undefined
|
||||
}): boolean {
|
||||
if (!sessionId || !activeSessionId) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (sessionId === activeSessionId) {
|
||||
return true
|
||||
}
|
||||
|
||||
const requestStoredId = storedIdForRuntimeId(sessionId) ?? sessionId
|
||||
const activeStoredId = storedIdForRuntimeId(activeSessionId) ?? activeSessionId
|
||||
|
||||
if (requestStoredId === activeStoredId) {
|
||||
return true
|
||||
}
|
||||
|
||||
return $sessions
|
||||
.get()
|
||||
.some(session => sessionMatchesStoredId(session, requestStoredId) && sessionMatchesStoredId(session, activeStoredId))
|
||||
}
|
||||
|
||||
/** This window hosts the session: it is the primary view or an open session tile. */
|
||||
export function windowHostsSession(sessionId: string, activeSessionId: null | string): boolean {
|
||||
return sessionId === activeSessionId || $sessionTiles.get().some(tile => tile.runtimeId === sessionId)
|
||||
export function windowHostsSession(
|
||||
sessionId: string,
|
||||
activeSessionId: null | string,
|
||||
storedIdForRuntimeId?: (runtimeId: string) => string | undefined
|
||||
): boolean {
|
||||
return (
|
||||
requestNamesActiveSession({ activeSessionId, sessionId, storedIdForRuntimeId }) ||
|
||||
$sessionTiles.get().some(tile => tile.runtimeId === sessionId)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -90,13 +142,15 @@ export function windowHostsSession(sessionId: string, activeSessionId: null | st
|
||||
export function previewSessionRoute({
|
||||
activeSessionId,
|
||||
replayed,
|
||||
sessionId
|
||||
sessionId,
|
||||
storedIdForRuntimeId
|
||||
}: {
|
||||
activeSessionId: null | string
|
||||
replayed: boolean | undefined
|
||||
sessionId: string
|
||||
storedIdForRuntimeId?: (runtimeId: string) => string | undefined
|
||||
}): PreviewSessionRoute {
|
||||
if (!sessionId || windowHostsSession(sessionId, activeSessionId)) {
|
||||
if (!sessionId || windowHostsSession(sessionId, activeSessionId, storedIdForRuntimeId)) {
|
||||
return 'run'
|
||||
}
|
||||
|
||||
@@ -532,8 +586,15 @@ export function handleServerRequest(
|
||||
|
||||
const sessionId = str(request.params.session_id)
|
||||
|
||||
// Resolve a request's runtime session id to its stored id through the state
|
||||
// cache the message stream maintains (rotation-aware: auto-compression
|
||||
// re-stamps `storedSessionId` on the same runtime entry). Unknown ids fall
|
||||
// through unchanged — they may already be stored ids.
|
||||
const storedIdForRuntimeId = (runtimeId: string) =>
|
||||
deps.sessionStateByRuntimeIdRef.current.get(runtimeId)?.storedSessionId ?? undefined
|
||||
|
||||
if (WINDOW_OWNED_REQUESTS.has(request.method)) {
|
||||
const route = previewSessionRoute({ activeSessionId, replayed: request.replayed, sessionId })
|
||||
const route = previewSessionRoute({ activeSessionId, replayed: request.replayed, sessionId, storedIdForRuntimeId })
|
||||
|
||||
if (route === 'ignore') {
|
||||
return true
|
||||
@@ -545,8 +606,12 @@ export function handleServerRequest(
|
||||
// turn. A second miss deliberately stays silent for another window.
|
||||
setTimeout(() => {
|
||||
if (
|
||||
previewSessionRoute({ activeSessionId: deps.activeSessionIdRef.current, replayed: false, sessionId }) ===
|
||||
'run'
|
||||
previewSessionRoute({
|
||||
activeSessionId: deps.activeSessionIdRef.current,
|
||||
replayed: false,
|
||||
sessionId,
|
||||
storedIdForRuntimeId
|
||||
}) === 'run'
|
||||
) {
|
||||
handler({ deps, request, sessionId, isActiveSession: true })
|
||||
}
|
||||
@@ -556,7 +621,12 @@ export function handleServerRequest(
|
||||
}
|
||||
}
|
||||
|
||||
handler({ deps, request, sessionId, isActiveSession: Boolean(sessionId) && sessionId === activeSessionId })
|
||||
handler({
|
||||
deps,
|
||||
request,
|
||||
sessionId,
|
||||
isActiveSession: requestNamesActiveSession({ activeSessionId, sessionId, storedIdForRuntimeId })
|
||||
})
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -1106,10 +1106,10 @@ export function useMessageStream({
|
||||
(request: ScopedServerRequest): boolean =>
|
||||
dispatchServerRequest(
|
||||
request,
|
||||
{ activeSessionIdRef, sessionInterrupted, updateSessionState, upsertToolCall },
|
||||
{ activeSessionIdRef, sessionInterrupted, sessionStateByRuntimeIdRef, updateSessionState, upsertToolCall },
|
||||
activeSessionIdRef.current
|
||||
),
|
||||
[activeSessionIdRef, sessionInterrupted, updateSessionState, upsertToolCall]
|
||||
[activeSessionIdRef, sessionInterrupted, sessionStateByRuntimeIdRef, updateSessionState, upsertToolCall]
|
||||
)
|
||||
|
||||
return {
|
||||
|
||||
@@ -47,6 +47,7 @@ describe('PromptOverlays', () => {
|
||||
const deps: ServerRequestContext['deps'] = {
|
||||
activeSessionIdRef: { current: 's1' },
|
||||
sessionInterrupted: () => false,
|
||||
sessionStateByRuntimeIdRef: { current: new Map() },
|
||||
updateSessionState: (_sid, update) => update(createClientSessionState('s1')),
|
||||
upsertToolCall: () => undefined
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user