fix(gitlab): thread MR search query and surface MR base failures (#6263) (#6591)

Defect 1: the typed GitLab MR search query was dropped before reaching
the API. Thread query?: string end-to-end through the renderer effect,
the source-lookup, the preload/RPC args, and the desktop IPC handlers
(which previously passed a hardcoded undefined), and honor it on both the
glab REST path (&search=) and the cwd-inferred 'glab mr list' fallback.

Defect 2: when MR base resolution failed the renderer silently returned,
leaving baseBranch undefined so the worktree was created off the repo
default branch (origin/master) with no feedback. Surface the failure via
toast and clear stale base state, mirroring the GitHub PR path. Also make
resolveManagedMrBase resilient to an optional compare-base (target branch)
fetch failure: degrade gracefully by dropping compareBaseRef instead of
aborting, so a merged MR with a deleted target ref still resolves to its
valid source-branch base.

Fixes #6263

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil 2026-06-28 18:05:29 -07:00 committed by GitHub
parent f49808c540
commit 31bfeff01d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 313 additions and 35 deletions

View File

@ -555,6 +555,20 @@ describe('gitlab client — MR operations', () => {
}
})
it('appends an encoded &search= param when a query is supplied', async () => {
glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} })
await listMergeRequests('/repo', 'opened', 1, 20, undefined, 'fix login')
const callArgs = glabApiWithHeadersMock.mock.calls[0][0] as string[]
expect(callArgs[0]).toContain('&search=fix%20login')
})
it('omits &search= for an empty or whitespace-only query', async () => {
glabApiWithHeadersMock.mockResolvedValueOnce({ body: '[]', headers: {} })
await listMergeRequests('/repo', 'opened', 1, 20, undefined, ' ')
const callArgs = glabApiWithHeadersMock.mock.calls[0][0] as string[]
expect(callArgs[0]).not.toContain('search=')
})
it('flags fork MRs as cross-repository', async () => {
glabApiWithHeadersMock.mockResolvedValueOnce({
body: JSON.stringify([
@ -618,6 +632,32 @@ describe('gitlab client — MR operations', () => {
)
})
it('threads --search into the cwd fallback when a query is supplied', async () => {
resolveIssueSourceMock.mockResolvedValueOnce({
source: null,
fellBack: false
})
glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' })
await listMergeRequests('/repo', 'opened', 1, 20, undefined, 'fix login')
expect(glabApiWithHeadersMock).not.toHaveBeenCalled()
const callArgs = glabExecFileAsyncMock.mock.calls[0][0] as string[]
// Why (#6263): the cwd-inferred fallback must honor the typed query too.
const searchIdx = callArgs.indexOf('--search')
expect(searchIdx).toBeGreaterThanOrEqual(0)
expect(callArgs[searchIdx + 1]).toBe('fix login')
})
it('omits --search from the cwd fallback for a whitespace-only query', async () => {
resolveIssueSourceMock.mockResolvedValueOnce({
source: null,
fellBack: false
})
glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' })
await listMergeRequests('/repo', 'opened', 1, 20, undefined, ' ')
const callArgs = glabExecFileAsyncMock.mock.calls[0][0] as string[]
expect(callArgs).not.toContain('--search')
})
it('classifies fallback errors into the result envelope', async () => {
resolveIssueSourceMock.mockResolvedValueOnce({
source: null,

View File

@ -433,6 +433,10 @@ export async function listMergeRequests(
// (e.g. a fresh self-hosted instance), but glab itself can still
// resolve it from the local git config.
const stateFlag = mrListStateFlags(state)
// Why: the cwd-inferred fallback must honor the same search the API path
// does, otherwise typing a query against a self-hosted / unresolved-projectRef
// repo silently returns the unfiltered list (the original #6263 symptom).
const searchFlag = query?.trim() ? ['--search', query.trim()] : []
await acquire()
try {
const { stdout } = await glabExecFileAsync(
@ -449,7 +453,8 @@ export async function listMergeRequests(
'updated_at',
'--sort',
'desc',
...stateFlag
...stateFlag,
...searchFlag
],
glabRepoExecOptions(repoPath, connectionId, localGitOptions)
)

View File

@ -21,6 +21,23 @@ export function normalizeGitLabIssueListState(value: unknown): GitLabIssueListSt
return value === 'closed' || value === 'all' ? value : 'opened'
}
// Why: cap the free-text MR search at the same byte budget the renderer
// enforces (SMART_WORKSPACE_SOURCE_QUERY_MAX_BYTES) so the RPC/SSH path —
// which can be driven by callers other than the desktop input — can't push
// an unbounded string into the glab `&search=` query.
const GITLAB_SEARCH_QUERY_MAX_BYTES = 2048
export function normalizeGitLabSearchQuery(value: unknown): string | undefined {
if (typeof value !== 'string') {
return undefined
}
const trimmed = value.trim()
if (!trimmed) {
return undefined
}
return Buffer.byteLength(trimmed, 'utf8') > GITLAB_SEARCH_QUERY_MAX_BYTES ? undefined : trimmed
}
export function normalizeGitLabIssueAssignee(value: unknown): '@me' | undefined {
// Why: the renderer only exposes "Assigned to me"; accepting arbitrary
// values would turn preload/RPC boundaries into a generic glab flag surface.

View File

@ -213,6 +213,68 @@ describe('GitLab IPC handlers', () => {
)
})
it('forwards the typed search query into listMRs and listWorkItems', async () => {
listMergeRequestsMock.mockResolvedValueOnce({ items: [] })
listWorkItemsMock.mockResolvedValueOnce({ items: [] })
registerGitLabHandlers(storeWithRepos([repo()]) as Store)
await ipcHandlers.get('gitlab:listMRs')?.(null, {
repoPath: '/local/orca',
state: 'opened',
page: 1,
perPage: 20,
query: ' fix login '
})
await ipcHandlers.get('gitlab:listWorkItems')?.(null, {
repoPath: '/local/orca',
state: 'opened',
page: 1,
perPage: 20,
query: 'fix login'
})
// Why (#6263): the trimmed query must land in the 6th positional arg —
// previously the slot was hardcoded to `undefined`, so search never worked.
expect(listMergeRequestsMock).toHaveBeenCalledWith(
'/local/orca',
'opened',
1,
20,
undefined,
'fix login',
null
)
expect(listWorkItemsMock).toHaveBeenCalledWith(
'/local/orca',
'opened',
1,
20,
undefined,
'fix login',
null
)
})
it('drops blank or whitespace-only search queries to undefined', async () => {
listMergeRequestsMock.mockResolvedValueOnce({ items: [] })
registerGitLabHandlers(storeWithRepos([repo()]) as Store)
await ipcHandlers.get('gitlab:listMRs')?.(null, {
repoPath: '/local/orca',
query: ' '
})
expect(listMergeRequestsMock).toHaveBeenCalledWith(
'/local/orca',
'opened',
1,
20,
undefined,
undefined,
null
)
})
it('rejects source context for a different host', async () => {
registerGitLabHandlers(
storeWithRepos([repo({ id: 'repo-local', path: '/local/orca' })]) as Store

View File

@ -17,7 +17,8 @@ import {
normalizeGitLabIssueAssignee,
normalizeGitLabIssueListState,
normalizeGitLabMRListState,
normalizeGitLabPositiveInteger
normalizeGitLabPositiveInteger,
normalizeGitLabSearchQuery
} from '../gitlab/gitlab-preload-args'
import { recordGitLabProjectRecent } from '../gitlab/gitlab-project-recents'
import {
@ -165,6 +166,7 @@ export function registerGitLabHandlers(store: Store): void {
state?: 'opened' | 'merged' | 'closed' | 'all'
page?: number
perPage?: number
query?: string
}
) => {
const repo = assertRegisteredRepo(args, store)
@ -177,7 +179,7 @@ export function registerGitLabHandlers(store: Store): void {
page,
perPage,
repo.issueSourcePreference,
undefined,
normalizeGitLabSearchQuery(args.query),
repoConnectionId(repo),
...localGitOptionArgs(store, repo)
)
@ -326,6 +328,7 @@ export function registerGitLabHandlers(store: Store): void {
state?: 'opened' | 'merged' | 'closed' | 'all'
page?: number
perPage?: number
query?: string
}
) => {
const repo = assertRegisteredRepo(args, store)
@ -335,7 +338,7 @@ export function registerGitLabHandlers(store: Store): void {
normalizeGitLabPositiveInteger(args.page, 1, 10_000),
normalizeGitLabPositiveInteger(args.perPage, 20, 100),
repo.issueSourcePreference,
undefined,
normalizeGitLabSearchQuery(args.query),
repoConnectionId(repo),
...localGitOptionArgs(store, repo)
)

View File

@ -21329,6 +21329,66 @@ describe('OrcaRuntimeService', () => {
)
})
it('keeps the MR source base when the optional compare-base fetch fails', async () => {
// Why (#6263): a merged MR may have had its target ref deleted. A failed
// compare-base fetch must not abort and silently drop the worktree onto
// the repo default branch — keep the verified source base, drop compareBaseRef.
const localRepo = {
id: TEST_REPO_ID,
path: TEST_REPO_PATH,
displayName: 'repo',
badgeColor: 'blue',
addedAt: 1,
issueSourcePreference: 'origin' as const
}
const runtimeStore = {
...store,
getRepos: () => [localRepo],
getRepo: (id: string) => (id === localRepo.id ? localRepo : undefined)
}
getGitLabProjectRefForRemoteMock.mockResolvedValue({
host: 'gitlab.example',
path: 'group/repo'
})
const runtime = new OrcaRuntimeService(runtimeStore as never)
const gitSpy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockImplementation(async (args) => {
if (
args[0] === 'fetch' &&
args[2] === '+refs/heads/feature/fix:refs/remotes/origin/feature/fix'
) {
return { stdout: '', stderr: '' }
}
if (args[0] === 'fetch' && args[2] === '+refs/heads/main:refs/remotes/origin/main') {
// Target branch was deleted on the remote (merged MR).
throw new Error("couldn't find remote ref refs/heads/main")
}
if (args[0] === 'rev-parse' && args[1] === '--verify' && args[2] === 'origin/feature/fix') {
return { stdout: 'same-repo-mr-sha\n', stderr: '' }
}
throw new Error(`unexpected git call: ${args.join(' ')}`)
})
gitSpy.mockClear()
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
try {
const result = await runtime.resolveManagedMrBase({
repoSelector: 'id:repo-1',
mrIid: 79,
sourceBranch: 'feature/fix',
targetBranch: 'main'
})
expect(result).toEqual({
baseBranch: 'origin/feature/fix',
pushTarget: { remoteName: 'origin', branchName: 'feature/fix' }
})
expect(result).not.toHaveProperty('compareBaseRef')
expect(result).not.toHaveProperty('error')
} finally {
warnSpy.mockRestore()
gitSpy.mockRestore()
}
})
it('creates the first terminal by id when duplicate repo entries expose the same path', async () => {
const runtime = new OrcaRuntimeService(store)
const spawn = vi.fn().mockResolvedValue({ id: 'pty-duplicate-path' })

View File

@ -14120,17 +14120,27 @@ export class OrcaRuntimeService {
? sshGitProvider.fetchRemoteTrackingRef(repo.path, remote, branch, ref)
: gitExec(['fetch', remote, `+refs/heads/${branch}:${ref}`]))
}
const fetchTargetBranch = async (): Promise<{ error: string } | null> => {
// Why: the target/compare branch is optional (it only powers the diff
// base). A merged MR may have had its target ref deleted, so a fetch
// failure must NOT abort the whole resolution — that would discard the
// already-verified source-branch base and silently fall back to the repo
// default branch. Degrade gracefully by dropping compareBaseRef instead.
const fetchCompareBaseRef = async (): Promise<boolean> => {
if (!targetBranch || !compareBaseRef) {
return null
return false
}
try {
await fetchRemoteTrackingRef(targetBranch, compareBaseRef)
return true
} catch (error) {
const message = error instanceof Error ? error.message : String(error)
return { error: `Failed to fetch ${remote}/${targetBranch}: ${message.split('\n')[0]}` }
console.warn('[runtime:resolveManagedMrBase] optional compare-base fetch failed', {
remote,
targetBranch,
mrIid: args.mrIid,
error: error instanceof Error ? error.message.split('\n')[0] : String(error)
})
return false
}
return null
}
if (isCrossRepository) {
@ -14155,11 +14165,8 @@ export class OrcaRuntimeService {
if (!sha) {
return { error: `Empty SHA resolving fork MR !${args.mrIid} head.` }
}
const targetFetchError = await fetchTargetBranch()
if (targetFetchError) {
return targetFetchError
}
return { baseBranch: sha, ...(compareBaseRef ? { compareBaseRef } : {}) }
const compareBaseFetched = await fetchCompareBaseRef()
return { baseBranch: sha, ...(compareBaseFetched ? { compareBaseRef } : {}) }
}
try {
@ -14175,13 +14182,10 @@ export class OrcaRuntimeService {
} catch {
return { error: `Remote ref ${remoteRef} does not exist after fetch.` }
}
const targetFetchError = await fetchTargetBranch()
if (targetFetchError) {
return targetFetchError
}
const compareBaseFetched = await fetchCompareBaseRef()
return {
baseBranch: remoteRef,
...(compareBaseRef ? { compareBaseRef } : {}),
...(compareBaseFetched ? { compareBaseRef } : {}),
pushTarget: { remoteName: remote, branchName: sourceBranch }
}
}

View File

@ -1507,6 +1507,7 @@ export type PreloadApi = {
state?: MRListState
page?: number
perPage?: number
query?: string
}
) => Promise<ListMergeRequestsResult>
/** Combined MR + issue list filtered by state. Issues are skipped
@ -1516,6 +1517,7 @@ export type PreloadApi = {
state?: MRListState
page?: number
perPage?: number
query?: string
}
) => Promise<ListMergeRequestsResult>
issue: (args: GitLabRepoSelectorArgs & { number: number }) => Promise<GitLabIssueInfo | null>

View File

@ -35,6 +35,7 @@ export const glApi = {
state?: 'opened' | 'merged' | 'closed' | 'all'
page?: number
perPage?: number
query?: string
}
): Promise<unknown> => ipcRenderer.invoke('gitlab:listMRs', args),
@ -43,6 +44,7 @@ export const glApi = {
state?: 'opened' | 'merged' | 'closed' | 'all'
page?: number
perPage?: number
query?: string
}
): Promise<unknown> => ipcRenderer.invoke('gitlab:listWorkItems', args),

View File

@ -976,6 +976,10 @@ export default function SmartWorkspaceNameField({
}
let stale = false
setGitlabLoading(true)
// Why: thread the typed query through so the GitLab API filters MRs by
// name/number (mirrors the GitHub effect). shouldQueryGitlab already
// gates on sourceQueryWithinLimit, so an oversized query never reaches here.
const trimmedQuery = debouncedQuery.trim() || undefined
void Promise.all(
repoBackedSearchTargets.map((target) =>
listGitLabMRsForSource({
@ -984,7 +988,8 @@ export default function SmartWorkspaceNameField({
sourceContext: target.gitlabSourceContext,
state: mrStateFilter,
page: 1,
perPage: RESULT_LIMIT
perPage: RESULT_LIMIT,
query: trimmedQuery
}).catch(() => ({ items: [], hasMore: false }))
)
)
@ -1013,6 +1018,7 @@ export default function SmartWorkspaceNameField({
stale = true
}
}, [
debouncedQuery,
disabled,
mode,
mrStateFilter,

View File

@ -45,6 +45,11 @@ describe('useComposerState host-context boundaries', () => {
expect(section).toContain('worktree.resolveMrBase')
expect(section).toContain('repo: runRepo.id')
expect(section).not.toContain('repoId: repoForItem.id')
// Why (#6263): an unresolved MR base must surface a toast and clear stale
// state instead of silently dropping the worktree onto origin/master.
expect(section).toContain('toast.error(result.error)')
expect(section).toContain("'Failed to resolve MR base.'")
expect(section).toMatch(/\.catch\(\(error: unknown\) =>/)
})
it('does not use local SSH gates for runtime-owned folder targets', () => {

View File

@ -2702,12 +2702,35 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
},
{ timeoutMs: 30_000 }
)
void resolveMrBase.then((result) => {
if ('error' in result) {
return
}
handleBaseBranchMrSelect(result.baseBranch, item, result.pushTarget, result.compareBaseRef)
})
void resolveMrBase
.then((result) => {
if ('error' in result) {
// Why: without surfacing the failure the worktree silently falls
// back to the repo default branch (origin/master), so clear stale
// base state and tell the user — mirrors the GitHub PR path.
setBaseBranch(undefined)
setCompareBaseRef(undefined)
setPushTarget(undefined)
toast.error(result.error)
return
}
handleBaseBranchMrSelect(
result.baseBranch,
item,
result.pushTarget,
result.compareBaseRef
)
})
.catch((error: unknown) => {
setBaseBranch(undefined)
setCompareBaseRef(undefined)
setPushTarget(undefined)
toast.error(
error instanceof Error
? error.message
: translate('auto.hooks.useComposerState.5f3d2c8a1b', 'Failed to resolve MR base.')
)
})
},
[
applyLinkedGitLabWorkItem,

View File

@ -543,7 +543,8 @@
"chooseOrAddProjectBeforeWorkspace": "Choose or add a project before creating a workspace.",
"folderWorkspaceCreateFailedTitle": "Folder workspace creation failed",
"folderWorkspaceCreateFailedMessage": "The folder workspace could not be created. Check the error details above, then try again.",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior."
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base."
},
"useGlobalFileDrop": {
"38c9f034ff": "Failed to upload dropped files.",

View File

@ -543,7 +543,8 @@
"chooseOrAddProjectBeforeWorkspace": "Elige o agrega un proyecto antes de crear un espacio de trabajo.",
"folderWorkspaceCreateFailedTitle": "No se pudo crear el espacio de trabajo de carpeta",
"folderWorkspaceCreateFailedMessage": "No se pudo crear el espacio de trabajo de carpeta. Revisa los detalles del error de arriba e inténtalo de nuevo.",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior."
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base."
},
"useGlobalFileDrop": {
"38c9f034ff": "No se pudieron cargar los archivos eliminados.",

View File

@ -543,7 +543,8 @@
"chooseOrAddProjectBeforeWorkspace": "ワークスペースを作成する前に、プロジェクトを選択または追加。",
"folderWorkspaceCreateFailedTitle": "フォルダーワークスペースを作成できませんでした",
"folderWorkspaceCreateFailedMessage": "フォルダーワークスペースを作成できませんでした。上のエラー詳細を確認して、もう一度お試しください。",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior."
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base."
},
"useGlobalFileDrop": {
"38c9f034ff": "ドロップされたファイルのアップロードに失敗しました。",

View File

@ -543,7 +543,8 @@
"chooseOrAddProjectBeforeWorkspace": "워크스페이스를 만들기 전에 프로젝트를 선택하거나 추가하세요.",
"folderWorkspaceCreateFailedTitle": "폴더 워크스페이스를 만들지 못했습니다",
"folderWorkspaceCreateFailedMessage": "폴더 워크스페이스를 만들 수 없습니다. 위의 오류 세부 정보를 확인한 뒤 다시 시도하세요.",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior."
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base."
},
"useGlobalFileDrop": {
"38c9f034ff": "드롭한 파일을 업로드하지 못했습니다.",

View File

@ -543,7 +543,8 @@
"chooseOrAddProjectBeforeWorkspace": "创建工作区前,请选择或添加项目。",
"folderWorkspaceCreateFailedTitle": "文件夹工作区创建失败",
"folderWorkspaceCreateFailedMessage": "无法创建文件夹工作区。请查看上方错误详情,然后重试。",
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior."
"setupAgentStartupPolicySaveFailed": "Failed to save setup startup behavior.",
"5f3d2c8a1b": "Failed to resolve MR base."
},
"useGlobalFileDrop": {
"38c9f034ff": "无法上传删除的文件。",

View File

@ -94,19 +94,60 @@ describe('GitLab source lookup routing', () => {
sourceContext: runtimeSourceContext,
state: 'opened',
page: 1,
perPage: 12
perPage: 12,
query: 'fix login'
})
).resolves.toMatchObject({ items: [{ repoId: 'renderer-repo', number: 7 }] })
// Why (#6263): the typed query must reach the runtime RPC so GitLab search
// actually filters; previously the field was never threaded through.
expect(callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'environment', environmentId: 'env-1' },
'gitlab.listMRs',
{ repo: 'runtime-repo', state: 'opened', page: 1, perPage: 12 },
{ repo: 'runtime-repo', state: 'opened', page: 1, perPage: 12, query: 'fix login' },
{ timeoutMs: 30_000 }
)
expect(window.api.gl.listMRs).not.toHaveBeenCalled()
})
it('forwards the search query to local GitLab MR lists over Electron IPC', async () => {
const localSourceContext: TaskSourceContext = {
...runtimeSourceContext,
hostId: 'local',
repoId: 'local-repo'
}
vi.mocked(window.api.gl.listMRs).mockResolvedValue({
items: [gitlabItem({ repoId: 'local-returned' })],
page: 1,
perPage: 12,
totalCount: 1,
totalPages: 1
})
await expect(
listGitLabMRsForSource({
repoPath: '/workspace/app',
repoId: 'local-repo',
sourceContext: localSourceContext,
state: 'opened',
page: 1,
perPage: 12,
query: 'fix login'
})
).resolves.toMatchObject({ items: [{ repoId: 'local-repo', number: 7 }] })
expect(window.api.gl.listMRs).toHaveBeenCalledWith({
repoPath: '/workspace/app',
repoId: 'local-repo',
sourceContext: localSourceContext,
state: 'opened',
page: 1,
perPage: 12,
query: 'fix login'
})
expect(callRuntimeRpc).not.toHaveBeenCalled()
})
it('keeps local GitLab lookups on Electron IPC with source context', async () => {
vi.mocked(window.api.gl.workItemByPath).mockResolvedValue(
gitlabItem({ repoId: 'local-returned' })

View File

@ -20,6 +20,7 @@ type GitLabMRListLookupArgs = GitLabSourceLookupArgs & {
state?: 'opened' | 'merged' | 'closed' | 'all'
page?: number
perPage?: number
query?: string
}
function runtimeRepoId(args: Pick<GitLabSourceLookupArgs, 'repoId' | 'sourceContext'>): string {
@ -73,7 +74,8 @@ export async function listGitLabMRsForSource(
repo: runtimeRepoId(args),
state: args.state,
page: args.page,
perPage: args.perPage
perPage: args.perPage,
query: args.query
},
{ timeoutMs: 30_000 }
)
@ -83,7 +85,8 @@ export async function listGitLabMRsForSource(
sourceContext: args.sourceContext,
state: args.state,
page: args.page,
perPage: args.perPage
perPage: args.perPage,
query: args.query
})) as ListMergeRequestsResult)
return {
...result,