From 25b51f2a1e6547514e6caad4d9ffb06cb7c8702f Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Mon, 16 Mar 2026 19:35:45 +0000 Subject: [PATCH] refactor: remove unused exported functions from core modules --- src/config/env.ts | 17 -- src/github/client.ts | 62 ----- src/pm/lifecycle.ts | 2 +- src/trello/client.ts | 2 +- src/trello/types.ts | 2 - src/triggers/shared/backlog-check.ts | 60 ----- tests/helpers/sharedMocks.ts | 3 - tests/unit/github/client.test.ts | 112 --------- tests/unit/pm/lifecycle.test.ts | 21 -- tests/unit/trello/client.test.ts | 20 +- .../triggers/shared/backlog-check.test.ts | 236 +----------------- 11 files changed, 5 insertions(+), 532 deletions(-) diff --git a/src/config/env.ts b/src/config/env.ts index a39d7a9a..36b9934b 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -5,27 +5,10 @@ export interface EnvConfig { sentryDsn?: string; } -function getEnvOrThrow(key: string): string { - const value = process.env[key]; - if (!value) { - throw new Error(`Missing required environment variable: ${key}`); - } - return value; -} - function getEnvOrDefault(key: string, defaultValue: string): string { return process.env[key] || defaultValue; } -export function loadEnvConfig(): EnvConfig { - return { - port: Number.parseInt(getEnvOrDefault('PORT', '3000'), 10), - logLevel: getEnvOrDefault('LOG_LEVEL', 'info'), - databaseUrl: getEnvOrThrow('DATABASE_URL'), - sentryDsn: process.env.SENTRY_DSN, - }; -} - export function loadEnvConfigSafe(): Omit & { databaseUrl?: string } { return { port: Number.parseInt(getEnvOrDefault('PORT', '3000'), 10), diff --git a/src/github/client.ts b/src/github/client.ts index 9cff0f92..b82b31b1 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -115,16 +115,6 @@ export interface CreatedPR { title: string; } -export type GitHubReactionContent = - | '+1' - | '-1' - | 'laugh' - | 'confused' - | 'heart' - | 'hooray' - | 'rocket' - | 'eyes'; - export const githubClient = { async getPR(owner: string, repo: string, prNumber: number): Promise { logger.debug('Fetching PR', { owner, repo, prNumber }); @@ -423,36 +413,6 @@ export const githubClient = { }; }, - async addIssueCommentReaction( - owner: string, - repo: string, - commentId: number, - content: GitHubReactionContent, - ): Promise { - logger.debug('Adding reaction to issue comment', { owner, repo, commentId, content }); - await getClient().reactions.createForIssueComment({ - owner, - repo, - comment_id: commentId, - content, - }); - }, - - async addReviewCommentReaction( - owner: string, - repo: string, - commentId: number, - content: GitHubReactionContent, - ): Promise { - logger.debug('Adding reaction to review comment', { owner, repo, commentId, content }); - await getClient().reactions.createForPullRequestReviewComment({ - owner, - repo, - comment_id: commentId, - content, - }); - }, - async getFailedWorkflowRunJobs( owner: string, repo: string, @@ -513,23 +473,6 @@ export const githubClient = { }; }, - async branchExists(owner: string, repo: string, branch: string): Promise { - logger.debug('Checking if branch exists', { owner, repo, branch }); - try { - await getClient().repos.getBranch({ - owner, - repo, - branch, - }); - return true; - } catch (error) { - if (error instanceof Error && 'status' in error && error.status === 404) { - return false; - } - throw error; - } - }, - async mergePR( owner: string, repo: string, @@ -546,11 +489,6 @@ export const githubClient = { }, }; -export async function getAuthenticatedUser(): Promise { - const { data } = await getClient().users.getAuthenticated(); - return data.login; -} - export async function getGitHubUserForToken(token: string | null): Promise { if (!token) return null; diff --git a/src/pm/lifecycle.ts b/src/pm/lifecycle.ts index e3970ca2..cbeae340 100644 --- a/src/pm/lifecycle.ts +++ b/src/pm/lifecycle.ts @@ -48,7 +48,7 @@ export function hasAutoLabel( * Extract a human-readable PR title from a GitHub PR URL. * E.g. "https://github.com/owner/repo/pull/123" → "Pull Request #123" */ -export function extractPRTitle(prUrl: string): string { +function extractPRTitle(prUrl: string): string { const match = prUrl.match(/\/pull\/(\d+)/); return match ? `Pull Request #${match[1]}` : 'Pull Request'; } diff --git a/src/trello/client.ts b/src/trello/client.ts index 86881b01..cf71b0db 100644 --- a/src/trello/client.ts +++ b/src/trello/client.ts @@ -12,7 +12,7 @@ export function withTrelloCredentials( return trelloCredentialStore.run(creds, fn); } -export function getTrelloCredentials(): TrelloCredentials { +function getTrelloCredentials(): TrelloCredentials { const scoped = trelloCredentialStore.getStore(); if (!scoped) { throw new Error( diff --git a/src/trello/types.ts b/src/trello/types.ts index 9501a01e..cb23e704 100644 --- a/src/trello/types.ts +++ b/src/trello/types.ts @@ -1,5 +1,3 @@ -export type { TrelloCard, TrelloComment, TrelloAttachment } from './client.js'; - export interface TrelloCredentials { apiKey: string; token: string; diff --git a/src/triggers/shared/backlog-check.ts b/src/triggers/shared/backlog-check.ts index 8b165b05..e9b1df1b 100644 --- a/src/triggers/shared/backlog-check.ts +++ b/src/triggers/shared/backlog-check.ts @@ -184,63 +184,3 @@ async function checkJiraCapacity( return { atCapacity: false, reason: 'below-capacity', inFlightCount, limit }; } - -// --------------------------------------------------------------------------- -// isBacklogEmpty (deprecated) -// --------------------------------------------------------------------------- - -/** - * @deprecated Use `isPipelineAtCapacity` instead. - * - * Returns `true` when the project's backlog list/queue is empty. - * - * Supports Trello and JIRA. For any other provider type, or when required - * config fields are missing, returns `false` (conservative: let the agent run). - * - * @param project - Resolved project configuration - * @param provider - An initialised PM provider instance - */ -export async function isBacklogEmpty( - project: ProjectConfig, - provider: PMProvider, -): Promise { - try { - if (provider.type === 'trello') { - const backlogListId = getTrelloConfig(project)?.lists?.backlog; - if (!backlogListId) { - logger.warn('isBacklogEmpty: no backlog list configured for Trello project', { - projectId: project.id, - }); - return false; - } - const items = await provider.listWorkItems(backlogListId); - return items.length === 0; - } - - if (provider.type === 'jira') { - const jiraConfig = getJiraConfig(project); - const backlogStatus = jiraConfig?.statuses?.backlog; - const projectKey = jiraConfig?.projectKey; - if (!backlogStatus || !projectKey) { - logger.warn('isBacklogEmpty: no backlog status or projectKey configured for JIRA project', { - projectId: project.id, - }); - return false; - } - const items = await provider.listWorkItems(projectKey, { status: backlogStatus }); - return items.length === 0; - } - - logger.warn('isBacklogEmpty: unsupported PM provider type', { - providerType: provider.type, - projectId: project.id, - }); - return false; - } catch (err) { - logger.warn('isBacklogEmpty: failed to check backlog, assuming non-empty', { - projectId: project.id, - error: String(err), - }); - return false; - } -} diff --git a/tests/helpers/sharedMocks.ts b/tests/helpers/sharedMocks.ts index 1e7003cc..ab313520 100644 --- a/tests/helpers/sharedMocks.ts +++ b/tests/helpers/sharedMocks.ts @@ -103,10 +103,7 @@ export const mockGithubClient = { createPRReview: vi.fn(), getOpenPRByBranch: vi.fn(), createPR: vi.fn(), - addIssueCommentReaction: vi.fn(), - addReviewCommentReaction: vi.fn(), getFailedWorkflowRunJobs: vi.fn(), - branchExists: vi.fn(), mergePR: vi.fn(), } satisfies GitHubClientContract; diff --git a/tests/unit/github/client.test.ts b/tests/unit/github/client.test.ts index caedb67f..92ebd7ef 100644 --- a/tests/unit/github/client.test.ts +++ b/tests/unit/github/client.test.ts @@ -28,15 +28,6 @@ const mockActions = { listJobsForWorkflowRun: vi.fn(), }; -const mockReactions = { - createForIssueComment: vi.fn(), - createForPullRequestReviewComment: vi.fn(), -}; - -const mockRepos = { - getBranch: vi.fn(), -}; - const mockUsers = { getAuthenticated: vi.fn(), }; @@ -47,8 +38,6 @@ vi.mock('@octokit/rest', () => ({ issues: mockIssues, checks: mockChecks, actions: mockActions, - reactions: mockReactions, - repos: mockRepos, users: mockUsers, })), })); @@ -63,7 +52,6 @@ vi.mock('../../../src/utils/logging.js', () => ({ })); import { - getAuthenticatedUser, getGitHubUserForToken, githubClient, withGitHubToken, @@ -646,50 +634,6 @@ describe('githubClient', () => { }); }); - describe('branchExists', () => { - it('returns true when branch exists', async () => { - mockRepos.getBranch.mockResolvedValue({ data: {} }); - - const result = await withGitHubToken('test-token', () => - githubClient.branchExists('owner', 'repo', 'main'), - ); - - expect(result).toBe(true); - }); - - it('returns false when branch does not exist (404)', async () => { - const error = new Error('Not Found') as Error & { status: number }; - error.status = 404; - mockRepos.getBranch.mockRejectedValue(error); - - const result = await withGitHubToken('test-token', () => - githubClient.branchExists('owner', 'repo', 'nonexistent'), - ); - - expect(result).toBe(false); - }); - - it('throws on other errors', async () => { - mockRepos.getBranch.mockRejectedValue(new Error('Server Error')); - - await expect( - withGitHubToken('test-token', () => githubClient.branchExists('owner', 'repo', 'branch')), - ).rejects.toThrow('Server Error'); - }); - }); - - describe('getAuthenticatedUser', () => { - it('returns authenticated user login', async () => { - mockUsers.getAuthenticated.mockResolvedValue({ - data: { login: 'cascade-bot' }, - }); - - const result = await withGitHubToken('test-token', () => getAuthenticatedUser()); - - expect(result).toBe('cascade-bot'); - }); - }); - describe('withGitHubToken', () => { it('scopes a different Octokit instance within the callback', async () => { mockPulls.get.mockResolvedValue({ @@ -715,62 +659,6 @@ describe('githubClient', () => { }); }); - describe('addIssueCommentReaction', () => { - it('calls reactions.createForIssueComment with correct params', async () => { - mockReactions.createForIssueComment.mockResolvedValue({ data: {} }); - - await withGitHubToken('test-token', () => - githubClient.addIssueCommentReaction('owner', 'repo', 42, 'eyes'), - ); - - expect(mockReactions.createForIssueComment).toHaveBeenCalledWith({ - owner: 'owner', - repo: 'repo', - comment_id: 42, - content: 'eyes', - }); - }); - - it('propagates errors from the API', async () => { - mockReactions.createForIssueComment.mockRejectedValue(new Error('403 Forbidden')); - - await expect( - withGitHubToken('test-token', () => - githubClient.addIssueCommentReaction('owner', 'repo', 42, 'eyes'), - ), - ).rejects.toThrow('403 Forbidden'); - }); - }); - - describe('addReviewCommentReaction', () => { - it('calls reactions.createForPullRequestReviewComment with correct params', async () => { - mockReactions.createForPullRequestReviewComment.mockResolvedValue({ data: {} }); - - await withGitHubToken('test-token', () => - githubClient.addReviewCommentReaction('owner', 'repo', 99, 'heart'), - ); - - expect(mockReactions.createForPullRequestReviewComment).toHaveBeenCalledWith({ - owner: 'owner', - repo: 'repo', - comment_id: 99, - content: 'heart', - }); - }); - - it('propagates errors from the API', async () => { - mockReactions.createForPullRequestReviewComment.mockRejectedValue( - new Error('422 Unprocessable'), - ); - - await expect( - withGitHubToken('test-token', () => - githubClient.addReviewCommentReaction('owner', 'repo', 99, 'eyes'), - ), - ).rejects.toThrow('422 Unprocessable'); - }); - }); - describe('getGitHubUserForToken', () => { it('returns null when token is null', async () => { const result = await getGitHubUserForToken(null); diff --git a/tests/unit/pm/lifecycle.test.ts b/tests/unit/pm/lifecycle.test.ts index c840af29..c0c519e3 100644 --- a/tests/unit/pm/lifecycle.test.ts +++ b/tests/unit/pm/lifecycle.test.ts @@ -28,33 +28,12 @@ import '../../../src/pm/index.js'; import { PMLifecycleManager, type ProjectPMConfig, - extractPRTitle, resolveProjectPMConfig, } from '../../../src/pm/lifecycle.js'; import type { PMProvider } from '../../../src/pm/types.js'; import type { ProjectConfig } from '../../../src/types/index.js'; describe('pm/lifecycle', () => { - describe('extractPRTitle', () => { - it('extracts PR number from a standard GitHub PR URL', () => { - expect(extractPRTitle('https://github.com/owner/repo/pull/123')).toBe('Pull Request #123'); - }); - - it('extracts PR number from a PR URL with trailing path', () => { - expect(extractPRTitle('https://github.com/owner/repo/pull/42/files')).toBe( - 'Pull Request #42', - ); - }); - - it('returns generic title when URL does not contain /pull/', () => { - expect(extractPRTitle('https://example.com/no-pull-here')).toBe('Pull Request'); - }); - - it('returns generic title for empty string', () => { - expect(extractPRTitle('')).toBe('Pull Request'); - }); - }); - describe('resolveProjectPMConfig', () => { it('returns JIRA config when project type is jira', () => { const project: ProjectConfig = { diff --git a/tests/unit/trello/client.test.ts b/tests/unit/trello/client.test.ts index 4e7d9567..b7e4c639 100644 --- a/tests/unit/trello/client.test.ts +++ b/tests/unit/trello/client.test.ts @@ -42,11 +42,7 @@ vi.mock('trello.js', () => ({ })); import { TrelloClient } from 'trello.js'; -import { - getTrelloCredentials, - trelloClient, - withTrelloCredentials, -} from '../../../src/trello/client.js'; +import { trelloClient, withTrelloCredentials } from '../../../src/trello/client.js'; describe('trelloClient', () => { const creds = { apiKey: 'test-key', token: 'test-token' }; @@ -279,20 +275,6 @@ describe('trelloClient', () => { }); }); - describe('getTrelloCredentials', () => { - it('throws when called outside scope', () => { - expect(() => getTrelloCredentials()).toThrow('No Trello credentials in scope'); - }); - - it('returns credentials when inside scope', async () => { - let captured: ReturnType | undefined; - await withTrelloCredentials(creds, async () => { - captured = getTrelloCredentials(); - }); - expect(captured).toEqual(creds); - }); - }); - describe('getCard', () => { it('returns a card with normalized fields', async () => { mockCards.getCard.mockResolvedValue({ diff --git a/tests/unit/triggers/shared/backlog-check.test.ts b/tests/unit/triggers/shared/backlog-check.test.ts index fd6c7906..89eff0a6 100644 --- a/tests/unit/triggers/shared/backlog-check.test.ts +++ b/tests/unit/triggers/shared/backlog-check.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; // --------------------------------------------------------------------------- // Hoisted mocks @@ -24,10 +24,7 @@ vi.mock('../../../../src/utils/logging.js', () => ({ logger: mockLogger, })); -import { - isBacklogEmpty, - isPipelineAtCapacity, -} from '../../../../src/triggers/shared/backlog-check.js'; +import { isPipelineAtCapacity } from '../../../../src/triggers/shared/backlog-check.js'; import { createMockJiraProject, createMockProject } from '../../../helpers/factories.js'; // --------------------------------------------------------------------------- @@ -593,232 +590,3 @@ describe('isPipelineAtCapacity', () => { }); }); }); - -// --------------------------------------------------------------------------- -// isBacklogEmpty tests (deprecated — kept for backward compat) -// --------------------------------------------------------------------------- - -describe('isBacklogEmpty', () => { - // ========================================================================= - // Trello - // ========================================================================= - - describe('Trello', () => { - const trelloProject = createMockProject({ - trello: { - boardId: 'board-1', - lists: { backlog: 'backlog-list-id', todo: 'todo-list-id' }, - labels: {}, - }, - }); - - it('returns true when the Trello backlog list is empty', async () => { - mockGetTrelloConfig.mockReturnValue({ lists: { backlog: 'backlog-list-id' } }); - const provider = { - type: 'trello' as const, - listWorkItems: vi.fn().mockResolvedValue([]), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(trelloProject, provider); - - expect(result).toBe(true); - expect(provider.listWorkItems).toHaveBeenCalledWith('backlog-list-id'); - }); - - it('returns false when the Trello backlog list has items', async () => { - mockGetTrelloConfig.mockReturnValue({ lists: { backlog: 'backlog-list-id' } }); - const provider = { - type: 'trello' as const, - listWorkItems: vi.fn().mockResolvedValue([{ id: 'card-1' }, { id: 'card-2' }]), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(trelloProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).toHaveBeenCalledWith('backlog-list-id'); - }); - - it('returns false when Trello config has no backlog list configured', async () => { - mockGetTrelloConfig.mockReturnValue({ lists: {} }); // no backlog key - const provider = { - type: 'trello' as const, - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(trelloProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'isBacklogEmpty: no backlog list configured for Trello project', - expect.objectContaining({ projectId: trelloProject.id }), - ); - }); - - it('returns false when Trello config is missing entirely', async () => { - mockGetTrelloConfig.mockReturnValue(undefined); - const provider = { - type: 'trello' as const, - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(trelloProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - }); - - it('returns false when the Trello API throws an error (conservative fallback)', async () => { - mockGetTrelloConfig.mockReturnValue({ lists: { backlog: 'backlog-list-id' } }); - const provider = { - type: 'trello' as const, - listWorkItems: vi.fn().mockRejectedValue(new Error('network error')), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(trelloProject, provider); - - expect(result).toBe(false); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'isBacklogEmpty: failed to check backlog, assuming non-empty', - expect.objectContaining({ projectId: trelloProject.id, error: expect.any(String) }), - ); - }); - }); - - // ========================================================================= - // JIRA - // ========================================================================= - - describe('JIRA', () => { - const jiraProject = createMockJiraProject({ - jira: { - projectKey: 'PROJ', - baseUrl: 'https://test.atlassian.net', - statuses: { backlog: 'Backlog', splitting: 'Briefing' }, - }, - }); - - it('returns true when the JIRA backlog status has no items', async () => { - mockGetJiraConfig.mockReturnValue({ - projectKey: 'PROJ', - statuses: { backlog: 'Backlog' }, - }); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn().mockResolvedValue([]), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(true); - expect(provider.listWorkItems).toHaveBeenCalledWith('PROJ', { status: 'Backlog' }); - }); - - it('returns false when the JIRA backlog status has items', async () => { - mockGetJiraConfig.mockReturnValue({ - projectKey: 'PROJ', - statuses: { backlog: 'Backlog' }, - }); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn().mockResolvedValue([{ id: 'PROJ-1' }]), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).toHaveBeenCalledWith('PROJ', { status: 'Backlog' }); - }); - - it('returns false when JIRA config has no backlog status', async () => { - mockGetJiraConfig.mockReturnValue({ - projectKey: 'PROJ', - statuses: {}, // no backlog key - }); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'isBacklogEmpty: no backlog status or projectKey configured for JIRA project', - expect.objectContaining({ projectId: jiraProject.id }), - ); - }); - - it('returns false when JIRA config has no projectKey', async () => { - mockGetJiraConfig.mockReturnValue({ - statuses: { backlog: 'Backlog' }, - // no projectKey - }); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - }); - - it('returns false when JIRA config is missing entirely', async () => { - mockGetJiraConfig.mockReturnValue(undefined); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - }); - - it('returns false when the JIRA API throws an error (conservative fallback)', async () => { - mockGetJiraConfig.mockReturnValue({ - projectKey: 'PROJ', - statuses: { backlog: 'Backlog' }, - }); - const provider = { - type: 'jira' as const, - listWorkItems: vi.fn().mockRejectedValue(new Error('network error')), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(jiraProject, provider); - - expect(result).toBe(false); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'isBacklogEmpty: failed to check backlog, assuming non-empty', - expect.objectContaining({ projectId: jiraProject.id, error: expect.any(String) }), - ); - }); - }); - - // ========================================================================= - // Unsupported provider type - // ========================================================================= - - describe('unsupported provider type', () => { - it('returns false for an unknown provider type', async () => { - const project = createMockProject(); - const provider = { - type: 'unknown-provider' as unknown as 'trello', - listWorkItems: vi.fn(), - } as unknown as Parameters[1]; - - const result = await isBacklogEmpty(project, provider); - - expect(result).toBe(false); - expect(provider.listWorkItems).not.toHaveBeenCalled(); - expect(mockLogger.warn).toHaveBeenCalledWith( - 'isBacklogEmpty: unsupported PM provider type', - expect.objectContaining({ providerType: 'unknown-provider' }), - ); - }); - }); -});