diff --git a/apps/api/src/handlers/environments/createEnvironment.ts b/apps/api/src/handlers/environments/createEnvironment.ts index 4cb06fa45..303fbfb12 100644 --- a/apps/api/src/handlers/environments/createEnvironment.ts +++ b/apps/api/src/handlers/environments/createEnvironment.ts @@ -15,7 +15,7 @@ import { type TaskPayload, environmentConfigSchema, getMissingEnvironmentRepositoryError, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, } from '@roomote/types'; import { captureActivationEnvironmentSaved } from '@roomote/telemetry/server'; @@ -66,10 +66,12 @@ export function isEnvironmentNameUniqueViolation(error: unknown): boolean { export function getEnvironmentRepositoryConfigError( repositoryRows: Array<{ fullName: string; + sourceControlProvider: string | null | undefined; + host: string | null | undefined; installationId: string | number | null | undefined; }>, ): string | null { - return getEnvironmentRepositoryInstallationError(repositoryRows); + return getEnvironmentRepositoryConnectionError(repositoryRows); } function extractRunId(auth: McpAuth): number | null { @@ -273,7 +275,13 @@ export async function createEnvironment( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; diff --git a/apps/api/src/handlers/environments/updateEnvironment.ts b/apps/api/src/handlers/environments/updateEnvironment.ts index 1b6e5a4e2..2c36ada60 100644 --- a/apps/api/src/handlers/environments/updateEnvironment.ts +++ b/apps/api/src/handlers/environments/updateEnvironment.ts @@ -132,7 +132,13 @@ export async function updateEnvironment( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; diff --git a/apps/api/src/handlers/tasks/launchTask.ts b/apps/api/src/handlers/tasks/launchTask.ts index 3080e2613..17c003a60 100644 --- a/apps/api/src/handlers/tasks/launchTask.ts +++ b/apps/api/src/handlers/tasks/launchTask.ts @@ -18,7 +18,7 @@ import { ADMIN_REQUIRED_LAUNCH_TYPES, ALL_REPOSITORIES, buildTaskTypePromptAndWorkspacePayload, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, type StandardTask, type SuggestedTasksTask, TaskPayloadKind, @@ -48,7 +48,7 @@ function normalizeRepositoryFullNames(body: TaskLaunchRequest): string[] { async function validateSelectedRepositories( repositoryFullNames: string[], - options: { requireSingleInstallation?: boolean } = {}, + options: { requireSingleConnection?: boolean } = {}, ): Promise<{ error: string; status: 400 | 404 } | null> { if (repositoryFullNames.length === 0) { return null; @@ -59,7 +59,12 @@ async function validateSelectedRepositories( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryFullNames), ), - columns: { fullName: true, installationId: true }, + columns: { + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }); const foundRepositories = new Set( @@ -76,12 +81,12 @@ async function validateSelectedRepositories( }; } - if (options.requireSingleInstallation) { - const installationError = - getEnvironmentRepositoryInstallationError(orgRepositories); + if (options.requireSingleConnection) { + const connectionError = + getEnvironmentRepositoryConnectionError(orgRepositories); - if (installationError) { - return { error: installationError, status: 400 }; + if (connectionError) { + return { error: connectionError, status: 400 }; } } @@ -223,7 +228,7 @@ export async function launchTask( const repositoryValidationError = shouldValidateRepositorySelection ? await validateSelectedRepositories(repositoryFullNames, { - requireSingleInstallation: requestedType === 'environment-definition', + requireSingleConnection: requestedType === 'environment-definition', }) : null; diff --git a/apps/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index b65bedf87..069b19ade 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -18,11 +18,15 @@ const { { id: 'repo-1', fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', installationId: 'installation-1', }, { id: 'repo-2', fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', installationId: 'installation-1', }, ]), @@ -93,7 +97,11 @@ vi.mock('@/lib/server', () => ({ getRepositories: mockGetRepositories, })); -import { TaskPayloadKind } from '@roomote/types'; +import { + MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + TaskPayloadKind, +} from '@roomote/types'; import type { UserAuthSuccess } from '@/types'; import { createEnvironmentCommand, @@ -392,6 +400,85 @@ describe('environment repository validation', () => { }); }); + it('rejects create when repositories span multiple instances of a provider', async () => { + mockDbSelect + .mockReturnValueOnce({ + from: () => ({ where: () => ({ limit: async () => [] }) }), + }) + .mockReturnValueOnce({ + from: () => ({ + where: async () => [ + { + id: 'repo-1', + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: 'gitea-one.example.com', + installationId: null, + }, + { + id: 'repo-2', + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea-two.example.com', + installationId: null, + }, + ], + }), + }); + + const result = await createEnvironmentCommand(buildMockAuth(), { + name: 'Gitea Test', + config: { + name: 'Gitea Test', + repositories: [{ repository: 'acme/api' }, { repository: 'acme/web' }], + }, + }); + + expect(result).toEqual({ + success: false, + error: MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + }); + }); + + it('reports mixed providers from validateConfigCommand', async () => { + mockDbSelect.mockReturnValueOnce({ + from: () => ({ + where: async () => [ + { + id: 'repo-1', + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + id: 'repo-2', + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ], + }), + }); + mockCheckRepoAccess.mockResolvedValue(true); + mockGetBranches.mockResolvedValue(['main']); + + const result = await validateConfigCommand(buildMockAuth(), { + config: { + name: 'Mixed Test', + repositories: [ + { repository: 'acme/api', branch: 'main' }, + { repository: 'acme/web', branch: 'main' }, + ], + }, + }); + + expect(result.errors).toContain( + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + ); + }); + it('does not validate Azure DevOps branches through GitHub', 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 b781300de..30971de6d 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -37,7 +37,7 @@ import { type ComputeProvider, type EnvironmentConfig, environmentConfigSchema, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, getMissingEnvironmentRepositoryError, isExitedRunStatus, normalizeRepositorySelection, @@ -131,21 +131,27 @@ function assertAdmin(auth: UserAuthSuccess) { type SelectedRepositorySummary = { id: string; fullName: string; + sourceControlProvider: string; + host: string | null; installationId: string | null; }; type EnvironmentRepositoryRow = { id: string; fullName: string; + sourceControlProvider: string; + host: string | null; installationId: string | null; }; function getEnvironmentRepositoryConfigError( repositoriesToValidate: EnvironmentRepositoryRow[], ): string | null { - return getEnvironmentRepositoryInstallationError( + return getEnvironmentRepositoryConnectionError( repositoriesToValidate.map((repository) => ({ fullName: repository.fullName, + sourceControlProvider: repository.sourceControlProvider, + host: repository.host, installationId: repository.installationId, })), ); @@ -168,6 +174,8 @@ async function getEnvironmentRepositoryRows( .select({ id: repositories.id, fullName: repositories.fullName, + sourceControlProvider: repositories.sourceControlProvider, + host: repositories.host, installationId: repositories.installationId, }) .from(repositories) @@ -203,6 +211,8 @@ async function resolveSelectedRepositories( return { id: repository.id, fullName: repository.fullName, + sourceControlProvider: repository.sourceControlProvider, + host: repository.host, installationId: repository.installationId, }; }); @@ -1203,6 +1213,7 @@ export async function validateConfigCommand( fullName: repositories.fullName, installationId: repositories.installationId, sourceControlProvider: repositories.sourceControlProvider, + host: repositories.host, }) .from(repositories) .where( diff --git a/packages/db/src/lib/declarative-environments.ts b/packages/db/src/lib/declarative-environments.ts index 24a0e70c9..075a00bca 100644 --- a/packages/db/src/lib/declarative-environments.ts +++ b/packages/db/src/lib/declarative-environments.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import type { EnvironmentConfig } from '@roomote/types'; import { environmentConfigSchema, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, getMissingEnvironmentRepositoryError, } from '@roomote/types'; import { and, eq, inArray, isNotNull, notInArray } from 'drizzle-orm'; @@ -364,7 +364,13 @@ async function resolveConfiguredRepositories(config: EnvironmentConfig) { eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; @@ -473,7 +479,7 @@ async function applyDefinition( await resolveConfiguredRepositories(config); const repositoryConfigError = - getEnvironmentRepositoryInstallationError(repositoryRows); + getEnvironmentRepositoryConnectionError(repositoryRows); if (repositoryConfigError) { return { skip: { source, reason: repositoryConfigError } }; diff --git a/packages/types/src/__tests__/environment-repository-connection.test.ts b/packages/types/src/__tests__/environment-repository-connection.test.ts new file mode 100644 index 000000000..30aad4f46 --- /dev/null +++ b/packages/types/src/__tests__/environment-repository-connection.test.ts @@ -0,0 +1,147 @@ +// pnpm --filter @roomote/types test src/__tests__/environment-repository-connection.test.ts + +import { + MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + getEnvironmentRepositoryConnectionError, +} from '../environment-config'; + +describe('getEnvironmentRepositoryConnectionError', () => { + it('accepts an empty repository list', () => { + expect(getEnvironmentRepositoryConnectionError([])).toBeNull(); + }); + + it('accepts repositories from a single connection', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + ]), + ).toBeNull(); + }); + + it('rejects repositories that span multiple providers', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ]), + ).toBe(MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('rejects repositories that span multiple GitHub App installations', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'other/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-2', + }, + ]), + ).toBe(MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('rejects repositories that span multiple instances of the same provider', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: 'gitea-one.example.com', + installationId: null, + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea-two.example.com', + installationId: null, + }, + ]), + ).toBe(MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('ignores null hosts on rows that predate host backfill', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: null, + installationId: null, + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ]), + ).toBeNull(); + }); + + it('ignores missing provider metadata', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: undefined, + host: undefined, + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + ]), + ).toBeNull(); + }); + + it('reports the provider mismatch before installation or host mismatches', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: 'installation-2', + }, + ]), + ).toBe(MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR); + }); +}); diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 4f6f32993..eb7cec4aa 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -1025,31 +1025,68 @@ export function hasEnvironmentOidcTargets( return getEnvironmentOidcTargets(config).length > 0; } +export const MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR = + 'Environment repositories must all belong to the same source control provider.'; + export const MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR = 'Environment repositories must all belong to the same GitHub App installation.'; -type EnvironmentRepositoryInstallationReference = { +export const MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR = + 'Environment repositories must all belong to the same source control instance.'; + +type EnvironmentRepositoryConnectionReference = { fullName: string; + sourceControlProvider: string | null | undefined; + /** + * Source-control host (e.g. a self-managed Gitea/GitLab instance). Nullable + * for historical rows that predate host backfill, so only non-null values + * participate in the same-instance comparison. + */ + host: string | null | undefined; installationId: string | number | null | undefined; }; -export function getEnvironmentRepositoryInstallationError( - repositories: EnvironmentRepositoryInstallationReference[], +function distinctDefined(values: Array): Set { + return new Set( + values.filter((value): value is T => value !== null && value !== undefined), + ); +} + +/** + * Environments must not mix repositories from different source control + * connections: one provider, one GitHub App installation, and one + * provider instance (host) per environment. Task launch resolves a single + * provider token and base URL per run, so a mixed environment could never + * authenticate against all of its repositories. + */ +export function getEnvironmentRepositoryConnectionError( + repositories: EnvironmentRepositoryConnectionReference[], ): string | null { - const installationIds = new Set( - repositories - .map((repository) => repository.installationId) - .filter( - (installationId): installationId is string | number => - installationId !== null && installationId !== undefined, - ), + const providers = distinctDefined( + repositories.map((repository) => repository.sourceControlProvider), ); - if (installationIds.size <= 1) { - return null; + if (providers.size > 1) { + return MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR; + } + + const installationIds = distinctDefined( + repositories.map((repository) => repository.installationId), + ); + + if (installationIds.size > 1) { + return MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR; + } + + const hosts = distinctDefined( + repositories.map((repository) => repository.host), + ); + + if (hosts.size > 1) { + return MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR; } - return MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR; + return null; } export function getMissingEnvironmentRepositoryError(