mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(gitlab): load MR reviewer choices from the paired server that owns the repo (#29917)
The runtime GitLab API answered listAssignableUsers with an empty list, so the reviewer picker was always empty for a paired server's repo. Add an optional gitlab.listAssignableUsers RPC on the host; a host that predates it answers method_not_found and the picker stays empty as before.
This commit is contained in:
@@ -220,6 +220,7 @@ export {
|
||||
listGitLabWorkItemsMock,
|
||||
listGitLabIssuesMock,
|
||||
listGitLabLabelsMock,
|
||||
listGitLabAssignableUsersMock,
|
||||
listGitLabTodosMock,
|
||||
getGitLabProjectRefForRemoteMock,
|
||||
getGitLabWorkItemByProjectRefMock,
|
||||
|
||||
@@ -162,6 +162,7 @@ const {
|
||||
listGitLabWorkItemsMock,
|
||||
listGitLabIssuesMock,
|
||||
listGitLabLabelsMock,
|
||||
listGitLabAssignableUsersMock,
|
||||
listGitLabTodosMock,
|
||||
getGitLabProjectRefForRemoteMock,
|
||||
getGitLabWorkItemByProjectRefMock,
|
||||
@@ -248,6 +249,7 @@ const {
|
||||
listGitLabWorkItemsMock: vi.fn() as TestMock,
|
||||
listGitLabIssuesMock: vi.fn() as TestMock,
|
||||
listGitLabLabelsMock: vi.fn() as TestMock,
|
||||
listGitLabAssignableUsersMock: vi.fn<TestMock>(),
|
||||
listGitLabTodosMock: vi.fn() as TestMock,
|
||||
getGitLabProjectRefForRemoteMock: vi.fn() as TestMock,
|
||||
getGitLabWorkItemByProjectRefMock: vi.fn() as TestMock,
|
||||
@@ -440,6 +442,7 @@ vi.mock('../../gitlab/client', async (importOriginal) => {
|
||||
listWorkItems: listGitLabWorkItemsMock,
|
||||
listIssues: listGitLabIssuesMock,
|
||||
listLabels: listGitLabLabelsMock,
|
||||
listAssignableUsers: listGitLabAssignableUsersMock,
|
||||
listTodos: listGitLabTodosMock,
|
||||
getProjectRefForRemote: getGitLabProjectRefForRemoteMock,
|
||||
getWorkItemByProjectRef: getGitLabWorkItemByProjectRefMock,
|
||||
@@ -589,6 +592,7 @@ export {
|
||||
listGitLabWorkItemsMock,
|
||||
listGitLabIssuesMock,
|
||||
listGitLabLabelsMock,
|
||||
listGitLabAssignableUsersMock,
|
||||
listGitLabTodosMock,
|
||||
getGitLabProjectRefForRemoteMock,
|
||||
getGitLabWorkItemByProjectRefMock,
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
getGitLabWorkItemDetailsMock,
|
||||
listGitLabIssuesMock,
|
||||
listGitLabLabelsMock,
|
||||
listGitLabAssignableUsersMock,
|
||||
listGitLabMergeRequestsMock,
|
||||
listGitLabTodosMock,
|
||||
listGitLabWorkItemsMock,
|
||||
@@ -45,6 +46,7 @@ describe('OrcaRuntimeService', () => {
|
||||
})
|
||||
listGitLabTodosMock.mockResolvedValue([])
|
||||
listGitLabLabelsMock.mockResolvedValue(['bug', 'frontend'])
|
||||
listGitLabAssignableUsersMock.mockResolvedValue([])
|
||||
getGitLabWorkItemByProjectRefMock.mockResolvedValue({
|
||||
id: 'gitlab-issue-7',
|
||||
type: 'issue',
|
||||
@@ -89,6 +91,7 @@ describe('OrcaRuntimeService', () => {
|
||||
const issues = await runtime.listGitLabRepoIssues(TEST_REPO_ID, 'opened', '@me', 50, 3)
|
||||
await runtime.listGitLabRepoTodos(TEST_REPO_ID)
|
||||
await runtime.listGitLabRepoLabels(TEST_REPO_ID)
|
||||
await runtime.listGitLabRepoAssignableUsers(TEST_REPO_ID)
|
||||
await runtime.createGitLabRepoIssue(TEST_REPO_ID, 'New issue', 'Body')
|
||||
await runtime.updateGitLabRepoIssue(TEST_REPO_ID, 7, { state: 'closed' })
|
||||
await runtime.addGitLabRepoIssueComment(TEST_REPO_ID, 7, 'Looks good')
|
||||
@@ -162,6 +165,7 @@ describe('OrcaRuntimeService', () => {
|
||||
expect(issues).toMatchObject({ totalPages: 3 })
|
||||
expect(listGitLabTodosMock).toHaveBeenCalledWith('/remote/repo', 'ssh-1')
|
||||
expect(listGitLabLabelsMock).toHaveBeenCalledWith('/remote/repo', 'origin', 'ssh-1')
|
||||
expect(listGitLabAssignableUsersMock).toHaveBeenCalledWith('/remote/repo', 'origin', 'ssh-1')
|
||||
expect(createGitLabIssueMock).toHaveBeenCalledWith(
|
||||
'/remote/repo',
|
||||
'New issue',
|
||||
|
||||
@@ -10,6 +10,26 @@ function makeRequest(method: string, params?: unknown): RpcRequest {
|
||||
}
|
||||
|
||||
describe('gitlab RPC methods', () => {
|
||||
it("lists a repo's assignable users on the server that owns the repo", async () => {
|
||||
const users = [
|
||||
{ id: 7, username: 'ana', name: 'Ana', avatarUrl: 'https://gitlab.example.com/ana.png' }
|
||||
]
|
||||
const listGitLabRepoAssignableUsers = vi.fn().mockResolvedValue(users)
|
||||
const runtime = { getRuntimeId: () => 'test-runtime', listGitLabRepoAssignableUsers }
|
||||
const dispatcher = new RpcDispatcher({
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these methods dispatch only listGitLabRepoAssignableUsers.
|
||||
runtime: runtime as unknown as OrcaRuntimeService,
|
||||
methods: GITLAB_METHODS
|
||||
})
|
||||
|
||||
const response = await dispatcher.dispatch(
|
||||
makeRequest('gitlab.listAssignableUsers', { repo: 'id:repo-1' })
|
||||
)
|
||||
|
||||
expect(listGitLabRepoAssignableUsers).toHaveBeenCalledWith('id:repo-1')
|
||||
expect(response).toMatchObject({ ok: true, result: users })
|
||||
})
|
||||
|
||||
it('routes GitLab task queries and mutations to the runtime server', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
|
||||
@@ -89,6 +89,12 @@ export const GITLAB_METHODS = [
|
||||
params: RepoSelector,
|
||||
handler: async (params, { runtime }) => runtime.listGitLabRepoLabels(params.repo)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'gitlab.listAssignableUsers',
|
||||
permission: 'workspace',
|
||||
params: RepoSelector,
|
||||
handler: async (params, { runtime }) => runtime.listGitLabRepoAssignableUsers(params.repo)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'gitlab.createIssue',
|
||||
permission: 'workspace',
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
diagnoseAuth,
|
||||
getRateLimit,
|
||||
getWorkItemByProjectRef,
|
||||
listAssignableUsers,
|
||||
listIssues,
|
||||
listLabels,
|
||||
listMergeRequests,
|
||||
@@ -141,6 +142,18 @@ export class RuntimeGitLabQueryCommands {
|
||||
)
|
||||
}
|
||||
|
||||
async listGitLabRepoAssignableUsers(
|
||||
repoSelector: string
|
||||
): Promise<Awaited<ReturnType<typeof listAssignableUsers>>> {
|
||||
const repo = await this.deps.resolveRepo(repoSelector)
|
||||
return listAssignableUsers(
|
||||
repo.path,
|
||||
repo.issueSourcePreference,
|
||||
repo.connectionId ?? null,
|
||||
...this.deps.getLocalGitArgs(repo)
|
||||
)
|
||||
}
|
||||
|
||||
async getGitLabRepoWorkItemDetails(
|
||||
repoSelector: string,
|
||||
iid: number,
|
||||
|
||||
@@ -68,6 +68,7 @@ export function installRuntimeReviewCommandSurface(
|
||||
diagnoseGitLabAuth: glq.diagnoseGitLabAuth.bind(glq),
|
||||
getGitLabRateLimit: glq.getGitLabRateLimit.bind(glq),
|
||||
listGitLabRepoLabels: glq.listGitLabRepoLabels.bind(glq),
|
||||
listGitLabRepoAssignableUsers: glq.listGitLabRepoAssignableUsers.bind(glq),
|
||||
getGitLabRepoWorkItemDetails: glq.getGitLabRepoWorkItemDetails.bind(glq),
|
||||
getGitLabRepoWorkItemByPath: glq.getGitLabRepoWorkItemByPath.bind(glq),
|
||||
createGitLabRepoIssue: glm.createGitLabRepoIssue.bind(glm),
|
||||
|
||||
@@ -37,6 +37,55 @@ describe('gitLabApiFor', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it("loads a server-owned repo's reviewers from that server", async () => {
|
||||
const reviewer = {
|
||||
id: 7,
|
||||
username: 'ana',
|
||||
name: 'Ana',
|
||||
avatarUrl: 'https://gitlab.example.com/ana.png'
|
||||
}
|
||||
rpc.call.mockResolvedValue([reviewer])
|
||||
const selector = {
|
||||
repoPath: '/work/app',
|
||||
repoId: 'server-repo',
|
||||
repoOwnerExecutionHostId: 'runtime:owner-server'
|
||||
}
|
||||
|
||||
await expect(gitLabApiFor(selector).listAssignableUsers(selector)).resolves.toEqual([reviewer])
|
||||
expect(rpc.call).toHaveBeenCalledWith(
|
||||
{ kind: 'environment', environmentId: 'owner-server' },
|
||||
'gitlab.listAssignableUsers',
|
||||
expect.objectContaining({ repo: 'id:server-repo' }),
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
})
|
||||
|
||||
it('offers no reviewers from a server older than the reviewer RPC', async () => {
|
||||
rpc.call.mockRejectedValue(
|
||||
Object.assign(new Error('Unknown method'), { code: 'method_not_found' })
|
||||
)
|
||||
const selector = {
|
||||
repoPath: '/work/app',
|
||||
repoId: 'server-repo',
|
||||
repoOwnerExecutionHostId: 'runtime:owner-server'
|
||||
}
|
||||
|
||||
await expect(gitLabApiFor(selector).listAssignableUsers(selector)).resolves.toEqual([])
|
||||
})
|
||||
|
||||
it('surfaces other reviewer lookup failures', async () => {
|
||||
rpc.call.mockRejectedValue(Object.assign(new Error('glab failed'), { code: 'runtime_error' }))
|
||||
const selector = {
|
||||
repoPath: '/work/app',
|
||||
repoId: 'server-repo',
|
||||
repoOwnerExecutionHostId: 'runtime:owner-server'
|
||||
}
|
||||
|
||||
await expect(gitLabApiFor(selector).listAssignableUsers(selector)).rejects.toThrow(
|
||||
'glab failed'
|
||||
)
|
||||
})
|
||||
|
||||
it.each(['ssh:devbox', 'local'])(
|
||||
'uses this computer for a %s repo',
|
||||
async (repoOwnerExecutionHostId) => {
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
type WebGitLabRuntimeMethod
|
||||
} from '@/web/preload-api/web-gitlab-routes'
|
||||
import { mapRepoPathArg } from './runtime-repo-selector-params'
|
||||
import { hasRuntimeRpcErrorCode } from './runtime-rpc-result'
|
||||
|
||||
export type GitLabApi = NonNullable<PreloadApi['gl']>
|
||||
|
||||
@@ -46,7 +47,17 @@ export function createRuntimeGitLabApi(transport: RuntimeGitLabTransport): GitLa
|
||||
route<GitLabApiResult<'addIssueComment'>>(GITLAB_WEB_RPC_METHODS.addIssueComment, args),
|
||||
listLabels: (args) =>
|
||||
route<GitLabApiResult<'listLabels'>>(GITLAB_WEB_RPC_METHODS.listLabels, args),
|
||||
listAssignableUsers: () => Promise.resolve([]),
|
||||
// Why empty on method_not_found: a host older than this RPC simply offers no reviewers.
|
||||
listAssignableUsers: (args) =>
|
||||
route<GitLabApiResult<'listAssignableUsers'>>(
|
||||
GITLAB_WEB_RPC_METHODS.listAssignableUsers,
|
||||
args
|
||||
).catch((error: unknown) => {
|
||||
if (hasRuntimeRpcErrorCode(error, 'method_not_found')) {
|
||||
return []
|
||||
}
|
||||
throw error
|
||||
}),
|
||||
todos: (args) => route<GitLabApiResult<'todos'>>(GITLAB_WEB_RPC_METHODS.todos, args),
|
||||
// Why: the owner guard names a host as this client sees it; the server resolves the repo itself.
|
||||
workItemDetails: ({ repoOwnerExecutionHostId: _owner, ...args }) =>
|
||||
|
||||
@@ -8,6 +8,7 @@ export type WebGitLabRouteKey =
|
||||
| 'updateIssue'
|
||||
| 'addIssueComment'
|
||||
| 'listLabels'
|
||||
| 'listAssignableUsers'
|
||||
| 'todos'
|
||||
| 'workItemDetails'
|
||||
| 'closeMR'
|
||||
@@ -32,6 +33,7 @@ export type WebGitLabRuntimeMethod =
|
||||
| 'gitlab.updateIssue'
|
||||
| 'gitlab.addIssueComment'
|
||||
| 'gitlab.listLabels'
|
||||
| 'gitlab.listAssignableUsers'
|
||||
| 'gitlab.todos'
|
||||
| 'gitlab.workItemDetails'
|
||||
| 'gitlab.updateMRState'
|
||||
@@ -55,6 +57,7 @@ export const GITLAB_WEB_RPC_METHODS = {
|
||||
updateIssue: 'gitlab.updateIssue',
|
||||
addIssueComment: 'gitlab.addIssueComment',
|
||||
listLabels: 'gitlab.listLabels',
|
||||
listAssignableUsers: 'gitlab.listAssignableUsers',
|
||||
todos: 'gitlab.todos',
|
||||
workItemDetails: 'gitlab.workItemDetails',
|
||||
closeMR: 'gitlab.updateMRState',
|
||||
|
||||
@@ -122,6 +122,12 @@ describe('web GitLab preload API', () => {
|
||||
expectedMethod: 'gitlab.listLabels',
|
||||
expectedParams: { repoPath, repo: repoPath }
|
||||
},
|
||||
{
|
||||
key: 'listAssignableUsers',
|
||||
invoke: (gl) => gl.listAssignableUsers({ repoPath }),
|
||||
expectedMethod: 'gitlab.listAssignableUsers',
|
||||
expectedParams: { repoPath, repo: repoPath }
|
||||
},
|
||||
{
|
||||
key: 'todos',
|
||||
invoke: (gl) => gl.todos({ repoPath }),
|
||||
|
||||
@@ -990,6 +990,7 @@ export const RPC_PARAMS_BY_METHOD = {
|
||||
'gitlab.createIssue': CreateIssueOfGitlabParams,
|
||||
'gitlab.diagnoseAuth': EmptyParams,
|
||||
'gitlab.jobTrace': JobTrace,
|
||||
'gitlab.listAssignableUsers': RepoSelectorOfGitlabParams,
|
||||
'gitlab.listIssues': IssuesListOfGitlabParams,
|
||||
'gitlab.listLabels': RepoSelectorOfGitlabParams,
|
||||
'gitlab.listMRs': WorkItemsListOfGitlabParams,
|
||||
|
||||
Reference in New Issue
Block a user