perf: preserve matching Terminal settings sections during search (#23164)

* perf: preserve matching Terminal settings sections during search

* fix(settings): keep a retained Terminal sessions section fresh and share the section stack

Move the search-stable section wrapper into SettingsSectionStack so every settings
pane keys sections by their own element key instead of the array index, and subscribe
ManageSessionsSection to the daemon session inventory invalidation signal so a kill
routed through another surface still reaches a section that no longer remounts.

Restates the read-count oracles as node identity plus non-growing read counts so a
future freshness fix does not read as a regression.

* test(settings): mock the Windows administrator section in the Terminal section-lifetime suite

* style: format Terminal section-lifetime reliability gate
This commit is contained in:
Neil
2026-10-19 15:39:20 -07:00
committed by GitHub
parent 8e1696f5d7
commit daad41f892
4 changed files with 359 additions and 12 deletions
+79
View File
@@ -1284,6 +1284,85 @@
],
"demotionRule": "Keep experimental until soak; investigate owner and refresh regressions without weakening assertions or adding retries."
},
{
"id": "settings.terminal-section-lifetime",
"title": "Terminal settings search preserves matching section state",
"maturity": "experimental",
"protection": "partial",
"owner": "terminal-runtime",
"layer": "renderer-unit",
"surfaces": [
"Terminal settings search",
"session diagnostic inventory",
"scrollback settings draft"
],
"platforms": ["macos", "linux", "windows"],
"providers": ["local", "daemon", "ssh", "wsl", "remote-runtime"],
"coveredPlatforms": ["macos"],
"coveredProviders": [],
"coverageNotes": "Actual TerminalPane, ManageSessionsSection, health listener and scrollback form with mocked store/provider and display/action boundaries. Unix/Windows and Mac-client/Windows-host flags simulated; no physical provider, process-stop or native input coverage.",
"motivatingLinks": [
"https://github.com/stablyai/orca/blob/main/src/renderer/src/components/settings/TerminalPane.tsx"
],
"invariant": "A matching settings section retains its state when preceding sections are filtered away; a section removed from search releases its owner and refreshes when shown again. A retained section still refreshes when another surface invalidates the session inventory. Diagnostic inventory never becomes authority for process death.",
"oracle": "Across 15 matched search edits the matched section keeps the same DOM node and the inventory/health read counts do not grow, instead of six reads each. A foreign inventory invalidation still reads once on the retained section and not after unmount. A draft and pending target survive matched search; actual hide/reopen reloads, explicit Refresh still reads, and hidden/unmounted health listeners do not fire.",
"commands": [
"ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/settings/TerminalPane.section-lifetime.test.tsx src/renderer/src/components/settings/TerminalTccAttributionNotice.test.tsx"
],
"testFiles": [
"src/renderer/src/components/settings/TerminalPane.section-lifetime.test.tsx",
"src/renderer/src/components/settings/TerminalTccAttributionNotice.test.tsx"
],
"assertionRefs": [
{
"file": "src/renderer/src/components/settings/TerminalPane.section-lifetime.test.tsx",
"assertions": [
"refreshes a retained section when another surface invalidates the inventory",
"refreshes explicitly, reloads genuinely hidden sections, and removes focus listeners",
"preserves an uncommitted scrollback draft while its section remains visible",
"retains a pending action target only while its section remains visible"
]
}
],
"evidenceRuns": [
{
"date": "2026-09-26",
"runner": "local",
"platform": "macos",
"command": "ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/settings/TerminalPane.section-lifetime.test.tsx src/renderer/src/components/settings/TerminalTccAttributionNotice.test.tsx",
"result": "passed",
"durationSeconds": 1.48,
"summary": "12 tests/2 suites pass after the shared SettingsSectionStack adoption and the inventory-invalidation subscription. Read-count oracles restated as node identity plus non-growing read counts; original main fails all three simulated platform cases, the draft, the pending target and the foreign-invalidation refresh."
}
],
"runtimeBudget": {
"p95Seconds": 10,
"scope": "Focused renderer-unit suites, p95 not established."
},
"flakeHistory": {
"status": "not-started",
"evidence": "Author and independent local evidence only; no CI soak."
},
"redGreenEvidence": {
"status": "complete",
"evidence": "Original new suite fails all three simulated platform read-count cases, draft and pending target; one cleanup/refresh control passes. Candidate6pass. Independent scratch5pass/original4fail1control."
},
"performanceBudget": {
"required": true,
"evidence": "Actual child inventory and health reads 6 to 1 across 15 matched search edits. Fixed small section filter/map now lives in the shared SettingsSectionStack; no new polling, cache or subprocess. One added in-renderer invalidation listener replaces the incidental search-driven refresh, so a retained section stays as fresh as the signal every other inventory reader already uses. No packaged CPU/frame/heap claim."
},
"knownGaps": [
"CI soak pending.",
"No physical local/daemon/SSH/WSL/remote-runtime provider, native Linux/Windows or process-stop behavior exercised; routing, PTY ownership, commands and wire formats unchanged.",
"Matched searches keep the loaded snapshot until existing Refresh, completed mutation, a foreign inventory invalidation or genuine hide/reopen; health also refreshes on focus/revision.",
"Pending action target and drafts remain while their parent section matches, including individual controls hidden inside a retained section.",
"No packaged app latency, heap, real focus or pending destructive-mutation completion evidence. Hidden Electron component visual proof is separate from the gate."
],
"promotionCriteria": [
"Retain count, real draft, pending target and cleanup oracles; obtain stable CI history and relevant full-app/provider evidence before promotion."
],
"demotionRule": "Keep experimental until soak; investigate stale state or cleanup failures without weakening read-count, draft or fresh-owner assertions."
},
{
"id": "agent-session.journal-streaming-replay",
"title": "Journal replay bounds obsolete revision memory without changing recovery",
@@ -9,7 +9,10 @@ import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane'
import { useDaemonActions, DaemonActionDialog } from '../shared/useDaemonActions'
import { ManageSessionKillDialog } from './ManageSessionKillDialog'
import { ManageSessionsTable } from './ManageSessionsTable'
import { notifyDaemonSessionInventoryInvalidated } from '../status-bar/daemon-session-inventory-invalidation'
import {
notifyDaemonSessionInventoryInvalidated,
subscribeDaemonSessionInventoryInvalidated
} from '../status-bar/daemon-session-inventory-invalidation'
import {
MANAGE_SESSIONS_SECTION_ID,
TerminalTccAttributionNotice
@@ -28,6 +31,7 @@ export function ManageSessionsSection(): React.JSX.Element {
const optimisticRollback = useRef<PtyManagementSession[] | null>(null)
const isMounted = useRef(true)
const mutationInFlight = useRef(false)
const notifyingOwnInvalidation = useRef(false)
const tabsByWorktree = useAppStore((s) => s.tabsByWorktree)
const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId)
@@ -109,6 +113,29 @@ export function ManageSessionsSection(): React.JSX.Element {
void refresh()
}, [refresh])
// Why: this section stays mounted for a whole settings search, so a kill routed through
// another surface (status-bar Kill All) must reach the table through the invalidation
// signal rather than a remount. Own mutations already read on settle.
useEffect(
() =>
subscribeDaemonSessionInventoryInvalidated(() => {
if (notifyingOwnInvalidation.current || mutationInFlight.current) {
return
}
void refresh()
}),
[refresh]
)
const notifyForeignSurfaces = useCallback((): void => {
notifyingOwnInvalidation.current = true
try {
notifyDaemonSessionInventoryInvalidated()
} finally {
notifyingOwnInvalidation.current = false
}
}, [])
const sessionCount = sessions.length
const daemonActions = useDaemonActions({
@@ -128,7 +155,7 @@ export function ManageSessionsSection(): React.JSX.Element {
void refresh()
},
onRestartSettled: () => {
notifyDaemonSessionInventoryInvalidated()
notifyForeignSurfaces()
setAttributionRefreshRevision((revision) => revision + 1)
void refresh()
}
@@ -158,7 +185,7 @@ export function ManageSessionsSection(): React.JSX.Element {
)
}
mutationInFlight.current = false
notifyDaemonSessionInventoryInvalidated()
notifyForeignSurfaces()
await refresh()
} catch (err) {
toast.error(
@@ -178,7 +205,7 @@ export function ManageSessionsSection(): React.JSX.Element {
}
}
},
[refresh]
[notifyForeignSurfaces, refresh]
)
const runConfirmed = useCallback(() => {
@@ -0,0 +1,246 @@
// @vitest-environment happy-dom
import type { ReactNode } from 'react'
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
import { getDefaultSettings } from '../../../../shared/constants'
import type { PtyManagementSession } from '../../../../preload/api-types'
import { notifyDaemonSessionInventoryInvalidated } from '../status-bar/daemon-session-inventory-invalidation'
import { TerminalPane } from './TerminalPane'
const fake = vi.hoisted(() => ({
query: '',
isWindows: false,
isMac: false,
listSessions: vi.fn(async () => ({ sessions: [] })),
attribution: vi.fn(async () => ({ health: 'intact' })),
tabs: {},
ptys: {},
action: vi.fn(),
session: {
sessionId: 'synthetic-session',
state: 'running' as const,
shellState: 'ready' as const,
isAlive: true,
pid: null,
cwd: null,
cols: 80,
rows: 24,
createdAt: 0,
protocolVersion: 1
}
}))
vi.mock('@/i18n/i18n', () => ({
i18n: { language: 'en' },
translate: (_key: string, fallback: string) => fallback
}))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
selector({
settingsSearchQuery: fake.query,
tabsByWorktree: fake.tabs,
ptyIdsByTabId: fake.ptys,
setActiveView: fake.action,
closeSettingsPage: fake.action,
openSettingsTarget: fake.action,
openSettingsPage: fake.action,
setSettingsSearchQuery: fake.action
})
}))
vi.mock('@/components/terminal-pane/pane-helpers', () => ({
isMacUserAgent: () => fake.isMac,
isWindowsUserAgent: () => fake.isWindows
}))
vi.mock('@/lib/keyboard-layout/use-effective-mac-option-as-alt', () => ({
useDetectedOptionAsAlt: () => 'us'
}))
vi.mock('@/components/settings/TerminalInteractionSection', () => ({
TerminalInteractionSection: () => null
}))
vi.mock('@/components/settings/TerminalRenderingSection', () => ({
TerminalRenderingSection: () => null
}))
vi.mock('@/components/settings/TerminalSetupScriptSection', () => ({
TerminalSetupScriptSection: () => null
}))
vi.mock('@/components/settings/TerminalWindowsShellSection', () => ({
TerminalWindowsShellSection: () => null
}))
vi.mock('@/components/settings/TerminalWindowsAdministratorSection', () => ({
TerminalWindowsAdministratorSection: () => null
}))
vi.mock('@/components/settings/SearchableSetting', () => ({
SearchableSetting: ({ children }: { children: ReactNode }) => <>{children}</>
}))
vi.mock('@/components/settings/ManageSessionsTable', () => ({
ManageSessionsTable: ({
onRefresh,
onRequestKill
}: {
onRefresh: () => void
onRequestKill: (session: PtyManagementSession) => void
}) => (
<>
<button onClick={onRefresh}>Refresh sessions</button>
<button onClick={() => onRequestKill(fake.session)}>Preview session action</button>
</>
)
}))
vi.mock('@/components/settings/ManageSessionKillDialog', () => ({
ManageSessionKillDialog: ({ session }: { session: PtyManagementSession | null }) =>
session ? <span>Pending action for {session.sessionId}</span> : null
}))
vi.mock('@/components/shared/useDaemonActions', () => ({
useDaemonActions: () => ({ isBusy: false, busyKind: null, setPending: fake.action }),
DaemonActionDialog: () => null
}))
vi.mock('@/lib/worktree-activation', () => ({ activateAndRevealWorktree: vi.fn() }))
vi.mock('@/lib/activate-tab-and-focus-pane', () => ({ activateTabAndFocusPane: vi.fn() }))
beforeEach(() => {
vi.clearAllMocks()
fake.query = ''
fake.isWindows = false
fake.isMac = false
Object.assign(window, {
api: {
pty: {
management: {
listSessions: fake.listSessions,
macTccAttribution: fake.attribution
}
}
}
})
})
afterEach(() => {
cleanup()
Reflect.deleteProperty(window, 'api')
})
function paneProps() {
return {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
scrollbackMode: 'preset' as const,
setScrollbackMode: vi.fn()
}
}
it.each([
{ name: 'Unix client and host', isWindows: false, isMac: false, hostIsWindows: false },
{ name: 'Windows client and host', isWindows: true, isMac: false, hostIsWindows: true },
{ name: 'Mac client and Windows host', isWindows: false, isMac: true, hostIsWindows: true }
])('keeps retained session sections mounted on $name', async (platform) => {
fake.isWindows = platform.isWindows
fake.isMac = platform.isMac
const props = { ...paneProps(), isWindowsTerminalHost: platform.hostIsWindows }
const view = render(<TerminalPane {...props} />)
await act(async () => {})
// The invariant is identity, not a magic number: the matched section keeps the same DOM
// node across every query edit, so editing the query cannot cost an extra read.
const mountedButton = screen.getByRole('button', { name: 'Refresh sessions' })
const inventoryReadsAfterMount = fake.listSessions.mock.calls.length
const healthReadsAfterMount = fake.attribution.mock.calls.length
for (const query of [
'm',
'ma',
'man',
'mana',
'manag',
'manage',
'',
't',
'te',
'ter',
'term',
'termi',
'termin',
'termina',
'terminal'
]) {
fake.query = query
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.getByRole('button', { name: 'Refresh sessions' })).toBe(mountedButton)
}
expect(fake.listSessions.mock.calls.length).toBe(inventoryReadsAfterMount)
expect(fake.attribution.mock.calls.length).toBe(healthReadsAfterMount)
})
it('refreshes a retained section when another surface invalidates the inventory', async () => {
const props = paneProps()
const view = render(<TerminalPane {...props} />)
await act(async () => {})
const mountedButton = screen.getByRole('button', { name: 'Refresh sessions' })
const readsAfterMount = fake.listSessions.mock.calls.length
fake.query = 'manage'
await act(async () => view.rerender(<TerminalPane {...props} />))
await act(async () => notifyDaemonSessionInventoryInvalidated())
expect(screen.getByRole('button', { name: 'Refresh sessions' })).toBe(mountedButton)
expect(fake.listSessions.mock.calls.length).toBe(readsAfterMount + 1)
view.unmount()
const readsBeforeUnmountedNotify = fake.listSessions.mock.calls.length
await act(async () => notifyDaemonSessionInventoryInvalidated())
expect(fake.listSessions.mock.calls.length).toBe(readsBeforeUnmountedNotify)
})
it('refreshes explicitly, reloads genuinely hidden sections, and removes focus listeners', async () => {
const props = paneProps()
const view = render(<TerminalPane {...props} />)
await act(async () => {})
fireEvent.click(screen.getByRole('button', { name: 'Refresh sessions' }))
await act(async () => {})
expect(fake.listSessions).toHaveBeenCalledTimes(2)
expect(fake.attribution).toHaveBeenCalledTimes(1)
fake.query = 'GPU Acceleration'
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.queryByRole('button', { name: 'Refresh sessions' })).toBeNull()
await act(async () => window.dispatchEvent(new Event('focus')))
expect(fake.attribution).toHaveBeenCalledTimes(1)
fake.query = 'manage'
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.getByRole('button', { name: 'Refresh sessions' })).toBeTruthy()
expect(fake.listSessions).toHaveBeenCalledTimes(3)
expect(fake.attribution).toHaveBeenCalledTimes(2)
await act(async () => window.dispatchEvent(new Event('focus')))
expect(fake.attribution).toHaveBeenCalledTimes(3)
view.unmount()
await act(async () => window.dispatchEvent(new Event('focus')))
expect(fake.attribution).toHaveBeenCalledTimes(3)
})
it('preserves an uncommitted scrollback draft while its section remains visible', async () => {
const props = { ...paneProps(), scrollbackMode: 'custom' as const }
const view = render(<TerminalPane {...props} />)
await act(async () => {})
fireEvent.change(screen.getByRole('spinbutton'), { target: { value: '12345' } })
expect(props.updateSettings).not.toHaveBeenCalled()
fake.query = 'scrollback'
await act(async () => view.rerender(<TerminalPane {...props} />))
const input = screen.getByRole('spinbutton')
expect(input.getAttribute('value')).toBe('12345')
fireEvent.blur(input)
expect(props.updateSettings).toHaveBeenCalledWith({ terminalScrollbackRows: 12345 })
})
it('retains a pending action target only while its section remains visible', async () => {
const props = paneProps()
const view = render(<TerminalPane {...props} />)
await act(async () => {})
fireEvent.click(screen.getByRole('button', { name: 'Preview session action' }))
fake.query = 'manage'
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.getByText('Pending action for synthetic-session')).toBeTruthy()
fake.query = 'GPU Acceleration'
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.queryByText('Pending action for synthetic-session')).toBeNull()
fake.query = 'manage'
await act(async () => view.rerender(<TerminalPane {...props} />))
expect(screen.queryByText('Pending action for synthetic-session')).toBeNull()
})
@@ -1,10 +1,10 @@
import { useState } from 'react'
import { ChevronDown } from 'lucide-react'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import { Separator } from '../ui/separator'
import { Textarea } from '../ui/textarea'
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '../ui/collapsible'
import { cn } from '@/lib/utils'
import { SettingsSectionStack } from './SettingsSectionStack'
import { matchesSettingsSearch } from './settings-search'
import { useAppStore } from '../../store'
import { isMacUserAgent, isWindowsUserAgent } from '@/components/terminal-pane/pane-helpers'
@@ -353,16 +353,11 @@ export function TerminalPane({
isMac={isMac}
/>
) : null
].filter(Boolean)
]
return (
<div className="space-y-6">
{visibleSections.map((section, index) => (
<div key={index} className="space-y-6">
{index > 0 ? <Separator /> : null}
{section}
</div>
))}
<SettingsSectionStack sections={visibleSections} spacing="section" />
</div>
)
}