fix(worktrees): stop a resolved-worktree snapshot answering for repos it never saw (#18295)

* fix(worktrees): stop a resolved-worktree snapshot answering for repos it never saw

`listResolvedWorktrees` caches one fleet-wide snapshot for
RESOLVED_WORKTREE_CACHE_TTL_MS (1s) and reuses it on time alone. Nothing
invalidates it when a repo is registered, so for up to a second after a repo
row lands, every caller reads a snapshot computed before that repo existed --
and reads the gap as a verdict.

The visible failure is the SSH skill install. `resolveSkillSshTarget` resolves
a workspace-scope destination through that snapshot, so installing into a
worktree on a host connected moments earlier threw
`skill-install-workspace-not-found`: the client asserting a remote workspace is
absent on the strength of client-side bookkeeping that had never looked at the
host. That is the shape `docs/reference/ssh-execution-boundary.md` rules out --
absence from a client-side set is not evidence about the execution host. It
made `tests/e2e/ssh-skill-installation.spec.ts:108` fail 3 runs in 4 locally
and deterministically in the Docker SSH lane, where connect-then-install lands
inside the one-second window every time.

The snapshot now carries the repo-registration revision it was computed under
and is only reused while that revision still holds. The counter is the one
`bumpLocalWorktreeScanGeneration` already advances on every repo add, removal
and update, so the check is O(1) and cannot drift from the mutation sites.

* fix(worktrees): key the snapshot on repo mutations only, not on generation reads

Two things the headless-reattach lane surfaced.

The revision I keyed the snapshot on was `generationSequence`, which
`getLocalWorktreeScanGeneration` also advances when it mints a key for a repo
id nothing has scanned yet. That is a read, not a mutation, so a read path
could discard a snapshot that was still perfectly valid -- the mirror image of
the staleness this fixes, and a way to make a lookup fail that would otherwise
have succeeded. The counter now advances only where the scan generation is
actually bumped: repo add, removal, update, and scan-cache invalidation.

Separately, `pty-restore-record-seeding.test.ts` primed the cache by writing
its private `resolved` field with a literal spelling out `worktrees`,
`platformByRepoId` and `expiresAt`. That literal is a second copy of the
cache's freshness contract, so adding a field to the real entry left the fake
one failing the check: the primed snapshot was rejected, resolution fell
through to a real scan, and the headless fixture -- which has no git -- got
`selector_not_found`. It now primes through `getSnapshot` so the cache stamps
its own entry and the two cannot drift again.

The revision never moved during that test (0 before and after), so nothing was
being invalidated; the fake entry simply never satisfied the contract.

src/main/ipc/pty-restore-record-seeding.test.ts

@@ -12,6 +12,9 @@ import { setupPtyIpcSuite } from './pty-ipc-test-harness'
 import { getDefaultWorkspaceSession } from '../../shared/constants'
 import { makePaneKey } from '../../shared/stable-pane-id'
 import { OrcaRuntimeService } from '../runtime/orca-runtime'
+import type { RuntimeResolvedWorktreeCache } from '../runtime/runtime-resolved-worktree-cache'
+import type { ResolvedWorktree } from '../runtime/runtime-worktree-path-identity'
+import { getWorktreeScanMutationRevision } from '../local-worktree-scan-generation'
 import {
   registerPtyHandlers,
   clearProviderPtyState,
@@ -359,15 +362,22 @@ describe('registerPtyHandlers', () => {
     } as never)
     // Why: selector resolution shells out to git for real repos; prime the
     // resolved-worktree cache so this headless fixture resolves offline.
+    //
+    // Why through getSnapshot and not a hand-written `resolved` entry: the cache decides freshness
+    // from fields it stamps itself, so a literal that mirrors them is a second copy of that
+    // contract and goes stale the moment a field is added. Let the cache stamp its own entry.
     const worktreeResolutionInternals = runtime as unknown as {
-      buildResolvedWorktreeFromId(id: string): unknown
-      resolvedWorktrees: object
+      buildResolvedWorktreeFromId(id: string): ResolvedWorktree
+      resolvedWorktrees: RuntimeResolvedWorktreeCache
     }
-    Reflect.set(worktreeResolutionInternals.resolvedWorktrees, 'resolved', {
-      worktrees: [worktreeResolutionInternals.buildResolvedWorktreeFromId(worktreeId)],
-      platformByRepoId: new Map([[repo.id, process.platform]]),
-      expiresAt: Date.now() + 60_000
-    })
+    await worktreeResolutionInternals.resolvedWorktrees.getSnapshot(
+      async () => ({
+        worktrees: [worktreeResolutionInternals.buildResolvedWorktreeFromId(worktreeId)],
+        platformByRepoId: new Map([[repo.id, process.platform]])
+      }),
+      60_000,
+      getWorktreeScanMutationRevision()
+    )
     setLocalPtyProvider({
       spawn: vi.fn(async () => ({
         id: ptyId,

src/main/local-worktree-scan-generation.ts

@@ -1,5 +1,6 @@
 const generationByRepoId = new Map<string, number>()
 let generationSequence = 0
+let mutationRevision = 0
 
 export function getLocalWorktreeScanGeneration(repoId: string): number {
   const existing = generationByRepoId.get(repoId)
@@ -13,6 +14,22 @@ export function getLocalWorktreeScanGeneration(repoId: string): number {
 
 export function bumpLocalWorktreeScanGeneration(repoId: string): void {
   generationByRepoId.set(repoId, ++generationSequence)
+  mutationRevision += 1
+}
+
+/**
+ * Advances on every event above that can change what a worktree scan would find — repo add,
+ * removal, update, and scan-cache invalidation — and on nothing else. A cache that must not answer
+ * for repos it never saw compares this in O(1) instead of walking the repo list.
+ *
+ * Why not `generationSequence`: that also advances when `getLocalWorktreeScanGeneration` mints a key
+ * for a repo id nothing has scanned yet, which is a read. Keying a snapshot on it would let a read
+ * path discard a snapshot that is still perfectly valid.
+ *
+ * Ordering-only: the value means nothing outside a same-process comparison.
+ */
+export function getWorktreeScanMutationRevision(): number {
+  return mutationRevision
 }
 
 export function isLocalWorktreeScanGenerationCurrent(repoId: string, generation: number): boolean {
@@ -21,5 +38,6 @@ export function isLocalWorktreeScanGenerationCurrent(repoId: string, generation:
 
 export function resetLocalWorktreeScanGenerationsForTests(): void {
   generationSequence += 1
+  mutationRevision += 1
   generationByRepoId.clear()
 }

src/main/runtime/orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts

@@ -5,6 +5,7 @@ import { splitWorktreeIdForFilesystem } from '../../shared/worktree/id'
 import { isPathInsideOrEqual } from '../../shared/cross-platform-path'
 import type { ResolvedWorktreeSnapshot } from './runtime-resolved-worktree-cache'
 import { RESOLVED_WORKTREE_CACHE_TTL_MS } from './orca-runtime-postlude'
+import { getWorktreeScanMutationRevision } from '../local-worktree-scan-generation'
 import {
   resolveLocalProjectRuntimeForRepo,
   resolveLocalProjectRuntimesForRepos
@@ -65,8 +66,7 @@ export class OrcaRuntimeWithListKnownResolvedWorktreesForExplicitTarget extends
 
   /** A warm fleet snapshot already answers any selector for free, so scoped scanning must yield to it. */
   protected hasFreshResolvedWorktreeCache(): boolean {
-    const cached = this.resolvedWorktrees.peek()
-    return Boolean(cached && cached.expiresAt > Date.now())
+    return this.resolvedWorktrees.isFresh(getWorktreeScanMutationRevision())
   }
 
   protected async listResolvedWorktrees(): Promise<ResolvedWorktree[]> {
@@ -79,7 +79,8 @@ export class OrcaRuntimeWithListKnownResolvedWorktreesForExplicitTarget extends
     }
     return this.resolvedWorktrees.getSnapshot(
       () => this.computeResolvedWorktrees(),
-      RESOLVED_WORKTREE_CACHE_TTL_MS
+      RESOLVED_WORKTREE_CACHE_TTL_MS,
+      getWorktreeScanMutationRevision()
     )
   }
 

src/main/runtime/orca-runtime-tests/worktree-scan-cache-ttl.spec.ts

@@ -5,6 +5,7 @@ import {
   resolveWorktreeScanCacheTtlMs
 } from '../orca-runtime-test-mocks.spec'
 import { store } from '../orca-runtime-test-fixtures.spec'
+import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation'
 
 describe('resolveWorktreeScanCacheTtlMs', () => {
   const BASE_TTL_MS = 30_000
@@ -84,4 +85,34 @@ describe('resolveWorktreeScanCacheTtlMs', () => {
       vi.useRealTimers()
     }
   })
+
+  it('scans a repo registered after the last snapshot instead of answering from it', async () => {
+    // Why: the fleet snapshot only covers the repos that existed when it ran, so for a full TTL it
+    // reported a just-connected SSH host as having no worktrees at all — and callers that resolve a
+    // workspace through it turned that gap into `skill-install-workspace-not-found`.
+    vi.mocked(listWorktrees).mockClear()
+    const addedPath = '/tmp/repo-registered-later'
+    const repos = [
+      { id: 'repo-1', path: '/tmp/repo', displayName: 'repo', badgeColor: 'blue', addedAt: 1 }
+    ]
+    const runtime = new OrcaRuntimeService({ ...store, getRepos: () => repos } as never)
+    const internals = runtime as unknown as { listResolvedWorktrees: () => Promise<unknown> }
+    const scanCallsFor = (path: string): number =>
+      vi.mocked(listWorktrees).mock.calls.filter((call) => call[0] === path).length
+
+    await internals.listResolvedWorktrees()
+    expect(scanCallsFor(addedPath)).toBe(0)
+
+    repos.push({
+      id: 'repo-added',
+      path: addedPath,
+      displayName: 'added',
+      badgeColor: 'blue',
+      addedAt: 2
+    })
+    bumpLocalWorktreeScanGeneration('repo-added')
+
+    await internals.listResolvedWorktrees()
+    expect(scanCallsFor(addedPath)).toBe(1)
+  })
 })

src/main/runtime/runtime-resolved-worktree-cache.test.ts

@@ -0,0 +1,112 @@
+import { describe, expect, it } from 'vitest'
+import { RuntimeResolvedWorktreeCache } from './runtime-resolved-worktree-cache'
+import type { ResolvedWorktreeSnapshot } from './runtime-resolved-worktree-cache'
+import {
+  bumpLocalWorktreeScanGeneration,
+  getLocalWorktreeScanGeneration,
+  getWorktreeScanMutationRevision
+} from '../local-worktree-scan-generation'
+
+function snapshotOf(ids: string[]): ResolvedWorktreeSnapshot {
+  return {
+    worktrees: ids.map((id) => ({ id }) as ResolvedWorktreeSnapshot['worktrees'][number]),
+    platformByRepoId: new Map()
+  }
+}
+
+describe('RuntimeResolvedWorktreeCache', () => {
+  it('reuses a snapshot inside the TTL while the repo inventory is unchanged', async () => {
+    const cache = new RuntimeResolvedWorktreeCache()
+    let computes = 0
+    const compute = async (): Promise<ResolvedWorktreeSnapshot> => {
+      computes += 1
+      return snapshotOf(['repo-1::/a'])
+    }
+
+    await cache.getSnapshot(compute, 60_000, 7)
+    const second = await cache.getSnapshot(compute, 60_000, 7)
+
+    expect(computes).toBe(1)
+    expect(second.worktrees.map((worktree) => worktree.id)).toEqual(['repo-1::/a'])
+  })
+
+  it('recomputes when the repo inventory moved, even well inside the TTL', async () => {
+    // Why: this is the whole point. A snapshot taken before a repo was registered cannot testify
+    // that the repo's worktrees are absent — callers read the gap as "workspace not found".
+    const cache = new RuntimeResolvedWorktreeCache()
+    const results = [snapshotOf(['repo-1::/a']), snapshotOf(['repo-1::/a', 'repo-2::/b'])]
+    let computes = 0
+    const compute = async (): Promise<ResolvedWorktreeSnapshot> => results[computes++]
+
+    await cache.getSnapshot(compute, 60_000, 7)
+    const afterRegistration = await cache.getSnapshot(compute, 60_000, 8)
+
+    expect(computes).toBe(2)
+    expect(afterRegistration.worktrees.map((worktree) => worktree.id)).toEqual([
+      'repo-1::/a',
+      'repo-2::/b'
+    ])
+  })
+
+  it('does not join an in-flight compute that started under a stale inventory', async () => {
+    const cache = new RuntimeResolvedWorktreeCache()
+    const computed: number[] = []
+    const compute = async (): Promise<ResolvedWorktreeSnapshot> => {
+      computed.push(computed.length)
+      return snapshotOf([])
+    }
+
+    const first = cache.getSnapshot(compute, 60_000, 7)
+    const second = cache.getSnapshot(compute, 60_000, 8)
+    await Promise.all([first, second])
+
+    expect(computed).toHaveLength(2)
+  })
+
+  it('reports freshness against the inventory the snapshot was computed under', async () => {
+    const cache = new RuntimeResolvedWorktreeCache()
+    await cache.getSnapshot(async () => snapshotOf([]), 60_000, 7)
+
+    expect(cache.isFresh(7)).toBe(true)
+    expect(cache.isFresh(8)).toBe(false)
+    cache.invalidateResolved()
+    expect(cache.isFresh(7)).toBe(false)
+  })
+
+  it('keeps a primed snapshot servable when nothing mutated', async () => {
+    // Why: the headless-reattach fixtures prime this cache once and then resolve a selector off it
+    // without any git available. Losing freshness for a reason other than a mutation strands them
+    // on a real scan, which is the failure this pairs with — a lookup that finds nothing because
+    // the snapshot was dropped, not because the worktree is gone.
+    const cache = new RuntimeResolvedWorktreeCache()
+    let computes = 0
+    const prime = async (): Promise<ResolvedWorktreeSnapshot> => {
+      computes += 1
+      return snapshotOf(['repo-restore::/tmp/restore-records'])
+    }
+    await cache.getSnapshot(prime, 60_000, getWorktreeScanMutationRevision())
+
+    // A read that mints a scan generation for a repo nothing has scanned yet is not a mutation.
+    getLocalWorktreeScanGeneration(`repo-never-scanned-${Math.random()}`)
+
+    expect(cache.isFresh(getWorktreeScanMutationRevision())).toBe(true)
+    const served = await cache.getSnapshot(prime, 60_000, getWorktreeScanMutationRevision())
+    expect(computes).toBe(1)
+    expect(served.worktrees.map((worktree) => worktree.id)).toEqual([
+      'repo-restore::/tmp/restore-records'
+    ])
+  })
+})
+
+describe('getWorktreeScanMutationRevision', () => {
+  it('advances on a repo mutation and not on a first-seen generation read', () => {
+    const repoId = `repo-${Math.random()}`
+    const before = getWorktreeScanMutationRevision()
+
+    getLocalWorktreeScanGeneration(repoId)
+    expect(getWorktreeScanMutationRevision()).toBe(before)
+
+    bumpLocalWorktreeScanGeneration(repoId)
+    expect(getWorktreeScanMutationRevision()).toBe(before + 1)
+  })
+})

src/main/runtime/runtime-resolved-worktree-cache.ts

@@ -5,9 +5,10 @@ export type ResolvedWorktreeSnapshot = {
   platformByRepoId: ReadonlyMap<string, NodeJS.Platform>
 }
 
-type ResolvedCache = ResolvedWorktreeSnapshot & { expiresAt: number }
+type ResolvedCache = ResolvedWorktreeSnapshot & { expiresAt: number; inventoryRevision: number }
 type ResolvedInFlight = {
   generation: number
+  inventoryRevision: number
   promise: Promise<ResolvedWorktreeSnapshot>
 }
 export class RuntimeResolvedWorktreeCache {
@@ -19,25 +20,43 @@ export class RuntimeResolvedWorktreeCache {
     return this.resolved
   }
 
+  /**
+   * Why the revision and not the TTL alone: a snapshot only answers for the repos that were
+   * registered when it ran. A repo added afterwards — a remote host the user just connected —
+   * is missing from it for reasons that have nothing to do with what exists on that host, and
+   * callers read the gap as a verdict that the worktree does not exist.
+   */
+  isFresh(inventoryRevision: number, now = Date.now()): boolean {
+    return Boolean(
+      this.resolved &&
+      this.resolved.inventoryRevision === inventoryRevision &&
+      this.resolved.expiresAt > now
+    )
+  }
+
   async getSnapshot(
     compute: () => Promise<ResolvedWorktreeSnapshot>,
-    ttlMs: number
+    ttlMs: number,
+    inventoryRevision: number
   ): Promise<ResolvedWorktreeSnapshot> {
-    if (this.resolved && this.resolved.expiresAt > Date.now()) {
+    if (this.resolved && this.isFresh(inventoryRevision)) {
       return this.resolved
     }
     const generation = this.resolvedGeneration
-    if (this.resolvedInFlight?.generation === generation) {
+    if (
+      this.resolvedInFlight?.generation === generation &&
+      this.resolvedInFlight.inventoryRevision === inventoryRevision
+    ) {
       return this.resolvedInFlight.promise
     }
     const promise = compute()
-    this.resolvedInFlight = { generation, promise }
+    this.resolvedInFlight = { generation, inventoryRevision, promise }
     try {
       const result = await promise
       if (generation === this.resolvedGeneration) {
         // Why stamped on completion, not entry: a compute that spent longer than the TTL would
         // otherwise publish an already-expired entry, so the next poll recomputes the same slow path.
-        this.resolved = { ...result, expiresAt: Date.now() + ttlMs }
+        this.resolved = { ...result, inventoryRevision, expiresAt: Date.now() + ttlMs }
       }
       return result
     } finally {