fix(repo-icon): keep a renamed fork's own owner avatar (#12271)

* fix(repo-icon): keep a renamed fork's own owner avatar

Fork repos always took the upstream owner's avatar, so a renamed fork
showed its parent project's logo. Same-name forks (personal copies)
still prefer the upstream owner; renamed forks now keep their origin
owner across auto-detect, the startup backfill, and the settings
avatar refresh.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(repo-icon): re-read repo state before backfill avatar write

The startup backfill computed icon updates from a pre-loop snapshot, so
an icon chosen in settings while the upstream/origin probes were pending
could be clobbered. Re-read the repo after the probes and only migrate
an icon that is still the auto-detected GitHub avatar.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(repo-icon): own the fork avatar rule in one shared selector

The renamed-fork rule was written out twice — once in the main-process
auto-detect and once in the renderer refresh — so the two copies could
drift. Move it next to `githubAvatarIcon` as `githubAvatarSlug`, which
collapses the renderer resolver to a single unbranched path.

Also stop swallowing a rejected origin probe: it cannot tell a renamed
fork from a same-name one, so degrading to the upstream owner would flip
a renamed fork's stored avatar back to the parent's. Letting it propagate
keeps the stored icon, matching how the non-fork path already behaved.

Adds coverage for the startup backfill, the third decision point the fix
claims, which had none.

