fix: accept Azure DevOps Git URLs without .git suffix in plugin marketplace#299576
fix: accept Azure DevOps Git URLs without .git suffix in plugin marketplace#299576connor4312 merged 3 commits intomainfrom
Conversation
…tplace
- Remove the `.git` suffix requirement from `normalizeGitRepoPath`; the function
now accepts any URL path that has at least two segments.
- Update `parseUriMarketplaceReference` to handle paths with and without `.git`:
- `cacheSegments` are built without the suffix in both cases.
- `canonicalId` is always normalized to include `.git` so that the same repo
specified with and without the suffix deduplicates correctly.
- Add a JSDoc comment on `normalizeGitRepoPath` explaining the new semantics.
- Update the test that expected HTTPS/SSH URLs without `.git` to be rejected;
these are now accepted. SCP-style (`git@host:path`) still requires `.git`.
- Add tests for Azure DevOps-style URLs and cross-suffix deduplication.
Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates marketplace Git URL parsing to accept Azure DevOps (and general HTTPS/SSH) clone URLs that omit the .git suffix, while normalizing canonical IDs to include .git for deduplication.
Changes:
- Relaxes
normalizeGitRepoPathby removing the hard.gitrequirement while still validating minimum path depth. - Updates URI parsing to strip
.gitfor cache segments but always include.gitincanonicalIdfor deduplication. - Updates/adds tests for Azure DevOps URLs and cross-suffix deduplication behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts | Adjusts Git URL normalization and URI parsing to accept URLs without .git while keeping canonical IDs stable. |
| src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts | Updates/extends tests to cover non-.git URLs (including Azure DevOps) and deduplication. |
You can also share your feedback on Copilot code review. Take the survey.
| const pathHasGitSuffix = normalizedPath.toLowerCase().endsWith('.git'); | ||
| const pathWithoutGit = pathHasGitSuffix ? normalizedPath.slice(1, -4) : normalizedPath.slice(1); | ||
| const pathSegments = pathWithoutGit.split('/').map(sanitizePathSegment); | ||
| // Always normalize the canonical path to include .git so that URLs with and without the suffix deduplicate. | ||
| const canonicalPath = pathHasGitSuffix ? normalizedPath.slice(1).toLowerCase() : `${normalizedPath.slice(1).toLowerCase()}.git`; |
There was a problem hiding this comment.
The .git suffix is handled via repeated string literals and the magic number -4. Consider introducing a single constant (e.g., const gitSuffix = '.git') and using gitSuffix.length in slice() calls. This reduces the risk of future inconsistencies and makes the intent clearer.
See below for a potential fix:
const gitSuffix = '.git';
const sanitizedAuthority = sanitizePathSegment(uri.authority.toLowerCase());
const pathHasGitSuffix = normalizedPath.toLowerCase().endsWith(gitSuffix);
const pathWithoutGit = pathHasGitSuffix ? normalizedPath.slice(1, normalizedPath.length - gitSuffix.length) : normalizedPath.slice(1);
const pathSegments = pathWithoutGit.split('/').map(sanitizePathSegment);
// Always normalize the canonical path to include .git so that URLs with and without the suffix deduplicate.
const canonicalPath = pathHasGitSuffix ? normalizedPath.slice(1).toLowerCase() : `${normalizedPath.slice(1).toLowerCase()}${gitSuffix}`;
There was a problem hiding this comment.
Done in 5c6077a — introduced a gitSuffix constant in both parseUriMarketplaceReference and normalizeGitRepoPath, replacing all -4 magic numbers and repeated '.git' literals.
| // Strip .git suffix (if present) only for the purposes of validating path depth. | ||
| const pathWithoutGit = withLeadingSlash.toLowerCase().endsWith('.git') | ||
| ? withLeadingSlash.slice(1, -4) | ||
| : withLeadingSlash.slice(1); |
There was a problem hiding this comment.
Same maintainability concern here: '.git' and -4 are duplicated in suffix handling. Reusing a shared gitSuffix constant and slicing with -gitSuffix.length would make this easier to maintain and less error-prone.
Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
parseMarketplaceReferencesilently rejected Azure DevOps HTTPS clone URLs (https://dev.azure.com/org/project/_git/repo) becausenormalizeGitRepoPathrequired a.gitsuffix. Users had to manually append.gitto the canonical URL provided by Azure DevOps.Changes
normalizeGitRepoPath: Removes the hard.gitrequirement. Path depth validation now strips.gitonly when present before checking for at least two segments. Uses agitSuffixconstant instead of the magic number-4for maintainability.parseUriMarketplaceReference: Handles both forms using agitSuffixconstant:cacheSegmentsstrip.gitwhen presentcanonicalIdalways normalizes to include.git, so…/repoand…/repo.gitdeduplicate to the same entry.gitto be rejected; added tests for Azure DevOps-style URLs and cross-suffix deduplication. SCP-style (git@host:path) still requires.git.Example — this now parses correctly:
And deduplicates with
https://dev.azure.com/org/project/_git/repo.gitto a single entry.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.