From 3a4da06ec31f68be96adff034e940d28ff75fedf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:36:09 -0700 Subject: [PATCH] fix(repos): forget remote-identity deadlines for removed repo locations (#14672) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(repos): forget remote-identity deadlines for removed repo locations `probeRetryAfterByLocation` is keyed by connection and path and was written for both resolved and unresolved probes, but never pruned — a removed repo or a retired SSH host kept its deadline for the life of the process. `isIdentityRefreshDue` also seeds an entry for every resolved repo on first sight, so the map grew with repository and host churn even with no probe activity. The candidate sweep already enumerates every live repo, so it now reconciles the deadline map against those location keys. `getRepos()` reads a hydrated in-memory array, so a repo is never transiently absent mid-sweep and cannot lose its startup delay or its backoff. Locations with a probe still in flight are kept, since that probe re-adds its own key when it settles. * refactor(repos): drop the no-op in-flight exemption from the deadline prune The exemption's own comment named the reason it was unnecessary: a probe re-adds its deadline when it settles, so skipping its key produced the same map state as deleting it, and only live repos are ever candidates so nothing read the entry in between. Removing it also stops a probe that never settles from pinning its deadline forever — the location's `git remote -v` can already hang with no timeout, and the exemption turned that one stranded entry into two. --- ...epo-git-remote-identity-enrichment.test.ts | 54 +++++++++++++++++++ .../repo-git-remote-identity-enrichment.ts | 20 +++++++ 2 files changed, 74 insertions(+) diff --git a/src/main/repo-git-remote-identity-enrichment.test.ts b/src/main/repo-git-remote-identity-enrichment.test.ts index b396e8a7e38..40f1c0ee2d9 100644 --- a/src/main/repo-git-remote-identity-enrichment.test.ts +++ b/src/main/repo-git-remote-identity-enrichment.test.ts @@ -329,6 +329,60 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { expect(probeGitRemoteIdentity).not.toHaveBeenCalled() }) + it('forgets a removed repo location so its deadline cannot outlive the repo', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + vi.mocked(probeGitRemoteIdentity).mockResolvedValue(resolvedProbe) + const repo = makeRepo({ gitRemoteIdentity: remoteIdentity }) + const live: Repo[] = [repo] + const store: RepoIdentityStore = { + getRepos: () => live, + getRepo: (id) => live.find((candidate) => candidate.id === id), + updateRepo: () => null + } + + // Seeds the startup-delay deadline for this location. + await sweep(store) + live.length = 0 + await sweep(store) + live.push(repo) + // Past the seeded deadline: a retained entry would make this location due at once. + vi.setSystemTime(1_000 + REFRESH_STARTUP_DELAY_MS + 1) + await sweep(store) + + expect(probeGitRemoteIdentity).not.toHaveBeenCalled() + }) + + it('keeps a surviving repo backoff when a sibling repo is removed', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000) + vi.mocked(probeGitRemoteIdentity).mockResolvedValue(resolvedProbe) + const kept = makeRepo({ gitRemoteIdentity: remoteIdentity }) + const removed = makeRepo({ + id: 'repo-2', + path: '/workspace/other-app', + gitRemoteIdentity: remoteIdentity + }) + const live: Repo[] = [kept, removed] + const store: RepoIdentityStore = { + getRepos: () => live, + getRepo: (id) => live.find((candidate) => candidate.id === id), + updateRepo: () => null + } + + await sweep(store) + vi.setSystemTime(1_000 + REFRESH_STARTUP_DELAY_MS + 1) + await sweep(store) + expect(probeGitRemoteIdentity).toHaveBeenCalledTimes(2) + + live.splice(1, 1) + await sweep(store) + + // The kept repo is still inside its 6h refresh window, so pruning its sibling + // must not make it due again. + expect(probeGitRemoteIdentity).toHaveBeenCalledTimes(2) + }) + it('does not write stale identity data after the repo path changes', async () => { const probe = deferred() vi.mocked(probeGitRemoteIdentity).mockReturnValue(probe.promise) diff --git a/src/main/repo-git-remote-identity-enrichment.ts b/src/main/repo-git-remote-identity-enrichment.ts index 145e33ddf90..08a059294a3 100644 --- a/src/main/repo-git-remote-identity-enrichment.ts +++ b/src/main/repo-git-remote-identity-enrichment.ts @@ -119,9 +119,29 @@ function isIdentityRefreshDue(repo: Repo, now: number): boolean { return dueAt <= now } +/** + * Drop deadlines for locations no longer backed by a repo, so removed repos and + * retired SSH hosts do not accumulate for the life of the process. + */ +function pruneRetryDeadlines(liveRepos: Repo[]): void { + const liveKeys = new Set(liveRepos.map(getRepoLocationKey)) + for (const locationKey of probeRetryAfterByLocation.keys()) { + // A probe still running for a dropped repo needs no exemption: it re-adds its + // own deadline on settle, and only live repos are ever candidates, so nothing + // reads this entry in between. + if (!liveKeys.has(locationKey)) { + probeRetryAfterByLocation.delete(locationKey) + } + } +} + function selectEnrichmentCandidates(store: RepoIdentityStore): Repo[] { const now = Date.now() const repos = store.getRepos().filter((repo) => repo.kind !== 'folder') + // Why here: this is the one place that already enumerates every live repo, and + // `getRepos()` builds its list synchronously from in-memory state, so a repo is + // never transiently absent mid-sweep and cannot lose its startup delay or backoff. + pruneRetryDeadlines(repos) // Why: the settled `null` marker stays a candidate on purpose — a repo that // gains a remote later must still resolve. Do not tighten this to // `=== undefined`; the retry TTL already bounds the cost and `writeIdentity`