* test(repo-icon): cover pending backfill icon change

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
This commit is contained in:
Alex Shan
2026-08-10 01:06:32 -07:00
committed by GitHub
co-authored by Claude Fable 5 Brennan Benson
parent 4e536a5232
commit ce3ec4d5ce
11 changed files with 289 additions and 31 deletions
+2 -1
View File
@@ -1669,7 +1669,8 @@ export async function getRepoSlug(
/**
* Resolve a fork's upstream/parent owner/repo, or null when not a fork.
* Why: a fork's `origin` is the personal copy, so repo identity (avatar) should prefer upstream.
* Why: drives the fork indicator, and a same-name fork's avatar prefers the
* upstream owner (a renamed fork keeps its own owner).
* Best-effort: any failure (offline, unauthed, non-GitHub) resolves to null.
*/
export async function getRepoUpstream(
+30
View File
@@ -214,6 +214,36 @@ describe('detectRepoIcon', () => {
})
})
it('keeps the renamed fork own owner avatar while storing the upstream metadata', async () => {
const repoPath = await makeTempRepoDir()
await gitExecFileAsync(['init'], { cwd: repoPath })
await gitExecFileAsync(['remote', 'add', 'origin', 'git@github.com:acme/rocket-pro.git'], {
cwd: repoPath
})
await gitExecFileAsync(
['remote', 'add', 'upstream', 'git@github.com:upstream-org/rocket.git'],
{
cwd: repoPath
}
)
await expect(detectRepoIconAndUpstream({ repoPath, kind: 'git' })).resolves.toEqual({
gitRemoteIdentity: {
canonicalKey: 'github.com/upstream-org/rocket',
remoteName: 'upstream',
remoteUrl: 'git@github.com:upstream-org/rocket.git'
},
// Why: a renamed fork is its own project, so the avatar stays on the origin owner.
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket-pro'
},
upstream: { owner: 'upstream-org', repo: 'rocket', host: 'github.com' }
})
})
it('detects a provider-neutral git remote identity for non-GitHub remotes', async () => {
const repoPath = await makeTempRepoDir()
await gitExecFileAsync(['init'], { cwd: repoPath })
+8 -4
View File
@@ -1,6 +1,11 @@
import { readFile, stat } from 'node:fs/promises'
import type { GitHubRepositoryIdentity, RepoKind } from '../shared/types'
import { faviconUrlFromWebsite, githubAvatarIcon, type RepoIcon } from '../shared/repo-icon'
import {
faviconUrlFromWebsite,
githubAvatarIcon,
githubAvatarSlug,
type RepoIcon
} from '../shared/repo-icon'
import { getRepoSlug, getRepoUpstream } from './github/client'
import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch'
import type { IFilesystemProvider } from './providers/types'
@@ -71,14 +76,13 @@ async function detectRemotePackageHomepageIcon(
}
}
async function detectGitHubAvatarIcon(
export async function detectGitHubAvatarIcon(
repoPath: string,
connectionId?: string | null,
upstream?: GitHubRepositoryIdentity | null
): Promise<RepoIcon | null> {
try {
// Why: a fork's origin is the personal copy, so prefer the upstream owner.
const slug = upstream ?? (await getRepoSlug(repoPath, connectionId))
const slug = githubAvatarSlug(await getRepoSlug(repoPath, connectionId), upstream)
return slug ? githubAvatarIcon(slug) : null
} catch {
return null
+18 -4
View File
@@ -1012,9 +1012,8 @@ import {
getSshGitProviderGeneration,
requireSshGitProvider
} from '../providers/ssh-git-dispatch'
import { detectRepoIconAndUpstream } from '../repo-icon-autodetect'
import { detectGitHubAvatarIcon, detectRepoIconAndUpstream } from '../repo-icon-autodetect'
import { enrichMissingRepoGitRemoteIdentities } from '../repo-git-remote-identity-enrichment'
import { githubAvatarIcon } from '../../shared/repo-icon'
import type { ClaudeAccountService } from '../claude-accounts/service'
import type {
CodexAccountService,
@@ -19272,10 +19271,25 @@ export class OrcaRuntimeService {
} catch {
continue
}
const repoIcon =
upstream && repo.repoIcon?.type === 'image' && repo.repoIcon.source === 'github'
? await detectGitHubAvatarIcon(repo.path, null, upstream)
: null
// Why: settings can change the repo while the probes above are pending, so
// re-read it — a stale snapshot must not clobber a user-chosen icon or an
// upstream another path already resolved.
const current = store.getRepos().find((candidate) => candidate.id === repo.id)
if (!current || current.upstream !== undefined) {
continue
}
const updates: Partial<Repo> = { upstream: upstream ?? null }
// Only migrate the auto-detected origin avatar; never touch a chosen icon.
if (upstream && repo.repoIcon?.type === 'image' && repo.repoIcon.source === 'github') {
updates.repoIcon = githubAvatarIcon(upstream)
if (
repoIcon &&
current.repoIcon?.type === 'image' &&
current.repoIcon.source === 'github'
) {
updates.repoIcon = repoIcon
}
store.updateRepo(repo.id, updates)
changed = true
@@ -0,0 +1,126 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { Repo } from '../../shared/types'
import * as client from '../github/client'
import { OrcaRuntimeService } from './orca-runtime'
vi.mock('../github/client', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
getRepoUpstream: vi.fn(),
getRepoSlug: vi.fn()
}))
const getRepoUpstream = vi.mocked(client.getRepoUpstream)
const getRepoSlug = vi.mocked(client.getRepoSlug)
type BackfillInternals = { backfillForkUpstreams(): Promise<void> }
function makeRepo(overrides: Partial<Repo> = {}): Repo {
return {
id: 'repo-1',
path: '/workspace/rocket-pro',
displayName: 'rocket-pro',
badgeColor: '#2563eb',
addedAt: 1,
kind: 'git',
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket-pro'
},
...overrides
}
}
function attachStore(runtime: OrcaRuntimeService, repos: Repo[]) {
const updateRepo = vi.fn((repoId: string, updates: Partial<Repo>) => {
const index = repos.findIndex((repo) => repo.id === repoId)
repos[index] = { ...repos[index], ...updates }
})
Object.assign(runtime, { store: { getRepos: () => repos, updateRepo } })
return updateRepo
}
afterEach(() => {
vi.clearAllMocks()
})
describe('startup fork-upstream backfill', () => {
it('keeps a renamed fork own owner avatar while recording the upstream', async () => {
const runtime = new OrcaRuntimeService()
const updateRepo = attachStore(runtime, [makeRepo()])
getRepoUpstream.mockResolvedValue({ owner: 'upstream-org', repo: 'rocket' })
getRepoSlug.mockResolvedValue({ owner: 'acme', repo: 'rocket-pro' })
await (runtime as unknown as BackfillInternals).backfillForkUpstreams()
expect(updateRepo).toHaveBeenCalledExactlyOnceWith('repo-1', {
upstream: { owner: 'upstream-org', repo: 'rocket' },
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket-pro'
}
})
})
it('migrates a same-name fork to the upstream owner avatar', async () => {
const runtime = new OrcaRuntimeService()
const updateRepo = attachStore(runtime, [
makeRepo({
path: '/workspace/rocket',
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket'
}
})
])
getRepoUpstream.mockResolvedValue({ owner: 'upstream-org', repo: 'rocket' })
getRepoSlug.mockResolvedValue({ owner: 'acme', repo: 'rocket' })
await (runtime as unknown as BackfillInternals).backfillForkUpstreams()
expect(updateRepo).toHaveBeenCalledExactlyOnceWith('repo-1', {
upstream: { owner: 'upstream-org', repo: 'rocket' },
repoIcon: {
type: 'image',
src: 'https://github.com/upstream-org.png?size=64',
source: 'github',
label: 'upstream-org/rocket'
}
})
})
it('keeps an icon chosen while avatar detection is pending', async () => {
const runtime = new OrcaRuntimeService()
const repo = makeRepo()
const repos = [repo]
const updateRepo = attachStore(runtime, repos)
getRepoUpstream.mockResolvedValue({ owner: 'upstream-org', repo: 'rocket' })
let resolveSlug!: (value: { owner: string; repo: string }) => void
let markSlugStarted!: () => void
const slugStarted = new Promise<void>((resolve) => {
markSlugStarted = resolve
})
getRepoSlug.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveSlug = resolve
markSlugStarted()
})
)
const backfill = (runtime as unknown as BackfillInternals).backfillForkUpstreams()
await slugStarted
repos[0] = { ...repo, repoIcon: { type: 'emoji', emoji: '🚀' } }
resolveSlug({ owner: 'acme', repo: 'rocket-pro' })
await backfill
expect(updateRepo).toHaveBeenCalledExactlyOnceWith('repo-1', {
upstream: { owner: 'upstream-org', repo: 'rocket' }
})
})
})
@@ -106,7 +106,7 @@ describe('RepositoryIconPicker GitHub avatar refresh', () => {
label: 'stablyai/orca'
}
})
// Offline/unauthed: the parent lookup returns null. The fork's own origin
// Offline/unauthed: the parent lookup returns null. The same-name origin
// owner must NOT be persisted over the parent identity.
apiMocks.repoUpstream.mockResolvedValueOnce(null)
apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'parkerrex', repo: 'orca' })
@@ -117,6 +117,5 @@ describe('RepositoryIconPicker GitHub avatar refresh', () => {
await flushEffects()
expect(updateRepo).not.toHaveBeenCalled()
expect(apiMocks.repoSlug).not.toHaveBeenCalled()
})
})
@@ -36,8 +36,10 @@ describe('repository GitHub avatar resolution', () => {
apiMocks.repoUpstream.mockReset()
})
it('uses stored upstream by default to avoid unnecessary live checks', async () => {
it('uses stored upstream by default and keeps the parent avatar for same-name forks', async () => {
const repo = makeRepo({ upstream: { owner: 'stablyai', repo: 'orca' } })
// The fork's own origin owner — same repo name, so the parent avatar wins.
apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'tmchow', repo: 'orca' })
await expect(resolveRepositoryGitHubAvatar({ kind: 'local' }, repo)).resolves.toEqual({
repoIcon: {
@@ -50,7 +52,40 @@ describe('repository GitHub avatar resolution', () => {
})
expect(apiMocks.repoUpstream).not.toHaveBeenCalled()
expect(apiMocks.repoSlug).not.toHaveBeenCalled()
// Only the origin slug is consulted (for the renamed-fork check).
expect(apiMocks.repoSlug).toHaveBeenCalledExactlyOnceWith({
repoPath: '/workspace/orca',
repoId: 'repo-1'
})
})
it('prefers the renamed fork own owner over the stored upstream', async () => {
const repo = makeRepo({ upstream: { owner: 'upstream-org', repo: 'rocket' } })
apiMocks.repoUpstream.mockResolvedValueOnce({ owner: 'upstream-org', repo: 'rocket' })
apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'acme', repo: 'rocket-pro' })
const resolution = await resolveRepositoryGitHubAvatar({ kind: 'local' }, repo, {
forceLive: true
})
expect(resolution).toEqual({
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket-pro'
},
upstream: { owner: 'upstream-org', repo: 'rocket' }
})
// The fork metadata is untouched; only the avatar moves to the fork's own owner.
expect(buildRepositoryGitHubAvatarUpdate(repo, resolution)).toEqual({
repoIcon: {
type: 'image',
src: 'https://github.com/acme.png?size=64',
source: 'github',
label: 'acme/rocket-pro'
}
})
})
it('force-resolves the live origin owner when a non-fork repo was transferred', async () => {
@@ -140,7 +175,7 @@ describe('repository GitHub avatar resolution', () => {
}
})
apiMocks.repoUpstream.mockResolvedValueOnce(null)
// The fork's own origin owner — the value we must NOT persist over the parent.
// The fork's own origin owner — same repo name, so it must NOT replace the parent.
apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'parkerrex', repo: 'orca' })
const resolution = await resolveRepositoryGitHubAvatar({ kind: 'local' }, repo, {
@@ -156,12 +191,26 @@ describe('repository GitHub avatar resolution', () => {
},
upstream: { owner: 'stablyai', repo: 'orca' }
})
// The origin slug must never be consulted once we fall back to the known parent.
expect(apiMocks.repoSlug).not.toHaveBeenCalled()
// Nothing changed, so no repo write is produced (no sticky null clobber).
expect(buildRepositoryGitHubAvatarUpdate(repo, resolution)).toBeNull()
})
it('propagates an ambiguous origin probe failure instead of flipping to the parent avatar', async () => {
// A renamed fork already showing its own owner. A rejected origin probe cannot
// tell renamed from same-name, so it must surface rather than resolve to the
// parent avatar — callers keep the stored icon.
const repo = makeRepo({
upstream: { owner: 'upstream-org', repo: 'rocket' },
repoIcon: githubAvatarIcon({ owner: 'acme', repo: 'rocket-pro' })
})
apiMocks.repoUpstream.mockResolvedValueOnce({ owner: 'upstream-org', repo: 'rocket' })
apiMocks.repoSlug.mockRejectedValueOnce(new Error('runtime rpc timeout'))
await expect(
resolveRepositoryGitHubAvatar({ kind: 'local' }, repo, { forceLive: true })
).rejects.toThrow('runtime rpc timeout')
})
it('persists an upstream change when only the GitHub host differs', () => {
const repo = makeRepo({
upstream: { owner: 'acme', repo: 'widgets', host: 'github.com' }
@@ -1,5 +1,5 @@
import type { GitHubRepositoryIdentity, Repo } from '../../../../shared/types'
import { githubAvatarIcon, type RepoIcon } from '../../../../shared/repo-icon'
import { githubAvatarIcon, githubAvatarSlug, type RepoIcon } from '../../../../shared/repo-icon'
import { githubRepoIdentityKey } from '../../../../shared/github-repository-identity-key'
import { callRuntimeRpc, type getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
@@ -55,20 +55,17 @@ export async function resolveRepositoryGitHubAvatar(
repo: Repo,
options: ResolveRepositoryGitHubAvatarOptions = {}
): Promise<RepositoryGitHubAvatarResolution> {
const upstream =
const liveUpstream =
!options.forceLive && repo.upstream !== undefined
? repo.upstream
: await resolveRepositoryUpstreamLive(runtimeTarget, repo).catch(() => null)
if (upstream) {
return { repoIcon: githubAvatarIcon(upstream), upstream }
}
// Why: a null live upstream is ambiguous (offline/unauthed vs. not-a-fork). Keep
// the last-known parent avatar so a transient failure can't clobber fork identity.
if (repo.upstream) {
return { repoIcon: githubAvatarIcon(repo.upstream), upstream: repo.upstream }
}
const slug = await resolveRepositorySlugLive(runtimeTarget, repo)
return { repoIcon: slug ? githubAvatarIcon(slug) : null, upstream: null }
// the last-known parent so a transient failure can't clobber fork identity.
const upstream = liveUpstream ?? repo.upstream ?? null
// Why: a rejected origin probe is also ambiguous, so it propagates — callers keep
// the stored icon rather than flipping a renamed fork back to the parent avatar.
const slug = githubAvatarSlug(await resolveRepositorySlugLive(runtimeTarget, repo), upstream)
return { repoIcon: slug ? githubAvatarIcon(slug) : null, upstream }
}
function sameRepositoryIdentity(
+22 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest'
import { githubAvatarIcon, sanitizeRepoIcon } from './repo-icon'
import { githubAvatarIcon, githubAvatarSlug, sanitizeRepoIcon } from './repo-icon'
const PNG_1X1_BASE64 =
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII='
@@ -167,3 +167,24 @@ describe('sanitizeRepoIcon', () => {
).toMatchObject({ src: 'https://github.com/acme.png?size=64' })
})
})
describe('githubAvatarSlug', () => {
const upstream = { owner: 'upstream-org', repo: 'rocket' }
it('keeps the upstream owner for a same-name fork, case-insensitively', () => {
expect(githubAvatarSlug({ owner: 'acme', repo: 'rocket' }, upstream)).toEqual(upstream)
expect(githubAvatarSlug({ owner: 'acme', repo: 'RocKet' }, upstream)).toEqual(upstream)
})
it('keeps the fork own owner once it has been renamed', () => {
const origin = { owner: 'acme', repo: 'rocket-pro' }
expect(githubAvatarSlug(origin, upstream)).toEqual(origin)
})
it('falls back to whichever identity is known', () => {
const origin = { owner: 'acme', repo: 'rocket-pro' }
expect(githubAvatarSlug(origin, null)).toEqual(origin)
expect(githubAvatarSlug(null, upstream)).toEqual(upstream)
expect(githubAvatarSlug(null, undefined)).toBeNull()
})
})
+17 -1
View File
@@ -31,8 +31,24 @@ export function faviconUrlFromWebsite(rawUrl: string): string | null {
}
}
type GitHubAvatarSlug = { owner: string; repo: string; host?: string }
/**
* Pick the owner whose avatar represents a repo, given its `origin` and fork parent.
* Why: a same-name fork is a personal copy, so it reads as the parent project; a
* renamed fork is its own project and keeps its own owner.
*/
export function githubAvatarSlug(
origin: GitHubAvatarSlug | null | undefined,
upstream: GitHubAvatarSlug | null | undefined
): GitHubAvatarSlug | null {
const renamedFork =
origin && upstream && origin.repo.toLowerCase() !== upstream.repo.toLowerCase()
return renamedFork ? origin : (upstream ?? origin ?? null)
}
// Why: shared default icon URL/label for main auto-detect and the renderer picker.
export function githubAvatarIcon(slug: { owner: string; repo: string; host?: string }): RepoIcon {
export function githubAvatarIcon(slug: GitHubAvatarSlug): RepoIcon {
// Why: GHES uses the same /<login>.png avatar path as github.com.
const host = normalizeGitHubAvatarHost(slug.host)
return {
+3 -2
View File
@@ -253,8 +253,9 @@ export type Repo = {
badgeColor: string
repoIcon?: RepoIcon | null
/** Set when the repo is a fork: the upstream/parent owner/repo. Drives the
* default avatar (upstream owner, not the personal fork) and the fork
* indicator. Absent = not a fork, or fork status not yet resolved. */
* fork indicator and the default avatar of same-name forks (renamed forks
* keep their own owner). Absent = not a fork, or fork status not yet
* resolved. */
upstream?: GitHubRepositoryIdentity | null
addedAt: number
kind?: RepoKind