From e68831f32c9fdfeb9df9972110dcceb849385ad9 Mon Sep 17 00:00:00 2001 From: Wooseong Kim <2222333+innocarpe@users.noreply.github.com> Date: Fri, 7 Aug 2026 10:43:28 +0900 Subject: [PATCH] fix(github-project): index fork upstream slugs for project row matching (#12822) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(github-project): index fork upstream slugs for project row matching Project cards often reference the public upstream repo while the open clone's origin is a personal fork. Map the parent slug to the same Repo so selected-repo filters no longer hide every board row. Preserves origin-based getRepoSlug identity for non-project callers. Fixes #12647 * fix(github-project): match project rows against fork upstream slugs Resolve the referenced call to a nonexistent `resolveRepoUpstreamSlug` and match the persisted `repo.upstream` parent instead of issuing an extra `github.repoUpstream` RPC per repo on every index build — that lookup shells out to `gh repo view` for non-forks, so it would have gated the Projects tab on N network calls. `repo.upstream` is already resolved at repo-add time and backfilled at startup, so the fix costs no IPC. Origin matches take precedence over upstream ones so an open clone of the upstream repo itself is never made ambiguous by someone's fork of it. Also covers the two surfaces the origin-only match broke alongside the desktop table: mobile's project row matcher and the store-slice row-mutation routing. * fix(github-project): scope fork upstream matching by host and selection Round-1 review fixes on top of the upstream-slug index: - Apply origin-over-upstream precedence among *selected* repos instead of globally. An open-but-unselected clone of the upstream repo was shadowing the selected fork, so #12647 still reproduced for anyone holding both — and repo selection collapses to one repo per project key, which is exactly that case. - Scope a fork's upstream identity key to the fork's own origin host. Persistence strips upstream.host, so GHES forks never matched their own rows and a GHES fork's parent could bind a same-named github.com row. * fix(github-project): skip the fork alias when its own origin is unresolved Round-2 review fix. `githubHostFromIdentityKey` cannot tell "origin resolved to github.com" from "origin did not resolve" — both yield no host. A GHES fork whose slug resolution had failed (auth lapse, unreachable runtime) therefore landed in the github.com namespace, so an unrelated public Project row matched it and Start work opened the wrong clone on the wrong server. Require a resolved origin before indexing the upstream alias: it is the only host evidence there is, and a repo with an unresolved origin was already absent from the origin index, so nothing is lost that origin matching had. * fix(repos): persist the fork upstream host instead of dropping it `sanitizeRepoUpstream` kept only `{owner, repo}`, so a fork's parent lost the server it lives on every time the record round-tripped through disk. That forced the Project row matcher to re-infer the host from `origin`. The inference is right for an API-resolved fork parent — `getRepoUpstream` stamps `origin.host` there precisely because "a fork parent lives on the same server as the fork". It is wrong for the other branch: a local `upstream` remote carries its own host, so a github.com clone with a GHES `upstream` remote was indexed into the github.com namespace, where an unrelated same-owner/name public repo could claim it and Start work would open the wrong clone. Keeping the host removes the guess. Absent stays absent, so records written before this hydrate unchanged and the origin-derived fallback still covers them. Also fixes the avatar for rehydrated GHES forks, which resolved against github.com for the same reason. * docs(github-project): correct upstream host fallback comment Persistence now keeps non-empty upstream.host; originIdentityKey remains the host fallback for older records without one (CodeRabbit nit). * fix(github-project): own slug-index retry timer cleanup Move the failure-retry setTimeout into its own effect so cleanup always clears it. Scheduling from the async buildIndex then-handler failed the react-doctor effect-needs-cleanup gate in static analysis. * test(github-project): guard the slug-index retry timer, fix the mobile twin comment Two follow-ups on 52298d82 and 2f89c20d: - Cover the retry timer both ways: a failed resolution still re-resolves after the TTL and recovers the match, and the pending timer is gone after unmount. The second fails if the timer moves back into the async then-handler, so the property is guarded by more than the lint rule. - The mobile matcher's comment made the same stale "persistence strips upstream.host" claim that 2f89c20d fixed on the renderer side. * test(github-project): unmount slug-index hooks so React cannot flush after teardown CI shard `tests node 24 6/16` failed with 10 unhandled `ReferenceError: window is not defined` traced to this file. The tests mounted hooks without unmounting, so React scheduler work flushed after the DOM environment was disposed. All assertions passed; the shard failed on the unhandled errors alone. `cleanup()` after each test unmounts the trees. Does not reproduce locally in isolation — it needs CI's worker pooling and file ordering. --------- Co-authored-by: Jinwoo-H --- mobile/app/h/[hostId]/tasks.tsx | 2 + .../tasks/github-project-repo-match.test.ts | 130 ++++++++++++ mobile/src/tasks/github-project-repo-match.ts | 38 ++++ src/main/persistence.test.ts | 37 ++++ src/main/persistence.ts | 12 +- .../github-project/ProjectViewWrapper.tsx | 19 +- .../project-row-filtering.test.ts | 79 ++++++-- .../github-project/project-row-filtering.ts | 25 ++- src/renderer/src/lib/repo-slug-cache.test.ts | 56 ++++++ src/renderer/src/lib/repo-slug-cache.ts | 42 +++- src/renderer/src/lib/repo-slug-index.test.ts | 189 ++++++++++++++++++ src/renderer/src/lib/repo-slug-index.ts | 89 ++++++--- src/shared/github-repository-identity-key.ts | 9 + 13 files changed, 672 insertions(+), 55 deletions(-) create mode 100644 src/renderer/src/lib/repo-slug-index.test.ts diff --git a/mobile/app/h/[hostId]/tasks.tsx b/mobile/app/h/[hostId]/tasks.tsx index c41b165cbf9..0794c317e41 100644 --- a/mobile/app/h/[hostId]/tasks.tsx +++ b/mobile/app/h/[hostId]/tasks.tsx @@ -165,6 +165,8 @@ type RepoSummary = { kind?: 'git' | 'folder' connectionId?: string | null issueSourcePreference?: IssueSourcePreference + /** Fork parent resolved by the host; drives upstream Project row matching. */ + upstream?: { owner: string; repo: string; host?: string } | null } type IssueSourcePreference = 'upstream' | 'origin' | 'auto' diff --git a/mobile/src/tasks/github-project-repo-match.test.ts b/mobile/src/tasks/github-project-repo-match.test.ts index bce71393dab..58c44a57830 100644 --- a/mobile/src/tasks/github-project-repo-match.test.ts +++ b/mobile/src/tasks/github-project-repo-match.test.ts @@ -115,6 +115,136 @@ describe('GitHub project repo matching', () => { ).toBe(repos[1]) }) + it('matches an upstream project row against a fork clone', () => { + const fork = { + id: 'repo-1', + path: '/Users/me/r2r-mirror', + displayName: 'r2r-mirror', + upstream: { owner: 'SciPhi-AI', repo: 'R2R' } + } + + expect( + findRepoForGitHubProjectRepository('SciPhi-AI/R2R', [fork], { + 'repo-1': { + path: '/Users/me/r2r-mirror', + repository: { owner: 'me', repo: 'r2r-mirror' } + } + }) + ).toBe(fork) + }) + + it('prefers the clone that owns the slug over a fork of it', () => { + const upstreamClone = { id: 'repo-1', path: '/Users/me/r2r', displayName: 'r2r' } + const fork = { + id: 'repo-2', + path: '/Users/me/r2r-mirror', + displayName: 'r2r-mirror', + upstream: { owner: 'SciPhi-AI', repo: 'R2R' } + } + + expect( + findRepoForGitHubProjectRepository('SciPhi-AI/R2R', [upstreamClone, fork], { + 'repo-1': { + path: '/Users/me/r2r', + repository: { owner: 'SciPhi-AI', repo: 'R2R' } + }, + 'repo-2': { + path: '/Users/me/r2r-mirror', + repository: { owner: 'me', repo: 'r2r-mirror' } + } + }) + ).toBe(upstreamClone) + }) + + it('does not pick a repo when two forks share the same upstream', () => { + const forks = [ + { + id: 'repo-1', + path: '/Users/me/a', + displayName: 'a', + upstream: { owner: 'SciPhi-AI', repo: 'R2R' } + }, + { + id: 'repo-2', + path: '/Users/me/b', + displayName: 'b', + upstream: { owner: 'SciPhi-AI', repo: 'R2R' } + } + ] + + expect( + findRepoForGitHubProjectRepository('SciPhi-AI/R2R', forks, { + 'repo-1': { path: '/Users/me/a', repository: { owner: 'me', repo: 'a' } }, + 'repo-2': { path: '/Users/me/b', repository: { owner: 'me', repo: 'b' } } + }) + ).toBeNull() + }) + + it('does not bind a github.com fork parent to a same-named Enterprise row', () => { + const fork = { + id: 'repo-1', + path: '/Users/me/r2r-mirror', + displayName: 'r2r-mirror', + upstream: { owner: 'SciPhi-AI', repo: 'R2R' } + } + + expect( + findRepoForGitHubProjectRepository( + 'SciPhi-AI/R2R', + [fork], + { + 'repo-1': { + path: '/Users/me/r2r-mirror', + repository: { owner: 'me', repo: 'r2r-mirror', host: 'github.com' } + } + }, + 'github.acme-corp.com' + ) + ).toBeNull() + }) + + it('drops the fork alias while its own origin is unresolved', () => { + const fork = { + id: 'repo-1', + path: '/Users/me/widgets-mirror', + displayName: 'widgets-mirror', + upstream: { owner: 'acme', repo: 'widgets' } + } + + for (const slugs of [ + {}, + { 'repo-1': { path: '/Users/me/widgets-mirror', repository: null } }, + { 'repo-1': { path: '/moved', repository: { owner: 'me', repo: 'widgets' } } } + ]) { + expect(findRepoForGitHubProjectRepository('acme/widgets', [fork], slugs)).toBeNull() + } + }) + + it('scopes a host-less fork parent to the host the fork itself was cloned from', () => { + const enterpriseFork = { + id: 'repo-1', + path: '/Users/me/widgets-mirror', + displayName: 'widgets-mirror', + upstream: { owner: 'acme', repo: 'widgets' } + } + const slugs = { + 'repo-1': { + path: '/Users/me/widgets-mirror', + repository: { owner: 'me', repo: 'widgets', host: 'github.acme-corp.com' } + } + } + + expect( + findRepoForGitHubProjectRepository( + 'acme/widgets', + [enterpriseFork], + slugs, + 'github.acme-corp.com' + ) + ).toBe(enterpriseFork) + expect(findRepoForGitHubProjectRepository('acme/widgets', [enterpriseFork], slugs)).toBeNull() + }) + it('does not use hostless path heuristics for Enterprise Project rows', () => { expect( findRepoForGitHubProjectRepository( diff --git a/mobile/src/tasks/github-project-repo-match.ts b/mobile/src/tasks/github-project-repo-match.ts index c4d1e1d2ea6..1e9cd61996a 100644 --- a/mobile/src/tasks/github-project-repo-match.ts +++ b/mobile/src/tasks/github-project-repo-match.ts @@ -7,6 +7,9 @@ export type GitHubProjectRepoMatch = { id: string path: string displayName: string + /** Fork parent resolved by the host and carried on `repo.list`. Absent = not + * a fork or not yet resolved. */ + upstream?: { owner: string; repo: string; host?: string } | null } export type GitHubRepoSlugCacheEntry = { @@ -45,6 +48,27 @@ function cachedSlugStateForRepo( return { status: 'resolved', repository: cached.repository } } +/** Identity key of the repo's fork parent, or null when it is not a fork or its + * origin has not resolved. Why: when `upstream.host` is absent (older persisted + * forks), the fork's origin host is the fallback so GHES parents do not collapse + * into github.com. Unresolved origins refuse the alias. */ +function upstreamIdentityKeyForRepo( + repo: GitHubProjectRepoMatch, + originState: CachedSlugState | undefined +): string | null { + const upstream = repo.upstream + if (!upstream?.owner || !upstream.repo) { + return null + } + if (originState?.status !== 'resolved' || !originState.repository) { + return null + } + return githubRepoIdentityKey({ + ...upstream, + host: upstream.host ?? originState.repository.host + }) +} + export function findRepoForGitHubProjectRepository( repository: string | null | undefined, repos: GitHubProjectRepoMatch[], @@ -79,6 +103,20 @@ export function findRepoForGitHubProjectRepository( return null } + // Why: a Project card references the upstream repo, but a contributor's clone + // has their personal fork as `origin`, so origin-only matching hid every row + // (#12647). Checked after origin so an open clone of the upstream repo itself + // always wins over someone's fork of it. + const upstreamMatches = repos.filter( + (repo) => upstreamIdentityKeyForRepo(repo, slugStates.get(repo.id)) === requestedIdentityKey + ) + if (upstreamMatches.length === 1) { + return upstreamMatches[0]! + } + if (upstreamMatches.length > 1) { + return null + } + if (!isDefaultGitHubHost(projectHost)) { // Why: display names and local paths contain no host evidence, so using // them for GHES rows could bind an Enterprise item to a github.com repo. diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 059402ae9ed..b59ca6f5986 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -4617,6 +4617,43 @@ describe('Store', () => { expect(store.getRepos()[0]!.upstream).toBeUndefined() }) + it('keeps the upstream host across reloads so it is never re-inferred from origin', async () => { + const store = await createStore() + store.addRepo(makeRepo()) + + const updated = store.updateRepo('r1', { + upstream: { owner: ' acme ', repo: ' widgets ', host: ' GHE.example:8443 ' } + }) + expect(updated!.upstream).toEqual({ + owner: 'acme', + repo: 'widgets', + host: 'GHE.example:8443' + }) + + store.flush() + const reloaded = await createStore() + expect(reloaded.getRepo('r1')!.upstream).toEqual({ + owner: 'acme', + repo: 'widgets', + host: 'GHE.example:8443' + }) + }) + + it('leaves a hostless persisted upstream hostless rather than inventing one', async () => { + const store = await createStore() + store.addRepo(makeRepo({ upstream: { owner: 'stablyai', repo: 'orca' } })) + + expect(store.getRepo('r1')!.upstream).toEqual({ owner: 'stablyai', repo: 'orca' }) + expect(store.getRepo('r1')!.upstream).not.toHaveProperty('host') + }) + + it('drops a blank upstream host instead of persisting an empty string', async () => { + const store = await createStore() + store.addRepo(makeRepo({ upstream: { owner: 'acme', repo: 'widgets', host: ' ' } })) + + expect(store.getRepo('r1')!.upstream).toEqual({ owner: 'acme', repo: 'widgets' }) + }) + it('updateRepo returns null for nonexistent id', async () => { const store = await createStore() expect(store.updateRepo('nope', { displayName: 'x' })).toBeNull() diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 5d581a7f6e6..04188206ad7 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -1411,10 +1411,18 @@ function sanitizeRepoUpstream(value: unknown): Repo['upstream'] | undefined { if (!value || typeof value !== 'object') { return undefined } - const candidate = value as { owner?: unknown; repo?: unknown } + const candidate = value as { owner?: unknown; repo?: unknown; host?: unknown } const owner = typeof candidate.owner === 'string' ? candidate.owner.trim() : '' const repo = typeof candidate.repo === 'string' ? candidate.repo.trim() : '' - return owner && repo ? { owner, repo } : undefined + if (!owner || !repo) { + return undefined + } + // Why: an `upstream` remote may live on a different server than `origin`, so + // dropping the host forced consumers to re-infer it from origin and could bind + // a GHES parent to a same-named github.com repo. Absent host stays absent so + // records written before this survive unchanged. + const host = typeof candidate.host === 'string' ? candidate.host.trim() : '' + return host ? { owner, repo, host } : { owner, repo } } function sanitizeGitRemoteIdentity(value: unknown): GitRemoteIdentity | null | undefined { diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index 39f43539ee9..59d02e24f31 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -141,7 +141,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS const patchProjectRowIssueType = useAppStore((s) => s.patchProjectRowIssueType) const addRepoFromStore = useAppStore((s) => s.addRepo) const repos = useAppStore((s) => s.repos) - const { lookupSlug, ready: slugIndexReady } = useRepoSlugIndex() + const { lookupSlug, lookupSlugMatches, ready: slugIndexReady } = useRepoSlugIndex() const mountedRef = useMountedRef() const activeProject = settings?.githubProjects?.activeProject ?? null @@ -375,9 +375,14 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS const filteredTable = useMemo( () => table && slugIndexReady - ? filterProjectTableRowsBySelectedRepos(table, lookupSlug, slugIndexReady, selectedRepoIds) + ? filterProjectTableRowsBySelectedRepos( + table, + lookupSlugMatches, + slugIndexReady, + selectedRepoIds + ) : null, - [table, slugIndexReady, lookupSlug, selectedRepoIds] + [table, slugIndexReady, lookupSlugMatches, selectedRepoIds] ) const lastFilteredTableRef = useRef(null) // Why: ref-cache prevents a blank table while the slug index rebuilds, without forcing a second render. @@ -528,7 +533,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS } const resolution = resolveSelectedProjectRowRepo({ row, - lookupSlug, + lookupSlugMatches, host: table.project.host, slugIndexReady, selectedRepoIds @@ -584,7 +589,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS currentCacheKey, table, buildOrigin, - lookupSlug, + lookupSlugMatches, slugIndexReady, selectedRepoIds, openProjectRowUrlWithToast @@ -602,7 +607,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS } const resolution = resolveSelectedProjectRowRepo({ row, - lookupSlug, + lookupSlugMatches, host: table.project.host, slugIndexReady, selectedRepoIds @@ -671,7 +676,7 @@ export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JS currentCacheKey, table, buildOrigin, - lookupSlug, + lookupSlugMatches, slugIndexReady, selectedRepoIds, openProjectRowUrlWithToast diff --git a/src/renderer/src/components/github-project/project-row-filtering.test.ts b/src/renderer/src/components/github-project/project-row-filtering.test.ts index 708cef2fac5..8871c047f7a 100644 --- a/src/renderer/src/components/github-project/project-row-filtering.test.ts +++ b/src/renderer/src/components/github-project/project-row-filtering.test.ts @@ -17,6 +17,11 @@ function repo(id: string): Repo { } } +/** The common case: every match owns the slug through its own origin remote. */ +function originOnly(matches: Repo[]): { origin: Repo[]; upstream: Repo[] } { + return { origin: matches, upstream: [] } +} + function row(id: string, repository: string | null): GitHubProjectRow { return { id, @@ -102,7 +107,8 @@ describe('filterProjectTableRowsBySelectedRepos', () => { const rows = [row('visible', 'acme/orca'), row('hidden', 'acme/tool')] const filtered = filterProjectTableRowsBySelectedRepos( table(rows), - (slug) => (slug?.toLowerCase() === 'acme/orca' ? [repo('repo-1')] : [repo('repo-2')]), + (slug) => + originOnly(slug?.toLowerCase() === 'acme/orca' ? [repo('repo-1')] : [repo('repo-2')]), true, new Set(['repo-1']) ) @@ -115,7 +121,7 @@ describe('filterProjectTableRowsBySelectedRepos', () => { const rows = [row('hidden', 'acme/orca')] const filtered = filterProjectTableRowsBySelectedRepos( table(rows), - () => [repo('repo-2')], + () => originOnly([repo('repo-2')]), true, new Set(['repo-1']) ) @@ -124,11 +130,24 @@ describe('filterProjectTableRowsBySelectedRepos', () => { expect(filtered.totalCount).toBe(0) }) + it('keeps a row whose only selected match is a fork of the row s repo', () => { + const rows = [row('fork-only', 'acme/orca')] + const filtered = filterProjectTableRowsBySelectedRepos( + table(rows), + () => ({ origin: [], upstream: [repo('fork')] }), + true, + new Set(['fork']) + ) + + expect(filtered.rows.map((r) => r.id)).toEqual(['fork-only']) + expect(filtered.totalCount).toBe(1) + }) + it('keeps a row with multiple selected matches for action ambiguity handling', () => { const rows = [row('ambiguous', 'acme/orca')] const filtered = filterProjectTableRowsBySelectedRepos( table(rows), - () => [repo('repo-1'), repo('repo-2'), repo('repo-3')], + () => originOnly([repo('repo-1'), repo('repo-2'), repo('repo-3')]), true, new Set(['repo-1', 'repo-2']) ) @@ -141,7 +160,7 @@ describe('resolveSelectedProjectRowRepo', () => { it('reports loading without reading stale slug matches', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('loading', 'acme/orca'), - lookupSlug: () => { + lookupSlugMatches: () => { throw new Error('should not read stale matches') }, slugIndexReady: false, @@ -154,7 +173,7 @@ describe('resolveSelectedProjectRowRepo', () => { it('reports invalid slug for rows without a repository', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('missing-slug', null), - lookupSlug: () => [repo('repo-1')], + lookupSlugMatches: () => originOnly([repo('repo-1')]), slugIndexReady: true, selectedRepoIds: new Set(['repo-1']) }) @@ -165,7 +184,7 @@ describe('resolveSelectedProjectRowRepo', () => { it('reports no global match when Orca has no repo for the slug', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('missing', 'acme/orca'), - lookupSlug: () => [], + lookupSlugMatches: () => originOnly([]), slugIndexReady: true, selectedRepoIds: new Set(['repo-1']) }) @@ -176,7 +195,7 @@ describe('resolveSelectedProjectRowRepo', () => { it('reports global-only matches when the repo is not selected', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('unselected', 'acme/orca'), - lookupSlug: () => [repo('repo-2')], + lookupSlugMatches: () => originOnly([repo('repo-2')]), slugIndexReady: true, selectedRepoIds: new Set(['repo-1']) }) @@ -187,7 +206,7 @@ describe('resolveSelectedProjectRowRepo', () => { it('returns the selected match when exactly one matching repo is selected', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('selected', 'acme/orca'), - lookupSlug: () => [repo('repo-1'), repo('repo-2')], + lookupSlugMatches: () => originOnly([repo('repo-1'), repo('repo-2')]), slugIndexReady: true, selectedRepoIds: new Set(['repo-2']) }) @@ -196,28 +215,64 @@ describe('resolveSelectedProjectRowRepo', () => { }) it('passes the project host into repository matching', () => { - const lookupSlug = vi.fn(() => [repo('repo-1')]) + const lookupSlugMatches = vi.fn(() => originOnly([repo('repo-1')])) expect( resolveSelectedProjectRowRepo({ row: row('enterprise', 'acme/orca'), - lookupSlug, + lookupSlugMatches, host: 'ghe.example:8443', slugIndexReady: true, selectedRepoIds: new Set(['repo-1']) }) ).toMatchObject({ status: 'selected_match' }) - expect(lookupSlug).toHaveBeenCalledWith('acme/orca', 'ghe.example:8443') + expect(lookupSlugMatches).toHaveBeenCalledWith('acme/orca', 'ghe.example:8443') }) it('reports ambiguity when multiple matching repos are selected', () => { const resolution = resolveSelectedProjectRowRepo({ row: row('ambiguous', 'acme/orca'), - lookupSlug: () => [repo('repo-1'), repo('repo-2')], + lookupSlugMatches: () => originOnly([repo('repo-1'), repo('repo-2')]), slugIndexReady: true, selectedRepoIds: new Set(['repo-1', 'repo-2']) }) expect(resolution.status).toBe('ambiguous_selected_match') }) + + it('falls through to a selected fork when the upstream clone is unselected', () => { + const resolution = resolveSelectedProjectRowRepo({ + row: row('fork-selected', 'acme/orca'), + lookupSlugMatches: () => ({ origin: [repo('upstream')], upstream: [repo('fork')] }), + slugIndexReady: true, + selectedRepoIds: new Set(['fork']) + }) + + expect(resolution).toMatchObject({ status: 'selected_match', repo: { id: 'fork' } }) + }) + + it('prefers the selected upstream clone over a selected fork of it', () => { + const resolution = resolveSelectedProjectRowRepo({ + row: row('both-selected', 'acme/orca'), + lookupSlugMatches: () => ({ origin: [repo('upstream')], upstream: [repo('fork')] }), + slugIndexReady: true, + selectedRepoIds: new Set(['upstream', 'fork']) + }) + + expect(resolution).toMatchObject({ status: 'selected_match', repo: { id: 'upstream' } }) + }) + + it('still reports no selection when neither the upstream clone nor the fork is selected', () => { + const resolution = resolveSelectedProjectRowRepo({ + row: row('neither', 'acme/orca'), + lookupSlugMatches: () => ({ origin: [repo('upstream')], upstream: [repo('fork')] }), + slugIndexReady: true, + selectedRepoIds: new Set(['other']) + }) + + expect(resolution).toMatchObject({ + status: 'unselected_match', + globalMatches: [{ id: 'upstream' }, { id: 'fork' }] + }) + }) }) diff --git a/src/renderer/src/components/github-project/project-row-filtering.ts b/src/renderer/src/components/github-project/project-row-filtering.ts index 1aa2f4c3ed8..52dd0138274 100644 --- a/src/renderer/src/components/github-project/project-row-filtering.ts +++ b/src/renderer/src/components/github-project/project-row-filtering.ts @@ -6,6 +6,13 @@ export type ProjectRowSlugLookup = ( host?: string ) => readonly Repo[] +/** Origin matches own the slug through their own remote; upstream matches are + * forks whose parent the row names. */ +export type ProjectRowSlugMatchLookup = ( + slug: string | null | undefined, + host?: string +) => { origin: readonly Repo[]; upstream: readonly Repo[] } + export type SelectedProjectRowResolution = | { status: 'loading' } | { status: 'invalid_slug' } @@ -20,7 +27,7 @@ export type SelectedProjectRowResolution = export function resolveSelectedProjectRowRepo(input: { row: GitHubProjectRow - lookupSlug: ProjectRowSlugLookup + lookupSlugMatches: ProjectRowSlugMatchLookup host?: string slugIndexReady: boolean selectedRepoIds: ReadonlySet @@ -38,12 +45,20 @@ export function resolveSelectedProjectRowRepo(input: { return { status: 'invalid_slug' } } - const globalMatches = input.lookupSlug(repository, input.host) + const { origin, upstream } = input.lookupSlugMatches(repository, input.host) + const globalMatches = [...origin, ...upstream] if (globalMatches.length === 0) { return { status: 'no_global_match' } } - const selectedMatches = globalMatches.filter((match) => input.selectedRepoIds.has(match.id)) + // Why: prefer origin only among repos the user actually selected. Applying + // that preference globally let an open-but-unselected clone of the upstream + // repo hide the selected fork, reproducing #12647 for anyone holding both. + const selectedOrigin = origin.filter((match) => input.selectedRepoIds.has(match.id)) + const selectedMatches = + selectedOrigin.length > 0 + ? selectedOrigin + : upstream.filter((match) => input.selectedRepoIds.has(match.id)) if (selectedMatches.length === 0) { return { status: 'unselected_match', globalMatches } } @@ -76,14 +91,14 @@ export function filterProjectTableRowsByOpenRepos( export function filterProjectTableRowsBySelectedRepos( table: GitHubProjectTable, - lookupSlug: ProjectRowSlugLookup, + lookupSlugMatches: ProjectRowSlugMatchLookup, slugIndexReady: boolean, selectedRepoIds: ReadonlySet ): GitHubProjectTable { const rows = table.rows.filter((row) => { const resolution = resolveSelectedProjectRowRepo({ row, - lookupSlug, + lookupSlugMatches, host: table.project.host, slugIndexReady, selectedRepoIds diff --git a/src/renderer/src/lib/repo-slug-cache.test.ts b/src/renderer/src/lib/repo-slug-cache.test.ts index dd0f9e4bd28..d9de7ae3e7b 100644 --- a/src/renderer/src/lib/repo-slug-cache.test.ts +++ b/src/renderer/src/lib/repo-slug-cache.test.ts @@ -46,6 +46,62 @@ describe('repo slug cache host identity', () => { ).toEqual([enterprise]) }) + it('routes an upstream project row to the fork clone that tracks it', () => { + const fork = { ...repo('fork'), upstream: { owner: 'SciPhi-AI', repo: 'R2R' } } + slugByRepoId.set( + slugCacheKey(fork.id, settingsForRepoOwner(fork, null)), + githubRepoIdentityKey({ owner: 'me', repo: 'r2r-mirror' }) + ) + + expect(lookupReposBySlugFromCache([fork], null, 'SciPhi-AI/R2R')).toEqual([fork]) + }) + + it('prefers the clone that owns the slug over a fork of it', () => { + const origin = repo('origin') + const fork = { ...repo('fork'), upstream: { owner: 'SciPhi-AI', repo: 'R2R' } } + slugByRepoId.set( + slugCacheKey(origin.id, settingsForRepoOwner(origin, null)), + githubRepoIdentityKey({ owner: 'SciPhi-AI', repo: 'R2R' }) + ) + slugByRepoId.set( + slugCacheKey(fork.id, settingsForRepoOwner(fork, null)), + githubRepoIdentityKey({ owner: 'me', repo: 'r2r-mirror' }) + ) + + expect(lookupReposBySlugFromCache([origin, fork], null, 'SciPhi-AI/R2R')).toEqual([origin]) + }) + + it('does not route a GHES row to a same-named github.com fork parent', () => { + const fork = { ...repo('fork'), upstream: { owner: 'acme', repo: 'widgets' } } + slugByRepoId.set( + slugCacheKey(fork.id, settingsForRepoOwner(fork, null)), + githubRepoIdentityKey({ owner: 'me', repo: 'widgets' }) + ) + + expect(lookupReposBySlugFromCache([fork], null, 'acme/widgets', 'ghe.example:8443')).toEqual([]) + }) + + it('scopes a host-less fork parent to the host the fork itself was cloned from', () => { + const enterpriseFork = { ...repo('fork'), upstream: { owner: 'acme', repo: 'widgets' } } + slugByRepoId.set( + slugCacheKey(enterpriseFork.id, settingsForRepoOwner(enterpriseFork, null)), + githubRepoIdentityKey({ owner: 'me', repo: 'widgets', host: 'ghe.example:8443' }) + ) + + expect( + lookupReposBySlugFromCache([enterpriseFork], null, 'acme/widgets', 'ghe.example:8443') + ).toEqual([enterpriseFork]) + expect(lookupReposBySlugFromCache([enterpriseFork], null, 'acme/widgets')).toEqual([]) + }) + + it('drops the fork alias while its own origin is unresolved', () => { + const fork = { ...repo('fork'), upstream: { owner: 'acme', repo: 'widgets' } } + + expect(lookupReposBySlugFromCache([fork], null, 'acme/widgets')).toEqual([]) + slugByRepoId.set(slugCacheKey(fork.id, settingsForRepoOwner(fork, null)), null) + expect(lookupReposBySlugFromCache([fork], null, 'acme/widgets')).toEqual([]) + }) + it('expires negative slug resolutions so an external GHES login can recover', () => { const key = slugCacheKey('enterprise', null) rememberRepoSlug(key, null, 1_000) diff --git a/src/renderer/src/lib/repo-slug-cache.ts b/src/renderer/src/lib/repo-slug-cache.ts index 38ea5d32995..cb6552aa572 100644 --- a/src/renderer/src/lib/repo-slug-cache.ts +++ b/src/renderer/src/lib/repo-slug-cache.ts @@ -4,11 +4,40 @@ import type { GlobalSettings, Repo } from '../../../shared/types' import { getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { getSettingsForRepoRuntimeOwner } from './repo-runtime-owner' -import { githubRepoIdentityKey } from '../../../shared/github-repository-identity-key' +import { + githubHostFromIdentityKey, + githubRepoIdentityKey +} from '../../../shared/github-repository-identity-key' /** Lowercased `owner/repo` → Repo[]. */ export type SlugIndex = Map +/** The two ways a slug can match a repo, kept apart so callers that also filter + * by repo selection can fall through to `upstream` instead of letting an + * unselected clone of the upstream repo shadow the selected fork. */ +export type RepoSlugMatches = { origin: Repo[]; upstream: Repo[] } + +/** Identity key of a fork's upstream parent — the second identity a Project row + * may legitimately match, since a contributor's clone has the personal fork as + * `origin`. `null` when the repo is not a fork or the key cannot be trusted. + * + * Why `originIdentityKey` is required: when `upstream.host` is absent (older + * persisted forks), the fork's origin host is the fallback so GHES parents do + * not collapse into github.com. Unresolved origins refuse the alias. */ +export function repoUpstreamIdentityKey( + repo: Repo, + originIdentityKey: string | null | undefined +): string | null { + const upstream = repo.upstream + if (!upstream?.owner || !upstream.repo || !originIdentityKey) { + return null + } + return githubRepoIdentityKey({ + ...upstream, + host: upstream.host ?? githubHostFromIdentityKey(originIdentityKey) + }) +} + /** Module-scope cache keyed by runtime scope + repo.id. A Repo that has already * failed resolution is recorded as `null` briefly so it is not retried on every * cell mount, while still recovering after an external GHES auth login. */ @@ -87,7 +116,8 @@ export function settingsForRepoOwner( /** Synchronous slug → Repo lookup against the already-resolved module cache. * Used by store slices (which can't run the async hook-based index) to route * project-row mutations to the matched repo's owner host; callers fall back to - * focused settings when nothing matches. */ + * focused settings when nothing matches. Origin matches win over upstream ones + * so a clone of the upstream repo itself is never shadowed by someone's fork. */ export function lookupReposBySlugFromCache( repos: readonly Repo[], settings: Pick | null | undefined, @@ -100,11 +130,15 @@ export function lookupReposBySlugFromCache( } const target = githubRepoIdentityKey({ owner, repo, host }) const matched: Repo[] = [] + const upstreamMatched: Repo[] = [] for (const repo of repos) { const cacheKey = slugCacheKey(repo.id, settingsForRepoOwner(repo, settings)) - if (slugByRepoId.get(cacheKey) === target) { + const originKey = slugByRepoId.get(cacheKey) + if (originKey === target) { matched.push(repo) + } else if (repoUpstreamIdentityKey(repo, originKey) === target) { + upstreamMatched.push(repo) } } - return matched + return matched.length > 0 ? matched : upstreamMatched } diff --git a/src/renderer/src/lib/repo-slug-index.test.ts b/src/renderer/src/lib/repo-slug-index.test.ts new file mode 100644 index 00000000000..0b710cf5438 --- /dev/null +++ b/src/renderer/src/lib/repo-slug-index.test.ts @@ -0,0 +1,189 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../shared/types' +import { useAppStore } from '@/store' +import { useRepoSlugIndex } from './repo-slug-index' +import { REPO_SLUG_FAILURE_TTL_MS, clearRepoSlugCacheValues } from './repo-slug-cache' + +const repoSlug = vi.fn() + +function makeRepo(overrides: Partial & { id: string }): Repo { + return { + path: `/repos/${overrides.id}`, + displayName: overrides.id, + badgeColor: '#000', + addedAt: 1, + executionHostId: 'local', + ...overrides + } +} + +const initialState = useAppStore.getInitialState() + +function setRepos(repos: Repo[]): void { + useAppStore.setState({ ...initialState, repos, settings: initialState.settings }, true) +} + +beforeEach(() => { + clearRepoSlugCacheValues() + repoSlug.mockReset() + // Why: origin resolution goes through the preload bridge; the index under + // test only reads `repo.upstream` from the store for the fork alias. + Object.assign(window, { api: { gh: { repoSlug } } }) +}) + +afterEach(() => { + // Why: React schedules work outside the test tick, so a hook left mounted + // flushes after the DOM environment is torn down and throws "window is not + // defined" as an unhandled error. + cleanup() + setRepos([]) +}) + +async function lookup(slug: string): Promise { + const { result } = renderHook(() => useRepoSlugIndex()) + await waitFor(() => expect(result.current.ready).toBe(true)) + return [...result.current.lookupSlug(slug)] +} + +describe('useRepoSlugIndex fork upstream matching', () => { + it('matches a project row against a fork clone via its upstream parent', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([fork]) + repoSlug.mockResolvedValue({ owner: 'me', repo: 'r2r-mirror' }) + + expect(await lookup('SciPhi-AI/R2R')).toEqual([fork]) + }) + + it('still matches the fork by its own origin slug', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([fork]) + repoSlug.mockResolvedValue({ owner: 'me', repo: 'r2r-mirror' }) + + expect(await lookup('me/r2r-mirror')).toEqual([fork]) + }) + + it('prefers the clone that owns the slug over a fork of it', async () => { + const upstreamClone = makeRepo({ id: 'upstream', upstream: null }) + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([upstreamClone, fork]) + repoSlug.mockImplementation(async (args: { repoId?: string }) => + args.repoId === 'upstream' + ? { owner: 'SciPhi-AI', repo: 'R2R' } + : { owner: 'me', repo: 'r2r-mirror' } + ) + + expect(await lookup('SciPhi-AI/R2R')).toEqual([upstreamClone]) + }) + + it('does not bind a github.com fork parent to a same-named Enterprise row', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([fork]) + repoSlug.mockResolvedValue({ owner: 'me', repo: 'r2r-mirror' }) + + const { result } = renderHook(() => useRepoSlugIndex()) + await waitFor(() => expect(result.current.ready).toBe(true)) + expect(result.current.lookupSlug('SciPhi-AI/R2R', 'ghe.example')).toEqual([]) + }) + + it('resolves the slug index without extra upstream IPC', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([fork]) + repoSlug.mockResolvedValue({ owner: 'me', repo: 'r2r-mirror' }) + + expect(await lookup('SciPhi-AI/R2R')).toEqual([fork]) + expect(repoSlug).toHaveBeenCalledTimes(1) + }) + + it('scopes a host-less fork parent to the host the fork itself was cloned from', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'acme', repo: 'widgets' } }) + setRepos([fork]) + repoSlug.mockResolvedValue({ owner: 'me', repo: 'widgets', host: 'ghe.example' }) + + const { result } = renderHook(() => useRepoSlugIndex()) + await waitFor(() => expect(result.current.ready).toBe(true)) + expect(result.current.lookupSlug('acme/widgets', 'ghe.example')).toEqual([fork]) + expect(result.current.lookupSlug('acme/widgets')).toEqual([]) + }) + + it('drops the fork alias while its own origin is unresolved', async () => { + const fork = makeRepo({ id: 'fork', upstream: { owner: 'acme', repo: 'widgets' } }) + setRepos([fork]) + repoSlug.mockResolvedValue(null) + + expect(await lookup('acme/widgets')).toEqual([]) + }) + + it('lists a repo once when its upstream is its own origin', async () => { + const selfAliased = makeRepo({ id: 'self', upstream: { owner: 'acme', repo: 'widgets' } }) + setRepos([selfAliased]) + repoSlug.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + + const { result } = renderHook(() => useRepoSlugIndex()) + await waitFor(() => expect(result.current.ready).toBe(true)) + expect(result.current.lookupSlugMatches('acme/widgets')).toEqual({ + origin: [selfAliased], + upstream: [] + }) + }) + + it('reports origin and upstream matches separately for selection filtering', async () => { + const upstreamClone = makeRepo({ id: 'upstream', upstream: null }) + const fork = makeRepo({ id: 'fork', upstream: { owner: 'SciPhi-AI', repo: 'R2R' } }) + setRepos([upstreamClone, fork]) + repoSlug.mockImplementation(async (args: { repoId?: string }) => + args.repoId === 'upstream' + ? { owner: 'SciPhi-AI', repo: 'R2R' } + : { owner: 'me', repo: 'r2r-mirror' } + ) + + const { result } = renderHook(() => useRepoSlugIndex()) + await waitFor(() => expect(result.current.ready).toBe(true)) + expect(result.current.lookupSlugMatches('SciPhi-AI/R2R')).toEqual({ + origin: [upstreamClone], + upstream: [fork] + }) + }) +}) + +describe('useRepoSlugIndex failure retry', () => { + afterEach(() => vi.useRealTimers()) + + it('re-resolves a failed slug after the failure TTL and stops once unmounted', async () => { + vi.useFakeTimers() + const repo = makeRepo({ id: 'flaky' }) + setRepos([repo]) + // Why: a null result is the negative-cached "not a GitHub repo" answer that + // arms the bounded retry. + repoSlug.mockResolvedValueOnce(null) + + const { result } = renderHook(() => useRepoSlugIndex()) + await act(async () => void (await vi.advanceTimersByTimeAsync(0))) + expect(result.current.ready).toBe(true) + expect(result.current.lookupSlug('acme/widgets')).toEqual([]) + expect(repoSlug).toHaveBeenCalledTimes(1) + + repoSlug.mockResolvedValue({ owner: 'acme', repo: 'widgets' }) + await act(async () => void (await vi.advanceTimersByTimeAsync(REPO_SLUG_FAILURE_TTL_MS + 10))) + expect(repoSlug.mock.calls.length).toBeGreaterThan(1) + expect(result.current.lookupSlug('acme/widgets')).toEqual([repo]) + }) + + it('clears the pending retry timer on unmount', async () => { + vi.useFakeTimers() + setRepos([makeRepo({ id: 'flaky' })]) + // Why: a permanently failing resolution keeps a retry armed, so an + // uncleaned timer is observable after teardown. The hook mounts once per + // project row, so such a timer would leak per row. + repoSlug.mockResolvedValue(null) + + const { unmount } = renderHook(() => useRepoSlugIndex()) + await act(async () => void (await vi.advanceTimersByTimeAsync(0))) + const armedCount = vi.getTimerCount() + + unmount() + expect(vi.getTimerCount()).toBeLessThan(armedCount) + }) +}) diff --git a/src/renderer/src/lib/repo-slug-index.ts b/src/renderer/src/lib/repo-slug-index.ts index 1d5f0a7f172..d48dbb869f3 100644 --- a/src/renderer/src/lib/repo-slug-index.ts +++ b/src/renderer/src/lib/repo-slug-index.ts @@ -22,9 +22,11 @@ import { nextRepoSlugFailureRetryDelay, readRepoSlugCache, rememberRepoSlug, + repoUpstreamIdentityKey, settingsForRepoOwner, slugByRepoId, slugCacheKey, + type RepoSlugMatches, type SlugIndex } from './repo-slug-cache' import { githubRepoIdentityKey } from '../../../shared/github-repository-identity-key' @@ -126,7 +128,7 @@ async function resolveRepoSlug( async function buildIndex( repos: Repo[], settings: Pick | null | undefined -): Promise<{ index: SlugIndex; retryDelayMs: number | null }> { +): Promise<{ index: SlugIndex; upstreamIndex: SlugIndex; retryDelayMs: number | null }> { // Why: evict cached entries for repos that no longer exist in state so // the cache cannot grow unbounded across long sessions where users add // and remove repos. Without this, every removed repo's id (and its @@ -139,6 +141,7 @@ async function buildIndex( } } const next: SlugIndex = new Map() + const upstreamNext: SlugIndex = new Map() const results = await Promise.all( repos.map(async (r) => ({ repo: r, @@ -151,12 +154,26 @@ async function buildIndex( if (slug) { next.set(slug, [...(next.get(slug) ?? []), repo]) } + // Why: a Project card references the upstream repo, but a contributor's + // clone has their personal fork as `origin`, so the origin-only index + // dropped every row (#12647). `repo.upstream` is already resolved when the + // repo is added, so this costs no extra IPC. + const upstreamKey = repoUpstreamIdentityKey(repo, slug) + if (upstreamKey && upstreamKey !== slug) { + upstreamNext.set(upstreamKey, [...(upstreamNext.get(upstreamKey) ?? []), repo]) + } + } + return { + index: next, + upstreamIndex: upstreamNext, + retryDelayMs: nextRepoSlugFailureRetryDelay(liveKeys) } - return { index: next, retryDelayMs: nextRepoSlugFailureRetryDelay(liveKeys) } } export type RepoSlugIndexState = { + /** Best available matches: origin when anything owns the slug, else forks. */ lookupSlug: (slug: string | null | undefined, host?: string) => Repo[] + lookupSlugMatches: (slug: string | null | undefined, host?: string) => RepoSlugMatches ready: boolean } @@ -167,45 +184,67 @@ export function useRepoSlugIndex(): RepoSlugIndexState { const repos = useAppStore((s) => s.repos) const settings = useAppStore((s) => s.settings) const [index, setIndex] = useState(() => new Map()) + const [upstreamIndex, setUpstreamIndex] = useState(() => new Map()) const [ready, setReady] = useState(false) const [retryGeneration, setRetryGeneration] = useState(0) + // Why: schedule retry in a dedicated effect so setTimeout cleanup is owned + // synchronously (react-doctor effect-needs-cleanup); async .then assignment + // was not statically owned by the buildIndex effect cleanup. + const [retryDelayMs, setRetryDelayMs] = useState(null) // Why: track the current repos snapshot so the effect can ignore stale // resolutions when repos change mid-flight. const generationRef = useRef(0) useEffect(() => { const gen = ++generationRef.current - let retryTimer: ReturnType | undefined setReady(false) - void buildIndex(repos, settings).then(({ index: next, retryDelayMs }) => { - if (gen !== generationRef.current) { - return + setRetryDelayMs(null) + void buildIndex(repos, settings).then( + ({ index: next, upstreamIndex: nextUpstream, retryDelayMs: nextRetryDelayMs }) => { + if (gen !== generationRef.current) { + return + } + setIndex(next) + setUpstreamIndex(nextUpstream) + setReady(true) + setRetryDelayMs(nextRetryDelayMs) } - setIndex(next) - setReady(true) - if (retryDelayMs !== null) { - retryTimer = setTimeout(() => setRetryGeneration((value) => value + 1), retryDelayMs) - } - }) + ) return () => { generationRef.current += 1 - if (retryTimer) { - clearTimeout(retryTimer) - } } }, [repos, retryGeneration, settings]) - return useMemo( - () => ({ + useEffect(() => { + if (retryDelayMs === null) { + return + } + const retryTimer = setTimeout(() => setRetryGeneration((value) => value + 1), retryDelayMs) + return () => { + clearTimeout(retryTimer) + } + }, [retryDelayMs]) + + return useMemo(() => { + const lookupSlugMatches = (slug: string | null | undefined, host?: string): RepoSlugMatches => { + const [owner, repo] = slug?.split('/') ?? [] + if (!owner || !repo) { + return { origin: [], upstream: [] } + } + const key = githubRepoIdentityKey({ owner, repo, host }) + return { origin: index.get(key) ?? [], upstream: upstreamIndex.get(key) ?? [] } + } + return { + lookupSlugMatches, + // Why: origin wins — when the upstream repo itself is open, a row must + // resolve to that clone rather than becoming ambiguous with someone's + // fork of it. Callers that also filter by selection use + // `lookupSlugMatches` so an unselected clone cannot hide a selected fork. lookupSlug: (slug: string | null | undefined, host?: string): Repo[] => { - const [owner, repo] = slug?.split('/') ?? [] - if (!owner || !repo) { - return [] - } - return index.get(githubRepoIdentityKey({ owner, repo, host })) ?? [] + const { origin, upstream } = lookupSlugMatches(slug, host) + return origin.length > 0 ? origin : upstream }, ready - }), - [index, ready] - ) + } + }, [index, upstreamIndex, ready]) } diff --git a/src/shared/github-repository-identity-key.ts b/src/shared/github-repository-identity-key.ts index df9b8152982..ac42fcd8ad6 100644 --- a/src/shared/github-repository-identity-key.ts +++ b/src/shared/github-repository-identity-key.ts @@ -17,3 +17,12 @@ export function githubRepoIdentityKey(repo: { const host = repo.host?.trim().toLowerCase() return host && !isDefaultGitHubHost(host) ? `${host}/${slug}` : slug } + +// Why: callers that only kept the key (not the identity it came from) still need +// its host segment to scope a second, host-less identity into the same namespace. +// `owner` and `repo` never contain `/`, so a three-segment key is host-qualified. +// `undefined` means github.com, so never pass a key that may be unresolved. +export function githubHostFromIdentityKey(key: string | null | undefined): string | undefined { + const segments = key?.split('/') ?? [] + return segments.length === 3 ? segments[0] : undefined +}