From c69e83176d6141e2e5e03f98f966c2ea01cf2e5f Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:36:52 +0000 Subject: [PATCH] fix(gitlab): find issues and #N in the new-workspace gitlab source (#32005) --- src/main/gitlab/client-work-items.test.ts | 12 ++ .../gitlab/gitlab-issue-source-resolution.ts | 115 ++++++++++++++++++ .../gitlab/gitlab-project-ref-resolution.ts | 81 ------------ src/main/gitlab/gl-utils.test.ts | 23 ++++ src/main/gitlab/gl-utils.ts | 12 +- src/main/gitlab/work-item-queries.ts | 12 +- .../use-smart-workspace-gitlab-search.ts | 6 +- .../gitlab-work-item-source-lookup.test.ts | 24 ++-- .../src/lib/gitlab-work-item-source-lookup.ts | 12 +- 9 files changed, 186 insertions(+), 111 deletions(-) create mode 100644 src/main/gitlab/gitlab-issue-source-resolution.ts diff --git a/src/main/gitlab/client-work-items.test.ts b/src/main/gitlab/client-work-items.test.ts index 80e412862d8..5cc48966a3d 100644 --- a/src/main/gitlab/client-work-items.test.ts +++ b/src/main/gitlab/client-work-items.test.ts @@ -119,6 +119,18 @@ describe('gitlab client — combined listWorkItems', () => { expect(issuesCallPath.at(-1)).toContain('search=ambiguous%20selector') }) + it('looks up `#N` as the issue iid instead of a text search', async () => { + glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} }) + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + await listWorkItems('/repo', 'opened', 1, 20, undefined, ' #23 ') + + const issuesCallPath = glabExecFileAsyncMock.mock.calls[0][0] as string[] + const issueParams = new URLSearchParams(issuesCallPath.at(-1)?.split('?')[1]) + expect(issueParams.getAll('iids[]')).toEqual(['23']) + expect(issueParams.has('search')).toBe(false) + }) + it('passes the requested page through to merge request and issue fetches', async () => { glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} }) glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) diff --git a/src/main/gitlab/gitlab-issue-source-resolution.ts b/src/main/gitlab/gitlab-issue-source-resolution.ts new file mode 100644 index 00000000000..baa6a0e4e8d --- /dev/null +++ b/src/main/gitlab/gitlab-issue-source-resolution.ts @@ -0,0 +1,115 @@ +import { listCachedRemoteNames, shouldProbeGitRemote } from '../git/remote-name-listing' +import type { IssueSourcePreference } from '../../shared/repo-types' +import { + getProjectRefForRemote, + type LocalGitExecOptions, + type ProjectRef +} from './gitlab-project-ref-resolution' + +export async function getIssueProjectRef( + repoPath: string, + knownHosts?: readonly string[], + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const originPromise = getProjectRefForRemote( + repoPath, + 'origin', + knownHosts, + connectionId, + localGitOptions + ) + if (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions)) { + const upstream = await getProjectRefForRemote( + repoPath, + 'upstream', + knownHosts, + connectionId, + localGitOptions + ) + if (upstream) { + return upstream + } + } + return ( + (await originPromise) ?? + getProjectRefForOtherRemote(repoPath, knownHosts, connectionId, localGitOptions) + ) +} + +// Why: glab infers a project from any GitLab remote, e.g. one named `gitlab`; the issue source should too. +async function getProjectRefForOtherRemote( + repoPath: string, + knownHosts?: readonly string[], + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + const remotes = await listCachedRemoteNames(repoPath, connectionId, localGitOptions) + const otherRemotes = (remotes ?? []).filter((name) => name !== 'origin' && name !== 'upstream') + // Why: glab ranks a remote named `gitlab` above the rest. + otherRemotes.sort((a, b) => Number(b === 'gitlab') - Number(a === 'gitlab')) + for (const remoteName of otherRemotes) { + const projectRef = await getProjectRefForRemote( + repoPath, + remoteName, + knownHosts, + connectionId, + localGitOptions + ) + if (projectRef) { + return projectRef + } + } + return null +} + +export type ResolvedIssueSource = { + source: ProjectRef | null + /** True when explicit upstream is gone and resolver fell back to origin. */ + fellBack: boolean +} + +export async function resolveIssueSource( + repoPath: string, + preference: IssueSourcePreference | undefined, + knownHosts?: readonly string[], + connectionId?: string | null, + localGitOptions: LocalGitExecOptions = {} +): Promise { + if (preference === 'upstream') { + const upstream = await getProjectRefForRemote( + repoPath, + 'upstream', + knownHosts, + connectionId, + localGitOptions + ) + if (upstream) { + return { source: upstream, fellBack: false } + } + const origin = await getProjectRefForRemote( + repoPath, + 'origin', + knownHosts, + connectionId, + localGitOptions + ) + return { source: origin, fellBack: origin !== null } + } + if (preference === 'origin') { + return { + source: await getProjectRefForRemote( + repoPath, + 'origin', + knownHosts, + connectionId, + localGitOptions + ), + fellBack: false + } + } + return { + source: await getIssueProjectRef(repoPath, knownHosts, connectionId, localGitOptions), + fellBack: false + } +} diff --git a/src/main/gitlab/gitlab-project-ref-resolution.ts b/src/main/gitlab/gitlab-project-ref-resolution.ts index d15eda18684..8b35419f1ab 100644 --- a/src/main/gitlab/gitlab-project-ref-resolution.ts +++ b/src/main/gitlab/gitlab-project-ref-resolution.ts @@ -1,10 +1,8 @@ import { glabExecFileAsync } from '../git/runner' import type { GitAdmissionTier } from '../git/command-runner/git-exec-options' -import { shouldProbeGitRemote } from '../git/remote-name-listing' 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 type { IssueSourcePreference } from '../../shared/repo-types' import { clearProjectRefInFlight, runProjectRefProbeOnce } from './project-ref-inflight' import { _resetGlabUnauthenticatedHosts, @@ -171,85 +169,6 @@ export async function getProjectRef( return getProjectRefForRemote(repoPath, 'origin', knownHosts, connectionId, localGitOptions) } -export async function getIssueProjectRef( - repoPath: string, - knownHosts?: readonly string[], - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - const originPromise = getProjectRefForRemote( - repoPath, - 'origin', - knownHosts, - connectionId, - localGitOptions - ) - if (await shouldProbeGitRemote(repoPath, 'upstream', connectionId, localGitOptions)) { - const upstream = await getProjectRefForRemote( - repoPath, - 'upstream', - knownHosts, - connectionId, - localGitOptions - ) - if (upstream) { - return upstream - } - } - return originPromise -} - -export type ResolvedIssueSource = { - source: ProjectRef | null - /** True when explicit upstream is gone and resolver fell back to origin. */ - fellBack: boolean -} - -export async function resolveIssueSource( - repoPath: string, - preference: IssueSourcePreference | undefined, - knownHosts?: readonly string[], - connectionId?: string | null, - localGitOptions: LocalGitExecOptions = {} -): Promise { - if (preference === 'upstream') { - const upstream = await getProjectRefForRemote( - repoPath, - 'upstream', - knownHosts, - connectionId, - localGitOptions - ) - if (upstream) { - return { source: upstream, fellBack: false } - } - const origin = await getProjectRefForRemote( - repoPath, - 'origin', - knownHosts, - connectionId, - localGitOptions - ) - return { source: origin, fellBack: origin !== null } - } - if (preference === 'origin') { - return { - source: await getProjectRefForRemote( - repoPath, - 'origin', - knownHosts, - connectionId, - localGitOptions - ), - fellBack: false - } - } - return { - source: await getIssueProjectRef(repoPath, knownHosts, connectionId, localGitOptions), - fellBack: false - } -} - export function glabRepoExecOptions( repoPath: string, connectionId?: string | null, diff --git a/src/main/gitlab/gl-utils.test.ts b/src/main/gitlab/gl-utils.test.ts index 82344aa2cc5..88f32dadb68 100644 --- a/src/main/gitlab/gl-utils.test.ts +++ b/src/main/gitlab/gl-utils.test.ts @@ -135,6 +135,29 @@ describe('gitlab project ref resolution', () => { expect(gitRemoteGetUrlCalls('origin')).toHaveLength(1) }) + it('finds the GitLab project on other remotes, preferring one named gitlab like glab', async () => { + mockGitRemoteCommands({ + origin: 'git@github.com:stablyai/orca.git\n', + 'a-fork': 'git@gitlab.com:someone/orca.git\n', + gitlab: 'git@gitlab.com:stablyai/orca.git\n' + }) + + await expect(getIssueProjectRef('/repo')).resolves.toEqual({ + host: 'gitlab.com', + path: 'stablyai/orca' + }) + }) + + it('adds no glab probe for extra remotes on a GitHub-only repo', async () => { + mockGitRemoteCommands({ + origin: 'git@github.com:stablyai/orca.git\n', + fork: 'git@github.com:someone/orca.git\n' + }) + + await expect(getIssueProjectRef('/repo')).resolves.toBeNull() + expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + it('does not mix origin and upstream cache entries for the same repo path', async () => { mockGitRemoteCommands({ origin: 'git@gitlab.com:fork/orca.git\n', diff --git a/src/main/gitlab/gl-utils.ts b/src/main/gitlab/gl-utils.ts index 0991ed520cd..d080101e7a8 100644 --- a/src/main/gitlab/gl-utils.ts +++ b/src/main/gitlab/gl-utils.ts @@ -15,21 +15,17 @@ export { _resetKnownHostsCache, _resetProjectRefCache, getGlabKnownHosts, - getIssueProjectRef, getProjectRef, getProjectRefForRemote, glabHostEnvOptions, glabHostnameArgs, glabRepoExecOptions, parseGlabAuthStatusHosts, - parseGitLabProjectRef, - resolveIssueSource -} from './gitlab-project-ref-resolution' -export type { - LocalGitExecOptions, - ProjectRef, - ResolvedIssueSource + parseGitLabProjectRef } from './gitlab-project-ref-resolution' +export type { LocalGitExecOptions, ProjectRef } from './gitlab-project-ref-resolution' +export { getIssueProjectRef, resolveIssueSource } from './gitlab-issue-source-resolution' +export type { ResolvedIssueSource } from './gitlab-issue-source-resolution' export { parseGlabApiResponse, parseGlabJsonList, diff --git a/src/main/gitlab/work-item-queries.ts b/src/main/gitlab/work-item-queries.ts index 4e6e3c3fc53..b8bf0450987 100644 --- a/src/main/gitlab/work-item-queries.ts +++ b/src/main/gitlab/work-item-queries.ts @@ -148,6 +148,16 @@ export async function listWorkItems( } } +function issueSearchParam(query?: string): string { + const trimmed = query?.trim() + if (!trimmed) { + return '' + } + // Why: GitLab's search matches text only, so `#23` (its issue reference) needs the iid filter. + const iid = /^#(\d+)$/.exec(trimmed)?.[1] + return iid ? `&iids[]=${iid}` : `&search=${encodeURIComponent(trimmed)}` +} + export async function fetchIssuesAsWorkItems( repoPath: string, projectRef: ProjectRef, @@ -161,7 +171,7 @@ export async function fetchIssuesAsWorkItems( await acquire() try { const stateParam = state === 'all' ? '' : `&state=${state}` - const searchParam = query?.trim() ? `&search=${encodeURIComponent(query.trim())}` : '' + const searchParam = issueSearchParam(query) const { stdout } = await glabExecFileAsync( [ 'api', diff --git a/src/renderer/src/components/new-workspace/use-smart-workspace-gitlab-search.ts b/src/renderer/src/components/new-workspace/use-smart-workspace-gitlab-search.ts index faa61be2d5f..156d720cba9 100644 --- a/src/renderer/src/components/new-workspace/use-smart-workspace-gitlab-search.ts +++ b/src/renderer/src/components/new-workspace/use-smart-workspace-gitlab-search.ts @@ -1,6 +1,6 @@ import { useEffect, useMemo } from 'react' import { - listGitLabMRsForSource, + listGitLabWorkItemsForSource, lookupGitLabWorkItemByPathForSource } from '@/lib/gitlab-work-item-source-lookup' import { parseGitLabIssueOrMRLink } from '@/lib/gitlab-links' @@ -97,7 +97,7 @@ export function useSmartWorkspaceGitlabSearch({ shouldQueryGitlab ]) - // Why: state chips mirror GitLab's default opened-MR list when no URL is pasted. + // Why: state chips mirror GitLab's default opened list when no URL is pasted. useEffect(() => { if (!shouldQueryGitlab || disabled || gitlabEnabled === false) { if (!shouldQueryGitlab) { @@ -123,7 +123,7 @@ export function useSmartWorkspaceGitlabSearch({ } void Promise.all( repoBackedSearchTargets.map((target) => - listGitLabMRsForSource({ + listGitLabWorkItemsForSource({ repoPath: target.repo.path, repoId: target.repo.id, sourceContext: target.gitlabSourceContext, diff --git a/src/renderer/src/lib/gitlab-work-item-source-lookup.test.ts b/src/renderer/src/lib/gitlab-work-item-source-lookup.test.ts index 4b13a04cfa2..dbfe537aa1a 100644 --- a/src/renderer/src/lib/gitlab-work-item-source-lookup.test.ts +++ b/src/renderer/src/lib/gitlab-work-item-source-lookup.test.ts @@ -2,10 +2,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import type * as RuntimeRpcClient from '@/runtime/runtime-rpc-client' import { - listGitLabMRsForSource, + listGitLabWorkItemsForSource, lookupGitLabWorkItemByPathForSource } from './gitlab-work-item-source-lookup' -import type { GitLabWorkItem, ListMergeRequestsResult } from '../../../shared/gitlab-types' +import type { GitLabPagedResult, GitLabWorkItem } from '../../../shared/gitlab-types' import type { TaskSourceContext } from '../../../shared/task-source-context' vi.mock('@/runtime/runtime-rpc-client', async () => { @@ -47,7 +47,7 @@ describe('GitLab source lookup routing', () => { api: { gl: { workItemByPath: vi.fn(), - listMRs: vi.fn() + listWorkItems: vi.fn() } } }) @@ -83,8 +83,8 @@ describe('GitLab source lookup routing', () => { expect(window.api.gl.workItemByPath).not.toHaveBeenCalled() }) - it('routes runtime-owned GitLab MR lists through runtime RPC', async () => { - const result: ListMergeRequestsResult = { + it('routes runtime-owned GitLab work item lists through runtime RPC', async () => { + const result: GitLabPagedResult = { items: [gitlabItem({ repoId: 'runtime-returned' })], page: 1, perPage: 12, @@ -94,7 +94,7 @@ describe('GitLab source lookup routing', () => { vi.mocked(callRuntimeRpc).mockResolvedValue(result) await expect( - listGitLabMRsForSource({ + listGitLabWorkItemsForSource({ repoPath: '/workspace/app', repoId: 'renderer-repo', sourceContext: runtimeSourceContext, @@ -109,7 +109,7 @@ describe('GitLab source lookup routing', () => { // actually filters; previously the field was never threaded through. expect(callRuntimeRpc).toHaveBeenCalledWith( { kind: 'environment', environmentId: 'env-1' }, - 'gitlab.listMRs', + 'gitlab.listWorkItems', expect.objectContaining({ repo: 'id:runtime-repo', state: 'opened', @@ -119,16 +119,16 @@ describe('GitLab source lookup routing', () => { }), { timeoutMs: 30_000 } ) - expect(window.api.gl.listMRs).not.toHaveBeenCalled() + expect(window.api.gl.listWorkItems).not.toHaveBeenCalled() }) - it('forwards the search query to local GitLab MR lists over Electron IPC', async () => { + it('forwards the search query to local GitLab work item lists over Electron IPC', async () => { const localSourceContext: TaskSourceContext = { ...runtimeSourceContext, hostId: 'local', repoId: 'local-repo' } - vi.mocked(window.api.gl.listMRs).mockResolvedValue({ + vi.mocked(window.api.gl.listWorkItems).mockResolvedValue({ items: [gitlabItem({ repoId: 'local-returned' })], page: 1, perPage: 12, @@ -137,7 +137,7 @@ describe('GitLab source lookup routing', () => { }) await expect( - listGitLabMRsForSource({ + listGitLabWorkItemsForSource({ repoPath: '/workspace/app', repoId: 'local-repo', sourceContext: localSourceContext, @@ -148,7 +148,7 @@ describe('GitLab source lookup routing', () => { }) ).resolves.toMatchObject({ items: [{ repoId: 'local-repo', number: 7 }] }) - expect(window.api.gl.listMRs).toHaveBeenCalledWith({ + expect(window.api.gl.listWorkItems).toHaveBeenCalledWith({ repoPath: '/workspace/app', repoId: 'local-repo', sourceContext: localSourceContext, diff --git a/src/renderer/src/lib/gitlab-work-item-source-lookup.ts b/src/renderer/src/lib/gitlab-work-item-source-lookup.ts index 0ebfee4ab90..5d09bcafafc 100644 --- a/src/renderer/src/lib/gitlab-work-item-source-lookup.ts +++ b/src/renderer/src/lib/gitlab-work-item-source-lookup.ts @@ -1,4 +1,4 @@ -import type { GitLabWorkItem, ListMergeRequestsResult } from '../../../shared/gitlab-types' +import type { GitLabPagedResult, GitLabWorkItem } from '../../../shared/gitlab-types' import type { TaskSourceContext } from '../../../shared/task-source-context' import { gitLabApiFor } from '@/runtime/gitlab-owner-api' @@ -15,7 +15,7 @@ type GitLabWorkItemByPathLookupArgs = GitLabSourceLookupArgs & { type: 'issue' | 'mr' } -type GitLabMRListLookupArgs = GitLabSourceLookupArgs & { +type GitLabWorkItemListLookupArgs = GitLabSourceLookupArgs & { state?: 'opened' | 'merged' | 'closed' | 'all' page?: number perPage?: number @@ -33,10 +33,10 @@ export async function lookupGitLabWorkItemByPathForSource( return item ? withRendererRepoId(item, args.repoId) : null } -export async function listGitLabMRsForSource( - args: GitLabMRListLookupArgs -): Promise { - const result = await gitLabApiFor(args).listMRs(args) +export async function listGitLabWorkItemsForSource( + args: GitLabWorkItemListLookupArgs +): Promise> { + const result = await gitLabApiFor(args).listWorkItems(args) return { ...result, items: result.items.map((item) => withRendererRepoId(item, args.repoId))