From 3c6d4058c59bb06aa70c03bcf7e8403a2cba3ca9 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 10:51:05 +0000 Subject: [PATCH 01/12] fix: stamp mixed workspace repository providers --- .../tasks/__tests__/launchTask.test.ts | 36 +++- apps/api/src/handlers/tasks/launchTask.ts | 24 +-- .../src/lib/server/source-control-provider.ts | 12 +- .../src/trpc/commands/setup-new/index.test.ts | 40 +++++ apps/web/src/trpc/commands/setup-new/index.ts | 12 +- .../src/trpc/commands/task-runs/index.test.ts | 34 +++- .../src/server/__tests__/enqueue-task.test.ts | 147 ++++++++++++++++ .../cloud-agents/src/server/task-run-queue.ts | 71 ++++---- .../__tests__/source-control-provider.test.ts | 159 ++++++++++++++++-- .../db/src/lib/source-control-provider.ts | 145 +++++++++++++--- .../types/src/__tests__/task-runs.test.ts | 27 +++ packages/types/src/task-runs.ts | 11 ++ 12 files changed, 603 insertions(+), 115 deletions(-) diff --git a/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts b/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts index 543d108fb..24e06f7ce 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' }); diff --git a/apps/api/src/handlers/tasks/launchTask.ts b/apps/api/src/handlers/tasks/launchTask.ts index 034a26edb..93299c7ec 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, @@ -129,23 +129,11 @@ async function resolveLaunchSourceControlProvider({ } 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/web/src/lib/server/source-control-provider.ts b/apps/web/src/lib/server/source-control-provider.ts index 1ef23c52c..6015d1e23 100644 --- a/apps/web/src/lib/server/source-control-provider.ts +++ b/apps/web/src/lib/server/source-control-provider.ts @@ -1,5 +1,5 @@ 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 @@ -30,11 +30,8 @@ export function resolveSingleSourceControlProvider( * 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 +40,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/setup-new/index.test.ts b/apps/web/src/trpc/commands/setup-new/index.test.ts index 1ac0123b6..6c8e98627 100644 --- a/apps/web/src/trpc/commands/setup-new/index.test.ts +++ b/apps/web/src/trpc/commands/setup-new/index.test.ts @@ -228,6 +228,7 @@ import { startSetupNewOnboardingTaskCommand, } from './index'; import { + ALL_REPOSITORIES, TaskPayloadKind, WORKER_RUNTIME_SCHEMA_VERSION, type SetupNewState, @@ -1199,6 +1200,45 @@ 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: ['group/web', 'octo/api'], + }); + mockOnboardingTransaction({ + slackInstallation: null, + setupNewState: { selectedRepositoryIds: ['repo-1', 'repo-2'] }, + }); + + await startSetupNewOnboardingTaskCommand(buildMockAuth()); + + expect(enqueueTask).toHaveBeenCalledWith( + expect.objectContaining({ + task: expect.objectContaining({ + payload: expect.objectContaining({ + sourceControlProvider: 'gitlab', + }), + }), + }), + ); + }); + 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..47bfbbe44 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -1,9 +1,6 @@ import * as GitHub from '@roomote/github'; 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'; @@ -2498,11 +2495,8 @@ export async function startSetupNewOnboardingTaskCommand( ); // 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/task-runs/index.test.ts b/apps/web/src/trpc/commands/task-runs/index.test.ts index c06218997..5896c00c3 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,30 @@ 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', + }), + }), + }), + ); + }); }); 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..1d6bb4ee0 100644 --- a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts +++ b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts @@ -787,6 +787,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 +819,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 () => { @@ -1314,6 +1323,144 @@ 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, + }); + 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', + }, + }); + }); + + 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, + }); + 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..235199ad8 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) && @@ -1797,6 +1778,29 @@ 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); + + if ( + new Set(providers).size > 1 && + payload.repositoryProviders === undefined + ) { + payload.repositoryProviders = repositoryProviders; + } + + const primaryProvider = providers[0]; + if (payload.sourceControlProvider === undefined && primaryProvider) { + 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 +1868,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,6 +2005,7 @@ function inheritSnapshotResumeSourceControlStamps( sourcePayload: unknown, ): void { const source = (sourcePayload ?? {}) as { + repositoryProviders?: unknown; sourceControlProvider?: unknown; sourceControlHost?: unknown; }; @@ -2027,6 +2020,16 @@ function inheritSnapshotResumeSourceControlStamps( if (provider.success) { payload.sourceControlProvider = provider.data; } + + if (payload.repositoryProviders === undefined) { + const repositoryProviders = z + .record(sourceControlProviderSchema) + .safeParse(source.repositoryProviders); + + if (repositoryProviders.success) { + payload.repositoryProviders = repositoryProviders.data; + } + } } if (inheritsProvider && payload.sourceControlHost === undefined) { 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..ca808556b 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,55 @@ // 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; + sourceControlProvider: 'github' | 'gitlab' | 'gitea' | 'ado' | 'bitbucket'; +}> = []; + +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), })), } as unknown as DatabaseOrTransaction; describe('resolveWorkspaceSourceControlProvider', () => { beforeEach(() => { + mockRows = []; mockWhere.mockReset(); + mockOrderBy.mockReset(); }); it('resolves the provider from an environment with a single-provider mapping', async () => { - mockWhere.mockResolvedValue([{ sourceControlProvider: 'ado' }]); + mockRows = [ + { + fullName: 'acme/Platform/backend', + host: 'dev.azure.com', + sourceControlProvider: 'ado', + }, + ]; await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { @@ -30,7 +60,13 @@ describe('resolveWorkspaceSourceControlProvider', () => { }); 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 +77,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 +99,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 +120,6 @@ describe('resolveWorkspaceSourceControlProvider', () => { }); it('returns undefined when no repository rows match', async () => { - mockWhere.mockResolvedValue([]); - await expect( resolveWorkspaceSourceControlProvider(dbOrTx, { type: 'repository', @@ -87,4 +137,79 @@ 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('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/source-control-provider.ts b/packages/db/src/lib/source-control-provider.ts index 73e256356..47a14f1d3 100644 --- a/packages/db/src/lib/source-control-provider.ts +++ b/packages/db/src/lib/source-control-provider.ts @@ -1,4 +1,4 @@ -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'; @@ -17,28 +17,86 @@ function toSingleProvider( return unique.length === 1 ? unique[0] : undefined; } -async function resolveProviderByFullNames( +type RepositoryProviderRow = { + fullName: string; + host: string | null; + 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 hostMatches = + matches.length > 1 && sourceControlHost + ? matches.filter((row) => row.host === sourceControlHost) + : matches; + + if (matches.length > 1 && hostMatches.length !== 1) { + console.warn( + `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${matches.length} active 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, + sourceControlProvider: repositories.sourceControlProvider, + }) .from(repositories) - .where(inArray(repositories.fullName, fullNames)); + .where( + and( + eq(repositories.isActive, true), + 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 rows = await dbOrTx - .select({ sourceControlProvider: repositories.sourceControlProvider }) + .select({ + fullName: repositories.fullName, + host: repositories.host, + sourceControlProvider: repositories.sourceControlProvider, + }) .from(environmentRepositoryMappings) .innerJoin( repositories, @@ -49,20 +107,60 @@ 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, + rows.map((row) => row.fullName), + ); } -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, + 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); + } } /** @@ -80,14 +178,9 @@ 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/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/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, }; } From 29cc536fa77c064a88639a36188988839d2801f3 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 10:42:26 +0000 Subject: [PATCH 02/12] fix: prepare mixed-provider worker repositories --- .../src/commands/__tests__/snapshot.test.ts | 19 +++ .../setup/__tests__/workspace.test.ts | 148 ++++++++++++++++++ .../commands/setup/workspace/repositories.ts | 24 ++- .../src/commands/setup/workspace/types.ts | 1 + apps/worker/src/commands/snapshot.ts | 5 + .../commands/utils/execute-task-run.test.ts | 6 + .../src/commands/utils/execute-task-run.ts | 4 + .../commands/utils/repository-providers.ts | 15 ++ .../utils/scrub-sandbox-secrets.test.ts | 14 ++ .../workspace/__tests__/tool-versions.test.ts | 48 ++++++ .../worker/src/workspace/workspace-manager.ts | 8 +- 11 files changed, 284 insertions(+), 8 deletions(-) create mode 100644 apps/worker/src/commands/utils/repository-providers.ts 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..b26c73ef7 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,44 @@ describe('initializeRepositories', () => { ); }); + it('applies mapped providers to repositories discovered for all-repositories workspaces', async () => { + vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( + undefined, + ); + mockListRepositories.mockResolvedValue([ + { fullName: 'acme/github-app' }, + { fullName: 'acme/gitlab-app' }, + ]); + 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/gitlab-app': 'gitlab' }, + }); + + expect(mockListRepositories).toHaveBeenCalledTimes(1); + 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 +525,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..242f4a3dd 100644 --- a/apps/worker/src/commands/setup/workspace/repositories.ts +++ b/apps/worker/src/commands/setup/workspace/repositories.ts @@ -70,14 +70,26 @@ 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 { workspaceRoot, workspaceManager } = createWorkspaceManager( envVars, logger, @@ -104,7 +116,7 @@ export async function initializeRepositories( sourceBranch: workspace.sourceBranch, sourceSha: workspace.sourceSha, }, - sourceControlPrepareOptions, + environmentSourceControlPrepareOptions, ), ); @@ -167,7 +179,7 @@ export async function initializeRepositories( preserveGitState, cleanupLegacyPaths, { - ...sourceControlPrepareOptions, + ...sourceControlPrepareOptions(repo.fullName), }, ), ); @@ -317,7 +329,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, }, From f368d7a08ddef843435127a1f4eaa79f50a4d121 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 10:47:30 +0000 Subject: [PATCH 03/12] fix: route source control by target repository --- .../createEnvironment.validation.test.ts | 23 ++++ .../__tests__/manageSourceControl.test.ts | 106 ++++++++++++++++++ .../src/handlers/tasks/manageSourceControl.ts | 19 +++- apps/docs/environments.mdx | 3 + .../trpc/commands/environments/index.test.ts | 18 +++ .../src/trpc/commands/environments/index.ts | 2 +- packages/sdk/src/server/index.ts | 1 + .../__tests__/source-control-issues.test.ts | 54 +++++++++ .../lib/issues/source-control-issues.ts | 7 +- .../source-control-pull-request-reads.test.ts | 21 ++-- ...source-control-pull-request-shared.test.ts | 31 ++++- ...source-control-pull-request-writes.test.ts | 11 +- .../source-control-pull-requests.test.ts | 11 +- .../source-control-pull-request-reads.ts | 8 +- .../source-control-pull-request-shared.ts | 25 +++++ .../source-control-pull-request-writes.ts | 8 +- .../source-control-pull-requests.ts | 8 +- 17 files changed, 319 insertions(+), 37 deletions(-) create mode 100644 apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts create mode 100644 apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts 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..b24ddcf67 --- /dev/null +++ b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts @@ -0,0 +1,23 @@ +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); + }); +}); 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..eabe469a9 --- /dev/null +++ b/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts @@ -0,0 +1,106 @@ +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.', + }), + }); + }); +}); diff --git a/apps/api/src/handlers/tasks/manageSourceControl.ts b/apps/api/src/handlers/tasks/manageSourceControl.ts index 1ee30b6ab..48d680ea1 100644 --- a/apps/api/src/handlers/tasks/manageSourceControl.ts +++ b/apps/api/src/handlers/tasks/manageSourceControl.ts @@ -6,13 +6,12 @@ import { completeClaimedLatestUserMessageForReplyQuote, restoreClaimedLatestUserMessageForReplyQuote, } from '@roomote/communication/messages'; -import { resolveSourceControlProviderFromPayload } from '@roomote/types'; - import { createOrUpdateSourceControlPullRequestForTaskRun, findTaskRunForSourceControlMutation, manageSourceControlIssueForTaskRun, readSourceControlPullRequestForTaskRun, + resolveSourceControlProviderForRepositoryFromPayload, sourceControlIssueInputSchema, sourceControlPullRequestMutationInputSchema, sourceControlPullRequestReadInputSchema, @@ -93,11 +92,21 @@ export async function manageSourceControl( runId: auth.authContext.runId, taskId, }); - const isGitHubTask = - resolveSourceControlProviderFromPayload(taskRun.payload) === 'github'; + const payload = + taskRun.payload && + typeof taskRun.payload === 'object' && + !Array.isArray(taskRun.payload) + ? (taskRun.payload as Record) + : {}; + const targetProvider = + input.sourceControlProvider ?? + 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/docs/environments.mdx b/apps/docs/environments.mdx index eead94d4b..218eff010 100644 --- a/apps/docs/environments.mdx +++ b/apps/docs/environments.mdx @@ -37,6 +37,9 @@ 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. + 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/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index e10413712..f8e16baab 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -466,6 +466,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..4e390949f 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -1249,7 +1249,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/packages/sdk/src/server/index.ts b/packages/sdk/src/server/index.ts index 1fba1362a..a7666126f 100644 --- a/packages/sdk/src/server/index.ts +++ b/packages/sdk/src/server/index.ts @@ -255,6 +255,7 @@ export { type SourceControlPullRequestMutationInput, type SourceControlPullRequestMutationResult, } from './lib/pull-requests/source-control-pull-requests'; +export { 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..c4927c67c 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,60 @@ 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', + 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..7eb333844 100644 --- a/packages/sdk/src/server/lib/issues/source-control-issues.ts +++ b/packages/sdk/src/server/lib/issues/source-control-issues.ts @@ -2,13 +2,13 @@ import { type TaskRun } from '@roomote/db/server'; import { getSourceControlProviderLabel, resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, } from '@roomote/types'; import { assertRepositoryInTaskRunScope, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlProviderForRepositoryFromPayload, type FetchImpl, } from '../pull-requests/source-control-pull-request-shared'; import { getIssueProviderOperations } from './source-control-issue-providers'; @@ -39,7 +39,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) { 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..dc8e6dfb2 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,36 @@ vi.mock('@roomote/gitlab', () => ({ isGitLabOAuthAccessToken: (token: string) => token === 'oauth-token', })); -import { buildGitLabTokenHeader } from '../source-control-pull-request-shared'; +import { + buildGitLabTokenHeader, + 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('falls back to the task primary provider when the repository is unmapped', () => { + expect( + resolveSourceControlProviderForRepositoryFromPayload( + { + sourceControlProvider: 'github', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + 'acme/frontend', + ), + ).toBe('github'); + }); +}); 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..9336f5e35 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 @@ -5,7 +5,6 @@ import { buildPullRequestUrl, getSourceControlProviderLabel, resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -26,6 +25,7 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -609,8 +609,10 @@ export async function readSourceControlPullRequestForTaskRun({ fetchImpl?: FetchImpl; }): Promise { const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); const provider = input.sourceControlProvider ?? 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..31df99915 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,8 @@ import { ALL_REPOSITORIES, environmentConfigSchema, getSourceControlProviderLabel, + normalizeSourceControlProvider, + resolveSourceControlProviderFromPayload, type SourceControlProvider, } from '@roomote/types'; import { isGitLabOAuthAccessToken } from '@roomote/gitlab'; @@ -35,6 +37,29 @@ export type RepositoryRow = { htmlUrl: string; }; +export function resolveSourceControlProviderForRepositoryFromPayload( + payload: Record, + repositoryFullName: string, +): SourceControlProvider { + const repositoryProviders = payload.repositoryProviders; + + if ( + repositoryProviders && + typeof repositoryProviders === 'object' && + !Array.isArray(repositoryProviders) + ) { + const repositoryProvider = (repositoryProviders as Record)[ + repositoryFullName + ]; + + if (repositoryProvider !== undefined) { + return normalizeSourceControlProvider(repositoryProvider); + } + } + + return resolveSourceControlProviderFromPayload(payload); +} + /** * 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..55d5ef039 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 @@ -4,7 +4,6 @@ import { type TaskRun } from '@roomote/db/server'; import { getSourceControlProviderLabel, resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -28,6 +27,7 @@ import { formatResponseBody, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -233,8 +233,10 @@ export async function writeSourceControlPullRequestForTaskRun({ assertWriteInputFields(input); const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); const provider = input.sourceControlProvider ?? 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..4d7370512 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 @@ -23,7 +23,6 @@ import { normalizePrBodyAttributionAppMention, prActions, resolveSourceControlHostFromPayload, - resolveSourceControlProviderFromPayload, sourceControlProviderSchema, type PrAction, type SourceControlProvider, @@ -57,6 +56,7 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, type RepositoryRow, @@ -182,8 +182,10 @@ export async function createOrUpdateSourceControlPullRequestForTaskRun({ fetchImpl?: FetchImpl; }): Promise { const payloadRecord = getPayloadRecord(taskRun.payload); - const payloadProvider = - resolveSourceControlProviderFromPayload(payloadRecord); + const payloadProvider = resolveSourceControlProviderForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); const provider = input.sourceControlProvider ?? payloadProvider; From 246e5fd192fc22ffb4c9d6df7d7e13ec269e4c5c Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 10:48:51 +0000 Subject: [PATCH 04/12] fix: aggregate mixed-provider task credentials --- packages/ado/src/api.ts | 22 ++- packages/bitbucket/src/api.ts | 22 ++- packages/gitea/src/api.ts | 26 +++- .../src/__tests__/task-run-token.test.ts | 59 ++++++++ packages/github/src/api.ts | 17 ++- packages/gitlab/src/__tests__/api.test.ts | 51 +++++++ packages/gitlab/src/api.ts | 26 +++- .../__tests__/dequeue-helpers.test.ts | 109 +++++++++++++++ .../__tests__/dequeue-resume-task-run.test.ts | 25 ++++ .../__tests__/dequeue-task-run.test.ts | 24 ++++ .../__tests__/fetch-snapshot-env.test.ts | 45 ++++++- .../server/lib/task-runs/dequeue-helpers.ts | 127 +++++++++++++----- .../lib/task-runs/dequeue-resume-task-run.ts | 18 ++- .../server/lib/task-runs/dequeue-task-run.ts | 18 ++- .../lib/task-runs/fetch-snapshot-env.ts | 14 +- 15 files changed, 530 insertions(+), 73 deletions(-) diff --git a/packages/ado/src/api.ts b/packages/ado/src/api.ts index 3aa16975b..4b7a37227 100644 --- a/packages/ado/src/api.ts +++ b/packages/ado/src/api.ts @@ -1482,6 +1482,18 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { async function resolveAdoRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { + const filterForAdo = (repositoryNames: string[]) => { + const repositoryProviders = ( + taskRun.payload as { repositoryProviders?: Record } + ).repositoryProviders; + + return repositoryNames.filter( + (repositoryName) => + repositoryProviders?.[repositoryName] === undefined || + repositoryProviders[repositoryName] === ADO_PROVIDER, + ); + }; + if (taskRun.payload.environmentId) { const environment = await db.query.environments.findFirst({ where: eq(environments.id, taskRun.payload.environmentId), @@ -1493,9 +1505,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 +1520,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..69f09fa1d 100644 --- a/packages/bitbucket/src/api.ts +++ b/packages/bitbucket/src/api.ts @@ -754,6 +754,18 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { async function resolveBitbucketRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { + const filterForBitbucket = (repositoryNames: string[]) => { + const repositoryProviders = ( + taskRun.payload as { repositoryProviders?: Record } + ).repositoryProviders; + + return repositoryNames.filter( + (repositoryName) => + repositoryProviders?.[repositoryName] === undefined || + repositoryProviders[repositoryName] === BITBUCKET_PROVIDER, + ); + }; + if (taskRun.payload.environmentId) { const environment = await db.query.environments.findFirst({ where: eq(environments.id, taskRun.payload.environmentId), @@ -765,9 +777,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 +792,7 @@ async function resolveBitbucketRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterForBitbucket(selectedRepositories); } } diff --git a/packages/gitea/src/api.ts b/packages/gitea/src/api.ts index 31410af03..3c2732423 100644 --- a/packages/gitea/src/api.ts +++ b/packages/gitea/src/api.ts @@ -481,6 +481,21 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { return [...new Set(repositoryNames.filter(Boolean))]; } +function filterRepositorySelectionForGitea( + taskRun: TaskRun, + repositoryNames: string[], +): string[] { + const repositoryProviders = ( + taskRun.payload as { repositoryProviders?: Record } + ).repositoryProviders; + + return repositoryNames.filter( + (repositoryName) => + repositoryProviders?.[repositoryName] === undefined || + repositoryProviders[repositoryName] === GITEA_PROVIDER, + ); +} + async function resolveGiteaRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { @@ -495,9 +510,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 +526,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..e50db6bfb 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,64 @@ 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('still rejects unknown selected repository names', 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']), + ), + ).rejects.toThrow( + 'Selected repositories not found for task run 123: unknown/repository', + ); + }); + 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..d866ca97c 100644 --- a/packages/github/src/api.ts +++ b/packages/github/src/api.ts @@ -43,7 +43,22 @@ async function createTokenForRepositoryNames({ missingMessagePrefix: string; spanningMessagePrefix: string; }): Promise { - const uniqueRepositoryNames = [...new Set(repositoryNames.filter(Boolean))]; + const repositoryProviders = ( + taskRun.payload as { + repositoryProviders?: Record; + } + ).repositoryProviders; + const uniqueRepositoryNames = [ + ...new Set( + repositoryNames.filter( + (repositoryName) => + repositoryName && + (repositoryProviders?.[repositoryName] === undefined || + repositoryProviders[repositoryName] === + 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..06a95afb2 100644 --- a/packages/gitlab/src/__tests__/api.test.ts +++ b/packages/gitlab/src/__tests__/api.test.ts @@ -530,6 +530,57 @@ 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('still rejects unknown selected repository names', async () => { + await expect( + 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']), + ), + ).rejects.toThrow( + 'Selected GitLab repositories not found for task run 123: unknown/repository', + ); + }); + 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..fac3662dc 100644 --- a/packages/gitlab/src/api.ts +++ b/packages/gitlab/src/api.ts @@ -913,6 +913,21 @@ function normalizeRepositorySelection(repositoryNames: string[]): string[] { return [...new Set(repositoryNames.filter(Boolean))]; } +function filterRepositorySelectionForGitLab( + taskRun: TaskRun, + repositoryNames: string[], +): string[] { + const repositoryProviders = ( + taskRun.payload as { repositoryProviders?: Record } + ).repositoryProviders; + + return repositoryNames.filter( + (repositoryName) => + repositoryProviders?.[repositoryName] === undefined || + repositoryProviders[repositoryName] === GITLAB_PROVIDER, + ); +} + async function resolveGitLabRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { @@ -927,9 +942,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 +958,7 @@ async function resolveGitLabRepositoryNamesForTaskRun( ); if (selectedRepositories.length > 0) { - return selectedRepositories; + return filterRepositorySelectionForGitLab(taskRun, selectedRepositories); } } 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..2ff60d25b 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,88 @@ describe('createSourceControlTokenForTaskRun', () => { expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalled(); }); + it('mints mapped providers primary-first and merges aggregate metadata', async () => { + const taskRun = makeTaskRun({ + repo: 'owner/repo', + selectedRepositories: ['owner/repo', 'group/project'], + 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 the whole mapped-provider operation 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(2); + expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalledTimes(2); + } 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.'), @@ -519,6 +611,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..cbdf75e0e 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,28 @@ export function redactControlPlaneEnvVars( export function redactSourceControlProviderEnvVars( envVars: Record, - sourceControlProvider?: SourceControlProvider, + sourceControlProvider?: SourceControlProvider | SourceControlProvider[], ): Record { - if (sourceControlProvider === 'github') { - return envVars; - } - - const providerTokenEnvVars = - sourceControlProvider === 'gitlab' + 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 +140,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 +250,7 @@ function redactModelRuntimeManagedEnvVars( export async function fetchResolvedRuntimeEnvVars( deploymentEnvVars?: Record, options?: { - sourceControlProvider?: SourceControlProvider; + sourceControlProvider?: SourceControlProvider | SourceControlProvider[]; }, ): Promise> { const envVars = @@ -406,23 +413,44 @@ 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 orderedEntries = Object.entries(payload.repositoryProviders); + const primaryProvider = payload.repo + ? orderedEntries.find(([repository]) => repository === payload.repo)?.[1] + : undefined; + const providers = [ + ...(primaryProvider === undefined ? [] : [primaryProvider]), + ...orderedEntries + .filter(([repository]) => repository !== payload.repo) + .map(([, provider]) => provider), + ].map(normalizeSourceControlProvider); + + 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 +459,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 +473,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,6 +563,36 @@ async function createProviderToken( } } +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, + ); +} + /** * Creates a source-control token for the task run with retry logic. * Retries up to {@link SOURCE_CONTROL_TOKEN_MAX_RETRIES} times with @@ -550,12 +607,18 @@ export async function createSourceControlTokenForTaskRun( baseDelayMs = SOURCE_CONTROL_TOKEN_BASE_DELAY_MS, } = {}, ): Promise { - const provider = await resolveTaskRunSourceControlProvider(taskRun); - const label = getSourceControlProviderLabel(provider); + const providers = await resolveTaskRunSourceControlProviders(taskRun); + const label = providers.map(getSourceControlProviderLabel).join(' + '); for (let attempt = 1; attempt <= maxRetries; attempt++) { try { - return await createProviderToken(taskRun); + const tokens: SourceControlRuntimeToken[] = []; + + for (const provider of providers) { + tokens.push(await createProviderToken(taskRun, provider)); + } + + return mergeProviderTokens(tokens); } catch (error) { const message = error instanceof Error ? error.message : String(error); 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 }; } From ade690c56fe0c9f1d45b64ca1dfd3ddc88ad2da6 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 11:07:39 +0000 Subject: [PATCH 05/12] fix: preserve mixed-provider runtime context --- .../src/server/__tests__/enqueue-task.test.ts | 108 +++++++++++++++++- .../cloud-agents/src/server/task-run-queue.ts | 35 +++--- .../__tests__/source-control-provider.test.ts | 41 +++++++ .../db/src/lib/source-control-provider.ts | 17 ++- .../__tests__/dequeue-helpers.test.ts | 5 +- .../server/lib/task-runs/dequeue-helpers.ts | 19 +-- 6 files changed, 196 insertions(+), 29 deletions(-) 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 1d6bb4ee0..bbc44698b 100644 --- a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts +++ b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts @@ -900,6 +900,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 }, @@ -933,7 +937,17 @@ describe('enqueueTask snapshot resume', () => { ).toBe('gitea'); expect( (resumeRun.payload as { sourceControlHost?: string }).sourceControlHost, - ).toBeUndefined(); + ).toBe('dev.azure.com'); + expect( + ( + resumeRun.payload as { + repositoryProviders?: Record; + } + ).repositoryProviders, + ).toEqual({ + 'acme/widgets': 'ado', + 'group/web': 'gitlab', + }); }); it('rejects a resume without a source run id', async () => { @@ -1290,6 +1304,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); @@ -1347,6 +1365,13 @@ describe('enqueueTask source-control provider stamping', () => { 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); @@ -1386,6 +1411,80 @@ describe('enqueueTask source-control provider stamping', () => { }); }); + 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), + }, + } 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', + }, + }); + }); + it('recomputes mixed-provider stamps for a failed-start relaunch', async () => { const userId = await createUser(); const primaryRepository = await repositoryFactory.create({ @@ -1404,6 +1503,13 @@ describe('enqueueTask source-control provider stamping', () => { 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([ diff --git a/packages/cloud-agents/src/server/task-run-queue.ts b/packages/cloud-agents/src/server/task-run-queue.ts index 235199ad8..d0ec1de1b 100644 --- a/packages/cloud-agents/src/server/task-run-queue.ts +++ b/packages/cloud-agents/src/server/task-run-queue.ts @@ -1354,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}`, @@ -1787,16 +1791,17 @@ async function stampWorkspaceSourceControlProviders( workspace, ); const providers = Object.values(repositoryProviders); + const spansProviders = new Set(providers).size > 1; - if ( - new Set(providers).size > 1 && - payload.repositoryProviders === undefined - ) { + if (spansProviders) { payload.repositoryProviders = repositoryProviders; } const primaryProvider = providers[0]; - if (payload.sourceControlProvider === undefined && primaryProvider) { + if ( + primaryProvider && + (spansProviders || payload.sourceControlProvider === undefined) + ) { payload.sourceControlProvider = primaryProvider; } } @@ -2010,9 +2015,7 @@ function inheritSnapshotResumeSourceControlStamps( sourceControlHost?: unknown; }; - const inheritsProvider = payload.sourceControlProvider === undefined; - - if (inheritsProvider) { + if (payload.sourceControlProvider === undefined) { const provider = sourceControlProviderSchema.safeParse( source.sourceControlProvider, ); @@ -2020,19 +2023,19 @@ function inheritSnapshotResumeSourceControlStamps( if (provider.success) { payload.sourceControlProvider = provider.data; } + } - if (payload.repositoryProviders === undefined) { - const repositoryProviders = z - .record(sourceControlProviderSchema) - .safeParse(source.repositoryProviders); + if (payload.repositoryProviders === undefined) { + const repositoryProviders = z + .record(sourceControlProviderSchema) + .safeParse(source.repositoryProviders); - if (repositoryProviders.success) { - payload.repositoryProviders = repositoryProviders.data; - } + if (repositoryProviders.success) { + payload.repositoryProviders = repositoryProviders.data; } } - if (inheritsProvider && payload.sourceControlHost === undefined) { + if (payload.sourceControlHost === undefined) { const host = typeof source.sourceControlHost === 'string' ? source.sourceControlHost.trim() 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 ca808556b..377e5d4b7 100644 --- a/packages/db/src/lib/__tests__/source-control-provider.test.ts +++ b/packages/db/src/lib/__tests__/source-control-provider.test.ts @@ -12,6 +12,7 @@ let mockRows: Array<{ host: string | null; sourceControlProvider: 'github' | 'gitlab' | 'gitea' | 'ado' | 'bitbucket'; }> = []; +let mockEnvironmentRepositories: string[] = []; const query = { innerJoin: vi.fn(() => query), @@ -33,16 +34,30 @@ const dbOrTx = { select: vi.fn(() => ({ 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 () => { + mockEnvironmentRepositories = ['acme/Platform/backend']; mockRows = [ { fullName: 'acme/Platform/backend', @@ -59,6 +74,32 @@ 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 () => { mockRows = [ { diff --git a/packages/db/src/lib/source-control-provider.ts b/packages/db/src/lib/source-control-provider.ts index 47a14f1d3..2958d88c0 100644 --- a/packages/db/src/lib/source-control-provider.ts +++ b/packages/db/src/lib/source-control-provider.ts @@ -2,7 +2,11 @@ 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 @@ -91,6 +95,15 @@ async function resolveEnvironmentProviders( dbOrTx: DatabaseOrTransaction, environmentId: string, ): Promise> { + const environment = await dbOrTx.query.environments.findFirst({ + where: eq(environments.id, environmentId), + columns: { config: true }, + }); + + if (!environment) { + return {}; + } + const rows = await dbOrTx .select({ fullName: repositories.fullName, @@ -115,7 +128,7 @@ async function resolveEnvironmentProviders( return toRepositoryProviderMap( rows, - rows.map((row) => row.fullName), + environment.config.repositories.map((repository) => repository.repository), ); } 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 2ff60d25b..2ee48cc3f 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 @@ -402,10 +402,11 @@ describe('createSourceControlTokenForTaskRun', () => { expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalled(); }); - it('mints mapped providers primary-first and merges aggregate metadata', async () => { + it('mints the stamped primary provider first and merges aggregate metadata', async () => { const taskRun = makeTaskRun({ - repo: 'owner/repo', + repo: 'group/project', selectedRepositories: ['owner/repo', 'group/project'], + sourceControlProvider: 'github', repositoryProviders: { 'group/project': 'gitlab', 'owner/repo': 'github', 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 cbdf75e0e..6b1f02e6e 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts @@ -431,16 +431,19 @@ export async function resolveTaskRunSourceControlProviders( payload.repositoryProviders && Object.keys(payload.repositoryProviders).length > 0 ) { - const orderedEntries = Object.entries(payload.repositoryProviders); - const primaryProvider = payload.repo - ? orderedEntries.find(([repository]) => repository === payload.repo)?.[1] - : undefined; + 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]), - ...orderedEntries - .filter(([repository]) => repository !== payload.repo) - .map(([, provider]) => provider), - ].map(normalizeSourceControlProvider); + ...mappedProviders, + ]; return [...new Set(providers)]; } From fa8210498348b3dc42da5298d5f027ca55083ccf Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 4 Aug 2026 12:05:28 +0000 Subject: [PATCH 06/12] fix: complete mixed-provider repository routing --- .../setup/__tests__/workspace.test.ts | 11 ++++--- .../commands/setup/workspace/repositories.ts | 19 +++++++----- packages/gitlab/src/__tests__/api.test.ts | 29 +++++++++++++++++++ packages/gitlab/src/api.ts | 13 +++++++++ .../__tests__/source-control-issues.test.ts | 1 + .../lib/issues/source-control-issues.ts | 11 +++---- ...source-control-pull-request-shared.test.ts | 29 +++++++++++++++++++ .../source-control-pull-request-reads.ts | 7 +++-- .../source-control-pull-request-shared.ts | 17 +++++++++++ .../source-control-pull-request-writes.ts | 7 +++-- .../source-control-pull-requests.ts | 7 +++-- 11 files changed, 126 insertions(+), 25 deletions(-) diff --git a/apps/worker/src/commands/setup/__tests__/workspace.test.ts b/apps/worker/src/commands/setup/__tests__/workspace.test.ts index b26c73ef7..d35c01fc5 100644 --- a/apps/worker/src/commands/setup/__tests__/workspace.test.ts +++ b/apps/worker/src/commands/setup/__tests__/workspace.test.ts @@ -308,10 +308,6 @@ describe('initializeRepositories', () => { vi.spyOn(WorkspaceManager.prototype, 'configure').mockResolvedValue( undefined, ); - mockListRepositories.mockResolvedValue([ - { fullName: 'acme/github-app' }, - { fullName: 'acme/gitlab-app' }, - ]); const prepareRepositorySpy = vi .spyOn(WorkspaceManager.prototype, 'prepareRepository') .mockImplementation(async (repo) => `/tmp/${repo}`); @@ -320,10 +316,13 @@ describe('initializeRepositories', () => { workspace: { type: 'all_repositories' }, envVars: {}, taskRunType: TaskPayloadKind.StandardTask, - repositoryProviders: { 'acme/gitlab-app': 'gitlab' }, + repositoryProviders: { + 'acme/github-app': 'github', + 'acme/gitlab-app': 'gitlab', + }, }); - expect(mockListRepositories).toHaveBeenCalledTimes(1); + expect(mockListRepositories).not.toHaveBeenCalled(); expect(prepareRepositorySpy).toHaveBeenCalledWith( 'acme/github-app', undefined, diff --git a/apps/worker/src/commands/setup/workspace/repositories.ts b/apps/worker/src/commands/setup/workspace/repositories.ts index 242f4a3dd..a88edf676 100644 --- a/apps/worker/src/commands/setup/workspace/repositories.ts +++ b/apps/worker/src/commands/setup/workspace/repositories.ts @@ -90,6 +90,7 @@ export async function initializeRepositories( : { sourceControlProvider: resolvedSourceControlProvider }), ...(repositoryProviders ? { repositoryProviders } : {}), }; + const mappedRepositoryNames = Object.keys(repositoryProviders ?? {}); const { workspaceRoot, workspaceManager } = createWorkspaceManager( envVars, logger, @@ -154,14 +155,16 @@ export async function initializeRepositories( 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); diff --git a/packages/gitlab/src/__tests__/api.test.ts b/packages/gitlab/src/__tests__/api.test.ts index 06a95afb2..1a5fbeede 100644 --- a/packages/gitlab/src/__tests__/api.test.ts +++ b/packages/gitlab/src/__tests__/api.test.ts @@ -559,6 +559,35 @@ describe('createTaskRunScopedGitLabTokens', () => { 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('still rejects unknown selected repository names', async () => { await expect( createTaskRunScopedGitLabTokens( diff --git a/packages/gitlab/src/api.ts b/packages/gitlab/src/api.ts index fac3662dc..8ccb04c50 100644 --- a/packages/gitlab/src/api.ts +++ b/packages/gitlab/src/api.ts @@ -966,6 +966,19 @@ async function resolveGitLabRepositoryNamesForTaskRun( return [taskRun.payload.repo]; } + const repositoryProviders = ( + taskRun.payload as { repositoryProviders?: Record } + ).repositoryProviders; + 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/lib/issues/__tests__/source-control-issues.test.ts b/packages/sdk/src/server/lib/issues/__tests__/source-control-issues.test.ts index c4927c67c..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 @@ -180,6 +180,7 @@ describe('manageSourceControlIssueForTaskRun', () => { repo: 'acme/frontend', selectedRepositories: ['acme/frontend', 'acme/backend'], sourceControlProvider: 'github', + sourceControlHost: 'github.com', repositoryProviders: { 'acme/backend': 'gitlab' }, } as unknown as TaskRun['payload']; 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 7eb333844..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,13 +1,11 @@ import { type TaskRun } from '@roomote/db/server'; -import { - getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, -} from '@roomote/types'; +import { getSourceControlProviderLabel } from '@roomote/types'; import { assertRepositoryInTaskRunScope, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, resolveSourceControlProviderForRepositoryFromPayload, type FetchImpl, } from '../pull-requests/source-control-pull-request-shared'; @@ -66,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-shared.test.ts b/packages/sdk/src/server/lib/pull-requests/__tests__/source-control-pull-request-shared.test.ts index dc8e6dfb2..940573295 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 @@ -6,6 +6,7 @@ vi.mock('@roomote/gitlab', () => ({ import { buildGitLabTokenHeader, + resolveSourceControlHostForRepositoryFromPayload, resolveSourceControlProviderForRepositoryFromPayload, } from '../source-control-pull-request-shared'; @@ -35,6 +36,34 @@ describe('resolveSourceControlProviderForRepositoryFromPayload', () => { }); }); +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 repositories on the primary provider', () => { + expect( + resolveSourceControlHostForRepositoryFromPayload( + { + sourceControlProvider: 'github', + sourceControlHost: 'github.com', + repositoryProviders: { 'acme/backend': 'gitlab' }, + }, + 'acme/frontend', + ), + ).toBe('github.com'); + }); +}); + describe('buildGitLabTokenHeader', () => { it('uses the Bearer authorization header for OAuth tokens', () => { expect(buildGitLabTokenHeader('oauth-token')).toEqual({ 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 9336f5e35..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,7 +4,6 @@ import { type TaskRun } from '@roomote/db/server'; import { buildPullRequestUrl, getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -25,6 +24,7 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, @@ -613,7 +613,10 @@ export async function readSourceControlPullRequestForTaskRun({ payloadRecord, input.repositoryFullName, ); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + 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 31df99915..42037c171 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 @@ -11,6 +11,7 @@ import { environmentConfigSchema, getSourceControlProviderLabel, normalizeSourceControlProvider, + resolveSourceControlHostFromPayload, resolveSourceControlProviderFromPayload, type SourceControlProvider, } from '@roomote/types'; @@ -60,6 +61,22 @@ export function resolveSourceControlProviderForRepositoryFromPayload( return resolveSourceControlProviderFromPayload(payload); } +export function resolveSourceControlHostForRepositoryFromPayload( + payload: Record, + repositoryFullName: string, +): string | 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 55d5ef039..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,7 +3,6 @@ import { getOctokit } from '@roomote/github'; import { type TaskRun } from '@roomote/db/server'; import { getSourceControlProviderLabel, - resolveSourceControlHostFromPayload, sourceControlProviderSchema, type SourceControlProvider, } from '@roomote/types'; @@ -27,6 +26,7 @@ import { formatResponseBody, getPayloadRecord, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, @@ -237,7 +237,10 @@ export async function writeSourceControlPullRequestForTaskRun({ payloadRecord, input.repositoryFullName, ); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + 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 4d7370512..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,7 +22,6 @@ import { getSourceControlProviderLabel, normalizePrBodyAttributionAppMention, prActions, - resolveSourceControlHostFromPayload, sourceControlProviderSchema, type PrAction, type SourceControlProvider, @@ -56,6 +55,7 @@ import { isDraftTitle, isGitLabDraft, resolveRepositoryRow, + resolveSourceControlHostForRepositoryFromPayload, resolveSourceControlProviderForRepositoryFromPayload, splitRepositoryFullName, type FetchImpl, @@ -186,7 +186,10 @@ export async function createOrUpdateSourceControlPullRequestForTaskRun({ payloadRecord, input.repositoryFullName, ); - const payloadHost = resolveSourceControlHostFromPayload(payloadRecord); + const payloadHost = resolveSourceControlHostForRepositoryFromPayload( + payloadRecord, + input.repositoryFullName, + ); const provider = input.sourceControlProvider ?? payloadProvider; if (provider !== payloadProvider) { From cc7800c8332bad12956e612fed1b714bddf9e1fc Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:56:29 +0000 Subject: [PATCH 07/12] fix: allow mixed-provider repository selections --- .../tasks/__tests__/launchTask.test.ts | 51 +++++++++++++++---- .../automation-work-items/repositories.ts | 4 +- apps/api/src/handlers/tasks/launchTask.ts | 42 +++++---------- .../handlers/tasks/submitTaskSuggestions.ts | 4 +- .../src/lib/environment-definition.test.ts | 20 ++++++-- apps/web/src/lib/environment-definition.ts | 6 +-- .../src/lib/server/source-control-provider.ts | 46 ++++++++++------- .../trpc/commands/environments/index.test.ts | 6 +-- .../src/trpc/commands/environments/index.ts | 4 +- .../src/trpc/commands/setup-new/index.test.ts | 4 +- apps/web/src/trpc/commands/setup-new/index.ts | 4 +- .../setup-new/launch-lifecycle.test.ts | 2 +- .../src/trpc/commands/task-runs/index.test.ts | 35 +++++++++++++ apps/web/src/trpc/commands/task-runs/index.ts | 13 ++--- .../commands/task-suggestions/onboarding.ts | 4 +- .../src/server/__tests__/enqueue-task.test.ts | 43 ++++++++++++++++ .../types/src/environment-definition-tasks.ts | 31 +++-------- 17 files changed, 207 insertions(+), 112 deletions(-) diff --git a/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts b/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts index 24e06f7ce..ba8dd7cde 100644 --- a/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts +++ b/apps/api/src/handlers/tasks/__tests__/launchTask.test.ts @@ -356,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( @@ -378,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/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 93299c7ec..caaa32abb 100644 --- a/apps/api/src/handlers/tasks/launchTask.ts +++ b/apps/api/src/handlers/tasks/launchTask.ts @@ -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,22 +96,22 @@ 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) { 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/web/src/lib/environment-definition.test.ts b/apps/web/src/lib/environment-definition.test.ts index 80ec0c59b..b4d578938 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,20 @@ describe('environment definition helpers', () => { buildEnvironmentDefinitionWorkspacePayload(['acme/web', 'acme/api']), ).toEqual({ repo: '__all_repositories__', - selectedRepositories: ['acme/api', 'acme/web'], + selectedRepositories: ['acme/web', 'acme/api'], }); }); + 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 +134,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 6015d1e23..3bb79cea6 100644 --- a/apps/web/src/lib/server/source-control-provider.ts +++ b/apps/web/src/lib/server/source-control-provider.ts @@ -2,28 +2,40 @@ import type { SourceControlProvider } from '@roomote/types'; 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; +} + +export function resolvePrimarySourceControlProvider( + providers: SourceControlProvider[], +): SourceControlProvider | undefined { + return providers[0]; } /** diff --git a/apps/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index f8e16baab..bdbd9b291 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -155,11 +155,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' }, @@ -203,7 +203,7 @@ describe('startEnvironmentDefinitionTaskCommand', () => { }); expect(mockGetRepositoryEmptyStates).toHaveBeenCalledWith({ - repositoryIds: ['repo-1', 'repo-2'], + repositoryIds: ['repo-2', 'repo-1'], }); const enqueueInput = mockEnqueueTask.mock.calls[0]?.[0] as { diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index 4e390949f..6062ebd3e 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -216,9 +216,7 @@ async function resolveSelectedRepositories( return { normalizedRepositoryIds: normalizeRepositorySelection(selectedRepositories), - selectedRepositories: selectedRepositories.sort((left, right) => - left.fullName.localeCompare(right.fullName), - ), + selectedRepositories, }; } 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 6c8e98627..436d400db 100644 --- a/apps/web/src/trpc/commands/setup-new/index.test.ts +++ b/apps/web/src/trpc/commands/setup-new/index.test.ts @@ -1219,7 +1219,7 @@ describe('setup-new onboarding task start command', () => { ]); vi.mocked(buildSetupNewWorkspacePayload).mockReturnValue({ repo: ALL_REPOSITORIES, - selectedRepositories: ['group/web', 'octo/api'], + selectedRepositories: ['octo/api', 'group/web'], }); mockOnboardingTransaction({ slackInstallation: null, @@ -1232,7 +1232,7 @@ describe('setup-new onboarding task start command', () => { expect.objectContaining({ task: expect.objectContaining({ payload: expect.objectContaining({ - sourceControlProvider: 'gitlab', + sourceControlProvider: 'github', }), }), }), diff --git a/apps/web/src/trpc/commands/setup-new/index.ts b/apps/web/src/trpc/commands/setup-new/index.ts index 47bfbbe44..be876dc12 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -305,9 +305,7 @@ async function resolveSelectedRepositories(repositoryIds: string[]): Promise<{ return { normalizedRepositoryIds: normalizeRepositorySelection(selectedRepositories), - selectedRepositories: selectedRepositories.sort((left, right) => - left.fullName.localeCompare(right.fullName), - ), + selectedRepositories, }; } 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..a3135c0f7 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,7 @@ 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), + resolvePrimarySourceControlProvider: 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 5896c00c3..15fee8b0b 100644 --- a/apps/web/src/trpc/commands/task-runs/index.test.ts +++ b/apps/web/src/trpc/commands/task-runs/index.test.ts @@ -324,4 +324,39 @@ describe('createStandardTaskRunCommand', () => { }), ); }); + + 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..fc3c71915 100644 --- a/apps/web/src/trpc/commands/task-suggestions/onboarding.ts +++ b/apps/web/src/trpc/commands/task-suggestions/onboarding.ts @@ -25,7 +25,7 @@ import { } from '@roomote/types'; import { getLatestTaskRunsByTaskId } from '@/lib/server'; -import { resolveSingleSourceControlProvider } from '@/lib/server/source-control-provider'; +import { resolvePrimarySourceControlProvider } from '@/lib/server/source-control-provider'; import type { UserAuthSuccess } from '@/types'; import { assertAdmin } from '../setup/shared'; import { decorateSuggestionsWithEnvironmentIds } from './launch-resolution'; @@ -141,7 +141,7 @@ async function launchSuggestedTasksTask(input: { .select({ sourceControlProvider: repositories.sourceControlProvider }) .from(repositories) .where(inArray(repositories.fullName, input.repositoryFullNames)); - const scanSourceControlProvider = resolveSingleSourceControlProvider( + const scanSourceControlProvider = resolvePrimarySourceControlProvider( scanRepositoryRows.map((row) => row.sourceControlProvider), ); const launchResult = await enqueueTask( 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 bbc44698b..3f8278710 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, @@ -1411,6 +1412,48 @@ describe('enqueueTask source-control provider stamping', () => { }); }); + 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({ diff --git a/packages/types/src/environment-definition-tasks.ts b/packages/types/src/environment-definition-tasks.ts index b367f7143..4182ad6be 100644 --- a/packages/types/src/environment-definition-tasks.ts +++ b/packages/types/src/environment-definition-tasks.ts @@ -33,36 +33,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 +121,7 @@ export function buildEnvironmentDefinitionWorkspacePayload( repo: string; selectedRepositories?: string[]; } { - const normalizedRepositories = [...new Set(repositoryFullNames)].sort( - (left, right) => left.localeCompare(right), - ); + const normalizedRepositories = [...new Set(repositoryFullNames)]; const primaryRepository = normalizedRepositories[0]; if (!primaryRepository) { From eed726dd7a3e7dc6240dbea9d67d3f57ea54f278 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:22:14 +0000 Subject: [PATCH 08/12] fix: harden mixed-provider task routing --- apps/docs/environments.mdx | 5 +- .../src/lib/environment-definition.test.ts | 11 +++ .../src/trpc/commands/setup-new/index.test.ts | 28 +++++++ apps/web/src/trpc/commands/setup-new/index.ts | 5 ++ packages/ado/src/api.ts | 13 ++-- packages/bitbucket/src/api.ts | 13 ++-- .../src/server/__tests__/enqueue-task.test.ts | 11 +-- .../cloud-agents/src/server/task-run-queue.ts | 27 +++++-- .../db/src/lib/source-control-provider.ts | 9 +-- packages/gitea/src/api.ts | 13 ++-- .../src/__tests__/task-run-token.test.ts | 6 +- packages/github/src/api.ts | 16 ++-- packages/gitlab/src/__tests__/api.test.ts | 46 +++++++----- packages/gitlab/src/api.ts | 13 ++-- ...source-control-pull-request-shared.test.ts | 27 +++++-- .../source-control-pull-request-shared.ts | 31 ++++---- .../__tests__/dequeue-helpers.test.ts | 39 +++++++++- .../server/lib/task-runs/dequeue-helpers.ts | 74 ++++++++++++++----- .../src/__tests__/source-control.test.ts | 33 +++++++++ .../types/src/environment-definition-tasks.ts | 15 ++++ packages/types/src/source-control.ts | 26 +++++++ 21 files changed, 337 insertions(+), 124 deletions(-) diff --git a/apps/docs/environments.mdx b/apps/docs/environments.mdx index 218eff010..184c5de22 100644 --- a/apps/docs/environments.mdx +++ b/apps/docs/environments.mdx @@ -38,7 +38,10 @@ 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. +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 diff --git a/apps/web/src/lib/environment-definition.test.ts b/apps/web/src/lib/environment-definition.test.ts index b4d578938..c2021c943 100644 --- a/apps/web/src/lib/environment-definition.test.ts +++ b/apps/web/src/lib/environment-definition.test.ts @@ -112,6 +112,17 @@ describe('environment definition helpers', () => { }); }); + 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([ 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 436d400db..0b4cc6f56 100644 --- a/apps/web/src/trpc/commands/setup-new/index.test.ts +++ b/apps/web/src/trpc/commands/setup-new/index.test.ts @@ -1239,6 +1239,34 @@ describe('setup-new onboarding task start command', () => { ); }); + 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.toThrow( + 'The selected repositories include multiple entries named "group/project".', + ); + 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 be876dc12..e6858d0b3 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -63,6 +63,7 @@ import { TaskPayloadKind, resolveEvalHarnessSelection, type ComputeProvider, + assertUniqueRepositoryFullNames, type DeploymentModelConfig, deriveWorkerImageFromReleaseVersion, getSetupAuthProvider, @@ -303,6 +304,10 @@ async function resolveSelectedRepositories(repositoryIds: string[]): Promise<{ }); } + assertUniqueRepositoryFullNames( + selectedRepositories.map((repository) => repository.fullName), + ); + return { normalizedRepositoryIds: normalizeRepositorySelection(selectedRepositories), selectedRepositories, diff --git a/packages/ado/src/api.ts b/packages/ado/src/api.ts index 4b7a37227..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'; @@ -1483,14 +1484,10 @@ async function resolveAdoRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { const filterForAdo = (repositoryNames: string[]) => { - const repositoryProviders = ( - taskRun.payload as { repositoryProviders?: Record } - ).repositoryProviders; - - return repositoryNames.filter( - (repositoryName) => - repositoryProviders?.[repositoryName] === undefined || - repositoryProviders[repositoryName] === ADO_PROVIDER, + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + ADO_PROVIDER, ); }; diff --git a/packages/bitbucket/src/api.ts b/packages/bitbucket/src/api.ts index 69f09fa1d..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 { @@ -755,14 +756,10 @@ async function resolveBitbucketRepositoryNamesForTaskRun( taskRun: TaskRun, ): Promise { const filterForBitbucket = (repositoryNames: string[]) => { - const repositoryProviders = ( - taskRun.payload as { repositoryProviders?: Record } - ).repositoryProviders; - - return repositoryNames.filter( - (repositoryName) => - repositoryProviders?.[repositoryName] === undefined || - repositoryProviders[repositoryName] === BITBUCKET_PROVIDER, + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + BITBUCKET_PROVIDER, ); }; 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 3f8278710..4fff004ae 100644 --- a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts +++ b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts @@ -938,17 +938,14 @@ describe('enqueueTask snapshot resume', () => { ).toBe('gitea'); expect( (resumeRun.payload as { sourceControlHost?: string }).sourceControlHost, - ).toBe('dev.azure.com'); + ).toBeUndefined(); expect( ( resumeRun.payload as { repositoryProviders?: Record; } ).repositoryProviders, - ).toEqual({ - 'acme/widgets': 'ado', - 'group/web': 'gitlab', - }); + ).toBeUndefined(); }); it('rejects a resume without a source run id', async () => { @@ -1410,6 +1407,7 @@ describe('enqueueTask source-control provider stamping', () => { 'acme/Platform/mixed-web': 'ado', }, }); + expect(run.payload.sourceControlHost).toBeUndefined(); }); it('stamps mixed selected repositories in selection order', async () => { @@ -1502,6 +1500,8 @@ describe('enqueueTask source-control provider stamping', () => { 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' }, @@ -1526,6 +1526,7 @@ describe('enqueueTask source-control provider stamping', () => { 'octo/pr-target': 'gitea', }, }); + expect(run.payload.sourceControlHost).toBeUndefined(); }); it('recomputes mixed-provider stamps for a failed-start relaunch', async () => { diff --git a/packages/cloud-agents/src/server/task-run-queue.ts b/packages/cloud-agents/src/server/task-run-queue.ts index d0ec1de1b..6792a77f6 100644 --- a/packages/cloud-agents/src/server/task-run-queue.ts +++ b/packages/cloud-agents/src/server/task-run-queue.ts @@ -1802,6 +1802,13 @@ async function stampWorkspaceSourceControlProviders( primaryProvider && (spansProviders || payload.sourceControlProvider === undefined) ) { + if ( + spansProviders || + (payload.sourceControlProvider !== undefined && + payload.sourceControlProvider !== primaryProvider) + ) { + payload.sourceControlHost = undefined; + } payload.sourceControlProvider = primaryProvider; } } @@ -2015,17 +2022,21 @@ function inheritSnapshotResumeSourceControlStamps( sourceControlHost?: unknown; }; - if (payload.sourceControlProvider === undefined) { - const provider = sourceControlProviderSchema.safeParse( - source.sourceControlProvider, - ); + const inheritsProvider = payload.sourceControlProvider === undefined; + const provider = sourceControlProviderSchema.safeParse( + source.sourceControlProvider, + ); + if (inheritsProvider) { if (provider.success) { payload.sourceControlProvider = provider.data; } } - if (payload.repositoryProviders === undefined) { + const usesSourceProvider = + provider.success && payload.sourceControlProvider === provider.data; + + if (usesSourceProvider && payload.repositoryProviders === undefined) { const repositoryProviders = z .record(sourceControlProviderSchema) .safeParse(source.repositoryProviders); @@ -2035,7 +2046,11 @@ function inheritSnapshotResumeSourceControlStamps( } } - if (payload.sourceControlHost === undefined) { + if ( + inheritsProvider && + provider.success && + payload.sourceControlHost === undefined + ) { const host = typeof source.sourceControlHost === 'string' ? source.sourceControlHost.trim() diff --git a/packages/db/src/lib/source-control-provider.ts b/packages/db/src/lib/source-control-provider.ts index 2958d88c0..749990190 100644 --- a/packages/db/src/lib/source-control-provider.ts +++ b/packages/db/src/lib/source-control-provider.ts @@ -51,7 +51,7 @@ function toRepositoryProviderMap( if (matches.length > 1 && hostMatches.length !== 1) { console.warn( - `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${matches.length} active rows.`, + `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${matches.length} rows.`, ); continue; } @@ -81,12 +81,7 @@ async function resolveProvidersByFullNames( sourceControlProvider: repositories.sourceControlProvider, }) .from(repositories) - .where( - and( - eq(repositories.isActive, true), - inArray(repositories.fullName, fullNames), - ), - ); + .where(inArray(repositories.fullName, fullNames)); return toRepositoryProviderMap(rows, fullNames, sourceControlHost); } diff --git a/packages/gitea/src/api.ts b/packages/gitea/src/api.ts index 3c2732423..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 { @@ -485,14 +486,10 @@ function filterRepositorySelectionForGitea( taskRun: TaskRun, repositoryNames: string[], ): string[] { - const repositoryProviders = ( - taskRun.payload as { repositoryProviders?: Record } - ).repositoryProviders; - - return repositoryNames.filter( - (repositoryName) => - repositoryProviders?.[repositoryName] === undefined || - repositoryProviders[repositoryName] === GITEA_PROVIDER, + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + GITEA_PROVIDER, ); } diff --git a/packages/github/src/__tests__/task-run-token.test.ts b/packages/github/src/__tests__/task-run-token.test.ts index e50db6bfb..45a12225f 100644 --- a/packages/github/src/__tests__/task-run-token.test.ts +++ b/packages/github/src/__tests__/task-run-token.test.ts @@ -153,7 +153,7 @@ describe('createTaskRunGitHubToken', () => { }); }); - it('still rejects unknown selected repository names', async () => { + it('ignores selected repository names omitted from a provider map', async () => { mockFindMany.mockResolvedValue([ { fullName: 'ExampleOrg/example-backend', @@ -177,9 +177,7 @@ describe('createTaskRunGitHubToken', () => { }, } as TaskRun['payload']), ), - ).rejects.toThrow( - 'Selected repositories not found for task run 123: unknown/repository', - ); + ).resolves.toBe('ghs_test_token'); }); it('uses the environment repositories installation for environment tasks', async () => { diff --git a/packages/github/src/api.ts b/packages/github/src/api.ts index d866ca97c..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,19 +44,12 @@ async function createTokenForRepositoryNames({ missingMessagePrefix: string; spanningMessagePrefix: string; }): Promise { - const repositoryProviders = ( - taskRun.payload as { - repositoryProviders?: Record; - } - ).repositoryProviders; const uniqueRepositoryNames = [ ...new Set( - repositoryNames.filter( - (repositoryName) => - repositoryName && - (repositoryProviders?.[repositoryName] === undefined || - repositoryProviders[repositoryName] === - DEFAULT_SOURCE_CONTROL_PROVIDER), + filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames.filter(Boolean), + DEFAULT_SOURCE_CONTROL_PROVIDER, ), ), ]; diff --git a/packages/gitlab/src/__tests__/api.test.ts b/packages/gitlab/src/__tests__/api.test.ts index 1a5fbeede..4f6617f88 100644 --- a/packages/gitlab/src/__tests__/api.test.ts +++ b/packages/gitlab/src/__tests__/api.test.ts @@ -588,26 +588,36 @@ describe('createTaskRunScopedGitLabTokens', () => { expect(result.credentials[0]?.repositoryFullName).toBe('group/project'); }); - it('still rejects unknown selected repository names', async () => { - await expect( - 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']), + 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 }, ), - ).rejects.toThrow( - 'Selected GitLab repositories not found for task run 123: unknown/repository', ); + 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 () => { diff --git a/packages/gitlab/src/api.ts b/packages/gitlab/src/api.ts index 8ccb04c50..e15956d47 100644 --- a/packages/gitlab/src/api.ts +++ b/packages/gitlab/src/api.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { ALL_REPOSITORIES, buildRepositoryCloneUrl, + filterRepositoryNamesForSourceControlProvider, type SourceControlProvider, } from '@roomote/types'; import { @@ -917,14 +918,10 @@ function filterRepositorySelectionForGitLab( taskRun: TaskRun, repositoryNames: string[], ): string[] { - const repositoryProviders = ( - taskRun.payload as { repositoryProviders?: Record } - ).repositoryProviders; - - return repositoryNames.filter( - (repositoryName) => - repositoryProviders?.[repositoryName] === undefined || - repositoryProviders[repositoryName] === GITLAB_PROVIDER, + return filterRepositoryNamesForSourceControlProvider( + taskRun.payload, + repositoryNames, + GITLAB_PROVIDER, ); } 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 940573295..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 @@ -23,8 +23,8 @@ describe('resolveSourceControlProviderForRepositoryFromPayload', () => { ).toBe('gitlab'); }); - it('falls back to the task primary provider when the repository is unmapped', () => { - expect( + it('rejects repositories omitted from a provider map', () => { + expect(() => resolveSourceControlProviderForRepositoryFromPayload( { sourceControlProvider: 'github', @@ -32,7 +32,9 @@ describe('resolveSourceControlProviderForRepositoryFromPayload', () => { }, 'acme/frontend', ), - ).toBe('github'); + ).toThrow( + 'Repository acme/frontend is not mapped to a source control provider.', + ); }); }); @@ -50,18 +52,33 @@ describe('resolveSourceControlHostForRepositoryFromPayload', () => { ).toBeUndefined(); }); - it('keeps the scalar host for repositories on the primary provider', () => { + it('keeps the scalar host for legacy payloads without a provider map', () => { expect( resolveSourceControlHostForRepositoryFromPayload( { sourceControlProvider: 'github', sourceControlHost: 'github.com', - repositoryProviders: { 'acme/backend': 'gitlab' }, }, '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', () => { 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 42037c171..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 @@ -11,6 +11,7 @@ import { environmentConfigSchema, getSourceControlProviderLabel, normalizeSourceControlProvider, + resolveRepositoryProvidersFromPayload, resolveSourceControlHostFromPayload, resolveSourceControlProviderFromPayload, type SourceControlProvider, @@ -42,20 +43,18 @@ export function resolveSourceControlProviderForRepositoryFromPayload( payload: Record, repositoryFullName: string, ): SourceControlProvider { - const repositoryProviders = payload.repositoryProviders; - - if ( - repositoryProviders && - typeof repositoryProviders === 'object' && - !Array.isArray(repositoryProviders) - ) { - const repositoryProvider = (repositoryProviders as Record)[ - repositoryFullName - ]; - - if (repositoryProvider !== undefined) { - return normalizeSourceControlProvider(repositoryProvider); + 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); @@ -65,6 +64,12 @@ export function resolveSourceControlHostForRepositoryFromPayload( payload: Record, repositoryFullName: string, ): string | undefined { + const repositoryProviders = resolveRepositoryProvidersFromPayload(payload); + + if (repositoryProviders?.[repositoryFullName] !== undefined) { + return undefined; + } + const repositoryProvider = resolveSourceControlProviderForRepositoryFromPayload( payload, 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 2ee48cc3f..d6ddd38b7 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 @@ -451,7 +451,7 @@ describe('createSourceControlTokenForTaskRun', () => { ); }); - it('retries the whole mapped-provider operation and returns no partial token', async () => { + it('retries only the failing provider and returns no partial token', async () => { mockCreateTaskRunScopedGitLabTokens.mockRejectedValue( new Error('GitLab unavailable'), ); @@ -477,7 +477,7 @@ describe('createSourceControlTokenForTaskRun', () => { ); expect(result).toBeNull(); - expect(mockCreateTaskRunWorkerGitHubToken).toHaveBeenCalledTimes(2); + expect(mockCreateTaskRunWorkerGitHubToken).toHaveBeenCalledTimes(1); expect(mockCreateTaskRunScopedGitLabTokens).toHaveBeenCalledTimes(2); } finally { consoleWarnSpy.mockRestore(); @@ -485,6 +485,41 @@ describe('createSourceControlTokenForTaskRun', () => { } }); + 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.'), 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 6b1f02e6e..f01edad23 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts @@ -596,32 +596,18 @@ function mergeProviderTokens( ); } -/** - * 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( +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 providers = await resolveTaskRunSourceControlProviders(taskRun); - const label = providers.map(getSourceControlProviderLabel).join(' + '); + const label = getSourceControlProviderLabel(provider); for (let attempt = 1; attempt <= maxRetries; attempt++) { try { - const tokens: SourceControlRuntimeToken[] = []; - - for (const provider of providers) { - tokens.push(await createProviderToken(taskRun, provider)); - } - - return mergeProviderTokens(tokens); + return await createProviderToken(taskRun, provider); } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -642,6 +628,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/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/environment-definition-tasks.ts b/packages/types/src/environment-definition-tasks.ts index 4182ad6be..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'; @@ -121,6 +135,7 @@ export function buildEnvironmentDefinitionWorkspacePayload( repo: string; selectedRepositories?: string[]; } { + assertUniqueRepositoryFullNames(repositoryFullNames); const normalizedRepositories = [...new Set(repositoryFullNames)]; const primaryRepository = normalizedRepositories[0]; 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 From 315e0a06c201acf370a55aa3a380a859dc1d790e Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:33:52 +0000 Subject: [PATCH 09/12] fix: preserve deduplicated repository order --- packages/db/src/lib/__tests__/repositories.test.ts | 5 ++--- packages/db/src/lib/repositories.ts | 5 +++-- 2 files changed, 5 insertions(+), 5 deletions(-) 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/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( ( From 04ffc57400be40ea463d6ee9f5acc7f7d0e8a444 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:10:08 +0000 Subject: [PATCH 10/12] fix: address mixed-provider review follow-ups --- .../createEnvironment.validation.test.ts | 11 ++++ .../environments/createEnvironment.ts | 6 ++- .../__tests__/manageSourceControl.test.ts | 25 +++++++++ .../src/handlers/tasks/manageSourceControl.ts | 21 ++++---- .../src/lib/server/source-control-provider.ts | 6 --- .../trpc/commands/environments/index.test.ts | 30 +++++++++++ .../src/trpc/commands/environments/index.ts | 47 +++++++++++++---- .../src/trpc/commands/setup-new/index.test.ts | 25 +++++++-- apps/web/src/trpc/commands/setup-new/index.ts | 52 +++++++++++++++---- .../setup-new/launch-lifecycle.test.ts | 1 - .../commands/task-suggestions/onboarding.ts | 17 +++--- .../commands/setup/workspace/repositories.ts | 3 ++ .../__tests__/source-control-provider.test.ts | 43 +++++++++++++++ .../db/src/lib/source-control-provider.ts | 23 ++++---- packages/gitlab/src/api.ts | 7 +-- packages/sdk/src/server/index.ts | 5 +- .../__tests__/dequeue-helpers.test.ts | 14 +++++ .../server/lib/task-runs/dequeue-helpers.ts | 3 ++ packages/types/src/environment-config.ts | 18 +++++++ 19 files changed, 293 insertions(+), 64 deletions(-) diff --git a/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts index b24ddcf67..2c3c2e5fa 100644 --- a/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts +++ b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts @@ -20,4 +20,15 @@ describe('getEnvironmentRepositoryConfigError', () => { ]), ).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 active repositories are named "acme/app". Environment repository names must be unique across source-control connections.', + ); + }); }); diff --git a/apps/api/src/handlers/environments/createEnvironment.ts b/apps/api/src/handlers/environments/createEnvironment.ts index 4cb06fa45..5db3f7ad7 100644 --- a/apps/api/src/handlers/environments/createEnvironment.ts +++ b/apps/api/src/handlers/environments/createEnvironment.ts @@ -14,6 +14,7 @@ import { import { type TaskPayload, environmentConfigSchema, + getAmbiguousEnvironmentRepositoryError, getMissingEnvironmentRepositoryError, getEnvironmentRepositoryInstallationError, } from '@roomote/types'; @@ -69,7 +70,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 { diff --git a/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts b/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts index eabe469a9..7d0a10296 100644 --- a/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts +++ b/apps/api/src/handlers/tasks/__tests__/manageSourceControl.test.ts @@ -103,4 +103,29 @@ describe('manageSourceControl', () => { }), }); }); + + 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/manageSourceControl.ts b/apps/api/src/handlers/tasks/manageSourceControl.ts index 48d680ea1..8393277ad 100644 --- a/apps/api/src/handlers/tasks/manageSourceControl.ts +++ b/apps/api/src/handlers/tasks/manageSourceControl.ts @@ -6,9 +6,11 @@ import { completeClaimedLatestUserMessageForReplyQuote, restoreClaimedLatestUserMessageForReplyQuote, } from '@roomote/communication/messages'; +import { resolveRepositoryProvidersFromPayload } from '@roomote/types'; import { createOrUpdateSourceControlPullRequestForTaskRun, findTaskRunForSourceControlMutation, + getPayloadRecord, manageSourceControlIssueForTaskRun, readSourceControlPullRequestForTaskRun, resolveSourceControlProviderForRepositoryFromPayload, @@ -92,18 +94,19 @@ export async function manageSourceControl( runId: auth.authContext.runId, taskId, }); - const payload = - taskRun.payload && - typeof taskRun.payload === 'object' && - !Array.isArray(taskRun.payload) - ? (taskRun.payload as Record) - : {}; - const targetProvider = - input.sourceControlProvider ?? - resolveSourceControlProviderForRepositoryFromPayload( + 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 = targetProvider === 'github' && diff --git a/apps/web/src/lib/server/source-control-provider.ts b/apps/web/src/lib/server/source-control-provider.ts index 3bb79cea6..37b57343e 100644 --- a/apps/web/src/lib/server/source-control-provider.ts +++ b/apps/web/src/lib/server/source-control-provider.ts @@ -32,12 +32,6 @@ export function resolveSelectedRepositorySourceControlProvider( return undefined; } -export function resolvePrimarySourceControlProvider( - providers: SourceControlProvider[], -): SourceControlProvider | undefined { - return providers[0]; -} - /** * Resolve the provider for an environment-backed launch by delegating to the * shared resolver (single source of truth for the environment-repository join). diff --git a/apps/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index bdbd9b291..bef2f57b4 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -100,12 +100,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 active repositories are named "acme/app"'); + }); +}); + function buildMockAuth(): UserAuthSuccess { return { success: true, @@ -170,6 +182,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 active 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'], diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index 6062ebd3e..c0267cc0b 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,7 @@ import { type ComputeProvider, type EnvironmentConfig, environmentConfigSchema, + getAmbiguousEnvironmentRepositoryError, getEnvironmentRepositoryInstallationError, getMissingEnvironmentRepositoryError, isExitedRunStatus, @@ -140,14 +142,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 +202,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,7 +219,10 @@ async function resolveSelectedRepositories( getEnvironmentRepositoryConfigError(selectedRepositories); if (repositoryConfigError) { - throw new Error(repositoryConfigError); + throw new TRPCError({ + code: 'BAD_REQUEST', + message: repositoryConfigError, + }); } return { @@ -815,9 +826,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, }); 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 0b4cc6f56..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, @@ -229,6 +230,7 @@ import { } from './index'; import { ALL_REPOSITORIES, + createEmptySetupNewState, TaskPayloadKind, WORKER_RUNTIME_SCHEMA_VERSION, type SetupNewState, @@ -299,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(); @@ -1261,9 +1278,11 @@ describe('setup-new onboarding task start command', () => { await expect( startSetupNewOnboardingTaskCommand(buildMockAuth()), - ).rejects.toThrow( - 'The selected repositories include multiple entries named "group/project".', - ); + ).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(); }); diff --git a/apps/web/src/trpc/commands/setup-new/index.ts b/apps/web/src/trpc/commands/setup-new/index.ts index e6858d0b3..898b737e1 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -1,4 +1,5 @@ import * as GitHub from '@roomote/github'; +import { TRPCError } from '@trpc/server'; import { enqueueTask } from '@roomote/cloud-agents/server'; import { resolveEnvironmentSourceControlProvider } from '@/lib/server/source-control-provider'; import { buildSetupKickoffText } from '@roomote/communication/chat-messages'; @@ -294,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({ @@ -304,9 +308,20 @@ async function resolveSelectedRepositories(repositoryIds: string[]): Promise<{ }); } - assertUniqueRepositoryFullNames( - selectedRepositories.map((repository) => repository.fullName), - ); + 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), @@ -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,9 +2511,21 @@ 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 = 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 a3135c0f7..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), - resolvePrimarySourceControlProvider: vi.fn(() => null), })); vi.mock('@/lib/server', () => ({ diff --git a/apps/web/src/trpc/commands/task-suggestions/onboarding.ts b/apps/web/src/trpc/commands/task-suggestions/onboarding.ts index fc3c71915..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 { resolvePrimarySourceControlProvider } 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 = resolvePrimarySourceControlProvider( - 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/setup/workspace/repositories.ts b/apps/worker/src/commands/setup/workspace/repositories.ts index a88edf676..96c2a0e86 100644 --- a/apps/worker/src/commands/setup/workspace/repositories.ts +++ b/apps/worker/src/commands/setup/workspace/repositories.ts @@ -152,6 +152,9 @@ 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 })) 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 377e5d4b7..3da2ceed5 100644 --- a/packages/db/src/lib/__tests__/source-control-provider.test.ts +++ b/packages/db/src/lib/__tests__/source-control-provider.test.ts @@ -10,6 +10,7 @@ const mockOrderBy = vi.fn(); let mockRows: Array<{ fullName: string; host: string | null; + isActive?: boolean; sourceControlProvider: 'github' | 'gitlab' | 'gitea' | 'ado' | 'bitbucket'; }> = []; let mockEnvironmentRepositories: string[] = []; @@ -227,6 +228,48 @@ describe('resolveWorkspaceSourceControlProvider', () => { ).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 = [ diff --git a/packages/db/src/lib/source-control-provider.ts b/packages/db/src/lib/source-control-provider.ts index 749990190..9a1e22eb1 100644 --- a/packages/db/src/lib/source-control-provider.ts +++ b/packages/db/src/lib/source-control-provider.ts @@ -24,6 +24,7 @@ function toSingleProvider( type RepositoryProviderRow = { fullName: string; host: string | null; + isActive?: boolean; sourceControlProvider: SourceControlProvider; }; @@ -44,14 +45,16 @@ function toRepositoryProviderMap( 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 = - matches.length > 1 && sourceControlHost - ? matches.filter((row) => row.host === sourceControlHost) - : matches; + candidates.length > 1 && sourceControlHost + ? candidates.filter((row) => row.host === sourceControlHost) + : candidates; - if (matches.length > 1 && hostMatches.length !== 1) { + if (candidates.length > 1 && hostMatches.length !== 1) { console.warn( - `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${matches.length} rows.`, + `[resolveWorkspaceRepositoryProviders] Omitting ambiguous repository ${fullName}; matched ${candidates.length} candidate rows.`, ); continue; } @@ -78,6 +81,7 @@ async function resolveProvidersByFullNames( .select({ fullName: repositories.fullName, host: repositories.host, + isActive: repositories.isActive, sourceControlProvider: repositories.sourceControlProvider, }) .from(repositories) @@ -103,6 +107,7 @@ async function resolveEnvironmentProviders( .select({ fullName: repositories.fullName, host: repositories.host, + isActive: repositories.isActive, sourceControlProvider: repositories.sourceControlProvider, }) .from(environmentRepositoryMappings) @@ -134,6 +139,7 @@ async function resolveAllRepositoriesProviders( .select({ fullName: repositories.fullName, host: repositories.host, + isActive: repositories.isActive, sourceControlProvider: repositories.sourceControlProvider, }) .from(repositories) @@ -177,10 +183,9 @@ export async function resolveWorkspaceRepositoryProviders( * 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, diff --git a/packages/gitlab/src/api.ts b/packages/gitlab/src/api.ts index e15956d47..08e9d8638 100644 --- a/packages/gitlab/src/api.ts +++ b/packages/gitlab/src/api.ts @@ -4,6 +4,7 @@ import { ALL_REPOSITORIES, buildRepositoryCloneUrl, filterRepositoryNamesForSourceControlProvider, + resolveRepositoryProvidersFromPayload, type SourceControlProvider, } from '@roomote/types'; import { @@ -963,9 +964,9 @@ async function resolveGitLabRepositoryNamesForTaskRun( return [taskRun.payload.repo]; } - const repositoryProviders = ( - taskRun.payload as { repositoryProviders?: Record } - ).repositoryProviders; + const repositoryProviders = resolveRepositoryProvidersFromPayload( + taskRun.payload, + ); if (repositoryProviders) { const mappedRepositories = Object.entries(repositoryProviders) .filter(([, provider]) => provider === GITLAB_PROVIDER) diff --git a/packages/sdk/src/server/index.ts b/packages/sdk/src/server/index.ts index a7666126f..1c84a7056 100644 --- a/packages/sdk/src/server/index.ts +++ b/packages/sdk/src/server/index.ts @@ -255,7 +255,10 @@ export { type SourceControlPullRequestMutationInput, type SourceControlPullRequestMutationResult, } from './lib/pull-requests/source-control-pull-requests'; -export { resolveSourceControlProviderForRepositoryFromPayload } from './lib/pull-requests/source-control-pull-request-shared'; +export { + getPayloadRecord, + resolveSourceControlProviderForRepositoryFromPayload, +} from './lib/pull-requests/source-control-pull-request-shared'; export { readSourceControlPullRequestForTaskRun, sourceControlPullRequestReadInputSchema, 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 d6ddd38b7..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 @@ -637,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', 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 f01edad23..4a027dc80 100644 --- a/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts +++ b/packages/sdk/src/server/lib/task-runs/dequeue-helpers.ts @@ -83,6 +83,9 @@ export function redactSourceControlProviderEnvVars( envVars: Record, sourceControlProvider?: SourceControlProvider | SourceControlProvider[], ): Record { + // 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 diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 4f6f32993..6423e3386 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -1028,6 +1028,24 @@ export function hasEnvironmentOidcTargets( export const MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR = 'Environment repositories must all belong to the same GitHub App installation.'; +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 active 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; From 8b2d4cced764b8d0ab23094c249d80dd89fa812c Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:33:23 +0000 Subject: [PATCH 11/12] fix: reject duplicate environment repositories --- .../__tests__/environment-write-auth.test.ts | 39 +++++++++++++++++++ .../trpc/commands/environments/index.test.ts | 32 +++++++++++++++ .../src/trpc/commands/environments/index.ts | 31 ++++++++------- .../src/__tests__/command-schema.test.ts | 24 ++++++++++++ packages/types/src/environment-config.ts | 14 +++++++ 5 files changed, 126 insertions(+), 14 deletions(-) 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/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index bef2f57b4..972d54ccf 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -428,6 +428,38 @@ 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 () => { + 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).not.toHaveBeenCalled(); + }); + it('rejects update when a configured repository is not linked', async () => { mockDbSelect .mockReturnValueOnce({ diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index c0267cc0b..56a2711b5 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -563,16 +563,6 @@ export async function updateEnvironmentCommand( ): Promise { assertAdmin(auth); - const [env] = await db - .select() - .from(environments) - .where(and(eq(environments.id, input.id), buildOwnershipFilter())) - .limit(1); - - if (!env) { - return { success: false, error: 'Environment not found' }; - } - let nextConfig: EnvironmentConfig | undefined; if (input.config) { @@ -586,10 +576,23 @@ export async function updateEnvironmentCommand( } nextConfig = parseResult.data; - } else if ( - input.name !== undefined || - input.description !== undefined || - input.agentInstructions !== undefined + } + + const [env] = await db + .select() + .from(environments) + .where(and(eq(environments.id, input.id), buildOwnershipFilter())) + .limit(1); + + if (!env) { + return { success: false, error: 'Environment not found' }; + } + + if ( + !input.config && + (input.name !== undefined || + input.description !== undefined || + input.agentInstructions !== undefined) ) { const parseResult = environmentConfigSchema.safeParse({ ...(env.config as EnvironmentConfig), diff --git a/packages/types/src/__tests__/command-schema.test.ts b/packages/types/src/__tests__/command-schema.test.ts index 7d10be0c4..8cfe1cb36 100644 --- a/packages/types/src/__tests__/command-schema.test.ts +++ b/packages/types/src/__tests__/command-schema.test.ts @@ -431,6 +431,30 @@ commands: }); describe('environmentConfigSchema', () => { + it('rejects duplicate repository entries', () => { + const result = environmentConfigSchema.safeParse({ + name: 'Env', + repositories: [ + { repository: 'owner/repo' }, + { repository: 'owner/repo' }, + ], + }); + + expect(result.success).toBe(false); + if (result.success) { + throw new Error('Expected duplicate repositories to fail validation'); + } + + expect(result.error.issues).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + message: 'Duplicate repository: owner/repo', + path: ['repositories', 1, 'repository'], + }), + ]), + ); + }); + describe('tool_versions', () => { it('should accept root-level tool_versions for environment workspaces', () => { const result = environmentConfigSchema.safeParse({ diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 6423e3386..62b8d1f40 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -830,6 +830,20 @@ export const environmentConfigSchema = z .optional(), }) .superRefine((data, ctx) => { + const seenRepositories = new Set(); + + data.repositories.forEach((repository, index) => { + if (seenRepositories.has(repository.repository)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `Duplicate repository: ${repository.repository}`, + path: ['repositories', index, 'repository'], + }); + } + + seenRepositories.add(repository.repository); + }); + for (const conflict of getReservedEnvironmentPortConflicts(data)) { ctx.addIssue({ code: z.ZodIssueCode.custom, From 24b3d69734f32aaf670f3a27de6ec3885847ebb5 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:58:43 +0000 Subject: [PATCH 12/12] fix: preserve legacy environment configs --- .../createEnvironment.validation.test.ts | 2 +- .../environments/createEnvironment.ts | 14 ++++ .../environments/updateEnvironment.ts | 12 +++ .../trpc/commands/environments/index.test.ts | 76 ++++++++++++++++++- .../src/trpc/commands/environments/index.ts | 53 +++++++++---- .../src/__tests__/command-schema.test.ts | 28 ++++--- packages/types/src/environment-config.ts | 32 ++++---- 7 files changed, 169 insertions(+), 48 deletions(-) diff --git a/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts index 2c3c2e5fa..0f8bdde24 100644 --- a/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts +++ b/apps/api/src/handlers/environments/__tests__/createEnvironment.validation.test.ts @@ -28,7 +28,7 @@ describe('getEnvironmentRepositoryConfigError', () => { { fullName: 'acme/app', installationId: null }, ]), ).toBe( - 'Multiple active repositories are named "acme/app". Environment repository names must be unique across source-control connections.', + 'Multiple repositories are named "acme/app". Environment repository names must be unique across source-control connections.', ); }); }); diff --git a/apps/api/src/handlers/environments/createEnvironment.ts b/apps/api/src/handlers/environments/createEnvironment.ts index 5db3f7ad7..3b9e4b8c5 100644 --- a/apps/api/src/handlers/environments/createEnvironment.ts +++ b/apps/api/src/handlers/environments/createEnvironment.ts @@ -15,6 +15,7 @@ import { type TaskPayload, environmentConfigSchema, getAmbiguousEnvironmentRepositoryError, + getDuplicateEnvironmentRepositoryConfigError, getMissingEnvironmentRepositoryError, getEnvironmentRepositoryInstallationError, } from '@roomote/types'; @@ -256,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/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index 972d54ccf..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, @@ -114,7 +118,7 @@ describe('getEnvironmentRepositoryConfigError', () => { { id: 'repo-github', fullName: 'acme/app', installationId: '1' }, { id: 'repo-gitlab', fullName: 'acme/app', installationId: null }, ]), - ).toContain('Multiple active repositories are named "acme/app"'); + ).toContain('Multiple repositories are named "acme/app"'); }); }); @@ -195,7 +199,7 @@ describe('startEnvironmentDefinitionTaskCommand', () => { ).rejects.toMatchObject({ code: 'BAD_REQUEST', message: - 'Multiple active repositories are named "acme/app". Environment repository names must be unique across source-control connections.', + 'Multiple repositories are named "acme/app". Environment repository names must be unique across source-control connections.', }); expect(mockEnqueueTask).not.toHaveBeenCalled(); }); @@ -445,6 +449,24 @@ describe('environment repository validation', () => { }); 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: { @@ -457,7 +479,53 @@ describe('environment repository validation', () => { success: false, error: 'Invalid configuration: Duplicate repository: acme/api', }); - expect(mockDbSelect).not.toHaveBeenCalled(); + 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 () => { diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index 56a2711b5..204ed725a 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -39,6 +39,7 @@ import { type EnvironmentConfig, environmentConfigSchema, getAmbiguousEnvironmentRepositoryError, + getDuplicateEnvironmentRepositoryConfigError, getEnvironmentRepositoryInstallationError, getMissingEnvironmentRepositoryError, isExitedRunStatus, @@ -463,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) @@ -563,6 +575,16 @@ export async function updateEnvironmentCommand( ): Promise { assertAdmin(auth); + const [env] = await db + .select() + .from(environments) + .where(and(eq(environments.id, input.id), buildOwnershipFilter())) + .limit(1); + + if (!env) { + return { success: false, error: 'Environment not found' }; + } + let nextConfig: EnvironmentConfig | undefined; if (input.config) { @@ -575,24 +597,23 @@ export async function updateEnvironmentCommand( }; } - nextConfig = parseResult.data; - } - - const [env] = await db - .select() - .from(environments) - .where(and(eq(environments.id, input.id), buildOwnershipFilter())) - .limit(1); + const duplicateRepositoryError = + getDuplicateEnvironmentRepositoryConfigError( + parseResult.data.repositories ?? [], + ); - if (!env) { - return { success: false, error: 'Environment not found' }; - } + if (duplicateRepositoryError) { + return { + success: false, + error: `Invalid configuration: ${duplicateRepositoryError}`, + }; + } - if ( - !input.config && - (input.name !== undefined || - input.description !== undefined || - input.agentInstructions !== undefined) + nextConfig = parseResult.data; + } else if ( + input.name !== undefined || + input.description !== undefined || + input.agentInstructions !== undefined ) { const parseResult = environmentConfigSchema.safeParse({ ...(env.config as EnvironmentConfig), diff --git a/packages/types/src/__tests__/command-schema.test.ts b/packages/types/src/__tests__/command-schema.test.ts index 8cfe1cb36..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,7 +432,7 @@ commands: }); describe('environmentConfigSchema', () => { - it('rejects duplicate repository entries', () => { + it('keeps legacy duplicate repository entries parseable on read', () => { const result = environmentConfigSchema.safeParse({ name: 'Env', repositories: [ @@ -440,19 +441,22 @@ describe('environmentConfigSchema', () => { ], }); - expect(result.success).toBe(false); - if (result.success) { - throw new Error('Expected duplicate repositories to fail validation'); - } + expect(result.success).toBe(true); + }); - expect(result.error.issues).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - message: 'Duplicate repository: owner/repo', - path: ['repositories', 1, 'repository'], - }), + 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', () => { diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 62b8d1f40..1fee7654a 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -830,20 +830,6 @@ export const environmentConfigSchema = z .optional(), }) .superRefine((data, ctx) => { - const seenRepositories = new Set(); - - data.repositories.forEach((repository, index) => { - if (seenRepositories.has(repository.repository)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `Duplicate repository: ${repository.repository}`, - path: ['repositories', index, 'repository'], - }); - } - - seenRepositories.add(repository.repository); - }); - for (const conflict of getReservedEnvironmentPortConflicts(data)) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -1042,6 +1028,22 @@ 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 { @@ -1056,7 +1058,7 @@ export function getAmbiguousEnvironmentRepositoryError( }); return duplicate - ? `Multiple active repositories are named "${duplicate.fullName}". Environment repository names must be unique across source-control connections.` + ? `Multiple repositories are named "${duplicate.fullName}". Environment repository names must be unique across source-control connections.` : null; }