diff --git a/src/main/gitlab/client-mr-review-actions.test.ts b/src/main/gitlab/client-mr-review-actions.test.ts index 4d651bdc618..37c5b49fa5f 100644 --- a/src/main/gitlab/client-mr-review-actions.test.ts +++ b/src/main/gitlab/client-mr-review-actions.test.ts @@ -451,4 +451,69 @@ describe('gitlab client — MR operations', () => { ) }) }) + // Why: only `api`/`auth` define --hostname, so the `mr` subcommands must take + // the self-hosted host through GITLAB_HOST or glab exits on an unknown flag. + describe('mr subcommands against a self-hosted SSH host', () => { + const projectRef = { host: 'gitlab.example.internal', path: 'g/p' } + + it('merges without --hostname and passes the host through GITLAB_HOST', async () => { + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(mergeMR('/repo', 6, 'merge', undefined, 'conn-1', projectRef)).resolves.toEqual({ + ok: true + }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['mr', 'merge', '6', '-R', 'g/p', '--yes']) + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + it('keeps the squash flag while routing the host through the environment', async () => { + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(mergeMR('/repo', 6, 'squash', undefined, 'conn-1', projectRef)).resolves.toEqual( + { ok: true } + ) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['mr', 'merge', '6', '-R', 'g/p', '--yes', '--squash']) + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + it('closes without --hostname and passes the host through GITLAB_HOST', async () => { + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(closeMR('/repo', 6, undefined, 'conn-1', projectRef)).resolves.toEqual({ + ok: true + }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['mr', 'close', '6', '-R', 'g/p']) + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + it('reopens without --hostname and passes the host through GITLAB_HOST', async () => { + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(reopenMR('/repo', 6, undefined, 'conn-1', projectRef)).resolves.toEqual({ + ok: true + }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['mr', 'reopen', '6', '-R', 'g/p']) + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + it('leaves the environment untouched for a local workspace', async () => { + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(mergeMR('/repo', 6, 'merge', undefined, null, projectRef)).resolves.toEqual({ + ok: true + }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['mr', 'merge', '6', '-R', 'g/p', '--yes']) + expect(options.env).toBeUndefined() + }) + }) }) diff --git a/src/main/gitlab/gitlab-project-ref-resolution.ts b/src/main/gitlab/gitlab-project-ref-resolution.ts index c337c01ae89..293030960e7 100644 --- a/src/main/gitlab/gitlab-project-ref-resolution.ts +++ b/src/main/gitlab/gitlab-project-ref-resolution.ts @@ -1,6 +1,7 @@ import { glabExecFileAsync } from '../git/runner' import type { GitAdmissionTier } from '../git/command-runner/git-exec-options' import { shouldProbeGitRemote } from '../git/remote-name-listing' +import { addWslEnvKeys } from '../wsl-env' import { isTransientGitProbeError, readRemoteUrl } from '../git/remote-url-probe' import { NEGATIVE_ENTRY_TTL_MS } from '../git/remote-ref-probe-cache' import { getSshGitProviderGeneration } from '../providers/ssh-git-dispatch' @@ -275,6 +276,22 @@ export function glabHostnameArgs( return connectionId && projectRef?.host ? ['--hostname', projectRef.host] : [] } +/** Why: only `api`/`auth` define `--hostname`; `mr` and `issue` subcommands reject it (#12193). */ +export function glabHostEnvOptions( + projectRef: Pick | null | undefined, + connectionId?: string | null +): { env?: NodeJS.ProcessEnv } { + if (!connectionId || !projectRef?.host) { + return {} + } + const env: NodeJS.ProcessEnv = { ...process.env, GITLAB_HOST: projectRef.host } + if (process.platform === 'win32') { + // Why: spawn env stops at the wsl.exe boundary unless WSLENV names the var. + addWslEnvKeys(env, ['GITLAB_HOST']) + } + return { env } +} + async function isGlabConfiguredForRemoteHost( repoPath: string, projectRef: Pick, diff --git a/src/main/gitlab/gl-utils.test.ts b/src/main/gitlab/gl-utils.test.ts index 2c8b36dd0ec..a895e4702e9 100644 --- a/src/main/gitlab/gl-utils.test.ts +++ b/src/main/gitlab/gl-utils.test.ts @@ -25,6 +25,7 @@ import { parseGlabJsonList, parseGlabPaginationHeader, isMissingJobLogError, + glabHostEnvOptions, getProjectRef, getProjectRefForRemote, parseGlabApiResponse, @@ -698,3 +699,47 @@ describe('parseGlabPaginationHeader', () => { expect(parseGlabPaginationHeader('-3', 0)).toBeUndefined() }) }) + +describe('glabHostEnvOptions', () => { + const projectRef = { host: 'gitlab.example.internal' } + + afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() + }) + + it('carries the host in GITLAB_HOST for a connection-backed workspace', () => { + const { env } = glabHostEnvOptions(projectRef, 'conn-1') + + expect(env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + it('returns nothing for a local workspace', () => { + expect(glabHostEnvOptions(projectRef, null)).toEqual({}) + expect(glabHostEnvOptions(projectRef, undefined)).toEqual({}) + }) + + it('returns nothing when the project ref carries no host', () => { + expect(glabHostEnvOptions(null, 'conn-1')).toEqual({}) + expect(glabHostEnvOptions({ host: '' }, 'conn-1')).toEqual({}) + }) + + // Why: spawn env stops at the wsl.exe boundary, so Windows has to name the + // variable in WSLENV or glab in the distro never sees the host. + it('forwards GITLAB_HOST through WSLENV on Windows', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + + const { env } = glabHostEnvOptions(projectRef, 'conn-1') + + expect(env?.GITLAB_HOST).toBe('gitlab.example.internal') + expect(env?.WSLENV?.split(':')).toContain('GITLAB_HOST') + }) + + it('leaves WSLENV alone off Windows', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + + const { env } = glabHostEnvOptions(projectRef, 'conn-1') + + expect(env?.WSLENV).toBe(process.env.WSLENV) + }) +}) diff --git a/src/main/gitlab/gl-utils.ts b/src/main/gitlab/gl-utils.ts index f31c47105de..0991ed520cd 100644 --- a/src/main/gitlab/gl-utils.ts +++ b/src/main/gitlab/gl-utils.ts @@ -18,6 +18,7 @@ export { getIssueProjectRef, getProjectRef, getProjectRefForRemote, + glabHostEnvOptions, glabHostnameArgs, glabRepoExecOptions, parseGlabAuthStatusHosts, diff --git a/src/main/gitlab/issue-update.ts b/src/main/gitlab/issue-update.ts index 369e889ff09..2181b6e8208 100644 --- a/src/main/gitlab/issue-update.ts +++ b/src/main/gitlab/issue-update.ts @@ -5,6 +5,7 @@ import { classifyGlabError, getGlabKnownHosts, glabExecFileAsync, + glabHostEnvOptions, glabHostnameArgs, glabRepoExecOptions, release, @@ -56,17 +57,10 @@ export async function updateIssue( await acquire() try { const cmd = updates.state === 'closed' ? 'close' : 'reopen' - await glabExecFileAsync( - [ - 'issue', - cmd, - String(issueNumber), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) + await glabExecFileAsync(['issue', cmd, String(issueNumber), '-R', repoFlag], { + ...glabRepoExecOptions(repoPath, connectionId, localGitOptions), + ...glabHostEnvOptions(projectRef, connectionId) + }) } catch (err) { const stderr = err instanceof Error ? err.message : String(err) // Treat "already closed/reopened" as a no-op (matches gh path). @@ -102,14 +96,7 @@ export async function updateIssue( } // Field edits via `glab issue update`. - const editArgs: string[] = [ - 'issue', - 'update', - String(issueNumber), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ] + const editArgs: string[] = ['issue', 'update', String(issueNumber), '-R', repoFlag] let hasEditArgs = false if (updates.title) { @@ -136,10 +123,10 @@ export async function updateIssue( if (hasEditArgs) { await acquire() try { - await glabExecFileAsync( - editArgs, - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) + await glabExecFileAsync(editArgs, { + ...glabRepoExecOptions(repoPath, connectionId, localGitOptions), + ...glabHostEnvOptions(projectRef, connectionId) + }) } catch (err) { const stderr = err instanceof Error ? err.message : String(err) errors.push(classifyGlabError(stderr).message) diff --git a/src/main/gitlab/issues.test.ts b/src/main/gitlab/issues.test.ts index 9baa049e65c..29009ee6235 100644 --- a/src/main/gitlab/issues.test.ts +++ b/src/main/gitlab/issues.test.ts @@ -340,6 +340,37 @@ describe('gitlab issue operations', () => { ) }) + // Why: `glab issue update` has no --hostname flag either, so field edits on an + // SSH workspace must carry the host in GITLAB_HOST (#12193). + it('updateIssue edits fields through GITLAB_HOST on an SSH workspace', async () => { + const projectRef = { host: 'gitlab.example.internal', path: 'stablyai/orca' } + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) + + await expect( + updateIssue('/repo-root', 5, { title: 'Renamed' }, undefined, 'conn-1', projectRef) + ).resolves.toEqual({ ok: true }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args.slice(0, 2)).toEqual(['issue', 'update']) + expect(args).not.toContain('--hostname') + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + // Why: `glab issue close` has no --hostname flag; a self-hosted instance is + // only reachable through GITLAB_HOST (#12193). + it('updateIssue closes through GITLAB_HOST on an SSH workspace', async () => { + const projectRef = { host: 'gitlab.example.internal', path: 'stablyai/orca' } + glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) + + await expect( + updateIssue('/repo-root', 5, { state: 'closed' }, undefined, 'conn-1', projectRef) + ).resolves.toEqual({ ok: true }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args).toEqual(['issue', 'close', '5', '-R', 'stablyai/orca']) + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + it("updateIssue treats 'already closed' as a no-op", async () => { getIssueProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'stablyai/orca' }) glabExecFileAsyncMock.mockRejectedValueOnce(new Error('Issue is already closed')) diff --git a/src/main/gitlab/merge-request-creation-lookup.ts b/src/main/gitlab/merge-request-creation-lookup.ts index e5c8303ce1a..256348ad976 100644 --- a/src/main/gitlab/merge-request-creation-lookup.ts +++ b/src/main/gitlab/merge-request-creation-lookup.ts @@ -4,7 +4,7 @@ import { } from '../source-control/hosted-review-git-options' import { glabExecFileAsync, - glabHostnameArgs, + glabHostEnvOptions, glabRepoExecOptions, type ProjectRef } from './gl-utils' @@ -65,12 +65,12 @@ export async function findOpenMRByHeadBase(args: { '--per-page', '2', '--output', - 'json', - ...glabHostnameArgs(args.projectRef, args.connectionId) + 'json' ], { ...glabRepoExecOptions(args.repoPath, args.connectionId), - ...(args.connectionId ? {} : getHostedReviewLocalGitOptions(args.options)) + ...(args.connectionId ? {} : getHostedReviewLocalGitOptions(args.options)), + ...glabHostEnvOptions(args.projectRef, args.connectionId) } ) const list = JSON.parse(stdout) as { diff --git a/src/main/gitlab/merge-request-creation.test.ts b/src/main/gitlab/merge-request-creation.test.ts index c5fc6e06970..6efdbd45d82 100644 --- a/src/main/gitlab/merge-request-creation.test.ts +++ b/src/main/gitlab/merge-request-creation.test.ts @@ -4,6 +4,7 @@ const { getProjectSlugMock, glabExecFileAsyncMock, glabHostnameArgsMock, + glabHostEnvOptionsMock, glabRepoExecOptionsMock, acquireMock, releaseMock, @@ -12,6 +13,9 @@ const { getProjectSlugMock: vi.fn(), glabExecFileAsyncMock: vi.fn(), glabHostnameArgsMock: vi.fn((projectRef: { host: string }) => ['--hostname', projectRef.host]), + glabHostEnvOptionsMock: vi.fn((projectRef: { host: string }, connectionId?: string | null) => + connectionId ? { env: { GITLAB_HOST: projectRef.host } } : {} + ), glabRepoExecOptionsMock: vi.fn((repoPath: string, connectionId?: string | null) => connectionId ? {} : { cwd: repoPath } ), @@ -29,6 +33,7 @@ vi.mock('./gl-utils', () => ({ release: releaseMock, glabExecFileAsync: glabExecFileAsyncMock, glabHostnameArgs: glabHostnameArgsMock, + glabHostEnvOptions: glabHostEnvOptionsMock, glabRepoExecOptions: glabRepoExecOptionsMock })) @@ -43,6 +48,7 @@ describe('createGitLabMergeRequest', () => { getProjectSlugMock.mockReset() glabExecFileAsyncMock.mockReset() glabHostnameArgsMock.mockClear() + glabHostEnvOptionsMock.mockClear() glabRepoExecOptionsMock.mockClear() acquireMock.mockReset() releaseMock.mockReset() @@ -94,12 +100,15 @@ describe('createGitLabMergeRequest', () => { '--draft' ]) ) - expect(args).toEqual(expect.arrayContaining(['--hostname', 'gitlab.com'])) + // Why: a local workspace resolves the host from cwd, so neither the flag + // nor the env override belongs on the command. + expect(args).not.toContain('--hostname') expect(options).toMatchObject({ cwd: '/repo-root', timeout: 60_000, idempotent: false }) + expect(options.env).toBeUndefined() expect(acquireMock).toHaveBeenCalledOnce() expect(releaseMock).toHaveBeenCalledOnce() }) @@ -268,4 +277,69 @@ describe('createGitLabMergeRequest', () => { ]) ) }) + + // Why: `glab mr create` has no --hostname flag either, so an SSH workspace + // has to reach its instance through GITLAB_HOST (#12193). + it('creates a merge request through GITLAB_HOST on an SSH workspace', async () => { + getProjectSlugMock.mockResolvedValue({ host: 'gitlab.example.internal', path: 'acme/widgets' }) + glabExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ + iid: 51, + web_url: 'https://gitlab.example.internal/acme/widgets/-/merge_requests/51' + }), + stderr: '' + }) + + await expect( + createGitLabMergeRequest( + '/remote/repo-root', + { + provider: 'gitlab', + base: 'main', + head: 'feature/ssh-host', + title: 'SSH host MR' + }, + 'ssh:ssh-1' + ) + ).resolves.toMatchObject({ ok: true, number: 51 }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[0] + expect(args.slice(0, 2)).toEqual(['mr', 'create']) + expect(args).not.toContain('--hostname') + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) + + // Why: `glab mr list` has no --hostname flag, so the duplicate lookup has to + // reach a self-hosted instance through GITLAB_HOST instead (#12193). + it('looks up an existing merge request through GITLAB_HOST on an SSH workspace', async () => { + glabExecFileAsyncMock + .mockRejectedValueOnce(new Error('merge request already exists')) + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + iid: 77, + web_url: 'https://gitlab.example.internal/acme/widgets/-/merge_requests/77' + } + ]), + stderr: '' + }) + getProjectSlugMock.mockResolvedValue({ host: 'gitlab.example.internal', path: 'acme/widgets' }) + + await expect( + createGitLabMergeRequest( + '/repo-root', + { + provider: 'gitlab', + base: 'main', + head: 'feature/existing', + title: 'Existing MR' + }, + 'ssh:ssh-1' + ) + ).resolves.toMatchObject({ ok: false, code: 'already_exists' }) + + const [args, options] = glabExecFileAsyncMock.mock.calls[1] + expect(args).not.toContain('--hostname') + expect(options.env?.GITLAB_HOST).toBe('gitlab.example.internal') + }) }) diff --git a/src/main/gitlab/merge-request-creation.ts b/src/main/gitlab/merge-request-creation.ts index 8d0fc520802..57153fb40ae 100644 --- a/src/main/gitlab/merge-request-creation.ts +++ b/src/main/gitlab/merge-request-creation.ts @@ -18,7 +18,7 @@ import { getProjectSlug } from './client' import { acquire, glabExecFileAsync, - glabHostnameArgs, + glabHostEnvOptions, glabRepoExecOptions, release } from './gl-utils' @@ -188,8 +188,7 @@ export async function createGitLabMergeRequest( title, '--description', body, - '--yes', - ...glabHostnameArgs(projectRef, connectionId) + '--yes' ] if (head) { createArgs.push('--source-branch', head) @@ -201,6 +200,7 @@ export async function createGitLabMergeRequest( const { stdout } = await glabExecFileAsync(createArgs, { ...glabRepoExecOptions(repoPath, connectionId), ...(connectionId ? {} : getHostedReviewLocalGitOptions(options)), + ...glabHostEnvOptions(projectRef, connectionId), timeout: 60_000, idempotent: false }) diff --git a/src/main/gitlab/merge-request-state-mutations.ts b/src/main/gitlab/merge-request-state-mutations.ts index fb5d33bb670..7aa70ceb5b0 100644 --- a/src/main/gitlab/merge-request-state-mutations.ts +++ b/src/main/gitlab/merge-request-state-mutations.ts @@ -1,7 +1,7 @@ import type { IssueSourcePreference } from '../../shared/repo-types' import { acquire, - glabHostnameArgs, + glabHostEnvOptions, glabRepoExecOptions, glabExecFileAsync, release, @@ -26,17 +26,10 @@ export async function closeMR( async (projectRef, repoFlag) => { await acquire() try { - await glabExecFileAsync( - [ - 'mr', - 'close', - String(iid), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) + await glabExecFileAsync(['mr', 'close', String(iid), '-R', repoFlag], { + ...glabRepoExecOptions(repoPath, connectionId, localGitOptions), + ...glabHostEnvOptions(projectRef, connectionId) + }) return { ok: true } } catch (err) { const msg = err instanceof Error ? err.message : String(err) @@ -70,17 +63,10 @@ export async function reopenMR( async (projectRef, repoFlag) => { await acquire() try { - await glabExecFileAsync( - [ - 'mr', - 'reopen', - String(iid), - '-R', - repoFlag, - ...glabHostnameArgs(projectRef, connectionId) - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) - ) + await glabExecFileAsync(['mr', 'reopen', String(iid), '-R', repoFlag], { + ...glabRepoExecOptions(repoPath, connectionId, localGitOptions), + ...glabHostEnvOptions(projectRef, connectionId) + }) return { ok: true } } catch (err) { const msg = err instanceof Error ? err.message : String(err) @@ -118,17 +104,11 @@ export async function mergeMR( const methodFlag = method === 'squash' ? ['--squash'] : method === 'rebase' ? ['--rebase'] : [] await glabExecFileAsync( - [ - 'mr', - 'merge', - String(iid), - '-R', - repoFlag, - '--yes', - ...methodFlag, - ...glabHostnameArgs(projectRef, connectionId) - ], - glabRepoExecOptions(repoPath, connectionId, localGitOptions) + ['mr', 'merge', String(iid), '-R', repoFlag, '--yes', ...methodFlag], + { + ...glabRepoExecOptions(repoPath, connectionId, localGitOptions), + ...glabHostEnvOptions(projectRef, connectionId) + } ) return { ok: true } } catch (err) {