diff --git a/src/main/git/task-repository-eligibility.ts b/src/main/git/task-repository-eligibility.ts new file mode 100644 index 00000000000..97fb829ab46 --- /dev/null +++ b/src/main/git/task-repository-eligibility.ts @@ -0,0 +1,51 @@ +import { parseGitRemoteVerboseOutput } from '../../shared/git-remote-identity' +import { isDefaultGitHubHost } from '../../shared/github/repository-identity-key' +import { isGitHubHostAuthenticated } from '../github/github-enterprise-repository' +import { parseGitHubRemoteIdentity } from '../github/github-remote-identity-parsing' +import { parseGitLabProjectRef } from '../gitlab/project-ref-parser' +import { REMOTE_URL_PROBE_TIMEOUT_MS } from './remote-url-probe' +import { gitExecFileAsync } from './runner' +import type { RemoteRefLocalGitOptions } from './remote-ref-probe-cache' + +/** Call only after the requested provider found no usable source; unknown is never exclusion evidence. */ +export async function isTaskRepositoryIneligible( + repoPath: string, + provider: 'github' | 'gitlab', + connectionId?: string | null, + localGitOptions: RemoteRefLocalGitOptions = {} +): Promise { + if (connectionId) { + return false + } + try { + // Read the complete current set: a cached primary remote cannot rule out a matching upstream or extra remote. + const { stdout } = await gitExecFileAsync(['remote', '-v'], { + cwd: repoPath, + timeout: REMOTE_URL_PROBE_TIMEOUT_MS, + ...localGitOptions + }) + const remotes = parseGitRemoteVerboseOutput(stdout) + if (remotes.length === 0) { + return false + } + for (const remote of remotes) { + if (provider === 'github') { + if (!parseGitLabProjectRef(remote.url)) { + return false + } + } else { + const identity = parseGitHubRemoteIdentity(remote.url) + if ( + !identity || + (!isDefaultGitHubHost(identity.host) && + !(await isGitHubHostAuthenticated(identity.host, repoPath, null, localGitOptions))) + ) { + return false + } + } + } + return true + } catch { + return false + } +} diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index 68816d867f5..4d2fa394b7f 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -163,6 +163,32 @@ describe('listWorkItems', () => { expect(calls.every(([, options]) => options?.host === 'github.acme-corp.com')).toBe(true) }) + it('marks proven GitLab remotes without listing them', async () => { + getIssueOwnerRepoMock.mockResolvedValue(null) + getOwnerRepoMock.mockResolvedValue(null) + gitExecFileAsyncMock.mockImplementation(async (args: string[]) => ({ + stdout: args[1] === '-v' ? 'origin\tgit@gitlab.com:team/project.git (fetch)\n' : 'origin\n' + })) + + const result = await listWorkItems('/repo', 24) + expect(result.sources).toMatchObject({ issues: null, prs: null, ineligible: true }) + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + }) + + it.each([ + 'origin\tgit@custom.example.com:team/project.git (fetch)\n', + 'origin\tgit@gitlab.com:team/project.git (fetch)\nupstream\tgit@github.com:team/project.git (fetch)\n' + ])('keeps unproven or mixed remote identity unresolved: %s', async (stdout) => { + getIssueOwnerRepoMock.mockResolvedValue(null) + getOwnerRepoMock.mockResolvedValue(null) + gitExecFileAsyncMock.mockResolvedValue({ stdout }) + + const result = await listWorkItems('/repo', 24) + + expect(result.sources.ineligible).toBeUndefined() + expect(ghExecFileAsyncMock).not.toHaveBeenCalled() + }) + it('does not run unscoped work-item queries when a local repository has no GitHub source', async () => { getIssueOwnerRepoMock.mockResolvedValue(null) getOwnerRepoMock.mockResolvedValue(null) diff --git a/src/main/github/client/list/list-work-items.ts b/src/main/github/client/list/list-work-items.ts index 2d4308cdeb1..836bdb1d9df 100644 --- a/src/main/github/client/list/list-work-items.ts +++ b/src/main/github/client/list/list-work-items.ts @@ -7,6 +7,7 @@ import { resolveIssueGitHubApiRepositorySource } from '../../github-api-reposito import type { MainWorkItem } from './../map/work-item-field-coercion' import { normalizeWorkItemPage, resolvePrWorkItemSource } from './work-item-list-request' import { listRecentWorkItems, listQueriedWorkItems } from './work-item-pages' +import { isTaskRepositoryIneligible } from '../../../git/task-repository-eligibility' export async function listWorkItems( repoPath: string, limit = 24, @@ -36,6 +37,10 @@ export async function listWorkItems( ]) const issueOwnerRepo = issueResolved.source const prOwnerRepo = prResolved.source + const ineligible = + !issueOwnerRepo && + !prOwnerRepo && + (await isTaskRepositoryIneligible(repoPath, 'github', connectionId, localGitOptions)) await acquire() try { // Preserve fetch failures for IPC; an empty fallback would hide per-repo outages. @@ -75,7 +80,8 @@ export async function listWorkItems( issues: issueOwnerRepo, prs: prOwnerRepo, originCandidate: prResolved.originCandidate, - upstreamCandidate: prResolved.upstreamCandidate + upstreamCandidate: prResolved.upstreamCandidate, + ...(ineligible ? { ineligible: true as const } : {}) }, ...(errors ? { errors } : {}), ...(issueResolved.fellBack ? { issueSourceFellBack: true } : {}) diff --git a/src/main/gitlab/client-mr-listing.test.ts b/src/main/gitlab/client-mr-listing.test.ts index 515814c5e2b..3c1d1292b7e 100644 --- a/src/main/gitlab/client-mr-listing.test.ts +++ b/src/main/gitlab/client-mr-listing.test.ts @@ -251,6 +251,22 @@ describe('gitlab client — MR operations', () => { expect((await listMergeRequests('/repo', 'all', 1, 2)).totalPages).toBe(5) }) + it('retains MR inference when a known-other remote has an unresolved companion', async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ source: null, fellBack: false }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: + 'origin\tgit@github.com:team/project.git (fetch)\nupstream\t/local/project.git (fetch)\n' + }) + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + const result = await listMergeRequests('/repo') + + expect(result.error).toBeUndefined() + expect(glabExecFileAsyncMock).toHaveBeenCalledWith(expect.arrayContaining(['mr', 'list']), { + cwd: '/repo' + }) + }) + it('falls back to glab mr list when project ref is unresolved', async () => { resolveIssueSourceMock.mockResolvedValueOnce({ source: null, diff --git a/src/main/gitlab/gitlab-issue-source-resolution.ts b/src/main/gitlab/gitlab-issue-source-resolution.ts index e4ec90fa6a6..b7f5da4b1d5 100644 --- a/src/main/gitlab/gitlab-issue-source-resolution.ts +++ b/src/main/gitlab/gitlab-issue-source-resolution.ts @@ -1,7 +1,8 @@ import { listCachedRemoteNames, shouldProbeGitRemote } from '../git/remote-name-listing' import type { IssueSourcePreference } from '../../shared/repo-types' +import { readLocalGitConfigSignature } from '../github/local-git-config-signature' import { - getProjectRefForRemote, + getProjectRefForRemoteWithSignature, type LocalGitExecOptions, type ProjectRef } from './gitlab-project-ref-resolution' @@ -12,7 +13,24 @@ export async function getIssueProjectRef( connectionId?: string | null, localGitOptions: LocalGitExecOptions = {} ): Promise { - const originPromise = getProjectRefForRemote( + return getIssueProjectRefWithSignature( + readLocalGitConfigSignature({ repoPath, connectionId, ...localGitOptions }), + repoPath, + knownHosts, + connectionId, + localGitOptions + ) +} + +async function getIssueProjectRefWithSignature( + initialSignature: Promise, + repoPath: string, + knownHosts?: readonly string[], + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const originPromise = getProjectRefForRemoteWithSignature( + initialSignature, repoPath, 'origin', knownHosts, @@ -20,7 +38,8 @@ export async function getIssueProjectRef( localGitOptions ) if (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions)) { - const upstream = await getProjectRefForRemote( + const upstream = await getProjectRefForRemoteWithSignature( + initialSignature, repoPath, 'upstream', knownHosts, @@ -33,12 +52,19 @@ export async function getIssueProjectRef( } return ( (await originPromise) ?? - getProjectRefForOtherRemote(repoPath, knownHosts, connectionId, localGitOptions) + getProjectRefForOtherRemote( + initialSignature, + repoPath, + knownHosts, + connectionId, + localGitOptions + ) ) } // Match glab project inference even when remotes use nonstandard names. async function getProjectRefForOtherRemote( + initialSignature: Promise, repoPath: string, knownHosts?: readonly string[], connectionId?: string | null, @@ -49,7 +75,8 @@ async function getProjectRefForOtherRemote( // glab prioritizes a remote named gitlab. otherRemotes.sort((a, b) => Number(b === 'gitlab') - Number(a === 'gitlab')) for (const remoteName of otherRemotes) { - const projectRef = await getProjectRefForRemote( + const projectRef = await getProjectRefForRemoteWithSignature( + initialSignature, repoPath, remoteName, knownHosts, @@ -76,8 +103,14 @@ export async function resolveIssueSource( connectionId?: string | null, localGitOptions: LocalGitExecOptions = {} ): Promise { + const initialSignature = readLocalGitConfigSignature({ + repoPath, + connectionId, + ...localGitOptions + }) if (preference === 'upstream') { - const upstream = await getProjectRefForRemote( + const upstream = await getProjectRefForRemoteWithSignature( + initialSignature, repoPath, 'upstream', knownHosts, @@ -87,7 +120,8 @@ export async function resolveIssueSource( if (upstream) { return { source: upstream, fellBack: false } } - const origin = await getProjectRefForRemote( + const origin = await getProjectRefForRemoteWithSignature( + initialSignature, repoPath, 'origin', knownHosts, @@ -98,7 +132,8 @@ export async function resolveIssueSource( } if (preference === 'origin') { return { - source: await getProjectRefForRemote( + source: await getProjectRefForRemoteWithSignature( + initialSignature, repoPath, 'origin', knownHosts, @@ -109,7 +144,13 @@ export async function resolveIssueSource( } } return { - source: await getIssueProjectRef(repoPath, knownHosts, connectionId, localGitOptions), + source: await getIssueProjectRefWithSignature( + initialSignature, + repoPath, + knownHosts, + connectionId, + localGitOptions + ), fellBack: false } } diff --git a/src/main/gitlab/gitlab-partial-auth-recovery.test.ts b/src/main/gitlab/gitlab-partial-auth-recovery.test.ts index 4f716c64f44..d8072a4c4cc 100644 --- a/src/main/gitlab/gitlab-partial-auth-recovery.test.ts +++ b/src/main/gitlab/gitlab-partial-auth-recovery.test.ts @@ -1,7 +1,13 @@ import { afterEach, beforeEach, expect, it, vi } from 'vitest' import type * as Ansi from '../../shared/ansi-escape-sequences' -const { git, execute, scan } = vi.hoisted(() => ({ git: vi.fn(), execute: vi.fn(), scan: vi.fn() })) +const { git, execute, scan, signature } = vi.hoisted(() => ({ + git: vi.fn(), + execute: vi.fn(), + scan: vi.fn(), + signature: vi.fn() +})) vi.mock('../git/runner', () => ({ gitExecFileAsync: git, glabExecFileAsync: execute })) +vi.mock('../github/local-git-config-signature', () => ({ readLocalGitConfigSignature: signature })) vi.mock('../../shared/ansi-escape-sequences', async (original) => { const actual = await original() return { @@ -33,12 +39,24 @@ const counts = () => [ execute.mock.calls.filter(([a]) => a.length === 2).length, execute.mock.calls.filter(([a]) => a[2] === '--hostname').length ] +const urlReads = () => git.mock.calls.filter(([args]) => args[1] === 'get-url').length +let remoteUrl: string let globalError: Error | undefined let hostError: Error | undefined beforeEach(() => { _resetKnownHostsCache() _resetProjectRefCache() - git.mockReset().mockResolvedValue({ stdout: 'https://alpha.test:8443/group/project.git\n' }) + signature.mockReset().mockResolvedValue('sig-1') + remoteUrl = 'https://alpha.test:8443/group/project.git' + git.mockReset().mockImplementation(async (args: string[]) => { + if (args[0] === 'remote' && args[1] === 'get-url') { + return { stdout: remoteUrl } + } + if (args[0] === 'remote' && args[1] === '-v') { + return { stdout: `origin\t${remoteUrl} (fetch)\n` } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) scan.mockClear() vi.spyOn(Date, 'now').mockReturnValue(1_000) globalError = failure(login('alpha.test:8443')) @@ -58,7 +76,7 @@ afterEach(() => vi.restoreAllMocks()) it.each([false, true])( 'preserves cached misses and current remote, late facts=%s', async (late) => { - git.mockResolvedValue({ stdout: 'https://beta.test/group/project.git\n' }) + remoteUrl = 'https://beta.test/group/project.git' expect((await fetchIssues()).error?.type).toBe('not_found') vi.mocked(Date.now).mockReturnValue(15_999) expect((await fetchIssues()).error?.type).toBe('not_found') @@ -73,15 +91,28 @@ it.each([false, true])( ) } else { expect(result.error?.type).toBe('not_found') - expect(git).toHaveBeenCalledTimes(1) - git.mockResolvedValue({ stdout: 'https://changed.test/group/project.git\n' }) + expect(urlReads()).toBe(1) + remoteUrl = 'https://changed.test/group/project.git' vi.mocked(Date.now).mockReturnValue(1_000 + NEGATIVE_ENTRY_TTL_MS) expect((await fetchIssues()).error?.type).toBe('not_found') - expect(git).toHaveBeenCalledTimes(2) + expect(urlReads()).toBe(2) } expect(counts()).toEqual(late ? [3, 2] : [2, 1]) } ) +it('applies late partial facts only to the current unsigned remote', async () => { + signature.mockResolvedValue(undefined) + remoteUrl = 'https://beta.test/group/project.git' + expect((await fetchIssues()).error?.type).toBe('not_found') + vi.mocked(Date.now).mockReturnValue(16_000) + globalError = failure(login('beta.test')) + remoteUrl = 'https://changed.test/group/project.git' + expect((await fetchIssues()).error?.type).toBe('not_found') + remoteUrl = 'https://beta.test/group/project.git' + expect((await fetchIssues()).items[0]?.number).toBe(7) + expect(urlReads()).toBe(3) + expect(counts()).toEqual([2, 2]) +}) it.each([100, 1_000])( 'bounds global/project work and finite evidence at %i consumers/hosts', async (n) => { diff --git a/src/main/gitlab/gitlab-project-ref-resolution.ts b/src/main/gitlab/gitlab-project-ref-resolution.ts index 77b2794d56c..03d9773a853 100644 --- a/src/main/gitlab/gitlab-project-ref-resolution.ts +++ b/src/main/gitlab/gitlab-project-ref-resolution.ts @@ -3,6 +3,7 @@ import type { GitAdmissionTier } from '../git/command-runner/git-exec-options' import { addWslEnvKeys } from '../wsl-env' import { isTransientGitProbeError, readRemoteUrl } from '../git/remote-url-probe' import { NEGATIVE_ENTRY_TTL_MS } from '../git/remote-ref-probe-cache' +import { readLocalGitConfigSignature } from '../github/local-git-config-signature' import { clearProjectRefInFlight, runProjectRefProbeOnce } from './project-ref-inflight' import { _resetGlabUnauthenticatedHosts, @@ -32,7 +33,7 @@ export type { LocalGitExecOptions } from './gitlab-known-host-probe' const PROJECT_REF_CACHE_MAX_ENTRIES = 512 -type CachedProjectRef = { value: ProjectRef | null; expiresAt: number } +type CachedProjectRef = { value: ProjectRef | null; expiresAt: number; configSignature?: string } const projectRefCache = new Map() @@ -51,12 +52,13 @@ export function _getProjectRefCacheSize(): number { function rememberProjectRefCacheEntry( cacheKey: string, value: ProjectRef | null, - finite = value === null + configSignature?: string ): void { - // Negative forge identity expires so remote/auth configuration changes can be discovered. + // Git config signatures omit global Git, SSH and auth configuration, so signed hits also expire. projectRefCache.set(cacheKey, { value, - expiresAt: finite ? Date.now() + NEGATIVE_ENTRY_TTL_MS : Number.POSITIVE_INFINITY + expiresAt: Date.now() + NEGATIVE_ENTRY_TTL_MS, + ...(configSignature !== undefined ? { configSignature } : {}) }) while (projectRefCache.size > PROJECT_REF_CACHE_MAX_ENTRIES) { const oldestKey = projectRefCache.keys().next().value @@ -73,19 +75,42 @@ export async function getProjectRefForRemote( knownHosts: readonly string[] = DEFAULT_GITLAB_HOSTS, connectionId?: string | null, localGitOptions: LocalGitExecOptions = {} +): Promise { + return getProjectRefForRemoteWithSignature( + readLocalGitConfigSignature({ repoPath, connectionId, ...localGitOptions }), + repoPath, + remoteName, + knownHosts, + connectionId, + localGitOptions + ) +} + +export async function getProjectRefForRemoteWithSignature( + initialSignature: Promise, + repoPath: string, + remoteName: string, + knownHosts: readonly string[] = DEFAULT_GITLAB_HOSTS, + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} ): Promise { // Authenticated host inventory participates in cache identity so new logins invalidate old misses. const runtimeKey = connectionId ?? `local:${localGitOptions.wslDistro ?? 'host'}` const cacheKey = `${runtimeKey}\0${repoPath}\0${remoteName}\0${knownHosts.join(',')}` + const configSignature = await initialSignature const cached = projectRefCache.get(cacheKey) if (cached) { - if (cached.expiresAt > Date.now()) { + if ( + cached.expiresAt > Date.now() && + configSignature !== undefined && + cached.configSignature === configSignature + ) { return cached.value } projectRefCache.delete(cacheKey) } - return runProjectRefProbeOnce(cacheKey, (ownsKey) => + return runProjectRefProbeOnce(`${cacheKey}\0${configSignature ?? ''}`, (ownsKey) => resolveProjectRefForRemote( repoPath, remoteName, @@ -93,7 +118,8 @@ export async function getProjectRefForRemote( connectionId, cacheKey, ownsKey, - localGitOptions + localGitOptions, + configSignature ) ) } @@ -105,12 +131,13 @@ async function resolveProjectRefForRemote( connectionId: string | null | undefined, cacheKey: string, ownsKey: () => boolean, - localGitOptions: LocalGitExecOptions + localGitOptions: LocalGitExecOptions, + configSignature: string | undefined ): Promise { // Abandoned probes may answer their callers but must not overwrite newer cached identity. - const publish = (value: ProjectRef | null, finite = value === null): void => { + const publish = (value: ProjectRef | null): void => { if (ownsKey()) { - rememberProjectRefCacheEntry(cacheKey, value, finite) + rememberProjectRefCacheEntry(cacheKey, value, configSignature) } } try { @@ -150,8 +177,8 @@ async function resolveProjectRefForRemote( getGlabPartialHosts(connectionId, localGitOptions) ) if (partial) { - // Evidence from an unsettled discovery keeps the original miss expiry and never remembers auth. - publish(partial, true) + // Evidence from an unsettled discovery uses the same finite expiry and never remembers auth. + publish(partial) return partial } } catch (error) { diff --git a/src/main/gitlab/gl-utils.test.ts b/src/main/gitlab/gl-utils.test.ts index 64d3572c128..1a4c1c117c8 100644 --- a/src/main/gitlab/gl-utils.test.ts +++ b/src/main/gitlab/gl-utils.test.ts @@ -1,15 +1,22 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { gitExecFileAsyncMock, glabExecFileAsyncMock } = vi.hoisted(() => ({ - gitExecFileAsyncMock: vi.fn(), - glabExecFileAsyncMock: vi.fn() -})) +const { gitExecFileAsyncMock, glabExecFileAsyncMock, readLocalGitConfigSignatureMock } = vi.hoisted( + () => ({ + gitExecFileAsyncMock: vi.fn(), + glabExecFileAsyncMock: vi.fn(), + readLocalGitConfigSignatureMock: vi.fn() + }) +) vi.mock('../git/runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock, glabExecFileAsync: glabExecFileAsyncMock })) +vi.mock('../github/local-git-config-signature', () => ({ + readLocalGitConfigSignature: readLocalGitConfigSignatureMock +})) + import { _getProjectRefCacheSize, _resetProjectRefCache, @@ -71,6 +78,11 @@ describe('gitlab project ref resolution', () => { glabExecFileAsyncMock.mockReset() _resetProjectRefCache() _resetRemoteNameListingCache() + readLocalGitConfigSignatureMock + .mockReset() + .mockImplementation(async (context: { wslDistro?: string }) => + context.wslDistro ? undefined : 'sig-1' + ) }) afterEach(() => { @@ -92,17 +104,89 @@ describe('gitlab project ref resolution', () => { }) }) - it('prefers upstream for issue project ref resolution', async () => { + it.each([undefined, 'Ubuntu'])( + 'rechecks a warm GitLab identity after migration (%s)', + async (wslDistro) => { + const localGitOptions = wslDistro ? { wslDistro } : {} + mockGitRemoteCommands({ origin: 'git@gitlab.com:team/project.git\n' }) + await expect(getProjectRef('/repo', undefined, null, localGitOptions)).resolves.toEqual({ + host: 'gitlab.com', + path: 'team/project' + }) + + mockGitRemoteCommands({ origin: 'git@github.com:team/project.git\n' }) + if (!wslDistro) { + readLocalGitConfigSignatureMock.mockResolvedValue('sig-2') + } + + await expect(getProjectRef('/repo', undefined, null, localGitOptions)).resolves.toBeNull() + } + ) + + it.each(['sig-2', undefined])( + 'does not return a warm identity after freshness was lost (%s) and the probe timed out', + async (signature) => { + mockGitRemoteCommands({ origin: 'git@gitlab.com:team/project.git\n' }) + await expect(getProjectRef('/repo')).resolves.toEqual({ + host: 'gitlab.com', + path: 'team/project' + }) + gitExecFileAsyncMock.mockRejectedValue(new Error('git remote get-url timed out')) + readLocalGitConfigSignatureMock.mockResolvedValue(signature) + + await expect(getProjectRef('/repo')).resolves.toBeNull() + } + ) + + it.each(['before capture', 'after capture', 'late publication'])( + 'refreshes on the next independent signature after an edit (%s)', + async (ordering) => { + const captured = Promise.withResolvers() + const release = Promise.withResolvers<{ stdout: string }>() + const oldUrl = 'git@gitlab.com:team/old.git' + const newUrl = 'git@gitlab.com:team/current.git' + readLocalGitConfigSignatureMock.mockResolvedValue('sig-0') + gitExecFileAsyncMock + .mockImplementationOnce(() => { + captured.resolve() + return release.promise + }) + .mockResolvedValue({ stdout: newUrl }) + const old = getProjectRefForRemote('/repo', 'origin') + await captured.promise + readLocalGitConfigSignatureMock.mockResolvedValue('sig-1') + if (ordering === 'late publication') { + expect((await getProjectRefForRemote('/repo', 'origin'))?.path).toBe('team/current') + } + release.resolve({ stdout: ordering === 'before capture' ? newUrl : oldUrl }) + expect((await old)?.path).toBe(ordering === 'before capture' ? 'team/current' : 'team/old') + expect((await getProjectRefForRemote('/repo', 'origin'))?.path).toBe('team/current') + const reads = gitExecFileAsyncMock.mock.calls.length + expect(reads).toBe(ordering === 'late publication' ? 3 : 2) + expect((await getProjectRefForRemote('/repo', 'origin'))?.path).toBe('team/current') + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(reads) + } + ) + + it('returns upstream before a slow origin, retaining the late origin cache fill', async () => { mockGitRemoteCommands({ origin: 'git@gitlab.com:fork/orca.git\n', upstream: 'git@gitlab.com:stablyai/orca.git\n' }) + const origin = Promise.withResolvers<{ stdout: string }>() + const serve = gitExecFileAsyncMock.getMockImplementation() + gitExecFileAsyncMock.mockImplementation((args: string[]) => + args[1] === 'get-url' && args[2] === 'origin' ? origin.promise : serve?.(args) + ) await expect(getIssueProjectRef('/repo')).resolves.toEqual({ host: 'gitlab.com', path: 'stablyai/orca' }) expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1) + origin.resolve({ stdout: 'git@gitlab.com:fork/orca.git' }) + expect((await getProjectRef('/repo'))?.path).toBe('fork/orca') + expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1) }) it('does not spawn git remote get-url upstream on an origin-only clone', async () => { @@ -121,18 +205,25 @@ describe('gitlab project ref resolution', () => { expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1) }) - it('falls back to origin when upstream is present but non-GitLab', async () => { + it('falls back on an upstream timeout and retries upstream on the next attempt', async () => { mockGitRemoteCommands({ origin: 'git@gitlab.com:fork/orca.git\n', upstream: 'git@example.com:stablyai/orca.git\n' }) + const serve = gitExecFileAsyncMock.getMockImplementation() + gitExecFileAsyncMock.mockImplementation((args: string[]) => + args[2] === 'upstream' ? Promise.reject(new Error('git timed out')) : serve?.(args) + ) await expect(getIssueProjectRef('/repo')).resolves.toEqual({ host: 'gitlab.com', path: 'fork/orca' }) expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(1) expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1) + mockGitRemoteCommands({ upstream: 'git@gitlab.com:team/recovered.git' }) + expect((await getIssueProjectRef('/repo'))?.path).toBe('team/recovered') + expect(gitRemoteGetUrlCalls('upstream')).toHaveLength(2) }) it('finds the GitLab project on other remotes, preferring one named gitlab like glab', async () => { @@ -177,7 +268,7 @@ describe('gitlab project ref resolution', () => { it('keeps local host and local WSL project-ref cache entries separate for the same path', async () => { gitExecFileAsyncMock .mockResolvedValueOnce({ stdout: 'git@gitlab.com:host/orca.git\n' }) - .mockResolvedValueOnce({ stdout: 'git@gitlab.com:wsl/orca.git\n' }) + .mockResolvedValue({ stdout: 'git@gitlab.com:wsl/orca.git\n' }) await expect(getProjectRef('/repo')).resolves.toEqual({ host: 'gitlab.com', @@ -196,7 +287,7 @@ describe('gitlab project ref resolution', () => { } ) - expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(3) expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['remote', 'get-url', 'origin'], { cwd: '/repo', timeout: REMOTE_URL_PROBE_TIMEOUT_MS @@ -279,7 +370,7 @@ describe('gitlab project ref resolution', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) }) - it('keeps a resolved project ref past the negative interval', async () => { + it('periodically revalidates a signed project ref for external configuration changes', async () => { vi.useFakeTimers() vi.setSystemTime(1_000_000) gitExecFileAsyncMock.mockResolvedValue({ stdout: 'git@gitlab.com:fork/orca.git\n' }) @@ -294,7 +385,7 @@ describe('gitlab project ref resolution', () => { path: 'fork/orca' }) - expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) }) it('re-resolves a self-hosted remote once glab auth knows its host', async () => { diff --git a/src/main/gitlab/issues.ts b/src/main/gitlab/issues.ts index 20c7717743b..bda905776b8 100644 --- a/src/main/gitlab/issues.ts +++ b/src/main/gitlab/issues.ts @@ -5,6 +5,7 @@ import { mapGitLabIssueInfo } from './mappers' // prettier-ignore import { glabApiWithHeaders, glabExecFileAsync, acquire, release, getIssueProjectRef, resolveIssueSource, classifyGlabError, classifyListFetchError, getGlabKnownHosts, glabRepoExecOptions, glabHostnameArgs, parseGlabJsonList, parseGlabPaginationHeader, type LocalGitExecOptions, type ProjectRef } from './gl-utils' import { encodedProject } from './project-path-encoding' +import { isTaskRepositoryIneligible } from '../git/task-repository-eligibility' import { codeHostFailure, classifiedFailure } from '../../shared/code-host-failure-copy' // Distinguish a confirmed empty list from fetch failure. @@ -72,6 +73,9 @@ export async function listIssues( ) // Isolate unresolved projects instead of letting cwd inference fail the cross-project aggregate. if (!projectRef) { + if (await isTaskRepositoryIneligible(repoPath, 'gitlab', connectionId, localGitOptions)) { + return { items: [], totalPages: 0 } + } return { items: [], totalPages: 0, diff --git a/src/main/gitlab/merge-request-list.ts b/src/main/gitlab/merge-request-list.ts index 0251ef0cd1b..3bfa147f3c1 100644 --- a/src/main/gitlab/merge-request-list.ts +++ b/src/main/gitlab/merge-request-list.ts @@ -16,6 +16,7 @@ import { type LocalGitExecOptions } from './gl-utils' import { encodedProject } from './project-path-encoding' +import { isTaskRepositoryIneligible } from '../git/task-repository-eligibility' function mrListStateFlags(state: MRListState): string[] { switch (state) { @@ -50,6 +51,9 @@ export async function listMergeRequests( localGitOptions ) if (!projectRef) { + if (await isTaskRepositoryIneligible(repoPath, 'gitlab', connectionId, localGitOptions)) { + return { items: [], page, perPage, totalCount: 0, totalPages: 0 } + } if (connectionId) { // Without local cwd, an unresolved SSH project could become an unrelated glab repository. return { diff --git a/src/main/gitlab/task-project-eligibility.test.ts b/src/main/gitlab/task-project-eligibility.test.ts new file mode 100644 index 00000000000..f7e45494d47 --- /dev/null +++ b/src/main/gitlab/task-project-eligibility.test.ts @@ -0,0 +1,179 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtemp, mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises' +import type * as FileSystem from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +vi.mock('node:fs/promises', async (importOriginal) => { + const fs = await importOriginal() + return { ...fs, readFile: vi.fn(fs.readFile), stat: vi.fn(fs.stat) } +}) + +const { git, glab, gh } = vi.hoisted(() => ({ git: vi.fn(), glab: vi.fn(), gh: vi.fn() })) + +vi.mock('../git/runner', () => ({ + gitExecFileAsync: git, + glabExecFileAsync: glab, + ghExecFileAsync: gh +})) + +import { listMergeRequests } from './merge-request-list' +import { listIssues } from './issues' +import { _resetProjectRefCache, _resetKnownHostsCache } from './gitlab-project-ref-resolution' +import { _resetRemoteNameListingCache } from '../git/remote-name-listing' +import { resolveIssueSource } from './gitlab-issue-source-resolution' + +function serveRemotes(repos: Record>): void { + git.mockImplementation(async (args: string[], options: { cwd: string }) => { + const remotes = repos[options.cwd] ?? {} + if (args[0] === 'remote' && args.length === 1) { + return { stdout: Object.keys(remotes).join('\n') } + } + if (args[0] === 'remote' && args[1] === '-v') { + return { + stdout: Object.entries(remotes) + .map(([name, url]) => `${name}\t${url} (fetch)`) + .join('\n') + } + } + if (args[0] === 'remote' && args[1] === 'get-url' && remotes[args[2]]) { + return { stdout: remotes[args[2]] } + } + throw new Error(`No such remote: ${args.join(' ')}`) + }) +} + +describe('GitLab task project admission', () => { + beforeEach(() => { + vi.resetAllMocks() + _resetProjectRefCache() + _resetKnownHostsCache() + _resetRemoteNameListingCache() + gh.mockResolvedValue({ stdout: '' }) + serveRemotes({ + '/gitlab': { origin: 'git@gitlab.com:team/project.git' }, + '/github': { origin: 'git@github.com:team/project.git' } + }) + glab.mockImplementation(async (args: string[]) => { + if (args[0] === 'auth') { + if (args.includes('github.com')) { + throw new Error('No GitLab account for github.com') + } + return { stdout: 'gitlab.com\n ✓ Logged in to gitlab.com\n' } + } + if (args[0] === 'api') { + return { stdout: 'HTTP/2.0 200 OK\r\nX-Total: 0\r\nX-Total-Pages: 1\r\n\r\n[]' } + } + throw new Error('None of the git remotes correspond to the GITLAB_HOST environment variable') + }) + }) + + it.each(['mrs', 'issues'] as const)( + 'keeps an eligible empty %s list clean when the shared selection also contains GitHub', + async (view) => { + const list = view === 'mrs' ? listMergeRequests : listIssues + const results = await Promise.all(['/gitlab', '/github'].map((repo) => list(repo))) + for (const result of results) { + expect(result.items).toEqual([]) + expect(result.error).toBeUndefined() + } + const listCalls = glab.mock.calls.filter(([args]) => args[0] !== 'auth') + expect(listCalls).toHaveLength(1) + expect(listCalls[0][0][0]).toBe('api') + expect(listCalls[0][1]).toMatchObject({ cwd: '/gitlab' }) + } + ) + + it.each(['upstream', 'gitlab'])( + 'keeps a matching %s remote when origin is hosted on another forge', + async (remoteName) => { + serveRemotes({ + '/mixed': { + origin: 'git@github.com:team/fork.git', + [remoteName]: 'git@gitlab.com:group/project.git' + } + }) + + const result = await listMergeRequests('/mixed') + + expect(result.error).toBeUndefined() + expect(glab).toHaveBeenCalledWith( + ['api', '-i', expect.stringContaining('projects/group%2Fproject/merge_requests?')], + { cwd: '/mixed' } + ) + } + ) + + it('never queries the old GitLab project after a warm WSL remote migrates to GitHub', async () => { + serveRemotes({ '/migrating': { origin: 'git@gitlab.com:team/project.git' } }) + const options = { wslDistro: 'Ubuntu' } + await listMergeRequests('/migrating', 'opened', 1, 20, undefined, undefined, null, options) + glab.mockClear() + serveRemotes({ '/migrating': { origin: 'git@github.com:team/project.git' } }) + + const result = await listMergeRequests( + '/migrating', + 'opened', + 1, + 20, + undefined, + undefined, + null, + options + ) + + expect(result.error).toBeUndefined() + expect(result.items).toEqual([]) + expect(glab.mock.calls.filter(([args]) => args[0] !== 'auth')).toEqual([]) + }) + + it.each([1, 3, 30])('bounds actual config traversal cold/warm at %i remotes', async (count) => { + const repo = await mkdtemp(join(tmpdir(), 'gitlab-signature-')) + try { + const remotes = Object.fromEntries( + Array.from({ length: count }, (_, i) => [ + i ? `fork-${i}` : 'origin', + `https://${count === 1 ? 'gitlab' : 'github'}.com/team/p${i}.git` + ]) + ) + const config = Object.entries(remotes) + .map(([name, url]) => `[remote "${name}"]\nurl=${url}\n`) + .join('') + await mkdir(join(repo, '.git')) + await writeFile(join(repo, '.git/config'), config) + serveRemotes({ [repo]: remotes }) + for (const warm of [false, true]) { + vi.mocked(readFile).mockClear() + vi.mocked(stat).mockClear() + git.mockClear() + glab.mockClear() + const { source } = await resolveIssueSource(repo, 'auto') + expect(source?.path ?? null).toBe(count === 1 ? 'team/p0' : null) + // Identity shares the initial observation; unchanged name discovery adds constant traversals. + const traversals = count === 1 ? (warm ? 1 : 2) : warm ? 2 : 3 + const reads = vi + .mocked(readFile) + .mock.calls.filter(([path]) => path === join(repo, '.git/config')).length + expect([ + reads, + vi.mocked(readFile).mock.calls.length, + vi.mocked(stat).mock.calls.length + ]).toEqual([traversals, traversals * 2, traversals * 3]) + const contents = await Promise.all( + vi + .mocked(readFile) + .mock.results.map(({ value }) => Promise.resolve(value).catch(() => '')) + ) + const bytes = contents.reduce((sum, text) => sum + Buffer.byteLength(text), 0) + expect(bytes).toBe(traversals * Buffer.byteLength(config)) + expect(git.mock.calls.filter(([args]) => args[1] === 'get-url')).toHaveLength( + warm ? 0 : count + ) + expect(git.mock.calls.filter(([args]) => args.length === 1)).toHaveLength(warm ? 0 : 1) + expect(glab).toHaveBeenCalledTimes(count === 1 || warm ? 0 : 1) + } + } finally { + await rm(repo, { recursive: true, force: true }) + } + }) +}) diff --git a/src/renderer/src/components/task-page/task-page-cache-selectors.test.ts b/src/renderer/src/components/task-page/task-page-cache-selectors.test.ts index ec0ee7fe04b..7f02609b08f 100644 --- a/src/renderer/src/components/task-page/task-page-cache-selectors.test.ts +++ b/src/renderer/src/components/task-page/task-page-cache-selectors.test.ts @@ -141,6 +141,77 @@ describe('task page cache selectors', () => { ]) }) + it('silently omits a proven other-forge source while keeping unknown identity retryable', () => { + const repos = [ + { id: 'gitlab-only', path: '/repos/gitlab-only' }, + { id: 'unknown', path: '/repos/unknown' } + ] + const entries = [ + { + data: [], + fetchedAt: 1, + sources: { + issues: null, + prs: null, + originCandidate: null, + upstreamCandidate: null, + ineligible: true as const + } + }, + { + data: [], + fetchedAt: 1, + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + } + ] + + const sourceState = buildTaskPageRepoSourceState(repos, entries) + + expect(selectTaskPageUnresolvedSourceRepos(repos, sourceState)).toEqual([ + { repoId: 'unknown', sourceKey: 'unknown::local', label: '/repos/unknown' } + ]) + }) + + it('keeps an unknown twin retryable while omitting only its proven other-forge source', () => { + const unknown = { id: 'twin', path: '/same/path', displayName: 'unknown twin' } + const sources = { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + const unknownEntry = { data: [], fetchedAt: 1, sources } + const ineligibleEntry = { + data: [], + fetchedAt: 1, + sources: { ...sources, ineligible: true as const } + } + const pairedScopes = [ + { repoFields: { executionHostId: 'runtime:paired' }, scope: 'runtime:paired' }, + { repoFields: { connectionId: 'devbox' }, scope: 'ssh:devbox' }, + { + repoFields: { sourceCacheScope: 'source:local:github:acme/widgets' }, + scope: 'source:local:github:acme/widgets' + } + ] + + for (const { repoFields, scope } of pairedScopes) { + const provenOther = { ...unknown, ...repoFields, displayName: 'proven other-forge twin' } + const cache = { + [workItemsCacheKey('twin', 20, '', 'local')]: unknownEntry, + [workItemsCacheKey('twin', 20, '', scope)]: ineligibleEntry + } + for (const repos of [ + [unknown, provenOther], + [provenOther, unknown] + ]) { + const entries = selectTaskPageWorkItemsCacheEntries(cache, repos, 20, '') + const sourceState = buildTaskPageRepoSourceState(repos, entries) + const expected = [{ repoId: 'twin', sourceKey: 'twin::local', label: 'unknown twin' }] + + expect(selectTaskPageUnresolvedSourceRepos(repos, sourceState)).toEqual(expected) + expect(selectTaskPageUnresolvedSourceRepos(repos, sourceState.toReversed())).toEqual( + expected + ) + } + } + }) + it('does not flag an unresolved-source repo that already carries a per-repo error', () => { const repos = [{ id: 'errored', path: '/repos/errored', displayName: 'errored-repo' }] const erroredEntry = { diff --git a/src/renderer/src/components/task-page/task-page-cache-selectors.ts b/src/renderer/src/components/task-page/task-page-cache-selectors.ts index 92a732c1571..7897c5f7bde 100644 --- a/src/renderer/src/components/task-page/task-page-cache-selectors.ts +++ b/src/renderer/src/components/task-page/task-page-cache-selectors.ts @@ -97,14 +97,20 @@ export type TaskPageUnresolvedSourceRepo = { /** Surface fetched both-null sources as unresolved; null means unfetched, and a non-null source can have genuine zero results. */ export function selectTaskPageUnresolvedSourceRepos( - repos: readonly { id: string; displayName?: string; path: string }[], + repos: readonly (TaskPageRepoCacheInput & { displayName?: string })[], sourceState: readonly TaskPageRepoSourceState[] ): TaskPageUnresolvedSourceRepo[] { - const stateByRepoId = new Map(sourceState.map((state) => [state.repoId, state])) + const stateBySourceKey = new Map(sourceState.map((state) => [state.sourceKey, state])) const unresolved: TaskPageUnresolvedSourceRepo[] = [] for (const repo of repos) { - const state = stateByRepoId.get(repo.id) - if (state?.sources && !state.sources.issues && !state.sources.prs && !state.error) { + const state = stateBySourceKey.get(`${repo.id}::${taskPageRepoCacheScope(repo)}`) + if ( + state?.sources && + !state.sources.ineligible && + !state.sources.issues && + !state.sources.prs && + !state.error + ) { unresolved.push({ repoId: repo.id, sourceKey: state.sourceKey, diff --git a/src/renderer/src/store/github/cache-model.ts b/src/renderer/src/store/github/cache-model.ts index fc317e04142..969233a2e86 100644 --- a/src/renderer/src/store/github/cache-model.ts +++ b/src/renderer/src/store/github/cache-model.ts @@ -5,6 +5,7 @@ import type { GitHubProjectViewError } from '../../../../shared/github/project-r import type { TaskSourceContext } from '../../../../shared/task-source-context' export type WorkItemsCacheSources = { + ineligible?: true issues: GitHubOwnerRepo | null prs: GitHubOwnerRepo | null /** Raw origin remote, kept distinct from the effective PR source. */ diff --git a/src/renderer/src/store/slices/github-work-items-error-envelope.test.ts b/src/renderer/src/store/slices/github-work-items-error-envelope.test.ts index afb11f74766..2265370d552 100644 --- a/src/renderer/src/store/slices/github-work-items-error-envelope.test.ts +++ b/src/renderer/src/store/slices/github-work-items-error-envelope.test.ts @@ -8,6 +8,11 @@ import { import { runtimeOwnedRepoState } from './store-test-helpers' import type { GitHubWorkItem } from '../../../../shared/github/work-item-types' import { GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE } from '../../../../shared/work-items' +import { + buildTaskPageRepoSourceState, + selectTaskPageUnresolvedSourceRepos, + selectTaskPageWorkItemsCacheEntries +} from '@/components/task-page/task-page-cache-selectors' describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { beforeEach(() => { @@ -49,6 +54,34 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { expect(result.error).toBeNull() }) + it('restores the unknown-source notice after a forced legacy runtime empty response', async () => { + const store = createTestStore() + store.setState(runtimeOwnedRepoState('env-1', 'repo-id', '/repo')) + const repos = store.getState().repos + const emptySources = { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + for (const ineligible of [true, false]) { + const sources = ineligible ? { ...emptySources, ineligible: true } : emptySources + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-marker', + ok: true, + result: { items: [], sources }, + _meta: { runtimeId: 'remote-runtime' } + }) + await store.getState().fetchWorkItems('repo-id', '/repo', 24, '', { force: !ineligible }) + const cache = store.getState().workItemsCache + const entries = selectTaskPageWorkItemsCacheEntries(cache, repos, 24, '') + expect(entries[0]?.sources).toEqual(sources) + expect( + selectTaskPageUnresolvedSourceRepos(repos, buildTaskPageRepoSourceState(repos, entries)) + ).toEqual( + ineligible + ? [] + : [{ repoId: 'repo-id', sourceKey: 'repo-id::runtime:env-1', label: 'repo' }] + ) + } + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(2) + }) + it('stamps the issues-side ClassifiedError with its source slug for banner copy', async () => { // Issue-only failure keeps successful PR items plus source-qualified error so list/banner coexist even after later reads (design §2). const store = createTestStore() diff --git a/src/shared/github/work-item-types.ts b/src/shared/github/work-item-types.ts index 9bd4b9d8acf..21a356b0c9c 100644 --- a/src/shared/github/work-item-types.ts +++ b/src/shared/github/work-item-types.ts @@ -83,6 +83,8 @@ export type GitHubWorkItemDetails = { export type ListWorkItemsResult = { items: T[] sources: { + /** Affirmative other-forge identity; absent on unresolved or legacy results. */ + ineligible?: true issues: GitHubOwnerRepo | null prs: GitHubOwnerRepo | null /** Raw origin independent of preferences; compare candidates without inferring from effective PR source. */