From 886fcf083f99e22de4b6b45f2ce859a3cbcff221 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 05:06:44 -0700 Subject: [PATCH] fix(runtime): let a scoped worktree listing report the host it could not cover (#18645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(runtime): let a scoped worktree listing report the host it could not cover `orca worktree list --repo ` passed `[]` as `knownHostIds`, so a scoped listing could never report a gap — for any host kind, reachable or not. With zero matched rows both scope lists are empty by construction, and the answer is `{hostIds: [], omittedHostIds: []}`: byte-identical to a repo that genuinely has no worktrees. docs/reference/ssh-execution-boundary.md forbids a listing from implying exactly that. Measured on one runtime with one refusing SSH host, in the same second: unscoped totalCount 0 omittedHostIds ["local","ssh:"] (+ --host selectors) scoped totalCount 0 hostScope {"hostIds":[],"omittedHostIds":[]} The same runtime reports nine omitted hosts unscoped and zero scoped against a live profile, so this is not a subtle inconsistency: it is one runtime giving two contradictory answers about its own coverage. A scoped listing now names the one host the caller asked about. That costs nothing when rows come back — the host lands in `covered`, so it is never reported omitted — and is the whole answer when they do not. Hosts the caller scoped out are still never named, which a test pins, because naming them all is the obvious over-correction. * test(runtime): pin the scoped host derivation for local and executionHostId repos Review flagged that the host-scope cases only covered a connectionId repo. getRepoExecutionHostId reads two spellings, and a scoped listing naming the wrong host would be worse than naming none, so both are pinned. Both fail with the fix reverted. --- .../runtime-managed-worktree-queries.test.ts | 120 +++++++++++++++++- .../runtime-managed-worktree-queries.ts | 12 +- .../runtime/worktree-listing-host-scope.ts | 28 +++- 3 files changed, 151 insertions(+), 9 deletions(-) diff --git a/src/main/runtime/runtime-managed-worktree-queries.test.ts b/src/main/runtime/runtime-managed-worktree-queries.test.ts index 01df53cbd1d..3fb792fcc7c 100644 --- a/src/main/runtime/runtime-managed-worktree-queries.test.ts +++ b/src/main/runtime/runtime-managed-worktree-queries.test.ts @@ -40,14 +40,18 @@ function metadata(overrides: Partial = {}): WorktreeMeta { } } -function queries(store: RuntimeStore): RuntimeManagedWorktreeQueries { +function queries( + store: RuntimeStore, + overrides: Partial[0]> = {} +): RuntimeManagedWorktreeQueries { return new RuntimeManagedWorktreeQueries({ getStore: () => store, listResolved: async () => [], resolveRepo: async () => store.getRepos()[0]!, selectRepos: () => store.getRepos(), scanRepo: async () => ({ ok: true, worktrees: [] }), - listKnownHostIds: () => [] + listKnownHostIds: () => [], + ...overrides }) } @@ -105,3 +109,115 @@ describe('RuntimeManagedWorktreeQueries.listDetected', () => { expect(legacy.worktrees[0]).not.toHaveProperty('visibilitySource') }) }) + +describe('RuntimeManagedWorktreeQueries.list host scope', () => { + // Measured on hardware before this fix, same runtime and same refusing SSH host in the same + // second: the UNSCOPED listing reported `omittedHostIds: ["local","ssh:ssh-scope-refused"]` with + // `--host` selectors, while the SCOPED listing reported `{"hostIds":[],"omittedHostIds":[]}`. + // A listing that covered nothing, reporting no gaps, is indistinguishable from a repo that + // genuinely has no worktrees -- the thing docs/reference/ssh-execution-boundary.md forbids. + function sshStore(): RuntimeStore { + const repo = folderRepo({ + id: 'repo-ssh', + kind: 'git', + connectionId: 'conn-1', + path: '/home/dev/app' + }) + return { + getRepos: () => [repo], + getRepo: () => repo, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + setWorktreeMeta: vi.fn(), + getAllWorktreeLineage: () => ({}), + getSettings: () => settings + } as unknown as RuntimeStore + } + + it('names the scoped repo host as omitted when the listing covered nothing', async () => { + const result = await queries(sshStore()).list('repo-ssh', 50) + + expect(result.totalCount).toBe(0) + expect(result.hostScope).toEqual({ + hostIds: [], + omittedHostIds: ['ssh:conn-1'] + }) + }) + + it('does not report the scoped host as omitted once it contributes rows', async () => { + const store = sshStore() + const result = await queries(store, { + listResolved: async () => + [ + { + id: 'repo-ssh::/home/dev/app', + repoId: 'repo-ssh', + path: '/home/dev/app', + hostId: 'ssh:conn-1' + } + ] as never + }).list('repo-ssh', 50) + + expect(result.hostScope?.hostIds).toEqual(['ssh:conn-1']) + expect(result.hostScope?.omittedHostIds).toEqual([]) + }) + + // The caller scoped the listing, so the hosts they excluded must not come back as gaps. + it('never names a host the caller scoped out', async () => { + const scoped = await queries(sshStore(), { + listKnownHostIds: () => ['local', 'ssh:other', 'runtime:elsewhere'] as never + }).list('repo-ssh', 50) + + expect(scoped.hostScope?.omittedHostIds).toEqual(['ssh:conn-1']) + }) + + // `getRepoExecutionHostId` derives the host from two spellings, and a scoped listing that named + // the wrong one would be worse than naming none. These pin both. + it('names the local host for a scoped local repo', async () => { + const repo = folderRepo({ id: 'repo-local', kind: 'git', path: '/workspace/local' }) + const store = { + getRepos: () => [repo], + getRepo: () => repo, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + setWorktreeMeta: vi.fn(), + getAllWorktreeLineage: () => ({}), + getSettings: () => settings + } as unknown as RuntimeStore + + const result = await queries(store).list('repo-local', 50) + + expect(result.hostScope?.omittedHostIds).toEqual(['local']) + }) + + it('prefers executionHostId over connectionId for the scoped host', async () => { + const repo = folderRepo({ + id: 'repo-runtime', + kind: 'git', + connectionId: 'conn-legacy', + executionHostId: 'runtime:env-1', + path: '/workspace/runtime' + }) + const store = { + getRepos: () => [repo], + getRepo: () => repo, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + setWorktreeMeta: vi.fn(), + getAllWorktreeLineage: () => ({}), + getSettings: () => settings + } as unknown as RuntimeStore + + const result = await queries(store).list('repo-runtime', 50) + + expect(result.hostScope?.omittedHostIds).toEqual(['runtime:env-1']) + }) + + it('still reports every configured host when the listing is unscoped', async () => { + const unscoped = await queries(sshStore(), { + listKnownHostIds: () => ['local', 'ssh:conn-1'] as never + }).list(undefined, 50) + + expect(unscoped.hostScope?.omittedHostIds).toEqual(['local', 'ssh:conn-1']) + }) +}) diff --git a/src/main/runtime/runtime-managed-worktree-queries.ts b/src/main/runtime/runtime-managed-worktree-queries.ts index 5a5812236af..158ab77cdd0 100644 --- a/src/main/runtime/runtime-managed-worktree-queries.ts +++ b/src/main/runtime/runtime-managed-worktree-queries.ts @@ -2,7 +2,7 @@ import type { DetectedWorktreeListResult, Worktree } from '../../shared/worktree import type { Repo } from '../../shared/repo-types' import type { RuntimeWorktreeListResult } from '../../shared/runtime-types' import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' -import { buildWorktreeListingPage } from './worktree-listing-host-scope' +import { buildWorktreeListingPage, listingKnownHostIds } from './worktree-listing-host-scope' import { readWorktreeMetaForHost } from '../persistence/host-qualified-worktree-meta' import { getRepoOwnedWorktreeMeta } from '../worktree-metadata-ownership' import type { WorktreeMeta } from '../../shared/worktree/meta-types' @@ -80,7 +80,7 @@ export class RuntimeManagedWorktreeQueries { throw new Error('invalid_limit') } const resolved = await this.deps.listResolved() - const repoId = repoSelector ? (await this.deps.resolveRepo(repoSelector)).id : null + const scopedRepo = repoSelector ? await this.deps.resolveRepo(repoSelector) : null const pathsByRepo = new Map() for (const worktree of resolved) { const paths = pathsByRepo.get(worktree.repoId) ?? [] @@ -100,12 +100,12 @@ export class RuntimeManagedWorktreeQueries { ) const worktrees = resolved.filter( (worktree) => - (!repoId || worktree.repoId === repoId) && + (!scopedRepo || worktree.repoId === scopedRepo.id) && this.isVisible(worktree, matchers.get(worktree.repoId), sourceDefaultsSupported) ) - // Why: a `--repo` listing was scoped by the caller, so naming every configured host as - // omitted would report a gap the caller deliberately excluded. - return buildWorktreeListingPage(worktrees, limit, repoId ? [] : this.deps.listKnownHostIds()) + // See `listingKnownHostIds`: a scoped listing must still name the host it was asked about. + const knownHostIds = listingKnownHostIds(scopedRepo, () => this.deps.listKnownHostIds()) + return buildWorktreeListingPage(worktrees, limit, knownHostIds) } resolveRepoForConnection(selector: string, connectionId?: string | null): Promise { diff --git a/src/main/runtime/worktree-listing-host-scope.ts b/src/main/runtime/worktree-listing-host-scope.ts index 99650882670..e499d7faaf3 100644 --- a/src/main/runtime/worktree-listing-host-scope.ts +++ b/src/main/runtime/worktree-listing-host-scope.ts @@ -1,4 +1,5 @@ -import type { ExecutionHostId } from '../../shared/execution-host' +import type { Repo } from '../../shared/repo-types' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import { selectHostBalancedPage } from '../../shared/host-balanced-listing-page' import type { RuntimeListingHostScope } from '../../shared/runtime-listing-host-scope' @@ -62,3 +63,28 @@ export function buildWorktreeListingHostScope(args: { } return { hostIds: [...covered].sort(), omittedHostIds: [...omitted].sort() } } + +/** + * Which hosts a listing claims to have been looking at. + * + * A `--repo` listing was scoped by the caller, so naming every configured host would report gaps + * the caller deliberately excluded. Naming NONE — which is what a scoped listing did before — means + * the scope can never report a gap at all, for any host kind, because `covered` and `omitted` are + * both derived from the returned rows plus this list. A scoped listing whose scan did not succeed + * then answers `{hostIds: [], omittedHostIds: []}`: byte-identical to a repo that genuinely has no + * worktrees, which is the one thing docs/reference/ssh-execution-boundary.md forbids a listing from + * implying. + * + * Measured before the fix, on one runtime with one refusing SSH host, in the same second: the + * unscoped listing reported `omittedHostIds: ["local", "ssh:"]` while the scoped listing + * reported `[]`. + * + * Naming the single host the caller asked about costs nothing when rows come back — it lands in + * `covered`, so it is never reported omitted — and is the whole answer when they do not. + */ +export function listingKnownHostIds( + scopedRepo: Repo | null, + listKnownHostIds: () => Iterable +): Iterable { + return scopedRepo ? [getRepoExecutionHostId(scopedRepo)] : listKnownHostIds() +}