From e5a1e79e8e4af790d38337cd5b8dd2fc10adadc5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 03:49:40 -0700 Subject: [PATCH 01/26] docs(linux): say which package to install and how updates arrive (#18123) * docs(linux): say which package to install and how updates arrive Closes #5188. Closes #10987. The install guide's entire Linux section was "AppImage and `.deb` builds are available. See the Releases page for details." It named two of the three published packages, gave no basis for choosing between them, and said nothing about updating -- which is the one thing that actually differs between them. Separately, nothing human-facing said the Linux CLI is `orca-ide`; only skills/orca-cli/SKILL.md carried it, which agents read and humans do not. Install page now picks the package by update behaviour: the AppImage self-updates, deb/rpm report the new version and hand over the install command, and a repackaged build is not offered a download it cannot apply. Records that Orca never escalates privileges for the package install, and points at #18086 for the signed repo as planned, not shipped. Adds .rpm to the download list. Release CI builds it (release-cut.yml: `--linux AppImage deb rpm`) and verify-release-required-assets.mjs requires the artifact, so omitting it was just wrong. The CLI command name is now stated where humans hit it -- the CLI reference and overview -- with the GNOME Orca collision as the reason, plus the two places bare `orca` does work: inside Orca-managed terminals (PTY PATH shim) and on a packaged `orca serve` host (the ~/.local/bin dispatcher). The headless guide gains the same note, which is what makes its `orca skills install` lines correct rather than a typo. * docs(linux): fix install ordering, CLI verification, and serve bootstrap Readiness review found ten defects. Two would have had a reader run the wrong program, and one would have had them install a .deb over a live app. Install ordering was reversed. The page said "run it, then quit and reopen Orca"; the ref this is gated to land with says the opposite in four places (linux-package-downloaded-status.ts LINUX_PACKAGE_MANUAL_INSTALL_MESSAGE, "Quit Orca before running the system package install command", plus the recovery card's title, summary and explainer). That wording came from main's older run-then-quit card, which the stack deliberately reversed when it retitled the card to "Manual Install Required". Now: quit first. CLI verification put the Linux caveat *below* `command -v orca`. That check succeeds on any GNOME desktop and resolves to the screen reader, so the reader got a confident hit from the page's own verification step and then invoked the wrong program. Caveat moved above, and the block now spells `orca-ide` literally instead of asking the reader to substitute. The serve bootstrap was circular: the bare-`orca` dispatcher is written *during* serve startup (main-process-runtime-launch.ts), so it can never be the command that starts serve. First launch is `orca-ide serve`. Fixed here and in the two pages this links to. Accuracy: the install command now matches what the code emits -- absolute paths resolved from the trusted directories and a POSIX-single-quoted package path, as pinned by linux-package-install-command.test.ts -- and names the manager fallbacks (dpkg; zypper/dnf/yum/rpm) rather than presenting apt as the only form. The pending path honours XDG_CACHE_HOME. rpm arch tokens are x86_64 and aarch64, not deb's amd64/arm64. arm64 AppImage is linked. Dropped the container example: isExternallyManagedLinuxInstall() needs a root marker AND no trusted package manager, and a Debian-based container has apt, so it is not flagged. --- docs/reference/headless-linux-server.md | 15 ++++++ docs/site/content/docs/cli/overview.mdx | 2 +- docs/site/content/docs/cli/reference.mdx | 18 +++++++- docs/site/content/docs/install.mdx | 56 +++++++++++++++++++++-- docs/site/content/docs/remote-servers.mdx | 8 ++++ docs/site/content/docs/ways-to-run.mdx | 4 +- 6 files changed, 96 insertions(+), 7 deletions(-) diff --git a/docs/reference/headless-linux-server.md b/docs/reference/headless-linux-server.md index 7368678c2e4..50a38cf446e 100644 --- a/docs/reference/headless-linux-server.md +++ b/docs/reference/headless-linux-server.md @@ -357,6 +357,21 @@ the command: This disables a security boundary. Prefer a dedicated unprivileged service user, especially when the listener is reachable beyond localhost. +The Linux CLI is named `orca-ide`, not `orca`, so it never shadows the GNOME +Orca screen reader at `/usr/bin/orca`. The `.deb` and `.rpm` packages put +`orca-ide` on `PATH` themselves at install time; with the AppImage it arrives +as `~/.local/bin/orca-ide` when the CLI is registered. + +A packaged `orca serve` start also writes a bare `orca` into `~/.local/bin` +that execs the same launcher, which is why the skills commands below can be +typed as `orca`. It writes it while starting, so it is never the command that +starts the server — the first launch is `orca-ide serve`, or the AppImage +invoked directly as above. The write is best-effort: it is gated on a packaged +build, it is skipped when no bundled launcher resolves, and it is skipped when +a file Orca does not own already holds that name (ownership is a marker on the +second line of the file). A host that really does run the screen reader keeps +its own `orca`. + ## Pairing troubleshooting - A pairing offer is a capability containing a device credential and E2EE diff --git a/docs/site/content/docs/cli/overview.mdx b/docs/site/content/docs/cli/overview.mdx index 3248e89e1eb..1e966c6217c 100644 --- a/docs/site/content/docs/cli/overview.mdx +++ b/docs/site/content/docs/cli/overview.mdx @@ -14,7 +14,7 @@ import { Callout } from '@/components/docs/prose' The Orca CLI is the `orca` command-line interface for scripting a running Orca editor from any shell. Use it to create and inspect worktrees, drive agent terminals, open files and diffs, automate the built-in browser, run scheduled automations, share HTML/Markdown artifacts, and control Orca-native tools from scripts or AI agents. -It ships with the desktop app; register it under [Settings → General → Orca CLI](/docs/settings). +It ships with the desktop app; register it under [Settings → General → Orca CLI](/docs/settings). On Linux the command is `orca-ide`, because GNOME Orca's screen reader already owns `/usr/bin/orca` — see [Install → Linux](/docs/install#linux). Agents can install the matching Orca CLI skill with: diff --git a/docs/site/content/docs/cli/reference.mdx b/docs/site/content/docs/cli/reference.mdx index 3df0773a4a7..a13ec0fdde4 100644 --- a/docs/site/content/docs/cli/reference.mdx +++ b/docs/site/content/docs/cli/reference.mdx @@ -9,13 +9,29 @@ The `orca` CLI talks to a running Orca runtime. Use it when a shell script or ag ## Verify the runtime -Register the CLI under [Settings → General → Orca CLI](/docs/settings), then check that it can reach Orca: +Register the CLI under [Settings → General → Orca CLI](/docs/settings), then check that it can reach Orca. + + + GNOME Orca — the screen reader that ships with most GNOME desktops — already owns `/usr/bin/orca`, + so Orca's Linux CLI installs as `orca-ide`. Do not check for it with `command -v orca`: that + succeeds on a GNOME desktop and resolves to the screen reader, not to Orca. This page writes + `orca` throughout — read it as `orca-ide` on Linux. See [Install → Linux](/docs/install#linux). + + +On macOS and Windows: ```bash command -v orca orca status --json ``` +On Linux: + +```bash +command -v orca-ide +orca-ide status --json +``` + If Orca is not already running: ```bash diff --git a/docs/site/content/docs/install.mdx b/docs/site/content/docs/install.mdx index f710644d19e..9341cb0fa0d 100644 --- a/docs/site/content/docs/install.mdx +++ b/docs/site/content/docs/install.mdx @@ -31,8 +31,11 @@ import { Callout } from '@/components/docs/prose'
  • **Linux:** - [AppImage](https://github.com/stablyai/orca/releases/latest/download/orca-linux.AppImage) · - [.deb](https://github.com/stablyai/orca/releases) + AppImage + [x64](https://github.com/stablyai/orca/releases/latest/download/orca-linux.AppImage) · + [arm64](https://github.com/stablyai/orca/releases/latest/download/orca-linux-arm64.AppImage) · + [.deb](https://github.com/stablyai/orca/releases) · + [.rpm](https://github.com/stablyai/orca/releases) — see [Linux](#linux) for which to pick
  • Older versions: [GitHub Releases](https://github.com/stablyai/orca/releases).
  • @@ -59,6 +62,8 @@ On first launch Orca will: Orca auto-updates by default, tracking the **stable** channel. Stable releases are vetted; **RC (release candidate)** builds ship new features first, often daily. +On Linux, whether Orca can apply an update itself depends on which package you installed. See [Linux](#linux) before you pick one. + There is no permanent in-app opt-in for the RC channel. Modifier clicks on **Check for Updates** ([Settings → General → Updates](/docs/settings), or the app / Help menu): | Modifier | Effect | @@ -87,4 +92,49 @@ The default shell can be set to PowerShell or CMD under [Settings → Terminal]( ### Linux -AppImage and `.deb` builds are available. See the Releases page for details. +Each published release ships three Linux packages — an **AppImage**, a **`.deb`**, and an **`.rpm`** — for both x64 and arm64. They contain the same app. What differs is how updates reach you, so pick on that. + +| Package | Pick it when | Updates | +| ------------ | --------------------------------------------------------- | ----------------------------------------------------------------- | +| **AppImage** | You want Orca to update itself, like on macOS and Windows | Orca downloads and applies the update in place | +| **`.deb`** | You manage software with `apt` on Debian or Ubuntu | Orca tells you a version is out and hands you the install command | +| **`.rpm`** | You manage software with `dnf`, `yum`, or `zypper` | Same as `.deb` | + +The AppImage has a stable download link per architecture — [`orca-linux.AppImage`](https://github.com/stablyai/orca/releases/latest/download/orca-linux.AppImage) for x64 and [`orca-linux-arm64.AppImage`](https://github.com/stablyai/orca/releases/latest/download/orca-linux-arm64.AppImage) for arm64 — and needs `chmod +x` before its first run, because GitHub release assets carry no permission bits. The `.deb` and `.rpm` filenames carry the version and architecture, and the two formats spell architecture differently (`orca-ide__amd64.deb` or `_arm64.deb`; `orca-ide-.x86_64.rpm` or `.aarch64.rpm`), so take those from the [Releases page](https://github.com/stablyai/orca/releases) rather than a fixed URL. + +#### How updating works + +**The AppImage self-updates.** Choose it if you want automatic updates. Orca checks for a new release, you click **Update**, and it replaces the AppImage in place — the same flow as macOS and Windows. + +**The `.deb` and `.rpm` do not self-update.** Orca still notices the new version and downloads the package, then gives you a **Copy Install Command** button. Copy it rather than retyping it: Orca resolves every program to an absolute path in a trusted system directory and single-quotes the package path, so what you paste looks like this: + +``` +/usr/bin/sudo /usr/bin/apt install -- '/home/you/.cache/orca-updater/pending/orca-ide_1.4.194_amd64.deb' +``` + +Which package manager appears depends on what your system actually has: `apt`, else `dpkg -i`, for a `.deb`; `zypper`, `dnf`, `yum`, then `rpm -Uvh` for an `.rpm`. The download directory follows `XDG_CACHE_HOME` when that is set and falls back to `~/.cache` when it is not. + +**Quit Orca before you run the command**, then reopen it once the install finishes. You are replacing the files of a running application, and the package manager cannot swap them safely underneath a live process. Orca deliberately never escalates privileges to do this for you: installing a system package needs root, `orca serve` runs as an unprivileged user, and a headless machine has no authentication agent to prompt. VS Code and Signal make the same call on `.deb`. + +**A distro-managed build is left alone.** If you are running a repackaged Orca — an AUR build, a Nix derivation — Orca sees that no package manager it can drive owns this install and stops offering a download it could never apply. It still reports that a new version exists, so you can update the way you normally would. + + + [#18086](https://github.com/stablyai/orca/issues/18086) tracks publishing a signed repository so + your OS package manager owns Orca updates the way it owns everything else. It does not exist yet — + today, `.deb` and `.rpm` updates are the manual step described above. + + +#### The CLI command is `orca-ide` + +On Linux the [Orca CLI](/docs/cli/reference) installs as **`orca-ide`**, not `orca`. GNOME Orca — the screen reader that ships by default on Ubuntu and other GNOME desktops — already owns `/usr/bin/orca`, and Orca will not shadow it. The `.deb` and `.rpm` packages are named `orca-ide` for the same reason. + +- The `.deb` and `.rpm` put `orca-ide` on your `PATH` at install time, as `/usr/bin/orca-ide`. +- With the AppImage, register the CLI from [Settings → General → Orca CLI](/docs/settings). That installs `~/.local/bin/orca-ide`. +- Inside Orca's own terminals, bare `orca` works. Orca puts a shim on the `PATH` of the terminals it manages, so agents and scripts running there use the same command as on macOS and Windows. +- On a headless host, a packaged `orca serve` writes a bare `orca` into `~/.local/bin` as it starts, unless a file it does not own already holds that name. It writes that *during* startup, so it is never what starts the server — the first launch is always [`orca-ide serve`](/docs/remote-servers). + +Do not verify with `command -v orca`: on a GNOME desktop that succeeds and resolves to the screen reader. Use `orca-ide` in your own shell and `orca` inside Orca. If you want the short name everywhere and you do not use the screen reader, link it yourself: + +``` +ln -s "$(command -v orca-ide)" ~/.local/bin/orca +``` diff --git a/docs/site/content/docs/remote-servers.mdx b/docs/site/content/docs/remote-servers.mdx index 63f94e35af8..37f86665d6e 100644 --- a/docs/site/content/docs/remote-servers.mdx +++ b/docs/site/content/docs/remote-servers.mdx @@ -126,6 +126,14 @@ Use `orca serve` when the host should run without the desktop window—for examp Install Orca and its bundled CLI on the server, then run: + + The Linux CLI is named `orca-ide`, because GNOME Orca's screen reader already owns + `/usr/bin/orca`. A packaged `orca serve` does write a bare `orca` into `~/.local/bin`, but only + while it is starting, so that shim can never be the command that starts the server. Read + `orca serve` as `orca-ide serve` throughout this page when the host is Linux. See + [Install → Linux](/docs/install#linux). + + ```bash orca serve --pairing-address ``` diff --git a/docs/site/content/docs/ways-to-run.mdx b/docs/site/content/docs/ways-to-run.mdx index 1a9c44ba1d1..e35b63eae90 100644 --- a/docs/site/content/docs/ways-to-run.mdx +++ b/docs/site/content/docs/ways-to-run.mdx @@ -52,10 +52,10 @@ Keep Orca running on a machine you control—an old laptop, Mac mini, home serve **Easiest setup:** install Orca and Tailscale on both computers. On the server, open **Settings → Remote Orca Servers → Advertise this app as a server → New Link**, choose its Tailscale address, and generate an access link. On the client, choose **Add Server** and paste that link. -For a headless Linux server or service-managed VM, use `orca serve` as the alternative: +For a headless Linux server or service-managed VM, use `orca serve` as the alternative. On Linux the CLI is named `orca-ide`, so the first launch is: ```bash -orca serve --pairing-address +orca-ide serve --pairing-address ``` Full detail: [Remote Orca Servers](/docs/remote-servers). From f737f3499f3f9194fc4984b110dd202e5d089856 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 05:36:54 -0700 Subject: [PATCH 02/26] fix(relay): stream an oversized fs.listFiles reply instead of refusing it (#17954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening Orca's own checkout over SSH cannot list its files in one response frame. 22,617 tracked paths average 58 characters, so the 20,001-row page the client asks for serializes to 1,223,415 bytes — past `DISPATCHER_CONTROL_QUEUE_MAX_BYTES`, so `sendResponse` demotes it to the `legacy-response` lane, where an unrelated producer backlog can refuse it as an opaque `ResponseOverCapacity`. Break-even is around 49 characters of average path; any `packages//src/...` monorepo is over the line. Picking a ceiling to refuse at does not fix that, it just moves where it shows up and refuses listings that would have been delivered. `__streamResponse` already exists for exactly this on the git methods, and it is its own negotiation in both directions: an old client never sends it and gets the plain array on the legacy-response lane as before, and an old relay ignores it and answers plainly, which the client detects by the sentinel marker being absent. So fs.listFiles opts into it — no new method, no new opcode, nothing to advertise — and the size of a listing stops being a correctness question. The response-stream registry becomes one per relay, shared by FsHandler and GitHandler. A second registry is not an option and the header of git-response-stream.ts says why: a client keys reassembly on `streamId` alone, so two would hand out the same id and cross-feed chunks, and only the handler that registers `git.responseAck` can credit the window a pump parks on. Also declares `maxResults` on the runtime-RPC `files.listAll` and forwards it. The mechanism "the client names its cap, so a full page reads as truncation" was wired only on the Electron IPC hop; web and mobile were saved incidentally by `remoteFileContentBudget` defaulting the cap inside `listRuntimeFiles`. A new optional field is additive in both directions (wire rule 1). The new Docker-gated spec is claimed by run-ssh-docker-e2e.mjs. The sharded e2e lanes set no ORCA_E2E_SSH_DOCKER, so a Docker-gated spec that no runner names self-skips everywhere and still reports green — pr-e2e-gate-contract enforces that. Closes #12547 --- config/scripts/run-ssh-docker-e2e.mjs | 1 + .../providers/ssh-filesystem-provider.test.ts | 44 +++--- src/main/providers/ssh-filesystem-provider.ts | 7 +- .../methods/files-list-all-page-size.test.ts | 53 +++++++ src/main/runtime/rpc/methods/files.ts | 8 +- src/relay/fs-handler.ts | 25 +++- ...t-files-large-response.integration.test.ts | 130 +++++++++++++++++ src/relay/git-handler.ts | 22 ++- src/relay/git-response-stream.ts | 47 +++++- src/relay/relay-runtime-services.ts | 9 +- tests/e2e/helpers/docker-ssh-relay-target.ts | 10 ++ ...sh-docker-quick-open-large-listing.spec.ts | 134 ++++++++++++++++++ 12 files changed, 441 insertions(+), 49 deletions(-) create mode 100644 src/main/runtime/rpc/methods/files-list-all-page-size.test.ts create mode 100644 src/relay/fs-list-files-large-response.integration.test.ts create mode 100644 tests/e2e/ssh-docker-quick-open-large-listing.spec.ts diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index 195811f7312..dddfa3e0148 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -71,6 +71,7 @@ const result = spawnSync( 'tests/e2e/ssh-ai-vault-session-history.spec.ts', 'tests/e2e/ssh-cold-activation-restore.spec.ts', 'tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts', + 'tests/e2e/ssh-docker-quick-open-large-listing.spec.ts', 'tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts', 'tests/e2e/ssh-docker-transport-drop-recovery.spec.ts', 'tests/e2e/ssh-external-image-preview.spec.ts', diff --git a/src/main/providers/ssh-filesystem-provider.test.ts b/src/main/providers/ssh-filesystem-provider.test.ts index 65913f0c7e9..b9cb21c3354 100644 --- a/src/main/providers/ssh-filesystem-provider.test.ts +++ b/src/main/providers/ssh-filesystem-provider.test.ts @@ -486,14 +486,16 @@ describe('SshFilesystemProvider', () => { expect(result).toEqual(searchResult) }) - it('listFiles sends fs.listFiles request', async () => { + // Why #12547: a monorepo listing does not fit one control-lane frame, so the request opts into + // response streaming. An old relay ignores `__streamResponse` and answers plainly, which is the + // plain-array case each of these asserts. + it('listFiles sends a streamable fs.listFiles request', async () => { mux.request.mockResolvedValue(['src/index.ts', 'package.json']) const result = await provider.listFiles('/home/user/project') - expect(mux.request).toHaveBeenCalledWith( - 'fs.listFiles', - { rootPath: '/home/user/project' }, - { signal: undefined } - ) + expect(mux.request).toHaveBeenCalledWith('fs.listFiles', { + rootPath: '/home/user/project', + __streamResponse: true + }) expect(result).toEqual(['src/index.ts', 'package.json']) }) @@ -503,26 +505,22 @@ describe('SshFilesystemProvider', () => { maxResults: 20_000, searchQuery: 'target' }) - expect(mux.request).toHaveBeenCalledWith( - 'fs.listFiles', - { - rootPath: '/home/user/project', - excludePaths: ['/home/user/project/worktrees/b'], - maxResults: 20_000, - searchQuery: 'target' - }, - { signal: undefined } - ) + expect(mux.request).toHaveBeenCalledWith('fs.listFiles', { + rootPath: '/home/user/project', + excludePaths: ['/home/user/project/worktrees/b'], + maxResults: 20_000, + searchQuery: 'target', + __streamResponse: true + }) }) it('listFiles omits excludePaths when empty', async () => { mux.request.mockResolvedValue([]) await provider.listFiles('/home/user/project', { excludePaths: [] }) - expect(mux.request).toHaveBeenCalledWith( - 'fs.listFiles', - { rootPath: '/home/user/project' }, - { signal: undefined } - ) + expect(mux.request).toHaveBeenCalledWith('fs.listFiles', { + rootPath: '/home/user/project', + __streamResponse: true + }) }) it('listFiles forwards the cancellation signal to the mux request (#7721)', async () => { @@ -531,8 +529,8 @@ describe('SshFilesystemProvider', () => { await provider.listFiles('/home/user/project', { signal: controller.signal }) expect(mux.request).toHaveBeenCalledWith( 'fs.listFiles', - { rootPath: '/home/user/project' }, - { signal: controller.signal } + { rootPath: '/home/user/project', __streamResponse: true }, + { signal: controller.signal, timeoutMs: undefined } ) }) diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index 70bb06730f8..f6208ea00e9 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -1,6 +1,7 @@ import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' import { isMethodNotFoundError, readFileViaStream } from '../ssh/ssh-filesystem-stream-reader' import { uploadBuffer } from '../ssh/sftp-upload' +import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' import { lstatViaSftp } from './ssh-filesystem-provider-sftp' import { downloadFileViaSftp, @@ -314,7 +315,11 @@ export class SshFilesystemProvider implements IFilesystemProvider { // Why #7721: the signal lets a workspace switch send rpc.cancel so the // relay aborts the full-tree scan instead of stacking abandoned scans // that starve interactive fs.readDir/fs.stat on the shared SSH channel. - return (await this.mux.request('fs.listFiles', params, { + // Why streamable: a monorepo listing serializes past the relay's 1 MiB control lane, and the + // lane it demotes to is refused under unrelated producer load. Opting in moves it to the bulk + // lane in chunks; an old relay ignores the flag and answers plainly, which the reader detects + // by the sentinel marker being absent. + return (await requestGitStreamable(this.mux, 'fs.listFiles', params, { signal: options?.signal })) as string[] } diff --git a/src/main/runtime/rpc/methods/files-list-all-page-size.test.ts b/src/main/runtime/rpc/methods/files-list-all-page-size.test.ts new file mode 100644 index 00000000000..826d0c0f3f0 --- /dev/null +++ b/src/main/runtime/rpc/methods/files-list-all-page-size.test.ts @@ -0,0 +1,53 @@ +/** + * #12547: `files.listAll` did not declare `maxResults`, so "the client names its cap and a full page + * means there is more" was wired only on the Electron IPC hop. Web and mobile were saved incidentally, + * by `remoteFileContentBudget` defaulting the cap inside `listRuntimeFiles`. + */ +import { describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from '../dispatcher' +import type { RpcRequest } from '../core' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { FILE_METHODS } from './files' + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +describe('files.listAll page size', () => { + // Why #12547: `maxResults` was wired only on the Electron IPC hop, so "a full page means there is + // more" was true for a desktop client and incidental for web/mobile. Declaring it here is a new + // optional field (wire rule 1): an older host strips it and keeps its own default. + it('forwards a client-named page size for a selected worktree', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + listRuntimeFiles: vi.fn().mockResolvedValue(['src/index.ts']) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('files.listAll', { worktree: 'id:wt-1', maxResults: 20_001 }) + ) + + expect(runtime.listRuntimeFiles).toHaveBeenCalledWith('id:wt-1', { + excludePaths: undefined, + maxResults: 20_001 + }) + expect(response).toMatchObject({ ok: true, result: ['src/index.ts'] }) + }) + + // Why refuse rather than fall back: no released client sends this field, so a malformed value is a + // bug in the caller, not skew — the same call `files.search` already makes for its own maxResults. + it('refuses a malformed page size instead of silently picking one', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + listRuntimeFiles: vi.fn().mockResolvedValue(['src/index.ts']) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('files.listAll', { worktree: 'id:wt-1', maxResults: -3 }) + ) + + expect(response).toMatchObject({ ok: false }) + }) +}) diff --git a/src/main/runtime/rpc/methods/files.ts b/src/main/runtime/rpc/methods/files.ts index d4aaae455bc..ef349a22f84 100644 --- a/src/main/runtime/rpc/methods/files.ts +++ b/src/main/runtime/rpc/methods/files.ts @@ -93,8 +93,13 @@ const FileSearch = WorktreeSelector.extend({ maxResults: z.number().int().positive().optional() }) +// Why: `maxResults` is a new optional field (wire rule 1) — an older host strips it and keeps its +// own default. It existed only on the Electron IPC hop, so "the client names its cap and a full page +// means there is more" was true for desktop and merely incidental for web and mobile, which were +// saved by `remoteFileContentBudget` defaulting the cap inside `listRuntimeFiles`. const FileListAll = WorktreeSelector.extend({ - excludePaths: z.array(z.string()).optional() + excludePaths: z.array(z.string()).optional(), + maxResults: z.number().int().positive().optional() }) const FileUnwatch = z.object({ @@ -236,6 +241,7 @@ export const FILE_METHODS: RpcAnyMethod[] = [ const maxContentBytes = remoteFileContentBudget(clientKind, requestId) return runtime.listRuntimeFiles(params.worktree, { excludePaths: params.excludePaths, + ...(params.maxResults === undefined ? {} : { maxResults: params.maxResults }), ...(signal === undefined ? {} : { signal }), ...(maxContentBytes === undefined ? {} : { maxContentBytes }) }) diff --git a/src/relay/fs-handler.ts b/src/relay/fs-handler.ts index f8514a47191..ec11a806bb8 100644 --- a/src/relay/fs-handler.ts +++ b/src/relay/fs-handler.ts @@ -25,6 +25,7 @@ import { writeRelayFile } from './fs-path-mutation-requests' import { buildExcludePathPrefixes } from '../shared/quick-open-filter' +import { maybeStreamRpcResponse, type GitResponseStreamRegistry } from './git-response-stream' import { readRelayFileContent, readRelayFileStreamMetadata } from './fs-handler-file-read' import { readRelayFileRange } from './fs-handler-file-range' import { FileRangeReadRequestError } from '../shared/file-range-read' @@ -47,12 +48,19 @@ export class FsHandler { private watchRegistry: RelayFilesystemWatchRegistry private streamRegistry = new RelayStreamRegistry() private listFilesScans = new ListFilesScanCoordinator() + private readonly responseStreams: GitResponseStreamRegistry | undefined constructor( dispatcher: RelayDispatcher, _context: RelayContext, - watcherPool?: RelayWatcherProcessPool + watcherPool?: RelayWatcherProcessPool, + // Why passed in rather than owned: GitHandler registers the `git.responseAck` route every pump + // is credited through, and a client keys reassembly on `streamId` alone — see the header of + // git-response-stream.ts. Without one this handler answers plainly, which is the pre-streaming + // behavior rather than a stream nothing can credit. + responseStreams?: GitResponseStreamRegistry ) { + this.responseStreams = responseStreams this.dispatcher = dispatcher this.watchRegistry = new RelayFilesystemWatchRegistry(dispatcher, watcherPool) this.registerHandlers() @@ -204,7 +212,10 @@ export class FsHandler { } } - private listFiles(params: Record, context?: RequestContext): Promise { + private async listFiles( + params: Record, + context?: RequestContext + ): Promise { const rootPath = expandTilde(params.rootPath as string) const maxResults = typeof params.maxResults === 'number' && @@ -224,13 +235,21 @@ export class FsHandler { // Why #7721: full-tree scans are the relay's most expensive request; the // coordinator caps them at one per client, coalescing duplicates and // aborting a stale scan when the workspace changes or the host cancels. - return this.listFilesScans.run({ + const files = await this.listFilesScans.run({ clientId: context?.clientId ?? 0, key: JSON.stringify([rootPath, excludePathPrefixes, maxResults, searchQuery]), signal: context?.signal, start: (signal) => runListFilesScan(rootPath, excludePathPrefixes, signal, maxResults, searchQuery) }) + // Why: a full listing of a real monorepo serializes past the 1 MiB control lane — Orca's own + // checkout is 22.6k paths averaging 58 characters, so a 20,001-row page is ~1.2MB — and the + // legacy-response lane it demotes to is refused under unrelated producer load. Streaming makes + // size stop being a correctness question instead of picking a row or byte ceiling to refuse at. + // A client that did not opt in still gets the plain array, exactly as before. + return this.responseStreams + ? maybeStreamRpcResponse(files, params, context, this.responseStreams, this.dispatcher) + : files } private async workspaceSpaceScan(params: Record, context: RequestContext) { diff --git a/src/relay/fs-list-files-large-response.integration.test.ts b/src/relay/fs-list-files-large-response.integration.test.ts new file mode 100644 index 00000000000..27c7ee7e648 --- /dev/null +++ b/src/relay/fs-list-files-large-response.integration.test.ts @@ -0,0 +1,130 @@ +/** + * #12547: a full `fs.listFiles` reply for a real monorepo does not fit the relay's control lane. + * + * Orca's own checkout is ~22.6k tracked paths averaging 58 characters, so a 20,001-row page + * serializes to ~1.2MB — past `DISPATCHER_CONTROL_QUEUE_MAX_BYTES`, which demotes it to the + * `legacy-response` lane where an unrelated producer backlog can refuse it. Refusing at a fixed row + * or byte ceiling only moves where that shows up; streaming removes it, so these run the real + * dispatcher, the real FsHandler and the real client multiplexer over an in-memory pipe and assert + * an over-budget listing arrives intact — in both wire directions. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { runListFilesScanMock } = vi.hoisted(() => ({ runListFilesScanMock: vi.fn() })) + +vi.mock('./fs-list-files-fallback-chain', () => ({ runListFilesScan: runListFilesScanMock })) +vi.mock('@parcel/watcher', () => ({ subscribe: vi.fn() })) + +import { + SshChannelMultiplexer, + type MultiplexerTransport +} from '../main/ssh/ssh-channel-multiplexer' +import { requestGitStreamable } from '../main/ssh/ssh-git-response-stream-reader' +import { RelayContext } from './context' +import { RelayDispatcher } from './dispatcher' +import { DISPATCHER_CONTROL_QUEUE_MAX_BYTES } from './dispatcher-writer-admission' +import { FsHandler } from './fs-handler' +import { GitHandler } from './git-handler' +import { GitResponseStreamRegistry } from './git-response-stream' +import { QUICK_OPEN_LISTING_MAX_RESULTS } from '../shared/quick-open-listing-limits' + +/** Shaped like this repository: `packages//src/...`, ~58 characters. */ +function monorepoPaths(count: number): string[] { + return Array.from( + { length: count }, + (_, index) => + `packages/pkg-${String(index % 64).padStart(2, '0')}/src/renderer/components/entry-${String(index).padStart(6, '0')}.tsx` + ) +} + +describe('Integration: an over-budget fs.listFiles reply (#12547)', () => { + let mux: SshChannelMultiplexer + let dispatcher: RelayDispatcher + let fsHandler: FsHandler + let gitHandler: GitHandler + let writtenFrames: number[] + + beforeEach(() => { + runListFilesScanMock.mockReset() + writtenFrames = [] + + let relayFeed: (data: Buffer) => void + const clientDataCallbacks: ((data: Buffer) => void)[] = [] + const clientTransport: MultiplexerTransport = { + write: (data: Buffer) => { + setImmediate(() => relayFeed?.(data)) + }, + onData: (cb) => { + clientDataCallbacks.push(cb) + }, + onClose: () => {} + } + dispatcher = new RelayDispatcher((data: Buffer) => { + writtenFrames.push(data.length) + setImmediate(() => { + for (const cb of clientDataCallbacks) { + cb(data) + } + }) + return true + }) + relayFeed = (data: Buffer) => dispatcher.feed(data) + // Why: the same single registry production wires, so `git.responseAck` — registered by + // GitHandler — credits the pump an fs.listFiles stream parks on. + const responseStreams = new GitResponseStreamRegistry() + const context = new RelayContext() + fsHandler = new FsHandler(dispatcher, context, undefined, responseStreams) + gitHandler = new GitHandler(dispatcher, context, undefined, responseStreams) + mux = new SshChannelMultiplexer(clientTransport) + }) + + afterEach(() => { + mux.dispose() + dispatcher.dispose() + fsHandler.dispose() + gitHandler.dispose() + }) + + it('delivers a page too large for the control lane, in chunks no frame has to carry', async () => { + const files = monorepoPaths(QUICK_OPEN_LISTING_MAX_RESULTS) + // Precondition, measured from the payload rather than asserted between two constants: this is + // the listing that does not fit, which is what makes the rest of the test mean anything. + expect(Buffer.byteLength(JSON.stringify(files), 'utf8')).toBeGreaterThan( + DISPATCHER_CONTROL_QUEUE_MAX_BYTES + ) + runListFilesScanMock.mockResolvedValue(files) + + const received = await requestGitStreamable(mux, 'fs.listFiles', { + rootPath: '/remote/root', + maxResults: QUICK_OPEN_LISTING_MAX_RESULTS + }) + + expect(received).toEqual(files) + expect(Math.max(...writtenFrames)).toBeLessThan(DISPATCHER_CONTROL_QUEUE_MAX_BYTES) + }) + + it('still answers a client that never opts into streaming, with the whole array', async () => { + const files = monorepoPaths(QUICK_OPEN_LISTING_MAX_RESULTS) + runListFilesScanMock.mockResolvedValue(files) + + // Why: an old client sends neither `__streamResponse` nor `maxResults`. It gets one plain frame + // on the legacy-response lane, as it did before this call ever learned to stream. + const received = await mux.request('fs.listFiles', { rootPath: '/remote/root' }) + + expect(received).toEqual(files) + expect(Math.max(...writtenFrames)).toBeGreaterThan(DISPATCHER_CONTROL_QUEUE_MAX_BYTES) + }) + + it('leaves a reply that fits on the plain response path', async () => { + const files = monorepoPaths(100) + runListFilesScanMock.mockResolvedValue(files) + + const received = await requestGitStreamable(mux, 'fs.listFiles', { + rootPath: '/remote/root', + maxResults: 100 + }) + + expect(received).toEqual(files) + expect(Math.max(...writtenFrames)).toBeLessThan(DISPATCHER_CONTROL_QUEUE_MAX_BYTES) + }) +}) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 58f47da9fce..66bbd6d3cd1 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -10,8 +10,7 @@ import { createSubmodulePathsCache, type SubmodulePathsCache } from './git-handler-submodule-ops' -import { GitResponseStreamRegistry } from './git-response-stream' -import { GIT_RESPONSE_STREAM_THRESHOLD } from './protocol' +import { GitResponseStreamRegistry, maybeStreamRpcResponse } from './git-response-stream' import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' import { invalidateGitBranchLineTotalInFlight } from '../shared/git-branch-line-total' import { buildRelayGitEnv, buildRelayUnattendedGitEnv } from './relay-command-env' @@ -68,9 +67,6 @@ export class GitHandler { private dispatcher: RelayDispatcher private readonly gitDiffReadDedupe = new InFlightPromiseDedupe() private readonly gitCapabilities = new GitCapabilityCache() - // Why: use the bulk lane so large responses do not block interactive PTY echo. - private readonly responseStreams = new GitResponseStreamRegistry() - // Why: cache .gitmodules per instance to avoid SSH reads and test leakage. private submodulePathsCache: SubmodulePathsCache = createSubmodulePathsCache() @@ -78,7 +74,12 @@ export class GitHandler { constructor( dispatcher: RelayDispatcher, _context: RelayContext, - private readonly watcherRegistry?: GitHandlerWatcherRegistry + private readonly watcherRegistry?: GitHandlerWatcherRegistry, + // Why: use the bulk lane so large responses do not block interactive PTY echo. This handler + // registers the `git.responseAck` route below, so in production it takes the relay's single + // registry and FsHandler is handed the same one — see the header of git-response-stream.ts for + // why a second registry both collides on stream ids and stalls on credit. + private readonly responseStreams: GitResponseStreamRegistry = new GitResponseStreamRegistry() ) { this.dispatcher = dispatcher const handlers = createGitHandlerOperationSet({ @@ -132,14 +133,7 @@ export class GitHandler { params: Record, context: RequestContext | undefined ): unknown { - if (params.__streamResponse !== true || !context) { - return result - } - const payload = Buffer.from(JSON.stringify(result ?? null), 'utf-8') - if (payload.length <= GIT_RESPONSE_STREAM_THRESHOLD) { - return result - } - return this.responseStreams.startStream(payload, this.dispatcher, context) + return maybeStreamRpcResponse(result, params, context, this.responseStreams, this.dispatcher) } private clearGitMutationReadCaches(): void { diff --git a/src/relay/git-response-stream.ts b/src/relay/git-response-stream.ts index 3ccbd66ee8d..c9d8d2ce290 100644 --- a/src/relay/git-response-stream.ts +++ b/src/relay/git-response-stream.ts @@ -1,11 +1,22 @@ -// Streams large git RPC responses (diff family + exec) onto the bulk lane in -// chunks instead of one JSON-RPC frame, so a big diff cannot head-of-line-block -// interactive pty.data echo on the shared SSH channel. Mirrors the fs -// read-stream credit-window pattern (see fs-handler-file-read.ts) but the -// payload is an in-memory serialized string rather than a file handle. +// Streams large RPC responses onto the bulk lane in chunks instead of one +// JSON-RPC frame, so a big reply cannot head-of-line-block interactive pty.data +// echo on the shared SSH channel. Mirrors the fs read-stream credit-window +// pattern (see fs-handler-file-read.ts) but the payload is an in-memory +// serialized string rather than a file handle. +// +// ONE REGISTRY PER RELAY. The `git.*` method names below are the shipped wire +// spelling and are permanent, the way an opcode number is, so a second handler +// that needs streaming (`fs.listFiles` is the first) shares this instance rather +// than minting its own. A second registry is not an option: a client keys +// reassembly on `streamId` alone, so two would hand out the same id and +// cross-feed each other's chunks, and only the handler that registers +// `git.responseAck` can credit the ack window a pump parks on — the other's +// streams would stall at STREAM_ACK_WINDOW_CHUNKS forever. See +// `relay-runtime-services.ts` for the wiring. import type { RelayDispatcher, RequestContext } from './dispatcher' import { GIT_RESPONSE_CHUNK_SIZE, + GIT_RESPONSE_STREAM_THRESHOLD, STREAM_ACK_WINDOW_CHUNKS, STREAM_ACK_STALL_RECHECK_MS, type GitResponseStreamMarker @@ -220,3 +231,29 @@ export class GitResponseStreamRegistry { this.streams.clear() } } + +/** + * Opt-in response streaming, shared by every handler that can answer with a + * payload too large for one control-lane frame. + * + * `__streamResponse` is its own negotiation in both directions: an old client + * never sends it and gets the plain result, and an old relay ignores it and + * answers plainly, which the client detects by the sentinel marker being absent. + * So there is no new method and no capability to advertise. + */ +export function maybeStreamRpcResponse( + result: unknown, + params: Record, + context: RequestContext | undefined, + registry: GitResponseStreamRegistry, + dispatcher: RelayDispatcher +): unknown { + if (params.__streamResponse !== true || !context) { + return result + } + const payload = Buffer.from(JSON.stringify(result ?? null), 'utf-8') + if (payload.length <= GIT_RESPONSE_STREAM_THRESHOLD) { + return result + } + return registry.startStream(payload, dispatcher, context) +} diff --git a/src/relay/relay-runtime-services.ts b/src/relay/relay-runtime-services.ts index 36276ed9b70..485c9e787d1 100644 --- a/src/relay/relay-runtime-services.ts +++ b/src/relay/relay-runtime-services.ts @@ -6,6 +6,7 @@ import { RelayContext, expandTilde } from './context' import { PtyHandler } from './pty-handler' import { FsHandler } from './fs-handler' import { GitHandler } from './git-handler' +import { GitResponseStreamRegistry } from './git-response-stream' import { PreflightHandler } from './preflight-handler' import { ExternalAutomationsHandler } from './external-automations-handler' import { PortScanHandler } from './port-scan-handler' @@ -51,13 +52,17 @@ export class RelayRuntimeServices { ) this.ptyHandler.setSourcePublication(this.ptySourcePublication) - this.fsHandler = new FsHandler(dispatcher, context) + // Why one instance for both handlers: a client reassembles a streamed reply by `streamId` alone, + // so two registries would hand out the same id, and only GitHandler routes the `git.responseAck` + // credit every pump waits on. A second registry is not an option — see git-response-stream.ts. + const responseStreams = new GitResponseStreamRegistry() + this.fsHandler = new FsHandler(dispatcher, context, undefined, responseStreams) const watchRegistry = this.fsHandler.getWatchRegistry() this.ptyHandler.setWorktreeRemovalCoordinator(watchRegistry) watchRegistry.setWorktreePtyTeardown((rootPath) => this.ptyHandler.shutdownForWorktreePath(rootPath) ) - this.gitHandler = new GitHandler(dispatcher, context, watchRegistry) + this.gitHandler = new GitHandler(dispatcher, context, watchRegistry, responseStreams) const preflightHandler = new PreflightHandler(dispatcher) this.skillInstallHandler = new SkillInstallHandler(dispatcher) const externalAutomationsHandler = new ExternalAutomationsHandler(dispatcher) diff --git a/tests/e2e/helpers/docker-ssh-relay-target.ts b/tests/e2e/helpers/docker-ssh-relay-target.ts index 78534499943..f535b22d073 100644 --- a/tests/e2e/helpers/docker-ssh-relay-target.ts +++ b/tests/e2e/helpers/docker-ssh-relay-target.ts @@ -211,6 +211,16 @@ export function writeDockerSshRelayTargetFile( ) } +/** Why not `writeDockerSshRelayTargetFile`: that one passes the contents as a shell argument, so a + * payload the size of a real repository's path list exceeds ARG_MAX before it reaches the shell. */ +export function copyFileIntoDockerSshRelayTarget( + target: DockerSshRelayTarget, + localPath: string, + remotePath: string +): void { + run('docker', ['cp', localPath, `${target.containerName}:${remotePath}`], { timeoutMs: 120_000 }) +} + export function startDockerSshRelayTarget(testInfo: TestInfo): DockerSshRelayTarget { const host = process.env.ORCA_E2E_SSH_TARGET_HOST?.trim() || '127.0.0.1' if (host === 'localhost' || host === '::1' || host.startsWith('127.')) { diff --git a/tests/e2e/ssh-docker-quick-open-large-listing.spec.ts b/tests/e2e/ssh-docker-quick-open-large-listing.spec.ts new file mode 100644 index 00000000000..ee814ab02db --- /dev/null +++ b/tests/e2e/ssh-docker-quick-open-large-listing.spec.ts @@ -0,0 +1,134 @@ +/** + * #12547 acceptance: open a repository the size of Orca's own checkout over SSH and list its files. + * + * The remote tree is seeded from this repository's real `git ls-files` output, so the payload has + * the shape that broke: ~22.6k paths averaging 58 characters, whose 20,001-row page serializes to + * ~1.2MB — past `DISPATCHER_CONTROL_QUEUE_MAX_BYTES`. Both wire directions are exercised over the + * real relay: a current client, and a client that names no `maxResults` at all. + */ +import { execFileSync } from 'node:child_process' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import { expect, test } from './helpers/orca-app' +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' +import { + cleanupDockerSshRelayTarget, + copyFileIntoDockerSshRelayTarget, + execDockerSshRelayTargetCommand, + shellQuote, + startDockerSshRelayTarget, + type DockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { ensureDockerSshRelayImage } from './helpers/docker-ssh-relay-image' +import { waitForSessionReady } from './helpers/store' +import { shouldIncludeQuickOpenPath } from '../../src/shared/quick-open-filter' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' +const REMOTE_REPO_PATH = '/tmp/orca-quick-open-large-listing-repo' +const REMOTE_PATH_LIST = '/tmp/orca-quick-open-large-listing-paths.txt' +/** What the desktop client asks for; a full page is what it reads as "there is more". */ +const CLIENT_PAGE_SIZE = 20_001 + +function thisRepositoryTrackedPaths(): string[] { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim() + // Why -z: `git ls-files` C-quotes any path with a special character, which would seed a tree that + // does not match the one being measured. + return execFileSync('git', ['ls-files', '-z'], { + cwd: root, + encoding: 'utf8', + maxBuffer: 1024 * 1024 * 256 + }) + .split('\0') + .filter(Boolean) +} + +function seedRemoteTree(target: DockerSshRelayTarget, paths: string[]): void { + const stagingDir = mkdtempSync(path.join(tmpdir(), 'orca-quick-open-large-listing-')) + try { + const localList = path.join(stagingDir, 'paths.txt') + writeFileSync(localList, `${paths.join('\n')}\n`) + copyFileIntoDockerSshRelayTarget(target, localList, REMOTE_PATH_LIST) + } finally { + rmSync(stagingDir, { recursive: true, force: true }) + } + const seedScript = [ + "const fs = require('fs'), path = require('path')", + `const list = fs.readFileSync(${JSON.stringify(REMOTE_PATH_LIST)}, 'utf8').split('\\n').filter(Boolean)`, + 'const seen = new Set()', + 'for (const entry of list) {', + ' const dir = path.dirname(entry)', + ' if (dir !== "." && !seen.has(dir)) { fs.mkdirSync(dir, { recursive: true }); seen.add(dir) }', + " fs.writeFileSync(entry, '')", + '}' + ].join(';') + const encoded = Buffer.from(seedScript, 'utf8').toString('base64') + execDockerSshRelayTargetCommand( + target, + [ + `rm -rf ${shellQuote(REMOTE_REPO_PATH)}`, + `mkdir -p ${shellQuote(REMOTE_REPO_PATH)}`, + `cd ${shellQuote(REMOTE_REPO_PATH)}`, + 'git init -q', + 'git config user.email e2e@test.local', + 'git config user.name "Orca Docker SSH E2E"', + `node -e ${shellQuote(`eval(Buffer.from('${encoded}', 'base64').toString('utf8'))`)}`, + 'git add -A', + 'git commit -q -m "seed monorepo-shaped tree"' + ].join(' && ') + ) +} + +test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run the Docker SSH relay lane') + +test('lists a monorepo-sized remote workspace, with and without a client page size (#12547)', async ({ + orcaPage +}, testInfo) => { + test.setTimeout(420_000) + let target: DockerSshRelayTarget | null = null + try { + const trackedPaths = thisRepositoryTrackedPaths() + expect(trackedPaths.length).toBeGreaterThan(CLIENT_PAGE_SIZE) + // Why the real predicate rather than a copy of it: Quick Open prunes a few tracked paths on + // purpose (`.husky/` among them), and a hand-written expectation would go stale the first time + // that list changes and read as a transport bug. + const listablePaths = trackedPaths.filter(shouldIncludeQuickOpenPath) + // Precondition, measured rather than assumed: the page a current client asks for does not fit + // one control-lane frame, which is the listing that used to be refused outright. + expect( + Buffer.byteLength(JSON.stringify(trackedPaths.slice(0, CLIENT_PAGE_SIZE)), 'utf8') + ).toBeGreaterThan(1024 * 1024) + + ensureDockerSshRelayImage(process.cwd()) + target = startDockerSshRelayTarget(testInfo) + seedRemoteTree(target, trackedPaths) + + await waitForSessionReady(orcaPage) + const connected = await connectDockerSshRelayTarget(orcaPage, target, { + remotePath: REMOTE_REPO_PATH + }) + + const listFiles = async (maxResults?: number): Promise => + orcaPage.evaluate( + ({ connectionId, rootPath, maxResults }) => + window.api.fs.listFiles({ + rootPath, + connectionId, + ...(maxResults === undefined ? {} : { maxResults }) + }), + { connectionId: connected.targetId, rootPath: REMOTE_REPO_PATH, maxResults } + ) + + const currentClient = await listFiles(CLIENT_PAGE_SIZE) + expect(currentClient).toHaveLength(CLIENT_PAGE_SIZE) + + // Why: a client that predates `maxResults` on this call sends none at all, and it cannot + // reassemble a streamed reply either — it has to be answered on the plain response path. + const oldClient = await listFiles() + expect(oldClient).toHaveLength(listablePaths.length) + expect(new Set(oldClient)).toEqual(new Set(listablePaths)) + } finally { + cleanupDockerSshRelayTarget(target) + } +}) From 8dc3c1dd9787935bd14f9bab03f5ebdbe706a8e5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:21:28 -0700 Subject: [PATCH 03/26] Display favicons for browser website entries (#18099) * Display favicons for browser website entries Capture favicons from pages as they load and persist them with browser history entries. Display favicons in tabs, tab creation search results, and palette searches to improve visual recognition of websites and help users identify pages at a glance. * Fix favicon retry on back navigation after load failure Reset the favicon failure cache when the favicon URL changes, enabling retry of a previously failed favicon when navigating back to the same URL. Distinguish between explicit null (clear cached favicon) and omitted (don't update history), so stale favicons don't persist incorrectly. --- .../src/components/browser-favicon.tsx | 59 +++++++++++++++++++ .../host-guest/attach-browser-page-webview.ts | 4 +- ...rowser-page-webview-navigation-handlers.ts | 6 +- .../components/tab-bar/BrowserTab.test.tsx | 34 ++++++++++- .../src/components/tab-bar/BrowserTab.tsx | 53 ++--------------- .../TabBarCreateEntry.history.test.tsx | 13 +++- .../tab-bar/TabBarCreateEntryRow.tsx | 5 +- .../tab-bar/open-tab-entry-dedupe.test.ts | 3 +- .../tab-bar/open-tab-search.test.ts | 10 +++- .../src/components/tab-bar/open-tab-search.ts | 4 +- .../open-tab-selection-routing.test.ts | 3 +- ...ee-jump-palette-browser-simulator-rows.tsx | 5 +- .../src/lib/browser-palette-search.test.ts | 20 +++++++ .../src/lib/browser-palette-search.ts | 2 + src/renderer/src/store/slices/browser.test.ts | 47 +++++++++++++++ .../slices/browser/browser-history-actions.ts | 11 +++- .../browser/browser-page-state-actions.ts | 12 ++++ .../slices/browser/browser-slice-contract.ts | 2 +- src/shared/browser-workspace-types.ts | 1 + .../workspace-session-browser-schema.ts | 1 + src/shared/workspace-session-schema.test.ts | 4 ++ 21 files changed, 233 insertions(+), 66 deletions(-) create mode 100644 src/renderer/src/components/browser-favicon.tsx diff --git a/src/renderer/src/components/browser-favicon.tsx b/src/renderer/src/components/browser-favicon.tsx new file mode 100644 index 00000000000..ecde4065f03 --- /dev/null +++ b/src/renderer/src/components/browser-favicon.tsx @@ -0,0 +1,59 @@ +import { useState } from 'react' +import { Globe } from 'lucide-react' +import { cn } from '@/lib/utils' + +function displayableFaviconUrl(faviconUrl: string | null | undefined): string | null { + const trimmed = faviconUrl?.trim() + if (!trimmed) { + return null + } + if (trimmed.startsWith('data:image/')) { + return trimmed + } + try { + const url = new URL(trimmed) + return url.protocol === 'http:' || url.protocol === 'https:' ? trimmed : null + } catch { + return null + } +} + +export function BrowserFavicon({ + faviconUrl, + className, + fallbackClassName +}: { + faviconUrl: string | null | undefined + className?: string + fallbackClassName?: string +}): React.JSX.Element { + const displayUrl = displayableFaviconUrl(faviconUrl) + const [failedUrl, setFailedUrl] = useState(null) + + // Why: reset during render on any favicon identity change — including a clear to null while + // a page loads — so navigating back to the same url retries instead of keeping the fallback. + if (failedUrl !== null && failedUrl !== displayUrl) { + setFailedUrl(null) + } + + if (displayUrl && failedUrl !== displayUrl) { + return ( + setFailedUrl(displayUrl)} + /> + ) + } + + return