diff --git a/src/renderer/src/components/settings/RepositoryIconPicker.github-avatar-refresh.test.tsx b/src/renderer/src/components/settings/RepositoryIconPicker.github-avatar-refresh.test.tsx new file mode 100644 index 0000000000..6fdbb4b5eb --- /dev/null +++ b/src/renderer/src/components/settings/RepositoryIconPicker.github-avatar-refresh.test.tsx @@ -0,0 +1,96 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/types' +import { RepositoryIconPicker } from './RepositoryIconPicker' + +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: vi.fn(), + getActiveRuntimeTarget: () => ({ kind: 'local' }) +})) + +vi.mock('./RepositoryIconColorSection', () => ({ + RepositoryIconColorSection: () => null +})) + +vi.mock('./RepositoryIconTabs', () => ({ + RepositoryIconTabs: () => null +})) + +const apiMocks = { + repoSlug: vi.fn(), + repoUpstream: vi.fn() +} + +let container: HTMLDivElement +let root: Root + +// @ts-expect-error test window mock +globalThis.window = { api: { gh: apiMocks } } + +function makeRepo(overrides: Partial = {}): Repo { + return { + id: 'repo-1', + path: '/workspace/orca', + displayName: 'orca', + badgeColor: '#2563eb', + addedAt: 1, + kind: 'git', + ...overrides + } +} + +async function flushEffects(): Promise { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +describe('RepositoryIconPicker GitHub avatar refresh', () => { + beforeEach(() => { + apiMocks.repoSlug.mockReset() + apiMocks.repoUpstream.mockReset() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + document.body.replaceChildren() + }) + + it('refreshes stale GitHub avatar metadata lazily when repo settings opens', async () => { + const updateRepo = vi.fn() + const repo = makeRepo({ + upstream: { owner: 'stablyai', repo: 'orca' }, + repoIcon: { + type: 'image', + src: 'https://github.com/stablyai.png?size=64', + source: 'github', + label: 'stablyai/orca' + } + }) + apiMocks.repoUpstream.mockResolvedValueOnce(null) + apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'parkerrex', repo: 'orca' }) + + act(() => { + root.render() + }) + await flushEffects() + + expect(updateRepo).toHaveBeenCalledExactlyOnceWith('repo-1', { + upstream: null, + repoIcon: { + type: 'image', + src: 'https://github.com/parkerrex.png?size=64', + source: 'github', + label: 'parkerrex/orca' + } + }) + }) +}) diff --git a/src/renderer/src/components/settings/RepositoryIconPicker.tsx b/src/renderer/src/components/settings/RepositoryIconPicker.tsx index f7b2b92466..1119483487 100644 --- a/src/renderer/src/components/settings/RepositoryIconPicker.tsx +++ b/src/renderer/src/components/settings/RepositoryIconPicker.tsx @@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { toast } from 'sonner' import { RotateCcw } from 'lucide-react' import type { Repo } from '../../../../shared/types' -import { githubAvatarIcon, type RepoIcon } from '../../../../shared/repo-icon' +import type { RepoIcon } from '../../../../shared/repo-icon' import { DEFAULT_REPO_BADGE_COLOR } from '../../../../shared/constants' import { normalizeRepoBadgeColor } from '../../../../shared/repo-badge-color' import { Button } from '../ui/button' @@ -15,7 +15,8 @@ import { useMountedRef } from '@/hooks/useMountedRef' import { RepositoryIconColorSection } from './RepositoryIconColorSection' import { RepositoryIconTabs } from './RepositoryIconTabs' import { - resolveRepositoryGitHubAvatarIcon, + buildRepositoryGitHubAvatarUpdate, + resolveRepositoryGitHubAvatar, resolveRepositoryUpstreamLive } from './repository-icon-github' import { translate } from '@/i18n/i18n' @@ -72,19 +73,20 @@ export function RepositoryIconPicker({ [runtimeTarget, repo] ) - const resolveGitHubAvatarIcon = useCallback( - () => resolveRepositoryGitHubAvatarIcon(runtimeTarget, repo), + const resolveGitHubAvatar = useCallback( + (options?: { forceLive?: boolean }) => + resolveRepositoryGitHubAvatar(runtimeTarget, repo, options), [runtimeTarget, repo] ) const handleUseGitHubAvatar = async () => { setLoadingGitHub(true) try { - const icon = await resolveGitHubAvatarIcon() + const resolution = await resolveGitHubAvatar({ forceLive: true }) if (!mountedRef.current) { return } - if (!icon) { + if (!resolution.repoIcon) { toast.error( translate( 'auto.components.settings.RepositoryIconPicker.f79972271a', @@ -93,7 +95,13 @@ export function RepositoryIconPicker({ ) return } - setIcon(icon) + updateRepo( + repo.id, + buildRepositoryGitHubAvatarUpdate(repo, resolution) ?? { + repoIcon: resolution.repoIcon, + upstream: resolution.upstream + } + ) } catch { if (mountedRef.current) { toast.error( @@ -113,11 +121,19 @@ export function RepositoryIconPicker({ const handleResetToDefault = async () => { setResetting(true) try { - const icon = await resolveGitHubAvatarIcon().catch(() => null) + const resolution = await resolveGitHubAvatar({ forceLive: true }).catch(() => null) if (!mountedRef.current) { return } - setIcon(icon) + updateRepo( + repo.id, + resolution + ? (buildRepositoryGitHubAvatarUpdate(repo, resolution, { clearMissingIcon: true }) ?? { + repoIcon: resolution.repoIcon, + upstream: resolution.upstream + }) + : { repoIcon: null } + ) } finally { if (mountedRef.current) { setResetting(false) @@ -125,33 +141,48 @@ export function RepositoryIconPicker({ } } - const upstreamBackfilledRef = useRef(null) + const githubIdentityRefreshedRef = useRef(null) useEffect(() => { - if (repo.upstream !== undefined || upstreamBackfilledRef.current === repo.id) { + const hasGitHubAvatar = repo.repoIcon?.type === 'image' && repo.repoIcon.source === 'github' + const shouldRefresh = hasGitHubAvatar || repo.upstream === undefined + if (!shouldRefresh || githubIdentityRefreshedRef.current === repo.id) { return } - upstreamBackfilledRef.current = repo.id + githubIdentityRefreshedRef.current = repo.id let cancelled = false void (async () => { - let upstream + let updates: Partial | null try { - upstream = await resolveUpstreamLive() + if (hasGitHubAvatar) { + // Why: stored upstream/icon metadata can outlive a GitHub repo transfer. + // Refresh only when settings opens for the affected GitHub-avatar repo. + const resolution = await resolveGitHubAvatar({ forceLive: true }) + updates = buildRepositoryGitHubAvatarUpdate(repo, resolution) + } else { + const upstream = await resolveUpstreamLive() + updates = { upstream: upstream ?? null } + } } catch { return } - if (cancelled || !mountedRef.current) { + if (cancelled || !mountedRef.current || !updates) { return } - const updates: Partial = { upstream: upstream ?? null } - if (upstream && repo.repoIcon?.type === 'image' && repo.repoIcon.source === 'github') { - updates.repoIcon = githubAvatarIcon(upstream) - } updateRepo(repo.id, updates) })() return () => { cancelled = true } - }, [repo.id, repo.upstream, repo.repoIcon, resolveUpstreamLive, updateRepo, mountedRef]) + }, [ + repo, + repo.id, + repo.upstream, + repo.repoIcon, + resolveGitHubAvatar, + resolveUpstreamLive, + updateRepo, + mountedRef + ]) return (
diff --git a/src/renderer/src/components/settings/repository-icon-github.test.ts b/src/renderer/src/components/settings/repository-icon-github.test.ts new file mode 100644 index 0000000000..1418911185 --- /dev/null +++ b/src/renderer/src/components/settings/repository-icon-github.test.ts @@ -0,0 +1,126 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/types' +import { + buildRepositoryGitHubAvatarUpdate, + resolveRepositoryGitHubAvatar +} from './repository-icon-github' + +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: vi.fn() +})) + +const apiMocks = { + repoSlug: vi.fn(), + repoUpstream: vi.fn() +} + +// @ts-expect-error test window mock +globalThis.window = { api: { gh: apiMocks } } + +function makeRepo(overrides: Partial = {}): Repo { + return { + id: 'repo-1', + path: '/workspace/orca', + displayName: 'orca', + badgeColor: '#2563eb', + addedAt: 1, + kind: 'git', + ...overrides + } +} + +describe('repository GitHub avatar resolution', () => { + beforeEach(() => { + apiMocks.repoSlug.mockReset() + apiMocks.repoUpstream.mockReset() + }) + + it('uses stored upstream by default to avoid unnecessary live checks', async () => { + const repo = makeRepo({ upstream: { owner: 'stablyai', repo: 'orca' } }) + + await expect(resolveRepositoryGitHubAvatar({ kind: 'local' }, repo)).resolves.toEqual({ + repoIcon: { + type: 'image', + src: 'https://github.com/stablyai.png?size=64', + source: 'github', + label: 'stablyai/orca' + }, + upstream: { owner: 'stablyai', repo: 'orca' } + }) + + expect(apiMocks.repoUpstream).not.toHaveBeenCalled() + expect(apiMocks.repoSlug).not.toHaveBeenCalled() + }) + + it('force-resolves live origin when stored upstream/avatar are stale', async () => { + const repo = makeRepo({ + upstream: { owner: 'stablyai', repo: 'orca' }, + repoIcon: { + type: 'image', + src: 'https://github.com/stablyai.png?size=64', + source: 'github', + label: 'stablyai/orca' + } + }) + apiMocks.repoUpstream.mockResolvedValueOnce(null) + apiMocks.repoSlug.mockResolvedValueOnce({ owner: 'parkerrex', repo: 'orca' }) + + const resolution = await resolveRepositoryGitHubAvatar({ kind: 'local' }, repo, { + forceLive: true + }) + + expect(resolution).toEqual({ + repoIcon: { + type: 'image', + src: 'https://github.com/parkerrex.png?size=64', + source: 'github', + label: 'parkerrex/orca' + }, + upstream: null + }) + expect(apiMocks.repoUpstream).toHaveBeenCalledExactlyOnceWith({ + repoPath: '/workspace/orca', + repoId: 'repo-1' + }) + expect(apiMocks.repoSlug).toHaveBeenCalledExactlyOnceWith({ + repoPath: '/workspace/orca', + repoId: 'repo-1' + }) + expect(buildRepositoryGitHubAvatarUpdate(repo, resolution)).toEqual({ + upstream: null, + repoIcon: { + type: 'image', + src: 'https://github.com/parkerrex.png?size=64', + source: 'github', + label: 'parkerrex/orca' + } + }) + }) + + it('does not clear a GitHub avatar on passive refresh when live slug is unavailable', async () => { + const repo = makeRepo({ + repoIcon: { + type: 'image', + src: 'https://github.com/stablyai.png?size=64', + source: 'github', + label: 'stablyai/orca' + } + }) + + expect(buildRepositoryGitHubAvatarUpdate(repo, { repoIcon: null, upstream: null })).toEqual({ + upstream: null + }) + expect( + buildRepositoryGitHubAvatarUpdate( + repo, + { repoIcon: null, upstream: null }, + { + clearMissingIcon: true + } + ) + ).toEqual({ + upstream: null, + repoIcon: null + }) + }) +}) diff --git a/src/renderer/src/components/settings/repository-icon-github.ts b/src/renderer/src/components/settings/repository-icon-github.ts index 10546f676f..9b389e13fe 100644 --- a/src/renderer/src/components/settings/repository-icon-github.ts +++ b/src/renderer/src/components/settings/repository-icon-github.ts @@ -3,6 +3,14 @@ import { githubAvatarIcon, type RepoIcon } from '../../../../shared/repo-icon' import { callRuntimeRpc, type getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' type RuntimeTarget = ReturnType +type ResolveRepositoryGitHubAvatarOptions = { + forceLive?: boolean +} + +export type RepositoryGitHubAvatarResolution = { + repoIcon: RepoIcon | null + upstream: GitHubRepositoryIdentity | null +} export async function resolveRepositoryUpstreamLive( runtimeTarget: RuntimeTarget, @@ -18,25 +26,87 @@ export async function resolveRepositoryUpstreamLive( : await window.api.gh.repoUpstream({ repoPath: repo.path, repoId: repo.id }) } -export async function resolveRepositoryGitHubAvatarIcon( +async function resolveRepositorySlugLive( runtimeTarget: RuntimeTarget, repo: Repo -): Promise { +): Promise { + return runtimeTarget.kind === 'environment' + ? await callRuntimeRpc( + runtimeTarget, + 'github.repoSlug', + { repo: repo.id }, + { timeoutMs: 30_000 } + ) + : await window.api.gh.repoSlug({ repoPath: repo.path, repoId: repo.id }) +} + +export async function resolveRepositoryGitHubAvatar( + runtimeTarget: RuntimeTarget, + repo: Repo, + options: ResolveRepositoryGitHubAvatarOptions = {} +): Promise { const upstream = - repo.upstream !== undefined + !options.forceLive && repo.upstream !== undefined ? repo.upstream : await resolveRepositoryUpstreamLive(runtimeTarget, repo).catch(() => null) if (upstream) { - return githubAvatarIcon(upstream) - } - const slug = - runtimeTarget.kind === 'environment' - ? await callRuntimeRpc<{ owner: string; repo: string } | null>( - runtimeTarget, - 'github.repoSlug', - { repo: repo.id }, - { timeoutMs: 30_000 } - ) - : await window.api.gh.repoSlug({ repoPath: repo.path, repoId: repo.id }) - return slug ? githubAvatarIcon(slug) : null + return { repoIcon: githubAvatarIcon(upstream), upstream } + } + const slug = await resolveRepositorySlugLive(runtimeTarget, repo) + return { repoIcon: slug ? githubAvatarIcon(slug) : null, upstream: null } +} + +export async function resolveRepositoryGitHubAvatarIcon( + runtimeTarget: RuntimeTarget, + repo: Repo, + options: ResolveRepositoryGitHubAvatarOptions = {} +): Promise { + return (await resolveRepositoryGitHubAvatar(runtimeTarget, repo, options)).repoIcon +} + +function sameRepositoryIdentity( + a: GitHubRepositoryIdentity | null | undefined, + b: GitHubRepositoryIdentity | null | undefined +): boolean { + if (!a || !b) { + return a === b + } + return a.owner === b.owner && a.repo === b.repo +} + +function sameRepoIcon(a: RepoIcon | null | undefined, b: RepoIcon | null | undefined): boolean { + if (!a || !b) { + return a === b + } + if (a.type !== b.type) { + return false + } + if (a.type === 'image' && b.type === 'image') { + return a.src === b.src && a.source === b.source && a.label === b.label + } + if (a.type === 'emoji' && b.type === 'emoji') { + return a.emoji === b.emoji + } + return a.type === 'lucide' && b.type === 'lucide' && a.name === b.name +} + +export function buildRepositoryGitHubAvatarUpdate( + repo: Repo, + resolution: RepositoryGitHubAvatarResolution, + options: { clearMissingIcon?: boolean } = {} +): Partial | null { + const updates: Partial = {} + + if (!sameRepositoryIdentity(repo.upstream, resolution.upstream)) { + updates.upstream = resolution.upstream + } + + if ( + (resolution.repoIcon || options.clearMissingIcon) && + !sameRepoIcon(repo.repoIcon, resolution.repoIcon) + ) { + updates.repoIcon = resolution.repoIcon + } + + return Object.keys(updates).length > 0 ? updates : null }