diff --git a/docs/architecture/04-agent-system.md b/docs/architecture/04-agent-system.md index eb583a7a..e385c82e 100644 --- a/docs/architecture/04-agent-system.md +++ b/docs/architecture/04-agent-system.md @@ -162,7 +162,7 @@ interface CapabilityDefinition { ```mermaid flowchart TD A["Agent definition
(capabilities.required + optional)"] --> B[Create integration checker] - B --> C["Check hasPmIntegration(),
hasScmIntegration(),
hasAlertingIntegration()"] + B --> C["integrationRegistry.getByCategory(cat)
.hasIntegration(projectId)
for pm, scm, alerting"] C --> D[resolveEffectiveCapabilities] D --> E["Built-in caps: always included"] D --> F["Integration caps: only if provider configured"] diff --git a/src/agents/capabilities/resolver.ts b/src/agents/capabilities/resolver.ts index c22e34a2..9cb5618f 100644 --- a/src/agents/capabilities/resolver.ts +++ b/src/agents/capabilities/resolver.ts @@ -44,6 +44,7 @@ import { Tmux } from '../../gadgets/tmux.js'; import { TodoDelete, TodoUpdateStatus, TodoUpsert } from '../../gadgets/todo/index.js'; import { VerifyChanges } from '../../gadgets/VerifyChanges.js'; import { WriteFile } from '../../gadgets/WriteFile.js'; +import { integrationRegistry } from '../../integrations/registry.js'; import type { ToolManifest } from '../contracts/index.js'; import type { IntegrationCategory } from '../definitions/schema.js'; import { @@ -378,28 +379,30 @@ export function generateUnavailableCapabilitiesNote(unavailableCaps: Capability[ * * This function pre-fetches integration availability for all categories * and returns a synchronous checker callback. + * + * Uses integrationRegistry.getByCategory() to check all registered integrations + * for each category — returns true if any integration in that category is configured. */ export async function createIntegrationChecker(projectId: string): Promise { - // Import integration checking functions dynamically to avoid circular deps - const [{ hasPmIntegration }, { hasScmIntegration }, { hasAlertingIntegration }] = - await Promise.all([ - import('../../pm/integration.js'), - import('../../github/integration.js'), - import('../../sentry/integration.js'), - ]); - - // Pre-fetch all integration statuses in parallel - const [hasPm, hasScm, hasAlerting] = await Promise.all([ - hasPmIntegration(projectId), - hasScmIntegration(projectId), - hasAlertingIntegration(projectId), - ]); + const categories: IntegrationCategory[] = ['pm', 'scm', 'alerting']; + + // Pre-fetch all integration statuses in parallel across all categories + const results = await Promise.all( + categories.map(async (cat) => { + const integrations = integrationRegistry.getByCategory(cat); + // Category is available if ANY registered integration for it is configured + const statuses = await Promise.all( + integrations.map((integration) => integration.hasIntegration(projectId)), + ); + return statuses.some(Boolean); + }), + ); // Return synchronous checker const availableIntegrations: Record = { - pm: hasPm, - scm: hasScm, - alerting: hasAlerting, + pm: results[0], + scm: results[1], + alerting: results[2], }; return (category: IntegrationCategory) => availableIntegrations[category] ?? false; diff --git a/src/github/integration.ts b/src/github/integration.ts deleted file mode 100644 index 9eccd21a..00000000 --- a/src/github/integration.ts +++ /dev/null @@ -1,37 +0,0 @@ -/** - * SCM (GitHub) integration — credential validation helpers. - * - * Provides hasScmIntegration() for checking if SCM integration is configured. - */ - -import { getIntegrationCredentialOrNull } from '../config/provider.js'; -import { getIntegrationProvider } from '../db/repositories/credentialsRepository.js'; - -/** - * Check if SCM integration is configured for a project. - * Returns true if the integration exists and has at least one token linked. - */ -export async function hasScmIntegration(projectId: string): Promise { - const provider = await getIntegrationProvider(projectId, 'scm'); - if (!provider) return false; - - // Check if either token is available (some agents only need one) - const [impl, rev] = await Promise.all([ - getIntegrationCredentialOrNull(projectId, 'scm', 'implementer_token'), - getIntegrationCredentialOrNull(projectId, 'scm', 'reviewer_token'), - ]); - - return impl !== null || rev !== null; -} - -/** - * Check if a specific SCM persona token is configured. - */ -export async function hasScmPersonaToken( - projectId: string, - persona: 'implementer' | 'reviewer', -): Promise { - const role = persona === 'implementer' ? 'implementer_token' : 'reviewer_token'; - const token = await getIntegrationCredentialOrNull(projectId, 'scm', role); - return token !== null; -} diff --git a/src/github/scm-integration.ts b/src/github/scm-integration.ts index 46c860bd..aedc6baf 100644 --- a/src/github/scm-integration.ts +++ b/src/github/scm-integration.ts @@ -4,13 +4,10 @@ * Encapsulates GitHub SCM credential resolution and validation * into a unified integration class following the IntegrationModule pattern. * - * Consolidates: - * - `hasScmIntegration()` logic from src/github/integration.ts - * - `hasScmPersonaToken()` logic from src/github/integration.ts - * - `withGitHubToken()` usage from src/github/client.ts - * - * Backward compatibility: the standalone functions in src/github/integration.ts - * remain exported and continue to work identically. + * Provides: + * - `hasIntegration()` — checks if at least one token (implementer or reviewer) is configured + * - `hasPersonaToken()` — checks if a specific persona token is configured + * - `withCredentials()` — runs a function within the implementer token credential scope */ import { getIntegrationCredential, getIntegrationCredentialOrNull } from '../config/provider.js'; diff --git a/src/pm/index.ts b/src/pm/index.ts index 332e2894..c8ec7d0b 100644 --- a/src/pm/index.ts +++ b/src/pm/index.ts @@ -1,7 +1,6 @@ export { getPMProvider, getPMProviderOrNull, withPMProvider } from './context.js'; // PMIntegration interface + registry export type { PMIntegration, PMWebhookEvent } from './integration.js'; -export { hasPmIntegration } from './integration.js'; export { JiraPMProvider } from './jira/adapter.js'; export type { ProjectPMConfig } from './lifecycle.js'; export { hasAutoLabel, PMLifecycleManager, resolveProjectPMConfig } from './lifecycle.js'; diff --git a/src/pm/integration.ts b/src/pm/integration.ts index cfe3b157..8f915cbc 100644 --- a/src/pm/integration.ts +++ b/src/pm/integration.ts @@ -11,9 +11,6 @@ * Extends IntegrationModule so PM providers participate in the unified registry. */ -import { PROVIDER_CREDENTIAL_ROLES } from '../config/integrationRoles.js'; -import { getIntegrationCredentialOrNull } from '../config/provider.js'; -import { getIntegrationProvider } from '../db/repositories/credentialsRepository.js'; import type { IntegrationModule } from '../integrations/types.js'; import type { AgentExecutionConfig } from '../triggers/shared/agent-execution.js'; import type { CascadeConfig, ProjectConfig } from '../types/index.js'; @@ -92,28 +89,3 @@ export interface PMIntegration extends IntegrationModule { /** Extract a work item ID from text (e.g. PR body). Returns null if not found. */ extractWorkItemId(text: string): string | null; } - -// ============================================================================ -// Integration check helpers -// ============================================================================ - -/** - * Check if PM integration is configured for a project. - * Returns true if a PM integration exists with all required credentials present. - * - * Uses the data-driven PROVIDER_CREDENTIAL_ROLES table so this function - * does not need to be updated when a new PM provider is added. - */ -export async function hasPmIntegration(projectId: string): Promise { - const provider = await getIntegrationProvider(projectId, 'pm'); - if (!provider) return false; - - const roles = PROVIDER_CREDENTIAL_ROLES[provider as keyof typeof PROVIDER_CREDENTIAL_ROLES]; - if (!roles || roles.length === 0) return false; - - const requiredRoles = roles.filter((r) => !r.optional); - const values = await Promise.all( - requiredRoles.map((roleDef) => getIntegrationCredentialOrNull(projectId, 'pm', roleDef.role)), - ); - return values.every((v) => v !== null); -} diff --git a/src/sentry/alerting-integration.ts b/src/sentry/alerting-integration.ts index 43cfc4d0..3f44e8ec 100644 --- a/src/sentry/alerting-integration.ts +++ b/src/sentry/alerting-integration.ts @@ -4,21 +4,13 @@ * Encapsulates Sentry alerting credential resolution and validation * into a unified integration class following the IntegrationModule pattern. * - * Consolidates: - * - `getSentryIntegrationConfig()` logic from src/sentry/integration.ts - * - `hasAlertingIntegration()` logic from src/sentry/integration.ts - * - * Backward compatibility: the standalone functions in src/sentry/integration.ts - * remain exported and continue to work identically. + * Inlines the hasIntegration logic directly (calls getSentryIntegrationConfig) + * rather than delegating to the now-deleted hasAlertingIntegration() standalone function. */ import { getIntegrationCredential } from '../config/provider.js'; import type { AlertingIntegration } from '../integrations/alerting.js'; -import { - getSentryIntegrationConfig, - hasAlertingIntegration, - type SentryIntegrationConfig, -} from './integration.js'; +import { getSentryIntegrationConfig, type SentryIntegrationConfig } from './integration.js'; export class SentryAlertingIntegration implements AlertingIntegration { readonly type = 'sentry'; @@ -26,15 +18,15 @@ export class SentryAlertingIntegration implements AlertingIntegration { /** * Check if Sentry alerting integration is configured for a project. - * Delegates to existing hasAlertingIntegration() logic. + * Returns true if getSentryIntegrationConfig returns a valid config. */ async hasIntegration(projectId: string): Promise { - return hasAlertingIntegration(projectId); + const config = await getSentryIntegrationConfig(projectId); + return config !== null; } /** * Get the Sentry integration config for a project. - * Delegates to existing getSentryIntegrationConfig() logic. */ async getConfig(projectId: string): Promise { return getSentryIntegrationConfig(projectId); diff --git a/src/sentry/integration.ts b/src/sentry/integration.ts index e8894ca2..54acdb18 100644 --- a/src/sentry/integration.ts +++ b/src/sentry/integration.ts @@ -37,12 +37,3 @@ export async function getSentryIntegrationConfig( organizationSlug: config.organizationSlug, }; } - -/** - * Returns true if a Sentry alerting integration is configured for the project. - * Used by createIntegrationChecker() in the capability resolver. - */ -export async function hasAlertingIntegration(projectId: string): Promise { - const config = await getSentryIntegrationConfig(projectId); - return config !== null; -} diff --git a/tests/integration/integration-validation.test.ts b/tests/integration/integration-validation.test.ts index e2fadcc7..205180c3 100644 --- a/tests/integration/integration-validation.test.ts +++ b/tests/integration/integration-validation.test.ts @@ -12,13 +12,13 @@ */ import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; -import { hasScmIntegration, hasScmPersonaToken } from '../../src/github/integration.js'; // Bootstrap the integration registry so validateIntegrations() can find registered modules. // The new registry-driven implementation requires integrations to be registered before // calling getByCategory() — without this import the registry is empty and all validations // report "none is registered" instead of checking actual project credentials. import '../../src/integrations/bootstrap.js'; -import { hasPmIntegration } from '../../src/pm/integration.js'; +import { integrationRegistry } from '../../src/integrations/registry.js'; +import type { SCMIntegration } from '../../src/integrations/scm.js'; import { formatValidationErrors, getIntegrationRequirements, @@ -50,6 +50,32 @@ beforeAll(async () => { await truncateAll(); }); +// Helper functions using the integration registry +async function hasPmIntegration(projectId: string): Promise { + const integrations = integrationRegistry.getByCategory('pm'); + const statuses = await Promise.all(integrations.map((i) => i.hasIntegration(projectId))); + return statuses.some(Boolean); +} + +async function hasScmIntegration(projectId: string): Promise { + const integrations = integrationRegistry.getByCategory('scm'); + const statuses = await Promise.all(integrations.map((i) => i.hasIntegration(projectId))); + return statuses.some(Boolean); +} + +async function hasScmPersonaToken( + projectId: string, + persona: 'implementer' | 'reviewer', +): Promise { + const integrations = integrationRegistry.getByCategory('scm'); + const statuses = await Promise.all( + integrations + .filter((i): i is SCMIntegration => 'hasPersonaToken' in i) + .map((i) => i.hasPersonaToken(projectId, persona)), + ); + return statuses.some(Boolean); +} + describe('Integration Validation (integration)', () => { beforeEach(async () => { await truncateAll(); diff --git a/tests/unit/agents/capabilities/resolver.test.ts b/tests/unit/agents/capabilities/resolver.test.ts index d03d8932..6a189b42 100644 --- a/tests/unit/agents/capabilities/resolver.test.ts +++ b/tests/unit/agents/capabilities/resolver.test.ts @@ -6,6 +6,14 @@ function mockClass(name: string) { return vi.fn().mockImplementation(() => new cls()); } +// Mock integrationRegistry +const mockGetByCategory = vi.fn(); +vi.mock('../../../../src/integrations/registry.js', () => ({ + integrationRegistry: { + getByCategory: (...args: unknown[]) => mockGetByCategory(...args), + }, +})); + // Mock all gadget imports vi.mock('../../../../src/gadgets/AstGrep.js', () => ({ AstGrep: mockClass('AstGrep') })); vi.mock('../../../../src/gadgets/FileMultiEdit.js', () => ({ @@ -57,6 +65,7 @@ vi.mock('../../../../src/gadgets/todo/index.js', () => ({ import type { Capability } from '../../../../src/agents/capabilities/index.js'; import { + createIntegrationChecker, deriveIntegrations, deriveRequiredIntegrations, filterToolManifests, @@ -291,3 +300,104 @@ describe('filterToolManifests', () => { warnSpy.mockRestore(); }); }); + +describe('createIntegrationChecker', () => { + it('returns true for pm category when a pm integration is configured', async () => { + const mockPmIntegration = { hasIntegration: vi.fn().mockResolvedValue(true) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'pm') return [mockPmIntegration]; + return []; + }); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('pm')).toBe(true); + expect(checker('scm')).toBe(false); + expect(checker('alerting')).toBe(false); + }); + + it('returns true for scm category when a scm integration is configured', async () => { + const mockScmIntegration = { hasIntegration: vi.fn().mockResolvedValue(true) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'scm') return [mockScmIntegration]; + return []; + }); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('scm')).toBe(true); + expect(checker('pm')).toBe(false); + expect(checker('alerting')).toBe(false); + }); + + it('returns true for alerting category when an alerting integration is configured', async () => { + const mockAlertingIntegration = { hasIntegration: vi.fn().mockResolvedValue(true) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'alerting') return [mockAlertingIntegration]; + return []; + }); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('alerting')).toBe(true); + expect(checker('pm')).toBe(false); + expect(checker('scm')).toBe(false); + }); + + it('returns false for all categories when no integrations are configured', async () => { + mockGetByCategory.mockReturnValue([]); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('pm')).toBe(false); + expect(checker('scm')).toBe(false); + expect(checker('alerting')).toBe(false); + }); + + it('returns false when integration hasIntegration() returns false', async () => { + const mockIntegration = { hasIntegration: vi.fn().mockResolvedValue(false) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'pm') return [mockIntegration]; + return []; + }); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('pm')).toBe(false); + }); + + it('returns true if ANY integration in a category is configured (OR logic)', async () => { + const mockInt1 = { hasIntegration: vi.fn().mockResolvedValue(false) }; + const mockInt2 = { hasIntegration: vi.fn().mockResolvedValue(true) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'pm') return [mockInt1, mockInt2]; + return []; + }); + + const checker = await createIntegrationChecker('proj-1'); + + expect(checker('pm')).toBe(true); + }); + + it('calls hasIntegration with the correct projectId', async () => { + const mockIntegration = { hasIntegration: vi.fn().mockResolvedValue(true) }; + mockGetByCategory.mockImplementation((cat: string) => { + if (cat === 'pm') return [mockIntegration]; + return []; + }); + + await createIntegrationChecker('my-project-id'); + + expect(mockIntegration.hasIntegration).toHaveBeenCalledWith('my-project-id'); + }); + + it('calls getByCategory for pm, scm, and alerting', async () => { + mockGetByCategory.mockReturnValue([]); + + await createIntegrationChecker('proj-1'); + + expect(mockGetByCategory).toHaveBeenCalledWith('pm'); + expect(mockGetByCategory).toHaveBeenCalledWith('scm'); + expect(mockGetByCategory).toHaveBeenCalledWith('alerting'); + }); +}); diff --git a/tests/unit/github/integration.test.ts b/tests/unit/github/integration.test.ts deleted file mode 100644 index 23760389..00000000 --- a/tests/unit/github/integration.test.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; - -// --------------------------------------------------------------------------- -// Mocks -// --------------------------------------------------------------------------- - -const mockGetIntegrationProvider = vi.fn(); -vi.mock('../../../src/db/repositories/credentialsRepository.js', () => ({ - getIntegrationProvider: (...args: unknown[]) => mockGetIntegrationProvider(...args), -})); - -const mockGetIntegrationCredentialOrNull = vi.fn(); -vi.mock('../../../src/config/provider.js', () => ({ - getIntegrationCredentialOrNull: (...args: unknown[]) => - mockGetIntegrationCredentialOrNull(...args), -})); - -import { hasScmIntegration, hasScmPersonaToken } from '../../../src/github/integration.js'; - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe('hasScmIntegration', () => { - it('returns false when no SCM integration provider configured', async () => { - mockGetIntegrationProvider.mockResolvedValue(null); - - const result = await hasScmIntegration('proj-1'); - - expect(result).toBe(false); - expect(mockGetIntegrationCredentialOrNull).not.toHaveBeenCalled(); - }); - - it('returns true when implementer_token is present (reviewer absent)', async () => { - mockGetIntegrationProvider.mockResolvedValue('github'); - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('ghp_implementer_token') // implementer_token - .mockResolvedValueOnce(null); // reviewer_token - - const result = await hasScmIntegration('proj-1'); - - expect(result).toBe(true); - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith( - 'proj-1', - 'scm', - 'implementer_token', - ); - }); - - it('returns true when reviewer_token is present (implementer absent)', async () => { - mockGetIntegrationProvider.mockResolvedValue('github'); - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce(null) // implementer_token - .mockResolvedValueOnce('ghp_reviewer_token'); // reviewer_token - - const result = await hasScmIntegration('proj-1'); - - expect(result).toBe(true); - }); - - it('returns true when both tokens are present', async () => { - mockGetIntegrationProvider.mockResolvedValue('github'); - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('ghp_impl') - .mockResolvedValueOnce('ghp_rev'); - - const result = await hasScmIntegration('proj-1'); - - expect(result).toBe(true); - }); - - it('returns false when provider exists but both tokens are missing', async () => { - mockGetIntegrationProvider.mockResolvedValue('github'); - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce(null) // implementer_token - .mockResolvedValueOnce(null); // reviewer_token - - const result = await hasScmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('passes correct projectId and category to getIntegrationProvider', async () => { - mockGetIntegrationProvider.mockResolvedValue(null); - - await hasScmIntegration('my-project'); - - expect(mockGetIntegrationProvider).toHaveBeenCalledWith('my-project', 'scm'); - }); -}); - -describe('hasScmPersonaToken', () => { - it('returns true when implementer token is present', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue('ghp_implementer'); - - const result = await hasScmPersonaToken('proj-1', 'implementer'); - - expect(result).toBe(true); - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith( - 'proj-1', - 'scm', - 'implementer_token', - ); - }); - - it('returns false when implementer token is absent', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue(null); - - const result = await hasScmPersonaToken('proj-1', 'implementer'); - - expect(result).toBe(false); - }); - - it('returns true when reviewer token is present', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue('ghp_reviewer'); - - const result = await hasScmPersonaToken('proj-1', 'reviewer'); - - expect(result).toBe(true); - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith( - 'proj-1', - 'scm', - 'reviewer_token', - ); - }); - - it('returns false when reviewer token is absent', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue(null); - - const result = await hasScmPersonaToken('proj-1', 'reviewer'); - - expect(result).toBe(false); - }); - - it('maps implementer persona to implementer_token role', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue('some-token'); - - await hasScmPersonaToken('proj-2', 'implementer'); - - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith( - 'proj-2', - 'scm', - 'implementer_token', - ); - }); - - it('maps reviewer persona to reviewer_token role', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue('some-token'); - - await hasScmPersonaToken('proj-2', 'reviewer'); - - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith( - 'proj-2', - 'scm', - 'reviewer_token', - ); - }); -}); diff --git a/tests/unit/pm/integration.test.ts b/tests/unit/pm/integration.test.ts deleted file mode 100644 index f8d186e2..00000000 --- a/tests/unit/pm/integration.test.ts +++ /dev/null @@ -1,160 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -// --------------------------------------------------------------------------- -// Mocks -// --------------------------------------------------------------------------- - -const mockGetIntegrationProvider = vi.fn(); -vi.mock('../../../src/db/repositories/credentialsRepository.js', () => ({ - getIntegrationProvider: (...args: unknown[]) => mockGetIntegrationProvider(...args), -})); - -const mockGetIntegrationCredentialOrNull = vi.fn(); -vi.mock('../../../src/config/provider.js', () => ({ - getIntegrationCredentialOrNull: (...args: unknown[]) => - mockGetIntegrationCredentialOrNull(...args), -})); - -import { hasPmIntegration } from '../../../src/pm/integration.js'; - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe('hasPmIntegration', () => { - it('returns false when no PM integration provider configured', async () => { - mockGetIntegrationProvider.mockResolvedValue(null); - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - expect(mockGetIntegrationCredentialOrNull).not.toHaveBeenCalled(); - }); - - it('returns false when provider is unknown (not in PROVIDER_CREDENTIAL_ROLES)', async () => { - mockGetIntegrationProvider.mockResolvedValue('unknown-provider'); - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('passes projectId and "pm" category to getIntegrationProvider', async () => { - mockGetIntegrationProvider.mockResolvedValue(null); - - await hasPmIntegration('my-project'); - - expect(mockGetIntegrationProvider).toHaveBeenCalledWith('my-project', 'pm'); - }); - - // ========================================================================= - // Trello - // ========================================================================= - describe('trello provider', () => { - beforeEach(() => { - mockGetIntegrationProvider.mockResolvedValue('trello'); - }); - - it('returns true when all required trello credentials are present', async () => { - // Trello required roles: api_key, token (api_secret is optional) - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('my-api-key') // api_key - .mockResolvedValueOnce('my-token'); // token - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(true); - }); - - it('returns false when trello api_key is missing', async () => { - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce(null) // api_key missing - .mockResolvedValueOnce('my-token'); // token present - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('returns false when trello token is missing', async () => { - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('my-api-key') // api_key present - .mockResolvedValueOnce(null); // token missing - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('returns false when both required trello credentials are missing', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue(null); - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('checks required roles (api_key, token) — not optional api_secret', async () => { - // Required: api_key, token. Optional: api_secret - // If api_key and token present → true, regardless of api_secret - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('my-api-key') - .mockResolvedValueOnce('my-token'); - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(true); - // Should only have checked 2 required credentials (not 3) - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledTimes(2); - }); - }); - - // ========================================================================= - // JIRA - // ========================================================================= - describe('jira provider', () => { - beforeEach(() => { - mockGetIntegrationProvider.mockResolvedValue('jira'); - }); - - it('returns true when all required jira credentials are present', async () => { - // JIRA required roles: email, api_token - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('bot@example.com') // email - .mockResolvedValueOnce('api-token-xxx'); // api_token - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(true); - }); - - it('returns false when jira email is missing', async () => { - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce(null) // email missing - .mockResolvedValueOnce('api-token-xxx'); - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('returns false when jira api_token is missing', async () => { - mockGetIntegrationCredentialOrNull - .mockResolvedValueOnce('bot@example.com') - .mockResolvedValueOnce(null); // api_token missing - - const result = await hasPmIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('checks for pm category credentials for jira', async () => { - mockGetIntegrationCredentialOrNull.mockResolvedValue('value'); - - await hasPmIntegration('proj-1'); - - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith('proj-1', 'pm', 'email'); - expect(mockGetIntegrationCredentialOrNull).toHaveBeenCalledWith('proj-1', 'pm', 'api_token'); - }); - }); -}); diff --git a/tests/unit/sentry/alerting-integration.test.ts b/tests/unit/sentry/alerting-integration.test.ts index 7cfd7d31..fbc88b73 100644 --- a/tests/unit/sentry/alerting-integration.test.ts +++ b/tests/unit/sentry/alerting-integration.test.ts @@ -11,11 +11,9 @@ vi.mock('../../../src/config/provider.js', () => ({ })); const mockGetSentryIntegrationConfig = vi.fn(); -const mockHasAlertingIntegration = vi.fn(); vi.mock('../../../src/sentry/integration.js', () => ({ getSentryIntegrationConfig: (...args: unknown[]) => mockGetSentryIntegrationConfig(...args), - hasAlertingIntegration: (...args: unknown[]) => mockHasAlertingIntegration(...args), })); import { SentryAlertingIntegration } from '../../../src/sentry/alerting-integration.js'; @@ -49,30 +47,30 @@ describe('SentryAlertingIntegration', () => { // hasIntegration // ========================================================================= describe('hasIntegration', () => { - it('returns true when sentry integration is configured', async () => { - mockHasAlertingIntegration.mockResolvedValue(true); + it('returns true when sentry integration config is non-null', async () => { + mockGetSentryIntegrationConfig.mockResolvedValue({ organizationSlug: 'my-org' }); const result = await integration.hasIntegration('proj-1'); expect(result).toBe(true); - expect(mockHasAlertingIntegration).toHaveBeenCalledWith('proj-1'); + expect(mockGetSentryIntegrationConfig).toHaveBeenCalledWith('proj-1'); }); - it('returns false when sentry integration is not configured', async () => { - mockHasAlertingIntegration.mockResolvedValue(false); + it('returns false when sentry integration config is null', async () => { + mockGetSentryIntegrationConfig.mockResolvedValue(null); const result = await integration.hasIntegration('proj-1'); expect(result).toBe(false); - expect(mockHasAlertingIntegration).toHaveBeenCalledWith('proj-1'); + expect(mockGetSentryIntegrationConfig).toHaveBeenCalledWith('proj-1'); }); - it('delegates to hasAlertingIntegration() with the correct projectId', async () => { - mockHasAlertingIntegration.mockResolvedValue(true); + it('calls getSentryIntegrationConfig with the correct projectId', async () => { + mockGetSentryIntegrationConfig.mockResolvedValue(null); await integration.hasIntegration('my-project-id'); - expect(mockHasAlertingIntegration).toHaveBeenCalledWith('my-project-id'); + expect(mockGetSentryIntegrationConfig).toHaveBeenCalledWith('my-project-id'); }); }); diff --git a/tests/unit/sentry/integration.test.ts b/tests/unit/sentry/integration.test.ts index cec38e40..08264b05 100644 --- a/tests/unit/sentry/integration.test.ts +++ b/tests/unit/sentry/integration.test.ts @@ -5,10 +5,7 @@ vi.mock('../../../src/db/repositories/integrationsRepository.js', () => ({ })); import { getIntegrationByProjectAndCategory } from '../../../src/db/repositories/integrationsRepository.js'; -import { - getSentryIntegrationConfig, - hasAlertingIntegration, -} from '../../../src/sentry/integration.js'; +import { getSentryIntegrationConfig } from '../../../src/sentry/integration.js'; const mockGetIntegrationByProjectAndCategory = vi.mocked(getIntegrationByProjectAndCategory); @@ -110,62 +107,4 @@ describe('sentry/integration', () => { ); }); }); - - describe('hasAlertingIntegration', () => { - it('returns true when sentry integration is configured', async () => { - mockGetIntegrationByProjectAndCategory.mockResolvedValueOnce({ - id: 'int-1', - provider: 'sentry', - config: { organizationSlug: 'my-org' }, - }); - - const result = await hasAlertingIntegration('proj-1'); - - expect(result).toBe(true); - }); - - it('returns false when no integration exists', async () => { - mockGetIntegrationByProjectAndCategory.mockResolvedValueOnce(null); - - const result = await hasAlertingIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('returns false when integration has wrong provider', async () => { - mockGetIntegrationByProjectAndCategory.mockResolvedValueOnce({ - id: 'int-1', - provider: 'pagerduty', - config: { organizationSlug: 'my-org' }, - }); - - const result = await hasAlertingIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('returns false when integration config is missing organizationSlug', async () => { - mockGetIntegrationByProjectAndCategory.mockResolvedValueOnce({ - id: 'int-1', - provider: 'sentry', - config: {}, - }); - - const result = await hasAlertingIntegration('proj-1'); - - expect(result).toBe(false); - }); - - it('delegates to getSentryIntegrationConfig (not null => true)', async () => { - mockGetIntegrationByProjectAndCategory.mockResolvedValueOnce({ - id: 'int-1', - provider: 'sentry', - config: { organizationSlug: 'org-slug' }, - }); - - const result = await hasAlertingIntegration('proj-with-sentry'); - - expect(result).toBe(true); - }); - }); });