mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(mobile): keep polling session tabs while an active terminal is pending-handle (STA-4256) (#14623)
A terminal tab published as `status: 'pending-handle'` renders the session screen's spinner. Leaving it requires a snapshot that carries the materialized handle, but a certified-live tabs stream parks `poll()` unless `hasRecoveryNeed()` says otherwise — and that predicate never considered a pending terminal. A host that mints the handle without republishing therefore stranded the pane on its spinner forever: measured live, zero further `session.tabs.list` calls over 90s while `terminal.list` kept firing every 2s. Mirrors the existing native-chat recovery-need pattern. Client-only; no wire change.
This commit is contained in:
@@ -207,6 +207,7 @@ import {
|
||||
confirmsMirroredTabSelection,
|
||||
type AppliedSnapshotMarker
|
||||
} from '../../../../src/session/session-tab-snapshot-gate'
|
||||
import { hasPendingTerminalHandleRecoveryNeed } from '../../../../src/session/pending-terminal-handle-recovery'
|
||||
import {
|
||||
createInitialSessionAutoCreateState,
|
||||
useInitialSessionTerminalAutoCreate,
|
||||
@@ -2283,6 +2284,10 @@ export default function SessionScreen() {
|
||||
() =>
|
||||
closedTabTombstonesRef.current.size > 0 ||
|
||||
pendingBrowserFocusPageIdRef.current !== null ||
|
||||
// Why: a pending-handle terminal only turns ready in a fresh snapshot. A live
|
||||
// stream otherwise parks the poll, and a host that mints the handle without
|
||||
// republishing strands the pane on its spinner forever (STA-4256).
|
||||
hasPendingTerminalHandleRecoveryNeed(sessionTabsRef.current, activeSessionTabIdRef.current) ||
|
||||
// Why: a chat-covered handle that ran out of rearms and left `terminal.list`
|
||||
// was reminted by a desktop graph reload. Only a fresh tab snapshot carries
|
||||
// the replacement handle, so force one instead of holding the composer locked.
|
||||
|
||||
@@ -186,4 +186,17 @@ describe('mobile session startup', () => {
|
||||
newTabActions.indexOf("label: 'Markdown Note'")
|
||||
)
|
||||
})
|
||||
|
||||
it('counts a pending-handle active terminal as a tabs recovery need (STA-4256)', () => {
|
||||
const recoveryNeed = sliceBetween(
|
||||
'const hasSessionTabsRecoveryNeed = useCallback(',
|
||||
'const getSessionTabsApplicationRevision ='
|
||||
)
|
||||
|
||||
expect(recoveryNeed).toContain('hasPendingTerminalHandleRecoveryNeed(')
|
||||
expect(recoveryNeed).toContain('sessionTabsRef.current')
|
||||
expect(recoveryNeed).toContain('activeSessionTabIdRef.current')
|
||||
// The predicate only reaches the poll loop through this hook's option.
|
||||
expect(source).toContain('hasRecoveryNeed: hasSessionTabsRecoveryNeed')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import type { RpcResponse } from '../transport/types'
|
||||
import type { MobileSessionTab, SessionTabsResult } from './mobile-session-route-types'
|
||||
import { hasPendingTerminalHandleRecoveryNeed } from './pending-terminal-handle-recovery'
|
||||
import {
|
||||
MobileSessionTabsStreamHealth,
|
||||
type SessionTabsApplyOutcome
|
||||
} from './mobile-session-tabs-stream-health'
|
||||
|
||||
const TAB_ID = 'tab-1::f47ac10b-58cc-4372-a567-0e02b2c3d479'
|
||||
|
||||
function terminalTab(terminal: string | null): MobileSessionTab {
|
||||
return {
|
||||
type: 'terminal',
|
||||
id: TAB_ID,
|
||||
title: 'zsh',
|
||||
parentTabId: 'tab-1',
|
||||
leafId: 'f47ac10b-58cc-4372-a567-0e02b2c3d479',
|
||||
status: terminal === null ? 'pending-handle' : 'ready',
|
||||
terminal,
|
||||
isActive: true
|
||||
}
|
||||
}
|
||||
|
||||
function snapshot(terminal: string | null, type?: 'snapshot' | 'updated'): SessionTabsResult {
|
||||
return {
|
||||
worktree: 'id:repo::worktree',
|
||||
publicationEpoch: 'host:1',
|
||||
snapshotVersion: 1,
|
||||
tabs: [terminalTab(terminal)],
|
||||
activeTabId: TAB_ID,
|
||||
activeTabType: 'terminal',
|
||||
...(type ? { type } : {})
|
||||
} as SessionTabsResult
|
||||
}
|
||||
|
||||
describe('hasPendingTerminalHandleRecoveryNeed', () => {
|
||||
it('reports a need while the active terminal tab has no handle', () => {
|
||||
expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], TAB_ID)).toBe(true)
|
||||
})
|
||||
|
||||
it('clears once the host publishes the materialized handle', () => {
|
||||
expect(hasPendingTerminalHandleRecoveryNeed([terminalTab('term-1')], TAB_ID)).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores a pending terminal the user is not looking at', () => {
|
||||
const other: MobileSessionTab = {
|
||||
type: 'markdown',
|
||||
id: 'md-1',
|
||||
title: 'NOTES.md',
|
||||
filePath: '/w/NOTES.md',
|
||||
relativePath: 'NOTES.md',
|
||||
isDirty: false,
|
||||
isActive: false,
|
||||
documentVersion: '1'
|
||||
}
|
||||
expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null), other], 'md-1')).toBe(false)
|
||||
})
|
||||
|
||||
it('reports no need with no selection or an unknown selection', () => {
|
||||
expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], null)).toBe(false)
|
||||
expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], 'gone')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* STA-4256: the phone rendered its spinner forever because a `live` tabs stream
|
||||
* parks `poll()`, so a host that mints the handle without republishing was never
|
||||
* asked again. These drive the real controller to prove the predicate is what
|
||||
* keeps `session.tabs.list` flowing, and that it stops once the handle lands.
|
||||
*/
|
||||
describe('pending-handle recovery through MobileSessionTabsStreamHealth', () => {
|
||||
function makeHarness() {
|
||||
const pending: Array<(response: RpcResponse) => void> = []
|
||||
const sendRequest = vi.fn(
|
||||
() =>
|
||||
new Promise<RpcResponse>((resolve) => {
|
||||
pending.push(resolve)
|
||||
})
|
||||
)
|
||||
const client = { sendRequest, getGeneration: () => 1 } as unknown as RpcClient
|
||||
let tabs: MobileSessionTab[] = []
|
||||
let activeTabId: string | null = null
|
||||
const controller = new MobileSessionTabsStreamHealth<SessionTabsResult, MobileSessionTab>({
|
||||
client,
|
||||
scope: 'id:repo::worktree',
|
||||
apply: (result): SessionTabsApplyOutcome<MobileSessionTab> => {
|
||||
tabs = result.tabs
|
||||
activeTabId = result.activeTabId
|
||||
return { accepted: true, effectiveTabs: result.tabs }
|
||||
},
|
||||
consumeAccepted: () => {},
|
||||
hasRecoveryNeed: () => hasPendingTerminalHandleRecoveryNeed(tabs, activeTabId)
|
||||
})
|
||||
return {
|
||||
controller,
|
||||
sendRequest,
|
||||
resolveNext(result: SessionTabsResult) {
|
||||
const resolve = pending.shift()
|
||||
expect(resolve).toBeDefined()
|
||||
resolve?.({ id: 'list', ok: true, result, _meta: { runtimeId: 'runtime-1' } })
|
||||
},
|
||||
readTabs: () => tabs
|
||||
}
|
||||
}
|
||||
|
||||
async function settle(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
/** Subscribe, snapshot, updated — the sequence that certifies the stream `live`. */
|
||||
async function driveToLiveStream(
|
||||
harness: ReturnType<typeof makeHarness>,
|
||||
terminal: string | null
|
||||
) {
|
||||
harness.controller.setReconciliationActive(true)
|
||||
const subscription = harness.controller.beginSubscription()
|
||||
subscription.listener(snapshot(terminal, 'snapshot'))
|
||||
await settle()
|
||||
harness.resolveNext(snapshot(terminal))
|
||||
await settle()
|
||||
subscription.listener(snapshot(terminal, 'updated'))
|
||||
await settle()
|
||||
harness.sendRequest.mockClear()
|
||||
expect(harness.controller.isCertified()).toBe(true)
|
||||
}
|
||||
|
||||
it('keeps polling session.tabs.list while the active terminal has no handle', async () => {
|
||||
const harness = makeHarness()
|
||||
await driveToLiveStream(harness, null)
|
||||
|
||||
expect(harness.controller.poll()).not.toBeNull()
|
||||
await settle()
|
||||
expect(harness.sendRequest).toHaveBeenCalledWith('session.tabs.list', {
|
||||
worktree: 'id:repo::worktree'
|
||||
})
|
||||
})
|
||||
|
||||
it('stops polling once a fresh snapshot carries the handle', async () => {
|
||||
const harness = makeHarness()
|
||||
await driveToLiveStream(harness, null)
|
||||
|
||||
expect(harness.controller.poll()).not.toBeNull()
|
||||
await settle()
|
||||
harness.resolveNext(snapshot('term-1'))
|
||||
await settle()
|
||||
|
||||
expect(harness.readTabs()[0]).toMatchObject({ status: 'ready', terminal: 'term-1' })
|
||||
expect(harness.controller.poll()).toBeNull()
|
||||
})
|
||||
|
||||
it('parks the poll when the active terminal already has its handle', async () => {
|
||||
const harness = makeHarness()
|
||||
await driveToLiveStream(harness, 'term-1')
|
||||
|
||||
expect(harness.controller.poll()).toBeNull()
|
||||
expect(harness.sendRequest).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,24 @@
|
||||
import type { MobileSessionTab } from './mobile-session-route-types'
|
||||
|
||||
/**
|
||||
* Whether the tab the user is looking at is a terminal the host published
|
||||
* without a PTY handle (`status: 'pending-handle'`). The session screen renders
|
||||
* a spinner for that tab and can only leave it when a snapshot carries the
|
||||
* materialized handle — so while this holds, the tabs reconciler must keep
|
||||
* asking. Without it a `live` stream parks the poll, and a host that mints the
|
||||
* handle without republishing strands the pane on the spinner forever
|
||||
* (STA-4256).
|
||||
*
|
||||
* Mirrors the route's `activePendingTerminalTab` derivation exactly, so this is
|
||||
* true precisely when the spinner is on screen.
|
||||
*/
|
||||
export function hasPendingTerminalHandleRecoveryNeed(
|
||||
tabs: readonly MobileSessionTab[],
|
||||
activeTabId: string | null
|
||||
): boolean {
|
||||
if (activeTabId === null) {
|
||||
return false
|
||||
}
|
||||
const active = tabs.find((tab) => tab.id === activeTabId)
|
||||
return active?.type === 'terminal' && typeof active.terminal !== 'string'
|
||||
}
|
||||
Reference in New Issue
Block a user