diff --git a/apps/web/src/components/settings/automations/AutomationsSettings.render.client.test.tsx b/apps/web/src/components/settings/automations/AutomationsSettings.render.client.test.tsx index b4acd476d..1919ca0a1 100644 --- a/apps/web/src/components/settings/automations/AutomationsSettings.render.client.test.tsx +++ b/apps/web/src/components/settings/automations/AutomationsSettings.render.client.test.tsx @@ -457,8 +457,9 @@ describe('AutomationsSettings', () => { it('shows exception-only capability badges from the shared descriptors', async () => { render(); - // Dependabot/CI triage and manager stats stay GitHub-only. - expect((await screen.findAllByText('GitHub only')).length).toBe(3); + // Dependabot and CI failure triage stay GitHub-only; manager stats is + // provider-neutral now and shows no source-control badge. + expect((await screen.findAllByText('GitHub only')).length).toBe(2); // Suggester and CI failure triage still report to Slack only. expect(screen.getAllByText('Slack only').length).toBe(2); // conflict_resolver supports GitHub, GitLab, and Azure DevOps (no diff --git a/packages/sdk/src/server/automations/__tests__/manager-stats.test.ts b/packages/sdk/src/server/automations/__tests__/manager-stats.test.ts index 4fd94739e..e6b533052 100644 --- a/packages/sdk/src/server/automations/__tests__/manager-stats.test.ts +++ b/packages/sdk/src/server/automations/__tests__/manager-stats.test.ts @@ -7,6 +7,7 @@ vi.mock('@roomote/env', () => ({ vi.mock('@roomote/db/server', () => ({ db: {}, githubInstallations: {}, + repositories: {}, slackInstallations: {}, getAutomationRuntime: vi.fn(), recordAutomationRunOutcome: vi.fn(), @@ -64,6 +65,7 @@ const stats = { mergedRoomotePullRequestPercentage: 67, additions: 123, deletions: 45, + locScope: 'all' as const, mostActiveRepo: { fullName: 'acme/app', pullRequestCount: 5, @@ -101,6 +103,23 @@ describe('formatManagerStatsMessage', () => { expect(message.text).not.toContain('Share of total PRs'); }); + it('includes the LOC line when all counted Roomote PRs are on GitHub', () => { + const message = formatManagerStatsMessage({ + stats, + }); + + expect(message.text).toContain('· LOC added / removed: *+123 / -45*'); + expect(message.text).not.toContain('(GitHub PRs only)'); + }); + + it('omits the LOC line entirely when non-GitHub Roomote PRs are counted', () => { + const message = formatManagerStatsMessage({ + stats: { ...stats, locScope: 'github_only' as const }, + }); + + expect(message.text).not.toContain('LOC added / removed'); + }); + it('adds an automation-settings context footer', () => { const message = formatManagerStatsMessage({ stats, diff --git a/packages/sdk/src/server/automations/manager-stats.ts b/packages/sdk/src/server/automations/manager-stats.ts index 59e3fc299..8219d579d 100644 --- a/packages/sdk/src/server/automations/manager-stats.ts +++ b/packages/sdk/src/server/automations/manager-stats.ts @@ -24,7 +24,7 @@ import { resolveAutomationRuntimeDestination, type ResolvedAutomationDestination, } from './destination'; -import { hasActiveGitHubInstallation } from './github-deployment-scope'; +import { hasAnyActiveRepository } from './github-deployment-scope'; import { isWeeklyRunDueOnLocalDay, resolveSlackWorkspaceTimezone, @@ -43,7 +43,12 @@ const WINDOW_DAYS = 7; interface DeploymentContext { slackBotToken: string | null; slackTeamId: string | null; - actorUserId: string; + /** + * User the GitHub data path runs as; null when the deployment has no Slack + * installer and no GitHub installation (e.g. GitLab + Teams). The digest + * then covers the non-GitHub repositories only, which is the whole set. + */ + actorUserId: string | null; } function buildAnalyticsUrl() { @@ -66,7 +71,12 @@ function formatManagerStatsText({ `· Active users: *${stats.activeUsers}*`, `· PRs opened with me: *${stats.roomotePullRequests} (${Math.round(stats.roomotePullRequestPercentage)}% of ${stats.totalPullRequests})* — ${stats.authoredPullRequests} authored, ${stats.reviewedPullRequests} reviewed`, `· PR merged with me: *${stats.mergedRoomotePullRequests} (${Math.round(stats.mergedRoomotePullRequestPercentage)}% of ${stats.authoredPullRequests} authored)*`, - `· LOC added / removed: *+${stats.additions} / -${stats.deletions}*`, + // Line counts are only available from GitHub; when the digest includes + // PRs from other providers the number would be partial, so omit the line + // entirely rather than annotate it. + ...(stats.locScope === 'all' + ? [`· LOC added / removed: *+${stats.additions} / -${stats.deletions}*`] + : []), `· Most active repo: ${ stats.mostActiveRepo ? `*${stats.mostActiveRepo.fullName}* (${stats.mostActiveRepo.pullRequestCount} PRs)` @@ -94,7 +104,9 @@ export function formatManagerStatsMessage({ } async function findEligibleDeployments(): Promise { - if (!(await hasActiveGitHubInstallation())) { + // PR stats come from the provider-neutral digest, so any active repository + // qualifies regardless of source-control provider. + if (!(await hasAnyActiveRepository())) { return []; } @@ -113,7 +125,8 @@ async function findEligibleDeployments(): Promise { // No Slack: the deployment is still eligible when another comms provider // can carry the stats report. GitHub calls then run as the user who - // installed the GitHub App. + // installed the GitHub App; without a GitHub installation there is no + // GitHub data path to authenticate, so no actor is needed. const connectedProviders = await listConnectedCommunicationProviders(); if (connectedProviders.length === 0) { @@ -126,15 +139,13 @@ async function findEligibleDeployments(): Promise { .where(isNull(githubInstallations.suspendedAt)) .limit(1); - return installation - ? [ - { - slackBotToken: null, - slackTeamId: null, - actorUserId: installation.actorUserId, - }, - ] - : []; + return [ + { + slackBotToken: null, + slackTeamId: null, + actorUserId: installation?.actorUserId ?? null, + }, + ]; } /** @@ -185,7 +196,7 @@ export async function managerStatsJob( if (eligibleDeployments.length === 0) { result.skippedReason = - 'GitHub and a communication provider must both be connected.'; + 'A repository and a communication provider must both be connected.'; } let processed = 0; diff --git a/packages/sdk/src/server/lib/__tests__/manager-stats.test.ts b/packages/sdk/src/server/lib/__tests__/manager-stats.test.ts index cb04c05d5..3999944b0 100644 --- a/packages/sdk/src/server/lib/__tests__/manager-stats.test.ts +++ b/packages/sdk/src/server/lib/__tests__/manager-stats.test.ts @@ -1,9 +1,10 @@ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -// The lib module imports the db and github packages at load time; stub them so -// the pure classification helpers can be exercised without a database or any -// GitHub network access. `isRoomoteGitHubLogin` is stubbed to recognize only -// `octomote[bot]` so the tests control the bot signal without slug resolution. +// The lib module imports the db, github, and provider-read packages at load +// time; stub them so the pure classification helpers can be exercised without +// a database or any network access. `isRoomoteGitHubLogin` is stubbed to +// recognize only `octomote[bot]` so the tests control the bot signal without +// slug resolution. vi.mock('@roomote/db/server', () => ({ db: {}, repositories: {}, @@ -18,14 +19,30 @@ vi.mock('@roomote/github', () => ({ Schemas: { isRoomoteGitHubLogin: (login: string) => login === 'octomote[bot]', }, + resolveConfiguredGitHubAppSlug: vi.fn(), getPullRequestsForAnalytics: vi.fn(), getPullRequest: vi.fn(), })); +vi.mock('../pull-requests/source-control-pull-request-reads', () => ({ + listOpenSourceControlPullRequestsForRepository: vi.fn(), + listMergedSourceControlPullRequestsForRepository: vi.fn(), +})); + import { buildRoomotePullRequestMetadata, + computeMostActiveRepo, + getSourceControlAnalyticsPullRequests, summarizeRoomotePullRequests, + toAnalyticsPullRequests, + type AnalyticsPullRequest, } from '../manager-stats'; +import { + listMergedSourceControlPullRequestsForRepository, + listOpenSourceControlPullRequestsForRepository, + type SourceControlPullRequestSummary, +} from '../pull-requests/source-control-pull-request-reads'; +import type { RepositoryRow } from '../pull-requests/source-control-pull-request-shared'; type MetadataRow = Parameters< typeof buildRoomotePullRequestMetadata @@ -34,6 +51,7 @@ type MetadataRow = Parameters< function metadataRow(overrides: Partial): MetadataRow { return { taskId: 'task-1', + sourceControlProvider: 'github', repository: 'acme/app', prNumber: 1, workflow: 'standard', @@ -68,9 +86,9 @@ describe('buildRoomotePullRequestMetadata', () => { }), ]); - expect(metadata.get('acme/app#1')?.classification).toBe('authored'); - expect(metadata.get('acme/app#2')?.classification).toBe('reviewed'); - expect(metadata.get('acme/app#3')?.classification).toBe('authored'); + expect(metadata.get('github:acme/app#1')?.classification).toBe('authored'); + expect(metadata.get('github:acme/app#2')?.classification).toBe('reviewed'); + expect(metadata.get('github:acme/app#3')?.classification).toBe('authored'); }); it('lets authored win when a key has both authored and review rows', () => { @@ -88,8 +106,10 @@ describe('buildRoomotePullRequestMetadata', () => { }), ]); - expect(reviewThenAuthor.get('acme/app#7')?.classification).toBe('authored'); - expect(reviewThenAuthor.get('acme/app#7')?.canonicalTaskId).toBe( + expect(reviewThenAuthor.get('github:acme/app#7')?.classification).toBe( + 'authored', + ); + expect(reviewThenAuthor.get('github:acme/app#7')?.canonicalTaskId).toBe( 'author-task', ); @@ -107,12 +127,37 @@ describe('buildRoomotePullRequestMetadata', () => { }), ]); - expect(authorThenReview.get('acme/app#7')?.classification).toBe('authored'); - expect(authorThenReview.get('acme/app#7')?.canonicalTaskId).toBe( + expect(authorThenReview.get('github:acme/app#7')?.classification).toBe( + 'authored', + ); + expect(authorThenReview.get('github:acme/app#7')?.canonicalTaskId).toBe( 'author-task', ); }); + it('keys the same repository and number separately per provider', () => { + const metadata = buildRoomotePullRequestMetadata([ + metadataRow({ + taskId: 'github-task', + sourceControlProvider: 'github', + prNumber: 9, + workflow: 'standard', + }), + metadataRow({ + taskId: 'gitlab-task', + sourceControlProvider: 'gitlab', + prNumber: 9, + workflow: 'pr_review', + }), + ]); + + expect(metadata.get('github:acme/app#9')?.classification).toBe('authored'); + expect(metadata.get('gitlab:acme/app#9')?.classification).toBe('reviewed'); + expect(metadata.get('gitlab:acme/app#9')?.canonicalTaskId).toBe( + 'gitlab-task', + ); + }); + it('skips rows without a repository or PR number', () => { const metadata = buildRoomotePullRequestMetadata([ metadataRow({ repository: null, prNumber: 1 }), @@ -123,12 +168,11 @@ describe('buildRoomotePullRequestMetadata', () => { }); }); -type AnalyticsPr = Parameters< - typeof summarizeRoomotePullRequests ->[0]['pullRequests'][number]; - -function analyticsPr(overrides: Partial): AnalyticsPr { +function analyticsPr( + overrides: Partial, +): AnalyticsPullRequest { return { + sourceControlProvider: 'github', repoFullName: 'acme/app', number: 1, state: 'open', @@ -201,4 +245,450 @@ describe('summarizeRoomotePullRequests', () => { expect(authored.map((entry) => entry.pullRequest.number)).toEqual([5]); expect(reviewed).toHaveLength(0); }); + + it('classifies non-GitHub PRs from task metadata only, never by author login', () => { + const metadataByKey = buildRoomotePullRequestMetadata([ + metadataRow({ + sourceControlProvider: 'gitlab', + prNumber: 1, + workflow: 'standard', + }), + metadataRow({ + sourceControlProvider: 'ado', + prNumber: 2, + workflow: 'pr_review', + }), + ]); + + const { authored, reviewed, roomotePullRequests } = + summarizeRoomotePullRequests({ + pullRequests: [ + analyticsPr({ sourceControlProvider: 'gitlab', number: 1 }), + analyticsPr({ sourceControlProvider: 'ado', number: 2 }), + // The GitHub bot login on a non-GitHub provider is someone else's + // account name, not Roomote; without a task row it is excluded. + analyticsPr({ + sourceControlProvider: 'gitea', + number: 3, + authorLogin: 'octomote[bot]', + }), + ], + metadataByKey, + }); + + expect(roomotePullRequests).toHaveLength(2); + expect(authored.map((entry) => entry.pullRequest.number)).toEqual([1]); + expect(reviewed.map((entry) => entry.pullRequest.number)).toEqual([2]); + }); + + it('does not leak metadata across providers for the same repo and number', () => { + const metadataByKey = buildRoomotePullRequestMetadata([ + metadataRow({ + sourceControlProvider: 'gitlab', + prNumber: 6, + workflow: 'standard', + }), + ]); + + const { roomotePullRequests } = summarizeRoomotePullRequests({ + pullRequests: [ + // Same repo full name and number, but on GitHub -> not Roomote's. + analyticsPr({ sourceControlProvider: 'github', number: 6 }), + ], + metadataByKey, + }); + + expect(roomotePullRequests).toHaveLength(0); + }); +}); + +const WINDOW_START = new Date('2026-07-07T00:00:00.000Z'); + +function summary( + overrides: Partial, +): SourceControlPullRequestSummary { + return { + number: 1, + externalId: null, + url: 'https://example.com/pr/1', + title: 'A change', + state: 'open', + draft: false, + sourceBranch: 'feature', + targetBranch: 'main', + author: { id: '42', login: 'human-dev' }, + updatedAt: '2026-07-10T10:00:00.000Z', + createdAt: '2026-07-10T09:00:00.000Z', + mergedAt: null, + closedAt: null, + labels: [], + headSha: null, + baseSha: null, + mergeable: null, + mergeStateDescription: null, + isCrossRepository: null, + headRepositoryFullName: null, + ...overrides, + }; +} + +describe('toAnalyticsPullRequests', () => { + it('maps GitLab-shaped summaries created inside the window', () => { + const results = toAnalyticsPullRequests({ + provider: 'gitlab', + repoFullName: 'group/app', + summaries: [ + summary({ number: 1, createdAt: '2026-07-10T09:00:00.000Z' }), + summary({ + number: 2, + state: 'merged', + createdAt: '2026-07-09T09:00:00.000Z', + mergedAt: '2026-07-10T09:00:00.000Z', + }), + // Created before the window -> excluded even though still open. + summary({ number: 3, createdAt: '2026-07-01T09:00:00.000Z' }), + ], + since: WINDOW_START, + }); + + expect(results).toEqual([ + { + sourceControlProvider: 'gitlab', + repoFullName: 'group/app', + number: 1, + state: 'open', + authorLogin: 'human-dev', + }, + { + sourceControlProvider: 'gitlab', + repoFullName: 'group/app', + number: 2, + state: 'merged', + authorLogin: 'human-dev', + }, + ]); + }); + + it('reports an open draft as draft, matching the GitHub analytics states', () => { + const results = toAnalyticsPullRequests({ + provider: 'gitea', + repoFullName: 'org/app', + summaries: [summary({ number: 4, draft: true })], + since: WINDOW_START, + }); + + expect(results[0]?.state).toBe('draft'); + }); + + it('drops Bitbucket-shaped summaries without a created timestamp instead of guessing', () => { + const results = toAnalyticsPullRequests({ + provider: 'bitbucket', + repoFullName: 'workspace/app', + summaries: [ + // Bitbucket merged lists carry no merge timestamp and, defensively, + // a row may lack created_on too; unknown age must not count. + summary({ + number: 5, + state: 'merged', + createdAt: null, + mergedAt: null, + updatedAt: '2026-07-11T09:00:00.000Z', + }), + summary({ number: 6, createdAt: '2026-07-11T09:00:00.000Z' }), + ], + since: WINDOW_START, + }); + + expect(results.map((pullRequest) => pullRequest.number)).toEqual([6]); + }); + + it('handles ADO-shaped summaries with null updatedAt and null author login', () => { + const results = toAnalyticsPullRequests({ + provider: 'ado', + repoFullName: 'org/project/repo', + summaries: [ + summary({ + number: 7, + state: 'merged', + createdAt: '2026-07-12T09:00:00.000Z', + mergedAt: '2026-07-13T09:00:00.000Z', + updatedAt: null, + author: null, + }), + ], + since: WINDOW_START, + }); + + expect(results).toEqual([ + { + sourceControlProvider: 'ado', + repoFullName: 'org/project/repo', + number: 7, + state: 'merged', + authorLogin: null, + }, + ]); + }); + + it('dedupes summaries that report the same PR number twice', () => { + const results = toAnalyticsPullRequests({ + provider: 'gitlab', + repoFullName: 'group/app', + summaries: [ + summary({ number: 8, state: 'open' }), + summary({ number: 8, state: 'open' }), + ], + since: WINDOW_START, + }); + + expect(results).toHaveLength(1); + expect(results[0]?.state).toBe('open'); + }); + + it('prefers the merged summary when a PR merges between the open and merged list reads', () => { + // The open and merged lists are fetched concurrently, so a PR that merges + // between the two requests appears in both. The merged row must win + // regardless of which list is concatenated first, or merged counts are + // underreported. + const openFirst = toAnalyticsPullRequests({ + provider: 'gitlab', + repoFullName: 'group/app', + summaries: [ + summary({ number: 9, state: 'open' }), + summary({ + number: 9, + state: 'merged', + mergedAt: '2026-07-10T10:00:00.000Z', + }), + ], + since: WINDOW_START, + }); + + expect(openFirst).toHaveLength(1); + expect(openFirst[0]?.state).toBe('merged'); + + const mergedFirst = toAnalyticsPullRequests({ + provider: 'gitlab', + repoFullName: 'group/app', + summaries: [ + summary({ + number: 9, + state: 'merged', + mergedAt: '2026-07-10T10:00:00.000Z', + }), + summary({ number: 9, state: 'open' }), + ], + since: WINDOW_START, + }); + + expect(mergedFirst).toHaveLength(1); + expect(mergedFirst[0]?.state).toBe('merged'); + }); + + it('does not let an open draft duplicate demote a merged row', () => { + const results = toAnalyticsPullRequests({ + provider: 'gitea', + repoFullName: 'org/app', + summaries: [ + summary({ + number: 10, + state: 'merged', + mergedAt: '2026-07-10T10:00:00.000Z', + }), + summary({ number: 10, state: 'open', draft: true }), + ], + since: WINDOW_START, + }); + + expect(results).toHaveLength(1); + expect(results[0]?.state).toBe('merged'); + }); +}); + +function repositoryRow(overrides: Partial): RepositoryRow { + return { + id: 'repo-1', + sourceControlProvider: 'gitlab', + host: null, + installationId: null, + externalRepoId: '42', + fullName: 'group/app', + htmlUrl: 'https://gitlab.example.com/group/app', + ...overrides, + }; +} + +function listResult({ + repository, + pullRequests, + warnings = [], +}: { + repository: RepositoryRow; + pullRequests: SourceControlPullRequestSummary[]; + warnings?: string[]; +}) { + return { + success: true as const, + provider: repository.sourceControlProvider, + repositoryFullName: repository.fullName, + pullRequests, + warnings, + }; +} + +const TRUNCATION_WARNING = + 'Result truncated to the 200 most relevant merged pull requests; more exist.'; + +describe('getSourceControlAnalyticsPullRequests', () => { + const listOpen = vi.mocked(listOpenSourceControlPullRequestsForRepository); + const listMerged = vi.mocked( + listMergedSourceControlPullRequestsForRepository, + ); + let warnSpy: ReturnType; + + beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + listOpen.mockReset(); + listMerged.mockReset(); + }); + + it('logs provider truncation warnings per repository instead of dropping them', async () => { + const repository = repositoryRow({}); + + listOpen.mockResolvedValue( + listResult({ + repository, + pullRequests: [summary({ number: 1 })], + }), + ); + listMerged.mockResolvedValue( + listResult({ + repository, + pullRequests: [ + summary({ + number: 2, + state: 'merged', + mergedAt: '2026-07-10T10:00:00.000Z', + }), + ], + warnings: [TRUNCATION_WARNING], + }), + ); + + const pullRequests = await getSourceControlAnalyticsPullRequests({ + repositoryRows: [repository], + since: WINDOW_START, + }); + + expect( + pullRequests.map((pullRequest) => pullRequest.number).sort(), + ).toEqual([1, 2]); + expect(warnSpy).toHaveBeenCalledWith( + `[managerStats] gitlab pull request list warning for group/app: ${TRUNCATION_WARNING}`, + ); + }); + + it('logs warnings for each affected repository when several repositories report them', async () => { + const truncatedRepo = repositoryRow({ + id: 'repo-1', + fullName: 'group/big', + }); + const cleanRepo = repositoryRow({ id: 'repo-2', fullName: 'group/small' }); + const openTruncationWarning = + 'Result truncated to the 200 most relevant open pull requests; more exist.'; + + listOpen.mockImplementation(async ({ repository }) => + listResult({ + repository, + pullRequests: [], + warnings: + repository.fullName === 'group/big' ? [openTruncationWarning] : [], + }), + ); + listMerged.mockImplementation(async ({ repository }) => + listResult({ repository, pullRequests: [] }), + ); + + await getSourceControlAnalyticsPullRequests({ + repositoryRows: [truncatedRepo, cleanRepo], + since: WINDOW_START, + }); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + `[managerStats] gitlab pull request list warning for group/big: ${openTruncationWarning}`, + ); + }); + + it('logs nothing when no repository reports a warning', async () => { + const repository = repositoryRow({}); + + listOpen.mockResolvedValue( + listResult({ repository, pullRequests: [summary({ number: 4 })] }), + ); + listMerged.mockResolvedValue(listResult({ repository, pullRequests: [] })); + + const pullRequests = await getSourceControlAnalyticsPullRequests({ + repositoryRows: [repository], + since: WINDOW_START, + }); + + expect(pullRequests).toHaveLength(1); + expect(warnSpy).not.toHaveBeenCalled(); + }); +}); + +describe('computeMostActiveRepo', () => { + it('counts equally named repositories on different providers separately', () => { + const result = computeMostActiveRepo([ + analyticsPr({ sourceControlProvider: 'github', number: 1 }), + analyticsPr({ sourceControlProvider: 'github', number: 2 }), + analyticsPr({ sourceControlProvider: 'gitlab', number: 1 }), + analyticsPr({ sourceControlProvider: 'gitlab', number: 2 }), + analyticsPr({ sourceControlProvider: 'gitlab', number: 3 }), + ]); + + // Combined across providers, the two acme/app repos would wrongly report + // 5 PRs; counted per provider, the GitLab one leads with 3. + expect(result).toEqual({ + fullName: 'acme/app', + pullRequestCount: 3, + }); + }); + + it('picks the repo with the most PRs across providers', () => { + const result = computeMostActiveRepo([ + analyticsPr({ sourceControlProvider: 'gitlab', number: 1 }), + analyticsPr({ sourceControlProvider: 'gitlab', number: 2 }), + analyticsPr({ + sourceControlProvider: 'github', + repoFullName: 'acme/other', + number: 1, + }), + ]); + + expect(result).toEqual({ fullName: 'acme/app', pullRequestCount: 2 }); + }); + + it('returns null when no Roomote PRs were counted', () => { + expect(computeMostActiveRepo([])).toBeNull(); + }); + + it('breaks equal-count same-name ties deterministically by provider', () => { + const prs = [ + { sourceControlProvider: 'gitlab', repoFullName: 'acme/app' }, + { sourceControlProvider: 'github', repoFullName: 'acme/app' }, + ] as never[]; + + const forward = computeMostActiveRepo(prs as never); + const reversed = computeMostActiveRepo( + [...(prs as never[])].reverse() as never, + ); + + expect(forward).toEqual(reversed); + }); }); diff --git a/packages/sdk/src/server/lib/manager-stats.ts b/packages/sdk/src/server/lib/manager-stats.ts index 79e3b0bdd..f9e48de1c 100644 --- a/packages/sdk/src/server/lib/manager-stats.ts +++ b/packages/sdk/src/server/lib/manager-stats.ts @@ -1,5 +1,8 @@ import * as GitHub from '@roomote/github'; -import { formatAutomationLabel } from '@roomote/types'; +import { + formatAutomationLabel, + type SourceControlProvider, +} from '@roomote/types'; import { db, @@ -11,6 +14,21 @@ import { gte, } from '@roomote/db/server'; +import { + listMergedSourceControlPullRequestsForRepository, + listOpenSourceControlPullRequestsForRepository, + type SourceControlPullRequestSummary, +} from './pull-requests/source-control-pull-request-reads'; +import type { RepositoryRow } from './pull-requests/source-control-pull-request-shared'; + +const LOG_PREFIX = '[managerStats]'; + +/** + * Cap on PRs fetched per non-GitHub repository per list state in one digest + * computation. Matches the provider-neutral list primitive's maximum. + */ +const SOURCE_CONTROL_ANALYTICS_LIST_LIMIT = 200; + type TaskInitiatorRow = { initiatorKind: 'user' | 'automation'; initiatorUserId: string | null; @@ -60,6 +78,7 @@ type PullRequestMetadata = { type PullRequestMetadataRow = TaskInitiatorRow & { taskId: string; + sourceControlProvider: SourceControlProvider; repository: string | null; prNumber: number | null; workflow: string | null; @@ -76,7 +95,20 @@ export type ManagerStatsDigest = { mergedRoomotePullRequestPercentage: number; additions: number; deletions: number; + /** + * Which Roomote PRs `additions`/`deletions` cover. Line counts come from + * the GitHub PR-details API and no equivalent bulk surface exists on the + * other providers (each would need its own diff-stat endpoint), so when + * non-GitHub Roomote PRs are in the digest the LOC numbers are partial and + * the digest omits its LOC line instead of silently undercounting. + */ + locScope: 'all' | 'github_only'; mostActiveRepo: { + /** + * Bare repository full name. PR counts behind it are provider-qualified, + * so equally named repositories on different providers never combine + * into one count. + */ fullName: string; pullRequestCount: number; } | null; @@ -86,8 +118,17 @@ export type ManagerStatsDigest = { }>; }; -function getPullRequestKey(repoFullName: string, prNumber: number) { - return `${repoFullName.toLowerCase()}#${prNumber}`; +/** + * PR identity is provider-qualified: `(repository full name, number)` pairs + * can collide across providers (e.g. a mirrored `owner/repo#1` on GitHub and + * GitLab), and taskPullRequests stamps the provider on every association. + */ +function getPullRequestKey( + provider: SourceControlProvider, + repoFullName: string, + prNumber: number, +) { + return `${provider}:${repoFullName.toLowerCase()}#${prNumber}`; } /** @@ -98,7 +139,7 @@ function classifyWorkflow(workflow: string | null): PullRequestClassification { return workflow === 'pr_review' ? 'reviewed' : 'authored'; } -function isRoomotePullRequestAuthor(login: string | null) { +function isRoomoteGitHubPullRequestAuthor(login: string | null) { if (!login) { return false; } @@ -109,7 +150,7 @@ function isRoomotePullRequestAuthor(login: string | null) { } /** - * Fold task/PR rows into per-PR metadata, keyed by repo#number. + * Fold task/PR rows into per-PR metadata, keyed by provider:repo#number. * * A single PR key can appear with rows from both an authoring task and a * review task. `authored` always wins: Roomote making the PR is not demoted by @@ -126,7 +167,11 @@ export function buildRoomotePullRequestMetadata( continue; } - const key = getPullRequestKey(row.repository, row.prNumber); + const key = getPullRequestKey( + row.sourceControlProvider, + row.repository, + row.prNumber, + ); const classification = classifyWorkflow(row.workflow); const existing = metadataByKey.get(key); @@ -155,6 +200,7 @@ async function getRoomotePullRequestMetadataByKey() { const results = await db .select({ taskId: taskPullRequests.taskId, + sourceControlProvider: taskPullRequests.sourceControlProvider, repository: taskPullRequests.repository, prNumber: taskPullRequests.prNumber, workflow: tasks.workflow, @@ -173,15 +219,19 @@ async function getRoomotePullRequestMetadataByKey() { return buildRoomotePullRequestMetadata(results); } -async function getAnalyticsRepositoryIds() { - const rows = await db.query.repositories.findMany({ +async function getAnalyticsRepositories(): Promise { + return db.query.repositories.findMany({ where: eq(repositories.isActive, true), columns: { id: true, + sourceControlProvider: true, + host: true, + installationId: true, + externalRepoId: true, + fullName: true, + htmlUrl: true, }, }); - - return rows.map((row) => row.id); } async function getActiveUserCount(since: Date) { @@ -209,44 +259,237 @@ async function getActiveUserCount(since: Date) { return new Set(humanCreatorKeys).size; } -type RoomotePullRequestInput = { +/** + * The provider-neutral analytics shape the digest is computed from: one row + * per PR created inside the stats window, from any source-control provider. + */ +export type AnalyticsPullRequest = { + sourceControlProvider: SourceControlProvider; repoFullName: string; number: number; state: string; authorLogin: string | null; }; -type ClassifiedPullRequest = { - pullRequest: T; +type ClassifiedPullRequest = { + pullRequest: AnalyticsPullRequest; classification: PullRequestClassification; }; +/** + * Map provider-neutral list summaries (open + merged) to the analytics shape, + * keeping only PRs created inside the stats window. Rows without a created + * timestamp are dropped: window membership cannot be confirmed, and counting + * a PR of unknown age would inflate the weekly totals. Summaries are deduped + * by PR number, preferring the merged summary on a collision: the open and + * merged lists are fetched concurrently, so a PR that merges between the two + * requests can appear in both, and keeping the stale open row would + * underreport merged counts. + */ +export function toAnalyticsPullRequests({ + provider, + repoFullName, + summaries, + since, +}: { + provider: SourceControlProvider; + repoFullName: string; + summaries: SourceControlPullRequestSummary[]; + since: Date; +}): AnalyticsPullRequest[] { + const byNumber = new Map(); + + for (const summary of summaries) { + if (!summary.createdAt || new Date(summary.createdAt) < since) { + continue; + } + + const existing = byNumber.get(summary.number); + + if ( + existing && + (existing.state === 'merged' || summary.state !== 'merged') + ) { + continue; + } + + byNumber.set(summary.number, { + sourceControlProvider: provider, + repoFullName, + number: summary.number, + // Match the GitHub analytics state vocabulary, where an open draft PR + // is reported as 'draft'. + state: + summary.state === 'open' && summary.draft ? 'draft' : summary.state, + authorLogin: summary.author?.login ?? null, + }); + } + + return [...byNumber.values()]; +} + +/** + * PRs created in the window for one non-GitHub repository, via the + * provider-neutral open/merged list primitives. + * + * Known degradation: the list primitive supports the open and merged states + * only, so PRs that were closed without merging inside the window are not + * counted for non-GitHub repositories (the GitHub path counts them). That + * slightly undercounts `totalPullRequests` on those providers; it never + * fabricates activity. + */ +async function listSourceControlAnalyticsPullRequests({ + repository, + since, +}: { + repository: RepositoryRow; + since: Date; +}): Promise { + const provider = repository.sourceControlProvider; + + // A PR merged in the window was necessarily updated in the window, so + // updatedAfter bounds the merged list to a superset of the created-after + // filter applied below. + const [openResult, mergedResult] = await Promise.all([ + listOpenSourceControlPullRequestsForRepository({ + repository, + provider, + limit: SOURCE_CONTROL_ANALYTICS_LIST_LIMIT, + }), + listMergedSourceControlPullRequestsForRepository({ + repository, + provider, + limit: SOURCE_CONTROL_ANALYTICS_LIST_LIMIT, + updatedAfter: since, + }), + ]); + + // The list primitives report degradation (most importantly truncation at + // the list cap, which undercounts a busy repository's window totals) via + // warnings; log every one per repository instead of silently dropping them. + for (const warning of [...openResult.warnings, ...mergedResult.warnings]) { + console.warn( + `${LOG_PREFIX} ${provider} pull request list warning for ${repository.fullName}: ${warning}`, + ); + } + + return toAnalyticsPullRequests({ + provider, + repoFullName: repository.fullName, + summaries: [...openResult.pullRequests, ...mergedResult.pullRequests], + since, + }); +} + +/** Exported for tests. */ +export async function getSourceControlAnalyticsPullRequests({ + repositoryRows, + since, +}: { + repositoryRows: RepositoryRow[]; + since: Date; +}): Promise { + const results: AnalyticsPullRequest[] = []; + + for (const repository of repositoryRows) { + try { + results.push( + ...(await listSourceControlAnalyticsPullRequests({ + repository, + since, + })), + ); + } catch (error) { + // One unreachable repository must not sink the whole digest; its PRs + // are simply missing from this week's counts. + console.warn( + `${LOG_PREFIX} Failed to list ${repository.sourceControlProvider} pull requests for ${repository.fullName}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + + return results; +} + +async function getGitHubAnalyticsPullRequests({ + actorUserId, + repositoryIds, + since, +}: { + actorUserId: string | null; + repositoryIds: string[]; + since: Date; +}): Promise { + if (repositoryIds.length === 0) { + return []; + } + + if (!actorUserId) { + // GitHub repositories only exist under an installation, and eligible + // deployments resolve an actor from that installation, so this is a + // defensive guard rather than an expected path. + console.warn( + `${LOG_PREFIX} Skipping ${repositoryIds.length} GitHub repositories: no GitHub actor is available.`, + ); + return []; + } + + // isRoomoteGitHubPullRequestAuthor classifies logins synchronously from the + // cached configured app slug. + await GitHub.resolveConfiguredGitHubAppSlug(); + + const items = await GitHub.getPullRequestsForAnalytics({ + userId: actorUserId, + repositoryIds, + createdAfter: since, + }); + + return items.map((item) => ({ + sourceControlProvider: 'github' as const, + repoFullName: item.repoFullName, + number: item.number, + state: item.state, + authorLogin: item.authorLogin, + })); +} + /** * Filter the analytics PRs down to Roomote-linked ones and classify each as * authored or reviewed. * - * A PR reaches here if it has task metadata OR was opened by the Roomote bot. - * `authored` wins whenever any authored signal is present: an authoring task - * row (`metadata.classification === 'authored'`) or a bot-authored PR, even if - * a `pr_review` task also touched it. A PR is only `reviewed` when its metadata - * is a review row and Roomote is not the PR author. + * A PR reaches here if it has task metadata OR (on GitHub) was opened by the + * Roomote bot account. `authored` wins whenever any authored signal is + * present: an authoring task row (`metadata.classification === 'authored'`) + * or a bot-authored PR, even if a `pr_review` task also touched it. A PR is + * only `reviewed` when its metadata is a review row and Roomote is not the PR + * author. + * + * Bot-login matching is GitHub-only: the other providers act through a plain + * access token whose user is deployment-specific, and every PR Roomote opens + * there comes from a task, so the task linkage already covers them. */ -export function summarizeRoomotePullRequests< - T extends RoomotePullRequestInput, ->({ +export function summarizeRoomotePullRequests({ pullRequests, metadataByKey, }: { - pullRequests: T[]; + pullRequests: AnalyticsPullRequest[]; metadataByKey: Map; }) { - const roomotePullRequests: Array> = []; + const roomotePullRequests: ClassifiedPullRequest[] = []; for (const pullRequest of pullRequests) { const metadata = metadataByKey.get( - getPullRequestKey(pullRequest.repoFullName, pullRequest.number), + getPullRequestKey( + pullRequest.sourceControlProvider, + pullRequest.repoFullName, + pullRequest.number, + ), ); - const isAuthorMatch = isRoomotePullRequestAuthor(pullRequest.authorLogin); + const isAuthorMatch = + pullRequest.sourceControlProvider === 'github' && + isRoomoteGitHubPullRequestAuthor(pullRequest.authorLogin); if (!metadata && !isAuthorMatch) { continue; @@ -273,17 +516,63 @@ export function summarizeRoomotePullRequests< return { roomotePullRequests, authored, reviewed, mergedAuthored }; } +/** + * The repository with the most counted Roomote PRs. Counting is + * provider-qualified so equally named repositories on different providers + * (e.g. a mirrored `org/repo` on GitHub and GitLab) are never combined into + * one wrong count; the reported label is the bare full name of the single + * winning repository. + */ +export function computeMostActiveRepo( + pullRequests: AnalyticsPullRequest[], +): ManagerStatsDigest['mostActiveRepo'] { + const repoCounts = new Map< + string, + { + fullName: string; + pullRequestCount: number; + } + >(); + + for (const pullRequest of pullRequests) { + const key = `${pullRequest.sourceControlProvider}:${pullRequest.repoFullName.toLowerCase()}`; + const existing = repoCounts.get(key); + + if (existing) { + existing.pullRequestCount += 1; + } else { + repoCounts.set(key, { + fullName: pullRequest.repoFullName, + pullRequestCount: 1, + }); + } + } + + return ( + [...repoCounts.entries()].sort(([leftKey, left], [rightKey, right]) => { + if (right.pullRequestCount !== left.pullRequestCount) { + return right.pullRequestCount - left.pullRequestCount; + } + + // Same-name repos on different providers tie on fullName too; the + // provider-qualified key keeps the selection deterministic. + return leftKey.localeCompare(rightKey); + })[0]?.[1] ?? null + ); +} + export async function buildManagerStatsDigest(params: { - actorUserId: string; + /** + * User the GitHub API calls run as. Only needed for the GitHub data path; + * null on deployments without a GitHub installation (their GitHub repo set + * is empty). + */ + actorUserId: string | null; since: Date; }) { - // isRoomotePullRequestAuthor classifies logins synchronously from the - // cached configured app slug. - await GitHub.resolveConfiguredGitHubAppSlug(); - - const repositoryIds = await getAnalyticsRepositoryIds(); + const repositoryRows = await getAnalyticsRepositories(); - if (repositoryIds.length === 0) { + if (repositoryRows.length === 0) { return { activeUsers: await getActiveUserCount(params.since), roomotePullRequests: 0, @@ -295,19 +584,34 @@ export async function buildManagerStatsDigest(params: { mergedRoomotePullRequestPercentage: 0, additions: 0, deletions: 0, + locScope: 'all', mostActiveRepo: null, topUsers: [], } satisfies ManagerStatsDigest; } - const [pullRequests, metadataByKey] = await Promise.all([ - GitHub.getPullRequestsForAnalytics({ - userId: params.actorUserId, - repositoryIds, - createdAfter: params.since, - }), - getRoomotePullRequestMetadataByKey(), - ]); + const githubRepositoryIds = repositoryRows + .filter((repository) => repository.sourceControlProvider === 'github') + .map((repository) => repository.id); + const sourceControlRepositories = repositoryRows.filter( + (repository) => repository.sourceControlProvider !== 'github', + ); + + const [githubPullRequests, sourceControlPullRequests, metadataByKey] = + await Promise.all([ + getGitHubAnalyticsPullRequests({ + actorUserId: params.actorUserId, + repositoryIds: githubRepositoryIds, + since: params.since, + }), + getSourceControlAnalyticsPullRequests({ + repositoryRows: sourceControlRepositories, + since: params.since, + }), + getRoomotePullRequestMetadataByKey(), + ]); + + const pullRequests = [...githubPullRequests, ...sourceControlPullRequests]; const { roomotePullRequests: classifiedPullRequests, @@ -328,47 +632,58 @@ export async function buildManagerStatsDigest(params: { let additions = 0; let deletions = 0; - for (const pullRequest of roomotePullRequests) { - const [owner, repo] = pullRequest.repoFullName.split('/'); + // Line counts come from the GitHub PR-details API; the other providers + // expose no comparable field on their PR payloads, so LOC covers GitHub + // PRs only and `locScope` flags the gap for the digest line. + const githubRoomotePullRequests = roomotePullRequests.filter( + (pullRequest) => pullRequest.sourceControlProvider === 'github', + ); + const locScope: ManagerStatsDigest['locScope'] = + githubRoomotePullRequests.length === roomotePullRequests.length + ? 'all' + : 'github_only'; - if (!owner || !repo) { - continue; - } + if (params.actorUserId) { + for (const pullRequest of githubRoomotePullRequests) { + const [owner, repo] = pullRequest.repoFullName.split('/'); - try { - const details = await GitHub.getPullRequest({ - userId: params.actorUserId, - owner, - repo, - prNumber: pullRequest.number, - }); - - if (!details.success) { + if (!owner || !repo) { continue; } - additions += details.data.additions ?? 0; - deletions += details.data.deletions ?? 0; - } catch (error) { - console.warn( - `[managerStats] Failed to load PR details for ${pullRequest.repoFullName}#${pullRequest.number}: ${ - error instanceof Error ? error.message : String(error) - }`, - ); + try { + const details = await GitHub.getPullRequest({ + userId: params.actorUserId, + owner, + repo, + prNumber: pullRequest.number, + }); + + if (!details.success) { + continue; + } + + additions += details.data.additions ?? 0; + deletions += details.data.deletions ?? 0; + } catch (error) { + console.warn( + `${LOG_PREFIX} Failed to load PR details for ${pullRequest.repoFullName}#${pullRequest.number}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } } } - const repoCounts = new Map(); const userCounts = new Map(); for (const pullRequest of roomotePullRequests) { - repoCounts.set( - pullRequest.repoFullName, - (repoCounts.get(pullRequest.repoFullName) ?? 0) + 1, - ); - const metadata = metadataByKey.get( - getPullRequestKey(pullRequest.repoFullName, pullRequest.number), + getPullRequestKey( + pullRequest.sourceControlProvider, + pullRequest.repoFullName, + pullRequest.number, + ), ); const label = metadata?.userLabel ?? pullRequest.authorLogin ?? 'Unknown user'; @@ -376,19 +691,7 @@ export async function buildManagerStatsDigest(params: { userCounts.set(label, (userCounts.get(label) ?? 0) + 1); } - const mostActiveRepo = - [...repoCounts.entries()] - .map(([fullName, pullRequestCount]) => ({ - fullName, - pullRequestCount, - })) - .sort((left, right) => { - if (right.pullRequestCount !== left.pullRequestCount) { - return right.pullRequestCount - left.pullRequestCount; - } - - return left.fullName.localeCompare(right.fullName); - })[0] ?? null; + const mostActiveRepo = computeMostActiveRepo(roomotePullRequests); const topUsers = [...userCounts.entries()] .map(([label, pullRequestCount]) => ({ @@ -422,6 +725,7 @@ export async function buildManagerStatsDigest(params: { : (mergedRoomotePullRequests.length / authored.length) * 100, additions, deletions, + locScope, mostActiveRepo, topUsers, } satisfies ManagerStatsDigest; diff --git a/packages/types/src/background-automation-registry.ts b/packages/types/src/background-automation-registry.ts index 4f08d203f..f61eebe81 100644 --- a/packages/types/src/background-automation-registry.ts +++ b/packages/types/src/background-automation-registry.ts @@ -159,10 +159,12 @@ export const TRIGGERABLE_BACKGROUND_AUTOMATION_DESCRIPTORS = [ label: 'Weekly Manager Stats', availability: 'stable', scheduleModes: MANAGER_STATS_SCHEDULE_MODES, - manualTriggerRequirements: ['slack', 'github'], + // The stats digest is computed from the provider-neutral PR list + // primitives plus taskPullRequests, so any active repository qualifies. + manualTriggerRequirements: ['slack', 'repository'], usesManagerChannel: true, supportedCommunicationProviders: ['slack', 'teams', 'telegram'], - supportedSourceControlProviders: ['github'], + supportedSourceControlProviders: sourceControlProviders, }, { automationKey: 'sentry_triage',