Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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 } }
Comment on lines +30 to +31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Preserve happy-dom’s real window object.

Lines 30-31 replace globalThis.window with a plain object, which strips the real happy-dom window of document, timers, constructors, and other DOM state. That makes this test run against a broken browser global and can hide future regressions. Patch window.api instead of replacing window.

Proposed fix
-// `@ts-expect-error` test window mock
-globalThis.window = { api: { gh: apiMocks } }
+Object.defineProperty(window, 'api', {
+  value: { gh: apiMocks },
+  configurable: true
+})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// @ts-expect-error test window mock
globalThis.window = { api: { gh: apiMocks } }
Object.defineProperty(window, 'api', {
value: { gh: apiMocks },
configurable: true
})


function makeRepo(overrides: Partial<Repo> = {}): Repo {
return {
id: 'repo-1',
path: '/workspace/orca',
displayName: 'orca',
badgeColor: '#2563eb',
addedAt: 1,
kind: 'git',
...overrides
}
}

async function flushEffects(): Promise<void> {
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(<RepositoryIconPicker repo={repo} updateRepo={updateRepo} />)
})
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'
}
})
})
})
71 changes: 51 additions & 20 deletions src/renderer/src/components/settings/RepositoryIconPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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',
Expand All @@ -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(
Expand All @@ -113,45 +121,68 @@ 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)
}
}
}

const upstreamBackfilledRef = useRef<string | null>(null)
const githubIdentityRefreshedRef = useRef<string | null>(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<Repo> | 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<Repo> = { 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 (
<div className="space-y-3">
Expand Down
126 changes: 126 additions & 0 deletions src/renderer/src/components/settings/repository-icon-github.test.ts
Original file line number Diff line number Diff line change
@@ -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> = {}): 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
})
})
})
Loading
Loading