Repository navigation
🦊 feat: Look Up GitLab Merge Requests Behind a Provider Setting - #16986
Conversation
cfd0663 to
a2921db
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2921db504
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const provider = resolvePullRequestProvider(settings?.provider); | ||
| if (provider == null || resolvePullRequestHost(settings?.host, provider) == null) return false; | ||
| if (settings?.token == null && resolvePullRequestToken(undefined, env, provider) == null) { |
There was a problem hiding this comment.
When the YAML selects GitLab using only GITLAB_TOKEN, a principal override that sets provider: github makes this function report the feature as disabled. Both authenticated /api/config and ToolService call it with the principal-resolved config, so affected users neither see the feature nor record lane Git data, even though handler.ts::readSettings correctly restores the YAML provider and could perform the GitLab lookup. Since this field is documented as YAML-only, those gating paths must also evaluate it using the base provider/host rather than the override.
Useful? React with 👍 / 👎.
| return routePullRequestSources({ | ||
| github: createGitHubPullRequestSource({ fetchFn }), | ||
| gitlab: createGitLabPullRequestSource({ fetchFn }), | ||
| }); |
There was a problem hiding this comment.
When an operator selects the newly schema-valid and documented bitbucket provider with BITBUCKET_PULL_REQUEST_TOKEN and a repository scope, the feature is advertised as enabled and requests reach this router, but the production source map contains no Bitbucket implementation. Every lookup consequently becomes NOT_CONFIGURED despite a configuration the schema and example file explicitly accept; either keep Bitbucket out of the provider enum/config documentation until its implementation lands or register its source here.
Useful? React with 👍 / 👎.
| provider: resolvePullRequestProvider(settings.provider) ?? undefined, | ||
| host: | ||
| resolvePullRequestHost( | ||
| settings.host, | ||
| resolvePullRequestProvider(settings.provider) ?? undefined, | ||
| ) ?? undefined, |
There was a problem hiding this comment.
When concurrent batch requests use the same token digest, repository, branch, head, limits, and scope but resolve to different providers, adding provider to the lookup input is insufficient because the cross-request shareKey later in this handler omits both input.provider and input.host. This can occur across tenant-specific base configurations or during a provider change, causing the later request to join the first provider's promise and receive its pull request result without ever executing its own lookup; include the provider and host in the shared identity just as lookup.ts now does for its cache key.
AGENTS.md reference: AGENTS.md:L187-L190
Useful? React with 👍 / 👎.
a2921db to
8896763
Compare
8896763 to
e1b5959
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1b59598ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| typeof value.sha !== 'string' || | ||
| !COMMIT_ID.test(value.sha) || |
There was a problem hiding this comment.
When GitLab has not finished preparing a merge request, or its source ref is unavailable, the API can legitimately expose an MR object with sha: null (GitLab example). Both this parser and parseListItem instead classify that expected state as UPSTREAM_ERROR, so a lookup immediately after glab mr create can return a 503, while one nullable item can abort an otherwise usable candidate page. Treat the missing SHA as a pending/nonmatching candidate or use an available diff/head ref rather than reporting provider failure.
AGENTS.md reference: AGENTS.md:L104-L107
Useful? React with 👍 / 👎.
| if (branch.length === 0) return null; | ||
| const lookup: Lookup = { | ||
| token, | ||
| origin: `https://${host}`, |
There was a problem hiding this comment.
When a self-managed GitLab instance is installed below a relative URL such as https://example.com/gitlab, which GitLab officially supports (GitLab installation documentation), the pathless host setting can only produce https://example.com here. Consequently the source requests /api/v4 and /api/graphql instead of /gitlab/api/v4 and /gitlab/api/graphql; the first 404 is interpreted as an absent project, so every merge request silently disappears. Add a YAML-configurable, validated URL prefix/base path rather than hard-coding the provider at the host root.
AGENTS.md reference: AGENTS.md:L91-L93
Useful? React with 👍 / 👎.
Pull Request
Summary
The feature only knew GitHub. This adds
endpoints.agents.pullRequests.provider(githubby default,gitlab,bitbucket) and a GitLab source for gitlab.com and self-managed hosts. Lookups go through a small router of per-provider sources, so the lookup and its cache stay provider-agnostic, and the response now names itsproviderso the chat header and card say "MR !7" and "Open in GitLab". Repository scope accepts nested group paths (group/sub/project,group/sub/*), and the lane and workspace descriptor accept them too.The GitLab source keeps GitHub's guarantees: a linked merge request is read directly, branch search is open first, a fork only counts when it carries the recorded commit (
merge_basestands in for compare), checks come from the head pipeline only when it ran for the head commit, and line counts come from one GraphQLdiffStatsSummaryquery. Likehost,provideris read from the operator's YAML only. Shared parsing and defaults moved fromgithub.tstoshared.ts. Bitbucket is wired in the next PR.Part of a stack on #16982, merge from the bottom up: host (1), GitLab (2), Bitbucket (3), shared cache (4), user OAuth backend (5), connected account setting (6). Each PR is based on the one below it, so its diff is only its own link.
Type of change
Testing
Tested environments/configuration:
Automated tests:
packages/api:npx jest src/pulls src/code/lane.spec.tspasses;gitlab.spec.tscovers mapping, nested groups on a self-managed host, page URL checks, states, conflicts, stale pipelines, GraphQL errors, rate limits, commit containment and its budget, forks and linked merge requests. Breaking the fork rule fails two tests (checked).client:npx jest src/components/Chat/PullRequestpasses, including GitLab, Bitbucket and legacy (no provider) cards.packages/api,packages/data-providerandclient, thepackages/apibuild, and the convos route specs pass.Screenshots / recordings
The card's label and link change only for GitLab and Bitbucket pull requests; GitHub cards are unchanged. No screenshot: rendering a GitLab card needs a live GitLab lane.
Risk / compatibility
A response without
provider(an older server) is treated as GitHub. Nested repository paths are now accepted from the worker's environment descriptor; the GitHub source still requiresowner/name.Checklist
pullRequestsfields are documented inlibrechat.example.yaml