feat(control-plane): Centralize GitHub client/auth/config caching

[dblinkhorn]

This PR introduces a singleton cache for Octokit clients, authentication objects, and GitHub App configuration, so we can avoid making redundant SSM lookups and client creation.

This PR represents the first step in what will be a broader effort to modularize GitHub interactions. Future PRs will build on this one by adding runner management, job operations, advanced caching for runner and job metadata, etc.

[npalm]

@dblinkhorn thx, not digged in the PR it self. But please can you run yarn format to fix the prettier errors in CI.

[Copilot]

Pull request overview

This PR introduces a singleton caching layer for GitHub API clients, authentication objects, and app configuration to optimize performance by avoiding redundant SSM parameter lookups and client instantiations. The caching implementation uses in-memory Maps for storing Octokit clients, app/installation authentication objects, and GitHub App configuration.

Key changes:

• Created a new cache module (github/cache.ts) with functions to cache and retrieve Octokit clients, authentication objects, and app configs
• Refactored existing authentication functions in auth.ts to utilize the new caching layer
• Added cache reset functionality in test setup to ensure test isolation

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
lambdas/functions/control-plane/src/github/types.ts	Defines GithubAppConfig interface for GitHub App configuration with appId, privateKey, and optional installationId
lambdas/functions/control-plane/src/github/index.ts	Exports cache and types modules to make them available to other parts of the application
lambdas/functions/control-plane/src/github/cache.ts	Implements singleton cache using Map structures with cache key generators and getter functions for clients, auth objects, and configs
lambdas/functions/control-plane/src/github/auth.ts	Integrates caching into existing authentication and client creation functions to avoid redundant SSM lookups
lambdas/functions/control-plane/src/github/auth.test.ts	Adds cache reset call in test setup and updates import statement to use type imports
lambdas/functions/control-plane/src/local.ts	Removes an eslint-disable comment that is no longer needed

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Installation access tokens have expiration times (typically 1 hour) and should not be cached indefinitely. The current implementation caches InstallationAccessTokenAuthentication objects without considering token expiration, which could lead to using expired tokens.

Consider either:

1. Not caching installation auth objects at all, or
2. Implementing TTL-based caching that respects the token's expiresAt field from the authentication object

[npalm]

@copilot Lambda max TTL is 15 minutes

[Copilot]

Storing tokens directly in cache keys is a security concern as it could expose sensitive credentials in memory dumps or logs. The full token value should not be part of the cache key.

Consider using a hash of the token instead:

import { createHash } from 'crypto';

export function createTokenCacheKey(token: string, ghesApiUrl: string = '') {
  const tokenHash = createHash('sha256').update(token).digest('hex').substring(0, 16);
  return `octokit-${tokenHash}-${ghesApiUrl}`;
}

[Copilot]

The installationId field is defined in the GithubAppConfig interface but is never populated or used. It should either be:

1. Removed from the interface if not needed, or
2. Populated in the createAuth function (line 109-112) if it's intended to be part of the config

Currently, the installationId is handled separately in createAuth (line 119) rather than being part of the cached config.

Suggested change

installationId?: number;

[Copilot]

The cache functions and key generation functions lack JSDoc documentation. Adding documentation would help other developers understand:

• The caching strategy and behavior
• When cached values are returned vs when the creator function is called
• The cache key format and what parameters affect it
• The purpose of the reset() function

Example:

/**
 * Creates a cache key for Octokit clients based on token and API URL.
 * @param token - The authentication token
 * @param ghesApiUrl - Optional GitHub Enterprise Server API URL
 * @returns A unique cache key for this token/URL combination
 */
export function createTokenCacheKey(token: string, ghesApiUrl: string = '') {
  return `octokit-${token}-${ghesApiUrl}`;
}

[Copilot]

The cache key generation uses installationId || 'none' which treats falsy values (0, null, undefined) the same way. While 0 is likely not a valid GitHub installation ID, this could lead to cache key collisions for different falsy values.

Consider using nullish coalescing instead for more precise handling:

const id = installationId ?? 'none';

This will only treat null and undefined as 'none', while preserving 0 if it were ever a valid value.

Suggested change

	const id = installationId || 'none';
	const id = installationId ?? 'none';

[Copilot]

String argument /[\\n]/g looks like a regular expression, but it will be interpreted as a string.

Suggested change

	.replace('/[\\n]/g', String.fromCharCode(10));
	.replace(/\\n/g, String.fromCharCode(10));