fix(repos): forget remote-identity deadlines for removed repo locations (#14672)

* 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.
This commit is contained in:
Neil
2026-08-14 21:36:09 -07:00
committed by GitHub
parent 2252de0f7f
commit 3a4da06ec3
2 changed files with 74 additions and 0 deletions
@@ -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<GitRemoteIdentityProbe>()
vi.mocked(probeGitRemoteIdentity).mockReturnValue(probe.promise)
@@ -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`