Files
orca/src/main/gitlab/merge-request-creation.test.ts
T
Neil d05dd8ef50 fix(source-control): route hosted reviews by resolved execution host (#18382)
`ForgeProvider.createReview(repoPath, input, connectionId, options)` and the
`connectionId` on `ForgeProviderRepositoryContext` carried the same collapse the
five prior migrations closed: `string | null` spells "genuinely local", "runtime
host" and "could not resolve" with one value. Because it was decided two layers
up -- `repo.connectionId ?? null` at the `hostedReview:*` IPC handlers and in
`RuntimeHostedReviewCommands` -- a row naming its owner only as
`executionHostId: ssh:<target>` ran the whole review path against this machine's
copy of a remote path (#11163): `git rev-parse`, `git status`, the base-on-remote
ref probe, the upstream divergence read, and `gh`/`glab` with no host flags.

Replace it with a required `ExecutionHostId` threaded from the decision point
through the contract, routed by #18296's `resolveGitRouteForHost`. The parameter
is removed rather than added beside, so all five implementations -- GitLab,
GitHub, Bitbucket, Azure DevOps, Gitea -- and every caller became a compile
error. None of these families carries `@ts-nocheck`, so unlike #18325 that
guarantee is real here; `orca-runtime-file-commands.ts` does, but it only
constructs `RuntimeHostedReviewCommands` with unchanged deps.

Also fixed at the sites:

- The branch cache scoped entries on `connectionId ?? ''`, so two rows at one
  path on different hosts shared one cached review, one backoff deadline and one
  invalidation. Keyed on the resolved host now, as #18377 did for its probe key.
- `hostedReview:create` resolved shared symlink paths and normalized worktree
  paths off the raw field, so an `executionHostId`-only SSH row read `orca.yaml`
  and `resolve()`d a remote POSIX path on the client. Those ask the file-holder
  question -- `getRepoSshConnectionId` -- not the dialable one.
- An SSH host with no provider now refuses inside the git-state layer instead of
  reaching the local branch, keeping "remote and unreachable" distinct from
  "local" (docs/reference/ssh-execution-boundary.md).

`runtime:` is a routing mistake inside `hostedReviewSshConnectionId` -- that
environment's server runs its own git, and the SSH target on its repo row is
nested in that server's namespace, so dialing it here reaches a same-named box of
ours. But store-backed callers ask `getRepoHostedReviewExecutionHostId` first,
which is "what may this client dial" and answers `local` for a `runtime:` row.
That is deliberate and matches #18377: the runtime registration controller only
adopts a `runtime:` stamp onto a row with no `connectionId`
(`runtimeRepoMatchesExecutionHost` refuses to match an SSH row), so the checkout
really is in this process and refusing would regress a runtime server creating
reviews for its own rows.

No wire change. `connectionId` on `CreateHostedReviewArgs`,
`CreateStackedHostedReviewArgs` and `HostedReviewCreationEligibilityArgs` in
src/shared/hosted-review.ts is untouched -- every host already ignores it in
favor of the repo row, and removing it from the request types would only churn
the schema older clients still populate. The main-side eligibility input `Omit`s
it so nothing on this side can read the ambiguous field again.
2026-09-03 01:32:46 -07:00

272 lines
7.3 KiB
TypeScript

import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
getProjectSlugMock,
glabExecFileAsyncMock,
glabHostnameArgsMock,
glabRepoExecOptionsMock,
acquireMock,
releaseMock,
getSshFilesystemProviderMock
} = vi.hoisted(() => ({
getProjectSlugMock: vi.fn(),
glabExecFileAsyncMock: vi.fn(),
glabHostnameArgsMock: vi.fn((projectRef: { host: string }) => ['--hostname', projectRef.host]),
glabRepoExecOptionsMock: vi.fn((repoPath: string, connectionId?: string | null) =>
connectionId ? {} : { cwd: repoPath }
),
acquireMock: vi.fn(),
releaseMock: vi.fn(),
getSshFilesystemProviderMock: vi.fn()
}))
vi.mock('./client', () => ({
getProjectSlug: getProjectSlugMock
}))
vi.mock('./gl-utils', () => ({
acquire: acquireMock,
release: releaseMock,
glabExecFileAsync: glabExecFileAsyncMock,
glabHostnameArgs: glabHostnameArgsMock,
glabRepoExecOptions: glabRepoExecOptionsMock
}))
vi.mock('../providers/ssh-filesystem-dispatch', () => ({
getSshFilesystemProvider: getSshFilesystemProviderMock
}))
import { createGitLabMergeRequest } from './merge-request-creation'
describe('createGitLabMergeRequest', () => {
beforeEach(() => {
getProjectSlugMock.mockReset()
glabExecFileAsyncMock.mockReset()
glabHostnameArgsMock.mockClear()
glabRepoExecOptionsMock.mockClear()
acquireMock.mockReset()
releaseMock.mockReset()
getSshFilesystemProviderMock.mockReset()
acquireMock.mockResolvedValue(undefined)
getProjectSlugMock.mockResolvedValue({ host: 'gitlab.com', path: 'acme/widgets' })
})
it('creates a GitLab merge request with normalized refs', async () => {
glabExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'https://gitlab.com/acme/widgets/-/merge_requests/42\n',
stderr: ''
})
await expect(
createGitLabMergeRequest(
'/repo-root',
{
provider: 'gitlab',
base: 'origin/main',
head: 'refs/heads/feature/create-mr',
title: ' Create MR UI ',
body: 'Body text',
draft: true
},
'local'
)
).resolves.toEqual({
ok: true,
number: 42,
url: 'https://gitlab.com/acme/widgets/-/merge_requests/42'
})
const [args, options] = glabExecFileAsyncMock.mock.calls[0]
expect(args).toEqual(
expect.arrayContaining([
'mr',
'create',
'-R',
'acme/widgets',
'--target-branch',
'main',
'--source-branch',
'feature/create-mr',
'--title',
'Create MR UI',
'--description',
'Body text',
'--draft'
])
)
expect(args).toEqual(expect.arrayContaining(['--hostname', 'gitlab.com']))
expect(options).toMatchObject({
cwd: '/repo-root',
timeout: 60_000,
idempotent: false
})
expect(acquireMock).toHaveBeenCalledOnce()
expect(releaseMock).toHaveBeenCalledOnce()
})
it('runs local WSL project merge request creation through the selected distro', async () => {
glabExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'https://gitlab.com/acme/widgets/-/merge_requests/43\n',
stderr: ''
})
await expect(
createGitLabMergeRequest(
'/repo-root',
{
provider: 'gitlab',
base: 'main',
head: 'feature/wsl-create-mr',
title: 'WSL Create MR'
},
'local',
{ localGitExecOptions: { wslDistro: 'Ubuntu' } }
)
).resolves.toEqual({
ok: true,
number: 43,
url: 'https://gitlab.com/acme/widgets/-/merge_requests/43'
})
const [, options] = glabExecFileAsyncMock.mock.calls[0]
expect(options).toMatchObject({
cwd: '/repo-root',
wslDistro: 'Ubuntu',
timeout: 60_000,
idempotent: false
})
})
it('creates SSH-backed merge requests without using the remote path as a local cwd', async () => {
glabExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({
iid: 45,
web_url: 'https://gitlab.com/acme/widgets/-/merge_requests/45'
}),
stderr: ''
})
await expect(
createGitLabMergeRequest(
'/remote/repo-root',
{
provider: 'gitlab',
base: 'main',
head: 'feature/ssh-create-mr',
title: 'SSH Create MR'
},
'ssh:ssh-1'
)
).resolves.toEqual({
ok: true,
number: 45,
url: 'https://gitlab.com/acme/widgets/-/merge_requests/45'
})
expect(getProjectSlugMock).toHaveBeenCalledWith('/remote/repo-root', 'ssh-1')
const [args, options] = glabExecFileAsyncMock.mock.calls[0]
expect(args).toEqual(
expect.arrayContaining([
'mr',
'create',
'-R',
'acme/widgets',
'--target-branch',
'main',
'--source-branch',
'feature/ssh-create-mr'
])
)
expect(options).toMatchObject({
timeout: 60_000,
idempotent: false
})
expect(options).not.toHaveProperty('cwd')
})
it('reads merge request templates from the SSH filesystem provider', async () => {
const readRemoteFile = vi.fn(async (path: string) => {
if (path === '/remote/repo-root/.gitlab/merge_request_templates/Default.md') {
return { content: 'Remote MR template body', isBinary: false }
}
throw new Error('missing template')
})
getSshFilesystemProviderMock.mockReturnValue({ readFile: readRemoteFile })
glabExecFileAsyncMock.mockResolvedValueOnce({
stdout: 'https://gitlab.com/acme/widgets/-/merge_requests/46\n',
stderr: ''
})
await expect(
createGitLabMergeRequest(
'/remote/repo-root',
{
provider: 'gitlab',
base: 'main',
head: 'feature/ssh-template',
title: 'SSH Template MR',
body: '',
useTemplate: true
},
'ssh:ssh-1'
)
).resolves.toEqual({
ok: true,
number: 46,
url: 'https://gitlab.com/acme/widgets/-/merge_requests/46'
})
const [args] = glabExecFileAsyncMock.mock.calls[0]
expect(readRemoteFile).toHaveBeenCalledWith(
'/remote/repo-root/.gitlab/merge_request_templates/Default.md'
)
expect(args[args.indexOf('--description') + 1]).toBe('Remote MR template body')
})
it('returns an existing merge request when GitLab reports a duplicate', async () => {
glabExecFileAsyncMock
.mockRejectedValueOnce(new Error('merge request already exists'))
.mockResolvedValueOnce({
stdout: JSON.stringify([
{
iid: 77,
web_url: 'https://gitlab.com/acme/widgets/-/merge_requests/77'
}
]),
stderr: ''
})
await expect(
createGitLabMergeRequest(
'/repo-root',
{
provider: 'gitlab',
base: 'main',
head: 'feature/existing',
title: 'Existing MR'
},
'local'
)
).resolves.toEqual({
ok: false,
code: 'already_exists',
error: 'A merge request already exists for this branch.',
existingReview: {
number: 77,
url: 'https://gitlab.com/acme/widgets/-/merge_requests/77'
}
})
expect(glabExecFileAsyncMock.mock.calls[1][0]).toEqual(
expect.arrayContaining([
'mr',
'list',
'--source-branch',
'feature/existing',
'--target-branch',
'main'
])
)
})
})