fix(gitlab): thread MR search query and surface MR base failures (#6263)#6591
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThis PR makes two independent improvements. First, it adds optional free-text search query filtering to GitLab MR and work-item listings: a new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
EvidenceUnit / integration tests (611 passing across the affected suites)New/extended regression tests:
Lint / typecheck (clean)Live end-to-end verification (running Orca dev build over CDP)Set up a real gitlab.com project ( Direct GitLab API/CLI confirm Through the running fixed app (CDP eval on I confirmed the running main bundle contains the fix ( Note on the visual repro: I drove this over CDP at the IPC layer rather than capturing a dropdown screenshot. The synthetic test repo (a thin local repo pointing at the real GitLab remote) occasionally fell through glab's local project-ref inference between rapid calls, making the dropdown-list screenshot flaky; the IPC-level result above is the deterministic proof that the query is now threaded end-to-end and filters. A normally-cloned GitLab repo takes the REST path consistently. Test MRs were closed after verification. |
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>
1278b5c to
dba412a
Compare
📸 Screenshot proof (rebased build — live Orca dev, CDP-driven)After rebasing onto
These shots are from the Create worktree → GitLab tab — the exact surface #6263 reported as broken. Defect 1 — search now filters the MR listUnfiltered (no query): all 3 open MRs are listed.
Typing
Typing
Before this PR the typed query was dropped before reaching the API, so all three rows stayed visible no matter what you typed. Now the query is threaded end-to-end and the list narrows to the match. Defect 2 — worktree lands on the MR's branch, not the defaultSelecting
Driving the real IPC the PR fixes ( window.api.worktrees.resolveMrBase({ repoId, mrIid: 1,
sourceBranch: 'stably-session-cmmb6alvc0002ky04i797mzsb', targetBranch: 'main' })
// =>
{
"baseBranch": "origin/stably-session-cmmb6alvc0002ky04i797mzsb", // the MR branch, not origin/main
"compareBaseRef": "refs/remotes/origin/main",
"pushTarget": { "remoteName": "origin", "branchName": "stably-session-cmmb6alvc0002ky04i797mzsb" }
}And the search query reaches the API through the same IPC the renderer calls ( listMRs({ repoPath, state:'opened', perPage:20 }) // -> 3 MRs (unfiltered)
listMRs({ repoPath, state:'opened', perPage:20, query:'login' }) // -> ["Add login validation flow"] (1 of 3)
listMRs({ repoPath, state:'opened', perPage:20, query:'payment' })// -> ["Refactor payment gateway timeout"] (1 of 3)Rebase verification
Test MRs were re-closed after capture. Images hosted on a public scratch repo and referenced by raw URL (no |
1 participant
🧑🤝🧑 Impact & ELI5
1) What's broken today (ELI5). GitLab users creating a new worktree hit two real blockers (both reported in #6263, reliably reproducible). First, the MR search box doesn't work: you type the merge-request's name or number to find it, the spinner spins, but the tiny dropdown only ever shows the same unfiltered list — so you basically can't pick the MR you want. The only escape hatch was pasting the full MR web link. Second, and worse, even once you finally got an MR selected, the new worktree was built off the wrong branch — it quietly landed on the repo's default branch (
origin/master) instead of the MR's actual branch, with no warning, so you'd start working on the wrong code without realizing it. This affects GitLab users specifically (GitHub already worked); for them it's a genuine blocker on a core action.2) What this PR does (ELI5). Think of the search box as a waiter who took your order but never carried it to the kitchen. Your order (the typed query) was thrown in the trash at the door — the kitchen (the GitLab API) actually knew how to handle it, nobody ever delivered it. This PR connects the whole chain end-to-end so what you type really reaches GitLab and filters the list. For the wrong-branch problem: previously, if Orca couldn't figure out the MR's base branch it just gave up silently and used the default branch. Now it (a) tells you with an error message when it can't resolve the base, and (b) is smarter — if a merged MR's target branch was already deleted, it no longer throws away the perfectly good source branch it already found; it keeps it. So: it now actually filters your search and lands you on the right branch (or clearly tells you when it can't), instead of silently failing.
3) Tradeoffs / regressions — honest answer. No regressions — nothing is disabled, no feature is turned off, no new setup or setting is required, and no behavior anyone relied on is lost. The search-query plumbing is purely additive and GitLab-only (GitHub already had it); when no query is typed the behavior is byte-for-byte identical to before. The base-resolution change strictly improves correctness: the only "behavior" it removes is the silent wrong-branch fallback, which was never a feature anyone wanted. One honest nuance worth naming: the optional diff-compare base ref can now be dropped when its branch fetch fails — but that only affects the diff comparison baseline in an edge case (a merged MR whose target ref was already deleted), and the previous behavior there was strictly worse (it aborted entirely and dumped you on the default branch). The search input is also length-capped (2048 bytes) at the boundary to match the existing renderer limit, which no real query approaches. Net: GitLab users gain, nobody loses.
Summary
Fixes #6263.
Two distinct GitLab create-worktree defects:
Defect 1 — GitLab search never filtered. The typed search query was dropped before it ever reached the GitLab API. The downstream layers already supported search (
listMergeRequestsbuilds&search=, and the runtime RPC forwardsparams.query), but no caller supplied it: the renderer GitLab list-mode effect calledlistGitLabMRsForSourcewithout the query (anddebouncedQuerywasn't even in its dependency array),listGitLabMRsForSourcehad noqueryparam on either branch, and the desktop IPC handlersgitlab:listMRs/gitlab:listWorkItemspassed a hardcodedundefinedin the query slot. So typing an MR name/number/!Njust re-fetched the unfiltered opened list (only pasting a full MR URL worked, via the separate paste-URL path).The fix threads
query?: stringend-to-end (renderer effect →listGitLabMRsForSource→ preload/RPC args +PreloadApitype → IPC handlers), normalizing it on the main side (normalizeGitLabSearchQuery: trim, cap at the same 2048-byte budget the renderer enforces). It is honored on both the glab REST path (&search=<encoded>) and the cwd-inferredglab mr listfallback (--search), so self-hosted / unresolved-projectRef repos filter too.Defect 2 — worktree created off the default branch when MR base resolution failed. The renderer silently early-returned on a
{error}result fromresolveMrBaseand had no.catch, unlike the GitHub path which toasts. When resolution errored,baseBranchstayed undefined and the worktree was created off the repo default branch (origin/master) with no feedback. The fix surfaces the failure viatoast.errorand clears stale base state, mirroring the GitHub PR handler. A contributing robustness bug inresolveManagedMrBaseaborted the entire resolution if the optional target/compare-branch fetch failed — even after the MR source branch was successfully fetched and verified — discarding a valid base (common for merged MRs whose target ref was deleted). It now degrades gracefully: log a warning, dropcompareBaseRef, and still return the valid source-branch base.All changes are platform-agnostic (no
metaKey/path-sep), consider SSH/remote (resolution already routes viasshGitProvider; the RPC query path already worked), and the query plumbing is GitLab-side only (GitHub already had it).Screenshots
This is a behavioral fix to the GitLab MR list/search and base-resolution plumbing. Verified end-to-end in a running Orca dev build over CDP rather than by a static screenshot — see the evidence comment below: a live
window.api.gl.listMRs({ query: 'login' })against a real gitlab.com project with three open MRs returned only the matching MR (Add login validation flow), proving the query now reaches the API and filters. Unit/integration tests cover all changed layers.Testing
pnpm lint(oxlint, scoped to changed files — clean)pnpm typecheck(tsgo node + web — clean)pnpm test(affected suites — 611 passing; see evidence comment)pnpm build--search, source-lookup query threading,resolveManagedMrBasecompare-base degradation, and the renderer error-surfacing path)AI Review Report
Reviewed the diff adversarially for correctness, edge cases, and cross-platform safety. Risks checked:
metaKey, no path-separator assumptions; the touched code routes git throughgitExec/sshGitProviderand glab throughglabExecFileAsync, identical across local/WSL/SSH. The WSL-routing IPC tests still pass with the newqueryarg (passed as the 6th positional,undefinedwhen absent).undefinedso no stray&search=/--searchis emitted; covered by tests.resolveManagedMrBasekeeps SSH/WSL/local paths identical; the compare-base degradation applies uniformly.debouncedQueryto the GitLab list-mode effect deps (it was missing);shouldQueryGitlabalready gates oversized queries viasourceQueryWithinLimit.Security Audit
execFilewith array args (no shell); the REST pathencodeURIComponents the query into the URL; the CLI fallback passes it as a discrete--search <value>arg. No injection vector.eval, no exfiltration. IPC handlers retain the existingassertRegisteredRepofilesystem-auth boundary.Notes
translate()key (Failed to resolve MR base.) and ranpnpm run sync:localization-catalog; non-English catalogs carry the English fallback pending translation, consistent with the existing workflow.glab mr listuses the deprecated--openedflag and approximates totals from the page length; both are pre-existing and unrelated to this bug.Made with Orca 🐋