fix(gitlab): select SSH workspace host for merge request and issue writes (#25985)

Co-authored-by: makoto-developer <72484465+makoto-developer@users.noreply.github.com>
This commit is contained in:
Neil
2026-10-06 15:42:22 -07:00
committed by GitHub
co-authored by makoto-developer
parent d8c871a1f0
commit fa42e57659
10 changed files with 265 additions and 65 deletions
@@ -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()
})
})
})
@@ -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<ProjectRef, 'host'> | 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<ProjectRef, 'host'>,
+45
View File
@@ -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)
})
})
+1
View File
@@ -18,6 +18,7 @@ export {
getIssueProjectRef,
getProjectRef,
getProjectRefForRemote,
glabHostEnvOptions,
glabHostnameArgs,
glabRepoExecOptions,
parseGlabAuthStatusHosts,
+10 -23
View File
@@ -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)
+31
View File
@@ -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'))
@@ -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 {
+75 -1
View File
@@ -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')
})
})
+3 -3
View File
@@ -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
})
@@ -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) {