fix: stop truncating remote project paths that end in git

Description

parseGitLabRemote strips an optional .git suffix from the remote path with (?:.git)? in the regex at src/desktop/git/git_remote_parser.ts#L105. The dot is not escaped, so it matches any character, and because the project path group ([^/]+?) is lazy, a project path that ends in any character followed by "git" loses its last four characters when the remote URL has no .git at the end: https://gitlab.com/group/legit parses as group/l, and https://gitlab.com/group/my-git as group/my. detectProjects in gitlab_project_repository.ts and the account preselection look the project up at that path, so the repository is left without a project, or gets a different project if one exists at the truncated path. Remotes that end in .git are not affected.

This MR escapes the dot ((?:\\.git)? in the template literal, so the regex reads (?:\.git)?), so that only a literal .git suffix is stripped. It adds parser rows for remotes without the suffix whose project path ends in "git" (HTTPS, HTTPS with a trailing slash, and SCP-style SSH in a subgroup), and one with the suffix to show that it is still removed. Nothing else changes.

The language server has its own copy of this parser with the same pattern (packages/core/repositories/src/git/utils/git_remote_parser.ts#L150), so with this MR alone the features that come from the language server would still use the truncated path. gitlab-org/editor-extensions/gitlab-lsp!4523 (merged) makes the same change there (gitlab-org/editor-extensions/gitlab-lsp#3365 (closed)). The two do not depend on each other and can be merged in either order.

Closes #2366 (closed)

How has this been tested?

  • In node:22.20.0, the image CI uses, on this branch: npm ci with its install scripts, npm run lint, npm run test:unit -- --coverage (291 suites, 2531 tests) and npx tsc --noEmit -p tsconfig.json all pass, and scripts/commit-lint/lint.js accepts both the title (squash on) and the commit.
  • The new rows in src/desktop/git/git_remote_parser.test.ts fail on main without the fix (4 of 34 tests; for example projectPath is "l" instead of "legit") and pass with it. The fifth row, https://gitlab.com/group/my-git.git, passes on both and keeps the suffix removal covered for a path that ends in "git".
  • npm run test:integration, set up as the test-integration job does (its packages and Xvfb on :99, in the same image), on this branch and on main: 69 passing and 1 pending on both.
  • I have not tried it in a running VS Code; the integration suite's workspace remote ends in .git, so it does not exercise this case.
  • src/browser and src/common are not modified.
Parser test before and after the fix

Before, on main at 2fb019cf with this MR's test file, for https://gitlab.com/group/legit:

    - Expected  - 2
    + Received  + 2

      Object {
        "host": "gitlab.com",
        "hostname": "gitlab.com",
        "namespace": "group",
    -   "namespaceWithPath": "group/legit",
    -   "projectPath": "legit",
    +   "namespaceWithPath": "group/l",
    +   "projectPath": "l",
        "protocol": "https:",
      }

Tests:       4 failed, 30 passed, 34 total

The four rows that fail on main:

Remote namespaceWithPath on main With this MR
https://gitlab.com/group/legit group/l group/legit
https://gitlab.com/group/my-git group/my group/my-git
https://gitlab.com/group/my-git/ group/my group/my-git
git@gitlab.com:group/subgroup/my_git group/subgroup/my group/subgroup/my_git

After:

Tests:       34 passed, 34 total

The tests of the parser and its callers (npx jest src/desktop/git src/desktop/gitlab/gitlab_project_repository src/desktop/accounts/account_preselection_service src/desktop/accounts/authenticate_command): 35 suites, 298 tests, all passing.

Screenshots (if appropriate)

What CHANGELOG entry will this MR create?

  • fix: Bug fix fixes - a user-facing issue in production - included in changelog
  • feat: New feature - a user-facing change which adds functionality - included in changelog
  • BREAKING CHANGE: (fix or feature that would cause existing functionality to change) - should bump major version, mentioned in the changelog
  • None - other non-user-facing changes
Edited by José M. Requena Plens

Merge request reports

Loading
Loading