From 3831dd1b57db980390f564fc3e4d05e6435eb91d Mon Sep 17 00:00:00 2001 From: Ben Limmer Date: Tue, 26 May 2026 09:31:05 -0600 Subject: [PATCH 1/2] fix: exclude GitHub App bots from the cost table The people/cost table is built from PR data fetched via GraphQL, where a Bot actor's login has no [bot] suffix (that suffix is REST-only). The old name-based filter therefore missed GitHub Apps like greptile-apps. Query each actor's __typename instead and drop anything typed Bot, for mergers, reviewers, and commenters alike. --- src/collectors/dependabotPrs.test.ts | 43 ++++++++++++++++++++++++++-- src/collectors/dependabotPrs.ts | 37 +++++++++++++++++------- 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/collectors/dependabotPrs.test.ts b/src/collectors/dependabotPrs.test.ts index c7c1d8b..de8fc68 100644 --- a/src/collectors/dependabotPrs.test.ts +++ b/src/collectors/dependabotPrs.test.ts @@ -12,9 +12,14 @@ test('maps a single page of search results to DependabotPr', async () => { rawPullRequest.build({ state: 'MERGED', mergedAt: '2026-04-05T00:00:00Z', - mergedBy: { login: 'alice' }, - reviews: { nodes: [{ author: { login: 'bob' } }, { author: { login: 'alice' } }] }, - comments: { nodes: [{ author: { login: 'alice' } }] }, + mergedBy: { __typename: 'User', login: 'alice' }, + reviews: { + nodes: [ + { author: { __typename: 'User', login: 'bob' } }, + { author: { __typename: 'User', login: 'alice' } }, + ], + }, + comments: { nodes: [{ author: { __typename: 'User', login: 'alice' } }] }, }), ], }, @@ -35,6 +40,38 @@ test('maps a single page of search results to DependabotPr', async () => { }); }); +test('drops bot actors from mergers, reviewers, and commenters', async () => { + const client = new FakeGithubClient(); + client.onGraphql('DependabotPrs').resolves({ + search: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + rawPullRequest.build({ + state: 'MERGED', + mergedAt: '2026-04-05T00:00:00Z', + // A GitHub App that merged the PR: Bot typename, no [bot] suffix. + mergedBy: { __typename: 'Bot', login: 'auto-merge-app' }, + reviews: { + nodes: [ + { author: { __typename: 'Bot', login: 'greptile-apps' } }, + { author: { __typename: 'User', login: 'carol' } }, + ], + }, + comments: { nodes: [{ author: { __typename: 'Bot', login: 'dependabot' } }] }, + }), + ], + }, + }); + + const result = await listDependabotPrs(client, 'acme', '2026-01-01T00:00:00Z'); + const prs = result.unwrapOr([]); + expect(prs[0]).toMatchObject({ + mergedBy: null, + reviewers: ['carol'], + commenters: [], + }); +}); + test('pages through results when hasNextPage is true', async () => { const client = new FakeGithubClient(); // First page returns cursor; second returns no more. diff --git a/src/collectors/dependabotPrs.ts b/src/collectors/dependabotPrs.ts index a5e48a6..3d1aa7c 100644 --- a/src/collectors/dependabotPrs.ts +++ b/src/collectors/dependabotPrs.ts @@ -10,6 +10,14 @@ interface GraphqlSearchResponse { }; } +// GitHub's GraphQL Actor interface. `__typename` is the source of truth for +// whether an actor is a bot: GitHub App accounts (e.g. greptile-apps) surface +// here as `Bot` and — unlike the REST API — carry no `[bot]` login suffix. +export interface RawActor { + __typename: string; + login: string; +} + export interface RawPullRequest { number: number; title: string; @@ -20,11 +28,11 @@ export interface RawPullRequest { url: string; baseRefName: string; headRefName: string; - mergedBy: { login: string } | null; + mergedBy: RawActor | null; autoMergeRequest: { enabledAt: string | null } | null; repository: { owner: { login: string }; name: string }; - reviews: { nodes: Array<{ author: { login: string } | null } | null> }; - comments: { nodes: Array<{ author: { login: string } | null } | null> }; + reviews: { nodes: Array<{ author: RawActor | null } | null> }; + comments: { nodes: Array<{ author: RawActor | null } | null> }; commits: { nodes: Array<{ commit: { @@ -61,6 +69,7 @@ const SEARCH_QUERY = /* GraphQL */ ` baseRefName headRefName mergedBy { + __typename login } autoMergeRequest { @@ -75,6 +84,7 @@ const SEARCH_QUERY = /* GraphQL */ ` reviews(first: 50) { nodes { author { + __typename login } } @@ -82,6 +92,7 @@ const SEARCH_QUERY = /* GraphQL */ ` comments(first: 50) { nodes { author { + __typename login } } @@ -145,8 +156,8 @@ function pageThrough( function toDependabotPr(raw: RawPullRequest): DependabotPr { const state: PrState = raw.state === 'OPEN' ? 'open' : 'closed'; const merged = raw.state === 'MERGED'; - const reviewers = uniqueLogins(raw.reviews.nodes.map((n) => n?.author?.login)); - const commenters = uniqueLogins(raw.comments.nodes.map((n) => n?.author?.login)); + const reviewers = uniqueLogins(raw.reviews.nodes); + const commenters = uniqueLogins(raw.comments.nodes); return { owner: raw.repository.owner.login, name: raw.repository.name, @@ -157,7 +168,7 @@ function toDependabotPr(raw: RawPullRequest): DependabotPr { createdAt: raw.createdAt, closedAt: raw.closedAt, mergedAt: raw.mergedAt, - mergedBy: raw.mergedBy?.login ?? null, + mergedBy: raw.mergedBy && !isBotActor(raw.mergedBy) ? raw.mergedBy.login : null, headRef: raw.headRefName, baseRef: raw.baseRefName, htmlUrl: raw.url, @@ -204,12 +215,16 @@ function summarizeChecks(raw: RawPullRequest): CheckSummary { return summary; } -function uniqueLogins(values: Array): string[] { +function uniqueLogins(nodes: Array<{ author: RawActor | null } | null>): string[] { const seen = new Set(); - for (const v of values) { - if (!v) continue; - if (v.endsWith('[bot]')) continue; - seen.add(v); + for (const node of nodes) { + const author = node?.author; + if (!author || isBotActor(author)) continue; + seen.add(author.login); } return [...seen].sort(); } + +function isBotActor(actor: RawActor): boolean { + return actor.__typename === 'Bot' || actor.login.endsWith('[bot]'); +} From e6fd770f073ecdfb32e9b3bb61962523f523e771 Mon Sep 17 00:00:00 2001 From: Ben Limmer Date: Tue, 26 May 2026 09:31:09 -0600 Subject: [PATCH 2/2] fix: exclude forked repos from the analysis Forks inherit upstream code and vulnerabilities the org doesn't own, so their Dependabot churn and CVE alerts are noise. Capture the repo fork flag (which the collector wasn't reading) and drop forks in filterRepos alongside archived repos, removing them from CVE exposure, toil, and coverage. --- src/cli.test.ts | 51 ++++++++++++++++++++++++++++++++++++ src/cli.ts | 2 +- src/collectors/repos.test.ts | 5 ++++ src/collectors/repos.ts | 2 ++ src/testFactories.ts | 1 + src/types.ts | 1 + 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/cli.test.ts b/src/cli.test.ts index 98464eb..5675c47 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -120,6 +120,57 @@ test('writes a report when the GitHub calls succeed', async () => { expect(JSON.stringify(analytics.captureCalls)).not.toContain('widgets'); }); +test('excludes forked repos from the crawl', async () => { + const { ctx, githubClient, analytics } = createFakeContext(); + + githubClient.onPaginate('GET /orgs/{org}/repos', {}).resolves([ + { + name: 'widgets', + owner: { login: 'acme' }, + private: true, + visibility: 'private', + archived: false, + fork: false, + default_branch: 'main', + language: 'TypeScript', + pushed_at: '2026-04-01T00:00:00Z', + }, + { + name: 'upstream-fork', + owner: { login: 'acme' }, + private: false, + visibility: 'public', + archived: false, + fork: true, + default_branch: 'main', + language: 'Go', + pushed_at: '2026-04-01T00:00:00Z', + }, + ]); + githubClient.onRequest('GET /repos/{owner}/{repo}/languages', {}).resolves({ TypeScript: 1000 }); + githubClient.onRequest('GET /repos/{owner}/{repo}/contents/{path}', {}).resolves({ + content: Buffer.from('updates:\n - package-ecosystem: "npm"\n', 'utf8').toString('base64'), + encoding: 'base64', + }); + githubClient.onPaginate('GET /repos/{owner}/{repo}/dependabot/alerts', {}).resolves([]); + githubClient + .onRequest('GET /repos/{owner}/{repo}/branches/{branch}/protection', {}) + .fails({ kind: 'not-found', message: 'no protection' }); + githubClient.onRequest('GET /repos/{owner}/{repo}/rules/branches/{branch}', {}).resolves([]); + githubClient.onPaginate('GET /repos/{owner}/{repo}/commits', {}).resolves([]); + githubClient.onGraphql('DependabotPrs').resolves({ + search: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + }); + + const result = await main(ctx, ['acme']); + expect(result.kind).toBe('completed'); + + expect(analytics.capturedEvents('run_completed')[0]?.properties).toMatchObject({ + repos_total: 2, + repos_included: 1, + }); +}); + test('captures run_failed when listOrgRepos fails', async () => { const { ctx, githubClient, analytics } = createFakeContext(); githubClient.onPaginate('GET /orgs/{org}/repos', {}).fails({ kind: 'forbidden', message: 'no access' }); diff --git a/src/cli.ts b/src/cli.ts index c9cd986..b64153d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -363,7 +363,7 @@ export function parseCli(argv: readonly string[]): ParseCliResult { } function filterRepos(repos: RepoMeta[], opts: ResolvedOptions): RepoMeta[] { - let out = repos.filter((r) => !r.archived); + let out = repos.filter((r) => !r.archived && !r.fork); const includeSet = opts.include === null ? null : new Set(opts.include); const excludeSet = new Set(opts.exclude); if (includeSet !== null) out = out.filter((r) => includeSet.has(r.name)); diff --git a/src/collectors/repos.test.ts b/src/collectors/repos.test.ts index bf22a2f..b34ee8d 100644 --- a/src/collectors/repos.test.ts +++ b/src/collectors/repos.test.ts @@ -11,6 +11,7 @@ test('maps raw repo payloads into RepoMeta and infers visibility', async () => { private: false, visibility: 'public', archived: false, + fork: false, default_branch: 'main', language: 'TypeScript', pushed_at: '2026-04-01T00:00:00Z', @@ -22,6 +23,7 @@ test('maps raw repo payloads into RepoMeta and infers visibility', async () => { private: true, visibility: 'internal', archived: true, + fork: true, default_branch: 'main', language: null, pushed_at: null, @@ -38,11 +40,13 @@ test('maps raw repo payloads into RepoMeta and infers visibility', async () => { name: 'widgets', visibility: 'public', archived: false, + fork: false, dependabotSecurityUpdates: true, }); expect(repos[1]).toMatchObject({ visibility: 'internal', archived: true, + fork: true, dependabotSecurityUpdates: null, }); } @@ -58,6 +62,7 @@ test('falls back to the user endpoint when the org endpoint 404s', async () => { private: false, visibility: 'public', archived: false, + fork: false, default_branch: 'main', language: 'TypeScript', pushed_at: '2026-04-01T00:00:00Z', diff --git a/src/collectors/repos.ts b/src/collectors/repos.ts index 028dd01..a103a1e 100644 --- a/src/collectors/repos.ts +++ b/src/collectors/repos.ts @@ -9,6 +9,7 @@ interface RawRepo { private: boolean; visibility?: string; archived: boolean; + fork: boolean; default_branch: string; language: string | null; pushed_at: string | null; @@ -53,6 +54,7 @@ function toRepoMeta(raw: RawRepo): RepoMeta { name: raw.name, visibility, archived: raw.archived, + fork: raw.fork, defaultBranch: raw.default_branch, primaryLanguage: raw.language, pushedAt: raw.pushed_at, diff --git a/src/testFactories.ts b/src/testFactories.ts index 4363f47..a614221 100644 --- a/src/testFactories.ts +++ b/src/testFactories.ts @@ -27,6 +27,7 @@ export const repoMeta = Factory.define(() => ({ name: 'widgets', visibility: 'private', archived: false, + fork: false, defaultBranch: 'main', primaryLanguage: 'TypeScript', pushedAt: '2026-04-01T00:00:00Z', diff --git a/src/types.ts b/src/types.ts index 0e742bb..2f0297e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,6 +10,7 @@ export interface RepoRef { export interface RepoMeta extends RepoRef { visibility: Visibility; archived: boolean; + fork: boolean; defaultBranch: string; primaryLanguage: string | null; pushedAt: string | null;