From aa9871183f43520ff93912c53a8c9f00c68b062f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:52:41 -0700 Subject: [PATCH 01/17] fix(settings): isolate provider account discovery failures (#8190) * Fix provider account discovery isolation * fix(review): mark remote WSL provider accounts active correctly Remote Windows lists host and WSL accounts together, but Active used the forced host selection only, so selected WSL accounts never lit. Scope Active to each account's own runtime slot, and cover reverse partial-load failures. * fix(review): scope remote WSL account restart checks * Fix WSL default-account row highlighting a stale "System default" badge - The forced-host activeAccountId (Codex/Claude) could show as active alongside a genuinely active WSL account when a WSL distro is flattened to host scope, since the host default id stays null in that case. - Derive activity from whether any visible row is active instead, and share a WSL_DEFAULT_DISTRO_KEY sentinel between the distro Select and the active-id resolver so they can't drift out of sync. * Fix provider account isolation and remove-dialog stale state bugs - Treat a failed provider's substituted empty roster as non-authoritative in fetchProviderAccountsSnapshot/watchProviderAccounts consumers (AccountsPane, StatusBar switchers), so a Codex/Claude fetch failure no longer wipes out the healthy provider's list or shows a false empty state. - Capture the account's runtime slot when the remove-account dialog opens (removeCodexTarget/removeClaudeTarget) instead of re-deriving it from the live roster on confirm, since the roster can change underneath an open dialog and misroute the restart check. - Compute "System default" active state against the full account roster when a remote owner's platform is unknown, since WSL rows fail-closed and hide from the visible list in that case. - Guard the deferred onError emission with a closed check so an onSnapshot handler that closes the watcher doesn't trigger a second error callback. --- .../src/components/settings/AccountsPane.tsx | 285 +++++++++--------- .../src/components/settings/Settings.tsx | 1 + .../provider-account-visibility.test.ts | 227 ++++++++++++++ .../settings/provider-account-visibility.ts | 101 +++++++ .../src/components/status-bar/StatusBar.tsx | 10 + .../runtime-provider-accounts-client.test.ts | 87 ++++++ .../runtime-provider-accounts-client.ts | 65 +++- 7 files changed, 632 insertions(+), 144 deletions(-) create mode 100644 src/renderer/src/components/settings/provider-account-visibility.test.ts create mode 100644 src/renderer/src/components/settings/provider-account-visibility.ts diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index dbd4b752e6b..5268a890e36 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -62,9 +62,19 @@ import { DialogTitle } from '../ui/dialog' import { getCodexAccountAuthWarning } from './codex-account-auth-warning' +import { + getProviderAccountActiveIdForView, + getProviderAccountRuntime, + providerAccountIsActiveInView, + providerAccountMatchesView, + WSL_DEFAULT_DISTRO_KEY, + type ProviderAccountRuntimeView +} from './provider-account-visibility' import { translate } from '@/i18n/i18n' import { cn } from '@/lib/utils' import { + emptyClaudeAccountsState, + emptyCodexAccountsState, hasRemoteProviderAccountOwner, removeClaudeProviderAccount, removeCodexProviderAccount, @@ -146,6 +156,7 @@ type AccountsPaneProps = { wslAvailable?: boolean wslDistros?: string[] wslCapabilitiesLoading?: boolean + accountOwnerPlatform?: NodeJS.Platform | null } function getHostRuntimeLabel(): string { @@ -164,46 +175,6 @@ function getCodexAccountLabel( return state.accounts.find((account) => account.id === accountId)?.email ?? 'Codex account' } -function getActiveCodexAccountIdForRuntime( - state: CodexRateLimitAccountsState, - runtime: LocalAccountRuntime -): string | null { - if (runtime.runtime === 'host') { - return state.activeAccountIdsByRuntime?.host ?? state.activeAccountId - } - if (runtime.wslDistro) { - return state.activeAccountIdsByRuntime?.wsl?.[runtime.wslDistro] ?? null - } - const defaultSelection = state.activeAccountIdsByRuntime?.wsl?.__default__ - if (defaultSelection) { - return defaultSelection - } - const selectedIds = Array.from( - new Set(Object.values(state.activeAccountIdsByRuntime?.wsl ?? {}).filter(Boolean)) - ) - return selectedIds.length === 1 ? selectedIds[0] : null -} - -function getActiveClaudeAccountIdForRuntime( - state: ClaudeRateLimitAccountsState, - runtime: LocalAccountRuntime -): string | null { - if (runtime.runtime === 'host') { - return state.activeAccountIdsByRuntime?.host ?? state.activeAccountId - } - if (runtime.wslDistro) { - return state.activeAccountIdsByRuntime?.wsl?.[runtime.wslDistro] ?? null - } - const defaultSelection = state.activeAccountIdsByRuntime?.wsl?.__default__ - if (defaultSelection) { - return defaultSelection - } - const selectedIds = Array.from( - new Set(Object.values(state.activeAccountIdsByRuntime?.wsl ?? {}).filter(Boolean)) - ) - return selectedIds.length === 1 ? selectedIds[0] : null -} - function getClaudeAccountLabel( state: ClaudeRateLimitAccountsState, accountId: string | null | undefined @@ -288,26 +259,6 @@ type LocalAccountRuntime = { label: string } -function accountMatchesRuntime( - account: - | CodexRateLimitAccountsState['accounts'][number] - | ClaudeRateLimitAccountsState['accounts'][number], - runtime: LocalAccountRuntime -): boolean { - const accountRuntime = - 'authMethod' in account - ? (account.managedAuthRuntime ?? 'host') - : (account.managedHomeRuntime ?? 'host') - const accountDistro = account.wslDistro ?? null - if (runtime.runtime === 'host') { - return accountRuntime !== 'wsl' - } - if (accountRuntime !== 'wsl') { - return false - } - return runtime.wslDistro ? accountDistro === runtime.wslDistro : true -} - function getSelectedAccountRuntime( settings: GlobalSettings, wslSupportedPlatform: boolean, @@ -344,7 +295,8 @@ export function AccountsPane({ wslSupportedPlatform = false, wslAvailable = false, wslDistros = EMPTY_WSL_DISTROS, - wslCapabilitiesLoading = false + wslCapabilitiesLoading = false, + accountOwnerPlatform = null }: AccountsPaneProps): React.JSX.Element { const searchQuery = useAppStore((s) => s.settingsSearchQuery) const codexRateLimits = useAppStore((s) => s.rateLimits.codex) @@ -388,33 +340,51 @@ export function AccountsPane({ ? `${localAccountRuntime.label.charAt(0).toLocaleLowerCase()}${localAccountRuntime.label.slice(1)}` : localAccountRuntime.label - const [codexAccounts, setCodexAccounts] = useState({ - accounts: [], - activeAccountId: null, - activeAccountIdsByRuntime: { host: null, wsl: {} } - }) + const [codexAccounts, setCodexAccounts] = + useState(emptyCodexAccountsState) const [codexAccountsLoaded, setCodexAccountsLoaded] = useState(false) const [codexAction, setCodexAction] = useState< 'idle' | 'adding' | `reauth:${string}` | `remove:${string}` | `select:${string | 'system'}` >('idle') - const [claudeAccounts, setClaudeAccounts] = useState({ - accounts: [], - activeAccountId: null, - activeAccountIdsByRuntime: { host: null, wsl: {} } - }) + const [claudeAccounts, setClaudeAccounts] = + useState(emptyClaudeAccountsState) const [claudeAction, setClaudeAction] = useState< 'idle' | 'adding' | `reauth:${string}` | `remove:${string}` | `select:${string | 'system'}` >('idle') - const [removeAccountId, setRemoveAccountId] = useState(null) - const [removeClaudeAccountId, setRemoveClaudeAccountId] = useState(null) + // Why: capture the account's runtime slot when the dialog opens; the roster + // can change underneath an open dialog and lose the slot to diff for restarts. + const [removeCodexTarget, setRemoveCodexTarget] = useState<{ + id: string + runtime: ProviderAccountRuntimeView + } | null>(null) + const [removeClaudeTarget, setRemoveClaudeTarget] = useState<{ + id: string + runtime: ProviderAccountRuntimeView + } | null>(null) + const accountVisibilityOptions = { + remoteOwner: isRemoteAccountScope, + ownerPlatform: accountOwnerPlatform + } const visibleClaudeAccounts = claudeAccounts.accounts.filter((account) => - accountMatchesRuntime(account, accountRuntime) + providerAccountMatchesView(account, accountRuntime, accountVisibilityOptions) ) const visibleCodexAccounts = codexAccounts.accounts.filter((account) => - accountMatchesRuntime(account, accountRuntime) + providerAccountMatchesView(account, accountRuntime, accountVisibilityOptions) + ) + const activeCodexAccountId = getProviderAccountActiveIdForView(codexAccounts, accountRuntime) + // Why: System default lights only when no account row is active; while a remote + // owner's platform is unknown WSL rows hide fail-closed, so check the full roster. + const ownerPlatformUnknown = isRemoteAccountScope && accountOwnerPlatform === null + const systemCodexActive = !( + ownerPlatformUnknown ? codexAccounts.accounts : visibleCodexAccounts + ).some((account) => + providerAccountIsActiveInView(account, codexAccounts, accountRuntime, accountVisibilityOptions) + ) + const systemClaudeActive = !( + ownerPlatformUnknown ? claudeAccounts.accounts : visibleClaudeAccounts + ).some((account) => + providerAccountIsActiveInView(account, claudeAccounts, accountRuntime, accountVisibilityOptions) ) - const activeCodexAccountId = getActiveCodexAccountIdForRuntime(codexAccounts, accountRuntime) - const activeClaudeAccountId = getActiveClaudeAccountIdForRuntime(claudeAccounts, accountRuntime) // Why: the auth warning is derived from the desktop's own rate-limit poll; // with a remote owner it would misattribute local auth state to the server. const activeCodexAuthWarning = @@ -518,9 +488,15 @@ export function AccountsPane({ { activeRuntimeEnvironmentId }, { onSnapshot: (snapshot) => { - setCodexAccounts(snapshot.codex) - setClaudeAccounts(snapshot.claude) - setCodexAccountsLoaded(true) + // Why: a failed provider's half is a substituted empty roster, not + // authoritative data; keep prior state and leave the loaded gate shut. + if (!snapshot.failedProviders?.includes('codex')) { + setCodexAccounts(snapshot.codex) + setCodexAccountsLoaded(true) + } + if (!snapshot.failedProviders?.includes('claude')) { + setClaudeAccounts(snapshot.claude) + } }, onError: (error) => { toast.error( @@ -615,11 +591,11 @@ export function AccountsPane({ /> {wslSupportedPlatform && accountRuntime.runtime === 'wsl' ? ( onSort(value as JiraIssueSortColumn)}> + + + + + {columns.map((column) => ( + + {column.label} + + ))} + + + + + + + + {toggleDirectionLabel} + + + + + ) +} diff --git a/src/renderer/src/components/task-page-jira-sorting.test.ts b/src/renderer/src/components/task-page-jira-sorting.test.ts new file mode 100644 index 00000000000..767fbf7c1cb --- /dev/null +++ b/src/renderer/src/components/task-page-jira-sorting.test.ts @@ -0,0 +1,269 @@ +import { describe, expect, it } from 'vitest' +import type { JiraIssue, JiraPriority } from '../../../shared/types' +import { getJiraPriorityWeight, sortJiraIssues } from './jira-issue-sorter' + +describe('TaskPage Jira sorting functionality', () => { + function jiraIssue( + key: string, + title: string, + statusName: string, + priorityName?: string, + priorityId?: string, + assigneeDisplayName?: string, + updatedAt = '2026-01-01T00:00:00.000Z', + siteId = 'site-1', + categoryKey = 'new' + ): JiraIssue { + return { + id: `${siteId}:${key}`, + key, + title, + url: `https://example.atlassian.net/browse/${key}`, + siteId, + siteName: 'Example Jira', + project: { id: '10000', key: 'ALP', name: 'Alpha', siteId }, + issueType: { id: '10001', name: 'Bug' }, + status: { id: '1', name: statusName, categoryKey, categoryName: statusName }, + priority: priorityName ? { id: priorityId ?? '1', name: priorityName } : undefined, + assignee: assigneeDisplayName + ? { accountId: 'user-1', displayName: assigneeDisplayName } + : undefined, + labels: [], + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt + } + } + + function jiraPriority(id: string, name: string): JiraPriority { + return { id, name } + } + + describe('getJiraPriorityWeight', () => { + it('returns default weight for missing priority', () => { + expect(getJiraPriorityWeight(undefined, undefined, [])).toBe(0) + }) + + it('uses priority index from Jira priorities list when available', () => { + const jiraPriorities: JiraPriority[] = [ + jiraPriority('1', 'Highest'), + jiraPriority('2', 'High'), + jiraPriority('3', 'Medium'), + jiraPriority('4', 'Low') + ] + + expect(getJiraPriorityWeight('High', '2', jiraPriorities)).toBeCloseTo(66.33, 2) + expect(getJiraPriorityWeight('Medium', '3', jiraPriorities)).toBeCloseTo(33.67, 2) + }) + + it('keeps missing priority below the lowest configured priority', () => { + const jiraPriorities = [jiraPriority('1', 'High'), jiraPriority('2', 'Low')] + + expect(getJiraPriorityWeight(undefined, undefined, jiraPriorities)).toBe(0) + expect(getJiraPriorityWeight('Low', '2', jiraPriorities)).toBe(1) + }) + + it('falls back to priority name mapping when not in priorities list', () => { + expect(getJiraPriorityWeight('Blocker', '1', [])).toBe(99) + expect(getJiraPriorityWeight('High', '2', [])).toBe(75) + expect(getJiraPriorityWeight('Medium', '3', [])).toBe(50) + expect(getJiraPriorityWeight('Low', '4', [])).toBe(25) + expect(getJiraPriorityWeight('Lowest', '5', [])).toBe(1) + }) + + it('does not treat opaque custom priority IDs as ordering ranks', () => { + expect(getJiraPriorityWeight('Custom Priority', '10', [])).toBe(50) + expect(getJiraPriorityWeight('Another', '5', [])).toBe(50) + expect(getJiraPriorityWeight('Unknown Priority', 'invalid', [])).toBe(50) + }) + }) + + describe('issue sorting', () => { + it('sorts by key in ascending order', () => { + const issues = [ + jiraIssue('ALP-10', 'Issue 10', 'To Do'), + jiraIssue('ALP-2', 'Issue 2', 'To Do'), + jiraIssue('ALP-1', 'Issue 1', 'To Do') + ] + + const sorted = sortJiraIssues(issues, 'key', 'asc') + + expect(sorted[0].key).toBe('ALP-1') + expect(sorted[1].key).toBe('ALP-2') + expect(sorted[2].key).toBe('ALP-10') + }) + + it('sorts by key in descending order', () => { + const issues = [ + jiraIssue('ALP-1', 'Issue 1', 'To Do'), + jiraIssue('ALP-2', 'Issue 2', 'To Do'), + jiraIssue('ALP-10', 'Issue 10', 'To Do') + ] + + const sorted = sortJiraIssues(issues, 'key', 'desc') + + expect(sorted[0].key).toBe('ALP-10') + expect(sorted[1].key).toBe('ALP-2') + expect(sorted[2].key).toBe('ALP-1') + }) + + it('sorts by title alphabetically', () => { + const issues = [ + jiraIssue('ALP-1', 'Zebra Issue', 'To Do'), + jiraIssue('ALP-2', 'Apple Issue', 'To Do'), + jiraIssue('ALP-3', 'Banana Issue', 'To Do') + ] + + const sorted = sortJiraIssues(issues, 'title', 'asc') + + expect(sorted[0].title).toBe('Apple Issue') + expect(sorted[1].title).toBe('Banana Issue') + expect(sorted[2].title).toBe('Zebra Issue') + }) + + it('sorts by priority weight (lowest priority first)', () => { + const issues = [ + jiraIssue('ALP-1', 'Issue 1', 'To Do', 'Low', '4'), + jiraIssue('ALP-2', 'Issue 2', 'To Do', 'High', '2'), + jiraIssue('ALP-3', 'Issue 3', 'To Do', 'Medium', '3') + ] + + const sorted = sortJiraIssues(issues, 'priority', 'asc') + + expect(sorted[0].priority?.name).toBe('Low') + expect(sorted[1].priority?.name).toBe('Medium') + expect(sorted[2].priority?.name).toBe('High') + }) + + it('uses each Jira site priority order when sorting an all-sites list', () => { + const issues = [ + jiraIssue( + 'BRV-1', + 'Site B low by name', + 'To Do', + 'Low', + '2', + undefined, + undefined, + 'site-b' + ), + jiraIssue( + 'BRV-2', + 'Site B high by name', + 'To Do', + 'High', + '1', + undefined, + undefined, + 'site-b' + ) + ] + const prioritiesBySite = new Map([ + ['site-a', [jiraPriority('1', 'High'), jiraPriority('2', 'Low')]], + ['site-b', [jiraPriority('2', 'Low'), jiraPriority('1', 'High')]] + ]) + + const sorted = sortJiraIssues(issues, 'priority', 'asc', prioritiesBySite) + + expect(sorted.map((issue) => issue.key)).toEqual(['BRV-2', 'BRV-1']) + }) + + it('normalizes priority ranks across sites with different tier counts', () => { + const issues = [ + jiraIssue( + 'ALP-1', + 'Site A highest', + 'To Do', + 'Highest', + '1', + undefined, + undefined, + 'site-a' + ), + jiraIssue('BRV-1', 'Site B medium', 'To Do', 'Medium', '3', undefined, undefined, 'site-b') + ] + const prioritiesBySite = new Map([ + ['site-a', [jiraPriority('1', 'Highest'), jiraPriority('2', 'Lowest')]], + [ + 'site-b', + [ + jiraPriority('1', 'Highest'), + jiraPriority('2', 'High'), + jiraPriority('3', 'Medium'), + jiraPriority('4', 'Low'), + jiraPriority('5', 'Lowest') + ] + ] + ]) + + const sorted = sortJiraIssues(issues, 'priority', 'asc', prioritiesBySite) + + expect(sorted.map((issue) => issue.key)).toEqual(['BRV-1', 'ALP-1']) + }) + + it('sorts by assignee alphabetically', () => { + const issues = [ + jiraIssue('ALP-1', 'Issue 1', 'To Do', undefined, undefined, 'Zoe'), + jiraIssue('ALP-2', 'Issue 2', 'To Do', undefined, undefined, 'Alice'), + jiraIssue('ALP-3', 'Issue 3', 'To Do', undefined, undefined, 'Bob') + ] + + const sorted = sortJiraIssues(issues, 'assignee', 'asc') + + expect(sorted[0].assignee?.displayName).toBe('Alice') + expect(sorted[1].assignee?.displayName).toBe('Bob') + expect(sorted[2].assignee?.displayName).toBe('Zoe') + }) + + it('sorts by updated date (newest first)', () => { + const issues = [ + jiraIssue( + 'ALP-1', + 'Issue 1', + 'To Do', + undefined, + undefined, + undefined, + '2026-01-01T00:00:00.000Z' + ), + jiraIssue( + 'ALP-2', + 'Issue 2', + 'To Do', + undefined, + undefined, + undefined, + '2026-01-03T00:00:00.000Z' + ), + jiraIssue( + 'ALP-3', + 'Issue 3', + 'To Do', + undefined, + undefined, + undefined, + '2026-01-02T00:00:00.000Z' + ) + ] + + const sorted = sortJiraIssues(issues, 'updated', 'desc') + + expect(sorted[0].key).toBe('ALP-2') + expect(sorted[1].key).toBe('ALP-3') + expect(sorted[2].key).toBe('ALP-1') + }) + + it('handles unassigned assignees in sorting', () => { + const issues = [ + jiraIssue('ALP-1', 'Issue 1', 'To Do', undefined, undefined, 'Alice'), + jiraIssue('ALP-2', 'Issue 2', 'To Do', undefined, undefined, undefined), + jiraIssue('ALP-3', 'Issue 3', 'To Do', undefined, undefined, 'Bob') + ] + + const sorted = sortJiraIssues(issues, 'assignee', 'asc') + + expect(sorted[0].assignee?.displayName).toBeUndefined() + expect(sorted[1].assignee?.displayName).toBe('Alice') + expect(sorted[2].assignee?.displayName).toBe('Bob') + }) + }) +}) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f5d65a28e03..eb1b76a447a 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1736,7 +1736,11 @@ "editReviewersWithCurrent": "Edit reviewers: {{value0}}", "linearEmptyAttributeFilter": "No issues match the selected filters. Clear a filter or try different criteria.", "linearEmptyUnfilteredScope": "No issues in this workspace scope. Try searching or adjusting teams.", - "linearFetchMore": "Fetch more" + "linearFetchMore": "Fetch more", + "jiraSortAscending": "ascending", + "jiraSortDescending": "descending", + "jiraSortBy": "Sort by", + "jiraToggleSortDirection": "Sort {{value0}}" }, "Terminal": { "73768427cf": "Close", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index a67ef117344..36873744e79 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -1736,7 +1736,11 @@ "editReviewersWithCurrent": "Editar revisores: {{value0}}", "linearEmptyAttributeFilter": "Ningún issue coincide con los filtros seleccionados. Limpia un filtro o prueba con otros criterios.", "linearEmptyUnfilteredScope": "No hay issues en el alcance de este espacio de trabajo. Intenta buscar o ajustar los equipos.", - "linearFetchMore": "Cargar más" + "linearFetchMore": "Cargar más", + "jiraSortAscending": "ascendente", + "jiraSortDescending": "descendente", + "jiraSortBy": "Ordenar por", + "jiraToggleSortDirection": "Ordenar de forma {{value0}}" }, "Terminal": { "73768427cf": "Cerrar", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 6a8e9f075bb..c4319b90e53 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -1736,7 +1736,11 @@ "editReviewersWithCurrent": "レビュアーを編集: {{value0}}", "linearEmptyAttributeFilter": "選択したフィルターに一致するイシューがありません。フィルターを解除するか、異なる条件を試してください。", "linearEmptyUnfilteredScope": "このワークスペーススコープにイシューがありません。検索するか、チームを調整してください。", - "linearFetchMore": "さらに読み込む" + "linearFetchMore": "さらに読み込む", + "jiraSortAscending": "昇順", + "jiraSortDescending": "降順", + "jiraSortBy": "並べ替え", + "jiraToggleSortDirection": "{{value0}}に並べ替え" }, "Terminal": { "73768427cf": "閉じる", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 75a5db8a494..d44092ab6a5 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -1736,7 +1736,11 @@ "editReviewersWithCurrent": "리뷰어 편집: {{value0}}", "linearEmptyAttributeFilter": "선택한 필터와 일치하는 이슈가 없습니다. 필터를 지우거나 다른 조건을 시도하세요.", "linearEmptyUnfilteredScope": "이 워크트리 범위에 이슈가 없습니다. 검색하거나 팀을 조정해 보세요.", - "linearFetchMore": "더 불러오기" + "linearFetchMore": "더 불러오기", + "jiraSortAscending": "오름차순", + "jiraSortDescending": "내림차순", + "jiraSortBy": "정렬 기준", + "jiraToggleSortDirection": "{{value0}}으로 정렬" }, "Terminal": { "73768427cf": "닫기", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index cc7d98f8a30..7110ab9ad65 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -1736,7 +1736,11 @@ "editReviewersWithCurrent": "编辑评审人：{{value0}}", "linearEmptyAttributeFilter": "没有匹配所选筛选条件的问题。请清除筛选条件或尝试其他条件。", "linearEmptyUnfilteredScope": "此工作区范围内没有问题。请尝试搜索或调整团队。", - "linearFetchMore": "加载更多" + "linearFetchMore": "加载更多", + "jiraSortAscending": "升序", + "jiraSortDescending": "降序", + "jiraSortBy": "排序方式", + "jiraToggleSortDirection": "按{{value0}}排序" }, "Terminal": { "73768427cf": "关闭", From 5a8078e75582bd76eb16c1246ae1ddbaeb0c5d97 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:40:40 -0700 Subject: [PATCH 09/17] Improve search box on mobile (#8187) * Redesign mobile search field as a shared, raised component - Extract MobileSearchField from duplicated Search icon + TextInput + clear button markup in worktree list and tasks screens into a reusable component - Give the field a raised bgRaised shell with focus/disabled states so it reads as a tappable control instead of blending into panel chrome - Fix delayed autoFocus via InteractionManager + timeout so the keyboard reliably appears after the search bar opens - Preserve per-screen clear behavior (preset/query fallback for GitHub, project-view filter) via configurable showClear/onClear props * Simplify GitHub project search state checks and fix stuck clear button - Extract `isGithubProjectSearch` to dedupe repeated `provider === 'github' && githubMode === 'project'` checks - Fix showClear so an explicit empty applied override doesn't leave the clear button visible forever --- mobile/app/h/[hostId]/index.tsx | 43 ++--- mobile/app/h/[hostId]/tasks.tsx | 112 +++++------ mobile/src/components/MobileSearchField.tsx | 201 ++++++++++++++++++++ 3 files changed, 264 insertions(+), 92 deletions(-) create mode 100644 mobile/src/components/MobileSearchField.tsx diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index 7fb6a1f6eea..d613bff4575 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -1,13 +1,5 @@ import { useState, useEffect, useCallback, useMemo, useRef } from 'react' -import { - View, - Text, - StyleSheet, - SectionList, - Pressable, - ActivityIndicator, - TextInput -} from 'react-native' +import { View, Text, StyleSheet, SectionList, Pressable, ActivityIndicator } from 'react-native' import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' import { useFocusEffect, useLocalSearchParams, usePathname, useRouter } from 'expo-router' import { @@ -56,6 +48,7 @@ import { ConfirmModal } from '../../../src/components/ConfirmModal' import { BottomDrawer } from '../../../src/components/BottomDrawer' import { ProtocolBlockScreen } from '../../../src/components/ProtocolBlockScreen' import { AuthFailedBanner } from '../../../src/components/AuthFailedBanner' +import { MobileSearchField } from '../../../src/components/MobileSearchField' import { WorkspaceDetailPlaceholder } from '../../../src/components/WorkspaceDetailPlaceholder' import { getCachedWorktrees } from '../../../src/cache/worktree-cache' import { setCachedRepos } from '../../../src/cache/repo-cache' @@ -1134,22 +1127,16 @@ export function HostScreen({ {/* Search bar */} {showSearch && ( - - - {search.length > 0 && ( - setSearch('')}> - - - )} )} @@ -1181,6 +1168,9 @@ export function HostScreen({ sections={sections} keyExtractor={(w) => w.sectionListKey ?? w.worktreeId} stickySectionHeadersEnabled={false} + // Why: keep the search IME up while tapping clear / scrolling results. + keyboardShouldPersistTaps="handled" + keyboardDismissMode="on-drag" onScrollToIndexFailed={onScrollToIndexFailed} // Why: edge-to-edge — the list scrolls under the system nav bar // while reserving insets.bottom keeps the last worktree row reachable @@ -1615,21 +1605,12 @@ const styles = StyleSheet.create({ padding: spacing.xs }, searchBar: { - flexDirection: 'row', - alignItems: 'center', paddingHorizontal: spacing.md, - paddingVertical: spacing.xs + 2, - gap: spacing.sm, - borderBottomWidth: 1, + paddingVertical: spacing.sm, + borderBottomWidth: StyleSheet.hairlineWidth, borderBottomColor: colors.borderSubtle, backgroundColor: colors.bgPanel }, - searchInput: { - flex: 1, - color: colors.textPrimary, - fontSize: 13, - paddingVertical: 2 - }, centered: { flex: 1, alignItems: 'center', diff --git a/mobile/app/h/[hostId]/tasks.tsx b/mobile/app/h/[hostId]/tasks.tsx index 08f69b4972f..fea23f01764 100644 --- a/mobile/app/h/[hostId]/tasks.tsx +++ b/mobile/app/h/[hostId]/tasks.tsx @@ -29,7 +29,6 @@ import { Pencil, Plus, RefreshCw, - Search, Send, X } from 'lucide-react-native' @@ -48,6 +47,7 @@ import { ConfirmModal } from '../../../src/components/ConfirmModal' import { MobileMarkdown } from '../../../src/components/MobileMarkdown' import { MobileAgentIcon } from '../../../src/components/MobileAgentIcon' import { MobileWorkspaceNameInput } from '../../../src/components/MobileWorkspaceNameInput' +import { MobileSearchField } from '../../../src/components/MobileSearchField' import { MobileSyntaxSegments } from '../../../src/components/MobileSyntaxSegments' import { PickerModal, type PickerOption } from '../../../src/components/PickerModal' import { TaskProviderLogo } from '../../../src/components/TaskProviderLogo' @@ -8668,6 +8668,7 @@ export default function MobileTasksScreen() { : provider === 'gitlab' ? 'No GitLab tasks' : 'No Linear tasks' + const isGithubProjectSearch = provider === 'github' && githubMode === 'project' return ( @@ -9048,31 +9049,33 @@ export default function MobileTasksScreen() { {provider === 'gitlab' && gitlabView === 'todos' ? null : provider === 'linear' && !linearConnected ? null : ( - - 0 || + (appliedGithubProjectSearch !== undefined && + appliedGithubProjectSearch.length > 0) + : provider === 'github' + ? query.trim() !== getTaskPresetQuery(githubPreset).trim() + : undefined + } + editable={taskUiReady} onSubmitEditing={() => { if (!taskUiReady) { return } - if (provider === 'github' && githubMode === 'project') { + if (isGithubProjectSearch) { applyGitHubProjectSearch() return } @@ -9091,42 +9094,38 @@ export default function MobileTasksScreen() { } }} onBlur={() => { - if (provider === 'github' && githubMode === 'project') { + if (isGithubProjectSearch) { applyGitHubProjectSearch() } }} + // Why: Project clear means unfiltered results ('' override when the view + // has a default filter), not restore view default. GitHub items clear + // restores the preset query. Linear clears and persists empty resume. + onClear={() => { + if (isGithubProjectSearch) { + const viewFilter = githubProjectTable?.selectedView.filter ?? '' + setGithubProjectSearch('') + // Why: undefined = use view default; '' = explicit unfiltered override. + setAppliedGithubProjectSearch(viewFilter ? '' : undefined) + return + } + if (provider === 'github') { + const nextQuery = getTaskPresetQuery(githubPreset) + setQuery(nextQuery) + setAppliedQuery(nextQuery) + persistTaskResumeState({ + githubItemsPreset: githubPreset, + githubItemsQuery: nextQuery + }) + return + } + setQuery('') + setAppliedQuery('') + if (provider === 'linear') { + persistTaskResumeState({ linearQuery: '' }) + } + }} /> - {(provider === 'github' && githubMode === 'project' - ? githubProjectSearch.length > 0 || appliedGithubProjectSearch !== undefined - : query.length > 0) && ( - { - if (provider === 'github' && githubMode === 'project') { - const viewFilter = githubProjectTable?.selectedView.filter ?? '' - setGithubProjectSearch('') - setAppliedGithubProjectSearch(viewFilter ? '' : undefined) - return - } - if (provider === 'github') { - const nextQuery = getTaskPresetQuery(githubPreset) - setQuery(nextQuery) - setAppliedQuery(nextQuery) - persistTaskResumeState({ - githubItemsPreset: githubPreset, - githubItemsQuery: nextQuery - }) - return - } - setQuery('') - setAppliedQuery('') - if (provider === 'linear') { - persistTaskResumeState({ linearQuery: '' }) - } - }} - > - - - )} )} @@ -13818,20 +13817,11 @@ const styles = StyleSheet.create({ color: colors.textSecondary }, searchBar: { - flexDirection: 'row', - alignItems: 'center', paddingHorizontal: spacing.md, - paddingVertical: spacing.xs + 2, - gap: spacing.sm, - borderTopWidth: 1, + paddingVertical: spacing.sm, + borderTopWidth: StyleSheet.hairlineWidth, borderTopColor: colors.borderSubtle }, - searchInput: { - flex: 1, - color: colors.textPrimary, - fontSize: 13, - paddingVertical: 2 - }, errorBanner: { paddingHorizontal: spacing.lg, paddingVertical: spacing.sm, diff --git a/mobile/src/components/MobileSearchField.tsx b/mobile/src/components/MobileSearchField.tsx new file mode 100644 index 00000000000..2dfd6bf26e4 --- /dev/null +++ b/mobile/src/components/MobileSearchField.tsx @@ -0,0 +1,201 @@ +import { useEffect, useRef, useState } from 'react' +import { + InteractionManager, + Platform, + Pressable, + StyleSheet, + TextInput, + View, + type TextInputProps +} from 'react-native' +import { Search, X } from 'lucide-react-native' +import { colors, radii, spacing, typography } from '../theme/mobile-theme' + +// Why: toolbar/list chrome paints and settles after the open tap; native +// autoFocus alone often fails to raise the soft keyboard on iOS/Android. +const SEARCH_AUTO_FOCUS_DELAY_MS = 120 + +type MobileSearchFieldProps = { + value: string + onChangeText: (text: string) => void + placeholder: string + onClear?: () => void + /** Override clear-button visibility (default: value is non-empty). */ + showClear?: boolean + clearAccessibilityLabel?: string + autoFocus?: boolean + /** Re-run delayed focus when this identity changes (e.g. each time search opens). */ + focusKey?: unknown + returnKeyType?: TextInputProps['returnKeyType'] + onSubmitEditing?: TextInputProps['onSubmitEditing'] + onBlur?: TextInputProps['onBlur'] + editable?: boolean + accessibilityLabel?: string +} + +/** + * Raised search field used on list screens. Sits above the base/panel canvas + * so it reads as a tappable control instead of chrome that blends into the list. + */ +export function MobileSearchField({ + value, + onChangeText, + placeholder, + onClear, + showClear, + clearAccessibilityLabel = 'Clear search', + autoFocus = false, + focusKey, + returnKeyType = 'search', + onSubmitEditing, + onBlur, + editable = true, + accessibilityLabel +}: MobileSearchFieldProps) { + const inputRef = useRef(null) + const [focused, setFocused] = useState(false) + const clearVisible = showClear ?? value.length > 0 + + useEffect(() => { + if (!autoFocus || !editable) { + return + } + + let timeout: ReturnType | undefined + // Why: wait for the open-press interaction + layout to finish, then focus + // so the soft keyboard actually appears (not just a caret with no IME). + const task = InteractionManager.runAfterInteractions(() => { + timeout = setTimeout(() => { + inputRef.current?.focus() + }, SEARCH_AUTO_FOCUS_DELAY_MS) + }) + + return () => { + task.cancel() + if (timeout) { + clearTimeout(timeout) + } + } + }, [autoFocus, editable, focusKey]) + + function handleClear() { + if (onClear) { + onClear() + } else { + onChangeText('') + } + // Why: pressing the clear chip steals focus and drops the keyboard; + // re-focus so the user can keep typing without tapping the field again. + requestAnimationFrame(() => { + inputRef.current?.focus() + }) + } + + return ( + + + setFocused(true)} + onBlur={(event) => { + setFocused(false) + onBlur?.(event) + }} + clearButtonMode="never" + accessibilityLabel={accessibilityLabel ?? placeholder} + selectionColor={colors.accentBlue} + /> + {clearVisible ? ( + [styles.clearButton, pressed && styles.clearButtonPressed]} + > + {/* Why: chip + larger hit target — a bare 14px X was hard to tap and + read as decoration rather than a clear control. */} + + + + + ) : null} + + ) +} + +const styles = StyleSheet.create({ + shell: { + minHeight: 42, + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm, + // Why: bgRaised lifts the field off bgBase/bgPanel so search is an obvious + // control, matching TextInputModal / MobilePrBasePicker input shells. + backgroundColor: colors.bgRaised, + borderWidth: 1, + borderColor: colors.borderSubtle, + borderRadius: radii.input, + paddingLeft: spacing.md, + paddingRight: spacing.xs, + paddingVertical: Platform.OS === 'ios' ? spacing.sm : spacing.xs + 2 + }, + shellFocused: { + // Why: monochrome focus cue without burning the blue accent token + // (reserved for state/selection). textMuted reads clearly on bgRaised. + borderColor: colors.textMuted + }, + shellDisabled: { + opacity: 0.55 + }, + input: { + flex: 1, + minWidth: 0, + padding: 0, + margin: 0, + color: colors.textPrimary, + fontSize: typography.bodySize, + // Why: Android TextInput draws extra vertical padding that misaligns the + // icon/clear chip unless we zero it out. + includeFontPadding: false, + textAlignVertical: 'center' + }, + clearButton: { + minWidth: 36, + minHeight: 36, + alignItems: 'center', + justifyContent: 'center' + }, + clearButtonPressed: { + opacity: 0.7 + }, + clearChip: { + width: 24, + height: 24, + borderRadius: 12, + alignItems: 'center', + justifyContent: 'center', + // Why: textMuted reads as a solid chip on bgRaised; borderSubtle was nearly + // invisible and made the clear control feel like decorative chrome. + backgroundColor: colors.textMuted + } +}) From a916a93df3bfaa82a62d8a1f170bec001505356e Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:41:27 -0700 Subject: [PATCH 10/17] Make explicit desktop take-back always release the mobile presence lock (#8200) Previously a failed resize during reclaimTerminalForDesktop left the override/lock in place (#7588 semantics), but for an explicit "take back all terminals" gesture that could strand banners on background panes whose resize can't converge. Now the take-back unconditionally releases the driver and clears any held fit-override via a new releaseDesktopTakeBack helper, while auto-restore and phone-initiated paths keep the original keep-lock-on-failure behavior. Also extract the local mapWithConcurrency helper out of workspace-cleanup.ts into a shared, tested src/shared/map-with-concurrency.ts and use it to bound concurrent desktop-fit reclaims in terminal-fit-restore.ts. --- src/main/runtime/mobile-presence-lock.test.ts | 72 +++++------- src/main/runtime/orca-runtime.ts | 111 +++++++++--------- .../terminal-pane/terminal-fit-restore.ts | 12 +- .../src/store/slices/workspace-cleanup.ts | 21 +--- src/shared/map-with-concurrency.test.ts | 50 ++++++++ src/shared/map-with-concurrency.ts | 24 ++++ 6 files changed, 169 insertions(+), 121 deletions(-) create mode 100644 src/shared/map-with-concurrency.test.ts create mode 100644 src/shared/map-with-concurrency.ts diff --git a/src/main/runtime/mobile-presence-lock.test.ts b/src/main/runtime/mobile-presence-lock.test.ts index a38084c5199..eddb3f299cf 100644 --- a/src/main/runtime/mobile-presence-lock.test.ts +++ b/src/main/runtime/mobile-presence-lock.test.ts @@ -574,9 +574,13 @@ describe('mobile presence lock — issue #7588 held-modal restore convergence', expect(listenerEvents.some((e) => e.mode === 'desktop-fit')).toBe(true) }) - // Scenario 4: a failing resize during a HELD (no-subscriber) restore returns - // false and keeps the override — no lying, no phantom desktop-fit event. - it('held restore with a failing resize returns false and keeps the override', async () => { + // Scenario 4: an explicit take-back on a HELD (no-subscriber) override must + // ALWAYS release, even when the desktop resize fails. The override is cleared + // optimistically with a paired desktop-fit 0×0 so the banner can't linger; the + // desktop renderer refits the PTY on its next settled frame. (The gesture + // deliberately overrides #7588's keep-lock-on-failed-resize rule, which still + // governs the auto-restore and phone-initiated paths.) + it('held restore with a failing resize still releases and clears the override', async () => { const { runtime, fitOverrideEvents, setResizeSucceeds } = createRuntime(null) const listenerEvents: { mode: string; cols: number; rows: number }[] = [] runtime.subscribeToFitOverrideChanges('pty-1', (e) => listenerEvents.push(e)) @@ -594,18 +598,17 @@ describe('mobile presence lock — issue #7588 held-modal restore convergence', const restored = await runtime.reclaimTerminalForDesktop('pty-1') - expect(restored).toBe(false) - expect(runtime.getTerminalFitOverride('pty-1')).not.toBeNull() - expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe( - false - ) - expect(listenerEvents.slice(listenerBefore).some((e) => e.mode === 'desktop-fit')).toBe(false) + expect(restored).toBe(true) + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) + expect(listenerEvents.slice(listenerBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) }) - // Scenario 5: a failing resize during an ACTIVE-SUBSCRIBER take-back returns - // false, keeps the override, leaves the driver on its mobile lock, and - // restores the prior display mode ('auto') — the fix #3 P1 correction. - it('active-subscriber take-back with a failing resize returns false and preserves the mobile lock', async () => { + // Scenario 5: an explicit ACTIVE-SUBSCRIBER take-back with a failing resize + // must still release — driver → desktop, override cleared, banner dismissed, + // mode reset to 'auto'. This is the "take back all terminals" guarantee: a + // background PTY that can't converge must not strand its banner. + it('active-subscriber take-back with a failing resize still releases the lock', async () => { const { runtime, fitOverrideEvents, setResizeSucceeds } = createRuntime() const listenerEvents: { mode: string; cols: number; rows: number }[] = [] runtime.subscribeToFitOverrideChanges('pty-1', (e) => listenerEvents.push(e)) @@ -619,43 +622,30 @@ describe('mobile presence lock — issue #7588 held-modal restore convergence', const restored = await runtime.reclaimTerminalForDesktop('pty-1') - expect(restored).toBe(false) - expect(runtime.getTerminalFitOverride('pty-1')).not.toBeNull() - // Driver stays mobile (lock retained) and mode is not left lying at 'desktop'. - expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + expect(restored).toBe(true) + // Lock released and banner dismissed despite the failed resize. + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() expect(runtime.getMobileDisplayMode('pty-1')).toBe('auto') - expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe( - false - ) - expect(listenerEvents.slice(listenerBefore).some((e) => e.mode === 'desktop-fit')).toBe(false) + expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) + expect(listenerEvents.slice(listenerBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) }) - // Scenario 5b: after a FAILED active-subscriber take-back, wasResizedToPhone - // must be re-armed so a later unsubscribe under a finite auto-restore setting - // still schedules its timer and eventually clears the override. Without the - // re-arm the flag would be stuck false and the phone-fit would strand. - it('failed take-back re-arms wasResizedToPhone so a later unsubscribe still auto-restores', async () => { - // Finite auto-restore (5s default from the rig store). - const { runtime, ptySizes, setResizeSucceeds } = createRuntime() + // Scenario 5b: the failed-resize take-back leaves NO stranded phone-fit — + // driver released to desktop, override cleared, mode reset to 'auto'. (The + // pre-revision behavior kept the lock and relied on a later auto-restore; the + // explicit gesture now releases unconditionally.) + it('failed take-back leaves no stranded override or lock', async () => { + const { runtime, setResizeSucceeds } = createRuntime() await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) - expect(ptySizes.get('pty-1')).toEqual({ cols: 45, rows: 20 }) - // Take-back fails → false, flag re-armed, mode rolled back to 'auto'. setResizeSucceeds(false) - expect(await runtime.reclaimTerminalForDesktop('pty-1')).toBe(false) - expect(runtime.getTerminalFitOverride('pty-1')).not.toBeNull() + expect(await runtime.reclaimTerminalForDesktop('pty-1')).toBe(true) - // Phone then leaves the terminal. Resize works again for the auto-restore. - setResizeSucceeds(true) - runtime.handleMobileUnsubscribe('pty-1', 'phone-A') - // Soft-leave grace, then the finite auto-restore timer. - await vi.advanceTimersByTimeAsync(300) - await vi.advanceTimersByTimeAsync(5_000) - - // The scheduled auto-restore fired: override cleared, PTY back to desktop. + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() - expect(ptySizes.get('pty-1')).toEqual({ cols: 150, rows: 40 }) + expect(runtime.getMobileDisplayMode('pty-1')).toBe('auto') }) // Scenario 6: a phone-initiated setDisplayMode('desktop') against a stale diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 88a8f69b8f5..b7e4c1d2c30 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -8197,42 +8197,35 @@ export class OrcaRuntimeService { } // Why: invoked from `runtime:restoreTerminalFit` IPC (the desktop "Take - // back" / "Restore" button). Forces the PTY back to desktop dims and - // flips the driver to `desktop`, suppressing further mobile-driven dim - // changes until a mobile actor takes the floor again. Two cases: - // 1. Active mobile subscriber: route through applyMobileDisplayMode so - // the existing 'resized' event reaches the phone. - // 2. Held with no mobile subscriber (post-indefinite-hold): no inner - // subscriber to notify; resolve restore target and enqueueLayout - // directly. applyLayout is the SOLE writer of terminalFitOverrides on - // the normal path, so the held branch defers the mutation to it — the - // one exception is the dead-pty orphan cleanup below, which mirrors - // onPtyExit's sanctioned delete. See docs/mobile-fit-hold.md. + // back" / "Restore" button). Forces the PTY back to desktop dims and flips + // the driver to `desktop`, suppressing further mobile-driven dim changes + // until a mobile actor takes the floor again. Three cases, each ending in + // releaseDesktopTakeBack: + // 1. Active mobile subscriber: route through applyMobileDisplayMode so the + // existing 'resized' event reaches the phone. + // 2. Held override, no subscriber (post-indefinite-hold): resolve the + // restore target and enqueueLayout directly. + // 3. Stale mobile driver, no subscriber and no override: nothing to resize, + // just drop the lock. See docs/mobile-fit-hold.md. // - // Returns `true` only when the pty ends converged (no fit-override held); - // `false` when a restore was attempted but the resize failed (#7588), or - // when there was nothing to reclaim. On a failed-restore `false`, driver/mode - // are left unchanged so a driving phone keeps its lock and the modal - // truthfully stays — the user can retry. + // Why: explicit desktop take-back is a user command to reclaim input control + // NOW. Unlike the auto-restore timer and phone-initiated setDisplayMode paths + // (which keep the lock when a resize can't converge, #7588), this gesture + // ALWAYS drops the presence lock and banner. "Take back all terminals" + // reclaims several PTYs at once; a background pane whose desktop resize can't + // converge must not strand its banner on the other terminals. The resize is + // best-effort — the desktop renderer refits the PTY on its next settled + // frame. Returns `true` whenever there was a lock to reclaim, `false` only + // when there was nothing to reclaim. async reclaimTerminalForDesktop(ptyId: string): Promise { if (this.isMobileSubscriberActive(ptyId)) { - // Why (#7588): capture the prior mode so a failed restore can roll the - // routing-only 'desktop' write back — driver/mode transitions must be - // gated on convergence, not committed before we know the resize took. - const priorMode = this.getMobileDisplayMode(ptyId) this.setMobileDisplayMode(ptyId, 'desktop') - const converged = await this.applyMobileDisplayMode(ptyId) - if (!converged) { - // Resize failed — override still held. Leave the driver on its mobile - // lock and undo the mode write so nothing is left lying at 'desktop'. - this.setMobileDisplayMode(ptyId, priorMode) - return false - } - this.setDriver(ptyId, { kind: 'desktop' }) - // Why: a desktop-initiated reclaim is "I'm taking over right now", - // not a sticky preference. The next mobile subscribe (e.g. user - // switches back to the terminal tab on the phone) must default to - // phone-fit again, not stay in passive desktop-watch mode. + await this.applyMobileDisplayMode(ptyId) + this.releaseDesktopTakeBack(ptyId) + // Why: a desktop-initiated reclaim is "I'm taking over right now", not a + // sticky preference. The next mobile subscribe (e.g. user switches back to + // the terminal tab on the phone) must default to phone-fit again, not stay + // in passive desktop-watch mode. this.setMobileDisplayMode(ptyId, 'auto') return true } @@ -8251,36 +8244,38 @@ export class OrcaRuntimeService { const renderer = this.lastRendererSizes.get(ptyId) const cols = renderer?.cols ?? heldOverride.previousCols ?? fallback.cols const rows = renderer?.rows ?? heldOverride.previousRows ?? fallback.rows - const result = await this.enqueueLayout(ptyId, { kind: 'desktop', cols, rows }) - if (result.ok) { - this.setDriver(ptyId, { kind: 'desktop' }) - // Why: a desktop-initiated reclaim is "I'm taking over right now", - // not a sticky preference. Reset to auto so the next mobile subscribe - // re-enters phone-fit. (Held-PTY branch may not have an entry, but - // calling setMobileDisplayMode('auto') is a no-op deletion in that - // case — safe and idempotent.) - this.setMobileDisplayMode(ptyId, 'auto') - return true - } - // Why (#7588): the layout entry is gone (pty exited) but the override is - // still held — unreachable via public APIs today (onPtyExit deletes both - // in lockstep), but reporting success while stranding the override would - // re-show the modal on the next hydrate. Run the same cleanup onPtyExit - // does (delete override + paired desktop-fit 0×0) so the renderer - // converges; nothing to resize on a dead pty. - if (result.reason === 'pty-exited' && this.terminalFitOverrides.has(ptyId)) { - this.terminalFitOverrides.delete(ptyId) - this.notifier?.terminalFitOverrideChanged(ptyId, 'desktop-fit', 0, 0) - this.notifyFitOverrideListeners(ptyId, 'desktop-fit', 0, 0) - return true - } - // resize-failed: the override remains held; report the truth so the - // modal correctly stays and the user can retry. Driver/mode untouched. - return false + await this.enqueueLayout(ptyId, { kind: 'desktop', cols, rows }) + this.releaseDesktopTakeBack(ptyId) + this.setMobileDisplayMode(ptyId, 'auto') + return true + } + // Why: a stale lock — driver still reads mobile with no active subscriber + // and no held override (e.g. reclaimed inside the soft-leave grace, or a + // subscriber that dropped without a clean unsubscribe). Release it so the + // banner can't linger; there is nothing to resize. + if (this.getDriver(ptyId).kind === 'mobile') { + this.releaseDesktopTakeBack(ptyId) + return true } return false } + // Why: the shared "banner must be gone now" step for an explicit desktop + // take-back. Releases the presence lock (driver → desktop) and, if the + // best-effort resize left a fit-override held (resize didn't converge), + // clears it optimistically with a paired desktop-fit 0×0 — the same signal + // onPtyExit emits — so neither the presence-lock banner nor the held-fit + // banner can survive the reclaim. The desktop renderer refits the PTY to real + // dims on its next settled frame. + private releaseDesktopTakeBack(ptyId: string): void { + this.setDriver(ptyId, { kind: 'desktop' }) + if (this.terminalFitOverrides.has(ptyId)) { + this.terminalFitOverrides.delete(ptyId) + this.notifier?.terminalFitOverrideChanged(ptyId, 'desktop-fit', 0, 0) + this.notifyFitOverrideListeners(ptyId, 'desktop-fit', 0, 0) + } + } + // Why: read-side clamp for mobileAutoRestoreFitMs. `null` means // indefinite hold (no auto-restore timer). A finite value is clamped // to [MIN, MAX] to defend against bad config — the smallest useful diff --git a/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts b/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts index bdf152a35b9..54738f59021 100644 --- a/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts +++ b/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts @@ -1,4 +1,5 @@ import type { GlobalSettings } from '../../../../shared/types' +import { mapWithConcurrency } from '../../../../shared/map-with-concurrency' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { getRemoteRuntimePtyEnvironmentId, @@ -7,6 +8,13 @@ import { type TerminalFitRestoreSettings = Pick | undefined +// Why: "take back all terminals" can target a phone that controls hundreds of +// PTYs. Fanning out one IPC/RPC reclaim per PTY unbounded would burst the +// runtime transport; cap the in-flight reclaims so a huge session degrades to +// steady throughput instead of a thundering herd. Each reclaim is a short +// round-trip, so a modest pool keeps latency low without overwhelming it. +const RESTORE_FIT_CONCURRENCY = 8 + const restoreFailedResult = (): { restored: boolean } => { // Why: terminal fit restore is best-effort when mobile/remote transports disappear. return { restored: false } @@ -37,8 +45,8 @@ export async function restoreTerminalFitsToDesktop( settings: TerminalFitRestoreSettings ): Promise { const uniquePtyIds = [...new Set(ptyIds)] - const results = await Promise.all( - uniquePtyIds.map((ptyId) => restoreTerminalFitToDesktop(ptyId, settings)) + const results = await mapWithConcurrency(uniquePtyIds, RESTORE_FIT_CONCURRENCY, (ptyId) => + restoreTerminalFitToDesktop(ptyId, settings) ) return results.some(Boolean) } diff --git a/src/renderer/src/store/slices/workspace-cleanup.ts b/src/renderer/src/store/slices/workspace-cleanup.ts index ed2e12d49be..bffe5fb93d4 100644 --- a/src/renderer/src/store/slices/workspace-cleanup.ts +++ b/src/renderer/src/store/slices/workspace-cleanup.ts @@ -21,6 +21,7 @@ import { type WorkspaceCleanupScanProgress, type WorkspaceCleanupScanResult } from '../../../../shared/workspace-cleanup' +import { mapWithConcurrency } from '../../../../shared/map-with-concurrency' import { classifyTitleActivity, isExplicitAgentStatusFresh } from '@/lib/pane-agent-evidence' import { translate } from '@/i18n/i18n' @@ -553,26 +554,6 @@ function getWorkspaceCleanupProgressCandidateIndex( } } -async function mapWithConcurrency( - items: readonly T[], - limit: number, - fn: (item: T) => Promise -): Promise { - const results: R[] = [] - let nextIndex = 0 - const workerCount = Math.min(limit, items.length) - await Promise.all( - Array.from({ length: workerCount }, async () => { - while (nextIndex < items.length) { - const index = nextIndex - nextIndex += 1 - results[index] = await fn(items[index]) - } - }) - ) - return results -} - function getInitialWorkspaceCleanupGitDeferrals(state: AppState): string[] { const ids = new Set() if (state.activeWorktreeId) { diff --git a/src/shared/map-with-concurrency.test.ts b/src/shared/map-with-concurrency.test.ts new file mode 100644 index 00000000000..fb81bc0ed75 --- /dev/null +++ b/src/shared/map-with-concurrency.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { mapWithConcurrency } from './map-with-concurrency' + +describe('mapWithConcurrency', () => { + it('preserves input order in the result array regardless of settle order', async () => { + // Later items resolve sooner, so a naive push-on-resolve would reorder. + const results = await mapWithConcurrency([30, 20, 10], 3, async (ms, index) => { + await new Promise((resolve) => setTimeout(resolve, ms)) + return index + }) + expect(results).toEqual([0, 1, 2]) + }) + + it('never exceeds the concurrency limit', async () => { + let inFlight = 0 + let peak = 0 + const items = Array.from({ length: 50 }, (_, i) => i) + await mapWithConcurrency(items, 8, async () => { + inFlight += 1 + peak = Math.max(peak, inFlight) + await new Promise((resolve) => setTimeout(resolve, 1)) + inFlight -= 1 + }) + expect(peak).toBeLessThanOrEqual(8) + // With 50 items and limit 8, the pool should actually saturate. + expect(peak).toBe(8) + }) + + it('processes every item exactly once', async () => { + const seen: number[] = [] + await mapWithConcurrency([1, 2, 3, 4, 5], 2, async (n) => { + seen.push(n) + }) + expect(seen.sort((a, b) => a - b)).toEqual([1, 2, 3, 4, 5]) + }) + + it('returns an empty array for no items without spawning workers', async () => { + let calls = 0 + const results = await mapWithConcurrency([], 4, async () => { + calls += 1 + }) + expect(results).toEqual([]) + expect(calls).toBe(0) + }) + + it('clamps a limit below one to a single worker instead of stalling', async () => { + const results = await mapWithConcurrency([1, 2, 3], 0, async (n) => n * 2) + expect(results).toEqual([2, 4, 6]) + }) +}) diff --git a/src/shared/map-with-concurrency.ts b/src/shared/map-with-concurrency.ts new file mode 100644 index 00000000000..5930ddbed9b --- /dev/null +++ b/src/shared/map-with-concurrency.ts @@ -0,0 +1,24 @@ +// Why: bounded-concurrency map. `Promise.all(items.map(fn))` fans out every +// item at once — fine for a handful, but a burst of hundreds of concurrent +// IPC/RPC round-trips can swamp the transport or its call queue. This runs at +// most `limit` calls in flight via a fixed worker pool while preserving input +// order in the result array (results[i] corresponds to items[i]). +export async function mapWithConcurrency( + items: readonly T[], + limit: number, + fn: (item: T, index: number) => Promise +): Promise { + const results: R[] = [] + let nextIndex = 0 + const workerCount = Math.max(1, Math.min(limit, items.length)) + await Promise.all( + Array.from({ length: workerCount }, async () => { + while (nextIndex < items.length) { + const index = nextIndex + nextIndex += 1 + results[index] = await fn(items[index], index) + } + }) + ) + return results +} From c7777e44a6e1656565d49da7ad38552ca99e5250 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:50:44 -0700 Subject: [PATCH 11/17] Reduce terminal redraw CPU on active TUIs (#8178) --- .../layout-serialization.test.ts | 4 +- .../terminal-pane/layout-serialization.ts | 9 +- .../terminal-pane/pty-connection.test.ts | 40 +++++ .../terminal-pane/pty-connection.ts | 21 ++- .../terminal-pane/replay-guard.test.ts | 116 +++++++++++-- .../components/terminal-pane/replay-guard.ts | 18 +- .../lib/pane-manager/pane-lifecycle.test.ts | 14 +- .../src/lib/pane-manager/pane-lifecycle.ts | 22 +-- .../src/lib/pane-manager/pane-manager.ts | 4 + .../pane-terminal-foreground-render-settle.ts | 32 ++-- .../pane-terminal-output-scheduler.test.ts | 122 ++++++++++++++ .../pane-terminal-output-scheduler.ts | 88 +++++++++- .../terminal-arabic-shaping-joiner.test.ts | 74 +++++++++ .../terminal-arabic-shaping-joiner.ts | 104 +++++++++++- .../terminal-ligatures-addon.test.ts | 154 ++++++++++++++++++ .../pane-manager/terminal-ligatures-addon.ts | 120 ++++++++++++++ .../terminal-foreground-redraw-freeze.spec.ts | 144 ++++++++++++++-- 17 files changed, 1006 insertions(+), 80 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/terminal-ligatures-addon.test.ts create mode 100644 src/renderer/src/lib/pane-manager/terminal-ligatures-addon.ts diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts index f4237e507f0..7977196979e 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts @@ -440,7 +440,8 @@ describe('restoreScrollbackBuffers', () => { } } const manager = { - getPanes: vi.fn(() => [pane]) + getPanes: vi.fn(() => [pane]), + hasWebglRenderer: vi.fn(() => true) } const replayingPanesRef = { current: new Map() } const restoredViewportBlankingPanesRef = { current: new Set() } @@ -454,6 +455,7 @@ describe('restoreScrollbackBuffers', () => { ) expect(writes).toEqual(['restored output', '\r\n', POST_REPLAY_MODE_RESET]) + expect(manager.hasWebglRenderer).toHaveBeenCalledWith(1) expect(restoredViewportBlankingPanesRef.current.has(1)).toBe(true) expect(replayingPanesRef.current.size).toBe(0) }) diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.ts b/src/renderer/src/components/terminal-pane/layout-serialization.ts index d47ced05b9b..5f581cb9e94 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.ts @@ -265,6 +265,9 @@ export function restoreScrollbackBuffers( continue } try { + const renderOptions = { + shouldRefreshViewportSynchronously: () => !manager.hasWebglRenderer(pane.id) + } let buf = buffer // If buffer ends in alt-screen mode (agent TUI was running at // shutdown), exit alt-screen so the user sees a usable terminal. @@ -278,14 +281,14 @@ export function restoreScrollbackBuffers( // sequences from the prior session (DA1, DECRQM, OSC 10/11, focus, // CPR). Writing those through xterm.write would trigger auto-replies // that land in the new shell's stdin. See replay-guard.ts. - replayIntoTerminal(pane, replayingPanesRef, buf) + replayIntoTerminal(pane, replayingPanesRef, buf, renderOptions) // Ensure cursor is on a new line so the new shell prompt // doesn't trigger zsh's PROMPT_EOL_MARK (%) indicator. - replayIntoTerminal(pane, replayingPanesRef, '\r\n') + replayIntoTerminal(pane, replayingPanesRef, '\r\n', renderOptions) // Clear any mode bits the serialized buffer replayed into xterm. // The shell underneath is fresh and has no TUI consuming these modes. // See POST_REPLAY_MODE_RESET comment. - replayIntoTerminal(pane, replayingPanesRef, POST_REPLAY_MODE_RESET) + replayIntoTerminal(pane, replayingPanesRef, POST_REPLAY_MODE_RESET, renderOptions) // Why: connection resolution happens after layout replay; only the // fresh-shell paths should move these visible rows into scrollback. restoredViewportBlankingPanesRef?.current.add(pane.id) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 52457e170f7..fe1b2e51c30 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -467,6 +467,7 @@ function createManager(paneCount = 1, initialActivePaneId: number | null = null) setPaneGpuRendering: vi.fn(), markPaneHasComplexScriptOutput: vi.fn(), rebuildPaneWebgl: vi.fn(), + hasWebglRenderer: vi.fn(() => false), getPanes: vi.fn(() => panes), closePane: vi.fn(), getActivePane: vi.fn<() => { id: number; leafId?: string } | null>(() => @@ -12635,6 +12636,45 @@ describe('connectPanePty', () => { } }) + it('coalesces forced foreground refreshes when WebGL is live', async () => { + const restoreNavigator = temporarilySetNavigatorUserAgent('Mozilla/5.0 (Macintosh)') + try { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + } + ) + transportFactoryQueue.push(transport) + + const pane = createPane(1) + const synchronousRefresh = vi.fn() + const debouncedRefresh = vi.fn() + const terminal = pane.terminal as typeof pane.terminal & { + _core?: { refresh: typeof synchronousRefresh } + refresh: typeof debouncedRefresh + } + terminal._core = { refresh: synchronousRefresh } + terminal.refresh = debouncedRefresh + terminal.write = vi.fn((_data: string, callback?: () => void) => callback?.()) + const manager = createManager(1) + manager.hasWebglRenderer.mockReturnValue(true) + + connectPanePty(pane as never, manager as never, createDeps() as never) + await flushAsyncTicks(6) + + capturedDataCallback.current?.('\r\x1b[3Gzzzx\x1b[K') + + expect(debouncedRefresh).toHaveBeenCalledWith(0, 39) + expect(synchronousRefresh).not.toHaveBeenCalled() + } finally { + restoreNavigator() + } + }) + it('drains a post-submit synchronized frame on the fast path when its end marker arrives late', async () => { // Why (STA-1041): OpenCode wraps each submit repaint in a DEC 2026 frame. // Under CPU contention ConPTY splits the closing chunk past the 150ms redraw diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 19046aa0071..f93bba90068 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -103,6 +103,7 @@ import { resolveWindowsShellOverride } from '@/lib/pane-manager/windows-pty-compatibility' import { recordTerminalOutput } from '@/lib/pane-manager/pane-scroll' +import { ensureArabicShapingJoinerForText } from '@/lib/pane-manager/terminal-arabic-shaping-joiner' import { captureTerminalWriteScrollIntent, enforceTerminalWriteScrollIntent @@ -935,6 +936,7 @@ export function connectPanePty( manager: PaneManager, deps: PtyConnectionDeps ): PanePtyBinding { + const shouldRefreshForegroundSynchronously = (): boolean => !manager.hasWebglRenderer(pane.id) exposeE2eTerminalPtyOutputDebug() let disposed = false let connectFrame: number | null = null @@ -2278,7 +2280,9 @@ export function connectPanePty( // frame still has mouse-tracking/bracketed-paste armed, which silently // eats every click and keystroke against a dead transport — disarm the // modes now and arm the reveal-time wake. - replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_MODE_RESET) + replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_MODE_RESET, { + shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously + }) hibernatedWakeTarget = { ptyId, record: sleepingRecordEntry.record } const pendingWakeMatches = pendingHibernatedWakeTarget?.ptyId === ptyId && @@ -4300,14 +4304,18 @@ export function connectPanePty( // Why: drain any queued background bytes BEFORE the replay paint, so the // scheduler's deferred drain cannot land older bytes on top of the replay. flushTerminalOutput(pane.terminal) - replayIntoTerminal(pane, deps.replayingPanesRef, data) + replayIntoTerminal(pane, deps.replayingPanesRef, data, { + shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously + }) } const writeReplayDataAsync = (data: string): Promise => { // Why: WebGL must be rebuilt after xterm has parsed replay bytes, not // merely after the write was queued. flushTerminalOutput(pane.terminal) - return replayIntoTerminalAsync(pane, deps.replayingPanesRef, data) + return replayIntoTerminalAsync(pane, deps.replayingPanesRef, data, { + shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously + }) } const reattachReplayResetSequence = (payload: string): string => { @@ -4742,7 +4750,9 @@ export function connectPanePty( modelRestoreSubscribedPtyId = null } - function beforeTerminalOutputWrite(): void { + function beforeTerminalOutputWrite(data: string): void { + // Why: shaping must register before xterm parses the RTL bytes that need it. + ensureArabicShapingJoinerForText(pane.terminal, data) recordTerminalOutput(pane.terminal) } @@ -5004,6 +5014,9 @@ export function connectPanePty( foregroundRenderRefreshNeeded), followupForegroundRefresh: nativeWindowsCursorRestore || nativeWindowsInPlaceRewriteFollowup, + // Why: xterm already queued a WebGL frame while parsing this chunk; + // merge the repair into it instead of rendering the full grid twice. + shouldRefreshForegroundSynchronously, onParsed: onParsedAtlasRecovery, stripTransientCursorShows: shouldProtectNativeWindowsSynchronizedOutput && foreground, coalesceForeground: synchronizedForegroundOutput && synchronizedOutputEnded, diff --git a/src/renderer/src/components/terminal-pane/replay-guard.test.ts b/src/renderer/src/components/terminal-pane/replay-guard.test.ts index c26acbfca7c..e69194db7cc 100644 --- a/src/renderer/src/components/terminal-pane/replay-guard.test.ts +++ b/src/renderer/src/components/terminal-pane/replay-guard.test.ts @@ -6,6 +6,7 @@ import { replayIntoTerminalAsync, type ReplayingPanesRef } from './replay-guard' +import { configureLazyArabicShapingJoiner } from '@/lib/pane-manager/terminal-arabic-shaping-joiner' const mocks = vi.hoisted(() => ({ recordRendererCrashBreadcrumb: vi.fn() @@ -41,6 +42,7 @@ type FakeTerminal = { _core: { refresh: (start: number, end: number, sync?: boolean) => void } + refresh: (start: number, end: number) => void /** Flush all pending xterm write callbacks, simulating parse completion. */ flush: () => void } @@ -60,6 +62,7 @@ function makeFakePane(paneId: number): { pane: ManagedPane; terminal: FakeTermin _core: { refresh() {} }, + refresh() {}, write(data: string, cb?: () => void) { terminal.lastData.push(data) if (cb) { @@ -83,6 +86,53 @@ describe('replay-guard', () => { expect(isPaneReplaying(ref, 1)).toBe(false) }) + it('registers Arabic shaping before replay bytes are written', () => { + const ref = makeRef() + const { pane, terminal } = makeFakePane(1) + const events: string[] = [] + const joinerTerminal = terminal as FakeTerminal & { + registerCharacterJoiner: (handler: (text: string) => [number, number][]) => number + deregisterCharacterJoiner: (joinerId: number) => void + } + joinerTerminal.registerCharacterJoiner = () => { + events.push('register') + return 5 + } + joinerTerminal.deregisterCharacterJoiner = () => undefined + terminal.write = (data: string, callback?: () => void) => { + events.push(`write:${data}`) + if (callback) { + terminal.pendingCallbacks.push(callback) + } + } + const cleanup = configureLazyArabicShapingJoiner(joinerTerminal as never, () => true) + + replayIntoTerminal(pane, ref, 'مرحبا') + + expect(events).toEqual(['register', 'write:مرحبا']) + cleanup() + }) + + it('still replays RTL bytes when joiner registration fails', () => { + const ref = makeRef() + const { pane, terminal } = makeFakePane(1) + const joinerTerminal = terminal as FakeTerminal & { + registerCharacterJoiner: () => number + deregisterCharacterJoiner: () => void + } + joinerTerminal.registerCharacterJoiner = () => { + throw new Error('terminal disposed') + } + joinerTerminal.deregisterCharacterJoiner = () => undefined + configureLazyArabicShapingJoiner(joinerTerminal as never, () => true) + + replayIntoTerminal(pane, ref, 'مرحبا') + + expect(terminal.lastData).toEqual(['مرحبا']) + terminal.flush() + expect(isPaneReplaying(ref, pane.id)).toBe(false) + }) + it('is replaying between write dispatch and xterm parse completion', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) @@ -168,9 +218,9 @@ describe('replay-guard', () => { try { const ref = makeRef() const { pane } = makeFakePane(1) - replayIntoTerminal(pane, ref, '\x1b[2J\x1b[3J\x1b[H', 400) - replayIntoTerminal(pane, ref, 'scrollback bytes', 400) - replayIntoTerminal(pane, ref, '--- session restored ---', 400) + replayIntoTerminal(pane, ref, '\x1b[2J\x1b[3J\x1b[H', { stallCheckMs: 400 }) + replayIntoTerminal(pane, ref, 'scrollback bytes', { stallCheckMs: 400 }) + replayIntoTerminal(pane, ref, '--- session restored ---', { stallCheckMs: 400 }) expect(isPaneReplaying(ref, 1)).toBe(true) // Never flush — the probe write never parses; the wedged release fires @@ -189,8 +239,8 @@ describe('replay-guard', () => { try { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'a', 400) - replayIntoTerminal(pane, ref, 'b', 400) + replayIntoTerminal(pane, ref, 'a', { stallCheckMs: 400 }) + replayIntoTerminal(pane, ref, 'b', { stallCheckMs: 400 }) terminal.flush() expect(isPaneReplaying(ref, 1)).toBe(false) @@ -210,7 +260,7 @@ describe('replay-guard', () => { const ref = makeRef() const { pane } = makeFakePane(1) let resolved = false - const promise = replayIntoTerminalAsync(pane, ref, 'x', 400).then(() => { + const promise = replayIntoTerminalAsync(pane, ref, 'x', { stallCheckMs: 400 }).then(() => { resolved = true }) expect(isPaneReplaying(ref, 1)).toBe(true) @@ -257,6 +307,42 @@ describe('replay-guard', () => { globalThis.cancelAnimationFrame = originalCancelAnimationFrame } }) + + it('coalesces WebGL replay refreshes and rechecks before the follow-up', () => { + const scheduledFrames: FrameRequestCallback[] = [] + const originalRequestAnimationFrame = globalThis.requestAnimationFrame + const originalCancelAnimationFrame = globalThis.cancelAnimationFrame + globalThis.requestAnimationFrame = ((callback: FrameRequestCallback) => { + scheduledFrames.push(callback) + return scheduledFrames.length + }) as typeof requestAnimationFrame + globalThis.cancelAnimationFrame = (() => {}) as typeof cancelAnimationFrame + + try { + const ref = makeRef() + const { pane, terminal } = makeFakePane(1) + const synchronousRefresh = vi.fn() + const debouncedRefresh = vi.fn() + terminal._core.refresh = synchronousRefresh + terminal.refresh = debouncedRefresh + let webglLive = true + + replayIntoTerminal(pane, ref, 'snapshot bytes', { + shouldRefreshViewportSynchronously: () => !webglLive + }) + terminal.flush() + + expect(debouncedRefresh).toHaveBeenCalledWith(0, 23) + expect(synchronousRefresh).not.toHaveBeenCalled() + webglLive = false + scheduledFrames[0]?.(16) + + expect(synchronousRefresh).toHaveBeenCalledWith(0, 23, true) + } finally { + globalThis.requestAnimationFrame = originalRequestAnimationFrame + globalThis.cancelAnimationFrame = originalCancelAnimationFrame + } + }) }) describe('replay-guard stall handling (probe-certified release)', () => { @@ -269,7 +355,7 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'slow but alive', 1_000) + replayIntoTerminal(pane, ref, 'slow but alive', { stallCheckMs: 1_000 }) expect(isPaneReplaying(ref, 1)).toBe(true) // Stall check fires: an empty probe write is enqueued behind the replay. @@ -298,7 +384,7 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'restored bytes', 1_000) + replayIntoTerminal(pane, ref, 'restored bytes', { stallCheckMs: 1_000 }) terminal.pendingCallbacks.shift() // xterm lost the replay's completion vi.advanceTimersByTime(1_000) // stall check → probe enqueued @@ -322,7 +408,7 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'restored bytes', 1_000) + replayIntoTerminal(pane, ref, 'restored bytes', { stallCheckMs: 1_000 }) vi.advanceTimersByTime(1_000) // stall check → probe enqueued expect(isPaneReplaying(ref, 1)).toBe(true) @@ -346,7 +432,7 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'restored bytes', 1_000) + replayIntoTerminal(pane, ref, 'restored bytes', { stallCheckMs: 1_000 }) terminal.write = () => { throw new Error('terminal disposed') } @@ -368,8 +454,8 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'lost completion', 1_000) - replayIntoTerminal(pane, ref, 'healthy completion', 60_000) + replayIntoTerminal(pane, ref, 'lost completion', { stallCheckMs: 1_000 }) + replayIntoTerminal(pane, ref, 'healthy completion', { stallCheckMs: 60_000 }) terminal.pendingCallbacks.shift() // drop only the first completion vi.advanceTimersByTime(1_000) // first engagement's probe enqueued @@ -389,7 +475,7 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane, terminal } = makeFakePane(1) - replayIntoTerminal(pane, ref, 'healthy', 1_000) + replayIntoTerminal(pane, ref, 'healthy', { stallCheckMs: 1_000 }) terminal.flush() expect(isPaneReplaying(ref, 1)).toBe(false) @@ -405,7 +491,9 @@ describe('replay-guard stall handling (probe-certified release)', () => { const ref = makeRef() const { pane } = makeFakePane(1) - const replayDone = replayIntoTerminalAsync(pane, ref, 'restored bytes', 1_000) + const replayDone = replayIntoTerminalAsync(pane, ref, 'restored bytes', { + stallCheckMs: 1_000 + }) let resolved = false void replayDone.then(() => { resolved = true diff --git a/src/renderer/src/components/terminal-pane/replay-guard.ts b/src/renderer/src/components/terminal-pane/replay-guard.ts index 6bf821ea62d..fa2cc1913be 100644 --- a/src/renderer/src/components/terminal-pane/replay-guard.ts +++ b/src/renderer/src/components/terminal-pane/replay-guard.ts @@ -1,6 +1,7 @@ import type { ManagedPane } from '@/lib/pane-manager/pane-manager' import { writeForegroundTerminalChunk } from '@/lib/pane-manager/pane-terminal-foreground-render-settle' import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' +import { ensureArabicShapingJoinerForText } from '@/lib/pane-manager/terminal-arabic-shaping-joiner' // Why: xterm.js auto-responds to terminal query sequences (DA1 `CSI c`, // DECRQM `CSI ? Ps $ p`, OSC 10/11 color queries, focus events, CPR) by @@ -52,6 +53,11 @@ export type ReplayingPanesRef = React.RefObject> // While the probe is pending (slow-but-alive replay), the guard HOLDS. const REPLAY_GUARD_STALL_CHECK_MS = 10_000 +type ReplayTerminalOptions = { + shouldRefreshViewportSynchronously?: () => boolean + stallCheckMs?: number +} + export function isPaneReplaying(ref: ReplayingPanesRef, paneId: number): boolean { return (ref.current.get(paneId) ?? 0) > 0 } @@ -130,22 +136,24 @@ export function replayIntoTerminal( pane: ManagedPane, replayingPanesRef: ReplayingPanesRef, data: string, - stallCheckMs: number = REPLAY_GUARD_STALL_CHECK_MS + options: ReplayTerminalOptions = {} ): void { if (!data) { return } + ensureArabicShapingJoinerForText(pane.terminal, data) const releaseParsed = engageReplayGuard( replayingPanesRef.current, pane.id, pane.terminal, - stallCheckMs + options.stallCheckMs ?? REPLAY_GUARD_STALL_CHECK_MS ) // Why: hidden/snapshot replay bypasses the live foreground write path, but // WebGL/canvas renderers still need a post-parse repaint to drop stale cells. writeForegroundTerminalChunk(pane.terminal, data, { forceViewportRefresh: true, followupViewportRefresh: true, + shouldRefreshViewportSynchronously: options.shouldRefreshViewportSynchronously, onParsed: releaseParsed }) } @@ -154,11 +162,12 @@ export function replayIntoTerminalAsync( pane: ManagedPane, replayingPanesRef: ReplayingPanesRef, data: string, - stallCheckMs: number = REPLAY_GUARD_STALL_CHECK_MS + options: ReplayTerminalOptions = {} ): Promise { if (!data) { return Promise.resolve() } + ensureArabicShapingJoinerForText(pane.terminal, data) return new Promise((resolve) => { // Why resolve on either release path: callers await this to sequence // restore steps; a lost write completion must not hang the restore chain. @@ -166,12 +175,13 @@ export function replayIntoTerminalAsync( replayingPanesRef.current, pane.id, pane.terminal, - stallCheckMs, + options.stallCheckMs ?? REPLAY_GUARD_STALL_CHECK_MS, resolve ) writeForegroundTerminalChunk(pane.terminal, data, { forceViewportRefresh: true, followupViewportRefresh: true, + shouldRefreshViewportSynchronously: options.shouldRefreshViewportSynchronously, onParsed: releaseParsed }) }) diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index c4cb7808225..4b0fef324ac 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -7,6 +7,7 @@ import { resetTerminalWebglSuggestion } from './pane-webgl-renderer' import { attachLigatures, disposePane, openTerminal } from './pane-lifecycle' +import { ensureArabicShapingJoinerForText } from './terminal-arabic-shaping-joiner' import { buildDefaultTerminalOptions, DEFAULT_TERMINAL_FAST_SCROLL_SENSITIVITY, @@ -585,17 +586,17 @@ describe('openTerminal — addon and provider wiring', () => { expect(events.indexOf('open')).toBeLessThan(loadUnicodeIdx) }) - // Why: terminal.dispose() does not deregister character joiners, so the - // pane lifecycle must — this locks the register/deregister pairing that - // makes Arabic/RTL shaping (#5262) actually reach a real terminal. - it('registers the Arabic shaping joiner on open and deregisters it on dispose', () => { + // Why: ordinary panes must avoid xterm's full-grid character-joiner scan, + // while the first RTL write still registers before xterm parses the text. + it('registers Arabic shaping lazily and deregisters it on dispose', () => { const { pane, events } = createOpenTerminalHarness() openTerminal(pane) - expect(events).toContain('registerCharacterJoiner') - expect(events.indexOf('open')).toBeLessThan(events.indexOf('registerCharacterJoiner')) + expect(events).not.toContain('registerCharacterJoiner') expect(pane.arabicShapingJoinerCleanup).toBeTypeOf('function') + ensureArabicShapingJoinerForText(pane.terminal, 'مرحبا') + expect(events).toContain('registerCharacterJoiner') disposePane(pane, new Map([[pane.id, pane]])) @@ -610,6 +611,7 @@ describe('openTerminal — addon and provider wiring', () => { const { pane, getRegisteredJoinHandler } = createOpenTerminalHarness() openTerminal(pane) + ensureArabicShapingJoinerForText(pane.terminal, 'مرحبا') const handler = getRegisteredJoinHandler()! expect(pane.webglAddon).toBeNull() diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index c984508d463..adc43af01c1 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -1,10 +1,3 @@ -// Upstream packaging bug: @xterm/addon-ligatures declares `"main": -// "lib/addon-ligatures.js"` but ships only the `.mjs` entry, so Vite fails to -// resolve the bare import. Fixed locally via config/patches/@xterm__addon-ligatures*. -// Tracking upstream: https://github.com/xtermjs/xterm.js/issues/5822 and -// https://github.com/xtermjs/xterm.js/pull/5828 — drop the patch once that lands. -import { LigaturesAddon } from '@xterm/addon-ligatures' - import type { ManagedPaneInternal } from './pane-manager-types' import { safeFit } from './pane-tree-ops' import { @@ -17,7 +10,8 @@ import { attachTerminalMouseWheelMultiplier } from './pane-terminal-mouse-wheel' import { attachTerminalScrollIntentTracking } from './terminal-scroll-intent' import { attachDomRendererFocusClassSync } from './pane-dom-focus-class-sync' import { attachWebgl, cancelPendingWebglRefresh, disposeWebgl } from './pane-webgl-renderer' -import { registerArabicShapingJoiner } from './terminal-arabic-shaping-joiner' +import { configureLazyArabicShapingJoiner } from './terminal-arabic-shaping-joiner' +import { TerminalLigaturesAddon } from './terminal-ligatures-addon' import { resolveCursorAgentImeAnchor } from './terminal-ime-anchor' // --------------------------------------------------------------------------- @@ -73,12 +67,10 @@ export function openTerminal(pane: ManagedPaneInternal): void { // so the activation must stay at this position. activateOrcaTerminalUnicodeProvider(terminal) - // Why: without run-joining, Arabic/Hebrew output renders as disconnected - // letters in reversed order (#5262). Registered up front so restored - // scrollback and reattach replays shape correctly, not just live output. - // Joining tracks the live WebGL renderer: the DOM fallback misrenders - // joined spans (see registerArabicShapingJoiner), so it stays per-cell. - pane.arabicShapingJoinerCleanup = registerArabicShapingJoiner( + // Why: any xterm character joiner makes every repaint scan the whole grid. + // Defer registration until the first RTL write; replay and live paths both + // ensure it before parsing, so restored Arabic still shapes immediately. + pane.arabicShapingJoinerCleanup = configureLazyArabicShapingJoiner( terminal, () => pane.webglAddon != null ) @@ -173,7 +165,7 @@ export function attachLigatures(pane: ManagedPaneInternal): void { return } try { - const ligaturesAddon = new LigaturesAddon() + const ligaturesAddon = new TerminalLigaturesAddon() pane.terminal.loadAddon(ligaturesAddon) pane.ligaturesAddon = ligaturesAddon // Why: ligatures can be enabled after rows already rendered, especially diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index f04d5d9c3e2..ab9ac757588 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -245,6 +245,10 @@ export class PaneManager { })) } + hasWebglRenderer(paneId: number): boolean { + return this.panes.get(paneId)?.webglAddon != null + } + getLeafId(numericPaneId: number): TerminalLeafId | null { return this.identities.getLeafId(numericPaneId) } diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-foreground-render-settle.ts b/src/renderer/src/lib/pane-manager/pane-terminal-foreground-render-settle.ts index eab1caa497d..5319e259a47 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-foreground-render-settle.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-foreground-render-settle.ts @@ -19,6 +19,7 @@ export type ForegroundTerminalOutputTarget = { type ForegroundTerminalWriteOptions = { forceViewportRefresh?: boolean followupViewportRefresh?: boolean + shouldRefreshViewportSynchronously?: () => boolean onParsed?: () => void } @@ -32,7 +33,10 @@ type ViewportSnapshot = { viewportY: number | null } -function refreshVisibleRowsNow(terminal: ForegroundTerminalOutputTarget): void { +function refreshVisibleRows( + terminal: ForegroundTerminalOutputTarget, + synchronously: boolean +): void { if (typeof terminal.rows !== 'number' || terminal.rows < 1) { return } @@ -40,14 +44,17 @@ function refreshVisibleRowsNow(terminal: ForegroundTerminalOutputTarget): void { const start = 0 const end = Math.max(0, terminal.rows - 1) try { - // Why: xterm's DOM renderer batches row paints; Windows ConPTY CR-style - // rewrites can leave stale CJK glyph cells until a resize unless we paint - // the parsed foreground state before Chromium's next frame. - if (typeof terminal._core?.refresh === 'function') { + // Why: DOM-rendered Windows ConPTY rewrites need an immediate repair, while + // WebGL can merge this full-grid request into xterm's already-queued frame. + if (synchronously && typeof terminal._core?.refresh === 'function') { terminal._core.refresh(start, end, true) return } - terminal.refresh?.(start, end) + if (typeof terminal.refresh === 'function') { + terminal.refresh(start, end) + return + } + terminal._core?.refresh?.(start, end, false) } catch { // Ignore disposed terminals; PTY output can race pane teardown. } @@ -90,12 +97,15 @@ function cancelScheduledViewportSettleRefresh(terminal: ForegroundTerminalOutput clearTimeout(pending.id) } -function scheduleViewportSettleRefresh(terminal: ForegroundTerminalOutputTarget): void { +function scheduleViewportSettleRefresh( + terminal: ForegroundTerminalOutputTarget, + shouldRefreshSynchronously?: () => boolean +): void { cancelScheduledViewportSettleRefresh(terminal) if (typeof requestAnimationFrame === 'function') { const id = requestAnimationFrame(() => { pendingViewportSettleRefreshByTerminal.delete(terminal) - refreshVisibleRowsNow(terminal) + refreshVisibleRows(terminal, shouldRefreshSynchronously?.() ?? true) }) pendingViewportSettleRefreshByTerminal.set(terminal, { kind: 'raf', id }) return @@ -103,7 +113,7 @@ function scheduleViewportSettleRefresh(terminal: ForegroundTerminalOutputTarget) const id = setTimeout(() => { pendingViewportSettleRefreshByTerminal.delete(terminal) - refreshVisibleRowsNow(terminal) + refreshVisibleRows(terminal, shouldRefreshSynchronously?.() ?? true) }, 16) pendingViewportSettleRefreshByTerminal.set(terminal, { kind: 'timeout', id }) } @@ -113,7 +123,7 @@ function settleForegroundRender( beforeWriteViewport: ViewportSnapshot, options: ForegroundTerminalWriteOptions ): void { - refreshVisibleRowsNow(terminal) + refreshVisibleRows(terminal, options.shouldRefreshViewportSynchronously?.() ?? true) // Why: when output advances the viewport, Chromium can paint the freshly // scrolled top row one frame later than xterm finishes parsing. Repaint once // more after the scroll settles so the user doesn't need to jiggle the window. @@ -121,7 +131,7 @@ function settleForegroundRender( options.followupViewportRefresh || viewportChangedDuringWrite(terminal, beforeWriteViewport) ) { - scheduleViewportSettleRefresh(terminal) + scheduleViewportSettleRefresh(terminal, options.shouldRefreshViewportSynchronously) } } diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index f85d9b8c56f..363264c9cb3 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -300,6 +300,93 @@ describe('pane terminal output scheduler', () => { expect(terminal.refresh).not.toHaveBeenCalled() }) + it('coalesces a WebGL foreground refresh through xterm public refresh', async () => { + const { writeTerminalOutput } = await loadScheduler() + const terminal = createForegroundTerminal() + + writeTerminalOutput(terminal, 'active TUI repaint\r\n', { + foreground: true, + forceForegroundRefresh: true, + shouldRefreshForegroundSynchronously: () => false + }) + + expect(terminal.refresh).toHaveBeenCalledWith(0, 23) + expect(terminal._core.refresh).not.toHaveBeenCalled() + }) + + it('resolves the live renderer after xterm finishes parsing', async () => { + const { writeTerminalOutput } = await loadScheduler() + const terminal = createForegroundTerminal() + let parseCallback: (() => void) | undefined + let webglLive = false + terminal.write.mockImplementation((_data: string, callback?: () => void) => { + parseCallback = callback + }) + + writeTerminalOutput(terminal, 'queued renderer transition\r\n', { + foreground: true, + forceForegroundRefresh: true, + shouldRefreshForegroundSynchronously: () => !webglLive + }) + webglLive = true + parseCallback?.() + + expect(terminal.refresh).toHaveBeenCalledWith(0, 23) + expect(terminal._core.refresh).not.toHaveBeenCalled() + }) + + it('keeps the WebGL follow-up repair on the debounced path', async () => { + const scheduledFrames: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + scheduledFrames.push(callback) + return scheduledFrames.length + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + const { writeTerminalOutput } = await loadScheduler() + const terminal = createForegroundTerminal() + + writeTerminalOutput(terminal, 'WebGL cursor restore', { + foreground: true, + forceForegroundRefresh: true, + followupForegroundRefresh: true, + shouldRefreshForegroundSynchronously: () => false + }) + + expect(terminal.refresh).toHaveBeenCalledTimes(1) + expect(scheduledFrames).toHaveLength(1) + scheduledFrames[0]?.(16) + + expect(terminal.refresh).toHaveBeenCalledTimes(2) + expect(terminal._core.refresh).not.toHaveBeenCalled() + }) + + it('resolves WebGL loss again before the follow-up repair', async () => { + const scheduledFrames: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + scheduledFrames.push(callback) + return scheduledFrames.length + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + const { writeTerminalOutput } = await loadScheduler() + const terminal = createForegroundTerminal() + let webglLive = true + + writeTerminalOutput(terminal, 'renderer transition', { + foreground: true, + forceForegroundRefresh: true, + followupForegroundRefresh: true, + shouldRefreshForegroundSynchronously: () => !webglLive + }) + + expect(terminal.refresh).toHaveBeenCalledTimes(1) + webglLive = false + scheduledFrames[0]?.(16) + + expect(terminal._core.refresh).toHaveBeenCalledWith(0, 23, true) + }) + it('repaints the viewport again on the next frame when foreground output scrolls', async () => { const scheduledFrames: FrameRequestCallback[] = [] vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { @@ -916,6 +1003,41 @@ describe('pane terminal output scheduler', () => { expect(terminal.write).toHaveBeenCalledWith('ab') }) + it('keeps preparation attached when a later producer omits it', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + const beforeWrite = vi.fn() + + writeTerminalOutput(terminal, 'مرحبا', { foreground: false, beforeWrite }) + writeTerminalOutput(terminal, ' fallback notice', { foreground: false }) + vi.advanceTimersByTime(50) + + expect(beforeWrite).toHaveBeenCalledWith('مرحبا fallback notice') + expect(terminal.write).toHaveBeenCalledWith('مرحبا fallback notice') + }) + + it('ignores unforced chunks when resolving a coalesced forced refresh', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createForegroundTerminal() + + writeTerminalOutput(terminal, 'forced', { + foreground: true, + latencySensitive: false, + forceForegroundRefresh: true, + shouldRefreshForegroundSynchronously: () => false + }) + writeTerminalOutput(terminal, ' ordinary', { + foreground: true, + latencySensitive: false + }) + vi.advanceTimersByTime(0) + + expect(terminal.refresh).toHaveBeenCalledWith(0, 23) + expect(terminal._core.refresh).not.toHaveBeenCalled() + }) + it('runs deferred write preparation before explicit background flushes', async () => { vi.useFakeTimers() const { flushTerminalOutput, writeTerminalOutput } = await loadScheduler() diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 79ddae6e086..e12eeaccb89 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -27,6 +27,7 @@ type TerminalOutputTarget = ForegroundTerminalOutputTarget type TerminalOutputBeforeWrite = (data: string) => void type TerminalBacklogRecoveryRequest = () => boolean type TerminalOutputParsedCallback = () => void +type ForegroundRefreshSyncResolver = () => boolean type WriteTerminalOutputOptions = { foreground: boolean @@ -42,6 +43,7 @@ type WriteTerminalOutputOptions = { latencySensitive?: boolean forceForegroundRefresh?: boolean followupForegroundRefresh?: boolean + shouldRefreshForegroundSynchronously?: ForegroundRefreshSyncResolver stripTransientCursorShows?: boolean coalesceForeground?: boolean holdForeground?: boolean @@ -52,7 +54,9 @@ type QueueChunk = { foreground: boolean forceForegroundRefresh: boolean followupForegroundRefresh: boolean + shouldRefreshForegroundSynchronously: ForegroundRefreshSyncResolver stripTransientCursorShows: boolean + beforeWrite?: TerminalOutputBeforeWrite onParsed?: TerminalOutputParsedCallback ackCredit?: () => void } @@ -62,7 +66,9 @@ type QueuedWrite = { foreground: boolean forceForegroundRefresh: boolean followupForegroundRefresh: boolean + shouldRefreshForegroundSynchronously: ForegroundRefreshSyncResolver stripTransientCursorShows: boolean + beforeWrite?: TerminalOutputBeforeWrite onParsed?: TerminalOutputParsedCallback ackCredits: (() => void)[] } @@ -72,7 +78,6 @@ type QueueEntry = { chunks: QueueChunk[] chunkIndex: number queuedChars: number - beforeWrite?: TerminalOutputBeforeWrite onBackgroundBacklogDropped?: () => void backgroundBacklogDropped: boolean highPriority: boolean @@ -129,6 +134,7 @@ const BACKGROUND_BACKLOG_WARNING = // skipped, not merely produced while hidden. const FOREGROUND_BACKLOG_WARNING = '\x18\x1b[0m\r\n[Orca skipped a burst of terminal output because the backlog grew too large.]\r\n' +const ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY = (): boolean => true const queuedByTerminal = new Map() const backlogRecoveryByTerminal = new WeakMap< @@ -334,7 +340,6 @@ function createQueueEntry( chunks: [], chunkIndex: 0, queuedChars: 0, - beforeWrite: options.beforeWrite, onBackgroundBacklogDropped: options.onBackgroundBacklogDropped, backgroundBacklogDropped: false, highPriority: true, @@ -571,7 +576,11 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { let foreground: boolean | null = null let forceForegroundRefresh = false let followupForegroundRefresh = false + let shouldRefreshForegroundSynchronously: ForegroundRefreshSyncResolver | null = null + let additionalRefreshSyncResolvers: ForegroundRefreshSyncResolver[] | null = null let stripTransientCursorShows = false + let beforeWrite: TerminalOutputBeforeWrite | undefined + let additionalBeforeWriteCallbacks: TerminalOutputBeforeWrite[] | null = null const parsedCallbacks: TerminalOutputParsedCallback[] = [] const ackCredits: (() => void)[] = [] @@ -583,7 +592,30 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { foreground ??= chunk.foreground forceForegroundRefresh ||= chunk.forceForegroundRefresh followupForegroundRefresh ||= chunk.followupForegroundRefresh + // Why: one drained write can combine chunks from different renderer + // states or producers; preserve every forced policy and preparation hook. + if (chunk.forceForegroundRefresh) { + if (shouldRefreshForegroundSynchronously === null) { + shouldRefreshForegroundSynchronously = chunk.shouldRefreshForegroundSynchronously + } else if ( + chunk.shouldRefreshForegroundSynchronously !== shouldRefreshForegroundSynchronously && + !additionalRefreshSyncResolvers?.includes(chunk.shouldRefreshForegroundSynchronously) + ) { + additionalRefreshSyncResolvers ??= [] + additionalRefreshSyncResolvers.push(chunk.shouldRefreshForegroundSynchronously) + } + } stripTransientCursorShows ||= chunk.stripTransientCursorShows + if (!beforeWrite) { + beforeWrite = chunk.beforeWrite + } else if ( + chunk.beforeWrite && + chunk.beforeWrite !== beforeWrite && + !additionalBeforeWriteCallbacks?.includes(chunk.beforeWrite) + ) { + additionalBeforeWriteCallbacks ??= [] + additionalBeforeWriteCallbacks.push(chunk.beforeWrite) + } if (chunk.data.length <= remaining) { data += chunk.data remaining -= chunk.data.length @@ -618,7 +650,22 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { foreground: foreground === true, forceForegroundRefresh, followupForegroundRefresh, + shouldRefreshForegroundSynchronously: + additionalRefreshSyncResolvers && shouldRefreshForegroundSynchronously + ? () => + shouldRefreshForegroundSynchronously() || + additionalRefreshSyncResolvers.some((resolve) => resolve()) + : (shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY), stripTransientCursorShows, + beforeWrite: + additionalBeforeWriteCallbacks && beforeWrite + ? (queuedData) => { + beforeWrite(queuedData) + for (const callback of additionalBeforeWriteCallbacks) { + callback(queuedData) + } + } + : beforeWrite, onParsed: parsedCallbacks.length > 0 ? () => { @@ -654,7 +701,9 @@ function enqueueChunk( foreground?: boolean forceForegroundRefresh?: boolean followupForegroundRefresh?: boolean + shouldRefreshForegroundSynchronously?: ForegroundRefreshSyncResolver stripTransientCursorShows?: boolean + beforeWrite?: TerminalOutputBeforeWrite onParsed?: TerminalOutputParsedCallback ackCredit?: () => void } @@ -664,7 +713,10 @@ function enqueueChunk( foreground: options?.foreground === true, forceForegroundRefresh: options?.forceForegroundRefresh === true, followupForegroundRefresh: options?.followupForegroundRefresh === true, + shouldRefreshForegroundSynchronously: + options?.shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, stripTransientCursorShows: options?.stripTransientCursorShows === true, + beforeWrite: options?.beforeWrite, onParsed: options?.onParsed, ackCredit: options?.ackCredit }) @@ -703,6 +755,13 @@ function replaceBacklogWithWarning( capChars: maxQueueChars }) } + let beforeWrite: TerminalOutputBeforeWrite | undefined + for (let index = entry.chunks.length - 1; index >= entry.chunkIndex; index--) { + if (entry.chunks[index]?.beforeWrite) { + beforeWrite = entry.chunks[index].beforeWrite + break + } + } clearForegroundHoldSafety(entry) fireQueuedAckCredits(entry) entry.chunks = [ @@ -711,7 +770,9 @@ function replaceBacklogWithWarning( foreground: false, forceForegroundRefresh: false, followupForegroundRefresh: false, - stripTransientCursorShows: false + shouldRefreshForegroundSynchronously: ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, + stripTransientCursorShows: false, + beforeWrite } ] entry.chunkIndex = 0 @@ -795,6 +856,7 @@ function writeForegroundTerminalChunkWithIntent( options: { forceViewportRefresh: boolean followupViewportRefresh: boolean + shouldRefreshViewportSynchronously: ForegroundRefreshSyncResolver onParsed?: TerminalOutputParsedCallback } ): void { @@ -802,6 +864,7 @@ function writeForegroundTerminalChunkWithIntent( writeForegroundTerminalChunk(terminal, data, { forceViewportRefresh: options.forceViewportRefresh, followupViewportRefresh: options.followupViewportRefresh, + shouldRefreshViewportSynchronously: options.shouldRefreshViewportSynchronously, onParsed: () => { // Why: recovery must repaint from the scrolled buffer state that xterm // will keep, not from a pre-intent-restored viewport snapshot. @@ -886,7 +949,7 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null const pacer = entry.highPriority ? makeParseClockPacer() : undefined const ackCreditsParsed = registerTerminalOutputAckCredits(entry.terminal, queuedWrite.ackCredits) try { - entry.beforeWrite?.(queuedWrite.data) + queuedWrite.beforeWrite?.(queuedWrite.data) if (queuedWrite.foreground) { writeForegroundTerminalChunkWithIntent( entry.terminal, @@ -896,6 +959,7 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null { forceViewportRefresh: queuedWrite.forceForegroundRefresh, followupViewportRefresh: queuedWrite.followupForegroundRefresh, + shouldRefreshViewportSynchronously: queuedWrite.shouldRefreshForegroundSynchronously, onParsed: composeParsedCallback(queuedWrite.onParsed, ackCreditsParsed, pacer) } ) @@ -1007,7 +1071,6 @@ export function writeTerminalOutput( const entry = queuedByTerminal.get(terminal) if (entry?.highPriority || options.coalesceForeground || options.holdForeground) { const queued = entry ?? createQueueEntry(terminal, options) - queued.beforeWrite = options.beforeWrite queued.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped queued.highPriority = true queuedByTerminal.set(terminal, queued) @@ -1015,7 +1078,9 @@ export function writeTerminalOutput( foreground: true, forceForegroundRefresh: options.forceForegroundRefresh, followupForegroundRefresh: options.followupForegroundRefresh, + shouldRefreshForegroundSynchronously: options.shouldRefreshForegroundSynchronously, stripTransientCursorShows: options.stripTransientCursorShows, + beforeWrite: options.beforeWrite, onParsed: options.onParsed, ackCredit: options.ackCredit }) @@ -1084,13 +1149,14 @@ export function writeTerminalOutput( return } if (entry && entry.queuedChars > SYNC_FOREGROUND_FLUSH_CHARS) { - entry.beforeWrite = options.beforeWrite entry.highPriority = true enqueueChunk(entry, data, { foreground: true, forceForegroundRefresh: options.forceForegroundRefresh, followupForegroundRefresh: options.followupForegroundRefresh, + shouldRefreshForegroundSynchronously: options.shouldRefreshForegroundSynchronously, stripTransientCursorShows: options.stripTransientCursorShows, + beforeWrite: options.beforeWrite, onParsed: options.onParsed, ackCredit: options.ackCredit }) @@ -1113,7 +1179,6 @@ export function writeTerminalOutput( queued = createQueueEntry(terminal, options) queuedByTerminal.set(terminal, queued) } else { - queued.beforeWrite = options.beforeWrite queued.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped queued.highPriority = true } @@ -1121,7 +1186,9 @@ export function writeTerminalOutput( foreground: true, forceForegroundRefresh: options.forceForegroundRefresh, followupForegroundRefresh: options.followupForegroundRefresh, + shouldRefreshForegroundSynchronously: options.shouldRefreshForegroundSynchronously, stripTransientCursorShows: options.stripTransientCursorShows, + beforeWrite: options.beforeWrite, onParsed: options.onParsed, ackCredit: options.ackCredit }) @@ -1154,6 +1221,8 @@ export function writeTerminalOutput( { forceViewportRefresh: options.forceForegroundRefresh === true, followupViewportRefresh: options.followupForegroundRefresh === true, + shouldRefreshViewportSynchronously: + options.shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, onParsed: composeParsedCallback(options.onParsed, ackCreditsParsed, undefined) } ) @@ -1172,10 +1241,10 @@ export function writeTerminalOutput( entry.highPriority = false queuedByTerminal.set(terminal, entry) } else { - entry.beforeWrite = options.beforeWrite entry.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped } enqueueChunk(entry, data, { + beforeWrite: options.beforeWrite, onParsed: options.onParsed, ackCredit: options.ackCredit }) @@ -1228,7 +1297,7 @@ export function flushTerminalOutput( } const ackCreditsParsed = registerTerminalOutputAckCredits(terminal, queuedWrite.ackCredits) try { - entry.beforeWrite?.(queuedWrite.data) + queuedWrite.beforeWrite?.(queuedWrite.data) if (queuedWrite.foreground) { writeForegroundTerminalChunkWithIntent( terminal, @@ -1238,6 +1307,7 @@ export function flushTerminalOutput( { forceViewportRefresh: queuedWrite.forceForegroundRefresh, followupViewportRefresh: queuedWrite.followupForegroundRefresh, + shouldRefreshViewportSynchronously: queuedWrite.shouldRefreshForegroundSynchronously, onParsed: composeParsedCallback(queuedWrite.onParsed, ackCreditsParsed, undefined) } ) diff --git a/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.test.ts b/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.test.ts index 6bed3d7c01d..6895d629f87 100644 --- a/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.test.ts +++ b/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest' import { + configureLazyArabicShapingJoiner, + ensureArabicShapingJoinerForText, findRtlJoinRanges, isStrongRtlCodePoint, registerArabicShapingJoiner @@ -248,3 +250,75 @@ describe('registerArabicShapingJoiner', () => { expect(handler('مرحبا')).toEqual([[0, 5]]) }) }) + +describe('configureLazyArabicShapingJoiner', () => { + function createLazyHost() { + const events: string[] = [] + let handler: ((text: string) => [number, number][]) | null = null + const terminal = { + registerCharacterJoiner(nextHandler: (text: string) => [number, number][]): number { + events.push('register') + handler = nextHandler + return 11 + }, + deregisterCharacterJoiner(joinerId: number): void { + events.push(`deregister:${joinerId}`) + } + } + return { events, terminal, getHandler: () => handler } + } + + it('does not register for ordinary terminal output', () => { + const host = createLazyHost() + const cleanup = configureLazyArabicShapingJoiner(host.terminal, () => true) + + ensureArabicShapingJoinerForText(host.terminal, 'ASCII, 中文, and emoji 😀') + + expect(host.events).toEqual([]) + expect(host.getHandler()).toBeNull() + cleanup() + expect(host.events).toEqual([]) + }) + + it('registers once before the first RTL write and cleans it up', () => { + const host = createLazyHost() + const cleanup = configureLazyArabicShapingJoiner(host.terminal, () => true) + + ensureArabicShapingJoinerForText(host.terminal, 'مرحبا') + ensureArabicShapingJoinerForText(host.terminal, 'שלום') + + expect(host.events).toEqual(['register']) + expect(host.getHandler()!('مرحبا')).toEqual([[0, 5]]) + cleanup() + expect(host.events).toEqual(['register', 'deregister:11']) + }) + + it('recognizes a supplementary-plane RTL code point split across writes', () => { + const host = createLazyHost() + configureLazyArabicShapingJoiner(host.terminal, () => true) + const adlam = String.fromCodePoint(0x1e900) + + ensureArabicShapingJoinerForText(host.terminal, adlam.charAt(0)) + expect(host.events).toEqual([]) + ensureArabicShapingJoinerForText(host.terminal, adlam.charAt(1)) + + expect(host.events).toEqual(['register']) + }) + + it('contains a registration failure and does not retry every write', () => { + let attempts = 0 + const terminal = { + registerCharacterJoiner(): number { + attempts++ + throw new Error('terminal disposed') + }, + deregisterCharacterJoiner(): void {} + } + configureLazyArabicShapingJoiner(terminal, () => true) + + expect(() => ensureArabicShapingJoinerForText(terminal, 'مرحبا')).not.toThrow() + expect(() => ensureArabicShapingJoinerForText(terminal, 'שלום')).not.toThrow() + + expect(attempts).toBe(1) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.ts b/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.ts index 5b9b5f8dc95..802f708b8e4 100644 --- a/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.ts +++ b/src/renderer/src/lib/pane-manager/terminal-arabic-shaping-joiner.ts @@ -1,5 +1,19 @@ import type { Terminal } from '@xterm/xterm' +type ArabicShapingTerminal = Pick + +type LazyArabicShapingJoinerState = { + cleanup: (() => void) | null + isShapingActive: () => boolean + registrationAttempted: boolean + trailingHighSurrogate: string +} + +const lazyArabicShapingJoinerByTerminal = new WeakMap< + ArabicShapingTerminal, + LazyArabicShapingJoinerState +>() + // Why: xterm draws every cell's glyph in isolation, so Arabic output shows // disconnected letterforms in logical (reversed) order — upstream has no // BiDi/shaping support (xtermjs/xterm.js#701, Orca #5262). Joining each RTL @@ -29,6 +43,27 @@ export function isStrongRtlCodePoint(codePoint: number): boolean { ) } +function containsStrongRtlText(text: string): boolean { + for (let index = 0; index < text.length; index++) { + const unit = text.charCodeAt(index) + if (unit < RTL_SCAN_FLOOR) { + continue + } + let codePoint = unit + if (unit >= 0xd800 && unit <= 0xdbff && index + 1 < text.length) { + const low = text.charCodeAt(index + 1) + if (low >= 0xdc00 && low <= 0xdfff) { + codePoint = (unit - 0xd800) * 0x400 + (low - 0xdc00) + 0x10000 + index++ + } + } + if (isStrongRtlCodePoint(codePoint)) { + return true + } + } + return false +} + // Neutral characters may sit inside an RTL run (so a multi-word phrase joins // as one unit and keeps right-to-left word order) but never start or end one: // ASCII space/digits/punctuation and NBSP. ASCII letters are strong LTR and @@ -169,7 +204,7 @@ export function findRtlJoinRanges(text: string): [number, number][] { * character joiners, so disposePane() must call this to avoid leaking the * registration (xtermjs/xterm.js#3289). */ export function registerArabicShapingJoiner( - terminal: Pick, + terminal: ArabicShapingTerminal, isShapingActive: () => boolean ): () => void { // Why: the DOM renderer sizes a joined span with one letter-spacing value @@ -185,3 +220,70 @@ export function registerArabicShapingJoiner( terminal.deregisterCharacterJoiner(joinerId) } } + +/** Configure RTL shaping without registering an xterm character joiner until + * output actually contains RTL text. Any registered joiner makes xterm scan + * every visible cell on every repaint, even when its handler returns no ranges. */ +export function configureLazyArabicShapingJoiner( + terminal: ArabicShapingTerminal, + isShapingActive: () => boolean +): () => void { + const previousState = lazyArabicShapingJoinerByTerminal.get(terminal) + try { + previousState?.cleanup?.() + } catch { + // A disposed terminal can reject deregistration; replace stale state anyway. + } + + const state: LazyArabicShapingJoinerState = { + cleanup: null, + isShapingActive, + registrationAttempted: false, + trailingHighSurrogate: '' + } + lazyArabicShapingJoinerByTerminal.set(terminal, state) + + return () => { + if (lazyArabicShapingJoinerByTerminal.get(terminal) !== state) { + return + } + try { + state.cleanup?.() + } catch { + // Pane teardown must continue if xterm disposed before deregistration. + } finally { + lazyArabicShapingJoinerByTerminal.delete(terminal) + } + } +} + +/** Register the configured joiner immediately before the first RTL write. */ +export function ensureArabicShapingJoinerForText( + terminal: ArabicShapingTerminal, + text: string +): void { + const state = lazyArabicShapingJoinerByTerminal.get(terminal) + if (!state || state.cleanup || state.registrationAttempted) { + return + } + + // Why: PTY/replay chunks can split supplementary-plane RTL code points + // between their surrogate halves; retain only that one boundary code unit. + const scanText = state.trailingHighSurrogate + text + const finalCharacter = scanText.at(-1) ?? '' + const finalCodeUnit = finalCharacter.charCodeAt(0) + state.trailingHighSurrogate = + finalCodeUnit >= 0xd800 && finalCodeUnit <= 0xdbff ? finalCharacter : '' + if (!containsStrongRtlText(scanText)) { + return + } + + state.trailingHighSurrogate = '' + state.registrationAttempted = true + try { + state.cleanup = registerArabicShapingJoiner(terminal, state.isShapingActive) + } catch { + // Why: shaping is optional; a registration race with pane disposal must + // never drop the PTY/replay bytes that triggered it or retry every chunk. + } +} diff --git a/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.test.ts b/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.test.ts new file mode 100644 index 00000000000..75f52f3f4f8 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.test.ts @@ -0,0 +1,154 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Terminal } from '@xterm/xterm' + +const addonMock = vi.hoisted(() => ({ + delegateTerminal: null as Terminal | null, + joiner: vi.fn<(text: string) => [number, number][]>() +})) + +vi.mock('@xterm/addon-ligatures', () => ({ + LigaturesAddon: class { + private joinerId: number | null = null + + activate(terminal: Terminal): void { + addonMock.delegateTerminal = terminal + this.joinerId = terminal.registerCharacterJoiner(addonMock.joiner) + } + + dispose(): void { + if (this.joinerId !== null) { + addonMock.delegateTerminal?.deregisterCharacterJoiner(this.joinerId) + } + } + } +})) + +import { TerminalLigaturesAddon } from './terminal-ligatures-addon' + +function createTerminalHarness() { + let registeredJoiner: ((text: string) => [number, number][]) | null = null + const refresh = vi.fn() + const deregisterCharacterJoiner = vi.fn() + const terminal = { + element: { style: {} }, + options: { fontFamily: 'Fira Code' }, + refresh, + registerCharacterJoiner(joiner: (text: string) => [number, number][]): number { + registeredJoiner = joiner + return 17 + }, + deregisterCharacterJoiner + } as unknown as Terminal + return { + terminal, + refresh, + deregisterCharacterJoiner, + getRegisteredJoiner: () => registeredJoiner! + } +} + +describe('TerminalLigaturesAddon', () => { + beforeEach(() => { + addonMock.delegateTerminal = null + addonMock.joiner.mockReset() + addonMock.joiner.mockImplementation((text) => (text.includes('=>') ? [[2, 4]] : [])) + }) + + it('reuses joiner results for unchanged row text', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + + expect(joiner('a => b')).toEqual([[2, 4]]) + expect(joiner('a => b')).toEqual([[2, 4]]) + + expect(addonMock.joiner).toHaveBeenCalledTimes(1) + }) + + it('returns fresh tuples because xterm mutates joiner ranges', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + + const first = joiner('a => b') + first[0]![0] = 99 + + expect(joiner('a => b')).toEqual([[2, 4]]) + }) + + it('invalidates fallback results when font discovery refreshes', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + joiner('a => b') + + addonMock.delegateTerminal!.refresh(0, 23) + joiner('a => b') + + expect(harness.refresh).toHaveBeenCalledWith(0, 23) + expect(addonMock.joiner).toHaveBeenCalledTimes(2) + }) + + it('does not reuse results after the terminal font changes', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + joiner('a => b') + + harness.terminal.options.fontFamily = 'JetBrains Mono' + joiner('a => b') + + expect(addonMock.joiner).toHaveBeenCalledTimes(2) + }) + + it('caps cached short segments by entry count', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + + for (let index = 0; index <= 2_048; index++) { + joiner(`s${index}`) + } + joiner('s0') + + expect(addonMock.joiner).toHaveBeenCalledTimes(2_050) + }) + + it('evicts the least-recently-used segment', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + + for (let index = 0; index < 2_048; index++) { + joiner(`s${index}`) + } + joiner('s0') + joiner('new segment') + joiner('s1') + joiner('s0') + + expect(addonMock.joiner).toHaveBeenCalledTimes(2_050) + }) + + it('does not retain a segment above the character budget', () => { + const harness = createTerminalHarness() + new TerminalLigaturesAddon().activate(harness.terminal) + const joiner = harness.getRegisteredJoiner() + const oversizedSegment = 'x'.repeat(100_001) + + joiner(oversizedSegment) + joiner(oversizedSegment) + + expect(addonMock.joiner).toHaveBeenCalledTimes(2) + }) + + it('deregisters the wrapped joiner through the real terminal', () => { + const harness = createTerminalHarness() + const addon = new TerminalLigaturesAddon() + addon.activate(harness.terminal) + + addon.dispose() + + expect(harness.deregisterCharacterJoiner).toHaveBeenCalledWith(17) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.ts b/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.ts new file mode 100644 index 00000000000..256f38befdc --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-ligatures-addon.ts @@ -0,0 +1,120 @@ +// Upstream packaging bug: @xterm/addon-ligatures declares a missing module +// entry. config/patches/@xterm__addon-ligatures* keeps the runtime import valid. +import { LigaturesAddon } from '@xterm/addon-ligatures' +import type { Terminal } from '@xterm/xterm' + +type LigatureRange = [number, number] +type CharacterJoiner = (text: string) => LigatureRange[] + +const LIGATURE_CACHE_CHARACTER_BUDGET = 100_000 +// Why: short attribute segments can otherwise create tens of thousands of Map +// entries per pane; 2K is still far beyond one visible grid's working set. +const LIGATURE_CACHE_ENTRY_BUDGET = 2_048 + +function cloneRanges(ranges: readonly LigatureRange[]): LigatureRange[] { + return ranges.map(([start, end]) => [start, end]) +} + +class LigatureRangeCache { + private readonly entries = new Map() + private cachedCharacters = 0 + generation = 0 + + get(text: string): LigatureRange[] | undefined { + const entry = this.entries.get(text) + if (!entry) { + return undefined + } + this.entries.delete(text) + this.entries.set(text, entry) + // Why: xterm translates and merges joiner ranges in place, so callers must + // never receive the cache's retained tuples. + return cloneRanges(entry.ranges) + } + + set(text: string, ranges: readonly LigatureRange[]): void { + if (text.length > LIGATURE_CACHE_CHARACTER_BUDGET) { + return + } + const previous = this.entries.get(text) + if (previous) { + this.cachedCharacters -= previous.size + this.entries.delete(text) + } + const entry = { ranges: cloneRanges(ranges), size: text.length } + this.entries.set(text, entry) + this.cachedCharacters += entry.size + while ( + this.cachedCharacters > LIGATURE_CACHE_CHARACTER_BUDGET || + this.entries.size > LIGATURE_CACHE_ENTRY_BUDGET + ) { + const oldest = this.entries.entries().next().value as + | [string, { ranges: LigatureRange[]; size: number }] + | undefined + if (!oldest) { + break + } + this.entries.delete(oldest[0]) + this.cachedCharacters -= oldest[1].size + } + } + + clear(): void { + this.entries.clear() + this.cachedCharacters = 0 + this.generation++ + } +} + +function createCachedCharacterJoiner( + terminal: Terminal, + joiner: CharacterJoiner, + cache: LigatureRangeCache +): CharacterJoiner { + let cachedFontFamily = terminal.options.fontFamily + return (text) => { + const fontFamily = terminal.options.fontFamily + if (fontFamily !== cachedFontFamily) { + cachedFontFamily = fontFamily + cache.clear() + } + const cached = cache.get(text) + if (cached) { + return cached + } + const generationBeforeJoin = cache.generation + const ranges = joiner(text) + // Why: the addon refreshes when async font discovery completes. If that + // happened during this call, do not repopulate the cleared fallback data. + if (cache.generation === generationBeforeJoin) { + cache.set(text, ranges) + } + return ranges + } +} + +/** LigaturesAddon with a bounded exact-row cache around its character joiner. + * Active TUIs repaint mostly unchanged rows, while the upstream fallback + * matcher otherwise retries every known ligature at every character. */ +export class TerminalLigaturesAddon extends LigaturesAddon { + override activate(terminal: Terminal): void { + const cache = new LigatureRangeCache() + const terminalForAddon = new Proxy(terminal, { + get(target, property) { + if (property === 'registerCharacterJoiner') { + return (joiner: CharacterJoiner): number => + target.registerCharacterJoiner(createCachedCharacterJoiner(target, joiner, cache)) + } + if (property === 'refresh') { + return (start: number, end: number): void => { + cache.clear() + target.refresh(start, end) + } + } + const value = Reflect.get(target, property, target) as unknown + return typeof value === 'function' ? value.bind(target) : value + } + }) + super.activate(terminalForAddon) + } +} diff --git a/tests/e2e/terminal-foreground-redraw-freeze.spec.ts b/tests/e2e/terminal-foreground-redraw-freeze.spec.ts index fe32e0236e0..fd6fad6f42d 100644 --- a/tests/e2e/terminal-foreground-redraw-freeze.spec.ts +++ b/tests/e2e/terminal-foreground-redraw-freeze.spec.ts @@ -41,11 +41,18 @@ type SchedulerDebugWindow = Window & { type RefreshProbeWindow = SchedulerDebugWindow & { __terminalRefreshProbe?: { - count: () => number + snapshot: () => RefreshProbeSnapshot dispose: () => void } } +type RefreshProbeSnapshot = { + synchronousWebgl: number + synchronousDom: number + debouncedWebgl: number + debouncedDom: number +} + const REDRAW_FRAME_COUNT = 270 const REDRAW_PAYLOAD_CHARS = 520 const REWRITE_REDRAW_FRAME_COUNT = REDRAW_FRAME_COUNT @@ -172,32 +179,115 @@ async function installActivePaneRefreshProbe(page: Page): Promise { } const terminal = pane.terminal as unknown as { _core?: { refresh?: (start: number, end: number, sync?: boolean) => void } + refresh: (start: number, end: number) => void } const originalCoreRefresh = terminal._core?.refresh?.bind(terminal._core) + const originalPublicRefresh = terminal.refresh.bind(terminal) if (!terminal._core || !originalCoreRefresh) { throw new Error('Active terminal core refresh hook is unavailable') } - let refreshCount = 0 + let synchronousWebgl = 0 + let synchronousDom = 0 + let debouncedWebgl = 0 + let debouncedDom = 0 terminal._core.refresh = (start, end, sync) => { if (sync === true) { - refreshCount += 1 + if (manager.hasWebglRenderer(pane.id)) { + synchronousWebgl += 1 + } else { + synchronousDom += 1 + } } originalCoreRefresh(start, end, sync) } + terminal.refresh = (start, end) => { + if (manager.hasWebglRenderer(pane.id)) { + debouncedWebgl += 1 + } else { + debouncedDom += 1 + } + originalPublicRefresh(start, end) + } ;(window as RefreshProbeWindow).__terminalRefreshProbe = { - count: () => refreshCount, + snapshot: () => ({ + synchronousWebgl, + synchronousDom, + debouncedWebgl, + debouncedDom + }), dispose: () => { if (terminal._core) { terminal._core.refresh = originalCoreRefresh } + terminal.refresh = originalPublicRefresh delete (window as RefreshProbeWindow).__terminalRefreshProbe } } }) } -async function readRefreshProbeCount(page: Page): Promise { - return page.evaluate(() => (window as RefreshProbeWindow).__terminalRefreshProbe?.count() ?? 0) +async function forceActivePaneWebglRenderer(page: Page): Promise { + await page.evaluate(() => { + const state = window.__store?.getState() + if (!state?.settings) { + throw new Error('Store unavailable') + } + window.__store?.setState({ + settings: { ...state.settings, terminalGpuAcceleration: 'on' } + }) + const worktreeId = state.activeWorktreeId + const tabId = + state.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + window.__paneManagers?.get(tabId ?? '')?.setTerminalGpuAcceleration?.('on') + }) + return page + .waitForFunction( + () => { + const state = window.__store?.getState() + const worktreeId = state?.activeWorktreeId + const tabId = + state?.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state?.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + const manager = tabId ? window.__paneManagers?.get(tabId) : null + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + return pane ? manager?.hasWebglRenderer(pane.id) === true : false + }, + null, + { timeout: 10_000 } + ) + .then(() => true) + .catch(() => false) +} + +async function readRefreshProbe(page: Page): Promise { + return page.evaluate( + () => + (window as RefreshProbeWindow).__terminalRefreshProbe?.snapshot() ?? { + synchronousWebgl: 0, + synchronousDom: 0, + debouncedWebgl: 0, + debouncedDom: 0 + } + ) +} + +function subtractRefreshProbe( + current: RefreshProbeSnapshot, + baseline: RefreshProbeSnapshot +): RefreshProbeSnapshot { + return { + synchronousWebgl: current.synchronousWebgl - baseline.synchronousWebgl, + synchronousDom: current.synchronousDom - baseline.synchronousDom, + debouncedWebgl: current.debouncedWebgl - baseline.debouncedWebgl, + debouncedDom: current.debouncedDom - baseline.debouncedDom + } } async function disposeActivePaneRefreshProbe(page: Page): Promise { @@ -249,7 +339,9 @@ function annotateMeasurement( } test.describe('Terminal foreground redraw freeze repro', () => { - test('Codex-style line rewrites request a visible row refresh', async ({ orcaPage }) => { + test('Codex-style line rewrites request a visible row refresh', async ({ + orcaPage + }, testInfo) => { await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) @@ -257,9 +349,16 @@ test.describe('Terminal foreground redraw freeze repro', () => { const { paneKey } = await waitForActivePaneHookDescriptor(orcaPage) await waitForTerminalPtyDataInjector(orcaPage, paneKey) + const webglAttached = await forceActivePaneWebglRenderer(orcaPage) + // Why: Linux headless CI intentionally disables GPU. Declare that + // environment unsupported instead of weakening the WebGL-only oracle. + test.skip(!webglAttached, 'WebGL is unavailable for the refresh-policy probe') + if (!webglAttached) { + return + } await installActivePaneRefreshProbe(orcaPage) try { - const refreshBaseline = await readRefreshProbeCount(orcaPage) + const refreshBaseline = await readRefreshProbe(orcaPage) await resetSchedulerDebug(orcaPage) const measurement = await measureRendererDuringRewriteBurst(orcaPage, paneKey) const scheduler = await readSchedulerDebug(orcaPage) @@ -268,11 +367,32 @@ test.describe('Terminal foreground redraw freeze repro', () => { expect(measurement.maxTimerDriftMs).toBeLessThan(MAX_RENDERER_TIMER_DRIFT_MS) expect(scheduler.deferredForegroundEnqueueCount).toBeGreaterThan(0) await expect - .poll(async () => (await readRefreshProbeCount(orcaPage)) - refreshBaseline, { - timeout: 5_000, - message: 'Codex-style terminal rewrites did not request an xterm refresh' - }) + .poll( + async () => { + const refresh = await readRefreshProbe(orcaPage) + const delta = subtractRefreshProbe(refresh, refreshBaseline) + return Object.values(delta).reduce((total, count) => total + count, 0) + }, + { + timeout: 5_000, + message: 'Codex-style terminal rewrites did not request an xterm refresh' + } + ) .toBeGreaterThan(0) + const refresh = await readRefreshProbe(orcaPage) + const refreshDelta = subtractRefreshProbe(refresh, refreshBaseline) + testInfo.annotations.push({ + type: 'terminal-refresh-probe', + description: `syncWebgl=${refreshDelta.synchronousWebgl} syncDom=${ + refreshDelta.synchronousDom + } debouncedWebgl=${refreshDelta.debouncedWebgl} debouncedDom=${refreshDelta.debouncedDom}` + }) + // Why: a synchronous full-grid WebGL refresh duplicates xterm's + // already-queued animation frame and was the #6655 CPU hotspot. + expect(refreshDelta.synchronousWebgl).toBe(0) + // Requiring an observed public WebGL refresh prevents a mid-run DOM + // fallback from turning the zero-sync assertion into a vacuous pass. + expect(refreshDelta.debouncedWebgl).toBeGreaterThan(0) } finally { await disposeActivePaneRefreshProbe(orcaPage) } From 0eafb0e62632de4f3ee899da1a98737a233eaede Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:55:57 -0700 Subject: [PATCH 12/17] perf(ai-vault): index visible session pane lookups (#8173) * perf(ai-vault): index visible session pane lookups * perf(ai-vault): build pane index lazily --- .../ai-vault-original-pane-actions.ts | 28 +- .../ai-vault-original-pane-index.test.ts | 254 ++++++++++++++++++ .../ai-vault-original-pane-index.ts | 189 +++++++++++++ .../right-sidebar/ai-vault-original-pane.ts | 8 +- 4 files changed, 466 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.test.ts create mode 100644 src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.ts diff --git a/src/renderer/src/components/right-sidebar/ai-vault-original-pane-actions.ts b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-actions.ts index 51646b559b7..86077e1c598 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-original-pane-actions.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-actions.ts @@ -1,4 +1,4 @@ -import { useCallback } from 'react' +import { useCallback, useMemo } from 'react' import { useShallow } from 'zustand/react/shallow' import { toast } from 'sonner' import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' @@ -7,10 +7,12 @@ import { useAppStore } from '@/store' import type { AgentStatusState } from '../../../../shared/agent-status-types' import type { AiVaultSession } from '../../../../shared/ai-vault-types' import { translate } from '@/i18n/i18n' +import { findOriginalAiVaultSessionPane } from './ai-vault-original-pane' import { - findAiVaultSessionLiveState, - findOriginalAiVaultSessionPane -} from './ai-vault-original-pane' + createLazyAiVaultOriginalPaneIndex, + findAiVaultSessionLiveStateInIndex, + findOriginalAiVaultSessionPaneInIndex +} from './ai-vault-original-pane-index' export function useAiVaultOriginalPaneActions(): { getOriginalPaneTarget: ( @@ -29,15 +31,23 @@ export function useAiVaultOriginalPaneActions(): { terminalLayoutsByTabId: s.terminalLayoutsByTabId })) ) - - const getOriginalPaneTarget = useCallback( - (session: AiVaultSession) => findOriginalAiVaultSessionPane(originalPaneLookupState, session), + // Why: loading, filtered, or collapsed views may render no session rows. + // Build once on the first actual lookup, then share it across visible rows. + const getOriginalPaneIndex = useMemo( + () => createLazyAiVaultOriginalPaneIndex(originalPaneLookupState), [originalPaneLookupState] ) + const getOriginalPaneTarget = useCallback( + (session: AiVaultSession) => + findOriginalAiVaultSessionPaneInIndex(getOriginalPaneIndex(), session), + [getOriginalPaneIndex] + ) + const getSessionLiveState = useCallback( - (session: AiVaultSession) => findAiVaultSessionLiveState(originalPaneLookupState, session), - [originalPaneLookupState] + (session: AiVaultSession) => + findAiVaultSessionLiveStateInIndex(getOriginalPaneIndex(), session), + [getOriginalPaneIndex] ) const jumpToOriginalPane = useCallback((session: AiVaultSession): void => { diff --git a/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.test.ts b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.test.ts new file mode 100644 index 00000000000..127ec6553b7 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.test.ts @@ -0,0 +1,254 @@ +import { describe, expect, it } from 'vitest' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { AiVaultSession } from '../../../../shared/ai-vault-types' +import { + buildAiVaultOriginalPaneIndex, + createLazyAiVaultOriginalPaneIndex, + findAiVaultSessionLiveStateInIndex, + findOriginalAiVaultSessionPaneInIndex +} from './ai-vault-original-pane-index' +import { + findAiVaultSessionLiveState, + findOriginalAiVaultSessionPane +} from './ai-vault-original-pane' + +const SESSION: AiVaultSession = { + id: 'codex:target-session', + executionHostId: 'local', + agent: 'codex', + sessionId: 'target-session', + title: 'Target session', + cwd: '/repo', + branch: null, + model: null, + filePath: '/tmp/target-session.jsonl', + codexHome: null, + createdAt: null, + updatedAt: '2026-07-10T00:00:00.000Z', + modifiedAt: '2026-07-10T00:00:00.000Z', + messageCount: 1, + totalTokens: 1, + previewMessages: [], + queuedMessageCount: 0, + subagentTranscriptCount: 0, + resumeCommand: "codex resume 'target-session'", + subagent: null +} + +function countedRecord(count: number, makeValue: (index: number) => T) { + const reads = { value: 0 } + const record: Record = {} + for (let index = 0; index < count; index += 1) { + Object.defineProperty(record, `entry-${index}`, { + enumerable: true, + get: () => { + reads.value += 1 + return makeValue(index) + } + }) + } + return { reads, record } +} + +function unrelatedEntry(index: number): AgentStatusEntry { + return { + state: 'done', + prompt: `Other task ${index}`, + updatedAt: index, + stateStartedAt: index, + agentType: 'claude', + paneKey: `other-tab-${index}:11111111-1111-4111-8111-111111111111`, + tabId: `other-tab-${index}`, + worktreeId: 'other-worktree', + stateHistory: [], + providerSession: { key: 'session_id', id: `other-session-${index}` } + } +} + +describe('AI Vault original-pane index', () => { + it('builds lazily on the first lookup and shares one index across callbacks', () => { + const live = countedRecord(500, unrelatedEntry) + const retained = countedRecord(500, (index) => ({ + entry: unrelatedEntry(index), + worktreeId: 'other-worktree', + tab: { id: `other-tab-${index}` }, + agentType: 'claude', + startedAt: index + })) + const sleeping = countedRecord(500, (index) => ({ + paneKey: `other-tab-${index}:11111111-1111-4111-8111-111111111111`, + tabId: `other-tab-${index}`, + worktreeId: 'other-worktree', + agent: 'claude', + providerSession: { key: 'session_id', id: `other-session-${index}` }, + prompt: `Other task ${index}`, + state: 'done', + capturedAt: index, + updatedAt: index, + origin: 'live' + })) + const getIndex = createLazyAiVaultOriginalPaneIndex({ + agentStatusByPaneKey: live.record, + retainedAgentsByPaneKey: retained.record, + sleepingAgentSessionsByPaneKey: sleeping.record, + tabsByWorktree: {}, + terminalLayoutsByTabId: {} + } as never) + + expect(live.reads.value + retained.reads.value + sleeping.reads.value).toBe(0) + expect(findAiVaultSessionLiveStateInIndex(getIndex(), SESSION)).toBeNull() + expect(live.reads.value + retained.reads.value + sleeping.reads.value).toBe(1_500) + expect(findOriginalAiVaultSessionPaneInIndex(getIndex(), SESSION)).toBeNull() + expect(live.reads.value + retained.reads.value + sleeping.reads.value).toBe(1_500) + }) + + it('builds once instead of rescanning every agent collection for every visible row', () => { + const live = countedRecord(500, unrelatedEntry) + const retained = countedRecord(500, (index) => ({ + entry: unrelatedEntry(index), + worktreeId: 'other-worktree', + tab: { id: `other-tab-${index}` }, + agentType: 'claude', + startedAt: index + })) + const sleeping = countedRecord(500, (index) => ({ + paneKey: `other-tab-${index}:11111111-1111-4111-8111-111111111111`, + tabId: `other-tab-${index}`, + worktreeId: 'other-worktree', + agent: 'claude', + providerSession: { key: 'session_id', id: `other-session-${index}` }, + prompt: `Other task ${index}`, + state: 'done', + capturedAt: index, + updatedAt: index, + origin: 'live' + })) + const state = { + agentStatusByPaneKey: live.record, + retainedAgentsByPaneKey: retained.record, + sleepingAgentSessionsByPaneKey: sleeping.record, + tabsByWorktree: {}, + terminalLayoutsByTabId: {} + } as never + + const index = buildAiVaultOriginalPaneIndex(state) + expect(live.reads.value + retained.reads.value + sleeping.reads.value).toBe(1_500) + + live.reads.value = 0 + retained.reads.value = 0 + sleeping.reads.value = 0 + for (let row = 0; row < 20; row += 1) { + expect(findOriginalAiVaultSessionPaneInIndex(index, SESSION)).toBeNull() + expect(findAiVaultSessionLiveStateInIndex(index, SESSION)).toBeNull() + } + expect(live.reads.value + retained.reads.value + sleeping.reads.value).toBe(0) + }) + + it('preserves provider, prompt-fallback, retained, sleeping, and missing matches', () => { + const leafIds = [ + '11111111-1111-4111-8111-111111111111', + '22222222-2222-4222-8222-222222222222', + '33333333-3333-4333-8333-333333333333', + '44444444-4444-4444-8444-444444444444' + ] + const tabs = leafIds.map((_, index) => ({ + id: `tab-${index + 1}`, + ptyId: null, + worktreeId: 'wt-1', + title: 'Agent', + customTitle: null, + color: null, + sortOrder: index, + createdAt: index + })) + const layouts = Object.fromEntries( + leafIds.map((leafId, index) => [ + `tab-${index + 1}`, + { + root: { type: 'leaf', leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: `pty-${index + 1}` } + } + ]) + ) + const liveDirect = { + ...unrelatedEntry(1), + agentType: 'codex', + paneKey: `tab-1:${leafIds[0]}`, + tabId: 'tab-1', + worktreeId: 'wt-1', + state: 'blocked', + providerSession: { key: 'session_id', id: 'live-direct' } + } as AgentStatusEntry + const livePrompt = { + ...unrelatedEntry(2), + agentType: 'codex', + paneKey: `tab-2:${leafIds[1]}`, + tabId: 'tab-2', + worktreeId: 'wt-1', + prompt: 'Unique prompt match that is long enough', + providerSession: undefined + } as AgentStatusEntry + const retainedEntry = { + ...unrelatedEntry(3), + agentType: 'codex', + paneKey: `tab-3:${leafIds[2]}`, + tabId: 'tab-3', + worktreeId: 'wt-1', + providerSession: { key: 'session_id', id: 'retained-direct' } + } as AgentStatusEntry + const state = { + agentStatusByPaneKey: { + [liveDirect.paneKey]: liveDirect, + [livePrompt.paneKey]: livePrompt + }, + retainedAgentsByPaneKey: { + [retainedEntry.paneKey]: { + entry: retainedEntry, + worktreeId: 'wt-1', + tab: tabs[2], + agentType: 'codex', + startedAt: 1 + } + }, + sleepingAgentSessionsByPaneKey: { + 'tab-4:1': { + paneKey: 'tab-4:1', + tabId: 'tab-4', + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: 'sleeping-direct' }, + prompt: 'Sleeping', + state: 'done', + capturedAt: 1, + updatedAt: 1, + origin: 'live' + } + }, + tabsByWorktree: { 'wt-1': tabs }, + terminalLayoutsByTabId: layouts + } as never + const index = buildAiVaultOriginalPaneIndex(state) + const sessions = [ + { ...SESSION, sessionId: 'live-direct' }, + { + ...SESSION, + sessionId: 'prompt-only', + title: 'Unique prompt match that is long enough' + }, + { ...SESSION, sessionId: 'retained-direct' }, + { ...SESSION, sessionId: 'sleeping-direct' }, + { ...SESSION, sessionId: 'missing' } + ] + + for (const session of sessions) { + expect(findOriginalAiVaultSessionPaneInIndex(index, session)).toEqual( + findOriginalAiVaultSessionPane(state, session) + ) + expect(findAiVaultSessionLiveStateInIndex(index, session)).toBe( + findAiVaultSessionLiveState(state, session) + ) + } + }) +}) diff --git a/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.ts b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.ts new file mode 100644 index 00000000000..95079f43593 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/ai-vault-original-pane-index.ts @@ -0,0 +1,189 @@ +import type { AgentStatusState } from '../../../../shared/agent-status-types' +import type { AiVaultSession } from '../../../../shared/ai-vault-types' +import { + promptsMatchSession, + resolveOriginalPaneTarget, + type OriginalPaneState, + type AiVaultOriginalPaneTarget +} from './ai-vault-original-pane' + +type LiveEntry = NonNullable +type RetainedEntry = NonNullable +type SleepingEntry = NonNullable + +type ProviderIndex = Map +type AgentIndex = Map + +export type AiVaultOriginalPaneIndex = { + state: OriginalPaneState + liveByProvider: ProviderIndex + liveWithoutProviderByAgent: AgentIndex + retainedByProvider: ProviderIndex + retainedWithoutProviderByAgent: AgentIndex + sleepingByProvider: ProviderIndex +} + +function providerKey(agent: string, sessionId: string): string { + // Why: unlike punctuation delimiters, NUL cannot collide with agent or + // provider-session text, so distinct identity pairs stay distinct keys. + return `${agent}\u0000${sessionId}` +} + +function appendToIndex(index: Map, key: string, value: T): void { + const entries = index.get(key) + if (entries) { + entries.push(value) + } else { + index.set(key, [value]) + } +} + +export function buildAiVaultOriginalPaneIndex(state: OriginalPaneState): AiVaultOriginalPaneIndex { + const liveByProvider: ProviderIndex = new Map() + const liveWithoutProviderByAgent: AgentIndex = new Map() + const retainedByProvider: ProviderIndex = new Map() + const retainedWithoutProviderByAgent: AgentIndex = new Map() + const sleepingByProvider: ProviderIndex = new Map() + + for (const entry of Object.values(state.agentStatusByPaneKey)) { + if (!entry?.agentType) { + continue + } + if (entry.providerSession) { + appendToIndex(liveByProvider, providerKey(entry.agentType, entry.providerSession.id), entry) + } else if (entry.providerSession === undefined) { + appendToIndex(liveWithoutProviderByAgent, entry.agentType, entry) + } + } + for (const retained of Object.values(state.retainedAgentsByPaneKey)) { + if (!retained?.agentType) { + continue + } + if (retained.entry.providerSession) { + appendToIndex( + retainedByProvider, + providerKey(retained.agentType, retained.entry.providerSession.id), + retained + ) + } else if (retained.entry.providerSession === undefined) { + appendToIndex(retainedWithoutProviderByAgent, retained.agentType, retained) + } + } + for (const record of Object.values(state.sleepingAgentSessionsByPaneKey)) { + if (record) { + appendToIndex( + sleepingByProvider, + providerKey(record.agent, record.providerSession.id), + record + ) + } + } + + return { + state, + liveByProvider, + liveWithoutProviderByAgent, + retainedByProvider, + retainedWithoutProviderByAgent, + sleepingByProvider + } +} + +export function createLazyAiVaultOriginalPaneIndex( + state: OriginalPaneState +): () => AiVaultOriginalPaneIndex { + let index: AiVaultOriginalPaneIndex | null = null + return () => { + index ??= buildAiVaultOriginalPaneIndex(state) + return index + } +} + +export function findOriginalAiVaultSessionPaneInIndex( + index: AiVaultOriginalPaneIndex, + session: AiVaultSession +): AiVaultOriginalPaneTarget | null { + const key = providerKey(session.agent, session.sessionId) + const promptMatchedTargets: AiVaultOriginalPaneTarget[] = [] + + for (const entry of index.liveByProvider.get(key) ?? []) { + const target = resolveOriginalPaneTarget({ + state: index.state, + paneKey: entry.paneKey, + worktreeIdHint: entry.worktreeId, + tabIdHint: entry.tabId + }) + if (target) { + return target + } + } + for (const entry of index.liveWithoutProviderByAgent.get(session.agent) ?? []) { + if (!promptsMatchSession(session, entry)) { + continue + } + const target = resolveOriginalPaneTarget({ + state: index.state, + paneKey: entry.paneKey, + worktreeIdHint: entry.worktreeId, + tabIdHint: entry.tabId + }) + if (target) { + promptMatchedTargets.push(target) + } + } + for (const retained of index.retainedByProvider.get(key) ?? []) { + const target = resolveOriginalPaneTarget({ + state: index.state, + paneKey: retained.entry.paneKey, + worktreeIdHint: retained.worktreeId, + tabIdHint: retained.entry.tabId ?? retained.tab.id + }) + if (target) { + return target + } + } + for (const retained of index.retainedWithoutProviderByAgent.get(session.agent) ?? []) { + if (!promptsMatchSession(session, retained.entry)) { + continue + } + const target = resolveOriginalPaneTarget({ + state: index.state, + paneKey: retained.entry.paneKey, + worktreeIdHint: retained.worktreeId, + tabIdHint: retained.entry.tabId ?? retained.tab.id + }) + if (target) { + promptMatchedTargets.push(target) + } + } + for (const record of index.sleepingByProvider.get(key) ?? []) { + const target = resolveOriginalPaneTarget({ + state: index.state, + paneKey: record.paneKey, + worktreeIdHint: record.worktreeId, + tabIdHint: record.tabId + }) + if (target) { + return target + } + } + + return promptMatchedTargets.length === 1 ? promptMatchedTargets[0] : null +} + +export function findAiVaultSessionLiveStateInIndex( + index: AiVaultOriginalPaneIndex, + session: AiVaultSession +): AgentStatusState | null { + const direct = index.liveByProvider.get(providerKey(session.agent, session.sessionId)) + if (direct?.[0]) { + return direct[0].state + } + const promptMatchedStates: AgentStatusState[] = [] + for (const entry of index.liveWithoutProviderByAgent.get(session.agent) ?? []) { + if (promptsMatchSession(session, entry)) { + promptMatchedStates.push(entry.state) + } + } + return promptMatchedStates.length === 1 ? promptMatchedStates[0] : null +} diff --git a/src/renderer/src/components/right-sidebar/ai-vault-original-pane.ts b/src/renderer/src/components/right-sidebar/ai-vault-original-pane.ts index 48f76ac29e2..013684c7870 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-original-pane.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-original-pane.ts @@ -12,7 +12,7 @@ export type AiVaultOriginalPaneTarget = { leafId: string } -type OriginalPaneState = Pick< +export type OriginalPaneState = Pick< AppState, | 'agentStatusByPaneKey' | 'retainedAgentsByPaneKey' @@ -87,10 +87,10 @@ function entryPromptCandidates(entry: { return [...candidates] } -function promptsMatchSession( +export function promptsMatchSession( session: AiVaultSession, entry: Parameters[0] -) { +): boolean { const sessionCandidates = sessionPromptCandidates(session) if (sessionCandidates.length === 0) { return false @@ -134,7 +134,7 @@ function getTabOwnerWorktreeId( return null } -function resolveOriginalPaneTarget(args: { +export function resolveOriginalPaneTarget(args: { state: OriginalPaneState paneKey: string worktreeIdHint?: string From 5eda948a7a07ff50d15ffa0e32cd1d79589ad47a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:56:00 -0700 Subject: [PATCH 13/17] perf(agent-startup): skip unrelated store flushes (#8179) --- ...gent-startup-delayed-delivery-perf.test.ts | 164 ++++++++++++++++++ .../src/lib/agent-startup-delayed-delivery.ts | 28 ++- 2 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/lib/agent-startup-delayed-delivery-perf.test.ts diff --git a/src/renderer/src/lib/agent-startup-delayed-delivery-perf.test.ts b/src/renderer/src/lib/agent-startup-delayed-delivery-perf.test.ts new file mode 100644 index 00000000000..3ea4a168b7c --- /dev/null +++ b/src/renderer/src/lib/agent-startup-delayed-delivery-perf.test.ts @@ -0,0 +1,164 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { + queuePendingAgentStartupDelivery, + resetAgentStartupDelayedDeliveryForTests +} from './agent-startup-delayed-delivery' + +const originalState = useAppStore.getState() +const LEAF_ID = '11111111-1111-4111-8111-111111111111' + +function seedPendingState(agentLaunchConfigByPaneKey: Record = {}): void { + useAppStore.setState({ + tabsByWorktree: { + 'wt-background': [ + { + id: 'tab-background', + ptyId: null, + worktreeId: 'wt-background', + title: 'Agent', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + pendingStartupByTabId: { + 'tab-background': { launchToken: 'target-launch' } + }, + agentLaunchConfigByPaneKey, + ptyIdsByTabId: { 'tab-background': [] }, + terminalLayoutsByTabId: {} + } as never) +} + +function bindPendingPty(): void { + useAppStore.setState({ + agentLaunchConfigByPaneKey: { + [`tab-background:${LEAF_ID}`]: { + identity: { + tabId: 'tab-background', + leafId: LEAF_ID, + launchToken: 'target-launch' + } + } + }, + ptyIdsByTabId: { 'tab-background': ['pty-background'] }, + terminalLayoutsByTabId: { + 'tab-background': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'pty-background' } + } + } + } as never) +} + +function countedLaunchConfigs(count: number): { + reads: { value: number } + record: Record +} { + const reads = { value: 0 } + const record: Record = {} + for (let index = 0; index < count; index += 1) { + Object.defineProperty(record, `other-pane-${index}`, { + enumerable: true, + get: () => { + reads.value += 1 + return { + identity: { + tabId: `other-tab-${index}`, + launchToken: `other-launch-${index}` + } + } + } + }) + } + return { reads, record } +} + +afterEach(() => { + resetAgentStartupDelayedDeliveryForTests() + useAppStore.setState(originalState, true) +}) + +describe('delayed agent startup subscription', () => { + it('does not rescan launch metadata for unrelated store updates', () => { + const launchConfigs = countedLaunchConfigs(500) + seedPendingState(launchConfigs.record) + + queuePendingAgentStartupDelivery({ + worktreeId: 'wt-background', + tabId: 'tab-background', + launchToken: 'target-launch', + startup: {} as never, + deliver: vi.fn() + }) + expect(launchConfigs.reads.value).toBe(500) + + launchConfigs.reads.value = 0 + for (let update = 0; update < 100; update += 1) { + useAppStore.setState({ activeView: update % 2 === 0 ? 'terminal' : 'settings' } as never) + } + + expect(launchConfigs.reads.value).toBe(0) + }) + + it('delivers when launch registration, PTY ownership, and layout binding arrive', () => { + seedPendingState() + const deliver = vi.fn().mockResolvedValue(undefined) + const startup = {} as never + queuePendingAgentStartupDelivery({ + worktreeId: 'wt-background', + tabId: 'tab-background', + launchToken: 'target-launch', + startup, + deliver + }) + + bindPendingPty() + + expect(deliver).toHaveBeenCalledWith('tab-background', 'pty-background', startup) + }) + + it('drops a delivery when its tab is removed before PTY binding', () => { + seedPendingState() + const deliver = vi.fn().mockResolvedValue(undefined) + queuePendingAgentStartupDelivery({ + worktreeId: 'wt-background', + tabId: 'tab-background', + launchToken: 'target-launch', + startup: {} as never, + deliver + }) + + useAppStore.setState({ tabsByWorktree: {} }) + seedPendingState() + bindPendingPty() + + expect(deliver).not.toHaveBeenCalled() + }) + + it('drops a delivery when a newer pending launch token replaces it', () => { + seedPendingState() + const deliver = vi.fn().mockResolvedValue(undefined) + queuePendingAgentStartupDelivery({ + worktreeId: 'wt-background', + tabId: 'tab-background', + launchToken: 'target-launch', + startup: {} as never, + deliver + }) + + useAppStore.setState({ + pendingStartupByTabId: { + 'tab-background': { launchToken: 'newer-launch' } + } + } as never) + bindPendingPty() + + expect(deliver).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/agent-startup-delayed-delivery.ts b/src/renderer/src/lib/agent-startup-delayed-delivery.ts index 5ca13944b32..2e38fad41d1 100644 --- a/src/renderer/src/lib/agent-startup-delayed-delivery.ts +++ b/src/renderer/src/lib/agent-startup-delayed-delivery.ts @@ -90,7 +90,33 @@ function ensurePendingAgentStartupSubscription(): void { if (unsubscribePendingAgentStartupDeliveries) { return } - unsubscribePendingAgentStartupDeliveries = useAppStore.subscribe(() => { + const initial = useAppStore.getState() + // Capture the individual references so the gate stays allocation-free and + // remains correct even if a subscribe adapter reuses its state object. + let previousTabs = initial.tabsByWorktree + let previousPendingStartups = initial.pendingStartupByTabId + let previousLaunchConfigs = initial.agentLaunchConfigByPaneKey + let previousPtyIds = initial.ptyIdsByTabId + let previousLayouts = initial.terminalLayoutsByTabId + unsubscribePendingAgentStartupDeliveries = useAppStore.subscribe((state) => { + // Why: a background workspace can stay unmounted indefinitely. Only these + // five immutable slices can change delivery eligibility; unrelated title, + // status, focus, and usage ticks must not rescan every launch registration. + if ( + state.tabsByWorktree === previousTabs && + state.pendingStartupByTabId === previousPendingStartups && + state.agentLaunchConfigByPaneKey === previousLaunchConfigs && + state.ptyIdsByTabId === previousPtyIds && + state.terminalLayoutsByTabId === previousLayouts + ) { + return + } + // Update before flushing because delivery can synchronously write the store. + previousTabs = state.tabsByWorktree + previousPendingStartups = state.pendingStartupByTabId + previousLaunchConfigs = state.agentLaunchConfigByPaneKey + previousPtyIds = state.ptyIdsByTabId + previousLayouts = state.terminalLayoutsByTabId flushPendingAgentStartupDeliveries() }) } From 66437d19c3e1ef06bf4f51f1f8a664516b4dbfc0 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:56:03 -0700 Subject: [PATCH 14/17] perf(hook-commands): skip unrelated store flushes (#8181) --- ...hook-command-delayed-delivery-perf.test.ts | 40 +++++++++++++++++++ .../src/lib/hook-command-delayed-delivery.ts | 26 +++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/lib/hook-command-delayed-delivery-perf.test.ts diff --git a/src/renderer/src/lib/hook-command-delayed-delivery-perf.test.ts b/src/renderer/src/lib/hook-command-delayed-delivery-perf.test.ts new file mode 100644 index 00000000000..f0b28c30887 --- /dev/null +++ b/src/renderer/src/lib/hook-command-delayed-delivery-perf.test.ts @@ -0,0 +1,40 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { + queueHookCommandsForFirstWorktreeTab, + resetHookCommandDelayedDeliveryForTests +} from './hook-command-delayed-delivery' + +const initialState = useAppStore.getState() + +afterEach(() => { + resetHookCommandDelayedDeliveryForTests() + useAppStore.setState(initialState, true) +}) + +describe('delayed hook-command subscription', () => { + it('does not rescan pending worktrees for unrelated store updates', () => { + const reads = { value: 0 } + useAppStore.setState({ + tabsByWorktree: {}, + getKnownWorktreeById: ((worktreeId: string) => { + reads.value += 1 + return { id: worktreeId } + }) as never + } as never) + + for (let index = 0; index < 500; index += 1) { + queueHookCommandsForFirstWorktreeTab({ + worktreeId: `runtime-worktree-${index}`, + deliver: vi.fn() + }) + } + + reads.value = 0 + for (let update = 0; update < 100; update += 1) { + useAppStore.setState({ activeView: update % 2 === 0 ? 'terminal' : 'settings' } as never) + } + + expect(reads.value).toBe(0) + }) +}) diff --git a/src/renderer/src/lib/hook-command-delayed-delivery.ts b/src/renderer/src/lib/hook-command-delayed-delivery.ts index 234126f73ad..5f2db5d0cd2 100644 --- a/src/renderer/src/lib/hook-command-delayed-delivery.ts +++ b/src/renderer/src/lib/hook-command-delayed-delivery.ts @@ -33,7 +33,31 @@ function ensurePendingHookCommandSubscription(): void { if (unsubscribePendingHookCommandDeliveries) { return } - unsubscribePendingHookCommandDeliveries = useAppStore.subscribe(() => { + const initial = useAppStore.getState() + // Capture references so unrelated PTY/status updates do not rescan every + // runtime worktree waiting for its first mirrored terminal tab. + let previousTabsByWorktree = initial.tabsByWorktree + let previousWorktreesByRepo = initial.worktreesByRepo + let previousDetectedWorktreesByRepo = initial.detectedWorktreesByRepo + let previousFolderWorkspaces = initial.folderWorkspaces + let previousWorktreeLookup = initial.getKnownWorktreeById + unsubscribePendingHookCommandDeliveries = useAppStore.subscribe((state) => { + // Why: only these slices (or the test-adapter lookup itself) can change + // whether a pending worktree exists or has received its first tab. + if ( + state.tabsByWorktree === previousTabsByWorktree && + state.worktreesByRepo === previousWorktreesByRepo && + state.detectedWorktreesByRepo === previousDetectedWorktreesByRepo && + state.folderWorkspaces === previousFolderWorkspaces && + state.getKnownWorktreeById === previousWorktreeLookup + ) { + return + } + previousTabsByWorktree = state.tabsByWorktree + previousWorktreesByRepo = state.worktreesByRepo + previousDetectedWorktreesByRepo = state.detectedWorktreesByRepo + previousFolderWorkspaces = state.folderWorkspaces + previousWorktreeLookup = state.getKnownWorktreeById flushPendingHookCommandDeliveries() }) } From 2cfb028163b1c1a55490ad65ba6c2c36c33f62f9 Mon Sep 17 00:00:00 2001 From: sonhyrd <163875127+sonhyrd@users.noreply.github.com> Date: Sat, 11 Jul 2026 10:02:45 +0700 Subject: [PATCH 15/17] fix: Cap automationRuns retention to stop unbounded state file growth (#8119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Cap automationRuns retention to stop unbounded state file growth `automationRuns` was the only unbounded durable collection in `orca-data.json`, and the whole blob is re-serialized and rewritten on every save. On a machine running four `* * * * *` automations it had grown to 11,184 rows / 21 MB of a 28.5 MB file — all of them `skipped_precheck` no-ops — so each synchronous `flush()` blocked the Electron main thread for 190-210 ms and macOS filed 24 `disk writes` diagnostic reports against Orca (12-30 MB/s sustained, 549 GB/session). Prune to the newest 100 runs per automation, on load and on append. This mirrors the retention that `pruneLocalTerminalScrollbackBuffers` and `pruneWorkspaceSessionBrowserHistory` already apply to their collections; `automationRuns` was simply missed. The load-path prune marks state dirty, so an oversized file heals on first load. Without that flag the shrink lives only in memory: the sole load-time save trigger is `normalized.changed || loadNeedsSave || adaptedProjectGroups`, and `normalized` covers pane identity only. A user who took the documented workaround (`automations edit --disabled`) fires no runs, so nothing would ever rewrite the file. Measured against the affected 28.5 MB file: flush() 190 ms -> 9 ms, bytes written per save 28.5 MB -> 1.3 MB. Pruning breaks the old `runNumber` derivation, which counted retained runs, so every run after the cap would have been titled "run 101". Carry the ordinal on the run itself and derive the next number from the highest survivor. Legacy rows are numbered from the highest number their automation already carries, not from their append position: a downgrade to a pre-`runNumber` build appends unnumbered runs after pruned survivors numbered 101+, and a position would reissue one of those, giving two runs the same title. Fixes #8118 * Never evict in-flight automation runs from retention A dispatched run's completion can land hours later (renderer round-trip or headless completion promise); pruning it makes updateAutomationRun throw 'Automation run not found.'. Only final-status runs are evictable now, with the final-status predicate shared between retention and the service. Co-authored-by: Orca * Skip the usage write when retention evicted the run mid-collection markDispatchResult finalizes a run, awaits usage collection, then writes usage by id. The run is final during that await, so a concurrent create-time prune can evict it and the write threw 'Automation run not found.' — in the headless path that cascaded into an unhandled rejection. Co-authored-by: Orca * Pin backfill-before-prune ordering with true legacy fixture rows The heal-on-load fixture rows carried runNumber, so swapping backfill and prune passed every test while renumbering real legacy survivors 1..100 and re-minting colliding titles. Seed unnumbered rows and assert healed ordinals. Co-authored-by: Orca --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca --- src/main/automations/service.test.ts | 56 +++++++ src/main/automations/service.ts | 32 ++-- src/main/persistence.test.ts | 122 ++++++++++++++ src/main/persistence.ts | 29 +++- src/shared/automation-run-retention.test.ts | 173 ++++++++++++++++++++ src/shared/automation-run-retention.ts | 65 ++++++++ src/shared/automations-types.ts | 15 ++ 7 files changed, 468 insertions(+), 24 deletions(-) create mode 100644 src/shared/automation-run-retention.test.ts create mode 100644 src/shared/automation-run-retention.ts diff --git a/src/main/automations/service.test.ts b/src/main/automations/service.test.ts index 786ff017bb2..1c9c314b41b 100644 --- a/src/main/automations/service.test.ts +++ b/src/main/automations/service.test.ts @@ -538,6 +538,62 @@ describe('AutomationService', () => { expect(getAutomationRunUsage).toHaveBeenCalledTimes(1) }) + it('does not throw when retention evicts the run during usage collection', async () => { + vi.setSystemTime(new Date('2026-05-13T10:00:00')) + const store = await createStore() + store.addRepo(makeRepo()) + const automation = store.createAutomation({ + name: 'Costed check', + prompt: 'Check spend', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'existing', + workspaceId: 'wt1', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-13T00:00:00Z').getTime() + }) + const base = Date.now() + const run = store.createAutomationRun(automation, base, 'manual') + store.updateAutomationRun({ + runId: run.id, + status: 'dispatched', + workspaceId: 'wt1', + terminalSessionId: 'tab-1', + error: null + }) + // While usage collection is awaited the run is already final, so a + // scheduler tick creating newer runs can prune it away mid-flight. + const getAutomationRunUsage = vi.fn().mockImplementation(async () => { + for (let i = 1; i <= 120; i++) { + const later = store.createAutomationRun(automation, base + i * 60_000) + store.updateAutomationRun({ + runId: later.id, + status: 'completed', + workspaceId: 'wt1', + error: null + }) + } + return null + }) + const service = new AutomationService(store, { + tickMs: 60_000, + claudeUsage: { getAutomationRunUsage } as never + }) + + const updated = await service.markDispatchResult({ + runId: run.id, + status: 'completed', + workspaceId: 'wt1', + terminalSessionId: 'tab-1', + error: null + }) + + expect(updated.id).toBe(run.id) + // The run really was evicted; the usage write was skipped instead of throwing. + expect(store.listAutomationRuns(automation.id).some((entry) => entry.id === run.id)).toBe(false) + }) + it('records unsupported usage cleanly for completed agents without local usage stores', async () => { vi.setSystemTime(new Date('2026-05-13T10:00:00')) const store = await createStore() diff --git a/src/main/automations/service.ts b/src/main/automations/service.ts index dc22550e7ec..91b70ff6088 100644 --- a/src/main/automations/service.ts +++ b/src/main/automations/service.ts @@ -1,12 +1,12 @@ import type { WebContents } from 'electron' import type { Store } from '../persistence' -import type { - Automation, - AutomationDispatchRequest, - AutomationDispatchResult, - AutomationPrecheckResult, - AutomationRun, - AutomationRunStatus +import { + isFinalAutomationRunStatus, + type Automation, + type AutomationDispatchRequest, + type AutomationDispatchResult, + type AutomationPrecheckResult, + type AutomationRun } from '../../shared/automations-types' import type { ClaudeUsageStore } from '../claude-usage/store' import type { CodexUsageStore } from '../codex-usage/store' @@ -135,7 +135,7 @@ export class AutomationService { async markDispatchResult(result: AutomationDispatchResult): Promise { const run = this.store.updateAutomationRun(result) clearAutomationDispatchTokens(run.automationId, run.id) - if (!isFinalRunStatus(run.status)) { + if (!isFinalAutomationRunStatus(run.status)) { return run } // Why: the renderer's mark-completed effect can re-fire for the same run @@ -151,6 +151,11 @@ export class AutomationService { claudeUsage: this.claudeUsage, codexUsage: this.codexUsage }) + // Why: the run is final during the await above, so a concurrent create-time + // retention prune may have evicted it — the usage write must not throw then. + if (!this.store.listAutomationRuns(run.automationId).some((entry) => entry.id === run.id)) { + return run + } return this.store.updateAutomationRun({ runId: run.id, status: run.status, @@ -309,14 +314,3 @@ export class AutomationService { } } } - -function isFinalRunStatus(status: AutomationRunStatus): boolean { - return ( - status === 'completed' || - status === 'dispatch_failed' || - status === 'skipped_precheck' || - status === 'skipped_missed' || - status === 'skipped_unavailable' || - status === 'skipped_needs_interactive_auth' - ) -} diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 74c6611f1d0..a0d3f760779 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -1699,6 +1699,128 @@ describe('Store', () => { expect(migratedRun?.sourceContext).toEqual(migratedAutomation?.sourceContext) }) + it('shrinks an oversized automationRuns file on load without any later mutation', async () => { + const seed = await createStore() + seed.addRepo( + makeRepo({ + upstream: { owner: 'stablyai', repo: 'orca' }, + connectionId: 'builder' + }) + ) + const automation = seed.createAutomation({ + name: 'Every minute', + prompt: 'Run checks', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'new_per_run', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-13T00:00:00Z').getTime() + }) + seed.createAutomationRun(automation, new Date('2026-05-13T09:00:00Z').getTime()) + seed.flush() + + // Why: a fresh store never stamps the one-shot UI migration flags, so a + // second load+flush settles them — otherwise they, not the prune, mark dirty. + const warm = await createStore() + warm.flush() + + const persisted = readDataFile() as { automationRuns: Record[] } + const template = persisted.automationRuns[0] + persisted.automationRuns = Array.from({ length: 250 }, (_, i) => { + const legacy: Record = { + ...template, + id: `legacy-run-${i}`, + // Only final runs are evictable; the real-world blowup was skipped_precheck rows. + status: 'skipped_precheck', + createdAt: 1_000 + i, + scheduledFor: 1_000 + i + } + // A real legacy file predates runNumber; backfill must run BEFORE the prune + // so survivors keep their true ordinals instead of restarting at 1. + delete legacy.runNumber + return legacy + }) + writeDataFile(persisted) + + vi.useFakeTimers() + try { + const reloaded = await createStore() + expect(reloaded.listAutomationRuns(automation.id)).toHaveLength(100) + + // The load-path prune must mark state dirty on its own; nothing else here saves. + vi.advanceTimersByTime(1000) + await reloaded.waitForPendingWrite() + } finally { + vi.useRealTimers() + } + + const healed = readDataFile() as { automationRuns: Record[] } + expect(healed.automationRuns).toHaveLength(100) + expect(healed.automationRuns.at(-1)?.id).toBe('legacy-run-249') + // Survivors carry their true lifetime ordinals (151..250), not restarted ones. + expect(healed.automationRuns[0]?.runNumber).toBe(151) + expect(healed.automationRuns.at(-1)?.runNumber).toBe(250) + }) + + it('does not strand an in-flight run whose completion lands after the retention cap', async () => { + const store = await createStore() + store.addRepo( + makeRepo({ + upstream: { owner: 'stablyai', repo: 'orca' }, + connectionId: 'builder' + }) + ) + const automation = store.createAutomation({ + name: 'Every minute', + prompt: 'Run checks', + agentId: 'claude', + projectId: 'r1', + workspaceMode: 'new_per_run', + timezone: 'UTC', + rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0', + dtstart: new Date('2026-05-13T00:00:00Z').getTime() + }) + const base = new Date('2026-05-13T09:00:00Z').getTime() + const inFlight = store.createAutomationRun(automation, base) + store.updateAutomationRun({ + runId: inFlight.id, + status: 'dispatched', + workspaceId: null, + error: null + }) + + // 120 later runs reach a final status while the first one is still dispatched. + let firstCompletedId = '' + for (let i = 1; i <= 120; i++) { + const later = store.createAutomationRun(automation, base + i * 60_000) + firstCompletedId ||= later.id + store.updateAutomationRun({ + runId: later.id, + status: 'completed', + workspaceId: null, + error: null + }) + } + + // The late completion must still find its row. + const completed = store.updateAutomationRun({ + runId: inFlight.id, + status: 'completed', + workspaceId: null, + error: null + }) + expect(completed.id).toBe(inFlight.id) + + const runs = store.listAutomationRuns(automation.id) + expect(runs.some((run) => run.id === inFlight.id)).toBe(true) + // Final runs beyond the cap were still evicted. The store can briefly hold + // cap + 2: the last-created run finalizes after the prune at its creation, + // and the late completion lands without a prune of its own. + expect(runs.some((run) => run.id === firstCompletedId)).toBe(false) + expect(runs.length).toBeLessThanOrEqual(102) + }) + it('persists automation precheck config and run results', async () => { const store = await createStore() store.addRepo(makeRepo()) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 3cd0fe52a72..d4b68e7f4b4 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -130,6 +130,11 @@ import { } from './agent-hooks/migration-unsupported-pty-state' import { agentHookServer } from './agent-hooks/server' import { pruneLocalTerminalScrollbackBuffers } from '../shared/workspace-session-terminal-buffers' +import { + backfillAutomationRunNumbers, + nextAutomationRunNumber, + pruneAutomationRuns +} from '../shared/automation-run-retention' import { pruneWorkspaceSessionBrowserHistory } from '../shared/workspace-session-browser-history' import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR, @@ -3387,7 +3392,18 @@ export class Store { parsed.legacyPaneKeyAliasEntries ), automations: Array.isArray(parsed.automations) ? parsed.automations : [], - automationRuns: Array.isArray(parsed.automationRuns) ? parsed.automationRuns : [], + automationRuns: (() => { + if (!Array.isArray(parsed.automationRuns)) { + return [] + } + const runs = pruneAutomationRuns(backfillAutomationRunNumbers(parsed.automationRuns)) + // Why: nothing else on the load path marks state dirty, so without + // this an oversized legacy file only shrinks at the next unrelated save. + if (runs.length !== parsed.automationRuns.length) { + this.loadNeedsSave = true + } + return runs + })(), onboarding: normalizedOnboarding } } @@ -4701,12 +4717,15 @@ export class Store { return existing } const now = Date.now() - const runNumber = - (this.state.automationRuns ?? []).filter((run) => run.automationId === automation.id).length + - 1 + // Why: retention prunes old runs, so the count of retained runs is no longer + // the run's ordinal — carry the number forward from the newest survivor. + const runNumber = nextAutomationRunNumber( + (this.state.automationRuns ?? []).filter((run) => run.automationId === automation.id) + ) const run: AutomationRun = { id: randomUUID(), automationId: automation.id, + runNumber, runContext: automation.runContext ?? null, sourceContext: automation.sourceContext ?? null, title: `${automation.name} run ${runNumber}`, @@ -4728,7 +4747,7 @@ export class Store { dispatchedAt: null, createdAt: now } - this.state.automationRuns = [...(this.state.automationRuns ?? []), run] + this.state.automationRuns = pruneAutomationRuns([...(this.state.automationRuns ?? []), run]) if (trigger === 'manual') { this.recordFeatureInteraction('automation-run') } diff --git a/src/shared/automation-run-retention.test.ts b/src/shared/automation-run-retention.test.ts new file mode 100644 index 00000000000..c405c43f37b --- /dev/null +++ b/src/shared/automation-run-retention.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it } from 'vitest' +import type { AutomationRun } from './automations-types' +import { + MAX_AUTOMATION_RUNS_PER_AUTOMATION, + backfillAutomationRunNumbers, + nextAutomationRunNumber, + pruneAutomationRuns +} from './automation-run-retention' + +function run(overrides: Partial & Pick) { + return { + runContext: null, + sourceContext: null, + title: overrides.id, + scheduledFor: 0, + status: 'skipped_precheck', + trigger: 'scheduled', + workspaceId: null, + sessionKind: 'terminal', + chatSessionId: null, + terminalSessionId: null, + terminalPaneKey: null, + terminalPtyId: null, + outputSnapshot: null, + precheckResult: null, + usage: null, + error: null, + startedAt: null, + dispatchedAt: null, + createdAt: 0, + ...overrides + } as AutomationRun +} + +function makeRuns(automationId: string, count: number, from = 0): AutomationRun[] { + return Array.from({ length: count }, (_, i) => + run({ id: `${automationId}-${from + i}`, automationId, createdAt: from + i }) + ) +} + +describe('pruneAutomationRuns', () => { + it('keeps everything below the cap', () => { + const runs = makeRuns('a', 5) + expect(pruneAutomationRuns(runs, 10)).toEqual(runs) + }) + + it('keeps only the newest N per automation', () => { + const kept = pruneAutomationRuns(makeRuns('a', 10), 3) + expect(kept.map((r) => r.id)).toEqual(['a-7', 'a-8', 'a-9']) + }) + + it('caps each automation independently', () => { + const runs = [...makeRuns('a', 6), ...makeRuns('b', 2)] + const kept = pruneAutomationRuns(runs, 3) + expect(kept.filter((r) => r.automationId === 'a')).toHaveLength(3) + // 'b' is under the cap and must survive intact. + expect(kept.filter((r) => r.automationId === 'b')).toHaveLength(2) + }) + + it('preserves original append order among survivors', () => { + const runs = [...makeRuns('a', 4), ...makeRuns('b', 4)] + const kept = pruneAutomationRuns(runs, 2) + expect(kept.map((r) => r.id)).toEqual(['a-2', 'a-3', 'b-2', 'b-3']) + }) + + it('breaks createdAt ties on scheduledFor so pruning is deterministic', () => { + const runs = [ + run({ id: 'x', automationId: 'a', createdAt: 5, scheduledFor: 1 }), + run({ id: 'y', automationId: 'a', createdAt: 5, scheduledFor: 2 }) + ] + expect(pruneAutomationRuns(runs, 1).map((r) => r.id)).toEqual(['y']) + }) + + it('returns nothing when the cap is zero or negative', () => { + expect(pruneAutomationRuns(makeRuns('a', 3), 0)).toEqual([]) + expect(pruneAutomationRuns(makeRuns('a', 3), -1)).toEqual([]) + }) + + // Why: a dispatched run's completion can land hours later; evicting the row + // would make updateAutomationRun throw 'Automation run not found.' + it('never evicts in-flight runs, even far past the cap', () => { + const inFlight = [ + run({ id: 'old-pending', automationId: 'a', status: 'pending', createdAt: -3 }), + run({ id: 'old-dispatching', automationId: 'a', status: 'dispatching', createdAt: -2 }), + run({ id: 'old-dispatched', automationId: 'a', status: 'dispatched', createdAt: -1 }) + ] + const kept = pruneAutomationRuns([...inFlight, ...makeRuns('a', 10)], 3) + expect(kept.map((r) => r.id)).toEqual([ + 'old-pending', + 'old-dispatching', + 'old-dispatched', + 'a-7', + 'a-8', + 'a-9' + ]) + }) + + it('shrinks a realistic runaway history to the cap', () => { + const runaway = [ + ...makeRuns('a', 2796), + ...makeRuns('b', 2796), + ...makeRuns('c', 2796), + ...makeRuns('d', 2796) + ] + expect(runaway).toHaveLength(11_184) + expect(pruneAutomationRuns(runaway)).toHaveLength(4 * MAX_AUTOMATION_RUNS_PER_AUTOMATION) + }) +}) + +describe('backfillAutomationRunNumbers', () => { + it('numbers legacy runs by append position within their automation', () => { + const runs = [ + run({ id: 'a-0', automationId: 'a' }), + run({ id: 'b-0', automationId: 'b' }), + run({ id: 'a-1', automationId: 'a' }) + ] + expect(backfillAutomationRunNumbers(runs).map((r) => r.runNumber)).toEqual([1, 1, 2]) + }) + + it('leaves an existing runNumber untouched', () => { + const runs = [run({ id: 'a-0', automationId: 'a', runNumber: 42 })] + expect(backfillAutomationRunNumbers(runs)[0].runNumber).toBe(42) + }) + + // Why: a downgrade to a build that predates `runNumber` appends unnumbered runs + // after pruned, high-numbered survivors. Numbering by append position reissues a + // number a survivor still holds, and the two runs end up sharing a title. + it('never reissues a number a numbered run already holds', () => { + const runs = [ + run({ id: 'a-0', automationId: 'a', runNumber: 2 }), + run({ id: 'a-1', automationId: 'a' }) + ] + const numbers = backfillAutomationRunNumbers(runs).map((r) => r.runNumber) + expect(numbers).toEqual([2, 3]) + expect(new Set(numbers).size).toBe(numbers.length) + }) + + it('numbers unnumbered runs above the highest survivor, per automation', () => { + const runs = [ + run({ id: 'a-0', automationId: 'a', runNumber: 200 }), + run({ id: 'b-0', automationId: 'b', runNumber: 7 }), + run({ id: 'a-1', automationId: 'a' }), + run({ id: 'b-1', automationId: 'b' }) + ] + expect(backfillAutomationRunNumbers(runs).map((r) => r.runNumber)).toEqual([200, 7, 201, 8]) + }) +}) + +describe('nextAutomationRunNumber', () => { + it('continues from the highest surviving run number, not the retained count', () => { + const retained = [ + run({ id: 'a-0', automationId: 'a', runNumber: 2795 }), + run({ id: 'a-1', automationId: 'a', runNumber: 2796 }) + ] + expect(nextAutomationRunNumber(retained)).toBe(2797) + }) + + it('falls back to the count for legacy runs with no numbers', () => { + expect(nextAutomationRunNumber(makeRuns('a', 100))).toBe(101) + }) + + it('starts at 1 for a brand new automation', () => { + expect(nextAutomationRunNumber([])).toBe(1) + }) + + it('never repeats a number across a prune cycle', () => { + let runs = backfillAutomationRunNumbers(makeRuns('a', 250)) + runs = pruneAutomationRuns(runs, 100) + const next = nextAutomationRunNumber(runs) + expect(next).toBe(251) + expect(runs.some((r) => r.runNumber === next)).toBe(false) + }) +}) diff --git a/src/shared/automation-run-retention.ts b/src/shared/automation-run-retention.ts new file mode 100644 index 00000000000..3b0eae648ad --- /dev/null +++ b/src/shared/automation-run-retention.ts @@ -0,0 +1,65 @@ +import { isFinalAutomationRunStatus, type AutomationRun } from './automations-types' + +export const MAX_AUTOMATION_RUNS_PER_AUTOMATION = 100 + +// Why: the whole state blob is re-serialized on every save, so an unbounded +// automationRuns made each flush() permanently slower (28.5 MB → 210 ms blocked). +export function pruneAutomationRuns( + runs: readonly AutomationRun[], + maxPerAutomation: number = MAX_AUTOMATION_RUNS_PER_AUTOMATION +): AutomationRun[] { + const kept = new Set() + // Why: a dispatched run's completion can land hours later, and + // updateAutomationRun throws if its row is gone — only final runs are evictable. + const finalRuns = runs.filter((run) => isFinalAutomationRunStatus(run.status)) + for (const automationRuns of Map.groupBy(finalRuns, (run) => run.automationId).values()) { + // Why: `createdAt` is the append time; `scheduledFor` breaks ties so runs + // minted in the same millisecond drop in a stable, reproducible order. + automationRuns.sort((a, b) => b.createdAt - a.createdAt || b.scheduledFor - a.scheduledFor) + // Why: clamp — a negative `slice` end drops from the tail instead of keeping nothing. + for (const run of automationRuns.slice(0, Math.max(0, maxPerAutomation))) { + kept.add(run.id) + } + } + + // Survivors keep their original append order — callers index by position. + return runs.filter((run) => kept.has(run.id) || !isFinalAutomationRunStatus(run.status)) +} + +/** + * Stamp `runNumber` onto legacy runs that predate the field. Must run before + * {@link pruneAutomationRuns} so the surviving runs carry their true numbers and + * numbering never restarts. + * + * Numbers continue from the highest number the automation already carries, not from + * the run's append position: a downgrade to a pre-`runNumber` build appends unnumbered + * runs after pruned survivors numbered 101+, and a position would reissue one of those. + */ +export function backfillAutomationRunNumbers(runs: readonly AutomationRun[]): AutomationRun[] { + const highestPerAutomation = new Map() + for (const run of runs) { + if (run.runNumber !== undefined) { + const highest = highestPerAutomation.get(run.automationId) ?? 0 + highestPerAutomation.set(run.automationId, Math.max(highest, run.runNumber)) + } + } + return runs.map((run) => { + if (run.runNumber !== undefined) { + return run + } + const runNumber = (highestPerAutomation.get(run.automationId) ?? 0) + 1 + highestPerAutomation.set(run.automationId, runNumber) + return { ...run, runNumber } + }) +} + +/** Next run number for one automation, given only the runs still retained. */ +export function nextAutomationRunNumber(runsForAutomation: readonly AutomationRun[]): number { + // Why: seed with the count so legacy runs that predate `runNumber` still advance. + return ( + runsForAutomation.reduce( + (n, run) => Math.max(n, run.runNumber ?? 0), + runsForAutomation.length + ) + 1 + ) +} diff --git a/src/shared/automations-types.ts b/src/shared/automations-types.ts index 56788595e95..a41b364ef2b 100644 --- a/src/shared/automations-types.ts +++ b/src/shared/automations-types.ts @@ -17,6 +17,18 @@ export type AutomationRunStatus = | 'dispatch_failed' export type AutomationRunTrigger = 'scheduled' | 'manual' +/** Statuses a run can never leave; only these are safe to evict from history. */ +export function isFinalAutomationRunStatus(status: AutomationRunStatus): boolean { + return ( + status === 'completed' || + status === 'dispatch_failed' || + status === 'skipped_precheck' || + status === 'skipped_missed' || + status === 'skipped_unavailable' || + status === 'skipped_needs_interactive_auth' + ) +} + export type AutomationSchedulePreset = 'hourly' | 'daily' | 'weekdays' | 'weekly' | 'custom' export type AutomationRunUsageProvider = 'claude' | 'codex' export type AutomationRunUsageStatus = 'known' | 'unavailable' @@ -140,6 +152,9 @@ export type AutomationRun = { startedAt: number | null dispatchedAt: number | null createdAt: number + /** Why: run titles must stay unique once retention prunes old runs, so the + * number can no longer be derived from how many runs are currently kept. */ + runNumber?: number } export type AutomationCreateInput = { From e825df8218b0bd177f134c443ffcba48dd7ba33e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:10:08 -0700 Subject: [PATCH 16/17] perf(memory): skip unrelated boot worktree scans (#8204) * perf(memory): scope boot worktree scans to live sessions * fix(memory): refresh daemon hydration snapshots * fix(memory): rescan repos referenced only by the re-read snapshot Repo selection previously came solely from the first daemon listing, so a session that only became visible on the post-enumeration re-read (e.g. a briefly unreachable adapter) could never be registered for the life of the process. Loop enumeration until the latest daemon snapshot references no unresolved repos, so selection and registration always share a snapshot. Co-authored-by: Orca --------- Co-authored-by: Orca --- .../memory/hydrate-local-pty-registry.test.ts | 175 +++++++++++++++++- src/main/memory/hydrate-local-pty-registry.ts | 82 +++++--- 2 files changed, 226 insertions(+), 31 deletions(-) diff --git a/src/main/memory/hydrate-local-pty-registry.test.ts b/src/main/memory/hydrate-local-pty-registry.test.ts index 0854fa2493b..714f18b3cba 100644 --- a/src/main/memory/hydrate-local-pty-registry.test.ts +++ b/src/main/memory/hydrate-local-pty-registry.test.ts @@ -32,9 +32,9 @@ vi.mock('../daemon/daemon-init', () => ({ getDaemonProvider: () => getDaemonProviderMock() })) -// Why: the hydrator builds its worktreeId → connectionId map by calling -// listRepoWorktrees(repo) for every repo in the store. The git I/O is -// out of scope for this unit; mock returns whatever the test wants. +// Why: the hydrator builds its worktreeId → connectionId map through +// listRepoWorktrees(repo). The git I/O is out of scope for this unit; the mock +// also lets count tests prove repos without live sessions launch no Git work. const listRepoWorktreesMock = vi.fn() vi.mock('../repo-worktrees', () => ({ listRepoWorktrees: (repo: unknown) => listRepoWorktreesMock(repo) @@ -79,6 +79,17 @@ function makeLocalSessions(repoId: string, worktreePath: string, count: number): return sessions } +function createDeferred(): { + promise: Promise + resolve: (value: T) => void +} { + let resolve!: (value: T) => void + const promise = new Promise((resolver) => { + resolve = resolver + }) + return { promise, resolve } +} + // Why: the module under test memoizes `hasHydrated` at module scope so it // only runs the git/RPC pass once per process. The pty-registry module // also stashes state in a module-level Map, so we have to load BOTH @@ -127,6 +138,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { await hydrate(makeStore([{ id: 'repo-a' }])) expect(provider.listSessions).toHaveBeenCalledTimes(1) + expect(listRepoWorktreesMock).not.toHaveBeenCalled() }) it('catches provider.listSessions rejection and does not throw', async () => { @@ -145,6 +157,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { warnSpy.mockRestore() expect(listRegisteredPtys()).toHaveLength(0) + expect(listRepoWorktreesMock).not.toHaveBeenCalled() }) it('does not clobber a pre-existing registry pid with a null pid from listSessions', async () => { @@ -180,6 +193,69 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry).toBeDefined() expect(entry!.pid).toBe(12345) expect(entry!.paneKey).toBe('tab-1:1') + expect(listRepoWorktreesMock).not.toHaveBeenCalled() + }) + + it('does not clobber a pty:spawn registration that arrives during worktree enumeration', async () => { + const { hydrate, listRegisteredPtys, registerPty } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@deadbeef' + const provider = makeProvider([ + { sessionId: ptyId, pid: null, cwd: '/local/Triton' } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + const worktrees = + createDeferred< + { path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[] + >() + listRepoWorktreesMock.mockReturnValue(worktrees.promise) + + const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1)) + registerPty({ + ptyId, + worktreeId: 'repo-a::/local/Triton', + sessionId: ptyId, + paneKey: 'tab-1:1', + pid: 12345 + }) + worktrees.resolve([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + await hydration + + expect(listRegisteredPtys()).toEqual([ + expect.objectContaining({ ptyId, pid: 12345, paneKey: 'tab-1:1' }) + ]) + }) + + it('does not resurrect a daemon session that exits during worktree enumeration', async () => { + const { hydrate, listRegisteredPtys, unregisterPty } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@deadbeef' + const provider = { + listSessions: vi + .fn() + .mockResolvedValueOnce([ + { sessionId: ptyId, pid: 4242, cwd: '/local/Triton' } as unknown as SessionInfo + ]) + .mockResolvedValueOnce([]) + } + getDaemonProviderMock.mockReturnValue(provider) + const worktrees = + createDeferred< + { path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[] + >() + listRepoWorktreesMock.mockReturnValue(worktrees.promise) + + const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1)) + unregisterPty(ptyId) + worktrees.resolve([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + await hydration + + expect(provider.listSessions).toHaveBeenCalledTimes(2) + expect(listRegisteredPtys()).toHaveLength(0) }) it('skips SSH repos before enumerating worktrees', async () => { @@ -223,6 +299,99 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry!.worktreeId).toBe('repo-a::/local/Triton') }) + it('does not register a daemon session whose worktree was removed', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + const provider = makeProvider([ + { + sessionId: 'repo-a::/local/removed@@deadbeef', + pid: 4242, + cwd: '/local/removed' + } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/local/current', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + + expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1) + expect(listRegisteredPtys()).toHaveLength(0) + }) + + it('enumerates worktrees only for repos referenced by live daemon sessions', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + const repos = Array.from({ length: 100 }, (_, index) => ({ id: `repo-${index}` })) + const activeRepoIds = ['repo-17', 'repo-83'] + const provider = makeProvider( + activeRepoIds.map( + (repoId, index) => + ({ + sessionId: `${repoId}::/local/${repoId}@@0000000${index}`, + pid: 4000 + index, + cwd: `/local/${repoId}` + }) as unknown as SessionInfo + ) + ) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [ + { + path: `/local/${repo.id}`, + head: '', + branch: '', + isBare: false, + isMainWorktree: true + } + ]) + + await hydrate(makeStore(repos)) + + expect(listRepoWorktreesMock).toHaveBeenCalledTimes(activeRepoIds.length) + expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual( + activeRepoIds + ) + expect(listRegisteredPtys()).toHaveLength(activeRepoIds.length) + }) + + it('scans a repo that only becomes visible on the post-enumeration re-read', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + + const sessionA = { + sessionId: 'repo-a::/local/repo-a@@00000001', + pid: 4001, + cwd: '/local/repo-a' + } as unknown as SessionInfo + const sessionB = { + sessionId: 'repo-b::/local/repo-b@@00000002', + pid: 4002, + cwd: '/local/repo-b' + } as unknown as SessionInfo + // Why: a briefly unreachable adapter can omit a session from the first + // listing; once it reappears on the re-read its repo must still be + // scanned and the session registered instead of silently dropped. + const listSessions = vi + .fn() + .mockResolvedValueOnce([sessionA]) + .mockResolvedValue([sessionA, sessionB]) + getDaemonProviderMock.mockReturnValue({ listSessions }) + listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [ + { path: `/local/${repo.id}`, head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate(makeStore([{ id: 'repo-a' }, { id: 'repo-b' }])) + + expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual([ + 'repo-a', + 'repo-b' + ]) + expect(listSessions).toHaveBeenCalledTimes(3) + const registered = listRegisteredPtys() + expect(registered.map((p) => p.ptyId).sort()).toEqual([ + 'repo-a::/local/repo-a@@00000001', + 'repo-b::/local/repo-b@@00000002' + ]) + }) + it('hydrates large daemon session lists', async () => { const { hydrate, listRegisteredPtys } = await loadFresh() diff --git a/src/main/memory/hydrate-local-pty-registry.ts b/src/main/memory/hydrate-local-pty-registry.ts index 4bb82763ec7..f6563049318 100644 --- a/src/main/memory/hydrate-local-pty-registry.ts +++ b/src/main/memory/hydrate-local-pty-registry.ts @@ -25,6 +25,7 @@ import type { SessionInfo } from '../daemon/types' import { listRegisteredPtys, registerPty } from './pty-registry' import { listRepoWorktrees } from '../repo-worktrees' import { parsePtySessionId } from '../../shared/pty-session-id-format' +import { splitWorktreeId } from '../../shared/worktree-id' import type { Store } from '../persistence' // Why: `attachMainWindowServices` runs on every macOS dock re-activation @@ -70,34 +71,62 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick() + // Why: ask the daemon which repos matter before launching Git worktree + // enumeration. Most configured repos have no preserved session at boot, + // so scanning all of them creates pure background subprocess churn. + const reposById = new Map(store.getRepos().map((repo) => [repo.id, repo])) + // Why: live git enumeration verifies that a referenced local worktree + // still exists instead of resurrecting removed worktrees. + const liveLocalWorktreeIds = new Set() + const resolvedRepoIds = new Set() - for (const repo of repos) { - const connectionId = repo.connectionId ?? null - if (connectionId) { - // Why: SSH PTYs are never registered for local process sampling, so - // avoid startup SSH/git enumeration for repos we will skip anyway. - continue + let sessionInfos = await collectSessionInfos(provider) + let alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) + + // Why: repo selection and registration must come from the same daemon + // snapshot. Git enumeration can take seconds, so after each scan pass we + // re-read daemon and registry state; sessions that exited or were + // authoritatively registered meanwhile are not resurrected or overwritten, + // and a session that only became visible during a slow scan (e.g. a + // briefly unreachable legacy adapter) gets its repo scanned on the next + // pass instead of being silently dropped. Terminates because every pass + // permanently resolves at least one new repo id. + for (;;) { + const newlyReferencedRepos = new Map[number]>() + for (const info of sessionInfos) { + if (alreadyRegistered.has(info.sessionId)) { + continue + } + const { worktreeId } = parsePtySessionId(info.sessionId) + const parsedWorktreeId = worktreeId ? splitWorktreeId(worktreeId) : null + if (!parsedWorktreeId || resolvedRepoIds.has(parsedWorktreeId.repoId)) { + continue + } + const repo = reposById.get(parsedWorktreeId.repoId) + if (!repo || (repo.connectionId ?? null)) { + // Why: unknown repos can't be proven local, and SSH PTYs are never + // registered for local process sampling — resolve without git + // enumeration so neither can extend the loop. + resolvedRepoIds.add(parsedWorktreeId.repoId) + continue + } + newlyReferencedRepos.set(repo.id, repo) } - const worktrees = await listRepoWorktrees(repo) - for (const wt of worktrees) { - const worktreeId = `${repo.id}::${wt.path}` - repoConnectionIdByWorktreeId.set(worktreeId, connectionId) + if (newlyReferencedRepos.size === 0) { + break } + + for (const repo of newlyReferencedRepos.values()) { + resolvedRepoIds.add(repo.id) + const worktrees = await listRepoWorktrees(repo) + for (const wt of worktrees) { + liveLocalWorktreeIds.add(`${repo.id}::${wt.path}`) + } + } + + sessionInfos = await collectSessionInfos(provider) + alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) } - - // Why: SessionInfo is read through the adapter's listSessions() so we - // get the pid alongside each id. Routing through every adapter - // (current + legacy) keeps protocol coverage symmetric with the - // orphan-cleanup path. - const sessionInfos = await collectSessionInfos(provider) - - const alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) - for (const info of sessionInfos) { // Why: pid-write ordering — `pty:spawn` is the authoritative // writer for in-session sessions; if that fired before this loop @@ -115,10 +144,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick Date: Fri, 10 Jul 2026 20:22:16 -0700 Subject: [PATCH 17/17] fix(mobile): recover terminal WebView/WebGL/viewport/theme state after iOS resume (#8196) * fix(mobile): recover terminal state after iOS resume * Refactor terminal record merge to extract snapshot-reconciliation helper Split the inline merge logic in mergeTerminalRecordsByCurrentOrder into a named mergeTerminalSnapshotWithKnownRecord function for clarity, preserving the existing behavior of keeping the last known theme when a snapshot omits it. --- .../app/h/[hostId]/session/[worktreeId].tsx | 9 +- .../session/mobile-session-route-types.ts | 2 +- mobile/src/session/TerminalPaneView.tsx | 14 +- .../session/mobile-session-route-helpers.ts | 2 +- .../session/mobile-terminal-records.test.ts | 12 ++ mobile/src/session/mobile-terminal-records.ts | 23 ++- .../src/session/use-mobile-terminal-paste.ts | 2 +- mobile/src/terminal/TerminalWebView.tsx | 100 ++++++++----- .../terminal-foreground-recovery.test.ts | 6 + .../terminal/terminal-viewport-refit.test.ts | 30 ++++ .../src/terminal/terminal-viewport-refit.ts | 51 ++++++- .../src/terminal/terminal-webview-contract.ts | 14 ++ .../terminal-webview-engine-error.test.ts | 108 +++++++++++++- .../terminal/terminal-webview-engine.test.ts | 135 +++++++++++++++++- mobile/src/terminal/terminal-webview-html.ts | 39 ++--- .../src/terminal/terminal-webview-messages.ts | 1 + .../terminal-webview-text-zoom.test.ts | 8 +- .../terminal-webview-theme-injected.ts | 27 ++++ ...erminal-webview-webgl-recovery-injected.ts | 62 ++++++++ 19 files changed, 558 insertions(+), 87 deletions(-) create mode 100644 mobile/src/terminal/terminal-webview-theme-injected.ts create mode 100644 mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index b881ddebabb..18a393cae28 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -93,7 +93,7 @@ import type { TerminalKeyboardAvoidanceMetrics, TerminalModes, TerminalWebViewHandle -} from '../../../../src/terminal/TerminalWebView' +} from '../../../../src/terminal/terminal-webview-contract' import { isTerminalOscLinkRanges } from '../../../../src/terminal/terminal-osc-link-ranges' import { useTerminalViewportRefit } from '../../../../src/terminal/terminal-viewport-refit' import { @@ -2530,8 +2530,12 @@ export default function SessionScreen() { if (!shouldRecover) { return } + for (const terminalRef of terminalRefs.current.values()) { + terminalRef.prepareForForegroundRecovery() + } // Why: iOS can resume a live WKWebView with a blank xterm backing store - // without firing web-ready/reconnect; replay scrollback to repaint it. + // without firing web-ready/reconnect; invalidate the native readiness + // latch before replay so init waits for the document's pong. recoverActiveTerminalAfterForeground({ activeHandleRef, terminalRefs, @@ -2559,6 +2563,7 @@ export default function SessionScreen() { clientRef, deviceTokenRef, initializedHandlesRef, + connState, tabStripVisible: terminals.length > 1, textScale: terminalTextScale, terminalFrameWidth, diff --git a/mobile/app/h/[hostId]/session/mobile-session-route-types.ts b/mobile/app/h/[hostId]/session/mobile-session-route-types.ts index 5f0f800d9c2..4bc6215f4a2 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-route-types.ts +++ b/mobile/app/h/[hostId]/session/mobile-session-route-types.ts @@ -1,5 +1,5 @@ import type { MobileBrowserTab } from '../../../../src/browser/MobileBrowserPane' -import type { MobileTerminalTheme } from '../../../../src/terminal/TerminalWebView' +import type { MobileTerminalTheme } from '../../../../src/terminal/terminal-webview-contract' import type { MobileDiffLine } from '../../../../src/session/mobile-diff-lines' import type { MobileHighlightedDiffLine, diff --git a/mobile/src/session/TerminalPaneView.tsx b/mobile/src/session/TerminalPaneView.tsx index da34ed870c1..beb8118dabf 100644 --- a/mobile/src/session/TerminalPaneView.tsx +++ b/mobile/src/session/TerminalPaneView.tsx @@ -1,12 +1,12 @@ import { useCallback } from 'react' import { StyleSheet, View } from 'react-native' -import { - TerminalWebView, - type MobileTerminalTheme, - type TerminalKeyboardAvoidanceMetrics, - type TerminalModes, - type TerminalWebViewHandle -} from '../terminal/TerminalWebView' +import { TerminalWebView } from '../terminal/TerminalWebView' +import type { + MobileTerminalTheme, + TerminalKeyboardAvoidanceMetrics, + TerminalModes, + TerminalWebViewHandle +} from '../terminal/terminal-webview-contract' type TerminalPaneViewProps = { handle: string diff --git a/mobile/src/session/mobile-session-route-helpers.ts b/mobile/src/session/mobile-session-route-helpers.ts index e5a8d4329b7..18434e21f5a 100644 --- a/mobile/src/session/mobile-session-route-helpers.ts +++ b/mobile/src/session/mobile-session-route-helpers.ts @@ -1,4 +1,4 @@ -import type { TerminalModes } from '../terminal/TerminalWebView' +import type { TerminalModes } from '../terminal/terminal-webview-contract' import type { ConnectionState } from '../transport/types' export const MOBILE_SESSION_STATUS_LABELS: Record = { diff --git a/mobile/src/session/mobile-terminal-records.test.ts b/mobile/src/session/mobile-terminal-records.test.ts index 4ebf7476682..0ebd2c55631 100644 --- a/mobile/src/session/mobile-terminal-records.test.ts +++ b/mobile/src/session/mobile-terminal-records.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { getTerminalRecordsFromSessionTabs, mergeTerminalListWithKnownRecords, + mergeTerminalRecordsByCurrentOrder, mobileSessionTabsEqual, type MobileTerminalSessionTab, type TerminalRecord @@ -24,6 +25,17 @@ const darkTheme = { } describe('mobile terminal records', () => { + it('keeps the known theme when a session-tab snapshot omits it', () => { + const known: TerminalRecord[] = [ + { handle: 'pty-1', title: 'Old title', terminalTheme: darkTheme, isActive: false } + ] + const snapshot: TerminalRecord[] = [{ handle: 'pty-1', title: 'Current title', isActive: true }] + + expect(mergeTerminalRecordsByCurrentOrder(snapshot, known)).toEqual([ + { handle: 'pty-1', title: 'Current title', terminalTheme: darkTheme, isActive: true } + ]) + }) + it('keeps session-tab terminal themes when terminal.list omits them', () => { const terminalList: TerminalRecord[] = [ { handle: 'pty-1', title: 'Terminal', isActive: true }, diff --git a/mobile/src/session/mobile-terminal-records.ts b/mobile/src/session/mobile-terminal-records.ts index 0f97af2529e..ffe93ac29dd 100644 --- a/mobile/src/session/mobile-terminal-records.ts +++ b/mobile/src/session/mobile-terminal-records.ts @@ -1,4 +1,4 @@ -import type { MobileTerminalTheme } from '../terminal/TerminalWebView' +import type { MobileTerminalTheme } from '../terminal/terminal-webview-contract' import type { AgentStatusEntry } from '../../../src/shared/agent-status-types' export type TerminalRecord = { @@ -116,6 +116,20 @@ function mobileSessionTabEqual( } } +// Reconcile a partial session snapshot against the last known record for the +// same terminal. The snapshot owns live fields (title, activity); the known +// theme only survives when the snapshot omits it, since lightweight snapshots +// can drop it. +function mergeTerminalSnapshotWithKnownRecord( + snapshot: TerminalRecord, + known: TerminalRecord +): TerminalRecord { + return { + ...snapshot, + terminalTheme: snapshot.terminalTheme ?? known.terminalTheme + } +} + export function mergeTerminalRecordsByCurrentOrder( terminalTabs: TerminalRecord[], currentTerminals: TerminalRecord[] @@ -126,7 +140,12 @@ export function mergeTerminalRecordsByCurrentOrder( const terminalTabsByHandle = new Map(terminalTabs.map((tab) => [tab.handle, tab])) const currentHandles = new Set(currentTerminals.map((terminal) => terminal.handle)) return [ - ...currentTerminals.map((terminal) => terminalTabsByHandle.get(terminal.handle) ?? terminal), + ...currentTerminals.map((terminal) => { + const snapshotTerminal = terminalTabsByHandle.get(terminal.handle) + return snapshotTerminal + ? mergeTerminalSnapshotWithKnownRecord(snapshotTerminal, terminal) + : terminal + }), ...terminalTabs.filter((terminal) => !currentHandles.has(terminal.handle)) ] } diff --git a/mobile/src/session/use-mobile-terminal-paste.ts b/mobile/src/session/use-mobile-terminal-paste.ts index e3148be67f0..57ea720c4c8 100644 --- a/mobile/src/session/use-mobile-terminal-paste.ts +++ b/mobile/src/session/use-mobile-terminal-paste.ts @@ -2,7 +2,7 @@ import { useCallback, type RefObject } from 'react' import * as Clipboard from 'expo-clipboard' import { File as FsFile, Paths } from 'expo-file-system' import { ImageManipulator, SaveFormat } from 'expo-image-manipulator' -import type { TerminalModes } from '../terminal/TerminalWebView' +import type { TerminalModes } from '../terminal/terminal-webview-contract' import type { RpcClient } from '../transport/rpc-client' import type { ConnectionState } from '../transport/types' import { diff --git a/mobile/src/terminal/TerminalWebView.tsx b/mobile/src/terminal/TerminalWebView.tsx index 1798fedd1d8..4b84e44081a 100644 --- a/mobile/src/terminal/TerminalWebView.tsx +++ b/mobile/src/terminal/TerminalWebView.tsx @@ -1,13 +1,9 @@ import { useRef, useCallback, forwardRef, useImperativeHandle, useEffect, useMemo } from 'react' -import { View, type StyleProp, type ViewStyle } from 'react-native' +import { Platform, View } from 'react-native' import { WebView } from 'react-native-webview' import type { WebViewMessageEvent } from 'react-native-webview' import type { TerminalOscLinkRange } from './terminal-osc-link-ranges' -import type { - MobileTerminalTheme, - TerminalSelectionEvents, - TerminalWebViewHandle -} from './terminal-webview-contract' +import type { TerminalWebViewHandle, TerminalWebViewProps } from './terminal-webview-contract' import { TerminalWebViewEngineErrorOverlay, useTerminalWebViewEngineErrorState @@ -18,23 +14,9 @@ import { XTERM_WEBVIEW_SOURCE } from './terminal-webview-html' import type { TerminalWebViewCommand } from './terminal-webview-messages' import { createTerminalWebViewPendingMessages } from './terminal-webview-pending-messages' -export type { - MobileTerminalTheme, - TerminalKeyboardAvoidanceMetrics, - TerminalModes, - TerminalSelectionEvents, - TerminalWebViewHandle -} from './terminal-webview-contract' +type Props = TerminalWebViewProps -type Props = { - style?: StyleProp - terminalTheme?: MobileTerminalTheme - // Why: baseline zoom multiplier ("text size") applied on top of the fit-to-width - // scale; raw xterm fontSize can't drive apparent size because the fit cancels it. - textScale?: number - onWebReady?: () => void - onEngineError?: (message: string) => void -} & TerminalSelectionEvents +export type { TerminalWebViewHandle } from './terminal-webview-contract' export const TerminalWebView = forwardRef(function TerminalWebView( { @@ -61,6 +43,7 @@ export const TerminalWebView = forwardRef(function const isWebReadyRef = useRef(false) const pendingMessages = useMemo(() => createTerminalWebViewPendingMessages(), []) const messageIdRef = useRef(0) + const pendingPingIdRef = useRef(null) const terminalThemeKey = useMemo(() => JSON.stringify(terminalTheme ?? null), [terminalTheme]) const measureResolveRef = useRef< ((result: { cols: number; rows: number } | null) => void) | null @@ -80,7 +63,9 @@ export const TerminalWebView = forwardRef(function const sendToWebView = useCallback((msg: TerminalWebViewCommand) => { messageIdRef.current += 1 - webViewRef.current?.postMessage(JSON.stringify({ ...msg, id: messageIdRef.current })) + const id = messageIdRef.current + webViewRef.current?.postMessage(JSON.stringify({ ...msg, id })) + return id }, []) const flushPendingMessages = useCallback(() => { @@ -98,6 +83,30 @@ export const TerminalWebView = forwardRef(function [pendingMessages, sendToWebView] ) + const confirmWebReady = useCallback( + (notifyParent: boolean) => { + pendingPingIdRef.current = null + isWebReadyRef.current = true + clearWebReadyWatchdog() + clearEngineError() + if (notifyParent) { + onWebReady?.() + } + // Why: reload clears queued commands, so readiness must always restore the + // native-selected theme even when its value did not change in React. + sendToWebView({ type: 'set-theme', terminalTheme }) + flushPendingMessages() + }, + [ + clearEngineError, + clearWebReadyWatchdog, + flushPendingMessages, + onWebReady, + sendToWebView, + terminalTheme + ] + ) + const handleMessage = useCallback( (event: WebViewMessageEvent) => { let msg: Record @@ -108,11 +117,13 @@ export const TerminalWebView = forwardRef(function } if (msg.type === 'web-ready') { - isWebReadyRef.current = true - clearWebReadyWatchdog() - clearEngineError() - onWebReady?.() - flushPendingMessages() + confirmWebReady(true) + } else if ( + msg.type === 'pong' && + typeof msg.pingId === 'number' && + msg.pingId === pendingPingIdRef.current + ) { + confirmWebReady(false) } else if (msg.type === 'ready') { // Why: the WebView's init() rAF chain has run — term is open, // renderService is populated, first paint has happened. Resolve @@ -208,11 +219,8 @@ export const TerminalWebView = forwardRef(function } }, [ - flushPendingMessages, - clearEngineError, - clearWebReadyWatchdog, + confirmWebReady, reportEngineError, - onWebReady, onSelectionMode, onSelectionCopy, onSelectionEvicted, @@ -229,6 +237,7 @@ export const TerminalWebView = forwardRef(function const handleLoadStart = useCallback(() => { isWebReadyRef.current = false + pendingPingIdRef.current = null armWebReadyWatchdog() // Why: messages queued for a previous WebView generation are stale after a reload; // dropping them avoids replaying terminal chunks before the next init snapshot. @@ -240,6 +249,17 @@ export const TerminalWebView = forwardRef(function webViewRef.current?.reload() }, [clearEngineError]) + const handleContentProcessDidTerminate = useCallback(() => { + // Why: WKWebView content-process loss is recoverable; stale commands belong + // to the dead document and the replacement must prove readiness before replay. + isWebReadyRef.current = false + pendingPingIdRef.current = null + pendingMessages.clear() + clearEngineError() + armWebReadyWatchdog() + webViewRef.current?.reload() + }, [armWebReadyWatchdog, clearEngineError, pendingMessages]) + useEffect(() => { postMessage({ type: 'set-theme', terminalTheme }) }, [postMessage, terminalThemeKey, terminalTheme]) @@ -253,6 +273,16 @@ export const TerminalWebView = forwardRef(function useImperativeHandle( ref, () => ({ + prepareForForegroundRecovery() { + if (Platform.OS !== 'ios') { + return + } + // Why: direct ping is the only command allowed through while readiness is + // invalid; init/write commands queue until this exact document answers. + isWebReadyRef.current = false + armWebReadyWatchdog() + pendingPingIdRef.current = sendToWebView({ type: 'ping' }) + }, write(data: string) { postMessage({ type: 'write', data }) }, @@ -363,7 +393,7 @@ export const TerminalWebView = forwardRef(function }) } }), - [postMessage, sendToWebView, terminalTheme, textScale] + [armWebReadyWatchdog, postMessage, sendToWebView, terminalTheme, textScale] ) return ( @@ -389,9 +419,7 @@ export const TerminalWebView = forwardRef(function onRenderProcessGone={(event) => reportNativeEngineError('Terminal WebView render process ended', event) } - onContentProcessDidTerminate={(event) => - reportNativeEngineError('Terminal WebView content process ended', event) - } + onContentProcessDidTerminate={handleContentProcessDidTerminate} /> {engineError ? ( diff --git a/mobile/src/terminal/terminal-foreground-recovery.test.ts b/mobile/src/terminal/terminal-foreground-recovery.test.ts index 4eda53a9c22..6aeceaaa14a 100644 --- a/mobile/src/terminal/terminal-foreground-recovery.test.ts +++ b/mobile/src/terminal/terminal-foreground-recovery.test.ts @@ -127,5 +127,11 @@ describe('terminal foreground recovery', () => { expect(foregroundPredicate).toContain('Platform.OS') expect(sessionSource).toContain('recoverActiveTerminalAfterForeground({') expect(sessionSource).toContain("AppState.addEventListener('change'") + const readinessInvalidation = sessionSource.indexOf( + 'terminalRef.prepareForForegroundRecovery()' + ) + const replay = sessionSource.indexOf('recoverActiveTerminalAfterForeground({') + expect(readinessInvalidation).toBeGreaterThanOrEqual(0) + expect(replay).toBeGreaterThan(readinessInvalidation) }) }) diff --git a/mobile/src/terminal/terminal-viewport-refit.test.ts b/mobile/src/terminal/terminal-viewport-refit.test.ts index 4cd3794ad0f..b658dad437a 100644 --- a/mobile/src/terminal/terminal-viewport-refit.test.ts +++ b/mobile/src/terminal/terminal-viewport-refit.test.ts @@ -52,6 +52,36 @@ describe('terminal viewport refit', () => { expect(sessionSource).toContain('useTerminalViewportRefit({') expect(sessionSource).toContain('tabStripVisible: terminals.length > 1') expect(sessionSource).toContain('textScale: terminalTextScale') + expect(sessionSource).toContain('connState,') + }) + + it('forces a refit on iOS foreground and connection recovery', () => { + const foregroundEffect = hookSource.slice( + hookSource.indexOf("if (Platform.OS !== 'ios')"), + hookSource.indexOf('const previousConnStateRef') + ) + const reconnectEffect = hookSource.slice( + hookSource.indexOf('const previousConnStateRef'), + hookSource.indexOf('disposedRef.current = false') + ) + + expect(foregroundEffect).toContain("AppState.addEventListener('change'") + expect(foregroundEffect).toContain('viewportMeasuredRef.current = false') + expect(foregroundEffect).toContain('scheduleForcedViewportRefit()') + expect(reconnectEffect).toContain("connState !== 'connected'") + expect(reconnectEffect).toContain('viewportMeasuredRef.current = false') + expect(reconnectEffect).toContain('scheduleForcedViewportRefit()') + }) + + it('bypasses the equal-dimensions guard for resume and reconnect refits', () => { + expect(hookSource).toContain('const forceRefit = forceNextRefitRef.current') + expect(hookSource).toContain( + 'if (!forceRefit && prev && prev.cols === dims.cols && prev.rows === dims.rows)' + ) + const forceRead = hookSource.indexOf('const forceRefit = forceNextRefitRef.current') + const updateViewport = hookSource.indexOf("sendRequest('terminal.updateViewport'") + expect(forceRead).toBeGreaterThanOrEqual(0) + expect(updateViewport).toBeGreaterThan(forceRead) }) it('prefers the in-place updateViewport RPC over resubscribe', () => { diff --git a/mobile/src/terminal/terminal-viewport-refit.ts b/mobile/src/terminal/terminal-viewport-refit.ts index b8a823be8c7..44c6a9b7e03 100644 --- a/mobile/src/terminal/terminal-viewport-refit.ts +++ b/mobile/src/terminal/terminal-viewport-refit.ts @@ -1,7 +1,9 @@ import { useCallback, useEffect, useRef, type RefObject } from 'react' -import { useWindowDimensions } from 'react-native' +import { AppState, Platform, useWindowDimensions, type AppStateStatus } from 'react-native' import type { RpcClient } from '../transport/rpc-client' +import type { ConnectionState } from '../transport/types' import type { TerminalWebViewHandle } from './TerminalWebView' +import { shouldRecoverTerminalOnAppStateChange } from './terminal-foreground-recovery' import { isTerminalUpdateViewportApplied, isTerminalUpdateViewportUpdated, @@ -19,6 +21,7 @@ type TerminalViewportRefitOptions = { clientRef: RefObject deviceTokenRef: RefObject initializedHandlesRef: RefObject> + connState: ConnectionState tabStripVisible: boolean // Why: terminal text size (font scale) — changing it changes the cell size, so // the PTY must be re-fitted to a new column count and reflowed. @@ -49,6 +52,7 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): clientRef, deviceTokenRef, initializedHandlesRef, + connState, tabStripVisible, textScale, terminalFrameWidth, @@ -58,6 +62,7 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): const refitTimerRef = useRef | null>(null) const refitRunSeqRef = useRef(0) + const forceNextRefitRef = useRef(false) const disposedRef = useRef(false) const scheduleViewportRefit = useCallback(() => { if (refitTimerRef.current) { @@ -93,8 +98,10 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): if (!dims) { return } + const forceRefit = forceNextRefitRef.current + forceNextRefitRef.current = false const prev = viewportRef.current - if (prev && prev.cols === dims.cols && prev.rows === dims.rows) { + if (!forceRefit && prev && prev.cols === dims.cols && prev.rows === dims.rows) { return } viewportRef.current = dims @@ -152,6 +159,10 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): unsubscribeTerminal, subscribeToTerminal ]) + const scheduleForcedViewportRefit = useCallback(() => { + forceNextRefitRef.current = true + scheduleViewportRefit() + }, [scheduleViewportRefit]) // Why: the tab strip is hidden when only one terminal exists and shown // once a second is created. Crossing the 1↔2 boundary changes the @@ -215,6 +226,42 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): scheduleViewportRefit() }, [terminalFrameWidth, viewportMeasuredRef, scheduleViewportRefit]) + useEffect(() => { + if (Platform.OS !== 'ios') { + return + } + let previousAppState: AppStateStatus | null = AppState.currentState + const sub = AppState.addEventListener('change', (nextAppState: AppStateStatus) => { + const shouldRefit = shouldRecoverTerminalOnAppStateChange( + previousAppState, + nextAppState, + Platform.OS + ) + previousAppState = nextAppState + if (!shouldRefit) { + return + } + // Why: the cached grid can match while the host PTY changed in background; + // reasserting equal dimensions is the convergence signal after iOS resume. + viewportMeasuredRef.current = false + scheduleForcedViewportRefit() + }) + return () => sub.remove() + }, [viewportMeasuredRef, scheduleForcedViewportRefit]) + + const previousConnStateRef = useRef(connState) + useEffect(() => { + const previous = previousConnStateRef.current + previousConnStateRef.current = connState + if (previous === 'connected' || connState !== 'connected') { + return + } + // Why: reconnect can restore a PTY whose host-side size changed while the + // socket was down, so equal cached dimensions still need reassertion. + viewportMeasuredRef.current = false + scheduleForcedViewportRefit() + }, [connState, viewportMeasuredRef, scheduleForcedViewportRefit]) + useEffect(() => { disposedRef.current = false return () => { diff --git a/mobile/src/terminal/terminal-webview-contract.ts b/mobile/src/terminal/terminal-webview-contract.ts index aff86cecad2..808dece2648 100644 --- a/mobile/src/terminal/terminal-webview-contract.ts +++ b/mobile/src/terminal/terminal-webview-contract.ts @@ -1,4 +1,5 @@ import type { RuntimeMobileTerminalTheme } from '../../../src/shared/runtime-types' +import type { StyleProp, ViewStyle } from 'react-native' import type { TerminalOscLinkRange } from './terminal-osc-link-ranges' type TerminalMouseTrackingMode = 'none' | 'x10' | 'vt200' | 'drag' | 'any' @@ -37,7 +38,20 @@ export type TerminalSelectionEvents = { onTextScaleChange?: (scale: number) => void } +export type TerminalWebViewProps = { + style?: StyleProp + terminalTheme?: MobileTerminalTheme + // Why: baseline zoom multiplier applied on top of fit-to-width scale; raw + // xterm fontSize alone cannot drive apparent size because fitting cancels it. + textScale?: number + onWebReady?: () => void + onEngineError?: (message: string) => void +} & TerminalSelectionEvents + export type TerminalWebViewHandle = { + // Why: iOS can preserve the native view while discarding its JS/backing-store + // state; foreground recovery must wait for the document to answer before replay. + prepareForForegroundRecovery: () => void write: (data: string) => void init: ( cols: number, diff --git a/mobile/src/terminal/terminal-webview-engine-error.test.ts b/mobile/src/terminal/terminal-webview-engine-error.test.ts index 82267967b7c..4e29c287321 100644 --- a/mobile/src/terminal/terminal-webview-engine-error.test.ts +++ b/mobile/src/terminal/terminal-webview-engine-error.test.ts @@ -1,10 +1,18 @@ -import { createElement } from 'react' +import { createElement, createRef } from 'react' +import { Platform } from 'react-native' import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, describe, expect, it, vi } from 'vitest' import { TerminalWebView } from './TerminalWebView' +import type { TerminalWebViewHandle } from './terminal-webview-contract' + +const nativeWebViewMethods = vi.hoisted(() => ({ + postMessage: vi.fn<(message: string) => void>(), + reload: vi.fn<() => void>() +})) vi.mock('react-native', () => ({ AppState: { currentState: 'active' }, + Platform: { OS: 'ios' }, Pressable: 'Pressable', StyleSheet: { absoluteFillObject: { @@ -20,10 +28,14 @@ vi.mock('react-native', () => ({ View: 'View' })) -vi.mock('react-native-webview', () => ({ - WebView: 'WebView', - default: 'WebView' -})) +vi.mock('react-native-webview', async () => { + const React = await import('react') + const WebView = React.forwardRef((props: Record, ref) => { + React.useImperativeHandle(ref, () => nativeWebViewMethods) + return React.createElement('WebView', props) + }) + return { WebView, default: WebView } +}) vi.mock('lucide-react-native', () => ({ RefreshCw: 'RefreshCw' @@ -45,12 +57,15 @@ function suppressReactTestRendererDeprecationWarning(): () => void { // unmount so the timer can't survive into later tests and fire in teardown. let activeRenderer: ReactTestRenderer | null = null -function createTerminalWebViewRenderer(onEngineError = vi.fn()) { +function createTerminalWebViewRenderer( + onEngineError = vi.fn(), + props: Record = {} +) { let renderer: ReactTestRenderer | null = null const restoreConsoleError = suppressReactTestRendererDeprecationWarning() try { act(() => { - renderer = create(createElement(TerminalWebView, { onEngineError })) + renderer = create(createElement(TerminalWebView, { onEngineError, ...props })) }) } finally { restoreConsoleError() @@ -76,6 +91,10 @@ function renderedText(renderer: ReactTestRenderer): string { .join(' ') } +function postedCommands(): Array> { + return nativeWebViewMethods.postMessage.mock.calls.map(([message]) => JSON.parse(message)) +} + describe('TerminalWebView engine errors', () => { afterEach(() => { if (activeRenderer) { @@ -84,6 +103,7 @@ describe('TerminalWebView engine errors', () => { }) activeRenderer = null } + vi.clearAllMocks() vi.restoreAllMocks() }) @@ -175,4 +195,78 @@ describe('TerminalWebView engine errors', () => { vi.useRealTimers() } }) + + it('queues iOS foreground traffic until the current document answers its ping', () => { + const terminalRef = createRef() + const onWebReady = vi.fn() + const terminalTheme = { + mode: 'dark', + theme: { background: '#111111', foreground: '#eeeeee' } + } + const { renderer } = createTerminalWebViewRenderer(vi.fn(), { + ref: terminalRef, + onWebReady, + terminalTheme + }) + postWebViewMessage(renderer, { type: 'web-ready' }) + nativeWebViewMethods.postMessage.mockClear() + + act(() => { + terminalRef.current?.prepareForForegroundRecovery() + terminalRef.current?.write('queued output') + }) + + const ping = postedCommands()[0] + expect(postedCommands().map((command) => command.type)).toEqual(['ping']) + postWebViewMessage(renderer, { type: 'pong', pingId: Number(ping?.id) + 1 }) + expect(postedCommands().map((command) => command.type)).toEqual(['ping']) + + postWebViewMessage(renderer, { type: 'pong', pingId: ping?.id }) + expect(postedCommands().map((command) => command.type)).toEqual(['ping', 'set-theme', 'write']) + expect(onWebReady).toHaveBeenCalledTimes(1) + }) + + it('keeps Android foreground traffic on the existing ready document', () => { + const terminalRef = createRef() + const { renderer } = createTerminalWebViewRenderer(vi.fn(), { ref: terminalRef }) + postWebViewMessage(renderer, { type: 'web-ready' }) + nativeWebViewMethods.postMessage.mockClear() + const mutablePlatform = Platform as { OS: string } + mutablePlatform.OS = 'android' + try { + act(() => { + terminalRef.current?.prepareForForegroundRecovery() + terminalRef.current?.write('android output') + }) + expect(postedCommands().map((command) => command.type)).toEqual(['write']) + } finally { + mutablePlatform.OS = 'ios' + } + }) + + it('reloads a terminated iOS content process and restores theme on readiness', () => { + const terminalRef = createRef() + const { onEngineError, renderer } = createTerminalWebViewRenderer(vi.fn(), { + ref: terminalRef, + terminalTheme: { + mode: 'light', + theme: { background: '#ffffff', foreground: '#111111' } + } + }) + postWebViewMessage(renderer, { type: 'web-ready' }) + nativeWebViewMethods.postMessage.mockClear() + const webView = renderer.root.findByType('WebView') + + act(() => { + webView.props.onContentProcessDidTerminate({ nativeEvent: {} }) + terminalRef.current?.write('after termination') + }) + + expect(nativeWebViewMethods.reload).toHaveBeenCalledTimes(1) + expect(nativeWebViewMethods.postMessage).not.toHaveBeenCalled() + expect(onEngineError).not.toHaveBeenCalled() + + postWebViewMessage(renderer, { type: 'web-ready' }) + expect(postedCommands().map((command) => command.type)).toEqual(['set-theme', 'write']) + }) }) diff --git a/mobile/src/terminal/terminal-webview-engine.test.ts b/mobile/src/terminal/terminal-webview-engine.test.ts index 36c8c039493..503a25a1961 100644 --- a/mobile/src/terminal/terminal-webview-engine.test.ts +++ b/mobile/src/terminal/terminal-webview-engine.test.ts @@ -1,15 +1,95 @@ import { readFileSync } from 'node:fs' import { Script } from 'node:vm' import { parse } from 'acorn' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { XTERM_ENGINE_CSS, XTERM_ENGINE_JS } from './terminal-webview-engine.generated' import { XTERM_HTML } from './terminal-webview-html' +import { TERMINAL_WEBGL_RECOVERY_JS } from './terminal-webview-webgl-recovery-injected' const terminalHtmlSource = readFileSync( new URL('./terminal-webview-html.ts', import.meta.url), 'utf8' ) +function createWebglRecoveryHarness(failSecondAttach = false) { + const variablesStart = terminalHtmlSource.indexOf(' var webglAddon = null;') + const variablesEnd = terminalHtmlSource.indexOf( + '\n', + terminalHtmlSource.indexOf(' var webglRecoveryTimer = null;') + ) + expect(variablesStart).toBeGreaterThanOrEqual(0) + expect(variablesEnd).toBeGreaterThan(variablesStart) + + const timers: Array<() => void> = [] + const addons: Array<{ + clearTextureAtlas: ReturnType + dispose: ReturnType + fireContextLoss: () => void + }> = [] + const term = { + rows: 24, + refresh: vi.fn(), + loadAddon: vi.fn(() => { + if (failSecondAttach && addons.length === 2) { + throw new Error('retry unavailable') + } + }) + } + function WebglAddon() { + let contextLoss = () => {} + const addon = { + clearTextureAtlas: vi.fn(), + dispose: vi.fn(), + fireContextLoss: () => contextLoss() + } + addons.push(addon) + return Object.assign(addon, { + onContextLoss: (listener: () => void) => { + contextLoss = listener + } + }) + } + let visibilityChange = () => {} + const document = { + addEventListener: vi.fn((eventName: string, listener: () => void) => { + if (eventName === 'visibilitychange') { + visibilityChange = listener + } + }), + visibilityState: 'hidden' + } + const applyTerminalTheme = vi.fn() + const flog = vi.fn() + const terminalThemeInput = { mode: 'dark' } + const context = { + applyTerminalTheme, + clearTimeout: vi.fn(), + document, + flog, + setTimeout: (callback: () => void) => { + timers.push(callback) + return timers.length + }, + term, + terminalGeneration: 1, + terminalThemeInput, + window: { WebglAddon: { WebglAddon } } + } + new Script(`${terminalHtmlSource.slice(variablesStart, variablesEnd)} +${TERMINAL_WEBGL_RECOVERY_JS} +attachWebglAddon(true);`).runInNewContext(context) + return { + addons, + applyTerminalTheme, + document, + fireVisibilityChange: () => visibilityChange(), + flog, + term, + terminalThemeInput, + timers + } +} + describe('terminal WebView bundled engine', () => { it('keeps the assembled terminal HTML free of external engine URLs', () => { expect(XTERM_HTML).not.toMatch(/\bhttps?:\/\//) @@ -106,4 +186,57 @@ describe('terminal WebView bundled engine', () => { expect(capSites.length).toBe(2) expect(terminalHtmlSource).toContain('nonFatalErrorNotifies > 5') }) + + it('recreates WebGL once after context loss, then stays on the DOM renderer', () => { + const { addons, flog, term, timers } = createWebglRecoveryHarness() + + expect(addons).toHaveLength(1) + addons[0]?.fireContextLoss() + expect(flog).toHaveBeenCalledWith( + 'webgl-context-loss', + expect.objectContaining({ retry: true }) + ) + expect(addons[0]?.dispose).toHaveBeenCalledTimes(1) + expect(term.refresh).toHaveBeenCalledTimes(1) + expect(timers).toHaveLength(1) + + timers.shift()?.() + expect(addons).toHaveLength(2) + expect(addons[1]?.clearTextureAtlas).toHaveBeenCalledTimes(1) + expect(term.refresh).toHaveBeenCalledTimes(2) + addons[1]?.fireContextLoss() + expect(addons[1]?.dispose).toHaveBeenCalledTimes(1) + expect(term.refresh).toHaveBeenCalledTimes(3) + expect(timers).toHaveLength(0) + }) + + it('falls back to a refreshed DOM renderer when the delayed WebGL retry fails', () => { + const { addons, term, timers } = createWebglRecoveryHarness(true) + + addons[0]?.fireContextLoss() + timers.shift()?.() + + expect(addons).toHaveLength(2) + expect(addons[1]?.dispose).toHaveBeenCalledTimes(1) + expect(term.refresh).toHaveBeenCalledTimes(2) + }) + + it('reapplies theme, clears the active atlas, and refreshes when visible', () => { + const harness = createWebglRecoveryHarness() + + harness.fireVisibilityChange() + expect(harness.applyTerminalTheme).not.toHaveBeenCalled() + + harness.document.visibilityState = 'visible' + harness.fireVisibilityChange() + + expect(harness.applyTerminalTheme).toHaveBeenCalledWith(harness.terminalThemeInput) + expect(harness.addons[0]?.clearTextureAtlas).toHaveBeenCalledTimes(1) + expect(harness.term.refresh).toHaveBeenCalledTimes(1) + }) + + it('answers native readiness probes from the live document', () => { + expect(terminalHtmlSource).toContain("if (msg.type === 'ping')") + expect(terminalHtmlSource).toContain("notify({ type: 'pong', pingId: msg.id })") + }) }) diff --git a/mobile/src/terminal/terminal-webview-html.ts b/mobile/src/terminal/terminal-webview-html.ts index 63e4d2b9ed7..fcd5ea9cafa 100644 --- a/mobile/src/terminal/terminal-webview-html.ts +++ b/mobile/src/terminal/terminal-webview-html.ts @@ -7,7 +7,9 @@ import { TERMINAL_PATH_TAP_JS } from './terminal-path-tap-injected' import { XTERM_ENGINE_CSS, XTERM_ENGINE_JS } from './terminal-webview-engine.generated' import { TERMINAL_REFLOW_JS } from './terminal-webview-reflow-injected' import { TERMINAL_TAP_DISPATCH_JS } from './terminal-webview-tap-dispatch-injected' +import { TERMINAL_WEBVIEW_THEME_JS } from './terminal-webview-theme-injected' import { URL_TAP_WEBVIEW_JS } from './terminal-webview-url-tap' +import { TERMINAL_WEBGL_RECOVERY_JS } from './terminal-webview-webgl-recovery-injected' const DEFAULT_TERMINAL_THEME: RuntimeMobileTerminalTheme['theme'] = { background: colors.terminalBg, @@ -292,7 +294,10 @@ window.onerror = function(msg) { var initRows = 24; var terminalGeneration = 0; var defaultTheme = ${JSON.stringify(DEFAULT_TERMINAL_THEME)}; + var terminalThemeInput = null; var terminalTheme = defaultTheme; + var webglAddon = null; + var webglRecoveryTimer = null; var activeAltScreenSnapshot = false; var trackedMouseTrackingMode = 'none'; var sgrMouseMode = false; @@ -380,27 +385,7 @@ window.onerror = function(msg) { }, 550); } - function normalizeTerminalTheme(input) { - var source = input && typeof input === 'object' && input.theme && typeof input.theme === 'object' - ? input.theme - : null; - if (!source) return defaultTheme; - var next = {}; - var keys = Object.keys(defaultTheme); - for (var i = 0; i < keys.length; i++) { - var key = keys[i]; - if (typeof source[key] === 'string') next[key] = source[key]; - } - return Object.assign({}, defaultTheme, next); - } - - function applyTerminalTheme(input) { - terminalTheme = normalizeTerminalTheme(input); - var background = terminalTheme.background || '${colors.terminalBg}'; - document.documentElement.style.background = background; - document.body.style.background = background; - if (term) term.options.theme = terminalTheme; - } +${TERMINAL_WEBVIEW_THEME_JS} function getCellHeight() { if (!term || !term._core) return 15; @@ -679,6 +664,8 @@ window.onerror = function(msg) { pumpWrites(terminalGeneration); } +${TERMINAL_WEBGL_RECOVERY_JS} + function init(cols, rows, initialData, nextTheme, nextFontScale, preserveScroll, nextOscLinks) { if (typeof nextFontScale === 'number' && nextFontScale > 0) currentTextScale = nextFontScale; // Why: a width-reflow re-stream rewraps the same content at new cols. @@ -688,6 +675,8 @@ window.onerror = function(msg) { var scrollAnchorRows = prevB ? Math.max(0, (prevB.baseY || 0) - (prevB.viewportY || 0)) : -1; terminalGeneration++; var gen = terminalGeneration; + cancelWebglContextRecovery(); + webglAddon = null; ready = false; resetWriteQueue(); statusDotPendingSelector = false; @@ -750,9 +739,7 @@ window.onerror = function(msg) { allowProposedApi: true }); term.open(surface); - if (window.WebglAddon && window.WebglAddon.WebglAddon) { - try { var webglAddon = new window.WebglAddon.WebglAddon(); term.loadAddon(webglAddon); if (webglAddon.onContextLoss) webglAddon.onContextLoss(function() { try { webglAddon && webglAddon.dispose && webglAddon.dispose(); } catch (e) {} }); } catch (e) {} - } + attachWebglAddon(true); if (window.Unicode11Addon && window.Unicode11Addon.Unicode11Addon) try { term.loadAddon(new window.Unicode11Addon.Unicode11Addon()); term.unicode.activeVersion = '11'; } catch (e) {} if (typeof replayData === 'string' && replayData.length > 0) { enqueueWrite(replayData); @@ -936,7 +923,9 @@ window.onerror = function(msg) { handledMessageIds.push(msg.id); if (handledMessageIds.length > 256) handledMessageIds.shift(); } - if (msg.type === 'init') { + if (msg.type === 'ping') { + notify({ type: 'pong', pingId: msg.id }); + } else if (msg.type === 'init') { init(msg.cols, msg.rows, msg.initialData, msg.terminalTheme, msg.fontScale, msg.preserveScroll, msg.oscLinks); } else if (msg.type === 'set-font-scale') { // Why: ignore RN echoing back the value a pinch just set (msg.fontScale === diff --git a/mobile/src/terminal/terminal-webview-messages.ts b/mobile/src/terminal/terminal-webview-messages.ts index f38c5a1dfbb..c031363268c 100644 --- a/mobile/src/terminal/terminal-webview-messages.ts +++ b/mobile/src/terminal/terminal-webview-messages.ts @@ -2,6 +2,7 @@ import type { RuntimeMobileTerminalTheme } from '../../../src/shared/runtime-typ import type { TerminalOscLinkRange } from './terminal-osc-link-ranges' export type TerminalWebViewCommand = + | { type: 'ping'; id?: number } | { type: 'write'; id?: number; data: string } | { type: 'init' diff --git a/mobile/src/terminal/terminal-webview-text-zoom.test.ts b/mobile/src/terminal/terminal-webview-text-zoom.test.ts index aeb7785751c..09aeb82d3f7 100644 --- a/mobile/src/terminal/terminal-webview-text-zoom.test.ts +++ b/mobile/src/terminal/terminal-webview-text-zoom.test.ts @@ -10,6 +10,10 @@ const terminalHtmlSource = readFileSync( new URL('./terminal-webview-html.ts', import.meta.url), 'utf8' ) +const terminalWebglRecoverySource = readFileSync( + new URL('./terminal-webview-webgl-recovery-injected.ts', import.meta.url), + 'utf8' +) function extractStatusDotNormalizer() { const declarationStart = terminalHtmlSource.indexOf(' var CLAUDE_STATUS_DOT =') @@ -148,12 +152,12 @@ describe('TerminalWebView text zoom', () => { it('uses the bundled WebGL-capable xterm stack and platform-safe font fallbacks', () => { expect(terminalHtmlSource).not.toContain('cdn.jsdelivr.net') - expect(terminalHtmlSource).toContain('window.WebglAddon.WebglAddon') + expect(terminalWebglRecoverySource).toContain('window.WebglAddon.WebglAddon') expect(terminalHtmlSource).toContain('function isIOSWebView()') expect(terminalHtmlSource).toContain('fontFamily: terminalFontFamily') expect(terminalHtmlSource).toContain("fontWeight: '300'") expect(terminalHtmlSource).toContain("fontWeightBold: '500'") - expect(terminalHtmlSource).toContain('new window.WebglAddon.WebglAddon()') + expect(terminalWebglRecoverySource).toContain('new window.WebglAddon.WebglAddon()') }) const IOS_IPHONE_NAVIGATOR = { diff --git a/mobile/src/terminal/terminal-webview-theme-injected.ts b/mobile/src/terminal/terminal-webview-theme-injected.ts new file mode 100644 index 00000000000..1f798fe337e --- /dev/null +++ b/mobile/src/terminal/terminal-webview-theme-injected.ts @@ -0,0 +1,27 @@ +import { colors } from '../theme/mobile-theme' + +// Theme normalization and page-surface painting injected into the WebView IIFE. +export const TERMINAL_WEBVIEW_THEME_JS = ` + function normalizeTerminalTheme(input) { + var source = input && typeof input === 'object' && input.theme && typeof input.theme === 'object' + ? input.theme + : null; + if (!source) return defaultTheme; + var next = {}; + var keys = Object.keys(defaultTheme); + for (var i = 0; i < keys.length; i++) { + var key = keys[i]; + if (typeof source[key] === 'string') next[key] = source[key]; + } + return Object.assign({}, defaultTheme, next); + } + + function applyTerminalTheme(input) { + terminalThemeInput = input; + terminalTheme = normalizeTerminalTheme(input); + var background = terminalTheme.background || '${colors.terminalBg}'; + document.documentElement.style.background = background; + document.body.style.background = background; + if (term) term.options.theme = terminalTheme; + } +` diff --git a/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts b/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts new file mode 100644 index 00000000000..8e4d27c2348 --- /dev/null +++ b/mobile/src/terminal/terminal-webview-webgl-recovery-injected.ts @@ -0,0 +1,62 @@ +// WebGL loss and visibility recovery injected into the terminal WebView IIFE. +// It closes over term, terminalGeneration, theme state, and xterm's addon global. +export const TERMINAL_WEBGL_RECOVERY_JS = ` + function refreshTerminalSurface() { + if (!term) return; + try { term.refresh(0, Math.max(0, term.rows - 1)); } catch (e) {} + } + + function cancelWebglContextRecovery() { + if (!webglRecoveryTimer) return; + clearTimeout(webglRecoveryTimer); + webglRecoveryTimer = null; + } + + function attachWebglAddon(allowRecovery) { + if (!term || !window.WebglAddon || !window.WebglAddon.WebglAddon) return false; + var addon = null; + try { + addon = new window.WebglAddon.WebglAddon(); + webglAddon = addon; + if (addon.onContextLoss) addon.onContextLoss(function() { + if (webglAddon !== addon) return; + flog('webgl-context-loss', { retry: allowRecovery }); + webglAddon = null; + try { addon.dispose(); } catch (e) {} + refreshTerminalSurface(); + if (!allowRecovery) return; + // Why: one delayed retry handles transient iOS context loss without + // entering a GPU crash loop; a second loss stays on the DOM renderer. + cancelWebglContextRecovery(); + var recoveryTerm = term; + var recoveryGeneration = terminalGeneration; + webglRecoveryTimer = setTimeout(function() { + webglRecoveryTimer = null; + if (term !== recoveryTerm || terminalGeneration !== recoveryGeneration) return; + attachWebglAddon(false); + }, 100); + }); + term.loadAddon(addon); + if (!allowRecovery) { + try { if (addon.clearTextureAtlas) addon.clearTextureAtlas(); } catch (e) {} + refreshTerminalSurface(); + } + return true; + } catch (e) { + flog('webgl-attach-failed', { retry: !allowRecovery, message: String(e) }); + if (webglAddon === addon) webglAddon = null; + try { if (addon) addon.dispose(); } catch (disposeError) {} + refreshTerminalSurface(); + return false; + } + } + + document.addEventListener('visibilitychange', function() { + if (document.visibilityState !== 'visible') return; + // Why: iOS may restore the xterm model while discarding GPU pixels/theme + // paint state, so visibility must rebuild the atlas and repaint every row. + applyTerminalTheme(terminalThemeInput); + try { if (webglAddon && webglAddon.clearTextureAtlas) webglAddon.clearTextureAtlas(); } catch (e) {} + refreshTerminalSurface(); + }); +`