From 61ebffa86e085663675af978da94f172d7254792 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sat, 5 Sep 2026 16:49:59 -0400 Subject: [PATCH 1/3] fix(runtime): bound terminal-wait blocked-prompt rules to the live screen bottom (#18817) The Codex prompt rules in terminal-wait-detection scanned the whole retained tail (up to 256 KiB) with lastIndexOf, so any quoted prompt phrase in scrollback registered as a live prompt. A Codex agent working on Orca prints rg hits from this very file; one such line ~300 lines above an idle input box made `orca terminal send` refuse with agent_prompt_blocked and `terminal wait --for tui-idle` report codex-interactive-prompt. `clear` did not help because the detector reads the retained tail, not the visible screen. A prompt that owns the terminal is at the screen bottom, so every blocked rule now runs over the last 12 non-blank lines (real Codex dialogs are 4-8 lines), the way the cursor approval rule already was. The returned index is offset back into full-tail coordinates so ready-header comparisons keep working. The sentinel fast path is unchanged. Line-window primitives move to terminal-wait-tail-window.ts to keep the detector under the max-lines cap. --- .../runtime/terminal-wait-detection.test.ts | 200 ++++++++++++++++++ src/main/runtime/terminal-wait-detection.ts | 39 ++-- src/main/runtime/terminal-wait-tail-window.ts | 48 +++++ 3 files changed, 269 insertions(+), 18 deletions(-) create mode 100644 src/main/runtime/terminal-wait-detection.test.ts create mode 100644 src/main/runtime/terminal-wait-tail-window.ts diff --git a/src/main/runtime/terminal-wait-detection.test.ts b/src/main/runtime/terminal-wait-detection.test.ts new file mode 100644 index 00000000000..eda02e60bbb --- /dev/null +++ b/src/main/runtime/terminal-wait-detection.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it } from 'vitest' +import { + detectTerminalWaitBlockedReason, + isKnownReadyPromptPreview +} from './terminal-wait-detection' +import { buildTerminalWaitText } from './terminal-wait-tail-state' + +// Why these shapes: Codex agents working on Orca print `rg` hits from this very detector and its +// specs, so quoted prompt wording lands in scrollback while the terminal sits at its input box. +const QUOTED_DETECTOR_SOURCE_LINE = + "└ if (hooksindex !== -1 && normalized.includes('press enter to confirm', hooksindex)) {" +const QUOTED_PERMISSION_FIXTURE_LINE = + " └ 236: 'Permission required\\nThis command requires permission\\nAllow once\\nAllow always\\nReject\\n'," + +function codexIdleScreen(): string[] { + return [ + '• Done. The detector bounding is in place and the suite passes.', + '', + '› Ask Codex to do anything', + '', + ' gpt-6-astra medium · ~/orca/workspaces/orca/fix-wait-detector-scrollback' + ] +} + +function codexScrollback(quotedLines: string[], trailingLineCount: number): string[] { + const lines: string[] = [ + '• Explored', + ' └ Search press enter to confirm in src/main/runtime', + ' Read terminal-wait-detection.ts', + '', + '• Ran rg -n "press enter to confirm" src/main/runtime/terminal-wait-detection.ts src/main/runtime/orca-runtime-tests/agent-status-and-waits.spec.ts', + ' └ src/main/runtime/terminal-wait-detection.ts', + ' src/main/runtime/orca-runtime-tests/agent-status-and-waits.spec.ts', + ' src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-07.spec.ts', + ...quotedLines + ] + for (let index = 0; index < trailingLineCount; index += 1) { + lines.push(` ${index}: unrelated codex narration about hook wiring and sandbox policy`) + } + return lines +} + +function waitTextFor(lines: string[]): string { + return buildTerminalWaitText(lines, '', '') +} + +describe('detectTerminalWaitBlockedReason scrollback bounding', () => { + it('ignores detector source quoted by rg output far above an idle Codex input box', () => { + const waitText = waitTextFor([ + ...codexScrollback([QUOTED_DETECTOR_SOURCE_LINE], 300), + ...codexIdleScreen() + ]) + + expect(waitText).toContain('press enter to confirm') + expect(detectTerminalWaitBlockedReason(waitText)).toBeNull() + }) + + it('ignores a quoted permission fixture in scrollback above an idle Codex input box', () => { + const waitText = waitTextFor([ + ...codexScrollback([QUOTED_PERMISSION_FIXTURE_LINE], 300), + ...codexIdleScreen() + ]) + + expect(waitText.toLowerCase()).toContain('allow once') + expect(detectTerminalWaitBlockedReason(waitText)).toBeNull() + }) + + it('ignores quoted prompt wording just above the live-dialog window', () => { + // Why 10: with the 3-line idle screen the quoted lines sit 13-14 non-blank lines from the bottom. + const waitText = waitTextFor([ + ...codexScrollback([QUOTED_DETECTOR_SOURCE_LINE, QUOTED_PERMISSION_FIXTURE_LINE], 10), + ...codexIdleScreen() + ]) + + expect(detectTerminalWaitBlockedReason(waitText)).toBeNull() + }) + + it('does not let quoted scrollback wording veto a Codex ready header', () => { + const waitText = waitTextFor([ + ...codexScrollback([QUOTED_PERMISSION_FIXTURE_LINE], 40), + ' >_ OpenAI Codex (v0.153.3)', + ' model: gpt-6-astra medium /model to change', + ' directory: ~/orca/workspaces/orca/fix-wait-detector-scrollback' + ]) + + expect(isKnownReadyPromptPreview(waitText)).toBe(true) + }) +}) + +// Real dialog text: terminal-creation-and-readiness-part-07.spec.ts and agent-status-and-waits.spec.ts. +const LIVE_CODEX_PROMPTS: { name: string; lines: string[]; reason: string }[] = [ + { + name: 'hooks review', + lines: [ + 'Hooks need review', + '2 hooks are new or changed.', + '1. Review hooks', + '2. Trust all and continue', + 'Press enter to confirm or esc to go back' + ], + reason: 'codex-hooks-review-prompt' + }, + { + name: 'trust workspace', + lines: ['Do you trust this workspace directory?', '1. Yes', '2. No'], + reason: 'codex-trust-workspace' + }, + { + name: 'update', + lines: [ + 'Update available! 0.131.0 -> 0.132.0', + '1. Update now', + '2. Skip', + 'Press enter to continue' + ], + reason: 'codex-update-prompt' + }, + { + name: 'cwd selection', + lines: [ + 'Choose working directory to resume this session', + ' Session = latest cwd recorded in the resumed session', + ' Current = your current working directory', + ' Press enter to continue' + ], + reason: 'codex-cwd-prompt' + }, + { + name: 'model migration', + lines: [ + 'Codex just got an upgrade. Introducing gpt-5.1-codex-max.', + 'We recommend switching from gpt-5-codex to gpt-5.1-codex-max.', + 'Press enter to continue' + ], + reason: 'codex-model-migration-prompt' + }, + { + name: 'grant permissions', + lines: [ + 'Would you like to grant these permissions?', + '1. Yes, grant these permissions for this turn', + '2. No, continue without permissions', + 'Press enter to confirm or esc to cancel' + ], + reason: 'codex-interactive-prompt' + }, + { + name: 'permission required', + lines: [ + 'Permission required', + 'This command requires permission', + 'Allow once', + 'Allow always', + 'Reject' + ], + reason: 'codex-interactive-prompt' + } +] + +describe('detectTerminalWaitBlockedReason live prompts', () => { + for (const prompt of LIVE_CODEX_PROMPTS) { + it(`still blocks on a live ${prompt.name} prompt after long scrollback`, () => { + const waitText = waitTextFor([ + ...codexScrollback([QUOTED_DETECTOR_SOURCE_LINE, QUOTED_PERMISSION_FIXTURE_LINE], 300), + ...prompt.lines + ]) + + expect(detectTerminalWaitBlockedReason(waitText)).toBe(prompt.reason) + }) + + it(`blocks on a live ${prompt.name} prompt rendered with blank spacer rows`, () => { + // Why: the visible-screen probe joins raw rows, so blank rows between dialog lines must not eat the window. + const spaced = prompt.lines.flatMap((line) => [line, '', '']) + const screen = [ + ' >_ OpenAI Codex (v0.153.3)', + '', + ...spaced, + '', + ' gpt-6-astra medium · ~/orca/workspaces/orca/fix-wait-detector-scrollback', + '' + ].join('\n') + + expect(detectTerminalWaitBlockedReason(screen)).toBe(prompt.reason) + }) + } + + it('reports the newest prompt when a live dialog follows a stale one at the bottom', () => { + const waitText = waitTextFor([ + 'Update available! 0.131.0 -> 0.132.0', + 'Press enter to continue', + ' >_ OpenAI Codex (v0.132.0)', + ' model: gpt-5.5 high /model to change', + ' directory: ~/orca/workspaces/orca/cli-debug', + 'Hooks need review', + 'Press enter to confirm' + ]) + + expect(detectTerminalWaitBlockedReason(waitText)).toBe('codex-hooks-review-prompt') + }) +}) diff --git a/src/main/runtime/terminal-wait-detection.ts b/src/main/runtime/terminal-wait-detection.ts index 85f05f8e699..ed957d1fe2d 100644 --- a/src/main/runtime/terminal-wait-detection.ts +++ b/src/main/runtime/terminal-wait-detection.ts @@ -4,6 +4,11 @@ import { type AgentStatus } from '../../shared/agent-detection' import type { RuntimeTerminalWaitBlockedReason } from '../../shared/runtime-types' +import { + isTerminalWaitWhitespace, + startOfLastLines, + startOfLastNonBlankLines +} from './terminal-wait-tail-window' const EXPLICIT_IDLE_TITLE_RE = /(^|\s)(ready|idle|done)(\s|$|[.!?])/i const CLAUDE_IDLE_PREFIX = '\u2733' @@ -153,11 +158,6 @@ function findAntigravityReadyPromptIndex(normalized: string): number | null { return modelIndex !== null && promptIndex !== null ? Math.max(modelIndex, promptIndex) : null } -function isTerminalWaitWhitespace(value: string, index: number): boolean { - const code = value.charCodeAt(index) - return code === 32 || (code >= 9 && code <= 13) -} - export const TERMINAL_WAIT_BLOCKED_SENTINEL_RE = /update available|choose working directory to|codex just got an upgrade|hooks need review|do you trust|trust this|trusted workspace|press enter to (?:confirm|continue|view|insert)|press t to trust|permission required|requires permission|allow once|allow always|run this command\?/i @@ -206,25 +206,28 @@ function isCursorApprovalChoiceLine(line: string): boolean { ) } -function startOfLastLines(value: string, count: number): number { - let cursor = value.length - for (let seen = 0; seen < count; seen += 1) { - const previous = value.lastIndexOf('\n', cursor - 1) - if (previous === -1) { - return 0 - } - cursor = previous - } - return cursor + 1 -} +// Why bounded: answered dialogs and quoted prompt wording (agents grep this file and its specs) stay in the +// retained tail; only a dialog owning the screen bottom is live. Real Codex dialogs (trust, hooks review, +// update, exec approval) are 4-8 lines; the slack covers a wrapped command or a longer hook list. +const LIVE_PROMPT_TAIL_LINES = 12 function findTerminalWaitBlockedSignal( - normalized: string + fullTail: string ): { reason: RuntimeTerminalWaitBlockedReason; index: number } | null { - // Why: one combined negative scan over the up-to-256 KiB tail avoids a dozen full-tail searches when no prompt can match. + const windowStart = startOfLastNonBlankLines(fullTail, LIVE_PROMPT_TAIL_LINES) + const normalized = windowStart === 0 ? fullTail : fullTail.slice(windowStart) + // Why: one combined negative scan avoids a dozen searches when no prompt can match. if (!TERMINAL_WAIT_BLOCKED_SENTINEL_RE.test(normalized)) { return null } + const signal = findBlockedSignalInLiveWindow(normalized) + // Why: callers compare this index against ready-header indexes found over the full tail. + return signal === null ? null : { reason: signal.reason, index: signal.index + windowStart } +} + +function findBlockedSignalInLiveWindow( + normalized: string +): { reason: RuntimeTerminalWaitBlockedReason; index: number } | null { const candidates: { reason: RuntimeTerminalWaitBlockedReason; index: number }[] = [] const updateIndex = normalized.lastIndexOf('update available') if (updateIndex !== -1 && normalized.includes('press enter to continue', updateIndex)) { diff --git a/src/main/runtime/terminal-wait-tail-window.ts b/src/main/runtime/terminal-wait-tail-window.ts new file mode 100644 index 00000000000..788000328f1 --- /dev/null +++ b/src/main/runtime/terminal-wait-tail-window.ts @@ -0,0 +1,48 @@ +// Line-window primitives over a newline-joined terminal tail, shared by the wait-blocked prompt rules. + +export function isTerminalWaitWhitespace(value: string, index: number): boolean { + const code = value.charCodeAt(index) + return code === 32 || (code >= 9 && code <= 13) +} + +/** Offset where the last `count` lines begin (0 when the tail is shorter). */ +export function startOfLastLines(value: string, count: number): number { + let cursor = value.length + for (let seen = 0; seen < count; seen += 1) { + const previous = value.lastIndexOf('\n', cursor - 1) + if (previous === -1) { + return 0 + } + cursor = previous + } + return cursor + 1 +} + +/** Like `startOfLastLines`, but blank rows don't count toward the window. */ +// Why: the visible-screen probe joins raw rows, so blank spacer rows must not eat a dialog's window. +export function startOfLastNonBlankLines(value: string, count: number): number { + let seen = 0 + let lineEnd = value.length + for (;;) { + const lineStart = value.lastIndexOf('\n', lineEnd - 1) + 1 + if (hasNonWhitespaceBetween(value, lineStart, lineEnd)) { + seen += 1 + if (seen >= count) { + return lineStart + } + } + if (lineStart === 0) { + return 0 + } + lineEnd = lineStart - 1 + } +} + +function hasNonWhitespaceBetween(value: string, start: number, end: number): boolean { + for (let index = start; index < end; index += 1) { + if (!isTerminalWaitWhitespace(value, index)) { + return true + } + } + return false +} From d8c4c830639bf3d603c9250a536e0323ca0a7a19 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:53:27 -0700 Subject: [PATCH 2/3] test(e2e): fence SSH recovery and release exited Electron pipes (#18880) --- config/reliability-gates.jsonc | 133 ++++++++++++++++++ .../helpers/docker-ssh-relay-connection.ts | 32 ++++- .../e2e/helpers/electron-process-shutdown.ts | 17 +++ .../electron-process-shutdown.unit.test.ts | 59 ++++++++ tests/e2e/ssh-docker-half-open-link.spec.ts | 20 +-- ...ssh-docker-transport-drop-recovery.spec.ts | 125 +++++++--------- 6 files changed, 302 insertions(+), 84 deletions(-) create mode 100644 tests/e2e/helpers/electron-process-shutdown.unit.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 9f5885edc8a..2dd39ad7c3f 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -17922,6 +17922,139 @@ "The sentinel changes a pane title within an existing layout; concurrent split and close conflicts remain separate coverage." ], "demotionRule": "Demote if a failed push suppresses an identical retry, a successful equal write resumes redundant churn, or the routed observer journey flakes without a diagnosed cause." + }, + { + "id": "ssh.docker-recovery-and-resource-lifecycle", + "title": "Docker SSH reconnect, host faults, listing and watcher lifecycle", + "maturity": "experimental", + "protection": "partial", + "owner": "terminal-runtime", + "layer": "electron-docker-ssh", + "surfaces": [ + "SSH terminal recovery", + "SSH remote resource ownership", + "remote file listing", + "remote explorer watcher recovery", + "Electron test process cleanup" + ], + "platforms": ["macos", "linux", "windows"], + "providers": ["ssh"], + "coveredPlatforms": ["macos"], + "coveredProviders": ["ssh"], + "coverageNotes": "A macOS Electron client drives a Linux Docker SSH execution host. The six-spec suite passed ten enabled cases with clean worker exit (5.2m). The formerly skipped frozen-host input case now waits for recovered authority before sending input and passed four separate executions (one initial and three repetitions). The existing flooded-shell fixme remains an explicitly reproduced application gap.", + "motivatingLinks": [ + "https://github.com/stablyai/orca/issues/18018", + "https://github.com/stablyai/orca/pull/18546", + "https://github.com/stablyai/orca/issues/12547" + ], + "invariant": "Transport loss and frozen-host silence must preserve the remote session; host relay loss may rebind a pane without accumulating reattachable leases. Reconnects must preserve usable terminal content, bounded PTYs/fds/processes, complete large listings, and independently recoverable watcher processes. Electron test shutdown must release inherited pipes after confirmed root exit without closing live-process pipes.", + "oracle": "Poll a changed connected SSH authority after injected faults, then require terminal output and appropriate PTY identity. Read remote process/fd state, listFiles replies, and rendered explorer rows. Resolve Playwright cleanup only after the root process exits and its inherited pipes close; live-process pipes remain untouched.", + "commands": [ + "ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/ssh-docker-transport-drop-recovery.spec.ts tests/e2e/ssh-docker-half-open-link.spec.ts tests/e2e/ssh-docker-quick-open-large-listing.spec.ts tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts tests/e2e/ssh-docker-resource-accumulation.spec.ts tests/e2e/ssh-docker-watcher-isolation.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", + "pnpm exec vitest run --config config/vitest.config.ts tests/e2e/helpers/electron-process-shutdown.unit.test.ts" + ], + "testFiles": [ + "tests/e2e/ssh-docker-transport-drop-recovery.spec.ts", + "tests/e2e/ssh-docker-half-open-link.spec.ts", + "tests/e2e/ssh-docker-quick-open-large-listing.spec.ts", + "tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts", + "tests/e2e/ssh-docker-resource-accumulation.spec.ts", + "tests/e2e/ssh-docker-watcher-isolation.spec.ts", + "tests/e2e/helpers/electron-process-shutdown.unit.test.ts" + ], + "assertionRefs": [ + { + "file": "tests/e2e/ssh-docker-transport-drop-recovery.spec.ts", + "assertions": [ + "preserves transport-drop PTY and scrollback, replaces relay-loss binding, and keeps one reattachable lease per pane" + ] + }, + { + "file": "tests/e2e/ssh-docker-half-open-link.spec.ts", + "assertions": [ + "leaves connected after host freeze and renders process-produced output after recovery" + ] + }, + { + "file": "tests/e2e/ssh-docker-quick-open-large-listing.spec.ts", + "assertions": [ + "returns both a bounded client page and a complete legacy-client remote listing" + ] + }, + { + "file": "tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts", + "assertions": [ + "restores shell scrollback and full-screen output and opens a usable fresh tab" + ] + }, + { + "file": "tests/e2e/ssh-docker-resource-accumulation.spec.ts", + "assertions": [ + "keeps remote pts devices, relay fds, process counts and inherited master fds bounded" + ] + }, + { + "file": "tests/e2e/ssh-docker-watcher-isolation.spec.ts", + "assertions": [ + "keeps rendered explorer changes and terminal output live after watcher crash and repairs a deleted watcher artifact" + ] + }, + { + "file": "tests/e2e/helpers/electron-process-shutdown.unit.test.ts", + "assertions": [ + "releases inherited pipes after confirmed exit, including prior exit", + "retains live-process pipes on shutdown timeout" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-09-05", + "runner": "local", + "platform": "macos", + "result": "passed", + "command": "pnpm exec vitest run --config config/vitest.config.ts tests/e2e/helpers/electron-process-shutdown.unit.test.ts", + "durationSeconds": 0.168, + "summary": "All three shutdown regression tests passed; disabling pipe release fails the first two by timeout. Two half-open Electron repetitions separately passed in 1.7m without worker teardown timeout." + }, + { + "date": "2026-09-05", + "runner": "local", + "platform": "macos", + "result": "passed", + "command": "ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/ssh-docker-transport-drop-recovery.spec.ts tests/e2e/ssh-docker-half-open-link.spec.ts tests/e2e/ssh-docker-quick-open-large-listing.spec.ts tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts tests/e2e/ssh-docker-resource-accumulation.spec.ts tests/e2e/ssh-docker-watcher-isolation.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", + "durationSeconds": 312, + "summary": "Six specs: ten passed, two existing fixme skipped, clean worker shutdown. Baseline same enabled suite: ten passed but worker teardown timed out (7.3m)." + } + ], + "runtimeBudget": { + "p95Seconds": 420, + "scope": "per Electron Docker test; measured suite p95 and CI soak not yet established" + }, + "flakeHistory": { + "status": "flaky", + "evidence": "Baseline: ten enabled tests passed, two fixme skipped, worker teardown timed out (7.3m). After pipe cleanup: ten passed and worker exited cleanly (5.2m); two half-open repeats passed (1.7m). The formerly skipped thaw-input case failed before its recovered-authority wait and passed 1+3 executions afterward (56.9s + 2.6m). Flood failed both its original input oracle and a strengthened producer-completion oracle after recovery." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "Disabling exited-process pipe release causes two shutdown contract tests to time out; restoring it passes 3/3. Baseline Docker worker teardown failed; final six-spec enabled run and half-open repeats exit successfully. Frozen-host input fails without the post-thaw recovered-authority wait and passes four runs with it. Full product fault/recovery mutation coverage and CI history remain missing." + }, + "performanceBudget": { + "required": true, + "evidence": "Test-only bounded pipe destruction and authority polling; no production polling, subprocesses, or runtime work added. Remote resources are counted instead of using wall-clock leak thresholds." + }, + "promotionCriteria": [ + "Require complete six-spec repeat runs with clean worker shutdown.", + "Resolve the remaining #18018 flooded-shell reproduction and remove its fixme marker.", + "Collect CI runtime and flake history plus product red/green evidence before blocking." + ], + "knownGaps": [ + "The disconnected 48MB flood still loses its relay channel: original post-flood input marker failed in 60s, and waiting for the finite producer completion marker failed in 120s. It remains an explicit #18018 fixme reproduction; frozen-host input is re-enabled after four successful runs.", + "Linux and Windows desktop clients, WSL, folder workspaces, paired runtimes and live agent CLIs are not exercised by these Docker specs.", + "Some legacy assertions inspect terminal serialization or backing state rather than rendered DOM; no blanket visual coverage claim.", + "No p95 CI history or full product mutation proof." + ], + "demotionRule": "Keep experimental while any recovery reproduction fails or any teardown, identity, resource-count, or rendered oracle flakes; never promote by extending sleeps or retries." } ] } diff --git a/tests/e2e/helpers/docker-ssh-relay-connection.ts b/tests/e2e/helpers/docker-ssh-relay-connection.ts index a17ad916e66..3e0c35f3c53 100644 --- a/tests/e2e/helpers/docker-ssh-relay-connection.ts +++ b/tests/e2e/helpers/docker-ssh-relay-connection.ts @@ -1,4 +1,4 @@ -import type { Page } from '@stablyai/playwright-test' +import { expect, type Page } from '@stablyai/playwright-test' import { DOCKER_SSH_PROXY_JUMP_REMOTE_REPO_PATH, @@ -227,3 +227,33 @@ export async function reconnectDisconnectedDockerSshRelayTarget( ): Promise { return performDockerSshRelayReconnect(page, targetId, false) } + +export async function recoverDockerSshRelayAfterFault( + page: Page, + targetId: string, + injectFault: () => void | Promise +): Promise { + const readAuthority = () => + page.evaluate((id) => window.__store?.getState().sshConnectionStates.get(id), targetId) + const before = await readAuthority() + expect(before).toMatchObject({ + status: 'connected', + providerEpoch: expect.any(String), + connectionGeneration: expect.any(Number) + }) + await injectFault() + // The pre-fault connected publication can remain visible until the next IPC event. + await expect + .poll( + async () => { + const after = await readAuthority() + return ( + after?.status === 'connected' && + (after.providerEpoch !== before?.providerEpoch || + after.connectionGeneration !== before?.connectionGeneration) + ) + }, + { timeout: 120_000, message: 'SSH authority did not recover after the injected fault' } + ) + .toBe(true) +} diff --git a/tests/e2e/helpers/electron-process-shutdown.ts b/tests/e2e/helpers/electron-process-shutdown.ts index 5180575f1a6..48ddb60bf43 100644 --- a/tests/e2e/helpers/electron-process-shutdown.ts +++ b/tests/e2e/helpers/electron-process-shutdown.ts @@ -19,6 +19,16 @@ function hasExited(proc: ChildProcess): boolean { return proc.exitCode !== null || proc.signalCode !== null } +function releaseExitedProcessPipes(proc: ChildProcess): void { + if (!hasExited(proc)) { + return + } + // Detached SSH helpers can retain inherited pipes after Electron itself exits. + for (const stream of proc.stdio) { + stream?.destroy() + } +} + function waitForExit(proc: ChildProcess, timeoutMs: number): Promise { if (hasExited(proc)) { return Promise.resolve(true) @@ -166,12 +176,16 @@ export async function forceQuitElectronAppForE2E(app: ElectronApplication): Prom } } await waitForExit(proc, PROCESS_EXIT_TIMEOUT_MS) + releaseExitedProcessPipes(proc) // Hands the dead app back to Playwright so worker teardown has nothing left to wait on. await app.close().catch(() => undefined) } export async function closeElectronAppForE2E(app: ElectronApplication): Promise { const proc = app.process() + const releasePipes = (): void => releaseExitedProcessPipes(proc) + proc.once('exit', releasePipes) + releasePipes() try { await withTimeout(app.close(), GRACEFUL_CLOSE_TIMEOUT_MS, 'Timed out closing Electron app') if (proc) { @@ -184,6 +198,9 @@ export async function closeElectronAppForE2E(app: ElectronApplication): Promise< if (proc) { await forceKillProcessTree(proc) } + } finally { + proc.off('exit', releasePipes) + releasePipes() } } diff --git a/tests/e2e/helpers/electron-process-shutdown.unit.test.ts b/tests/e2e/helpers/electron-process-shutdown.unit.test.ts new file mode 100644 index 00000000000..316aec32ed5 --- /dev/null +++ b/tests/e2e/helpers/electron-process-shutdown.unit.test.ts @@ -0,0 +1,59 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import type { ChildProcess } from 'node:child_process' +import type { ElectronApplication } from '@stablyai/playwright-test' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { closeElectronAppForE2E } from './electron-process-shutdown' + +function exitedAppFixture() { + const proc = Object.assign(new EventEmitter(), { + exitCode: null as number | null, + signalCode: null, + stdio: [new PassThrough(), new PassThrough(), new PassThrough()] + }) + const pipesClosed = Promise.all( + proc.stdio.map((stream) => new Promise((resolve) => stream.once('close', resolve))) + ) + const close = vi.fn(() => pipesClosed) + const app = { + process: () => proc as unknown as ChildProcess, + close + } as unknown as ElectronApplication + return { proc, app, close } +} + +afterEach(() => vi.useRealTimers()) + +describe('Electron shutdown with inherited pipes', () => { + it('releases retained pipes only after Electron exits, settling Playwright cleanup', async () => { + const { proc, app, close } = exitedAppFixture() + const closing = closeElectronAppForE2E(app) + expect(close).toHaveBeenCalledOnce() + expect(proc.stdio.every((stream) => !stream.destroyed)).toBe(true) + proc.exitCode = 0 + proc.emit('exit', 0, null) + await closing + expect(proc.stdio.every((stream) => stream.destroyed)).toBe(true) + expect(proc.listenerCount('exit')).toBe(0) + }) + + it('releases pipes when Electron already exited before cleanup starts', async () => { + const { proc, app } = exitedAppFixture() + proc.exitCode = 0 + await closeElectronAppForE2E(app) + expect(proc.stdio.every((stream) => stream.destroyed)).toBe(true) + }) + + it('does not release pipes if shutdown times out without confirmed process exit', async () => { + vi.useFakeTimers() + const { proc, app } = exitedAppFixture() + const closing = closeElectronAppForE2E(app) + await vi.advanceTimersByTimeAsync(10_000) + await closing + expect(proc.stdio.every((stream) => !stream.destroyed)).toBe(true) + expect(proc.listenerCount('exit')).toBe(0) + for (const stream of proc.stdio) { + stream.destroy() + } + }) +}) diff --git a/tests/e2e/ssh-docker-half-open-link.spec.ts b/tests/e2e/ssh-docker-half-open-link.spec.ts index c5b6f715dc9..d77eba3a264 100644 --- a/tests/e2e/ssh-docker-half-open-link.spec.ts +++ b/tests/e2e/ssh-docker-half-open-link.spec.ts @@ -68,7 +68,7 @@ test.describe('Docker SSH half-open link', () => { const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) const runId = String(Date.now()) - await execInTerminal(orcaPage, ptyId, `echo LIVE_${runId}`) + await execInTerminal(orcaPage, ptyId, `printf 'LIVE_%s\\n' ${runId}`) await waitForTerminalOutput(orcaPage, `LIVE_${runId}`, 60_000) expect(await readSshStatus(orcaPage, remote.targetId)).toBe('connected') @@ -78,13 +78,15 @@ test.describe('Docker SSH half-open link', () => { const frozenAt = Date.now() let verdict: string | null = 'connected' - while (Date.now() - frozenAt < LOST_VERDICT_BUDGET_MS) { - verdict = await readSshStatus(orcaPage, remote.targetId) - if (verdict !== 'connected') { - break - } - await orcaPage.waitForTimeout(1_000) - } + await expect + .poll( + async () => { + verdict = await readSshStatus(orcaPage, remote.targetId) + return verdict + }, + { timeout: LOST_VERDICT_BUDGET_MS, message: 'frozen host remained connected' } + ) + .not.toBe('connected') const verdictMs = Date.now() - frozenAt console.log( `[half-open] ${JSON.stringify({ verdict, verdictMs, budgetMs: LOST_VERDICT_BUDGET_MS })}` @@ -105,7 +107,7 @@ test.describe('Docker SSH half-open link', () => { .poll(() => readSshStatus(orcaPage, remote.targetId), { timeout: 120_000 }) .toBe('connected') const recoveredPtyId = await waitForActivePanePtyId(orcaPage, 60_000) - await execInTerminal(orcaPage, recoveredPtyId, `echo RECOVERED_${runId}`) + await execInTerminal(orcaPage, recoveredPtyId, `printf 'RECOVERED_%s\\n' ${runId}`) await waitForTerminalOutput(orcaPage, `RECOVERED_${runId}`, 90_000) } finally { if (target && paused) { diff --git a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts index e18336d12c5..42ac316790c 100644 --- a/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts +++ b/tests/e2e/ssh-docker-transport-drop-recovery.spec.ts @@ -1,6 +1,6 @@ import path from 'node:path' import { readFileSync } from 'node:fs' -import type { ElectronApplication, Page } from '@playwright/test' +import type { ElectronApplication } from '@playwright/test' import { test, expect } from './helpers/orca-app' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import { sshRemotePtyLeaseAllowsReattach, type SshRemotePtyLease } from '../../src/shared/ssh-types' @@ -18,7 +18,10 @@ import { startDockerSshRelayTarget, type DockerSshRelayTarget } from './helpers/docker-ssh-relay-target' -import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { + connectDockerSshRelayTarget, + recoverDockerSshRelayAfterFault +} from './helpers/docker-ssh-relay-connection' import { clearDockerSshRelayFaults, dropDockerSshRelayTransport, @@ -46,13 +49,6 @@ const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' * with only the first cannot tell a resume from a silent cold start * (docs/reference/ssh-execution-boundary.md). */ -async function readSshStatus(orcaPage: Page, targetId: string) { - return orcaPage.evaluate( - (targetId) => window.__store?.getState().sshConnectionStates.get(targetId)?.status ?? null, - targetId - ) -} - /** * Every lease `reattachKnownPtys` would feed to `pty.attach` on the next connect, read from the * durable store rather than from the renderer — leases are main-owned and never published. @@ -122,7 +118,9 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - const remote = await connectDockerSshRelayTarget(orcaPage, target) + const remote = await connectDockerSshRelayTarget(orcaPage, target, { + relayGracePeriodSeconds: 0 + }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 60_000) const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) @@ -134,20 +132,12 @@ test.describe('SSH transport drop recovery', () => { await execInTerminal(orcaPage, ptyId, `printf 'DROP_MARKER_%s\\n' ${markerSuffix}`) await waitForTerminalOutput(orcaPage, marker, 30_000) - const dropped = dropDockerSshRelayTransport(target) - expect(dropped, 'no live SSH connection was found to drop').toBeGreaterThan(0) - - // Nothing below calls ssh.connect(). Recovery has to come from the client's own ladder, - // which is the behaviour users depend on and the thing a scripted reconnect never exercised. - await expect - .poll(() => readSshStatus(orcaPage, remote.targetId), { - timeout: 120_000, - message: 'SSH target never returned to connected after the transport was dropped' - }) - .toBe('connected') + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, () => { + expect(dropDockerSshRelayTransport(target!)).toBeGreaterThan(0) + }) await waitForActiveTerminalManager(orcaPage, 60_000) - await waitForActivePanePtyId(orcaPage, 60_000) + expect(await waitForActivePanePtyId(orcaPage, 60_000)).toBe(ptyId) // The pane must still show what it had. A blank pane here is the reported bug. await waitForTerminalOutput(orcaPage, marker, 60_000) @@ -170,10 +160,7 @@ test.describe('SSH transport drop recovery', () => { } }) - // Fixme: fails in CI on its first real run — the pane keeps its PTY and repaints, but a command - // run after the flood produces no output within the poll budget. Same shape as #18018 (deaf pane - // after a stalled host resumes), and not caused by this spec. Tracked there; the three verdict - // assertions around it stay enforced. + // #18018: local authority-aware recovery still loses the flooded pane's relay channel. test.fixme('stays bounded when a disconnected shell floods its pty', async ({ orcaPage }, testInfo) => { @@ -195,7 +182,9 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - const remote = await connectDockerSshRelayTarget(orcaPage, target) + const remote = await connectDockerSshRelayTarget(orcaPage, target, { + relayGracePeriodSeconds: 0 + }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 240_000) const ptyId = await waitForActivePanePtyId(orcaPage, 240_000) @@ -214,18 +203,12 @@ test.describe('SSH transport drop recovery', () => { await execInTerminal( orcaPage, ptyId, - `yes "$(printf 'ORCA_%s' FLOOD_LINE)" | head -c 48000000; echo FLOODED` + `yes "$(printf 'ORCA_%s' FLOOD_LINE)" | head -c 48000000; printf 'FLOO%s\\n' DED` ) await waitForTerminalOutput(orcaPage, 'ORCA_FLOOD_LINE', 30_000, 20_000) - const dropped = dropDockerSshRelayTransport(target) - expect(dropped).toBeGreaterThan(0) - - await expect - .poll(() => readSshStatus(orcaPage, remote.targetId), { - timeout: 120_000, - message: 'SSH target never returned to connected' - }) - .toBe('connected') + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, () => { + expect(dropDockerSshRelayTransport(target!)).toBeGreaterThan(0) + }) await waitForActiveTerminalManager(orcaPage, 240_000) // Why a generous ceiling: this is an OOM guard, not a memory budget. Unbounded retention of @@ -236,6 +219,9 @@ test.describe('SSH transport drop recovery', () => { `relay grew ${afterRssKb - baselineRssKb}KB after 48MB of undeliverable output` ).toBeLessThan(200_000) + // Wait for the finite producer to finish before sending a shell command behind it. + await waitForTerminalOutput(orcaPage, 'FLOODED', 120_000, 20_000) + // And the session must still be usable, not merely alive. const markerSuffix = Date.now() const marker = `FLOOD_AFTER_${markerSuffix}` @@ -273,7 +259,9 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - const remote = await connectDockerSshRelayTarget(orcaPage, target) + const remote = await connectDockerSshRelayTarget(orcaPage, target, { + relayGracePeriodSeconds: 0 + }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 60_000) const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) @@ -283,15 +271,9 @@ test.describe('SSH transport drop recovery', () => { await execInTerminal(orcaPage, ptyId, `printf 'KILL_MARKER_%s\\n' ${markerSuffix}`) await waitForTerminalOutput(orcaPage, marker, 30_000) - const killed = killDockerSshRelayDaemon(target) - expect(killed, 'no relay process was found to kill').toBeGreaterThan(0) - - await expect - .poll(() => readSshStatus(orcaPage, remote.targetId), { - timeout: 120_000, - message: 'SSH target never returned to connected after the relay was killed' - }) - .toBe('connected') + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, () => { + expect(killDockerSshRelayDaemon(target!)).toBeGreaterThan(0) + }) await waitForActiveTerminalManager(orcaPage, 60_000) // The verdict, expressed as the only thing a user can observe: the pane is now backed by a @@ -345,7 +327,9 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - const remote = await connectDockerSshRelayTarget(orcaPage, target) + const remote = await connectDockerSshRelayTarget(orcaPage, target, { + relayGracePeriodSeconds: 0 + }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 60_000) await waitForActivePanePtyId(orcaPage, 60_000) @@ -354,22 +338,20 @@ test.describe('SSH transport drop recovery', () => { const generations: string[][] = [] for (let generation = 1; generation <= 5; generation++) { - expect( - killDockerSshRelayDaemon(target), - 'no relay process was found to kill' - ).toBeGreaterThan(0) + const predecessor = await waitForActivePanePtyId(orcaPage, 60_000) + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, () => { + expect(killDockerSshRelayDaemon(target!)).toBeGreaterThan(0) + }) await expect - .poll(() => readSshStatus(orcaPage, remote.targetId), { - timeout: 120_000, - message: `SSH target never reconnected after relay kill ${generation}` - }) - .toBe('connected') + .poll(() => waitForActivePanePtyId(orcaPage, 60_000), { timeout: 120_000 }) + .not.toBe(predecessor) await waitForActiveTerminalManager(orcaPage, 120_000) // The pane must be usable again before the count is meaningful: recovery is what mints the // successor lease that retires the generation before it. const ptyId = await waitForActivePanePtyId(orcaPage, 120_000) - const marker = `LEASE_GEN_${generation}_${Date.now()}` - await execInTerminal(orcaPage, ptyId, `printf '%s\\n' ${marker}`) + const markerSuffix = `${generation}_${Date.now()}` + const marker = `LEASE_GEN_${markerSuffix}` + await execInTerminal(orcaPage, ptyId, `printf 'LEASE_GEN_%s\\n' ${markerSuffix}`) await waitForTerminalOutput(orcaPage, marker, 60_000) try { @@ -418,7 +400,7 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - await connectDockerSshRelayTarget(orcaPage, target) + await connectDockerSshRelayTarget(orcaPage, target, { relayGracePeriodSeconds: 0 }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 60_000) const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) @@ -446,17 +428,8 @@ test.describe('SSH transport drop recovery', () => { } }) - /** - * Known broken on main, kept as the reproduction. The verdict test above passes: after a 30s - * freeze the pane keeps its PTY and repaints its scrollback. What does not come back is the - * shell — a command run afterwards produces no output within 60s, so the pane is live-looking and - * deaf. Measured twice at `waitForTerminalOutput(STALL_AFTER_…)`, and it reproduces unchanged - * with the reattach-token/delivery-ownership fix applied, so that is not the cause. - * - * Split out rather than folded into the test above so the `unverifiable` verdict stays enforced - * in CI instead of being masked by this failure. - */ - test.fixme('accepts input again after a frozen host resumes', async ({ orcaPage }, testInfo) => { + // #18018: wait for the recovered authority before input; a retained manager can still be disconnected. + test('accepts input again after a frozen host resumes', async ({ orcaPage }, testInfo) => { test.slow() let target: DockerSshRelayTarget | null = null try { @@ -464,13 +437,17 @@ test.describe('SSH transport drop recovery', () => { enableDockerSshRelayTargetShellTitle(target) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) - await connectDockerSshRelayTarget(orcaPage, target) + const remote = await connectDockerSshRelayTarget(orcaPage, target, { + relayGracePeriodSeconds: 0 + }) await ensureTerminalVisible(orcaPage, 45_000) await waitForActiveTerminalManager(orcaPage, 60_000) const ptyId = await waitForActivePanePtyId(orcaPage, 60_000) - await withStalledDockerSshRelayTarget(target, async () => { - await orcaPage.waitForTimeout(30_000) + await recoverDockerSshRelayAfterFault(orcaPage, remote.targetId, async () => { + await withStalledDockerSshRelayTarget(target!, async () => { + await orcaPage.waitForTimeout(30_000) + }) }) await waitForActiveTerminalManager(orcaPage, 60_000) From 1924c8f5b1ec6e63dad5269966edba1de7c7d34a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:56:06 -0700 Subject: [PATCH 3/3] feat(perf): lint repeated sort setup and schedule regression contracts (#18822) * feat(perf): audit comparator setup and schedule performance contracts * test(sqlite): close readers after expected busy failures * ci(perf): trigger contract workflow on the contract files themselves Without these paths a contract rename lands green on PR CI and only breaks the next nightly, where nobody owns the failure. Also run the OS-independent source audit once instead of on all three runners. --- .github/workflows/performance-contracts.yml | 63 +++++++++++++++++++ .oxlintrc.json | 5 ++ config/oxlint-performance-audit.json | 35 +++++++++++ .../sort-comparator-performance.mjs | 60 ++++++++++++++++++ config/performance-audit.md | 38 +++++++++++ ...ort-comparator-performance-plugin.test.mjs | 45 +++++++++++++ config/vitest.performance.config.ts | 33 ++++++++++ package.json | 3 +- src/main/sqlite/sync-database.test.ts | 12 ++-- 9 files changed, 287 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/performance-contracts.yml create mode 100644 config/oxlint-performance-audit.json create mode 100644 config/oxlint-plugins/sort-comparator-performance.mjs create mode 100644 config/performance-audit.md create mode 100644 config/scripts/sort-comparator-performance-plugin.test.mjs create mode 100644 config/vitest.performance.config.ts diff --git a/.github/workflows/performance-contracts.yml b/.github/workflows/performance-contracts.yml new file mode 100644 index 00000000000..d45d8b8f45a --- /dev/null +++ b/.github/workflows/performance-contracts.yml @@ -0,0 +1,63 @@ +name: Performance contracts + +on: + schedule: + - cron: '15 9 * * *' + workflow_dispatch: + pull_request: + paths: + - '.github/workflows/performance-contracts.yml' + - 'config/vitest.performance.config.ts' + - 'config/oxlint-performance-audit.json' + - 'config/oxlint-plugins/*performance.mjs' + - 'config/oxlint-plugins/quadratic-buffer-concat.mjs' + - 'config/scripts/*-plugin.test.mjs' + # Keep in sync with the contract list in config/vitest.performance.config.ts; + # without these a rename lands green and only breaks the next nightly. + - 'src/main/sqlite/sync-database.test.ts' + - 'src/main/runtime/orchestration/db/row-column-lists.test.ts' + - 'src/relay/fs-path-metadata-symlink-concurrency.test.ts' + - 'src/renderer/src/components/editor/rich-markdown-list-tokenizers.test.ts' + - 'src/renderer/src/components/editor/rich-markdown-lowlight-cache.test.ts' + - 'src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts' + - 'src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-queue-retention.test.ts' + +permissions: + contents: read + +concurrency: + group: performance-contracts-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + contracts: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - uses: ./.github/actions/install-node-dependencies + - name: Run operation-count and retention contracts + run: pnpm test:perf:contracts --reporter=default --reporter=json --outputFile=performance-contracts.json + # Source-only scan: identical on every OS, so run it once. + - name: Audit production performance patterns + if: always() && matrix.os == 'ubuntu-latest' + shell: bash + run: pnpm --silent audit:perf > performance-audit.json + - uses: actions/upload-artifact@v7 + if: always() + with: + name: performance-contracts-${{ matrix.os }} + path: performance-contracts.json + if-no-files-found: error + - uses: actions/upload-artifact@v7 + if: always() && matrix.os == 'ubuntu-latest' + with: + name: performance-audit + path: performance-audit.json + if-no-files-found: error diff --git a/.oxlintrc.json b/.oxlintrc.json index 77a7e43e807..03cc659f494 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -2,6 +2,10 @@ "$schema": "./node_modules/oxlint/configuration_schema.json", "plugins": ["typescript", "react", "react-hooks", "react-perf", "unicorn"], "jsPlugins": [ + { + "name": "sort-comparator-performance", + "specifier": "./config/oxlint-plugins/sort-comparator-performance.mjs" + }, { "name": "mobile-pairing", "specifier": "./config/oxlint-plugins/mobile-pairing-qrcode-import.mjs" @@ -23,6 +27,7 @@ "correctness": "error" }, "rules": { + "sort-comparator-performance/no-repeated-collator": "warn", "app-store-performance/require-selector": "error", "app-store-performance/no-identity-selector": "error", "app-store-performance/no-fresh-selector-result": "error", diff --git a/config/oxlint-performance-audit.json b/config/oxlint-performance-audit.json new file mode 100644 index 00000000000..2912c6b8e03 --- /dev/null +++ b/config/oxlint-performance-audit.json @@ -0,0 +1,35 @@ +{ + "$schema": "../node_modules/oxlint/configuration_schema.json", + "plugins": [], + "categories": { + "correctness": "off", + "suspicious": "off", + "pedantic": "off", + "perf": "off", + "style": "off", + "restriction": "off", + "nursery": "off" + }, + "jsPlugins": [ + { + "name": "app-store-performance", + "specifier": "../config/oxlint-plugins/app-store-performance.mjs" + }, + { + "name": "quadratic-buffer-concat", + "specifier": "../config/oxlint-plugins/quadratic-buffer-concat.mjs" + }, + { + "name": "sort-comparator-performance", + "specifier": "../config/oxlint-plugins/sort-comparator-performance.mjs" + } + ], + "rules": { + "app-store-performance/require-selector": "warn", + "app-store-performance/no-identity-selector": "warn", + "app-store-performance/no-fresh-selector-result": "warn", + "quadratic-buffer-concat/no-loop-carried-concat": "warn", + "sort-comparator-performance/no-repeated-collator": "warn" + }, + "ignorePatterns": ["**/node_modules", "**/dist", "**/out", "**/*.test.*", "**/*.spec.*"] +} diff --git a/config/oxlint-plugins/sort-comparator-performance.mjs b/config/oxlint-plugins/sort-comparator-performance.mjs new file mode 100644 index 00000000000..cd3444cf65f --- /dev/null +++ b/config/oxlint-plugins/sort-comparator-performance.mjs @@ -0,0 +1,60 @@ +const FUNCTION_TYPES = new Set([ + 'ArrowFunctionExpression', + 'FunctionExpression', + 'FunctionDeclaration' +]) + +function propertyName(node) { + if (node?.type !== 'MemberExpression') { + return null + } + if (!node.computed && node.property.type === 'Identifier') { + return node.property.name + } + return node.property.type === 'Literal' ? node.property.value : null +} + +function isInlineSortComparator(node) { + for (let parent = node.parent; parent; parent = parent.parent) { + if (!FUNCTION_TYPES.has(parent.type)) { + continue + } + const call = parent.parent + return ( + call?.type === 'CallExpression' && + call.arguments[0] === parent && + ['sort', 'toSorted'].includes(propertyName(call.callee)) + ) + } + return false +} + +function isCollatorConstruction(node) { + return ( + node.callee?.object?.type === 'Identifier' && + node.callee.object.name === 'Intl' && + propertyName(node.callee) === 'Collator' + ) +} + +function createRule(context) { + function inspect(node) { + const optionedComparison = + node.type === 'CallExpression' && + propertyName(node.callee) === 'localeCompare' && + node.arguments.length >= 3 + if ((optionedComparison || isCollatorConstruction(node)) && isInlineSortComparator(node)) { + context.report({ + node, + message: + 'Create one Intl.Collator before sorting and reuse its compare method; resolving collation options inside the comparator repeats setup for every comparison. Preserve the locale, options, and tie-breaker.' + }) + } + } + return { CallExpression: inspect, NewExpression: inspect } +} + +export default { + meta: { name: 'sort-comparator-performance' }, + rules: { 'no-repeated-collator': { create: createRule } } +} diff --git a/config/performance-audit.md b/config/performance-audit.md new file mode 100644 index 00000000000..f105bfbe787 --- /dev/null +++ b/config/performance-audit.md @@ -0,0 +1,38 @@ +# Performance regression checks + +`pnpm --silent audit:perf > performance-audit.json` scans production `src/` with +the existing app-store and buffer-concatenation rules plus the sort-comparator +rule. Warnings are advisory in this full inventory; tool/parser failures fail. +New warning findings on changed lines fail `pnpm check:code-quality:changed`. +Tests, generated files, `mobile/` and `cloud/` are outside this source audit. + +The sort rule detects optioned `localeCompare` and `Intl.Collator` construction +inside inline `sort`/`toSorted` callbacks. Construct one collator outside the +callback, preserving locale, options and tie-breakers. If the locale changes at +runtime, reconstruct at the next sort or key the cache by locale. Bare comparisons +and standalone equality checks are allowed. There is no autofix or interprocedural +analysis: named comparators, aliases, custom methods and deferred callbacks need +manual review. A warning identifies repeated setup, not proof of visible lag. + +`pnpm test:perf:contracts` runs the explicit selection in +`vitest.performance.config.ts`: SQLite statement reuse and schema parity, relay +filesystem concurrency, tokenizer rejection, highlighting cache, queued +cancellation, terminal backing-memory retention and detector fixtures. Missing +listed files fail configuration loading. Tests run serially, without retries, +and inherit the full suite's setup and forced-GC support. This makes existing +regression coverage easy to run and attribute; it does not create new workload +coverage by itself. + +`.github/workflows/performance-contracts.yml` runs daily and manually on Linux, +macOS and Windows, and on PRs changing this tooling or any listed contract file. +It uploads per-OS JSON test results, plus the source inventory once from Linux +because that scan is OS-independent. Its schedule starts after merge. Run the existing +`test:e2e:terminal-perf:scale:report` for rendered typing/frame budgets and +`test:e2e:ssh-docker-perf` for real transport behavior. Relay unit tests do not +measure SSH RTT, WSL scheduling or a packaged Electron renderer. + +To extend coverage, select a production-path regression with an operation-count, +identity, queue-admission or retained-memory oracle. Confirm it fails with the +old behavior. Use controlled, counterbalanced benchmark samples for timings; +avoid new machine-dependent millisecond gates in the normal unit suite. A green +source scan and these contracts cannot establish that the whole app is fast. diff --git a/config/scripts/sort-comparator-performance-plugin.test.mjs b/config/scripts/sort-comparator-performance-plugin.test.mjs new file mode 100644 index 00000000000..a9319c6238d --- /dev/null +++ b/config/scripts/sort-comparator-performance-plugin.test.mjs @@ -0,0 +1,45 @@ +import path from 'node:path' +import { describe, expect, it } from 'vitest' +import { runOxlintPluginOnSource } from './oxlint-plugin-test-runner.mjs' + +function lint(source) { + return runOxlintPluginOnSource({ + pluginName: 'sort-comparator-performance', + pluginPath: path.resolve('config/oxlint-plugins/sort-comparator-performance.mjs'), + rules: { 'sort-comparator-performance/no-repeated-collator': 'warn' }, + source + }) +} + +describe('sort comparator performance', () => { + it('reports repeated collation setup in inline sort and toSorted callbacks', () => { + const findings = lint(` + rows.sort((a, b) => a.name.localeCompare(b.name, locale, { sensitivity: 'base' })) + rows.toSorted(function (a, b) { return new Intl.Collator('sv').compare(a, b) }) + rows['sort']((a, b) => Intl.Collator('en', { numeric: true }).compare(a, b)) + rows.sort((a, b) => a['localeCompare'](b, undefined, options)) + `) + expect(findings).toHaveLength(4) + expect( + findings.every( + (finding) => finding.code === 'sort-comparator-performance(no-repeated-collator)' + ) + ).toBe(true) + }) + + it('allows one collator per sort, bare comparisons, and unrelated callbacks', () => { + expect( + lint(` + const collator = new Intl.Collator(locale, options) + rows.sort((a, b) => collator.compare(a.name, b.name) || a.id.localeCompare(b.id)) + rows.toSorted(collator.compare) + const equal = a.localeCompare(b, undefined, { sensitivity: 'accent' }) === 0 + rows.map(a => new Intl.Collator(a.locale)) + rows.sort((a, b) => { + function deferred() { return new Intl.Collator(locale) } + return a - b + }) + `) + ).toEqual([]) + }) +}) diff --git a/config/vitest.performance.config.ts b/config/vitest.performance.config.ts new file mode 100644 index 00000000000..7682b7b9698 --- /dev/null +++ b/config/vitest.performance.config.ts @@ -0,0 +1,33 @@ +import { existsSync } from 'node:fs' +import { resolve } from 'node:path' +import { defineConfig } from 'vitest/config' +import baseConfig from './vitest.config' + +const contracts = [ + 'src/main/sqlite/sync-database.test.ts', + 'src/main/runtime/orchestration/db/row-column-lists.test.ts', + 'src/relay/fs-path-metadata-symlink-concurrency.test.ts', + 'src/renderer/src/components/editor/rich-markdown-list-tokenizers.test.ts', + 'src/renderer/src/components/editor/rich-markdown-lowlight-cache.test.ts', + 'src/renderer/src/components/terminal-pane/agent-completion-coordinator-queued-inspection-disposal.test.ts', + 'src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-queue-retention.test.ts', + 'config/scripts/app-store-performance-plugin.test.mjs', + 'config/scripts/quadratic-buffer-concat-plugin.test.mjs', + 'config/scripts/sort-comparator-performance-plugin.test.mjs' +] + +for (const contract of contracts) { + if (!existsSync(resolve(contract))) { + throw new Error(`Missing performance contract: ${contract}`) + } +} + +export default defineConfig({ + ...baseConfig, + test: { + ...baseConfig.test, + include: contracts, + fileParallelism: false, + retry: 0 + } +}) diff --git a/package.json b/package.json index 21636f94e9c..7b27dffc8c7 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,8 @@ }, "main": "./out/main/index.js", "scripts": { + "audit:perf": "oxlint --config config/oxlint-performance-audit.json --format json src", + "test:perf:contracts": "vitest run --config config/vitest.performance.config.ts", "format": "oxfmt --write .", "lint": "oxlint && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalog && pnpm run verify:localization-runtime-catalog && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage", "audit:code-quality": "pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run audit:react-doctor", @@ -144,7 +146,6 @@ "bench:agent-inspection-cadence": "node config/scripts/agent-inspection-cadence-batching-benchmark.mjs", "bench:renderer-quadratic-scans": "node config/scripts/renderer-quadratic-scan-benchmark.mjs", "bench:session-write-hot-path": "node config/scripts/session-write-hot-path-benchmark.mjs", - "bench:terminal-partial-escape-tail": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON config/scripts/terminal-partial-escape-tail-benchmark.mjs", "bench:terminal-partial-escape-tail": "node config/scripts/terminal-partial-escape-tail-benchmark.mjs", "bench:worktree-refresh-churn": "node --disable-warning=MODULE_TYPELESS_PACKAGE_JSON config/scripts/worktree-refresh-churn-benchmark.mjs", "bench:multi-workspace-typing": "pnpm run ensure:electron-runtime && node config/scripts/run-multi-workspace-typing-bench.mjs", diff --git a/src/main/sqlite/sync-database.test.ts b/src/main/sqlite/sync-database.test.ts index fe3ba7e388d..5a028e68fd9 100644 --- a/src/main/sqlite/sync-database.test.ts +++ b/src/main/sqlite/sync-database.test.ts @@ -194,9 +194,11 @@ describe('SyncDatabase read-only opens under contention', () => { const contended = await contendedDatabase(10_000) const startedAt = Date.now() + const reader = new SyncDatabase(contended.path, { readonly: true }) + openDatabases.push(reader) let thrown: unknown try { - new SyncDatabase(contended.path, { readonly: true }).prepare('SELECT id FROM items').all() + reader.prepare('SELECT id FROM items').all() } catch (error) { thrown = error } @@ -210,11 +212,9 @@ describe('SyncDatabase read-only opens under contention', () => { const contended = await contendedDatabase(10_000) const startedAt = Date.now() - expect(() => - new SyncDatabase(contended.path, { readonly: true, timeout: 400 }) - .prepare('SELECT id FROM items') - .all() - ).toThrow(/database is locked/) + const reader = new SyncDatabase(contended.path, { readonly: true, timeout: 400 }) + openDatabases.push(reader) + expect(() => reader.prepare('SELECT id FROM items').all()).toThrow(/database is locked/) expect(Date.now() - startedAt).toBeGreaterThanOrEqual(350) })