Files
orca/src/main/source-control/forge-provider.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

487 lines
15 KiB
TypeScript

import { supportsHostedReviewCreation } from '../../shared/hosted-review-creation-providers'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
createGitHubPullRequestMock,
createBitbucketPullRequestMock,
createGitLabMergeRequestMock,
createAzureDevOpsPullRequestMock,
createGiteaPullRequestMock,
getAzureDevOpsRepoSlugMock,
getBitbucketRepoSlugMock,
getGiteaRepoSlugMock,
getMergeRequestForBranchMock,
getProjectSlugMock,
getPRForBranchOutcomeMock,
getRepoSlugMock,
getGitHubPRLookupRateLimitBlockMock,
getEnterpriseGitHubRepoSlugMock
} = vi.hoisted(() => ({
createGitHubPullRequestMock: vi.fn(),
createBitbucketPullRequestMock: vi.fn(),
createGitLabMergeRequestMock: vi.fn(),
createAzureDevOpsPullRequestMock: vi.fn(),
createGiteaPullRequestMock: vi.fn(),
getAzureDevOpsRepoSlugMock: vi.fn(),
getBitbucketRepoSlugMock: vi.fn(),
getGiteaRepoSlugMock: vi.fn(),
getMergeRequestForBranchMock: vi.fn(),
getProjectSlugMock: vi.fn(),
getPRForBranchOutcomeMock: vi.fn(),
getRepoSlugMock: vi.fn(),
getGitHubPRLookupRateLimitBlockMock: vi.fn(async () => null),
getEnterpriseGitHubRepoSlugMock: vi.fn()
}))
vi.mock('../gitlab/client', () => ({
getProjectSlug: getProjectSlugMock,
getMergeRequestForBranch: getMergeRequestForBranchMock,
// Why: forge-provider resolves branch reviews via the OrThrow variant so
// lookup failures surface as unavailable instead of "no MR found".
getMergeRequestForBranchOrThrow: getMergeRequestForBranchMock,
getMergeRequest: vi.fn()
}))
vi.mock('../gitlab/merge-request-creation', () => ({
createGitLabMergeRequest: createGitLabMergeRequestMock
}))
vi.mock('../github/client', () => ({
createGitHubPullRequest: createGitHubPullRequestMock,
getRepoSlug: getRepoSlugMock,
getPRForBranchOutcome: getPRForBranchOutcomeMock,
getGitHubPRLookupRateLimitBlock: getGitHubPRLookupRateLimitBlockMock
}))
vi.mock('../github/github-enterprise-repository', () => ({
getEnterpriseGitHubRepoSlug: getEnterpriseGitHubRepoSlugMock
}))
vi.mock('../bitbucket/client', () => ({
getBitbucketRepoSlug: getBitbucketRepoSlugMock,
getBitbucketPullRequestForBranch: vi.fn(),
getBitbucketPullRequest: vi.fn()
}))
vi.mock('../bitbucket/pull-request-creation', () => ({
createBitbucketPullRequest: createBitbucketPullRequestMock
}))
vi.mock('../azure-devops/client', () => ({
getAzureDevOpsRepoSlug: getAzureDevOpsRepoSlugMock,
getAzureDevOpsPullRequestForBranch: vi.fn(),
getAzureDevOpsPullRequest: vi.fn()
}))
vi.mock('../azure-devops/pull-request-creation', () => ({
createAzureDevOpsPullRequest: createAzureDevOpsPullRequestMock
}))
vi.mock('../gitea/client', () => ({
getGiteaRepoSlug: getGiteaRepoSlugMock,
getGiteaPullRequestForBranch: vi.fn(),
getGiteaPullRequest: vi.fn()
}))
vi.mock('../gitea/pull-request-creation', () => ({
createGiteaPullRequest: createGiteaPullRequestMock
}))
import {
FORGE_PROVIDERS,
detectHostedReviewProvider,
getForgeProviderById,
getForgeProviderForRepository
} from './forge-provider'
import { _resetOriginGitHubApiRepositoryCache } from '../github/github-api-repository'
// The origin-repository cache is module-level state; reset it so slugs
// resolved by one test cannot leak into the next.
beforeEach(() => {
_resetOriginGitHubApiRepositoryCache()
})
describe('forge provider interface', () => {
beforeEach(() => {
createGitHubPullRequestMock.mockReset()
createGitLabMergeRequestMock.mockReset()
createBitbucketPullRequestMock.mockReset()
createAzureDevOpsPullRequestMock.mockReset()
createGiteaPullRequestMock.mockReset()
getAzureDevOpsRepoSlugMock.mockReset()
getBitbucketRepoSlugMock.mockReset()
getGiteaRepoSlugMock.mockReset()
getMergeRequestForBranchMock.mockReset()
getProjectSlugMock.mockReset()
getPRForBranchOutcomeMock.mockReset()
getRepoSlugMock.mockReset()
getEnterpriseGitHubRepoSlugMock.mockReset()
getGitHubPRLookupRateLimitBlockMock.mockReset()
getGitHubPRLookupRateLimitBlockMock.mockResolvedValue(null)
})
it('preserves the existing hosted provider detection order', async () => {
getProjectSlugMock.mockResolvedValue({ host: 'gitlab.com', path: 'team/orca' })
getRepoSlugMock.mockResolvedValue({ owner: 'team', repo: 'orca' })
await expect(
detectHostedReviewProvider({ executionHostId: 'local', repoPath: '/repo' })
).resolves.toBe('gitlab')
await expect(
getForgeProviderForRepository({ executionHostId: 'local', repoPath: '/repo' })
).resolves.toMatchObject({
id: 'gitlab'
})
expect(getRepoSlugMock).not.toHaveBeenCalled()
})
it('detects a GitHub Enterprise Server remote as the GitHub provider, not Gitea', async () => {
// Regression for #8312: a GHES host is not github.com, so github.com-only
// slug parsing returns null. Detection must claim it via the enterprise
// resolver instead of falling through to Gitea's demand for ORCA_GITEA_TOKEN.
getProjectSlugMock.mockResolvedValue(null)
// Why: getRepoSlug resolves hosted identities itself now — a GHES remote
// comes back host-qualified instead of null + separate enterprise fallback.
getRepoSlugMock.mockResolvedValue({
owner: 'team',
repo: 'orca',
host: 'github.acme-corp.com'
})
await expect(
detectHostedReviewProvider({ executionHostId: 'local', repoPath: '/repo' })
).resolves.toBe('github')
await expect(
getForgeProviderForRepository({ executionHostId: 'local', repoPath: '/repo' })
).resolves.toMatchObject({
id: 'github'
})
// Gitea must never be consulted once GitHub claims the enterprise host.
expect(getGiteaRepoSlugMock).not.toHaveBeenCalled()
})
it('leaves a genuinely non-GitHub remote for later providers when gh is not authenticated', async () => {
getProjectSlugMock.mockResolvedValue(null)
getRepoSlugMock.mockResolvedValue(null)
// gh is not logged in to this host, so the enterprise resolver declines and
// the Gitea provider is free to claim its own self-hosted remote.
getEnterpriseGitHubRepoSlugMock.mockResolvedValue(null)
getBitbucketRepoSlugMock.mockResolvedValue(null)
getAzureDevOpsRepoSlugMock.mockResolvedValue(null)
getGiteaRepoSlugMock.mockResolvedValue({
host: 'gitea.example.com',
owner: 'team',
repo: 'orca',
apiBaseUrl: 'https://gitea.example.com/api/v1',
webBaseUrl: 'https://gitea.example.com'
})
await expect(
detectHostedReviewProvider({ executionHostId: 'local', repoPath: '/repo' })
).resolves.toBe('gitea')
})
it('keeps review creation capability scoped to providers with creation support', async () => {
expect(
FORGE_PROVIDERS.map((provider) => [provider.id, provider.supportsReviewCreation])
).toEqual([
['gitlab', true],
['github', true],
['bitbucket', true],
['azure-devops', true],
['gitea', true]
])
// Why: the shared list is what the Create blocker and the renderer read.
// When it drifted from this one, Bitbucket had a working createReview but
// still reported "provider does not support creating a pull request".
for (const provider of FORGE_PROVIDERS) {
expect(supportsHostedReviewCreation(provider.id)).toBe(provider.supportsReviewCreation)
}
createGitHubPullRequestMock.mockResolvedValue({
ok: true,
number: 12,
url: 'https://github.com/team/orca/pull/12'
})
const provider = getForgeProviderById('github')
await expect(
provider.createReview?.(
'/repo',
{
provider: 'github',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'local'
)
).resolves.toEqual({
ok: true,
number: 12,
url: 'https://github.com/team/orca/pull/12'
})
expect(createGitHubPullRequestMock).toHaveBeenCalledWith(
'/repo',
{
provider: 'github',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'local'
)
})
it('routes Bitbucket review creation through the shared provider contract', async () => {
createBitbucketPullRequestMock.mockResolvedValue({
ok: true,
number: 23,
url: 'https://bitbucket.org/team/orca/pull-requests/23'
})
const provider = getForgeProviderById('bitbucket')
const input = {
provider: 'bitbucket' as const,
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
}
await expect(provider.createReview?.('/repo', input, 'local')).resolves.toEqual({
ok: true,
number: 23,
url: 'https://bitbucket.org/team/orca/pull-requests/23'
})
expect(createBitbucketPullRequestMock).toHaveBeenCalledWith('/repo', input, 'local')
})
it('routes GitLab review creation through the shared provider contract', async () => {
createGitLabMergeRequestMock.mockResolvedValue({
ok: true,
number: 44,
url: 'https://gitlab.com/team/orca/-/merge_requests/44'
})
const provider = getForgeProviderById('gitlab')
await expect(
provider.createReview?.(
'/repo',
{
provider: 'gitlab',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
).resolves.toEqual({
ok: true,
number: 44,
url: 'https://gitlab.com/team/orca/-/merge_requests/44'
})
expect(createGitLabMergeRequestMock).toHaveBeenCalledWith(
'/repo',
{
provider: 'gitlab',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
})
it('routes Azure DevOps review creation through the shared provider contract', async () => {
createAzureDevOpsPullRequestMock.mockResolvedValue({
ok: true,
number: 88,
url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88'
})
const provider = getForgeProviderById('azure-devops')
await expect(
provider.createReview?.(
'/repo',
{
provider: 'azure-devops',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
).resolves.toEqual({
ok: true,
number: 88,
url: 'https://dev.azure.com/acme/Project/_git/orca/pullrequest/88'
})
expect(createAzureDevOpsPullRequestMock).toHaveBeenCalledWith(
'/repo',
{
provider: 'azure-devops',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
})
it('routes Gitea review creation through the shared provider contract', async () => {
createGiteaPullRequestMock.mockResolvedValue({
ok: true,
number: 19,
url: 'https://git.example.com/team/orca/pulls/19'
})
const provider = getForgeProviderById('gitea')
await expect(
provider.createReview?.(
'/repo',
{
provider: 'gitea',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
).resolves.toEqual({
ok: true,
number: 19,
url: 'https://git.example.com/team/orca/pulls/19'
})
expect(createGiteaPullRequestMock).toHaveBeenCalledWith(
'/repo',
{
provider: 'gitea',
base: 'main',
head: 'feature/provider-interface',
title: 'Add provider interface'
},
'ssh:ssh-1'
)
})
it('adapts GitHub branch lookup through the shared provider contract', async () => {
getPRForBranchOutcomeMock.mockResolvedValue({
kind: 'found',
fetchedAt: 1,
pr: {
number: 7,
title: 'Provider branch',
state: 'open',
url: 'https://github.com/team/orca/pull/7',
checksStatus: 'success',
updatedAt: '2026-05-29T00:00:00.000Z',
mergeable: 'MERGEABLE'
}
})
await expect(
getForgeProviderById('github').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'ssh:ssh-1',
branch: '',
fallbackReviewNumber: 7
})
).resolves.toMatchObject({
provider: 'github',
number: 7,
status: 'success'
})
expect(getPRForBranchOutcomeMock).toHaveBeenCalledWith('/repo', '', null, 'ssh-1', 7, {
acceptMergedFallbackPR: true,
currentHeadOid: null
})
})
it('passes the worktree HEAD oid through to the GitHub lookup', async () => {
getPRForBranchOutcomeMock.mockResolvedValue({ kind: 'no-pr', fetchedAt: 1 })
await getForgeProviderById('github').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'local',
branch: 'feature/x',
githubCurrentHeadOid: 'abc1234'
})
expect(getPRForBranchOutcomeMock).toHaveBeenCalledWith('/repo', 'feature/x', null, null, null, {
currentHeadOid: 'abc1234'
})
})
it('returns null for a confirmed GitHub no-pr lookup', async () => {
getPRForBranchOutcomeMock.mockResolvedValue({ kind: 'no-pr', fetchedAt: 1 })
await expect(
getForgeProviderById('github').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'local',
branch: 'feature/x'
})
).resolves.toBeNull()
})
it('throws on a GitHub upstream error instead of reporting no review', async () => {
getPRForBranchOutcomeMock.mockResolvedValue({
kind: 'upstream-error',
errorType: 'network',
message: 'connection reset',
fetchedAt: 1
})
await expect(
getForgeProviderById('github').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'local',
branch: 'feature/x'
})
).rejects.toThrow(/network/)
})
it('refuses a GitHub branch lookup while the rate-limit budget is exhausted (#11532)', async () => {
getGitHubPRLookupRateLimitBlockMock.mockResolvedValueOnce({
resetAt: 1_800_000_000
} as never)
await expect(
getForgeProviderById('github').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'local',
branch: 'feature/x'
})
// Throwing (not null) keeps a low budget from reading as "no pull request".
).rejects.toThrow(/rate_limited/)
expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled()
})
it('refuses a GitHub lookup by number while the rate-limit budget is exhausted (#11532)', async () => {
getGitHubPRLookupRateLimitBlockMock.mockResolvedValueOnce({
resetAt: 1_800_000_000
} as never)
await expect(
getForgeProviderById('github').getReviewByNumber({
repoPath: '/repo',
executionHostId: 'local',
number: 42
})
).rejects.toThrow(/rate_limited/)
expect(getPRForBranchOutcomeMock).not.toHaveBeenCalled()
})
it('does not gate non-GitHub providers on the GitHub rate limit', async () => {
getGitHubPRLookupRateLimitBlockMock.mockResolvedValue({ resetAt: 1_800_000_000 } as never)
getMergeRequestForBranchMock.mockResolvedValue(null)
await expect(
getForgeProviderById('gitlab').getReviewForBranch({
repoPath: '/repo',
executionHostId: 'local',
branch: 'feature/x'
})
).resolves.toBeNull()
})
})