fix(repos): route icon and remote-identity probes on a resolved execution host (#18377)

`detectRepoIcon`, `detectRepoIconAndUpstream`, `detectGitHubAvatarIcon`,
`detectRepoFileIcon` and `probeGitRemoteIdentity` took a `connectionId`-shaped
parameter threaded down from their callers. That shape spells "runtime host",
"unresolved" and "genuinely local" all as one falsy value, and because it is a
*parameter* each caller decided independently what to pass — a wrong answer was
invisible at the boundary.

Replace it with a required `ExecutionHostId` and route through #18296's
`resolveGitRouteForHost` / `resolveFilesystemRouteForHost`. The parameter is
removed rather than added beside, so every caller became a compile error. No new
resolver, no wire change: nothing these modules return carries a host id.

Fixed at the call sites:

- `repo-git-remote-identity-enrichment` read `repo.connectionId` raw, so a row
  minted with only `executionHostId: ssh:<t>` ran `git remote -v` against this
  machine's copy of the path (#11163), and a `runtime:` row handed its *nested*
  SSH target to this client's dispatch table — a same-named box of ours.
- Its location key had the same collapse, so two rows at one path on different
  hosts shared a probe, an abort controller and a backoff deadline.
- `runtime-repository-fork-backfill` guarded on `repo.connectionId`, so an
  `executionHostId`-only SSH row had its upstream read off the client.

`runtime:` is refused inside the modules (this process does not execute another
environment's git or filesystem), but store-backed callers ask
`getSshTargetIdForExecutionHost` — "what may this client dial" — so a `runtime:`
row keeps the probe this process has always run for it. Registering and cloning
stay `local` on purpose: those controllers do the filesystem work here, whatever
host id is stamped on the row (see `assertCloneHostIsSupported`).
This commit is contained in:
Neil
2026-09-03 00:49:36 -07:00
committed by GitHub
parent 968dbd905f
commit 316ec38f67
17 changed files with 470 additions and 118 deletions
@@ -11,6 +11,7 @@ import {
getLinkedWorktreeMainRepoRoot,
getRepoName
} from '../../git/repo'
import { LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host'
import { detectRepoIconAndUpstream } from '../../repo-icon-autodetect'
import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation'
@@ -73,7 +74,11 @@ export async function addLocalRepoFromPath(
}
}
const detected = await detectRepoIconAndUpstream({ repoPath: resolvedPath, kind: repoKind })
const detected = await detectRepoIconAndUpstream({
repoPath: resolvedPath,
kind: repoKind,
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path: resolvedPath,
@@ -14,6 +14,7 @@ import {
} from '../../project-groups/nested-repo-import'
import { createNestedRepoImportTargetResolver } from '../../project-groups/nested-repo-import-target'
import { getSshGitProvider } from '../../providers/ssh-git-dispatch'
import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../../shared/execution-host'
import { detectRepoIconAndUpstream } from '../../repo-icon-autodetect'
import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation'
import { getActiveMultiplexer } from '../ssh'
@@ -122,7 +123,9 @@ export function registerNestedRepoImportHandler(mainWindow: BrowserWindow, store
const detected = await detectRepoIconAndUpstream({
repoPath: importRepoPath,
kind: 'git',
connectionId: args.connectionId
executionHostId: args.connectionId
? toSshExecutionHostId(args.connectionId)
: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
@@ -84,7 +84,7 @@ export async function addRemoteRepoFromPath(
const detected = await detectRepoIconAndUpstream({
repoPath: resolvedPath,
kind: repoKind,
connectionId: args.connectionId
executionHostId: toSshExecutionHostId(args.connectionId)
})
const repo: Repo = {
id: randomUUID(),
+6 -1
View File
@@ -17,6 +17,7 @@ import {
deriveValidatedClonePath,
getClonePathComparisonKey
} from '../../git/repo-clone-path'
import { LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host'
import { detectRepoIconAndUpstream } from '../../repo-icon-autodetect'
import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation'
import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache'
@@ -257,7 +258,11 @@ export function registerRepoCloneHandlers(mainWindow: BrowserWindow, store: Stor
return existing
}
const detected = await detectRepoIconAndUpstream({ repoPath: clonePath, kind: 'git' })
const detected = await detectRepoIconAndUpstream({
repoPath: clonePath,
kind: 'git',
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path: clonePath,
+5 -1
View File
@@ -264,7 +264,11 @@ export function registerRepoCreationHandlers(mainWindow: BrowserWindow, store: S
return { repo: raceWinner }
}
const detected = await detectRepoIconAndUpstream({ repoPath: targetPath, kind: repoKind })
const detected = await detectRepoIconAndUpstream({
repoPath: targetPath,
kind: repoKind,
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path: targetPath,
@@ -103,7 +103,7 @@ describe('enrichMissingRepoGitRemoteIdentities', () => {
expect(repo.gitRemoteIdentity).toBeUndefined()
expect(probeGitRemoteIdentity).toHaveBeenCalledWith(
'/workspace/sample-app',
undefined,
'local',
expect.objectContaining({ signal: expect.any(AbortSignal) })
)
@@ -113,6 +113,60 @@ describe('enrichMissingRepoGitRemoteIdentities', () => {
expect(onChanged).toHaveBeenCalledTimes(1)
})
it('probes an SSH row that carries only executionHostId on its own host', async () => {
// Why: a row minted with the unified spelling has no `connectionId`, and reading the raw field
// would run `git remote -v` against a same-named path on this machine (#11163).
vi.mocked(probeGitRemoteIdentity).mockResolvedValue(resolvedProbe)
const store = makeStore(makeRepo({ executionHostId: 'ssh:builder' }))
enrichMissingRepoGitRemoteIdentities(store)
expect(probeGitRemoteIdentity).toHaveBeenCalledWith(
'/workspace/sample-app',
'ssh:builder',
expect.objectContaining({ signal: expect.any(AbortSignal) })
)
await flushRepoGitRemoteIdentityEnrichmentForTests()
})
it('never hands a runtime row nested SSH target to this client dispatch table', 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. The row keeps the probe this process has
// always run for it; what it must never do is dial our same-named target.
vi.mocked(probeGitRemoteIdentity).mockResolvedValue({ status: 'unavailable' })
const store = makeStore(
makeRepo({ connectionId: 'nested-1', executionHostId: 'runtime:env-a' })
)
enrichMissingRepoGitRemoteIdentities(store)
expect(probeGitRemoteIdentity).toHaveBeenCalledWith(
'/workspace/sample-app',
'local',
expect.objectContaining({ signal: expect.any(AbortSignal) })
)
await flushRepoGitRemoteIdentityEnrichmentForTests()
})
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
// the probe for the second one entirely.
vi.mocked(probeGitRemoteIdentity).mockResolvedValue({ status: 'unavailable' })
const store = makeStore(
makeRepo({ id: 'repo-m4air', executionHostId: 'ssh:m4air' }),
makeRepo({ id: 'repo-openclaw', executionHostId: 'ssh:openclaw' })
)
await sweep(store)
expect(probeGitRemoteIdentity).toHaveBeenCalledTimes(2)
expect(vi.mocked(probeGitRemoteIdentity).mock.calls.map((call) => call[1])).toEqual([
'ssh:m4air',
'ssh:openclaw'
])
})
it('coalesces concurrent probes for the same repo location', async () => {
const probe = deferred<GitRemoteIdentityProbe>()
vi.mocked(probeGitRemoteIdentity).mockReturnValue(probe.promise)
@@ -1,3 +1,9 @@
import {
getRepoExecutionHostId,
getSshTargetIdForExecutionHost,
LOCAL_EXECUTION_HOST_ID,
type ExecutionHostId
} from '../shared/execution-host'
import type { Repo } from '../shared/repo-types'
import { probeGitRemoteIdentity } from './repo-git-remote-identity'
@@ -39,8 +45,20 @@ const pendingChangeListeners = new Set<() => void>()
let sweepInFlight: Promise<void> | null = null
let rerunRequested = false
function getRepoLocationKey(repo: Pick<Repo, 'path' | 'connectionId'>): string {
return `${repo.connectionId ?? 'local'}\0${repo.path}`
// Keyed on the resolved host, not the raw field: two rows at the same path on different hosts are
// different locations, and collapsing them shares one probe (and one abort) across both.
function getRepoLocationKey(repo: Pick<Repo, 'path' | 'connectionId' | 'executionHostId'>): string {
return `${getRepoExecutionHostId(repo)}\0${repo.path}`
}
/**
* The host *this* process may run the probe on. `getSshTargetIdForExecutionHost`, not the row's
* file-holding target: a `runtime:` row's nested SSH target lives in that server's namespace, so
* dialing it here would reach a same-named box of ours — a wrong-host answer, not a local one.
*/
function getRepoProbeHostId(repo: Repo): ExecutionHostId {
const hostId = getRepoExecutionHostId(repo)
return getSshTargetIdForExecutionHost(hostId) ? hostId : LOCAL_EXECUTION_HOST_ID
}
function getCurrentRepo(store: RepoIdentityStore, id: string): Repo | undefined {
@@ -52,7 +70,7 @@ function isSameProbedRepo(snapshot: Repo, current: Repo | undefined): current is
!!current &&
current.kind !== 'folder' &&
current.path === snapshot.path &&
(current.connectionId ?? null) === (snapshot.connectionId ?? null)
getRepoExecutionHostId(current) === getRepoExecutionHostId(snapshot)
)
}
@@ -101,7 +119,7 @@ async function enrichRepoGitRemoteIdentity(store: RepoIdentityStore, repo: Repo)
: NO_IDENTITY_RETRY_TTL_MS
const controller = new AbortController()
const promise = (async () => {
const result = await probeGitRemoteIdentity(repo.path, repo.connectionId, {
const result = await probeGitRemoteIdentity(repo.path, getRepoProbeHostId(repo), {
signal: controller.signal
})
// Why the signal and not a catch: probeGitRemoteIdentity swallows the AbortError and RESOLVES
+81 -26
View File
@@ -1,56 +1,112 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { gitExecFileAsync } from './git/runner'
import { getSshGitProvider } from './providers/ssh-git-dispatch'
import { registerSshGitProvider, unregisterSshGitProvider } from './providers/ssh-git-dispatch'
import { probeGitRemoteIdentity } from './repo-git-remote-identity'
vi.mock('./git/runner', () => ({ gitExecFileAsync: vi.fn() }))
vi.mock('./providers/ssh-git-dispatch', () => ({ getSshGitProvider: vi.fn() }))
const gitlabRemote = 'origin\tgit@gitlab.example.com:team/orca.git (fetch)\n'
const gitlabIdentity = {
canonicalKey: 'gitlab.example.com/team/orca',
remoteName: 'origin',
remoteUrl: 'git@gitlab.example.com:team/orca.git'
}
const registered: string[] = []
function registerHost(connectionId: string, stdout = gitlabRemote) {
const exec = vi.fn().mockResolvedValue({ stdout, stderr: '' })
registerSshGitProvider(connectionId, { exec } as never)
registered.push(connectionId)
return exec
}
beforeEach(() => {
vi.clearAllMocks()
})
afterEach(() => {
for (const connectionId of registered.splice(0)) {
unregisterSshGitProvider(connectionId)
}
})
describe('probeGitRemoteIdentity', () => {
it('resolves the canonical identity for a non-GitHub remote', async () => {
vi.mocked(gitExecFileAsync).mockResolvedValue({ stdout: gitlabRemote, stderr: '' })
await expect(probeGitRemoteIdentity('/repos/orca')).resolves.toEqual({
await expect(probeGitRemoteIdentity('/repos/orca', 'local')).resolves.toEqual({
status: 'resolved',
identity: {
canonicalKey: 'gitlab.example.com/team/orca',
remoteName: 'origin',
remoteUrl: 'git@gitlab.example.com:team/orca.git'
}
identity: gitlabIdentity
})
})
it('settles on no-remote when git answers with nothing usable', async () => {
vi.mocked(gitExecFileAsync).mockResolvedValue({ stdout: '', stderr: '' })
await expect(probeGitRemoteIdentity('/repos/orca')).resolves.toEqual({ status: 'no-remote' })
await expect(probeGitRemoteIdentity('/repos/orca', 'local')).resolves.toEqual({
status: 'no-remote'
})
})
it('routes each SSH host to its own git provider', async () => {
const m4air = registerHost('m4air')
const openclaw = registerHost(
'openclaw',
'origin\tgit@gitlab.example.com:team/other.git (fetch)\n'
)
await expect(probeGitRemoteIdentity('/repos/orca', 'ssh:m4air')).resolves.toEqual({
status: 'resolved',
identity: gitlabIdentity
})
await expect(probeGitRemoteIdentity('/repos/orca', 'ssh:openclaw')).resolves.toEqual({
status: 'resolved',
identity: {
canonicalKey: 'gitlab.example.com/team/other',
remoteName: 'origin',
remoteUrl: 'git@gitlab.example.com:team/other.git'
}
})
expect(m4air).toHaveBeenCalledTimes(1)
expect(openclaw).toHaveBeenCalledTimes(1)
expect(gitExecFileAsync).not.toHaveBeenCalled()
})
it('reports unavailable when the SSH host has no connected git provider', async () => {
vi.mocked(getSshGitProvider).mockReturnValue(undefined)
await expect(probeGitRemoteIdentity('/repos/orca', 'builder')).resolves.toEqual({
await expect(probeGitRemoteIdentity('/repos/orca', 'ssh:builder')).resolves.toEqual({
status: 'unavailable'
})
expect(gitExecFileAsync).not.toHaveBeenCalled()
})
// A runtime host's Git is executed by that environment's own server, and the SSH target on its
// repo row lives in that server's namespace. Dialing a same-named target here answers for
// another machine's repository.
it('refuses a runtime host even when its nested SSH target is registered on this client', async () => {
const nested = registerHost('nested-1')
await expect(probeGitRemoteIdentity('/repos/orca', 'runtime:env-a')).resolves.toEqual({
status: 'unavailable'
})
expect(nested).not.toHaveBeenCalled()
expect(gitExecFileAsync).not.toHaveBeenCalled()
})
it('reports unavailable when the local git command fails', async () => {
vi.mocked(gitExecFileAsync).mockRejectedValue(new Error('not a git repository'))
await expect(probeGitRemoteIdentity('/repos/orca')).resolves.toEqual({ status: 'unavailable' })
await expect(probeGitRemoteIdentity('/repos/orca', 'local')).resolves.toEqual({
status: 'unavailable'
})
})
it('reports unavailable when a connected SSH provider cannot reach the host', async () => {
const exec = vi.fn().mockRejectedValue(new Error('ssh: connect to host builder: down'))
vi.mocked(getSshGitProvider).mockReturnValue({ exec } as never)
registerSshGitProvider('builder', { exec } as never)
registered.push('builder')
await expect(probeGitRemoteIdentity('/repos/orca', 'builder')).resolves.toEqual({
await expect(probeGitRemoteIdentity('/repos/orca', 'ssh:builder')).resolves.toEqual({
status: 'unavailable'
})
expect(exec).toHaveBeenCalledWith(
@@ -62,11 +118,9 @@ describe('probeGitRemoteIdentity', () => {
})
it('settles on no-remote for an SSH repo git answered for with no remotes', async () => {
vi.mocked(getSshGitProvider).mockReturnValue({
exec: vi.fn().mockResolvedValue({ stdout: '', stderr: '' })
} as never)
registerHost('builder', '')
await expect(probeGitRemoteIdentity('/repos/orca', 'builder')).resolves.toEqual({
await expect(probeGitRemoteIdentity('/repos/orca', 'ssh:builder')).resolves.toEqual({
status: 'no-remote'
})
})
@@ -75,7 +129,7 @@ describe('probeGitRemoteIdentity', () => {
vi.mocked(gitExecFileAsync).mockResolvedValue({ stdout: gitlabRemote, stderr: '' })
const controller = new AbortController()
await probeGitRemoteIdentity('/repos/orca', null, { signal: controller.signal })
await probeGitRemoteIdentity('/repos/orca', 'local', { signal: controller.signal })
expect(gitExecFileAsync).toHaveBeenCalledWith(
['remote', '-v'],
@@ -90,11 +144,10 @@ describe('probeGitRemoteIdentity', () => {
})
it('bounds the SSH probe under the relay request timeout and forwards the caller signal', async () => {
const exec = vi.fn().mockResolvedValue({ stdout: gitlabRemote, stderr: '' })
vi.mocked(getSshGitProvider).mockReturnValue({ exec } as never)
const exec = registerHost('builder')
const controller = new AbortController()
await probeGitRemoteIdentity('/repos/orca', 'builder', { signal: controller.signal })
await probeGitRemoteIdentity('/repos/orca', 'ssh:builder', { signal: controller.signal })
expect(exec).toHaveBeenCalledWith(
['remote', '-v'],
@@ -108,7 +161,9 @@ describe('probeGitRemoteIdentity', () => {
it('maps a timed-out local probe to unavailable, never no-remote', async () => {
vi.mocked(gitExecFileAsync).mockRejectedValue(new Error('git timed out.'))
await expect(probeGitRemoteIdentity('/repos/orca')).resolves.toEqual({ status: 'unavailable' })
await expect(probeGitRemoteIdentity('/repos/orca', 'local')).resolves.toEqual({
status: 'unavailable'
})
})
it('maps an aborted probe to unavailable, never no-remote', async () => {
@@ -119,7 +174,7 @@ describe('probeGitRemoteIdentity', () => {
controller.abort()
await expect(
probeGitRemoteIdentity('/repos/orca', null, { signal: controller.signal })
probeGitRemoteIdentity('/repos/orca', 'local', { signal: controller.signal })
).resolves.toEqual({ status: 'unavailable' })
})
})
+24 -14
View File
@@ -1,6 +1,7 @@
import type { ExecutionHostId } from '../shared/execution-host'
import { deriveGitRemoteIdentity, type GitRemoteIdentity } from '../shared/git-remote-identity'
import { gitExecFileAsync } from './git/runner'
import { getSshGitProvider } from './providers/ssh-git-dispatch'
import { resolveGitRouteForHost } from './providers/execution-host-provider-dispatch'
// Why: the runner only arms its kill timer when a timeout is passed, so an unbounded local probe
// never settles on a hung NFS/SMB cwd (the path walk blocks) or a wedged `wsl.exe -d <distro>`.
@@ -26,20 +27,29 @@ export type GitRemoteIdentityProbe =
export async function probeGitRemoteIdentity(
repoPath: string,
connectionId?: string | null,
executionHostId: ExecutionHostId,
options: GitRemoteIdentityProbeOptions = {}
): Promise<GitRemoteIdentityProbe> {
try {
const result = connectionId
? await getSshGitProvider(connectionId)?.exec(['remote', '-v'], repoPath, {
signal: options.signal,
timeoutMs: options.timeoutMs ?? SSH_PROBE_TIMEOUT_MS
})
: await gitExecFileAsync(['remote', '-v'], {
cwd: repoPath,
timeout: options.timeoutMs ?? LOCAL_PROBE_TIMEOUT_MS,
signal: options.signal
})
// Inside the try on purpose: an id naming no host must land on `unavailable` like every other
// probe that never reached git. It must never become the local answer for a remote path.
const route = resolveGitRouteForHost(executionHostId)
if (route.kind === 'runtime') {
// That environment's server runs its own git, and the SSH target on its repo row is nested in
// that server's namespace — dialing it here answers for a same-named box of ours.
return { status: 'unavailable' }
}
const result =
route.kind === 'ssh'
? await route.provider?.exec(['remote', '-v'], repoPath, {
signal: options.signal,
timeoutMs: options.timeoutMs ?? SSH_PROBE_TIMEOUT_MS
})
: await gitExecFileAsync(['remote', '-v'], {
cwd: repoPath,
timeout: options.timeoutMs ?? LOCAL_PROBE_TIMEOUT_MS,
signal: options.signal
})
if (!result) {
return { status: 'unavailable' }
}
@@ -54,9 +64,9 @@ export async function probeGitRemoteIdentity(
export async function detectGitRemoteIdentity(
repoPath: string,
connectionId?: string | null,
executionHostId: ExecutionHostId,
options: GitRemoteIdentityProbeOptions = {}
): Promise<GitRemoteIdentity | null> {
const probe = await probeGitRemoteIdentity(repoPath, connectionId, options)
const probe = await probeGitRemoteIdentity(repoPath, executionHostId, options)
return probe.status === 'resolved' ? probe.identity : null
}
+109 -18
View File
@@ -1,8 +1,13 @@
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import { afterEach, describe, expect, it } from 'vitest'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { gitExecFileAsync } from './git/runner'
import {
registerSshFilesystemProvider,
unregisterSshFilesystemProvider
} from './providers/ssh-filesystem-dispatch'
import type { IFilesystemProvider } from './providers/types'
import { detectRepoIcon, detectRepoIconAndUpstream } from './repo-icon-autodetect'
const PNG_1X1_BASE64 =
@@ -16,7 +21,30 @@ async function makeTempRepoDir(): Promise<string> {
return dir
}
const registeredHosts: string[] = []
/** A remote host whose only readable file is a package.json naming a host-specific homepage. */
function registerHomepageHost(connectionId: string, homepage: string) {
const stat = vi.fn(async (filePath: string) => {
if (!filePath.endsWith('/package.json')) {
throw new Error('ENOENT')
}
return { type: 'file', size: 64, mtime: 0 }
})
const readFile = vi.fn(async () => ({
content: JSON.stringify({ homepage }),
isBinary: false,
mimeType: 'application/json'
}))
registerSshFilesystemProvider(connectionId, { stat, readFile } as unknown as IFilesystemProvider)
registeredHosts.push(connectionId)
return { stat, readFile }
}
afterEach(async () => {
for (const connectionId of registeredHosts.splice(0)) {
unregisterSshFilesystemProvider(connectionId)
}
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })))
})
@@ -29,7 +57,9 @@ describe('detectRepoIcon', () => {
JSON.stringify({ homepage: 'https://example.com' })
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: `data:image/png;base64,${PNG_1X1_BASE64}`,
source: 'file',
@@ -45,7 +75,9 @@ describe('detectRepoIcon', () => {
Buffer.from(PNG_1X1_BASE64, 'base64')
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: `data:image/png;base64,${PNG_1X1_BASE64}`,
source: 'file',
@@ -59,7 +91,9 @@ describe('detectRepoIcon', () => {
await mkdir(join(repoPath, 'public'), { recursive: true })
await writeFile(join(repoPath, 'public', 'icon.webp'), Buffer.from(webpBase64, 'base64'))
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: `data:image/webp;base64,${webpBase64}`,
source: 'file',
@@ -74,7 +108,9 @@ describe('detectRepoIcon', () => {
JSON.stringify({ homepage: 'https://app.example.com/docs' })
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: 'https://www.google.com/s2/favicons?domain=app.example.com&sz=64',
source: 'favicon',
@@ -91,7 +127,9 @@ describe('detectRepoIcon', () => {
Buffer.from(PNG_1X1_BASE64, 'base64')
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: `data:image/png;base64,${PNG_1X1_BASE64}`,
source: 'file',
@@ -111,7 +149,9 @@ describe('detectRepoIcon', () => {
Buffer.from(PNG_1X1_BASE64, 'base64')
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: `data:image/png;base64,${PNG_1X1_BASE64}`,
source: 'file',
@@ -131,7 +171,9 @@ describe('detectRepoIcon', () => {
Buffer.from(PNG_1X1_BASE64, 'base64')
)
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toBeUndefined()
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toBeUndefined()
})
it('does not resolve declared icon hrefs outside the repo', async () => {
@@ -141,7 +183,9 @@ describe('detectRepoIcon', () => {
await writeFile(join(parentPath, 'outside.png'), Buffer.from(PNG_1X1_BASE64, 'base64'))
await writeFile(join(repoPath, 'index.html'), '<link rel="icon" href="../outside.png">')
await expect(detectRepoIcon({ repoPath, kind: 'folder' })).resolves.toBeUndefined()
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toBeUndefined()
})
it('returns no icon for an SSH-hosted repo whose filesystem provider is missing', async () => {
@@ -155,19 +199,54 @@ describe('detectRepoIcon', () => {
)
await expect(
detectRepoIcon({ repoPath, kind: 'folder', connectionId: 'ssh-target-not-connected' })
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'ssh:not-connected' })
).resolves.toBeUndefined()
})
it('still detects local icons when the repo has no connection', async () => {
it('still detects local icons for a repo on this machine', async () => {
const repoPath = await makeTempRepoDir()
await writeFile(join(repoPath, 'favicon.png'), Buffer.from(PNG_1X1_BASE64, 'base64'))
await expect(
detectRepoIcon({ repoPath, kind: 'folder', connectionId: null })
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'local' })
).resolves.toMatchObject({ source: 'file', label: 'favicon.png' })
})
it('routes each SSH host to its own filesystem provider', async () => {
const repoPath = await makeTempRepoDir()
// On disk on this machine, so a host-blind probe would answer with this one for both hosts.
await writeFile(join(repoPath, 'favicon.png'), Buffer.from(PNG_1X1_BASE64, 'base64'))
registerHomepageHost('m4air', 'https://m4air.example.com')
registerHomepageHost('openclaw', 'https://openclaw.example.com')
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'ssh:m4air' })
).resolves.toMatchObject({
source: 'favicon',
src: expect.stringContaining('m4air.example.com')
})
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'ssh:openclaw' })
).resolves.toMatchObject({
source: 'favicon',
src: expect.stringContaining('openclaw.example.com')
})
})
it('reads nothing for a runtime host even when its nested SSH target is registered here', async () => {
// Why: a `runtime:` repo row's `connectionId` names a target in that server's namespace. Both
// spellings of the incumbent shape are wrong here — a null one reads this machine's copy of
// the path, and the nested id dials a same-named box of ours.
const repoPath = await makeTempRepoDir()
await writeFile(join(repoPath, 'favicon.png'), Buffer.from(PNG_1X1_BASE64, 'base64'))
const nested = registerHomepageHost('nested-1', 'https://nested.example.com')
await expect(
detectRepoIcon({ repoPath, kind: 'folder', executionHostId: 'runtime:env-a' })
).resolves.toBeUndefined()
expect(nested.stat).not.toHaveBeenCalled()
})
it('falls back to the GitHub owner avatar for GitHub repos', async () => {
const repoPath = await makeTempRepoDir()
await gitExecFileAsync(['init'], { cwd: repoPath })
@@ -175,7 +254,9 @@ describe('detectRepoIcon', () => {
cwd: repoPath
})
await expect(detectRepoIcon({ repoPath, kind: 'git' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: 'https://github.com/stablyai.png?size=64',
source: 'github',
@@ -194,7 +275,9 @@ describe('detectRepoIcon', () => {
cwd: repoPath
})
await expect(detectRepoIcon({ repoPath, kind: 'git' })).resolves.toEqual({
await expect(
detectRepoIcon({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toEqual({
type: 'image',
src: 'https://github.com/stablyai.png?size=64',
source: 'github',
@@ -206,7 +289,9 @@ describe('detectRepoIcon', () => {
const repoPath = await makeTempRepoDir()
await gitExecFileAsync(['init'], { cwd: repoPath })
await expect(detectRepoIconAndUpstream({ repoPath, kind: 'git' })).resolves.toEqual({
await expect(
detectRepoIconAndUpstream({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toEqual({
upstream: null
})
})
@@ -221,7 +306,9 @@ describe('detectRepoIcon', () => {
cwd: repoPath
})
await expect(detectRepoIconAndUpstream({ repoPath, kind: 'git' })).resolves.toEqual({
await expect(
detectRepoIconAndUpstream({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toEqual({
gitRemoteIdentity: {
canonicalKey: 'github.com/stablyai/orca',
remoteName: 'upstream',
@@ -251,7 +338,9 @@ describe('detectRepoIcon', () => {
}
)
await expect(detectRepoIconAndUpstream({ repoPath, kind: 'git' })).resolves.toEqual({
await expect(
detectRepoIconAndUpstream({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toEqual({
gitRemoteIdentity: {
canonicalKey: 'github.com/upstream-org/rocket',
remoteName: 'upstream',
@@ -276,7 +365,9 @@ describe('detectRepoIcon', () => {
{ cwd: repoPath }
)
await expect(detectRepoIconAndUpstream({ repoPath, kind: 'git' })).resolves.toMatchObject({
await expect(
detectRepoIconAndUpstream({ repoPath, kind: 'git', executionHostId: 'local' })
).resolves.toMatchObject({
gitRemoteIdentity: {
canonicalKey: 'git.company.test/platform/tools/sample-app',
remoteName: 'origin',
+57 -25
View File
@@ -1,4 +1,5 @@
import { readFile, stat } from 'node:fs/promises'
import type { ExecutionHostId } from '../shared/execution-host'
import type { GitHubRepositoryIdentity } from '../shared/github/pull-request-types'
import type { RepoKind } from '../shared/repo-types'
import {
@@ -8,7 +9,10 @@ import {
type RepoIcon
} from '../shared/repo-icon'
import { getRepoSlug, getRepoUpstream } from './github/client'
import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch'
import {
resolveFilesystemRouteForHost,
resolveGitRouteForHost
} from './providers/execution-host-provider-dispatch'
import type { IFilesystemProvider } from './providers/types'
import { detectGitRemoteIdentity } from './repo-git-remote-identity'
import { detectRepoFileIcon } from './repo-icon-file-detection'
@@ -77,13 +81,36 @@ async function detectRemotePackageHomepageIcon(
}
}
/**
* The connection this client may dial to read `executionHostId`'s remotes, or `refuse` when it may
* dial none. `runtime:` is refused rather than degraded to `null`: that server runs its own git,
* and answering "no connection" would read this machine's copy of the path instead.
*/
function repoRemoteReadConnection(
executionHostId: ExecutionHostId
): { kind: 'refuse' } | { kind: 'dial'; connectionId: string | null } {
const route = resolveGitRouteForHost(executionHostId)
switch (route.kind) {
case 'local':
return { kind: 'dial', connectionId: null }
case 'ssh':
return { kind: 'dial', connectionId: route.connectionId }
case 'runtime':
return { kind: 'refuse' }
}
}
export async function detectGitHubAvatarIcon(
repoPath: string,
connectionId?: string | null,
executionHostId: ExecutionHostId,
upstream?: GitHubRepositoryIdentity | null
): Promise<RepoIcon | null> {
try {
const slug = githubAvatarSlug(await getRepoSlug(repoPath, connectionId), upstream)
const target = repoRemoteReadConnection(executionHostId)
if (target.kind === 'refuse') {
return null
}
const slug = githubAvatarSlug(await getRepoSlug(repoPath, target.connectionId), upstream)
return slug ? githubAvatarIcon(slug) : null
} catch {
return null
@@ -93,34 +120,35 @@ export async function detectGitHubAvatarIcon(
export async function detectRepoIcon({
repoPath,
kind,
connectionId,
executionHostId,
upstream
}: {
repoPath: string
kind: RepoKind
connectionId?: string | null
executionHostId: ExecutionHostId
upstream?: GitHubRepositoryIdentity | null
}): Promise<RepoIcon | undefined> {
try {
const fsProvider = connectionId ? getSshFilesystemProvider(connectionId) : undefined
// Why: a remote repoPath with no provider must not be probed on the client
// filesystem — a same-named local path answers for the wrong repository.
if (fsProvider || !connectionId) {
const fileIcon = await detectRepoFileIcon(repoPath, { connectionId, fsProvider })
if (fileIcon) {
return fileIcon
}
const route = resolveFilesystemRouteForHost(executionHostId)
const fileIcon = await detectRepoFileIcon(repoPath, route)
if (fileIcon) {
return fileIcon
}
const homepageIcon = fsProvider
? await detectRemotePackageHomepageIcon(repoPath, fsProvider)
: await detectLocalPackageHomepageIcon(repoPath)
if (homepageIcon) {
return homepageIcon
}
// Why the same route again: a remote repoPath with no provider, and every runtime host, must
// not be probed on the client filesystem — a same-named local path answers for the wrong repo.
const remoteProvider = route.kind === 'ssh' ? route.provider : null
const homepageIcon = remoteProvider
? await detectRemotePackageHomepageIcon(repoPath, remoteProvider)
: route.kind === 'local'
? await detectLocalPackageHomepageIcon(repoPath)
: null
if (homepageIcon) {
return homepageIcon
}
if (kind === 'git') {
return (await detectGitHubAvatarIcon(repoPath, connectionId, upstream)) ?? undefined
return (await detectGitHubAvatarIcon(repoPath, executionHostId, upstream)) ?? undefined
}
} catch {
// Repo creation must not fail because a best-effort icon probe failed.
@@ -133,16 +161,20 @@ export async function detectRepoIcon({
export async function detectRepoIconAndUpstream({
repoPath,
kind,
connectionId
executionHostId
}: {
repoPath: string
kind: RepoKind
connectionId?: string | null
executionHostId: ExecutionHostId
}) {
const upstream = kind === 'git' ? await getRepoUpstream(repoPath, connectionId) : null
const remoteRead = repoRemoteReadConnection(executionHostId)
const upstream =
kind === 'git' && remoteRead.kind === 'dial'
? await getRepoUpstream(repoPath, remoteRead.connectionId)
: null
const gitRemoteIdentity =
kind === 'git' ? await detectGitRemoteIdentity(repoPath, connectionId) : null
const repoIcon = await detectRepoIcon({ repoPath, kind, connectionId, upstream })
kind === 'git' ? await detectGitRemoteIdentity(repoPath, executionHostId) : null
const repoIcon = await detectRepoIcon({ repoPath, kind, executionHostId, upstream })
return {
...(repoIcon ? { repoIcon } : {}),
...(gitRemoteIdentity ? { gitRemoteIdentity } : {}),
+31 -6
View File
@@ -1,9 +1,17 @@
import { readFile, stat } from 'node:fs/promises'
import type * as FsPromisesModule from 'node:fs/promises'
import { describe, expect, it, vi } from 'vitest'
import type { ExecutionHostFilesystemRoute } from './providers/execution-host-provider-dispatch'
import type { FileReadResult, FileStat, IFilesystemProvider } from './providers/types'
import { detectRepoFileIcon } from './repo-icon-file-detection'
function sshRoute(
connectionId: string,
provider: IFilesystemProvider | null
): ExecutionHostFilesystemRoute {
return { kind: 'ssh', hostId: `ssh:${connectionId}`, connectionId, provider }
}
// Why: the boundary assertion is "no local read happened", which needs the real
// fs entrypoints spied rather than stubbed.
vi.mock('node:fs/promises', async (importOriginal) => {
@@ -37,7 +45,7 @@ describe('detectRepoFileIcon remote probing', () => {
readFile: async () => ({ content: WEBP_BASE64, isBinary: true, mimeType: 'image/webp' })
})
await expect(detectRepoFileIcon('/repo', { fsProvider: provider })).resolves.toEqual({
await expect(detectRepoFileIcon('/repo', sshRoute('m4air', provider))).resolves.toEqual({
type: 'image',
src: `data:image/webp;base64,${WEBP_BASE64}`,
source: 'file',
@@ -61,7 +69,7 @@ describe('detectRepoFileIcon remote probing', () => {
}
})
await expect(detectRepoFileIcon('/repo', { fsProvider: provider })).resolves.toMatchObject({
await expect(detectRepoFileIcon('/repo', sshRoute('m4air', provider))).resolves.toMatchObject({
source: 'file',
label: 'favicon.png'
})
@@ -84,7 +92,7 @@ describe('detectRepoFileIcon remote probing', () => {
}
})
await expect(detectRepoFileIcon('/repo', { fsProvider: provider })).resolves.toBeNull()
await expect(detectRepoFileIcon('/repo', sshRoute('m4air', provider))).resolves.toBeNull()
expect(maxActiveStats).toBeGreaterThan(1)
expect(maxActiveStats).toBeLessThanOrEqual(6)
})
@@ -95,18 +103,35 @@ describe('detectRepoFileIcon connection boundary', () => {
vi.mocked(stat).mockClear()
vi.mocked(readFile).mockClear()
await expect(detectRepoFileIcon('/repo', sshRoute('ssh-target-1', null))).resolves.toBeNull()
expect(stat).not.toHaveBeenCalled()
expect(readFile).not.toHaveBeenCalled()
})
it('never reads the client filesystem for a runtime host', async () => {
// Why: a runtime host's files live on that server. It is not "local with no provider".
vi.mocked(stat).mockClear()
vi.mocked(readFile).mockClear()
await expect(
detectRepoFileIcon('/repo', { connectionId: 'ssh-target-1', fsProvider: undefined })
detectRepoFileIcon('/repo', {
kind: 'runtime',
hostId: 'runtime:env-a',
environmentId: 'env-a'
})
).resolves.toBeNull()
expect(stat).not.toHaveBeenCalled()
expect(readFile).not.toHaveBeenCalled()
})
it('still probes the local filesystem for a repo with no connection', async () => {
it('still probes the local filesystem for a repo on this machine', async () => {
vi.mocked(stat).mockClear()
await expect(detectRepoFileIcon('/repo', { connectionId: null })).resolves.toBeNull()
await expect(
detectRepoFileIcon('/repo', { kind: 'local', hostId: 'local' })
).resolves.toBeNull()
expect(stat).toHaveBeenCalled()
})
+19 -12
View File
@@ -1,6 +1,7 @@
import { readFile, stat } from 'node:fs/promises'
import { buildImageDataUri } from '../shared/image-data-uri'
import { MAX_REPO_ICON_UPLOAD_BYTES, type RepoIcon } from '../shared/repo-icon'
import type { ExecutionHostFilesystemRoute } from './providers/execution-host-provider-dispatch'
import type { IFilesystemProvider } from './providers/types'
import { iconHrefCandidates } from './repo-icon-href-candidates'
import { joinWorktreeRelativePath } from './runtime/runtime-relative-paths'
@@ -258,20 +259,26 @@ async function detectRemoteImageIcon(
return null
}
/**
* Takes the resolved host route rather than a `connectionId`, because the incumbent shape spelled
* "this is local", "this host is unreachable" and "this is a runtime host" all as a falsy id — and
* only the first of those may read this machine's filesystem.
*/
export function detectRepoFileIcon(
repoPath: string,
{
connectionId,
fsProvider
}: { connectionId?: string | null; fsProvider?: IFilesystemProvider } = {}
route: ExecutionHostFilesystemRoute
): Promise<RepoIcon | null> {
if (fsProvider) {
return detectRemoteImageIcon(repoPath, fsProvider)
switch (route.kind) {
case 'local':
return detectLocalImageIcon(repoPath)
case 'ssh':
// A dropped provider fails closed: repoPath lives on the SSH host, so a same-named local
// path would hand back another repository's icon.
return route.provider
? detectRemoteImageIcon(repoPath, route.provider)
: Promise.resolve(null)
case 'runtime':
// That environment's server holds these files; this process has no route to them.
return Promise.resolve(null)
}
if (connectionId) {
// Why: repoPath lives on the SSH host, so a dropped provider must fail closed —
// a same-named local path would hand back another repository's icon.
return Promise.resolve(null)
}
return detectLocalImageIcon(repoPath)
}
@@ -94,6 +94,24 @@ describe('startup fork-upstream backfill', () => {
})
})
it('skips every row whose files sit on an SSH host', async () => {
// Why: the incumbent guard read `repo.connectionId`, so a row minted with only the unified
// spelling fell through and had its upstream read off this client's copy of the path. A
// runtime row's nested target holds its files too, and is equally not ours to read.
const runtime = new OrcaRuntimeService()
const updateRepo = attachStore(runtime, [
makeRepo({ id: 'repo-ssh', executionHostId: 'ssh:builder' }),
makeRepo({ id: 'repo-openclaw', executionHostId: 'ssh:openclaw' }),
makeRepo({ id: 'repo-nested', connectionId: 'nested-1', executionHostId: 'runtime:env-a' })
])
await (runtime as unknown as BackfillInternals).repositoryForkBackfill.run()
expect(getRepoUpstream).not.toHaveBeenCalled()
expect(getRepoSlug).not.toHaveBeenCalled()
expect(updateRepo).not.toHaveBeenCalled()
})
it('keeps an icon chosen while avatar detection is pending', async () => {
const runtime = new OrcaRuntimeService()
const repo = makeRepo()
@@ -1,7 +1,7 @@
import { randomUUID } from 'node:crypto'
import { mkdir } from 'node:fs/promises'
import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants'
import type { ExecutionHostId } from '../../shared/execution-host'
import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../shared/execution-host'
import type { Repo } from '../../shared/repo-types'
import { getGitCloneFailureMessage } from '../../shared/git-clone-failure-message'
import {
@@ -161,7 +161,13 @@ export class RuntimeRepositoryCloneController {
}
return existing
}
const detected = await detectRepoIconAndUpstream({ repoPath: clonePath, kind: 'git' })
// `cloneRepo` ran `git clone` in this process (see `assertCloneHostIsSupported`), so the
// checkout is here regardless of the host id stamped on the row.
const detected = await detectRepoIconAndUpstream({
repoPath: clonePath,
kind: 'git',
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path: clonePath,
@@ -1,3 +1,4 @@
import { getRepoSshConnectionId, LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types'
import type { Repo } from '../../shared/repo-types'
import { getRepoUpstream } from '../github/client'
@@ -28,7 +29,10 @@ export class RuntimeRepositoryForkBackfill {
}
let changed = false
for (const repo of store.getRepos()) {
if (repo.upstream !== undefined || repo.kind === 'folder' || repo.connectionId) {
// Why the resolved SSH target and not the raw `connectionId`: this backfill runs `gh`/git
// in this process, so any row whose files sit on an SSH host must be skipped — including
// one that carries only `executionHostId: ssh:…`, which the raw field reads as local.
if (repo.upstream !== undefined || repo.kind === 'folder' || getRepoSshConnectionId(repo)) {
continue
}
let upstream: GitHubOwnerRepo | null
@@ -39,7 +43,7 @@ export class RuntimeRepositoryForkBackfill {
}
const repoIcon =
upstream && repo.repoIcon?.type === 'image' && repo.repoIcon.source === 'github'
? await detectGitHubAvatarIcon(repo.path, null, upstream)
? await detectGitHubAvatarIcon(repo.path, LOCAL_EXECUTION_HOST_ID, upstream)
: null
const current = store.getRepos().find((candidate) => candidate.id === repo.id)
if (!current || current.upstream !== undefined) {
@@ -2,7 +2,11 @@ import { randomUUID } from 'node:crypto'
import { mkdir, readdir, rm, stat } from 'node:fs/promises'
import { isAbsolute, join } from 'node:path'
import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants'
import { parseExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import {
LOCAL_EXECUTION_HOST_ID,
parseExecutionHostId,
type ExecutionHostId
} from '../../shared/execution-host'
import type { Repo } from '../../shared/repo-types'
import { gitExecFileAsync, awaitWindowsHostGitEnvironmentReady } from '../git/runner'
import { getRepoName, isGitRepo } from '../git/repo'
@@ -57,7 +61,14 @@ export class RuntimeRepositoryRegistrationController {
}
return existing
}
const detected = await detectRepoIconAndUpstream({ repoPath: path, kind })
// Local on purpose, whatever `executionHostId` stamps on the row: this controller already
// validated and will read `path` in this process. A `runtime:` stamp is how a paired client
// addresses the row, not a second machine holding the files.
const detected = await detectRepoIconAndUpstream({
repoPath: path,
kind,
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path,
@@ -137,7 +148,11 @@ export class RuntimeRepositoryRegistrationController {
if (raceWinner) {
return { repo: raceWinner }
}
const detected = await detectRepoIconAndUpstream({ repoPath: targetPath, kind: repoKind })
const detected = await detectRepoIconAndUpstream({
repoPath: targetPath,
kind: repoKind,
executionHostId: LOCAL_EXECUTION_HOST_ID
})
const repo: Repo = {
id: randomUUID(),
path: targetPath,