From 57d237550a568865c7c8d83cba856b0291487642 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 14:36:13 +0200 Subject: [PATCH 01/17] =?UTF-8?q?=E2=9C=A8=20feat(node-utils):=20tell=20wh?= =?UTF-8?q?ether=20a=20git=20remote=20and=20a=20provider=20URL=20are=20on?= =?UTF-8?q?=20the=20same=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gitHostOf reads the host of any remote or provider URL (https, scp-like or ssh:// SSH, with a user, a port or a path prefix); sameGitHost compares two of them. extractBaseUrl no longer returns a whole ssh:// remote or keeps a user, which created one CLI-managed provider per repository. Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../node-utils/src/git/extractBaseUrl.spec.ts | 46 +++++++++ packages/node-utils/src/git/extractBaseUrl.ts | 15 +-- packages/node-utils/src/git/gitHost.spec.ts | 98 +++++++++++++++++++ packages/node-utils/src/git/gitHost.ts | 38 +++++++ packages/node-utils/src/git/index.ts | 1 + 5 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 packages/node-utils/src/git/extractBaseUrl.spec.ts create mode 100644 packages/node-utils/src/git/gitHost.spec.ts create mode 100644 packages/node-utils/src/git/gitHost.ts diff --git a/packages/node-utils/src/git/extractBaseUrl.spec.ts b/packages/node-utils/src/git/extractBaseUrl.spec.ts new file mode 100644 index 0000000000..f207e5a7d2 --- /dev/null +++ b/packages/node-utils/src/git/extractBaseUrl.spec.ts @@ -0,0 +1,46 @@ +import { extractBaseUrl } from './extractBaseUrl'; + +describe('extractBaseUrl', () => { + describe.each([ + [ + 'an https remote', + 'https://gitlab.acme.io/acme/app.git', + 'https://gitlab.acme.io', + ], + [ + 'an http remote', + 'http://gitlab.acme.io/acme/app.git', + 'http://gitlab.acme.io', + ], + [ + 'an https remote with a port', + 'https://gitlab.acme.io:8443/acme/app.git', + 'https://gitlab.acme.io:8443', + ], + [ + 'a remote carrying a user', + 'https://jdoe@gitlab.acme.io/acme/app.git', + 'https://gitlab.acme.io', + ], + [ + 'an scp-like SSH remote', + 'git@gitlab.acme.io:acme/app.git', + 'https://gitlab.acme.io', + ], + [ + 'an ssh:// remote with a port', + 'ssh://git@gitlab.acme.io:2222/acme/app.git', + 'https://gitlab.acme.io', + ], + ])('with %s', (_label, remote, expected) => { + it(`returns ${expected}`, () => { + expect(extractBaseUrl(remote)).toBe(expected); + }); + }); + + describe('when no host can be read', () => { + it('returns the input', () => { + expect(extractBaseUrl('not a remote')).toBe('not a remote'); + }); + }); +}); diff --git a/packages/node-utils/src/git/extractBaseUrl.ts b/packages/node-utils/src/git/extractBaseUrl.ts index 07a80ac621..fba28b0f08 100644 --- a/packages/node-utils/src/git/extractBaseUrl.ts +++ b/packages/node-utils/src/git/extractBaseUrl.ts @@ -1,17 +1,20 @@ +import { gitHostOf } from './gitHost'; + /** * The host part of a git remote URL, always as `https://host`: an SSH remote is * rewritten to that form so the result can be compared against a stored git - * provider URL whatever scheme the remote used. + * provider URL whatever scheme the remote used. A user in the remote is + * dropped; the port of an http(s) remote is kept, that of an SSH one is not. */ export function extractBaseUrl(gitRemoteUrl: string): string { - const httpsMatch = gitRemoteUrl.match(/^(https?:\/\/[^/]+)/i); + const httpsMatch = gitRemoteUrl.match(/^(https?:\/\/)(?:[^@/]*@)?([^/]+)/i); if (httpsMatch) { - return httpsMatch[1]; + return `${httpsMatch[1]}${httpsMatch[2]}`; } - const sshMatch = gitRemoteUrl.match(/^git@([^:]+):/i); - if (sshMatch) { - return `https://${sshMatch[1]}`; + const host = gitHostOf(gitRemoteUrl); + if (host) { + return `https://${host}`; } // Neither shape matched: hand back the input rather than throwing. diff --git a/packages/node-utils/src/git/gitHost.spec.ts b/packages/node-utils/src/git/gitHost.spec.ts new file mode 100644 index 0000000000..8fcad2717d --- /dev/null +++ b/packages/node-utils/src/git/gitHost.spec.ts @@ -0,0 +1,98 @@ +import { gitHostOf, sameGitHost } from './gitHost'; + +describe('gitHostOf', () => { + describe.each([ + ['an https remote', 'https://gitlab.acme.io/acme/app.git'], + ['an http remote', 'http://gitlab.acme.io/acme/app.git'], + ['an scp-like SSH remote', 'git@gitlab.acme.io:acme/app.git'], + [ + 'an ssh:// remote with a port', + 'ssh://git@gitlab.acme.io:2222/acme/app.git', + ], + ['a git+ssh:// remote', 'git+ssh://git@gitlab.acme.io/acme/app.git'], + ['a remote carrying a user', 'https://jdoe@gitlab.acme.io/acme/app.git'], + [ + 'a remote carrying credentials', + 'https://jdoe:secret@gitlab.acme.io/acme/app.git', + ], + ['a provider URL with a trailing slash', 'https://gitlab.acme.io/'], + ['a provider URL with the default port', 'https://gitlab.acme.io:443'], + ['a provider URL with a path prefix', 'https://gitlab.acme.io/gitlab'], + ['a bare host', 'gitlab.acme.io'], + ['a host in another case', 'https://GitLab.Acme.IO'], + ])('with %s', (_label, url) => { + it('returns the lowercased host', () => { + expect(gitHostOf(url)).toBe('gitlab.acme.io'); + }); + }); + + describe.each([ + ['null', null], + ['undefined', undefined], + ['an empty string', ''], + ['blanks', ' '], + ])('with %s', (_label, url) => { + it('returns null', () => { + expect(gitHostOf(url)).toBeNull(); + }); + }); +}); + +describe('sameGitHost', () => { + describe('when an SSH remote points at the provider host', () => { + it('returns true', () => { + expect( + sameGitHost( + 'https://gitlab.acme.io', + 'git@gitlab.acme.io:acme/app.git', + ), + ).toBe(true); + }); + }); + + describe('when the provider URL carries a path prefix', () => { + it('returns true', () => { + expect( + sameGitHost( + 'https://devtools.acme.io/gitlab', + 'https://devtools.acme.io/gitlab/acme/app.git', + ), + ).toBe(true); + }); + }); + + describe('when the provider URL is a whole ssh:// remote', () => { + it('returns true', () => { + expect( + sameGitHost( + 'ssh://git@gitlab.acme.io:2222/acme/app.git', + 'https://gitlab.acme.io', + ), + ).toBe(true); + }); + }); + + describe.each([ + ['a lookalike host', 'https://gitlab.acme.io.evil.com'], + ['the parent domain', 'https://acme.io'], + ['a sibling subdomain', 'https://git.acme.io'], + ])('when the remote is on %s', (_label, remote) => { + it('returns false', () => { + expect(sameGitHost('https://gitlab.acme.io', remote)).toBe(false); + }); + }); + + describe('when the provider URL is null', () => { + it('returns false', () => { + expect(sameGitHost(null, 'https://gitlab.acme.io/acme/app.git')).toBe( + false, + ); + }); + }); + + describe('when neither host can be read', () => { + it('returns false', () => { + expect(sameGitHost('', '')).toBe(false); + }); + }); +}); diff --git a/packages/node-utils/src/git/gitHost.ts b/packages/node-utils/src/git/gitHost.ts new file mode 100644 index 0000000000..f15387d159 --- /dev/null +++ b/packages/node-utils/src/git/gitHost.ts @@ -0,0 +1,38 @@ +// `scheme://[user@]host[:port]/...`: https, http, ssh, git+ssh alike. +const SCHEME_URL = /^[a-z][a-z0-9+.-]*:\/\/(?:[^@/]*@)?([^/:?#]+)/i; +// scp-like SSH, `[user@]host:path`, which git writes without a scheme. +const SCP_LIKE = /^(?:[^@/\s]+@)?([^/:\s]+):(?!\/\/)/; +// A bare `host[:port][/path]`, as an admin may type a provider URL. +const BARE_HOST = /^([^/:@\s]+)(?::\d+)?(?:\/|$)/; + +/** + * The host of a git remote or of a git provider URL, lowercased, without + * scheme, user or port — or null when none can be read. The port is dropped + * on purpose: an SSH remote reaches the same instance as its web URL on + * another port. + */ +export function gitHostOf(url: string | null | undefined): string | null { + const trimmed = url?.trim(); + if (!trimmed) { + return null; + } + const match = + trimmed.match(SCHEME_URL) ?? + trimmed.match(SCP_LIKE) ?? + trimmed.match(BARE_HOST); + return match ? match[1].toLowerCase() : null; +} + +/** + * Whether a git provider URL and a git remote (or another provider URL) point + * at the same instance. Compares hosts only: a path prefix on the provider + * URL, the scheme, the user and the port do not matter. False when either + * host cannot be read. + */ +export function sameGitHost( + providerUrl: string | null | undefined, + gitRemoteUrl: string | null | undefined, +): boolean { + const providerHost = gitHostOf(providerUrl); + return providerHost !== null && providerHost === gitHostOf(gitRemoteUrl); +} diff --git a/packages/node-utils/src/git/index.ts b/packages/node-utils/src/git/index.ts index 80a48dce86..8b7e428f81 100644 --- a/packages/node-utils/src/git/index.ts +++ b/packages/node-utils/src/git/index.ts @@ -1,5 +1,6 @@ export * from './extractBaseUrl'; export * from './gitBlobSha'; +export * from './gitHost'; export * from './parseGitProviderVendor'; export * from './parseGitRepoInfo'; export * from './stripGitRemoteCredentials'; From dc8bd6d2d3bdcafdc425e7f2a98bcc6f2848f6ae Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 14:48:18 +0200 Subject: [PATCH 02/17] =?UTF-8?q?=E2=9C=A8=20feat(git):=20resolve=20a=20CL?= =?UTF-8?q?I=20remote=20against=20the=20providers=20of=20its=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote decides which providers are candidates, by host, and the vendor a CLI sends is only trusted when it sends no remote. A self-hosted remote now reaches the token provider an admin configured instead of always falling back to a CLI-managed one, and that fallback warns when a token provider of the host exists. Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../FindOrCreateGitRepoUseCase.spec.ts | 197 ++++++++++++++++++ .../FindOrCreateGitRepoUseCase.ts | 171 ++++++++------- .../useCases/shared/providerHostUrl.ts | 17 ++ 3 files changed, 311 insertions(+), 74 deletions(-) create mode 100644 packages/git/src/application/useCases/shared/providerHostUrl.ts diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts index 6fa8ac0a35..fc68dd9315 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts @@ -6,6 +6,7 @@ import { createUserId, FindOrCreateGitRepoCommand, GitProvider, + GitProviderListItem, GitProviderVendors, GitRepo, IAccountsPort, @@ -174,4 +175,200 @@ describe('FindOrCreateGitRepoUseCase', () => { expect(result).toEqual(createdRepo); }); }); + + describe('when the remote is on a self-hosted instance', () => { + const tokenProviderId = createGitProviderId(uuidv4()); + const ghostProviderId = createGitProviderId(uuidv4()); + const selfHostedCommand: FindOrCreateGitRepoCommand = { + ...command, + providerVendor: 'unknown', + gitRemoteUrl: 'https://gitlab.acme.io/acme/widgets.git', + }; + let createdRepo: GitRepo; + + const listItem = ( + overrides: Partial, + ): GitProviderListItem => ({ + id: tokenProviderId, + source: GitProviderVendors.gitlab, + organizationId, + url: 'https://gitlab.acme.io', + authMethod: 'token', + displayName: '', + hasAuth: true, + lastDistributionAt: null, + ...overrides, + }); + + const reachable = (owner = 'acme', name = 'widgets') => ({ + currentPage: 1, + availablePages: 1, + lastLoadedPage: 1, + partial: false, + repositories: [ + { owner, name, private: true, defaultBranch: 'dev', stars: 0 }, + ], + }); + + beforeEach(() => { + createdRepo = gitRepoFactory({ owner: 'acme', repo: 'widgets' }); + mockGitPort.listRepos.mockResolvedValue([]); + mockGitPort.addGitRepo.mockResolvedValue(createdRepo); + mockGitPort.addGitProvider.mockImplementation( + async ({ gitProvider }) => ({ + ...gitProvider, + id: createGitProviderId(uuidv4()), + organizationId, + }), + ); + }); + + describe.each([ + ['an https remote', 'https://gitlab.acme.io/acme/widgets.git'], + ['an SSH remote', 'git@gitlab.acme.io:acme/widgets.git'], + ])( + 'and an authenticated provider is on the host, with %s', + (_label, gitRemoteUrl) => { + beforeEach(async () => { + mockGitPort.listProviders.mockResolvedValue({ + providers: [listItem({})], + }); + mockGitPort.listAvailableRepos.mockResolvedValue(reachable()); + + await useCase.execute({ ...selfHostedCommand, gitRemoteUrl }); + }); + + it('creates the repository under that provider', () => { + expect(mockGitPort.addGitRepo).toHaveBeenCalledWith( + expect.objectContaining({ gitProviderId: tokenProviderId }), + ); + }); + + it('creates no tokenless provider', () => { + expect(mockGitPort.addGitProvider).not.toHaveBeenCalled(); + }); + }, + ); + + describe('and the authenticated provider is on another host', () => { + beforeEach(async () => { + mockGitPort.listProviders.mockResolvedValue({ + providers: [listItem({ url: 'https://gitlab.other.io' })], + }); + + await useCase.execute(selfHostedCommand); + }); + + it('does not probe it', () => { + expect(mockGitPort.listAvailableRepos).not.toHaveBeenCalled(); + }); + + it('creates a tokenless provider for the host', () => { + expect(mockGitPort.addGitProvider).toHaveBeenCalledWith( + expect.objectContaining({ + gitProvider: expect.objectContaining({ + source: 'unknown', + url: 'https://gitlab.acme.io', + token: null, + }), + }), + ); + }); + }); + + describe('and a tokenless provider was stored with a whole ssh:// remote', () => { + beforeEach(async () => { + mockGitPort.listProviders.mockResolvedValue({ + providers: [ + listItem({ + id: ghostProviderId, + source: GitProviderVendors.unknown, + url: 'ssh://git@gitlab.acme.io:2222/acme/other.git', + hasAuth: false, + }), + ], + }); + + await useCase.execute(selfHostedCommand); + }); + + it('reuses it', () => { + expect(mockGitPort.addGitRepo).toHaveBeenCalledWith( + expect.objectContaining({ gitProviderId: ghostProviderId }), + ); + }); + + it('creates no tokenless provider', () => { + expect(mockGitPort.addGitProvider).not.toHaveBeenCalled(); + }); + }); + + describe('and the CLI sends a wrong vendor', () => { + beforeEach(async () => { + mockGitPort.listProviders.mockResolvedValue({ providers: [] }); + + await useCase.execute({ + ...selfHostedCommand, + providerVendor: 'github', + }); + }); + + it('records the host of the remote, not github.com', () => { + expect(mockGitPort.addGitProvider).toHaveBeenCalledWith( + expect.objectContaining({ + gitProvider: expect.objectContaining({ + source: 'unknown', + url: 'https://gitlab.acme.io', + }), + }), + ); + }); + }); + }); + + describe('when an old CLI sends a vendor but no remote', () => { + const tokenProviderId = createGitProviderId(uuidv4()); + + beforeEach(async () => { + mockGitPort.listProviders.mockResolvedValue({ + providers: [ + { + id: tokenProviderId, + source: GitProviderVendors.github, + organizationId, + url: null, + authMethod: 'app', + displayName: '', + hasAuth: true, + lastDistributionAt: null, + }, + ], + }); + mockGitPort.listRepos.mockResolvedValue([]); + mockGitPort.listAvailableRepos.mockResolvedValue({ + currentPage: 1, + availablePages: 1, + lastLoadedPage: 1, + partial: false, + repositories: [ + { + owner: 'acme', + name: 'widgets', + private: true, + defaultBranch: 'dev', + stars: 0, + }, + ], + }); + mockGitPort.addGitRepo.mockResolvedValue(gitRepoFactory()); + + await useCase.execute({ ...command, gitRemoteUrl: undefined }); + }); + + it('resolves the providers of that vendor', () => { + expect(mockGitPort.addGitRepo).toHaveBeenCalledWith( + expect.objectContaining({ gitProviderId: tokenProviderId }), + ); + }); + }); }); diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts index 6380dfc168..4880687607 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts @@ -11,7 +11,13 @@ import { IGitPort, UnresolvableGitProviderError, } from '@packmind/types'; -import { extractBaseUrl, parseGitProviderVendor } from '@packmind/node-utils'; +import { + extractBaseUrl, + parseGitProviderVendor, + sameGitHost, +} from '@packmind/node-utils'; +import { isProbeableSource } from '../shared/probeCandidateCredentials'; +import { providerHostUrl } from '../shared/providerHostUrl'; const origin = 'FindOrCreateGitRepoUseCase'; @@ -42,12 +48,12 @@ export class FindOrCreateGitRepoUseCase const { owner, repo, branch, organization, userId } = command; const gitRemoteUrl = command.gitRemoteUrl; - const providerVendor: GitProviderVendor = - command.providerVendor !== undefined - ? (command.providerVendor as GitProviderVendor) - : gitRemoteUrl - ? parseGitProviderVendor(gitRemoteUrl) - : 'unknown'; + // The remote is the server's own evidence; the vendor a CLI sends is only + // trusted from CLIs that predate sending the remote. + const providerVendor: GitProviderVendor = gitRemoteUrl + ? parseGitProviderVendor(gitRemoteUrl) + : ((command.providerVendor as GitProviderVendor | undefined) ?? + 'unknown'); const organizationId = organization.id; @@ -63,81 +69,94 @@ export class FindOrCreateGitRepoUseCase organizationId, }); - const vendorProviders = providersResponse.providers.filter( - (p) => p.source === providerVendor, + // A provider's URL states which instance it reaches, so a self-hosted + // remote finds the provider an admin configured for it, whatever vendor a + // substring of the remote suggests. + const hostProviders = providersResponse.providers.filter((p) => + gitRemoteUrl + ? sameGitHost(providerHostUrl(p), gitRemoteUrl) + : p.source === providerVendor, + ); + const tokenProviders = hostProviders.filter( + (p) => p.hasAuth && isProbeableSource(p.source), ); - // Unknown vendors expose no API to list repositories, so there is nothing - // to probe a token against. - if (providerVendor !== 'unknown') { - const tokenProviders = vendorProviders.filter((p) => p.hasAuth); - - // Every provider is probed before anything is created: creating on the - // first match would raise a duplicate-repo error when a later provider - // already hosts the repo. - type ProviderInfo = (typeof tokenProviders)[number]; - const providersWithAccess: ProviderInfo[] = []; + // Every provider is probed before anything is created: creating on the + // first match would raise a duplicate-repo error when a later provider + // already hosts the repo. + type ProviderInfo = (typeof tokenProviders)[number]; + const providersWithAccess: ProviderInfo[] = []; + + for (const provider of tokenProviders) { + const existingRepos = await this.gitPort.listRepos(provider.id); + const existingRepo = existingRepos.find( + (r) => + r.owner.toLowerCase() === owner.toLowerCase() && + r.repo.toLowerCase() === repo.toLowerCase() && + r.branch === branch, + ); + + if (existingRepo) { + this.logger.info('Found existing repo under token provider', { + providerId: provider.id, + repoId: existingRepo.id, + }); + return existingRepo; + } - for (const provider of tokenProviders) { - const existingRepos = await this.gitPort.listRepos(provider.id); - const existingRepo = existingRepos.find( + try { + const availableRepos = await this.gitPort.listAvailableRepos({ + gitProviderId: provider.id, + userId, + organizationId, + }); + const canAccess = availableRepos.repositories.some( (r) => r.owner.toLowerCase() === owner.toLowerCase() && - r.repo.toLowerCase() === repo.toLowerCase() && - r.branch === branch, + r.name.toLowerCase() === repo.toLowerCase(), ); - if (existingRepo) { - this.logger.info('Found existing repo under token provider', { - providerId: provider.id, - repoId: existingRepo.id, - }); - return existingRepo; - } - - try { - const availableRepos = await this.gitPort.listAvailableRepos({ - gitProviderId: provider.id, - userId, - organizationId, - }); - const canAccess = availableRepos.repositories.some( - (r) => - r.owner.toLowerCase() === owner.toLowerCase() && - r.name.toLowerCase() === repo.toLowerCase(), - ); - - if (canAccess) { - providersWithAccess.push(provider); - } - } catch (error) { - this.logger.info('Failed to list available repos for provider', { - providerId: provider.id, - error: error instanceof Error ? error.message : String(error), - }); - // Swallowed: this provider's token may be expired or revoked, which - // only rules out this provider. + if (canAccess) { + providersWithAccess.push(provider); } - } - - if (providersWithAccess.length > 0) { - const provider = providersWithAccess[0]; - this.logger.info( - 'Token can access repo, creating under token provider', - { providerId: provider.id }, - ); - return this.gitPort.addGitRepo({ - userId, - organizationId, - gitProviderId: provider.id, - owner, - repo, - branch, + } catch (error) { + this.logger.info('Failed to list available repos for provider', { + providerId: provider.id, + error: error instanceof Error ? error.message : String(error), }); + // Swallowed: this provider's token may be expired or revoked, which + // only rules out this provider. } } - this.logger.info('No token provider has access, falling back to tokenless'); + if (providersWithAccess.length > 0) { + const provider = providersWithAccess[0]; + this.logger.info('Token can access repo, creating under token provider', { + providerId: provider.id, + }); + return this.gitPort.addGitRepo({ + userId, + organizationId, + gitProviderId: provider.id, + owner, + repo, + branch, + }); + } + + if (tokenProviders.length > 0) { + this.logger.warn( + 'No authenticated provider of the host can reach the repository, falling back to a CLI-managed provider', + { + organizationId, + gitProviderIds: tokenProviders.map((p) => p.id), + }, + ); + } else { + this.logger.info( + 'No token provider has access, falling back to tokenless', + ); + } let expectedProviderUrl: string; if (providerVendor === 'github') { @@ -150,11 +169,15 @@ export class FindOrCreateGitRepoUseCase throw new UnresolvableGitProviderError(owner, repo); } - let tokenlessProvider = vendorProviders.find( - (p) => - !p.hasAuth && - p.url?.toLowerCase() === expectedProviderUrl.toLowerCase(), + // Older servers stored a whole ssh:// remote as the URL, so the host is + // what matches; an exact URL still wins when several providers qualify. + const tokenlessProviders = hostProviders.filter( + (p) => !p.hasAuth && sameGitHost(p.url, expectedProviderUrl), ); + let tokenlessProvider = + tokenlessProviders.find( + (p) => p.url?.toLowerCase() === expectedProviderUrl.toLowerCase(), + ) ?? tokenlessProviders[0]; if (!tokenlessProvider) { const newProvider = await this.gitPort.addGitProvider({ diff --git a/packages/git/src/application/useCases/shared/providerHostUrl.ts b/packages/git/src/application/useCases/shared/providerHostUrl.ts new file mode 100644 index 0000000000..7311fc2cbd --- /dev/null +++ b/packages/git/src/application/useCases/shared/providerHostUrl.ts @@ -0,0 +1,17 @@ +import { GitProvider } from '@packmind/types'; + +const DEFAULT_HOST_BY_SOURCE: Partial> = { + github: 'https://github.com', + gitlab: 'https://gitlab.com', +}; + +/** + * The URL of the instance a provider reaches. GitHub providers, App installs + * included, may store no URL: their API client always targets github.com, as + * GitLab's falls back to gitlab.com. + */ +export function providerHostUrl( + provider: Pick, +): string | null { + return provider.url || DEFAULT_HOST_BY_SOURCE[provider.source] || null; +} From 05b296885d2eab3e484274e65c9f11a371743130 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 14:48:30 +0200 Subject: [PATCH 03/17] =?UTF-8?q?=E2=9C=A8=20feat(git):=20adopt=20a=20self?= =?UTF-8?q?-hosted=20CLI-managed=20repository=20into=20its=20token=20provi?= =?UTF-8?q?der?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adoption compares hosts instead of sources, since the CLI records a self-hosted instance under an 'unknown' provider. A CLI-managed provider emptied by an adoption is removed, so the repository only shows under the token connection. Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../src/application/GitRepoService.spec.ts | 35 + .../git/src/application/GitRepoService.ts | 8 + .../addGitRepo/AddGitRepoUseCase.spec.ts | 54 +- .../useCases/addGitRepo/AddGitRepoUseCase.ts | 36 +- .../src/self-hosted-repo-adoption.spec.ts | 685 ++++++++++++++++++ 5 files changed, 794 insertions(+), 24 deletions(-) create mode 100644 packages/integration-tests/src/self-hosted-repo-adoption.spec.ts diff --git a/packages/git/src/application/GitRepoService.spec.ts b/packages/git/src/application/GitRepoService.spec.ts index 3c902d8017..66b93911ef 100644 --- a/packages/git/src/application/GitRepoService.spec.ts +++ b/packages/git/src/application/GitRepoService.spec.ts @@ -200,6 +200,41 @@ describe('GitRepoService', () => { }); }); + describe('hasGitRepos', () => { + describe('when the provider holds a repository of any type', () => { + beforeEach(() => { + mockGitRepoRepository.findByProviderId.mockResolvedValue([ + { ...mockGitRepo, type: 'marketplace' }, + ]); + }); + + it('returns true', async () => { + expect( + await gitRepoService.hasGitRepos(createGitProviderId('provider-1')), + ).toBe(true); + }); + + it('counts every repository type', async () => { + await gitRepoService.hasGitRepos(createGitProviderId('provider-1')); + + expect(mockGitRepoRepository.findByProviderId).toHaveBeenCalledWith( + createGitProviderId('provider-1'), + { type: 'any' }, + ); + }); + }); + + describe('when the provider holds no repository', () => { + it('returns false', async () => { + mockGitRepoRepository.findByProviderId.mockResolvedValue([]); + + expect( + await gitRepoService.hasGitRepos(createGitProviderId('provider-1')), + ).toBe(false); + }); + }); + }); + describe('findGitReposByOrganizationId', () => { const mockRepos = [mockGitRepo]; let result: GitRepo[]; diff --git a/packages/git/src/application/GitRepoService.ts b/packages/git/src/application/GitRepoService.ts index a9cc4ce38e..b6fc81d82e 100644 --- a/packages/git/src/application/GitRepoService.ts +++ b/packages/git/src/application/GitRepoService.ts @@ -72,6 +72,14 @@ export class GitRepoService { }); } + // Marketplace repositories count too: a provider holding one still serves. + async hasGitRepos(providerId: GitProviderId): Promise { + const gitRepos = await this.gitRepoRepository.findByProviderId(providerId, { + type: 'any', + }); + return gitRepos.length > 0; + } + async findGitReposByOrganizationId( organizationId: OrganizationId, ): Promise { diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts index 11281d9e98..36ffa7d391 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts @@ -42,6 +42,7 @@ describe('AddGitRepoUseCase', () => { beforeEach(() => { mockGitProviderService = { findGitProviderById: jest.fn(), + deleteGitProvider: jest.fn(), } as Partial< jest.Mocked > as jest.Mocked; @@ -50,6 +51,7 @@ describe('AddGitRepoUseCase', () => { findGitRepoByOwnerRepoAndBranchInOrganization: jest.fn(), addGitRepo: jest.fn(), adoptGitRepo: jest.fn(), + hasGitRepos: jest.fn(), } as Partial> as jest.Mocked; mockDeploymentPort = { @@ -704,6 +706,7 @@ describe('AddGitRepoUseCase', () => { existingRepo, ); mockGitRepoService.adoptGitRepo.mockResolvedValue(adoptedRepo); + mockGitRepoService.hasGitRepos.mockResolvedValue(false); }); describe('when the authenticated provider targets the same host', () => { @@ -737,6 +740,26 @@ describe('AddGitRepoUseCase', () => { it('creates no new target', () => { expect(mockDeploymentPort.addTarget).not.toHaveBeenCalled(); }); + + it('removes the emptied CLI-managed provider', () => { + expect(mockGitProviderService.deleteGitProvider).toHaveBeenCalledWith( + holdingProviderId, + userId, + ); + }); + }); + + describe('when the CLI-managed provider still holds other repositories', () => { + beforeEach(async () => { + mockGitRepoService.hasGitRepos.mockResolvedValue(true); + givenProviders(authenticatedProvider(), cliManagedProvider()); + + await addRepo(); + }); + + it('keeps the CLI-managed provider', () => { + expect(mockGitProviderService.deleteGitProvider).not.toHaveBeenCalled(); + }); }); describe('when a GitHub App installation stores no URL', () => { @@ -780,11 +803,36 @@ describe('AddGitRepoUseCase', () => { }); }); - describe('when the providers are of different vendors', () => { + describe('when the CLI recorded a self-hosted instance as an unknown vendor', () => { + beforeEach(async () => { + givenProviders( + authenticatedProvider({ url: 'https://gitlab.acme.io' }), + cliManagedProvider({ + source: GitProviderVendors.unknown, + url: 'https://gitlab.acme.io', + }), + ); + + await addRepo(); + }); + + it('adopts the repository', () => { + expect(mockGitRepoService.adoptGitRepo).toHaveBeenCalledWith( + existingRepo, + gitProviderId, + organizationId, + ); + }); + }); + + describe('when the providers are on different hosts', () => { it('throws GitRepoAlreadyExistsError', async () => { givenProviders( - authenticatedProvider(), - cliManagedProvider({ source: GitProviderVendors.unknown }), + authenticatedProvider({ url: 'https://gitlab.acme.io' }), + cliManagedProvider({ + source: GitProviderVendors.unknown, + url: 'https://gitlab.acme.io.evil.com', + }), ); await expect(addRepo()).rejects.toThrow(GitRepoAlreadyExistsError); diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts index f2ff44da24..f8c5926346 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts @@ -1,8 +1,8 @@ import { PackmindLogger } from '@packmind/logger'; import { AbstractMemberUseCase, - extractBaseUrl, MemberContext, + sameGitHost, } from '@packmind/node-utils'; import { AddGitRepoCommand, @@ -20,40 +20,25 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; +import { providerHostUrl } from '../shared/providerHostUrl'; const origin = 'AddGitRepoUseCase'; -const DEFAULT_HOST_BY_SOURCE: Partial> = { - github: 'https://github.com', - gitlab: 'https://gitlab.com', -}; - -// GitHub providers, App installs included, may store no URL: their API client -// always targets github.com, as GitLab's falls back to gitlab.com. -function hostOf(provider: GitProvider): string | null { - const url = provider.url - ? extractBaseUrl(provider.url) - : DEFAULT_HOST_BY_SOURCE[provider.source]; - return url?.toLowerCase() ?? null; -} - /** * A repository the CLI recorded before the organization connected an * authenticated provider for the same host moves under it, rather than - * blocking it as a duplicate. + * blocking it as a duplicate. The host decides, not the source: the CLI + * records a self-hosted instance under an `unknown` provider. */ function isAdoptableBy( holdingProvider: GitProvider, gitProvider: GitProvider, ): boolean { - const host = hostOf(gitProvider); return ( holdingProvider.id !== gitProvider.id && !providerHasAuth(holdingProvider) && providerHasAuth(gitProvider) && - holdingProvider.source === gitProvider.source && - host !== null && - hostOf(holdingProvider) === host + sameGitHost(providerHostUrl(gitProvider), providerHostUrl(holdingProvider)) ); } @@ -158,11 +143,20 @@ export class AddGitRepoUseCase fromGitProviderId: holdingProvider.id, toGitProviderId: gitProvider.id, }); - return this.gitRepoService.adoptGitRepo( + const adoptedRepo = await this.gitRepoService.adoptGitRepo( existingRepo, gitProvider.id, organization.id, ); + // An emptied CLI-managed provider would stay listed beside the + // connection that now holds its repositories. + if (!(await this.gitRepoService.hasGitRepos(holdingProvider.id))) { + await this.gitProviderService.deleteGitProvider( + holdingProvider.id, + createUserId(userId), + ); + } + return adoptedRepo; } this.logger.error('Repository already exists in organization', { diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts new file mode 100644 index 0000000000..23ba5ba310 --- /dev/null +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -0,0 +1,685 @@ +import { PackmindLogger } from '@packmind/logger'; +import { + GitCommitSchema, + GitProviderSchema, + GitRepoSchema, +} from '@packmind/git'; +import { + gitCommitFactory, + gitProviderFactory, + gitRepoFactory, +} from '@packmind/git/test'; +import { + DistributionHistoryEntry, + GitCommit, + GitProvider, + GitProviderId, + GitRepo, + GitRepoAlreadyExistsError, + Package, + Target, + UnresolvableGitProviderError, + createGitProviderId, +} from '@packmind/types'; +import { createIntegrationTestFixture } from './helpers/createIntegrationTestFixture'; +import { DataFactory } from './helpers/DataFactory'; +import { integrationTestSchemas } from './helpers/makeIntegrationTestDataSource'; +import { TestApp } from './helpers/TestApp'; + +const HOST = 'https://gitlab.acme.io'; +const OWNER = 'acme'; +const REPO = 'app'; +const BRANCH = 'main'; +const HTTPS_REMOTE = 'https://gitlab.acme.io/acme/app.git'; + +type Coordinate = { owner?: string; repo?: string; branch?: string }; + +/** + * A self-hosted GitLab first used through `packmind` CLI sessions only, then + * connected with a token: adding a repository on the token connection must + * move it there with its history, and the CLI must keep finding it there. + * The CLI of today sends `providerVendor: 'unknown'` for any self-hosted host. + */ +describe('Self-hosted CLI-managed repository adoption integration', () => { + const fixture = createIntegrationTestFixture(integrationTestSchemas); + + let testApp: TestApp; + let admin: DataFactory; + let distributedPackage: Package; + let commit: GitCommit; + let accessibleRepos: Map; + let warnSpy: jest.SpyInstance; + + beforeAll(async () => { + await fixture.initialize(); + + testApp = new TestApp(fixture.datasource); + await testApp.initialize(); + + admin = new DataFactory(testApp); + await admin.withUserAndOrganization({ email: 'admin@example.com' }); + + const standard = await admin.withStandard({ name: 'Adopted Standard' }); + const { package: created } = await testApp.deploymentsHexa + .getAdapter() + .createPackage({ + ...admin.packmindCommand(), + spaceId: admin.space.id, + name: 'Adopted Package', + description: 'Distributed before the token connection existed', + recipeIds: [], + standardIds: [standard.id], + skillIds: [], + }); + distributedPackage = created; + + commit = await fixture.datasource + .getRepository(GitCommitSchema) + .save(gitCommitFactory()); + + fixture.snapshot(); + }); + + // The publish job runs inline and the self-hosted API is not reachable from + // the tests: commits and repository listings are stubbed. Spies are + // restored after each test, hence beforeEach rather than beforeAll. + beforeEach(() => { + accessibleRepos = new Map(); + const gitAdapter = testApp.gitHexa.getAdapter(); + jest.spyOn(gitAdapter, 'commitToGit').mockResolvedValue(commit); + jest + .spyOn(gitAdapter, 'listAvailableRepos') + .mockImplementation(async ({ gitProviderId }) => { + const repos = accessibleRepos.get(gitProviderId); + if (repos === undefined) { + throw new Error('Request failed with status code 401'); + } + return { + currentPage: 1, + availablePages: 1, + lastLoadedPage: 1, + partial: false, + repositories: repos.map((fullName) => { + const [owner, name] = fullName.split('/'); + return { + owner, + name, + private: true, + defaultBranch: BRANCH, + stars: 0, + }; + }), + }; + }); + warnSpy = jest.spyOn(PackmindLogger.prototype, 'warn'); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + await fixture.cleanup(); + }); + + afterAll(() => fixture.destroy()); + + function recordFromCli( + options: Coordinate & { + remote?: string; + providerVendor?: string; + by?: DataFactory; + } = {}, + ): Promise { + const by = options.by ?? admin; + return testApp.gitHexa.getAdapter().findOrCreateGitRepo({ + ...by.packmindCommand(), + owner: options.owner ?? OWNER, + repo: options.repo ?? REPO, + branch: options.branch ?? BRANCH, + providerVendor: options.providerVendor ?? 'unknown', + gitRemoteUrl: options.remote ?? HTTPS_REMOTE, + }); + } + + function trackFromCli(): Promise { + return testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: OWNER, + repo: REPO, + branch: BRANCH, + origin: 'track', + providerVendor: 'unknown', + gitRemoteUrl: HTTPS_REMOTE, + }); + } + + // Saved straight through the schema: connecting it through the use case + // would probe the token against the self-hosted instance. + async function connectTokenProvider( + url = HOST, + options: { accessTo?: string[] } = {}, + ): Promise { + const provider = await fixture.datasource + .getRepository(GitProviderSchema) + .save( + gitProviderFactory({ + organizationId: admin.organization.id, + source: 'gitlab', + url, + token: 'glpat-self-hosted', + authMethod: 'token', + }), + ); + accessibleRepos.set(provider.id, options.accessTo ?? [`${OWNER}/${REPO}`]); + return provider; + } + + // A ghost as older servers left it, written without going through the CLI. + async function saveGhost(url: string, repos: Coordinate[]) { + const ghost = await fixture.datasource + .getRepository(GitProviderSchema) + .save( + gitProviderFactory({ + organizationId: admin.organization.id, + source: 'unknown', + url, + token: null, + authMethod: 'token', + }), + ); + const saved = await Promise.all( + repos.map((coordinate) => + fixture.datasource.getRepository(GitRepoSchema).save( + gitRepoFactory({ + providerId: createGitProviderId(ghost.id), + owner: coordinate.owner ?? OWNER, + repo: coordinate.repo ?? REPO, + branch: coordinate.branch ?? BRANCH, + }), + ), + ), + ); + return { ghost, repos: saved }; + } + + function addFromApp( + provider: GitProvider, + coordinate: Coordinate = {}, + ): Promise { + return testApp.gitHexa.getAdapter().addGitRepo({ + ...admin.packmindCommand(), + gitProviderId: provider.id, + owner: coordinate.owner ?? OWNER, + repo: coordinate.repo ?? REPO, + branch: coordinate.branch ?? BRANCH, + }); + } + + async function providerIds(by: DataFactory = admin): Promise { + const { providers } = await testApp.gitHexa.getAdapter().listProviders({ + ...by.packmindCommand(), + organizationId: by.organization.id, + }); + return providers.map((provider) => provider.id); + } + + async function currentProviderOf(gitRepo: GitRepo): Promise { + const stored = await fixture.datasource + .getRepository(GitRepoSchema) + .findOneByOrFail({ id: gitRepo.id }); + return stored.providerId; + } + + function targetsOf(gitRepo: GitRepo): Promise { + return testApp.deploymentsHexa.getAdapter().getTargetsByGitRepo({ + ...admin.packmindCommand(), + gitRepoId: gitRepo.id, + }); + } + + async function distributeTo(gitRepo: GitRepo): Promise { + const [target] = await targetsOf(gitRepo); + await testApp.deploymentsHexa.getAdapter().publishPackages({ + ...admin.packmindCommand(), + packageIds: [distributedPackage.id], + targetIds: [target.id], + }); + } + + async function historyRepoIds(): Promise<(string | undefined)[]> { + const history: DistributionHistoryEntry[] = await testApp.deploymentsHexa + .getAdapter() + .listDeploymentsByPackage({ + ...admin.packmindCommand(), + organizationId: admin.organization.id, + spaceId: admin.space.id, + packageId: distributedPackage.id, + }); + return history.map((entry) => entry.target.gitRepo?.id); + } + + describe('when the admin adds the CLI repository on the token connection', () => { + let cliRepo: GitRepo; + let cliTargets: Target[]; + let token: GitProvider; + let adopted: GitRepo; + + beforeEach(async () => { + cliRepo = await recordFromCli(); + cliTargets = await targetsOf(cliRepo); + await distributeTo(cliRepo); + + token = await connectTokenProvider(); + adopted = await addFromApp(token); + }); + + it('keeps the repository id', () => { + expect(adopted.id).toBe(cliRepo.id); + }); + + it('moves the repository under the token connection', () => { + expect(adopted.providerId).toBe(token.id); + }); + + it('keeps the targets', async () => { + expect(await targetsOf(adopted)).toEqual(cliTargets); + }); + + it('keeps the distribution history', async () => { + expect(await historyRepoIds()).toEqual([cliRepo.id]); + }); + + it('no longer lists the CLI-managed connection', async () => { + expect(await providerIds()).toEqual([token.id]); + }); + }); + + describe('when the CLI runs again after the adoption', () => { + let cliRepo: GitRepo; + let token: GitProvider; + let again: GitRepo; + + beforeEach(async () => { + cliRepo = await recordFromCli(); + token = await connectTokenProvider(); + await addFromApp(token); + + again = await recordFromCli(); + }); + + it('returns the adopted repository', () => { + expect(again.id).toBe(cliRepo.id); + }); + + it('does not recreate a CLI-managed connection', async () => { + expect(await providerIds()).toEqual([token.id]); + }); + + it('lets `packmind track` track it', async () => { + expect((await trackFromCli()).id).toBe(cliRepo.id); + }); + }); + + describe('when the CLI first sees the repository after the token connection exists', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(); + repo = await recordFromCli(); + }); + + it('creates it under the token connection', () => { + expect(repo.providerId).toBe(token.id); + }); + + it('creates no CLI-managed connection', async () => { + expect(await providerIds()).toEqual([token.id]); + }); + }); + + describe.each([ + ['an SSH remote', 'git@gitlab.acme.io:acme/app.git', HOST], + [ + 'an ssh:// remote with a port', + 'ssh://git@gitlab.acme.io:2222/acme/app.git', + HOST, + ], + [ + 'a remote carrying a user', + 'https://jdoe@gitlab.acme.io/acme/app.git', + HOST, + ], + [ + 'a remote host in another case', + 'https://GitLab.Acme.io/acme/app.git', + HOST, + ], + [ + 'a provider URL with a trailing slash', + HTTPS_REMOTE, + 'https://gitlab.acme.io/', + ], + [ + 'a provider URL with the default port', + HTTPS_REMOTE, + 'https://gitlab.acme.io:443', + ], + [ + 'a provider URL with a path prefix', + 'https://devtools.acme.io/gitlab/acme/app.git', + 'https://devtools.acme.io/gitlab', + ], + ])('when the CLI used %s', (_label, remote, providerUrl) => { + let cliRepo: GitRepo; + let adopted: GitRepo; + + beforeEach(async () => { + cliRepo = await recordFromCli({ remote }); + adopted = await addFromApp(await connectTokenProvider(providerUrl)); + }); + + it('adopts the same repository', () => { + expect(adopted.id).toBe(cliRepo.id); + }); + }); + + // Older servers stored the whole ssh:// remote as the provider URL. + describe('when the ghost connection was stored with a whole ssh:// remote as its URL', () => { + let ghostRepo: GitRepo; + let token: GitProvider; + let adopted: GitRepo; + + beforeEach(async () => { + ({ + repos: [ghostRepo], + } = await saveGhost('ssh://git@gitlab.acme.io:2222/acme/app.git', [{}])); + token = await connectTokenProvider(); + adopted = await addFromApp(token); + }); + + it('adopts the repository', () => { + expect(adopted).toMatchObject({ id: ghostRepo.id, providerId: token.id }); + }); + }); + + describe.each([ + ['a lookalike host', 'https://gitlab.acme.io.evil.com'], + ['the parent domain', 'https://acme.io'], + ['a sibling subdomain', 'https://git.acme.io'], + ])('when the token connection is on %s', (_label, providerUrl) => { + let cliRepo: GitRepo; + let token: GitProvider; + + beforeEach(async () => { + cliRepo = await recordFromCli(); + token = await connectTokenProvider(providerUrl); + }); + + it('refuses the repository as already existing', async () => { + await expect(addFromApp(token)).rejects.toBeInstanceOf( + GitRepoAlreadyExistsError, + ); + }); + + it('keeps the CLI repository under its CLI-managed connection', async () => { + await addFromApp(token).catch(() => undefined); + + expect(await currentProviderOf(cliRepo)).toBe(cliRepo.providerId); + }); + }); + + describe('when two token connections share the host', () => { + let second: GitProvider; + let adopted: GitRepo; + let again: GitRepo; + + beforeEach(async () => { + await recordFromCli(); + await connectTokenProvider(); + second = await connectTokenProvider(); + + adopted = await addFromApp(second); + again = await recordFromCli(); + }); + + it('adopts into the connection the admin picked', () => { + expect(adopted.providerId).toBe(second.id); + }); + + it('lets the CLI find it under that connection', () => { + expect(again).toMatchObject({ id: adopted.id, providerId: second.id }); + }); + }); + + describe('when the token cannot list repositories on the first CLI run', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(); + accessibleRepos.delete(token.id); + + repo = await recordFromCli(); + }); + + it('falls back to a CLI-managed connection', () => { + expect(repo.providerId).not.toBe(token.id); + }); + + it('warns that a token connection exists on the host', () => { + expect(warnSpy).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ gitProviderIds: [token.id] }), + ); + }); + }); + + describe('when the token has no access to the repository on the first CLI run', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(HOST, { accessTo: ['acme/other'] }); + + repo = await recordFromCli(); + }); + + it('falls back to a CLI-managed connection', () => { + expect(repo.providerId).not.toBe(token.id); + }); + }); + + describe('when the CLI-managed connection holds other repositories', () => { + let other: GitRepo; + + beforeEach(async () => { + await recordFromCli(); + other = await recordFromCli({ repo: 'other' }); + + await addFromApp(await connectTokenProvider()); + }); + + it('keeps listing the CLI-managed connection', async () => { + expect(await providerIds()).toContain(other.providerId); + }); + + it('leaves the other repository under it', async () => { + expect(await currentProviderOf(other)).toBe(other.providerId); + }); + }); + + describe('when the CLI-managed connection holds two branches of the repository', () => { + let develop: GitRepo; + let token: GitProvider; + + beforeEach(async () => { + await recordFromCli(); + develop = await recordFromCli({ branch: 'develop' }); + + token = await connectTokenProvider(); + await addFromApp(token); + }); + + it('keeps listing the CLI-managed connection while develop is on it', async () => { + expect(await providerIds()).toContain(develop.providerId); + }); + + describe('and the CLI runs on develop', () => { + let developAgain: GitRepo; + + beforeEach(async () => { + developAgain = await recordFromCli({ branch: 'develop' }); + }); + + it('adopts develop too, keeping its id', () => { + expect(developAgain).toMatchObject({ + id: develop.id, + providerId: token.id, + }); + }); + + it('no longer lists the CLI-managed connection', async () => { + expect(await providerIds()).toEqual([token.id]); + }); + }); + }); + + describe('when two CLI-managed connections exist for the host', () => { + let app: GitRepo; + let lib: GitRepo; + let token: GitProvider; + + beforeEach(async () => { + ({ + repos: [app], + } = await saveGhost(HOST, [{}])); + ({ + repos: [lib], + } = await saveGhost('ssh://git@gitlab.acme.io:2222/acme/lib.git', [ + { repo: 'lib' }, + ])); + + token = await connectTokenProvider(HOST, { + accessTo: ['acme/app', 'acme/lib'], + }); + await addFromApp(token); + await addFromApp(token, { repo: 'lib' }); + }); + + it('adopts from each of them', async () => { + expect([ + await currentProviderOf(app), + await currentProviderOf(lib), + ]).toEqual([token.id, token.id]); + }); + + it('no longer lists either of them', async () => { + expect(await providerIds()).toEqual([token.id]); + }); + }); + + describe('when the CLI-managed connection belongs to another organization', () => { + let otherOrgRepo: GitRepo; + let repo: GitRepo; + + beforeEach(async () => { + const otherAdmin = new DataFactory(testApp); + await otherAdmin.withUserAndOrganization({ email: 'other@example.com' }); + otherOrgRepo = await recordFromCli({ by: otherAdmin }); + + repo = await addFromApp(await connectTokenProvider()); + }); + + it('creates a new repository in this organization', () => { + expect(repo.id).not.toBe(otherOrgRepo.id); + }); + + it('leaves the other organization repository where it was', async () => { + expect(await currentProviderOf(otherOrgRepo)).toBe( + otherOrgRepo.providerId, + ); + }); + }); + + describe('when the CLI spelled the repository with another case', () => { + let cliRepo: GitRepo; + let adopted: GitRepo; + + beforeEach(async () => { + cliRepo = await recordFromCli({ owner: 'Acme', repo: 'App' }); + adopted = await addFromApp(await connectTokenProvider()); + }); + + it('adopts the same repository', () => { + expect(adopted.id).toBe(cliRepo.id); + }); + }); + + describe('when the repository was tracked with `packmind track`', () => { + let tracked: GitRepo; + + beforeEach(async () => { + tracked = await trackFromCli(); + await addFromApp(await connectTokenProvider()); + }); + + it('stays tracked after the adoption', async () => { + const { gitRepo } = await testApp.gitHexa + .getAdapter() + .getTrackedRepository({ + ...admin.packmindCommand(), + owner: OWNER, + repo: REPO, + }); + + expect(gitRepo?.id).toBe(tracked.id); + }); + }); + + describe('when its tracking had been removed', () => { + let tracked: GitRepo; + + beforeEach(async () => { + tracked = await trackFromCli(); + await distributeTo(tracked); + await testApp.gitHexa.getAdapter().removeTrackedRepository({ + ...admin.packmindCommand(), + owner: OWNER, + repo: REPO, + }); + + await addFromApp(await connectTokenProvider()); + }); + + it('shows its history again', async () => { + expect(await historyRepoIds()).toEqual([tracked.id]); + }); + }); + + describe('when an old CLI sends no remote URL', () => { + it('still refuses the repository as unresolvable', async () => { + await expect( + testApp.gitHexa.getAdapter().findOrCreateGitRepo({ + ...admin.packmindCommand(), + owner: OWNER, + repo: REPO, + branch: BRANCH, + providerVendor: 'unknown', + }), + ).rejects.toBeInstanceOf(UnresolvableGitProviderError); + }); + }); + + describe('when a CLI sends a wrong vendor for the self-hosted remote', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(); + repo = await recordFromCli({ providerVendor: 'github' }); + }); + + it('resolves by host and uses the token connection', () => { + expect(repo.providerId).toBe(token.id); + }); + }); +}); From 8f57dc1d87ba5ef40011acde6aa540b17e5747c0 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 15:55:51 +0200 Subject: [PATCH 04/17] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(cli):=20let?= =?UTF-8?q?=20the=20server=20work=20out=20the=20git=20provider=20from=20th?= =?UTF-8?q?e=20remote?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #887 Co-Authored-By: Claude Opus 5.5 --- apps/cli/CHANGELOG.MD | 2 ++ .../TrackRepositoryUseCase.spec.ts | 1 - .../trackRepository/TrackRepositoryUseCase.ts | 17 ----------------- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/apps/cli/CHANGELOG.MD b/apps/cli/CHANGELOG.MD index aa664a7013..38db1d5e54 100644 --- a/apps/cli/CHANGELOG.MD +++ b/apps/cli/CHANGELOG.MD @@ -8,6 +8,8 @@ ## Changed +- `packmind track` and `packmind init` no longer send a provider vendor guessed from the remote URL: the server works it out from the remote, which is what lets a self-managed GitLab use its connection + ## Fixed - The CLI drops any token embedded in the repository's remote URL before sending it to Packmind diff --git a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.spec.ts b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.spec.ts index 93fe6be810..f4a07ee7bc 100644 --- a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.spec.ts +++ b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.spec.ts @@ -82,7 +82,6 @@ describe('TrackRepositoryUseCase', () => { repo: 'my-repo', branch: 'dev', origin: 'track', - providerVendor: 'github', gitRemoteUrl: REMOTE_URL, }); }); diff --git a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts index 1a7795256d..bb7f41f954 100644 --- a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts +++ b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts @@ -1,4 +1,3 @@ -import { GitProviderVendor } from '@packmind/types'; import { ITrackRepositoryUseCase, TrackRepositoryCommand, @@ -27,20 +26,6 @@ export function parseOwnerRepo(gitRemoteUrl: string): { }; } -/** - * Infer the git provider vendor from a remote URL. - */ -function parseProviderVendor(gitRemoteUrl: string): GitProviderVendor { - const normalized = gitRemoteUrl.toLowerCase(); - if (normalized.includes('github.com')) { - return 'github'; - } - if (normalized.includes('gitlab.com')) { - return 'gitlab'; - } - return 'unknown'; -} - /** * Orchestrates setting or moving the tracked repository+branch. Shared by both * the `track` command and the `init` tracking prompt. Business-only: no console @@ -74,7 +59,6 @@ export class TrackRepositoryUseCase implements ITrackRepositoryUseCase { this.gitService.getCurrentBranch(repoPath); const branch = requestedBranch ?? currentBranch; const { owner, repo } = parseOwnerRepo(gitRemoteUrl); - const providerVendor = parseProviderVendor(gitRemoteUrl); // Falling back to the checked-out branch is meaningless with a detached // HEAD: git names it `HEAD`, and tracking that would record nothing under a @@ -199,7 +183,6 @@ export class TrackRepositoryUseCase implements ITrackRepositoryUseCase { repo, branch, origin, - providerVendor, gitRemoteUrl, }); return { status: 'set', owner, repo, branch, gitRepo }; From 7c1f29e4ae42affeeb2c7bc301323922ebc22693 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 15:55:54 +0200 Subject: [PATCH 05/17] =?UTF-8?q?=F0=9F=93=9D=20docs:=20explain=20how=20a?= =?UTF-8?q?=20connection=20takes=20over=20repositories=20the=20CLI=20recor?= =?UTF-8?q?ded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #887 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.MD | 5 +++++ apps/doc/governance/distribution.mdx | 4 +++- apps/doc/governance/git-repository-connection.mdx | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index cf614bc7dc..6baee5ffcd 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -19,6 +19,11 @@ - A clone URL embedding a token (`https://oauth2:@host`) no longer leaks it into server logs or the Git connections page - Remotes stored with such a token are cleaned by a migration +- On a self-managed GitLab, `packmind install`, `init` and `track` now use the connection configured for the instance instead of a CLI-managed one +- That connection is matched by host, so a remote cloned over HTTPS or SSH reaches it alike +- Adding a repository the CLI already recorded on that instance moves it under the connection with its targets and history, instead of refusing it as a duplicate +- A CLI-managed connection is no longer listed once it holds no repository +- A repository cloned over `ssh://` or with a user in its URL no longer gets a CLI-managed connection of its own - Opening a component from All components keeps the rows already picked, and closing it goes back to that list rather than to the package carrying the component ## Removed diff --git a/apps/doc/governance/distribution.mdx b/apps/doc/governance/distribution.mdx index 5519857f39..0ec2740e9c 100644 --- a/apps/doc/governance/distribution.mdx +++ b/apps/doc/governance/distribution.mdx @@ -110,6 +110,8 @@ When you distribute packages using the CLI, Packmind automatically creates a Git Connection](/governance/git-repository-connection) for the full setup. +Connecting a provider later does not lose what the CLI recorded. Once a connection exists for the same host — github.com, gitlab.com or your self-managed instance — the CLI records new repositories under it, and adding a repository the CLI already recorded moves it there with its targets and distribution history. The CLI-created entry disappears from your Git settings once it holds no repository. + ### When to Use CLI Distribution The CLI approach is ideal for: @@ -117,7 +119,7 @@ The CLI approach is ideal for: - **CI/CD pipelines** - Automate distribution as part of your deployment process - **Local development** - Quickly install packages without leaving your terminal - **Monorepos** - Use `packmind install` to install packages for all `packmind.json` files in the repository -- **Self-hosted Git instances** - Distribute to repositories that aren't connected to the app +- **Repositories without a connection** - Distribute to repositories that aren't connected to the app ## Distribution History and the Tracked Branch diff --git a/apps/doc/governance/git-repository-connection.mdx b/apps/doc/governance/git-repository-connection.mdx index 10cab21635..51fafe4e69 100644 --- a/apps/doc/governance/git-repository-connection.mdx +++ b/apps/doc/governance/git-repository-connection.mdx @@ -85,6 +85,8 @@ GitLab connections use a personal access token: 2. Create a new token with the `api` scope (full API access). 3. Copy your token (it starts with `glpat-`) and paste it into Packmind. +For a self-managed GitLab, enter your instance URL (for example `https://gitlab.acme.io`). Packmind matches repositories to the connection by host, so a clone over HTTPS or SSH reaches the same connection. + ## Managing Repository Access To change which repositories a GitHub App connection can reach, open the connection from the **Connections** tab and use **View Packmind on GitHub**. This opens the Packmind app's page on GitHub, where you can add or remove repositories. The new access takes effect in Packmind without reconnecting. @@ -95,6 +97,8 @@ Once you've added your providers, **add repositories** for each provider. When you add a Git repository, Packmind automatically creates a default target with the root path "/" for that repository. This allows you to immediately start distributing standards and commands to the entire repository. You can later create additional targets for specific paths within the repository if needed. +A repository the CLI already distributed to keeps its history when you add it: it moves under the connection with its existing targets instead of being created again, and the CLI-created entry for that host disappears once it holds no repository. + ## Distribution Targets Before distributing your standards and commands, you can configure targets in **Settings** → **Distribution** → **Targets**. A target defines a specific path within your Git repository where standards and commands will be distributed. From a727fd57cc7ec36d16df9ab9c1d8a97f38132b12 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 16:37:44 +0200 Subject: [PATCH 06/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20keep=20the=20w?= =?UTF-8?q?hole=20group=20path=20of=20a=20GitLab=20subgroup=20remote?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote parser kept only its last two path segments, so promyze/sandbox/quentin-nuxt became sandbox/quentin-nuxt and a repository tracked from the CLI never matched the one a token connection lists. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.MD | 1 + apps/cli/CHANGELOG.MD | 2 + .../trackRepository/TrackRepositoryUseCase.ts | 22 ++++++++--- .../trackRepository/parseOwnerRepo.spec.ts | 32 ++++++++++++++++ .../src/git/parseGitRepoInfo.spec.ts | 38 +++++++++++++++++++ .../node-utils/src/git/parseGitRepoInfo.ts | 31 ++++++++++----- 6 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts create mode 100644 packages/node-utils/src/git/parseGitRepoInfo.spec.ts diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 6baee5ffcd..d17420224a 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -25,6 +25,7 @@ - A CLI-managed connection is no longer listed once it holds no repository - A repository cloned over `ssh://` or with a user in its URL no longer gets a CLI-managed connection of its own - Opening a component from All components keeps the rows already picked, and closing it goes back to that list rather than to the package carrying the component +- A GitLab repository in a subgroup keeps its full path (`group/subgroup/repo`) when tracked from the CLI, so a token or app connection links it instead of duplicating it ## Removed diff --git a/apps/cli/CHANGELOG.MD b/apps/cli/CHANGELOG.MD index 38db1d5e54..4b21ab4113 100644 --- a/apps/cli/CHANGELOG.MD +++ b/apps/cli/CHANGELOG.MD @@ -18,6 +18,8 @@ # [0.36.1] - 2026-09-28 +- `packmind track`, `init` and `install` keep the full group path of a GitLab remote (`group/subgroup/repo`) instead of its last two segments + ## Added - Errors are now recorded with their stack in `~/.packmind/error.log`, and network failures with the request and cause behind them diff --git a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts index bb7f41f954..2bdee038a2 100644 --- a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts +++ b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts @@ -6,23 +6,35 @@ import { import { IRepositoryTrackingGateway } from '../../../domain/repositories/IRepositoryTrackingGateway'; import { IGitService } from '../../../domain/services/IGitService'; +// `scheme://[user@]host[:port]/path`: https, http, ssh, git+ssh alike. +const SCHEME_URL_PATH = /^[a-z][a-z0-9+.-]*:\/\/[^/]+\/(.+)$/i; +// scp-like SSH, `[user@]host:path`, which git writes without a scheme. +const SCP_LIKE_PATH = /^(?:[^@/\s]+@)?[^/:\s]+:(?!\/\/)(.+)$/; + /** - * Parse a git remote URL to extract owner and repo. + * Parse a git remote URL to extract owner and repo. The repo is the last path + * segment and the owner everything before it, so a GitLab subgroup stays whole. * Mirrors the backend `parseGitRepoInfo` helper so both sides agree. */ export function parseOwnerRepo(gitRemoteUrl: string): { owner: string; repo: string; } { - const match = gitRemoteUrl.match(/[/:]([^/:]+)\/([^/]+?)(?:\.git)?\/?$/i); + const path = (gitRemoteUrl.trim().match(SCHEME_URL_PATH) ?? + gitRemoteUrl.trim().match(SCP_LIKE_PATH))?.[1]; + + const segments = path + ?.replace(/\/+$/, '') + .replace(/\.git$/i, '') + .split('/'); - if (!match) { + if (!segments || segments.length < 2 || segments.some((s) => s === '')) { throw new Error(`Unable to parse git remote URL: ${gitRemoteUrl}`); } return { - owner: match[1], - repo: match[2].replace(/\.git$/, ''), + owner: segments.slice(0, -1).join('/'), + repo: segments[segments.length - 1], }; } diff --git a/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts b/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts new file mode 100644 index 0000000000..6b451209c9 --- /dev/null +++ b/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts @@ -0,0 +1,32 @@ +import { parseOwnerRepo } from './TrackRepositoryUseCase'; + +describe('parseOwnerRepo', () => { + describe('when the remote has a GitLab subgroup', () => { + it('keeps the whole group path as owner for an HTTPS remote', () => { + expect( + parseOwnerRepo('https://gitlab.com/promyze/sandbox/quentin-nuxt.git'), + ).toEqual({ owner: 'promyze/sandbox', repo: 'quentin-nuxt' }); + }); + + it('keeps the whole group path as owner for an scp-like SSH remote', () => { + expect( + parseOwnerRepo('git@gitlab.com:promyze/sandbox/quentin-nuxt.git'), + ).toEqual({ owner: 'promyze/sandbox', repo: 'quentin-nuxt' }); + }); + }); + + it('parses a plain owner/repo remote', () => { + expect(parseOwnerRepo('https://github.com/owner/repo.git')).toEqual({ + owner: 'owner', + repo: 'repo', + }); + }); + + describe('when the remote has no owner', () => { + it('throws', () => { + expect(() => parseOwnerRepo('https://github.com/repo')).toThrow( + 'Unable to parse git remote URL: https://github.com/repo', + ); + }); + }); +}); diff --git a/packages/node-utils/src/git/parseGitRepoInfo.spec.ts b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts new file mode 100644 index 0000000000..4955d4967c --- /dev/null +++ b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts @@ -0,0 +1,38 @@ +import { parseGitRepoInfo } from './parseGitRepoInfo'; + +describe('parseGitRepoInfo', () => { + describe('when the remote has a GitLab subgroup', () => { + it('keeps the whole group path as owner for an HTTPS remote', () => { + expect( + parseGitRepoInfo('https://gitlab.com/promyze/sandbox/quentin-nuxt.git'), + ).toEqual({ owner: 'promyze/sandbox', repo: 'quentin-nuxt' }); + }); + + it('keeps the whole group path as owner for an scp-like SSH remote', () => { + expect( + parseGitRepoInfo('git@gitlab.com:promyze/sandbox/quentin-nuxt.git'), + ).toEqual({ owner: 'promyze/sandbox', repo: 'quentin-nuxt' }); + }); + + it('keeps the whole group path as owner for an ssh:// remote with a port', () => { + expect( + parseGitRepoInfo('ssh://git@gitlab.example.com:2222/a/b/c/repo/'), + ).toEqual({ owner: 'a/b/c', repo: 'repo' }); + }); + }); + + it('parses a plain owner/repo remote', () => { + expect(parseGitRepoInfo('https://github.com/owner/repo.git')).toEqual({ + owner: 'owner', + repo: 'repo', + }); + }); + + describe('when the remote has no owner', () => { + it('throws', () => { + expect(() => parseGitRepoInfo('https://github.com/repo')).toThrow( + 'Unable to parse git remote URL: https://github.com/repo', + ); + }); + }); +}); diff --git a/packages/node-utils/src/git/parseGitRepoInfo.ts b/packages/node-utils/src/git/parseGitRepoInfo.ts index b7d54ec35d..3824fcae97 100644 --- a/packages/node-utils/src/git/parseGitRepoInfo.ts +++ b/packages/node-utils/src/git/parseGitRepoInfo.ts @@ -1,6 +1,13 @@ +// `scheme://[user@]host[:port]/path`: https, http, ssh, git+ssh alike. +const SCHEME_URL_PATH = /^[a-z][a-z0-9+.-]*:\/\/[^/]+\/(.+)$/i; +// scp-like SSH, `[user@]host:path`, which git writes without a scheme. +const SCP_LIKE_PATH = /^(?:[^@/\s]+@)?[^/:\s]+:(?!\/\/)(.+)$/; + /** - * Owner and repo from a git remote URL, for any host. Throws when the URL does - * not end in `owner/repo`. + * Owner and repo from a git remote URL, for any host. The repo is the last + * path segment; the owner is everything before it, so a GitLab subgroup + * (`group/subgroup/repo`) stays whole. Throws when the URL does not end in + * `owner/repo`. */ export function parseGitRepoInfo(gitRemoteUrl: string): { owner: string; @@ -8,14 +15,20 @@ export function parseGitRepoInfo(gitRemoteUrl: string): { } { // Accepts HTTPS (`https://host/owner/repo`) and SSH (`git@host:owner/repo`) // alike, with or without a `.git` suffix or a trailing slash. - const match = gitRemoteUrl.match(/[/:]([^/:]+)\/([^/]+?)(?:\.git)?\/?$/i); + const path = (gitRemoteUrl.trim().match(SCHEME_URL_PATH) ?? + gitRemoteUrl.trim().match(SCP_LIKE_PATH))?.[1]; + + const segments = path + ?.replace(/\/+$/, '') + .replace(/\.git$/i, '') + .split('/'); - if (match) { - return { - owner: match[1], - repo: match[2].replace(/\.git$/, ''), - }; + if (!segments || segments.length < 2 || segments.some((s) => s === '')) { + throw new Error(`Unable to parse git remote URL: ${gitRemoteUrl}`); } - throw new Error(`Unable to parse git remote URL: ${gitRemoteUrl}`); + return { + owner: segments.slice(0, -1).join('/'), + repo: segments[segments.length - 1], + }; } From 8de4e423c0e207086127ca2cc20f90888ebddec8 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 16:47:55 +0200 Subject: [PATCH 07/17] =?UTF-8?q?=F0=9F=9A=A8=20fix(integration-tests):=20?= =?UTF-8?q?stop=20asserting=20on=20the=20logger=20in=20the=20self-hosted?= =?UTF-8?q?=20adoption=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It imported @packmind/logger, which the package does not depend on, and asserting on logger output goes against the backend test standard. Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../src/self-hosted-repo-adoption.spec.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts index 23ba5ba310..dbe075144c 100644 --- a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -1,4 +1,3 @@ -import { PackmindLogger } from '@packmind/logger'; import { GitCommitSchema, GitProviderSchema, @@ -48,7 +47,6 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { let distributedPackage: Package; let commit: GitCommit; let accessibleRepos: Map; - let warnSpy: jest.SpyInstance; beforeAll(async () => { await fixture.initialize(); @@ -111,7 +109,6 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }), }; }); - warnSpy = jest.spyOn(PackmindLogger.prototype, 'warn'); }); afterEach(async () => { @@ -464,13 +461,6 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { it('falls back to a CLI-managed connection', () => { expect(repo.providerId).not.toBe(token.id); }); - - it('warns that a token connection exists on the host', () => { - expect(warnSpy).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ gitProviderIds: [token.id] }), - ); - }); }); describe('when the token has no access to the repository on the first CLI run', () => { From ff028636fd1f03859d7dac0fdb52279a25aec32c Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 17:26:33 +0200 Subject: [PATCH 08/17] =?UTF-8?q?=F0=9F=90=9B=20fix(node-utils):=20read=20?= =?UTF-8?q?a=20bracketed=20IPv6=20host=20as=20a=20whole?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #887 Co-Authored-By: Claude Opus 5.5 --- packages/node-utils/src/git/gitHost.spec.ts | 35 +++++++++++++++++++++ packages/node-utils/src/git/gitHost.ts | 11 +++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/node-utils/src/git/gitHost.spec.ts b/packages/node-utils/src/git/gitHost.spec.ts index 8fcad2717d..4b6725cf12 100644 --- a/packages/node-utils/src/git/gitHost.spec.ts +++ b/packages/node-utils/src/git/gitHost.spec.ts @@ -26,6 +26,19 @@ describe('gitHostOf', () => { }); }); + describe.each([ + ['an https IPv6 remote', 'https://[2001:db8::1]/acme/app.git'], + [ + 'an https IPv6 remote with a port', + 'https://[2001:db8::1]:8443/acme/app.git', + ], + ['an scp-like IPv6 SSH remote', 'git@[2001:db8::1]:acme/app.git'], + ])('with %s', (_label, url) => { + it('returns the bracketed address', () => { + expect(gitHostOf(url)).toBe('[2001:db8::1]'); + }); + }); + describe.each([ ['null', null], ['undefined', undefined], @@ -82,6 +95,28 @@ describe('sameGitHost', () => { }); }); + describe('when two remotes are on different IPv6 hosts', () => { + it('returns false', () => { + expect( + sameGitHost( + 'https://[2001:db8::1]/acme/app.git', + 'https://[2001:db8::2]/acme/app.git', + ), + ).toBe(false); + }); + }); + + describe('when an SSH remote points at the provider IPv6 host', () => { + it('returns true', () => { + expect( + sameGitHost( + 'https://[2001:DB8::1]:8443', + 'ssh://git@[2001:db8::1]:2222/acme/app.git', + ), + ).toBe(true); + }); + }); + describe('when the provider URL is null', () => { it('returns false', () => { expect(sameGitHost(null, 'https://gitlab.acme.io/acme/app.git')).toBe( diff --git a/packages/node-utils/src/git/gitHost.ts b/packages/node-utils/src/git/gitHost.ts index f15387d159..faf2fb5dad 100644 --- a/packages/node-utils/src/git/gitHost.ts +++ b/packages/node-utils/src/git/gitHost.ts @@ -1,9 +1,14 @@ +// An IPv6 literal is bracketed and holds colons, so it cannot end at one. +const HOST = String.raw`(\[[0-9a-f:.]+\]|[^/:?#@\s\[\]]+)`; // `scheme://[user@]host[:port]/...`: https, http, ssh, git+ssh alike. -const SCHEME_URL = /^[a-z][a-z0-9+.-]*:\/\/(?:[^@/]*@)?([^/:?#]+)/i; +const SCHEME_URL = new RegExp( + String.raw`^[a-z][a-z0-9+.-]*:\/\/(?:[^@/]*@)?${HOST}`, + 'i', +); // scp-like SSH, `[user@]host:path`, which git writes without a scheme. -const SCP_LIKE = /^(?:[^@/\s]+@)?([^/:\s]+):(?!\/\/)/; +const SCP_LIKE = new RegExp(String.raw`^(?:[^@/\s]+@)?${HOST}:(?!\/\/)`, 'i'); // A bare `host[:port][/path]`, as an admin may type a provider URL. -const BARE_HOST = /^([^/:@\s]+)(?::\d+)?(?:\/|$)/; +const BARE_HOST = new RegExp(String.raw`^${HOST}(?::\d+)?(?:\/|$)`, 'i'); /** * The host of a git remote or of a git provider URL, lowercased, without From da936afbedccc12cec5fee2b99e8724a7e508614 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 17:26:44 +0200 Subject: [PATCH 09/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20delete=20an=20?= =?UTF-8?q?emptied=20CLI-managed=20provider=20in=20one=20statement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checking for repositories and deleting were two calls, so a repository the CLI added in between was left on a deleted provider. Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../git/src/application/GitProviderService.ts | 7 ++ .../src/application/GitRepoService.spec.ts | 35 ------ .../git/src/application/GitRepoService.ts | 8 -- .../addGitRepo/AddGitRepoUseCase.spec.ts | 26 +--- .../useCases/addGitRepo/AddGitRepoUseCase.ts | 10 +- .../repositories/IGitProviderRepository.ts | 11 +- .../GitProviderRepository.spec.ts | 116 +++++++++++++++++- .../repositories/GitProviderRepository.ts | 34 +++++ 8 files changed, 175 insertions(+), 72 deletions(-) diff --git a/packages/git/src/application/GitProviderService.ts b/packages/git/src/application/GitProviderService.ts index 42acac1245..08bece9551 100644 --- a/packages/git/src/application/GitProviderService.ts +++ b/packages/git/src/application/GitProviderService.ts @@ -66,6 +66,13 @@ export class GitProviderService { return this.gitProviderRepository.deleteById(id, userId); } + async deleteGitProviderIfEmpty( + id: GitProviderId, + userId: UserId, + ): Promise { + return this.gitProviderRepository.deleteIfHoldsNoRepository(id, userId); + } + async getAvailableRepos( gitProviderId: GitProviderId, page = 1, diff --git a/packages/git/src/application/GitRepoService.spec.ts b/packages/git/src/application/GitRepoService.spec.ts index 66b93911ef..3c902d8017 100644 --- a/packages/git/src/application/GitRepoService.spec.ts +++ b/packages/git/src/application/GitRepoService.spec.ts @@ -200,41 +200,6 @@ describe('GitRepoService', () => { }); }); - describe('hasGitRepos', () => { - describe('when the provider holds a repository of any type', () => { - beforeEach(() => { - mockGitRepoRepository.findByProviderId.mockResolvedValue([ - { ...mockGitRepo, type: 'marketplace' }, - ]); - }); - - it('returns true', async () => { - expect( - await gitRepoService.hasGitRepos(createGitProviderId('provider-1')), - ).toBe(true); - }); - - it('counts every repository type', async () => { - await gitRepoService.hasGitRepos(createGitProviderId('provider-1')); - - expect(mockGitRepoRepository.findByProviderId).toHaveBeenCalledWith( - createGitProviderId('provider-1'), - { type: 'any' }, - ); - }); - }); - - describe('when the provider holds no repository', () => { - it('returns false', async () => { - mockGitRepoRepository.findByProviderId.mockResolvedValue([]); - - expect( - await gitRepoService.hasGitRepos(createGitProviderId('provider-1')), - ).toBe(false); - }); - }); - }); - describe('findGitReposByOrganizationId', () => { const mockRepos = [mockGitRepo]; let result: GitRepo[]; diff --git a/packages/git/src/application/GitRepoService.ts b/packages/git/src/application/GitRepoService.ts index b6fc81d82e..a9cc4ce38e 100644 --- a/packages/git/src/application/GitRepoService.ts +++ b/packages/git/src/application/GitRepoService.ts @@ -72,14 +72,6 @@ export class GitRepoService { }); } - // Marketplace repositories count too: a provider holding one still serves. - async hasGitRepos(providerId: GitProviderId): Promise { - const gitRepos = await this.gitRepoRepository.findByProviderId(providerId, { - type: 'any', - }); - return gitRepos.length > 0; - } - async findGitReposByOrganizationId( organizationId: OrganizationId, ): Promise { diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts index 36ffa7d391..ae1818fb18 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts @@ -42,7 +42,7 @@ describe('AddGitRepoUseCase', () => { beforeEach(() => { mockGitProviderService = { findGitProviderById: jest.fn(), - deleteGitProvider: jest.fn(), + deleteGitProviderIfEmpty: jest.fn(), } as Partial< jest.Mocked > as jest.Mocked; @@ -51,7 +51,6 @@ describe('AddGitRepoUseCase', () => { findGitRepoByOwnerRepoAndBranchInOrganization: jest.fn(), addGitRepo: jest.fn(), adoptGitRepo: jest.fn(), - hasGitRepos: jest.fn(), } as Partial> as jest.Mocked; mockDeploymentPort = { @@ -706,7 +705,6 @@ describe('AddGitRepoUseCase', () => { existingRepo, ); mockGitRepoService.adoptGitRepo.mockResolvedValue(adoptedRepo); - mockGitRepoService.hasGitRepos.mockResolvedValue(false); }); describe('when the authenticated provider targets the same host', () => { @@ -741,24 +739,10 @@ describe('AddGitRepoUseCase', () => { expect(mockDeploymentPort.addTarget).not.toHaveBeenCalled(); }); - it('removes the emptied CLI-managed provider', () => { - expect(mockGitProviderService.deleteGitProvider).toHaveBeenCalledWith( - holdingProviderId, - userId, - ); - }); - }); - - describe('when the CLI-managed provider still holds other repositories', () => { - beforeEach(async () => { - mockGitRepoService.hasGitRepos.mockResolvedValue(true); - givenProviders(authenticatedProvider(), cliManagedProvider()); - - await addRepo(); - }); - - it('keeps the CLI-managed provider', () => { - expect(mockGitProviderService.deleteGitProvider).not.toHaveBeenCalled(); + it('removes the CLI-managed provider if it holds no repository any more', () => { + expect( + mockGitProviderService.deleteGitProviderIfEmpty, + ).toHaveBeenCalledWith(holdingProviderId, userId); }); }); diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts index f8c5926346..a684259759 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts @@ -150,12 +150,10 @@ export class AddGitRepoUseCase ); // An emptied CLI-managed provider would stay listed beside the // connection that now holds its repositories. - if (!(await this.gitRepoService.hasGitRepos(holdingProvider.id))) { - await this.gitProviderService.deleteGitProvider( - holdingProvider.id, - createUserId(userId), - ); - } + await this.gitProviderService.deleteGitProviderIfEmpty( + holdingProvider.id, + createUserId(userId), + ); return adoptedRepo; } diff --git a/packages/git/src/domain/repositories/IGitProviderRepository.ts b/packages/git/src/domain/repositories/IGitProviderRepository.ts index 34d55dbe48..a82ba5968e 100644 --- a/packages/git/src/domain/repositories/IGitProviderRepository.ts +++ b/packages/git/src/domain/repositories/IGitProviderRepository.ts @@ -1,4 +1,4 @@ -import { GitProvider } from '@packmind/types'; +import { GitProvider, GitProviderId } from '@packmind/types'; import { OrganizationId } from '@packmind/types'; import { IRepository } from '@packmind/types'; @@ -12,4 +12,13 @@ export interface IGitProviderRepository extends IRepository { id: string, gitProvider: Partial>, ): Promise; + /** + * Soft-deletes the provider only while it holds no live repository, as one + * statement, so a repository added concurrently keeps its provider. + * Resolves whether the provider was deleted. + */ + deleteIfHoldsNoRepository( + id: GitProviderId, + deletedBy: string, + ): Promise; } diff --git a/packages/git/src/infra/repositories/GitProviderRepository.spec.ts b/packages/git/src/infra/repositories/GitProviderRepository.spec.ts index 936ae699d8..552017276b 100644 --- a/packages/git/src/infra/repositories/GitProviderRepository.spec.ts +++ b/packages/git/src/infra/repositories/GitProviderRepository.spec.ts @@ -15,10 +15,15 @@ import { createUserId, GitProvider, GitProviderNotFoundError, + GitRepo, } from '@packmind/types'; import { PackmindLogger } from '@packmind/logger'; import { Configuration, EncryptionService } from '@packmind/node-utils'; -import { gitProviderFactory, gitlabProviderFactory } from '../../../test'; +import { + gitProviderFactory, + gitlabProviderFactory, + gitRepoFactory, +} from '../../../test'; import { createOrganizationId, Organization } from '@packmind/types'; import { OrganizationSchema } from '@packmind/accounts'; @@ -687,4 +692,113 @@ describe('GitProviderRepository', () => { expect(rawProvider?.token).toBeDefined(); }); }); + describe('deleteIfHoldsNoRepository', () => { + let provider: GitProvider; + const deletedBy = createUserId(uuidv4()); + + const saveRepo = (overrides: Partial = {}) => + fixture.datasource.getRepository(GitRepoSchema).save( + gitRepoFactory({ + providerId: createGitProviderId(provider.id), + ...overrides, + }), + ); + + beforeEach(async () => { + provider = await gitProviderRepository.add( + gitProviderFactory({ + organizationId: testOrganization.id, + token: null, + }), + ); + }); + + describe('when the provider holds no repository', () => { + let deleted: boolean; + + beforeEach(async () => { + deleted = await gitProviderRepository.deleteIfHoldsNoRepository( + createGitProviderId(provider.id), + deletedBy, + ); + }); + + it('resolves true', () => { + expect(deleted).toBe(true); + }); + + it('soft-deletes the provider', async () => { + expect( + await gitProviderRepository.findById( + createGitProviderId(provider.id), + ), + ).toBeNull(); + }); + + it('records who deleted it', async () => { + const row = await fixture.datasource + .getRepository(GitProviderSchema) + .findOne({ where: { id: provider.id }, withDeleted: true }); + + expect(row).toMatchObject({ deletedBy }); + }); + }); + + describe('when the provider still holds a repository', () => { + let deleted: boolean; + + beforeEach(async () => { + await saveRepo({ type: 'marketplace' }); + + deleted = await gitProviderRepository.deleteIfHoldsNoRepository( + createGitProviderId(provider.id), + deletedBy, + ); + }); + + it('resolves false', () => { + expect(deleted).toBe(false); + }); + + it('keeps the provider', async () => { + expect( + await gitProviderRepository.findById( + createGitProviderId(provider.id), + ), + ).not.toBeNull(); + }); + }); + + describe('when its only repository was deleted', () => { + it('deletes the provider', async () => { + const repo = await saveRepo(); + await fixture.datasource.getRepository(GitRepoSchema).softDelete({ + id: repo.id, + }); + + expect( + await gitProviderRepository.deleteIfHoldsNoRepository( + createGitProviderId(provider.id), + deletedBy, + ), + ).toBe(true); + }); + }); + + describe('when the provider is already deleted', () => { + it('resolves false', async () => { + await gitProviderRepository.deleteById( + createGitProviderId(provider.id), + deletedBy, + ); + + expect( + await gitProviderRepository.deleteIfHoldsNoRepository( + createGitProviderId(provider.id), + deletedBy, + ), + ).toBe(false); + }); + }); + }); }); diff --git a/packages/git/src/infra/repositories/GitProviderRepository.ts b/packages/git/src/infra/repositories/GitProviderRepository.ts index 007aebaa58..8a39056d2f 100644 --- a/packages/git/src/infra/repositories/GitProviderRepository.ts +++ b/packages/git/src/infra/repositories/GitProviderRepository.ts @@ -1,7 +1,9 @@ import { GitProvider, GitProviderId } from '@packmind/types'; import { IGitProviderRepository } from '../../domain/repositories/IGitProviderRepository'; import { GitProviderSchema } from '../schemas/GitProviderSchema'; +import { GitRepoSchema } from '../schemas/GitRepoSchema'; import { Repository } from 'typeorm'; +import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; import { PackmindLogger } from '@packmind/logger'; import { localDataSource, @@ -248,4 +250,36 @@ export class GitProviderRepository throw error; } } + + async deleteIfHoldsNoRepository( + id: GitProviderId, + deletedBy: string, + ): Promise { + const liveRepository = this.repository.manager + .createQueryBuilder() + .subQuery() + .select('1') + .from(GitRepoSchema, 'gitRepo') + .where('gitRepo.providerId = :id') + .andWhere('gitRepo.deletedAt IS NULL') + .getQuery(); + const result = await this.repository + .createQueryBuilder() + .update() + // The soft-delete columns are not part of the domain type. + .set({ + deletedAt: () => 'CURRENT_TIMESTAMP', + deletedBy, + } as QueryDeepPartialEntity) + .where('id = :id', { id }) + .andWhere('deleted_at IS NULL') + .andWhere(`NOT EXISTS ${liveRepository}`) + .execute(); + const deleted = (result.affected ?? 0) > 0; + this.logger.info('Deleted git provider holding no repository', { + id, + deleted, + }); + return deleted; + } } From 36f7be2adbd6433d20bcf5bed433d8d9bead845f Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 17:26:47 +0200 Subject: [PATCH 10/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20accept=20an=20?= =?UTF-8?q?scp-like=20remote=20naming=20an=20absolute=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #887 Co-Authored-By: Claude Opus 5.5 --- .../useCases/trackRepository/TrackRepositoryUseCase.ts | 3 ++- .../useCases/trackRepository/parseOwnerRepo.spec.ts | 9 +++++++++ packages/node-utils/src/git/parseGitRepoInfo.spec.ts | 9 +++++++++ packages/node-utils/src/git/parseGitRepoInfo.ts | 3 ++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts index 2bdee038a2..aecc90185d 100644 --- a/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts +++ b/apps/cli/src/application/useCases/trackRepository/TrackRepositoryUseCase.ts @@ -23,8 +23,9 @@ export function parseOwnerRepo(gitRemoteUrl: string): { const path = (gitRemoteUrl.trim().match(SCHEME_URL_PATH) ?? gitRemoteUrl.trim().match(SCP_LIKE_PATH))?.[1]; + // An scp-like remote may name an absolute path: `git@host:/group/repo.git`. const segments = path - ?.replace(/\/+$/, '') + ?.replace(/^\/+|\/+$/g, '') .replace(/\.git$/i, '') .split('/'); diff --git a/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts b/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts index 6b451209c9..1ea93f440e 100644 --- a/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts +++ b/apps/cli/src/application/useCases/trackRepository/parseOwnerRepo.spec.ts @@ -22,6 +22,15 @@ describe('parseOwnerRepo', () => { }); }); + describe('when an scp-like remote names an absolute path', () => { + it('ignores the leading slash', () => { + expect(parseOwnerRepo('git@gitlab.acme.io:/acme/app.git')).toEqual({ + owner: 'acme', + repo: 'app', + }); + }); + }); + describe('when the remote has no owner', () => { it('throws', () => { expect(() => parseOwnerRepo('https://github.com/repo')).toThrow( diff --git a/packages/node-utils/src/git/parseGitRepoInfo.spec.ts b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts index 4955d4967c..493a0f439d 100644 --- a/packages/node-utils/src/git/parseGitRepoInfo.spec.ts +++ b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts @@ -28,6 +28,15 @@ describe('parseGitRepoInfo', () => { }); }); + describe('when an scp-like remote names an absolute path', () => { + it('ignores the leading slash', () => { + expect(parseGitRepoInfo('git@gitlab.acme.io:/acme/app.git')).toEqual({ + owner: 'acme', + repo: 'app', + }); + }); + }); + describe('when the remote has no owner', () => { it('throws', () => { expect(() => parseGitRepoInfo('https://github.com/repo')).toThrow( diff --git a/packages/node-utils/src/git/parseGitRepoInfo.ts b/packages/node-utils/src/git/parseGitRepoInfo.ts index 3824fcae97..158142b8ff 100644 --- a/packages/node-utils/src/git/parseGitRepoInfo.ts +++ b/packages/node-utils/src/git/parseGitRepoInfo.ts @@ -18,8 +18,9 @@ export function parseGitRepoInfo(gitRemoteUrl: string): { const path = (gitRemoteUrl.trim().match(SCHEME_URL_PATH) ?? gitRemoteUrl.trim().match(SCP_LIKE_PATH))?.[1]; + // An scp-like remote may name an absolute path: `git@host:/group/repo.git`. const segments = path - ?.replace(/\/+$/, '') + ?.replace(/^\/+|\/+$/g, '') .replace(/\.git$/i, '') .split('/'); From 9f7a7248da98e9068b320350f06c8435020646e7 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 17:36:27 +0200 Subject: [PATCH 11/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20match=20a=20re?= =?UTF-8?q?mote=20of=20an=20instance=20installed=20under=20a=20path=20pref?= =?UTF-8?q?ix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keeping the whole group path made a remote of https://host/gitlab/group/app read as owner gitlab/group, which the provider reports as group. The owner now drops the path prefix of a provider on the remote's host, and adding a repository adopts a row the CLI recorded with the prefix, under the owner the provider reports. Refs #887 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.MD | 1 + .../services/TargetResolutionService.ts | 15 +++- .../git/src/application/GitProviderService.ts | 15 ++++ .../src/application/GitRepoService.spec.ts | 1 + .../git/src/application/GitRepoService.ts | 7 +- .../git/src/application/adapter/GitAdapter.ts | 3 + .../addGitRepo/AddGitRepoUseCase.spec.ts | 31 +++++++ .../useCases/addGitRepo/AddGitRepoUseCase.ts | 17 +++- .../FindOrCreateGitRepoUseCase.ts | 12 ++- .../GetTrackedRepositoryUseCase.spec.ts | 10 +++ .../GetTrackedRepositoryUseCase.ts | 10 ++- .../RemoveTrackedRepositoryUseCase.spec.ts | 10 +++ .../RemoveTrackedRepositoryUseCase.ts | 10 ++- .../SetTrackedRepositoryUseCase.spec.ts | 10 +++ .../SetTrackedRepositoryUseCase.ts | 11 ++- .../UpdateTrackedBranchUseCase.spec.ts | 1 + .../UpdateTrackedBranchUseCase.ts | 8 +- .../domain/repositories/IGitRepoRepository.ts | 4 +- .../repositories/GitRepoRepository.spec.ts | 18 +++++ .../infra/repositories/GitRepoRepository.ts | 7 +- .../src/self-hosted-repo-adoption.spec.ts | 72 +++++++++++++++++ packages/node-utils/src/git/gitHost.spec.ts | 80 ++++++++++++++++++- packages/node-utils/src/git/gitHost.ts | 44 ++++++++++ 23 files changed, 383 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index d17420224a..328c32c079 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -24,6 +24,7 @@ - Adding a repository the CLI already recorded on that instance moves it under the connection with its targets and history, instead of refusing it as a duplicate - A CLI-managed connection is no longer listed once it holds no repository - A repository cloned over `ssh://` or with a user in its URL no longer gets a CLI-managed connection of its own +- A self-managed GitLab installed under a path such as `/gitlab` matches its connection: the path is no longer read as part of the group - Opening a component from All components keeps the rows already picked, and closing it goes back to that list rather than to the package carrying the component - A GitLab repository in a subgroup keeps its full path (`group/subgroup/repo`) when tracked from the CLI, so a token or app connection links it instead of duplicating it diff --git a/packages/deployments/src/application/services/TargetResolutionService.ts b/packages/deployments/src/application/services/TargetResolutionService.ts index b636939e8b..3df055a1b8 100644 --- a/packages/deployments/src/application/services/TargetResolutionService.ts +++ b/packages/deployments/src/application/services/TargetResolutionService.ts @@ -14,7 +14,11 @@ import { v4 as uuidv4 } from 'uuid'; import { TargetService } from './TargetService'; import { generateTargetName, normalizeRelativePath } from './gitInfoHelpers'; import { IDistributionRepository } from '../../domain/repositories/IDistributionRepository'; -import { parseGitRepoInfo, parseGitProviderVendor } from '@packmind/node-utils'; +import { + ownerWithoutProviderPrefix, + parseGitRepoInfo, + parseGitProviderVendor, +} from '@packmind/node-utils'; const origin = 'TargetResolutionService'; @@ -36,12 +40,19 @@ export class TargetResolutionService { gitBranch: string, relativePath: string, ): Promise { - const { owner, repo } = parseGitRepoInfo(gitRemoteUrl); + const { owner: remoteOwner, repo } = parseGitRepoInfo(gitRemoteUrl); const providersResponse = await this.gitPort.listProviders({ userId, organizationId, }); + // The provider reports the group without the installation path prefix a + // remote of a prefixed instance carries. + const owner = ownerWithoutProviderPrefix( + remoteOwner, + providersResponse.providers.map((provider) => provider.url), + gitRemoteUrl, + ); let gitRepoId: string | null = null; for (const provider of providersResponse.providers) { diff --git a/packages/git/src/application/GitProviderService.ts b/packages/git/src/application/GitProviderService.ts index 08bece9551..46f7af3715 100644 --- a/packages/git/src/application/GitProviderService.ts +++ b/packages/git/src/application/GitProviderService.ts @@ -16,6 +16,7 @@ import { } from '@packmind/types'; import { GitBranchComparison, GitRepo } from '@packmind/types'; import { OrganizationId, UserId } from '@packmind/types'; +import { ownerWithoutProviderPrefix } from '@packmind/node-utils'; import { v4 as uuidv4 } from 'uuid'; export class GitProviderService { @@ -66,6 +67,20 @@ export class GitProviderService { return this.gitProviderRepository.deleteById(id, userId); } + async ownerAsProvidersNameIt( + organizationId: OrganizationId, + owner: string, + gitRemoteUrl?: string, + ): Promise { + const providers = + await this.gitProviderRepository.findByOrganizationId(organizationId); + return ownerWithoutProviderPrefix( + owner, + providers.map((provider) => provider.url), + gitRemoteUrl, + ); + } + async deleteGitProviderIfEmpty( id: GitProviderId, userId: UserId, diff --git a/packages/git/src/application/GitRepoService.spec.ts b/packages/git/src/application/GitRepoService.spec.ts index 3c902d8017..37a9423063 100644 --- a/packages/git/src/application/GitRepoService.spec.ts +++ b/packages/git/src/application/GitRepoService.spec.ts @@ -317,6 +317,7 @@ describe('GitRepoService', () => { expect(mockGitRepoRepository.reassignProvider).toHaveBeenCalledWith( mockGitRepo.id, newProviderId, + mockGitRepo.owner, ); }); diff --git a/packages/git/src/application/GitRepoService.ts b/packages/git/src/application/GitRepoService.ts index a9cc4ce38e..88e1cc8347 100644 --- a/packages/git/src/application/GitRepoService.ts +++ b/packages/git/src/application/GitRepoService.ts @@ -115,13 +115,18 @@ export class GitRepoService { gitRepo: GitRepo, providerId: GitProviderId, organizationId: OrganizationId, + owner: string = gitRepo.owner, ): Promise { await this.gitRepoRepository.clearTrackingRemoved( gitRepo.owner, gitRepo.repo, organizationId, ); - return this.gitRepoRepository.reassignProvider(gitRepo.id, providerId); + return this.gitRepoRepository.reassignProvider( + gitRepo.id, + providerId, + owner, + ); } /** diff --git a/packages/git/src/application/adapter/GitAdapter.ts b/packages/git/src/application/adapter/GitAdapter.ts index bd95b04251..3b2d299d98 100644 --- a/packages/git/src/application/adapter/GitAdapter.ts +++ b/packages/git/src/application/adapter/GitAdapter.ts @@ -280,11 +280,13 @@ export class GitAdapter implements IBaseAdapter, IGitPort { this._getTrackedRepositoryUseCase = new GetTrackedRepositoryUseCase( this.gitServices.getGitRepoService(), + this.gitServices.getGitProviderService(), this.accountsPort, ); this._setTrackedRepositoryUseCase = new SetTrackedRepositoryUseCase( this.gitServices.getGitRepoService(), + this.gitServices.getGitProviderService(), this._findOrCreateGitRepo, this.eventEmitterService, this.accountsPort, @@ -300,6 +302,7 @@ export class GitAdapter implements IBaseAdapter, IGitPort { this._removeTrackedRepositoryUseCase = new RemoveTrackedRepositoryUseCase( this.gitServices.getGitRepoService(), + this.gitServices.getGitProviderService(), this.eventEmitterService, this.accountsPort, ); diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts index ae1818fb18..3ed828f0b9 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.spec.ts @@ -728,6 +728,7 @@ describe('AddGitRepoUseCase', () => { existingRepo, gitProviderId, organizationId, + 'optimetriks', ); }); @@ -770,6 +771,7 @@ describe('AddGitRepoUseCase', () => { existingRepo, gitProviderId, organizationId, + 'optimetriks', ); }); }); @@ -805,6 +807,35 @@ describe('AddGitRepoUseCase', () => { existingRepo, gitProviderId, organizationId, + 'optimetriks', + ); + }); + }); + + describe('when the CLI recorded the owner under an installation path prefix', () => { + beforeEach(async () => { + const prefixedRepo = { ...existingRepo, owner: 'gitlab/optimetriks' }; + mockGitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization.mockImplementation( + async (owner) => + owner === 'gitlab/optimetriks' ? prefixedRepo : null, + ); + givenProviders( + authenticatedProvider({ url: 'https://devtools.acme.io/gitlab' }), + cliManagedProvider({ + source: GitProviderVendors.unknown, + url: 'https://devtools.acme.io', + }), + ); + + await addRepo(); + }); + + it('adopts it under the owner the provider reports', () => { + expect(mockGitRepoService.adoptGitRepo).toHaveBeenCalledWith( + expect.objectContaining({ owner: 'gitlab/optimetriks' }), + gitProviderId, + organizationId, + 'optimetriks', ); }); }); diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts index a684259759..a41a8edbf3 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts @@ -2,6 +2,7 @@ import { PackmindLogger } from '@packmind/logger'; import { AbstractMemberUseCase, MemberContext, + providerPathPrefix, sameGitHost, } from '@packmind/node-utils'; import { @@ -118,13 +119,24 @@ export class AddGitRepoUseCase throw new GitProviderMissingTokenError(gitProviderId); } + // The CLI recorded a remote of an instance installed under a path prefix + // with that prefix before the group, which the provider does not report. + const pathPrefix = providerPathPrefix(gitProvider.url); const existingRepo = - await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( + (await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( owner, repo, branch, organization.id, - ); + )) ?? + (pathPrefix + ? await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( + `${pathPrefix}/${owner}`, + repo, + branch, + organization.id, + ) + : null); if (existingRepo) { const holdingProvider = @@ -147,6 +159,7 @@ export class AddGitRepoUseCase existingRepo, gitProvider.id, organization.id, + owner, ); // An emptied CLI-managed provider would stay listed beside the // connection that now holds its repositories. diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts index 4880687607..c3ba62df5b 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts @@ -13,6 +13,7 @@ import { } from '@packmind/types'; import { extractBaseUrl, + ownerWithoutProviderPrefix, parseGitProviderVendor, sameGitHost, } from '@packmind/node-utils'; @@ -45,7 +46,7 @@ export class FindOrCreateGitRepoUseCase protected async executeForMembers( command: FindOrCreateGitRepoCommand & MemberContext, ): Promise { - const { owner, repo, branch, organization, userId } = command; + const { repo, branch, organization, userId } = command; const gitRemoteUrl = command.gitRemoteUrl; // The remote is the server's own evidence; the vendor a CLI sends is only @@ -59,7 +60,7 @@ export class FindOrCreateGitRepoUseCase this.logger.info('Finding or creating git repo', { providerVendor, - owner, + owner: command.owner, repo, branch, }); @@ -68,6 +69,13 @@ export class FindOrCreateGitRepoUseCase userId, organizationId, }); + // A remote cloned from an instance installed under a path prefix carries + // that prefix before the group; the provider reports the group alone. + const owner = ownerWithoutProviderPrefix( + command.owner, + providersResponse.providers.map((p) => p.url), + gitRemoteUrl, + ); // A provider's URL states which instance it reaches, so a self-hosted // remote finds the provider an admin configured for it, whatever vendor a diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts index e3d7d2dbbf..5ae9aa069d 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts @@ -1,3 +1,4 @@ +import { GitProviderService } from '../../GitProviderService'; import { stubLogger, mockInterface } from '@packmind/test-utils'; import { createGitProviderId, @@ -15,6 +16,14 @@ import { gitRepoFactory } from '../../../../test'; import { GitRepoService } from '../../GitRepoService'; import { GetTrackedRepositoryUseCase } from './GetTrackedRepositoryUseCase'; +// Owners in these specs carry no installation prefix, so they pass through. +const ownerAsIsProviderService = () => + ({ + ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + }) as Partial< + jest.Mocked + > as jest.Mocked; + describe('GetTrackedRepositoryUseCase', () => { let useCase: GetTrackedRepositoryUseCase; let mockGitRepoService: jest.Mocked; @@ -63,6 +72,7 @@ describe('GetTrackedRepositoryUseCase', () => { useCase = new GetTrackedRepositoryUseCase( mockGitRepoService, + ownerAsIsProviderService(), mockAccountsAdapter, stubLogger(), ); diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts index 1d7979727a..94205b9e7a 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts @@ -6,6 +6,7 @@ import { IAccountsPort, IGetTrackedRepositoryUseCase, } from '@packmind/types'; +import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; const origin = 'GetTrackedRepositoryUseCase'; @@ -19,6 +20,7 @@ export class GetTrackedRepositoryUseCase { constructor( private readonly gitRepoService: GitRepoService, + private readonly gitProviderService: GitProviderService, accountsAdapter: IAccountsPort, logger: PackmindLogger = new PackmindLogger(origin), ) { @@ -28,7 +30,13 @@ export class GetTrackedRepositoryUseCase protected async executeForMembers( command: GetTrackedRepositoryCommand & MemberContext, ): Promise { - const { owner, repo, organization } = command; + const { owner: remoteOwner, repo, organization } = command; + // A remote cloned from an instance installed under a path prefix carries + // that prefix before the group; the repository is recorded without it. + const owner = await this.gitProviderService.ownerAsProvidersNameIt( + organization.id, + remoteOwner, + ); const gitRepo = await this.gitRepoService.findTrackedByOwnerRepoInOrganization( diff --git a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts index e2feec3469..3f1db699ad 100644 --- a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts @@ -1,3 +1,4 @@ +import { GitProviderService } from '../../GitProviderService'; import { PackmindEventEmitterService } from '@packmind/node-utils'; import { OrganizationAdminRequiredError } from '@packmind/node-utils'; import { stubLogger, mockInterface } from '@packmind/test-utils'; @@ -19,6 +20,14 @@ import { v4 as uuidv4 } from 'uuid'; import { GitRepoService } from '../../GitRepoService'; import { RemoveTrackedRepositoryUseCase } from './RemoveTrackedRepositoryUseCase'; +// Owners in these specs carry no installation prefix, so they pass through. +const ownerAsIsProviderService = () => + ({ + ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + }) as Partial< + jest.Mocked + > as jest.Mocked; + describe('RemoveTrackedRepositoryUseCase', () => { let useCase: RemoveTrackedRepositoryUseCase; let mockGitRepoService: jest.Mocked; @@ -69,6 +78,7 @@ describe('RemoveTrackedRepositoryUseCase', () => { const buildUseCase = () => new RemoveTrackedRepositoryUseCase( mockGitRepoService, + ownerAsIsProviderService(), mockEventEmitter, mockAccountsAdapter, stubLogger(), diff --git a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts index 99f51a059a..e0a30713b5 100644 --- a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts @@ -13,6 +13,7 @@ import { RepositoryNotTrackableError, RepositoryTrackingRemovedEvent, } from '@packmind/types'; +import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; const origin = 'RemoveTrackedRepositoryUseCase'; @@ -31,6 +32,7 @@ export class RemoveTrackedRepositoryUseCase { constructor( private readonly gitRepoService: GitRepoService, + private readonly gitProviderService: GitProviderService, private readonly eventEmitterService: PackmindEventEmitterService, accountsAdapter: IAccountsPort, logger: PackmindLogger = new PackmindLogger(origin), @@ -41,7 +43,13 @@ export class RemoveTrackedRepositoryUseCase protected async executeForAdmins( command: RemoveTrackedRepositoryCommand & AdminContext, ): Promise { - const { owner, repo, organization, userId } = command; + const { owner: remoteOwner, repo, organization, userId } = command; + // A remote cloned from an instance installed under a path prefix carries + // that prefix before the group; the repository is recorded without it. + const owner = await this.gitProviderService.ownerAsProvidersNameIt( + organization.id, + remoteOwner, + ); const existingTracked = await this.gitRepoService.findTrackedByOwnerRepoInOrganization( diff --git a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts index 7d4eecc32c..30d15177de 100644 --- a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts @@ -1,3 +1,4 @@ +import { GitProviderService } from '../../GitProviderService'; import { PackmindEventEmitterService } from '@packmind/node-utils'; import { OrganizationAdminRequiredError } from '@packmind/node-utils'; import { stubLogger, mockInterface } from '@packmind/test-utils'; @@ -21,6 +22,14 @@ import { gitRepoFactory } from '../../../../test'; import { GitRepoService } from '../../GitRepoService'; import { SetTrackedRepositoryUseCase } from './SetTrackedRepositoryUseCase'; +// Owners in these specs carry no installation prefix, so they pass through. +const ownerAsIsProviderService = () => + ({ + ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + }) as Partial< + jest.Mocked + > as jest.Mocked; + describe('SetTrackedRepositoryUseCase', () => { let useCase: SetTrackedRepositoryUseCase; let mockGitRepoService: jest.Mocked; @@ -63,6 +72,7 @@ describe('SetTrackedRepositoryUseCase', () => { const buildUseCase = () => new SetTrackedRepositoryUseCase( mockGitRepoService, + ownerAsIsProviderService(), mockFindOrCreate, mockEventEmitter, mockAccountsAdapter, diff --git a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts index 34856bf4a8..a98f187240 100644 --- a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts @@ -14,6 +14,7 @@ import { SetTrackedRepositoryCommand, SetTrackedRepositoryResponse, } from '@packmind/types'; +import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; const origin = 'SetTrackedRepositoryUseCase'; @@ -27,6 +28,7 @@ export class SetTrackedRepositoryUseCase { constructor( private readonly gitRepoService: GitRepoService, + private readonly gitProviderService: GitProviderService, private readonly findOrCreateGitRepo: IFindOrCreateGitRepoUseCase, private readonly eventEmitterService: PackmindEventEmitterService, accountsAdapter: IAccountsPort, @@ -39,7 +41,7 @@ export class SetTrackedRepositoryUseCase command: SetTrackedRepositoryCommand & AdminContext, ): Promise { const { - owner, + owner: remoteOwner, repo, branch, origin: trackingOrigin, @@ -48,6 +50,13 @@ export class SetTrackedRepositoryUseCase organization, userId, } = command; + // A remote cloned from an instance installed under a path prefix carries + // that prefix before the group; the repository is recorded without it. + const owner = await this.gitProviderService.ownerAsProvidersNameIt( + organization.id, + remoteOwner, + gitRemoteUrl, + ); const existingTracked = await this.gitRepoService.findTrackedByOwnerRepoInOrganization( diff --git a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts index a78de164b4..0c93f502cf 100644 --- a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts +++ b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts @@ -94,6 +94,7 @@ describe('UpdateTrackedBranchUseCase', () => { mockGitProviderService = { findGitProviderById: jest.fn().mockResolvedValue(provider), + ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), } as Partial< jest.Mocked > as jest.Mocked; diff --git a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts index 373ab7aed7..aac6201909 100644 --- a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts +++ b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts @@ -41,7 +41,13 @@ export class UpdateTrackedBranchUseCase protected async executeForAdmins( command: UpdateTrackedBranchCommand & AdminContext, ): Promise { - const { owner, repo, branch, organization, userId } = command; + const { owner: remoteOwner, repo, branch, organization, userId } = command; + // A remote cloned from an instance installed under a path prefix carries + // that prefix before the group; the repository is recorded without it. + const owner = await this.gitProviderService.ownerAsProvidersNameIt( + organization.id, + remoteOwner, + ); const existingTracked = await this.gitRepoService.findTrackedByOwnerRepoInOrganization( diff --git a/packages/git/src/domain/repositories/IGitRepoRepository.ts b/packages/git/src/domain/repositories/IGitRepoRepository.ts index e87c13aebc..0e4d6b7180 100644 --- a/packages/git/src/domain/repositories/IGitRepoRepository.ts +++ b/packages/git/src/domain/repositories/IGitRepoRepository.ts @@ -61,11 +61,13 @@ export interface IGitRepoRepository extends IRepository { markTrackingRemoved(gitRepoId: GitRepoId): Promise; /** * Moves a repository under another provider while keeping its id, so its - * targets and distribution history follow it. + * targets and distribution history follow it. The owner is rewritten when + * the new provider names it differently. */ reassignProvider( gitRepoId: GitRepoId, providerId: GitProviderId, + owner?: string, ): Promise; /** * Clears the removal stamp on every branch of a repository: a stamp on any diff --git a/packages/git/src/infra/repositories/GitRepoRepository.spec.ts b/packages/git/src/infra/repositories/GitRepoRepository.spec.ts index ec0048f867..10accf434d 100644 --- a/packages/git/src/infra/repositories/GitRepoRepository.spec.ts +++ b/packages/git/src/infra/repositories/GitRepoRepository.spec.ts @@ -535,6 +535,24 @@ describe('GitRepoRepository', () => { it('keeps the tracked flag', () => { expect(reloaded?.isTracked).toBe(true); }); + + it('keeps the owner', () => { + expect(reloaded?.owner).toEqual(gitRepo.owner); + }); + }); + + describe('when the new provider names the owner differently', () => { + it('rewrites the owner', async () => { + await gitRepoRepository.reassignProvider( + gitRepo.id, + otherProvider.id, + 'renamed-owner', + ); + + expect((await gitRepoRepository.findById(gitRepo.id))?.owner).toEqual( + 'renamed-owner', + ); + }); }); describe('when the repository does not exist', () => { diff --git a/packages/git/src/infra/repositories/GitRepoRepository.ts b/packages/git/src/infra/repositories/GitRepoRepository.ts index 428972a36f..ec5645a5f9 100644 --- a/packages/git/src/infra/repositories/GitRepoRepository.ts +++ b/packages/git/src/infra/repositories/GitRepoRepository.ts @@ -282,6 +282,7 @@ export class GitRepoRepository async reassignProvider( gitRepoId: GitRepoId, providerId: GitProviderId, + owner?: string, ): Promise { this.logger.info('Reassigning git repo provider', { gitRepoId, @@ -297,7 +298,11 @@ export class GitRepoRepository throw new GitRepoNotFoundError(gitRepoId); } - const updated = await this.repository.save({ ...gitRepo, providerId }); + const updated = await this.repository.save({ + ...gitRepo, + providerId, + owner: owner ?? gitRepo.owner, + }); this.logger.info('Git repo provider reassigned', { gitRepoId, diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts index dbe075144c..f0489d69da 100644 --- a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -590,6 +590,78 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }); }); + describe('when the instance is installed under a path prefix', () => { + const PREFIXED_HOST = 'https://devtools.acme.io/gitlab'; + const PREFIXED_REMOTE = 'https://devtools.acme.io/gitlab/acme/app.git'; + // The CLI reads every segment before the repository as the owner. + const recordPrefixedFromCli = () => + recordFromCli({ owner: 'gitlab/acme', remote: PREFIXED_REMOTE }); + + describe('and the CLI recorded the repository before the connection', () => { + let cliRepo: GitRepo; + let adopted: GitRepo; + + beforeEach(async () => { + cliRepo = await recordPrefixedFromCli(); + adopted = await addFromApp(await connectTokenProvider(PREFIXED_HOST)); + }); + + it('adopts the same repository', () => { + expect(adopted.id).toBe(cliRepo.id); + }); + + it('records the owner the provider reports', () => { + expect(adopted.owner).toBe(OWNER); + }); + + it('lets the CLI find it afterwards', async () => { + expect((await recordPrefixedFromCli()).id).toBe(cliRepo.id); + }); + }); + + describe('and the connection exists before the CLI runs', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(PREFIXED_HOST); + repo = await recordPrefixedFromCli(); + }); + + it('creates it under the connection', () => { + expect(repo).toMatchObject({ providerId: token.id, owner: OWNER }); + }); + }); + + describe('and the repository is tracked from the CLI', () => { + let tracked: GitRepo; + + beforeEach(async () => { + await connectTokenProvider(PREFIXED_HOST); + tracked = await testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: 'gitlab/acme', + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: PREFIXED_REMOTE, + }); + }); + + it('finds it by the owner the CLI reads', async () => { + const { gitRepo } = await testApp.gitHexa + .getAdapter() + .getTrackedRepository({ + ...admin.packmindCommand(), + owner: 'gitlab/acme', + repo: REPO, + }); + + expect(gitRepo?.id).toBe(tracked.id); + }); + }); + }); + describe('when the CLI spelled the repository with another case', () => { let cliRepo: GitRepo; let adopted: GitRepo; diff --git a/packages/node-utils/src/git/gitHost.spec.ts b/packages/node-utils/src/git/gitHost.spec.ts index 4b6725cf12..6f33972a61 100644 --- a/packages/node-utils/src/git/gitHost.spec.ts +++ b/packages/node-utils/src/git/gitHost.spec.ts @@ -1,4 +1,9 @@ -import { gitHostOf, sameGitHost } from './gitHost'; +import { + gitHostOf, + ownerWithoutProviderPrefix, + providerPathPrefix, + sameGitHost, +} from './gitHost'; describe('gitHostOf', () => { describe.each([ @@ -131,3 +136,76 @@ describe('sameGitHost', () => { }); }); }); + +describe('providerPathPrefix', () => { + describe.each([ + ['a URL at the root of its host', 'https://gitlab.acme.io', null], + ['a URL with a trailing slash', 'https://gitlab.acme.io/', null], + ['a URL under a prefix', 'https://devtools.acme.io/GitLab/', 'gitlab'], + [ + 'a URL under a nested prefix', + 'https://acme.io/tools/gitlab', + 'tools/gitlab', + ], + [ + 'a whole ssh:// remote', + 'ssh://git@gitlab.acme.io:2222/acme/app.git', + null, + ], + ['null', null, null], + ])('with %s', (_label, url, expected) => { + it(`returns ${expected}`, () => { + expect(providerPathPrefix(url)).toBe(expected); + }); + }); +}); + +describe('ownerWithoutProviderPrefix', () => { + const prefixed = 'https://devtools.acme.io/gitlab'; + + describe('when the remote is on a provider installed under a prefix', () => { + it('drops the prefix from the owner', () => { + expect( + ownerWithoutProviderPrefix( + 'gitlab/group/sub', + [prefixed], + 'git@devtools.acme.io:gitlab/group/sub/app.git', + ), + ).toBe('group/sub'); + }); + }); + + describe('when the remote is on another host', () => { + it('keeps the owner', () => { + expect( + ownerWithoutProviderPrefix( + 'gitlab/group', + [prefixed], + 'https://gitlab.other.io/gitlab/group/app.git', + ), + ).toBe('gitlab/group'); + }); + }); + + describe('when no remote is known', () => { + it('drops the prefix of any provider', () => { + expect(ownerWithoutProviderPrefix('GitLab/group', [null, prefixed])).toBe( + 'group', + ); + }); + }); + + describe('when the owner does not start with the prefix', () => { + it('keeps the owner', () => { + expect(ownerWithoutProviderPrefix('gitlabber/group', [prefixed])).toBe( + 'gitlabber/group', + ); + }); + }); + + describe('when the owner is the prefix alone', () => { + it('keeps the owner', () => { + expect(ownerWithoutProviderPrefix('gitlab', [prefixed])).toBe('gitlab'); + }); + }); +}); diff --git a/packages/node-utils/src/git/gitHost.ts b/packages/node-utils/src/git/gitHost.ts index faf2fb5dad..864f2f3911 100644 --- a/packages/node-utils/src/git/gitHost.ts +++ b/packages/node-utils/src/git/gitHost.ts @@ -41,3 +41,47 @@ export function sameGitHost( const providerHost = gitHostOf(providerUrl); return providerHost !== null && providerHost === gitHostOf(gitRemoteUrl); } + +// Only a web URL can carry an installation prefix: a CLI-managed provider may +// hold a whole ssh:// remote, whose path is a repository, not a prefix. +const WEB_URL_PATH = /^https?:\/\/[^/]+\/([^?#]*)/i; + +/** + * The path a provider URL puts before every group, lowercased and without + * slashes — `gitlab` for `https://devtools.acme.io/gitlab` — or null when the + * instance sits at the root of its host. + */ +export function providerPathPrefix( + providerUrl: string | null | undefined, +): string | null { + const path = providerUrl + ?.trim() + .match(WEB_URL_PATH)?.[1] + .replace(/^\/+|\/+$/g, '') + .toLowerCase(); + return path ? path : null; +} + +/** + * The owner as the provider names it. A remote cloned from an instance + * installed under a path prefix carries that prefix before the group + * (`gitlab/group` under `https://devtools.acme.io/gitlab`), which the provider + * never reports. Only providers on the remote's host are considered when the + * remote is known. + */ +export function ownerWithoutProviderPrefix( + owner: string, + providerUrls: ReadonlyArray, + gitRemoteUrl?: string, +): string { + for (const providerUrl of providerUrls) { + if (gitRemoteUrl && !sameGitHost(providerUrl, gitRemoteUrl)) { + continue; + } + const prefix = providerPathPrefix(providerUrl); + if (prefix && owner.toLowerCase().startsWith(`${prefix}/`)) { + return owner.slice(prefix.length + 1); + } + } + return owner; +} From b666124ffa1fbbdf8f45147f1570296bf1b3b319 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 21:38:54 +0200 Subject: [PATCH 12/17] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(git):=20bin?= =?UTF-8?q?d=20the=20provider=20id=20in=20the=20live-repository=20subquery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- packages/git/src/infra/repositories/GitProviderRepository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/git/src/infra/repositories/GitProviderRepository.ts b/packages/git/src/infra/repositories/GitProviderRepository.ts index 8a39056d2f..d8843e3450 100644 --- a/packages/git/src/infra/repositories/GitProviderRepository.ts +++ b/packages/git/src/infra/repositories/GitProviderRepository.ts @@ -260,7 +260,7 @@ export class GitProviderRepository .subQuery() .select('1') .from(GitRepoSchema, 'gitRepo') - .where('gitRepo.providerId = :id') + .where('gitRepo.providerId = :id', { id }) .andWhere('gitRepo.deletedAt IS NULL') .getQuery(); const result = await this.repository From a1cb36200822d32a4f6efbf840f9ee46deb3357c Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 21:39:02 +0200 Subject: [PATCH 13/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20read=20a=20pat?= =?UTF-8?q?h=20prefix=20as=20its=20own=20connection's=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A group read without an installation prefix now only names repositories of the connection installed under it: tracking lookups, CLI resolution, target resolution and adoption no longer strip the prefix from another host's group, nor twice from a group that starts with it. Co-Authored-By: Claude Opus 5.5 --- .../services/TargetResolutionService.spec.ts | 45 ++++++++++ .../services/TargetResolutionService.ts | 18 ++-- .../git/src/application/GitProviderService.ts | 25 ++++-- .../useCases/addGitRepo/AddGitRepoUseCase.ts | 56 +++++++++--- .../FindOrCreateGitRepoUseCase.ts | 57 ++++++------ .../GetTrackedRepositoryUseCase.spec.ts | 4 +- .../GetTrackedRepositoryUseCase.ts | 19 ++-- .../RemoveTrackedRepositoryUseCase.spec.ts | 4 +- .../RemoveTrackedRepositoryUseCase.ts | 40 +++++---- .../SetTrackedRepositoryUseCase.spec.ts | 4 +- .../SetTrackedRepositoryUseCase.ts | 25 +++--- .../useCases/shared/findByOwnerReadings.ts | 23 +++++ .../UpdateTrackedBranchUseCase.spec.ts | 4 +- .../UpdateTrackedBranchUseCase.ts | 27 +++--- .../src/self-hosted-repo-adoption.spec.ts | 87 ++++++++++++++++++- packages/node-utils/src/git/gitHost.spec.ts | 45 ++++++---- packages/node-utils/src/git/gitHost.ts | 33 ++++--- 17 files changed, 373 insertions(+), 143 deletions(-) create mode 100644 packages/git/src/application/useCases/shared/findByOwnerReadings.ts diff --git a/packages/deployments/src/application/services/TargetResolutionService.spec.ts b/packages/deployments/src/application/services/TargetResolutionService.spec.ts index f1181f512e..6c1af987d4 100644 --- a/packages/deployments/src/application/services/TargetResolutionService.spec.ts +++ b/packages/deployments/src/application/services/TargetResolutionService.spec.ts @@ -112,6 +112,51 @@ describe('TargetResolutionService', () => { }); }); + describe('when the remote is on an instance installed under a path prefix', () => { + const prefixedProvider: GitProviderListItem = { + ...githubProvider, + id: createGitProviderId(uuidv4()), + source: GitProviderVendors.gitlab, + url: 'https://devtools.acme.io/gitlab', + }; + const otherHostProvider: GitProviderListItem = { + ...githubProvider, + id: createGitProviderId(uuidv4()), + source: GitProviderVendors.gitlab, + url: 'https://gitlab.com', + }; + const otherHostRepoId = createGitRepoId(uuidv4()); + + beforeEach(() => { + gitPort.listProviders.mockResolvedValue({ + providers: [otherHostProvider, prefixedProvider], + }); + gitPort.listRepos.mockImplementation(async (id) => [ + gitRepoFactory({ + id: id === prefixedProvider.id ? gitRepoId : otherHostRepoId, + owner: 'team', + repo: 'app', + branch: 'main', + }), + ]); + targetService.getTargetsByGitRepoId.mockImplementation(async (id) => + id === gitRepoId ? [target] : [], + ); + }); + + it('returns the target of the repository of that instance', async () => { + const result = await service.findTargetFromGitInfo( + organizationId, + userId, + 'https://devtools.acme.io/gitlab/team/app.git', + gitBranch, + '/', + ); + + expect(result).toEqual(target); + }); + }); + describe('when no matching repo exists', () => { beforeEach(() => { gitPort.listProviders.mockResolvedValue({ diff --git a/packages/deployments/src/application/services/TargetResolutionService.ts b/packages/deployments/src/application/services/TargetResolutionService.ts index 3df055a1b8..65b891056c 100644 --- a/packages/deployments/src/application/services/TargetResolutionService.ts +++ b/packages/deployments/src/application/services/TargetResolutionService.ts @@ -15,7 +15,7 @@ import { TargetService } from './TargetService'; import { generateTargetName, normalizeRelativePath } from './gitInfoHelpers'; import { IDistributionRepository } from '../../domain/repositories/IDistributionRepository'; import { - ownerWithoutProviderPrefix, + ownerReadingsOf, parseGitRepoInfo, parseGitProviderVendor, } from '@packmind/node-utils'; @@ -40,26 +40,24 @@ export class TargetResolutionService { gitBranch: string, relativePath: string, ): Promise { - const { owner: remoteOwner, repo } = parseGitRepoInfo(gitRemoteUrl); + const { owner, repo } = parseGitRepoInfo(gitRemoteUrl); const providersResponse = await this.gitPort.listProviders({ userId, organizationId, }); - // The provider reports the group without the installation path prefix a - // remote of a prefixed instance carries. - const owner = ownerWithoutProviderPrefix( - remoteOwner, - providersResponse.providers.map((provider) => provider.url), - gitRemoteUrl, - ); let gitRepoId: string | null = null; for (const provider of providersResponse.providers) { + // A provider installed under a path prefix names the group without the + // prefix its remotes carry. + const owners = ownerReadingsOf(owner, provider.url, gitRemoteUrl).map( + (reading) => reading.toLowerCase(), + ); const repos = await this.gitPort.listRepos(provider.id); const matchingRepo = repos.find( (r) => - r.owner.toLowerCase() === owner.toLowerCase() && + owners.includes(r.owner.toLowerCase()) && r.repo.toLowerCase() === repo.toLowerCase() && r.branch === gitBranch, ); diff --git a/packages/git/src/application/GitProviderService.ts b/packages/git/src/application/GitProviderService.ts index 46f7af3715..971a8bef6f 100644 --- a/packages/git/src/application/GitProviderService.ts +++ b/packages/git/src/application/GitProviderService.ts @@ -16,7 +16,8 @@ import { } from '@packmind/types'; import { GitBranchComparison, GitRepo } from '@packmind/types'; import { OrganizationId, UserId } from '@packmind/types'; -import { ownerWithoutProviderPrefix } from '@packmind/node-utils'; +import { ownerReadingsOf } from '@packmind/node-utils'; +import { OwnerReading } from './useCases/shared/findByOwnerReadings'; import { v4 as uuidv4 } from 'uuid'; export class GitProviderService { @@ -67,18 +68,26 @@ export class GitProviderService { return this.gitProviderRepository.deleteById(id, userId); } - async ownerAsProvidersNameIt( + /** + * The owners a remote's group may be recorded under, the remote's own first. + * A group read without an installation path prefix only names the + * repositories of the provider installed under that prefix. + */ + async ownerReadings( organizationId: OrganizationId, owner: string, gitRemoteUrl?: string, - ): Promise { + ): Promise { const providers = await this.gitProviderRepository.findByOrganizationId(organizationId); - return ownerWithoutProviderPrefix( - owner, - providers.map((provider) => provider.url), - gitRemoteUrl, - ); + return [ + { owner, providerId: null }, + ...providers.flatMap((provider) => + ownerReadingsOf(owner, provider.url, gitRemoteUrl) + .slice(1) + .map((reading) => ({ owner: reading, providerId: provider.id })), + ), + ]; } async deleteGitProviderIfEmpty( diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts index a41a8edbf3..c4b9cc8bbc 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts @@ -12,11 +12,13 @@ import { GitProvider, GitProviderMissingTokenError, GitProviderOrganizationMismatchError, + GitRepo, GitRepoAlreadyExistsError, IAccountsPort, IAddGitRepoUseCase, IDeploymentPort, MissingGitInputError, + OrganizationId, providerHasAuth, } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; @@ -119,9 +121,6 @@ export class AddGitRepoUseCase throw new GitProviderMissingTokenError(gitProviderId); } - // The CLI recorded a remote of an instance installed under a path prefix - // with that prefix before the group, which the provider does not report. - const pathPrefix = providerPathPrefix(gitProvider.url); const existingRepo = (await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( owner, @@ -129,14 +128,13 @@ export class AddGitRepoUseCase branch, organization.id, )) ?? - (pathPrefix - ? await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( - `${pathPrefix}/${owner}`, - repo, - branch, - organization.id, - ) - : null); + (await this.findRecordedUnderPathPrefix( + gitProvider, + owner, + repo, + branch, + organization.id, + )); if (existingRepo) { const holdingProvider = @@ -219,4 +217,40 @@ export class AddGitRepoUseCase return createdRepo; } + + // The CLI recorded a remote of an instance installed under a path prefix + // with that prefix before the group, which the provider does not report. On + // another host the prefixed owner is a group of its own. + private async findRecordedUnderPathPrefix( + gitProvider: GitProvider, + owner: string, + repo: string, + branch: string, + organizationId: OrganizationId, + ): Promise { + const pathPrefix = providerPathPrefix(gitProvider.url); + if (!pathPrefix) { + return null; + } + const gitRepo = + await this.gitRepoService.findGitRepoByOwnerRepoAndBranchInOrganization( + `${pathPrefix}/${owner}`, + repo, + branch, + organizationId, + ); + if (!gitRepo) { + return null; + } + const holdingProvider = await this.gitProviderService.findGitProviderById( + gitRepo.providerId, + ); + return holdingProvider && + sameGitHost( + providerHostUrl(holdingProvider), + providerHostUrl(gitProvider), + ) + ? gitRepo + : null; + } } diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts index c3ba62df5b..6f17343288 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts @@ -13,7 +13,7 @@ import { } from '@packmind/types'; import { extractBaseUrl, - ownerWithoutProviderPrefix, + ownerReadingsOf, parseGitProviderVendor, sameGitHost, } from '@packmind/node-utils'; @@ -69,13 +69,8 @@ export class FindOrCreateGitRepoUseCase userId, organizationId, }); - // A remote cloned from an instance installed under a path prefix carries - // that prefix before the group; the provider reports the group alone. - const owner = ownerWithoutProviderPrefix( - command.owner, - providersResponse.providers.map((p) => p.url), - gitRemoteUrl, - ); + // Repositories of a CLI-managed provider keep the owner the remote names. + const owner = command.owner; // A provider's URL states which instance it reaches, so a self-hosted // remote finds the provider an admin configured for it, whatever vendor a @@ -93,16 +88,24 @@ export class FindOrCreateGitRepoUseCase // first match would raise a duplicate-repo error when a later provider // already hosts the repo. type ProviderInfo = (typeof tokenProviders)[number]; - const providersWithAccess: ProviderInfo[] = []; + const providersWithAccess: { provider: ProviderInfo; owner: string }[] = []; for (const provider of tokenProviders) { - const existingRepos = await this.gitPort.listRepos(provider.id); - const existingRepo = existingRepos.find( - (r) => - r.owner.toLowerCase() === owner.toLowerCase() && - r.repo.toLowerCase() === repo.toLowerCase() && - r.branch === branch, + // A provider installed under a path prefix names the group without it. + const owners = ownerReadingsOf(owner, provider.url, gitRemoteUrl).map( + (reading) => reading.toLowerCase(), ); + const existingRepos = await this.gitPort.listRepos(provider.id); + const existingRepo = owners + .map((reading) => + existingRepos.find( + (r) => + r.owner.toLowerCase() === reading && + r.repo.toLowerCase() === repo.toLowerCase() && + r.branch === branch, + ), + ) + .find(Boolean); if (existingRepo) { this.logger.info('Found existing repo under token provider', { @@ -118,14 +121,18 @@ export class FindOrCreateGitRepoUseCase userId, organizationId, }); - const canAccess = availableRepos.repositories.some( - (r) => - r.owner.toLowerCase() === owner.toLowerCase() && - r.name.toLowerCase() === repo.toLowerCase(), - ); - - if (canAccess) { - providersWithAccess.push(provider); + const accessibleRepo = owners + .map((reading) => + availableRepos.repositories.find( + (r) => + r.owner.toLowerCase() === reading && + r.name.toLowerCase() === repo.toLowerCase(), + ), + ) + .find(Boolean); + + if (accessibleRepo) { + providersWithAccess.push({ provider, owner: accessibleRepo.owner }); } } catch (error) { this.logger.info('Failed to list available repos for provider', { @@ -138,7 +145,7 @@ export class FindOrCreateGitRepoUseCase } if (providersWithAccess.length > 0) { - const provider = providersWithAccess[0]; + const { provider, owner: providerOwner } = providersWithAccess[0]; this.logger.info('Token can access repo, creating under token provider', { providerId: provider.id, }); @@ -146,7 +153,7 @@ export class FindOrCreateGitRepoUseCase userId, organizationId, gitProviderId: provider.id, - owner, + owner: providerOwner, repo, branch, }); diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts index 5ae9aa069d..e65744a8e4 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts @@ -19,7 +19,9 @@ import { GetTrackedRepositoryUseCase } from './GetTrackedRepositoryUseCase'; // Owners in these specs carry no installation prefix, so they pass through. const ownerAsIsProviderService = () => ({ - ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + ownerReadings: jest.fn(async (_organizationId, owner) => [ + { owner, providerId: null }, + ]), }) as Partial< jest.Mocked > as jest.Mocked; diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts index 94205b9e7a..b6e297b233 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts @@ -8,6 +8,7 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; +import { findByOwnerReadings } from '../shared/findByOwnerReadings'; const origin = 'GetTrackedRepositoryUseCase'; @@ -30,20 +31,20 @@ export class GetTrackedRepositoryUseCase protected async executeForMembers( command: GetTrackedRepositoryCommand & MemberContext, ): Promise { - const { owner: remoteOwner, repo, organization } = command; + const { owner, repo, organization } = command; // A remote cloned from an instance installed under a path prefix carries - // that prefix before the group; the repository is recorded without it. - const owner = await this.gitProviderService.ownerAsProvidersNameIt( + // that prefix before the group; its repository may be recorded without it. + const ownerReadings = await this.gitProviderService.ownerReadings( organization.id, - remoteOwner, + owner, ); - - const gitRepo = - await this.gitRepoService.findTrackedByOwnerRepoInOrganization( + const gitRepo = await findByOwnerReadings(ownerReadings, (ownerReading) => + this.gitRepoService.findTrackedByOwnerRepoInOrganization( organization.id, - owner, + ownerReading, repo, - ); + ), + ); return { gitRepo }; } diff --git a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts index 3f1db699ad..2520c1b64a 100644 --- a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.spec.ts @@ -23,7 +23,9 @@ import { RemoveTrackedRepositoryUseCase } from './RemoveTrackedRepositoryUseCase // Owners in these specs carry no installation prefix, so they pass through. const ownerAsIsProviderService = () => ({ - ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + ownerReadings: jest.fn(async (_organizationId, owner) => [ + { owner, providerId: null }, + ]), }) as Partial< jest.Mocked > as jest.Mocked; diff --git a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts index e0a30713b5..10a8acfd64 100644 --- a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts @@ -15,6 +15,7 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; +import { findByOwnerReadings } from '../shared/findByOwnerReadings'; const origin = 'RemoveTrackedRepositoryUseCase'; @@ -43,31 +44,36 @@ export class RemoveTrackedRepositoryUseCase protected async executeForAdmins( command: RemoveTrackedRepositoryCommand & AdminContext, ): Promise { - const { owner: remoteOwner, repo, organization, userId } = command; + const { owner, repo, organization, userId } = command; // A remote cloned from an instance installed under a path prefix carries - // that prefix before the group; the repository is recorded without it. - const owner = await this.gitProviderService.ownerAsProvidersNameIt( + // that prefix before the group; its repository may be recorded without it. + const ownerReadings = await this.gitProviderService.ownerReadings( organization.id, - remoteOwner, + owner, + ); + const existingTracked = await findByOwnerReadings( + ownerReadings, + (ownerReading) => + this.gitRepoService.findTrackedByOwnerRepoInOrganization( + organization.id, + ownerReading, + repo, + ), ); - - const existingTracked = - await this.gitRepoService.findTrackedByOwnerRepoInOrganization( - organization.id, - owner, - repo, - ); if (!existingTracked) { // Nothing tracked. Distinguish "connected but not governed" — a warning // the caller can ignore, and safe to repeat — from a repository Packmind // has never seen, which is a mistake worth failing on. - const knownRepo = - await this.gitRepoService.findByOwnerAndRepoInOrganization( - owner, - repo, - organization.id, - ); + const knownRepo = await findByOwnerReadings( + ownerReadings, + (ownerReading) => + this.gitRepoService.findByOwnerAndRepoInOrganization( + ownerReading, + repo, + organization.id, + ), + ); if (!knownRepo) { this.logger.warn('Tracking removal targets an unknown repository', { diff --git a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts index 30d15177de..79e0b6e3f8 100644 --- a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.spec.ts @@ -25,7 +25,9 @@ import { SetTrackedRepositoryUseCase } from './SetTrackedRepositoryUseCase'; // Owners in these specs carry no installation prefix, so they pass through. const ownerAsIsProviderService = () => ({ - ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + ownerReadings: jest.fn(async (_organizationId, owner) => [ + { owner, providerId: null }, + ]), }) as Partial< jest.Mocked > as jest.Mocked; diff --git a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts index a98f187240..a88e60bd3d 100644 --- a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts @@ -16,6 +16,7 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; +import { findByOwnerReadings } from '../shared/findByOwnerReadings'; const origin = 'SetTrackedRepositoryUseCase'; @@ -41,7 +42,7 @@ export class SetTrackedRepositoryUseCase command: SetTrackedRepositoryCommand & AdminContext, ): Promise { const { - owner: remoteOwner, + owner, repo, branch, origin: trackingOrigin, @@ -51,19 +52,21 @@ export class SetTrackedRepositoryUseCase userId, } = command; // A remote cloned from an instance installed under a path prefix carries - // that prefix before the group; the repository is recorded without it. - const owner = await this.gitProviderService.ownerAsProvidersNameIt( + // that prefix before the group; its repository may be recorded without it. + const ownerReadings = await this.gitProviderService.ownerReadings( organization.id, - remoteOwner, + owner, gitRemoteUrl, ); - - const existingTracked = - await this.gitRepoService.findTrackedByOwnerRepoInOrganization( - organization.id, - owner, - repo, - ); + const existingTracked = await findByOwnerReadings( + ownerReadings, + (ownerReading) => + this.gitRepoService.findTrackedByOwnerRepoInOrganization( + organization.id, + ownerReading, + repo, + ), + ); if (existingTracked) { // Idempotent: the requested branch is already tracked. diff --git a/packages/git/src/application/useCases/shared/findByOwnerReadings.ts b/packages/git/src/application/useCases/shared/findByOwnerReadings.ts new file mode 100644 index 0000000000..46ebb87cd1 --- /dev/null +++ b/packages/git/src/application/useCases/shared/findByOwnerReadings.ts @@ -0,0 +1,23 @@ +import { GitProviderId, GitRepo } from '@packmind/types'; + +export type OwnerReading = { + owner: string; + // The only provider whose repositories the reading names, null for any. + providerId: GitProviderId | null; +}; + +/** + * The first repository a reading of the owner finds, readings tried in order. + */ +export async function findByOwnerReadings( + readings: ReadonlyArray, + find: (owner: string) => Promise, +): Promise { + for (const { owner, providerId } of readings) { + const gitRepo = await find(owner); + if (gitRepo && (providerId === null || gitRepo.providerId === providerId)) { + return gitRepo; + } + } + return null; +} diff --git a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts index 0c93f502cf..ab87a0427e 100644 --- a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts +++ b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.spec.ts @@ -94,7 +94,9 @@ describe('UpdateTrackedBranchUseCase', () => { mockGitProviderService = { findGitProviderById: jest.fn().mockResolvedValue(provider), - ownerAsProvidersNameIt: jest.fn(async (_organizationId, owner) => owner), + ownerReadings: jest.fn(async (_organizationId, owner) => [ + { owner, providerId: null }, + ]), } as Partial< jest.Mocked > as jest.Mocked; diff --git a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts index aac6201909..40453604a9 100644 --- a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts +++ b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts @@ -17,6 +17,7 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; +import { findByOwnerReadings } from '../shared/findByOwnerReadings'; const origin = 'UpdateTrackedBranchUseCase'; @@ -41,20 +42,22 @@ export class UpdateTrackedBranchUseCase protected async executeForAdmins( command: UpdateTrackedBranchCommand & AdminContext, ): Promise { - const { owner: remoteOwner, repo, branch, organization, userId } = command; + const { owner, repo, branch, organization, userId } = command; // A remote cloned from an instance installed under a path prefix carries - // that prefix before the group; the repository is recorded without it. - const owner = await this.gitProviderService.ownerAsProvidersNameIt( + // that prefix before the group; its repository may be recorded without it. + const ownerReadings = await this.gitProviderService.ownerReadings( organization.id, - remoteOwner, + owner, + ); + const existingTracked = await findByOwnerReadings( + ownerReadings, + (ownerReading) => + this.gitRepoService.findTrackedByOwnerRepoInOrganization( + organization.id, + ownerReading, + repo, + ), ); - - const existingTracked = - await this.gitRepoService.findTrackedByOwnerRepoInOrganization( - organization.id, - owner, - repo, - ); // Nothing tracked yet — the caller must init/track first. if (!existingTracked) { @@ -95,7 +98,7 @@ export class UpdateTrackedBranchUseCase const gitRepo = await this.findOrCreateGitRepo.execute({ userId, organizationId: organization.id, - owner, + owner: existingTracked.owner, repo, branch, providerVendor: provider?.source, diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts index f0489d69da..d5e1e90a4f 100644 --- a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -98,7 +98,9 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { lastLoadedPage: 1, partial: false, repositories: repos.map((fullName) => { - const [owner, name] = fullName.split('/'); + const separator = fullName.lastIndexOf('/'); + const owner = fullName.slice(0, separator); + const name = fullName.slice(separator + 1); return { owner, name, @@ -660,6 +662,89 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { expect(gitRepo?.id).toBe(tracked.id); }); }); + + describe('and its group itself starts with the prefix', () => { + let token: GitProvider; + let tracked: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(PREFIXED_HOST, { + accessTo: [`gitlab/${OWNER}/${REPO}`], + }); + tracked = await testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/gitlab/${OWNER}`, + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: `https://devtools.acme.io/gitlab/gitlab/${OWNER}/${REPO}.git`, + }); + }); + + it('tracks it under the connection with the group it reports', () => { + expect(tracked).toMatchObject({ + providerId: token.id, + owner: `gitlab/${OWNER}`, + }); + }); + }); + + describe('and another host has a group named after the prefix', () => { + let tracked: GitRepo; + + beforeEach(async () => { + await connectTokenProvider(PREFIXED_HOST); + tracked = await testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/${OWNER}`, + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: `https://gitlab.other.io/gitlab/${OWNER}/${REPO}.git`, + }); + }); + + it('keeps the group of the other host', () => { + expect(tracked.owner).toBe(`gitlab/${OWNER}`); + }); + + it('finds its tracking by that group', async () => { + const { gitRepo } = await testApp.gitHexa + .getAdapter() + .getTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/${OWNER}`, + repo: REPO, + }); + + expect(gitRepo?.id).toBe(tracked.id); + }); + }); + + describe('and a connection of another host holds the prefixed group', () => { + let otherHostRepo: GitRepo; + let added: GitRepo; + + beforeEach(async () => { + otherHostRepo = await addFromApp( + await connectTokenProvider('https://gitlab.other.io', { + accessTo: [`gitlab/${OWNER}/${REPO}`], + }), + { owner: `gitlab/${OWNER}` }, + ); + added = await addFromApp(await connectTokenProvider(PREFIXED_HOST)); + }); + + it('adds the repository on the prefixed connection', () => { + expect(added.owner).toBe(OWNER); + }); + + it('leaves the other connection its repository', async () => { + expect(await currentProviderOf(otherHostRepo)).toBe( + otherHostRepo.providerId, + ); + }); + }); }); describe('when the CLI spelled the repository with another case', () => { diff --git a/packages/node-utils/src/git/gitHost.spec.ts b/packages/node-utils/src/git/gitHost.spec.ts index 6f33972a61..d85de66387 100644 --- a/packages/node-utils/src/git/gitHost.spec.ts +++ b/packages/node-utils/src/git/gitHost.spec.ts @@ -1,6 +1,6 @@ import { gitHostOf, - ownerWithoutProviderPrefix, + ownerReadingsOf, providerPathPrefix, sameGitHost, } from './gitHost'; @@ -160,52 +160,61 @@ describe('providerPathPrefix', () => { }); }); -describe('ownerWithoutProviderPrefix', () => { +describe('ownerReadingsOf', () => { const prefixed = 'https://devtools.acme.io/gitlab'; describe('when the remote is on a provider installed under a prefix', () => { - it('drops the prefix from the owner', () => { + it('reads the owner with and without the prefix', () => { expect( - ownerWithoutProviderPrefix( + ownerReadingsOf( 'gitlab/group/sub', - [prefixed], + prefixed, 'git@devtools.acme.io:gitlab/group/sub/app.git', ), - ).toBe('group/sub'); + ).toEqual(['gitlab/group/sub', 'group/sub']); }); }); describe('when the remote is on another host', () => { - it('keeps the owner', () => { + it('reads the owner as is', () => { expect( - ownerWithoutProviderPrefix( + ownerReadingsOf( 'gitlab/group', - [prefixed], + prefixed, 'https://gitlab.other.io/gitlab/group/app.git', ), - ).toBe('gitlab/group'); + ).toEqual(['gitlab/group']); }); }); describe('when no remote is known', () => { - it('drops the prefix of any provider', () => { - expect(ownerWithoutProviderPrefix('GitLab/group', [null, prefixed])).toBe( + it('reads the owner without the prefix too', () => { + expect(ownerReadingsOf('GitLab/group', prefixed)).toEqual([ + 'GitLab/group', 'group', - ); + ]); }); }); describe('when the owner does not start with the prefix', () => { - it('keeps the owner', () => { - expect(ownerWithoutProviderPrefix('gitlabber/group', [prefixed])).toBe( + it('reads the owner as is', () => { + expect(ownerReadingsOf('gitlabber/group', prefixed)).toEqual([ 'gitlabber/group', - ); + ]); }); }); describe('when the owner is the prefix alone', () => { - it('keeps the owner', () => { - expect(ownerWithoutProviderPrefix('gitlab', [prefixed])).toBe('gitlab'); + it('reads the owner as is', () => { + expect(ownerReadingsOf('gitlab', prefixed)).toEqual(['gitlab']); + }); + }); + + describe('when the provider sits at the root of its host', () => { + it('reads the owner as is', () => { + expect( + ownerReadingsOf('gitlab/group', 'https://devtools.acme.io'), + ).toEqual(['gitlab/group']); }); }); }); diff --git a/packages/node-utils/src/git/gitHost.ts b/packages/node-utils/src/git/gitHost.ts index 864f2f3911..1a0304efee 100644 --- a/packages/node-utils/src/git/gitHost.ts +++ b/packages/node-utils/src/git/gitHost.ts @@ -63,25 +63,24 @@ export function providerPathPrefix( } /** - * The owner as the provider names it. A remote cloned from an instance - * installed under a path prefix carries that prefix before the group - * (`gitlab/group` under `https://devtools.acme.io/gitlab`), which the provider - * never reports. Only providers on the remote's host are considered when the - * remote is known. + * The owners a provider may name a remote's group, the remote's own first. A + * remote cloned from an instance installed under a path prefix carries that + * prefix before the group (`gitlab/group` under + * `https://devtools.acme.io/gitlab`), which the provider never reports. The + * prefix is only this provider's when the remote, if known, is on its host. */ -export function ownerWithoutProviderPrefix( +export function ownerReadingsOf( owner: string, - providerUrls: ReadonlyArray, + providerUrl: string | null | undefined, gitRemoteUrl?: string, -): string { - for (const providerUrl of providerUrls) { - if (gitRemoteUrl && !sameGitHost(providerUrl, gitRemoteUrl)) { - continue; - } - const prefix = providerPathPrefix(providerUrl); - if (prefix && owner.toLowerCase().startsWith(`${prefix}/`)) { - return owner.slice(prefix.length + 1); - } +): string[] { + const prefix = providerPathPrefix(providerUrl); + if ( + !prefix || + !owner.toLowerCase().startsWith(`${prefix}/`) || + (gitRemoteUrl && !sameGitHost(providerUrl, gitRemoteUrl)) + ) { + return [owner]; } - return owner; + return [owner, owner.slice(prefix.length + 1)]; } From c09b37d770bb35613c6dae18b076176000160094 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Tue, 29 Sep 2026 22:02:50 +0200 Subject: [PATCH 14/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20harden=20self-?= =?UTF-8?q?hosted=20resolution=20against=20legacy=20and=20prefixed=20setup?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A web remote of a prefixed instance is read without the prefix, an SSH one as is; moving a tracked branch gives the owner as such a remote carries it. - A prefix naming the GitLab API root (/api/v4) is not part of the groups. - A repository is found under any CLI-managed provider of its host, as older servers kept one per SSH remote. - GitHub providers always reach github.com, whatever URL they store. - A remote naming no host is refused instead of adding a provider each call. - Tracked and known-repository lookups ignore case, as adoption records the owner as the provider spells it; per-provider readings are queried scoped. Co-Authored-By: Claude Opus 5.5 --- .../git/src/application/GitProviderService.ts | 9 +- .../git/src/application/GitRepoService.ts | 4 + .../FindOrCreateGitRepoUseCase.spec.ts | 54 +++++++ .../FindOrCreateGitRepoUseCase.ts | 49 ++++--- .../GetTrackedRepositoryUseCase.spec.ts | 2 +- .../GetTrackedRepositoryUseCase.ts | 15 +- .../RemoveTrackedRepositoryUseCase.ts | 6 +- .../SetTrackedRepositoryUseCase.ts | 3 +- .../useCases/shared/findByOwnerReadings.ts | 19 ++- .../useCases/shared/providerHostUrl.ts | 9 +- .../UpdateTrackedBranchUseCase.ts | 11 +- .../domain/repositories/IGitRepoRepository.ts | 1 + .../infra/repositories/GitRepoRepository.ts | 24 +++- .../src/self-hosted-repo-adoption.spec.ts | 132 ++++++++++++++++++ packages/node-utils/src/git/gitHost.spec.ts | 27 +++- packages/node-utils/src/git/gitHost.ts | 27 ++-- 16 files changed, 321 insertions(+), 71 deletions(-) diff --git a/packages/git/src/application/GitProviderService.ts b/packages/git/src/application/GitProviderService.ts index 971a8bef6f..121ea3597b 100644 --- a/packages/git/src/application/GitProviderService.ts +++ b/packages/git/src/application/GitProviderService.ts @@ -17,9 +17,14 @@ import { import { GitBranchComparison, GitRepo } from '@packmind/types'; import { OrganizationId, UserId } from '@packmind/types'; import { ownerReadingsOf } from '@packmind/node-utils'; -import { OwnerReading } from './useCases/shared/findByOwnerReadings'; import { v4 as uuidv4 } from 'uuid'; +export type OwnerReading = { + owner: string; + // The only provider whose repositories the reading names, null for any. + providerId: GitProviderId | null; +}; + export class GitProviderService { constructor( private readonly gitProviderRepository: IGitProviderRepository, @@ -84,7 +89,7 @@ export class GitProviderService { { owner, providerId: null }, ...providers.flatMap((provider) => ownerReadingsOf(owner, provider.url, gitRemoteUrl) - .slice(1) + .filter((reading) => reading !== owner) .map((reading) => ({ owner: reading, providerId: provider.id })), ), ]; diff --git a/packages/git/src/application/GitRepoService.ts b/packages/git/src/application/GitRepoService.ts index 88e1cc8347..2557497e8d 100644 --- a/packages/git/src/application/GitRepoService.ts +++ b/packages/git/src/application/GitRepoService.ts @@ -88,11 +88,13 @@ export class GitRepoService { organizationId: OrganizationId, owner: string, repo: string, + opts?: { providerId?: GitProviderId }, ): Promise { return this.gitRepoRepository.findTrackedByOwnerRepoInOrganization( organizationId, owner, repo, + opts, ); } @@ -137,11 +139,13 @@ export class GitRepoService { owner: string, repo: string, organizationId: OrganizationId, + opts?: { providerId?: GitProviderId }, ): Promise { return this.gitRepoRepository.findByOwnerAndRepoInOrganization( owner, repo, organizationId, + opts, ); } diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts index fc68dd9315..48c20f41e1 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.spec.ts @@ -12,6 +12,7 @@ import { IAccountsPort, IGitPort, Organization, + UnresolvableGitProviderError, User, } from '@packmind/types'; import { v4 as uuidv4 } from 'uuid'; @@ -114,6 +115,43 @@ describe('FindOrCreateGitRepoUseCase', () => { }); }); + // The GitHub API client targets github.com whatever URL the provider stores. + describe('when a GitHub token provider stores another github.com URL', () => { + let existingRepo: GitRepo; + let result: GitRepo; + + beforeEach(async () => { + const tokenProviderId = createGitProviderId(uuidv4()); + existingRepo = gitRepoFactory({ + owner: 'acme', + repo: 'widgets', + branch: 'dev', + providerId: tokenProviderId, + }); + mockGitPort.listProviders.mockResolvedValue({ + providers: [ + { + id: tokenProviderId, + source: GitProviderVendors.github, + organizationId, + url: 'https://api.github.com', + authMethod: 'token', + displayName: 'token-provider', + hasAuth: true, + lastDistributionAt: null, + }, + ], + }); + mockGitPort.listRepos.mockResolvedValue([existingRepo]); + + result = await useCase.execute(command); + }); + + it('finds the repository under it', () => { + expect(result).toEqual(existingRepo); + }); + }); + describe('when no provider hosts the repo', () => { let createdProvider: GitProvider; let createdRepo: GitRepo; @@ -326,6 +364,22 @@ describe('FindOrCreateGitRepoUseCase', () => { }); }); + describe('when the remote names no host', () => { + beforeEach(() => { + mockGitPort.listProviders.mockResolvedValue({ providers: [] }); + }); + + it('refuses the repository as unresolvable', async () => { + await expect( + useCase.execute({ + ...command, + providerVendor: 'unknown', + gitRemoteUrl: 'file:///srv/repos/acme/widgets.git', + }), + ).rejects.toBeInstanceOf(UnresolvableGitProviderError); + }); + }); + describe('when an old CLI sends a vendor but no remote', () => { const tokenProviderId = createGitProviderId(uuidv4()); diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts index 6f17343288..0ee7ae4030 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts @@ -13,6 +13,7 @@ import { } from '@packmind/types'; import { extractBaseUrl, + gitHostOf, ownerReadingsOf, parseGitProviderVendor, sameGitHost, @@ -46,7 +47,7 @@ export class FindOrCreateGitRepoUseCase protected async executeForMembers( command: FindOrCreateGitRepoCommand & MemberContext, ): Promise { - const { repo, branch, organization, userId } = command; + const { owner, repo, branch, organization, userId } = command; const gitRemoteUrl = command.gitRemoteUrl; // The remote is the server's own evidence; the vendor a CLI sends is only @@ -60,7 +61,7 @@ export class FindOrCreateGitRepoUseCase this.logger.info('Finding or creating git repo', { providerVendor, - owner: command.owner, + owner, repo, branch, }); @@ -69,9 +70,6 @@ export class FindOrCreateGitRepoUseCase userId, organizationId, }); - // Repositories of a CLI-managed provider keep the owner the remote names. - const owner = command.owner; - // A provider's URL states which instance it reaches, so a self-hosted // remote finds the provider an admin configured for it, whatever vendor a // substring of the remote suggests. @@ -178,7 +176,9 @@ export class FindOrCreateGitRepoUseCase expectedProviderUrl = 'https://github.com'; } else if (providerVendor === 'gitlab') { expectedProviderUrl = 'https://gitlab.com'; - } else if (gitRemoteUrl) { + } else if (gitRemoteUrl && gitHostOf(gitRemoteUrl)) { + // Without a host, no provider could ever match the remote again, and a + // new one would be created on every call. expectedProviderUrl = extractBaseUrl(gitRemoteUrl); } else { throw new UnresolvableGitProviderError(owner, repo); @@ -189,6 +189,27 @@ export class FindOrCreateGitRepoUseCase const tokenlessProviders = hostProviders.filter( (p) => !p.hasAuth && sameGitHost(p.url, expectedProviderUrl), ); + + // Older servers kept one provider per ssh:// remote, so the repository + // may sit under any CLI-managed provider of the host. + for (const provider of tokenlessProviders) { + const tokenlessRepos = await this.gitPort.listRepos(provider.id); + const existingTokenlessRepo = tokenlessRepos.find( + (r) => + r.owner.toLowerCase() === owner.toLowerCase() && + r.repo.toLowerCase() === repo.toLowerCase() && + r.branch === branch, + ); + + if (existingTokenlessRepo) { + this.logger.info('Found existing repo under tokenless provider', { + providerId: provider.id, + repoId: existingTokenlessRepo.id, + }); + return existingTokenlessRepo; + } + } + let tokenlessProvider = tokenlessProviders.find( (p) => p.url?.toLowerCase() === expectedProviderUrl.toLowerCase(), @@ -217,22 +238,6 @@ export class FindOrCreateGitRepoUseCase }; } - const tokenlessRepos = await this.gitPort.listRepos(tokenlessProvider.id); - const existingTokenlessRepo = tokenlessRepos.find( - (r) => - r.owner.toLowerCase() === owner.toLowerCase() && - r.repo.toLowerCase() === repo.toLowerCase() && - r.branch === branch, - ); - - if (existingTokenlessRepo) { - this.logger.info('Found existing repo under tokenless provider', { - providerId: tokenlessProvider.id, - repoId: existingTokenlessRepo.id, - }); - return existingTokenlessRepo; - } - this.logger.info('Creating repo under tokenless provider', { providerId: tokenlessProvider.id, }); diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts index e65744a8e4..d2861fa9af 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.spec.ts @@ -102,7 +102,7 @@ describe('GetTrackedRepositoryUseCase', () => { it('queries by organization, owner and repo', () => { expect( mockGitRepoService.findTrackedByOwnerRepoInOrganization, - ).toHaveBeenCalledWith(organizationId, 'acme', 'widgets'); + ).toHaveBeenCalledWith(organizationId, 'acme', 'widgets', {}); }); }); diff --git a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts index b6e297b233..20971d3712 100644 --- a/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/getTrackedRepository/GetTrackedRepositoryUseCase.ts @@ -38,12 +38,15 @@ export class GetTrackedRepositoryUseCase organization.id, owner, ); - const gitRepo = await findByOwnerReadings(ownerReadings, (ownerReading) => - this.gitRepoService.findTrackedByOwnerRepoInOrganization( - organization.id, - ownerReading, - repo, - ), + const gitRepo = await findByOwnerReadings( + ownerReadings, + (ownerReading, opts) => + this.gitRepoService.findTrackedByOwnerRepoInOrganization( + organization.id, + ownerReading, + repo, + opts, + ), ); return { gitRepo }; diff --git a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts index 10a8acfd64..03739430df 100644 --- a/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/removeTrackedRepository/RemoveTrackedRepositoryUseCase.ts @@ -53,11 +53,12 @@ export class RemoveTrackedRepositoryUseCase ); const existingTracked = await findByOwnerReadings( ownerReadings, - (ownerReading) => + (ownerReading, opts) => this.gitRepoService.findTrackedByOwnerRepoInOrganization( organization.id, ownerReading, repo, + opts, ), ); @@ -67,11 +68,12 @@ export class RemoveTrackedRepositoryUseCase // has never seen, which is a mistake worth failing on. const knownRepo = await findByOwnerReadings( ownerReadings, - (ownerReading) => + (ownerReading, opts) => this.gitRepoService.findByOwnerAndRepoInOrganization( ownerReading, repo, organization.id, + opts, ), ); diff --git a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts index a88e60bd3d..b054b43537 100644 --- a/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts +++ b/packages/git/src/application/useCases/setTrackedRepository/SetTrackedRepositoryUseCase.ts @@ -60,11 +60,12 @@ export class SetTrackedRepositoryUseCase ); const existingTracked = await findByOwnerReadings( ownerReadings, - (ownerReading) => + (ownerReading, opts) => this.gitRepoService.findTrackedByOwnerRepoInOrganization( organization.id, ownerReading, repo, + opts, ), ); diff --git a/packages/git/src/application/useCases/shared/findByOwnerReadings.ts b/packages/git/src/application/useCases/shared/findByOwnerReadings.ts index 46ebb87cd1..9341dff26d 100644 --- a/packages/git/src/application/useCases/shared/findByOwnerReadings.ts +++ b/packages/git/src/application/useCases/shared/findByOwnerReadings.ts @@ -1,21 +1,20 @@ import { GitProviderId, GitRepo } from '@packmind/types'; - -export type OwnerReading = { - owner: string; - // The only provider whose repositories the reading names, null for any. - providerId: GitProviderId | null; -}; +import { OwnerReading } from '../../GitProviderService'; /** - * The first repository a reading of the owner finds, readings tried in order. + * The first repository a reading of the owner finds, readings tried in order, + * each looked up among the repositories of the provider it names, if any. */ export async function findByOwnerReadings( readings: ReadonlyArray, - find: (owner: string) => Promise, + find: ( + owner: string, + opts: { providerId?: GitProviderId }, + ) => Promise, ): Promise { for (const { owner, providerId } of readings) { - const gitRepo = await find(owner); - if (gitRepo && (providerId === null || gitRepo.providerId === providerId)) { + const gitRepo = await find(owner, providerId ? { providerId } : {}); + if (gitRepo) { return gitRepo; } } diff --git a/packages/git/src/application/useCases/shared/providerHostUrl.ts b/packages/git/src/application/useCases/shared/providerHostUrl.ts index 7311fc2cbd..7577671ce3 100644 --- a/packages/git/src/application/useCases/shared/providerHostUrl.ts +++ b/packages/git/src/application/useCases/shared/providerHostUrl.ts @@ -6,12 +6,15 @@ const DEFAULT_HOST_BY_SOURCE: Partial> = { }; /** - * The URL of the instance a provider reaches. GitHub providers, App installs - * included, may store no URL: their API client always targets github.com, as - * GitLab's falls back to gitlab.com. + * The URL of the instance a provider reaches. The GitHub API client always + * targets github.com, whatever URL the provider stores, App installs + * included; GitLab's falls back to gitlab.com when it stores none. */ export function providerHostUrl( provider: Pick, ): string | null { + if (provider.source === 'github') { + return DEFAULT_HOST_BY_SOURCE.github ?? null; + } return provider.url || DEFAULT_HOST_BY_SOURCE[provider.source] || null; } diff --git a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts index 40453604a9..fc684f53cb 100644 --- a/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts +++ b/packages/git/src/application/useCases/updateTrackedBranch/UpdateTrackedBranchUseCase.ts @@ -3,6 +3,7 @@ import { AbstractAdminUseCase, AdminContext, PackmindEventEmitterService, + providerPathPrefix, } from '@packmind/node-utils'; import { createUserId, @@ -51,11 +52,12 @@ export class UpdateTrackedBranchUseCase ); const existingTracked = await findByOwnerReadings( ownerReadings, - (ownerReading) => + (ownerReading, opts) => this.gitRepoService.findTrackedByOwnerRepoInOrganization( organization.id, ownerReading, repo, + opts, ), ); @@ -95,10 +97,15 @@ export class UpdateTrackedBranchUseCase // Clear-then-set = last-one-wins (plain update, no locking). await this.gitRepoService.updateTracked(existingTracked.id, false); + // The provider URL stands in for the remote, so the owner is given as a + // web remote of that provider would carry it. + const pathPrefix = providerPathPrefix(provider?.url); const gitRepo = await this.findOrCreateGitRepo.execute({ userId, organizationId: organization.id, - owner: existingTracked.owner, + owner: pathPrefix + ? `${pathPrefix}/${existingTracked.owner}` + : existingTracked.owner, repo, branch, providerVendor: provider?.source, diff --git a/packages/git/src/domain/repositories/IGitRepoRepository.ts b/packages/git/src/domain/repositories/IGitRepoRepository.ts index 0e4d6b7180..4d0f47cb9a 100644 --- a/packages/git/src/domain/repositories/IGitRepoRepository.ts +++ b/packages/git/src/domain/repositories/IGitRepoRepository.ts @@ -51,6 +51,7 @@ export interface IGitRepoRepository extends IRepository { organizationId: OrganizationId, owner: string, repo: string, + opts?: { providerId?: GitProviderId }, ): Promise; updateTracked(gitRepoId: GitRepoId, isTracked: boolean): Promise; /** diff --git a/packages/git/src/infra/repositories/GitRepoRepository.ts b/packages/git/src/infra/repositories/GitRepoRepository.ts index ec5645a5f9..75bdd4a993 100644 --- a/packages/git/src/infra/repositories/GitRepoRepository.ts +++ b/packages/git/src/infra/repositories/GitRepoRepository.ts @@ -167,6 +167,7 @@ export class GitRepoRepository organizationId: OrganizationId, owner: string, repo: string, + opts?: { providerId?: GitProviderId }, ): Promise { this.logger.info('Finding tracked git repo by owner, repo, organization', { organizationId, @@ -175,20 +176,29 @@ export class GitRepoRepository }); try { - const gitRepo = await this.repository + const queryBuilder = this.repository .createQueryBuilder('gitRepo') .innerJoin( GitProviderSchema.options.name, 'provider', 'gitRepo.providerId = provider.id', ) - .where('gitRepo.owner = :owner', { owner }) - .andWhere('gitRepo.repo = :repo', { repo }) + // Hosts treat owner and repo case-insensitively, and an adoption + // records the owner as the provider spells it. + .where('LOWER(gitRepo.owner) = LOWER(:owner)', { owner }) + .andWhere('LOWER(gitRepo.repo) = LOWER(:repo)', { repo }) .andWhere('gitRepo.isTracked = :isTracked', { isTracked: true }) .andWhere('provider.organizationId = :organizationId', { organizationId, - }) - .getOne(); + }); + + if (opts?.providerId) { + queryBuilder.andWhere('gitRepo.providerId = :providerId', { + providerId: opts.providerId, + }); + } + + const gitRepo = await queryBuilder.getOne(); this.logger.info('Tracked git repo lookup completed', { organizationId, @@ -397,8 +407,8 @@ export class GitRepoRepository 'provider', 'gitRepo.providerId = provider.id', ) - .where('gitRepo.owner = :owner', { owner }) - .andWhere('gitRepo.repo = :repo', { repo }) + .where('LOWER(gitRepo.owner) = LOWER(:owner)', { owner }) + .andWhere('LOWER(gitRepo.repo) = LOWER(:repo)', { repo }) .andWhere('provider.organizationId = :organizationId', { organizationId, }); diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts index d5e1e90a4f..da210a036a 100644 --- a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -569,6 +569,29 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }); }); + describe('when older servers kept one CLI-managed connection per SSH remote', () => { + let lib: GitRepo; + let found: GitRepo; + + beforeEach(async () => { + await saveGhost('ssh://git@gitlab.acme.io:2222/acme/app.git', [{}]); + ({ + repos: [lib], + } = await saveGhost('ssh://git@gitlab.acme.io:2222/acme/lib.git', [ + { repo: 'lib' }, + ])); + + found = await recordFromCli({ + repo: 'lib', + remote: 'ssh://git@gitlab.acme.io:2222/acme/lib.git', + }); + }); + + it('finds the repository under the connection holding it', () => { + expect(found.id).toBe(lib.id); + }); + }); + describe('when the CLI-managed connection belongs to another organization', () => { let otherOrgRepo: GitRepo; let repo: GitRepo; @@ -689,6 +712,91 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }); }); + describe('and the connection URL names the API root', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(`${PREFIXED_HOST}/api/v4`); + repo = await recordPrefixedFromCli(); + }); + + it('creates it under the connection', () => { + expect(repo).toMatchObject({ providerId: token.id, owner: OWNER }); + }); + }); + + describe('and the instance also has a group named after the prefix', () => { + let repo: GitRepo; + + beforeEach(async () => { + await connectTokenProvider(PREFIXED_HOST, { + accessTo: [`${OWNER}/${REPO}`, `gitlab/${OWNER}/${REPO}`], + }); + repo = await recordPrefixedFromCli(); + }); + + it('reads the web remote with its prefix removed', () => { + expect(repo.owner).toBe(OWNER); + }); + }); + + // GitLab leaves its relative URL root out of SSH clone URLs. + describe('and the remote is an SSH one', () => { + let token: GitProvider; + let repo: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(PREFIXED_HOST, { + accessTo: [`gitlab/${OWNER}/${REPO}`], + }); + repo = await recordFromCli({ + owner: `gitlab/${OWNER}`, + remote: `git@devtools.acme.io:gitlab/${OWNER}/${REPO}.git`, + }); + }); + + it('reads its owner as is', () => { + expect(repo).toMatchObject({ + providerId: token.id, + owner: `gitlab/${OWNER}`, + }); + }); + }); + + describe('and the tracked branch of a group starting with the prefix moves', () => { + let token: GitProvider; + let moved: GitRepo; + + beforeEach(async () => { + token = await connectTokenProvider(PREFIXED_HOST, { + accessTo: [`gitlab/${OWNER}/${REPO}`], + }); + await testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/gitlab/${OWNER}`, + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: `https://devtools.acme.io/gitlab/gitlab/${OWNER}/${REPO}.git`, + }); + moved = await testApp.gitHexa.getAdapter().updateTrackedBranch({ + ...admin.packmindCommand(), + owner: `gitlab/gitlab/${OWNER}`, + repo: REPO, + branch: 'develop', + }); + }); + + it('tracks the new branch under the connection with the same group', () => { + expect(moved).toMatchObject({ + providerId: token.id, + owner: `gitlab/${OWNER}`, + branch: 'develop', + }); + }); + }); + describe('and another host has a group named after the prefix', () => { let tracked: GitRepo; @@ -782,6 +890,30 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }); }); + describe('when the connection spells the tracked owner with another case', () => { + let tracked: GitRepo; + + beforeEach(async () => { + tracked = await trackFromCli(); + await addFromApp( + await connectTokenProvider(HOST, { accessTo: [`Acme/${REPO}`] }), + { owner: 'Acme' }, + ); + }); + + it('stays found by the owner the CLI reads', async () => { + const { gitRepo } = await testApp.gitHexa + .getAdapter() + .getTrackedRepository({ + ...admin.packmindCommand(), + owner: OWNER, + repo: REPO, + }); + + expect(gitRepo?.id).toBe(tracked.id); + }); + }); + describe('when its tracking had been removed', () => { let tracked: GitRepo; diff --git a/packages/node-utils/src/git/gitHost.spec.ts b/packages/node-utils/src/git/gitHost.spec.ts index d85de66387..d31471a0dc 100644 --- a/packages/node-utils/src/git/gitHost.spec.ts +++ b/packages/node-utils/src/git/gitHost.spec.ts @@ -152,6 +152,12 @@ describe('providerPathPrefix', () => { 'ssh://git@gitlab.acme.io:2222/acme/app.git', null, ], + [ + 'a URL under a prefix naming the API root', + 'https://devtools.acme.io/gitlab/api/v4', + 'gitlab', + ], + ['a URL naming the API root', 'https://gitlab.acme.io/api/v4/', null], ['null', null, null], ])('with %s', (_label, url, expected) => { it(`returns ${expected}`, () => { @@ -163,15 +169,28 @@ describe('providerPathPrefix', () => { describe('ownerReadingsOf', () => { const prefixed = 'https://devtools.acme.io/gitlab'; - describe('when the remote is on a provider installed under a prefix', () => { - it('reads the owner with and without the prefix', () => { + describe('when a web remote is on a provider installed under a prefix', () => { + it('reads the owner without the prefix', () => { expect( ownerReadingsOf( 'gitlab/group/sub', prefixed, - 'git@devtools.acme.io:gitlab/group/sub/app.git', + 'https://devtools.acme.io/gitlab/group/sub/app.git', + ), + ).toEqual(['group/sub']); + }); + }); + + // GitLab leaves its relative URL root out of SSH clone URLs. + describe('when an SSH remote is on a provider installed under a prefix', () => { + it('reads the owner as is', () => { + expect( + ownerReadingsOf( + 'gitlab/group', + prefixed, + 'git@devtools.acme.io:gitlab/group/app.git', ), - ).toEqual(['gitlab/group/sub', 'group/sub']); + ).toEqual(['gitlab/group']); }); }); diff --git a/packages/node-utils/src/git/gitHost.ts b/packages/node-utils/src/git/gitHost.ts index 1a0304efee..ba9ca8f3bc 100644 --- a/packages/node-utils/src/git/gitHost.ts +++ b/packages/node-utils/src/git/gitHost.ts @@ -45,6 +45,7 @@ export function sameGitHost( // Only a web URL can carry an installation prefix: a CLI-managed provider may // hold a whole ssh:// remote, whose path is a repository, not a prefix. const WEB_URL_PATH = /^https?:\/\/[^/]+\/([^?#]*)/i; +const WEB_REMOTE = /^https?:\/\//i; /** * The path a provider URL puts before every group, lowercased and without @@ -58,16 +59,18 @@ export function providerPathPrefix( ?.trim() .match(WEB_URL_PATH)?.[1] .replace(/^\/+|\/+$/g, '') + // The GitLab client accepts a URL that already names its API root. + .replace(/(^|\/)api\/v4$/i, '') .toLowerCase(); return path ? path : null; } /** - * The owners a provider may name a remote's group, the remote's own first. A - * remote cloned from an instance installed under a path prefix carries that - * prefix before the group (`gitlab/group` under - * `https://devtools.acme.io/gitlab`), which the provider never reports. The - * prefix is only this provider's when the remote, if known, is on its host. + * The owners a provider may name a remote's group. A web remote of an instance + * installed under a path prefix carries that prefix before the group + * (`gitlab/group` under `https://devtools.acme.io/gitlab`), which the provider + * never reports; an SSH remote never carries it. When the remote is unknown, + * both readings remain, the owner as given first. */ export function ownerReadingsOf( owner: string, @@ -75,12 +78,14 @@ export function ownerReadingsOf( gitRemoteUrl?: string, ): string[] { const prefix = providerPathPrefix(providerUrl); - if ( - !prefix || - !owner.toLowerCase().startsWith(`${prefix}/`) || - (gitRemoteUrl && !sameGitHost(providerUrl, gitRemoteUrl)) - ) { + if (!prefix || !owner.toLowerCase().startsWith(`${prefix}/`)) { return [owner]; } - return [owner, owner.slice(prefix.length + 1)]; + const withoutPrefix = owner.slice(prefix.length + 1); + if (!gitRemoteUrl) { + return [owner, withoutPrefix]; + } + const remoteCarriesPrefix = + sameGitHost(providerUrl, gitRemoteUrl) && WEB_REMOTE.test(gitRemoteUrl); + return [remoteCarriesPrefix ? withoutPrefix : owner]; } From 78e37a2d4848377c3751aa415ce0813368cd0f51 Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Wed, 30 Sep 2026 09:17:03 +0200 Subject: [PATCH 15/17] =?UTF-8?q?=F0=9F=90=9B=20fix(node-utils):=20read=20?= =?UTF-8?q?a=20remote=20sent=20without=20a=20scheme=20again?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Older CLIs send `github.com/owner/repo`; the whole-path parser refused it and the distribution notification failed. Co-Authored-By: Claude Opus 5.5 --- .../shared => services}/providerHostUrl.ts | 0 .../node-utils/src/git/parseGitRepoInfo.spec.ts | 16 ++++++++++++++++ packages/node-utils/src/git/parseGitRepoInfo.ts | 9 +++++++-- 3 files changed, 23 insertions(+), 2 deletions(-) rename packages/git/src/application/{useCases/shared => services}/providerHostUrl.ts (100%) diff --git a/packages/git/src/application/useCases/shared/providerHostUrl.ts b/packages/git/src/application/services/providerHostUrl.ts similarity index 100% rename from packages/git/src/application/useCases/shared/providerHostUrl.ts rename to packages/git/src/application/services/providerHostUrl.ts diff --git a/packages/node-utils/src/git/parseGitRepoInfo.spec.ts b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts index 493a0f439d..68c7b82bc8 100644 --- a/packages/node-utils/src/git/parseGitRepoInfo.spec.ts +++ b/packages/node-utils/src/git/parseGitRepoInfo.spec.ts @@ -37,6 +37,22 @@ describe('parseGitRepoInfo', () => { }); }); + describe('when the remote has no scheme, as older CLIs sent it', () => { + it('reads the path after the host', () => { + expect(parseGitRepoInfo('github.com/my-company/my-repo')).toEqual({ + owner: 'my-company', + repo: 'my-repo', + }); + }); + + it('keeps the whole group path as owner', () => { + expect(parseGitRepoInfo('gitlab.acme.io/group/sub/app')).toEqual({ + owner: 'group/sub', + repo: 'app', + }); + }); + }); + describe('when the remote has no owner', () => { it('throws', () => { expect(() => parseGitRepoInfo('https://github.com/repo')).toThrow( diff --git a/packages/node-utils/src/git/parseGitRepoInfo.ts b/packages/node-utils/src/git/parseGitRepoInfo.ts index 158142b8ff..8c84e70449 100644 --- a/packages/node-utils/src/git/parseGitRepoInfo.ts +++ b/packages/node-utils/src/git/parseGitRepoInfo.ts @@ -2,6 +2,9 @@ const SCHEME_URL_PATH = /^[a-z][a-z0-9+.-]*:\/\/[^/]+\/(.+)$/i; // scp-like SSH, `[user@]host:path`, which git writes without a scheme. const SCP_LIKE_PATH = /^(?:[^@/\s]+@)?[^/:\s]+:(?!\/\/)(.+)$/; +// `host[:port]/path` without a scheme, as older CLIs sent it: the host is told +// apart from a group by its dot. +const BARE_HOST_PATH = /^[^/:@\s]+\.[^/:@\s]+(?::\d+)?\/(.+)$/; /** * Owner and repo from a git remote URL, for any host. The repo is the last @@ -15,8 +18,10 @@ export function parseGitRepoInfo(gitRemoteUrl: string): { } { // Accepts HTTPS (`https://host/owner/repo`) and SSH (`git@host:owner/repo`) // alike, with or without a `.git` suffix or a trailing slash. - const path = (gitRemoteUrl.trim().match(SCHEME_URL_PATH) ?? - gitRemoteUrl.trim().match(SCP_LIKE_PATH))?.[1]; + const trimmed = gitRemoteUrl.trim(); + const path = (trimmed.match(SCHEME_URL_PATH) ?? + trimmed.match(SCP_LIKE_PATH) ?? + trimmed.match(BARE_HOST_PATH))?.[1]; // An scp-like remote may name an absolute path: `git@host:/group/repo.git`. const segments = path From b54d30d6f22b28a48ed76bf1f1472ec056c6bfca Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Wed, 30 Sep 2026 09:17:11 +0200 Subject: [PATCH 16/17] =?UTF-8?q?=F0=9F=90=9B=20fix(git):=20read=20a=20tra?= =?UTF-8?q?cked=20owner=20on=20the=20remote's=20host=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the remote known, tracking no longer finds a repository of another host at the same path. Co-Authored-By: Claude Opus 5.5 --- .../git/src/application/GitProviderService.ts | 24 ++++++++++--- .../useCases/addGitRepo/AddGitRepoUseCase.ts | 2 +- .../FindOrCreateGitRepoUseCase.ts | 2 +- .../src/self-hosted-repo-adoption.spec.ts | 36 +++++++++++++++++++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/packages/git/src/application/GitProviderService.ts b/packages/git/src/application/GitProviderService.ts index 121ea3597b..116b1d172d 100644 --- a/packages/git/src/application/GitProviderService.ts +++ b/packages/git/src/application/GitProviderService.ts @@ -16,7 +16,8 @@ import { } from '@packmind/types'; import { GitBranchComparison, GitRepo } from '@packmind/types'; import { OrganizationId, UserId } from '@packmind/types'; -import { ownerReadingsOf } from '@packmind/node-utils'; +import { ownerReadingsOf, sameGitHost } from '@packmind/node-utils'; +import { providerHostUrl } from './services/providerHostUrl'; import { v4 as uuidv4 } from 'uuid'; export type OwnerReading = { @@ -74,9 +75,10 @@ export class GitProviderService { } /** - * The owners a remote's group may be recorded under, the remote's own first. - * A group read without an installation path prefix only names the - * repositories of the provider installed under that prefix. + * The owners a remote's group may be recorded under. With the remote, only + * the providers of its host are read; without it, the owner as given names + * a repository of any provider, and a group read without an installation + * path prefix only one of the provider installed under that prefix. */ async ownerReadings( organizationId: OrganizationId, @@ -85,10 +87,22 @@ export class GitProviderService { ): Promise { const providers = await this.gitProviderRepository.findByOrganizationId(organizationId); + if (gitRemoteUrl) { + return providers + .filter((provider) => + sameGitHost(providerHostUrl(provider), gitRemoteUrl), + ) + .flatMap((provider) => + ownerReadingsOf(owner, provider.url, gitRemoteUrl).map((reading) => ({ + owner: reading, + providerId: provider.id, + })), + ); + } return [ { owner, providerId: null }, ...providers.flatMap((provider) => - ownerReadingsOf(owner, provider.url, gitRemoteUrl) + ownerReadingsOf(owner, provider.url) .filter((reading) => reading !== owner) .map((reading) => ({ owner: reading, providerId: provider.id })), ), diff --git a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts index c4b9cc8bbc..76f485806c 100644 --- a/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/addGitRepo/AddGitRepoUseCase.ts @@ -23,7 +23,7 @@ import { } from '@packmind/types'; import { GitProviderService } from '../../GitProviderService'; import { GitRepoService } from '../../GitRepoService'; -import { providerHostUrl } from '../shared/providerHostUrl'; +import { providerHostUrl } from '../../services/providerHostUrl'; const origin = 'AddGitRepoUseCase'; diff --git a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts index 0ee7ae4030..d82ee556dc 100644 --- a/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts +++ b/packages/git/src/application/useCases/findOrCreateGitRepo/FindOrCreateGitRepoUseCase.ts @@ -19,7 +19,7 @@ import { sameGitHost, } from '@packmind/node-utils'; import { isProbeableSource } from '../shared/probeCandidateCredentials'; -import { providerHostUrl } from '../shared/providerHostUrl'; +import { providerHostUrl } from '../../services/providerHostUrl'; const origin = 'FindOrCreateGitRepoUseCase'; diff --git a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts index da210a036a..15ff34bb42 100644 --- a/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts +++ b/packages/integration-tests/src/self-hosted-repo-adoption.spec.ts @@ -797,6 +797,42 @@ describe('Self-hosted CLI-managed repository adoption integration', () => { }); }); + describe('and another host tracks a repository at the prefixed path', () => { + let otherHostTracked: GitRepo; + let token: GitProvider; + let tracked: GitRepo; + + beforeEach(async () => { + otherHostTracked = await testApp.gitHexa + .getAdapter() + .setTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/${OWNER}`, + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: `https://gitlab.other.io/gitlab/${OWNER}/${REPO}.git`, + }); + token = await connectTokenProvider(PREFIXED_HOST); + tracked = await testApp.gitHexa.getAdapter().setTrackedRepository({ + ...admin.packmindCommand(), + owner: `gitlab/${OWNER}`, + repo: REPO, + branch: BRANCH, + origin: 'track', + gitRemoteUrl: PREFIXED_REMOTE, + }); + }); + + it('tracks the repository of the remote host', () => { + expect(tracked).toMatchObject({ providerId: token.id, owner: OWNER }); + }); + + it('leaves the other host its tracked repository', () => { + expect(tracked.id).not.toBe(otherHostTracked.id); + }); + }); + describe('and another host has a group named after the prefix', () => { let tracked: GitRepo; From a1a4322e8c585bab40d79e33a72844472a7f3a0d Mon Sep 17 00:00:00 2001 From: Quentin Le Bourles Date: Wed, 30 Sep 2026 09:38:56 +0200 Subject: [PATCH 17/17] =?UTF-8?q?=E2=9C=85=20test(cli-e2e):=20gate=20insta?= =?UTF-8?q?ll-at-a-version=20above=20the=20published=200.36.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.36.1 is published without the feature, so the registry leg ran every scenario against a binary that answers `*` once pnpm's release-age window let it install that version. Co-Authored-By: Claude Opus 5.5 --- apps/cli-e2e-tests/src/install-package-versions.spec.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/cli-e2e-tests/src/install-package-versions.spec.ts b/apps/cli-e2e-tests/src/install-package-versions.spec.ts index a5378bad97..ba91a208fe 100644 --- a/apps/cli-e2e-tests/src/install-package-versions.spec.ts +++ b/apps/cli-e2e-tests/src/install-package-versions.spec.ts @@ -20,11 +20,12 @@ import { Package, Skill } from '@packmind/types'; * tracking the package" and its absence says "this repo is on a release". */ /* - * Strictly greater, not `>= 0.36.0`: the registry leg runs the latest - * published CLI, and 0.36.0 is published without any of this. A `>=` gate let - * every scenario below run against a binary that answers `*` to all of them. + * Strictly greater, not `>= 0.36.1`: the registry leg runs the latest + * published CLI, and 0.36.0 and 0.36.1 are published without any of this. A + * looser gate lets every scenario below run against a binary that answers `*` + * to all of them. */ -describeForVersion('> 0.36.0', 'install at a version', () => { +describeForVersion('> 0.36.1', 'install at a version', () => { describeWithUserSignedUp('install at a version', (getContext) => { let context: UserSignedUpContext; let pkg: Package;