fix(server): normalize full repository URLs in GitLab lookups - #7862
fix(server): normalize full repository URLs in GitLab lookups#7862Exotic209093 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| yield* providerService.interruptTurn({ threadId: event.payload.threadId }); | ||
| // Some providers discard their callbacks without emitting a matching | ||
| // resolution event. Close those requests after the interrupt succeeds. | ||
| yield* Effect.forEach(pendingUserInputRequests(thread.activities), ({ requestId, turnId }) => |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderCommandReactor.ts:1267
Interrupts can produce duplicate terminal activities for the same user-input requestId: thread.activities is the pre-interrupt snapshot, while the provider may emit user-input.resolved during interruptTurn, so this loop still appends a synthetic cancellation for an already-resolved request. Re-read the thread after interruption and append cancellations only for requests that remain unresolved, or make the completion idempotent.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 1267:
Interrupts can produce duplicate terminal activities for the same user-input `requestId`: `thread.activities` is the pre-interrupt snapshot, while the provider may emit `user-input.resolved` during `interruptTurn`, so this loop still appends a synthetic cancellation for an already-resolved request. Re-read the thread after interruption and append cancellations only for requests that remain unresolved, or make the completion idempotent.
|
|
||
| try { | ||
| const url = new URL(trimmed); | ||
| const segments = url.pathname.split("/").filter((segment) => segment.length > 0); |
There was a problem hiding this comment.
🟠 High sourceControl/GitLabCli.ts:406
For a relative-root GitLab installation, normalizeGitLabRepositoryPath includes the instance base path in the project identifier: https://example.com/gitlab/group/project becomes gitlab/group/project instead of group/project, so getRepositoryCloneUrls queries the wrong project. Remove the host's configured GitLab base path before extracting the namespace/project path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/sourceControl/GitLabCli.ts around line 406:
For a relative-root GitLab installation, `normalizeGitLabRepositoryPath` includes the instance base path in the project identifier: `https://example.com/gitlab/group/project` becomes `gitlab/group/project` instead of `group/project`, so `getRepositoryCloneUrls` queries the wrong project. Remove the host's configured GitLab base path before extracting the namespace/project path.
![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: 64c066175f
ℹ️ 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 url = new URL(trimmed); | ||
| const segments = url.pathname.split("/").filter((segment) => segment.length > 0); |
There was a problem hiding this comment.
When a user is authenticated to multiple GitLab instances and pastes a URL whose host is not the host selected by glab for the current directory, this discards url.host, so URLs such as https://gitlab.example.com/group/project and https://gitlab.com/group/project invoke the same API command and can query the wrong instance. The lookup must carry the pasted host through to the glab invocation rather than using only the pathname.
Useful? React with 👍 / 👎.
| const url = new URL(trimmed); | ||
| const segments = url.pathname.split("/").filter((segment) => segment.length > 0); |
There was a problem hiding this comment.
For a self-hosted instance configured under a relative URL root, such as https://example.com/gitlab, the valid project URL https://example.com/gitlab/group/project is normalized to gitlab/group/project; the API project identifier is actually group/project, so the lookup still fails for this supported self-hosted layout. The installation prefix needs to be distinguished from the repository namespace instead of treating every pathname segment as part of the project ID.
Useful? React with 👍 / 👎.
| yield* providerService.interruptTurn({ threadId: event.payload.threadId }); | ||
| // Some providers discard their callbacks without emitting a matching | ||
| // resolution event. Close those requests after the interrupt succeeds. | ||
| yield* Effect.forEach(pendingUserInputRequests(thread.activities), ({ requestId, turnId }) => |
There was a problem hiding this comment.
These lines introduce a separate production behavior change for every provider interruption, including synthetic user-input resolution and a dedicated reactor test, even though the stated change concerns GitLab repository URL lookup. Move this lifecycle change into its own focused change so it can be reviewed, tested, and reverted independently, as required by the repository's one-concern rule.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 64c0661. Configure here.
| turnId, | ||
| createdAt: event.payload.createdAt, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Stale interrupt cancels open prompts
Medium Severity
processTurnInterruptRequested loads thread activities once, then runs interruptTurn, then cancels every request still present in that snapshot. Providers that do emit a resolution during interrupt can already close the prompt, so this path still appends a later user-input.resolved with cancelled: true, producing a spurious cancellation after a real resolution.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 64c0661. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — Adds GitLab URL normalization with tests. Unresolved comments identify design limitations: discarding the URL hostname could route multi-host users to the wrong GitLab instance, and relative-root installations produce incorrect paths. These behavioral concerns warrant human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
64c0661 to
6133897
Compare
github-actions
Bot
added
size:M
1 participant
New-project GitLab lookup sent the entire pasted URL to \glab api projects/\ whenever the host wasn't a known one like gitlab.com, so self-hosted instances (e.g. sourcecontrol.example.com) failed on full URLs while bare \group/project\ worked.
This adds
ormalizeGitLabRepositoryPath, which parses any http(s) URL and extracts the owner/repo path generically instead of sniffing hostnames. It handles trailing slashes, .git\ suffixes, nested groups (GitLab supports them), and strips /-/...\ web UI sections, then encodes the result exactly as the bare-path case does.
Fixes #7849
ox-alpha via opencode
Note
Medium Risk
Touches orchestration interrupt handling (synthetic cancel events) and GitLab project-path parsing used for clone lookups. Logic is localized and covered by tests, but a bad parse or missed pending request could leave prompts stuck or look up the wrong project.
Overview
Two independent server fixes.
Turn interrupt now closes open user-input prompts after a successful provider interrupt. Some providers drop callbacks without a matching resolution event; the reactor scans thread activities for pending
user-input.requesteditems and appendsuser-input.resolvedwithcancelled: true.GitLab clone lookup no longer sends a full pasted URL to
glab api projects/....normalizeGitLabRepositoryPathextractsnamespace/projectfrom any http(s) host (self-hosted included), stripping.git, trailing slashes, nested groups, and/-/...web UI paths.Reviewed by Cursor Bugbot for commit 64c0661. Configure here.
Note
Normalize full repository URLs in
GitLabCli.getRepositoryCloneUrlsnormalizeGitLabRepositoryPathin GitLabCli.ts: trims input, parses HTTP(S) URLs, drops UI sections (segments starting with-), strips case-insensitive.gitsuffix, and returns thenamespace/projectpath. Bare paths pass through unchanged.GitLabCli.make.service.getRepositoryCloneUrlsnow encodes the normalized path instead ofinput.repositorydirectly, so pasted clone URLs (including self-hosted) resolve correctly viaglab api projects/<encoded path>..gitstripping, nested groups, UI subpaths, and bare-path passthrough.Macroscope summarized 6133897.