mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 08:02:21 +00:00
fix(projects): enrich git remote identity for runtime-addressed repo rows (#23300)
A repo row stamped `executionHostId: runtime:<env>` 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:<id>` 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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<Partial<Repo>, 'gitRemoteIdentity'>) => Repo | null
|
||||
updateRepo: (
|
||||
id: string,
|
||||
updates: Pick<Partial<Repo>, '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
|
||||
|
||||
@@ -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<Repo, 'path' | 'connectionId' | 'executio
|
||||
return `${getRepoExecutionHostId(repo)}\0${repo.path}`
|
||||
}
|
||||
|
||||
// A peer's nested SSH targets belong to its dispatch table, never this client's.
|
||||
// A peer's nested SSH targets belong to its dispatch table, never this client's, so a `runtime:`
|
||||
// row that names one has no host here to probe. A `runtime:` stamp on its own is how a paired
|
||||
// client addresses files registered in *this* process (`getStoredRepoExecutionHostId`), so that row
|
||||
// keeps the local probe it has always had — dropping it leaves its identity permanently pending.
|
||||
function getRepoProbeHostId(repo: Repo): ExecutionHostId | null {
|
||||
const hostId = getRepoExecutionHostId(repo)
|
||||
return hostId === LOCAL_EXECUTION_HOST_ID || getSshTargetIdForExecutionHost(hostId)
|
||||
? hostId
|
||||
: null
|
||||
const hostId = getStoredRepoExecutionHostId(repo)
|
||||
return hostId === LOCAL_EXECUTION_HOST_ID && getRepoSshConnectionId(repo) ? null : hostId
|
||||
}
|
||||
|
||||
function getCurrentRepo(store: RepoIdentityStore, snapshot: Repo): Repo | undefined {
|
||||
@@ -131,8 +133,7 @@ function writeIdentity(
|
||||
gitRemoteIdentity: Repo['gitRemoteIdentity']
|
||||
): boolean {
|
||||
// A peer's repo metadata must never be repaired from a client-local probe.
|
||||
const hostId = getRepoProbeHostId(snapshot)
|
||||
if (!hostId) {
|
||||
if (!getRepoProbeHostId(snapshot)) {
|
||||
return false
|
||||
}
|
||||
const current = getCurrentRepo(store, snapshot)
|
||||
@@ -143,8 +144,11 @@ function writeIdentity(
|
||||
const icon = gitRemoteIdentity
|
||||
? getAutomaticGitHubIconRefresh(current, gitRemoteIdentity)
|
||||
: undefined
|
||||
// The row's own host, not the probe's: the store matches this argument against the row's stamp,
|
||||
// so a `runtime:` row probed locally is only addressable here under that stamp.
|
||||
const storeHostId = getRepoExecutionHostId(snapshot)
|
||||
const update = (updates: Pick<Partial<Repo>, '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 })
|
||||
|
||||
Reference in New Issue
Block a user