From 268f0e9e8de6bf02da76355b19961da26b09e579 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:31:46 -0700 Subject: [PATCH] fix(projects): enrich git remote identity for runtime-addressed repo rows (#23300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repo row stamped `executionHostId: runtime:` is how a paired client addresses a project registered in *this* process, so its files are local (`runtime-repository-registration-controller.ts`, and the helper #23184 added in `repo-execution-host.ts`). #22421 reclassified those rows as peer-owned and dropped them from the identity sweep, so `gitRemoteIdentity` never settled on remote-runtime and paired-web setups: the project stayed pending, fell back to a host-local `repo:` that never grouped across hosts, and a stale automatic avatar never repaired. Reuse `getStoredRepoExecutionHostId` instead of a second host classifier, and keep skipping only a `runtime:` row that also names a nested SSH target — that target lives in the peer's dispatch table and the same path here is a different checkout. The store matches a write against the row's own stamp, so address `updateRepo` by that stamp rather than the probe host, which is `local` for these rows and would match no row at all. --- .../repo-git-remote-avatar-refresh.test.ts | 80 ++++++++++++------- ...epo-git-remote-identity-enrichment.test.ts | 45 ++++++++++- .../repo-git-remote-identity-enrichment.ts | 22 ++--- 3 files changed, 107 insertions(+), 40 deletions(-) diff --git a/src/main/repo-git-remote-avatar-refresh.test.ts b/src/main/repo-git-remote-avatar-refresh.test.ts index ab5eb7ae114..a787f5501bf 100644 --- a/src/main/repo-git-remote-avatar-refresh.test.ts +++ b/src/main/repo-git-remote-avatar-refresh.test.ts @@ -231,34 +231,60 @@ it('repairs only the matching owner when repo IDs and paths collide across hosts expect(projectHostSetupProjectionFromRepos([local, ssh]).projects).toHaveLength(1) }) -it.each(['ssh:build', 'runtime:peer'] as const)( - 'keeps local metadata writes scoped when a same-id %s row comes first', - async (hostId) => { - const foreignIdentity = identity('https://github.com/foreign/app.git') - const foreignIcon = githubAvatarIcon({ owner: 'foreign', repo: 'app' }) - const foreign = repo({ - executionHostId: hostId, - gitRemoteIdentity: foreignIdentity, - repoIcon: foreignIcon - }) - const local = repo({ gitRemoteIdentity: identity('https://github.com/old-local/app.git') }) - const store = storeFor([foreign, local]) - vi.mocked(probeGitRemoteIdentity).mockImplementation(async (_path, probeHostId) => ({ - status: 'resolved', - identity: probeHostId === 'local' ? identity() : foreignIdentity - })) - await refresh(store) - expect(foreign.gitRemoteIdentity).toBe(foreignIdentity) - expect(foreign.repoIcon).toBe(foreignIcon) - expect(local.gitRemoteIdentity).toEqual(identity()) - expect(local.repoIcon).toEqual(githubAvatarIcon({ owner: 'org-b', repo: 'app' })) - expect(store.updateRepo).toHaveBeenCalledExactlyOnceWith( - 'app', - { gitRemoteIdentity: identity(), repoIcon: local.repoIcon }, - 'local' - ) +it('keeps local metadata writes scoped when a same-id ssh:build row comes first', async () => { + const foreignIdentity = identity('https://github.com/foreign/app.git') + const foreignIcon = githubAvatarIcon({ owner: 'foreign', repo: 'app' }) + const foreign = repo({ + executionHostId: 'ssh:build', + gitRemoteIdentity: foreignIdentity, + repoIcon: foreignIcon + }) + const local = repo({ gitRemoteIdentity: identity('https://github.com/old-local/app.git') }) + const store = storeFor([foreign, local]) + vi.mocked(probeGitRemoteIdentity).mockImplementation(async (_path, probeHostId) => ({ + status: 'resolved', + identity: probeHostId === 'local' ? identity() : foreignIdentity + })) + await refresh(store) + expect(foreign.gitRemoteIdentity).toBe(foreignIdentity) + expect(foreign.repoIcon).toBe(foreignIcon) + expect(local.gitRemoteIdentity).toEqual(identity()) + expect(local.repoIcon).toEqual(githubAvatarIcon({ owner: 'org-b', repo: 'app' })) + expect(store.updateRepo).toHaveBeenCalledExactlyOnceWith( + 'app', + { gitRemoteIdentity: identity(), repoIcon: local.repoIcon }, + 'local' + ) +}) + +// Why this row is repaired where the `ssh:build` one above is not: a `runtime:` stamp reaches this +// store only from a client addressing *this* host, so the files are here and `local` is the only +// host that can answer for them (`getStoredRepoExecutionHostId`). Both rows are written, each +// addressed by its own stamp, because that is what the store matches a write against. +it('repairs a same-id runtime-addressed row under its own stamp', async () => { + const runtimeRow = repo({ + executionHostId: 'runtime:env-a', + gitRemoteIdentity: identity('https://github.com/old-runtime/app.git'), + repoIcon: githubAvatarIcon({ owner: 'old-runtime', repo: 'app' }) + }) + const local = repo({ gitRemoteIdentity: identity('https://github.com/old-local/app.git') }) + const store = storeFor([runtimeRow, local]) + vi.mocked(probeGitRemoteIdentity).mockResolvedValue({ status: 'resolved', identity: identity() }) + await refresh(store) + expect(probeGitRemoteIdentity).toHaveBeenCalledWith( + '/workspace/app', + 'local', + expect.objectContaining({ signal: expect.any(AbortSignal) }) + ) + const repaired = { + gitRemoteIdentity: identity(), + repoIcon: githubAvatarIcon({ owner: 'org-b', repo: 'app' }) } -) + expect(runtimeRow).toMatchObject(repaired) + expect(local).toMatchObject(repaired) + expect(store.updateRepo).toHaveBeenCalledWith('app', repaired, 'runtime:env-a') + expect(store.updateRepo).toHaveBeenCalledWith('app', repaired, 'local') +}) it('does not write after a pending local probe becomes peer-owned', async () => { let answer: ((value: GitRemoteIdentityProbe) => void) | undefined diff --git a/src/main/repo-git-remote-identity-enrichment.test.ts b/src/main/repo-git-remote-identity-enrichment.test.ts index 8012145fdbc..d1e54e11de8 100644 --- a/src/main/repo-git-remote-identity-enrichment.test.ts +++ b/src/main/repo-git-remote-identity-enrichment.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +import { getRepoExecutionHostId, type ExecutionHostId } from '../shared/execution-host' import type { GitRemoteIdentity } from '../shared/git-remote-identity' import type { Repo } from '../shared/repo-types' import { type GitRemoteIdentityProbe, probeGitRemoteIdentity } from './repo-git-remote-identity' @@ -15,7 +16,11 @@ vi.mock('./repo-git-remote-identity', () => ({ type RepoIdentityStore = { getRepos: () => Repo[] getRepo: (id: string) => Repo | undefined - updateRepo: (id: string, updates: Pick, 'gitRemoteIdentity'>) => Repo | null + updateRepo: ( + id: string, + updates: Pick, 'gitRemoteIdentity'>, + hostId?: ExecutionHostId + ) => Repo | null } const remoteIdentity: GitRemoteIdentity = { @@ -42,8 +47,13 @@ function makeStore(...repos: Repo[]): RepoIdentityStore & { updateRepo: ReturnTy return { getRepos: () => repos, getRepo: (id) => repos.find((candidate) => candidate.id === id), - updateRepo: vi.fn((id, updates) => { - const target = repos.find((candidate) => candidate.id === id) + // Mirrors the real store: `hostId` is matched against the row's own stamp, so a write + // addressed to the wrong host finds no row (src/main/persistence/tracking-repos). + updateRepo: vi.fn((id, updates, hostId) => { + const target = repos.find( + (candidate) => + candidate.id === id && (!hostId || getRepoExecutionHostId(candidate) === hostId) + ) if (!target) { return null } @@ -129,7 +139,10 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { await flushRepoGitRemoteIdentityEnrichmentForTests() }) - it('never probes a runtime row through this client dispatch table', async () => { + it('never probes a runtime row that names a nested SSH target', async () => { + // Why: `connectionId` on a `runtime:` row names a target inside that server's namespace, so + // dialing it here reaches a same-named box of ours, and the same path on this machine is a + // different checkout. Neither host is probeable from here, so the row is skipped outright. vi.mocked(probeGitRemoteIdentity).mockResolvedValue({ status: 'unavailable' }) const store = makeStore( makeRepo({ connectionId: 'nested-1', executionHostId: 'runtime:env-a' }) @@ -141,6 +154,30 @@ describe('enrichMissingRepoGitRemoteIdentities', () => { expect(store.updateRepo).not.toHaveBeenCalled() }) + it('probes a self-addressed runtime row here and writes it back under its own stamp', async () => { + // Why: a bare `runtime:` stamp is how a paired client addresses a repo registered in *this* + // process, so its files are local. Skipping it left `gitRemoteIdentity` unset forever, which + // consumers read as pending. The write must carry the row's stamp, not the probe host — the + // store matches that argument against the stamp, so `local` would match no row. + vi.mocked(probeGitRemoteIdentity).mockResolvedValue(resolvedProbe) + const repo = makeRepo({ executionHostId: 'runtime:env-a' }) + const store = makeStore(repo) + + await sweep(store) + + expect(probeGitRemoteIdentity).toHaveBeenCalledWith( + '/workspace/sample-app', + 'local', + expect.objectContaining({ signal: expect.any(AbortSignal) }) + ) + expect(store.updateRepo).toHaveBeenCalledWith( + 'repo-1', + { gitRemoteIdentity: remoteIdentity }, + 'runtime:env-a' + ) + expect(repo.gitRemoteIdentity).toEqual(remoteIdentity) + }) + it('keeps same-path rows on two different SSH hosts from sharing one backoff', async () => { // Why: the location key decides coalescing and backoff. Keyed on the raw field, two rows that // carry only `executionHostId` collapse onto one key, so the first host being down suppresses diff --git a/src/main/repo-git-remote-identity-enrichment.ts b/src/main/repo-git-remote-identity-enrichment.ts index 4c81174a79b..f1cea725efc 100644 --- a/src/main/repo-git-remote-identity-enrichment.ts +++ b/src/main/repo-git-remote-identity-enrichment.ts @@ -1,6 +1,6 @@ import { getRepoExecutionHostId, - getSshTargetIdForExecutionHost, + getRepoSshConnectionId, LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../shared/execution-host' @@ -8,6 +8,7 @@ import type { Repo } from '../shared/repo-types' import { githubAvatarIcon, type RepoIcon } from '../shared/repo-icon' import { isUnresolvedSshHostAlias } from '../shared/git-remote-host-alias' import { getProjectProviderIdentity } from '../shared/project-host-setup-projection' +import { getStoredRepoExecutionHostId } from './repo-execution-host' import { probeGitRemoteIdentity } from './repo-git-remote-identity' const NO_IDENTITY_RETRY_TTL_MS = 5 * 60 * 1000 @@ -58,12 +59,13 @@ function getRepoLocationKey(repo: Pick, 'gitRemoteIdentity' | 'repoIcon'>): Repo | null => { - return store.updateRepo(snapshot.id, updates, hostId) + return store.updateRepo(snapshot.id, updates, storeHostId) } if (icon) { return !!update({ ...(writeRemote ? { gitRemoteIdentity } : {}), repoIcon: icon })