From dca0e90d0dc2fe67f485c3c77b61bd5e870aeaad Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Wed, 11 Feb 2026 13:57:22 +0000 Subject: [PATCH] test: add comprehensive unit tests for triggers, github client, config, and utils Add 17 new test files covering previously untested modules, increasing test count from 139 to 436 and line coverage from 16.4% to 29.2%. Tier 1 - Triggers and core logic (10 files): - GitHub type guards: all 5 validators with valid/invalid/edge cases - PR opened: matches(), handle() with Trello URL extraction - Check suite success: CI pass detection, PR fetching, check verification - Check suite failure: failure detection, attempt tracking, max retries - PR review submitted: self-review skip, bot detection, auth fallback - Issue comment: PR vs issue distinction, self-comment skip - PR review comment: file path extraction, auth error tolerance - PR ready to merge: dual-trigger (check_suite + review), merge readiness - Projects config: load/cache/clear lifecycle, finder functions, tokens - Safe operation: error swallowing with/without logging Tier 2 - Client, utils, and gadgets (7 files): - GitHub client: all 15+ Octokit methods with response mapping - LLM metrics: cost calculation, token estimation, structured logging - Todo storage: init/load/save/format with fs mocking - Lifecycle: timer management, watchdog, shutdown scheduling - Attachment added: zip pattern parsing, user verification, debug loop prevention - Repo utils: workspace dirs, temp dirs, clone, runCommand with spawn mocking - LLM logging: call number formatting, request/response file logging Co-Authored-By: Claude Opus 4.6 --- tests/unit/config/projects.test.ts | 181 ++++++ tests/unit/gadgets/todo-storage.test.ts | 169 +++++ tests/unit/github/client.test.ts | 594 ++++++++++++++++++ tests/unit/triggers/attachment-added.test.ts | 367 +++++++++++ .../unit/triggers/check-suite-failure.test.ts | 339 ++++++++++ .../unit/triggers/check-suite-success.test.ts | 230 +++++++ tests/unit/triggers/github-types.test.ts | 326 ++++++++++ tests/unit/triggers/issue-comment.test.ts | 251 ++++++++ tests/unit/triggers/pr-opened.test.ts | 231 +++++++ tests/unit/triggers/pr-ready-to-merge.test.ts | 563 +++++++++++++++++ tests/unit/triggers/pr-review-comment.test.ts | 239 +++++++ .../unit/triggers/pr-review-submitted.test.ts | 260 ++++++++ tests/unit/utils/lifecycle.test.ts | 166 +++++ tests/unit/utils/llmLogging.test.ts | 142 +++++ tests/unit/utils/llmMetrics.test.ts | 148 +++++ tests/unit/utils/repo.test.ts | 203 ++++++ tests/unit/utils/safeOperation.test.ts | 81 +++ 17 files changed, 4490 insertions(+) create mode 100644 tests/unit/config/projects.test.ts create mode 100644 tests/unit/gadgets/todo-storage.test.ts create mode 100644 tests/unit/github/client.test.ts create mode 100644 tests/unit/triggers/attachment-added.test.ts create mode 100644 tests/unit/triggers/check-suite-failure.test.ts create mode 100644 tests/unit/triggers/check-suite-success.test.ts create mode 100644 tests/unit/triggers/github-types.test.ts create mode 100644 tests/unit/triggers/issue-comment.test.ts create mode 100644 tests/unit/triggers/pr-opened.test.ts create mode 100644 tests/unit/triggers/pr-ready-to-merge.test.ts create mode 100644 tests/unit/triggers/pr-review-comment.test.ts create mode 100644 tests/unit/triggers/pr-review-submitted.test.ts create mode 100644 tests/unit/utils/lifecycle.test.ts create mode 100644 tests/unit/utils/llmLogging.test.ts create mode 100644 tests/unit/utils/llmMetrics.test.ts create mode 100644 tests/unit/utils/repo.test.ts create mode 100644 tests/unit/utils/safeOperation.test.ts diff --git a/tests/unit/config/projects.test.ts b/tests/unit/config/projects.test.ts new file mode 100644 index 00000000..bbd05c36 --- /dev/null +++ b/tests/unit/config/projects.test.ts @@ -0,0 +1,181 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('node:fs', () => ({ + existsSync: vi.fn(), + readFileSync: vi.fn(), +})); + +vi.mock('../../../src/config/schema.js', () => ({ + validateConfig: vi.fn((config: unknown) => config), +})); + +import { existsSync, readFileSync } from 'node:fs'; +import { + clearConfigCache, + findProjectByBoardId, + findProjectById, + findProjectByRepo, + getProjectGitHubToken, + loadProjectsConfig, +} from '../../../src/config/projects.js'; + +describe('projects config', () => { + const mockConfig = { + defaults: { + model: 'test-model', + maxIterations: 50, + }, + projects: [ + { + id: 'project1', + name: 'Project 1', + repo: 'owner/repo1', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN_1', + trello: { + boardId: 'board1', + lists: { todo: 'list1' }, + labels: {}, + }, + }, + { + id: 'project2', + name: 'Project 2', + repo: 'owner/repo2', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board2', + lists: { todo: 'list2' }, + labels: {}, + }, + }, + ], + }; + + beforeEach(() => { + vi.clearAllMocks(); + clearConfigCache(); + }); + + afterEach(() => { + clearConfigCache(); + }); + + describe('loadProjectsConfig', () => { + it('loads and validates config from file', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfig)); + + const result = loadProjectsConfig('/path/to/config.json'); + + expect(existsSync).toHaveBeenCalledWith('/path/to/config.json'); + expect(readFileSync).toHaveBeenCalledWith('/path/to/config.json', 'utf-8'); + expect(result).toEqual(mockConfig); + }); + + it('throws when config file does not exist', () => { + vi.mocked(existsSync).mockReturnValue(false); + + expect(() => loadProjectsConfig('/missing/config.json')).toThrow( + 'Config file not found: /missing/config.json', + ); + }); + + it('caches config after first load', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfig)); + + loadProjectsConfig('/path/to/config.json'); + loadProjectsConfig('/path/to/config.json'); + + expect(readFileSync).toHaveBeenCalledTimes(1); + }); + + it('reloads config after cache is cleared', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfig)); + + loadProjectsConfig('/path/to/config.json'); + clearConfigCache(); + loadProjectsConfig('/path/to/config.json'); + + expect(readFileSync).toHaveBeenCalledTimes(2); + }); + }); + + describe('findProjectByBoardId', () => { + it('finds project by board ID', () => { + const result = findProjectByBoardId(mockConfig, 'board1'); + expect(result?.id).toBe('project1'); + }); + + it('returns undefined for unknown board ID', () => { + const result = findProjectByBoardId(mockConfig, 'unknown'); + expect(result).toBeUndefined(); + }); + }); + + describe('findProjectById', () => { + it('finds project by ID', () => { + const result = findProjectById(mockConfig, 'project2'); + expect(result?.id).toBe('project2'); + }); + + it('returns undefined for unknown ID', () => { + const result = findProjectById(mockConfig, 'unknown'); + expect(result).toBeUndefined(); + }); + }); + + describe('findProjectByRepo', () => { + it('finds project by repo full name', () => { + const result = findProjectByRepo(mockConfig, 'owner/repo1'); + expect(result?.id).toBe('project1'); + }); + + it('returns undefined for unknown repo', () => { + const result = findProjectByRepo(mockConfig, 'owner/unknown'); + expect(result).toBeUndefined(); + }); + }); + + describe('getProjectGitHubToken', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('returns token from environment variable', () => { + process.env.GITHUB_TOKEN_1 = 'test-token-123'; + + const result = getProjectGitHubToken(mockConfig.projects[0]); + + expect(result).toBe('test-token-123'); + }); + + it('uses GITHUB_TOKEN as default when githubTokenEnv is not set', () => { + process.env.GITHUB_TOKEN = 'default-token'; + + const project = { ...mockConfig.projects[0], githubTokenEnv: undefined }; + const result = getProjectGitHubToken(project); + + expect(result).toBe('default-token'); + }); + + it('throws when token environment variable is not set', () => { + process.env.GITHUB_TOKEN_1 = undefined; + + expect(() => getProjectGitHubToken(mockConfig.projects[0])).toThrow( + 'Missing GitHub token for project project1: GITHUB_TOKEN_1 not set', + ); + }); + }); +}); diff --git a/tests/unit/gadgets/todo-storage.test.ts b/tests/unit/gadgets/todo-storage.test.ts new file mode 100644 index 00000000..7bb6b8c0 --- /dev/null +++ b/tests/unit/gadgets/todo-storage.test.ts @@ -0,0 +1,169 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('node:fs', () => ({ + existsSync: vi.fn(), + mkdirSync: vi.fn(), + readFileSync: vi.fn(), + writeFileSync: vi.fn(), +})); + +vi.mock('../../../src/utils/repo.js', () => ({ + getWorkspaceDir: vi.fn(() => '/workspace'), +})); + +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { + type Todo, + formatTodoList, + getNextId, + getSessionId, + initTodoSession, + loadTodos, + saveTodos, +} from '../../../src/gadgets/todo/storage.js'; + +describe('todo storage', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset session state by re-initializing + vi.mocked(existsSync).mockReturnValue(true); + }); + + describe('initTodoSession', () => { + it('sets session ID and ensures directory exists', () => { + vi.mocked(existsSync).mockReturnValue(false); + + initTodoSession('test-session-123'); + + expect(getSessionId()).toBe('test-session-123'); + expect(mkdirSync).toHaveBeenCalledWith(expect.stringContaining('todos'), { recursive: true }); + }); + }); + + describe('getSessionId', () => { + it('returns initialized session ID', () => { + initTodoSession('my-session'); + expect(getSessionId()).toBe('my-session'); + }); + }); + + describe('loadTodos', () => { + it('returns empty array when session file does not exist', () => { + initTodoSession('load-test'); + vi.mocked(existsSync) + .mockReturnValueOnce(true) // todos dir exists + .mockReturnValueOnce(false); // session file doesn't exist + + const todos = loadTodos(); + + expect(todos).toEqual([]); + }); + + it('loads todos from session file', () => { + initTodoSession('load-test-2'); + const mockTodos: Todo[] = [ + { + id: '1', + content: 'First task', + status: 'pending', + createdAt: '2024-01-01', + updatedAt: '2024-01-01', + }, + ]; + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockTodos)); + + const todos = loadTodos(); + + expect(todos).toEqual(mockTodos); + }); + }); + + describe('saveTodos', () => { + it('writes todos to session file', () => { + initTodoSession('save-test'); + const todos: Todo[] = [ + { + id: '1', + content: 'Task 1', + status: 'done', + createdAt: '2024-01-01', + updatedAt: '2024-01-01', + }, + ]; + + saveTodos(todos); + + expect(writeFileSync).toHaveBeenCalledWith( + expect.stringContaining('save-test.json'), + JSON.stringify(todos, null, 2), + ); + }); + }); + + describe('getNextId', () => { + it('returns "1" for empty list', () => { + expect(getNextId([])).toBe('1'); + }); + + it('returns next sequential ID', () => { + const todos: Todo[] = [ + { id: '1', content: 'First', status: 'pending', createdAt: '', updatedAt: '' }, + { id: '3', content: 'Third', status: 'pending', createdAt: '', updatedAt: '' }, + ]; + + expect(getNextId(todos)).toBe('4'); + }); + + it('handles non-numeric IDs gracefully', () => { + const todos: Todo[] = [ + { id: 'abc', content: 'Non-numeric', status: 'pending', createdAt: '', updatedAt: '' }, + ]; + + expect(getNextId(todos)).toBe('1'); + }); + }); + + describe('formatTodoList', () => { + it('returns empty message for no todos', () => { + expect(formatTodoList([])).toContain('empty'); + }); + + it('formats todos with status icons', () => { + const todos: Todo[] = [ + { id: '1', content: 'Pending task', status: 'pending', createdAt: '', updatedAt: '' }, + { + id: '2', + content: 'In progress task', + status: 'in_progress', + createdAt: '', + updatedAt: '', + }, + { id: '3', content: 'Done task', status: 'done', createdAt: '', updatedAt: '' }, + ]; + + const formatted = formatTodoList(todos); + + expect(formatted).toContain('#1'); + expect(formatted).toContain('Pending task'); + expect(formatted).toContain('#2'); + expect(formatted).toContain('In progress task'); + expect(formatted).toContain('#3'); + expect(formatted).toContain('Done task'); + expect(formatted).toContain('1/3 done'); + expect(formatted).toContain('1 in progress'); + expect(formatted).toContain('1 pending'); + }); + + it('includes progress summary', () => { + const todos: Todo[] = [ + { id: '1', content: 'Task 1', status: 'done', createdAt: '', updatedAt: '' }, + { id: '2', content: 'Task 2', status: 'done', createdAt: '', updatedAt: '' }, + ]; + + const formatted = formatTodoList(todos); + + expect(formatted).toContain('2/2 done'); + }); + }); +}); diff --git a/tests/unit/github/client.test.ts b/tests/unit/github/client.test.ts new file mode 100644 index 00000000..7eee9809 --- /dev/null +++ b/tests/unit/github/client.test.ts @@ -0,0 +1,594 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock Octokit before importing client +const mockPulls = { + get: vi.fn(), + listReviewComments: vi.fn(), + createReplyForReviewComment: vi.fn(), + listReviews: vi.fn(), + listFiles: vi.fn(), + createReview: vi.fn(), + create: vi.fn(), +}; + +const mockIssues = { + createComment: vi.fn(), + updateComment: vi.fn(), + listComments: vi.fn(), +}; + +const mockChecks = { + listForRef: vi.fn(), +}; + +const mockRepos = { + getBranch: vi.fn(), +}; + +const mockUsers = { + getAuthenticated: vi.fn(), +}; + +vi.mock('@octokit/rest', () => ({ + Octokit: vi.fn().mockImplementation(() => ({ + pulls: mockPulls, + issues: mockIssues, + checks: mockChecks, + repos: mockRepos, + users: mockUsers, + })), +})); + +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +import { + getAuthenticatedUser, + githubClient, + resetGitHubClient, +} from '../../../src/github/client.js'; + +describe('githubClient', () => { + const originalEnv = process.env; + + beforeEach(() => { + vi.clearAllMocks(); + resetGitHubClient(); + process.env = { ...originalEnv, GITHUB_TOKEN: 'test-token' }; + }); + + afterEach(() => { + process.env = originalEnv; + resetGitHubClient(); + }); + + describe('getPR', () => { + it('returns PR details', async () => { + mockPulls.get.mockResolvedValue({ + data: { + number: 42, + title: 'Test PR', + body: 'PR body', + state: 'open', + html_url: 'https://github.com/owner/repo/pull/42', + head: { ref: 'feature/test', sha: 'sha123' }, + base: { ref: 'main' }, + merged: false, + }, + }); + + const result = await githubClient.getPR('owner', 'repo', 42); + + expect(result).toEqual({ + number: 42, + title: 'Test PR', + body: 'PR body', + state: 'open', + htmlUrl: 'https://github.com/owner/repo/pull/42', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + expect(mockPulls.get).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + pull_number: 42, + }); + }); + + it('handles null merged field', async () => { + mockPulls.get.mockResolvedValue({ + data: { + number: 42, + title: 'PR', + body: null, + state: 'open', + html_url: 'url', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + merged: null, + }, + }); + + const result = await githubClient.getPR('owner', 'repo', 42); + + expect(result.merged).toBe(false); + }); + }); + + describe('getPRReviewComments', () => { + it('returns mapped review comments', async () => { + mockPulls.listReviewComments.mockResolvedValue({ + data: [ + { + id: 1, + body: 'Comment 1', + path: 'src/index.ts', + line: 10, + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + created_at: '2024-01-01', + in_reply_to_id: undefined, + }, + ], + }); + + const result = await githubClient.getPRReviewComments('owner', 'repo', 42); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + id: 1, + body: 'Comment 1', + path: 'src/index.ts', + line: 10, + htmlUrl: 'https://github.com/...', + user: { login: 'reviewer' }, + createdAt: '2024-01-01', + inReplyToId: undefined, + }); + }); + + it('handles null line', async () => { + mockPulls.listReviewComments.mockResolvedValue({ + data: [ + { + id: 1, + body: 'Comment', + path: 'file.ts', + line: null, + html_url: 'url', + user: { login: 'user' }, + created_at: '2024-01-01', + in_reply_to_id: 5, + }, + ], + }); + + const result = await githubClient.getPRReviewComments('owner', 'repo', 42); + + expect(result[0].line).toBeNull(); + expect(result[0].inReplyToId).toBe(5); + }); + + it('handles missing user login', async () => { + mockPulls.listReviewComments.mockResolvedValue({ + data: [ + { + id: 1, + body: 'Comment', + path: 'file.ts', + line: null, + html_url: 'url', + user: null, + created_at: '2024-01-01', + }, + ], + }); + + const result = await githubClient.getPRReviewComments('owner', 'repo', 42); + + expect(result[0].user.login).toBe('unknown'); + }); + }); + + describe('replyToReviewComment', () => { + it('creates reply and returns mapped result', async () => { + mockPulls.createReplyForReviewComment.mockResolvedValue({ + data: { + id: 99, + body: 'Reply body', + path: 'src/index.ts', + line: 5, + html_url: 'https://github.com/...', + user: { login: 'bot' }, + created_at: '2024-01-01', + in_reply_to_id: 1, + }, + }); + + const result = await githubClient.replyToReviewComment('owner', 'repo', 42, 1, 'Reply body'); + + expect(result.id).toBe(99); + expect(result.inReplyToId).toBe(1); + expect(mockPulls.createReplyForReviewComment).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + pull_number: 42, + comment_id: 1, + body: 'Reply body', + }); + }); + }); + + describe('createPRComment', () => { + it('creates issue comment and returns id and url', async () => { + mockIssues.createComment.mockResolvedValue({ + data: { + id: 200, + html_url: 'https://github.com/owner/repo/pull/42#issuecomment-200', + }, + }); + + const result = await githubClient.createPRComment('owner', 'repo', 42, 'Hello'); + + expect(result).toEqual({ + id: 200, + htmlUrl: 'https://github.com/owner/repo/pull/42#issuecomment-200', + }); + expect(mockIssues.createComment).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + issue_number: 42, + body: 'Hello', + }); + }); + }); + + describe('updatePRComment', () => { + it('updates comment and returns result', async () => { + mockIssues.updateComment.mockResolvedValue({ + data: { + id: 200, + html_url: 'https://github.com/...', + }, + }); + + const result = await githubClient.updatePRComment('owner', 'repo', 200, 'Updated'); + + expect(result.id).toBe(200); + expect(mockIssues.updateComment).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + comment_id: 200, + body: 'Updated', + }); + }); + }); + + describe('getPRReviews', () => { + it('returns mapped reviews', async () => { + mockPulls.listReviews.mockResolvedValue({ + data: [ + { + id: 1, + state: 'APPROVED', + body: 'LGTM', + user: { login: 'reviewer' }, + submitted_at: '2024-01-01', + }, + ], + }); + + const result = await githubClient.getPRReviews('owner', 'repo', 42); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + id: 1, + state: 'approved', + body: 'LGTM', + user: { login: 'reviewer' }, + submittedAt: '2024-01-01', + }); + }); + + it('handles null body and user', async () => { + mockPulls.listReviews.mockResolvedValue({ + data: [ + { + id: 1, + state: 'COMMENTED', + body: '', + user: null, + submitted_at: null, + }, + ], + }); + + const result = await githubClient.getPRReviews('owner', 'repo', 42); + + expect(result[0].body).toBeNull(); + expect(result[0].user.login).toBe('unknown'); + expect(result[0].submittedAt).toBe(''); + }); + }); + + describe('getPRIssueComments', () => { + it('returns mapped issue comments', async () => { + mockIssues.listComments.mockResolvedValue({ + data: [ + { + id: 100, + body: 'Comment body', + user: { login: 'commenter' }, + html_url: 'https://github.com/...', + created_at: '2024-01-01', + }, + ], + }); + + const result = await githubClient.getPRIssueComments('owner', 'repo', 42); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + id: 100, + body: 'Comment body', + user: { login: 'commenter' }, + htmlUrl: 'https://github.com/...', + createdAt: '2024-01-01', + }); + }); + + it('handles null body and user', async () => { + mockIssues.listComments.mockResolvedValue({ + data: [ + { + id: 100, + body: null, + user: null, + html_url: 'url', + created_at: '2024-01-01', + }, + ], + }); + + const result = await githubClient.getPRIssueComments('owner', 'repo', 42); + + expect(result[0].body).toBe(''); + expect(result[0].user.login).toBe('unknown'); + }); + }); + + describe('getCheckSuiteStatus', () => { + it('returns status with all passing', async () => { + mockChecks.listForRef.mockResolvedValue({ + data: { + total_count: 2, + check_runs: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'success' }, + ], + }, + }); + + const result = await githubClient.getCheckSuiteStatus('owner', 'repo', 'sha123'); + + expect(result.allPassing).toBe(true); + expect(result.totalCount).toBe(2); + expect(result.checkRuns).toHaveLength(2); + }); + + it('returns allPassing false when some checks fail', async () => { + mockChecks.listForRef.mockResolvedValue({ + data: { + total_count: 2, + check_runs: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'failure' }, + ], + }, + }); + + const result = await githubClient.getCheckSuiteStatus('owner', 'repo', 'sha123'); + + expect(result.allPassing).toBe(false); + }); + + it('treats skipped and neutral as passing', async () => { + mockChecks.listForRef.mockResolvedValue({ + data: { + total_count: 2, + check_runs: [ + { name: 'lint', status: 'completed', conclusion: 'skipped' }, + { name: 'test', status: 'completed', conclusion: 'neutral' }, + ], + }, + }); + + const result = await githubClient.getCheckSuiteStatus('owner', 'repo', 'sha123'); + + expect(result.allPassing).toBe(true); + }); + + it('returns allPassing false when checks are still in_progress', async () => { + mockChecks.listForRef.mockResolvedValue({ + data: { + total_count: 1, + check_runs: [{ name: 'test', status: 'in_progress', conclusion: null }], + }, + }); + + const result = await githubClient.getCheckSuiteStatus('owner', 'repo', 'sha123'); + + expect(result.allPassing).toBe(false); + }); + }); + + describe('getPRDiff', () => { + it('returns mapped diff files', async () => { + mockPulls.listFiles.mockResolvedValue({ + data: [ + { + filename: 'src/index.ts', + status: 'modified', + additions: 10, + deletions: 5, + changes: 15, + patch: '@@ -1,5 +1,10 @@', + }, + ], + }); + + const result = await githubClient.getPRDiff('owner', 'repo', 42); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + filename: 'src/index.ts', + status: 'modified', + additions: 10, + deletions: 5, + changes: 15, + patch: '@@ -1,5 +1,10 @@', + }); + }); + }); + + describe('createPRReview', () => { + it('creates review and returns result', async () => { + mockPulls.createReview.mockResolvedValue({ + data: { + id: 500, + html_url: 'https://github.com/...', + }, + }); + + const result = await githubClient.createPRReview('owner', 'repo', 42, 'APPROVE', 'LGTM'); + + expect(result).toEqual({ + id: 500, + htmlUrl: 'https://github.com/...', + }); + expect(mockPulls.createReview).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + pull_number: 42, + event: 'APPROVE', + body: 'LGTM', + comments: undefined, + }); + }); + + it('passes file comments when provided', async () => { + mockPulls.createReview.mockResolvedValue({ + data: { id: 501, html_url: 'url' }, + }); + + await githubClient.createPRReview('owner', 'repo', 42, 'REQUEST_CHANGES', 'Please fix', [ + { path: 'src/index.ts', line: 10, body: 'Fix this line' }, + ]); + + expect(mockPulls.createReview).toHaveBeenCalledWith( + expect.objectContaining({ + comments: [{ path: 'src/index.ts', line: 10, body: 'Fix this line' }], + }), + ); + }); + }); + + describe('createPR', () => { + it('creates PR and returns result', async () => { + mockPulls.create.mockResolvedValue({ + data: { + number: 100, + html_url: 'https://github.com/owner/repo/pull/100', + title: 'New Feature', + }, + }); + + const result = await githubClient.createPR('owner', 'repo', { + title: 'New Feature', + body: 'Description', + head: 'feature/new', + base: 'main', + }); + + expect(result).toEqual({ + number: 100, + htmlUrl: 'https://github.com/owner/repo/pull/100', + title: 'New Feature', + }); + }); + + it('defaults draft to false', async () => { + mockPulls.create.mockResolvedValue({ + data: { number: 101, html_url: 'url', title: 'PR' }, + }); + + await githubClient.createPR('owner', 'repo', { + title: 'PR', + body: 'body', + head: 'feat', + base: 'main', + }); + + expect(mockPulls.create).toHaveBeenCalledWith(expect.objectContaining({ draft: false })); + }); + }); + + describe('branchExists', () => { + it('returns true when branch exists', async () => { + mockRepos.getBranch.mockResolvedValue({ data: {} }); + + const result = await 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 githubClient.branchExists('owner', 'repo', 'nonexistent'); + + expect(result).toBe(false); + }); + + it('throws on other errors', async () => { + mockRepos.getBranch.mockRejectedValue(new Error('Server Error')); + + await expect(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 getAuthenticatedUser(); + + expect(result).toBe('cascade-bot'); + }); + }); + + describe('GITHUB_TOKEN required', () => { + it('throws when GITHUB_TOKEN is not set', async () => { + resetGitHubClient(); + process.env.GITHUB_TOKEN = undefined; + + await expect(githubClient.getPR('owner', 'repo', 1)).rejects.toThrow( + 'GITHUB_TOKEN must be set', + ); + }); + }); +}); diff --git a/tests/unit/triggers/attachment-added.test.ts b/tests/unit/triggers/attachment-added.test.ts new file mode 100644 index 00000000..4b521837 --- /dev/null +++ b/tests/unit/triggers/attachment-added.test.ts @@ -0,0 +1,367 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { AttachmentAddedTrigger } from '../../../src/triggers/trello/attachment-added.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/trello/client.js', () => ({ + trelloClient: { + getMe: vi.fn(), + getCard: vi.fn(), + }, +})); + +// Mock global fetch +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +// Mock AdmZip +vi.mock('adm-zip', () => ({ + default: vi.fn().mockImplementation(() => ({ + extractAllTo: vi.fn(), + getEntries: vi.fn(() => [{ entryName: 'session.log' }]), + })), +})); + +import { trelloClient } from '../../../src/trello/client.js'; + +describe('AttachmentAddedTrigger', () => { + const trigger = new AttachmentAddedTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + debug: 'debug-list-id', + }, + labels: {}, + }, + }; + + const makeTrelloPayload = (overrides: Record = {}) => ({ + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Test Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'implementation-2026-01-02T12-34-56-789Z.zip', + url: 'https://trello.com/attachments/att123.zip', + mimeType: 'application/zip', + }, + board: { id: 'board123', name: 'Test Board', shortLink: 'xyz' }, + }, + }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches attachment added with agent log zip pattern', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: makeTrelloPayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match github source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-attachment actions', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'updateCard', + date: '2024-01-01', + data: { card: { id: 'card123', name: 'Card', idShort: 1, shortLink: 'abc' } }, + }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-zip attachments', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'image.png', + url: 'https://trello.com/attachments/image.png', + mimeType: 'image/png', + }, + }, + }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match zip files that do not match agent log pattern', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'random-file.zip', + url: 'https://trello.com/attachments/random.zip', + mimeType: 'application/zip', + }, + }, + }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match debug agent logs (prevent infinite loop)', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'debug-2026-01-02T12-34-56-789Z.zip', + url: 'https://trello.com/attachments/debug.zip', + mimeType: 'application/zip', + }, + }, + }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match when debug list is not configured', () => { + const projectWithoutDebug = { + ...mockProject, + trello: { + ...mockProject.trello, + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + // no debug list + }, + }, + }; + + const ctx: TriggerContext = { + project: projectWithoutDebug, + source: 'trello', + payload: makeTrelloPayload(), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('matches timeout log filenames', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'briefing-timeout-2026-01-02T12-34-56-789Z.zip', + url: 'https://trello.com/attachments/briefing-timeout.zip', + mimeType: 'application/zip', + }, + }, + }, + }, + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + }); + + describe('handle', () => { + it('returns debug agent result for valid attachment from authenticated user', async () => { + vi.mocked(trelloClient.getMe).mockResolvedValue({ + id: 'member123', + fullName: 'Cascade Bot', + username: 'cascadebot', + }); + vi.mocked(trelloClient.getCard).mockResolvedValue({ + id: 'card123', + name: 'Test Card', + shortUrl: 'https://trello.com/c/abc', + desc: '', + idList: 'todo-list-id', + labels: [], + }); + + mockFetch.mockResolvedValue({ + ok: true, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(100)), + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: makeTrelloPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('debug'); + expect(result?.agentInput.originalCardId).toBe('card123'); + expect(result?.agentInput.originalCardName).toBe('Test Card'); + expect(result?.agentInput.detectedAgentType).toBe('implementation'); + expect(result?.cardId).toBe('card123'); + }); + + it('returns null when attachment uploaded by different user', async () => { + vi.mocked(trelloClient.getMe).mockResolvedValue({ + id: 'member123', + fullName: 'Cascade Bot', + username: 'cascadebot', + }); + + // Use a payload with a different uploader + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'other-member-456', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + card: { id: 'card123', name: 'Test Card', idShort: 1, shortLink: 'abc' }, + attachment: { + id: 'att123', + name: 'implementation-2026-01-02T12-34-56-789Z.zip', + url: 'https://trello.com/attachments/att123.zip', + mimeType: 'application/zip', + }, + board: { id: 'board123', name: 'Test Board', shortLink: 'xyz' }, + }, + }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('throws when download fails', async () => { + vi.mocked(trelloClient.getMe).mockResolvedValue({ + id: 'member123', + fullName: 'Cascade Bot', + username: 'cascadebot', + }); + + mockFetch.mockResolvedValue({ + ok: false, + status: 500, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: makeTrelloPayload(), + }; + + await expect(trigger.handle(ctx)).rejects.toThrow('Failed to download attachment'); + }); + + it('throws when card or attachment data is missing', async () => { + vi.mocked(trelloClient.getMe).mockResolvedValue({ + id: 'member123', + fullName: 'Cascade Bot', + username: 'cascadebot', + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: { + model: { id: 'board123', name: 'Test Board' }, + action: { + id: 'action123', + idMemberCreator: 'member123', + type: 'addAttachmentToCard', + date: '2024-01-01', + data: { + // missing card and attachment + }, + }, + }, + }; + + await expect(trigger.handle(ctx)).rejects.toThrow('Missing card or attachment data'); + }); + }); +}); diff --git a/tests/unit/triggers/check-suite-failure.test.ts b/tests/unit/triggers/check-suite-failure.test.ts new file mode 100644 index 00000000..3cc025ae --- /dev/null +++ b/tests/unit/triggers/check-suite-failure.test.ts @@ -0,0 +1,339 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + CheckSuiteFailureTrigger, + resetFixAttempts, +} from '../../../src/triggers/github/check-suite-failure.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + githubClient: { + getPR: vi.fn(), + getCheckSuiteStatus: vi.fn(), + createPRComment: vi.fn(), + }, +})); + +import { githubClient } from '../../../src/github/client.js'; + +describe('CheckSuiteFailureTrigger', () => { + const trigger = new CheckSuiteFailureTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + const makeFailurePayload = (overrides: Record = {}) => ({ + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'failure', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feature/test', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + resetFixAttempts(42); + }); + + describe('matches', () => { + it('matches completed check suite with failure conclusion and PRs', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-completed action', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload({ action: 'requested' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match success conclusion', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match when no PRs associated', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'failure', + head_sha: 'sha123', + pull_requests: [], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns respond-to-ci result when PR has Trello URL and checks failed', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'failure' }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toEqual({ + agentType: 'respond-to-ci', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + headSha: 'sha123', + triggerType: 'check-failure', + cardId: 'abc123', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null when PR has no Trello URL', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No Trello link', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null when not all checks are complete', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'in_progress', conclusion: null }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null when all checks actually passed (no failures)', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'success' }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('posts warning and returns null after MAX_ATTEMPTS (3)', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'failure' }], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + // First 3 attempts should succeed + await trigger.handle(ctx); + await trigger.handle(ctx); + await trigger.handle(ctx); + + // 4th attempt should be blocked + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.createPRComment).toHaveBeenCalledWith( + 'owner', + 'repo', + 42, + expect.stringContaining('Unable to automatically fix'), + ); + }); + + it('resetFixAttempts clears attempts for a PR', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'failure' }], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeFailurePayload(), + }; + + // Use up 3 attempts + await trigger.handle(ctx); + await trigger.handle(ctx); + await trigger.handle(ctx); + + // Reset + resetFixAttempts(42); + + // Should work again + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-ci'); + }); + }); +}); diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts new file mode 100644 index 00000000..2058554a --- /dev/null +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -0,0 +1,230 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { CheckSuiteSuccessTrigger } from '../../../src/triggers/github/check-suite-success.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + githubClient: { + getPR: vi.fn(), + getCheckSuiteStatus: vi.fn(), + }, +})); + +import { githubClient } from '../../../src/github/client.js'; + +describe('CheckSuiteSuccessTrigger', () => { + const trigger = new CheckSuiteSuccessTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + const makeCheckSuitePayload = (overrides: Record = {}) => ({ + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feature/test', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches completed check suite with success conclusion and PRs', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-completed action', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload({ + action: 'requested', + }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match failure conclusion', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'failure', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match when no PRs associated', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns review result when PR has Trello URL and all checks pass', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'closed', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'success' }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(githubClient.getPR).toHaveBeenCalledWith('owner', 'repo', 42); + expect(githubClient.getCheckSuiteStatus).toHaveBeenCalledWith('owner', 'repo', 'sha123'); + expect(result).toEqual({ + agentType: 'review', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + headSha: 'sha123', + triggerType: 'ci-success', + cardId: 'abc123', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null when PR has no Trello URL', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No Trello link here', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); + }); + + it('returns null when not all checks are passing', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'failure' }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/tests/unit/triggers/github-types.test.ts b/tests/unit/triggers/github-types.test.ts new file mode 100644 index 00000000..d6fe581d --- /dev/null +++ b/tests/unit/triggers/github-types.test.ts @@ -0,0 +1,326 @@ +import { describe, expect, it } from 'vitest'; +import { + isGitHubCheckSuitePayload, + isGitHubIssueCommentPayload, + isGitHubPRReviewCommentPayload, + isGitHubPullRequestPayload, + isGitHubPullRequestReviewPayload, +} from '../../../src/triggers/github/types.js'; + +describe('GitHub Type Guards', () => { + describe('isGitHubPRReviewCommentPayload', () => { + const validPayload = { + action: 'created', + comment: { + id: 1, + body: 'test', + path: 'src/index.ts', + line: 10, + user: { login: 'user1' }, + html_url: 'https://github.com/...', + }, + pull_request: { + number: 1, + title: 'PR', + html_url: 'https://github.com/...', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'user1' }, + }; + + it('returns true for valid payload', () => { + expect(isGitHubPRReviewCommentPayload(validPayload)).toBe(true); + }); + + it('returns false for null', () => { + expect(isGitHubPRReviewCommentPayload(null)).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isGitHubPRReviewCommentPayload(undefined)).toBe(false); + }); + + it('returns false for primitive', () => { + expect(isGitHubPRReviewCommentPayload('string')).toBe(false); + expect(isGitHubPRReviewCommentPayload(42)).toBe(false); + }); + + it('returns false when action is missing', () => { + const { action, ...rest } = validPayload; + expect(isGitHubPRReviewCommentPayload(rest)).toBe(false); + }); + + it('returns false when comment is missing', () => { + const { comment, ...rest } = validPayload; + expect(isGitHubPRReviewCommentPayload(rest)).toBe(false); + }); + + it('returns false when comment is null', () => { + expect(isGitHubPRReviewCommentPayload({ ...validPayload, comment: null })).toBe(false); + }); + + it('returns false when pull_request is missing', () => { + const { pull_request, ...rest } = validPayload; + expect(isGitHubPRReviewCommentPayload(rest)).toBe(false); + }); + + it('returns false when repository is missing', () => { + const { repository, ...rest } = validPayload; + expect(isGitHubPRReviewCommentPayload(rest)).toBe(false); + }); + + it('returns false when repository is null', () => { + expect(isGitHubPRReviewCommentPayload({ ...validPayload, repository: null })).toBe(false); + }); + }); + + describe('isGitHubCheckSuitePayload', () => { + const validPayload = { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'abc', + pull_requests: [], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'user1' }, + }; + + it('returns true for valid payload', () => { + expect(isGitHubCheckSuitePayload(validPayload)).toBe(true); + }); + + it('returns false for null', () => { + expect(isGitHubCheckSuitePayload(null)).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isGitHubCheckSuitePayload(undefined)).toBe(false); + }); + + it('returns false for primitive', () => { + expect(isGitHubCheckSuitePayload(123)).toBe(false); + }); + + it('returns false when action is missing', () => { + const { action, ...rest } = validPayload; + expect(isGitHubCheckSuitePayload(rest)).toBe(false); + }); + + it('returns false when check_suite is missing', () => { + const { check_suite, ...rest } = validPayload; + expect(isGitHubCheckSuitePayload(rest)).toBe(false); + }); + + it('returns false when check_suite is null', () => { + expect(isGitHubCheckSuitePayload({ ...validPayload, check_suite: null })).toBe(false); + }); + + it('returns false when repository is missing', () => { + const { repository, ...rest } = validPayload; + expect(isGitHubCheckSuitePayload(rest)).toBe(false); + }); + }); + + describe('isGitHubPullRequestReviewPayload', () => { + const validPayload = { + action: 'submitted', + review: { + id: 1, + state: 'approved', + body: 'LGTM', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + pull_request: { + number: 1, + title: 'PR', + body: 'desc', + html_url: 'https://github.com/...', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + }; + + it('returns true for valid payload', () => { + expect(isGitHubPullRequestReviewPayload(validPayload)).toBe(true); + }); + + it('returns false for null', () => { + expect(isGitHubPullRequestReviewPayload(null)).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isGitHubPullRequestReviewPayload(undefined)).toBe(false); + }); + + it('returns false for primitive', () => { + expect(isGitHubPullRequestReviewPayload('hello')).toBe(false); + }); + + it('returns false when review is missing', () => { + const { review, ...rest } = validPayload; + expect(isGitHubPullRequestReviewPayload(rest)).toBe(false); + }); + + it('returns false when review is null', () => { + expect(isGitHubPullRequestReviewPayload({ ...validPayload, review: null })).toBe(false); + }); + + it('returns false when pull_request is missing', () => { + const { pull_request, ...rest } = validPayload; + expect(isGitHubPullRequestReviewPayload(rest)).toBe(false); + }); + + it('returns false when pull_request is null', () => { + expect(isGitHubPullRequestReviewPayload({ ...validPayload, pull_request: null })).toBe(false); + }); + + it('returns false when repository is missing', () => { + const { repository, ...rest } = validPayload; + expect(isGitHubPullRequestReviewPayload(rest)).toBe(false); + }); + }); + + describe('isGitHubPullRequestPayload', () => { + const validPayload = { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'PR', + body: 'desc', + html_url: 'https://github.com/...', + state: 'open', + draft: false, + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }; + + it('returns true for valid payload', () => { + expect(isGitHubPullRequestPayload(validPayload)).toBe(true); + }); + + it('returns false for null', () => { + expect(isGitHubPullRequestPayload(null)).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isGitHubPullRequestPayload(undefined)).toBe(false); + }); + + it('returns false for primitive', () => { + expect(isGitHubPullRequestPayload(true)).toBe(false); + }); + + it('returns false when action is missing', () => { + const { action, ...rest } = validPayload; + expect(isGitHubPullRequestPayload(rest)).toBe(false); + }); + + it('returns false when number is missing', () => { + const { number, ...rest } = validPayload; + expect(isGitHubPullRequestPayload(rest)).toBe(false); + }); + + it('returns false when number is not a number', () => { + expect(isGitHubPullRequestPayload({ ...validPayload, number: 'not-a-number' })).toBe(false); + }); + + it('returns false when pull_request is missing', () => { + const { pull_request, ...rest } = validPayload; + expect(isGitHubPullRequestPayload(rest)).toBe(false); + }); + + it('returns false when pull_request is null', () => { + expect(isGitHubPullRequestPayload({ ...validPayload, pull_request: null })).toBe(false); + }); + + it('returns false when repository is missing', () => { + const { repository, ...rest } = validPayload; + expect(isGitHubPullRequestPayload(rest)).toBe(false); + }); + + it('returns false when repository is null', () => { + expect(isGitHubPullRequestPayload({ ...validPayload, repository: null })).toBe(false); + }); + }); + + describe('isGitHubIssueCommentPayload', () => { + const validPayload = { + action: 'created', + issue: { + number: 1, + title: 'Issue', + html_url: 'https://github.com/...', + pull_request: { url: 'https://api.github.com/...' }, + }, + comment: { + id: 1, + body: 'test comment', + html_url: 'https://github.com/...', + user: { login: 'commenter' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'commenter' }, + }; + + it('returns true for valid payload', () => { + expect(isGitHubIssueCommentPayload(validPayload)).toBe(true); + }); + + it('returns false for null', () => { + expect(isGitHubIssueCommentPayload(null)).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isGitHubIssueCommentPayload(undefined)).toBe(false); + }); + + it('returns false for primitive', () => { + expect(isGitHubIssueCommentPayload(0)).toBe(false); + }); + + it('returns false when action is missing', () => { + const { action, ...rest } = validPayload; + expect(isGitHubIssueCommentPayload(rest)).toBe(false); + }); + + it('returns false when issue is missing', () => { + const { issue, ...rest } = validPayload; + expect(isGitHubIssueCommentPayload(rest)).toBe(false); + }); + + it('returns false when issue is null', () => { + expect(isGitHubIssueCommentPayload({ ...validPayload, issue: null })).toBe(false); + }); + + it('returns false when comment is missing', () => { + const { comment, ...rest } = validPayload; + expect(isGitHubIssueCommentPayload(rest)).toBe(false); + }); + + it('returns false when comment is null', () => { + expect(isGitHubIssueCommentPayload({ ...validPayload, comment: null })).toBe(false); + }); + + it('returns false when repository is missing', () => { + const { repository, ...rest } = validPayload; + expect(isGitHubIssueCommentPayload(rest)).toBe(false); + }); + + it('returns false when repository is null', () => { + expect(isGitHubIssueCommentPayload({ ...validPayload, repository: null })).toBe(false); + }); + }); +}); diff --git a/tests/unit/triggers/issue-comment.test.ts b/tests/unit/triggers/issue-comment.test.ts new file mode 100644 index 00000000..91f9f528 --- /dev/null +++ b/tests/unit/triggers/issue-comment.test.ts @@ -0,0 +1,251 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { IssueCommentTrigger } from '../../../src/triggers/github/issue-comment.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + getAuthenticatedUser: vi.fn(), + githubClient: { + getPR: vi.fn(), + }, +})); + +import { getAuthenticatedUser, githubClient } from '../../../src/github/client.js'; + +describe('IssueCommentTrigger', () => { + const trigger = new IssueCommentTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + const makeIssueCommentPayload = (overrides: Record = {}) => ({ + action: 'created', + issue: { + number: 42, + title: 'Test PR', + html_url: 'https://github.com/owner/repo/pull/42', + pull_request: { url: 'https://api.github.com/repos/owner/repo/pulls/42' }, + }, + comment: { + id: 200, + body: 'Please review this section', + html_url: 'https://github.com/owner/repo/pull/42#issuecomment-200', + user: { login: 'reviewer' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches created comment on a PR', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match edited comments', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload({ action: 'edited' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match deleted comments', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload({ action: 'deleted' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match comments on regular issues (not PRs)', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload({ + issue: { + number: 42, + title: 'Bug report', + html_url: 'https://github.com/owner/repo/issues/42', + // no pull_request field + }, + }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-issue-comment payloads', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { action: 'created' }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns respond-to-review result for valid comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(githubClient.getPR).toHaveBeenCalledWith('owner', 'repo', 42); + expect(result).toEqual({ + agentType: 'respond-to-review', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + triggerCommentId: 200, + triggerCommentBody: 'Please review this section', + triggerCommentPath: '', + triggerCommentUrl: 'https://github.com/owner/repo/pull/42#issuecomment-200', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null for self-authored comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getPR).not.toHaveBeenCalled(); + }); + + it('returns null for bot user comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload({ + comment: { + id: 200, + body: 'Automated comment', + html_url: 'https://github.com/...', + user: { login: 'reviewer[bot]' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('proceeds when getAuthenticatedUser fails', async () => { + vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Auth error')); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-review'); + }); + + it('returns null when PR has no Trello URL', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No Trello link here', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeIssueCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/tests/unit/triggers/pr-opened.test.ts b/tests/unit/triggers/pr-opened.test.ts new file mode 100644 index 00000000..86b54988 --- /dev/null +++ b/tests/unit/triggers/pr-opened.test.ts @@ -0,0 +1,231 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { PROpenedTrigger } from '../../../src/triggers/github/pr-opened.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +describe('PROpenedTrigger', () => { + const trigger = new PROpenedTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches when action is opened and not draft', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc123' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match when source is not github', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match when action is not opened', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'closed', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'closed', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match draft PRs', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Draft PR', + body: 'desc', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: true, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-PR payloads', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + // missing number and pull_request + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns result when PR body has Trello URL', async () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'Implements https://trello.com/c/abc123/card-name', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toEqual({ + agentType: 'respond-to-review', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + triggerCommentId: 0, + triggerCommentBody: 'New PR: Test PR\n\nImplements https://trello.com/c/abc123/card-name', + triggerCommentPath: '', + triggerCommentUrl: 'https://github.com/owner/repo/pull/42', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null when PR body has no Trello URL', async () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: 'Just a regular PR', + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('handles null PR body', async () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'Test PR', + body: null, + html_url: 'https://github.com/owner/repo/pull/42', + state: 'open', + draft: false, + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/tests/unit/triggers/pr-ready-to-merge.test.ts b/tests/unit/triggers/pr-ready-to-merge.test.ts new file mode 100644 index 00000000..93b4a146 --- /dev/null +++ b/tests/unit/triggers/pr-ready-to-merge.test.ts @@ -0,0 +1,563 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { PRReadyToMergeTrigger } from '../../../src/triggers/github/pr-ready-to-merge.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + githubClient: { + getPR: vi.fn(), + getCheckSuiteStatus: vi.fn(), + getPRReviews: vi.fn(), + }, +})); + +vi.mock('../../../src/trello/client.js', () => ({ + trelloClient: { + moveCardToList: vi.fn(), + addComment: vi.fn(), + }, +})); + +import { githubClient } from '../../../src/github/client.js'; +import { trelloClient } from '../../../src/trello/client.js'; + +describe('PRReadyToMergeTrigger', () => { + const trigger = new PRReadyToMergeTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + done: 'done-list-id', + }, + labels: {}, + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches check_suite completed with success and PRs', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('matches review submitted with approved state', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'submitted', + review: { + id: 100, + state: 'approved', + body: 'LGTM', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + pull_request: { + number: 42, + title: 'PR', + body: 'desc', + html_url: 'https://github.com/...', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match check_suite with failure', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'failure', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match check_suite with no PRs', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match review with changes_requested', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'submitted', + review: { + id: 100, + state: 'changes_requested', + body: 'Fix this', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + pull_request: { + number: 42, + title: 'PR', + body: 'desc', + html_url: 'https://github.com/...', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match unrelated github events', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'opened', + number: 42, + pull_request: { + number: 42, + title: 'PR', + body: 'desc', + html_url: 'https://github.com/...', + state: 'open', + draft: false, + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + user: { login: 'author' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'author' }, + }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('moves card to DONE when check_suite triggers and all conditions met', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'success' }, + ], + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([ + { + id: 1, + state: 'approved', + body: 'LGTM', + user: { login: 'reviewer' }, + submitted_at: '2024-01-01', + }, + ]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feature/test', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(trelloClient.moveCardToList).toHaveBeenCalledWith('abc123', 'done-list-id'); + expect(trelloClient.addComment).toHaveBeenCalledWith( + 'abc123', + 'PR #42 approved and all checks passing - moved to DONE', + ); + expect(result).toEqual({ + agentType: '', + agentInput: {}, + cardId: 'abc123', + prNumber: 42, + }); + }); + + it('moves card to DONE when review approved triggers and all conditions met', async () => { + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'success' }, + ], + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([ + { + id: 1, + state: 'approved', + body: 'LGTM', + user: { login: 'reviewer' }, + submitted_at: '2024-01-01', + }, + ]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'submitted', + review: { + id: 100, + state: 'approved', + body: 'LGTM', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + pull_request: { + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + html_url: 'https://github.com/...', + head: { ref: 'feature/test', sha: 'sha123' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(trelloClient.moveCardToList).toHaveBeenCalledWith('abc123', 'done-list-id'); + expect(result?.cardId).toBe('abc123'); + }); + + it('returns null when PR has no Trello URL (check_suite path)', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'No Trello link', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + }); + + it('returns null when checks are not all passing', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: false, + totalCount: 2, + checkRuns: [ + { name: 'lint', status: 'completed', conclusion: 'success' }, + { name: 'test', status: 'completed', conclusion: 'failure' }, + ], + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + }); + + it('returns null when no approval exists', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([ + { + id: 1, + state: 'commented', + body: 'Looks ok', + user: { login: 'reviewer' }, + submitted_at: '2024-01-01', + }, + ]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null when there are outstanding change requests', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([ + { + id: 1, + state: 'approved', + body: 'LGTM', + user: { login: 'reviewer1' }, + submitted_at: '2024-01-01', + }, + { + id: 2, + state: 'changes_requested', + body: 'Fix this', + user: { login: 'reviewer2' }, + submitted_at: '2024-01-02', + }, + ]); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null when done list is not configured', async () => { + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + vi.mocked(githubClient.getCheckSuiteStatus).mockResolvedValue({ + allPassing: true, + totalCount: 1, + checkRuns: [{ name: 'test', status: 'completed', conclusion: 'success' }], + }); + vi.mocked(githubClient.getPRReviews).mockResolvedValue([ + { + id: 1, + state: 'approved', + body: 'LGTM', + user: { login: 'reviewer' }, + submitted_at: '2024-01-01', + }, + ]); + + const projectWithoutDone = { + ...mockProject, + trello: { + ...mockProject.trello, + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + // no done list + }, + }, + }; + + const ctx: TriggerContext = { + project: projectWithoutDone, + source: 'github', + payload: { + action: 'completed', + check_suite: { + id: 1, + status: 'completed', + conclusion: 'success', + head_sha: 'sha123', + pull_requests: [{ number: 42, head: { ref: 'feat', sha: 'sha123' } }], + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'github-actions' }, + }, + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(trelloClient.moveCardToList).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/tests/unit/triggers/pr-review-comment.test.ts b/tests/unit/triggers/pr-review-comment.test.ts new file mode 100644 index 00000000..d928e8e1 --- /dev/null +++ b/tests/unit/triggers/pr-review-comment.test.ts @@ -0,0 +1,239 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { PRReviewCommentTrigger } from '../../../src/triggers/github/pr-review-comment.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + getAuthenticatedUser: vi.fn(), + githubClient: { + getPR: vi.fn(), + }, +})); + +import { getAuthenticatedUser, githubClient } from '../../../src/github/client.js'; + +describe('PRReviewCommentTrigger', () => { + const trigger = new PRReviewCommentTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + const makeReviewCommentPayload = (overrides: Record = {}) => ({ + action: 'created', + comment: { + id: 300, + body: 'This should be refactored', + path: 'src/index.ts', + line: 42, + user: { login: 'reviewer' }, + html_url: 'https://github.com/owner/repo/pull/42#discussion_r300', + }, + pull_request: { + number: 42, + title: 'Test PR', + html_url: 'https://github.com/owner/repo/pull/42', + head: { ref: 'feature/test', sha: 'abc123' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches created review comment', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match edited comments', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload({ action: 'edited' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match deleted comments', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload({ action: 'deleted' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-review-comment payloads', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { action: 'created' }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns respond-to-review result with file path', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(githubClient.getPR).toHaveBeenCalledWith('owner', 'repo', 42); + expect(result).toEqual({ + agentType: 'respond-to-review', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + triggerCommentId: 300, + triggerCommentBody: 'This should be refactored', + triggerCommentPath: 'src/index.ts', + triggerCommentUrl: 'https://github.com/owner/repo/pull/42#discussion_r300', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null for self-authored comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getPR).not.toHaveBeenCalled(); + }); + + it('returns null for bot user comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload({ + comment: { + id: 300, + body: 'Bot comment', + path: 'src/index.ts', + line: 42, + user: { login: 'reviewer[bot]' }, + html_url: 'https://github.com/...', + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('proceeds when getAuthenticatedUser fails', async () => { + vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Token error')); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-review'); + }); + + it('returns null when PR has no Trello URL', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(githubClient.getPR).mockResolvedValue({ + number: 42, + title: 'Test PR', + body: 'Just a regular PR', + state: 'open', + headRef: 'feature/test', + headSha: 'sha123', + baseRef: 'main', + merged: false, + }); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewCommentPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + }); +}); diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts new file mode 100644 index 00000000..c32f5c20 --- /dev/null +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -0,0 +1,260 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { PRReviewSubmittedTrigger } from '../../../src/triggers/github/pr-review-submitted.js'; +import type { TriggerContext } from '../../../src/triggers/types.js'; + +vi.mock('../../../src/github/client.js', () => ({ + getAuthenticatedUser: vi.fn(), +})); + +import { getAuthenticatedUser } from '../../../src/github/client.js'; + +describe('PRReviewSubmittedTrigger', () => { + const trigger = new PRReviewSubmittedTrigger(); + + const mockProject = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { + boardId: 'board123', + lists: { + briefing: 'briefing-list-id', + planning: 'planning-list-id', + todo: 'todo-list-id', + }, + labels: {}, + }, + }; + + const makeReviewPayload = (overrides: Record = {}) => ({ + action: 'submitted', + review: { + id: 100, + state: 'changes_requested', + body: 'Please fix the bug', + html_url: 'https://github.com/owner/repo/pull/42#pullrequestreview-100', + user: { login: 'reviewer' }, + }, + pull_request: { + number: 42, + title: 'Test PR', + body: 'https://trello.com/c/abc123/card-name', + html_url: 'https://github.com/owner/repo/pull/42', + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + }, + repository: { full_name: 'owner/repo', html_url: 'https://github.com/owner/repo' }, + sender: { login: 'reviewer' }, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('matches', () => { + it('matches submitted review with changes_requested', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload(), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('matches submitted review with commented state', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ + review: { + id: 100, + state: 'commented', + body: 'Nice work', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + }), + }; + + expect(trigger.matches(ctx)).toBe(true); + }); + + it('does not match trello source', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'trello', + payload: {}, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-submitted action', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ action: 'edited' }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match approved reviews', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ + review: { + id: 100, + state: 'approved', + body: 'LGTM', + html_url: 'https://github.com/...', + user: { login: 'reviewer' }, + }, + }), + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + + it('does not match non-review payloads', () => { + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: { action: 'submitted' }, + }; + + expect(trigger.matches(ctx)).toBe(false); + }); + }); + + describe('handle', () => { + it('returns respond-to-review result for valid review', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toEqual({ + agentType: 'respond-to-review', + agentInput: { + prNumber: 42, + prBranch: 'feature/test', + repoFullName: 'owner/repo', + triggerCommentId: 100, + triggerCommentBody: 'Please fix the bug', + triggerCommentPath: '', + triggerCommentUrl: 'https://github.com/owner/repo/pull/42#pullrequestreview-100', + }, + prNumber: 42, + cardId: 'abc123', + }); + }); + + it('returns null for self-authored review', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('returns null for bot user review (appended [bot])', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('reviewer'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ + review: { + id: 100, + state: 'changes_requested', + body: 'Fix this', + html_url: 'https://github.com/...', + user: { login: 'reviewer[bot]' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('proceeds when getAuthenticatedUser fails', async () => { + vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Token error')); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).not.toBeNull(); + expect(result?.agentType).toBe('respond-to-review'); + }); + + it('returns null when PR has no Trello URL', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ + pull_request: { + number: 42, + title: 'Test PR', + body: 'No Trello link', + html_url: 'https://github.com/owner/repo/pull/42', + head: { ref: 'feature/test', sha: 'abc' }, + base: { ref: 'main' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + + it('uses review state as fallback when review body is null', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + + const ctx: TriggerContext = { + project: mockProject, + source: 'github', + payload: makeReviewPayload({ + review: { + id: 100, + state: 'changes_requested', + body: null, + html_url: 'https://github.com/owner/repo/pull/42#pullrequestreview-100', + user: { login: 'reviewer' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result?.agentInput.triggerCommentBody).toBe('Review: changes_requested'); + }); + }); +}); diff --git a/tests/unit/utils/lifecycle.test.ts b/tests/unit/utils/lifecycle.test.ts new file mode 100644 index 00000000..f3347c0f --- /dev/null +++ b/tests/unit/utils/lifecycle.test.ts @@ -0,0 +1,166 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { + info: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }, +})); + +import { + cancelFreshMachineTimer, + clearWatchdog, + clearWatchdogCleanup, + isCurrentlyProcessing, + scheduleShutdownAfterJob, + setProcessing, + setWatchdogCleanup, + startFreshMachineTimer, + startWatchdog, +} from '../../../src/utils/lifecycle.js'; + +describe('lifecycle', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + }); + + afterEach(() => { + // Clean up all timers + cancelFreshMachineTimer(); + clearWatchdog(); + clearWatchdogCleanup(); + setProcessing(false); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + describe('processing state', () => { + it('defaults to not processing', () => { + expect(isCurrentlyProcessing()).toBe(false); + }); + + it('can set processing to true', () => { + setProcessing(true); + expect(isCurrentlyProcessing()).toBe(true); + }); + + it('can set processing back to false', () => { + setProcessing(true); + setProcessing(false); + expect(isCurrentlyProcessing()).toBe(false); + }); + }); + + describe('fresh machine timer', () => { + it('exits after timeout when no work received', () => { + startFreshMachineTimer(5000); + + vi.advanceTimersByTime(5000); + + expect(process.exit).toHaveBeenCalledWith(0); + }); + + it('does not exit before timeout', () => { + startFreshMachineTimer(5000); + + vi.advanceTimersByTime(4999); + + expect(process.exit).not.toHaveBeenCalled(); + }); + + it('can be cancelled', () => { + startFreshMachineTimer(5000); + cancelFreshMachineTimer(); + + vi.advanceTimersByTime(10000); + + expect(process.exit).not.toHaveBeenCalled(); + }); + + it('replaces existing timer when started again', () => { + startFreshMachineTimer(5000); + startFreshMachineTimer(10000); + + vi.advanceTimersByTime(5000); + expect(process.exit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(5000); + expect(process.exit).toHaveBeenCalledWith(0); + }); + }); + + describe('watchdog', () => { + it('force exits after timeout', () => { + startWatchdog(30000); + + vi.advanceTimersByTime(30000); + + expect(process.exit).toHaveBeenCalledWith(1); + }); + + it('can be cleared', () => { + startWatchdog(30000); + clearWatchdog(); + + vi.advanceTimersByTime(60000); + + expect(process.exit).not.toHaveBeenCalled(); + }); + + it('clears previous watchdog when starting new one', () => { + startWatchdog(5000); + startWatchdog(10000); + + vi.advanceTimersByTime(5000); + expect(process.exit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(5000); + expect(process.exit).toHaveBeenCalledWith(1); + }); + }); + + describe('shutdown after job', () => { + it('exits after grace period', () => { + scheduleShutdownAfterJob(5000); + + vi.advanceTimersByTime(5000); + + expect(process.exit).toHaveBeenCalledWith(0); + }); + + it('clears watchdog when scheduling shutdown', () => { + startWatchdog(30000); + scheduleShutdownAfterJob(5000); + + vi.advanceTimersByTime(30000); + + // Should exit at 5000 (shutdown), not at 30000 (watchdog) + expect(process.exit).toHaveBeenCalledTimes(1); + expect(process.exit).toHaveBeenCalledWith(0); + }); + + it('replaces existing shutdown timer', () => { + scheduleShutdownAfterJob(5000); + scheduleShutdownAfterJob(10000); + + vi.advanceTimersByTime(5000); + expect(process.exit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(5000); + expect(process.exit).toHaveBeenCalledWith(0); + }); + }); + + describe('watchdog cleanup', () => { + it('can set and clear watchdog cleanup', () => { + const cleanup = vi.fn().mockResolvedValue(undefined); + setWatchdogCleanup(cleanup); + clearWatchdogCleanup(); + + // No assertion needed, just ensuring no errors + }); + }); +}); diff --git a/tests/unit/utils/llmLogging.test.ts b/tests/unit/utils/llmLogging.test.ts new file mode 100644 index 00000000..3da19850 --- /dev/null +++ b/tests/unit/utils/llmLogging.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('node:fs', () => ({ + default: { + mkdirSync: vi.fn(), + writeFileSync: vi.fn(), + readdirSync: vi.fn(), + }, +})); + +vi.mock('llmist', () => ({ + extractMessageText: vi.fn((content: unknown) => { + if (typeof content === 'string') return content; + return JSON.stringify(content); + }), +})); + +import fs from 'node:fs'; +import { + createLLMCallLogger, + formatCallNumber, + formatLlmRequest, +} from '../../../src/utils/llmLogging.js'; + +describe('llmLogging', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('formatCallNumber', () => { + it('pads single digit', () => { + expect(formatCallNumber(1)).toBe('0001'); + }); + + it('pads double digit', () => { + expect(formatCallNumber(42)).toBe('0042'); + }); + + it('pads triple digit', () => { + expect(formatCallNumber(123)).toBe('0123'); + }); + + it('does not pad 4+ digits', () => { + expect(formatCallNumber(9999)).toBe('9999'); + expect(formatCallNumber(10000)).toBe('10000'); + }); + }); + + describe('formatLlmRequest', () => { + it('formats messages with role headers', () => { + const messages = [ + { role: 'system', content: 'You are helpful' }, + { role: 'user', content: 'Hello' }, + ]; + + const result = formatLlmRequest(messages); + + expect(result).toContain('=== SYSTEM ==='); + expect(result).toContain('You are helpful'); + expect(result).toContain('=== USER ==='); + expect(result).toContain('Hello'); + }); + + it('handles undefined content', () => { + const messages = [{ role: 'user', content: undefined }]; + + const result = formatLlmRequest(messages); + + expect(result).toContain('=== USER ==='); + // Should not throw + }); + + it('handles empty messages array', () => { + const result = formatLlmRequest([]); + expect(result).toBe(''); + }); + }); + + describe('createLLMCallLogger', () => { + it('creates log directory', () => { + createLLMCallLogger('/tmp/base', 'test-agent'); + + expect(fs.mkdirSync).toHaveBeenCalledWith(expect.stringContaining('test-agent-llm-calls'), { + recursive: true, + }); + }); + + it('logRequest writes request file', () => { + const logger = createLLMCallLogger('/tmp/base', 'agent'); + + logger.logRequest(1, [{ role: 'user', content: 'test' }]); + + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.stringContaining('0001.request'), + expect.any(String), + 'utf-8', + ); + }); + + it('logResponse writes response file', () => { + const logger = createLLMCallLogger('/tmp/base', 'agent'); + + logger.logResponse(1, 'Response text'); + + expect(fs.writeFileSync).toHaveBeenCalledWith( + expect.stringContaining('0001.response'), + 'Response text', + 'utf-8', + ); + }); + + it('getLogFiles returns file paths', () => { + vi.mocked(fs.readdirSync).mockReturnValue([ + '0001.request', + '0001.response', + ] as unknown as ReturnType); + + const logger = createLLMCallLogger('/tmp/base', 'agent'); + const files = logger.getLogFiles(); + + expect(files).toHaveLength(2); + expect(files[0]).toContain('0001.request'); + }); + + it('getLogFiles returns empty array on error', () => { + vi.mocked(fs.readdirSync).mockImplementation(() => { + throw new Error('ENOENT'); + }); + + const logger = createLLMCallLogger('/tmp/base', 'agent'); + const files = logger.getLogFiles(); + + expect(files).toEqual([]); + }); + + it('exposes logDir', () => { + const logger = createLLMCallLogger('/tmp/base', 'agent'); + + expect(logger.logDir).toContain('agent-llm-calls'); + }); + }); +}); diff --git a/tests/unit/utils/llmMetrics.test.ts b/tests/unit/utils/llmMetrics.test.ts new file mode 100644 index 00000000..7ee5e827 --- /dev/null +++ b/tests/unit/utils/llmMetrics.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + calculateCost, + estimateInputTokens, + logLLMCallStart, + logLLMMetrics, +} from '../../../src/utils/llmMetrics.js'; + +describe('llmMetrics', () => { + describe('calculateCost', () => { + it('calculates cost for known model', () => { + const cost = calculateCost('gemini:gemini-2.5-flash', { + inputTokens: 1_000_000, + outputTokens: 1_000_000, + }); + + // $0.15 input + $0.60 output = $0.75 + expect(cost).toBeCloseTo(0.75, 6); + }); + + it('returns 0 for unknown model', () => { + const cost = calculateCost('unknown:model', { + inputTokens: 1000, + outputTokens: 1000, + }); + + expect(cost).toBe(0); + }); + + it('handles zero tokens', () => { + const cost = calculateCost('gemini:gemini-2.5-flash', { + inputTokens: 0, + outputTokens: 0, + }); + + expect(cost).toBe(0); + }); + + it('applies cached input discount for models that support it', () => { + // Anthropic Claude Sonnet 4.5: input=$3, output=$15, cachedInput=$0.3 + const costWithCache = calculateCost('anthropic:claude-sonnet-4-5', { + inputTokens: 1_000_000, + outputTokens: 500_000, + cachedInputTokens: 500_000, + }); + + const costWithoutCache = calculateCost('anthropic:claude-sonnet-4-5', { + inputTokens: 1_000_000, + outputTokens: 500_000, + cachedInputTokens: 0, + }); + + // Cached should be cheaper + expect(costWithCache).toBeLessThan(costWithoutCache); + }); + + it('does not apply cached discount for models without cachedInput pricing', () => { + const cost = calculateCost('gemini:gemini-2.5-flash', { + inputTokens: 1_000_000, + outputTokens: 1_000_000, + cachedInputTokens: 500_000, + }); + + // No cached discount, same as without cached tokens + expect(cost).toBeCloseTo(0.75, 6); + }); + + it('calculates correct cost for small token counts', () => { + // 1000 input tokens at $0.15/1M = $0.00015 + // 500 output tokens at $0.60/1M = $0.0003 + const cost = calculateCost('gemini:gemini-2.5-flash', { + inputTokens: 1000, + outputTokens: 500, + }); + + expect(cost).toBeCloseTo(0.00015 + 0.0003, 8); + }); + }); + + describe('estimateInputTokens', () => { + it('estimates tokens from messages', () => { + const messages = [{ role: 'user', content: 'Hello world' }]; + const estimate = estimateInputTokens(messages); + + // JSON.stringify length / 4, ceiling + expect(estimate).toBeGreaterThan(0); + expect(estimate).toBe(Math.ceil(JSON.stringify(messages).length / 4)); + }); + + it('handles empty messages array', () => { + const estimate = estimateInputTokens([]); + + expect(estimate).toBeGreaterThan(0); // [] still has length 2 + }); + + it('handles large messages', () => { + const longContent = 'a'.repeat(4000); + const messages = [{ role: 'user', content: longContent }]; + const estimate = estimateInputTokens(messages); + + expect(estimate).toBeGreaterThanOrEqual(1000); + }); + }); + + describe('logLLMMetrics', () => { + it('logs metrics with formatted cost', () => { + const mockLogger = { info: vi.fn() }; + + logLLMMetrics(mockLogger, { + model: 'test-model', + iteration: 5, + inputTokens: 1000, + outputTokens: 500, + cachedTokens: 200, + durationMs: 1500, + cost: 0.003456, + }); + + expect(mockLogger.info).toHaveBeenCalledWith('LLM call complete', { + model: 'test-model', + iteration: 5, + inputTokens: 1000, + outputTokens: 500, + cachedTokens: 200, + durationMs: 1500, + cost: '$0.003456', + }); + }); + }); + + describe('logLLMCallStart', () => { + it('logs call start with estimated tokens and message count', () => { + const mockLogger = { info: vi.fn() }; + const messages = [ + { role: 'system', content: 'You are helpful' }, + { role: 'user', content: 'Hello' }, + ]; + + logLLMCallStart(mockLogger, 3, messages); + + expect(mockLogger.info).toHaveBeenCalledWith('LLM call starting', { + iteration: 3, + estimatedInputTokens: expect.any(Number), + messageCount: 2, + }); + }); + }); +}); diff --git a/tests/unit/utils/repo.test.ts b/tests/unit/utils/repo.test.ts new file mode 100644 index 00000000..f3cf6fcd --- /dev/null +++ b/tests/unit/utils/repo.test.ts @@ -0,0 +1,203 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('node:child_process', async () => { + const actual = await vi.importActual('node:child_process'); + return { + ...actual, + execSync: vi.fn(), + spawn: vi.fn(), + }; +}); + +vi.mock('node:fs', () => ({ + existsSync: vi.fn(), + mkdirSync: vi.fn(), + rmSync: vi.fn(), +})); + +vi.mock('../../../src/config/projects.js', () => ({ + getProjectGitHubToken: vi.fn(() => 'test-token'), +})); + +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { + info: vi.fn(), + debug: vi.fn(), + }, +})); + +import { execSync, spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { existsSync, mkdirSync, rmSync } from 'node:fs'; +import { Readable } from 'node:stream'; +import { + cleanupTempDir, + cloneRepo, + createTempDir, + getWorkspaceDir, + runCommand, +} from '../../../src/utils/repo.js'; + +describe('repo utils', () => { + const originalEnv = process.env; + + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + describe('getWorkspaceDir', () => { + it('returns CASCADE_WORKSPACE_DIR when set', () => { + process.env.CASCADE_WORKSPACE_DIR = '/custom/workspace'; + expect(getWorkspaceDir()).toBe('/custom/workspace'); + }); + + it('returns /workspace as default', () => { + process.env.CASCADE_WORKSPACE_DIR = undefined; + expect(getWorkspaceDir()).toBe('/workspace'); + }); + }); + + describe('createTempDir', () => { + it('creates directory with project ID and timestamp', () => { + const dir = createTempDir('my-project'); + + expect(dir).toMatch(/cascade-my-project-\d+/); + expect(mkdirSync).toHaveBeenCalledWith(dir, { recursive: true }); + }); + }); + + describe('cloneRepo', () => { + it('clones repo and configures git user', () => { + const project = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + baseBranch: 'main', + branchPrefix: 'feature/', + githubTokenEnv: 'GITHUB_TOKEN', + trello: { boardId: 'board', lists: {}, labels: {} }, + }; + + cloneRepo(project, '/tmp/repo'); + + expect(execSync).toHaveBeenCalledWith( + expect.stringContaining('git clone'), + expect.objectContaining({ stdio: 'pipe' }), + ); + expect(execSync).toHaveBeenCalledWith( + expect.stringContaining('git config user.name'), + expect.objectContaining({ cwd: '/tmp/repo' }), + ); + expect(execSync).toHaveBeenCalledWith( + expect.stringContaining('git config user.email'), + expect.objectContaining({ cwd: '/tmp/repo' }), + ); + }); + }); + + describe('cleanupTempDir', () => { + it('removes directory when it exists and matches pattern', () => { + vi.mocked(existsSync).mockReturnValue(true); + + cleanupTempDir('/workspace/cascade-test-123'); + + expect(rmSync).toHaveBeenCalledWith('/workspace/cascade-test-123', { + recursive: true, + force: true, + }); + }); + + it('does not remove directory that does not exist', () => { + vi.mocked(existsSync).mockReturnValue(false); + + cleanupTempDir('/workspace/cascade-test-123'); + + expect(rmSync).not.toHaveBeenCalled(); + }); + + it('does not remove directory that does not match cascade pattern', () => { + vi.mocked(existsSync).mockReturnValue(true); + + cleanupTempDir('/workspace/other-dir'); + + expect(rmSync).not.toHaveBeenCalled(); + }); + }); + + describe('runCommand', () => { + function createMockChild() { + const stdout = new Readable({ read() {} }); + const stderr = new Readable({ read() {} }); + const child = new EventEmitter() as EventEmitter & { + stdout: Readable; + stderr: Readable; + stdin: { write: vi.Mock; end: vi.Mock }; + }; + child.stdout = stdout; + child.stderr = stderr; + child.stdin = { write: vi.fn(), end: vi.fn() }; + return child; + } + + it('runs command and returns stdout/stderr/exitCode', async () => { + const mockChild = createMockChild(); + vi.mocked(spawn).mockReturnValue(mockChild as unknown as ReturnType); + + const promise = runCommand('echo', ['hello'], '/tmp'); + + // Need to yield to allow event handlers to be attached + await new Promise((r) => setTimeout(r, 0)); + + mockChild.stdout.push('hello\n'); + mockChild.stdout.push(null); + mockChild.stderr.push(null); + mockChild.emit('close', 0); + + const result = await promise; + + expect(result.stdout).toBe('hello\n'); + expect(result.stderr).toBe(''); + expect(result.exitCode).toBe(0); + }); + + it('handles command error', async () => { + const mockChild = createMockChild(); + vi.mocked(spawn).mockReturnValue(mockChild as unknown as ReturnType); + + const promise = runCommand('bad-command', [], '/tmp'); + + await new Promise((r) => setTimeout(r, 0)); + + mockChild.stdout.push(null); + mockChild.stderr.push(null); + mockChild.emit('error', new Error('spawn ENOENT')); + + const result = await promise; + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('spawn ENOENT'); + }); + + it('handles null exit code', async () => { + const mockChild = createMockChild(); + vi.mocked(spawn).mockReturnValue(mockChild as unknown as ReturnType); + + const promise = runCommand('cmd', [], '/tmp'); + + await new Promise((r) => setTimeout(r, 0)); + + mockChild.stdout.push(null); + mockChild.stderr.push(null); + mockChild.emit('close', null); + + const result = await promise; + + expect(result.exitCode).toBe(1); + }); + }); +}); diff --git a/tests/unit/utils/safeOperation.test.ts b/tests/unit/utils/safeOperation.test.ts new file mode 100644 index 00000000..1d0490a7 --- /dev/null +++ b/tests/unit/utils/safeOperation.test.ts @@ -0,0 +1,81 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { safeOperation, silentOperation } from '../../../src/utils/safeOperation.js'; + +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { + warn: vi.fn(), + }, +})); + +import { logger } from '../../../src/utils/logging.js'; + +describe('safeOperation', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('safeOperation', () => { + it('returns result on success', async () => { + const result = await safeOperation(() => Promise.resolve('hello'), { + action: 'test operation', + }); + + expect(result).toBe('hello'); + }); + + it('returns undefined on failure', async () => { + const result = await safeOperation(() => Promise.reject(new Error('fail')), { + action: 'test operation', + }); + + expect(result).toBeUndefined(); + }); + + it('logs warning on failure with context', async () => { + await safeOperation(() => Promise.reject(new Error('something broke')), { + action: 'fetch data', + prNumber: 42, + }); + + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to fetch data', + expect.objectContaining({ + error: 'Error: something broke', + action: 'fetch data', + prNumber: 42, + }), + ); + }); + + it('handles non-Error thrown values', async () => { + await safeOperation(() => Promise.reject('string error'), { action: 'do thing' }); + + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to do thing', + expect.objectContaining({ + error: 'string error', + }), + ); + }); + }); + + describe('silentOperation', () => { + it('returns result on success', async () => { + const result = await silentOperation(() => Promise.resolve(42)); + + expect(result).toBe(42); + }); + + it('returns undefined on failure', async () => { + const result = await silentOperation(() => Promise.reject(new Error('fail'))); + + expect(result).toBeUndefined(); + }); + + it('does not log on failure', async () => { + await silentOperation(() => Promise.reject(new Error('fail'))); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + }); +});