fix: cache GitHub team members every 30 min (#38)

Author: julio-rocketchat

.changeset/cache-team-members.md

@@ -0,0 +1,5 @@
+---
+"layne": patch
+---
+
+Cache GitHub team member lookups for 30 minutes to avoid redundant API calls on every exception-approve command.

src/__tests__/github.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 
 // Build a mock Octokit instance with spies for the methods we use.
 const mockChecksCreate          = vi.fn().mockResolvedValue({ data: { id: 42 } });
@@ -11,6 +11,7 @@ const mockListPRsForCommit       = vi.fn().mockResolvedValue({ data: [] });
 const mockCompareCommits         = vi.fn().mockResolvedValue({ data: { merge_base_commit: { sha: 'merge-base-sha' } } });
 const mockPullsGet               = vi.fn().mockResolvedValue({ data: { number: 7, head: { sha: 'abc', ref: 'feat' }, base: { sha: 'base', ref: 'main' }, labels: [] } });
 const mockIssuesCreateComment    = vi.fn().mockResolvedValue({});
+const mockListMembersInOrg       = vi.fn().mockResolvedValue({ data: [{ login: 'alice' }, { login: 'bob' }] });
 
 const mockOctokit = {
   checks: {
@@ -31,6 +32,9 @@ const mockOctokit = {
     listPullRequestsAssociatedWithCommit: mockListPRsForCommit,
     compareCommits:                       mockCompareCommits,
   },
+  teams: {
+    listMembersInOrg: mockListMembersInOrg,
+  },
 };
 
 vi.mock('../auth.js', () => ({
@@ -41,6 +45,7 @@ const {
   createCheckRun, startCheckRun, completeCheckRun,
   skipCheckRun, findPullRequestBySha, getMergeBaseSha,
   ensureLabelsExist, setLabels, getPullRequest, createPrComment,
+  getTeamMembers, clearTeamMemberCache,
 } = await import('../github.js');
 
 const BASE = {
@@ -368,3 +373,81 @@ describe('createPrComment()', () => {
     }));
   });
 });
+
+describe('getTeamMembers()', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    clearTeamMemberCache();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it('returns empty array when no team slugs are provided', async () => {
+    const result = await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: [] });
+    expect(result).toEqual([]);
+    expect(mockListMembersInOrg).not.toHaveBeenCalled();
+  });
+
+  it('calls the API and returns members for each slug', async () => {
+    const result = await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+
+    expect(mockListMembersInOrg).toHaveBeenCalledOnce();
+    expect(mockListMembersInOrg).toHaveBeenCalledWith({ org: 'org', team_slug: 'security-team' });
+    expect(result).toEqual(['alice', 'bob']);
+  });
+
+  it('uses the cached result on a second call within TTL', async () => {
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+
+    expect(mockListMembersInOrg).toHaveBeenCalledOnce();
+  });
+
+  it('re-fetches after the TTL expires', async () => {
+    vi.useFakeTimers();
+
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+    vi.advanceTimersByTime(31 * 60 * 1000); // past 30-minute TTL
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+
+    expect(mockListMembersInOrg).toHaveBeenCalledTimes(2);
+  });
+
+  it('caches slugs independently so shared teams are not double-fetched', async () => {
+    mockListMembersInOrg
+      .mockResolvedValueOnce({ data: [{ login: 'alice' }] })
+      .mockResolvedValueOnce({ data: [{ login: 'carol' }] });
+
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['team-a', 'team-b'] });
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['team-a'] });
+
+    // team-a and team-b fetched once each on first call; second call hits cache
+    expect(mockListMembersInOrg).toHaveBeenCalledTimes(2);
+  });
+
+  it('scopes the cache by installationId', async () => {
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['security-team'] });
+    await getTeamMembers({ installationId: 2, org: 'org', teamSlugs: ['security-team'] });
+
+    expect(mockListMembersInOrg).toHaveBeenCalledTimes(2);
+  });
+
+  it('parses org/slug format and uses the explicit org', async () => {
+    await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['other-org/security-team'] });
+
+    expect(mockListMembersInOrg).toHaveBeenCalledWith({ org: 'other-org', team_slug: 'security-team' });
+  });
+
+  it('logs and continues when the API call fails for a slug', async () => {
+    mockListMembersInOrg.mockRejectedValueOnce(new Error('API error'));
+    const error = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    const result = await getTeamMembers({ installationId: 1, org: 'org', teamSlugs: ['bad-team'] });
+
+    expect(result).toEqual([]);
+    expect(error).toHaveBeenCalledWith(expect.stringContaining('[github]'));
+    error.mockRestore();
+  });
+});

src/github.ts

@@ -4,6 +4,20 @@ import type { Annotation } from './types.js';
 
 const CHECK_NAME = 'Layne Security Scan';
 
+const TEAM_CACHE_TTL_MS = 30 * 60 * 1000; // 30 minutes
+
+interface TeamCacheEntry {
+  members: string[];
+  expiresAt: number;
+}
+
+const teamMemberCache = new Map<string, TeamCacheEntry>();
+
+/** Clears the in-process team member cache. Exposed for testing. */
+export function clearTeamMemberCache(): void {
+  teamMemberCache.clear();
+}
+
 /**
  * Creates a Check Run in "queued" state immediately after receiving the webhook.
  * Returns the Check Run ID, which the worker uses to update it later.
@@ -309,12 +323,22 @@ export async function getTeamMembers({ installationId, org, teamSlugs }: {
       teamSlug = slug;
     }
 
+    const cacheKey = `${installationId}:${teamOrg}/${teamSlug}`;
+    const cached = teamMemberCache.get(cacheKey);
+    if (cached && cached.expiresAt > Date.now()) {
+      debug('github', `team cache hit for ${teamOrg}/${teamSlug}: ${cached.members.length} member(s)`);
+      cached.members.forEach(m => members.add(m));
+      continue;
+    }
+
     try {
       const { data } = await octokit.teams.listMembersInOrg({
         org: teamOrg,
         team_slug: teamSlug,
       });
-      data.forEach(m => members.add(m.login));
+      const slugMembers = data.map(m => m.login);
+      slugMembers.forEach(m => members.add(m));
+      teamMemberCache.set(cacheKey, { members: slugMembers, expiresAt: Date.now() + TEAM_CACHE_TTL_MS });
       debug('github', `resolved team ${teamOrg}/${teamSlug}: ${data.length} member(s)`);
     } catch (err) {
       console.error(`[github] Failed to list members for team ${teamOrg}/${teamSlug}: ${(err as Error).message}`);