From b852aa74a6c6f96b6dbc68ddf2499001c7cdb362 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:03:01 -0700 Subject: [PATCH 01/43] fix(ci): store vetted refs in a reftable so case-twin branches don't fail the fetch (#18970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The adhoc mac and dev-channel Windows builds vet the requested ref by mirroring every branch and tag of this repo into a scratch bare repo and proving the commit is reachable. Both runner disks are case-insensitive, and the repo now has two branches differing only in casing, so the files backend refuses the fetch outright — the whole job dies before checkout. reftable keys refs in a table rather than as file paths, so both refs store and every ref stays in the reachability set. --- .github/workflows/adhoc-mac-build.yml | 7 ++-- .github/workflows/dev-channel-win-build.yml | 7 ++-- .../workflow-ref-mirror-case-safety.test.mjs | 34 +++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 config/scripts/workflow-ref-mirror-case-safety.test.mjs diff --git a/.github/workflows/adhoc-mac-build.yml b/.github/workflows/adhoc-mac-build.yml index d7dd6d5ffb6..761a9585b73 100644 --- a/.github/workflows/adhoc-mac-build.yml +++ b/.github/workflows/adhoc-mac-build.yml @@ -127,9 +127,12 @@ jobs: esac # Bare: a work-tree repo refuses to fetch over its own checked-out # branch. tree:0 keeps the fetch to the commit graph — no trees, no - # blobs — so this stays cheap next to the build it fronts. + # blobs — so this stays cheap next to the build it fronts. reftable + # because this repo has branches that differ only in casing, and the + # files backend cannot store both on a case-insensitive runner disk — + # it fails the entire fetch, not just the one ref. scratch="$RUNNER_TEMP/vet-requested-ref" - git init -q --bare "$scratch" + git init -q --bare --ref-format=reftable "$scratch" git -C "$scratch" fetch -q --filter=tree:0 "$REPO_URL" '+refs/heads/*:refs/heads/*' '+refs/tags/*:refs/tags/*' # Branch first to keep actions/checkout's old tie-break: bare # rev-parse would prefer the tag when a branch shares its name. diff --git a/.github/workflows/dev-channel-win-build.yml b/.github/workflows/dev-channel-win-build.yml index e16a50f1c3c..89fda2ebef9 100644 --- a/.github/workflows/dev-channel-win-build.yml +++ b/.github/workflows/dev-channel-win-build.yml @@ -149,9 +149,12 @@ jobs: fi # Reachability is the trust test: GitHub serves PR-only commits by SHA, # so resolving the object is not proof a branch or tag of this repo - # reaches it. Bare + tree:0 keeps this to the commit graph. + # reaches it. Bare + tree:0 keeps this to the commit graph; reftable + # because branches that differ only in casing cannot both be stored by + # the files backend on a case-insensitive runner disk, which fails the + # entire fetch rather than the one ref. scratch="$RUNNER_TEMP/vet-requested-ref" - git init -q --bare "$scratch" + git init -q --bare --ref-format=reftable "$scratch" git -C "$scratch" fetch -q --filter=tree:0 "$REPO_URL" '+refs/heads/*:refs/heads/*' '+refs/tags/*:refs/tags/*' if ! git -C "$scratch" rev-parse --verify --quiet "$REQUESTED_SHA^{commit}" >/dev/null; then echo "::error::Commit $REQUESTED_SHA is not in stablyai/orca." diff --git a/config/scripts/workflow-ref-mirror-case-safety.test.mjs b/config/scripts/workflow-ref-mirror-case-safety.test.mjs new file mode 100644 index 00000000000..6008c9d8d5f --- /dev/null +++ b/config/scripts/workflow-ref-mirror-case-safety.test.mjs @@ -0,0 +1,34 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' + +const projectDir = resolve(import.meta.dirname, '../..') + +const readWorkflow = (relativePath) => parse(readFileSync(join(projectDir, relativePath), 'utf8')) + +// Every step that mirrors this repo's whole ref namespace onto a runner disk to +// prove a commit is reachable from a branch or tag before signing it. +const REF_MIRRORS = [ + ['.github/workflows/adhoc-mac-build.yml', 'build-adhoc-mac', 'Vet the requested ref'], + ['.github/workflows/dev-channel-win-build.yml', 'build-win', 'Vet the requested inputs'] +] + +describe('ref-mirroring vet steps', () => { + // Why: macOS and Windows runner disks are case-insensitive, and this repo has + // branches that differ only in casing. The files backend cannot store both, and + // it fails the whole fetch rather than the one ref — so the vet step dies before + // any build runs. reftable keys refs in a table instead of file paths. + it.each(REF_MIRRORS)( + '%s creates its scratch repo with the reftable backend', + (path, job, step) => { + const run = readWorkflow(path).jobs[job].steps.find( + (candidate) => candidate.name === step + ).run + + expect(run).toContain('+refs/heads/*:refs/heads/*') + expect(run).toMatch(/git init\b[^\n]*--ref-format=reftable/) + expect(run).not.toMatch(/git init -q --bare "\$scratch"/) + } + ) +}) From 75d4add34414a79530bf2513d5207a6c72676a80 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:05:38 +0000 Subject: [PATCH 02/43] Update README downloads badge --- docs/assets/readme-downloads.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index fe660c42295..33ad276aa2d 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 40m + + downloads: 41m @@ -15,7 +15,7 @@ downloads downloads - 40m - 40m + 41m + 41m From d7722a698ce148c82602b21e8abf9a1bc5d47c6e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:09:53 -0700 Subject: [PATCH 03/43] test: drain project menu focus restoration before teardown (#18971) --- .../AgentMapWorkspaceContextMenu.test.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx b/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx index c5477a997e9..bc4958408a2 100644 --- a/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentMapWorkspaceContextMenu.test.tsx @@ -314,7 +314,19 @@ describe('Agent Map workspace context menu', () => { clientX: 100, clientY: 110 }) - fireEvent.click(await screen.findByText('Create new worktree for Orca', {}, { timeout: 5_000 })) + const createWorktree = await screen.findByText( + 'Create new worktree for Orca', + {}, + { timeout: 5_000 } + ) + // Radix restores focus after unmount; drain it before the next test opens a menu. + const focusRestored = new Promise((resolve) => { + screen + .getByRole('menu') + .addEventListener('focusScope.autoFocusOnUnmount', () => resolve(), { once: true }) + }) + fireEvent.click(createWorktree) + await act(async () => focusRestored) expect(useAppStore.getState().activeModal).toBe('new-workspace-composer') expect(useAppStore.getState().modalData).toEqual({ From 6031c19e9fb260e661c29ccea3a196f15e24774b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:20:12 -0700 Subject: [PATCH 04/43] ci: reduce dependency, checkout, and test deadline overhead (#18968) * ci: reduce dependency, checkout, and test deadline overhead * ci: avoid generic E2E jobs for native-only IME changes --- .github/workflows/cloud-verify.yml | 9 +-- .github/workflows/pr.yml | 5 +- .github/workflows/release-cut.yml | 23 ++++--- .github/workflows/skill-update-roundtrip.yml | 2 + .github/workflows/terminal-ime-e2e.yml | 18 +---- .github/workflows/terminal-perf.yml | 11 ++-- .../workflows/windows-signing-rehearsal.yml | 1 + .../pr-e2e-native-only-routing.test.mjs | 33 ++++++++++ config/scripts/pr-e2e-source-routing.mjs | 12 ++++ docs/reference/ci-runner-efficiency.md | 66 +++++++++++++++++++ ...ace-snapshot-unplaced-tab-adoption.test.ts | 29 +++++--- .../remote-workspace-target-sync.test.ts | 10 ++- .../startup-ssh-connection-restore.test.ts | 8 ++- 13 files changed, 177 insertions(+), 50 deletions(-) create mode 100644 config/scripts/pr-e2e-native-only-routing.test.mjs diff --git a/.github/workflows/cloud-verify.yml b/.github/workflows/cloud-verify.yml index f0cc2df2bad..e2ba9407ac4 100644 --- a/.github/workflows/cloud-verify.yml +++ b/.github/workflows/cloud-verify.yml @@ -25,9 +25,10 @@ defaults: working-directory: cloud jobs: + # Public-repository hosted runners preserve Blacksmith allowance for macOS. security: name: Secret scan - runs-on: blacksmith-2vcpu-ubuntu-2204 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 with: @@ -53,7 +54,7 @@ jobs: # Compiles the workspace. No Postgres service: nothing here reaches a # database, and the service container costs ~13s of startup. build: - runs-on: blacksmith-4vcpu-ubuntu-2204 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 @@ -73,7 +74,7 @@ jobs: # package it needs through the relay pretest hook, so it does not depend on # `pnpm build` having run. test: - runs-on: blacksmith-4vcpu-ubuntu-2204 + runs-on: ubuntu-22.04 services: postgres: image: postgres:16-alpine @@ -107,7 +108,7 @@ jobs: # Fork pull requests reach this job, so it never configures a backend, never plans, and never # holds a credential. Only the relay root ships here; foundation and apps stay private. terraform: - runs-on: blacksmith-2vcpu-ubuntu-2204 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 93bc4c0afc8..0e2fa3f273c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -93,12 +93,13 @@ jobs: NATIVE_IME_SOURCE_CHANGED="$(printf '%s\n' "$CHANGED" | node config/scripts/pr-e2e-source-routing.mjs --native-ime-source)" echo "native_ime_source_changed=$NATIVE_IME_SOURCE_CHANGED" >> "$GITHUB_OUTPUT" echo "Native IME source changed: $NATIVE_IME_SOURCE_CHANGED" - if [ "$TEST_FILES_JSON" != '[]' ]; then + SHOULD_RUN="$(printf '%s\n' "$CHANGED" | node config/scripts/pr-e2e-source-routing.mjs --reusable-workflow)" + if [ "$SHOULD_RUN" = true ]; then echo "should_run=true" >> "$GITHUB_OUTPUT" echo "Changed E2E specs: $TEST_FILES_JSON" else echo "should_run=false" >> "$GITHUB_OUTPUT" - echo "No changed E2E specs" + echo "No specs requiring the reusable E2E workflow" fi static_analysis: diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 6f888c3a512..001eee4e03c 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -858,16 +858,17 @@ jobs: if: runner.os == 'Linux' run: sudo apt-get update && sudo apt-get install -y build-essential python3 xvfb - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version-file: package.json - - name: Setup pnpm uses: pnpm/setup@v2 with: install: false + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version-file: package.json + cache: pnpm + # Why: Linux terminal golden E2E uses the same native install path as # release CI, which needs pnpm to bypass its non-executable gyp_main.py. - name: Use external node-gyp to avoid pnpm's bundled copy (Linux only) @@ -1074,16 +1075,17 @@ jobs: if: runner.os == 'Linux' run: sudo apt-get update && sudo apt-get install -y build-essential python3 xvfb - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version-file: package.json - - name: Setup pnpm uses: pnpm/setup@v2 with: install: false + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version-file: package.json + cache: pnpm + # Why: keep the non-blocking evidence lane on the same Linux native # install path as the blocking golden and release build jobs. - name: Use external node-gyp to avoid pnpm's bundled copy (Linux only) @@ -1716,6 +1718,7 @@ jobs: with: name: orca-windows-unsigned-${{ needs.cut.outputs.tag }} path: dist/orca-windows-setup.exe + compression-level: 0 if-no-files-found: error # Why: SignPath Foundation production certificates require manual review, diff --git a/.github/workflows/skill-update-roundtrip.yml b/.github/workflows/skill-update-roundtrip.yml index 239f1b2f27c..96de1101275 100644 --- a/.github/workflows/skill-update-roundtrip.yml +++ b/.github/workflows/skill-update-roundtrip.yml @@ -45,7 +45,9 @@ jobs: steps: - uses: actions/checkout@v6 with: + # Historical skill snapshots need tags, but only their blobs are read. fetch-depth: 0 + filter: blob:none persist-credentials: false - uses: actions/setup-node@v6 with: diff --git a/.github/workflows/terminal-ime-e2e.yml b/.github/workflows/terminal-ime-e2e.yml index b9957b2daa9..1ab905d8783 100644 --- a/.github/workflows/terminal-ime-e2e.yml +++ b/.github/workflows/terminal-ime-e2e.yml @@ -38,23 +38,9 @@ jobs: xfwm4 xvfb - - name: Setup Node.js - uses: actions/setup-node@v6 + - uses: ./.github/actions/install-node-dependencies with: - node-version-file: package.json - - - name: Setup pnpm - uses: pnpm/setup@v2 - with: - install: false - - - name: Use external node-gyp to avoid pnpm bundled copy - run: | - npm install -g node-gyp@11.5.0 - echo "npm_config_node_gyp=$(npm root -g)/node-gyp/bin/node-gyp.js" >> "$GITHUB_ENV" - - - name: Install dependencies - run: pnpm install --frozen-lockfile + native-runtime: electron - name: Build Electron app for E2E run: pnpm exec electron-vite build --mode e2e diff --git a/.github/workflows/terminal-perf.yml b/.github/workflows/terminal-perf.yml index 38d0a25bbb7..72a0fec8992 100644 --- a/.github/workflows/terminal-perf.yml +++ b/.github/workflows/terminal-perf.yml @@ -67,16 +67,17 @@ jobs: - name: Install native build tools and xvfb run: sudo apt-get update && sudo apt-get install -y build-essential python3 xvfb zsh - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version-file: package.json - - name: Setup pnpm uses: pnpm/setup@v2 with: install: false + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version-file: package.json + cache: pnpm + # Why: this scheduled/manual workflow uses the same native install path as # PR and E2E CI, which needs pnpm to bypass its bundled gyp_main.py. - name: Use external node-gyp to avoid pnpm's bundled copy diff --git a/.github/workflows/windows-signing-rehearsal.yml b/.github/workflows/windows-signing-rehearsal.yml index 54b751908dc..6fc6fab7193 100644 --- a/.github/workflows/windows-signing-rehearsal.yml +++ b/.github/workflows/windows-signing-rehearsal.yml @@ -215,6 +215,7 @@ jobs: with: name: orca-windows-installer-unsigned-${{ github.run_id }} path: dist/orca-windows-setup.exe + compression-level: 0 if-no-files-found: error - name: Submit Windows installer signing request diff --git a/config/scripts/pr-e2e-native-only-routing.test.mjs b/config/scripts/pr-e2e-native-only-routing.test.mjs new file mode 100644 index 00000000000..b6c3662cd1d --- /dev/null +++ b/config/scripts/pr-e2e-native-only-routing.test.mjs @@ -0,0 +1,33 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' +import { hasNativeImeSourceChange, shouldRunReusablePrE2e } from './pr-e2e-source-routing.mjs' + +const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) +const filterStep = workflow.jobs.code_paths.steps.find((step) => step.id === 'e2e_filter') + +describe('native-only PR E2E routing', () => { + it('avoids generic E2E allocation for native-only changes while preserving its IME lane', () => { + for (const file of [ + 'tests/e2e/terminal-ibus-hangul-native.spec.ts', + 'config/scripts/run-terminal-ibus-hangul-e2e.mjs' + ]) { + expect(hasNativeImeSourceChange([file])).toBe(true) + expect(shouldRunReusablePrE2e([file])).toBe(false) + } + expect(shouldRunReusablePrE2e([])).toBe(false) + for (const spec of [ + 'tests/e2e/ssh-startup-exec-readiness.spec.ts', + 'tests/e2e/paired-startup-exec-readiness.spec.ts', + 'tests/e2e/terminal-ime-exact-byte.spec.ts', + 'tests/e2e/future.spec.ts' + ]) { + expect(shouldRunReusablePrE2e([spec])).toBe(true) + expect(shouldRunReusablePrE2e(['tests/e2e/terminal-ibus-hangul-native.spec.ts', spec])).toBe( + true + ) + } + expect(filterStep.run).toContain('pr-e2e-source-routing.mjs --reusable-workflow') + expect(filterStep.run).toContain('if [ "$SHOULD_RUN" = true ]; then') + }) +}) diff --git a/config/scripts/pr-e2e-source-routing.mjs b/config/scripts/pr-e2e-source-routing.mjs index 78814b663cb..5b698fb0b42 100644 --- a/config/scripts/pr-e2e-source-routing.mjs +++ b/config/scripts/pr-e2e-source-routing.mjs @@ -217,6 +217,16 @@ export function hasNativeImeSourceChange(changedPaths) { ).some((route) => changedPaths.some(route.matches)) } +export function shouldRunReusablePrE2e(changedPaths) { + // Native IME has its own workflow; SSH still runs inside the reusable workflow. + return ( + hasSshSourceChange(changedPaths) || + selectPrE2eSpecs(changedPaths).some( + (spec) => spec !== 'tests/e2e/terminal-ibus-hangul-native.spec.ts' + ) + ) +} + if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { let input = '' process.stdin.setEncoding('utf8') @@ -226,6 +236,8 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) const changedPaths = input.split(/\r?\n/).filter(Boolean) if (process.argv.includes('--ssh-source')) { process.stdout.write(`${hasSshSourceChange(changedPaths)}\n`) + } else if (process.argv.includes('--reusable-workflow')) { + process.stdout.write(`${shouldRunReusablePrE2e(changedPaths)}\n`) } else if (process.argv.includes('--native-ime-source')) { process.stdout.write(`${hasNativeImeSourceChange(changedPaths)}\n`) } else { diff --git a/docs/reference/ci-runner-efficiency.md b/docs/reference/ci-runner-efficiency.md index a9f644bc435..6d688598097 100644 --- a/docs/reference/ci-runner-efficiency.md +++ b/docs/reference/ci-runner-efficiency.md @@ -131,3 +131,69 @@ environments currently exist. An environment-gated design adds a GitHub approval after each SignPath approval and changes the current automatic inner signing timeout fallback; those are explicit release-policy decisions, so this PR leaves production signing behavior unchanged. + +## Second audit and hosted trials + +- Cloud Verify ran 100 times in a sampled 39-hour window (84 PR and 16 push + runs). Move its four Ubuntu 22.04 jobs from Blacksmith to standard hosted + Ubuntu 22.04, preserving Postgres, secret scanning, build, tests, and Terraform + validation. Baseline [34001538145](https://github.com/stablyai/orca/actions/runs/34001538145) + used 64/72/26/19 seconds for security/test/build/Terraform respectively. + This conserves the shared provider allowance; hosted latency must be checked. +- Keep full tag history for the 13-job skill round-trip matrix, but fetch blobs + lazily. Only two historical SKILL.md files are materialized. Baseline + [33999994876](https://github.com/stablyai/orca/actions/runs/33999994876) + spent 42–84 seconds per checkout, about 14 aggregate runner minutes. A hosted + trial must verify historical blob fetches on all three operating systems. +- Use the existing Electron/native dependency cache for native IME CI. Keep + both deterministic boundary and real IBus tests. Add pnpm store caching to + terminal perf and release golden/evidence lanes; retain their raw installs + because manually selected older refs may not contain the shared action. +- Disable ZIP recompression only for already-compressed NSIS installers sent + to SignPath. Installer contents, release compression, and signing stay intact. +- Advance existing placement and startup deadlines with scoped fake timers in + three renderer test files. All 34 tests pass in 62 ms of local test execution, + versus 65.182 seconds in the sampled hosted baseline. Imports and transforms + still dominate invocation time; this is not a claim of equal PR wall savings. + +Eight unit shards already have balanced 260–296-second sample durations. +Reducing shards or removing test isolation lacks evidence of a net gain. Real +subprocess tests intentionally cover lifecycle behavior and retain real clocks. +The 14-way E2E split retains headroom after earlier 12-way timeouts. Lowering +coverage or schedule frequency is outside this efficiency pass. Cache complexity +for a seven-second docs install is unlikely to pay back. Release build reuse +across modes risks differing telemetry identities and native platform artifacts. + +Terminal Perf's baseline [33955846492](https://github.com/stablyai/orca/actions/runs/33955846492) +failed waiting 30 seconds for workspaceSessionReady in its shared-page fixture, +before measuring terminal performance. Compare hosted trials against that known +failure rather than attributing it to dependency cache changes. + +Hosted trials for the second audit: + +- [Cloud Verify 34002295216](https://github.com/stablyai/orca/actions/runs/34002295216) + passed all four jobs on standard hosted Ubuntu: security 57s, test 102s, build + 35s, Terraform 19s. The test lane is 30s slower than the Blacksmith sample; + retain this modest latency tradeoff to conserve shared allowance. +- [Skill matrix 34002295221](https://github.com/stablyai/orca/actions/runs/34002295221) + passed all 13 legs, including historical blob materialization. Checkout took + 18–20s on Linux, 39–45s on macOS, and 49–58s on Windows, versus the earlier + 42–84s range across platforms. These are observational samples. +- [Native IME 34002299594](https://github.com/stablyai/orca/actions/runs/34002299594) + passed both deterministic and real IBus checks. Shared dependency setup took + 29s, versus 35s for the old install/toolchain steps in the sampled baseline. +- Native-IME-only source/spec changes no longer allocate the reusable E2E + build, cache, and consumer jobs just to filter out the native spec. The + separate native workflow still runs; SSH-only and mixed spec lists still + allocate the reusable workflow. Routing contracts exercise these cases. +- [Hourly 34001816449](https://github.com/stablyai/orca/actions/runs/34001816449) + exercised the new five-second preflight and successfully published macOS. + The Windows follow-up failed in its unchanged input-vetting fetch because + remote refs differ only by case on its case-insensitive filesystem. The + requested SHA was correct; this does not validate an unchanged-main skip yet. + +Moving the daily Mac freshness check has lower expected value than hourly: +only one potential idle allocation per day, and active development usually +requires that build. Defer another release-graph change until skip frequency +justifies it. The substantive remaining release occupancy opportunity is the +separately documented asynchronous signing policy decision. diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts b/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts index 13daf0c9f20..391bdf10693 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts @@ -199,16 +199,25 @@ async function applySnapshot( store: TestStore, snap: RemoteWorkspaceObservedSnapshot ): Promise { - await applyDirectSshRemoteWorkspaceSnapshot({ - store, - snapshot: snap, - token: token(snap.revision), - arrival: 1, - isArrivalCurrent: () => true, - isPreparationTokenCurrent: () => true, - waitForWorkspaceSessionReady: async () => true, - finalizeHydratedTerminals: () => 0 - }) + vi.useFakeTimers() + try { + const pending = applyDirectSshRemoteWorkspaceSnapshot({ + store, + snapshot: snap, + token: token(snap.revision), + arrival: 1, + isArrivalCurrent: () => true, + isPreparationTokenCurrent: () => true, + waitForWorkspaceSessionReady: async () => true, + finalizeHydratedTerminals: () => 0 + }) + // Exercise the real placement deadline without spending ten wall-clock seconds per snapshot. + await vi.advanceTimersByTimeAsync(10_000) + await pending + } finally { + vi.clearAllTimers() + vi.useRealTimers() + } } function adoptedTabIds(store: TestStore): string[] { diff --git a/src/renderer/src/hooks/remote-workspace-target-sync.test.ts b/src/renderer/src/hooks/remote-workspace-target-sync.test.ts index bc0082c03ce..7d9f662ffe0 100644 --- a/src/renderer/src/hooks/remote-workspace-target-sync.test.ts +++ b/src/renderer/src/hooks/remote-workspace-target-sync.test.ts @@ -680,7 +680,15 @@ describe('createRemoteWorkspaceTargetSync', () => { ] }) - await harness.sync.applyUnsolicitedSnapshot('target-a', incoming) + vi.useFakeTimers() + try { + const pending = harness.sync.applyUnsolicitedSnapshot('target-a', incoming) + await vi.advanceTimersByTimeAsync(10_000) + await pending + } finally { + harness.sync.stop() + vi.useRealTimers() + } const merged = hydrateTabsSession.mock.calls[0][0] expect(merged.tabsByWorktree).toEqual({ diff --git a/src/renderer/src/startup/startup-ssh-connection-restore.test.ts b/src/renderer/src/startup/startup-ssh-connection-restore.test.ts index 3464816d0d8..3f74655454a 100644 --- a/src/renderer/src/startup/startup-ssh-connection-restore.test.ts +++ b/src/renderer/src/startup/startup-ssh-connection-restore.test.ts @@ -146,6 +146,7 @@ describe('restoreSshConnectionsForStartup', () => { }) it('does not push a connected background target back into the deferred list', async () => { + vi.useFakeTimers() installWindowApi([target('ssh-active'), target('ssh-bg')]) // The active host never answers and times out; the background host connects first. harness.connect.mockImplementation((targetId: string) => @@ -154,7 +155,7 @@ describe('restoreSshConnectionsForStartup', () => { : new Promise(() => {}) ) - await restoreSshConnectionsForStartup({ + const restore = restoreSshConnectionsForStartup({ connectionIds: ['ssh-active', 'ssh-bg'], blockingConnectionIds: ['ssh-active'], setDeferredSshReconnectTargets: harness.setDeferredSshReconnectTargets, @@ -162,11 +163,14 @@ describe('restoreSshConnectionsForStartup', () => { publishSshConnectionState: harness.publishSshConnectionState }) + await vi.advanceTimersByTimeAsync(15_000) + await restore + expect(harness.removeDeferredSshReconnectTarget).toHaveBeenCalledWith('ssh-bg') // The timed-out rewrite must not resurrect the reachable background target: a deferred // connected target sends fresh panes down the cold-restore path instead of the normal one. expect(harness.setDeferredSshReconnectTargets).toHaveBeenLastCalledWith(['ssh-active']) - }, 30_000) + }) it('keeps passphrase targets deferred and never dials them', async () => { installWindowApi([target('ssh-key', true), target('ssh-bg')]) From ef3f507903bb522bb7e0f74db994020402f3254e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:38:45 -0700 Subject: [PATCH 05/43] ci: verify release ref trust and preserve case twins during checkout (#18980) --- .github/workflows/adhoc-mac-build.yml | 3 + .github/workflows/release-ref-validation.yml | 38 ++++++ .../workflow-ref-mirror-case-safety.test.mjs | 10 ++ .../workflow-ref-reachability.test.mjs | 125 ++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 .github/workflows/release-ref-validation.yml create mode 100644 config/scripts/workflow-ref-reachability.test.mjs diff --git a/.github/workflows/adhoc-mac-build.yml b/.github/workflows/adhoc-mac-build.yml index 761a9585b73..3e17eee9b68 100644 --- a/.github/workflows/adhoc-mac-build.yml +++ b/.github/workflows/adhoc-mac-build.yml @@ -160,6 +160,9 @@ jobs: - name: Checkout the requested ref uses: actions/checkout@v6 + env: + # Full-history checkout must also preserve case-twin branch and tag names. + GIT_DEFAULT_REF_FORMAT: reftable with: # Why an input at all rather than just github.ref: the whole point is to # build code that has not landed, and the workflow definition itself diff --git a/.github/workflows/release-ref-validation.yml b/.github/workflows/release-ref-validation.yml new file mode 100644 index 00000000000..6995f9db174 --- /dev/null +++ b/.github/workflows/release-ref-validation.yml @@ -0,0 +1,38 @@ +name: Release ref validation + +on: + pull_request: + paths: + - '.github/workflows/adhoc-mac-build.yml' + - '.github/workflows/dev-channel-win-build.yml' + - '.github/workflows/release-ref-validation.yml' + - 'config/scripts/workflow-ref-reachability.test.mjs' + - 'config/scripts/workflow-ref-mirror-case-safety.test.mjs' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: release-ref-validation-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + validate: + strategy: + fail-fast: false + matrix: + os: [macos-15, windows-2022] + runs-on: ${{ matrix.os }} + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - uses: ./.github/actions/install-node-dependencies + - name: Verify case-twin refs and release trust boundary + run: >- + pnpm exec vitest run --config config/vitest.config.ts + config/scripts/workflow-ref-reachability.test.mjs + config/scripts/workflow-ref-mirror-case-safety.test.mjs + config/scripts/dev-channel-windows-workflow-contract.test.mjs diff --git a/config/scripts/workflow-ref-mirror-case-safety.test.mjs b/config/scripts/workflow-ref-mirror-case-safety.test.mjs index 6008c9d8d5f..31366f5e489 100644 --- a/config/scripts/workflow-ref-mirror-case-safety.test.mjs +++ b/config/scripts/workflow-ref-mirror-case-safety.test.mjs @@ -15,6 +15,16 @@ const REF_MIRRORS = [ ] describe('ref-mirroring vet steps', () => { + it('keeps the full-history adhoc checkout on the same case-safe backend', () => { + const steps = readWorkflow('.github/workflows/adhoc-mac-build.yml').jobs['build-adhoc-mac'] + .steps + const checkout = steps.find((step) => step.name === 'Checkout the requested ref') + expect(checkout.env.GIT_DEFAULT_REF_FORMAT).toBe('reftable') + expect(checkout.with.ref).toBe('${{ steps.vetted.outputs.sha }}') + expect(checkout.with['fetch-depth']).toBe(0) + expect(checkout.with['persist-credentials']).toBe(false) + }) + // Why: macOS and Windows runner disks are case-insensitive, and this repo has // branches that differ only in casing. The files backend cannot store both, and // it fails the whole fetch rather than the one ref — so the vet step dies before diff --git a/config/scripts/workflow-ref-reachability.test.mjs b/config/scripts/workflow-ref-reachability.test.mjs new file mode 100644 index 00000000000..d71c3094c56 --- /dev/null +++ b/config/scripts/workflow-ref-reachability.test.mjs @@ -0,0 +1,125 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { parse } from 'yaml' +import { runProcess } from '../../src/shared/child-process/run-process' + +const readWorkflow = (name) => parse(readFileSync(`.github/workflows/${name}.yml`, 'utf8')) +const windowsVet = readWorkflow('dev-channel-win-build').jobs['build-win'].steps.find( + (step) => step.id === 'vetted' +) +const macSteps = readWorkflow('adhoc-mac-build').jobs['build-adhoc-mac'].steps +const macVet = macSteps.find((step) => step.id === 'vetted') +const macCheckout = macSteps.find((step) => step.name === 'Checkout the requested ref') +const directory = mkdtempSync(join(tmpdir(), 'workflow-ref-reachability-')) +const repository = join(directory, 'remote.git') +const identity = { + ...process.env, + GIT_AUTHOR_NAME: 'Ref test', + GIT_AUTHOR_EMAIL: 'ref-test@example.com', + GIT_COMMITTER_NAME: 'Ref test', + GIT_COMMITTER_EMAIL: 'ref-test@example.com' +} +let ancestor, upper, lower, untrusted + +async function git(args, env = identity) { + const result = await runProcess({ program: 'git', args, env }) + expect(result.code, result.stderr).toBe(0) + return result.stdout.trim() +} + +beforeAll(async () => { + await git(['init', '--bare', '--ref-format=reftable', repository]) + const tree = await git(['-C', repository, 'mktree']) + ancestor = await git(['-C', repository, 'commit-tree', tree, '-m', 'ancestor']) + upper = await git(['-C', repository, 'commit-tree', tree, '-p', ancestor, '-m', 'upper']) + lower = await git(['-C', repository, 'commit-tree', tree, '-p', ancestor, '-m', 'lower']) + untrusted = await git(['-C', repository, 'commit-tree', tree, '-m', 'PR only']) + for (const [ref, sha] of [ + ['refs/heads/Fix', upper], + ['refs/heads/fix', lower], + ['refs/pull/1/head', untrusted] + ]) { + await git(['-C', repository, 'update-ref', ref, sha]) + } + await git(['-C', repository, 'tag', '-a', 'Release', upper, '-m', 'upper tag']) + await git(['-C', repository, 'tag', '-a', 'release', lower, '-m', 'lower tag']) + await git(['-C', repository, 'config', 'uploadpack.allowFilter', 'true']) +}) + +afterAll(() => rmSync(directory, { recursive: true, force: true })) + +async function vet(step, ref) { + const scratch = mkdtempSync(join(directory, 'attempt-')) + const script = join(scratch, 'vet.sh') + writeFileSync(script, step.run) + return runProcess({ + program: 'bash', + args: [script], + env: { + ...identity, + REPO_URL: pathToFileURL(repository).href, + RUNNER_TEMP: scratch, + GITHUB_OUTPUT: join(scratch, 'output'), + REQUESTED_REF: ref, + REQUESTED_SHA: ref, + CHANNEL: 'hourly', + TAG: 'v1.0.0-hourly.test', + VERSION: '1.0.0-hourly.test' + } + }) +} + +describe('release ref trust with case-twin names', () => { + it('accepts both branch tips, annotated tags, and their common ancestor', async () => { + for (const sha of [upper, lower, ancestor]) { + const result = await vet(windowsVet, sha) + expect(result.code, result.stderr).toBe(0) + } + for (const ref of ['Fix', 'fix', 'Release', 'release', ancestor]) { + const result = await vet(macVet, ref) + expect(result.code, result.stderr).toBe(0) + } + }) + + it('rejects PR-only commits even when the server has their objects', async () => { + for (const step of [windowsVet, macVet]) { + const result = await vet(step, untrusted) + expect(result.code).not.toBe(0) + expect(result.stdout).toContain('not reachable from any branch or tag') + } + const result = await vet(macVet, 'refs/pull/1/head') + expect(result.code).not.toBe(0) + expect(result.stdout).toContain('Refusing to build PR ref') + }) + + it('preserves both case variants in the subsequent full-history checkout', async () => { + const checkout = join(directory, 'checkout') + const env = { ...identity, ...macCheckout.env } + await git(['init', checkout], env) + await git( + [ + '-C', + checkout, + 'fetch', + '--no-tags', + repository, + '+refs/heads/*:refs/remotes/origin/*', + '+refs/tags/*:refs/tags/*' + ], + env + ) + await git(['-C', checkout, 'checkout', '--detach', upper], env) + for (const [ref, sha] of [ + ['refs/remotes/origin/Fix', upper], + ['refs/remotes/origin/fix', lower], + ['refs/tags/Release', upper], + ['refs/tags/release', lower] + ]) { + expect(await git(['-C', checkout, 'rev-parse', `${ref}^{commit}`], env)).toBe(sha) + } + expect(await git(['-C', checkout, 'rev-parse', 'HEAD'], env)).toBe(upper) + }) +}) From e7dc9b60995d73a0206c34891188e45cd9b708f2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:41:24 -0700 Subject: [PATCH 06/43] test: honor background launch in paired client window helpers (#18978) --- AGENTS.md | 6 +++ tests/AGENTS.md | 5 +- .../helpers/paired-client-window-reveal.ts | 15 ++++-- .../paired-client-window-reveal.unit.test.ts | 47 ++++++++++++++++++- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8b0156ba6b1..306c9c8d5ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,12 @@ All UI work — layout, color, typography, spacing, component selection, UX beha ## Electron UI Validation +Always run tests and agent-launched apps in the background with `ORCA_BACKGROUND_LAUNCH=1`. +Never steal monitor focus or reveal test windows: no `show()`, `showInactive()`, `bringToFront()`, +`app.focus()`, or OS activation. Use CDP screenshots of hidden renderers. Keep native-focus and +visible-window tests paused on the user's desktop; run them on an isolated display or CI. +Rebuild modified launch-policy code before running an app; stale build wrappers are not safe. + Use the `$electron` skill and Playwright CDP for rendered Orca UI checks. Do not use computer-use for Orca UI validation. # Style diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 26987a87c25..30f522652fe 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -18,6 +18,5 @@ Rules when adding tests or scripts: - Do not reveal windows in explicit background or headless runs. Only an explicitly headful run may call `showInactive()`; never call `show()` or `bringToFront()` in automated background checks. - Tag a spec `@headful` only when it needs real pixels; it still runs in the background. -- `ORCA_E2E_FOREGROUND=1` is the only opt-out, for runs whose subject _is_ native focus (IME and - other OS-level key injection). Clear `ORCA_BACKGROUND_LAUNCH` for that isolated run and add a - comment saying why; an explicit background request takes precedence. +- Native-focus tests belong on an isolated display or CI. Do not set `ORCA_E2E_FOREGROUND=1` + on the user’s desktop; it cannot override explicit background mode. diff --git a/tests/e2e/helpers/paired-client-window-reveal.ts b/tests/e2e/helpers/paired-client-window-reveal.ts index 302d573c3d1..1ec5b635d77 100644 --- a/tests/e2e/helpers/paired-client-window-reveal.ts +++ b/tests/e2e/helpers/paired-client-window-reveal.ts @@ -29,22 +29,24 @@ export function assertPairedClientWindowRevealed(report: PairedClientWindowRevea export type PairedClientWindowFocusReport = PairedClientWindowRevealReport & { isFocused: boolean } /** - * Brings a paired client to the front, which a launched-but-background window never is. Main-side - * policies that ask whether the reader is looking at a WebContents read the OS focus state, so a - * spec driving real presses through such a policy has to put the window there first. + * Native-focus coverage must run on an isolated display or CI, never in background mode. */ export async function focusPairedClientWindow( client: RevealablePairedClient, { timeoutMs = 15_000 }: { timeoutMs?: number } = {} ): Promise { + await client.app.evaluate(() => { + if (process.env.ORCA_BACKGROUND_LAUNCH === '1') { + throw new Error('Native focus is forbidden by ORCA_BACKGROUND_LAUNCH') + } + }) const revealed = await revealPairedClientWindow(client) const deadline = Date.now() + timeoutMs let isFocused = false while (!isFocused) { isFocused = await client.app.evaluate(({ app, BrowserWindow }) => { const window = BrowserWindow.getAllWindows()[0] - // Why steal: nothing else in the run is asking for the front, and the window manager keeps - // the launching terminal there otherwise. + // Native-focus coverage requires a dedicated foreground session. app.focus({ steal: true }) window?.focus() return window?.isFocused() ?? false @@ -61,6 +63,9 @@ export async function revealPairedClientWindow( client: RevealablePairedClient ): Promise { const report = await client.app.evaluate(({ BrowserWindow }) => { + if (process.env.ORCA_BACKGROUND_LAUNCH === '1') { + throw new Error('Window reveal is forbidden by ORCA_BACKGROUND_LAUNCH') + } const windows = BrowserWindow.getAllWindows() const window = windows[0] const wasVisible = window?.isVisible() ?? false diff --git a/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts b/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts index dfb83e4c441..706088e6762 100644 --- a/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts +++ b/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest' -import { assertPairedClientWindowRevealed } from './paired-client-window-reveal' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + assertPairedClientWindowRevealed, + focusPairedClientWindow, + revealPairedClientWindow, + type RevealablePairedClient +} from './paired-client-window-reveal' describe('assertPairedClientWindowRevealed', () => { it('accepts a window that the reveal made visible', () => { @@ -42,3 +47,41 @@ describe('assertPairedClientWindowRevealed', () => { ).toThrow(/stayed hidden after showInactive\(\)/) }) }) + +describe('paired client background safety', () => { + afterEach(() => vi.unstubAllEnvs()) + + function makeClient() { + const showInactive = vi.fn() + const focus = vi.fn() + const getAllWindows = vi.fn(() => [{ isVisible: () => false, showInactive, focus }]) + const evaluate = vi.fn(async (callback) => + callback({ + app: { focus }, + BrowserWindow: { getAllWindows } + }) + ) + const client = { + app: { evaluate }, + page: { waitForFunction: vi.fn() } + } as unknown as RevealablePairedClient + return { client, showInactive, focus, getAllWindows } + } + + it('rejects an explicit reveal before touching native windows', async () => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', '1') + const { client, getAllWindows, showInactive } = makeClient() + await expect(revealPairedClientWindow(client)).rejects.toThrow('Window reveal is forbidden') + expect(getAllWindows).not.toHaveBeenCalled() + expect(showInactive).not.toHaveBeenCalled() + }) + + it.each(['0', '1'])('rejects focus in background mode with foreground=%s', async (foreground) => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', '1') + vi.stubEnv('ORCA_E2E_FOREGROUND', foreground) + const { client, focus, getAllWindows } = makeClient() + await expect(focusPairedClientWindow(client)).rejects.toThrow('Native focus is forbidden') + expect(getAllWindows).not.toHaveBeenCalled() + expect(focus).not.toHaveBeenCalled() + }) +}) From 6d691a4c04c40fb3734f7066aecd002fe126e0cc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:43:36 -0700 Subject: [PATCH 07/43] test: bound release checkout lock fixtures and gate delayed imports (#18981) --- .../release-checkout.unit.test.ts | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/tests/e2e/cross-version-wire/release-checkout.unit.test.ts b/tests/e2e/cross-version-wire/release-checkout.unit.test.ts index 7057a38babd..106e2ea778e 100644 --- a/tests/e2e/cross-version-wire/release-checkout.unit.test.ts +++ b/tests/e2e/cross-version-wire/release-checkout.unit.test.ts @@ -263,12 +263,24 @@ afterEach(() => { describe('release checkout materialization', () => { it('single-flights concurrent consumers of one release identity', async () => { const cacheRoot = temporaryCacheRoot() + let publications = 0 + const options = { + cacheRoot, + testHooks: { + populateStaging: async (context: CheckoutStagingContext) => { + publications++ + await populateMinimalStaging(context) + } + } + } const checkouts = await Promise.all([ - materializeReleaseCheckout('v1.4.190', { cacheRoot }), - materializeReleaseCheckout('v1.4.190', { cacheRoot }), - materializeReleaseCheckout('v1.4.190', { cacheRoot }) + materializeReleaseCheckout('v1.4.190', options), + materializeReleaseCheckout('v1.4.190', options), + materializeReleaseCheckout('v1.4.190', options) ]) + expect(publications).toBe(1) + expect(new Set(checkouts.map(({ root }) => root))).toHaveLength(1) expect(relative(cacheRoot, checkouts[0]!.root)).not.toMatch(/^\.\./) }) @@ -291,18 +303,29 @@ describe('release checkout materialization', () => { ) const cacheRoot = temporaryCacheRoot() - const first = await materializeReleaseCheckout(firstRef, { cacheRoot }) + const options = { cacheRoot, testHooks: { populateStaging: populateMinimalStaging } } + const first = await materializeReleaseCheckout(firstRef, options) const dependency = join(first.root, 'delayed-dependency.mjs') const entry = join(first.root, 'delayed-entry.mjs') + const importStarted = join(cacheRoot, 'import-started') + const continueImport = join(cacheRoot, 'continue-import') writeFileSync(dependency, "export const loaded = 'first-release'\n") writeFileSync( entry, - 'await new Promise((resolve) => setTimeout(resolve, 100))\n' + + "import { existsSync, writeFileSync } from 'node:fs'\n" + + `writeFileSync(${JSON.stringify(importStarted)}, '')\n` + + `while (!existsSync(${JSON.stringify(continueImport)})) await new Promise((resolve) => setTimeout(resolve, 10))\n` + "export const loaded = (await import('./delayed-dependency.mjs')).loaded\n" ) const loading = importReleaseCheckoutModule(first, '/delayed-entry.mjs') - const second = await materializeReleaseCheckout(secondRef, { cacheRoot }) + let second: ReleaseCheckout + try { + await waitForFile(importStarted, 5_000) + second = await materializeReleaseCheckout(secondRef, options) + } finally { + writeFileSync(continueImport, '') + } await expect(loading).resolves.toMatchObject({ loaded: 'first-release' }) expect(first.root).not.toBe(second.root) @@ -311,7 +334,10 @@ describe('release checkout materialization', () => { it('causally single-flights a rival process before publishing an in-use checkout', async () => { const cacheRoot = temporaryCacheRoot() const scratch = temporaryCacheRoot() - const published = await materializeReleaseCheckout('v1.4.190', { cacheRoot }) + const published = await materializeReleaseCheckout('v1.4.190', { + cacheRoot, + testHooks: { populateStaging: populateMinimalStaging } + }) await expect(runContentionPhase(published, scratch, 'locked', false)).resolves.toBe(true) // In the same causally acknowledged interleaving, a no-lock materializer From 84432d3aa1584c135283fb4be22ee25e1bb5258f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:45:34 -0700 Subject: [PATCH 08/43] fix(native-chat): repair a structured chat tab permanently fenced by an inherited publication epoch (#18906) * fix(native-chat): repair a structured tab fenced out by a returning publisher A publication epoch is retired whenever another publisher takes over a worktree, and a retired epoch is then rejected forever. But a live publisher can return after transient interlopers - a `removed:` retraction, then a headless rebuild whose version restarts at 1 - and the structured tab publish inherits the worktree's existing epoch rather than minting one, so it arrives under the blacklisted epoch and is dropped. The chat tab never reaches the tab bar. The fence is right to reject the frame: it cannot tell a returning publisher apart from a delayed frame queued by a dead generation, whose version can outrank the live cursor. So the drop is no longer final - it schedules one bounded, debounced authoritative `session.tabs.listAll`, and only that census may revive an epoch, and only the one it names current. Subscription frames stay fenced exactly as before. * fix(native-chat): decay the structured tab repair cap and prune its state The attempt cap latched: three transient RPC failures left `exhausted` set for the renderer's lifetime, permanently hiding a chat tab behind a single console warning. It now decays, so a worktree that has been quiet for a minute gets its full budget back. The repair map was also missing from the sweep that drops publisher cursors for vanished worktrees, leaking an entry per deleted worktree. Pruning it there required inverting the repair lane's dependency on the inventory refresh, which is now injected. --------- Co-authored-by: Merge Sim --- ...tured-session-retired-epoch-repair.test.ts | 198 ++++++++++++++++++ .../inventory-refresh.ts | 10 +- .../retired-epoch-repair.test.ts | 121 +++++++++++ .../retired-epoch-repair.ts | 131 ++++++++++++ .../snapshot-apply.ts | 37 +++- .../subscription.ts | 19 +- .../publisher-identity-fences.ts | 13 ++ 7 files changed, 518 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/runtime/local-structured-session-retired-epoch-repair.test.ts create mode 100644 src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.test.ts create mode 100644 src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.ts diff --git a/src/renderer/src/runtime/local-structured-session-retired-epoch-repair.test.ts b/src/renderer/src/runtime/local-structured-session-retired-epoch-repair.test.ts new file mode 100644 index 00000000000..95edbadbf58 --- /dev/null +++ b/src/renderer/src/runtime/local-structured-session-retired-epoch-repair.test.ts @@ -0,0 +1,198 @@ +/** + * A live publisher can return to a worktree after another one briefly owned it, and the epoch it + * returns under is already in `retired`. The fence rejects that frame — correctly, because it + * cannot tell it apart from a delayed frame queued by a dead generation — so the drop has to be + * repaired from authority instead of being final. + * + * The sequence below is the measured one: a renderer publication, a `removed:` retraction, a + * headless rebuild whose version restarts at 1, then the same renderer epoch returning at a higher + * version carrying a newly published chat tab. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import type { Tab } from '../../../shared/tab-types' +import { + applyLocalStructuredSessionTabSnapshots, + resetLocalStructuredSessionVersionForTests +} from './local-structured-session-tabs-sync' +import { localStructuredSessionEpochHistoryByWorktree } from './local-structured-session-tabs-sync/inventory-generation-fence' +import type { WebSessionTabsSyncState } from './web-session-tabs-sync' +import { resetWebSessionFocusIntentForTests } from './web-session-focus-intent' + +const WORKTREE = 'folder:ws-1' +const ROOT_GROUP = 'local-root-group' +const RENDERER_EPOCH = 'renderer:53c8f87d' +const HEADLESS_EPOCH = 'headless:pty-backed:mtovsn3x' +const REMOVED_EPOCH = 'removed:mtovryl4' + +afterEach(() => { + resetWebSessionFocusIntentForTests() + resetLocalStructuredSessionVersionForTests() +}) + +/** + * The worktree must stay "known" or the trailing cursor sweep deletes its epoch history every + * round and nothing ever accumulates in `retired` — which makes this whole scenario vacuous. + */ +function stateWithCoordinatorTerminal(): WebSessionTabsSyncState { + const terminalTab: Tab = { + id: 'u-term-1', + entityId: 'term-1', + groupId: ROOT_GROUP, + worktreeId: WORKTREE, + contentType: 'terminal', + label: 'Terminal 1', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + return { + activeBrowserTabId: null, + activeBrowserTabIdByWorktree: {}, + activeFileId: null, + activeFileIdByWorktree: {}, + activeGroupIdByWorktree: { [WORKTREE]: ROOT_GROUP }, + activeTabId: 'u-term-1', + activeTabIdByWorktree: { [WORKTREE]: 'u-term-1' }, + activeTabType: 'terminal', + activeTabTypeByWorktree: { [WORKTREE]: 'terminal' }, + activeWorktreeId: WORKTREE, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + browserCertificateFailuresByPageId: {}, + browserPagesByWorkspace: {}, + browserTabsByWorktree: {}, + folderWorkspaces: [{ id: 'ws-1', name: 'ws', folderPath: '/tmp/ws' }], + groupsByWorktree: { + [WORKTREE]: [ + { id: ROOT_GROUP, worktreeId: WORKTREE, activeTabId: 'u-term-1', tabOrder: ['u-term-1'] } + ] + }, + layoutByWorktree: { [WORKTREE]: { type: 'leaf', groupId: ROOT_GROUP } }, + openFiles: [], + ptyIdsByTabId: { 'term-1': ['pty-1'] }, + remoteBrowserPageHandlesByPageId: {}, + tabBarOrderByWorktree: {}, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + unifiedTabsByWorktree: { [WORKTREE]: [terminalTab] }, + unreadTerminalTabs: {}, + sortEpoch: 0 + } as unknown as WebSessionTabsSyncState +} + +function frame( + publicationEpoch: string, + snapshotVersion: number, + sessionId: string | null +): RuntimeMobileSessionTabsResult { + const id = sessionId ? `agent-session:${sessionId}` : null + return { + worktree: WORKTREE, + publicationEpoch, + snapshotVersion, + activeGroupId: ROOT_GROUP, + activeTabId: null, + activeTabType: null, + tabGroups: [{ id: ROOT_GROUP, activeTabId: null, tabOrder: id ? [id] : [] }], + tabs: id + ? [ + { + type: 'agent-session', + id, + title: 'Claude Chat', + sessionId, + agent: 'claude', + isActive: false + } + ] + : [] + } as RuntimeMobileSessionTabsResult +} + +function chatTabs(state: WebSessionTabsSyncState): string[] { + return (state.unifiedTabsByWorktree[WORKTREE] ?? []) + .filter((tab) => tab.contentType === 'agent-session') + .map((tab) => tab.label) +} + +/** Everything up to and including the drop; returns the state the repair has to fix. */ +function replayUntilDrop( + onRetiredEpochDrop?: (worktreeId: string, publicationEpoch: string) => void +): WebSessionTabsSyncState { + let state = stateWithCoordinatorTerminal() + state = applyLocalStructuredSessionTabSnapshots(state, [frame(RENDERER_EPOCH, 6, null)]) + state = applyLocalStructuredSessionTabSnapshots(state, [frame(REMOVED_EPOCH, 0, null)]) + state = applyLocalStructuredSessionTabSnapshots(state, [frame(HEADLESS_EPOCH, 1, null)]) + return applyLocalStructuredSessionTabSnapshots( + state, + [frame(RENDERER_EPOCH, 7, 'claude-1')], + undefined, + undefined, + onRetiredEpochDrop ? { onRetiredEpochDrop } : {} + ) +} + +describe('retired-epoch repair for a returning publisher', () => { + it('POSITIVE CONTROL: the same frame lands when no epoch has been retired', () => { + const applied = applyLocalStructuredSessionTabSnapshots(stateWithCoordinatorTerminal(), [ + frame(RENDERER_EPOCH, 7, 'claude-1') + ]) + + expect(chatTabs(applied)).toEqual(['Claude Chat']) + }) + + it('drops the returning publisher and reports it to the repair lane', () => { + const onRetiredEpochDrop = vi.fn() + + const dropped = replayUntilDrop(onRetiredEpochDrop) + + expect(chatTabs(dropped)).toEqual([]) + expect(onRetiredEpochDrop).toHaveBeenCalledWith(WORKTREE, RENDERER_EPOCH) + }) + + it('lands the tab when the authoritative census re-delivers the same frame', () => { + const dropped = replayUntilDrop() + expect(chatTabs(dropped)).toEqual([]) + + const repaired = applyLocalStructuredSessionTabSnapshots( + dropped, + [frame(RENDERER_EPOCH, 7, 'claude-1')], + undefined, + undefined, + { authoritative: true } + ) + + expect(chatTabs(repaired)).toEqual(['Claude Chat']) + }) + + it('a non-authoritative redelivery stays dropped, so only authority repairs it', () => { + const dropped = replayUntilDrop() + + const redelivered = applyLocalStructuredSessionTabSnapshots(dropped, [ + frame(RENDERER_EPOCH, 7, 'claude-1') + ]) + + expect(chatTabs(redelivered)).toEqual([]) + }) + + it('revives only the epoch authority names, leaving other generations fenced', () => { + const dropped = replayUntilDrop() + applyLocalStructuredSessionTabSnapshots( + dropped, + [frame(RENDERER_EPOCH, 7, 'claude-1')], + undefined, + undefined, + { authoritative: true } + ) + + // The census named the renderer epoch current, so the headless generation it displaced is now + // the retired one — and a delayed frame from it must still be rejected. + const history = localStructuredSessionEpochHistoryByWorktree.get(WORKTREE) + expect(history?.current).toBe(RENDERER_EPOCH) + expect(history?.retired).toContain(HEADLESS_EPOCH) + expect(history?.retired).not.toContain(RENDERER_EPOCH) + }) +}) diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts index af756458707..136e4fa2e30 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts @@ -21,9 +21,13 @@ export function restoreLocalStructuredSessionTabsOnce( ) } -/** Fetch the current host inventory even after the startup restore has settled. */ +/** Fetch the current host inventory even after the startup restore has settled. + * + * `authoritative` is opt-in and belongs to the repair lane alone: the startup restore stays + * fenced exactly as before, so nothing about first paint changes. */ export function refreshLocalStructuredSessionTabs( - expectedGeneration = localStructuredSessionGeneration() + expectedGeneration = localStructuredSessionGeneration(), + options: { authoritative?: boolean } = {} ): Promise { return window.api.runtime .call({ method: 'session.tabs.listAll', params: {} }) @@ -34,7 +38,7 @@ export function refreshLocalStructuredSessionTabs( const result = response.result as { snapshots?: RuntimeMobileSessionTabsResult[] } const snapshots = result.snapshots ?? [] if (isCurrentLocalStructuredSessionGeneration(expectedGeneration)) { - applyStructuredSessionTabSnapshots(snapshots) + applyStructuredSessionTabSnapshots(snapshots, undefined, options) } return snapshots }) diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.test.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.test.ts new file mode 100644 index 00000000000..563a7c3598b --- /dev/null +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.test.ts @@ -0,0 +1,121 @@ +/** + * The repair lane must not become worse than the bug it fixes: a publisher that keeps re-sending a + * retired epoch would otherwise drive an unbounded refetch loop, and a cap that never decays would + * hide a chat tab for the renderer's lifetime after a run of transient RPC failures. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { localStructuredSessionEpochHistoryByWorktree } from './inventory-generation-fence' +import { + forgetRetiredEpochRepairsOutside, + resetRetiredEpochRepairsForTests, + scheduleRetiredEpochRepair +} from './retired-epoch-repair' + +const WORKTREE = 'folder:ws-1' +const EPOCH = 'renderer:53c8f87d' + +const runRepair = vi.fn(async (_generation: number) => undefined) + +function markRetired(): void { + localStructuredSessionEpochHistoryByWorktree.set(WORKTREE, { + current: 'headless:pty-backed:x', + retired: [EPOCH] + }) +} + +beforeEach(() => { + vi.useFakeTimers() + runRepair.mockReset() + runRepair.mockImplementation(async () => undefined) + resetRetiredEpochRepairsForTests() + localStructuredSessionEpochHistoryByWorktree.clear() +}) + +afterEach(() => { + resetRetiredEpochRepairsForTests() + localStructuredSessionEpochHistoryByWorktree.clear() + vi.useRealTimers() +}) + +describe('retired-epoch repair scheduling', () => { + it('asks the host once for a burst of drops on one worktree', async () => { + markRetired() + + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + expect(runRepair).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(300) + + expect(runRepair).toHaveBeenCalledTimes(1) + }) + + it('stops after a bounded number of attempts and says so', async () => { + markRetired() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + // The epoch stays retired, so every refresh counts as a failed repair. + for (let attempt = 0; attempt < 6; attempt += 1) { + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + await vi.advanceTimersByTimeAsync(5000) + } + + expect(runRepair).toHaveBeenCalledTimes(3) + expect(warn).toHaveBeenCalledWith( + '[structured-session-tabs] retired publication epoch still unrepaired', + expect.objectContaining({ worktree: WORKTREE, publicationEpoch: EPOCH }) + ) + warn.mockRestore() + }) + + it('decays the cap instead of latching, so a later drop is still repairable', async () => { + markRetired() + vi.spyOn(console, 'warn').mockImplementation(() => {}) + + for (let attempt = 0; attempt < 4; attempt += 1) { + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + await vi.advanceTimersByTimeAsync(5000) + } + expect(runRepair).toHaveBeenCalledTimes(3) + + // A quiet minute later the worktree gets its budget back rather than staying hidden forever. + await vi.advanceTimersByTimeAsync(60_000) + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + await vi.advanceTimersByTimeAsync(300) + + expect(runRepair).toHaveBeenCalledTimes(4) + }) + + it('rearms once a repair actually revives the epoch', async () => { + markRetired() + runRepair.mockImplementation(async () => { + localStructuredSessionEpochHistoryByWorktree.set(WORKTREE, { + current: EPOCH, + retired: ['headless:pty-backed:x'] + }) + }) + + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + await vi.advanceTimersByTimeAsync(300) + expect(runRepair).toHaveBeenCalledTimes(1) + + markRetired() + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + await vi.advanceTimersByTimeAsync(300) + + expect(runRepair).toHaveBeenCalledTimes(2) + }) + + it('forgets repair state for worktrees that no longer exist', async () => { + markRetired() + scheduleRetiredEpochRepair(WORKTREE, EPOCH, runRepair) + + forgetRetiredEpochRepairsOutside(new Set(['folder:other'])) + await vi.advanceTimersByTimeAsync(5000) + + // The pending refetch for the vanished worktree is cancelled, not merely orphaned. + expect(runRepair).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.ts new file mode 100644 index 00000000000..1916fb21546 --- /dev/null +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/retired-epoch-repair.ts @@ -0,0 +1,131 @@ +/** + * Repairing a structured session snapshot the retired-epoch fence rejected. + * + * The fence is right to reject a subscription frame carrying a retired epoch — it cannot tell that + * frame apart from a delayed one queued by a dead publisher generation, whose version can be + * higher than the live cursor. What it cannot do is notice when the epoch's publisher is actually + * still alive and has simply returned after another publisher briefly owned the worktree. + * + * So the drop is not treated as final: it schedules one authoritative `session.tabs.listAll`, whose + * answer settles which epoch is current. If the epoch really is dead the census changes nothing; if + * it is live, the census carries it and the tab lands. The fence itself is never relaxed for + * subscription frames. + * + * The refresh is injected rather than imported so this module depends on nothing that in turn + * depends on the snapshot apply — which is what lets the apply prune this module's state. + */ + +import { + isCurrentLocalStructuredSessionGeneration, + localStructuredSessionEpochHistoryByWorktree, + localStructuredSessionGeneration +} from './inventory-generation-fence' + +/** Bounded so a publisher that keeps re-sending a retired epoch cannot drive an endless refetch. */ +const MAX_REPAIR_ATTEMPTS = 3 +const BASE_REPAIR_DELAY_MS = 250 +const MAX_REPAIR_DELAY_MS = 5000 +/** + * The cap decays rather than latching. A run of transient RPC failures must not hide a chat tab for + * the renderer's lifetime; once a worktree has been quiet this long, a fresh drop is a fresh + * problem and gets its full budget back. + */ +const REPAIR_ATTEMPT_DECAY_MS = 60_000 + +type RepairState = { + attempts: number + lastAttemptAt: number + timer: ReturnType | null +} + +export type RetiredEpochRepairRunner = (expectedGeneration: number) => Promise + +const repairsByWorktree = new Map() + +function repairState(worktreeId: string, now: number): RepairState { + const existing = repairsByWorktree.get(worktreeId) + if (!existing) { + const created: RepairState = { attempts: 0, lastAttemptAt: now, timer: null } + repairsByWorktree.set(worktreeId, created) + return created + } + if (now - existing.lastAttemptAt >= REPAIR_ATTEMPT_DECAY_MS) { + existing.attempts = 0 + } + return existing +} + +/** + * Schedules the authoritative refetch for a dropped snapshot, coalescing repeat drops for the same + * worktree into the one already pending. + */ +export function scheduleRetiredEpochRepair( + worktreeId: string, + publicationEpoch: string, + runRepair: RetiredEpochRepairRunner +): void { + const now = Date.now() + const state = repairState(worktreeId, now) + if (state.timer !== null) { + return + } + if (state.attempts >= MAX_REPAIR_ATTEMPTS) { + console.warn('[structured-session-tabs] retired publication epoch still unrepaired', { + worktree: worktreeId, + publicationEpoch, + attempts: state.attempts, + retryAfterMs: Math.max(0, REPAIR_ATTEMPT_DECAY_MS - (now - state.lastAttemptAt)) + }) + return + } + const generation = localStructuredSessionGeneration() + const delay = Math.min(BASE_REPAIR_DELAY_MS * 2 ** state.attempts, MAX_REPAIR_DELAY_MS) + state.attempts += 1 + state.lastAttemptAt = now + state.timer = setTimeout(() => { + state.timer = null + if (!isCurrentLocalStructuredSessionGeneration(generation)) { + repairsByWorktree.delete(worktreeId) + return + } + void runRepair(generation) + .then(() => { + // Why re-check rather than trust the call: a refresh that succeeds without reviving the + // epoch has not repaired anything, and counting it as success would loop forever. + const stillRetired = + localStructuredSessionEpochHistoryByWorktree + .get(worktreeId) + ?.retired.includes(publicationEpoch) ?? false + if (!stillRetired) { + repairsByWorktree.delete(worktreeId) + } + }) + .catch((error) => { + console.warn('[structured-session-tabs] retired-epoch repair refresh failed', error) + }) + }, delay) +} + +/** + * Drops repair state for worktrees that no longer exist, alongside the publisher cursors it + * shadows — without this every deleted worktree leaks an entry for the renderer's lifetime. + */ +export function forgetRetiredEpochRepairsOutside(knownWorktreeIds: ReadonlySet): void { + for (const [worktreeId, state] of repairsByWorktree) { + if (!knownWorktreeIds.has(worktreeId)) { + if (state.timer !== null) { + clearTimeout(state.timer) + } + repairsByWorktree.delete(worktreeId) + } + } +} + +export function resetRetiredEpochRepairsForTests(): void { + for (const state of repairsByWorktree.values()) { + if (state.timer !== null) { + clearTimeout(state.timer) + } + } + repairsByWorktree.clear() +} diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts index fc254de62dc..ad0d99bfeac 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply.ts @@ -7,7 +7,9 @@ import { } from '../web-session-tabs-sync' import type { WebSessionTabsSyncState } from '../web-session-tabs-sync' import { + hasRetiredValue, noteRetiredValue, + reviveRetiredValue, sameSessionTabsPublicationLineage } from '../web-session-tabs-sync/publisher-identity-fences' import { @@ -21,16 +23,30 @@ import { localStructuredSessionVersionByWorktree, supersedeLocalStructuredSessionGeneration } from './inventory-generation-fence' +import { forgetRetiredEpochRepairsOutside } from './retired-epoch-repair' import { projectLocalStructuredSessionTabs } from './snapshot-projection' export const LOCAL_STRUCTURED_SESSION_OWNER = 'local-structured-session' +export type StructuredSessionSnapshotApplyOptions = { + /** + * Marks these snapshots as an authoritative `session.tabs.listAll` response, which exempts them + * from the retired-epoch fence. A census is the synchronous answer to a request we just issued, + * so it cannot be the delayed frame from a dead generation that the fence exists to reject — + * whereas a subscription frame can be, and stays fenced. + */ + authoritative?: boolean + /** Called for each snapshot the retired-epoch fence rejects; the repair lane listens here. */ + onRetiredEpochDrop?: (worktreeId: string, publicationEpoch: string) => void +} + export function applyStructuredSessionTabSnapshots( snapshots: readonly RuntimeMobileSessionTabsResult[], - owner = LOCAL_STRUCTURED_SESSION_OWNER + owner = LOCAL_STRUCTURED_SESSION_OWNER, + options: StructuredSessionSnapshotApplyOptions = {} ): void { const settleStructuredSessionMirror = applyWebSessionTabsStorePatch( - (state) => applyLocalStructuredSessionTabSnapshots(state, snapshots, owner), + (state) => applyLocalStructuredSessionTabSnapshots(state, snapshots, owner, undefined, options), { frames: [] } ) settleStructuredSessionMirror() @@ -65,7 +81,8 @@ export function applyLocalStructuredSessionTabSnapshots< state: State, snapshots: readonly RuntimeMobileSessionTabsResult[], owner = LOCAL_STRUCTURED_SESSION_OWNER, - now = Date.now() + now = Date.now(), + options: StructuredSessionSnapshotApplyOptions = {} ): State { let next = state for (const snapshot of snapshots) { @@ -78,8 +95,17 @@ export function applyLocalStructuredSessionTabSnapshots< prior && sameSessionTabsPublicationLineage(prior.publicationEpoch, snapshot.publicationEpoch) ) const epochHistory = localStructuredSessionEpochHistoryByWorktree.get(snapshot.worktree) - if (epochHistory?.retired.includes(snapshot.publicationEpoch) && !sharesLineage) { - continue + // Why not just drop: an epoch is retired whenever another publisher takes over the worktree, + // but a live publisher can return after transient interlopers (a `removed:` retraction, then a + // headless rebuild), and the structured publish inherits the worktree's existing epoch rather + // than minting its own. So a retired epoch is not proof of a dead generation — only authority + // can settle it, and the repair lane goes and asks. + if (hasRetiredValue(epochHistory, snapshot.publicationEpoch) && !sharesLineage) { + if (!options.authoritative) { + options.onRetiredEpochDrop?.(snapshot.worktree, snapshot.publicationEpoch) + continue + } + reviveRetiredValue(epochHistory, snapshot.publicationEpoch) } if (prior && sharesLineage && snapshot.snapshotVersion <= prior.snapshotVersion) { continue @@ -114,5 +140,6 @@ export function applyLocalStructuredSessionTabSnapshots< localStructuredSessionEpochHistoryByWorktree.delete(worktreeId) } } + forgetRetiredEpochRepairsOutside(knownWorktreeIds) return next } diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync/subscription.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync/subscription.ts index b074cfb1c41..fef55f07a16 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync/subscription.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync/subscription.ts @@ -9,7 +9,20 @@ import { refreshLocalStructuredSessionTabs, restoreLocalStructuredSessionTabsOnce } from './inventory-refresh' -import { applyStructuredSessionTabSnapshots } from './snapshot-apply' +import { scheduleRetiredEpochRepair } from './retired-epoch-repair' +import { + applyStructuredSessionTabSnapshots, + type StructuredSessionSnapshotApplyOptions +} from './snapshot-apply' + +// The refresh is supplied here rather than imported by the repair lane, so nothing the snapshot +// apply depends on depends back on it. +const REPAIR_DROPPED_EPOCHS: StructuredSessionSnapshotApplyOptions = { + onRetiredEpochDrop: (worktreeId, publicationEpoch) => + scheduleRetiredEpochRepair(worktreeId, publicationEpoch, (generation) => + refreshLocalStructuredSessionTabs(generation, { authoritative: true }) + ) +} type SessionTabsEvent = | (RuntimeMobileSessionTabsResult & { type: 'snapshot' | 'updated' }) @@ -84,9 +97,9 @@ export async function startLocalStructuredSessionTabsSync(args: { } const event = response.result as SessionTabsEvent if (event.type === 'snapshots') { - applyStructuredSessionTabSnapshots(event.snapshots) + applyStructuredSessionTabSnapshots(event.snapshots, undefined, REPAIR_DROPPED_EPOCHS) } else if (event.type === 'snapshot' || event.type === 'updated') { - applyStructuredSessionTabSnapshots([event]) + applyStructuredSessionTabSnapshots([event], undefined, REPAIR_DROPPED_EPOCHS) } else if (event.type === 'end' && generation === subscriptionGeneration) { // Reattach with one refresh so a runtime-restart boundary cannot strand stale tabs. subscriptionGeneration += 1 diff --git a/src/renderer/src/runtime/web-session-tabs-sync/publisher-identity-fences.ts b/src/renderer/src/runtime/web-session-tabs-sync/publisher-identity-fences.ts index 96ebe2293c6..fbab01b591b 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/publisher-identity-fences.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/publisher-identity-fences.ts @@ -36,6 +36,19 @@ export function noteRetiredValue( return history } +/** + * Un-retires one value, leaving every other retired generation fenced. + * + * Only an authority that names the value current may call this; reviving on a delayed frame's own + * say-so is exactly the resurrection `retired` exists to prevent. + */ +export function reviveRetiredValue(history: RetiredValueHistory | undefined, value: string): void { + const index = history?.retired.indexOf(value) ?? -1 + if (history && index >= 0) { + history.retired.splice(index, 1) + } +} + function normalizeSessionTabsRuntimeId(runtimeId: unknown): string | undefined { if (typeof runtimeId !== 'string') { return undefined From fd10758eae985564b9f3258074fe6f175a47364e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:13:55 -0700 Subject: [PATCH 09/43] ci: expose existing E2E spec selection for manual dispatch (#18987) --- .github/workflows/e2e.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 3560d302a79..5f80c2090ad 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -27,6 +27,10 @@ on: description: Ref to check out (defaults to the workflow ref) required: false type: string + test_files: + description: JSON array of specs to run; empty runs the full suite + required: false + type: string schedule: # Why: GitHub cron uses UTC; these slots map to 10am and 3pm # America/Phoenix for the default-branch E2E run. From 681119dc05bab24469037ab50ce0be6c3cb1faa2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:33:47 -0700 Subject: [PATCH 10/43] test: isolate window mocks from inherited launch flags (#18989) * test: isolate mocked window activation from inherited launch flags * Preserve background window regressions added on main --- src/main/ipc/dashboard-popout.test.ts | 8 ++++++++ src/main/ipc/notifications-retention-lifecycle.test.ts | 10 +++++++++- .../window/createMainWindow-startup-reveal.test.ts | 10 +++++++++- src/main/window/dashboard-popout-window.test.ts | 8 ++++++++ src/main/window/focus-existing-window.test.ts | 8 +++++++- 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/main/ipc/dashboard-popout.test.ts b/src/main/ipc/dashboard-popout.test.ts index ce10b3556fc..9bead362816 100644 --- a/src/main/ipc/dashboard-popout.test.ts +++ b/src/main/ipc/dashboard-popout.test.ts @@ -97,6 +97,14 @@ function makeStore(enabled = true) { } } +// These cases exercise foreground behavior against Electron mocks. +beforeEach(() => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', undefined) + vi.stubEnv('ORCA_E2E_HEADLESS', undefined) + vi.stubEnv('ORCA_E2E_HEADFUL', undefined) +}) +afterEach(() => vi.unstubAllEnvs()) + describe('registerDashboardPopoutHandlers', () => { let store: ReturnType diff --git a/src/main/ipc/notifications-retention-lifecycle.test.ts b/src/main/ipc/notifications-retention-lifecycle.test.ts index 278ca8f13ae..9705cf85a58 100644 --- a/src/main/ipc/notifications-retention-lifecycle.test.ts +++ b/src/main/ipc/notifications-retention-lifecycle.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { getAllWindowsMock, @@ -30,6 +30,14 @@ vi.mock('../tray/system-tray', async () => import { registerNotificationHandlers } from './notifications' +// These cases exercise foreground behavior against Electron mocks. +beforeEach(() => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', undefined) + vi.stubEnv('ORCA_E2E_HEADLESS', undefined) + vi.stubEnv('ORCA_E2E_HEADFUL', undefined) +}) +afterEach(() => vi.unstubAllEnvs()) + describe('registerNotificationHandlers', () => { beforeEach(() => { vi.useFakeTimers() diff --git a/src/main/window/createMainWindow-startup-reveal.test.ts b/src/main/window/createMainWindow-startup-reveal.test.ts index f103881ea83..bd7eeadc6b2 100644 --- a/src/main/window/createMainWindow-startup-reveal.test.ts +++ b/src/main/window/createMainWindow-startup-reveal.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', async () => (await import('./createMainWindow-test-harness')).electronModuleMock() @@ -22,6 +22,14 @@ import { withPlatform } from './createMainWindow-test-harness' +// These cases exercise foreground behavior against Electron mocks. +beforeEach(() => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', undefined) + vi.stubEnv('ORCA_E2E_HEADLESS', undefined) + vi.stubEnv('ORCA_E2E_HEADFUL', undefined) +}) +afterEach(() => vi.unstubAllEnvs()) + describe('createMainWindow', () => { beforeEach(() => { resetMainWindowMocks() diff --git a/src/main/window/dashboard-popout-window.test.ts b/src/main/window/dashboard-popout-window.test.ts index 86735811bb8..0e64d573f76 100644 --- a/src/main/window/dashboard-popout-window.test.ts +++ b/src/main/window/dashboard-popout-window.test.ts @@ -171,6 +171,14 @@ function makeStore(ui: Record = {}): { const RENDERER_URL = 'http://localhost:5173' +// These cases exercise foreground behavior against Electron mocks. +beforeEach(() => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', undefined) + vi.stubEnv('ORCA_E2E_HEADLESS', undefined) + vi.stubEnv('ORCA_E2E_HEADFUL', undefined) +}) +afterEach(() => vi.unstubAllEnvs()) + describe('createOrFocusDashboardPopout', () => { beforeEach(() => { instances.length = 0 diff --git a/src/main/window/focus-existing-window.test.ts b/src/main/window/focus-existing-window.test.ts index 9f5dc522150..697b423ab37 100644 --- a/src/main/window/focus-existing-window.test.ts +++ b/src/main/window/focus-existing-window.test.ts @@ -1,5 +1,5 @@ import type { App, BrowserWindow } from 'electron' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { focusExistingMainWindow } from './focus-existing-window' type FakeWindowOptions = { @@ -78,6 +78,12 @@ function makeTimer(): { } } +// These cases exercise foreground behavior against Electron mocks. +beforeEach(() => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', undefined) + vi.stubEnv('ORCA_E2E_HEADLESS', undefined) + vi.stubEnv('ORCA_E2E_HEADFUL', undefined) +}) afterEach(() => vi.unstubAllEnvs()) describe('focusExistingMainWindow', () => { From bedbe5997ba86cc43d8142fa21d92f512a559567 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:45:42 -0700 Subject: [PATCH 11/43] test: match explorer filenames independently of git badges (#18997) --- tests/e2e/file-explorer-watch-refresh.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/file-explorer-watch-refresh.spec.ts b/tests/e2e/file-explorer-watch-refresh.spec.ts index d8a1bc1e4e7..85fbd66409e 100644 --- a/tests/e2e/file-explorer-watch-refresh.spec.ts +++ b/tests/e2e/file-explorer-watch-refresh.spec.ts @@ -35,7 +35,7 @@ test('refreshes the visible tree after external Windows file changes', async ({ const row = (name: string) => orcaPage .locator('[data-file-explorer-row]') - .filter({ hasText: new RegExp(`^${name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`) }) + .filter({ has: orcaPage.getByText(name, { exact: true }) }) rmSync(originalPath, { force: true }) rmSync(renamedPath, { force: true }) From 712cf1facb124149c7076bc1f80912e0196b043d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:57:29 -0700 Subject: [PATCH 12/43] test: synchronize large repository recovery with Retry request (#18999) --- tests/e2e/helpers/git-status-retry-barrier.ts | 61 +++++++++++++++++++ .../git-status-retry-barrier.unit.test.ts | 39 ++++++++++++ .../source-control-large-file-count.spec.ts | 19 ++++-- 3 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/helpers/git-status-retry-barrier.ts create mode 100644 tests/e2e/helpers/git-status-retry-barrier.unit.test.ts diff --git a/tests/e2e/helpers/git-status-retry-barrier.ts b/tests/e2e/helpers/git-status-retry-barrier.ts new file mode 100644 index 00000000000..e166313d003 --- /dev/null +++ b/tests/e2e/helpers/git-status-retry-barrier.ts @@ -0,0 +1,61 @@ +import type { ElectronApplication } from '@stablyai/playwright-test' + +type StatusArgs = { worktreePath?: string; admissionTier?: string } +type StatusHandler = (event: unknown, args?: StatusArgs) => unknown +type RetryBarrier = { + captured: boolean + release: () => void + original: StatusHandler +} +type BarrierScope = typeof globalThis & { __gitStatusRetryBarrier?: RetryBarrier } + +export async function installGitStatusRetryBarrier( + app: ElectronApplication, + repoPath: string +): Promise { + await app.evaluate(({ ipcMain }, repoPath) => { + const scope = globalThis as BarrierScope + const handlers = (ipcMain as unknown as { _invokeHandlers: Map }) + ._invokeHandlers + const original = handlers.get('git:status') + if (!original || scope.__gitStatusRetryBarrier) { + throw new Error('Git status handler unavailable or retry barrier already installed') + } + let release!: () => void + const pending = new Promise((resolve) => { + release = resolve + }) + const state: RetryBarrier = { captured: false, release, original } + scope.__gitStatusRetryBarrier = state + handlers.set('git:status', async (event, args) => { + if ( + !state.captured && + args?.worktreePath === repoPath && + args.admissionTier === 'interactive' + ) { + state.captured = true + await pending + } + return original(event, args) + }) + }, repoPath) +} + +export async function hasCapturedGitStatusRetry(app: ElectronApplication): Promise { + return app.evaluate(() => (globalThis as BarrierScope).__gitStatusRetryBarrier?.captured ?? false) +} + +export async function restoreGitStatusRetryHandler(app: ElectronApplication): Promise { + await app.evaluate(({ ipcMain }) => { + const scope = globalThis as BarrierScope + const state = scope.__gitStatusRetryBarrier + if (!state) { + return + } + const handlers = (ipcMain as unknown as { _invokeHandlers: Map }) + ._invokeHandlers + handlers.set('git:status', state.original) + state.release() + delete scope.__gitStatusRetryBarrier + }) +} diff --git a/tests/e2e/helpers/git-status-retry-barrier.unit.test.ts b/tests/e2e/helpers/git-status-retry-barrier.unit.test.ts new file mode 100644 index 00000000000..74bdd8d8159 --- /dev/null +++ b/tests/e2e/helpers/git-status-retry-barrier.unit.test.ts @@ -0,0 +1,39 @@ +import type { ElectronApplication } from '@stablyai/playwright-test' +import { describe, expect, it, vi } from 'vitest' +import { + hasCapturedGitStatusRetry, + installGitStatusRetryBarrier, + restoreGitStatusRetryHandler +} from './git-status-retry-barrier' + +describe('Git status retry barrier', () => { + it('holds the target interactive request and restores the real handler on cleanup', async () => { + const original = vi.fn(async (_event: unknown, args: unknown) => args) + const handlers = new Map([['git:status', original]]) + const app = { + evaluate: (callback: (electron: unknown, arg?: unknown) => unknown, arg?: unknown) => + Promise.resolve(callback({ ipcMain: { _invokeHandlers: handlers } }, arg)) + } as unknown as ElectronApplication + await installGitStatusRetryBarrier(app, 'target-repo') + try { + const handler = handlers.get('git:status')! + const background = { worktreePath: 'target-repo', admissionTier: 'background' } + const otherRepo = { worktreePath: 'another-repo', admissionTier: 'interactive' } + await expect(handler({}, background)).resolves.toEqual(background) + await expect(handler({}, otherRepo)).resolves.toEqual(otherRepo) + expect(await hasCapturedGitStatusRetry(app)).toBe(false) + + const retry = { worktreePath: 'target-repo', admissionTier: 'interactive' } + const event = {} + const pending = handler(event, retry) + expect(await hasCapturedGitStatusRetry(app)).toBe(true) + expect(original).toHaveBeenCalledTimes(2) + await restoreGitStatusRetryHandler(app) + await expect(pending).resolves.toEqual(retry) + expect(original).toHaveBeenLastCalledWith(event, retry) + expect(handlers.get('git:status')).toBe(original) + } finally { + await restoreGitStatusRetryHandler(app) + } + }) +}) diff --git a/tests/e2e/source-control-large-file-count.spec.ts b/tests/e2e/source-control-large-file-count.spec.ts index 28a5e7758f2..85f3710b308 100644 --- a/tests/e2e/source-control-large-file-count.spec.ts +++ b/tests/e2e/source-control-large-file-count.spec.ts @@ -20,6 +20,11 @@ import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { waitForSessionReady } from './helpers/store' +import { + hasCapturedGitStatusRetry, + installGitStatusRetryBarrier, + restoreGitStatusRetryHandler +} from './helpers/git-status-retry-barrier' import { createLargeFileCountRepo, removeLargeFileCountRepo, @@ -434,13 +439,17 @@ test.describe('Source Control large file count (#8013)', () => { ) expect(hugeState).not.toBeNull() - // Why: watcher refreshes stay parked while huge; the visible Retry is the - // explicit recovery path after the underlying change count drops. - removeLargeFileCountUntrackedTree(fixture.repoPath) - await expect(tooManyChangesBanner).toBeVisible() const retryButton = tooManyChangesBanner.locator('..').getByRole('button', { name: 'Retry' }) await expect(retryButton).toBeVisible() - await retryButton.click() + // Keep automatic refreshes from removing Retry before its real request starts. + await installGitStatusRetryBarrier(electronApp, fixture.repoPath) + try { + await retryButton.click() + await expect.poll(() => hasCapturedGitStatusRetry(electronApp)).toBe(true) + removeLargeFileCountUntrackedTree(fixture.repoPath) + } finally { + await restoreGitStatusRetryHandler(electronApp) + } await expect(tooManyChangesBanner).not.toBeVisible() await expect .poll(() => From 1c41d59203d1d68c30b85d3e5f8a86478e45aa40 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:02:20 -0700 Subject: [PATCH 13/43] perf(relay): drain fragmented frame buffers in linear time (#18891) * perf(relay): drain fragmented frame buffers in linear time * style: follow block-body lint in relay buffer checks --- .../scripts/relay-frame-buffer-benchmark.mjs | 62 +++++++++++++++++ src/shared/relay-frame-buffer.test.ts | 69 +++++++++++++++++++ src/shared/relay-frame-buffer.ts | 45 ++++++++---- 3 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 config/scripts/relay-frame-buffer-benchmark.mjs create mode 100644 src/shared/relay-frame-buffer.test.ts diff --git a/config/scripts/relay-frame-buffer-benchmark.mjs b/config/scripts/relay-frame-buffer-benchmark.mjs new file mode 100644 index 00000000000..24d7b565400 --- /dev/null +++ b/config/scripts/relay-frame-buffer-benchmark.mjs @@ -0,0 +1,62 @@ +#!/usr/bin/env node +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import { stripTypeScriptTypes } from 'node:module' +import { performance } from 'node:perf_hooks' + +// Pass the pre-change source saved with git show :src/shared/relay-frame-buffer.ts. +const baselinePath = process.argv[2] +if (!baselinePath) { + throw new Error('Usage: node config/scripts/relay-frame-buffer-benchmark.mjs ') +} +async function load(source) { + return ( + await import( + `data:text/javascript;base64,${Buffer.from(stripTypeScriptTypes(source)).toString('base64')}` + ) + ).RelayFrameBuffer +} +const Before = await load(readFileSync(baselinePath, 'utf8')) +const After = await load( + readFileSync(new URL('../../src/shared/relay-frame-buffer.ts', import.meta.url), 'utf8') +) +function median(values) { + return values.sort((a, b) => a - b)[Math.floor(values.length / 2)] +} +for (const count of [1, 256, 16384, 65536]) { + const chunks = Array.from({ length: count }, (_, index) => Buffer.alloc(64, index % 256)) + const expected = Buffer.concat(chunks) + for (const mode of ['take', 'discard']) { + const times = [[], []] + for (let round = 0; round < 9; round += 1) { + for (const arm of round % 2 === 0 ? [0, 1] : [1, 0]) { + const FrameBuffer = arm === 0 ? Before : After + const buffer = new FrameBuffer() + for (const chunk of chunks) { + buffer.append(chunk) + } + const start = performance.now() + const output = buffer[mode](expected.length) + times[arm].push(performance.now() - start) + if (mode === 'take') { + assert.deepEqual(output, expected) + } + assert.equal(buffer.length, 0) + buffer.append(Buffer.from('tail')) + assert.equal(buffer.drain().toString(), 'tail') + } + } + const beforeMs = median(times[0]), + afterMs = median(times[1]) + console.log( + JSON.stringify({ + mode, + chunks: count, + bytes: expected.length, + beforeMs, + afterMs, + speedup: beforeMs / afterMs + }) + ) + } +} diff --git a/src/shared/relay-frame-buffer.test.ts b/src/shared/relay-frame-buffer.test.ts new file mode 100644 index 00000000000..55dd744e57a --- /dev/null +++ b/src/shared/relay-frame-buffer.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi } from 'vitest' +import { RelayFrameBuffer } from './relay-frame-buffer' + +describe('RelayFrameBuffer', () => { + it('preserves a byte stream across fragmented peeks, takes, discards and drains', () => { + const buffer = new RelayFrameBuffer() + let expected = Buffer.alloc(0) + for (let step = 0; step < 5000; step += 1) { + const chunk = Buffer.from([step % 256, (step + 1) % 256, (step + 2) % 256]) + buffer.append(chunk) + expected = Buffer.concat([expected, chunk]) + if (step % 3 === 0) { + const count = Math.min(expected.length, 5) + expect(buffer.peek(count).subarray(0, count)).toEqual(expected.subarray(0, count)) + expect(buffer.take(count)).toEqual(expected.subarray(0, count)) + expected = expected.subarray(count) + } + if (step % 7 === 0) { + const count = Math.min(expected.length, 4) + buffer.discard(count) + expected = expected.subarray(count) + } + if (step % 101 === 0) { + expect(buffer.drain()).toEqual(expected) + expected = Buffer.alloc(0) + } + expect(buffer.length).toBe(expected.length) + } + expect(buffer.drain()).toEqual(expected) + expect(buffer.drain()).toEqual(Buffer.alloc(0)) + }) + + it('releases consumed references and amortizes storage compaction in a large backlog', () => { + const buffer = new RelayFrameBuffer() + const chunks = Array.from({ length: 32768 }, (_, index) => Buffer.from([index % 256])) + for (const chunk of chunks) { + buffer.append(chunk) + } + const shifted = vi.spyOn(Array.prototype, 'shift') + let shiftCount: number + try { + buffer.discard(16000) + shiftCount = shifted.mock.calls.length + } finally { + shifted.mockRestore() + } + expect(shiftCount).toBe(0) + const storage = buffer as unknown as { chunks: (Buffer | undefined)[]; head: number } + expect(storage.chunks.slice(0, storage.head).every((chunk) => chunk === undefined)).toBe(true) + expect(buffer.take(1000)).toEqual(Buffer.concat(chunks.slice(16000, 17000))) + expect(storage.chunks.length).toBeLessThan(chunks.length) + expect(buffer.drain()).toEqual(Buffer.concat(chunks.slice(17000))) + expect(storage.chunks).toHaveLength(0) + expect(buffer.length).toBe(0) + }) + + it('keeps single-chunk views and clears partial data before reuse', () => { + const buffer = new RelayFrameBuffer() + const chunk = Buffer.from('abcdef') + buffer.append(chunk) + expect(buffer.peek(2)).toBe(chunk) + const taken = buffer.take(2) + expect(taken.buffer).toBe(chunk.buffer) + expect(taken.toString()).toBe('ab') + buffer.clear() + buffer.append(Buffer.from('fresh')) + expect(buffer.drain().toString()).toBe('fresh') + }) +}) diff --git a/src/shared/relay-frame-buffer.ts b/src/shared/relay-frame-buffer.ts index 5083804f1e1..a851426c6d8 100644 --- a/src/shared/relay-frame-buffer.ts +++ b/src/shared/relay-frame-buffer.ts @@ -1,5 +1,6 @@ export class RelayFrameBuffer { - private chunks: Buffer[] = [] + private chunks: (Buffer | undefined)[] = [] + private head = 0 private bytes = 0 get length(): number { @@ -13,23 +14,28 @@ export class RelayFrameBuffer { clear(): void { this.chunks = [] + this.head = 0 this.bytes = 0 } drain(): Buffer { - const out = this.chunks.length === 1 ? this.chunks[0] : Buffer.concat(this.chunks, this.bytes) + const out = + this.chunks.length - this.head === 1 + ? this.chunks[this.head]! + : Buffer.concat(this.chunks.slice(this.head) as Buffer[], this.bytes) this.clear() return out } peek(count: number): Buffer { - const first = this.chunks[0] + const first = this.chunks[this.head]! if (first.length >= count) { return first } const out = Buffer.allocUnsafe(count) let copied = 0 - for (const part of this.chunks) { + for (let index = this.head; index < this.chunks.length; index += 1) { + const part = this.chunks[index]! copied += part.copy(out, copied, 0, Math.min(part.length, count - copied)) if (copied >= count) { break @@ -39,43 +45,56 @@ export class RelayFrameBuffer { } take(count: number): Buffer { - const first = this.chunks[0] + const first = this.chunks[this.head]! if (first.length === count) { - this.chunks.shift() + this.removeHead() this.bytes -= count return first } if (first.length > count) { - this.chunks[0] = first.subarray(count) + this.chunks[this.head] = first.subarray(count) this.bytes -= count return first.subarray(0, count) } const out = Buffer.allocUnsafe(count) let copied = 0 while (copied < count) { - const part = this.chunks[0] + const part = this.chunks[this.head]! const take = Math.min(part.length, count - copied) part.copy(out, copied, 0, take) copied += take if (take === part.length) { - this.chunks.shift() + this.removeHead() } else { - this.chunks[0] = part.subarray(take) + this.chunks[this.head] = part.subarray(take) } } this.bytes -= count return out } + private removeHead(): void { + this.chunks[this.head] = undefined + this.head += 1 + // Amortize compaction without retaining consumed buffers. + if ( + this.head === this.chunks.length || + (this.head >= 1024 && this.head * 2 >= this.chunks.length) + ) { + this.chunks = this.chunks.slice(this.head) + this.head = 0 + } + } + discard(count: number): void { let remaining = count while (remaining > 0) { - const part = this.chunks[0] + const part = this.chunks[this.head]! if (part.length <= remaining) { - this.chunks.shift() + this.removeHead() remaining -= part.length } else { - this.chunks[0] = part.subarray(remaining) + this.chunks[this.head] = part.subarray(remaining) remaining = 0 } } From bf87b1290f612fed83fb5e4bb514a0d2d0446b0b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:02:25 -0700 Subject: [PATCH 14/43] perf(repos): avoid quadratic icon source scans (#18892) * perf(repos): avoid quadratic icon source scans * perf: avoid repeated malformed HTML icon scans * bench: balance icon parser timing samples --- .../repo-icon-source-href-benchmark.mjs | 55 ++++++++++++++ src/main/repo-icon-file-detection.test.ts | 56 +++++++++++++- src/main/repo-icon-file-detection.ts | 10 +-- src/main/repo-icon-source-href.test.ts | 76 +++++++++++++++++++ src/main/repo-icon-source-href.ts | 46 +++++++++++ 5 files changed, 233 insertions(+), 10 deletions(-) create mode 100644 config/scripts/repo-icon-source-href-benchmark.mjs create mode 100644 src/main/repo-icon-source-href.test.ts create mode 100644 src/main/repo-icon-source-href.ts diff --git a/config/scripts/repo-icon-source-href-benchmark.mjs b/config/scripts/repo-icon-source-href-benchmark.mjs new file mode 100644 index 00000000000..76c42d261b4 --- /dev/null +++ b/config/scripts/repo-icon-source-href-benchmark.mjs @@ -0,0 +1,55 @@ +import assert from 'node:assert/strict' +import { performance } from 'node:perf_hooks' +import { extractIconHref } from '../../src/main/repo-icon-source-href.ts' + +// Original production expressions, preserved for the before/after measurement. +const html = + /]*\brel=["'](?:icon|shortcut icon)["'])(?=[^>]*\bhref=["']([^"'?]+))[^>]*>/i +const object = + /(?=[^}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^}]*\bhref\s*:\s*["']([^"'?]+))[^}]*/i +const original = (source) => source.match(html)?.[1] ?? source.match(object)?.[1] ?? null + +function measurePair(source) { + original(source) + extractIconHref(source) + const beforeSamples = [] + const afterSamples = [] + for (let run = 0; run < 5; run++) { + const measurements = [ + [original, beforeSamples], + [extractIconHref, afterSamples] + ] + if (run % 2 === 1) { + measurements.reverse() + } + for (const [fn, samples] of measurements) { + const started = performance.now() + fn(source) + samples.push(performance.now() - started) + } + } + return { + beforeMs: beforeSamples.sort((a, b) => a - b)[2], + afterMs: afterSamples.sort((a, b) => a - b)[2] + } +} + +const results = [] +for (const size of [8192, 16384, 32768]) { + for (const shape of ['no icon', 'rel without href', 'unterminated link starts']) { + const source = + shape === 'unterminated link starts' + ? ' { expect(stat).toHaveBeenCalled() }) }) + +describe('declared repo icons through production filesystem routes', () => { + it.each([ + ['local', false], + ['ssh', false], + ['local', true], + ['ssh', true] + ] as const)('preserves declared icon detection on %s (no icon: %s)', async (kind, noIcon) => { + const directory = await mkdtemp(join(tmpdir(), 'orca-icon-href-')) + const source = noIcon + ? 'a'.repeat(256 * 1024) + : `${'a'.repeat(32768)}{ rel: "icon", href: "/first.png", href: "/chosen.png" }` + try { + await mkdir(join(directory, 'public')) + await writeFile(join(directory, 'index.html'), source) + await writeFile(join(directory, 'public', 'chosen.png'), Buffer.from(PNG_BASE64, 'base64')) + const provider = remoteFilesystemProvider({ + stat: async (path) => { + const info = await stat(path) + return { + type: info.isFile() ? 'file' : 'directory', + size: info.size, + mtime: info.mtimeMs + } + }, + readFile: async (path) => { + const buffer = await readFile(path) + const isBinary = path.endsWith('.png') + return { + content: buffer.toString(isBinary ? 'base64' : 'utf8'), + isBinary, + mimeType: isBinary ? 'image/png' : 'text/html' + } + } + }) + const route: ExecutionHostFilesystemRoute = + kind === 'local' ? { kind: 'local', hostId: 'local' } : sshRoute('icon-oracle', provider) + await expect(detectRepoFileIcon(directory, route)).resolves.toEqual( + noIcon + ? null + : { + type: 'image', + src: `data:image/png;base64,${PNG_BASE64}`, + source: 'file', + label: 'public/chosen.png' + } + ) + } finally { + await rm(directory, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/repo-icon-file-detection.ts b/src/main/repo-icon-file-detection.ts index f4289924b08..831248b94b3 100644 --- a/src/main/repo-icon-file-detection.ts +++ b/src/main/repo-icon-file-detection.ts @@ -3,6 +3,7 @@ import { buildImageDataUri } from '../shared/image-data-uri' import { MAX_REPO_ICON_UPLOAD_BYTES, type RepoIcon } from '../shared/repo-icon' import type { ExecutionHostFilesystemRoute } from './providers/execution-host-provider-dispatch' import type { IFilesystemProvider } from './providers/types' +import { extractIconHref } from './repo-icon-source-href' import { iconHrefCandidates } from './repo-icon-href-candidates' import { joinWorktreeRelativePath } from './runtime/runtime-relative-paths' @@ -49,11 +50,6 @@ const REPO_ICON_SOURCE_FILE_CANDIDATES = [ // not read large app entrypoints just to find a small favicon href. const MAX_REPO_ICON_SOURCE_BYTES = 256 * 1024 -const LINK_ICON_HTML_RE = - /]*\brel=["'](?:icon|shortcut icon)["'])(?=[^>]*\bhref=["']([^"'?]+))[^>]*>/i -const LINK_ICON_OBJECT_RE = - /(?=[^}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^}]*\bhref\s*:\s*["']([^"'?]+))[^}]*/i - type DetectedImageFormat = { mimeType: 'image/png' | 'image/webp' } @@ -98,10 +94,6 @@ function detectImageFormat(buffer: Buffer): DetectedImageFormat | null { return null } -function extractIconHref(source: string): string | null { - return source.match(LINK_ICON_HTML_RE)?.[1] ?? source.match(LINK_ICON_OBJECT_RE)?.[1] ?? null -} - function repoIconFromImageBuffer(buffer: Buffer, relativePath: string): RepoIcon | null { const format = detectImageFormat(buffer) if (!format) { diff --git a/src/main/repo-icon-source-href.test.ts b/src/main/repo-icon-source-href.test.ts new file mode 100644 index 00000000000..7c74511f3d6 --- /dev/null +++ b/src/main/repo-icon-source-href.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest' +import { extractIconHref } from './repo-icon-source-href' + +// Original production expressions are the compatibility oracle. +const HTML_RE = + /]*\brel=["'](?:icon|shortcut icon)["'])(?=[^>]*\bhref=["']([^"'?]+))[^>]*>/i +const OBJECT_RE = + /(?=[^}]*\brel\s*:\s*["'](?:icon|shortcut icon)["'])(?=[^}]*\bhref\s*:\s*["']([^"'?]+))[^}]*/i + +export function originalIconHref(source: string): string | null { + return source.match(HTML_RE)?.[1] ?? source.match(OBJECT_RE)?.[1] ?? null +} + +describe('repo icon source href compatibility', () => { + it.each([ + '

Use `Array` and bold.

'], + ...[2048, 8192, 16384].map((length) => [ + `${length} underscore collision`, + `\uE000ORCA_MD_CODE_${'_'.repeat(length)}0\uE000 and \`Array\`` + ]) +]) { + assert.equal(after(input), before(input)) + results.push({ + shape, + bytes: Buffer.byteLength(input), + beforeMs: measure(before, input, 5), + afterMs: measure(after, input, 15) + }) +} +console.log(JSON.stringify({ node: process.version, platform: process.platform, results }, null, 2)) diff --git a/mobile/src/components/mobile-markdown-preview-html.ts b/mobile/src/components/mobile-markdown-preview-html.ts index 8bb5fca0934..a5866b171e6 100644 --- a/mobile/src/components/mobile-markdown-preview-html.ts +++ b/mobile/src/components/mobile-markdown-preview-html.ts @@ -204,11 +204,18 @@ function escapeRegExp(value: string): string { } function codePlaceholderPrefix(content: string): string { - let prefix = CODE_PLACEHOLDER_PREFIX_BASE - while (content.includes(prefix)) { - prefix = `${prefix}_` + let suffixLength = 0 + let cursor = 0 + while ((cursor = content.indexOf(CODE_PLACEHOLDER_PREFIX_BASE, cursor)) !== -1) { + cursor += CODE_PLACEHOLDER_PREFIX_BASE.length + const suffixStart = cursor + while (content[cursor] === '_') { + cursor += 1 + } + // One extra underscore keeps the prefix longer than every authored run. + suffixLength = Math.max(suffixLength, cursor - suffixStart + 1) } - return prefix + return CODE_PLACEHOLDER_PREFIX_BASE + '_'.repeat(suffixLength) } function protectMarkdownCode(content: string): { diff --git a/mobile/src/components/mobile-markdown-preview-placeholder.test.ts b/mobile/src/components/mobile-markdown-preview-placeholder.test.ts new file mode 100644 index 00000000000..f751608af78 --- /dev/null +++ b/mobile/src/components/mobile-markdown-preview-placeholder.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { normalizeMobileMarkdownPreviewHtml } from './mobile-markdown-preview-html' + +const marker = '\uE000ORCA_MD_CODE_' +const suffix = '\uE000' + +describe('mobile Markdown code placeholder collisions', () => { + it.each([0, 1, 2, 15, 128, 16384])('preserves a literal marker with %i underscores', (length) => { + const literal = `${marker}${'_'.repeat(length)}0${suffix}` + const input = `${literal} and \`Array\`\n\n\`\`\`html\n

literal

\n\`\`\`` + expect(normalizeMobileMarkdownPreviewHtml(input)).toBe(input) + }) + + it('handles adjacent markers and repeated maximum suffixes', () => { + const literal = `${marker}${marker}__0${suffix}${marker}__1${suffix}${marker}_2${suffix}` + expect(normalizeMobileMarkdownPreviewHtml(`

${literal} and \`

\`

`)).toBe( + `${literal} and \`
\`` + ) + }) + + it('preserves authored markers across generated suffix orders and HTML islands', () => { + let seed = 173 + for (let sample = 0; sample < 500; sample++) { + const literals: string[] = [] + for (let index = 0; index < 8; index++) { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0 + literals.push(`${marker}${'_'.repeat(seed % 32)}${index}${suffix}`) + } + const text = literals.join(' ') + ' and `Array`' + expect(normalizeMobileMarkdownPreviewHtml(`

${text}

`)).toBe(text) + } + }) +}) From 4204bdf7172f9e2a822cbfde7349a8dc63b2f4ae Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:02:57 -0700 Subject: [PATCH 21/43] perf: avoid repeated Quick Open exclusion string allocations (#18916) --- .../quick-open-exclusion-benchmark.mjs | 60 +++++++++++++++++++ src/shared/quick-open-filter.test.ts | 37 ++++++++++++ src/shared/quick-open-filter.ts | 2 +- 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 config/scripts/quick-open-exclusion-benchmark.mjs diff --git a/config/scripts/quick-open-exclusion-benchmark.mjs b/config/scripts/quick-open-exclusion-benchmark.mjs new file mode 100644 index 00000000000..399302c5a2b --- /dev/null +++ b/config/scripts/quick-open-exclusion-benchmark.mjs @@ -0,0 +1,60 @@ +import assert from 'node:assert/strict' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' + +const bundled = await build({ + entryPoints: ['src/shared/quick-open-filter.ts'], + bundle: true, + platform: 'node', + format: 'esm', + write: false, + logLevel: 'silent' +}) +const { shouldExcludeQuickOpenRelPath: after } = await import( + `data:text/javascript;base64,${Buffer.from(bundled.outputFiles[0].text).toString('base64')}` +) +// Original production predicate, including its exact boundary check. +function before(relPath, prefixes) { + for (const prefix of prefixes) { + if (relPath === prefix) { + return true + } + if (relPath.length > prefix.length && relPath.startsWith(`${prefix}/`)) { + return true + } + } + return false +} +const files = Array.from( + { length: 100000 }, + (_, index) => `src/components/group-${index % 100}/file-${index}.tsx` +) +function run(fn, prefixes) { + let excluded = 0 + for (const file of files) { + excluded += Number(fn(file, prefixes)) + } + return excluded +} +function measure(fn, prefixes) { + run(fn, prefixes) + const samples = [] + for (let index = 0; index < 5; index++) { + const start = performance.now() + run(fn, prefixes) + samples.push(performance.now() - start) + } + return samples.sort((a, b) => a - b)[2] +} +const results = [] +for (const count of [0, 10, 100, 500]) { + const prefixes = Array.from({ length: count }, (_, index) => `nested-worktrees/worktree-${index}`) + assert.equal(run(after, prefixes), run(before, prefixes)) + results.push({ + files: files.length, + exclusions: count, + beforeMs: measure(before, prefixes), + afterMs: measure(after, prefixes) + }) +} +console.log(JSON.stringify({ node: process.version, platform: process.platform, results }, null, 2)) diff --git a/src/shared/quick-open-filter.test.ts b/src/shared/quick-open-filter.test.ts index 1d96bc1744f..c481a1e8854 100644 --- a/src/shared/quick-open-filter.test.ts +++ b/src/shared/quick-open-filter.test.ts @@ -102,6 +102,43 @@ describe('buildExcludePathPrefixes', () => { }) describe('shouldExcludeQuickOpenRelPath', () => { + it('matches the original filter across boundary and Unicode path combinations', () => { + const paths = [ + '', + '/', + 'a', + 'a/', + 'a//', + 'ab', + 'a/b', + 'a\\b', + 'A/b', + '界/😀', + '界/😀x', + 'a[1]/x', + 'a./x' + ] + for (const prefix of paths) { + for (const relPath of paths) { + const expected = + relPath === prefix || (relPath.length > prefix.length && relPath.startsWith(`${prefix}/`)) + expect(shouldExcludeQuickOpenRelPath(relPath, [prefix])).toBe(expected) + } + } + }) + + it('preserves normalized Windows and UNC exclusion boundaries', () => { + for (const [root, excluded] of [ + ['C:\\Repo', 'C:\\Repo\\trees\\one'], + ['\\\\server\\share\\repo', '\\\\server\\share\\repo\\trees\\one'] + ]) { + const prefixes = buildExcludePathPrefixes(root, [excluded]) + expect(prefixes).toEqual(['trees/one']) + expect(shouldExcludeQuickOpenRelPath('trees/one/file.ts', prefixes)).toBe(true) + expect(shouldExcludeQuickOpenRelPath('trees/one-more/file.ts', prefixes)).toBe(false) + } + }) + it('matches exact and boundary paths only', () => { expect(shouldExcludeQuickOpenRelPath('packages/app', ['packages/app'])).toBe(true) expect(shouldExcludeQuickOpenRelPath('packages/app/x.ts', ['packages/app'])).toBe(true) diff --git a/src/shared/quick-open-filter.ts b/src/shared/quick-open-filter.ts index 9d5bebd80b3..b7592657a11 100644 --- a/src/shared/quick-open-filter.ts +++ b/src/shared/quick-open-filter.ts @@ -119,7 +119,7 @@ export function shouldExcludeQuickOpenRelPath( if (relPath === prefix) { return true } - if (relPath.length > prefix.length && relPath.startsWith(`${prefix}/`)) { + if (relPath[prefix.length] === '/' && relPath.startsWith(prefix)) { return true } } From 295684dc6d4b5d1f777e9fdd65ef614b1d089d5c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:02 -0700 Subject: [PATCH 22/43] perf: skip unrelated shared symlink probes during Git status (#18918) --- src/main/git/source-control/status-read.ts | 17 +++- .../git/status-symlink-probe-budget.test.ts | 81 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 src/main/git/status-symlink-probe-budget.test.ts diff --git a/src/main/git/source-control/status-read.ts b/src/main/git/source-control/status-read.ts index 276bf26e646..1ec81694d33 100644 --- a/src/main/git/source-control/status-read.ts +++ b/src/main/git/source-control/status-read.ts @@ -14,7 +14,10 @@ import { } from '../../../shared/git-status-line-stats-cache' import { resolveWorktreeHostPath } from '../../../shared/git-metadata-path' import { gitOptionalLocksDisabledEnv, gitStreamStdout } from '../runner' -import { findExistingWorktreeSymlinkPaths } from '../worktree-symlink-detection' +import { + findExistingWorktreeSymlinkPaths, + getSafeRelativePath +} from '../worktree-symlink-detection' import type { GetStatusOptions } from './get-status-options' import { statusReadLeaseOwner } from './git-read-cache-invalidation' import { detectConflictOperation } from './git-conflict-operation' @@ -89,8 +92,18 @@ async function dropSharedSymlinkUntrackedEntries( if (sharedLinkPaths.length === 0 || !entries.some((entry) => entry.area === 'untracked')) { return } + const untrackedPaths = new Set( + entries.filter((entry) => entry.area === 'untracked').map((entry) => entry.path) + ) + const candidatePaths = sharedLinkPaths.filter((rawPath) => { + const path = getSafeRelativePath(rawPath) + return path.safe && untrackedPaths.has(path.rel) + }) + if (candidatePaths.length === 0) { + return + } const sharedLinks = new Set( - await findExistingWorktreeSymlinkPaths(worktreePath, sharedLinkPaths, { + await findExistingWorktreeSymlinkPaths(worktreePath, candidatePaths, { wslDistro: options.wslDistro }) ) diff --git a/src/main/git/status-symlink-probe-budget.test.ts b/src/main/git/status-symlink-probe-budget.test.ts new file mode 100644 index 00000000000..b312048078b --- /dev/null +++ b/src/main/git/status-symlink-probe-budget.test.ts @@ -0,0 +1,81 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resolve } from 'node:path' +import { getStatus } from './source-control/status-read' + +const { lstat, stream, conflict } = vi.hoisted(() => ({ + lstat: vi.fn(), + stream: vi.fn(), + conflict: vi.fn() +})) +vi.mock('node:fs/promises', () => ({ lstat })) +vi.mock('./source-control/git-conflict-operation', () => ({ detectConflictOperation: conflict })) +vi.mock('./runner', () => ({ + gitStreamStdout: stream, + gitOptionalLocksDisabledEnv: () => ({ GIT_OPTIONAL_LOCKS: '0' }) +})) + +beforeEach(() => { + vi.clearAllMocks() + conflict.mockResolvedValue(undefined) + lstat.mockResolvedValue({ isSymbolicLink: () => true }) + stream.mockImplementation(async (_args, options) => { + options.onStdout('? unrelated.txt\n') + return { stoppedEarly: false } + }) +}) + +function status(sharedLinkPaths: string[]) { + return getStatus('/repo', { sharedLinkPaths, includeLineStats: false }) +} + +describe('status shared symlink probe budget', () => { + it.each([1, 8, 32])( + 'does no unrelated symlink probes for %i configured paths over 100 refreshes', + async (count) => { + const paths = Array.from({ length: count }, (_, index) => `shared-${index}`) + for (let refresh = 0; refresh < 100; refresh++) { + expect((await status(paths)).entries).toEqual([ + { path: 'unrelated.txt', status: 'untracked', area: 'untracked' } + ]) + } + expect(lstat).not.toHaveBeenCalled() + expect(stream).toHaveBeenCalledTimes(100) + } + ) + + it('probes matching normalized paths, retaining duplicate probes and original order', async () => { + stream.mockImplementation(async (_args, options) => { + options.onStdout('? link\n? 日本 語\n? unrelated.txt\n') + return { stoppedEarly: false } + }) + const result = await status(['absent', ' /link ', '\\日本 語', 'link', '../link', 'C:link']) + expect(lstat.mock.calls.map(([path]) => path)).toEqual([ + resolve('/repo', 'link'), + resolve('/repo', '日本 語'), + resolve('/repo', 'link') + ]) + expect(result.entries.map((entry) => entry.path)).toEqual(['unrelated.txt']) + }) + + it('rechecks matching paths after the filesystem changes and preserves unreadable paths', async () => { + const paths = ['unrelated.txt'] + expect((await status(paths)).entries).toEqual([]) + lstat.mockResolvedValueOnce({ isSymbolicLink: () => false }) + expect((await status(paths)).entries).toHaveLength(1) + lstat.mockRejectedValueOnce(new Error('EACCES')) + expect((await status(paths)).entries).toHaveLength(1) + expect(lstat).toHaveBeenCalledTimes(3) + }) + + it('does not broaden exact path matching to descendants or case variants', async () => { + stream.mockImplementation(async (_args, options) => { + options.onStdout('? link/child\n? LINK\n') + return { stoppedEarly: false } + }) + expect((await status(['link'])).entries.map((entry) => entry.path)).toEqual([ + 'link/child', + 'LINK' + ]) + expect(lstat).not.toHaveBeenCalled() + }) +}) From 4e8e14424d108caa3e38923a80cfce16587f5970 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:08 -0700 Subject: [PATCH 23/43] perf: avoid splitting every path during file autocomplete (#18919) --- .../scripts/mobile-file-ranking-benchmark.mjs | 53 +++++++++++++++++++ .../mobile-native-chat-autocomplete.ts | 2 +- .../runtime-mobile-file-path-search.test.ts | 31 +++++++++++ .../runtime-mobile-file-path-search.ts | 2 +- 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 config/scripts/mobile-file-ranking-benchmark.mjs diff --git a/config/scripts/mobile-file-ranking-benchmark.mjs b/config/scripts/mobile-file-ranking-benchmark.mjs new file mode 100644 index 00000000000..68ac5b9d977 --- /dev/null +++ b/config/scripts/mobile-file-ranking-benchmark.mjs @@ -0,0 +1,53 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { stripTypeScriptTypes } from 'node:module' +import { performance } from 'node:perf_hooks' + +const baseline = process.argv[2] +if (!baseline) { + throw new Error('Usage: node config/scripts/mobile-file-ranking-benchmark.mjs ') +} +async function load(source) { + const js = stripTypeScriptTypes(source, { mode: 'transform' }) + return await import(`data:text/javascript;base64,${Buffer.from(js).toString('base64')}`) +} +function measure(fn, paths, query) { + for (let warmup = 0; warmup < 10; warmup++) { + fn(paths, query, 16) + } + const samples = [] + for (let i = 0; i < 9; i++) { + const start = performance.now() + fn(paths, query, 16) + samples.push(performance.now() - start) + } + return samples.sort((a, b) => a - b)[4] +} +const results = [] +for (const [file, name] of [ + ['src/main/runtime/runtime-mobile-file-path-search.ts', 'rankRuntimeMobileFilePaths'], + ['mobile/src/session/mobile-native-chat-autocomplete.ts', 'rankSuggestions'] +]) { + const before = ( + await load(execFileSync('git', ['show', `${baseline}:${file}`], { encoding: 'utf8' })) + )[name] + const after = (await load(readFileSync(file, 'utf8')))[name] + for (const count of [100, 100000]) { + const paths = Array.from( + { length: count }, + (_, i) => `src/components/workspace/group-${i % 100}/file-${i}.tsx` + ) + for (const query of ['file-9', 'missing', 'workspace']) { + assert.deepEqual(after(paths, query, 16), before(paths, query, 16)) + results.push({ + function: name, + paths: count, + query, + beforeMs: measure(before, paths, query), + afterMs: measure(after, paths, query) + }) + } + } +} +console.log(JSON.stringify({ node: process.version, platform: process.platform, results }, null, 2)) diff --git a/mobile/src/session/mobile-native-chat-autocomplete.ts b/mobile/src/session/mobile-native-chat-autocomplete.ts index 548d0614837..8de107c9fb0 100644 --- a/mobile/src/session/mobile-native-chat-autocomplete.ts +++ b/mobile/src/session/mobile-native-chat-autocomplete.ts @@ -81,7 +81,7 @@ export function rankSuggestions(candidates: readonly string[], query: string, li const substring: string[] = [] for (const candidate of candidates) { const lower = candidate.toLowerCase() - const base = lower.split('/').pop() ?? lower + const base = lower.slice(lower.lastIndexOf('/') + 1) if (lower.startsWith(q) || base.startsWith(q)) { prefix.push(candidate) } else if (lower.includes(q)) { diff --git a/src/main/runtime/runtime-mobile-file-path-search.test.ts b/src/main/runtime/runtime-mobile-file-path-search.test.ts index 495a7933842..0be5ecad50e 100644 --- a/src/main/runtime/runtime-mobile-file-path-search.test.ts +++ b/src/main/runtime/runtime-mobile-file-path-search.test.ts @@ -6,6 +6,37 @@ import { } from './runtime-mobile-file-path-search' describe('rankRuntimeMobileFilePaths', () => { + it('preserves basename matching, ordering and total counts for unusual paths', () => { + const paths = [ + '', + '/', + 'a/', + 'a//b.ts', + 'b.ts', + 'B.TS', + 'a\\b.ts', + '界/😀.ts', + '.hidden', + 'a/./b.ts' + ] + for (const query of ['', ' ', 'b', '.ts', '😀', '/', 'a\\', 'missing']) { + for (const limit of [0, 1, 3, 100]) { + const q = query.trim().toLowerCase() + const prefix = paths.filter((path) => { + const lower = path.toLowerCase() + return lower.startsWith(q) || (lower.split('/').pop() ?? lower).startsWith(q) + }) + const other = paths.filter( + (path) => !prefix.includes(path) && path.toLowerCase().includes(q) + ) + expect(rankRuntimeMobileFilePaths(paths, query, limit)).toEqual({ + paths: [...prefix, ...other].slice(0, limit), + totalCount: prefix.length + other.length + }) + } + } + }) + it('ranks path and basename prefixes before substrings and caps output', () => { expect( rankRuntimeMobileFilePaths( diff --git a/src/main/runtime/runtime-mobile-file-path-search.ts b/src/main/runtime/runtime-mobile-file-path-search.ts index 13213d6dfcf..1455028c798 100644 --- a/src/main/runtime/runtime-mobile-file-path-search.ts +++ b/src/main/runtime/runtime-mobile-file-path-search.ts @@ -76,7 +76,7 @@ export function rankRuntimeMobileFilePaths( let totalCount = 0 for (const path of paths) { const lower = path.toLowerCase() - const basename = lower.split('/').pop() ?? lower + const basename = lower.slice(lower.lastIndexOf('/') + 1) if (lower.startsWith(normalizedQuery) || basename.startsWith(normalizedQuery)) { totalCount++ if (prefix.length < limit) { From 388e9fb77667d46e5732201bb8000da23d145cbf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:14 -0700 Subject: [PATCH 24/43] perf: avoid rescanning emitted source in analysis guards (#18920) --- .../source-string-blanking-benchmark.mjs | 79 +++++++++++++++++++ .../source-scan/source-tree-scan.test.ts | 7 ++ src/shared/source-scan/source-tree-scan.ts | 13 ++- 3 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 config/scripts/source-string-blanking-benchmark.mjs diff --git a/config/scripts/source-string-blanking-benchmark.mjs b/config/scripts/source-string-blanking-benchmark.mjs new file mode 100644 index 00000000000..de677b9baa9 --- /dev/null +++ b/config/scripts/source-string-blanking-benchmark.mjs @@ -0,0 +1,79 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { stripTypeScriptTypes } from 'node:module' +import { performance } from 'node:perf_hooks' +import { blankStringContents as after } from '../../src/shared/source-scan/source-tree-scan.ts' + +const ref = process.argv[2] +if (!ref) { + throw new Error('Usage: node config/scripts/source-string-blanking-benchmark.mjs ') +} +const source = execFileSync('git', ['show', `${ref}:src/shared/source-scan/source-tree-scan.ts`], { + encoding: 'utf8' +}) +const { blankStringContents: before } = await import( + `data:text/javascript;base64,${Buffer.from(stripTypeScriptTypes(source)).toString('base64')}` +) +const tokens = [ + 'a', + '/', + '*', + ' ', + '\n', + '\r', + '\t', + '\u00a0', + '\u2028', + '"', + "'", + '`', + '${', + '}', + '{', + '\\', + '(', + ')', + '[', + ']', + '=', + '+', + '-', + ';' +] +let seed = 173 +for (let sample = 0; sample < 3000; sample++) { + let input = '' + for (let token = 0; token < 40; token++) { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0 + input += tokens[seed % tokens.length] + } + assert.equal(after(input), before(input), JSON.stringify(input)) + assert.equal(after(input, true), before(input, true), JSON.stringify(input)) +} +function measure(fn, input) { + const samples = [] + for (let run = 0; run < 3; run++) { + const start = performance.now() + fn(input) + samples.push(performance.now() - start) + } + return samples.sort((a, b) => a - b)[1] +} +const results = [] +for (const lines of [100, 1000, 5000, 10000]) { + const input = 'const x = value / 2;\n'.repeat(lines) + assert.equal(after(input), before(input)) + results.push({ + lines, + bytes: Buffer.byteLength(input), + beforeMs: measure(before, input), + afterMs: measure(after, input) + }) +} +console.log( + JSON.stringify( + { node: process.version, platform: process.platform, differentialCases: 3000, results }, + null, + 2 + ) +) diff --git a/src/shared/source-scan/source-tree-scan.test.ts b/src/shared/source-scan/source-tree-scan.test.ts index be4b72e4ce9..07ddb52a845 100644 --- a/src/shared/source-scan/source-tree-scan.test.ts +++ b/src/shared/source-scan/source-tree-scan.test.ts @@ -47,6 +47,13 @@ describe('stripComments', () => { }) describe('blankStringContents', () => { + it('does not rescan the accumulated source for each division operator', () => { + const source = 'const x = value / 2;\n'.repeat(10000) + const started = performance.now() + expect(blankStringContents(source)).toBe(source) + expect(performance.now() - started).toBeLessThan(200) + }) + it('neutralises parentheses inside a string so a call is matched whole', () => { // A shell script embedded as a string closed the call early, so the options // object fell outside the match and its flags read as absent. diff --git a/src/shared/source-scan/source-tree-scan.ts b/src/shared/source-scan/source-tree-scan.ts index dce7ae1a274..937a2fc39db 100644 --- a/src/shared/source-scan/source-tree-scan.ts +++ b/src/shared/source-scan/source-tree-scan.ts @@ -179,8 +179,7 @@ export function blankStringContentsDesynced(source: string): boolean { * each also has a prefix reading: `!` (non-null assertion vs `!/re/.test(x)`), * `+` `-` `*` `%` `^` `~` (postfix `--`/`++`), and `>` `}` (JSX close). */ -function startsRegexLiteral(emitted: string): boolean { - const prev = emitted.replace(/\s+$/, '').at(-1) +function startsRegexLiteral(prev: string | undefined): boolean { return prev === undefined || '(,=:[&|?;'.includes(prev) } @@ -209,6 +208,7 @@ function findRegexLiteralEnd(source: string, start: number): number { export function blankStringContents(source: string, reportDesync = false): string { let out = '' + let lastSignificantChar: string | undefined let index = 0 let quote: string | null = null // Brace depth per interpolation, so a `}` inside `${ { a: 1 } }` does not @@ -220,6 +220,7 @@ export function blankStringContents(source: string, reportDesync = false): strin templates.push(0) quote = null out += '${' + lastSignificantChar = '{' index += 2 continue } @@ -232,6 +233,7 @@ export function blankStringContents(source: string, reportDesync = false): strin templates.pop() quote = '`' out += char + lastSignificantChar = char index += 1 continue } @@ -256,6 +258,7 @@ export function blankStringContents(source: string, reportDesync = false): strin if (char === quote) { quote = null out += char + lastSignificantChar = char } else { out += char === '\n' ? char : ' ' } @@ -270,10 +273,11 @@ export function blankStringContents(source: string, reportDesync = false): strin // comments first, but this runs standalone too, and at index 0 a file // starting with a banner comment read as one giant regex. const next = source[index + 1] - if (char === '/' && next !== '/' && next !== '*' && startsRegexLiteral(out)) { + if (char === '/' && next !== '/' && next !== '*' && startsRegexLiteral(lastSignificantChar)) { const end = findRegexLiteralEnd(source, index) if (end !== -1) { out += `/${' '.repeat(end - index - 1)}` + lastSignificantChar = '/' index = end continue } @@ -282,6 +286,9 @@ export function blankStringContents(source: string, reportDesync = false): strin quote = char } out += char + if (/\S/.test(char)) { + lastSignificantChar = char + } index += 1 } if (reportDesync) { From fb7b75d55dc57a4b6c5ce9437869e6505f63178a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:19 -0700 Subject: [PATCH 25/43] perf(cli): skip feature formatters during help and error startup (#18923) * perf(cli): load error reporting without feature formatters * test(cli): follow extracted error reporter in import guard * chore(cli): track cli-error.ts in deferral equivalence baseline The equivalence script restores TOUCHED files from the baseline rev to rebuild the pre-deferral CLI. reportCliError/formatCliError moved from format.ts into cli-error.ts, so the baseline arm must also drop cli-error.ts (absent at older revs) or the old tree would still compile against the new module. --- .../scripts/benchmark-cli-error-imports.mjs | 121 ++++++++++++++ ...li-runtime-client-deferral-equivalence.mjs | 23 ++- src/cli/cli-error.ts | 144 +++++++++++++++++ src/cli/format.ts | 147 +----------------- src/cli/index.ts | 2 +- src/cli/runtime-client-deferral.test.ts | 7 +- 6 files changed, 289 insertions(+), 155 deletions(-) create mode 100644 config/scripts/benchmark-cli-error-imports.mjs create mode 100644 src/cli/cli-error.ts diff --git a/config/scripts/benchmark-cli-error-imports.mjs b/config/scripts/benchmark-cli-error-imports.mjs new file mode 100644 index 00000000000..a4648f84aec --- /dev/null +++ b/config/scripts/benchmark-cli-error-imports.mjs @@ -0,0 +1,121 @@ +import assert from 'node:assert/strict' +import { createRequire } from 'node:module' +import { existsSync, realpathSync } from 'node:fs' +import { delimiter, join, resolve } from 'node:path' + +// Emit each revision with tsc -p config/tsconfig.cli.json --outDir --composite false --incremental false. +// Run: node config/scripts/benchmark-cli-error-imports.mjs +const [beforeDir, afterDir] = process.argv.slice(2) +assert.ok(beforeDir && afterDir, 'Pass distinct before and after TypeScript output directories.') +assert.notEqual( + realpathSync(beforeDir), + realpathSync(afterDir), + 'Do not compare a build to itself.' +) +const entries = { + before: join(resolve(beforeDir), 'cli', 'index.js'), + after: join(resolve(afterDir), 'cli', 'index.js') +} +for (const entry of Object.values(entries)) { + assert.ok(existsSync(entry), `Missing emitted CLI: ${entry}`) +} + +const { runProcessSync } = createRequire(import.meta.url)( + join(resolve(afterDir), 'shared', 'child-process', 'run-process.js') +) + +const child = String.raw` + const { performance } = require('node:perf_hooks') + const { writeSync } = require('node:fs') + const { createHash } = require('node:crypto') + const { basename } = require('node:path') + let stdout = '', stderr = '' + process.stdout.write = (text) => { stdout += text; return true } + process.stderr.write = (text) => { stderr += text; return true } + const started = performance.now() + const cli = require(process.argv[1]) + const importMs = performance.now() - started + cli.main(JSON.parse(process.argv[2])).then(() => { + const totalMs = performance.now() - started + const modules = Object.keys(require.cache) + writeSync(1, JSON.stringify({ + importMs, totalMs, modules: modules.length, + featureFormatters: modules.filter((file) => ['browser', 'terminal', 'project', 'automation', 'workspace', 'computer'].some((name) => basename(file) === name + '-format.js')), + stdout: createHash('sha256').update(stdout).digest('hex'), + stderr: createHash('sha256').update(stderr).digest('hex'), + exitCode: process.exitCode || 0 + })) + process.exitCode = 0 + }).catch((error) => { writeSync(2, String(error)); process.exitCode = 1 }) +` +const cases = [ + ['--help'], + ['help', 'terminal', 'read'], + ['does-not-exist'], + ['computer', 'click', '--does-not-exist'], + ['does-not-exist', '--json'] +] +const median = (values) => [...values].sort((a, b) => a - b)[Math.floor(values.length / 2)] +const summarize = (samples) => ({ + importMs: median(samples.map((sample) => sample.importMs)), + totalMs: median(samples.map((sample) => sample.totalMs)), + modules: samples[0].modules +}) +const rows = [] +for (const args of cases) { + const samples = { before: [], after: [] } + let expected + for (let run = 0; run < 22; run++) { + for (const variant of run % 2 ? ['after', 'before'] : ['before', 'after']) { + const result = runProcessSync({ + program: process.execPath, + args: ['-e', child, entries[variant], JSON.stringify(args)], + timeoutMs: 30_000, + env: { + ...process.env, + NODE_PATH: [resolve('node_modules'), process.env.NODE_PATH] + .filter(Boolean) + .join(delimiter) + } + }) + assert.equal(result.timedOut, false, 'CLI child timed out.') + assert.equal(result.code, 0, result.stderr) + const sample = JSON.parse(result.stdout) + const output = { stdout: sample.stdout, stderr: sample.stderr, exitCode: sample.exitCode } + expected ??= output + assert.deepEqual(output, expected, `${variant} output changed for ${args.join(' ')}`) + if (variant === 'after') { + assert.deepEqual( + sample.featureFormatters, + [], + 'Help and syntax errors must skip feature formatters.' + ) + } + if (run >= 2) { + samples[variant].push(sample) + } + } + } + assert.ok(samples.after[0].modules < samples.before[0].modules, 'Expected fewer loaded modules.') + rows.push({ + args, + before: summarize(samples.before), + after: summarize(samples.after), + output: expected, + samples + }) +} +console.log( + JSON.stringify( + { + node: process.version, + platform: process.platform, + measurement: + 'Fresh-process import + main; excludes process creation; warmed filesystem; 2 warmups and 20 samples per variant, alternating order.', + entries, + rows + }, + null, + 2 + ) +) diff --git a/config/scripts/cli-runtime-client-deferral-equivalence.mjs b/config/scripts/cli-runtime-client-deferral-equivalence.mjs index f443bf3b9f9..a231b5ba75a 100644 --- a/config/scripts/cli-runtime-client-deferral-equivalence.mjs +++ b/config/scripts/cli-runtime-client-deferral-equivalence.mjs @@ -2,7 +2,7 @@ // Equivalence check for deferring the RuntimeClient module graph in the CLI. // // Builds the CLI twice with the REAL tsc emit — once from the working tree and -// once with the seven touched files restored from git HEAD~ (the pre-deferral +// once with the touched files restored from git HEAD~ (the pre-deferral // implementation) — then compares stdout, stderr and exit code BYTE FOR BYTE // across a matrix of invocations. // @@ -13,7 +13,7 @@ // // Usage: node config/scripts/cli-runtime-client-deferral-equivalence.mjs [--baseline ] import { execFileSync, spawnSync } from 'node:child_process' -import { mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs' import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' @@ -21,8 +21,11 @@ const REPO = fileURLToPath(new URL('../..', import.meta.url)) // The files this change touches. Restoring exactly these from the baseline rev // reconstructs the old implementation without disturbing anything else. +// Files absent at the baseline (e.g. cli-error.ts, split out of format.ts +// later) are removed for the baseline build and put back afterwards. const TOUCHED = [ 'src/cli/args.ts', + 'src/cli/cli-error.ts', 'src/cli/dispatch.ts', 'src/cli/flags.ts', 'src/cli/format.ts', @@ -72,12 +75,16 @@ function buildTree(label, baselineRev) { if (baselineRev) { for (const file of TOUCHED) { const path = join(REPO, file) - restored.push([path, readFileSync(path)]) - const old = execFileSync('git', ['show', `${baselineRev}:${file}`], { + restored.push([path, existsSync(path) ? readFileSync(path) : null]) + const old = spawnSync('git', ['show', `${baselineRev}:${file}`], { cwd: REPO, maxBuffer: 64 * 1024 * 1024 }) - writeFileSync(path, old) + if (old.status === 0) { + writeFileSync(path, old.stdout) + } else { + rmSync(path, { force: true }) + } } } execFileSync( @@ -97,7 +104,11 @@ function buildTree(label, baselineRev) { ) } finally { for (const [path, contents] of restored) { - writeFileSync(path, contents) + if (contents === null) { + rmSync(path, { force: true }) + } else { + writeFileSync(path, contents) + } } } return join(outDir, 'cli/index.js') diff --git a/src/cli/cli-error.ts b/src/cli/cli-error.ts new file mode 100644 index 00000000000..6a87f149079 --- /dev/null +++ b/src/cli/cli-error.ts @@ -0,0 +1,144 @@ +import { computerUseErrorRecoveryData } from '../shared/computer-use-error-recovery' +import { + matchAutomationOwnerConflict, + stripAutomationOwnerConflictCode +} from '../shared/automation-owner-conflict' +import { automationOwnerConflictRecovery } from './automation-owner-conflict-recovery' +import type { RuntimeRpcFailure } from './runtime-client' +import { RuntimeClientError, RuntimeRpcFailureError } from './runtime/types' + +type CliErrorContext = { + commandPath?: readonly string[] +} + +export function formatCliError(error: unknown, context: CliErrorContext = {}): string { + const message = error instanceof Error ? error.message : String(error) + if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') { + if (hasOrchestrationRequestId(error.data)) { + return message + } + return `${message}\nOrca is not running. Run 'orca open' first.` + } + // Why: error-specific recovery must win over the generic computer fallback. + // Classified from the whole error, not just `.code`: a hop that flattens the class leaves only the token. + const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error)) + if (conflict) { + return formatMessageWithNextSteps(stripAutomationOwnerConflictCode(message), conflict.nextSteps) + } + if (error instanceof RuntimeClientError) { + const nextSteps = nextStepsFromData(error.data) + if (nextSteps.length > 0) { + return formatMessageWithNextSteps(message, nextSteps) + } + if (error.code === 'invalid_argument' && context.commandPath?.[0] === 'computer') { + return formatMessageWithNextSteps( + message, + computerUseErrorRecoveryData('invalid_argument')?.nextSteps ?? [] + ) + } + } + if ( + error instanceof RuntimeRpcFailureError && + error.response.error.code === 'runtime_unavailable' + ) { + return `${message}\nOrca is not running. Run 'orca open' first.` + } + if (error instanceof RuntimeRpcFailureError) { + return formatMessageWithNextSteps(message, nextStepsFromData(error.response.error.data)) + } + return message +} + +function hasOrchestrationRequestId(data: unknown): boolean { + return ( + data !== null && + typeof data === 'object' && + typeof (data as { orchestrationRequestId?: unknown }).orchestrationRequestId === 'string' + ) +} + +export function reportCliError(error: unknown, json: boolean, context: CliErrorContext = {}): void { + if (json) { + if (error instanceof RuntimeRpcFailureError) { + console.log(JSON.stringify(withAutomationOwnerConflictRecovery(error.response), null, 2)) + } else { + const response: RuntimeRpcFailure = { + id: 'local', + ok: false, + error: { + code: + matchAutomationOwnerConflict(error) ?? + (error instanceof RuntimeClientError ? error.code : 'runtime_error'), + message: stripAutomationOwnerConflictCode( + error instanceof Error ? error.message : String(error) + ), + data: localCliErrorData(error, context) + }, + _meta: { + runtimeId: null + } + } + console.log(JSON.stringify(response, null, 2)) + } + } else { + console.error(formatCliError(error, context)) + } +} + +/** Machine-readable half of the same recovery the human message carries. */ +function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): RuntimeRpcFailure { + const code = matchAutomationOwnerConflict(response) + const conflict = automationOwnerConflictRecovery(code) + if (!conflict || !code) { + return response + } + return { + ...response, + error: { + ...response.error, + // Restores the classification a flattening hop dropped, so --json consumers read the conflict, not the transport. + code, + message: stripAutomationOwnerConflictCode(response.error.message), + data: response.error.data ?? conflict + } + } +} + +function formatMessageWithNextSteps(message: string, nextSteps: readonly string[]): string { + if (nextSteps.length === 0) { + return message + } + return `${message}\n${nextSteps.map((step) => `Next step: ${step}`).join('\n')}` +} + +function nextStepsFromData(data: unknown): string[] { + if ( + data && + typeof data === 'object' && + Array.isArray((data as { nextSteps?: unknown }).nextSteps) + ) { + return (data as { nextSteps: unknown[] }).nextSteps.filter( + (step): step is string => typeof step === 'string' + ) + } + return [] +} + +function localCliErrorData(error: unknown, context: CliErrorContext): unknown { + // Why: error-specific recovery must win over the generic computer fallback. + if (error instanceof RuntimeClientError && error.data !== undefined) { + return error.data + } + const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error)) + if (conflict) { + return conflict + } + if ( + error instanceof RuntimeClientError && + error.code === 'invalid_argument' && + context.commandPath?.[0] === 'computer' + ) { + return computerUseErrorRecoveryData('invalid_argument') + } + return undefined +} diff --git a/src/cli/format.ts b/src/cli/format.ts index 1487a69eea0..dd6b7b739c7 100644 --- a/src/cli/format.ts +++ b/src/cli/format.ts @@ -1,13 +1,8 @@ import type { CliStatusResult } from '../shared/runtime-types' -import { computerUseErrorRecoveryData } from '../shared/computer-use-error-recovery' -import { - matchAutomationOwnerConflict, - stripAutomationOwnerConflictCode -} from '../shared/automation-owner-conflict' -import { automationOwnerConflictRecovery } from './automation-owner-conflict-recovery' import { prepareComputerCliJsonResult } from './computer-format' -import type { RuntimeRpcFailure, RuntimeRpcSuccess } from './runtime-client' -import { RuntimeClientError, RuntimeRpcFailureError } from './runtime/types' +import type { RuntimeRpcSuccess } from './runtime-client' + +export { formatCliError, reportCliError } from './cli-error' export { formatBrowserProfileList, @@ -67,10 +62,6 @@ export { formatWorktreeShow } from './workspace-format' -type CliErrorContext = { - commandPath?: readonly string[] -} - export function printResult( response: RuntimeRpcSuccess, json: boolean, @@ -83,138 +74,6 @@ export function printResult( console.log(formatter(response.result)) } -export function formatCliError(error: unknown, context: CliErrorContext = {}): string { - const message = error instanceof Error ? error.message : String(error) - if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') { - if (hasOrchestrationRequestId(error.data)) { - return message - } - return `${message}\nOrca is not running. Run 'orca open' first.` - } - // Why: error-specific recovery must win over the generic computer fallback. - // Classified from the whole error, not just `.code`: a hop that flattens the class leaves only the token. - const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error)) - if (conflict) { - return formatMessageWithNextSteps(stripAutomationOwnerConflictCode(message), conflict.nextSteps) - } - if (error instanceof RuntimeClientError) { - const nextSteps = nextStepsFromData(error.data) - if (nextSteps.length > 0) { - return formatMessageWithNextSteps(message, nextSteps) - } - if (error.code === 'invalid_argument' && context.commandPath?.[0] === 'computer') { - return formatMessageWithNextSteps( - message, - computerUseErrorRecoveryData('invalid_argument')?.nextSteps ?? [] - ) - } - } - if ( - error instanceof RuntimeRpcFailureError && - error.response.error.code === 'runtime_unavailable' - ) { - return `${message}\nOrca is not running. Run 'orca open' first.` - } - if (error instanceof RuntimeRpcFailureError) { - return formatMessageWithNextSteps(message, nextStepsFromData(error.response.error.data)) - } - return message -} - -function hasOrchestrationRequestId(data: unknown): boolean { - return ( - data !== null && - typeof data === 'object' && - typeof (data as { orchestrationRequestId?: unknown }).orchestrationRequestId === 'string' - ) -} - -export function reportCliError(error: unknown, json: boolean, context: CliErrorContext = {}): void { - if (json) { - if (error instanceof RuntimeRpcFailureError) { - console.log(JSON.stringify(withAutomationOwnerConflictRecovery(error.response), null, 2)) - } else { - const response: RuntimeRpcFailure = { - id: 'local', - ok: false, - error: { - code: - matchAutomationOwnerConflict(error) ?? - (error instanceof RuntimeClientError ? error.code : 'runtime_error'), - message: stripAutomationOwnerConflictCode( - error instanceof Error ? error.message : String(error) - ), - data: localCliErrorData(error, context) - }, - _meta: { - runtimeId: null - } - } - console.log(JSON.stringify(response, null, 2)) - } - } else { - console.error(formatCliError(error, context)) - } -} - -/** Machine-readable half of the same recovery the human message carries. */ -function withAutomationOwnerConflictRecovery(response: RuntimeRpcFailure): RuntimeRpcFailure { - const code = matchAutomationOwnerConflict(response) - const conflict = automationOwnerConflictRecovery(code) - if (!conflict || !code) { - return response - } - return { - ...response, - error: { - ...response.error, - // Restores the classification a flattening hop dropped, so --json consumers read the conflict, not the transport. - code, - message: stripAutomationOwnerConflictCode(response.error.message), - data: response.error.data ?? conflict - } - } -} - -function formatMessageWithNextSteps(message: string, nextSteps: readonly string[]): string { - if (nextSteps.length === 0) { - return message - } - return `${message}\n${nextSteps.map((step) => `Next step: ${step}`).join('\n')}` -} - -function nextStepsFromData(data: unknown): string[] { - if ( - data && - typeof data === 'object' && - Array.isArray((data as { nextSteps?: unknown }).nextSteps) - ) { - return (data as { nextSteps: unknown[] }).nextSteps.filter( - (step): step is string => typeof step === 'string' - ) - } - return [] -} - -function localCliErrorData(error: unknown, context: CliErrorContext): unknown { - // Why: error-specific recovery must win over the generic computer fallback. - if (error instanceof RuntimeClientError && error.data !== undefined) { - return error.data - } - const conflict = automationOwnerConflictRecovery(matchAutomationOwnerConflict(error)) - if (conflict) { - return conflict - } - if ( - error instanceof RuntimeClientError && - error.code === 'invalid_argument' && - context.commandPath?.[0] === 'computer' - ) { - return computerUseErrorRecoveryData('invalid_argument') - } - return undefined -} - export type HostListEntry = { kind: 'local' | 'ssh' | 'environment' name: string diff --git a/src/cli/index.ts b/src/cli/index.ts index 9389113b195..a0e1354307f 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -15,7 +15,7 @@ import { resolveHostFlagEnvironmentId } from './execution-host-flag' import { listSshTargets } from './host-selector-alternatives' -import { reportCliError } from './format' +import { reportCliError } from './cli-error' import { printHelp } from './help' import type { RuntimeClient } from './runtime-client' import { COMMAND_SPECS } from './specs' diff --git a/src/cli/runtime-client-deferral.test.ts b/src/cli/runtime-client-deferral.test.ts index 658cc60f0a4..5fcdf8b9686 100644 --- a/src/cli/runtime-client-deferral.test.ts +++ b/src/cli/runtime-client-deferral.test.ts @@ -84,14 +84,12 @@ describe('RuntimeClient module-graph deferral', () => { process.exitCode = 0 }) - // Why: the whole point of the change. These six modules load on EVERY - // invocation, so a value-import of the barrel from any of them drags the - // RuntimeClient graph (zod, ws, tweetnacl) back onto the --help path. + // These eager modules must not pull the RuntimeClient dependency graph into help. it.each([ 'args.ts', 'flags.ts', 'dispatch.ts', - 'format.ts', + 'cli-error.ts', 'selectors.ts', 'execution-host-flag.ts' ])('%s imports error classes from ./runtime/types, not the barrel', (file) => { @@ -110,6 +108,7 @@ describe('RuntimeClient module-graph deferral', () => { expect(source).toContain("import type { RuntimeClient } from './runtime-client'") expect(source).not.toMatch(/^import \{[^}]*RuntimeClient[^}]*\} from '\.\/runtime-client'/m) expect(source).toContain("await import('./runtime-client.js')") + expect(source).toContain("import { reportCliError } from './cli-error'") }) it('constructs no client for --help', async () => { From 9b76ff9217461bdb81f9acb4af55b042dcf70301 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:24 -0700 Subject: [PATCH 26/43] perf(explorer): avoid redundant dotfile path filtering (#18929) --- .../benchmark-explorer-dotfile-filter.mjs | 165 ++++++++++++++++++ .../right-sidebar/file-explorer-entries.ts | 5 +- 2 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 config/scripts/benchmark-explorer-dotfile-filter.mjs diff --git a/config/scripts/benchmark-explorer-dotfile-filter.mjs b/config/scripts/benchmark-explorer-dotfile-filter.mjs new file mode 100644 index 00000000000..e66a0ceb5af --- /dev/null +++ b/config/scripts/benchmark-explorer-dotfile-filter.mjs @@ -0,0 +1,165 @@ +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import Module from 'node:module' +import { resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' + +// Pass the pre-change file-explorer-entries.ts snapshot as the only argument. +const baselinePath = process.argv[2] +assert.ok(baselinePath, 'Pass a pre-change file-explorer-entries.ts snapshot.') +const entry = 'src/renderer/src/components/right-sidebar/file-explorer-entries.ts' +const baseline = readFileSync(baselinePath, 'utf8') +assert.notEqual(baseline, readFileSync(entry, 'utf8'), 'Do not compare the source to itself.') + +async function load(useBaseline) { + const result = await build({ + stdin: { + contents: `export { isDotfileRelativePath } from './${entry}'; +export { createNameFilteredFileExplorerProjection } from './src/renderer/src/components/right-sidebar/file-explorer-name-filter-projection.ts';`, + resolveDir: process.cwd(), + loader: 'ts' + }, + bundle: true, + platform: 'node', + format: 'cjs', + write: false, + logLevel: 'silent', + alias: { '@': resolve('src/renderer/src') }, + plugins: useBaseline + ? [ + { + name: 'baseline-dotfile-predicate', + setup(builder) { + builder.onLoad({ filter: /file-explorer-entries\.ts$/ }, () => ({ + contents: baseline, + loader: 'ts' + })) + } + } + ] + : [] + }) + const module = new Module(resolve('dotfile-benchmark.cjs')) + module.paths = Module._nodeModulePaths(process.cwd()) + module._compile(result.outputFiles[0].text, module.id) + return module.exports +} + +const versions = [await load(true), await load(false)] +let parityCases = 0 +function check(path, depth) { + assert.equal( + versions[0].isDotfileRelativePath(path), + versions[1].isDotfileRelativePath(path), + path + ) + parityCases++ + if (depth > 0) { + for (const character of ['.', '/', '\\', 'a', '\n']) { + check(path + character, depth - 1) + } + } +} +check('', 8) + +function measure(functions, iterations = 1) { + let sink = 0 + const run = (fn) => { + for (let i = 0; i < iterations; i++) { + sink += Number(fn()) + } + } + for (const fn of functions) { + for (let warmup = 0; warmup < 3; warmup++) { + run(fn) + } + } + const samples = [[], []] + for (let round = 0; round < 11; round++) { + for (const variant of round % 2 ? [1, 0] : [0, 1]) { + const start = performance.now() + run(functions[variant]) + samples[variant].push(performance.now() - start) + } + } + return { + beforeMs: samples[0].sort((a, b) => a - b)[5], + afterMs: samples[1].sort((a, b) => a - b)[5], + iterations, + sink + } +} + +const predicates = [] +for (const path of [ + 'a', + '.env', + 'packages/pkg/src/file.tsx', + `a${'.'.repeat(254)}`, + `${'/'.repeat(4096)}.`, + `${'../'.repeat(1000)}file.ts`, + '😀/.你好', + '\n/.\n' +]) { + check(path, 0) + predicates.push({ + pathLength: path.length, + prefix: path.slice(0, 40), + ...measure( + versions.map((version) => () => version.isDotfileRelativePath(path)), + 10_000 + ) + }) +} + +const projections = [] +for (const count of [1000, 10_000, 100_000]) { + for (const query of ['nonmatching-needle', 'file-42']) { + const args = { + ignoredSet: new Set(['unrelated']), + nameFilter: { + query, + relativePaths: Array.from( + { length: count }, + (_, i) => `packages/package-${i % 50}/src/components/section-${i % 10}/file-${i}.tsx` + ) + }, + showDotfiles: false, + showGitIgnoredFiles: false, + worktreePath: '/workspace' + } + const functions = versions.map( + (version) => () => version.createNameFilteredFileExplorerProjection(args) + ) + const rows = functions.map((fn) => { + const projection = fn() + return Array.from({ length: projection.getVisibleCount() }, (_, i) => + projection.getRowAtIndex(i) + ) + }) + assert.deepEqual(rows[0], rows[1]) + projections.push({ + count, + query, + visibleRows: rows[0].length, + ...measure(functions.map((fn) => () => fn().getVisibleCount())) + }) + } +} +console.log( + JSON.stringify( + { + node: process.version, + platform: process.platform, + baselinePath: resolve(baselinePath), + parityCases, + samples: 11, + warmups: 3, + predicates, + projections + }, + null, + 2 + ) +) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-entries.ts b/src/renderer/src/components/right-sidebar/file-explorer-entries.ts index 4e5b6e7b423..f21116c7822 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-entries.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-entries.ts @@ -9,8 +9,5 @@ function isDotfileSegment(segment: string): boolean { } export function isDotfileRelativePath(relativePath: string): boolean { - return relativePath - .split(/[\\/]+/) - .filter(Boolean) - .some(isDotfileSegment) + return relativePath.split(/[\\/]+/).some(isDotfileSegment) } From d1e62419b6af046a337e1d9a4dee0c7f79ff4508 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:28 -0700 Subject: [PATCH 27/43] perf(watcher): stop admitting stats after batch cancellation (#18931) --- .../filesystem-watcher-local-events.test.ts | 30 +++++++++++++++++++ .../ipc/filesystem-watcher-local-events.ts | 5 +++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/main/ipc/filesystem-watcher-local-events.test.ts b/src/main/ipc/filesystem-watcher-local-events.test.ts index e08145e7ad2..1907383bd6b 100644 --- a/src/main/ipc/filesystem-watcher-local-events.test.ts +++ b/src/main/ipc/filesystem-watcher-local-events.test.ts @@ -12,6 +12,7 @@ vi.mock('fs/promises', () => ({ stat: statMock })) vi.mock('./parcel-watcher-process', () => ({ subscribeViaWatcherProcess: subscribeMock })) import { createLocalWatcher } from './filesystem-watcher-local-events' +import { cancelLocalBatchFlush } from './filesystem-watcher-batch-control' function deferred(): { promise: Promise; resolve: (value: T) => void } { let resolve!: (value: T) => void @@ -170,6 +171,35 @@ describe('local filesystem watcher flush serialization', () => { ) }) + it('starts no further stats when a full inflight batch is cancelled', async () => { + const eventCount = 5_000 + const pendingStats = deferred<{ isDirectory: () => boolean }>() + statMock.mockReturnValue(pendingStats.promise) + const root = await createLocalWatcher('/repo', '/repo') + root.listeners.set(1, sender as never) + + watcherCallback?.( + null, + Array.from({ length: eventCount }, (_, index) => ({ + type: 'update' as const, + path: `/repo/file-${index}.ts` + })) + ) + vi.advanceTimersByTime(WATCH_BATCH_TRAILING_MS) + await flushMicrotasks() + expect(statMock).toHaveBeenCalledTimes(8) + + cancelLocalBatchFlush(root) + pendingStats.resolve({ isDirectory: () => false }) + for (let i = 0; i < eventCount * 4 && root.batch.flushInFlight; i++) { + await Promise.resolve() + } + + expect(root.batch.flushInFlight).toBe(false) + expect(statMock).toHaveBeenCalledTimes(8) + expect(sender.send).not.toHaveBeenCalled() + }) + it('leaves an open debounce window to the armed timer instead of draining early', async () => { const firstStat = deferred<{ isDirectory: () => boolean }>() const secondStat = deferred<{ isDirectory: () => boolean }>() diff --git a/src/main/ipc/filesystem-watcher-local-events.ts b/src/main/ipc/filesystem-watcher-local-events.ts index 57ef7da4e1f..9f31adde4bd 100644 --- a/src/main/ipc/filesystem-watcher-local-events.ts +++ b/src/main/ipc/filesystem-watcher-local-events.ts @@ -139,7 +139,10 @@ async function flushBatch(root: WatchedRoot): Promise { DIRECTORY_STAT_CONCURRENCY, async (evt) => { // Why: a deleted path can't be stat'd; leave isDirectory undefined and let the renderer infer from dirCache. - const isDirectory = evt.type === 'delete' ? undefined : await tryStatIsDirectory(evt.path) + const isDirectory = + root.batch.cancelled || evt.type === 'delete' + ? undefined + : await tryStatIsDirectory(evt.path) return { kind: evt.type, From 6f28e019b5f52e858ee11286956982c975030af7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:33 -0700 Subject: [PATCH 28/43] perf(hooks): use native reverse search for transcript lines (#18936) --- .../benchmark-transcript-reverse-lines.mjs | 124 ++++++++++++++++++ .../transcript-reader.test.ts | 70 ++++++++++ .../agent-hook-listener/transcript-reader.ts | 6 +- 3 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 config/scripts/benchmark-transcript-reverse-lines.mjs create mode 100644 src/shared/agent-hook-listener/transcript-reader.test.ts diff --git a/config/scripts/benchmark-transcript-reverse-lines.mjs b/config/scripts/benchmark-transcript-reverse-lines.mjs new file mode 100644 index 00000000000..e9d370d1839 --- /dev/null +++ b/config/scripts/benchmark-transcript-reverse-lines.mjs @@ -0,0 +1,124 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import Module from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' + +const entry = 'src/shared/agent-hook-listener/transcript-reader.ts' +assert.ok(process.argv[2], 'Pass a pre-change transcript-reader.ts snapshot.') +const baseline = readFileSync(process.argv[2], 'utf8') +assert.notEqual(baseline, readFileSync(entry, 'utf8'), 'Do not compare the source to itself.') + +async function load(useBaseline) { + const result = await build({ + stdin: { + contents: `export * from './${entry}'; +export { extractAssistantTextFromLine } from './src/shared/agent-hook-listener/transcript-entry-text.ts';`, + resolveDir: process.cwd(), + loader: 'ts' + }, + bundle: true, + platform: 'node', + format: 'cjs', + write: false, + logLevel: 'silent', + plugins: useBaseline + ? [ + { + name: 'baseline-transcript-reader', + setup(builder) { + builder.onLoad({ filter: /transcript-reader\.ts$/ }, () => ({ + contents: baseline, + loader: 'ts' + })) + } + } + ] + : [] + }) + const module = new Module(resolve('transcript-benchmark.cjs')) + module.paths = Module._nodeModulePaths(process.cwd()) + module._compile(result.outputFiles[0].text, module.id) + return module.exports +} + +const versions = [await load(true), await load(false)] +function measure(functions, iterations) { + let sink = 0 + const run = (fn) => { + for (let i = 0; i < iterations; i++) { + sink += fn()?.length ?? 0 + } + } + for (const fn of functions) { + for (let i = 0; i < 3; i++) { + run(fn) + } + } + const samples = [[], []] + for (let round = 0; round < 11; round++) { + for (const index of round % 2 ? [1, 0] : [0, 1]) { + const start = performance.now() + run(functions[index]) + samples[index].push((performance.now() - start) / iterations) + } + } + return { + beforeMs: samples[0].sort((a, b) => a - b)[5], + afterMs: samples[1].sort((a, b) => a - b)[5], + iterations, + sink + } +} + +const cases = [ + ['tiny', `${JSON.stringify({ role: 'assistant', content: 'hello' })}\n`, 10000], + ['64KiB line', `${JSON.stringify({ role: 'assistant', content: 'x'.repeat(65500) })}\n`, 100], + [ + '4MiB line', + `${JSON.stringify({ role: 'assistant', content: 'x'.repeat(4 * 1024 * 1024 - 40) })}\n`, + 10 + ], + [ + '1000 short tool lines', + Array.from({ length: 1000 }, () => + JSON.stringify({ role: 'tool', content: 'x'.repeat(100) }) + ).join('\n'), + 50 + ], + [ + 'Unicode line', + `${JSON.stringify({ role: 'assistant', content: '😀漢字'.repeat(16000) })}\n`, + 100 + ], + [ + 'leading and trailing blank lines', + `\n\r\n${JSON.stringify({ role: 'assistant', content: 'hello' })}\n\n`, + 10000 + ] +] +const directory = mkdtempSync(join(tmpdir(), 'orca-transcript-benchmark-')) +try { + for (const [name, text, iterations] of cases) { + const file = join(directory, 'transcript.jsonl') + writeFileSync(file, text) + const scanners = versions.map( + (v) => () => v.findLastExtractedTranscriptLineText(text, v.extractAssistantTextFromLine) + ) + const readers = versions.map((v) => () => v.readLastAssistantFromTranscriptOnce(file)) + assert.equal(scanners[0](), scanners[1](), name) + assert.equal(readers[0](), readers[1](), name) + console.log( + JSON.stringify({ + name, + bytes: Buffer.byteLength(text), + scanner: measure(scanners, iterations), + warmFileReader: measure(readers, Math.min(iterations, 100)) + }) + ) + } +} finally { + rmSync(directory, { recursive: true, force: true }) +} diff --git a/src/shared/agent-hook-listener/transcript-reader.test.ts b/src/shared/agent-hook-listener/transcript-reader.test.ts new file mode 100644 index 00000000000..83780b08388 --- /dev/null +++ b/src/shared/agent-hook-listener/transcript-reader.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest' +import { findLastExtractedTranscriptLineText } from './transcript-reader' +import { extractAssistantTextFromLine } from './transcript-entry-text' + +function expectedLines(text: string): string[] { + return text + .split('\n') + .toReversed() + .map((line) => line.trim()) + .filter((line) => line.length > 0) +} + +describe('backward transcript line extraction', () => { + it.each(['', '\n', '\n\n', '\r\n', ' \t\r\n', '\na\n', 'a\nb', '😀\r\n漢字'])( + 'visits each nonblank line once in reverse order for %j', + (text) => { + const seen: string[] = [] + expect( + findLastExtractedTranscriptLineText(text, (line) => { + seen.push(line) + return undefined + }) + ).toBeUndefined() + expect(seen).toEqual(expectedLines(text)) + } + ) + + it('preserves line order and early return across generated delimiters', () => { + let seed = 29 + const fragments = ['a', '\n', '\r\n', ' ', '\t', '😀', '\u2028', '\0'] + for (let sample = 0; sample < 1000; sample++) { + let text = '' + for (let i = 0; i < sample % 100; i++) { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0 + text += fragments[seed % fragments.length] + } + const lines = expectedLines(text) + const stop = sample % (lines.length + 1) + const seen: string[] = [] + const result = findLastExtractedTranscriptLineText(text, (line) => { + seen.push(line) + return seen.length === stop + 1 ? line : undefined + }) + expect(seen).toEqual(lines.slice(0, stop + 1)) + expect(result).toBe(lines[stop]) + } + }) + + it('returns the newest assistant message behind a long tool line', () => { + const message = JSON.stringify({ role: 'assistant', content: 'latest 😀' }) + const tool = JSON.stringify({ role: 'tool', content: 'x'.repeat(256 * 1024) }) + expect( + findLastExtractedTranscriptLineText( + `\n{"role":"assistant","content":"older"}\r\n${message}\r\n${tool}\n`, + extractAssistantTextFromLine + ) + ).toBe('latest 😀') + }) + + it('treats an empty extracted string as a result and stops before older lines', () => { + const seen: string[] = [] + expect( + findLastExtractedTranscriptLineText('older\nlatest\n', (line) => { + seen.push(line) + return '' + }) + ).toBe('') + expect(seen).toEqual(['latest']) + }) +}) diff --git a/src/shared/agent-hook-listener/transcript-reader.ts b/src/shared/agent-hook-listener/transcript-reader.ts index f9b1472384e..89df71da2df 100644 --- a/src/shared/agent-hook-listener/transcript-reader.ts +++ b/src/shared/agent-hook-listener/transcript-reader.ts @@ -88,10 +88,8 @@ export function findLastExtractedTranscriptLineText( ): string | undefined { let lineEnd = text.length - for (let index = text.length - 1; index >= -1; index--) { - if (index >= 0 && text.charCodeAt(index) !== 10) { - continue - } + while (lineEnd > 0) { + const index = text.lastIndexOf('\n', lineEnd - 1) const line = text.slice(index + 1, lineEnd).trim() if (line.length > 0) { From bf073b833e648b235d0d0ae49e29f87f5146d0ac Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:37 -0700 Subject: [PATCH 29/43] perf(skills): skip symlink probes beyond discovery depth (#18937) --- config/scripts/benchmark-skill-depth.mjs | 122 +++++++++++++++++++ src/main/skills/skill-root-file-walk.test.ts | 28 +++++ src/main/skills/skill-root-file-walk.ts | 14 ++- 3 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 config/scripts/benchmark-skill-depth.mjs diff --git a/config/scripts/benchmark-skill-depth.mjs b/config/scripts/benchmark-skill-depth.mjs new file mode 100644 index 00000000000..1ebb606f93c --- /dev/null +++ b/config/scripts/benchmark-skill-depth.mjs @@ -0,0 +1,122 @@ +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import * as fs from 'node:fs/promises' +import Module from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' + +// Pass a pre-change skill-root-file-walk.ts snapshot as the only argument. +const baselinePath = process.argv[2] +const brokenLinks = process.argv.includes('--broken') +assert.ok(baselinePath, 'Pass a pre-change skill-root-file-walk.ts snapshot.') +const entry = 'src/main/skills/skill-root-file-walk.ts' +const baseline = readFileSync(baselinePath, 'utf8') +assert.notEqual(baseline, readFileSync(entry, 'utf8'), 'Do not compare the source to itself.') +let statCalls = 0 + +async function load(useBaseline) { + const result = await build({ + entryPoints: [entry], + bundle: true, + platform: 'node', + format: 'cjs', + write: false, + logLevel: 'silent', + plugins: useBaseline + ? [ + { + name: 'baseline-skill-depth', + setup(builder) { + builder.onLoad({ filter: /skill-root-file-walk\.ts$/ }, () => ({ + contents: baseline, + loader: 'ts' + })) + } + } + ] + : [] + }) + const module = new Module(resolve('skill-depth-benchmark.cjs')) + module.paths = Module._nodeModulePaths(process.cwd()) + const originalRequire = module.require.bind(module) + module.require = (name) => + name === 'node:fs/promises' + ? { + ...fs, + stat: (...args) => { + statCalls++ + return fs.stat(...args) + } + } + : originalRequire(name) + module._compile(result.outputFiles[0].text, module.id) + return module.exports.findSkillFiles +} + +const before = await load(true) +const after = await load(false) +const median = (values) => values.sort((a, b) => a - b)[Math.floor(values.length / 2)] +const temporaryRoot = await fs.mkdtemp(join(tmpdir(), 'orca-skill-depth-benchmark-')) +try { + for (const links of [0, 8, 100, 1000]) { + const root = join(temporaryRoot, String(links)) + const edge = join(root, 'a', 'b', 'c', 'd') + const target = join(temporaryRoot, 'target') + await fs.mkdir(edge, { recursive: true }) + await fs.mkdir(target, { recursive: true }) + await fs.writeFile(join(target, 'SKILL.md'), 'skill') + await fs.writeFile(join(edge, 'SKILL.md'), 'edge') + for (let index = 0; index < links; index++) { + await fs.symlink( + brokenLinks ? join(target, 'missing') : target, + join(edge, `link${index}`), + process.platform === 'win32' ? 'junction' : 'dir' + ) + } + for (const depth of [4, 5]) { + const timings = { before: [], after: [] } + const counts = {} + let rows + for (let sample = 0; sample < 13; sample++) { + const versions = + sample % 2 + ? [ + ['after', after], + ['before', before] + ] + : [ + ['before', before], + ['after', after] + ] + for (const [name, walk] of versions) { + statCalls = 0 + const start = performance.now() + const result = await walk(root, depth) + const elapsed = performance.now() - start + if (rows) { + assert.deepEqual(result, rows) + } + rows = result + counts[name] = statCalls + if (sample >= 2) { + timings[name].push(elapsed) + } + } + } + console.log( + JSON.stringify({ + links, + brokenLinks, + depth, + statCalls: counts, + rows: rows.length, + medianMs: { before: median(timings.before), after: median(timings.after) } + }) + ) + } + } +} finally { + await fs.rm(temporaryRoot, { recursive: true, force: true }) +} diff --git a/src/main/skills/skill-root-file-walk.test.ts b/src/main/skills/skill-root-file-walk.test.ts index 3ca80bdb495..ad84eced6b6 100644 --- a/src/main/skills/skill-root-file-walk.test.ts +++ b/src/main/skills/skill-root-file-walk.test.ts @@ -45,6 +45,34 @@ describe('findSkillFiles', () => { expect(found).toEqual([join(root, 'near', 'SKILL.md')]) }) + it('does not stat directory links beyond the depth bound but still follows in-bound links', async () => { + const base = await makeTree() + const root = join(base, 'skills') + const edge = join(root, 'a', 'b', 'c', 'd') + const target = join(base, 'linked') + await writeFileAt(join(edge, 'SKILL.md')) + await writeFileAt(join(target, 'SKILL.md')) + for (let index = 0; index < 32; index += 1) { + await symlink( + target, + join(edge, `link${index.toString().padStart(2, '0')}`), + process.platform === 'win32' ? 'junction' : 'dir' + ) + } + const statPaths: string[] = [] + onStat = async (path) => { + statPaths.push(path) + } + + expect(await findSkillFiles(root, 4)).toEqual([join(edge, 'SKILL.md')]) + expect(statPaths).toEqual([]) + expect(await findSkillFiles(root, 5)).toEqual([ + join(edge, 'SKILL.md'), + join(edge, 'link00', 'SKILL.md') + ]) + expect(statPaths).toHaveLength(32) + }) + it('returns nothing for a missing root rather than throwing', async () => { expect(await findSkillFiles(join(await makeTree(), 'absent'), 4)).toEqual([]) }) diff --git a/src/main/skills/skill-root-file-walk.ts b/src/main/skills/skill-root-file-walk.ts index 261a650a967..4be1843f602 100644 --- a/src/main/skills/skill-root-file-walk.ts +++ b/src/main/skills/skill-root-file-walk.ts @@ -43,9 +43,6 @@ export async function findSkillFiles( // indistinguishable from a genuinely small root, and a caller that cached it // would publish "these skills no longer exist". signal?.throwIfAborted() - if (!isWithinDepth(rootPath, dirPath, maxDepth)) { - return - } let resolvedDirPath: string try { resolvedDirPath = await realpath(dirPath) @@ -61,6 +58,8 @@ export async function findSkillFiles( if (!entries) { return } + // Directory entry names add one segment, so siblings share the depth verdict. + let childrenWithinDepth: boolean | undefined for (const entry of entries) { signal?.throwIfAborted() // Why: a staged sibling sits directly in a scanned root, so without this a @@ -86,10 +85,15 @@ export async function findSkillFiles( continue } if (entry.isDirectory()) { - await visit(entryPath) + if ((childrenWithinDepth ??= isWithinDepth(rootPath, entryPath, maxDepth))) { + await visit(entryPath) + } continue } - if (entry.isSymbolicLink()) { + if ( + entry.isSymbolicLink() && + (childrenWithinDepth ??= isWithinDepth(rootPath, entryPath, maxDepth)) + ) { // Why: users commonly symlink agent skill dirs across providers; follow // directory links but guard by realpath so recursive links cannot loop. let linksToDirectory = false From 992360f12126cb4c387273fe805a99baa6fccb4f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:42 -0700 Subject: [PATCH 30/43] perf(mobile): cancel direct probes when their owner stops (#18940) * perf(mobile): cancel direct probes when their owner stops * fix(mobile): fence direct migration after supervisor stop --- .../transport/mobile-direct-endpoint-probe.ts | 28 ++- .../mobile-direct-probe-stop-budget.test.ts | 175 ++++++++++++++++++ .../transport/mobile-direct-return-probe.ts | 43 ++++- .../transport/mobile-endpoint-supervisor.ts | 3 +- 4 files changed, 241 insertions(+), 8 deletions(-) create mode 100644 mobile/src/transport/mobile-direct-probe-stop-budget.test.ts diff --git a/mobile/src/transport/mobile-direct-endpoint-probe.ts b/mobile/src/transport/mobile-direct-endpoint-probe.ts index 03264d5f7e5..038f73b93b8 100644 --- a/mobile/src/transport/mobile-direct-endpoint-probe.ts +++ b/mobile/src/transport/mobile-direct-endpoint-probe.ts @@ -31,7 +31,14 @@ export function directPathForEndpoint( // instead of holding the supervisor's operation mutex for the full outer bound. const RECONNECT_GRACE_MS = 2_000 -function waitForAuthenticatedSession(session: RpcClient, timeoutMs: number): Promise { +function waitForAuthenticatedSession( + session: RpcClient, + timeoutMs: number, + signal?: AbortSignal +): Promise { + if (signal?.aborted) { + return Promise.reject(new Error('probe cancelled')) + } if (session.getState() === 'connected') { return Promise.resolve() } @@ -72,7 +79,13 @@ function waitForAuthenticatedSession(session: RpcClient, timeoutMs: number): Pro finish() reject(new Error('probe session authentication timed out')) }, timeoutMs) + const onAbort = (): void => { + finish() + reject(new Error('probe cancelled')) + } + signal?.addEventListener('abort', onAbort, { once: true }) function finish(): void { + signal?.removeEventListener('abort', onAbort) if (timer) { clearTimeout(timer) } @@ -87,8 +100,12 @@ function waitForAuthenticatedSession(session: RpcClient, timeoutMs: number): Pro export async function openAuthenticatedDirectEndpoint( host: HostProfile, openDirect: (endpoint: string) => RpcClient, - timeoutMs: number + timeoutMs: number, + signal?: AbortSignal ): Promise<{ client: RpcClient; path: Exclude } | null> { + if (signal?.aborted) { + return null + } const endpoints = directEndpointUrls(host) return await new Promise((resolve) => { const clients = new Set() @@ -110,8 +127,13 @@ export async function openAuthenticatedDirectEndpoint( continue } clients.add(client) - void waitForAuthenticatedSession(client, timeoutMs).then( + void waitForAuthenticatedSession(client, timeoutMs, signal).then( () => { + if (signal?.aborted) { + client.close() + rejectCandidate() + return + } if (settled) { client.close() return diff --git a/mobile/src/transport/mobile-direct-probe-stop-budget.test.ts b/mobile/src/transport/mobile-direct-probe-stop-budget.test.ts new file mode 100644 index 00000000000..535790ed480 --- /dev/null +++ b/mobile/src/transport/mobile-direct-probe-stop-budget.test.ts @@ -0,0 +1,175 @@ +import { expect, it, vi } from 'vitest' +import { + dependencies, + FakeLogicalClient, + FakeSession, + host +} from './mobile-endpoint-supervisor-test-fakes' +import { MobileEndpointHysteresis } from './mobile-endpoint-hysteresis' +import { createStableLogicalRpcClient } from './stable-logical-rpc-client' +import { MobileEndpointSupervisor } from './mobile-endpoint-supervisor' +vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) +vi.mock('expo-secure-store', () => ({ WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'when-unlocked' })) +vi.mock('expo-crypto', () => ({ getRandomBytes: (length: number) => new Uint8Array(length) })) +it('closes in-flight candidates and clears their timeout when the owner stops', async () => { + vi.useFakeTimers() + try { + const candidate = new FakeSession('connecting') + const logical = new FakeLogicalClient('connected', 'relay') + const deps = dependencies({ openDirect: vi.fn(() => candidate) }) + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + await supervisor.start() + await vi.advanceTimersByTimeAsync(15_000) + expect(deps.openDirect).toHaveBeenCalledOnce() + supervisor.stop() + await vi.advanceTimersByTimeAsync(0) + expect(candidate.close).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + await vi.advanceTimersByTimeAsync(12_000) + expect(vi.getTimerCount()).toBe(0) + expect(candidate.close).toHaveBeenCalledOnce() + expect(logical.migrateTo).not.toHaveBeenCalled() + expect(deps.openDirect).toHaveBeenCalledOnce() + } finally { + vi.restoreAllMocks() + vi.useRealTimers() + } +}) + +it('closes an authenticated candidate when stop races its completion', async () => { + vi.useFakeTimers() + try { + const candidate = new FakeSession('connecting') + const logical = new FakeLogicalClient('connected', 'relay') + const deps = dependencies({ openDirect: vi.fn(() => candidate) }) + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + await supervisor.start() + await vi.advanceTimersByTimeAsync(15_000) + candidate.publishState('connected') + supervisor.stop() + await vi.advanceTimersByTimeAsync(0) + expect(candidate.close).toHaveBeenCalledOnce() + expect(logical.migrateTo).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.restoreAllMocks() + vi.useRealTimers() + } +}) + +it('preserves an in-flight probe across a transient background pause', async () => { + vi.useFakeTimers() + try { + const candidate = new FakeSession('connecting') + const logical = new FakeLogicalClient('connected', 'relay') + const deps = dependencies({ openDirect: vi.fn(() => candidate) }) + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + await supervisor.start() + await vi.advanceTimersByTimeAsync(15_000) + supervisor.setForeground(false) + await vi.advanceTimersByTimeAsync(0) + expect(candidate.close).not.toHaveBeenCalled() + supervisor.stop() + await vi.advanceTimersByTimeAsync(0) + expect(candidate.close).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.restoreAllMocks() + vi.useRealTimers() + } +}) + +it('releases every candidate when multiple endpoint probes are pending', async () => { + vi.useFakeTimers() + try { + const candidates: FakeSession[] = [] + const logical = new FakeLogicalClient('connected', 'relay') + const deps = dependencies({ + openDirect: vi.fn(() => { + const candidate = new FakeSession('connecting') + candidates.push(candidate) + return candidate + }) + }) + const supervisor = new MobileEndpointSupervisor( + logical, + { + ...host, + endpoints: [{ id: 'alternate', kind: 'tailscale', url: 'ws://100.64.0.2:6768' }] + }, + deps + ) + await supervisor.start() + await vi.advanceTimersByTimeAsync(15_000) + expect(candidates).toHaveLength(2) + supervisor.stop() + await vi.advanceTimersByTimeAsync(0) + expect(vi.getTimerCount()).toBe(0) + for (const candidate of candidates) { + expect(candidate.close).toHaveBeenCalledOnce() + candidate.publishState('connected') + } + await vi.advanceTimersByTimeAsync(60_000) + expect(logical.migrateTo).not.toHaveBeenCalled() + expect(deps.openDirect).toHaveBeenCalledTimes(2) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.restoreAllMocks() + vi.useRealTimers() + } +}) + +it.each([false, true])( + 'fences migration finishing after stop (already swapped: %s)', + async (alreadySwapped) => { + vi.useFakeTimers() + try { + const recordedMigration = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordMigration') + const relay = new FakeSession('connected') + const logical = createStableLogicalRpcClient(relay, 'relay') + const candidates: FakeSession[] = [] + const deps = dependencies({ + openDirect: vi.fn(() => { + const candidate = new FakeSession('connected') + candidates.push(candidate) + return candidate + }) + }) + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + const migrate = logical.migrateTo.bind(logical) + let release!: () => void + const pending = new Promise((resolve) => { + release = resolve + }) + const migration = vi.spyOn(logical, 'migrateTo').mockImplementation(async (...args) => { + if (alreadySwapped) { + await migrate(...args) + } + await pending + if (!alreadySwapped) { + await migrate(...args) + } + }) + await supervisor.start() + await vi.advanceTimersByTimeAsync(60_000) + expect(migration).toHaveBeenCalledOnce() + const requestsBeforeStop = relay.sendRequest.mock.calls.length + const candidateRequestsBeforeStop = candidates[3].sendRequest.mock.calls.length + const migrationsBeforeStop = recordedMigration.mock.calls.length + supervisor.stop() + release() + await vi.advanceTimersByTimeAsync(0) + expect(logical.getActivePath()).toBe(alreadySwapped ? 'lan' : 'relay') + expect(logical.getGeneration()).toBe(alreadySwapped ? 2 : 1) + expect(relay.sendRequest).toHaveBeenCalledTimes(requestsBeforeStop) + expect(candidates[3].sendRequest).toHaveBeenCalledTimes(candidateRequestsBeforeStop) + expect(recordedMigration).toHaveBeenCalledTimes(migrationsBeforeStop) + expect(candidates[3].close).toHaveBeenCalledTimes(alreadySwapped ? 0 : 1) + expect(vi.getTimerCount()).toBe(0) + logical.close() + } finally { + vi.restoreAllMocks() + vi.useRealTimers() + } + } +) diff --git a/mobile/src/transport/mobile-direct-return-probe.ts b/mobile/src/transport/mobile-direct-return-probe.ts index dfb0572aa38..3ae31edd07f 100644 --- a/mobile/src/transport/mobile-direct-return-probe.ts +++ b/mobile/src/transport/mobile-direct-return-probe.ts @@ -11,6 +11,9 @@ const DIRECT_PROBE_INTERVAL_MS = 15_000 export class DirectReturnProbe { private timer: ReturnType | null = null + private stopped = false + private activeProbe: AbortController | null = null + constructor( private readonly deps: { now: () => number @@ -24,14 +27,18 @@ export class DirectReturnProbe { canSchedule: () => boolean canAttempt: () => boolean beginOperation: () => void - migrate: (client: RpcClient, path: MobileConnectionPath) => Promise + migrate: ( + client: RpcClient, + path: MobileConnectionPath, + shouldAbort: () => boolean + ) => Promise onDirectMigrated: () => Promise afterProbe: () => void } ) {} schedule(delayMs = DIRECT_PROBE_INTERVAL_MS): void { - if (!this.hooks.canSchedule() || this.timer) { + if (this.stopped || !this.hooks.canSchedule() || this.timer) { return } this.timer = this.deps.setTimer(() => { @@ -47,19 +54,34 @@ export class DirectReturnProbe { } } + stop(): void { + this.stopped = true + this.clear() + this.activeProbe?.abort() + } + private async probe(): Promise { + if (this.stopped) { + return + } if (!this.hooks.canAttempt() || !this.hooks.hysteresis.canProbe(this.deps.now())) { this.schedule() return } + const controller = new AbortController() + this.activeProbe = controller this.hooks.beginOperation() let successful: Awaited> = null try { successful = await openAuthenticatedDirectEndpoint( this.hooks.host(), this.deps.openDirect, - 12_000 + 12_000, + controller.signal ) + if (this.stopped) { + return + } if (!successful) { this.hooks.hysteresis.recordDirectFailure(this.deps.now()) return @@ -68,11 +90,24 @@ export class DirectReturnProbe { successful.client.close() return } - await this.hooks.migrate(successful.client, successful.path) + const candidate = successful + // Migration owns the candidate, including closing it if cutover is canceled. successful = null + try { + await this.hooks.migrate(candidate.client, candidate.path, () => this.stopped) + } catch (error) { + if (this.stopped) { + return + } + throw error + } + if (this.stopped) { + return + } this.hooks.hysteresis.recordMigration(this.deps.now()) await this.hooks.onDirectMigrated() } finally { + this.activeProbe = null successful?.client.close() // Why: a relay drop or backoff timer can arrive while the probe owns the // operation mutex; afterProbe releases it and replays deferred recovery. diff --git a/mobile/src/transport/mobile-endpoint-supervisor.ts b/mobile/src/transport/mobile-endpoint-supervisor.ts index 6fca6c0cc17..9ba12f35112 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor.ts @@ -120,7 +120,7 @@ export class MobileEndpointSupervisor { canSchedule: () => this.isActive() && this.logical.getActivePath() === 'relay', canAttempt: () => this.isActive() && !this.operationInFlight, beginOperation: () => (this.operationInFlight = true), - migrate: (client, path) => this.logical.migrateTo(client, path), + migrate: (client, path, abort) => this.logical.migrateTo(client, path, undefined, abort), onDirectMigrated: async () => { this.leaseRotation.clear() this.relayRotationPending = false @@ -195,6 +195,7 @@ export class MobileEndpointSupervisor { stop(): void { this.stopped = true + this.directProbe.stop() this.unsubscribeState?.() this.unsubscribeState = null this.backgroundGrace.stop() From d969af9ecc2d98d2fc52af4d214e7e8c630f7c1f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:48 -0700 Subject: [PATCH 31/43] perf(jira): preserve replacement attachment download singleflight (#18944) --- .../attachment-image-cache-generation.test.ts | 57 +++++++++++++++++++ src/main/jira/attachment-image-cache.ts | 5 +- 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/main/jira/attachment-image-cache-generation.test.ts diff --git a/src/main/jira/attachment-image-cache-generation.test.ts b/src/main/jira/attachment-image-cache-generation.test.ts new file mode 100644 index 00000000000..e92ca5c5a53 --- /dev/null +++ b/src/main/jira/attachment-image-cache-generation.test.ts @@ -0,0 +1,57 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + _resetAttachmentImageCache, + clearAttachmentImagesForSite, + getCachedAttachmentDataUrl, + loadAttachmentDataUrlWithCache +} from './attachment-image-cache' + +type Image = { dataUrl: string; byteSize: number } | null +function deferredImage() { + let resolve!: (image: Image) => void + let reject!: (error: Error) => void + const promise = new Promise((done, fail) => { + resolve = done + reject = fail + }) + return { promise, resolve, reject } +} + +beforeEach(_resetAttachmentImageCache) + +describe.each(['site', 'all'] as const)('attachment download after clearing %s', (scope) => { + it.each(['success', 'empty', 'failure'] as const)( + 'keeps the replacement singleflight when the old download completes with %s', + async (outcome) => { + const old = deferredImage() + const replacement = deferredImage() + let downloads = 0 + const load = () => { + downloads += 1 + return downloads === 1 ? old.promise : replacement.promise + } + const args = { siteId: 'site-a', attachmentId: 'image-1', load } + const first = loadAttachmentDataUrlWithCache(args).catch(() => 'old failure') + clearAttachmentImagesForSite(scope === 'site' ? 'site-a' : undefined) + const second = loadAttachmentDataUrlWithCache(args) + expect(downloads).toBe(2) + + if (outcome === 'failure') { + old.reject(new Error('old failure')) + } else { + old.resolve(outcome === 'empty' ? null : { dataUrl: 'old image', byteSize: 3 }) + } + expect(await first).toBe( + outcome === 'failure' ? 'old failure' : outcome === 'empty' ? null : 'old image' + ) + expect(getCachedAttachmentDataUrl('site-a', 'image-1')).toBeNull() + + const third = loadAttachmentDataUrlWithCache(args) + expect(downloads).toBe(2) + replacement.resolve({ dataUrl: 'new image', byteSize: 3 }) + expect(await second).toBe('new image') + expect(await third).toBe('new image') + expect(getCachedAttachmentDataUrl('site-a', 'image-1')).toBe('new image') + } + ) +}) diff --git a/src/main/jira/attachment-image-cache.ts b/src/main/jira/attachment-image-cache.ts index f9fdbfe3f7d..9d118ffdc9c 100644 --- a/src/main/jira/attachment-image-cache.ts +++ b/src/main/jira/attachment-image-cache.ts @@ -133,7 +133,10 @@ export async function loadAttachmentDataUrlWithCache(args: { } return loaded.dataUrl } finally { - inFlight.delete(key) + // A cleared generation no longer owns the current download's singleflight slot. + if (currentEpoch(args.siteId) === epochAtStart) { + inFlight.delete(key) + } } })() From 445c1aeaaf7c2cd43a175658c0b8a59117cb292f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:53 -0700 Subject: [PATCH 32/43] perf(speech): reuse the model download idle timer (#18945) --- .../model-manager-stream-cleanup.test.ts | 24 ++++++++++++++----- src/main/speech/speech-model-http-download.ts | 7 ++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/main/speech/model-manager-stream-cleanup.test.ts b/src/main/speech/model-manager-stream-cleanup.test.ts index dcebaca37a3..34b7aa893d8 100644 --- a/src/main/speech/model-manager-stream-cleanup.test.ts +++ b/src/main/speech/model-manager-stream-cleanup.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync } from 'node:fs' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { PassThrough } from 'node:stream' @@ -34,15 +34,17 @@ describe('ModelManager stream cleanup', () => { netRequestMock.mockReset() }) - it('removes response progress listeners after a model download finishes', async () => { + it('reuses the idle timer and removes progress listeners after a fragmented download', async () => { const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-')) + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + const timeoutSpy = vi.spyOn(globalThis, 'setTimeout') try { const response = new PassThrough() as PassThrough & { statusCode: number headers: Record } response.statusCode = 200 - response.headers = { 'content-length': '4' } + response.headers = { 'content-length': '1000' } const responseHandlers: ((response: unknown) => void)[] = [] const request = { abort: vi.fn(() => request), @@ -66,16 +68,26 @@ describe('ModelManager stream cleanup', () => { const download = manager.downloadFile( 'https://example.com/model.bin', join(dir, 'model.bin'), - 4, + 1000, 'm', () => false ) - response.write(Buffer.from('ab')) - response.end(Buffer.from('cd')) + await vi.advanceTimersByTimeAsync(60_000) + for (let index = 0; index < 1000; index += 1) { + response.write(Buffer.from('a')) + } + await vi.advanceTimersByTimeAsync(119_999) + expect(request.abort).not.toHaveBeenCalled() + response.end() await expect(download).resolves.toBeUndefined() expect(response.listenerCount('data')).toBe(0) + expect(vi.getTimerCount()).toBe(0) + expect(readFileSync(join(dir, 'model.bin'), 'utf8')).toBe('a'.repeat(1000)) + expect(timeoutSpy.mock.calls.filter(([, delay]) => delay === 120_000)).toHaveLength(1) } finally { + timeoutSpy.mockRestore() + vi.useRealTimers() rmSync(dir, { recursive: true, force: true }) } }) diff --git a/src/main/speech/speech-model-http-download.ts b/src/main/speech/speech-model-http-download.ts index ae2bae9648e..3bd2f24ab97 100644 --- a/src/main/speech/speech-model-http-download.ts +++ b/src/main/speech/speech-model-http-download.ts @@ -71,8 +71,11 @@ export abstract class SpeechModelHttpDownload { request = null } const resetIdleTimeout = (): void => { - clearIdleTimeout() - idleTimeout = setTimeout(onRequestTimeout, DOWNLOAD_IDLE_TIMEOUT_MS) + if (idleTimeout) { + idleTimeout.refresh() + } else { + idleTimeout = setTimeout(onRequestTimeout, DOWNLOAD_IDLE_TIMEOUT_MS) + } } const resolveOnce = (): void => { if (settled) { From 56626e7daad08b554ad124b586d6082629d886cc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:03:58 -0700 Subject: [PATCH 33/43] perf(ssh): reuse and release relay startup buffers (#18953) * perf(ssh): reuse the searched relay startup prefix * perf(ssh): release startup banners after relay readiness --- .../scripts/benchmark-sentinel-retention.mjs | 72 +++++++++++++++++++ src/main/ssh/ssh-relay-deploy-helpers.ts | 3 +- .../ssh-relay-sentinel-copy-budget.test.ts | 62 ++++++++++++++++ 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 config/scripts/benchmark-sentinel-retention.mjs create mode 100644 src/main/ssh/ssh-relay-sentinel-copy-budget.test.ts diff --git a/config/scripts/benchmark-sentinel-retention.mjs b/config/scripts/benchmark-sentinel-retention.mjs new file mode 100644 index 00000000000..93564eeac01 --- /dev/null +++ b/config/scripts/benchmark-sentinel-retention.mjs @@ -0,0 +1,72 @@ +import { strict as assert } from 'node:assert' +import { EventEmitter } from 'node:events' +import { mkdtemp, rm } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { build } from 'esbuild' + +if (!global.gc) { + throw new Error('Run with node --expose-gc') +} +const root = resolve(import.meta.dirname, '../..') +const directory = await mkdtemp(join(tmpdir(), 'orca-sentinel-retention-')) +const output = join(directory, 'sentinel.cjs') +try { + await build({ + stdin: { + contents: `export {waitForSentinel} from './src/main/ssh/ssh-relay-deploy-helpers'; +export {RELAY_SENTINEL} from './src/main/ssh/relay-protocol';`, + resolveDir: root, + loader: 'ts' + }, + bundle: true, + platform: 'node', + format: 'cjs', + packages: 'external', + banner: { + js: `var require = require('node:module').createRequire(${JSON.stringify(join(root, 'package.json'))});` + }, + outfile: output + }) + const { waitForSentinel, RELAY_SENTINEL } = createRequire(import.meta.url)(output) + const held = [] + const banners = [] + for (let i = 0; i < 100; i++) { + const channel = Object.assign(new EventEmitter(), { + stderr: new EventEmitter(), + stdin: { write: () => true }, + close: () => {} + }) + const pending = waitForSentinel(channel) + banners.push(feedBanner(channel)) + channel.emit('data', Buffer.from(RELAY_SENTINEL)) + const transport = await pending + const received = [] + transport.onData((bytes) => received.push(bytes.toString())) + channel.emit('data', Buffer.from('frame')) + assert.deepEqual(received, ['frame']) + held.push({ channel, transport }) + } + await new Promise((resolve) => setImmediate(resolve)) + for (let i = 0; i < 5; i++) { + global.gc() + } + const retained = banners.filter((reference) => reference.deref() !== undefined).length + console.log( + JSON.stringify({ + connections: held.length, + bannerBytes: 65536, + retainedBannerBuffers: retained, + retainedBannerBytes: retained * 65536 + }) + ) +} finally { + await rm(directory, { recursive: true, force: true }) +} + +function feedBanner(channel) { + const banner = Buffer.alloc(65536, 120) + channel.emit('data', banner) + return new WeakRef(banner.buffer) +} diff --git a/src/main/ssh/ssh-relay-deploy-helpers.ts b/src/main/ssh/ssh-relay-deploy-helpers.ts index a035133ddc5..f9a8f167752 100644 --- a/src/main/ssh/ssh-relay-deploy-helpers.ts +++ b/src/main/ssh/ssh-relay-deploy-helpers.ts @@ -209,6 +209,7 @@ export function waitForSentinel( const afterSentinelOffset = sentinelIdx + RELAY_SENTINEL_BUFFER.length - bufferedStdout.length const afterSentinel = data.subarray(Math.max(0, afterSentinelOffset)) + bufferedStdout = Buffer.alloc(0) if (afterSentinel.length > 0) { pendingAfterSentinel = afterSentinel @@ -258,7 +259,7 @@ export function waitForSentinel( return } - bufferedStdout = bufferedStdout.length === 0 ? data : Buffer.concat([bufferedStdout, data]) + bufferedStdout = startupStdout }) }) } diff --git a/src/main/ssh/ssh-relay-sentinel-copy-budget.test.ts b/src/main/ssh/ssh-relay-sentinel-copy-budget.test.ts new file mode 100644 index 00000000000..b05c8a71d92 --- /dev/null +++ b/src/main/ssh/ssh-relay-sentinel-copy-budget.test.ts @@ -0,0 +1,62 @@ +import { EventEmitter } from 'node:events' +import type { ClientChannel } from 'ssh2' +import { expect, it, vi } from 'vitest' +import { RELAY_SENTINEL } from './relay-protocol' +import { waitForSentinel } from './ssh-relay-deploy-helpers' + +it.each([1, 256])('copies each startup prefix once across %i chunks', async (chunks) => { + const channel = Object.assign(new EventEmitter(), { + stderr: new EventEmitter(), + stdin: { write: vi.fn(() => true) }, + close: vi.fn(), + pause: vi.fn(), + resume: vi.fn() + }) + const pending = waitForSentinel(channel as unknown as ClientChannel) + const chunk = Buffer.alloc((64 * 1024) / chunks, 120) + const concat = vi.spyOn(Buffer, 'concat') + let calls = 0 + let copied = 0 + try { + for (let i = 0; i < chunks; i++) { + channel.emit('data', chunk) + } + calls = concat.mock.calls.length + copied = concat.mock.calls.reduce( + (sum, [buffers]) => sum + buffers.reduce((bytes, buffer) => bytes + buffer.length, 0), + 0 + ) + } finally { + concat.mockRestore() + } + channel.emit('data', Buffer.from(`${RELAY_SENTINEL}first-frame`)) + const transport = await pending + const received: string[] = [] + transport.onData((bytes) => received.push(bytes.toString())) + expect(received).toEqual(['first-frame']) + expect(channel.close).not.toHaveBeenCalled() + expect(calls).toBe(chunks - 1) + expect(copied).toBe(chunk.length * ((chunks * (chunks + 1)) / 2 - 1)) +}) + +it.each(Array.from({ length: RELAY_SENTINEL.length + 1 }, (_, i) => i))( + 'preserves the marker and binary payload when split at byte %i', + async (split) => { + const channel = Object.assign(new EventEmitter(), { + stderr: new EventEmitter(), + stdin: { write: vi.fn(() => true) }, + close: vi.fn() + }) + const pending = waitForSentinel(channel as unknown as ClientChannel) + const marker = Buffer.from(RELAY_SENTINEL) + const payload = Buffer.from([0, 255, 128, 10, 13, 1]) + channel.emit('data', Buffer.alloc(63 * 1024, 120)) + channel.emit('data', marker.subarray(0, split)) + channel.emit('data', Buffer.concat([marker.subarray(split), payload])) + const transport = await pending + const received: Buffer[] = [] + transport.onData((bytes) => received.push(bytes)) + expect(Buffer.concat(received)).toEqual(payload) + expect(channel.close).not.toHaveBeenCalled() + } +) From 5cc432eead0729f711cf9fde977dfeef2b46dda7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:03 -0700 Subject: [PATCH 34/43] perf(ssh): reuse streamed response idle timers (#18956) --- ...sh-file-stream-inactivity-deadline.test.ts | 87 +++++++++++++++++++ .../ssh-file-stream-inactivity-deadline.ts | 5 +- .../ssh/ssh-git-response-stream-reader.ts | 5 +- .../ssh/ssh-git-stream-idle-timer.test.ts | 80 +++++++++++++++++ 4 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 src/main/ssh/ssh-file-stream-inactivity-deadline.test.ts create mode 100644 src/main/ssh/ssh-git-stream-idle-timer.test.ts diff --git a/src/main/ssh/ssh-file-stream-inactivity-deadline.test.ts b/src/main/ssh/ssh-file-stream-inactivity-deadline.test.ts new file mode 100644 index 00000000000..f87d1e6f778 --- /dev/null +++ b/src/main/ssh/ssh-file-stream-inactivity-deadline.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createSshFileStreamInactivityDeadline } from './ssh-file-stream-inactivity-deadline' +import type { SystemPowerLifecycleListener } from '../system-power-lifecycle' + +afterEach(() => { + vi.restoreAllMocks() + vi.useRealTimers() +}) + +describe('SSH file stream inactivity timer', () => { + it('reuses one timer while retaining the deadline of the latest chunk', () => { + vi.useFakeTimers() + const allocate = vi.spyOn(globalThis, 'setTimeout') + const onTimeout = vi.fn() + const unsubscribe = vi.fn() + const deadline = createSshFileStreamInactivityDeadline(onTimeout, (listener) => { + listener.onResume() + return unsubscribe + }) + deadline.reset() + vi.advanceTimersByTime(30_000) + for (let chunk = 0; chunk < 1000; chunk += 1) { + deadline.reset() + } + expect(allocate).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(59_999) + expect(onTimeout).not.toHaveBeenCalled() + vi.advanceTimersByTime(1) + expect(onTimeout).toHaveBeenCalledTimes(1) + deadline.clear() + expect(unsubscribe).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('releases on suspend and creates a fresh timer on resume', () => { + vi.useFakeTimers() + const allocate = vi.spyOn(globalThis, 'setTimeout') + const onTimeout = vi.fn() + let power!: SystemPowerLifecycleListener + const deadline = createSshFileStreamInactivityDeadline(onTimeout, (listener) => { + power = listener + listener.onResume() + return vi.fn() + }) + deadline.reset() + vi.advanceTimersByTime(30_000) + power.onSuspend() + for (let chunk = 0; chunk < 1000; chunk += 1) { + deadline.reset() + } + expect(vi.getTimerCount()).toBe(0) + vi.advanceTimersByTime(120_000) + expect(onTimeout).not.toHaveBeenCalled() + power.onResume() + expect(allocate).toHaveBeenCalledTimes(2) + vi.advanceTimersByTime(59_999) + expect(onTimeout).not.toHaveBeenCalled() + vi.advanceTimersByTime(1) + expect(onTimeout).toHaveBeenCalledTimes(1) + deadline.clear() + expect(vi.getTimerCount()).toBe(0) + }) + + it('clears the timer and subscription and supports a later reset', () => { + vi.useFakeTimers() + const onTimeout = vi.fn() + const unsubscribe = vi.fn() + const subscribe = vi.fn((listener: SystemPowerLifecycleListener) => { + listener.onResume() + return unsubscribe + }) + const deadline = createSshFileStreamInactivityDeadline(onTimeout, subscribe) + deadline.reset() + deadline.clear() + deadline.clear() + expect(unsubscribe).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + vi.advanceTimersByTime(120_000) + expect(onTimeout).not.toHaveBeenCalled() + deadline.reset() + expect(subscribe).toHaveBeenCalledTimes(2) + expect(vi.getTimerCount()).toBe(1) + deadline.clear() + expect(unsubscribe).toHaveBeenCalledTimes(2) + expect(vi.getTimerCount()).toBe(0) + }) +}) diff --git a/src/main/ssh/ssh-file-stream-inactivity-deadline.ts b/src/main/ssh/ssh-file-stream-inactivity-deadline.ts index 5470e8121fe..cbb9839f9b3 100644 --- a/src/main/ssh/ssh-file-stream-inactivity-deadline.ts +++ b/src/main/ssh/ssh-file-stream-inactivity-deadline.ts @@ -24,10 +24,13 @@ export function createSshFileStreamInactivityDeadline( } } const arm = (): void => { - clearTimer() if (suspended) { return } + if (timer) { + timer.refresh() + return + } timer = setTimeout(onTimeout, SSH_FILE_STREAM_INACTIVITY_TIMEOUT_MS) timer.unref?.() } diff --git a/src/main/ssh/ssh-git-response-stream-reader.ts b/src/main/ssh/ssh-git-response-stream-reader.ts index 6a50dc28a72..0a8b26aa779 100644 --- a/src/main/ssh/ssh-git-response-stream-reader.ts +++ b/src/main/ssh/ssh-git-response-stream-reader.ts @@ -90,7 +90,10 @@ export function requestGitStreamable( // killed, but a wedged stream (no frames arriving) rejects instead of // hanging the caller forever. const armInactivity = (): void => { - clearInactivity() + if (inactivityTimer) { + inactivityTimer.refresh() + return + } inactivityTimer = setTimeout(() => { fail( new GitResponseStreamError( diff --git a/src/main/ssh/ssh-git-stream-idle-timer.test.ts b/src/main/ssh/ssh-git-stream-idle-timer.test.ts new file mode 100644 index 00000000000..7cd5207c63a --- /dev/null +++ b/src/main/ssh/ssh-git-stream-idle-timer.test.ts @@ -0,0 +1,80 @@ +import { expect, it, vi } from 'vitest' +import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' +import { requestGitStreamable } from './ssh-git-response-stream-reader' + +it.each(['end', 'abort', 'timeout'] as const)( + 'reuses the idle deadline across 1000 chunks and cleans up on %s', + async (finish) => { + vi.useFakeTimers() + const setTimer = vi.spyOn(globalThis, 'setTimeout') + try { + const listeners = new Map) => void>() + const controller = new AbortController() + const content = 'x'.repeat(998) + const encoded = Buffer.from(JSON.stringify(content)) + const notify = vi.fn() + const mux = { + request: vi.fn(async () => ({ + __orcaGitResponseStream: { streamId: 7, totalBytes: encoded.length, chunkCount: 1000 } + })), + isDisposed: () => false, + notify, + onDispose: () => () => {}, + onNotificationByMethod: ( + method: string, + callback: (params: Record) => void + ) => { + listeners.set(method, callback) + return () => listeners.delete(method) + } + } + const promise = requestGitStreamable( + mux as unknown as SshChannelMultiplexer, + 'git.diff', + {}, + { + signal: controller.signal + } + ) + const outcome = promise.then( + (value) => ({ value }), + (error: Error) => ({ error: error.message }) + ) + await vi.advanceTimersByTimeAsync(15_000) + for (let seq = 0; seq < encoded.length; seq++) { + listeners.get('git.responseChunk')!({ + streamId: 7, + seq, + data: encoded.subarray(seq, seq + 1).toString('base64') + }) + } + await vi.advanceTimersByTimeAsync(29_999) + expect(listeners.size).toBe(3) + expect(notify.mock.calls.filter(([method]) => method === 'git.responseAck')).toHaveLength( + 1000 + ) + const allocations = setTimer.mock.calls.filter(([, delay]) => delay === 30_000).length + if (finish === 'end') { + listeners.get('git.responseEnd')!({ streamId: 7 }) + expect(await outcome).toEqual({ value: content }) + } else if (finish === 'abort') { + controller.abort() + expect(await outcome).toEqual({ error: 'Request was cancelled' }) + } else { + await vi.advanceTimersByTimeAsync(1) + expect(await outcome).toEqual({ + error: 'Git response stream stalled (>30000ms without data)' + }) + } + expect(allocations).toBe(1) + expect(vi.getTimerCount()).toBe(0) + expect(listeners.size).toBe(0) + expect( + notify.mock.calls.filter(([method]) => method === 'git.cancelResponseStream') + ).toHaveLength(finish === 'end' ? 0 : 1) + } finally { + setTimer.mockRestore() + vi.useRealTimers() + } + } +) From 0a573ceac88e05bc729ebcee925ec9e238265f33 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:07 -0700 Subject: [PATCH 35/43] perf(browser): reuse decoded single-chunk upload buffers (#18960) --- .../browser-client-upload-transfer.test.ts | 37 ++++++++++++++++++- .../browser/browser-client-upload-transfer.ts | 2 +- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/main/browser/browser-client-upload-transfer.test.ts b/src/main/browser/browser-client-upload-transfer.test.ts index fdc129618e8..84bb649abd3 100644 --- a/src/main/browser/browser-client-upload-transfer.test.ts +++ b/src/main/browser/browser-client-upload-transfer.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { BrowserClientHostCommandEvent } from '../../shared/browser-client-host-protocol' import { @@ -102,3 +102,38 @@ describe('readBrowserClientUploadPaths', () => { ) }) }) + +it.each([0, 1, 128 * 1024])( + 'avoids recopying 16 single-chunk uploads of %i bytes', + async (size) => { + const source = Buffer.alloc(size, 171) + const response = { + contentBase64: source.toString('base64'), + bytesRead: size, + totalBytes: size, + eof: true + } + const remotePaths = Array.from({ length: 16 }, (_, i) => `file-${i}.bin`) + const request = vi.fn(async () => response) + const concat = vi.spyOn(Buffer, 'concat') + let copies = 0 + let files: Awaited> + try { + files = await fetchBrowserClientUploadFiles({ request, event, remotePaths }) + copies = concat.mock.calls.length + } finally { + concat.mockRestore() + } + expect(copies).toBe(0) + expect(request).toHaveBeenCalledTimes(16) + expect(files.map((file) => file.remotePath)).toEqual(remotePaths) + for (const file of files) { + expect(file.contents).toEqual(source) + } + if (size > 0) { + files[0].contents[0] = 0 + expect(files[1].contents[0]).toBe(171) + expect(source[0]).toBe(171) + } + } +) diff --git a/src/main/browser/browser-client-upload-transfer.ts b/src/main/browser/browser-client-upload-transfer.ts index 1863f7b9f75..f85af071633 100644 --- a/src/main/browser/browser-client-upload-transfer.ts +++ b/src/main/browser/browser-client-upload-transfer.ts @@ -74,7 +74,7 @@ export async function fetchBrowserClientUploadFiles(options: { throw new Error('browser_client_upload_transfer_stalled') } } - files.push({ remotePath, contents: Buffer.concat(chunks) }) + files.push({ remotePath, contents: chunks.length === 1 ? chunks[0] : Buffer.concat(chunks) }) } return files } From 8ed81ceb8d53d5d057381fb1cee0fc41916dbc9d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:11 -0700 Subject: [PATCH 36/43] perf(tabs): index saved tab order during hydration repair (#18964) --- config/scripts/benchmark-tab-group-repair.mjs | 80 +++++++++++++++++++ .../slices/tab-group-reference-repair.test.ts | 54 +++++++++++++ .../slices/tab-group-reference-repair.ts | 3 +- 3 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 config/scripts/benchmark-tab-group-repair.mjs create mode 100644 src/renderer/src/store/slices/tab-group-reference-repair.test.ts diff --git a/config/scripts/benchmark-tab-group-repair.mjs b/config/scripts/benchmark-tab-group-repair.mjs new file mode 100644 index 00000000000..17a1161fc4c --- /dev/null +++ b/config/scripts/benchmark-tab-group-repair.mjs @@ -0,0 +1,80 @@ +import { strict as assert } from 'node:assert' +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' + +const root = resolve(import.meta.dirname, '../..') +const source = join(root, 'src/renderer/src/store/slices/tab-group-reference-repair.ts') +const directory = await mkdtemp(join(tmpdir(), 'orca-tab-repair-')) +const current = await readFile(source, 'utf8') +const indexed = `const orderedTabIds = new Set(group.tabOrder) + const missingTabIds = ownedTabIds.filter((tabId) => !orderedTabIds.has(tabId))` +assert(current.includes(indexed), 'Expected indexed implementation') +try { + const implementations = [] + for (const baseline of [true, false]) { + const outfile = join(directory, baseline ? 'before.cjs' : 'after.cjs') + await build({ + stdin: { + contents: baseline + ? current.replace( + indexed, + 'const missingTabIds = ownedTabIds.filter((tabId) => !group.tabOrder.includes(tabId))' + ) + : current, + resolveDir: resolve(source, '..'), + loader: 'ts' + }, + bundle: true, + platform: 'node', + format: 'cjs', + outfile, + alias: { '@': join(root, 'src/renderer/src') } + }) + implementations.push(createRequire(import.meta.url)(outfile).appendOwnedTabIdsToGroups) + } + const rows = [] + for (const count of [1, 10, 100, 1_000, 10_000]) { + for (const missing of [false, true]) { + const ids = Array.from({ length: count }, (_, i) => `tab-${i}`) + const groups = [ + { id: 'group', worktreeId: 'workspace', activeTabId: null, tabOrder: ids, recentTabIds: [] } + ] + const owners = new Map(ids.map((id) => [missing ? `missing-${id}` : id, 'group'])) + assert.deepEqual(implementations[0](groups, owners), implementations[1](groups, owners)) + const iterations = Math.max(1, Math.floor(10_000 / count)) + const samples = [[], []] + for (let sample = -3; sample < 11; sample++) { + for (const index of sample % 2 === 0 ? [0, 1] : [1, 0]) { + const start = performance.now() + for (let i = 0; i < iterations; i++) { + implementations[index](groups, owners) + } + const elapsed = (performance.now() - start) / iterations + if (sample >= 0) { + samples[index].push(elapsed) + } + } + } + rows.push({ + count, + missing, + iterations, + beforeMs: samples[0].sort((a, b) => a - b)[5], + afterMs: samples[1].sort((a, b) => a - b)[5] + }) + } + } + console.log( + JSON.stringify( + { node: process.version, platform: process.platform, samples: 11, warmups: 3, rows }, + null, + 2 + ) + ) +} finally { + await rm(directory, { recursive: true, force: true }) +} diff --git a/src/renderer/src/store/slices/tab-group-reference-repair.test.ts b/src/renderer/src/store/slices/tab-group-reference-repair.test.ts new file mode 100644 index 00000000000..a7cb3dca125 --- /dev/null +++ b/src/renderer/src/store/slices/tab-group-reference-repair.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import type { TabGroup } from '../../../../shared/tab-types' +import { appendOwnedTabIdsToGroups } from './tab-group-reference-repair' + +function group(id: string, tabOrder: string[]): TabGroup { + return { id, worktreeId: 'workspace', activeTabId: null, tabOrder, recentTabIds: [] } +} + +describe('appendOwnedTabIdsToGroups', () => { + it('preserves existing order, duplicates, and untouched group identities', () => { + const complete = group('complete', ['b', 'a', 'a']) + const missing = group('missing', ['stale', 'c']) + const unowned = group('unowned', ['external']) + const owners = new Map([ + ['a', 'complete'], + ['b', 'complete'], + ['d', 'missing'], + ['c', 'missing'], + ['e', 'missing'], + ['elsewhere', 'absent'] + ]) + const result = appendOwnedTabIdsToGroups([complete, missing, unowned], owners) + expect(result).toEqual([complete, { ...missing, tabOrder: ['stale', 'c', 'd', 'e'] }, unowned]) + expect(result[0]).toBe(complete) + expect(result[2]).toBe(unowned) + expect(missing.tabOrder).toEqual(['stale', 'c']) + }) + + it.each([false, true])('bounds saved-order reads with missing tabs: %s', (missing) => { + const count = 1_000 + const ids = Array.from({ length: count }, (_, i) => `tab-${i}`) + let reads = 0 + const order = new Proxy(ids, { + get(target, property, receiver) { + if (typeof property === 'string' && /^\d+$/.test(property)) { + reads++ + } + return Reflect.get(target, property, receiver) + } + }) + const original = group('group', order) + const ownedIds = missing ? ids.map((id) => `missing-${id}`) : ids + const result = appendOwnedTabIdsToGroups( + [original], + new Map(ownedIds.map((id) => [id, original.id])) + ) + const repairReads = reads + expect(result[0].tabOrder).toEqual(missing ? [...ids, ...ownedIds] : ids) + if (!missing) { + expect(result[0]).toBe(original) + } + expect(repairReads).toBeLessThanOrEqual(count * 2) + }) +}) diff --git a/src/renderer/src/store/slices/tab-group-reference-repair.ts b/src/renderer/src/store/slices/tab-group-reference-repair.ts index 2bf1c468093..77d6dd38c81 100644 --- a/src/renderer/src/store/slices/tab-group-reference-repair.ts +++ b/src/renderer/src/store/slices/tab-group-reference-repair.ts @@ -72,7 +72,8 @@ export function appendOwnedTabIdsToGroups( if (!ownedTabIds) { return group } - const missingTabIds = ownedTabIds.filter((tabId) => !group.tabOrder.includes(tabId)) + const orderedTabIds = new Set(group.tabOrder) + const missingTabIds = ownedTabIds.filter((tabId) => !orderedTabIds.has(tabId)) return missingTabIds.length > 0 ? { ...group, tabOrder: [...group.tabOrder, ...missingTabIds] } : group From 78e3721c2331ba54bbfa2dbb3065bfa019fa5b58 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:16 -0700 Subject: [PATCH 37/43] perf(palette): reuse allowed quality arrays during matching (#18966) --- .../match-field-allocation.test.ts | 90 +++++++++++++++++++ .../src/lib/palette-match/match-field.ts | 26 +++--- 2 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/lib/palette-match/match-field-allocation.test.ts diff --git a/src/renderer/src/lib/palette-match/match-field-allocation.test.ts b/src/renderer/src/lib/palette-match/match-field-allocation.test.ts new file mode 100644 index 00000000000..5b0021d2e71 --- /dev/null +++ b/src/renderer/src/lib/palette-match/match-field-allocation.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it, vi } from 'vitest' +import { + indexPaletteField, + type PaletteIdentifierKind, + type PaletteFieldProfile +} from './indexed-field' +import { matchPaletteField } from './match-field' +import { createPaletteQueryToken } from './palette-query' + +describe('palette field quality allocation', () => { + it.each(['scan', 's', '123', 'scna', 'zzz'])( + 'does not allocate a Set per field for %s', + (query) => { + const profiles: PaletteFieldProfile[] = [ + 'structured-label', + 'identifier', + 'path', + 'prose', + 'exact-alias' + ] + const fields = Array.from({ length: 1_000 }, (_, i) => + indexPaletteField({ + id: String(i), + profile: profiles[i % profiles.length], + text: 'scan daily 1234 workspace', + ...(i % 2 === 0 ? { identifier: { kind: 'number' as const } } : {}) + })! + ) + const token = createPaletteQueryToken(query, 0) + let allocations = 0 + const NativeSet = globalThis.Set + class CountedSet extends NativeSet { + constructor(values?: Iterable | null) { + super(values) + allocations++ + } + } + vi.stubGlobal('Set', CountedSet) + try { + for (const field of fields) { + matchPaletteField(field, token) + } + } finally { + vi.unstubAllGlobals() + } + expect(allocations).toBe(0) + } + ) +}) + +describe('palette quality restrictions remain local to each match', () => { + it.each(['number', 'version', 'date', 'port', 'sha', 'key'])( + 'preserves prefix permissions for %s', + (kind) => { + const field = indexPaletteField({ + id: 'id', + profile: 'identifier', + text: '12345', + identifier: { kind } + })! + const prefix = createPaletteQueryToken('123', 0) + const exact = createPaletteQueryToken('12345', 0) + const expected = ['port', 'sha', 'key'].includes(kind) + ? { quality: 'field-prefix', ranges: [{ start: 0, end: 3 }] } + : null + expect(matchPaletteField(field, prefix)).toEqual(expected) + expect(matchPaletteField(field, exact)).toEqual({ + quality: 'field-exact', + ranges: [{ start: 0, end: 5 }] + }) + expect(matchPaletteField(field, prefix)).toEqual(expected) + } + ) + + it.each(['structured-label', 'identifier', 'path', 'prose', 'exact-alias'])( + 'preserves typo restrictions for %s without mutating the profile', + (profile) => { + const field = indexPaletteField({ id: 'id', profile, text: 'scan' })! + expect(matchPaletteField(field, createPaletteQueryToken('s', 0))).toEqual({ + quality: 'field-prefix', + ranges: [{ start: 0, end: 1 }] + }) + expect(matchPaletteField(field, createPaletteQueryToken('scam', 0))).toEqual( + ['structured-label', 'prose'].includes(profile) + ? { quality: 'typo', ranges: [{ start: 0, end: 4 }] } + : null + ) + } + ) +}) diff --git a/src/renderer/src/lib/palette-match/match-field.ts b/src/renderer/src/lib/palette-match/match-field.ts index f9015ec49a5..1744219524c 100644 --- a/src/renderer/src/lib/palette-match/match-field.ts +++ b/src/renderer/src/lib/palette-match/match-field.ts @@ -32,7 +32,7 @@ const SIGILS = new Set(['#', '!']) function allowedQualities( field: PaletteIndexedField, token: PaletteQueryToken -): ReadonlySet { +): readonly PaletteMatchQuality[] { let qualities = paletteProfileAllowedQualities(field.profile) if (field.identifier && !identifierKindAllowsPrefix(field.identifier.kind)) { qualities = qualities.filter((quality) => !PREFIX_QUALITIES.has(quality)) @@ -43,7 +43,7 @@ function allowedQualities( if (token.isIdentifierLike) { qualities = qualities.filter((quality) => quality !== 'typo') } - return new Set(qualities) + return qualities } /** `#123` must not reach a GitLab MR, and `!123` must not reach a GitHub PR. */ @@ -83,15 +83,15 @@ function toRanges(field: PaletteIndexedField, start: number, end: number): reado function matchLiteral( field: PaletteIndexedField, token: PaletteQueryToken, - qualities: ReadonlySet + qualities: readonly PaletteMatchQuality[] ): PaletteFieldMatch | null { const normalized = field.text.normalized const text = token.text - if (qualities.has('field-exact') && normalized === text) { + if (qualities.includes('field-exact') && normalized === text) { return { quality: 'field-exact', ranges: toRanges(field, 0, normalized.length) } } - if (qualities.has('word-exact')) { + if (qualities.includes('word-exact')) { const word = field.words.find((entry) => entry.text === text) if (word) { return { quality: 'word-exact', ranges: toRanges(field, word.start, word.end) } @@ -101,10 +101,10 @@ function matchLiteral( return { quality: 'word-exact', ranges: toRanges(field, atom.start, atom.end) } } } - if (qualities.has('field-prefix') && normalized.startsWith(text)) { + if (qualities.includes('field-prefix') && normalized.startsWith(text)) { return { quality: 'field-prefix', ranges: toRanges(field, 0, text.length) } } - if (qualities.has('word-prefix')) { + if (qualities.includes('word-prefix')) { const word = field.words.find((entry) => entry.text.startsWith(text)) const atom = field.atoms.find((entry) => normalized.startsWith(text, entry.start)) const start = word && atom ? Math.min(word.start, atom.start) : (word?.start ?? atom?.start) @@ -117,13 +117,13 @@ function matchLiteral( if (literalIndex === -1) { return null } - if (qualities.has('boundary-substring') && isWordStart(field, literalIndex)) { + if (qualities.includes('boundary-substring') && isWordStart(field, literalIndex)) { return { quality: 'boundary-substring', ranges: toRanges(field, literalIndex, literalIndex + text.length) } } - if (qualities.has('literal-substring')) { + if (qualities.includes('literal-substring')) { return { quality: 'literal-substring', ranges: toRanges(field, literalIndex, literalIndex + text.length) @@ -135,9 +135,9 @@ function matchLiteral( function matchCompact( field: PaletteIndexedField, token: PaletteQueryToken, - qualities: ReadonlySet + qualities: readonly PaletteMatchQuality[] ): PaletteFieldMatch | null { - if (!qualities.has('compact') || token.compact.length < MIN_COMPACT_LENGTH) { + if (!qualities.includes('compact') || token.compact.length < MIN_COMPACT_LENGTH) { return null } for (const atom of field.atoms) { @@ -152,9 +152,9 @@ function matchCompact( function matchTypo( field: PaletteIndexedField, token: PaletteQueryToken, - qualities: ReadonlySet + qualities: readonly PaletteMatchQuality[] ): PaletteFieldMatch | null { - if (!qualities.has('typo') || !token.isLetterOnly || !isPaletteTypoCandidate(token.text)) { + if (!qualities.includes('typo') || !token.isLetterOnly || !isPaletteTypoCandidate(token.text)) { return null } for (const word of field.words) { From 37427bfd1a7f88b015b50178318733a75f8ffd9f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:21 -0700 Subject: [PATCH 38/43] perf: remember equivalent session tab source identities (#18976) --- ...ession-write-subscriber-allocation.test.ts | 28 +++++++++++++++++++ .../src/lib/session-write-subscriber.ts | 5 ++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/lib/session-write-subscriber-allocation.test.ts b/src/renderer/src/lib/session-write-subscriber-allocation.test.ts index 9681af5a973..74c9c6db968 100644 --- a/src/renderer/src/lib/session-write-subscriber-allocation.test.ts +++ b/src/renderer/src/lib/session-write-subscriber-allocation.test.ts @@ -90,6 +90,34 @@ afterEach(() => { }) describe('session write subscriber allocation', () => { + it.each(['tabsByWorktree', 'unifiedTabsByWorktree'] as const)( + 'remembers an equivalent %s source before unrelated writes', + (field) => { + const harness = createHarness() + try { + harness.write(() => ({ [field]: { 'wt-1': [] } })) + vi.advanceTimersByTime(500) + harness.persisted.length = 0 + + harness.write(() => ({ [field]: { 'wt-1': [] } })) + const calls = countFilterCalls(() => { + for (let write = 0; write < 200; write += 1) { + harness.write(() => ({ runtimePaneTitlesByTabId: {} })) + } + }) + expect(calls).toBe(0) + vi.advanceTimersByTime(500) + expect(harness.persisted).toHaveLength(0) + + harness.write(() => ({ activeTabId: 'next-tab' })) + vi.advanceTimersByTime(500) + expect(harness.persisted).toHaveLength(1) + } finally { + harness.dispose() + } + } + ) + it('allocates nothing for store writes that touch no session field', () => { const harness = createHarness() try { diff --git a/src/renderer/src/lib/session-write-subscriber.ts b/src/renderer/src/lib/session-write-subscriber.ts index e0e366ef6ba..d54675e15ab 100644 --- a/src/renderer/src/lib/session-write-subscriber.ts +++ b/src/renderer/src/lib/session-write-subscriber.ts @@ -233,12 +233,13 @@ export function createSessionWriteSubscriber({ prev === null ? [...SESSION_RELEVANT_FIELDS] : SESSION_RELEVANT_FIELDS.filter((key) => prev?.[key] !== next[key]) + // Equivalent projections still consume the new source identities. + prevTabsSource = state.tabsByWorktree + prevUnifiedTabsSource = state.unifiedTabsByWorktree if (changedFields.length === 0 && pendingChangedFields.size === 0) { return } prev = next - prevTabsSource = state.tabsByWorktree - prevUnifiedTabsSource = state.unifiedTabsByWorktree for (const field of changedFields) { pendingChangedFields.add(field) } From 64374d5dffb79e6c3b2407b76db331cf7ac8217f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:04:26 -0700 Subject: [PATCH 39/43] perf(cli): skip impossible typo distance comparisons (#18977) --- src/cli/command-suggestion-budget.test.ts | 44 +++++++++++++++++++++++ src/cli/command-suggestion.ts | 19 +++++++--- 2 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 src/cli/command-suggestion-budget.test.ts diff --git a/src/cli/command-suggestion-budget.test.ts b/src/cli/command-suggestion-budget.test.ts new file mode 100644 index 00000000000..7902ad7fdba --- /dev/null +++ b/src/cli/command-suggestion-budget.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import * as distance from '../shared/edit-distance' +import { suggestCommands, unknownFlagData } from './command-suggestion' +import type { CommandSpec } from './command-spec' + +const specs: CommandSpec[] = [ + { path: ['list'], summary: '', usage: '', allowedFlags: [] }, + { path: ['remove'], summary: '', usage: '', allowedFlags: [], destructive: true } +] + +afterEach(() => vi.restoreAllMocks()) + +describe('suggestion distance work', () => { + it('does no distance calculations for a long command, including destructive intent', () => { + const spy = vi.spyOn(distance, 'levenshtein') + expect(suggestCommands(specs, ['x'.repeat(32_768)])).toEqual([]) + expect(spy).not.toHaveBeenCalled() + }) + + it('does no distance calculations for a long flag but still lists valid flags', () => { + const spy = vi.spyOn(distance, 'levenshtein') + expect(unknownFlagData('x'.repeat(32_768), ['worktree', 'json'])).toEqual({ + validFlags: ['json', 'worktree'], + suggestions: [], + nextSteps: ['Valid flags: --json, --worktree'] + }) + expect(spy).not.toHaveBeenCalled() + }) + + it('keeps the inclusive three-edit suggestion boundary', () => { + expect(suggestCommands(specs, ['listxxx'])).toEqual(['list']) + expect(unknownFlagData('jsonxxx', ['json']).suggestions).toEqual(['json']) + }) + + it('keeps the inclusive one-edit destructive intent boundary', () => { + expect(suggestCommands(specs, ['remov'])).toEqual(['remove']) + expect(suggestCommands(specs, ['remo'])).toEqual([]) + }) + + it('retains UTF-16 distance semantics at the length boundary', () => { + expect(unknownFlagData('json😀x', ['json']).suggestions).toEqual(['json']) + expect(unknownFlagData('json😀😀', ['json']).suggestions).toEqual([]) + }) +}) diff --git a/src/cli/command-suggestion.ts b/src/cli/command-suggestion.ts index 7b80138e2f3..9c935f694fa 100644 --- a/src/cli/command-suggestion.ts +++ b/src/cli/command-suggestion.ts @@ -37,7 +37,10 @@ function destructiveVerbs(specs: CommandSpec[]): Set { // input token is itself a near-miss of a destructive verb. #6303 function intendsDestruction(inputToken: string, verbs: Set): boolean { for (const verb of verbs) { - if (levenshtein(inputToken, verb) <= DESTRUCTIVE_INTENT_THRESHOLD) { + if ( + Math.abs(inputToken.length - verb.length) <= DESTRUCTIVE_INTENT_THRESHOLD && + levenshtein(inputToken, verb) <= DESTRUCTIVE_INTENT_THRESHOLD + ) { return true } } @@ -85,7 +88,9 @@ export function suggestCommands(specs: CommandSpec[], commandPath: string[]): st continue } seen.add(joined) - scored.push({ label: joined, distance: levenshtein(input, joined) }) + if (Math.abs(input.length - joined.length) <= SUGGESTION_THRESHOLD) { + scored.push({ label: joined, distance: levenshtein(input, joined) }) + } } } return rankByDistance(scored) @@ -106,9 +111,13 @@ export type FlagErrorData = { } function suggestFlags(flag: string, validFlags: string[]): string[] { - return rankByDistance( - validFlags.map((candidate) => ({ label: candidate, distance: levenshtein(flag, candidate) })) - ) + const scored: { label: string; distance: number }[] = [] + for (const candidate of validFlags) { + if (Math.abs(flag.length - candidate.length) <= SUGGESTION_THRESHOLD) { + scored.push({ label: candidate, distance: levenshtein(flag, candidate) }) + } + } + return rankByDistance(scored) } // Why: include the accepted set so agents can recover without another help call. From 97526f65adb590dc3790f00b646d5fc66c98914f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:59:33 -0700 Subject: [PATCH 40/43] fix(tests): stabilize divider viewport and pointer-capture event ordering (#19004) * fix(tests): size divider capture-loss viewport deterministically * test: advance pointer events before awaiting capture loss --- ...terminal-pane-divider-capture-loss.spec.ts | 40 +++++-------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/tests/e2e/terminal-pane-divider-capture-loss.spec.ts b/tests/e2e/terminal-pane-divider-capture-loss.spec.ts index 5f3bfca177b..8120c2a60e7 100644 --- a/tests/e2e/terminal-pane-divider-capture-loss.spec.ts +++ b/tests/e2e/terminal-pane-divider-capture-loss.spec.ts @@ -1,4 +1,4 @@ -import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import type { Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { splitActiveTerminalPane, @@ -22,32 +22,6 @@ type DividerGeometry = { test.use({ seedTestRepo: false }) -async function setFullscreen(electronApp: ElectronApplication, page: Page): Promise { - await expect - .poll(async () => { - try { - return await electronApp.evaluate(({ BrowserWindow }) => { - const window = BrowserWindow.getAllWindows()[0] - if (!window) { - return false - } - if (window.isMinimized()) { - window.restore() - } - window.show() - window.focus() - window.setFullScreen(true) - return window.isFullScreen() - }) - } catch { - return false - } - }) - .toBe(true) - await expect.poll(() => page.evaluate(() => innerWidth >= 1000 && innerHeight >= 700)).toBe(true) - await page.waitForTimeout(1200) -} - async function addTestRepo(page: Page, repoPath: string): Promise { const repoId = await page.evaluate(async (path) => { const result = await window.api.repos.add({ path }) @@ -122,17 +96,21 @@ function gridsMatch(geometry: DividerGeometry): boolean { } test('@headful keeps resizing after the divider loses pointer capture', async ({ - electronApp, orcaPage, testRepoPath }, testInfo) => { - await setFullscreen(electronApp, orcaPage) + // Keep the 260px drag above the fit floor regardless of the CI display resolution. + await orcaPage.setViewportSize({ width: 1600, height: 1000 }) await addTestRepo(orcaPage, testRepoPath) await ensureTerminalVisible(orcaPage, 30_000) await waitForActiveTerminalManager(orcaPage, 30_000) await splitActiveTerminalPane(orcaPage, 'vertical') await waitForPaneCount(orcaPage, 2, 30_000) + await expect + .poll(async () => (await readDividerGeometry(orcaPage)).second.width) + .toBeGreaterThan(400) + const divider = orcaPage.locator('.pane-divider.is-vertical').first() await expect(divider).toBeVisible() const box = await divider.boundingBox() @@ -170,11 +148,11 @@ test('@headful keeps resizing after the divider loses pointer capture', async ({ } element.releasePointerCapture(pointerId) }) + // Pending capture changes are dispatched with the next pointer event. + await orcaPage.mouse.move(startX + 260, startY, { steps: 10 }) await expect .poll(() => divider.evaluate((element) => Number(element.dataset.captureLossCount ?? '0'))) .toBe(1) - - await orcaPage.mouse.move(startX + 260, startY, { steps: 10 }) await orcaPage.mouse.up() await expect.poll(async () => gridsMatch(await readDividerGeometry(orcaPage))).toBe(true) const after = await readDividerGeometry(orcaPage) From 09ee4c1b1857484eddfb93d357163b3731d5c8f2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:07:46 -0700 Subject: [PATCH 41/43] fix(mobile): stop host streams after relay subscription cancellation (#18926) * fix(mobile): release cancelled relay stream subscriptions * fix(mobile): keep shared-token relay siblings live on unsubscribe nativeChat and terminal unsubscribe tokens are deterministic per view target, and the host evicts on duplicate registration. Skip the unsubscribe RPC while a live sibling on the same connection still owns that token. --- ...mobile-relay-browser-cancel-budget.test.ts | 109 ++++++++ .../mobile-relay-rpc-session.test.ts | 30 ++ .../src/transport/mobile-relay-rpc-session.ts | 1 + ...bile-relay-rpc-stream-cancellation.test.ts | 259 ++++++++++++++++++ .../src/transport/mobile-relay-rpc-streams.ts | 119 +++++++- .../rpc-client-terminal-subscription.ts | 8 +- 6 files changed, 509 insertions(+), 17 deletions(-) create mode 100644 mobile/src/transport/mobile-relay-browser-cancel-budget.test.ts create mode 100644 mobile/src/transport/mobile-relay-rpc-stream-cancellation.test.ts diff --git a/mobile/src/transport/mobile-relay-browser-cancel-budget.test.ts b/mobile/src/transport/mobile-relay-browser-cancel-budget.test.ts new file mode 100644 index 00000000000..59426fc42da --- /dev/null +++ b/mobile/src/transport/mobile-relay-browser-cancel-budget.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from 'vitest' +import { RuntimeBrowserScreencastController } from '../../../src/main/runtime/runtime-browser-screencast-controller' +import type { RuntimeBrowserCommands } from '../../../src/main/runtime/orca-runtime-browser' +import type { BrowserScreencastResult } from '../../../src/shared/runtime-types' +import { MobileRelayRpcStreams } from './mobile-relay-rpc-streams' +import type { RpcResponse } from './types' + +describe('relay browser cancellation resource budget', () => { + it.each([false, true])('stops host frames when cancellation precedes ready=%s', async (early) => { + const subscriptions = new Map void | Promise>() + const done = Promise.withResolvers() + const ready = Promise.withResolvers() + let sequence = 0 + let stopped = false + let frameSends = 0 + let frameBytes = 0 + let sendBinary: (bytes: Uint8Array) => boolean | void = () => false + let hostRun: Promise | undefined + const methods: string[] = [] + const cleanup = (id: string): void => { + const release = subscriptions.get(id) + subscriptions.delete(id) + void release?.() + } + const host = new RuntimeBrowserScreencastController({ + getCommands: () => + ({ + browserScreencast: async (_params, stream) => { + sendBinary = stream.sendBinary + return { + subscriptionId: 'server-stream', + ready: { type: 'ready', subscriptionId: 'server-stream', browserPageId: 'page' }, + session: { + done: done.promise, + stop: () => { + stopped = true + done.resolve() + } + }, + flushPendingFrame: () => {} + } + } + }) as RuntimeBrowserCommands, + registerSubscriptionCleanup: (id, release) => subscriptions.set(id, release), + cleanupSubscription: cleanup, + getDriver: () => ({ kind: 'idle' }), + setDriver: () => {}, + notifyRemoteViewersChanged: () => {} + }) + const streams = new MobileRelayRpcStreams({ + nextId: () => `request-${++sequence}`, + waitForConnected: async () => {}, + sendFrame: (request) => { + methods.push(request.method) + if (request.method === 'browser.screencast' && (request.params as { page?: string }).page) { + hostRun = host.start(request.params as Parameters[0], { + connectionId: 'relay-connection', + sendBinary: (bytes) => { + frameSends++ + frameBytes += bytes.byteLength + return true + }, + emit: (result: BrowserScreencastResult) => { + if (result.type === 'ready') { + ready.resolve({ + id: request.id, + ok: true, + streaming: true, + result, + _meta: { runtimeId: 'host' } + }) + } + } + }) + } else if (request.method === 'browser.screencast.unsubscribe') { + cleanup((request.params as { subscriptionId: string }).subscriptionId) + } + return true + } + }) + const cancel = streams.subscribe('browser.screencast', { page: 'page' }, () => {}) + try { + const response = await ready.promise + if (early) { + cancel() + } + streams.handleResponse(response) + if (!early) { + cancel() + } + for (let frame = 0; frame < 100; frame++) { + if (!stopped) { + sendBinary(new Uint8Array(65_536)) + } + } + expect({ stopped, subscriptions: subscriptions.size, frameSends, frameBytes }).toEqual({ + stopped: true, + subscriptions: 0, + frameSends: 0, + frameBytes: 0 + }) + expect(methods).toEqual(['browser.screencast', 'browser.screencast.unsubscribe']) + } finally { + cleanup('server-stream') + await hostRun + streams.clear() + } + }) +}) diff --git a/mobile/src/transport/mobile-relay-rpc-session.test.ts b/mobile/src/transport/mobile-relay-rpc-session.test.ts index 5887dffc73d..4bf617faf50 100644 --- a/mobile/src/transport/mobile-relay-rpc-session.test.ts +++ b/mobile/src/transport/mobile-relay-rpc-session.test.ts @@ -136,6 +136,36 @@ describe('mobile relay RPC session', () => { }) afterEach(() => vi.useRealTimers()) + it('releases stream listeners on failure even when close follows it', async () => { + const { session } = await authenticateSession() + const listener = vi.fn() + session.subscribe('runtime.clientEvents.subscribe', {}, listener) + await Promise.resolve() + const request = JSON.parse(fakes.sendText.mock.calls[0]![0] as string) as { id: string } + fakes.linkOptions!.onText( + JSON.stringify({ + id: request.id, + ok: true, + streaming: true, + result: { type: 'ready', subscriptionId: 'server-events' }, + _meta: { runtimeId: 'runtime-1' } + }) + ) + expect(listener).toHaveBeenCalledTimes(1) + fakes.linkOptions!.onError(new Error('relay lost')) + session.close() + fakes.linkOptions!.onText( + JSON.stringify({ + id: request.id, + ok: true, + streaming: true, + result: { type: 'event' }, + _meta: { runtimeId: 'runtime-1' } + }) + ) + expect(listener).toHaveBeenCalledTimes(1) + }) + it('requires exact resume observations and confirms by request ID before becoming connected', async () => { const { session, confirmationRequest, capabilityRequest } = await authenticateSession() diff --git a/mobile/src/transport/mobile-relay-rpc-session.ts b/mobile/src/transport/mobile-relay-rpc-session.ts index 203a0329192..67b50ea591e 100644 --- a/mobile/src/transport/mobile-relay-rpc-session.ts +++ b/mobile/src/transport/mobile-relay-rpc-session.ts @@ -294,6 +294,7 @@ export function connectMobileRelayRpcSession(args: { closed = true failure = error livenessWatchdog.stop(livenessIdentity) + streams.clear() link.close() pending.rejectAll(error) publishState(error instanceof MobileE2EEAuthenticationError ? 'auth-failed' : 'disconnected') diff --git a/mobile/src/transport/mobile-relay-rpc-stream-cancellation.test.ts b/mobile/src/transport/mobile-relay-rpc-stream-cancellation.test.ts new file mode 100644 index 00000000000..0a7a25dfbd7 --- /dev/null +++ b/mobile/src/transport/mobile-relay-rpc-stream-cancellation.test.ts @@ -0,0 +1,259 @@ +import { describe, expect, it, vi } from 'vitest' +import { MobileRelayRpcStreams } from './mobile-relay-rpc-streams' +import type { RpcResponse } from './types' + +function createStreams(waitForConnected = async () => {}) { + let sequence = 0 + const sendFrame = vi.fn((_request: { id: string; method: string; params?: unknown }) => true) + const streams = new MobileRelayRpcStreams({ + nextId: () => `request-${++sequence}`, + sendFrame, + waitForConnected + }) + return { streams, sendFrame } +} + +function response(id: string, result: unknown): RpcResponse { + return { id, ok: true, streaming: true, result, _meta: { runtimeId: 'test' } } +} + +const serverSubscriptions = [ + ['browser.screencast', 'browser.screencast.unsubscribe'], + ['runtime.clientEvents.subscribe', 'runtime.clientEvents.unsubscribe'] +] as const + +describe('mobile relay subscription cancellation', () => { + it.each(serverSubscriptions)('cleans up ready %s exactly once', async (method, unsubscribe) => { + const { streams, sendFrame } = createStreams() + const listener = vi.fn() + const cancel = streams.subscribe(method, {}, listener) + await Promise.resolve() + streams.handleResponse(response('request-1', { type: 'ready', subscriptionId: 'server-1' })) + cancel() + cancel() + expect(sendFrame.mock.calls).toEqual([ + [{ id: 'request-1', method, params: {} }], + [{ id: 'request-2', method: unsubscribe, params: { subscriptionId: 'server-1' } }] + ]) + expect(streams.handleResponse(response('request-1', { type: 'end' }))).toBe(false) + expect(listener).toHaveBeenCalledTimes(1) + }) + + it.each(serverSubscriptions)( + 'cleans up late-ready %s without calling disposed listeners', + async (method, unsubscribe) => { + const { streams, sendFrame } = createStreams() + const listener = vi.fn() + const cancel = streams.subscribe(method, {}, listener) + await Promise.resolve() + cancel() + cancel() + expect(sendFrame).toHaveBeenCalledTimes(1) + expect(streams.handleResponse(response('request-1', { type: 'starting' }))).toBe(true) + streams.handleResponse(response('request-1', { type: 'ready', subscriptionId: 'server-1' })) + expect(sendFrame).toHaveBeenLastCalledWith({ + id: 'request-2', + method: unsubscribe, + params: { subscriptionId: 'server-1' } + }) + expect( + streams.handleResponse(response('request-1', { type: 'ready', subscriptionId: 'server-1' })) + ).toBe(false) + expect(listener).not.toHaveBeenCalled() + } + ) + + it.each(['error', 'end', 'disconnect', 'completed'])( + 'forgets cancelled cleanup routes on %s', + async (ending) => { + const { streams, sendFrame } = createStreams() + const cancel = streams.subscribe('browser.screencast', {}, vi.fn()) + await Promise.resolve() + cancel() + if (ending === 'disconnect') { + streams.clear() + } else if (ending === 'completed') { + streams.handleResponse({ + id: 'request-1', + ok: true, + result: null, + _meta: { runtimeId: 'test' } + }) + } else if (ending === 'error') { + streams.handleResponse({ + id: 'request-1', + ok: false, + error: { code: 'unsupported', message: 'failed' }, + _meta: { runtimeId: 'test' } + }) + } else { + streams.handleResponse(response('request-1', { type: 'end', subscriptionId: 'server-1' })) + } + expect( + streams.handleResponse(response('request-1', { type: 'ready', subscriptionId: 'server-1' })) + ).toBe(false) + expect(sendFrame).toHaveBeenCalledTimes(1) + } + ) + + it.each([ + [ + 'terminal.subscribe', + { terminal: 'term', client: { id: 'phone' } }, + 'terminal.unsubscribe', + { subscriptionId: 'term:phone', client: { id: 'phone' } } + ], + [ + 'session.tabs.subscribe', + { worktree: 'id:workspace' }, + 'session.tabs.unsubscribe', + { worktree: 'id:workspace', subscriptionId: 'request-1' } + ], + [ + 'nativeChat.subscribe', + { subscriptionId: 'chat' }, + 'nativeChat.unsubscribe', + { subscriptionId: 'chat' } + ] + ])( + 'cancels %s using its request cleanup identity', + async (method, params, unsubscribe, unsubscribeParams) => { + const { streams, sendFrame } = createStreams() + const cancel = streams.subscribe(method as string, params, vi.fn()) + await Promise.resolve() + if (method === 'session.tabs.subscribe') { + streams.handleResponse(response('request-1', { type: 'snapshot' })) + } + cancel() + expect(sendFrame).toHaveBeenLastCalledWith({ + id: 'request-2', + method: unsubscribe, + params: unsubscribeParams + }) + } + ) + + it.each([ + 'terminal.subscribe', + 'browser.screencast', + 'runtime.clientEvents.subscribe', + 'session.tabs.subscribe', + 'nativeChat.subscribe' + ])('does not unsubscribe an unsent %s', async (method) => { + const wait = Promise.withResolvers() + const { streams, sendFrame } = createStreams(() => wait.promise) + const cancel = streams.subscribe( + method, + { terminal: 'term', worktree: 'id:workspace', subscriptionId: 'chat' }, + vi.fn() + ) + cancel() + wait.resolve() + await Promise.resolve() + expect(sendFrame).not.toHaveBeenCalled() + expect( + streams.handleResponse(response('request-1', { type: 'ready', subscriptionId: 'server-1' })) + ).toBe(false) + }) + + it.each([false, true])( + 'preserves a same-worktree sibling when cancellation precedes snapshot=%s', + async (early) => { + const { streams, sendFrame } = createStreams() + const first = vi.fn() + const second = vi.fn() + const cancel = streams.subscribe( + 'session.tabs.subscribe', + { worktree: 'id:workspace' }, + first + ) + streams.subscribe('session.tabs.subscribe', { worktree: 'id:workspace' }, second) + await Promise.resolve() + if (early) { + cancel() + } + expect(sendFrame).toHaveBeenCalledTimes(2) + streams.handleResponse(response('request-1', { type: 'snapshot' })) + if (!early) { + cancel() + } + expect(sendFrame).toHaveBeenLastCalledWith({ + id: 'request-3', + method: 'session.tabs.unsubscribe', + params: { worktree: 'id:workspace', subscriptionId: 'request-1' } + }) + streams.handleResponse(response('request-2', { type: 'snapshot' })) + streams.handleResponse(response('request-2', { type: 'updated' })) + expect(second).toHaveBeenCalledTimes(2) + expect(first).toHaveBeenCalledTimes(early ? 0 : 1) + expect(streams.handleResponse(response('request-1', { type: 'updated' }))).toBe(false) + } + ) + + it.each([ + ['nativeChat.subscribe', { agent: 'claude', sessionId: 's1', subscriptionId: 'claude:s1' }], + ['terminal.subscribe', { terminal: 'term', client: { id: 'phone' } }] + ])( + 'keeps the newer %s live when an older same-token subscription unmounts', + async (method, params) => { + const { streams, sendFrame } = createStreams() + const older = vi.fn() + const newer = vi.fn() + const cancelOlder = streams.subscribe(method, params, older) + const cancelNewer = streams.subscribe(method, { ...params }, newer) + await Promise.resolve() + expect(sendFrame).toHaveBeenCalledTimes(2) + cancelOlder() + // The host keys cleanup by the deterministic token, so unsubscribing would evict the newer. + expect(sendFrame).toHaveBeenCalledTimes(2) + streams.handleResponse(response('request-2', { type: 'snapshot' })) + expect(newer).toHaveBeenCalledTimes(1) + expect(streams.handleResponse(response('request-1', { type: 'snapshot' }))).toBe(false) + expect(older).not.toHaveBeenCalled() + cancelNewer() + expect(sendFrame).toHaveBeenCalledTimes(3) + expect(sendFrame).toHaveBeenLastCalledWith( + expect.objectContaining({ method: method.replace(/\.subscribe$/, '.unsubscribe') }) + ) + } + ) + + it('still unsubscribes a shared-token nativeChat stream when the sibling is unsent', async () => { + const wait = Promise.withResolvers() + let connected = false + const { streams, sendFrame } = createStreams(() => + connected ? Promise.resolve() : wait.promise + ) + const params = { agent: 'claude', sessionId: 's1', subscriptionId: 'claude:s1' } + connected = true + const cancelOlder = streams.subscribe('nativeChat.subscribe', params, vi.fn()) + await Promise.resolve() + connected = false + streams.subscribe('nativeChat.subscribe', params, vi.fn()) + cancelOlder() + expect(sendFrame).toHaveBeenCalledTimes(2) + expect(sendFrame).toHaveBeenLastCalledWith({ + id: 'request-3', + method: 'nativeChat.unsubscribe', + params: { subscriptionId: 'claude:s1' } + }) + }) + + it('cleans up every cancelled server subscription across repeated late-ready cycles', async () => { + const { streams, sendFrame } = createStreams() + const listener = vi.fn() + for (let i = 0; i < 100; i++) { + const cancel = streams.subscribe('runtime.clientEvents.subscribe', {}, listener) + await Promise.resolve() + const requestId = `request-${2 * i + 1}` + cancel() + streams.handleResponse(response(requestId, { type: 'ready', subscriptionId: `server-${i}` })) + } + expect( + sendFrame.mock.calls.filter( + ([request]) => (request as { method: string }).method === 'runtime.clientEvents.unsubscribe' + ) + ).toHaveLength(100) + expect(listener).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/transport/mobile-relay-rpc-streams.ts b/mobile/src/transport/mobile-relay-rpc-streams.ts index 2eacbd2168f..abb2c12564b 100644 --- a/mobile/src/transport/mobile-relay-rpc-streams.ts +++ b/mobile/src/transport/mobile-relay-rpc-streams.ts @@ -4,9 +4,11 @@ import { type TerminalSnapshotState } from './rpc-client-terminal-binary-frame' import { + buildStreamUnsubscribe, buildTerminalUnsubscribeParams, updateTerminalSubscriptionViewport } from './rpc-client-terminal-subscription' +import { buildReadyStreamUnsubscribe } from './rpc-client-server-subscription' import type { RpcClient } from './rpc-client' import type { RpcResponse, RpcSuccess } from './types' @@ -22,6 +24,23 @@ type StreamRecord = { streamIds: Set subscriptionId?: string cancelled: boolean + sent: boolean + receivedSnapshot?: boolean +} + +type StreamUnsubscribe = { method: string; params: unknown } + +/** Unsubscribe derived from the subscribe params alone (no server-assigned id). */ +function buildParamsUnsubscribe( + method: string, + params: unknown, + requestId: string +): StreamUnsubscribe | null { + if (method === 'terminal.subscribe') { + const unsubscribeParams = buildTerminalUnsubscribeParams(params) + return unsubscribeParams ? { method: 'terminal.unsubscribe', params: unsubscribeParams } : null + } + return buildStreamUnsubscribe(method, params, requestId) } type StreamManagerOptions = { @@ -32,6 +51,10 @@ type StreamManagerOptions = { export class MobileRelayRpcStreams { private readonly streams = new Map() + private readonly cancelledSubscriptions = new Map< + string, + { method: string; unsubscribe?: StreamUnsubscribe } + >() private readonly terminalListeners = new Map void>() private readonly terminalSnapshots = new Map() private activeBrowserStream: StreamRecord | null = null @@ -51,13 +74,15 @@ export class MobileRelayRpcStreams { listener, onBinaryFrame: subscribeOptions?.onBinaryFrame, streamIds: new Set(), - cancelled: false + cancelled: false, + sent: false } this.streams.set(id, stream) void this.options .waitForConnected() .then(() => { if (!stream.cancelled) { + stream.sent = true if (!this.options.sendFrame({ id, method, params: stream.params })) { this.fail(id, stream, 'Connection interrupted') } @@ -75,6 +100,30 @@ export class MobileRelayRpcStreams { } handleResponse(response: RpcResponse): boolean { + const cancelled = this.cancelledSubscriptions.get(response.id) + if (cancelled) { + if (!response.ok) { + this.cancelledSubscriptions.delete(response.id) + } else if (response.result && typeof response.result === 'object') { + const result = response.result as { subscriptionId?: unknown; type?: unknown } + if (result.type === 'end') { + this.cancelledSubscriptions.delete(response.id) + } else if (result.type === 'snapshot' && cancelled.unsubscribe) { + this.cancelledSubscriptions.delete(response.id) + this.options.sendFrame({ id: this.options.nextId(), ...cancelled.unsubscribe }) + } else if (typeof result.subscriptionId === 'string') { + this.cancelledSubscriptions.delete(response.id) + const unsubscribe = buildReadyStreamUnsubscribe(cancelled.method, result.subscriptionId) + if (unsubscribe) { + this.options.sendFrame({ id: this.options.nextId(), ...unsubscribe }) + } + } + } + if (response.ok && response.streaming !== true) { + this.cancelledSubscriptions.delete(response.id) + } + return true + } const stream = this.streams.get(response.id) if (!stream) { return false @@ -86,6 +135,9 @@ export class MobileRelayRpcStreams { const result = (response as RpcSuccess).result if (result && typeof result === 'object') { const metadata = result as { subscriptionId?: unknown; streamId?: unknown; type?: unknown } + if (stream.method === 'session.tabs.subscribe' && metadata.type === 'snapshot') { + stream.receivedSnapshot = true + } if (typeof metadata.subscriptionId === 'string') { stream.subscriptionId = metadata.subscriptionId } @@ -125,6 +177,7 @@ export class MobileRelayRpcStreams { stream.cancelled = true } this.streams.clear() + this.cancelledSubscriptions.clear() this.terminalListeners.clear() this.terminalSnapshots.clear() this.activeBrowserStream = null @@ -136,25 +189,61 @@ export class MobileRelayRpcStreams { return } stream.cancelled = true - if (stream.method === 'terminal.subscribe') { - const params = buildTerminalUnsubscribeParams(stream.params) - if (params) { - this.options.sendFrame({ - id: this.options.nextId(), - method: 'terminal.unsubscribe', - params - }) + if (stream.sent) { + const byParams = buildParamsUnsubscribe(stream.method, stream.params, id) + if (stream.method === 'terminal.subscribe') { + if (byParams) { + this.sendUnsubscribe(byParams) + } + } else { + const unsubscribe = stream.subscriptionId + ? buildReadyStreamUnsubscribe(stream.method, stream.subscriptionId) + : null + if (byParams && stream.method === 'session.tabs.subscribe' && !stream.receivedSnapshot) { + // The host registers cleanup only after resolving the initial snapshot. + this.cancelledSubscriptions.set(id, { method: stream.method, unsubscribe: byParams }) + } else if (unsubscribe || byParams) { + this.sendUnsubscribe((unsubscribe ?? byParams)!) + } else if ( + stream.method === 'browser.screencast' || + stream.method === 'runtime.clientEvents.subscribe' + ) { + // Keep only the cleanup route while the server assigns its subscription ID. + this.cancelledSubscriptions.set(id, { method: stream.method }) + } else if (stream.subscriptionId) { + this.sendUnsubscribe({ + method: stream.method.replace(/\.subscribe$/, '.unsubscribe'), + params: { subscriptionId: stream.subscriptionId } + }) + } } - } else if (stream.subscriptionId) { - this.options.sendFrame({ - id: this.options.nextId(), - method: stream.method.replace(/\.subscribe$/, '.unsubscribe'), - params: { subscriptionId: stream.subscriptionId } - }) } this.remove(id) } + /** Skip the unsubscribe when a live sibling shares the host cleanup token (e.g. nativeChat's + * deterministic `agent:sessionId`), since the host would evict the sibling's registration. */ + private sendUnsubscribe(unsubscribe: StreamUnsubscribe): void { + if (this.hasLiveOwner(unsubscribe)) { + return + } + this.options.sendFrame({ id: this.options.nextId(), ...unsubscribe }) + } + + private hasLiveOwner(unsubscribe: StreamUnsubscribe): boolean { + const token = JSON.stringify(unsubscribe) + for (const [siblingId, sibling] of this.streams) { + if (sibling.cancelled || !sibling.sent) { + continue + } + const siblingUnsubscribe = buildParamsUnsubscribe(sibling.method, sibling.params, siblingId) + if (siblingUnsubscribe && JSON.stringify(siblingUnsubscribe) === token) { + return true + } + } + return false + } + private remove(id: string): void { const stream = this.streams.get(id) if (!stream) { diff --git a/mobile/src/transport/rpc-client-terminal-subscription.ts b/mobile/src/transport/rpc-client-terminal-subscription.ts index 471bc3c4a4e..405f7f1d9e0 100644 --- a/mobile/src/transport/rpc-client-terminal-subscription.ts +++ b/mobile/src/transport/rpc-client-terminal-subscription.ts @@ -38,7 +38,8 @@ export function updateTerminalSubscriptionViewport( * the per-method echo logic out of the rpc-client teardown closure. */ export function buildStreamUnsubscribe( method: string | undefined, - params: unknown + params: unknown, + requestId?: string ): { method: string; params: Record } | null { if (!params || typeof params !== 'object') { return null @@ -46,7 +47,10 @@ export function buildStreamUnsubscribe( if (method === 'session.tabs.subscribe') { const worktree = (params as { worktree?: unknown }).worktree return typeof worktree === 'string' - ? { method: 'session.tabs.unsubscribe', params: { worktree } } + ? { + method: 'session.tabs.unsubscribe', + params: { worktree, ...(requestId ? { subscriptionId: requestId } : {}) } + } : null } if (method === 'nativeChat.subscribe') { From eebedf206f7b23f94859f78ddcf29deab4c36418 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 21:08:18 -0700 Subject: [PATCH 42/43] fix(tests): provide a window manager for Linux Electron CI (#19007) --- .github/scripts/e2e-with-window-manager.sh | 26 +++++++++++++++++++ .github/workflows/e2e.yml | 16 ++++++------ ...red-remote-terminal-stall-recovery.spec.ts | 22 +++++++++++++--- 3 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 .github/scripts/e2e-with-window-manager.sh diff --git a/.github/scripts/e2e-with-window-manager.sh b/.github/scripts/e2e-with-window-manager.sh new file mode 100644 index 00000000000..d431a039809 --- /dev/null +++ b/.github/scripts/e2e-with-window-manager.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +set -euo pipefail +openbox --sm-disable > /tmp/orca-e2e-window-manager.log 2>&1 & +wm_pid=$! +cleanup() { + kill "$wm_pid" 2>/dev/null || true + wait "$wm_pid" 2>/dev/null || true +} +trap cleanup EXIT +ready=false +for attempt in {1..100}; do + if xprop -root _NET_SUPPORTING_WM_CHECK 2>/dev/null | rg -q 'window id # 0x[1-9a-fA-F]'; then + ready=true + break + fi + if ! kill -0 "$wm_pid" 2>/dev/null; then + cat /tmp/orca-e2e-window-manager.log + exit 1 + fi + sleep 0.1 +done +if [ "$ready" != true ]; then + echo 'Window manager did not acquire the Xvfb root window' >&2 + exit 1 +fi +"$@" diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 5f80c2090ad..a94a7ea2ba5 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -150,7 +150,7 @@ jobs: # Native cache misses need the compiler, Electron needs Xvfb, and paired # Quick Open needs ripgrep. Install them in one apt transaction per shard. - name: Install native build and headless UI tools - run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk python3 ripgrep xvfb zsh + run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk python3 ripgrep xvfb zsh openbox x11-utils - uses: ./.github/actions/install-node-dependencies with: @@ -171,7 +171,7 @@ jobs: # ORCA_E2E_FORWARD_APP_LOGS keeps startup failures visible when Electron # launches but never creates a BrowserWindow. - name: Run E2E tests (${{ matrix.shard_name }}) - run: xvfb-run --auto-servernum env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 ORCA_E2E_WEB_CLIENT=1 ORCA_RELAY_PATH="$GITHUB_WORKSPACE/out/relay" pnpm run test:e2e --shard=${{ matrix.shard }} + run: xvfb-run --auto-servernum bash .github/scripts/e2e-with-window-manager.sh env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 ORCA_E2E_WEB_CLIENT=1 ORCA_RELAY_PATH="$GITHUB_WORKSPACE/out/relay" pnpm run test:e2e --shard=${{ matrix.shard }} # Why: Playwright retains traces/screenshots only on failure. Uploading # them as an artifact makes post-mortem debugging on CI possible without @@ -205,7 +205,7 @@ jobs: # unbounded inventory fallback; the paired fixture exercises that real boundary. # Why openssh-client: the Docker-SSH fixture shells out to ssh/ssh-keygen, and this # lane now receives those specs from pr.yml's SSH source mapping. - run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk openssh-client python3 ripgrep xvfb zsh + run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk openssh-client python3 ripgrep xvfb zsh openbox x11-utils - uses: ./.github/actions/install-node-dependencies with: @@ -245,7 +245,7 @@ jobs: if grep -l '@headful' "${TEST_FILES[@]}" >/dev/null; then E2E_PROJECT_ARGS+=(--project=electron-headful) fi - xvfb-run --auto-servernum env "${E2E_ENV[@]}" \ + xvfb-run --auto-servernum bash .github/scripts/e2e-with-window-manager.sh env "${E2E_ENV[@]}" \ pnpm run test:e2e "${TEST_FILES[@]}" --workers=1 "${E2E_PROJECT_ARGS[@]}" - name: Upload Playwright traces @@ -282,7 +282,7 @@ jobs: ref: ${{ inputs.ref || github.ref }} - name: Install native build and headless UI tools - run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk openssh-client python3 xvfb zsh + run: sudo apt-get update && sudo apt-get install -y build-essential fonts-noto-cjk openssh-client python3 ripgrep xvfb zsh openbox x11-utils - uses: ./.github/actions/install-node-dependencies with: @@ -297,7 +297,7 @@ jobs: # Why: this is the release-path proof that the deployed Linux relay keeps # its PTY and explorer live across a real watcher SIGSEGV. - name: Run Docker SSH watcher isolation E2E - run: xvfb-run --auto-servernum env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker-watcher-isolation + run: xvfb-run --auto-servernum bash .github/scripts/e2e-with-window-manager.sh env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker-watcher-isolation # Why: Playwright empties test-results/ when it starts, so each step here used to # destroy the previous step's traces. Only the last lane's failure was ever @@ -314,7 +314,7 @@ jobs: # readiness across live SSH, headed paired, and headless serve topologies. - name: Run Docker SSH terminal parking + startup readiness E2E if: always() - run: xvfb-run --auto-servernum env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker-terminal-parking + run: xvfb-run --auto-servernum bash .github/scripts/e2e-with-window-manager.sh env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker-terminal-parking - name: Keep terminal-parking traces if: always() @@ -330,7 +330,7 @@ jobs: # legible as an SSH-named failure. - name: Run remaining Docker SSH E2E if: always() - run: xvfb-run --auto-servernum env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker + run: xvfb-run --auto-servernum bash .github/scripts/e2e-with-window-manager.sh env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 pnpm run test:e2e:ssh-docker - name: Keep remaining-ssh-docker traces if: always() diff --git a/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts b/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts index 4406ebd2f18..2c81b76f077 100644 --- a/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts +++ b/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts @@ -1,3 +1,4 @@ +import { runProcess } from '../../src/shared/child-process/run-process' import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import os from 'node:os' import path from 'node:path' @@ -101,11 +102,26 @@ async function minimizeHeadedHost(electronApp: ElectronApplication, page: Page): .poll(() => host.evaluate((window) => ({ backgroundThrottling: window.webContents.getBackgroundThrottling(), - minimized: window.isMinimized(), - visible: window.isVisible() + minimized: window.isMinimized() })) ) - .toEqual({ backgroundThrottling: true, minimized: true, visible: false }) + .toEqual({ backgroundThrottling: true, minimized: true }) + // Linux reports isVisible/document visibility differently; the window manager owns iconification. + if (process.platform === 'linux') { + const nativeId = await host.evaluate((window) => window.getNativeWindowHandle().readUInt32LE(0)) + await expect + .poll(async () => { + const result = await runProcess({ + program: 'xprop', + args: ['-id', String(nativeId), '_NET_WM_STATE'], + timeoutMs: 5_000 + }) + return result.stdout + }) + .toContain('_NET_WM_STATE_HIDDEN') + } else { + await expect.poll(() => page.evaluate(() => document.visibilityState)).toBe('hidden') + } } async function restoreHeadedHost(electronApp: ElectronApplication, page: Page): Promise { From fba90e017c81eff36697373a728e7e9029669738 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 5 Sep 2026 21:11:28 -0700 Subject: [PATCH 43/43] fix(windows): copy the daemon host exe verbatim instead of renaming it (MDE T1036) (#17865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(windows): document the EDR signal surface Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in eight days on one enterprise Windows 11 / Intune tenant. All six were behavioural process-tree scoring, not signature hits; two escalated to multi-stage incidents mapped to ATT&CK Execution and Collection. Add a reference doc mapping each attack-technique-shaped behaviour to the code that produces it and to why it exists: the renamed daemon image (T1036), the per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL (T1113). Records that signing is not the gate -- reputation is signer plus hash-keyed prevalence -- and carries the two evidence gaps the report noted. Adds an engineer checklist, deployment guidance for admins (AV path exclusions do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and an explicit pre-deployment warning about computer use. * docs(windows): correct the PowerShell flag inventory and admin paths Review corrections to the EDR posture doc. The "encoded, policy-bypassing PowerShell" list conflated three different shapes and was incomplete. Split it into the three tiers an EDR actually scores differently -- bypass plus encoding, encoding alone, and bypass alone -- and add the sites it missed, including windows-mobile-firewall.ts, which encodes a script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts (-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded and are not. Notes that a raw grep under-reports, because the hook sites reach -EncodedCommand through wrapWindowsPowerShellEncodedCommand. Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to #16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and record that the launcher's own tradeoff is unverified on a real box. Admin guidance was missing two ways a suppression rule pinned to one full path misses real activity: the .staging- sibling that exists mid-update, which is when the update-cluster incidents fire, and the userData fallback when LOCALAPPDATA is unset. Also: state the measurement conditions on the process-table timings, note that Hermes has surface even though we have no telemetry for it, note that the uninstaller names are electron-builder-generated and in no repo file, drop a volatile line count, and mark the per-operation computer-use shape as being addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping the indexed bullet. * docs(windows): reconcile the EDR posture doc with the shipped remediation Three claims in this doc became false once the rest of the Windows EDR set landed, and two told engineers the opposite of what the release does. The process-table section still described one shared snapshot taken with `Memory | CommandLine | CreationTime`, argued that splitting the cache per field set "would restore exactly the fan-out it exists to prevent", and concluded the shape was unfixable because "the information is only in the PEB". The split shipped (identity opens no handle at all), `Memory` is retired, and the command line now comes from the kernel through `ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the compiled addon and a ratchet asserts it against the import table. An engineer reading the old text would have concluded both fixes were dead ends. The PowerShell site inventories were stale in three of four lists: the port scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair was dropped as a measured no-op, and of the unencoded-bypass list only `wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand` and never spell it, which a raw `rg` misses. Incident-evidence sections are left alone: they record what the tenant observed on 1.4.192, not what the code does now. * fix(windows): copy the daemon host exe verbatim instead of renaming it Microsoft Defender for Endpoint flagged `orca-terminal-daemon.exe` as MITRE T1036 (Masquerading): Orca copied its own `Orca.exe` into %LOCALAPPDATA% under a different name, specifically so the NSIS updater's `taskkill /IM Orca.exe` could not match, then ran it detached. Because that process is what every other flagged action was attributed to, the name mismatch acted as a reputation multiplier on unrelated findings. The rename was never what made the daemon survive. In app-builder-lib 26.15.3 the installer's FIND_PROCESS/KILL_PROCESS select processes whose image path is under $INSTDIR; `taskkill /IM` is only the fallback for hosts where PowerShell is missing or blocked. Survival is a property of the path, and %LOCALAPPDATA%\Orca\daemon-host is outside $INSTDIR whatever the file is called. Derive the host exe name from process.execPath so the copy is byte-for-byte, name included — it keeps its Authenticode signature and carries no renamed-image signal. On the no-PowerShell fallback the daemon is now killed with the app and terminals cold-restore, which is the documented pre-relocation outcome the update harness already asserts, not a regression. The uninstall macro no longer needs a distinct name to find the daemon; it kills the app's own image name (plus the legacy name, for hosts left by older builds). Adds docs/reference/windows-daemon-host-relocation.md with the survival contract, the rejected alternatives and their measured costs, and the invariants to keep. * fix(windows): apply daemon-host relocation review corrections Scope the uninstall taskkill to the current user with `/FI "USERNAME eq %USERNAME%"` via cmd.exe, matching upstream's per-user KILL_PROCESS — without it an elevated machine-wide uninstall reaches another logged-on user's session, so the "no collateral" claim in the comment was overstated. Comment the rmSync-before-publish: Windows refuses to delete a running image, so a live daemon already hosted in this version's dir (same-version reinstall, or a dev channel reusing a version) throws and materialization fails open. Doc corrections: - The fallback selector is the full per-user `taskkill /F /IM ".exe" /FI "PID ne $pid" /FI "USERNAME eq %USERNAME%"`, not a bare `taskkill /IM`. - The probe reads `Get-ExecutionPolicy -Scope Process`, not the effective policy, and GPO writes MachinePolicy/UserPolicy — so GPO-managed hosts take the primary path-scoped branch. Narrow the fallback triggers accordingly. - Drop the Authenticode sentence: the old name was equally byte-identical and equally signed, so a filename has no bearing on signature validity. - Name the new update-abort path: the daemon now matches FIND_PROCESS, so on the fallback branch an unkillable host reaches the retry loop's MessageBox /SD IDCANCEL and Quits, aborting a silent update. - Correct the customCheckAppRunning rejection. It is ~6 lines, not a rewrite; it is wrong because forcing the PowerShell branch where PowerShell is absent makes FIND/KILL silently no-op and leaves the real app running with files in use. - Bound the win honestly: OriginalFilename is empty on the shipped binary, so the strongest T1036 indicator never fired, and the residual copy-and-run-detached shape still maps to T1036.005. Reconcile docs/reference/windows-edr-posture.md, which documents the rename as a live finding and would otherwise contradict this change. Content-only edit: markdown under docs/reference/ is not oxfmt-formatted as a matter of practice and nothing in CI gates it, so the file is left consistent with its neighbours. * fix(windows): expand USERNAME in NSIS instead of spawning cmd.exe The uninstall macro routed both taskkills through `"$SYSDIR\cmd.exe" /C` purely so `%USERNAME%` would expand — two extra interpreter spawns on the uninstall path, in a change whose whole point is not adding scored behaviour, and the exact `cmd.exe /c` shape the new AGENTS.md EDR bullet warns about. NSIS reads the variable itself with ReadEnvStr, so the spawns buy nothing. Verified on Windows 11 that the generated command line does what the filter is there for: a copy of cmd.exe running as orca-nonexistent-probe.exe (pid 34244) was terminated by `taskkill /F /IM "orca-nonexistent-probe.exe" /FI "USERNAME eq "` — SUCCESS, exit 0, process gone. Guarded on an empty USERNAME because the degenerate case is silent: taskkill rejects an empty filter value outright ("The search filter cannot be recognized") and kills nothing, which would leave exactly the orphaned daemon this macro exists to reap. `*` is rejected as a filter value too, so there is no branchless spelling. With no USERNAME to scope by it kills unfiltered, as the macro did before the filter was added. Stack stays balanced: three pushes, two nsExec pops, three restores. Also strike the last stale row in windows-edr-posture.md's remediation table. "Copying our own image under a different name" read as outstanding work; it is done by this change, so the row now points at the relocation doc. Same class of staleness as the section reconciled in the previous commit, and git would not have flagged it either. * fix(windows): port the daemon-host uninstall sweep into the live NSIS include The uninstall macro this branch rewrote lived in config/nsis/daemon-host-uninstall.nsh, which main no longer includes: #17906 consolidated every Windows installer hook into config/nsis/orca-installer-hooks.nsh because electron-builder accepts exactly one `nsis.include`. Merged as-is, the rewritten macro would have been dead code while the shipped uninstaller kept running main's stale sweep — `taskkill /F /IM orca-terminal-daemon.exe`, which matches nothing now that the relocated host is a verbatim Orca.exe copy. The RMDir that follows then cannot delete the running image, so a live orphaned daemon and its ~224 MB tree would survive every uninstall. Ported into the live include: the ${APP_EXECUTABLE_FILENAME} kill, the USERNAME filter that keeps an elevated machine-wide uninstall out of another logged-on user's session, and the register save/restore around both. The legacy orca-terminal-daemon.exe kill stays so hosts left by older builds are still reaped. The ratchet that was meant to catch exactly this pinned only the legacy image name, which main's stale macro already satisfied, so it passed both ways. It now asserts the app-exe kill and the USERNAME filter, against comment-stripped script — the prose above the macro names both image names, so a toContain over the raw file proves nothing. --------- Co-authored-by: Orca Worker --- .gitignore | 1 + AGENTS.md | 1 + config/nsis/orca-installer-hooks.nsh | 44 +++++-- ...ron-builder-markdown-associations.test.mjs | 20 ++- .../windows-daemon-host-relocation.md | 118 ++++++++++++++++++ docs/reference/windows-edr-posture.md | 37 ++++-- .../daemon/daemon-host-relocation.test.ts | 38 ++++-- src/main/daemon/daemon-host-relocation.ts | 24 +++- tests/tools/win-crash-survival-e2e/README.md | 4 +- .../win-crash-survival-e2e/crash-step.mjs | 2 +- tests/tools/win-crash-survival-e2e/run.mjs | 2 +- 11 files changed, 247 insertions(+), 44 deletions(-) create mode 100644 docs/reference/windows-daemon-host-relocation.md diff --git a/.gitignore b/.gitignore index 8be3fc5b6f4..08be52f0751 100644 --- a/.gitignore +++ b/.gitignore @@ -110,6 +110,7 @@ docs/** !docs/reference/macos-press-and-hold.md !docs/reference/orcad-operations.md !docs/reference/relay-grace-time-reconfiguration.md +!docs/reference/windows-daemon-host-relocation.md !docs/reference/windows-edr-posture.md !docs/reference/windows-process-enumeration.md !docs/reference/wsl-runner-verification.md diff --git a/AGENTS.md b/AGENTS.md index 306c9c8d5ed..491d270b815 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh - **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md). - **Windows child processes**: start them through `runProcess`/`spawnProcess` in `src/shared/child-process/` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. A ratchet test fails on any new direct import. - **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. See [`docs/reference/windows-process-enumeration.md`](./docs/reference/windows-process-enumeration.md). +- **Windows daemon-host relocation**: the terminal daemon runs from a copy of the app runtime under `%LOCALAPPDATA%`, which is what survives an auto-update. Before touching that copy, its exe name, or the NSIS uninstall macro, read [`docs/reference/windows-daemon-host-relocation.md`](./docs/reference/windows-daemon-host-relocation.md). - **Windows EDR signal**: don't add `-ExecutionPolicy Bypass`, `-EncodedCommand`, `cmd.exe /c` with escaped free text, per-operation interpreter spawning, or runtime `Add-Type` compilation without reading [`docs/reference/windows-edr-posture.md`](./docs/reference/windows-edr-posture.md) first — behavioural EDR scores each of those, and being signed does not clear them. - **WSL commands**: build argv with `buildWslExecArgs` (always `--exec` — under `--`, `wsl.exe` expands `$name` in every argument and silently rewrites the script), and fence anything whose stdout you parse with `buildWslCapturedLoginShellCommand`, because the interactive login shell prints the distro banner to stdout. See [`docs/reference/wsl-command-execution.md`](./docs/reference/wsl-command-execution.md). - **Linux native modules**: keep the glibc floor at Ubuntu 20.04 / glibc 2.31. A module compiled from source on a newer runner can reference symbol versions absent on the floor and crash the app on startup. See [`docs/reference/linux-glibc-compatibility.md`](./docs/reference/linux-glibc-compatibility.md); packaging fails if a bundled native binary needs newer glibc. diff --git a/config/nsis/orca-installer-hooks.nsh b/config/nsis/orca-installer-hooks.nsh index ca80c99fc6d..d89439073ab 100644 --- a/config/nsis/orca-installer-hooks.nsh +++ b/config/nsis/orca-installer-hooks.nsh @@ -49,22 +49,48 @@ ; --------------------------------------------------------------------------- ; Clean up the relocated terminal daemon on a REAL uninstall. ; -; Why: the daemon host is deliberately copied to a distinct image name -; (orca-terminal-daemon.exe) under %LOCALAPPDATA%\Orca\daemon-host so that app -; UPDATES cannot kill it — that relocation is what keeps terminals alive across -; updates. The same design means a normal uninstall's process sweep and file -; removal both miss it, leaving an orphaned daemon plus its runtime copy behind. +; Why: the daemon host is deliberately copied OUT of the install dir into +; %LOCALAPPDATA%\Orca\daemon-host so that app UPDATES cannot kill it — +; electron-builder's kill sweep selects processes whose image path is under +; $INSTDIR, and that relocation is what keeps terminals alive across updates. +; The same design means a normal uninstall's process sweep and file removal both +; miss it, leaving an orphaned daemon plus its runtime copy behind. ; ; The ${isUpdated} guard is essential: electron-builder runs this uninstaller as ; part of uninstallOldVersion on EVERY update, and killing the daemon there would ; defeat the whole feature. Only clean up on a genuine uninstall. ; -; The image name and the LOCALAPPDATA folder name must stay in sync with -; DAEMON_HOST_EXE_NAME and LOCAL_HOST_ROOT_NAME in -; src/main/daemon/daemon-host-relocation.ts. +; The LOCALAPPDATA folder name must stay in sync with LOCAL_HOST_ROOT_NAME in +; src/main/daemon/daemon-host-relocation.ts. See +; docs/reference/windows-daemon-host-relocation.md. !macro customUnInstall ${ifNot} ${isUpdated} - nsExec::Exec 'taskkill /F /IM orca-terminal-daemon.exe' + Push $0 + Push $1 + Push $2 + ; The host exe is a verbatim copy of the app exe, so the app's own image name + ; reaches it; the second name covers hosts left by builds that renamed the copy. + ; Filtered to the current user like upstream's per-user KILL_PROCESS, so an + ; elevated machine-wide uninstall cannot reach another logged-on user's session. + ; NSIS expands USERNAME itself: routing through cmd.exe only to get %USERNAME% + ; would add two interpreter spawns to the uninstall path for nothing. + ReadEnvStr $1 USERNAME + ${if} $1 == "" + ; Measured: taskkill rejects an empty filter value outright ("The search filter + ; cannot be recognized") and kills nothing, so with no USERNAME to scope by, + ; kill unfiltered rather than not at all. USERNAME is set in every session an + ; uninstaller runs in, so this is a backstop, not the expected path. + StrCpy $2 "" + ${else} + StrCpy $2 '/FI "USERNAME eq $1"' + ${endIf} + nsExec::Exec 'taskkill /F /IM "${APP_EXECUTABLE_FILENAME}" $2' + Pop $0 + nsExec::Exec 'taskkill /F /IM "orca-terminal-daemon.exe" $2' + Pop $0 + Pop $2 + Pop $1 + Pop $0 ; Give the OS a moment to release the image lock before removing the tree. Sleep 500 RMDir /r "$LOCALAPPDATA\Orca\daemon-host" diff --git a/config/scripts/electron-builder-markdown-associations.test.mjs b/config/scripts/electron-builder-markdown-associations.test.mjs index 7ae3b1c9428..58f6f8d8865 100644 --- a/config/scripts/electron-builder-markdown-associations.test.mjs +++ b/config/scripts/electron-builder-markdown-associations.test.mjs @@ -103,14 +103,24 @@ describe('electron-builder markdown file associations', () => { // Why: this include was renamed from daemon-host-uninstall.nsh to carry the markdown // hooks too. electron-builder allows only one include, so a merge that drops the daemon - // sweep would silently orphan a running orca-terminal-daemon.exe on every uninstall. + // sweep would silently orphan a running daemon host on every uninstall. + // + // Asserted against comment-stripped script, and on the app exe name first: the relocated + // host is a verbatim copy of the app exe (daemonHostExeName, daemon-host-relocation.ts), + // so a macro that kills only orca-terminal-daemon.exe matches no running process. The + // prose above the macro names both, so a toContain over the raw file proves nothing. it('keeps the daemon-host uninstall sweep across the include rename', async () => { - const hooks = await readInstallerHooks() + const script = stripNsisCommentLines(await readInstallerHooks()) - expect(hooks).toContain('orca-terminal-daemon.exe') - expect(hooks).toContain('$LOCALAPPDATA\\Orca\\daemon-host') + expect(script).toMatch(/taskkill[^\n]*\/IM\s+"?\$\{APP_EXECUTABLE_FILENAME\}"?/) + // Legacy name, so hosts left by builds that renamed the copy still get reaped. + expect(script).toMatch(/taskkill[^\n]*\/IM\s+"?orca-terminal-daemon\.exe"?/) + // Scopes both kills to the uninstalling user: an elevated machine-wide uninstall must + // not reach another logged-on user's session. + expect(script).toMatch(/\/FI\s+"USERNAME eq /) + expect(script).toContain('$LOCALAPPDATA\\Orca\\daemon-host') // Without this guard, uninstallOldVersion would kill the daemon on every update — // defeating the relocation that keeps terminals alive across updates. - expect(hooks).toMatch(/\$\{ifNot\}\s+\$\{isUpdated\}/) + expect(script).toMatch(/\$\{ifNot\}\s+\$\{isUpdated\}/) }) }) diff --git a/docs/reference/windows-daemon-host-relocation.md b/docs/reference/windows-daemon-host-relocation.md new file mode 100644 index 00000000000..f560597e25e --- /dev/null +++ b/docs/reference/windows-daemon-host-relocation.md @@ -0,0 +1,118 @@ +# Windows daemon-host relocation + +On Windows the terminal daemon does not run from the install directory. Before it forks the +daemon, Orca materializes a trimmed copy of its own runtime under +`%LOCALAPPDATA%\Orca\daemon-host\\` and forks the daemon from there +(`src/main/daemon/daemon-host-relocation.ts`). This is what keeps live terminals alive across an +auto-update and across a crash of the main process. + +Read this before changing the copy plan, the host exe name, the LOCALAPPDATA layout, or +`config/nsis/orca-installer-hooks.nsh`. + +## What the relocation actually escapes + +The killer is **electron-builder's process sweep, matched on image path** — not file deletion. +Windows will not delete a running image, so `RMDir /r "$INSTDIR"` cannot end the daemon on its own. + +In app-builder-lib's `allowOnlyOneInstallerInstance.nsh`, `FIND_PROCESS` / `KILL_PROCESS` have two +branches: + +| Branch | Condition | Selector | +| -------- | --------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Primary | `powershell.exe` runs, `Get-CimInstance` resolves, and `Get-ExecutionPolicy -Scope Process` is not `Restricted` | `Win32_Process` where `$_.Path.StartsWith('$INSTDIR', 'CurrentCultureIgnoreCase')` — **path-scoped** | +| Fallback | otherwise | per-user: `taskkill /F /IM ".exe" /FI "PID ne $pid" /FI "USERNAME eq %USERNAME%"`; per-machine: the same without the username filter — **image-name-scoped** | + +The probe reads the **process** scope, not the effective policy, and Group Policy writes +`MachinePolicy`/`UserPolicy` — so a GPO-managed host whose effective policy is `Restricted` still +exits 0 and takes the primary branch. The fallback is reached only when `powershell.exe` is absent, +`Get-CimInstance` does not resolve, PowerShell is blocked outright (WDAC/AppLocker, Server Core), or +an inherited `PSExecutionPolicyPreference=Restricted` is in the environment. + +So on essentially every machine the sweep is path-scoped, and a daemon whose image lives under +`%LOCALAPPDATA%` is out of range regardless of what the file is called. **Survival is a property of +the path.** The name only matters on the fallback branch. + +## Why the exe is copied verbatim (and not renamed) + +The host exe keeps the app exe's own file name (`daemonHostExeName()` returns +`basename(process.execPath)`), so the relocated image is a byte-for-byte copy of the app binary +under its original name. + +An earlier revision copied it as `orca-terminal-daemon.exe` specifically so the fallback +`taskkill /IM Orca.exe` could not match. That bought survival on the rare no-PowerShell host and +cost a textbook defence-evasion signature: _a process copies its own image into a user-writable +directory under a different name so a kill-by-image-name cannot match it, then runs detached and +survives the installer._ Microsoft Defender for Endpoint flagged it as MITRE **T1036 +(Masquerading)**, and — because it is the process every other flagged action is attributed to — it +acted as a reputation multiplier on unrelated findings. No VS Code fork does this. + +Trading the fallback branch for the name is the right trade: + +- On the primary branch nothing changes: the daemon still survives the update. +- On the fallback branch the daemon is killed with the app and terminals **cold-restore** on + relaunch. That is the documented pre-relocation behaviour, a first-class outcome the update + harness already asserts (`--expect cold-restore`), not a failure. +- Relocation is fail-open end to end anyway: any materialization failure returns `null` and the + caller forks the install-dir host. + +One new failure mode comes with it, on the fallback branch only. The daemon now matches +`FIND_PROCESS` under the app's image name, so it enters electron-builder's retry loop +(`allowOnlyOneInstallerInstance.nsh:136-141`). If the `taskkill` there fails to end it — an elevated +or otherwise unkillable host — the loop reaches `MessageBox ... /SD IDCANCEL` and `Quit`s, aborting a +silent update rather than completing it. Under the old distinct name the daemon was invisible to +that loop. Low probability (fallback branch _and_ an unkillable daemon), but it is a real new path. + +What this does **not** buy. Two things bound the win honestly: + +- The strongest T1036 indicator is a PE-resource-vs-disk-name mismatch, and it was **never firing**: + the shipped binary's `OriginalFilename` is empty (only `InternalName = Orca` is set), so there was + no embedded name for the old disk name to contradict. +- The remaining behaviour — a signed app copying its own ~225 MB image into user-writable + `%LOCALAPPDATA%` and running it detached under `ELECTRON_RUN_AS_NODE=1` — is still execution from + a non-standard user-writable location, which maps to **T1036.005** and is a standard heuristic on + its own. + +So this removes a real but partial signal. Expect the score to drop; do not expect the process to +stop being scored. + +## Options that were rejected + +| Option | Why not | +| ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Materialize the tree from the NSIS installer | The daemon host is ~246 MB. Writing it at install time doubles install footprint and lengthens the window in which the app is down during a silent update. Worse, on a per-machine install (`INSTALL_MODE_PER_ALL_USERS`) the installer runs as the installing admin, so `$LOCALAPPDATA` is the wrong user's — every other user still needs the runtime path, which means the runtime self-copy stays in the product and the signal is only made rarer. | +| Ship a second signed `orca-terminal-daemon.exe` in the installer | `Orca.exe` is 235,555,328 bytes (224.6 MiB). electron-builder's NSIS uses solid LZMA with a 64 MB dictionary, so a second copy 224 MB downstream does not dedupe; the compressed installer grows by roughly a whole compressed Electron binary, paid by every user on every update download. It also does not remove the runtime copy — the helper still has to reach `%LOCALAPPDATA%` to escape the sweep — so it buys the same signal reduction as the verbatim copy at a large download cost. | +| Override `customCheckAppRunning` to force a path-scoped kill on both branches | Cheap to write (~6 lines: `!include "getProcessInfo.nsh"`, `Var pid`, and a macro that pins `IsPowerShellAvailable`, reusing upstream's dialog, retry loop and elevated handling) — but wrong at any size. Forcing the PowerShell branch on a host where PowerShell is genuinely absent makes `FIND_PROCESS` and `KILL_PROCESS` silently no-op, so the installer proceeds with the **real app** still running and its files in use. That is a worse outcome than the cold restore it would prevent, so this is not worth doing ever, not merely not now. | +| Hardlink instead of copy | Avoids the 246 MB entirely and is not a "copy" at all, but is NTFS-and-same-volume-only and introduces fresh failure modes (link counts, AV interception, cross-volume installs). Worth revisiting deliberately, not as part of a signal fix. | + +## Invariants to preserve + +- The host exe name is **derived from `process.execPath`**, never a literal. A future + `executableName` or dev-channel rename must follow automatically; pinning a name of our own is + how the mismatch creeps back. +- The daemon is identified by **PID and command line**, never by image name — in the product + (`daemon-pid-file-parse`, `daemon-process-inspection`) and in the harness + (`tests/tools/win-update-e2e/daemon-processes.mjs`). Nothing may start matching on the exe name. +- `config/nsis/orca-installer-hooks.nsh` kills the daemon by image name. That now also matches the + app's own exe, which is correct on a genuine uninstall — the product is being removed — but its + `${isUpdated}` guard must stay: electron-builder runs the uninstaller during every update's + `uninstallOldVersion`, and killing the daemon there defeats the whole feature. The legacy + `orca-terminal-daemon.exe` name stays in the macro to reap hosts left by older builds. +- `LOCAL_HOST_ROOT_NAME` in `daemon-host-relocation.ts` and the path in the uninstall macro are the + same directory. Change both together. + +## Verifying a change + +Unit coverage lives in `src/main/daemon/daemon-host-relocation.test.ts` (copy plan, verbatim +naming, marker/atomic publish, fail-open, prune veto). Nothing in unit tests can prove survival, so +any change to this file or to the NSIS macro needs the packaged harnesses: + +- `.github/workflows/win-update-survival-e2e.yml` — builds an installer from the branch and updates + it over itself with `--expect survival`. The primary proof. +- `.github/workflows/win-crash-survival-e2e.yml` — proves the daemon survives a main-process crash. +- `.github/workflows/windows-terminal-restart-e2e.yml` — terminal restart behaviour. +- `.github/workflows/win-update-e2e.yml` — release-tag-to-release-tag update, both `survival` and + `cold-restore` profiles. + +All four are `workflow_dispatch`-only (the two update workflows also carry a push trigger pinned to +one historical feature branch), so they must be dispatched by hand against this branch before +merging a change here — which requires the workflow files to already exist on `main`. diff --git a/docs/reference/windows-edr-posture.md b/docs/reference/windows-edr-posture.md index 65287ac0459..68614932c14 100644 --- a/docs/reference/windows-edr-posture.md +++ b/docs/reference/windows-edr-posture.md @@ -50,25 +50,37 @@ and `orca-terminal-daemon.exe` report `Valid CN=SignPath Foundation`. ## The behaviours, and why each one exists -### The daemon runs from a renamed copy of our own image +### The daemon runs from a copy of our own image `src/main/daemon/daemon-host-relocation.ts` copies the Electron runtime into -`%LOCALAPPDATA%\Orca\daemon-host\\` and renames `Orca.exe` to -`orca-terminal-daemon.exe`. The comment on `DAEMON_HOST_EXE_NAME` states the -reason without varnish: _"so the NSIS updater's `taskkill /IM Orca.exe` can't -match it."_ +`%LOCALAPPDATA%\Orca\daemon-host\\` and forks the terminal daemon from +there. It exists because the NSIS installer deletes the old install directory and force- kills every process imaged under it. Without relocation, an auto-update kills the terminal daemon and every live terminal with it. The copy is a run-as-node `Orca.exe` rather than `node.exe` so there is no console flash and asar still -resolves; `config/nsis/daemon-host-uninstall.nsh` reaps it on a real uninstall +resolves; `config/nsis/orca-installer-hooks.nsh` reaps it on a real uninstall (guarded by `${isUpdated}` so an update's `uninstallOldVersion` never fires it). -**How an EDR reads it: MITRE T1036, masquerading.** A signed executable copied -out of the install directory into `%LOCALAPPDATA%` under a different name, which -then spawns shells, matches the textbook description closely enough that no -behavioural engine can be expected to score it low. +**At the time of these incidents the copy was also renamed** to +`orca-terminal-daemon.exe`, the image name every incident here reports, and +`DAEMON_HOST_EXE_NAME`'s comment stated the reason without varnish: _"so the NSIS +updater's `taskkill /IM Orca.exe` can't match it."_ The rename has since been +removed; the copy now keeps the app exe's own file name, because the updater's +kill sweep is path-scoped on every host that has PowerShell and the rename only +ever bought the no-PowerShell fallback. See +[`windows-daemon-host-relocation.md`](./windows-daemon-host-relocation.md). + +**How an EDR reads it: MITRE T1036, masquerading** — and, for what remains, +**T1036.005**. A signed executable copied out of the install directory into +`%LOCALAPPDATA%` under a different name, which then spawns shells, matches the +textbook description closely enough that no behavioural engine can be expected to +score it low. Dropping the rename removes that literal indicator but not the +underlying shape: execution from a non-standard user-writable location is scored +on its own. Note also that the strongest form of the T1036 signal was never +present here — the shipped binary's `OriginalFilename` is empty, so there was no +embedded name for the old disk name to contradict. ### Every process gets a handle, on a timer @@ -232,7 +244,8 @@ obfuscated-command-line detector is tuned on. ### The spawn tree itself -`Orca.exe` → `orca-terminal-daemon.exe` → a shell → an agent CLI is what a +`Orca.exe` → the relocated daemon host (`orca-terminal-daemon.exe` in the builds +these incidents cover, `Orca.exe` since) → a shell → an agent CLI is what a terminal multiplexer for coding agents *is*. `reg.exe` appears from `src/main/win32-utils.ts`, `src/main/agent-hooks/managed-hook-owner-identity.ts` and @@ -363,7 +376,7 @@ The checklist. On Windows, do not reach for: | Forking `powershell.exe` to read system state | The native reader — [`windows-process-enumeration.md`](./windows-process-enumeration.md) is the standing rule for the process table | | A process per operation in a loop | One long-lived helper with a request channel. A burst of short-lived interpreters under one parent is itself the signal | | `Add-Type -TypeDefinition` at runtime | A precompiled, signed assembly, or a native helper | -| Copying our own image under a different name | An installer or updater that does not need the rename. Where the rename is load-bearing, document it as such | +| Copying our own image under a different name | Copy it verbatim — [`windows-daemon-host-relocation.md`](./windows-daemon-host-relocation.md) (done for the daemon host) | | Deriving a script runner from a UI preference | [`windows-setup-shell.md`](./windows-setup-shell.md) — the script declares its own interpreter | Two framing rules that outlast the table: diff --git a/src/main/daemon/daemon-host-relocation.test.ts b/src/main/daemon/daemon-host-relocation.test.ts index 0d2323fd449..e899a67ed43 100644 --- a/src/main/daemon/daemon-host-relocation.test.ts +++ b/src/main/daemon/daemon-host-relocation.test.ts @@ -5,12 +5,13 @@ import { mkdtempSync, readFileSync, readdirSync, + renameSync, rmSync, utimesSync, writeFileSync } from 'node:fs' import os from 'node:os' -import { dirname, join } from 'node:path' +import { basename, dirname, join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { setAppEnvironment, type AppEnvironment } from '../../shared/app-environment' @@ -141,12 +142,11 @@ describe('buildDaemonHostManifest', () => { entryRelPath: 'resources/app.asar.unpacked/out/main/daemon-entry.js' }) const byDest = new Map(ops.map((op) => [op.destRel, op])) - // The host exe is renamed to a distinct image name (NOT the source basename) - // so the NSIS updater's name-based `taskkill /IM Orca.exe` can't kill it. - expect(byDest.get('orca-terminal-daemon.exe')?.kind).toBe('file') - expect(byDest.has('Orca.exe')).toBe(false) + // The host exe keeps the source basename: a verbatim, signature-preserving copy with no + // image-name mismatch. What escapes the updater's sweep is the path, not the name. + expect(byDest.get('Orca.exe')?.kind).toBe('file') const exeOp = ops.find((op) => op.sourcePath === 'C:\\app\\Orca.exe') - expect(exeOp?.destRel).not.toBe('Orca.exe') + expect(exeOp?.destRel).toBe('Orca.exe') // V8/ICU data blobs are read by the Electron bootstrap and kept. expect(byDest.has('icudtl.dat')).toBe(true) // GPU/graphics DLLs are never loaded by the windowless host, so not copied. @@ -170,7 +170,7 @@ describe('materializeRelocatedDaemonHost', () => { const result = materializeRelocatedDaemonHost() expect(result).not.toBeNull() const dest = join(localAppDataDir, 'Orca', 'daemon-host', '9.9.9') - expect(result?.execPath).toBe(join(dest, 'orca-terminal-daemon.exe')) + expect(result?.execPath).toBe(join(dest, 'Orca.exe')) expect(result?.entryPath).toBe( join(dest, 'resources', 'app.asar.unpacked', 'out', 'main', 'daemon-entry.js') ) @@ -203,6 +203,28 @@ describe('materializeRelocatedDaemonHost', () => { expect(marker.entryRelPath).toBe('resources/app.asar.unpacked/out/main/daemon-entry.js') }) + it('copies the exe verbatim: same file name and same bytes as the install-dir exe', () => { + const result = materializeRelocatedDaemonHost() + const sourceExe = join(installDir, 'Orca.exe') + // Byte-for-byte under the same name is what preserves the Authenticode signature and leaves + // no renamed-image signal for endpoint detection to read as masquerading. + expect(basename(result!.execPath)).toBe(basename(sourceExe)) + expect(readFileSync(result!.execPath)).toEqual(readFileSync(sourceExe)) + }) + + it('tracks a differently-named app exe rather than pinning an image name of its own', () => { + // A dev-channel or rebranded build ships a different executableName; the host copy must follow + // it, which is what keeps the copy verbatim instead of reintroducing a name mismatch. + renameSync(join(installDir, 'Orca.exe'), join(installDir, 'Orca Nightly.exe')) + setProcessProp('execPath', join(installDir, 'Orca Nightly.exe')) + const result = materializeRelocatedDaemonHost() + const dest = join(localAppDataDir, 'Orca', 'daemon-host', '9.9.9') + expect(result?.execPath).toBe(join(dest, 'Orca Nightly.exe')) + expect(existsSync(join(dest, 'orca-terminal-daemon.exe'))).toBe(false) + // Re-resolution must agree with materialization or the fork would target a missing exe. + expect(getRelocatedDaemonHost()?.execPath).toBe(join(dest, 'Orca Nightly.exe')) + }) + it('is idempotent: a valid marker short-circuits without recopying', () => { materializeRelocatedDaemonHost() const dest = join(localAppDataDir, 'Orca', 'daemon-host', '9.9.9') @@ -210,7 +232,7 @@ describe('materializeRelocatedDaemonHost', () => { const sentinel = join(dest, 'sentinel.txt') writeFileSync(sentinel, 'keep') const result = materializeRelocatedDaemonHost() - expect(result?.execPath).toBe(join(dest, 'orca-terminal-daemon.exe')) + expect(result?.execPath).toBe(join(dest, 'Orca.exe')) expect(existsSync(sentinel)).toBe(true) }) diff --git a/src/main/daemon/daemon-host-relocation.ts b/src/main/daemon/daemon-host-relocation.ts index 6d94bea06e4..13aab9fd8f9 100644 --- a/src/main/daemon/daemon-host-relocation.ts +++ b/src/main/daemon/daemon-host-relocation.ts @@ -22,6 +22,10 @@ import { inspectProcessLiveness, mergeProcessLivenessVerdict } from './daemon-pr * imaged under it, which would otherwise kill the daemon and its live terminals. The relocated exe is a * run-as-node Orca.exe copy (not node.exe) so there's no console flash and asar still resolves. Fail-open: * any failure returns null and the caller forks the install-dir host (pre-relocation behavior). + * + * What escapes the updater is the PATH, not the file name: electron-builder's kill sweep selects + * processes whose image path sits under $INSTDIR. See docs/reference/windows-daemon-host-relocation.md + * for the survival contract and why the exe is copied verbatim rather than renamed. */ export type RelocatedDaemonHost = { @@ -37,8 +41,14 @@ const MARKER_NAME = '.materialized.json' // LOCAL appData (not roaming) so OneDrive/roaming never syncs this ~260MB runtime. Shared with NSIS uninstall (config/nsis/orca-installer-hooks.nsh) — keep in sync. const LOCAL_HOST_ROOT_NAME = 'Orca' -// Copy of Orca.exe renamed to a distinct image name so the NSIS updater's `taskkill /IM Orca.exe` can't match it. -const DAEMON_HOST_EXE_NAME = 'orca-terminal-daemon.exe' +/** + * The host exe keeps the app exe's own file name, so the relocated image is a byte-for-byte, + * name-included copy of a signed binary — nothing for EDR to read as a renamed image (MITRE T1036). + * Survival comes from the path (see the module header). The one name-sensitive updater path is the + * no-PowerShell `taskkill /IM` fallback, where the daemon is killed and terminals cold-restore — + * the documented pre-relocation outcome, not a failure. + */ +const daemonHostExeName = (execPath: string): string => winPath.basename(execPath) // V8 snapshots + ICU data the Electron bootstrap reads even under ELECTRON_RUN_AS_NODE; siblings of Orca.exe. const RUNTIME_DATA_FILES = ['icudtl.dat', 'snapshot_blob.bin', 'v8_context_snapshot.bin'] @@ -146,8 +156,8 @@ export function buildDaemonHostManifest(sources: DaemonHostSources): CopyOp[] { const { appDir, execPath, resourcesPath, entrySourcePath, entryRelPath } = sources const ops: CopyOp[] = [] - // Host exe (renamed) + V8/ICU blobs at dest root. Top-level DLLs omitted: GPU/media libs a windowless run-as-node host never loads (~48MB saved). - ops.push({ sourcePath: execPath, destRel: DAEMON_HOST_EXE_NAME, kind: 'file' }) + // Host exe (verbatim name) + V8/ICU blobs at dest root. Top-level DLLs omitted: GPU/media libs a windowless run-as-node host never loads (~48MB saved). + ops.push({ sourcePath: execPath, destRel: daemonHostExeName(execPath), kind: 'file' }) for (const name of RUNTIME_DATA_FILES) { ops.push({ sourcePath: join(appDir, name), destRel: name, kind: 'file', optional: true }) } @@ -245,7 +255,7 @@ export function getRelocatedDaemonHost(): RelocatedDaemonHost | null { if (!marker || marker.version !== version) { return null } - const execPath = join(dest, DAEMON_HOST_EXE_NAME) + const execPath = join(dest, daemonHostExeName(sources.execPath)) const entryPath = destPath(dest, marker.entryRelPath) if (!existsSync(execPath) || !existsSync(entryPath)) { return null @@ -281,7 +291,9 @@ export function materializeRelocatedDaemonHost(): RelocatedDaemonHost | null { entryRelPath: sources.entryRelPath } writeFileSync(join(staging, MARKER_NAME), JSON.stringify(marker)) - // Replace any stale/partial dest, then publish the staging dir atomically. + // Replace any stale/partial dest, then publish atomically. Windows refuses to delete a running + // image, so a live daemon already hosted in THIS version's dir (same-version reinstall, or a dev + // channel reusing a version) throws here and materialization fails open to the install-dir host. rmSync(dest, { recursive: true, force: true }) renameSync(staging, dest) } catch { diff --git a/tests/tools/win-crash-survival-e2e/README.md b/tests/tools/win-crash-survival-e2e/README.md index beb56cf8c67..e0e773767c4 100644 --- a/tests/tools/win-crash-survival-e2e/README.md +++ b/tests/tools/win-crash-survival-e2e/README.md @@ -13,8 +13,8 @@ orphaned and PowerShell hard-crashed with a `0xE9` "No process is on the other end of the pipe" `FailFast`. Root cause: the terminal **daemon** (which hosts the ConPTYs) died together with the main process, severing the console pipe. -The fix re-architected the daemon into a standalone, relocated -`orca-terminal-daemon.exe` (see +The fix re-architected the daemon into a standalone daemon host relocated out of +the install dir (see [`src/main/daemon/daemon-host-relocation.ts`](../../src/main/daemon/daemon-host-relocation.ts)) that is spawned **detached** and **survives main-process death**. diff --git a/tests/tools/win-crash-survival-e2e/crash-step.mjs b/tests/tools/win-crash-survival-e2e/crash-step.mjs index 43dc18f04cd..3c7d80f51e1 100644 --- a/tests/tools/win-crash-survival-e2e/crash-step.mjs +++ b/tests/tools/win-crash-survival-e2e/crash-step.mjs @@ -4,7 +4,7 @@ // daemon (which hosts the ConPTYs) died with it, severing the console pipe, and // PowerShell hard-crashed with a 0xE9 "No process is on the other end of the // pipe" FailFast. The fix relocates the daemon into a standalone, detached -// orca-terminal-daemon.exe that SURVIVES main death (src/main/daemon/ +// host process outside the install dir that SURVIVES main death (src/main/daemon/ // daemon-host-relocation.ts). This module reproduces the crash and scans for the // pwsh FailFast that must no longer occur. diff --git a/tests/tools/win-crash-survival-e2e/run.mjs b/tests/tools/win-crash-survival-e2e/run.mjs index 4f9d8b242b0..68d8a7a3bd6 100644 --- a/tests/tools/win-crash-survival-e2e/run.mjs +++ b/tests/tools/win-crash-survival-e2e/run.mjs @@ -5,7 +5,7 @@ // process is on the other end of the pipe" FailFast, because the terminal daemon // (hosting the ConPTYs) died together with the main process and severed the // console pipe. The fix relocates the daemon into a standalone, detached -// orca-terminal-daemon.exe that survives main death (src/main/daemon/ +// host process outside the install dir that survives main death (src/main/daemon/ // daemon-host-relocation.ts). win-update-e2e proves the daemon survives a // Windows UPDATE; this harness proves it survives a CRASH of the main process. //