diff --git a/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts new file mode 100644 index 000000000..0f8bdde24 --- /dev/null +++ b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts @@ -0,0 +1,34 @@ +import { MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR } from '@roomote/types'; + +import { getEnvironmentRepositoryConfigError } from '../createEnvironment'; + +describe('getEnvironmentRepositoryConfigError', () => { + it('allows GitHub and GitLab repositories in the same environment', () => { + expect( + getEnvironmentRepositoryConfigError([ + { fullName: 'acme/frontend', installationId: 'github-installation' }, + { fullName: 'acme/backend', installationId: null }, + ]), + ).toBeNull(); + }); + + it('rejects repositories from two GitHub App installations', () => { + expect( + getEnvironmentRepositoryConfigError([ + { fullName: 'acme/frontend', installationId: 'github-installation-1' }, + { fullName: 'other/backend', installationId: 'github-installation-2' }, + ]), + ).toBe(MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('rejects ambiguous repository names across source-control connections', () => { + expect( + getEnvironmentRepositoryConfigError([ + { fullName: 'acme/app', installationId: 'github-installation' }, + { fullName: 'acme/app', installationId: null }, + ]), + ).toBe( + 'Multiple repositories are named "acme/app". Environment repository names must be unique across source-control connections.', + ); + }); +}); diff --git a/apps/api/src/handlers/environments/__tests__/environment-write-auth.test.ts b/apps/api/src/handlers/environments/__tests__/environment-write-auth.test.ts index aa58cd14e..3fe966c99 100644 --- a/apps/api/src/handlers/environments/__tests__/environment-write-auth.test.ts +++ b/apps/api/src/handlers/environments/__tests__/environment-write-auth.test.ts @@ -162,6 +162,45 @@ describe.each([ }); }); +describe.each([ + ['createEnvironment', 'POST', '/environments'], + ['updateEnvironment', 'PATCH', '/environments/env-1'], +] as const)('%s config validation', (_name, method, path) => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('rejects duplicate repositories before lookup or persistence', async () => { + const app = createApp({ + userId: 'user-1', + tokenType: 'auth', + version: 1, + }); + const response = await app.request(path, { + method, + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + config: { + name: 'Duplicate repository', + repositories: [ + { repository: 'acme/api' }, + { repository: 'acme/api' }, + ], + }, + }), + }); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: + 'Invalid environment configuration: Duplicate repository: acme/api', + }); + expect(mockEnvironmentsFindFirst).not.toHaveBeenCalled(); + expect(mockRepositoriesFindMany).not.toHaveBeenCalled(); + expect(mockEnvironmentInsertValues).not.toHaveBeenCalled(); + }); +}); + describe('createEnvironment attribution', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/apps/api/src/handlers/environments/createEnvironment.ts b/apps/api/src/handlers/environments/createEnvironment.ts index 4cb06fa45..3b9e4b8c5 100644 --- a/apps/api/src/handlers/environments/createEnvironment.ts +++ b/apps/api/src/handlers/environments/createEnvironment.ts @@ -14,6 +14,8 @@ import { import { type TaskPayload, environmentConfigSchema, + getAmbiguousEnvironmentRepositoryError, + getDuplicateEnvironmentRepositoryConfigError, getMissingEnvironmentRepositoryError, getEnvironmentRepositoryInstallationError, } from '@roomote/types'; @@ -69,7 +71,10 @@ export function getEnvironmentRepositoryConfigError( installationId: string | number | null | undefined; }>, ): string | null { - return getEnvironmentRepositoryInstallationError(repositoryRows); + return ( + getAmbiguousEnvironmentRepositoryError(repositoryRows) ?? + getEnvironmentRepositoryInstallationError(repositoryRows) + ); } function extractRunId(auth: McpAuth): number | null { @@ -252,6 +257,19 @@ export async function createEnvironment( } const config = parsedConfig.data; + const duplicateRepositoryError = getDuplicateEnvironmentRepositoryConfigError( + config.repositories, + ); + + if (duplicateRepositoryError) { + return c.json( + { + error: `Invalid environment configuration: ${duplicateRepositoryError}`, + }, + 400, + ); + } + try { const existing = await db.query.environments.findFirst({ where: eq(environments.name, config.name), diff --git a/apps/api/src/handlers/environments/updateEnvironment.ts b/apps/api/src/handlers/environments/updateEnvironment.ts index 1b6e5a4e2..a3204cc82 100644 --- a/apps/api/src/handlers/environments/updateEnvironment.ts +++ b/apps/api/src/handlers/environments/updateEnvironment.ts @@ -11,6 +11,7 @@ import { } from '@roomote/db/server'; import { environmentConfigSchema, + getDuplicateEnvironmentRepositoryConfigError, getMissingEnvironmentRepositoryError, } from '@roomote/types'; @@ -90,6 +91,17 @@ export async function updateEnvironment( } const config = parsedConfig.data; + const duplicateRepositoryError = + getDuplicateEnvironmentRepositoryConfigError(config.repositories); + + if (duplicateRepositoryError) { + return c.json( + { + error: `Invalid environment configuration: ${duplicateRepositoryError}`, + }, + 400, + ); + } const environment = await db.query.environments.findFirst({ where: eq(environments.id, id), diff --git a/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts b/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts index 543d108fb..ba8dd7cde 100644 --- a/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts +++ b/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts @@ -11,12 +11,14 @@ const { mockEnvironmentsFindFirst, mockRepositoriesFindMany, mockSelectRows, + mockResolveWorkspaceRepositoryProviders, mockGetMembershipRole, } = vi.hoisted(() => ({ mockEnqueueTask: vi.fn(), mockEnvironmentsFindFirst: vi.fn(), mockRepositoriesFindMany: vi.fn(), mockSelectRows: vi.fn(), + mockResolveWorkspaceRepositoryProviders: vi.fn(), mockGetMembershipRole: vi.fn(), })); @@ -35,6 +37,8 @@ vi.mock('@roomote/db/server', () => ({ environments: {}, environmentRepositoryMappings: {}, repositories: {}, + resolveWorkspaceRepositoryProviders: (...args: unknown[]) => + mockResolveWorkspaceRepositoryProviders(...args), db: { query: { environments: { @@ -88,6 +92,8 @@ describe('launchTask', () => { mockRepositoriesFindMany.mockReset(); mockSelectRows.mockReset(); mockSelectRows.mockReturnValue([]); + mockResolveWorkspaceRepositoryProviders.mockReset(); + mockResolveWorkspaceRepositoryProviders.mockResolvedValue({}); mockGetMembershipRole.mockReset(); mockGetMembershipRole.mockResolvedValue('org:admin'); }); @@ -137,7 +143,9 @@ describe('launchTask', () => { it('stamps the source-control provider resolved from environment repositories into the payload', async () => { mockEnqueueTask.mockResolvedValue({ id: 100, taskId: 'task-gl' }); - mockSelectRows.mockReturnValue([{ sourceControlProvider: 'gitlab' }]); + mockResolveWorkspaceRepositoryProviders.mockResolvedValue({ + 'group/project': 'gitlab', + }); const app = createApp(authContext); const response = await app.request( @@ -158,6 +166,32 @@ describe('launchTask', () => { expect(enqueuedTask.task.payload.sourceControlProvider).toBe('gitlab'); }); + it('uses the first environment repository provider for mixed environments', async () => { + mockEnqueueTask.mockResolvedValue({ id: 100, taskId: 'task-mixed' }); + mockResolveWorkspaceRepositoryProviders.mockResolvedValue({ + 'octo/api': 'github', + 'group/web': 'gitlab', + }); + + const app = createApp(authContext); + const response = await app.request( + new Request('http://localhost/tasks', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + prompt: 'Ship a mixed-provider change', + environmentId: '6f1f3f0a-9f5e-4d2a-8f4e-1a2b3c4d5e6f', + }), + }), + ); + + expect(response.status).toBe(200); + const enqueuedTask = mockEnqueueTask.mock.calls[0]?.[0] as { + task: { payload: { sourceControlProvider?: string } }; + }; + expect(enqueuedTask.task.payload.sourceControlProvider).toBe('github'); + }); + it('leaves the provider unset for prompt-only launches with no repository context', async () => { mockEnqueueTask.mockResolvedValue({ id: 101, taskId: 'task-plain' }); @@ -322,15 +356,16 @@ describe('launchTask', () => { expect(enqueuedTask.task.payload.notifySourceRunOnSettle).toBeUndefined(); }); - it('rejects launches whose selected repositories span multiple providers', async () => { + it('allows selected repositories that span multiple providers', async () => { + mockEnqueueTask.mockResolvedValue({ id: 105, taskId: 'task-mixed-set' }); mockRepositoriesFindMany.mockResolvedValue([ { fullName: 'octo/github-repo', installationId: 1 }, { fullName: 'group/gitlab-repo', installationId: null }, ]); - mockSelectRows.mockReturnValue([ - { sourceControlProvider: 'github' }, - { sourceControlProvider: 'gitlab' }, - ]); + mockResolveWorkspaceRepositoryProviders.mockResolvedValue({ + 'octo/github-repo': 'github', + 'group/gitlab-repo': 'gitlab', + }); const app = createApp(authContext); const response = await app.request( @@ -344,11 +379,43 @@ describe('launchTask', () => { }), ); - expect(response.status).toBe(400); - const json = (await response.json()) as { error: string }; - expect(json.error).toBe( - 'Selected repositories must belong to a single source control provider.', + expect(response.status).toBe(200); + expect(mockEnqueueTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.objectContaining({ + payload: expect.objectContaining({ + selectedRepositories: ['octo/github-repo', 'group/gitlab-repo'], + sourceControlProvider: 'github', + }), + }), + }), + expect.anything(), ); + }); + + it('rejects selected repositories whose source control is ambiguous', async () => { + mockRepositoriesFindMany.mockResolvedValue([ + { fullName: 'group/project', installationId: null }, + ]); + mockResolveWorkspaceRepositoryProviders.mockResolvedValue({}); + + const app = createApp(authContext); + const response = await app.request( + new Request('http://localhost/tasks', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + prompt: 'Ship a change', + selectedRepositories: ['group/project'], + }), + }), + ); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: + 'Could not unambiguously resolve source control for: group/project', + }); expect(mockEnqueueTask).not.toHaveBeenCalled(); }); diff --git a/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts b/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts new file mode 100644 index 000000000..7d0a10296 --- /dev/null +++ b/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts @@ -0,0 +1,131 @@ +import { Hono } from 'hono'; + +import type { Variables } from '../../../types'; +import type { McpAuth } from '../../mcp/middleware'; +import { manageSourceControl } from '../manageSourceControl'; + +const { + mockAssertTaskRunTokenTargetExists, + mockClaimLatestUserMessageForReplyQuote, + mockFindTaskRunForSourceControlMutation, + mockManageSourceControlIssueForTaskRun, +} = vi.hoisted(() => ({ + mockAssertTaskRunTokenTargetExists: vi.fn(), + mockClaimLatestUserMessageForReplyQuote: vi.fn(), + mockFindTaskRunForSourceControlMutation: vi.fn(), + mockManageSourceControlIssueForTaskRun: vi.fn(), +})); + +vi.mock('@roomote/communication/messages', () => ({ + claimLatestUserMessageForReplyQuote: mockClaimLatestUserMessageForReplyQuote, + completeClaimedLatestUserMessageForReplyQuote: vi.fn(), + restoreClaimedLatestUserMessageForReplyQuote: vi.fn(), +})); + +vi.mock('@roomote/sdk/server', async (importOriginal) => ({ + ...(await importOriginal()), + findTaskRunForSourceControlMutation: mockFindTaskRunForSourceControlMutation, + manageSourceControlIssueForTaskRun: mockManageSourceControlIssueForTaskRun, +})); + +vi.mock('../../mcp/proxy-utils', async (importOriginal) => ({ + ...(await importOriginal()), + assertTaskRunTokenTargetExists: mockAssertTaskRunTokenTargetExists, +})); + +function createApp() { + const app = new Hono<{ + Variables: Variables & { mcpAuth: McpAuth }; + }>(); + const mcpAuth = { + authContext: { + runId: 123, + userId: 'user-1', + principal: 'user', + tokenType: 'run', + version: 1, + }, + userId: 'user-1', + } as McpAuth; + + app.use('*', async (c, next) => { + c.set('mcpAuth', mcpAuth); + await next(); + }); + app.post('/:taskId/source_control', manageSourceControl); + + return app; +} + +describe('manageSourceControl', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAssertTaskRunTokenTargetExists.mockResolvedValue(undefined); + mockFindTaskRunForSourceControlMutation.mockResolvedValue({ + id: 123, + payload: { + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + }); + mockManageSourceControlIssueForTaskRun.mockResolvedValue({ + success: true, + action: 'create_issue_comment', + provider: 'gitlab', + repositoryFullName: 'acme/backend', + number: 1082, + commentId: '9', + warnings: [], + }); + }); + + it('does not apply GitHub reply quoting to a GitLab target in a GitHub-primary task', async () => { + const response = await createApp().request('/task-1/source_control', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + action: 'create_issue_comment', + repositoryFullName: 'acme/backend', + issueNumber: 1082, + body: 'Fixed in the latest branch.', + }), + }); + + expect(response.status).toBe(200); + expect(mockClaimLatestUserMessageForReplyQuote).not.toHaveBeenCalled(); + expect(mockManageSourceControlIssueForTaskRun).toHaveBeenCalledWith({ + taskRun: expect.objectContaining({ id: 123 }), + input: expect.objectContaining({ + repositoryFullName: 'acme/backend', + body: 'Fixed in the latest branch.', + }), + }); + }); + + it('defers unmapped provider errors until after repository scope validation', async () => { + mockManageSourceControlIssueForTaskRun.mockRejectedValueOnce( + new Error( + "Repository other/repo is outside this task's source-control scope.", + ), + ); + + const response = await createApp().request('/task-1/source_control', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + action: 'get_issue', + repositoryFullName: 'other/repo', + issueNumber: 42, + }), + }); + + expect(response.status).toBe(500); + await expect(response.json()).resolves.toEqual({ + error: + "Repository other/repo is outside this task's source-control scope.", + }); + expect(mockManageSourceControlIssueForTaskRun).toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/handlers/tasks/automation-work-items/repositories.ts b/apps/api/src/handlers/tasks/automation-work-items/repositories.ts index 6ef695a0e..2d75e082a 100644 --- a/apps/api/src/handlers/tasks/automation-work-items/repositories.ts +++ b/apps/api/src/handlers/tasks/automation-work-items/repositories.ts @@ -7,9 +7,7 @@ function getSuggestedTaskRepositoryFullNames( payload: SuggestedTasksPayload, ): string[] { if (payload.repo === ALL_REPOSITORIES) { - return [...new Set(payload.selectedRepositories ?? [])].sort( - (left, right) => left.localeCompare(right), - ); + return [...new Set(payload.selectedRepositories ?? [])]; } if (payload.repo?.trim()) { diff --git a/apps/api/src/handlers/tasks/launchTask.ts b/apps/api/src/handlers/tasks/launchTask.ts index 034a26edb..caaa32abb 100644 --- a/apps/api/src/handlers/tasks/launchTask.ts +++ b/apps/api/src/handlers/tasks/launchTask.ts @@ -8,11 +8,11 @@ import { import { and, db, - environmentRepositoryMappings, environments, eq, inArray, repositories, + resolveWorkspaceRepositoryProviders, } from '@roomote/db/server'; import { ADMIN_REQUIRED_LAUNCH_TYPES, @@ -43,7 +43,7 @@ function normalizeRepositoryFullNames(body: TaskLaunchRequest): string[] { body.repo && body.repo !== ALL_REPOSITORIES ? body.repo : null, ].filter((value): value is string => Boolean(value)), ), - ].sort((left, right) => left.localeCompare(right)); + ]; } async function validateSelectedRepositories( @@ -88,20 +88,6 @@ async function validateSelectedRepositories( return null; } -function resolveSingleSourceControlProvider( - providers: SourceControlProvider[], -): SourceControlProvider | undefined { - const uniqueProviders = [...new Set(providers)]; - - if (uniqueProviders.length > 1) { - throw new Error( - 'Selected repositories must belong to a single source control provider.', - ); - } - - return uniqueProviders[0]; -} - async function resolveLaunchSourceControlProvider({ repositoryFullNames, environmentId, @@ -110,42 +96,30 @@ async function resolveLaunchSourceControlProvider({ environmentId: string | undefined; }): Promise { if (repositoryFullNames.length > 0) { - const rows = await db - .select({ sourceControlProvider: repositories.sourceControlProvider }) - .from(repositories) - .where( - and( - eq(repositories.isActive, true), - inArray(repositories.fullName, repositoryFullNames), - ), - ); - const provider = resolveSingleSourceControlProvider( - rows.map((row) => row.sourceControlProvider), + const repositoryProviders = await resolveWorkspaceRepositoryProviders(db, { + type: 'repository_set', + repositories: repositoryFullNames, + }); + const unresolvedRepositories = repositoryFullNames.filter( + (repositoryFullName) => + repositoryProviders[repositoryFullName] === undefined, ); - if (provider) { - return provider; + if (unresolvedRepositories.length > 0) { + throw new Error( + `Could not unambiguously resolve source control for: ${unresolvedRepositories.join(', ')}`, + ); } + + return Object.values(repositoryProviders)[0]; } if (environmentId) { - const rows = await db - .select({ sourceControlProvider: repositories.sourceControlProvider }) - .from(environmentRepositoryMappings) - .innerJoin( - repositories, - eq(environmentRepositoryMappings.repositoryId, repositories.id), - ) - .where( - and( - eq(environmentRepositoryMappings.environmentId, environmentId), - eq(repositories.isActive, true), - ), - ); - - return resolveSingleSourceControlProvider( - rows.map((row) => row.sourceControlProvider), - ); + const repositoryProviders = await resolveWorkspaceRepositoryProviders(db, { + type: 'environment', + environmentId, + }); + return Object.values(repositoryProviders)[0]; } return undefined; diff --git a/apps/api/src/handlers/tasks/manageSourceControl.ts b/apps/api/src/handlers/tasks/manageSourceControl.ts index 1ee30b6ab..8393277ad 100644 --- a/apps/api/src/handlers/tasks/manageSourceControl.ts +++ b/apps/api/src/handlers/tasks/manageSourceControl.ts @@ -6,13 +6,14 @@ import { completeClaimedLatestUserMessageForReplyQuote, restoreClaimedLatestUserMessageForReplyQuote, } from '@roomote/communication/messages'; -import { resolveSourceControlProviderFromPayload } from '@roomote/types'; - +import { resolveRepositoryProvidersFromPayload } from '@roomote/types'; import { createOrUpdateSourceControlPullRequestForTaskRun, findTaskRunForSourceControlMutation, + getPayloadRecord, manageSourceControlIssueForTaskRun, readSourceControlPullRequestForTaskRun, + resolveSourceControlProviderForRepositoryFromPayload, sourceControlIssueInputSchema, sourceControlPullRequestMutationInputSchema, sourceControlPullRequestReadInputSchema, @@ -93,11 +94,22 @@ export async function manageSourceControl( runId: auth.authContext.runId, taskId, }); - const isGitHubTask = - resolveSourceControlProviderFromPayload(taskRun.payload) === 'github'; + const payload = getPayloadRecord(taskRun.payload); + let targetProvider = input.sourceControlProvider; + const repositoryProviders = resolveRepositoryProvidersFromPayload(payload); + + if ( + !targetProvider && + (!repositoryProviders || input.repositoryFullName in repositoryProviders) + ) { + targetProvider = resolveSourceControlProviderForRepositoryFromPayload( + payload, + input.repositoryFullName, + ); + } const bodyInput = 'body' in input ? input : null; const shouldQuote = - isGitHubTask && + targetProvider === 'github' && (input.action === 'reply_to_pull_request_comment' || input.action === 'create_pull_request_comment' || input.action === 'create_issue_comment') && diff --git a/apps/api/src/handlers/tasks/submitTaskSuggestions.ts b/apps/api/src/handlers/tasks/submitTaskSuggestions.ts index 86db79712..e433348d3 100644 --- a/apps/api/src/handlers/tasks/submitTaskSuggestions.ts +++ b/apps/api/src/handlers/tasks/submitTaskSuggestions.ts @@ -217,9 +217,7 @@ function getSuggestedTaskRepositoryFullNames( payload: SuggestedTasksPayload, ): string[] { if (payload.repo === ALL_REPOSITORIES) { - return [...new Set(payload.selectedRepositories ?? [])].sort( - (left, right) => left.localeCompare(right), - ); + return [...new Set(payload.selectedRepositories ?? [])]; } if (payload.repo?.trim()) { diff --git a/apps/docs/environments.mdx b/apps/docs/environments.mdx index eead94d4b..184c5de22 100644 --- a/apps/docs/environments.mdx +++ b/apps/docs/environments.mdx @@ -37,6 +37,12 @@ environment and add any setup guidance you already know. Roomote then starts an environment setup task that figures out how to run the app locally and prepare the workspace for future tasks. +An environment can combine repositories from different source-control +providers, such as GitHub and GitLab. Each provider must have working +credentials in the deployment or the task cannot prepare the complete +workspace. Repositories connected through GitHub must also belong to the same +GitHub App installation. + The setup task is meant to produce a working environment Roomote can reuse. If it cannot finish, adjust the input and try again from **Settings > Environments**. diff --git a/apps/web/src/lib/environment-definition.test.ts b/apps/web/src/lib/environment-definition.test.ts index 80ec0c59b..c2021c943 100644 --- a/apps/web/src/lib/environment-definition.test.ts +++ b/apps/web/src/lib/environment-definition.test.ts @@ -4,6 +4,7 @@ import { buildCreateEnvironmentDefinitionPrompt, RunStatus, getEnvironmentDefinitionIdFromPayload, + normalizeRepositorySelection, type EnvironmentConfig, } from '@roomote/types'; @@ -36,14 +37,14 @@ describe('environment definition helpers', () => { ); }); - it('builds the create prompt with the environment-setup skill and sorted repositories', () => { + it('builds the create prompt with repositories in selection order', () => { const prompt = buildCreateEnvironmentDefinitionPrompt([ 'acme/web', 'acme/api', ]); expect(prompt).toContain('$environment-setup'); - expect(prompt).toContain('- acme/api\n- acme/web'); + expect(prompt).toContain('- acme/web\n- acme/api'); expect(prompt).toContain( 'Do not mock or stub required services just to make the environment appear to work.', ); @@ -107,10 +108,31 @@ describe('environment definition helpers', () => { buildEnvironmentDefinitionWorkspacePayload(['acme/web', 'acme/api']), ).toEqual({ repo: '__all_repositories__', - selectedRepositories: ['acme/api', 'acme/web'], + selectedRepositories: ['acme/web', 'acme/api'], }); }); + it('rejects duplicate repository names before building a workspace', () => { + expect(() => + buildEnvironmentDefinitionWorkspacePayload([ + 'group/project', + 'group/project', + ]), + ).toThrow( + 'The selected repositories include multiple entries named "group/project".', + ); + }); + + it('deduplicates repository selections without changing their order', () => { + expect( + normalizeRepositorySelection([ + { id: 'repo-web', fullName: 'acme/web' }, + { id: 'repo-api', fullName: 'acme/api' }, + { id: 'repo-web', fullName: 'acme/web' }, + ]), + ).toEqual(['repo-web', 'repo-api']); + }); + it('builds the update prompt with the existing environment context', () => { const prompt = buildUpdateEnvironmentDefinitionPrompt({ environmentId: 'env-123', @@ -123,6 +145,9 @@ describe('environment definition helpers', () => { 'Update the existing Roomote environment definition instead of creating a new one.', ); expect(prompt).toContain('- ID: env-123'); + expect(prompt).toContain( + 'Repositories to inspect:\n- acme/web\n- acme/api', + ); expect(prompt).toContain( 'Keep the existing environment name unless the user explicitly asked to rename it.', ); diff --git a/apps/web/src/lib/environment-definition.ts b/apps/web/src/lib/environment-definition.ts index d444b00b9..45a0bf1a7 100644 --- a/apps/web/src/lib/environment-definition.ts +++ b/apps/web/src/lib/environment-definition.ts @@ -97,11 +97,9 @@ export function buildUpdateEnvironmentDefinitionPrompt(input: { repositoryFullNames: string[]; config: EnvironmentConfig; }): string { - const sortedRepositories = [...input.repositoryFullNames].sort( - (left, right) => left.localeCompare(right), - ); + const orderedRepositories = [...new Set(input.repositoryFullNames)]; - const repositoryLines = sortedRepositories + const repositoryLines = orderedRepositories .map((repositoryFullName) => `- ${repositoryFullName}`) .join('\n'); diff --git a/apps/web/src/lib/server/source-control-provider.ts b/apps/web/src/lib/server/source-control-provider.ts index 1ef23c52c..37b57343e 100644 --- a/apps/web/src/lib/server/source-control-provider.ts +++ b/apps/web/src/lib/server/source-control-provider.ts @@ -1,40 +1,43 @@ import type { SourceControlProvider } from '@roomote/types'; -import { db, resolveWorkspaceSourceControlProvider } from '@roomote/db/server'; +import { db, resolveWorkspaceRepositoryProviders } from '@roomote/db/server'; /** - * Resolve the single provider a launch's explicitly selected repositories - * belong to, so the task payload can carry an explicit `sourceControlProvider`. - * Without it, dequeue falls back to the GitHub default and non-GitHub - * deployments fail source control token creation. - * - * This is the web launch-validation path: it THROWS when the selection spans - * multiple providers, surfacing a clear error before enqueue. This differs from - * the shared {@link resolveWorkspaceSourceControlProvider}, which returns - * `undefined` on ambiguity and defers to the downstream GitHub fallback. + * Resolve the primary provider for an explicitly selected repository set. + * Mixed-provider sets are supported; the queue stamps the complete provider + * map before persistence. */ -export function resolveSingleSourceControlProvider( - providers: SourceControlProvider[], +export function resolveSelectedRepositorySourceControlProvider( + repositories: Array<{ + fullName: string; + sourceControlProvider: SourceControlProvider; + }>, + repositoryOrder: string[], ): SourceControlProvider | undefined { - const uniqueProviders = [...new Set(providers)]; - - if (uniqueProviders.length > 1) { - throw new Error( - 'Selected repositories must belong to a single source control provider.', + for (const repositoryFullName of repositoryOrder) { + const matches = repositories.filter( + (repository) => repository.fullName === repositoryFullName, ); + + if (matches.length > 1) { + throw new Error( + `Could not unambiguously resolve source control for: ${repositoryFullName}`, + ); + } + + if (matches[0]) { + return matches[0].sourceControlProvider; + } } - return uniqueProviders[0]; + return undefined; } /** * Resolve the provider for an environment-backed launch by delegating to the * shared resolver (single source of truth for the environment-repository join). * - * Unlike {@link resolveSingleSourceControlProvider}, this returns `undefined` - * (rather than throwing) when the environment's repositories span multiple - * providers, deferring to the dequeue-time GitHub fallback. Environment - * launches are a secondary fallback behind the explicit repository selection, - * so an ambiguous environment should not hard-fail the launch. + * Mixed environments use their first repository's provider as the scalar + * compatibility value. Queue stamping adds the complete repository map. */ export async function resolveEnvironmentSourceControlProvider( environmentId: string | undefined, @@ -43,8 +46,9 @@ export async function resolveEnvironmentSourceControlProvider( return undefined; } - return resolveWorkspaceSourceControlProvider(db, { + const repositoryProviders = await resolveWorkspaceRepositoryProviders(db, { type: 'environment', environmentId, }); + return Object.values(repositoryProviders)[0]; } diff --git a/apps/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index e10413712..f2297c035 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -5,6 +5,7 @@ const { mockGetBranches, mockGetRepositoryEmptyStates, mockGetRepositories, + mockUpdateEnvironmentDefinition, mockBeginEnvironmentVerification, mockActiveVerificationRuns, } = vi.hoisted(() => ({ @@ -28,6 +29,7 @@ const { installationId: 'installation-1', }, ]), + mockUpdateEnvironmentDefinition: vi.fn(), mockBeginEnvironmentVerification: vi.fn(), // Active verification run seen inside the retry critical section. Each entry // is returned by the locked transaction's active-run lookup. @@ -51,6 +53,8 @@ vi.mock('@roomote/db/server', () => ({ createEnvironmentConfigVersionSnapshot: vi.fn(), db: { select: mockDbSelect, + transaction: async (callback: (tx: unknown) => Promise) => + callback({}), }, desc: vi.fn(), environmentConfigVersions: {}, @@ -66,7 +70,7 @@ vi.mock('@roomote/db/server', () => ({ sql: vi.fn(), taskRuns: {}, tasks: {}, - updateEnvironmentDefinition: vi.fn(), + updateEnvironmentDefinition: mockUpdateEnvironmentDefinition, users: {}, withEnvironmentVerificationRetryLock: async ( _environmentId: string, @@ -100,12 +104,24 @@ import { TaskPayloadKind } from '@roomote/types'; import type { UserAuthSuccess } from '@/types'; import { createEnvironmentCommand, + getEnvironmentRepositoryConfigError, retryEnvironmentVerificationCommand, startEnvironmentDefinitionTaskCommand, updateEnvironmentCommand, validateConfigCommand, } from './index'; +describe('getEnvironmentRepositoryConfigError', () => { + it('rejects ambiguous active repository names', () => { + expect( + getEnvironmentRepositoryConfigError([ + { id: 'repo-github', fullName: 'acme/app', installationId: '1' }, + { id: 'repo-gitlab', fullName: 'acme/app', installationId: null }, + ]), + ).toContain('Multiple repositories are named "acme/app"'); + }); +}); + function buildMockAuth(): UserAuthSuccess { return { success: true, @@ -155,11 +171,11 @@ describe('startEnvironmentDefinitionTaskCommand', () => { expect(result.taskId).toBe('task-env-definition-1'); expect(mockEnqueueTask).toHaveBeenCalledWith( expect.objectContaining({ - title: 'Set up the api + web environment', + title: 'Set up the web + api environment', task: expect.objectContaining({ type: TaskPayloadKind.StandardTask, payload: expect.objectContaining({ - selectedRepositories: ['acme/api', 'acme/web'], + selectedRepositories: ['acme/web', 'acme/api'], }), }), initiator: { kind: 'user', userId: 'user-1' }, @@ -170,6 +186,24 @@ describe('startEnvironmentDefinitionTaskCommand', () => { ); }); + it('surfaces duplicate repository names as a validation error', async () => { + mockGetRepositories.mockResolvedValueOnce([ + { id: 'repo-github', fullName: 'acme/app', installationId: '1' }, + { id: 'repo-gitlab', fullName: 'acme/app', installationId: null }, + ]); + + await expect( + startEnvironmentDefinitionTaskCommand(buildMockAuth(), { + repositoryIds: ['repo-github', 'repo-gitlab'], + }), + ).rejects.toMatchObject({ + code: 'BAD_REQUEST', + message: + 'Multiple repositories are named "acme/app". Environment repository names must be unique across source-control connections.', + }); + expect(mockEnqueueTask).not.toHaveBeenCalled(); + }); + it('applies the selected model to settings-created setup tasks', async () => { await startEnvironmentDefinitionTaskCommand(buildMockAuth(), { repositoryIds: ['repo-1'], @@ -203,7 +237,7 @@ describe('startEnvironmentDefinitionTaskCommand', () => { }); expect(mockGetRepositoryEmptyStates).toHaveBeenCalledWith({ - repositoryIds: ['repo-1', 'repo-2'], + repositoryIds: ['repo-2', 'repo-1'], }); const enqueueInput = mockEnqueueTask.mock.calls[0]?.[0] as { @@ -398,6 +432,102 @@ describe('environment repository validation', () => { }); }); + it('rejects duplicate configured repositories before creating mappings', async () => { + const result = await createEnvironmentCommand(buildMockAuth(), { + name: 'Duplicate repository', + config: { + name: 'Duplicate repository', + repositories: [{ repository: 'acme/api' }, { repository: 'acme/api' }], + }, + }); + + expect(result).toEqual({ + success: false, + error: 'Invalid configuration: Duplicate repository: acme/api', + }); + expect(mockDbSelect).not.toHaveBeenCalled(); + }); + + it('rejects duplicate configured repositories before updating mappings', async () => { + mockDbSelect.mockReturnValueOnce({ + from: () => ({ + where: () => ({ + limit: async () => [ + { + id: 'env-1', + name: 'Existing environment', + description: null, + config: { + name: 'Existing environment', + repositories: [{ repository: 'acme/api' }], + }, + }, + ], + }), + }), + }); + + const result = await updateEnvironmentCommand(buildMockAuth(), { + id: 'env-1', + config: { + name: 'Duplicate repository', + repositories: [{ repository: 'acme/api' }, { repository: 'acme/api' }], + }, + }); + + expect(result).toEqual({ + success: false, + error: 'Invalid configuration: Duplicate repository: acme/api', + }); + expect(mockDbSelect).toHaveBeenCalledTimes(1); + }); + + it('keeps legacy duplicate repositories editable for metadata-only updates', async () => { + mockDbSelect.mockReturnValueOnce({ + from: () => ({ + where: () => ({ + limit: async () => [ + { + id: 'env-1', + name: 'Legacy environment', + description: null, + config: { + name: 'Legacy environment', + repositories: [ + { repository: 'acme/api' }, + { repository: 'acme/api' }, + ], + }, + }, + ], + }), + }), + }); + + const result = await updateEnvironmentCommand(buildMockAuth(), { + id: 'env-1', + description: 'Updated description', + }); + + expect(result).toEqual({ success: true, data: undefined }); + expect(mockUpdateEnvironmentDefinition).toHaveBeenCalledWith( + {}, + expect.objectContaining({ + environmentId: 'env-1', + fields: expect.objectContaining({ + description: 'Updated description', + config: expect.objectContaining({ + repositories: [ + { repository: 'acme/api' }, + { repository: 'acme/api' }, + ], + }), + }), + repositoryIds: undefined, + }), + ); + }); + it('rejects update when a configured repository is not linked', async () => { mockDbSelect .mockReturnValueOnce({ @@ -466,6 +596,24 @@ describe('environment repository validation', () => { expect(mockGetBranches).not.toHaveBeenCalled(); }); + it('uses provider-neutral guidance when repository access fails', async () => { + mockDbSelect.mockReturnValueOnce({ + from: () => ({ where: async () => [] }), + }); + mockCheckRepoAccess.mockResolvedValue(false); + + const result = await validateConfigCommand(buildMockAuth(), { + config: { + name: 'GitLab Test', + repositories: [{ repository: 'acme/backend' }], + }, + }); + + expect(result.errors).toEqual([ + "Repository 'acme/backend' is not accessible. Ensure it is connected through its source-control provider.", + ]); + }); + it('continues warning when a GitHub branch is missing', async () => { mockDbSelect.mockReturnValueOnce({ from: () => ({ diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index 4802f39ac..204ed725a 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -1,4 +1,5 @@ import { enqueueTask } from '@roomote/cloud-agents/server'; +import { TRPCError } from '@trpc/server'; import { createEnvironmentConfigVersionSnapshot, db, @@ -37,6 +38,8 @@ import { type ComputeProvider, type EnvironmentConfig, environmentConfigSchema, + getAmbiguousEnvironmentRepositoryError, + getDuplicateEnvironmentRepositoryConfigError, getEnvironmentRepositoryInstallationError, getMissingEnvironmentRepositoryError, isExitedRunStatus, @@ -140,14 +143,17 @@ type EnvironmentRepositoryRow = { installationId: string | null; }; -function getEnvironmentRepositoryConfigError( +export function getEnvironmentRepositoryConfigError( repositoriesToValidate: EnvironmentRepositoryRow[], ): string | null { - return getEnvironmentRepositoryInstallationError( - repositoriesToValidate.map((repository) => ({ - fullName: repository.fullName, - installationId: repository.installationId, - })), + return ( + getAmbiguousEnvironmentRepositoryError(repositoriesToValidate) ?? + getEnvironmentRepositoryInstallationError( + repositoriesToValidate.map((repository) => ({ + fullName: repository.fullName, + installationId: repository.installationId, + })), + ) ); } @@ -197,7 +203,10 @@ async function resolveSelectedRepositories( const repository = repositoriesById.get(repositoryId); if (!repository) { - throw new Error('Selected repositories are no longer available.'); + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Selected repositories are no longer available.', + }); } return { @@ -211,14 +220,15 @@ async function resolveSelectedRepositories( getEnvironmentRepositoryConfigError(selectedRepositories); if (repositoryConfigError) { - throw new Error(repositoryConfigError); + throw new TRPCError({ + code: 'BAD_REQUEST', + message: repositoryConfigError, + }); } return { normalizedRepositoryIds: normalizeRepositorySelection(selectedRepositories), - selectedRepositories: selectedRepositories.sort((left, right) => - left.fullName.localeCompare(right.fullName), - ), + selectedRepositories, }; } @@ -454,6 +464,17 @@ export async function createEnvironmentCommand( }; } + const duplicateRepositoryError = getDuplicateEnvironmentRepositoryConfigError( + parseResult.data.repositories ?? [], + ); + + if (duplicateRepositoryError) { + return { + success: false, + error: `Invalid configuration: ${duplicateRepositoryError}`, + }; + } + const [existing] = await db .select() .from(environments) @@ -576,6 +597,18 @@ export async function updateEnvironmentCommand( }; } + const duplicateRepositoryError = + getDuplicateEnvironmentRepositoryConfigError( + parseResult.data.repositories ?? [], + ); + + if (duplicateRepositoryError) { + return { + success: false, + error: `Invalid configuration: ${duplicateRepositoryError}`, + }; + } + nextConfig = parseResult.data; } else if ( input.name !== undefined || @@ -817,9 +850,23 @@ export async function startEnvironmentDefinitionTaskCommand( (repository) => repository.fullName, ); const title = buildSetupEnvironmentTaskTitle(selectedRepositoryFullNames); - const workspacePayload = buildEnvironmentDefinitionWorkspacePayload( - selectedRepositoryFullNames, - ); + let workspacePayload: ReturnType< + typeof buildEnvironmentDefinitionWorkspacePayload + >; + try { + workspacePayload = buildEnvironmentDefinitionWorkspacePayload( + selectedRepositoryFullNames, + ); + } catch (error) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: + error instanceof Error + ? error.message + : 'The selected repositories are invalid.', + cause: error, + }); + } const modelSelection = resolveEvalHarnessSelection({ model: input.selectedModelId, }); @@ -1249,7 +1296,7 @@ export async function validateConfigCommand( if (!hasAccess) { errors.push( - `Repository '${repo.repository}' is not accessible. Ensure it is installed via the GitHub App.`, + `Repository '${repo.repository}' is not accessible. Ensure it is connected through its source-control provider.`, ); return; // skip branch check if repo itself is inaccessible } diff --git a/apps/web/src/trpc/commands/setup-new/index.test.ts b/apps/web/src/trpc/commands/setup-new/index.test.ts index 1ac0123b6..5af2b95af 100644 --- a/apps/web/src/trpc/commands/setup-new/index.test.ts +++ b/apps/web/src/trpc/commands/setup-new/index.test.ts @@ -217,6 +217,7 @@ vi.mock('../setup/shared', () => ({ })); import { + didSuggestionSourceChange, getSetupBootstrapStatusCommand, saveSetupBootstrapAuthConfigCommand, saveSetupBootstrapAuthProviderChoiceCommand, @@ -228,6 +229,8 @@ import { startSetupNewOnboardingTaskCommand, } from './index'; import { + ALL_REPOSITORIES, + createEmptySetupNewState, TaskPayloadKind, WORKER_RUNTIME_SCHEMA_VERSION, type SetupNewState, @@ -298,6 +301,21 @@ function createFromOnlySelectChain(result: unknown) { }; } +describe('didSuggestionSourceChange', () => { + it('treats reordered repository ids as the same suggestion source', () => { + expect( + didSuggestionSourceChange({ + currentState: { + ...createEmptySetupNewState(), + selectedRepositoryIds: ['repo-a', 'repo-b'], + }, + nextRepositoryIds: ['repo-b', 'repo-a'], + nextSetupGuidance: null, + }), + ).toBe(false); + }); +}); + describe('setup-new auth config commands', () => { beforeEach(() => { vi.clearAllMocks(); @@ -1199,6 +1217,75 @@ describe('setup-new onboarding task start command', () => { ); }); + it('uses the first workspace provider when setup repositories are mixed', async () => { + vi.mocked(getRepositories).mockResolvedValue([ + { + id: 'repo-1', + fullName: 'octo/api', + sourceControlProvider: 'github', + }, + { + id: 'repo-2', + fullName: 'group/web', + sourceControlProvider: 'gitlab', + }, + ] as Awaited>); + vi.mocked(normalizeRepositorySelection).mockReturnValue([ + 'repo-1', + 'repo-2', + ]); + vi.mocked(buildSetupNewWorkspacePayload).mockReturnValue({ + repo: ALL_REPOSITORIES, + selectedRepositories: ['octo/api', 'group/web'], + }); + mockOnboardingTransaction({ + slackInstallation: null, + setupNewState: { selectedRepositoryIds: ['repo-1', 'repo-2'] }, + }); + + await startSetupNewOnboardingTaskCommand(buildMockAuth()); + + expect(enqueueTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.objectContaining({ + payload: expect.objectContaining({ + sourceControlProvider: 'github', + }), + }), + }), + ); + }); + + it('rejects selected repositories with duplicate full names', async () => { + vi.mocked(getRepositories).mockResolvedValue([ + { + id: 'repo-github', + fullName: 'group/project', + sourceControlProvider: 'github', + }, + { + id: 'repo-gitlab', + fullName: 'group/project', + sourceControlProvider: 'gitlab', + }, + ] as Awaited>); + mockOnboardingTransaction({ + slackInstallation: null, + setupNewState: { + selectedRepositoryIds: ['repo-github', 'repo-gitlab'], + }, + }); + + await expect( + startSetupNewOnboardingTaskCommand(buildMockAuth()), + ).rejects.toMatchObject({ + code: 'BAD_REQUEST', + message: + 'The selected repositories include multiple entries named "group/project". Select only one because task workspaces identify repositories by full name.', + }); + expect(enqueueTask).not.toHaveBeenCalled(); + }); + it('launches with bootstrap instructions instead of blocking when every selected repo is empty', async () => { const { getRepositoryEmptyStates } = await import('@roomote/github'); vi.mocked(getRepositoryEmptyStates).mockResolvedValue( diff --git a/apps/web/src/trpc/commands/setup-new/index.ts b/apps/web/src/trpc/commands/setup-new/index.ts index 120e0488d..898b737e1 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -1,9 +1,7 @@ import * as GitHub from '@roomote/github'; +import { TRPCError } from '@trpc/server'; import { enqueueTask } from '@roomote/cloud-agents/server'; -import { - resolveEnvironmentSourceControlProvider, - resolveSingleSourceControlProvider, -} from '@/lib/server/source-control-provider'; +import { resolveEnvironmentSourceControlProvider } from '@/lib/server/source-control-provider'; import { buildSetupKickoffText } from '@roomote/communication/chat-messages'; import { DiscordCommunicationProvider } from '@roomote/communication/discord-provider'; import type { TeamsCommunicationProvider } from '@roomote/communication/teams-provider'; @@ -66,6 +64,7 @@ import { TaskPayloadKind, resolveEvalHarnessSelection, type ComputeProvider, + assertUniqueRepositoryFullNames, type DeploymentModelConfig, deriveWorkerImageFromReleaseVersion, getSetupAuthProvider, @@ -296,7 +295,10 @@ async function resolveSelectedRepositories(repositoryIds: string[]): Promise<{ const repository = availableRepositoriesById.get(repositoryId); if (!repository) { - throw new Error('Selected repositories are no longer available.'); + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'Selected repositories are no longer available.', + }); } selectedRepositories.push({ @@ -306,11 +308,24 @@ async function resolveSelectedRepositories(repositoryIds: string[]): Promise<{ }); } + try { + assertUniqueRepositoryFullNames( + selectedRepositories.map((repository) => repository.fullName), + ); + } catch (error) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: + error instanceof Error + ? error.message + : 'The selected repositories are invalid.', + cause: error, + }); + } + return { normalizedRepositoryIds: normalizeRepositorySelection(selectedRepositories), - selectedRepositories: selectedRepositories.sort((left, right) => - left.fullName.localeCompare(right.fullName), - ), + selectedRepositories, }; } @@ -558,7 +573,7 @@ async function resolveSetupChatFallbackHandoffTarget( return null; } -function didSuggestionSourceChange({ +export function didSuggestionSourceChange({ currentState, nextRepositoryIds, nextSetupGuidance, @@ -567,11 +582,14 @@ function didSuggestionSourceChange({ nextRepositoryIds: string[]; nextSetupGuidance: string | null; }): boolean { + const currentRepositoryIdSet = new Set(currentState.selectedRepositoryIds); + const nextRepositoryIdSet = new Set(nextRepositoryIds); + return ( currentState.setupGuidance !== nextSetupGuidance || - currentState.selectedRepositoryIds.length !== nextRepositoryIds.length || - currentState.selectedRepositoryIds.some( - (repositoryId, index) => repositoryId !== nextRepositoryIds[index], + currentRepositoryIdSet.size !== nextRepositoryIdSet.size || + [...currentRepositoryIdSet].some( + (repositoryId) => !nextRepositoryIdSet.has(repositoryId), ) ); } @@ -2493,16 +2511,25 @@ export async function startSetupNewOnboardingTaskCommand( const onboardingTaskTitle = buildSetupEnvironmentTaskTitle( selectedRepositoryFullNames, ); - const workspacePayload = buildSetupNewWorkspacePayload( - selectedRepositoryFullNames, - ); + let workspacePayload: ReturnType; + try { + workspacePayload = buildSetupNewWorkspacePayload( + selectedRepositoryFullNames, + ); + } catch (error) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: + error instanceof Error + ? error.message + : 'The selected repositories are invalid.', + cause: error, + }); + } // Stamp the provider explicitly: dequeue defaults to GitHub when the // payload omits it, which breaks non-GitHub deployments. - const setupSourceControlProvider = resolveSingleSourceControlProvider( - selectedRepositories.map( - (repository) => repository.sourceControlProvider, - ), - ); + const setupSourceControlProvider = + selectedRepositories[0]?.sourceControlProvider; const emptyRepositoryFullNames = selectedRepositories .filter((repository) => repositoryEmptyStates.get(repository.id) === true) .map((repository) => repository.fullName); diff --git a/apps/web/src/trpc/commands/setup-new/launch-lifecycle.test.ts b/apps/web/src/trpc/commands/setup-new/launch-lifecycle.test.ts index 38d42eb9e..9270c91ae 100644 --- a/apps/web/src/trpc/commands/setup-new/launch-lifecycle.test.ts +++ b/apps/web/src/trpc/commands/setup-new/launch-lifecycle.test.ts @@ -89,7 +89,6 @@ vi.mock('@roomote/sdk/server', () => ({ // to null so the enqueue payload is deterministic. vi.mock('@/lib/server/source-control-provider', () => ({ resolveEnvironmentSourceControlProvider: vi.fn(async () => null), - resolveSingleSourceControlProvider: vi.fn(() => null), })); vi.mock('@/lib/server', () => ({ diff --git a/apps/web/src/trpc/commands/task-runs/index.test.ts b/apps/web/src/trpc/commands/task-runs/index.test.ts index c06218997..15fee8b0b 100644 --- a/apps/web/src/trpc/commands/task-runs/index.test.ts +++ b/apps/web/src/trpc/commands/task-runs/index.test.ts @@ -52,7 +52,7 @@ vi.mock('@roomote/db/server', () => ({ right, })), markTaskStartParallelCountEndedAt: vi.fn(), - resolveWorkspaceSourceControlProvider: (...args: unknown[]) => + resolveWorkspaceRepositoryProviders: (...args: unknown[]) => mockResolveWorkspaceProvider(...args), repositories: { id: 'repositories.id', @@ -138,7 +138,7 @@ describe('createStandardTaskRunCommand', () => { }); mockDbWhere.mockResolvedValue([]); // Shared resolver defaults to unresolved; the environment test overrides it. - mockResolveWorkspaceProvider.mockResolvedValue(undefined); + mockResolveWorkspaceProvider.mockResolvedValue({}); mockSuccessfulEnqueue(); }); @@ -265,7 +265,9 @@ describe('createStandardTaskRunCommand', () => { it('stamps an environment source-control provider from its repository mappings', async () => { // The environment resolver delegates to the shared @roomote/db resolver. - mockResolveWorkspaceProvider.mockResolvedValue('ado'); + mockResolveWorkspaceProvider.mockResolvedValue({ + 'acme/Platform/backend': 'ado', + }); const result = await createStandardTaskRunCommand(auth, { payload: { @@ -296,4 +298,65 @@ describe('createStandardTaskRunCommand', () => { }), ); }); + + it('uses the first repository provider for a mixed environment', async () => { + mockResolveWorkspaceProvider.mockResolvedValue({ + 'octo/api': 'github', + 'group/web': 'gitlab', + }); + + const result = await createStandardTaskRunCommand(auth, { + payload: { + repo: ALL_REPOSITORIES, + environmentId: '7bb91386-6282-4c98-9b31-0eb181116822', + description: 'Update the environment', + }, + }); + + expect(result.success).toBe(true); + expect(mockEnqueueTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.objectContaining({ + payload: expect.objectContaining({ + sourceControlProvider: 'github', + }), + }), + }), + ); + }); + + it('allows mixed selected repositories and keeps selection order for the primary provider', async () => { + mockGetRepositories.mockResolvedValue([ + { + id: 'repo-github', + fullName: 'octo/api', + sourceControlProvider: 'github', + }, + { + id: 'repo-gitlab', + fullName: 'group/web', + sourceControlProvider: 'gitlab', + }, + ]); + + const result = await createStandardTaskRunCommand(auth, { + payload: { + repo: ALL_REPOSITORIES, + selectedRepositories: ['octo/api', 'group/web'], + description: 'Update selected repositories', + }, + }); + + expect(result.success).toBe(true); + expect(mockEnqueueTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.objectContaining({ + payload: expect.objectContaining({ + selectedRepositories: ['octo/api', 'group/web'], + sourceControlProvider: 'github', + }), + }), + }), + ); + }); }); diff --git a/apps/web/src/trpc/commands/task-runs/index.ts b/apps/web/src/trpc/commands/task-runs/index.ts index 8cf9062b2..4bf343023 100644 --- a/apps/web/src/trpc/commands/task-runs/index.ts +++ b/apps/web/src/trpc/commands/task-runs/index.ts @@ -36,7 +36,7 @@ import type { UserAuthSuccess } from '@/types'; import { Env, getArtifactById, getRepositories } from '@/lib/server'; import { resolveEnvironmentSourceControlProvider, - resolveSingleSourceControlProvider, + resolveSelectedRepositorySourceControlProvider, } from '@/lib/server/source-control-provider'; import { humanizeFilename } from '@/lib/task-utils'; @@ -59,9 +59,7 @@ function getManualTaskRepositoryFullNames( payload: TaskPayload, ) { if (payload.selectedRepositories?.length) { - return [...new Set(payload.selectedRepositories.filter(Boolean))].sort( - (left, right) => left.localeCompare(right), - ); + return [...new Set(payload.selectedRepositories.filter(Boolean))]; } if (payload.repo && payload.repo !== ALL_REPOSITORIES) { @@ -362,10 +360,9 @@ export async function createStandardTaskRunCommand( ); const sourceControlProvider = input.payload.sourceControlProvider ?? - resolveSingleSourceControlProvider( - selectedRepositories.map( - (repository) => repository.sourceControlProvider, - ), + resolveSelectedRepositorySourceControlProvider( + selectedRepositories, + selectedRepositoryFullNames, ) ?? (await resolveEnvironmentSourceControlProvider( input.payload.environmentId, diff --git a/apps/web/src/trpc/commands/task-suggestions/onboarding.ts b/apps/web/src/trpc/commands/task-suggestions/onboarding.ts index cce2fe55d..e24a821ca 100644 --- a/apps/web/src/trpc/commands/task-suggestions/onboarding.ts +++ b/apps/web/src/trpc/commands/task-suggestions/onboarding.ts @@ -10,9 +10,8 @@ import { deploymentSettings, eq, getBackgroundAgentSettingsForDeployment, - inArray, - repositories, resolveRepositorySelectionByIds, + resolveWorkspaceRepositoryProviders, workItems, } from '@roomote/db/server'; import { @@ -22,10 +21,10 @@ import { createEmptySetupNewState, isExitedRunStatus, normalizeSetupNewState, + resolveTaskWorkspace, } from '@roomote/types'; import { getLatestTaskRunsByTaskId } from '@/lib/server'; -import { resolveSingleSourceControlProvider } from '@/lib/server/source-control-provider'; import type { UserAuthSuccess } from '@/types'; import { assertAdmin } from '../setup/shared'; import { decorateSuggestionsWithEnvironmentIds } from './launch-resolution'; @@ -137,13 +136,13 @@ async function launchSuggestedTasksTask(input: { ); // Stamp the provider explicitly: dequeue defaults to GitHub when the // payload omits it, which breaks non-GitHub deployments. - const scanRepositoryRows = await db - .select({ sourceControlProvider: repositories.sourceControlProvider }) - .from(repositories) - .where(inArray(repositories.fullName, input.repositoryFullNames)); - const scanSourceControlProvider = resolveSingleSourceControlProvider( - scanRepositoryRows.map((row) => row.sourceControlProvider), + const scanRepositoryProviders = await resolveWorkspaceRepositoryProviders( + db, + resolveTaskWorkspace(workspacePayload), ); + const scanSourceControlProvider = input.repositoryFullNames + .map((repositoryFullName) => scanRepositoryProviders[repositoryFullName]) + .find((provider) => provider !== undefined); const launchResult = await enqueueTask( { task: { diff --git a/apps/worker/src/commands/__tests__/snapshot.test.ts b/apps/worker/src/commands/__tests__/snapshot.test.ts index 9b7f4af19..559d1c3b8 100644 --- a/apps/worker/src/commands/__tests__/snapshot.test.ts +++ b/apps/worker/src/commands/__tests__/snapshot.test.ts @@ -3,6 +3,7 @@ import { RunStatus } from '@roomote/types'; const { mockTaskRunsUpdate, mockFetchSnapshotEnv, + mockFindTaskRun, mockFindEnvironment, mockDone, mockUpdateSnapshotStatus, @@ -16,6 +17,7 @@ const { } = vi.hoisted(() => ({ mockTaskRunsUpdate: vi.fn(), mockFetchSnapshotEnv: vi.fn(), + mockFindTaskRun: vi.fn(), mockFindEnvironment: vi.fn(), mockDone: vi.fn(), mockUpdateSnapshotStatus: vi.fn(), @@ -33,6 +35,7 @@ vi.mock('@roomote/sdk/client', () => ({ taskRuns: { update: mockTaskRunsUpdate, fetchSnapshotEnv: mockFetchSnapshotEnv, + findFirstById: mockFindTaskRun, done: mockDone, }, environments: { @@ -91,6 +94,13 @@ describe('snapshot', () => { repositories: [{ repository: 'Roomote/example-app' }], }, }); + mockFindTaskRun.mockResolvedValue({ + payload: { + repositoryProviders: { + 'Roomote/example-app': 'gitlab', + }, + }, + }); mockWorkerEnvFromProcessEnv.mockReturnValue({}); mockCreateStartupLogger.mockReturnValue({ userLog: { log: vi.fn() } }); mockDone.mockResolvedValue(undefined); @@ -120,6 +130,15 @@ describe('snapshot', () => { environmentId: 'env-1', snapshotStatus: 'failed', }); + expect(mockSetup).toHaveBeenCalledWith( + expect.objectContaining({ + workspace: expect.objectContaining({ + repositoryProviders: { + 'Roomote/example-app': 'gitlab', + }, + }), + }), + ); const injectCallOrder = mockInjectEnvVars.mock.invocationCallOrder[0]; const findEnvironmentCallOrder = mockFindEnvironment.mock.invocationCallOrder[0]; diff --git a/apps/worker/src/commands/setup/__tests__/workspace.test.ts b/apps/worker/src/commands/setup/__tests__/workspace.test.ts index ceedbb19c..d35c01fc5 100644 --- a/apps/worker/src/commands/setup/__tests__/workspace.test.ts +++ b/apps/worker/src/commands/setup/__tests__/workspace.test.ts @@ -140,6 +140,73 @@ describe('initializeRepositories', () => { ); }); + it('resolves repository providers from the map before the scalar fallback', async () => { + vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( + undefined, + ); + const prepareRepositorySpy = vi + .spyOn(WorkspaceManager.prototype, 'prepareRepository') + .mockImplementation(async (repo) => `/tmp/${repo}`); + + await initializeRepositories(createLogger(), { + workspace: { + type: 'repository_set', + repositories: ['acme/github-app', 'acme/gitlab-app'], + }, + envVars: {}, + taskRunType: TaskPayloadKind.StandardTask, + sourceControlProvider: 'github', + repositoryProviders: { + 'acme/gitlab-app': 'gitlab', + }, + }); + + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/github-app', + undefined, + undefined, + false, + false, + {}, + ); + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/gitlab-app', + undefined, + undefined, + false, + false, + { sourceControlProvider: 'gitlab' }, + ); + }); + + it('uses a mapped provider for a single-repository workspace', async () => { + vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( + undefined, + ); + const prepareRepositorySpy = vi + .spyOn(WorkspaceManager.prototype, 'prepareRepository') + .mockResolvedValue('/tmp/acme/app'); + + await initializeRepositories(createLogger(), { + workspace: { + type: 'repository', + repository: 'acme/app', + }, + envVars: {}, + taskRunType: TaskPayloadKind.StandardTask, + repositoryProviders: { 'acme/app': 'gitlab' }, + }); + + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/app', + undefined, + undefined, + false, + false, + { sourceControlProvider: 'gitlab' }, + ); + }); + it('continues scoped multi-repo workspace setup when at least one selected repository prepares successfully', async () => { const logger = createLogger(); vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( @@ -237,6 +304,43 @@ describe('initializeRepositories', () => { ); }); + it('applies mapped providers to repositories discovered for all-repositories workspaces', async () => { + vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( + undefined, + ); + const prepareRepositorySpy = vi + .spyOn(WorkspaceManager.prototype, 'prepareRepository') + .mockImplementation(async (repo) => `/tmp/${repo}`); + + await initializeRepositories(createLogger(), { + workspace: { type: 'all_repositories' }, + envVars: {}, + taskRunType: TaskPayloadKind.StandardTask, + repositoryProviders: { + 'acme/github-app': 'github', + 'acme/gitlab-app': 'gitlab', + }, + }); + + expect(mockListRepositories).not.toHaveBeenCalled(); + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/github-app', + undefined, + undefined, + false, + false, + {}, + ); + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/gitlab-app', + undefined, + undefined, + false, + false, + { sourceControlProvider: 'gitlab' }, + ); + }); + it('fails all-repositories workspace setup when no repositories can be prepared', async () => { vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( undefined, @@ -420,6 +524,49 @@ describe('initializeRepositories', () => { {}, ); }); + + it('passes repository provider overrides to environment preparation', async () => { + vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( + undefined, + ); + const prepareEnvironmentRepositoriesSpy = vi + .spyOn(WorkspaceManager.prototype, 'prepareEnvironmentRepositories') + .mockResolvedValue({ repoPaths: {} }); + vi.spyOn( + WorkspaceManager.prototype, + 'installWorkspaceToolVersions', + ).mockResolvedValue(undefined); + + await initializeRepositories(createLogger(), { + workspace: { + type: 'environment', + environmentId: 'env_123', + environmentConfig: { + name: 'Mixed Providers', + repositories: [ + { repository: 'acme/github-app' }, + { repository: 'acme/gitlab-app' }, + ], + }, + } as WorkspaceConfig, + envVars: {}, + taskRunType: TaskPayloadKind.StandardTask, + sourceControlProvider: 'github', + repositoryProviders: { + 'acme/gitlab-app': 'gitlab', + }, + }); + + expect(prepareEnvironmentRepositoriesSpy).toHaveBeenCalledWith( + expect.any(Object), + false, + false, + expect.any(Object), + { + repositoryProviders: { 'acme/gitlab-app': 'gitlab' }, + }, + ); + }); }); describe('initializeSystemServices', () => { diff --git a/apps/worker/src/commands/setup/workspace/repositories.ts b/apps/worker/src/commands/setup/workspace/repositories.ts index c25d05196..96c2a0e86 100644 --- a/apps/worker/src/commands/setup/workspace/repositories.ts +++ b/apps/worker/src/commands/setup/workspace/repositories.ts @@ -70,14 +70,27 @@ export async function initializeRepositories( gitAuthorName, gitAuthorEmail, sourceControlProvider, + repositoryProviders, }: PrepareWorkspaceOptions, ): Promise { const resolvedSourceControlProvider = sourceControlProvider ?? DEFAULT_SOURCE_CONTROL_PROVIDER; - const sourceControlPrepareOptions = - resolvedSourceControlProvider === DEFAULT_SOURCE_CONTROL_PROVIDER + const resolveRepositoryProvider = (repository: string) => + repositoryProviders?.[repository] ?? resolvedSourceControlProvider; + const sourceControlPrepareOptions = (repository: string) => { + const repositoryProvider = resolveRepositoryProvider(repository); + + return repositoryProvider === DEFAULT_SOURCE_CONTROL_PROVIDER + ? {} + : { sourceControlProvider: repositoryProvider }; + }; + const environmentSourceControlPrepareOptions = { + ...(resolvedSourceControlProvider === DEFAULT_SOURCE_CONTROL_PROVIDER ? {} - : { sourceControlProvider: resolvedSourceControlProvider }; + : { sourceControlProvider: resolvedSourceControlProvider }), + ...(repositoryProviders ? { repositoryProviders } : {}), + }; + const mappedRepositoryNames = Object.keys(repositoryProviders ?? {}); const { workspaceRoot, workspaceManager } = createWorkspaceManager( envVars, logger, @@ -104,7 +117,7 @@ export async function initializeRepositories( sourceBranch: workspace.sourceBranch, sourceSha: workspace.sourceSha, }, - sourceControlPrepareOptions, + environmentSourceControlPrepareOptions, ), ); @@ -139,17 +152,22 @@ export async function initializeRepositories( case 'repository_set': case 'all_repositories': { + // A stamped map is the launch-time workspace snapshot. Prefer it over + // a live provider-filtered list so mixed-provider tasks keep every + // repository selected when the task was queued. const repositoriesToPrepare = workspace.type === 'repository_set' ? workspace.repositories.map((fullName) => ({ fullName })) - : await timedStep( - logger, - 'initializeRepositories: list repositories', - () => - sdk.repositories.listRepositories({ - sourceControlProvider: resolvedSourceControlProvider, - }), - ); + : mappedRepositoryNames.length > 0 + ? mappedRepositoryNames.map((fullName) => ({ fullName })) + : await timedStep( + logger, + 'initializeRepositories: list repositories', + () => + sdk.repositories.listRepositories({ + sourceControlProvider: resolvedSourceControlProvider, + }), + ); const limit = pLimit(REPO_PREPARATION_CONCURRENCY); @@ -167,7 +185,7 @@ export async function initializeRepositories( preserveGitState, cleanupLegacyPaths, { - ...sourceControlPrepareOptions, + ...sourceControlPrepareOptions(repo.fullName), }, ), ); @@ -317,7 +335,7 @@ export async function initializeRepositories( workspace.sha, preserveGitState, cleanupLegacyPaths, - sourceControlPrepareOptions, + sourceControlPrepareOptions(workspace.repository), ), ); } catch (error) { diff --git a/apps/worker/src/commands/setup/workspace/types.ts b/apps/worker/src/commands/setup/workspace/types.ts index 64750c34e..d1ba1208f 100644 --- a/apps/worker/src/commands/setup/workspace/types.ts +++ b/apps/worker/src/commands/setup/workspace/types.ts @@ -66,6 +66,7 @@ export interface PrepareWorkspaceOptions { preserveGitState?: boolean; cleanupLegacyPaths?: boolean; sourceControlProvider?: SourceControlProvider; + repositoryProviders?: Record; serviceContext?: ServiceContext; gitAuthorName?: string; gitAuthorEmail?: string; diff --git a/apps/worker/src/commands/snapshot.ts b/apps/worker/src/commands/snapshot.ts index d65989536..f5ad71d92 100644 --- a/apps/worker/src/commands/snapshot.ts +++ b/apps/worker/src/commands/snapshot.ts @@ -17,6 +17,7 @@ import { import { setup } from './setup'; import { injectEnvVars } from './utils/env-vars'; +import { resolveRepositoryProvidersFromPayload } from './utils/repository-providers'; import { scrubSandboxSecretsBeforeSnapshot } from './utils/scrub-sandbox-secrets'; import { findRuntimeEnvironmentConfig } from './utils/workspace-config'; @@ -82,6 +83,7 @@ export async function snapshot({ await injectEnvVars(envVars, undefined, { sourceControlToken }); const environmentConfig = await findRuntimeEnvironmentConfig(environmentId); + const taskRun = await sdk.taskRuns.findFirstById(runId); if (!environmentConfig) { throw new Error(`Environment not found`); @@ -99,6 +101,9 @@ export async function snapshot({ taskRunType: TaskPayloadKind.SnapshotEnvironment, sourceControlProvider: sourceControlToken?.provider ?? DEFAULT_SOURCE_CONTROL_PROVIDER, + repositoryProviders: resolveRepositoryProvidersFromPayload( + taskRun?.payload, + ), }, logger: startupLogger, workerEnv, diff --git a/apps/worker/src/commands/utils/execute-task-run.test.ts b/apps/worker/src/commands/utils/execute-task-run.test.ts index 75b0cc0d1..a390b635a 100644 --- a/apps/worker/src/commands/utils/execute-task-run.test.ts +++ b/apps/worker/src/commands/utils/execute-task-run.test.ts @@ -178,6 +178,9 @@ describe('executeTaskRun', () => { harness: 'opencode-server', payload: { repo: 'owner/repo', + repositoryProviders: { + 'owner/repo': 'gitlab', + }, }, }, envVars: { @@ -234,6 +237,9 @@ describe('executeTaskRun', () => { expect(setupArgs.workspace.envVars).toMatchObject({ FOO: 'bar', }); + expect(setupArgs.workspace.repositoryProviders).toEqual({ + 'owner/repo': 'gitlab', + }); expect(typeof setupArgs.recordPhase).toBe('function'); expect(sdkTaskRunsStampMilestoneMock).toHaveBeenCalledWith({ runId: 42, diff --git a/apps/worker/src/commands/utils/execute-task-run.ts b/apps/worker/src/commands/utils/execute-task-run.ts index f08420aa7..1f92976d8 100644 --- a/apps/worker/src/commands/utils/execute-task-run.ts +++ b/apps/worker/src/commands/utils/execute-task-run.ts @@ -50,6 +50,7 @@ import { import { BackgroundEnvironmentSetupController } from './background-environment-setup-controller'; import { injectEnvVars, writeBashrc } from './env-vars'; +import { resolveRepositoryProvidersFromPayload } from './repository-providers'; import { buildServiceContextForPreviewProxy } from './service-context'; import { finalizeJob, handleTaskRunError } from './task-run-lifecycle'; @@ -539,6 +540,9 @@ export async function executeTaskRun({ sourceControlProvider: resolveSourceControlProviderFromPayload( currentTaskRun.payload, ), + repositoryProviders: resolveRepositoryProvidersFromPayload( + currentTaskRun.payload, + ), gitAuthorName: jobContext.gitAuthor?.name, gitAuthorEmail: jobContext.gitAuthor?.email, }, diff --git a/apps/worker/src/commands/utils/repository-providers.ts b/apps/worker/src/commands/utils/repository-providers.ts new file mode 100644 index 000000000..7dd160673 --- /dev/null +++ b/apps/worker/src/commands/utils/repository-providers.ts @@ -0,0 +1,15 @@ +import type { SourceControlProvider } from '@roomote/types'; + +export function resolveRepositoryProvidersFromPayload( + payload: unknown, +): Record | undefined { + if (!payload || typeof payload !== 'object') { + return undefined; + } + + return ( + payload as { + repositoryProviders?: Record; + } + ).repositoryProviders; +} diff --git a/apps/worker/src/commands/utils/scrub-sandbox-secrets.test.ts b/apps/worker/src/commands/utils/scrub-sandbox-secrets.test.ts index a940a202e..2ba043abe 100644 --- a/apps/worker/src/commands/utils/scrub-sandbox-secrets.test.ts +++ b/apps/worker/src/commands/utils/scrub-sandbox-secrets.test.ts @@ -30,6 +30,12 @@ vi.mock('os', async (importOriginal) => { }); const COMMON_ENV_PATH = '/home/testuser/.roomote/env.sh'; +const TOKEN_ENV_PATHS = [ + '/home/testuser/.roomote/gh-token-env.sh', + '/home/testuser/.roomote/gitlab-token-env.sh', + '/home/testuser/.roomote/gitea-token-env.sh', + '/home/testuser/.roomote/ado-token-env.sh', +]; const EXPECTED_REMOVED_PATHS = [ '/home/testuser/.roomote/gh-token', @@ -89,6 +95,14 @@ describe('scrubSandboxSecretsBeforeSnapshot', () => { } }); + it('recreates every provider token env script before snapshotting', async () => { + await scrubSandboxSecretsBeforeSnapshot(); + + for (const path of TOKEN_ENV_PATHS) { + expect(findWrite(path)).toBeDefined(); + } + }); + it('respects XDG_DATA_HOME when locating OpenCode credential files', async () => { process.env.XDG_DATA_HOME = '/custom/data'; diff --git a/apps/worker/src/workspace/__tests__/tool-versions.test.ts b/apps/worker/src/workspace/__tests__/tool-versions.test.ts index 604a0f9c1..dbb26226b 100644 --- a/apps/worker/src/workspace/__tests__/tool-versions.test.ts +++ b/apps/worker/src/workspace/__tests__/tool-versions.test.ts @@ -876,6 +876,54 @@ describe('WorkspaceManager tool versions', () => { }, ); }); + + it('chooses the source control provider separately for each repository', async () => { + const prepareRepositorySpy = vi + .spyOn(manager, 'prepareRepository') + .mockImplementation(async (repository) => `/workspace/${repository}`); + + await manager.prepareEnvironmentRepositories( + { + name: 'Mixed Providers', + repositories: [ + { repository: 'acme/github-app' }, + { repository: 'acme/gitlab-app' }, + ], + }, + false, + false, + undefined, + { + sourceControlProvider: 'github', + repositoryProviders: { 'acme/gitlab-app': 'gitlab' }, + }, + ); + + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/github-app', + undefined, + undefined, + false, + false, + { + sourceControlProvider: 'github', + setDefaultRemote: false, + toolVersionsConfig: undefined, + }, + ); + expect(prepareRepositorySpy).toHaveBeenCalledWith( + 'acme/gitlab-app', + undefined, + undefined, + false, + false, + { + sourceControlProvider: 'gitlab', + setDefaultRemote: false, + toolVersionsConfig: undefined, + }, + ); + }); }); describe('installWorkspaceToolVersions', () => { diff --git a/apps/worker/src/workspace/workspace-manager.ts b/apps/worker/src/workspace/workspace-manager.ts index daa4074a3..5f1757899 100644 --- a/apps/worker/src/workspace/workspace-manager.ts +++ b/apps/worker/src/workspace/workspace-manager.ts @@ -766,7 +766,9 @@ export class WorkspaceManager { sourceBranch?: string; sourceSha?: string; }, - options: Pick = {}, + options: Pick & { + repositoryProviders?: Record; + } = {}, ): Promise<{ repoPaths: Record; }> { @@ -794,7 +796,9 @@ export class WorkspaceManager { preserveGitState, cleanupLegacyPaths, { - sourceControlProvider: options.sourceControlProvider, + sourceControlProvider: + options.repositoryProviders?.[repoConfig.repository] ?? + options.sourceControlProvider, toolVersionsConfig: repoConfig.tool_versions, setDefaultRemote: false, }, diff --git a/packages/ado/src/api.ts b/packages/ado/src/api.ts index 3aa16975b..f7f0fce6f 100644 --- a/packages/ado/src/api.ts +++ b/packages/ado/src/api.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { ALL_REPOSITORIES, buildRepositoryCloneUrl, + filterRepositoryNamesForSourceControlProvider, stripCloneUrlUserInfo, type SourceControlProvider, } from '@roomote/types'; @@ -1482,6 +1483,14 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { async function resolveAdoRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { + const filterForAdo = (repositoryNames: string[]) => { + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + ADO_PROVIDER, + ); + }; + if (taskRun.payload.environmentId) { const environment = await db.query.environments.findFirst({ where: eq(environments.id, taskRun.payload.environmentId), @@ -1493,9 +1502,11 @@ async function resolveAdoRepositoryNamesForTaskRun( ); } - return normalizeRepositorySelection( - environment.config.repositories.map( - (repository) => repository.repository, + return filterForAdo( + normalizeRepositorySelection( + environment.config.repositories.map( + (repository) => repository.repository, + ), ), ); } @@ -1506,7 +1517,7 @@ async function resolveAdoRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterForAdo(selectedRepositories); } } diff --git a/packages/bitbucket/src/api.ts b/packages/bitbucket/src/api.ts index ca757695d..c16d4f90d 100644 --- a/packages/bitbucket/src/api.ts +++ b/packages/bitbucket/src/api.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { ALL_REPOSITORIES, buildRepositoryCloneUrl, + filterRepositoryNamesForSourceControlProvider, type SourceControlProvider, } from '@roomote/types'; import { @@ -754,6 +755,14 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { async function resolveBitbucketRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { + const filterForBitbucket = (repositoryNames: string[]) => { + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + BITBUCKET_PROVIDER, + ); + }; + if (taskRun.payload.environmentId) { const environment = await db.query.environments.findFirst({ where: eq(environments.id, taskRun.payload.environmentId), @@ -765,9 +774,11 @@ async function resolveBitbucketRepositoryNamesForTaskRun( ); } - return normalizeRepositorySelection( - environment.config.repositories.map( - (repository) => repository.repository, + return filterForBitbucket( + normalizeRepositorySelection( + environment.config.repositories.map( + (repository) => repository.repository, + ), ), ); } @@ -778,7 +789,7 @@ async function resolveBitbucketRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterForBitbucket(selectedRepositories); } } diff --git a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts index 7e471ab0d..4fff004ae 100644 --- a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts +++ b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts @@ -15,6 +15,7 @@ vi.mock('../llm-task-title', async (importOriginal) => ({ })); import { + ALL_REPOSITORIES, type TaskSpec, type SnapshotResumeTask, RunStatus, @@ -787,6 +788,10 @@ describe('enqueueTask snapshot resume', () => { description: 'Do the thing', sourceControlProvider: 'ado', sourceControlHost: 'dev.azure.com', + repositoryProviders: { + 'roomote/Test ADO/Test ADO': 'ado', + 'group/web': 'gitlab', + }, }, }), initiator: { kind: 'user', userId }, @@ -815,10 +820,15 @@ describe('enqueueTask snapshot resume', () => { const resumePayload = resumeRun.payload as { sourceControlProvider?: string; sourceControlHost?: string; + repositoryProviders?: Record; }; expect(resumePayload.sourceControlProvider).toBe('ado'); expect(resumePayload.sourceControlHost).toBe('dev.azure.com'); + expect(resumePayload.repositoryProviders).toEqual({ + 'roomote/Test ADO/Test ADO': 'ado', + 'group/web': 'gitlab', + }); }); it('walks the resume chain for stamps when the source run predates inheritance', async () => { @@ -891,6 +901,10 @@ describe('enqueueTask snapshot resume', () => { description: 'Do the thing', sourceControlProvider: 'ado', sourceControlHost: 'dev.azure.com', + repositoryProviders: { + 'acme/widgets': 'ado', + 'group/web': 'gitlab', + }, }, }), initiator: { kind: 'user', userId }, @@ -925,6 +939,13 @@ describe('enqueueTask snapshot resume', () => { expect( (resumeRun.payload as { sourceControlHost?: string }).sourceControlHost, ).toBeUndefined(); + expect( + ( + resumeRun.payload as { + repositoryProviders?: Record; + } + ).repositoryProviders, + ).toBeUndefined(); }); it('rejects a resume without a source run id', async () => { @@ -1281,6 +1302,10 @@ describe('enqueueTask source-control provider stamping', () => { const environment = await environmentFactory.create({ createdByUserId: userId, + config: { + name: 'GitLab environment', + repositories: [{ repository: 'group/project' }], + }, }); createdEnvironmentIds.push(environment.id); @@ -1314,6 +1339,278 @@ describe('enqueueTask source-control provider stamping', () => { (persistedRun!.payload as { sourceControlProvider?: string }) .sourceControlProvider, ).toBe('gitlab'); + expect( + (persistedRun!.payload as { repositoryProviders?: unknown }) + .repositoryProviders, + ).toBeUndefined(); + }); + + it('stamps a provider map and the first repository provider for a mixed environment', async () => { + const userId = await createUser(); + const primaryRepository = await repositoryFactory.create({ + sourceControlProvider: 'gitlab', + linkedByUserId: userId, + fullName: 'group/mixed-api', + isActive: true, + }); + const secondaryRepository = await repositoryFactory.create({ + sourceControlProvider: 'ado', + linkedByUserId: userId, + fullName: 'acme/Platform/mixed-web', + isActive: true, + }); + createdRepositoryIds.push(primaryRepository.id, secondaryRepository.id); + + const environment = await environmentFactory.create({ + createdByUserId: userId, + config: { + name: 'Mixed environment', + repositories: [ + { repository: 'group/mixed-api' }, + { repository: 'acme/Platform/mixed-web' }, + ], + }, + }); + createdEnvironmentIds.push(environment.id); + + await db.insert(environmentRepositoryMappings).values([ + { + environmentId: environment.id, + repositoryId: primaryRepository.id, + createdAt: new Date('2026-01-01T00:00:00.000Z'), + }, + { + environmentId: environment.id, + repositoryId: secondaryRepository.id, + createdAt: new Date('2026-01-01T00:00:01.000Z'), + }, + ]); + + const run = await launchFresh({ + task: standardTaskInput({ + payload: { + repo: 'group/mixed-api', + environmentId: environment.id, + description: 'Work in the mixed environment', + }, + }), + initiator: { kind: 'user', userId }, + workflow: 'standard', + surface: 'web', + trigger: 'manual', + }); + + expect(run.payload).toMatchObject({ + sourceControlProvider: 'gitlab', + repositoryProviders: { + 'group/mixed-api': 'gitlab', + 'acme/Platform/mixed-web': 'ado', + }, + }); + expect(run.payload.sourceControlHost).toBeUndefined(); + }); + + it('stamps mixed selected repositories in selection order', async () => { + const userId = await createUser(); + const gitLabRepository = await repositoryFactory.create({ + sourceControlProvider: 'gitlab', + linkedByUserId: userId, + fullName: 'group/selected-web', + isActive: true, + }); + const adoRepository = await repositoryFactory.create({ + sourceControlProvider: 'ado', + linkedByUserId: userId, + fullName: 'acme/Platform/selected-api', + isActive: true, + }); + createdRepositoryIds.push(gitLabRepository.id, adoRepository.id); + + const run = await launchFresh({ + task: standardTaskInput({ + payload: { + repo: ALL_REPOSITORIES, + selectedRepositories: [ + 'group/selected-web', + 'acme/Platform/selected-api', + ], + description: 'Work across selected providers', + }, + }), + initiator: { kind: 'user', userId }, + workflow: 'standard', + surface: 'web', + trigger: 'manual', + }); + + expect(run.payload).toMatchObject({ + sourceControlProvider: 'gitlab', + repositoryProviders: { + 'group/selected-web': 'gitlab', + 'acme/Platform/selected-api': 'ado', + }, + }); + }); + + it('re-stamps a PR launch after auto-resolving a mixed environment', async () => { + const userId = await createUser(); + const primaryRepository = await repositoryFactory.create({ + sourceControlProvider: 'gitlab', + linkedByUserId: userId, + fullName: 'group/pr-context', + isActive: true, + }); + const pullRequestRepository = await repositoryFactory.create({ + sourceControlProvider: 'gitea', + linkedByUserId: userId, + fullName: 'octo/pr-target', + isActive: true, + }); + createdRepositoryIds.push(primaryRepository.id, pullRequestRepository.id); + + const environment = await environmentFactory.create({ + createdByUserId: userId, + config: { + name: 'PR mixed environment', + repositories: [ + { repository: 'group/pr-context' }, + { repository: 'octo/pr-target' }, + ], + }, + }); + createdEnvironmentIds.push(environment.id); + await db.insert(environmentRepositoryMappings).values([ + { + environmentId: environment.id, + repositoryId: primaryRepository.id, + }, + { + environmentId: environment.id, + repositoryId: pullRequestRepository.id, + }, + ]); + + const run = await launchFresh({ + task: { + type: TaskPayloadKind.GithubPrReview, + requestedWorkKindDecision: explicitWorkKind, + payload: { + repo: 'octo/pr-target', + prNumber: 1082, + prTitle: 'Support mixed environments', + prUrl: 'https://github.com/octo/pr-target/pull/1082', + headSha: 'a'.repeat(40), + sourceControlProvider: 'gitea', + sourceControlHost: 'gitea.example.com', + }, + } as Extract, + initiator: { kind: 'automation', key: 'review_code' }, + workflow: 'pr_review', + surface: 'github', + trigger: 'webhook', + prLinkage: { + provider: 'gitea', + repository: 'octo/pr-target', + prNumber: 1082, + prUrl: 'https://github.com/octo/pr-target/pull/1082', + prTitle: 'Support mixed environments', + prSha: 'a'.repeat(40), + }, + }); + + expect(run.payload).toMatchObject({ + environmentId: environment.id, + sourceControlProvider: 'gitlab', + repositoryProviders: { + 'group/pr-context': 'gitlab', + 'octo/pr-target': 'gitea', + }, + }); + expect(run.payload.sourceControlHost).toBeUndefined(); + }); + + it('recomputes mixed-provider stamps for a failed-start relaunch', async () => { + const userId = await createUser(); + const primaryRepository = await repositoryFactory.create({ + sourceControlProvider: 'gitlab', + linkedByUserId: userId, + fullName: 'group/relaunch-api', + isActive: true, + }); + const secondaryRepository = await repositoryFactory.create({ + sourceControlProvider: 'ado', + linkedByUserId: userId, + fullName: 'acme/Platform/relaunch-web', + isActive: true, + }); + createdRepositoryIds.push(primaryRepository.id, secondaryRepository.id); + + const environment = await environmentFactory.create({ + createdByUserId: userId, + config: { + name: 'Relaunch environment', + repositories: [ + { repository: 'group/relaunch-api' }, + { repository: 'acme/Platform/relaunch-web' }, + ], + }, + }); + createdEnvironmentIds.push(environment.id); + await db.insert(environmentRepositoryMappings).values([ + { + environmentId: environment.id, + repositoryId: primaryRepository.id, + createdAt: new Date('2026-01-02T00:00:00.000Z'), + }, + { + environmentId: environment.id, + repositoryId: secondaryRepository.id, + createdAt: new Date('2026-01-02T00:00:01.000Z'), + }, + ]); + + const failedRun = await launchFresh({ + task: standardTaskInput({ + payload: { + repo: 'group/relaunch-api', + environmentId: environment.id, + description: 'Work in the mixed environment', + }, + }), + initiator: { kind: 'user', userId }, + workflow: 'standard', + surface: 'web', + trigger: 'manual', + }); + const legacyPayload = { ...failedRun.payload }; + delete legacyPayload.sourceControlProvider; + delete legacyPayload.repositoryProviders; + await db + .update(taskRuns) + .set({ + payload: legacyPayload, + status: RunStatus.Failed, + error: 'Failed to create source control token', + completedAt: new Date(), + }) + .where(eq(taskRuns.id, failedRun.id)); + await db + .update(tasks) + .set({ state: 'failed' }) + .where(eq(tasks.id, failedRun.taskId)); + + const relaunchRun = await enqueueTaskRelaunch( + { sourceRunId: failedRun.id, actingUserId: userId }, + { enqueue: false }, + ); + + expect(relaunchRun.payload).toMatchObject({ + sourceControlProvider: 'gitlab', + repositoryProviders: { + 'group/relaunch-api': 'gitlab', + 'acme/Platform/relaunch-web': 'ado', + }, + }); }); }); diff --git a/packages/cloud-agents/src/server/task-run-queue.ts b/packages/cloud-agents/src/server/task-run-queue.ts index 65ea543c0..6792a77f6 100644 --- a/packages/cloud-agents/src/server/task-run-queue.ts +++ b/packages/cloud-agents/src/server/task-run-queue.ts @@ -64,7 +64,7 @@ import { lt, recordSnapshotResumeEvent, resolveDefaultComputeProvider, - resolveWorkspaceSourceControlProvider, + resolveWorkspaceRepositoryProviders, sql, } from '@roomote/db/server'; import { type Redis, getRedis } from '@roomote/redis'; @@ -1340,26 +1340,7 @@ async function enqueueFreshLaunch( ]); const workspace = resolveTaskWorkspace(task.payload); - // Stamp the source-control provider once at launch when the caller omitted - // it. Downstream consumers (token minting, worker repository resolution) - // otherwise fall back to the GitHub default, which breaks GitLab/Gitea/ADO - // deployments for any launch surface that forgot the stamp. The shared - // resolver covers every workspace shape (repository, repository_set, - // environment, all_repositories) so environment- and all-repositories-based - // launches (e.g. Linear) get stamped too. - if ( - !('sourceControlProvider' in task.payload) || - !task.payload.sourceControlProvider - ) { - const resolvedProvider = await resolveWorkspaceSourceControlProvider( - db, - workspace, - ); - - if (resolvedProvider) { - task.payload.sourceControlProvider = resolvedProvider; - } - } + await stampWorkspaceSourceControlProviders(task.payload, workspace); if ( PR_TASK_TYPES.has(task.type) && @@ -1373,6 +1354,10 @@ async function enqueueFreshLaunch( if (envId) { task.payload.environmentId = envId; + await stampWorkspaceSourceControlProviders(task.payload, { + type: 'environment', + environmentId: envId, + }); console.log( `[enqueueTask] Auto-resolved environment ${envId} for ${workspace.repo}`, @@ -1797,6 +1782,37 @@ function reconstructFreshTaskFromFailedRun(sourceRun: TaskRun): FreshTask { } as FreshTask; } +async function stampWorkspaceSourceControlProviders( + payload: FreshTask['payload'], + workspace: ReturnType, +): Promise { + const repositoryProviders = await resolveWorkspaceRepositoryProviders( + db, + workspace, + ); + const providers = Object.values(repositoryProviders); + const spansProviders = new Set(providers).size > 1; + + if (spansProviders) { + payload.repositoryProviders = repositoryProviders; + } + + const primaryProvider = providers[0]; + if ( + primaryProvider && + (spansProviders || payload.sourceControlProvider === undefined) + ) { + if ( + spansProviders || + (payload.sourceControlProvider !== undefined && + payload.sourceControlProvider !== primaryProvider) + ) { + payload.sourceControlHost = undefined; + } + payload.sourceControlProvider = primaryProvider; + } +} + /** * Re-enqueues a failed first-start run on the same task (new run row, same task * id). Used when environment creation fails before the session can start and the @@ -1864,19 +1880,7 @@ export async function enqueueTaskRelaunch( const workspace = resolveTaskWorkspace(task.payload); - if ( - !('sourceControlProvider' in task.payload) || - !task.payload.sourceControlProvider - ) { - const resolvedProvider = await resolveWorkspaceSourceControlProvider( - db, - workspace, - ); - - if (resolvedProvider) { - task.payload.sourceControlProvider = resolvedProvider; - } - } + await stampWorkspaceSourceControlProviders(task.payload, workspace); const { initialPaths } = await resolveEnvironmentContext(task); const resolvedHarness = await resolveRequestedHarness(task); @@ -2013,23 +2017,40 @@ function inheritSnapshotResumeSourceControlStamps( sourcePayload: unknown, ): void { const source = (sourcePayload ?? {}) as { + repositoryProviders?: unknown; sourceControlProvider?: unknown; sourceControlHost?: unknown; }; const inheritsProvider = payload.sourceControlProvider === undefined; + const provider = sourceControlProviderSchema.safeParse( + source.sourceControlProvider, + ); if (inheritsProvider) { - const provider = sourceControlProviderSchema.safeParse( - source.sourceControlProvider, - ); - if (provider.success) { payload.sourceControlProvider = provider.data; } } - if (inheritsProvider && payload.sourceControlHost === undefined) { + const usesSourceProvider = + provider.success && payload.sourceControlProvider === provider.data; + + if (usesSourceProvider && payload.repositoryProviders === undefined) { + const repositoryProviders = z + .record(sourceControlProviderSchema) + .safeParse(source.repositoryProviders); + + if (repositoryProviders.success) { + payload.repositoryProviders = repositoryProviders.data; + } + } + + if ( + inheritsProvider && + provider.success && + payload.sourceControlHost === undefined + ) { const host = typeof source.sourceControlHost === 'string' ? source.sourceControlHost.trim() diff --git a/packages/db/src/lib/__tests__/repositories.test.ts b/packages/db/src/lib/__tests__/repositories.test.ts index f2a643310..9195481d0 100644 --- a/packages/db/src/lib/__tests__/repositories.test.ts +++ b/packages/db/src/lib/__tests__/repositories.test.ts @@ -94,14 +94,14 @@ describe('resolveRepositorySelectionByIds', () => { repositoryIds: [repoB.id, repoA.id], }); - expect(result.normalizedRepositoryIds).toEqual([repoA.id, repoB.id]); + expect(result.normalizedRepositoryIds).toEqual([repoB.id, repoA.id]); expect(result.selectedRepositories).toEqual([ { id: repoB.id, fullName: repoB.fullName }, { id: repoA.id, fullName: repoA.fullName }, ]); expect(result.workspacePayload).toEqual({ repo: '__all_repositories__', - selectedRepositories: [repoA.fullName, repoB.fullName], + selectedRepositories: [repoB.fullName, repoA.fullName], }); }); @@ -116,7 +116,6 @@ describe('resolveRepositorySelectionByIds', () => { expect(result.normalizedRepositoryIds).toEqual([repoA.id]); expect(result.selectedRepositories).toEqual([ { id: repoA.id, fullName: repoA.fullName }, - { id: repoA.id, fullName: repoA.fullName }, ]); expect(result.workspacePayload).toEqual({ repo: repoA.fullName }); }); diff --git a/packages/db/src/lib/__tests__/source-control-provider.test.ts b/packages/db/src/lib/__tests__/source-control-provider.test.ts index 84b26fb59..3da2ceed5 100644 --- a/packages/db/src/lib/__tests__/source-control-provider.test.ts +++ b/packages/db/src/lib/__tests__/source-control-provider.test.ts @@ -1,25 +1,71 @@ // pnpm --filter @roomote/db exec vitest run src/lib/__tests__/source-control-provider.test.ts import type { DatabaseOrTransaction } from '../../db'; -import { resolveWorkspaceSourceControlProvider } from '../source-control-provider'; +import { + resolveWorkspaceRepositoryProviders, + resolveWorkspaceSourceControlProvider, +} from '../source-control-provider'; const mockWhere = vi.fn(); +const mockOrderBy = vi.fn(); +let mockRows: Array<{ + fullName: string; + host: string | null; + isActive?: boolean; + sourceControlProvider: 'github' | 'gitlab' | 'gitea' | 'ado' | 'bitbucket'; +}> = []; +let mockEnvironmentRepositories: string[] = []; + +const query = { + innerJoin: vi.fn(() => query), + where: vi.fn((...args: unknown[]) => { + mockWhere(...args); + return query; + }), + orderBy: vi.fn((...args: unknown[]) => { + mockOrderBy(...args); + return Promise.resolve(mockRows); + }), + then: ( + resolve: (value: typeof mockRows) => unknown, + reject: (reason: unknown) => unknown, + ) => Promise.resolve(mockRows).then(resolve, reject), +}; const dbOrTx = { select: vi.fn(() => ({ - from: vi.fn(() => ({ - innerJoin: vi.fn(() => ({ where: mockWhere })), - where: mockWhere, - })), + from: vi.fn(() => query), })), + query: { + environments: { + findFirst: vi.fn(async () => ({ + config: { + name: 'Test environment', + repositories: mockEnvironmentRepositories.map((repository) => ({ + repository, + })), + }, + })), + }, + }, } as unknown as DatabaseOrTransaction; describe('resolveWorkspaceSourceControlProvider', () => { beforeEach(() => { + mockRows = []; + mockEnvironmentRepositories = []; mockWhere.mockReset(); + mockOrderBy.mockReset(); }); it('resolves the provider from an environment with a single-provider mapping', async () => { - mockWhere.mockResolvedValue([{ sourceControlProvider: 'ado' }]); + mockEnvironmentRepositories = ['acme/Platform/backend']; + mockRows = [ + { + fullName: 'acme/Platform/backend', + host: 'dev.azure.com', + sourceControlProvider: 'ado', + }, + ]; await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { @@ -29,8 +75,40 @@ describe('resolveWorkspaceSourceControlProvider', () => { ).resolves.toBe('ado'); }); + it('orders environment providers by the declared repository config', async () => { + mockEnvironmentRepositories = ['group/web', 'octo/api']; + mockRows = [ + { + fullName: 'octo/api', + host: 'github.com', + sourceControlProvider: 'github', + }, + { + fullName: 'group/web', + host: 'gitlab.com', + sourceControlProvider: 'gitlab', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'environment', + environmentId: 'env-1', + }), + ).resolves.toEqual({ + 'group/web': 'gitlab', + 'octo/api': 'github', + }); + }); + it('resolves the provider from a single repository workspace', async () => { - mockWhere.mockResolvedValue([{ sourceControlProvider: 'gitlab' }]); + mockRows = [ + { + fullName: 'group/subgroup/repo', + host: 'gitlab.com', + sourceControlProvider: 'gitlab', + }, + ]; await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { @@ -41,10 +119,18 @@ describe('resolveWorkspaceSourceControlProvider', () => { }); it('resolves the provider from a repository set sharing one provider', async () => { - mockWhere.mockResolvedValue([ - { sourceControlProvider: 'ado' }, - { sourceControlProvider: 'ado' }, - ]); + mockRows = [ + { + fullName: 'acme/Platform/frontend', + host: 'dev.azure.com', + sourceControlProvider: 'ado', + }, + { + fullName: 'acme/Platform/backend', + host: 'dev.azure.com', + sourceControlProvider: 'ado', + }, + ]; await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { @@ -55,10 +141,18 @@ describe('resolveWorkspaceSourceControlProvider', () => { }); it('returns undefined when the workspace spans multiple providers', async () => { - mockWhere.mockResolvedValue([ - { sourceControlProvider: 'ado' }, - { sourceControlProvider: 'github' }, - ]); + mockRows = [ + { + fullName: 'acme/Platform/backend', + host: 'dev.azure.com', + sourceControlProvider: 'ado', + }, + { + fullName: 'octo/web', + host: 'github.com', + sourceControlProvider: 'github', + }, + ]; await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { @@ -68,8 +162,6 @@ describe('resolveWorkspaceSourceControlProvider', () => { }); it('returns undefined when no repository rows match', async () => { - mockWhere.mockResolvedValue([]); - await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { type: 'repository', @@ -87,4 +179,121 @@ describe('resolveWorkspaceSourceControlProvider', () => { ).resolves.toBeUndefined(); expect(mockWhere).not.toHaveBeenCalled(); }); + + it('returns a mixed-provider map in repository workspace order', async () => { + mockRows = [ + { + fullName: 'group/web', + host: 'gitlab.com', + sourceControlProvider: 'gitlab', + }, + { + fullName: 'octo/api', + host: 'github.com', + sourceControlProvider: 'github', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'repository_set', + repositories: ['octo/api', 'group/web'], + }), + ).resolves.toEqual({ + 'octo/api': 'github', + 'group/web': 'gitlab', + }); + }); + + it('uses sourceControlHost to disambiguate same-name repository rows', async () => { + mockRows = [ + { + fullName: 'group/project', + host: 'gitlab.alpha.example', + sourceControlProvider: 'gitlab', + }, + { + fullName: 'group/project', + host: 'git.example.com', + sourceControlProvider: 'gitea', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'repository', + repo: 'group/project', + sourceControlHost: 'git.example.com', + }), + ).resolves.toEqual({ 'group/project': 'gitea' }); + }); + + it('prefers active rows over stale inactive rows with the same name', async () => { + mockRows = [ + { + fullName: 'group/project', + host: 'gitlab.example.com', + isActive: false, + sourceControlProvider: 'gitlab', + }, + { + fullName: 'group/project', + host: 'github.com', + isActive: true, + sourceControlProvider: 'github', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'repository', + repo: 'group/project', + }), + ).resolves.toEqual({ 'group/project': 'github' }); + }); + + it('falls back to inactive rows when no active row matches', async () => { + mockRows = [ + { + fullName: 'group/project', + host: 'gitlab.example.com', + isActive: false, + sourceControlProvider: 'gitlab', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'repository', + repo: 'group/project', + }), + ).resolves.toEqual({ 'group/project': 'gitlab' }); + }); + + it('omits and logs ambiguous same-name repository rows', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + mockRows = [ + { + fullName: 'group/project', + host: 'gitlab.alpha.example', + sourceControlProvider: 'gitlab', + }, + { + fullName: 'group/project', + host: 'gitlab.beta.example', + sourceControlProvider: 'gitlab', + }, + ]; + + await expect( + resolveWorkspaceRepositoryProviders(dbOrTx, { + type: 'repository', + repo: 'group/project', + }), + ).resolves.toEqual({}); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('Omitting ambiguous repository group/project'), + ); + warn.mockRestore(); + }); }); diff --git a/packages/db/src/lib/repositories.ts b/packages/db/src/lib/repositories.ts index 60d09b45a..437eeaab2 100644 --- a/packages/db/src/lib/repositories.ts +++ b/packages/db/src/lib/repositories.ts @@ -30,18 +30,19 @@ export async function resolveRepositorySelectionByIds(params: { } const executor = params.executor ?? db; + const repositoryIds = [...new Set(params.repositoryIds)]; const rows = await executor .select({ id: repositories.id, fullName: repositories.fullName, }) .from(repositories) - .where(inArray(repositories.id, params.repositoryIds)); + .where(inArray(repositories.id, repositoryIds)); const rowsById = new Map( rows.map((repository) => [repository.id, repository]), ); - const selectedRepositories = params.repositoryIds + const selectedRepositories = repositoryIds .map((repositoryId) => rowsById.get(repositoryId)) .filter( ( diff --git a/packages/db/src/lib/source-control-provider.ts b/packages/db/src/lib/source-control-provider.ts index 73e256356..9a1e22eb1 100644 --- a/packages/db/src/lib/source-control-provider.ts +++ b/packages/db/src/lib/source-control-provider.ts @@ -1,8 +1,12 @@ -import { and, eq, inArray } from 'drizzle-orm'; +import { and, asc, eq, inArray } from 'drizzle-orm'; import type { TaskWorkspace, SourceControlProvider } from '@roomote/types'; import type { DatabaseOrTransaction } from '../db'; -import { environmentRepositoryMappings, repositories } from '../schema'; +import { + environmentRepositoryMappings, + environments, + repositories, +} from '../schema'; /** * Collapse a set of repository providers to the single provider they all @@ -17,28 +21,95 @@ function toSingleProvider( return unique.length === 1 ? unique[0] : undefined; } -async function resolveProviderByFullNames( +type RepositoryProviderRow = { + fullName: string; + host: string | null; + isActive?: boolean; + sourceControlProvider: SourceControlProvider; +}; + +function toRepositoryProviderMap( + rows: RepositoryProviderRow[], + repositoryOrder: string[], + sourceControlHost?: string, +): Record { + const rowsByFullName = new Map(); + + for (const row of rows) { + const matches = rowsByFullName.get(row.fullName) ?? []; + matches.push(row); + rowsByFullName.set(row.fullName, matches); + } + + const result: Record = {}; + + for (const fullName of [...new Set(repositoryOrder)]) { + const matches = rowsByFullName.get(fullName) ?? []; + const activeMatches = matches.filter((row) => row.isActive === true); + const candidates = activeMatches.length > 0 ? activeMatches : matches; + const hostMatches = + candidates.length > 1 && sourceControlHost + ? candidates.filter((row) => row.host === sourceControlHost) + : candidates; + + if (candidates.length > 1 && hostMatches.length !== 1) { + console.warn( + `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${candidates.length} candidate rows.`, + ); + continue; + } + + const match = hostMatches[0]; + if (match) { + result[fullName] = match.sourceControlProvider; + } + } + + return result; +} + +async function resolveProvidersByFullNames( dbOrTx: DatabaseOrTransaction, fullNames: string[], -): Promise { + sourceControlHost?: string, +): Promise> { if (fullNames.length === 0) { - return undefined; + return {}; } const rows = await dbOrTx - .select({ sourceControlProvider: repositories.sourceControlProvider }) + .select({ + fullName: repositories.fullName, + host: repositories.host, + isActive: repositories.isActive, + sourceControlProvider: repositories.sourceControlProvider, + }) .from(repositories) .where(inArray(repositories.fullName, fullNames)); - return toSingleProvider(rows.map((row) => row.sourceControlProvider)); + return toRepositoryProviderMap(rows, fullNames, sourceControlHost); } -async function resolveEnvironmentProvider( +async function resolveEnvironmentProviders( dbOrTx: DatabaseOrTransaction, environmentId: string, -): Promise { +): Promise> { + const environment = await dbOrTx.query.environments.findFirst({ + where: eq(environments.id, environmentId), + columns: { config: true }, + }); + + if (!environment) { + return {}; + } + const rows = await dbOrTx - .select({ sourceControlProvider: repositories.sourceControlProvider }) + .select({ + fullName: repositories.fullName, + host: repositories.host, + isActive: repositories.isActive, + sourceControlProvider: repositories.sourceControlProvider, + }) .from(environmentRepositoryMappings) .innerJoin( repositories, @@ -49,20 +120,61 @@ async function resolveEnvironmentProvider( eq(environmentRepositoryMappings.environmentId, environmentId), eq(repositories.isActive, true), ), + ) + .orderBy( + asc(environmentRepositoryMappings.createdAt), + asc(environmentRepositoryMappings.id), ); - return toSingleProvider(rows.map((row) => row.sourceControlProvider)); + return toRepositoryProviderMap( + rows, + environment.config.repositories.map((repository) => repository.repository), + ); } -async function resolveAllRepositoriesProvider( +async function resolveAllRepositoriesProviders( dbOrTx: DatabaseOrTransaction, -): Promise { +): Promise> { const rows = await dbOrTx - .select({ sourceControlProvider: repositories.sourceControlProvider }) + .select({ + fullName: repositories.fullName, + host: repositories.host, + isActive: repositories.isActive, + sourceControlProvider: repositories.sourceControlProvider, + }) .from(repositories) - .where(eq(repositories.isActive, true)); + .where(eq(repositories.isActive, true)) + .orderBy(asc(repositories.createdAt), asc(repositories.id)); - return toSingleProvider(rows.map((row) => row.sourceControlProvider)); + return toRepositoryProviderMap( + rows, + rows.map((row) => row.fullName), + ); +} + +/** Resolve repository full names to providers in workspace order. */ +export async function resolveWorkspaceRepositoryProviders( + dbOrTx: DatabaseOrTransaction, + workspace: TaskWorkspace, +): Promise> { + switch (workspace.type) { + case 'repository': + return resolveProvidersByFullNames( + dbOrTx, + [workspace.repo], + workspace.sourceControlHost, + ); + case 'repository_set': + return resolveProvidersByFullNames( + dbOrTx, + workspace.repositories, + workspace.sourceControlHost, + ); + case 'environment': + return resolveEnvironmentProviders(dbOrTx, workspace.environmentId); + case 'all_repositories': + return resolveAllRepositoriesProviders(dbOrTx); + } } /** @@ -71,23 +183,17 @@ async function resolveAllRepositoriesProvider( * every workspace shape (single repo, repo set, environment, all repositories). * * Returns `undefined` when the provider is ambiguous (spans multiple providers) - * or unknown (no matching repositories). This never throws — an unresolved - * provider means the caller should leave the payload unstamped and let the - * downstream GitHub default apply. The web launch-validation path wraps this - * resolver to add its own throw-on-multi-provider behavior. + * or unknown (no matching repositories). This never throws — callers that + * require a resolved provider validate the returned repository map before + * enqueue, while legacy callers may leave the scalar provider unstamped. */ export async function resolveWorkspaceSourceControlProvider( dbOrTx: DatabaseOrTransaction, workspace: TaskWorkspace, ): Promise { - switch (workspace.type) { - case 'repository': - return resolveProviderByFullNames(dbOrTx, [workspace.repo]); - case 'repository_set': - return resolveProviderByFullNames(dbOrTx, workspace.repositories); - case 'environment': - return resolveEnvironmentProvider(dbOrTx, workspace.environmentId); - case 'all_repositories': - return resolveAllRepositoriesProvider(dbOrTx); - } + const repositoryProviders = await resolveWorkspaceRepositoryProviders( + dbOrTx, + workspace, + ); + return toSingleProvider(Object.values(repositoryProviders)); } diff --git a/packages/gitea/src/api.ts b/packages/gitea/src/api.ts index 31410af03..3d462bc8a 100644 --- a/packages/gitea/src/api.ts +++ b/packages/gitea/src/api.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { ALL_REPOSITORIES, buildRepositoryCloneUrl, + filterRepositoryNamesForSourceControlProvider, type SourceControlProvider, } from '@roomote/types'; import { @@ -481,6 +482,17 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { return [...new Set(repositoryNames.filter(Boolean))]; } +function filterRepositorySelectionForGitea( + taskRun: TaskRun, + repositoryNames: string[], +): string[] { + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + GITEA_PROVIDER, + ); +} + async function resolveGiteaRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { @@ -495,9 +507,12 @@ async function resolveGiteaRepositoryNamesForTaskRun( ); } - return normalizeRepositorySelection( - environment.config.repositories.map( - (repository) => repository.repository, + return filterRepositorySelectionForGitea( + taskRun, + normalizeRepositorySelection( + environment.config.repositories.map( + (repository) => repository.repository, + ), ), ); } @@ -508,7 +523,7 @@ async function resolveGiteaRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterRepositorySelectionForGitea(taskRun, selectedRepositories); } } diff --git a/packages/github/src/__tests__/task-run-token.test.ts b/packages/github/src/__tests__/task-run-token.test.ts index a9cc2647f..45a12225f 100644 --- a/packages/github/src/__tests__/task-run-token.test.ts +++ b/packages/github/src/__tests__/task-run-token.test.ts @@ -42,6 +42,7 @@ vi.mock('@roomote/db/server', () => ({ repositories: { fullName: 'repositories.fullName', isActive: 'repositories.isActive', + sourceControlProvider: 'repositories.sourceControlProvider', }, })); @@ -123,6 +124,62 @@ describe('createTaskRunGitHubToken', () => { }); }); + it('ignores selected repositories mapped to another provider', async () => { + mockFindMany.mockResolvedValue([ + { + fullName: 'ExampleOrg/example-backend', + installationId: 'install-exampleorg', + githubRepoId: 101, + }, + ]); + + await expect( + createTaskRunGitHubToken( + buildTaskRun({ + repo: 'ExampleOrg/example-backend', + selectedRepositories: ['ExampleOrg/example-backend', 'group/project'], + repositoryProviders: { + 'ExampleOrg/example-backend': 'github', + 'group/project': 'gitlab', + }, + } as TaskRun['payload']), + ), + ).resolves.toBe('ghs_test_token'); + + expect(mockCreateGitHubToken).toHaveBeenCalledWith({ + type: 'installationId', + installationId: 'install-exampleorg', + repositoryIds: [101], + }); + }); + + it('ignores selected repository names omitted from a provider map', async () => { + mockFindMany.mockResolvedValue([ + { + fullName: 'ExampleOrg/example-backend', + installationId: 'install-exampleorg', + githubRepoId: 101, + }, + ]); + + await expect( + createTaskRunGitHubToken( + buildTaskRun({ + repo: '__all_repositories__', + selectedRepositories: [ + 'ExampleOrg/example-backend', + 'group/project', + 'unknown/repository', + ], + repositoryProviders: { + 'ExampleOrg/example-backend': 'github', + 'group/project': 'gitlab', + }, + } as TaskRun['payload']), + ), + ).resolves.toBe('ghs_test_token'); + }); + it('uses the environment repositories installation for environment tasks', async () => { mockFindEnvironmentFirst.mockResolvedValue({ id: '14f1f7c4-b126-4b3f-a6a8-e37f7d299f4d', diff --git a/packages/github/src/api.ts b/packages/github/src/api.ts index e4f4ed76c..8b286d643 100644 --- a/packages/github/src/api.ts +++ b/packages/github/src/api.ts @@ -7,6 +7,7 @@ import pMap from 'p-map'; import { createGitHubToken, resolveGitHubAppCredentials } from '@roomote/auth'; import { DEFAULT_SOURCE_CONTROL_PROVIDER, + filterRepositoryNamesForSourceControlProvider, normalizePemEnvValue, } from '@roomote/types'; import { @@ -43,7 +44,15 @@ async function createTokenForRepositoryNames({ missingMessagePrefix: string; spanningMessagePrefix: string; }): Promise { - const uniqueRepositoryNames = [...new Set(repositoryNames.filter(Boolean))]; + const uniqueRepositoryNames = [ + ...new Set( + filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames.filter(Boolean), + DEFAULT_SOURCE_CONTROL_PROVIDER, + ), + ), + ]; const selectedRepoRows = await db.query.repositories.findMany({ where: and( diff --git a/packages/gitlab/src/__tests__/api.test.ts b/packages/gitlab/src/__tests__/api.test.ts index 454ae20a4..4f6617f88 100644 --- a/packages/gitlab/src/__tests__/api.test.ts +++ b/packages/gitlab/src/__tests__/api.test.ts @@ -530,6 +530,96 @@ describe('createTaskRunScopedGitLabTokens', () => { ); }); + it('ignores selected repositories mapped to another provider', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + id: 999, + token: 'glptt_repo_scoped', + username: 'oauth2', + }), + { status: 201 }, + ), + ); + + const result = await createTaskRunScopedGitLabTokens( + makeTaskRun({ + repo: 'group/project', + selectedRepositories: ['group/project', 'ExampleOrg/example-backend'], + repositoryProviders: { + 'group/project': 'gitlab', + 'ExampleOrg/example-backend': 'github', + }, + description: 'Work across providers', + } as TaskRun['payload']), + { fetchImpl: fetchMock }, + ); + + expect(result.credentials).toHaveLength(1); + expect(result.credentials[0]?.repositoryFullName).toBe('group/project'); + }); + + it('uses mapped GitLab repositories as the scope for all-repositories tasks', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + id: 999, + token: 'glptt_repo_scoped', + username: 'oauth2', + }), + { status: 201 }, + ), + ); + + const result = await createTaskRunScopedGitLabTokens( + makeTaskRun({ + repo: '__all_repositories__', + sourceControlProvider: 'github', + repositoryProviders: { + 'group/project': 'gitlab', + 'ExampleOrg/example-backend': 'github', + }, + description: 'Work across all mapped repositories', + } as TaskRun['payload']), + { fetchImpl: fetchMock }, + ); + + expect(result.credentials).toHaveLength(1); + expect(result.credentials[0]?.repositoryFullName).toBe('group/project'); + }); + + it('ignores selected repository names omitted from a provider map', async () => { + const fetchMock = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + id: 999, + token: 'glptt_repo_scoped', + username: 'oauth2', + }), + { status: 201 }, + ), + ); + const result = await createTaskRunScopedGitLabTokens( + makeTaskRun({ + repo: 'group/project', + selectedRepositories: [ + 'group/project', + 'ExampleOrg/example-backend', + 'unknown/repository', + ], + repositoryProviders: { + 'group/project': 'gitlab', + 'ExampleOrg/example-backend': 'github', + }, + description: 'Work across providers', + } as TaskRun['payload']), + { fetchImpl: fetchMock }, + ); + + expect(result.credentials).toHaveLength(1); + expect(result.credentials[0]?.repositoryFullName).toBe('group/project'); + }); + it('mints scoped tokens against a self-managed GITLAB_BASE_URL with the self-managed credential host', async () => { process.env.GITLAB_BASE_URL = 'https://gitlab.example.com'; diff --git a/packages/gitlab/src/api.ts b/packages/gitlab/src/api.ts index f35f6ad3d..08e9d8638 100644 --- a/packages/gitlab/src/api.ts +++ b/packages/gitlab/src/api.ts @@ -3,6 +3,8 @@ import { z } from 'zod'; import { ALL_REPOSITORIES, buildRepositoryCloneUrl, + filterRepositoryNamesForSourceControlProvider, + resolveRepositoryProvidersFromPayload, type SourceControlProvider, } from '@roomote/types'; import { @@ -913,6 +915,17 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { return [...new Set(repositoryNames.filter(Boolean))]; } +function filterRepositorySelectionForGitLab( + taskRun: TaskRun, + repositoryNames: string[], +): string[] { + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + GITLAB_PROVIDER, + ); +} + async function resolveGitLabRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { @@ -927,9 +940,12 @@ async function resolveGitLabRepositoryNamesForTaskRun( ); } - return normalizeRepositorySelection( - environment.config.repositories.map( - (repository) => repository.repository, + return filterRepositorySelectionForGitLab( + taskRun, + normalizeRepositorySelection( + environment.config.repositories.map( + (repository) => repository.repository, + ), ), ); } @@ -940,7 +956,7 @@ async function resolveGitLabRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterRepositorySelectionForGitLab(taskRun, selectedRepositories); } } @@ -948,6 +964,19 @@ async function resolveGitLabRepositoryNamesForTaskRun( return [taskRun.payload.repo]; } + const repositoryProviders = resolveRepositoryProvidersFromPayload( + taskRun.payload, + ); + if (repositoryProviders) { + const mappedRepositories = Object.entries(repositoryProviders) + .filter(([, provider]) => provider === GITLAB_PROVIDER) + .map(([repositoryName]) => repositoryName); + + if (mappedRepositories.length > 0) { + return mappedRepositories; + } + } + throw new Error( `GitLab source control jobs require an explicit repository scope for task run ${taskRun.id}.`, ); diff --git a/packages/sdk/src/server/index.ts b/packages/sdk/src/server/index.ts index 1fba1362a..1c84a7056 100644 --- a/packages/sdk/src/server/index.ts +++ b/packages/sdk/src/server/index.ts @@ -255,6 +255,10 @@ export { type SourceControlPullRequestMutationInput, type SourceControlPullRequestMutationResult, } from './lib/pull-requests/source-control-pull-requests'; +export { + getPayloadRecord, + resolveSourceControlProviderForRepositoryFromPayload, +} from './lib/pull-requests/source-control-pull-request-shared'; export { readSourceControlPullRequestForTaskRun, sourceControlPullRequestReadInputSchema, diff --git a/packages/sdk/src/server/lib/issues/__tests__/source-control-issues.test.ts b/packages/sdk/src/server/lib/issues/__tests__/source-control-issues.test.ts index ec52a4843..8538331d7 100644 --- a/packages/sdk/src/server/lib/issues/__tests__/source-control-issues.test.ts +++ b/packages/sdk/src/server/lib/issues/__tests__/source-control-issues.test.ts @@ -147,6 +147,61 @@ describe('manageSourceControlIssueForTaskRun', () => { }); }); + it('routes a GitLab issue in a GitHub-primary mixed task', async () => { + mockResolveRepositoryRow.mockResolvedValue({ + id: 'repo-1', + sourceControlProvider: 'gitlab', + host: null, + installationId: null, + externalRepoId: '123', + fullName: 'acme/backend', + htmlUrl: 'https://gitlab.com/acme/backend', + }); + mockResolveGitLabProviderContext.mockResolvedValue({ + projectId: '123', + token: 'server-side-token', + apiBaseUrl: 'https://gitlab.com/api/v4', + }); + const fetchImpl = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + iid: 9, + title: 'Broken checkout', + state: 'opened', + web_url: 'https://gitlab.com/acme/backend/-/issues/9', + author: { username: 'alice' }, + labels: [], + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ), + ); + const mixedTaskRun = taskRun('github'); + mixedTaskRun.payload = { + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + sourceControlHost: 'github.com', + repositoryProviders: { 'acme/backend': 'gitlab' }, + } as unknown as TaskRun['payload']; + + const result = await manageSourceControlIssueForTaskRun({ + taskRun: mixedTaskRun, + input: { + action: 'get_issue', + repositoryFullName: 'acme/backend', + issueNumber: 9, + }, + fetchImpl, + }); + + expect(mockResolveRepositoryRow).toHaveBeenCalledWith({ + provider: 'gitlab', + repositoryFullName: 'acme/backend', + host: undefined, + }); + expect(result).toMatchObject({ provider: 'gitlab', number: 9 }); + }); + it('posts a Gitea issue comment with server-resolved credentials', async () => { mockResolveRepositoryRow.mockResolvedValue({ id: 'repo-2', diff --git a/packages/sdk/src/server/lib/issues/source-control-issues.ts b/packages/sdk/src/server/lib/issues/source-control-issues.ts index 86a3322c1..a3679a1de 100644 --- a/packages/sdk/src/server/lib/issues/source-control-issues.ts +++ b/packages/sdk/src/server/lib/issues/source-control-issues.ts @@ -1,14 +1,12 @@ import { type TaskRun } from '@roomote/db/server'; -import { - getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, -} from '@roomote/types'; +import { getSourceControlProviderLabel } from '@roomote/types'; import { assertRepositoryInTaskRunScope, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, + resolveSourceControlProviderForRepositoryFromPayload, type FetchImpl, } from '../pull-requests/source-control-pull-request-shared'; import { getIssueProviderOperations } from './source-control-issue-providers'; @@ -39,7 +37,10 @@ export async function manageSourceControlIssueForTaskRun({ fetchImpl?: FetchImpl; }): Promise { const payload = getPayloadRecord(taskRun.payload); - const payloadProvider = resolveSourceControlProviderFromPayload(payload); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payload, + input.repositoryFullName, + ); const provider = input.sourceControlProvider ?? payloadProvider; if (provider !== payloadProvider) { @@ -63,7 +64,10 @@ export async function manageSourceControlIssueForTaskRun({ const repository = await resolveRepositoryRow({ provider, repositoryFullName: input.repositoryFullName, - host: resolveSourceControlHostFromPayload(payload), + host: resolveSourceControlHostForRepositoryFromPayload( + payload, + input.repositoryFullName, + ), }); const ops = getIssueProviderOperations(provider); diff --git a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-reads.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-reads.test.ts index c00fe2064..a65884519 100644 --- a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-reads.test.ts +++ b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-reads.test.ts @@ -223,7 +223,7 @@ describe('readSourceControlPullRequestForTaskRun', () => { }); }); - it('reads GitLab merge request details through the deployment token', async () => { + it('reads a GitLab merge request in a GitHub-primary mixed task', async () => { mockRepositoriesFindFirst.mockResolvedValue({ installationId: null, externalRepoId: '101', @@ -253,14 +253,15 @@ describe('readSourceControlPullRequestForTaskRun', () => { const result = await readSourceControlPullRequestForTaskRun({ taskRun: makeTaskRun({ - repo: 'acme/backend', - sourceControlProvider: 'gitlab', - }), + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + } as unknown as TaskRun['payload']), input: { action: 'get_pull_request', repositoryFullName: 'acme/backend', prNumber: 42, - sourceControlProvider: 'gitlab', }, fetchImpl, }); @@ -1163,15 +1164,17 @@ describe('readSourceControlPullRequestForTaskRun', () => { ).rejects.toThrow('prNumber is required for get_pull_request.'); }); - it('rejects reads whose provider does not match the task payload', async () => { + it('rejects an explicit provider that conflicts with the repository map', async () => { const fetchImpl = vi.fn(); await expect( readSourceControlPullRequestForTaskRun({ taskRun: makeTaskRun({ - repo: 'acme/backend', - sourceControlProvider: 'gitlab', - }), + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + } as unknown as TaskRun['payload']), input: { action: 'get_pull_request', repositoryFullName: 'acme/backend', diff --git a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-shared.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-shared.test.ts index 0a75ad389..90142b817 100644 --- a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-shared.test.ts +++ b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-shared.test.ts @@ -4,7 +4,82 @@ vi.mock('@roomote/gitlab', () => ({ isGitLabOAuthAccessToken: (token: string) => token === 'oauth-token', })); -import { buildGitLabTokenHeader } from '../source-control-pull-request-shared'; +import { + buildGitLabTokenHeader, + resolveSourceControlHostForRepositoryFromPayload, + resolveSourceControlProviderForRepositoryFromPayload, +} from '../source-control-pull-request-shared'; + +describe('resolveSourceControlProviderForRepositoryFromPayload', () => { + it('prefers the target repository provider over the task primary provider', () => { + expect( + resolveSourceControlProviderForRepositoryFromPayload( + { + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + 'acme/backend', + ), + ).toBe('gitlab'); + }); + + it('rejects repositories omitted from a provider map', () => { + expect(() => + resolveSourceControlProviderForRepositoryFromPayload( + { + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + 'acme/frontend', + ), + ).toThrow( + 'Repository acme/frontend is not mapped to a source control provider.', + ); + }); +}); + +describe('resolveSourceControlHostForRepositoryFromPayload', () => { + it('does not apply the primary provider host to a mapped secondary repository', () => { + expect( + resolveSourceControlHostForRepositoryFromPayload( + { + sourceControlProvider: 'github', + sourceControlHost: 'github.com', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + 'acme/backend', + ), + ).toBeUndefined(); + }); + + it('keeps the scalar host for legacy payloads without a provider map', () => { + expect( + resolveSourceControlHostForRepositoryFromPayload( + { + sourceControlProvider: 'github', + sourceControlHost: 'github.com', + }, + 'acme/frontend', + ), + ).toBe('github.com'); + }); + + it('does not apply a scalar host to a mapped primary repository', () => { + expect( + resolveSourceControlHostForRepositoryFromPayload( + { + sourceControlProvider: 'github', + sourceControlHost: 'github.enterprise.example', + repositoryProviders: { + 'acme/frontend': 'github', + 'acme/backend': 'gitlab', + }, + }, + 'acme/frontend', + ), + ).toBeUndefined(); + }); +}); describe('buildGitLabTokenHeader', () => { it('uses the Bearer authorization header for OAuth tokens', () => { diff --git a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-writes.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-writes.test.ts index b4c1d9766..5884c1ec7 100644 --- a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-writes.test.ts +++ b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-writes.test.ts @@ -128,7 +128,7 @@ describe('writeSourceControlPullRequestForTaskRun', () => { ); }); - it('replies to a GitLab discussion through the deployment token', async () => { + it('replies to a GitLab discussion in a GitHub-primary mixed task', async () => { mockRepositoriesFindFirst.mockResolvedValue({ installationId: null, externalRepoId: '101', @@ -141,16 +141,17 @@ describe('writeSourceControlPullRequestForTaskRun', () => { const result = await writeSourceControlPullRequestForTaskRun({ taskRun: makeTaskRun({ - repo: 'acme/backend', - sourceControlProvider: 'gitlab', - }), + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + } as unknown as TaskRun['payload']), input: { action: 'reply_to_pull_request_comment', repositoryFullName: 'acme/backend', prNumber: 42, threadId: 'abc123', body: 'Thanks, fixed.', - sourceControlProvider: 'gitlab', }, fetchImpl, }); diff --git a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts index 9ed53fef6..765008932 100644 --- a/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts +++ b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-requests.test.ts @@ -192,7 +192,7 @@ describe('createOrUpdateSourceControlPullRequestForTaskRun', () => { ); }); - it('creates a GitLab merge request through the deployment token', async () => { + it('creates a GitLab merge request in a GitHub-primary mixed task', async () => { mockGetDeploymentPrAction.mockResolvedValue('create'); mockRepositoriesFindFirst.mockResolvedValue({ installationId: null, @@ -214,9 +214,11 @@ describe('createOrUpdateSourceControlPullRequestForTaskRun', () => { const result = await createOrUpdateSourceControlPullRequestForTaskRun({ taskRun: makeTaskRun({ - repo: 'acme/backend', - sourceControlProvider: 'gitlab', - }), + repo: 'acme/frontend', + selectedRepositories: ['acme/frontend', 'acme/backend'], + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + } as unknown as TaskRun['payload']), input: { action: 'create_or_update_pull_request', repositoryFullName: 'acme/backend', @@ -226,7 +228,6 @@ describe('createOrUpdateSourceControlPullRequestForTaskRun', () => { body: 'Body', labels: ['roomote'], assignees: [], - sourceControlProvider: 'gitlab', }, fetchImpl, }); diff --git a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-reads.ts b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-reads.ts index 0c6e3500c..ec71d3442 100644 --- a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-reads.ts +++ b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-reads.ts @@ -4,8 +4,6 @@ import { type TaskRun } from '@roomote/db/server'; import { buildPullRequestUrl, getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -26,6 +24,8 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -609,9 +609,14 @@ export async function readSourceControlPullRequestForTaskRun({ fetchImpl?: FetchImpl; }): Promise { const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); + const payloadHost = resolveSourceControlHostForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const provider = input.sourceControlProvider ?? payloadProvider; if (provider !== payloadProvider) { diff --git a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-shared.ts b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-shared.ts index 3c311281a..3d17ee81a 100644 --- a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-shared.ts +++ b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-shared.ts @@ -10,6 +10,10 @@ import { ALL_REPOSITORIES, environmentConfigSchema, getSourceControlProviderLabel, + normalizeSourceControlProvider, + resolveRepositoryProvidersFromPayload, + resolveSourceControlHostFromPayload, + resolveSourceControlProviderFromPayload, type SourceControlProvider, } from '@roomote/types'; import { isGitLabOAuthAccessToken } from '@roomote/gitlab'; @@ -35,6 +39,49 @@ export type RepositoryRow = { htmlUrl: string; }; +export function resolveSourceControlProviderForRepositoryFromPayload( + payload: Record, + repositoryFullName: string, +): SourceControlProvider { + const repositoryProviders = resolveRepositoryProvidersFromPayload(payload); + + if (repositoryProviders) { + const repositoryProvider = repositoryProviders[repositoryFullName]; + + if (repositoryProvider === undefined) { + throw new Error( + `Repository ${repositoryFullName} is not mapped to a source control provider.`, + ); + } + + return normalizeSourceControlProvider(repositoryProvider); + } + + return resolveSourceControlProviderFromPayload(payload); +} + +export function resolveSourceControlHostForRepositoryFromPayload( + payload: Record, + repositoryFullName: string, +): string | undefined { + const repositoryProviders = resolveRepositoryProvidersFromPayload(payload); + + if (repositoryProviders?.[repositoryFullName] !== undefined) { + return undefined; + } + + const repositoryProvider = + resolveSourceControlProviderForRepositoryFromPayload( + payload, + repositoryFullName, + ); + const primaryProvider = resolveSourceControlProviderFromPayload(payload); + + return repositoryProvider === primaryProvider + ? resolveSourceControlHostFromPayload(payload) + : undefined; +} + /** * Shared provider-resolution and name/url plumbing for the provider-neutral * source-control pull-request surface. HTTP transport lives in diff --git a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-writes.ts b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-writes.ts index ec93eac48..29059b0d7 100644 --- a/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-writes.ts +++ b/packages/sdk/src/server/lib/pull-requests/source-control-pull-request-writes.ts @@ -3,8 +3,6 @@ import { getOctokit } from '@roomote/github'; import { type TaskRun } from '@roomote/db/server'; import { getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -28,6 +26,8 @@ import { formatResponseBody, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -233,9 +233,14 @@ export async function writeSourceControlPullRequestForTaskRun({ assertWriteInputFields(input); const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); + const payloadHost = resolveSourceControlHostForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const provider = input.sourceControlProvider ?? payloadProvider; if (provider !== payloadProvider) { diff --git a/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts b/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts index 7563e4116..09f16d51b 100644 --- a/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts +++ b/packages/sdk/src/server/lib/pull-requests/source-control-pull-requests.ts @@ -22,8 +22,6 @@ import { getSourceControlProviderLabel, normalizePrBodyAttributionAppMention, prActions, - resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type PrAction, type SourceControlProvider, @@ -57,6 +55,8 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -182,9 +182,14 @@ export async function createOrUpdateSourceControlPullRequestForTaskRun({ fetchImpl?: FetchImpl; }): Promise { const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); + const payloadHost = resolveSourceControlHostForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const provider = input.sourceControlProvider ?? payloadProvider; if (provider !== payloadProvider) { diff --git a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-helpers.test.ts b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-helpers.test.ts index 341fc57f5..6f47819fd 100644 --- a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-helpers.test.ts +++ b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-helpers.test.ts @@ -8,6 +8,7 @@ const { mockCreateTaskRunScopedGitLabTokens, mockCreateTaskRunGiteaCredentials, mockCreateTaskRunAdoCredentials, + mockCreateTaskRunBitbucketCredentials, mockResolveSandboxModelRuntimeEnv, mockTaskRunsFindFirst, mockNotifySourceRunOnSettle, @@ -19,6 +20,7 @@ const { mockCreateTaskRunScopedGitLabTokens: vi.fn(), mockCreateTaskRunGiteaCredentials: vi.fn(), mockCreateTaskRunAdoCredentials: vi.fn(), + mockCreateTaskRunBitbucketCredentials: vi.fn(), mockResolveSandboxModelRuntimeEnv: vi.fn(), mockTaskRunsFindFirst: vi.fn(), mockNotifySourceRunOnSettle: vi.fn(), @@ -86,6 +88,11 @@ vi.mock('@roomote/ado', () => ({ mockCreateTaskRunAdoCredentials(...args), })); +vi.mock('@roomote/bitbucket', () => ({ + createTaskRunBitbucketCredentials: (...args: unknown[]) => + mockCreateTaskRunBitbucketCredentials(...args), +})); + vi.mock('@roomote/cloud-agents/server', () => ({ releaseTaskRun: vi.fn(), })); @@ -175,6 +182,9 @@ describe('createSourceControlTokenForTaskRun', () => { }, ], }); + mockCreateTaskRunBitbucketCredentials.mockResolvedValue({ + credentials: [], + }); }); it('creates GitHub token metadata by default', async () => { @@ -392,6 +402,124 @@ describe('createSourceControlTokenForTaskRun', () => { expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalled(); }); + it('mints the stamped primary provider first and merges aggregate metadata', async () => { + const taskRun = makeTaskRun({ + repo: 'group/project', + selectedRepositories: ['owner/repo', 'group/project'], + sourceControlProvider: 'github', + repositoryProviders: { + 'group/project': 'gitlab', + 'owner/repo': 'github', + }, + description: 'Work across providers', + } as TaskRun['payload']); + + const result = await createSourceControlTokenForTaskRun(taskRun, '[test]', { + maxRetries: 1, + }); + + expect(result).toEqual({ + provider: 'github', + token: 'ghs_app_token', + envVar: 'GH_TOKEN', + envVars: { GH_TOKEN: 'ghs_app_token' }, + gitCredentials: [ + { + host: 'gitlab.com', + repositoryFullName: 'group/project', + username: 'oauth2', + token: 'glptt_scoped_token', + }, + ], + gitProxyCredentials: [], + source: 'app', + expiresAt: null, + artifactsPatch: { + gitlabScopedProjectTokens: [ + { + repositoryFullName: 'group/project', + projectId: '101', + tokenId: 202, + }, + ], + }, + }); + expect( + mockCreateTaskRunWorkerGitHubToken.mock.invocationCallOrder[0], + ).toBeLessThan( + mockCreateTaskRunScopedGitLabTokens.mock.invocationCallOrder[0]!, + ); + }); + + it('retries only the failing provider and returns no partial token', async () => { + mockCreateTaskRunScopedGitLabTokens.mockRejectedValue( + new Error('GitLab unavailable'), + ); + const consoleWarnSpy = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + try { + const result = await createSourceControlTokenForTaskRun( + makeTaskRun({ + repo: 'owner/repo', + repositoryProviders: { + 'owner/repo': 'github', + 'group/project': 'gitlab', + }, + description: 'Work across providers', + } as TaskRun['payload']), + '[test]', + { maxRetries: 2, baseDelayMs: 0 }, + ); + + expect(result).toBeNull(); + expect(mockCreateTaskRunWorkerGitHubToken).toHaveBeenCalledTimes(1); + expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalledTimes(2); + } finally { + consoleWarnSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + } + }); + + it('does not mint GitLab scoped tokens before a later provider succeeds', async () => { + mockCreateTaskRunAdoCredentials.mockRejectedValue( + new Error('Azure DevOps unavailable'), + ); + const consoleWarnSpy = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + try { + const result = await createSourceControlTokenForTaskRun( + makeTaskRun({ + repo: 'group/project', + sourceControlProvider: 'gitlab', + repositoryProviders: { + 'group/project': 'gitlab', + 'acme/Platform/backend': 'ado', + }, + description: 'Work across providers', + } as TaskRun['payload']), + '[test]', + { maxRetries: 2, baseDelayMs: 0 }, + ); + + expect(result).toBeNull(); + expect(mockCreateTaskRunAdoCredentials).toHaveBeenCalledTimes(2); + expect(mockCreateTaskRunScopedGitLabTokens).not.toHaveBeenCalled(); + } finally { + consoleWarnSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + } + }); + it('returns null when GitLab token is missing', async () => { mockCreateTaskRunScopedGitLabTokens.mockRejectedValueOnce( new Error('GITLAB_TOKEN is required for GitLab source control jobs.'), @@ -509,6 +637,20 @@ describe('redactSourceControlProviderEnvVars', () => { }); }); + it('removes the deployment Bitbucket token after credentials are derived', () => { + expect( + redactSourceControlProviderEnvVars( + { + BITBUCKET_OAUTH: 'bitbucket_deployment_token', + OPENAI_API_KEY: 'sk-test', + }, + 'bitbucket', + ), + ).toEqual({ + OPENAI_API_KEY: 'sk-test', + }); + }); + it('leaves unrelated env vars intact for GitHub jobs', () => { const envVars = { GITLAB_TOKEN: 'glpat_deployment_token', @@ -519,6 +661,23 @@ describe('redactSourceControlProviderEnvVars', () => { expect(redactSourceControlProviderEnvVars(envVars, 'github')).toBe(envVars); }); + + it('redacts every non-GitHub deployment token for mixed-provider jobs', () => { + expect( + redactSourceControlProviderEnvVars( + { + GH_TOKEN: 'operator-github-token', + GITLAB_TOKEN: 'glpat_deployment_token', + BITBUCKET_OAUTH: 'bitbucket_deployment_token', + OPENAI_API_KEY: 'sk-test', + }, + ['github', 'gitlab', 'bitbucket'], + ), + ).toEqual({ + GH_TOKEN: 'operator-github-token', + OPENAI_API_KEY: 'sk-test', + }); + }); }); describe('redactControlPlaneEnvVars', () => { diff --git a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-resume-task-run.test.ts b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-resume-task-run.test.ts index d7290f758..b3149a3e9 100644 --- a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-resume-task-run.test.ts +++ b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-resume-task-run.test.ts @@ -24,6 +24,7 @@ const { mockRecordTaskRunLifecycleEvent, mockRecordSnapshotResumeEvent, mockResolveSlackTaskRunRouting, + mockResolveTaskRunSourceControlProviders, onBootstrapFailureMock, } = vi.hoisted(() => ({ mockDbTransaction: vi.fn(), @@ -48,6 +49,7 @@ const { mockRecordTaskRunLifecycleEvent: vi.fn(), mockRecordSnapshotResumeEvent: vi.fn(), mockResolveSlackTaskRunRouting: vi.fn(), + mockResolveTaskRunSourceControlProviders: vi.fn(), onBootstrapFailureMock: vi.fn(), })); @@ -88,6 +90,8 @@ vi.mock('../dequeue-helpers', () => ({ reportBootstrapFailure: (...args: unknown[]) => mockReportBootstrapFailure(...args), resolveGitAuthor: (...args: unknown[]) => mockResolveGitAuthor(...args), + resolveTaskRunSourceControlProviders: (...args: unknown[]) => + mockResolveTaskRunSourceControlProviders(...args), })); vi.mock('../slack-task-run-routing', () => ({ @@ -149,6 +153,7 @@ describe('dequeueResumeTaskRun', () => { mockEq.mockReturnValue('eq-clause'); mockClaimJobById.mockReturnValue('claim-query'); mockFetchEnvVars.mockResolvedValue({ ORG_ENV: '1' }); + mockResolveTaskRunSourceControlProviders.mockResolvedValue(['github']); mockResolveGitAuthor.mockResolvedValue({ name: 'Roomote', email: 'roomote@example.com', @@ -217,6 +222,18 @@ describe('dequeueResumeTaskRun', () => { it("returns the task's harnessSessionId for snapshot resume", async () => { const resumeRun = makeSnapshotResumeRun(); + mockResolveTaskRunSourceControlProviders.mockResolvedValue([ + 'gitlab', + 'github', + ]); + mockCreateSourceControlTokenForTaskRun.mockResolvedValue({ + provider: 'gitlab', + token: 'gl-token', + envVar: 'GITLAB_TOKEN', + envVars: { GH_TOKEN: 'gh-token' }, + source: 'app', + expiresAt: null, + }); mockTxExecute.mockResolvedValue([{ id: resumeRun.id }]); mockTxFindFirstTaskRuns.mockResolvedValueOnce(resumeRun); @@ -225,6 +242,14 @@ describe('dequeueResumeTaskRun', () => { }); expect(result?.harnessSessionId).toBe('session-canonical'); + expect(result?.gitHubToken).toBe('gh-token'); + expect(mockFetchEnvVars).toHaveBeenCalledWith(expect.anything(), { + sourceControlProvider: ['gitlab', 'github'], + }); + expect(mockFetchResolvedRuntimeEnvVars).toHaveBeenCalledWith( + { ORG_ENV: '1' }, + { sourceControlProvider: ['gitlab', 'github'] }, + ); expect(result?.harnessInstructions).toBe('preserved instructions'); expect(result?.sourceSelectedRepositories).toEqual([ 'acme/api', diff --git a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-task-run.test.ts b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-task-run.test.ts index 5f8849341..c294261c3 100644 --- a/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-task-run.test.ts +++ b/packages/sdk/src/server/lib/task-runs/__tests__/dequeue-task-run.test.ts @@ -25,6 +25,7 @@ const { mockRecordTaskRunLifecycleEvent, mockGetRedis, mockResolveSlackTaskRunRouting, + mockResolveTaskRunSourceControlProviders, onBootstrapFailureMock, } = vi.hoisted(() => ({ mockDbTransaction: vi.fn(), @@ -50,6 +51,7 @@ const { mockRecordTaskRunLifecycleEvent: vi.fn(), mockGetRedis: vi.fn(() => 'redis-client'), mockResolveSlackTaskRunRouting: vi.fn(), + mockResolveTaskRunSourceControlProviders: vi.fn(), onBootstrapFailureMock: vi.fn(), })); @@ -99,6 +101,8 @@ vi.mock('../dequeue-helpers', () => ({ reportBootstrapFailure: (...args: unknown[]) => mockReportBootstrapFailure(...args), resolveGitAuthor: (...args: unknown[]) => mockResolveGitAuthor(...args), + resolveTaskRunSourceControlProviders: (...args: unknown[]) => + mockResolveTaskRunSourceControlProviders(...args), })); import { dequeueTaskRun } from '../dequeue-task-run'; @@ -217,6 +221,7 @@ describe('dequeueTaskRun', () => { mockSql.mockReturnValue('claim-query'); mockClaimJobById.mockReturnValue('claim-query'); mockFetchEnvVars.mockResolvedValue({ ORG_ENV: '1' }); + mockResolveTaskRunSourceControlProviders.mockResolvedValue(['github']); mockResolveGitAuthor.mockResolvedValue({ name: 'Roomote', email: 'roomote@example.com', @@ -301,6 +306,18 @@ describe('dequeueTaskRun', () => { it('treats StandardTask jobs without identity metadata as runnable', async () => { const taskRun = makeStandardTaskRun(); + mockResolveTaskRunSourceControlProviders.mockResolvedValue([ + 'gitlab', + 'github', + ]); + mockCreateSourceControlTokenForTaskRun.mockResolvedValue({ + provider: 'gitlab', + token: 'gl-token', + envVar: 'GITLAB_TOKEN', + envVars: { GH_TOKEN: 'gh-token' }, + source: 'app', + expiresAt: null, + }); mockTxExecute.mockResolvedValue([{ id: taskRun.id }]); mockTxFindFirstTaskRuns.mockResolvedValue(taskRun); @@ -319,6 +336,13 @@ describe('dequeueTaskRun', () => { expect(result?.prompt).toBe('prompt'); expect(result?.harnessInstructions).toBe('instructions'); expect(result?.requestedWorkKind).toBe('unknown'); + expect(mockFetchEnvVars).toHaveBeenCalledWith(expect.anything(), { + sourceControlProvider: ['gitlab', 'github'], + }); + expect(mockFetchResolvedRuntimeEnvVars).toHaveBeenCalledWith( + { ORG_ENV: '1' }, + { sourceControlProvider: ['gitlab', 'github'] }, + ); expect(result?.task).toMatchObject({ id: 'task-101', title: 'Task 101', diff --git a/packages/sdk/src/server/lib/task-runs/__tests__/fetch-snapshot-env.test.ts b/packages/sdk/src/server/lib/task-runs/__tests__/fetch-snapshot-env.test.ts index b48f67639..27b1f0662 100644 --- a/packages/sdk/src/server/lib/task-runs/__tests__/fetch-snapshot-env.test.ts +++ b/packages/sdk/src/server/lib/task-runs/__tests__/fetch-snapshot-env.test.ts @@ -6,10 +6,12 @@ const { mockFindFirst, mockFetchResolvedRuntimeEnvVars, mockCreateSourceControlTokenForTaskRun, + mockResolveTaskRunSourceControlProviders, } = vi.hoisted(() => ({ mockFindFirst: vi.fn(), mockFetchResolvedRuntimeEnvVars: vi.fn(), mockCreateSourceControlTokenForTaskRun: vi.fn(), + mockResolveTaskRunSourceControlProviders: vi.fn(), })); vi.mock('@roomote/db/server', () => ({ @@ -31,6 +33,8 @@ vi.mock('@roomote/db/server', () => ({ vi.mock('../dequeue-helpers', () => ({ fetchResolvedRuntimeEnvVars: mockFetchResolvedRuntimeEnvVars, createSourceControlTokenForTaskRun: mockCreateSourceControlTokenForTaskRun, + resolveTaskRunSourceControlProviders: + mockResolveTaskRunSourceControlProviders, })); import { fetchSnapshotEnv } from '../fetch-snapshot-env'; @@ -62,6 +66,7 @@ function makeGitHubToken(token: string) { describe('fetchSnapshotEnv', () => { beforeEach(() => { vi.clearAllMocks(); + mockResolveTaskRunSourceControlProviders.mockResolvedValue(['github']); }); // ── Happy path: deployment-scoped env vars ─────────────────────────── @@ -84,7 +89,7 @@ describe('fetchSnapshotEnv', () => { const result = await fetchSnapshotEnv(auth, { runId: 42 }); expect(result).toEqual({ - envVars: { MY_SECRET: 'value123' }, + envVars: { MY_SECRET: 'value123', GH_TOKEN: 'ghs_token_abc' }, gitHubToken: 'ghs_token_abc', sourceControlToken: token, taskId: 'task_123', @@ -96,7 +101,7 @@ describe('fetchSnapshotEnv', () => { // Verify the gateway-aware resolution was used (so snapshot env withholds // gateway-served provider keys, like the task dequeue path). expect(mockFetchResolvedRuntimeEnvVars).toHaveBeenCalledWith(undefined, { - sourceControlProvider: 'github', + sourceControlProvider: ['github'], }); // Verify createSourceControlTokenForTaskRun was called with the task run. @@ -126,14 +131,14 @@ describe('fetchSnapshotEnv', () => { const result = await fetchSnapshotEnv(auth, { runId: 42 }); expect(result).toEqual({ - envVars: {}, + envVars: { GH_TOKEN: 'ghs_job_token' }, gitHubToken: 'ghs_job_token', sourceControlToken: token, taskId: 'task_123', }); expect(mockFetchResolvedRuntimeEnvVars).toHaveBeenCalledWith(undefined, { - sourceControlProvider: 'github', + sourceControlProvider: ['github'], }); }); @@ -174,11 +179,41 @@ describe('fetchSnapshotEnv', () => { const result = await fetchSnapshotEnv(auth, { runId: 42 }); - expect(result.envVars).toEqual({}); + expect(result.envVars).toEqual({ GH_TOKEN: 'ghs_token_xyz' }); expect(result.gitHubToken).toBe('ghs_token_xyz'); expect(result.taskId).toBe('task_123'); }); + it('derives gitHubToken from merged source-control env vars', async () => { + const auth: AuthTokenContext = { + userId: 'user_456', + tokenType: 'auth', + version: 1, + }; + mockFindFirst.mockResolvedValue(makeTaskRun()); + mockResolveTaskRunSourceControlProviders.mockResolvedValue([ + 'gitlab', + 'github', + ]); + mockFetchResolvedRuntimeEnvVars.mockResolvedValue({ MY_SECRET: 'value' }); + mockCreateSourceControlTokenForTaskRun.mockResolvedValue({ + provider: 'gitlab', + token: 'glptt_primary', + envVar: 'GITLAB_TOKEN', + envVars: { GH_TOKEN: 'ghs_merged' }, + source: 'app', + expiresAt: null, + }); + + const result = await fetchSnapshotEnv(auth, { runId: 42 }); + + expect(result.gitHubToken).toBe('ghs_merged'); + expect(result.envVars).toEqual({ + MY_SECRET: 'value', + GH_TOKEN: 'ghs_merged', + }); + }); + // ── Source-control token creation fails ───────────────────────────── it('throws when createSourceControlTokenForTaskRun returns null', async () => { diff --git a/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts b/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts index a0a8f8f20..4a027dc80 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts @@ -9,6 +9,7 @@ import { RunStatus, buildSourceControlTokenMetadata, getSourceControlProviderLabel, + normalizeSourceControlProvider, resolveTaskWorkspace, resolveSourceControlProviderFromPayload, type SourceControlProvider, @@ -80,22 +81,31 @@ export function redactControlPlaneEnvVars( export function redactSourceControlProviderEnvVars( envVars: Record, - sourceControlProvider?: SourceControlProvider, + sourceControlProvider?: SourceControlProvider | SourceControlProvider[], ): Record { - if (sourceControlProvider === 'github') { - return envVars; - } - - const providerTokenEnvVars = - sourceControlProvider === 'gitlab' + // Provider credential builders have already converted deployment secrets + // into scoped or proxy-backed worker credentials. Never expose the broad + // deployment token itself inside the task sandbox, including Bitbucket. + const providers = Array.isArray(sourceControlProvider) + ? sourceControlProvider + : sourceControlProvider + ? [sourceControlProvider] + : []; + const providerTokenEnvVars = providers.flatMap((provider) => + provider === 'gitlab' ? ['GITLAB_TOKEN'] - : sourceControlProvider === 'gitea' + : provider === 'gitea' ? ['GITEA_TOKEN'] - : sourceControlProvider === 'bitbucket' - ? [] - : sourceControlProvider === 'ado' + : provider === 'bitbucket' + ? ['BITBUCKET_OAUTH'] + : provider === 'ado' ? ['ADO_TOKEN'] - : []; + : [], + ); + + if (providerTokenEnvVars.length === 0) { + return envVars; + } const shouldRedact = providerTokenEnvVars.some( (envVar) => envVars[envVar] !== undefined, ); @@ -133,7 +143,7 @@ export function claimJobById(runId: number) { export async function fetchEnvVars( tx: Parameters[0]>[0], options?: { - sourceControlProvider?: SourceControlProvider; + sourceControlProvider?: SourceControlProvider | SourceControlProvider[]; }, ): Promise> { const encryptedEnvVars = await tx.query.environmentVariables.findMany(); @@ -243,7 +253,7 @@ function redactModelRuntimeManagedEnvVars( export async function fetchResolvedRuntimeEnvVars( deploymentEnvVars?: Record, options?: { - sourceControlProvider?: SourceControlProvider; + sourceControlProvider?: SourceControlProvider | SourceControlProvider[]; }, ): Promise> { const envVars = @@ -406,23 +416,47 @@ export type SourceControlRuntimeToken = SourceControlTokenMetadata & { }; /** - * Resolve the provider for a run's source-control token. Prefers the explicit - * payload stamp; when absent, resolves from the synced repositories the - * workspace references, so non-GitHub deployments work even when a launch - * site forgot to stamp the payload. Falls back to the GitHub default only - * when the workspace repositories are unknown or span providers. + * Resolve the ordered providers for a run's source-control tokens. A repository + * map is authoritative and keeps the primary repository's provider first. + * Legacy payloads retain the existing scalar, workspace, and default fallback. */ -async function resolveTaskRunSourceControlProvider( +export async function resolveTaskRunSourceControlProviders( taskRun: Pick, -): Promise { - const payload = taskRun.payload as { sourceControlProvider?: unknown }; + dbOrTx: Parameters[0] = db, +): Promise { + const payload = taskRun.payload as { + repo?: string; + repositoryProviders?: Record; + sourceControlProvider?: unknown; + }; + + if ( + payload.repositoryProviders && + Object.keys(payload.repositoryProviders).length > 0 + ) { + const mappedProviders = Object.values(payload.repositoryProviders).map( + normalizeSourceControlProvider, + ); + const primaryProvider = + payload.sourceControlProvider === undefined || + payload.sourceControlProvider === null || + payload.sourceControlProvider === '' + ? mappedProviders[0] + : resolveSourceControlProviderFromPayload(payload); + const providers = [ + ...(primaryProvider === undefined ? [] : [primaryProvider]), + ...mappedProviders, + ]; + + return [...new Set(providers)]; + } if ( payload.sourceControlProvider !== undefined && payload.sourceControlProvider !== null && payload.sourceControlProvider !== '' ) { - return resolveSourceControlProviderFromPayload(payload); + return [resolveSourceControlProviderFromPayload(payload)]; } // No explicit stamp: resolve from the workspace's synced repositories via the @@ -431,12 +465,12 @@ async function resolveTaskRunSourceControlProvider( // GitHub default that resolveSourceControlProviderFromPayload applies. const workspace = resolveTaskWorkspace(taskRun.payload); const resolvedProvider = await resolveWorkspaceSourceControlProvider( - db, + dbOrTx, workspace, ); if (resolvedProvider) { - return resolvedProvider; + return [resolvedProvider]; } // The GitHub default is wrong whenever the workspace actually spans @@ -445,17 +479,16 @@ async function resolveTaskRunSourceControlProvider( // scopes into per-provider runs; log loudly so the surface that forgot is // diagnosable from the run's cancellation. console.warn( - `[resolveTaskRunSourceControlProvider] Task run ${taskRun.id} has no sourceControlProvider stamp and its ${workspace.type} workspace resolves to no single provider; falling back to the GitHub default. The launch site should stamp the payload or split multi-provider scopes into per-provider runs.`, + `[resolveTaskRunSourceControlProviders] Task run ${taskRun.id} has no sourceControlProvider stamp and its ${workspace.type} workspace resolves to no single provider; falling back to the GitHub default. The launch site should stamp the payload or split multi-provider scopes into per-provider runs.`, ); - return resolveSourceControlProviderFromPayload(taskRun.payload); + return [resolveSourceControlProviderFromPayload(taskRun.payload)]; } async function createProviderToken( taskRun: TaskRun, + provider: SourceControlProvider, ): Promise { - const provider = await resolveTaskRunSourceControlProvider(taskRun); - switch (provider) { case 'github': { const token = await createTaskRunWorkerGitHubToken(taskRun); @@ -536,26 +569,48 @@ async function createProviderToken( } } -/** - * Creates a source-control token for the task run with retry logic. - * Retries up to {@link SOURCE_CONTROL_TOKEN_MAX_RETRIES} times with - * exponential backoff (1s, 2s, 4s) to handle transient provider API failures. - * Returns null if all attempts fail (caller should handle the error). - */ -export async function createSourceControlTokenForTaskRun( +function mergeProviderTokens( + tokens: SourceControlRuntimeToken[], +): SourceControlRuntimeToken { + const [primaryToken, ...secondaryTokens] = tokens; + + if (!primaryToken) { + throw new Error('No source control providers resolved for task run.'); + } + + return secondaryTokens.reduce( + (merged, token) => ({ + ...merged, + envVars: { ...merged.envVars, ...token.envVars }, + gitCredentials: [ + ...(merged.gitCredentials ?? []), + ...(token.gitCredentials ?? []), + ], + gitProxyCredentials: [ + ...(merged.gitProxyCredentials ?? []), + ...(token.gitProxyCredentials ?? []), + ], + artifactsPatch: { + ...(merged.artifactsPatch ?? {}), + ...(token.artifactsPatch ?? {}), + }, + }), + primaryToken, + ); +} + +async function createProviderTokenWithRetry( taskRun: TaskRun, + provider: SourceControlProvider, logPrefix: string, - { - maxRetries = SOURCE_CONTROL_TOKEN_MAX_RETRIES, - baseDelayMs = SOURCE_CONTROL_TOKEN_BASE_DELAY_MS, - } = {}, + maxRetries: number, + baseDelayMs: number, ): Promise { - const provider = await resolveTaskRunSourceControlProvider(taskRun); const label = getSourceControlProviderLabel(provider); for (let attempt = 1; attempt <= maxRetries; attempt++) { try { - return await createProviderToken(taskRun); + return await createProviderToken(taskRun, provider); } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -576,6 +631,54 @@ export async function createSourceControlTokenForTaskRun( return null; } +/** + * Creates a source-control token for the task run with retry logic. + * Retries up to {@link SOURCE_CONTROL_TOKEN_MAX_RETRIES} times with + * exponential backoff (1s, 2s, 4s) to handle transient provider API failures. + * Returns null if all attempts fail (caller should handle the error). + */ +export async function createSourceControlTokenForTaskRun( + taskRun: TaskRun, + logPrefix: string, + { + maxRetries = SOURCE_CONTROL_TOKEN_MAX_RETRIES, + baseDelayMs = SOURCE_CONTROL_TOKEN_BASE_DELAY_MS, + } = {}, +): Promise { + const providers = await resolveTaskRunSourceControlProviders(taskRun); + + // GitLab scoped tokens create revocable remote resources. Mint them last so + // a later provider failure cannot orphan a successful GitLab token set. + const mintOrder = [ + ...providers.filter((provider) => provider !== 'gitlab'), + ...providers.filter((provider) => provider === 'gitlab'), + ]; + const tokensByProvider = new Map< + SourceControlProvider, + SourceControlRuntimeToken + >(); + + for (const provider of mintOrder) { + const token = await createProviderTokenWithRetry( + taskRun, + provider, + logPrefix, + maxRetries, + baseDelayMs, + ); + + if (!token) { + return null; + } + + tokensByProvider.set(provider, token); + } + + return mergeProviderTokens( + providers.map((provider) => tokensByProvider.get(provider)!), + ); +} + /** * Marks a task run as canceled with an error message. */ diff --git a/packages/sdk/src/server/lib/task-runs/dequeue-resume-task-run.ts b/packages/sdk/src/server/lib/task-runs/dequeue-resume-task-run.ts index 42432b6ae..df77e8c46 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-resume-task-run.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-resume-task-run.ts @@ -25,6 +25,7 @@ import { claimJobById, fetchEnvVars, fetchResolvedRuntimeEnvVars, + resolveTaskRunSourceControlProviders, cancelAndReleaseTaskRun, createSourceControlTokenForTaskRun, type SourceControlRuntimeToken, @@ -135,6 +136,9 @@ export const dequeueResumeTaskRun = async ( sourceRepo?: string; sourceEnvironmentId?: string; sourceSelectedRepositories?: string[]; + sourceControlProviders: Awaited< + ReturnType + >; }; const result: TransactionResult = await db.transaction(async (tx) => { @@ -283,10 +287,12 @@ export const dequeueResumeTaskRun = async ( ); // Fetch environment variables + const sourceControlProviders = await resolveTaskRunSourceControlProviders( + taskRun, + tx, + ); const envVars = await fetchEnvVars(tx, { - sourceControlProvider: resolveSourceControlProviderFromPayload( - taskRun.payload, - ), + sourceControlProvider: sourceControlProviders, }); const settings = await tx.query.deploymentSettings.findFirst({ columns: { @@ -342,6 +348,7 @@ export const dequeueResumeTaskRun = async ( sourceRepo, sourceEnvironmentId, sourceSelectedRepositories, + sourceControlProviders, }; }); @@ -410,14 +417,13 @@ export const dequeueResumeTaskRun = async ( return undefined; } - const gitHubToken = - sourceControlToken.provider === 'github' ? sourceControlToken.token : ''; + const gitHubToken = sourceControlToken.envVars.GH_TOKEN ?? ''; let resolvedEnvVars: Record; try { resolvedEnvVars = await fetchResolvedRuntimeEnvVars(result.envVars, { - sourceControlProvider: sourceControlToken.provider, + sourceControlProvider: result.sourceControlProviders, }); } catch (error) { const message = diff --git a/packages/sdk/src/server/lib/task-runs/dequeue-task-run.ts b/packages/sdk/src/server/lib/task-runs/dequeue-task-run.ts index ac5ea054e..990c37b6a 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-task-run.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-task-run.ts @@ -24,6 +24,7 @@ import { type GitAuthor, fetchEnvVars, fetchResolvedRuntimeEnvVars, + resolveTaskRunSourceControlProviders, cancelAndReleaseTaskRun, createSourceControlTokenForTaskRun, type SourceControlRuntimeToken, @@ -281,6 +282,9 @@ export const dequeueTaskRun = async ( envVars: Record; orgAgentInstructions?: string; gitAuthor: GitAuthor; + sourceControlProviders: Awaited< + ReturnType + >; }; // Phase 1: Transaction — claim the run and fetch all data needed for @@ -309,10 +313,12 @@ export const dequeueTaskRun = async ( const task = taskRun.task; + const sourceControlProviders = await resolveTaskRunSourceControlProviders( + taskRun, + tx, + ); const envVars = await fetchEnvVars(tx, { - sourceControlProvider: resolveSourceControlProviderFromPayload( - taskRun.payload, - ), + sourceControlProvider: sourceControlProviders, }); const settings = await tx.query.deploymentSettings.findFirst({ columns: { @@ -386,6 +392,7 @@ export const dequeueTaskRun = async ( envVars, orgAgentInstructions: settings?.globalAgentInstructions ?? undefined, gitAuthor, + sourceControlProviders, }; }); @@ -455,8 +462,7 @@ export const dequeueTaskRun = async ( return undefined; } - const gitHubToken = - sourceControlToken.provider === 'github' ? sourceControlToken.token : ''; + const gitHubToken = sourceControlToken.envVars.GH_TOKEN ?? ''; const sourceControlArtifacts = sourceControlToken.artifactsPatch ?? {}; let prompt: string; @@ -531,7 +537,7 @@ export const dequeueTaskRun = async ( }, fn: async () => await fetchResolvedRuntimeEnvVars(txResult.envVars, { - sourceControlProvider: sourceControlToken.provider, + sourceControlProvider: txResult.sourceControlProviders, }), }); } catch (error) { diff --git a/packages/sdk/src/server/lib/task-runs/fetch-snapshot-env.ts b/packages/sdk/src/server/lib/task-runs/fetch-snapshot-env.ts index f2c6186df..13aa5a4d3 100644 --- a/packages/sdk/src/server/lib/task-runs/fetch-snapshot-env.ts +++ b/packages/sdk/src/server/lib/task-runs/fetch-snapshot-env.ts @@ -1,5 +1,4 @@ import { - resolveSourceControlProviderFromPayload, type AuthTokenContext, type RunTokenContext, type SourceControlTokenMetadata, @@ -9,6 +8,7 @@ import { db, taskRuns, eq } from '@roomote/db/server'; import { fetchResolvedRuntimeEnvVars, createSourceControlTokenForTaskRun, + resolveTaskRunSourceControlProviders, } from './dequeue-helpers'; /** @@ -40,10 +40,10 @@ export async function fetchSnapshotEnv( // dequeue so gateway-covered provider keys are withheld here too; otherwise // a snapshot taken with the flag on would bake raw provider keys into the // snapshot's shell env and the persisted image. - const envVars = await fetchResolvedRuntimeEnvVars(undefined, { - sourceControlProvider: resolveSourceControlProviderFromPayload( - taskRun.payload, - ), + const sourceControlProviders = + await resolveTaskRunSourceControlProviders(taskRun); + const deploymentEnvVars = await fetchResolvedRuntimeEnvVars(undefined, { + sourceControlProvider: sourceControlProviders, }); const sourceControlToken = await createSourceControlTokenForTaskRun( @@ -76,8 +76,8 @@ export async function fetchSnapshotEnv( .where(eq(taskRuns.id, taskRun.id)); } - const gitHubToken = - sourceControlToken.provider === 'github' ? sourceControlToken.token : ''; + const envVars = { ...deploymentEnvVars, ...sourceControlToken.envVars }; + const gitHubToken = envVars.GH_TOKEN ?? ''; return { envVars, gitHubToken, sourceControlToken, taskId: taskRun.taskId }; } diff --git a/packages/types/src/__tests__/command-schema.test.ts b/packages/types/src/__tests__/command-schema.test.ts index 7d10be0c4..bf79ef53f 100644 --- a/packages/types/src/__tests__/command-schema.test.ts +++ b/packages/types/src/__tests__/command-schema.test.ts @@ -6,6 +6,7 @@ import { commandSchema, environmentConfigSchema, environmentRepositoryConfigSchema, + getDuplicateEnvironmentRepositoryConfigError, getMissingEnvironmentRepositoryError, } from '../environment-config'; @@ -431,6 +432,33 @@ commands: }); describe('environmentConfigSchema', () => { + it('keeps legacy duplicate repository entries parseable on read', () => { + const result = environmentConfigSchema.safeParse({ + name: 'Env', + repositories: [ + { repository: 'owner/repo' }, + { repository: 'owner/repo' }, + ], + }); + + expect(result.success).toBe(true); + }); + + it('reports duplicate repository entries for write validation', () => { + expect( + getDuplicateEnvironmentRepositoryConfigError([ + { repository: 'owner/repo' }, + { repository: 'owner/repo' }, + ]), + ).toBe('Duplicate repository: owner/repo'); + expect( + getDuplicateEnvironmentRepositoryConfigError([ + { repository: 'owner/repo' }, + { repository: 'owner/other' }, + ]), + ).toBeNull(); + }); + describe('tool_versions', () => { it('should accept root-level tool_versions for environment workspaces', () => { const result = environmentConfigSchema.safeParse({ diff --git a/packages/types/src/__tests__/source-control.test.ts b/packages/types/src/__tests__/source-control.test.ts index d411dc909..f058030bb 100644 --- a/packages/types/src/__tests__/source-control.test.ts +++ b/packages/types/src/__tests__/source-control.test.ts @@ -2,6 +2,7 @@ import { buildPullRequestUrl, buildRepositoryCloneUrl, buildSourceControlTokenMetadata, + filterRepositoryNamesForSourceControlProvider, getSourceControlTokenEnvVar, normalizeSourceControlProvider, parsePullRequestUrl, @@ -36,6 +37,38 @@ describe('source control provider helpers', () => { ).toBe('ado'); }); + it('uses provider maps as an authoritative repository allowlist', () => { + const repositoryNames = ['octo/api', 'group/web', 'unknown/repository']; + const payload = { + repositoryProviders: { + 'octo/api': 'github', + 'group/web': 'gitlab', + }, + }; + + expect( + filterRepositoryNamesForSourceControlProvider( + payload, + repositoryNames, + 'github', + ), + ).toEqual(['octo/api']); + expect( + filterRepositoryNamesForSourceControlProvider( + payload, + repositoryNames, + 'gitlab', + ), + ).toEqual(['group/web']); + expect( + filterRepositoryNamesForSourceControlProvider( + {}, + repositoryNames, + 'github', + ), + ).toEqual(repositoryNames); + }); + it('maps providers to their runtime token environment variable', () => { expect(getSourceControlTokenEnvVar('github')).toBe('GH_TOKEN'); expect(getSourceControlTokenEnvVar('gitlab')).toBe('GITLAB_TOKEN'); diff --git a/packages/types/src/__tests__/task-runs.test.ts b/packages/types/src/__tests__/task-runs.test.ts index db39107bd..1653ccfa5 100644 --- a/packages/types/src/__tests__/task-runs.test.ts +++ b/packages/types/src/__tests__/task-runs.test.ts @@ -39,6 +39,7 @@ import { WORK_ITEM_ACTIVE_STATUSES, shouldUseAppTokenOnly, } from '../task-runs'; +import { ALL_REPOSITORIES } from '../constants'; describe('isSourceControlTaskSurface', () => { it.each(['github', 'gitlab', 'gitea', 'bitbucket', 'ado'] as const)( @@ -287,6 +288,32 @@ describe('taskSpecSchema', () => { expect(parsed.payload.sourceControlProvider).toBe('gitlab'); }); + it('preserves repositoryProviders on mixed-provider task payloads', () => { + const parsed = taskSpecSchema.parse({ + userId: 'user-1', + type: TaskPayloadKind.StandardTask, + payload: { + repo: ALL_REPOSITORIES, + selectedRepositories: ['octo/api', 'group/web'], + sourceControlProvider: 'github', + repositoryProviders: { + 'octo/api': 'github', + 'group/web': 'gitlab', + }, + description: 'Update a mixed-provider workspace', + }, + }); + + if (parsed.type !== TaskPayloadKind.StandardTask) { + throw new Error('Expected StandardTask payload'); + } + + expect(parsed.payload.repositoryProviders).toEqual({ + 'octo/api': 'github', + 'group/web': 'gitlab', + }); + }); + it('preserves customAutomationId and Slack channel context on StandardTask payloads', () => { const parsed = taskSpecSchema.parse({ userId: 'user-1', diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 4f6f32993..1fee7654a 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -1028,6 +1028,40 @@ export function hasEnvironmentOidcTargets( export const MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR = 'Environment repositories must all belong to the same GitHub App installation.'; +export function getDuplicateEnvironmentRepositoryConfigError( + repositories: Array<{ repository: string }>, +): string | null { + const seen = new Set(); + const duplicate = repositories.find((repository) => { + if (seen.has(repository.repository)) { + return true; + } + + seen.add(repository.repository); + return false; + }); + + return duplicate ? `Duplicate repository: ${duplicate.repository}` : null; +} + +export function getAmbiguousEnvironmentRepositoryError( + repositories: Array<{ fullName: string }>, +): string | null { + const seen = new Set(); + const duplicate = repositories.find((repository) => { + if (seen.has(repository.fullName)) { + return true; + } + + seen.add(repository.fullName); + return false; + }); + + return duplicate + ? `Multiple repositories are named "${duplicate.fullName}". Environment repository names must be unique across source-control connections.` + : null; +} + type EnvironmentRepositoryInstallationReference = { fullName: string; installationId: string | number | null | undefined; diff --git a/packages/types/src/environment-definition-tasks.ts b/packages/types/src/environment-definition-tasks.ts index b367f7143..1905c7aaa 100644 --- a/packages/types/src/environment-definition-tasks.ts +++ b/packages/types/src/environment-definition-tasks.ts @@ -5,6 +5,20 @@ type RepositoryReference = { fullName: string; }; +export function assertUniqueRepositoryFullNames( + repositoryFullNames: string[], +): void { + const duplicateRepository = repositoryFullNames.find( + (repository, index) => repositoryFullNames.indexOf(repository) !== index, + ); + + if (duplicateRepository) { + throw new Error( + `The selected repositories include multiple entries named "${duplicateRepository}". Select only one because task workspaces identify repositories by full name.`, + ); + } +} + export const ENVIRONMENT_DEFINITION_SETUP_GUIDANCE_PLACEHOLDER = 'Optional agent guidance, like what services in a monorepo to set up or context that may be missing from the repo itself'; @@ -33,36 +47,23 @@ export function getEnvironmentDefinitionIdFromPayload( export function normalizeRepositorySelection( repositories: RepositoryReference[], ): string[] { - const uniqueRepositories = Array.from( - new Map( - repositories.map((repository) => [repository.id, repository]), - ).values(), - ); - - return uniqueRepositories - .sort( - (left, right) => - left.fullName.localeCompare(right.fullName) || - left.id.localeCompare(right.id), - ) - .map((repository) => repository.id); + return [...new Set(repositories.map((repository) => repository.id))]; } export function buildCreateEnvironmentDefinitionPrompt( repositoryFullNames: string[], options?: { emptyRepositoryFullNames?: string[] }, ): string { - const sortedRepositories = [...repositoryFullNames].sort((left, right) => - left.localeCompare(right), - ); + const orderedRepositories = [...new Set(repositoryFullNames)]; - const repositoryLines = sortedRepositories + const repositoryLines = orderedRepositories .map((repositoryFullName) => `- ${repositoryFullName}`) .join('\n'); - const emptyRepositories = [...(options?.emptyRepositoryFullNames ?? [])] - .filter((fullName) => repositoryFullNames.includes(fullName)) - .sort((left, right) => left.localeCompare(right)); + const emptyRepositoryNames = new Set(options?.emptyRepositoryFullNames ?? []); + const emptyRepositories = orderedRepositories.filter((fullName) => + emptyRepositoryNames.has(fullName), + ); // Restate the skill's empty-repository bootstrap rules inline so a worker // whose packaged environment-setup skill predates the bootstrap section @@ -134,9 +135,8 @@ export function buildEnvironmentDefinitionWorkspacePayload( repo: string; selectedRepositories?: string[]; } { - const normalizedRepositories = [...new Set(repositoryFullNames)].sort( - (left, right) => left.localeCompare(right), - ); + assertUniqueRepositoryFullNames(repositoryFullNames); + const normalizedRepositories = [...new Set(repositoryFullNames)]; const primaryRepository = normalizedRepositories[0]; if (!primaryRepository) { diff --git a/packages/types/src/source-control.ts b/packages/types/src/source-control.ts index 63caa5f1e..2fbdce63c 100644 --- a/packages/types/src/source-control.ts +++ b/packages/types/src/source-control.ts @@ -153,6 +153,32 @@ export function resolveSourceControlProviderFromPayload(payload: { return normalizeSourceControlProvider(payload.sourceControlProvider); } +export function resolveRepositoryProvidersFromPayload(payload: { + repositoryProviders?: unknown; +}): Record | undefined { + const parsed = z + .record(sourceControlProviderSchema) + .safeParse(payload.repositoryProviders); + + return parsed.success && Object.keys(parsed.data).length > 0 + ? parsed.data + : undefined; +} + +export function filterRepositoryNamesForSourceControlProvider( + payload: { repositoryProviders?: unknown }, + repositoryNames: string[], + provider: SourceControlProvider, +): string[] { + const repositoryProviders = resolveRepositoryProvidersFromPayload(payload); + + return repositoryProviders + ? repositoryNames.filter( + (repositoryName) => repositoryProviders[repositoryName] === provider, + ) + : repositoryNames; +} + /** * Reads the optional `sourceControlHost` field from a task payload. Returns * the trimmed host, or undefined when the payload carries no usable host so diff --git a/packages/types/src/task-runs.ts b/packages/types/src/task-runs.ts index d57bc6ccf..2d4ef0bcf 100644 --- a/packages/types/src/task-runs.ts +++ b/packages/types/src/task-runs.ts @@ -822,6 +822,12 @@ const sharedTaskPayloadSchema = z.object({ */ sourceControlProvider: sourceControlProviderSchema.optional(), + /** + * Source-control provider keyed by repository full name for workspaces that + * span multiple providers. Single-provider payloads omit this field. + */ + repositoryProviders: z.record(sourceControlProviderSchema).optional(), + /** * Source-control instance host for repository resolution (for example * `gitlab.example.com`), matching `repositories.host`. Stamped by launch @@ -1924,6 +1930,7 @@ type TaskWorkspacePayload = { repo?: string; branch?: string; sha?: string; + sourceControlHost?: string; environmentId?: string; selectedRepositories?: string[]; }; @@ -1934,10 +1941,12 @@ export type TaskWorkspace = repo: string; branch?: string; sha?: string; + sourceControlHost?: string; } | { type: 'repository_set'; repositories: string[]; + sourceControlHost?: string; } | { type: 'all_repositories'; @@ -1983,6 +1992,7 @@ export function resolveTaskWorkspace( ? { type: 'repository_set', repositories, + sourceControlHost: payload.sourceControlHost, } : { type: 'all_repositories', @@ -2000,6 +2010,7 @@ export function resolveTaskWorkspace( repo: payload.repo, branch: payload.branch, sha: payload.sha, + sourceControlHost: payload.sourceControlHost, }; }