diff --git a/.env.example b/.env.example index 94232d9f..39950c95 100644 --- a/.env.example +++ b/.env.example @@ -5,6 +5,9 @@ TRELLO_TOKEN= # GitHub token for cloning repos and creating PRs GITHUB_TOKEN= +# GitHub reviewer PAT (separate account for submitting PR reviews) +GITHUB_REVIEWER_TOKEN= + # LLM API keys GEMINI_API_KEY= ANTHROPIC_API_KEY= diff --git a/config/projects.json b/config/projects.json index 94466142..01742420 100644 --- a/config/projects.json +++ b/config/projects.json @@ -21,6 +21,7 @@ "repo": "zbigniewsobiecki/niu", "baseBranch": "dev", "branchPrefix": "feature/", + "reviewerTokenEnv": "GITHUB_REVIEWER_TOKEN", "trello": { "boardId": "694ec393370da080b52eb64c", "lists": { @@ -51,6 +52,7 @@ "repo": "zbigniewsobiecki/car-dealership", "baseBranch": "dev", "branchPrefix": "feature/", + "reviewerTokenEnv": "GITHUB_REVIEWER_TOKEN", "trello": { "boardId": "698db5df2b873930c7c38bc0", "lists": { diff --git a/src/agents/prompts/templates/review.eta b/src/agents/prompts/templates/review.eta index c9bffdf5..40635c5f 100644 --- a/src/agents/prompts/templates/review.eta +++ b/src/agents/prompts/templates/review.eta @@ -10,7 +10,7 @@ You are a code reviewer. Your goal is to identify **real issues** that affect co ## Process -1. **Understand the change**: Read the PR description and all modified files. Understand WHAT changed and WHY. +1. **Understand the change**: Read the PR description and all modified files. Understand WHAT changed and WHY. An initial comment has already been posted on the PR acknowledging the review is in progress. 2. **Consult the pre-loaded Squint overview BEFORE reading files**: The overview maps module boundaries, dependencies (→ arrows), and feature flows. Use `squint modules show --json` via Tmux to drill into modules touched by the PR. This lets you verify changes fit established patterns without manually tracing the architecture. @@ -170,3 +170,4 @@ CreatePRReview({ - Run tests/linting via Tmux when you need to verify functionality - One review submission per PR - include all feedback in a single review - If the PR is good, approve it. Don't manufacture issues to appear thorough. +- After submitting your review, update the initial PR comment with your review summary using UpdatePRComment (the comment ID is available from the pre-loaded PostPRComment call) diff --git a/src/agents/review.ts b/src/agents/review.ts index 0d83a449..8aeee398 100644 --- a/src/agents/review.ts +++ b/src/agents/review.ts @@ -12,11 +12,13 @@ import { GetPRChecks, GetPRDetails, GetPRDiff, + UpdatePRComment, formatCheckStatus, } from '../gadgets/github/index.js'; +import { recordInitialComment } from '../gadgets/sessionState.js'; import { Tmux } from '../gadgets/tmux.js'; import { TodoDelete, TodoUpdateStatus, TodoUpsert } from '../gadgets/todo/index.js'; -import { githubClient } from '../github/client.js'; +import { githubClient, withGitHubToken } from '../github/client.js'; import type { AgentInput, AgentResult, CascadeConfig, ProjectConfig } from '../types/index.js'; import { logger } from '../utils/logging.js'; import { type BuilderType, createConfiguredBuilder } from './shared/builderFactory.js'; @@ -190,6 +192,7 @@ function getReviewGadgets() { new GetPRDiff(), new GetPRChecks(), new CreatePRReview(), + new UpdatePRComment(), new Finish(), ]; } @@ -227,10 +230,34 @@ async function injectReviewSyntheticCalls( trackingContext: TrackingContext, repoDir: string, ): Promise { - // Inject PR details, diff, and check status + // Post initial "reviewing" comment on the PR + const initialCommentBody = '🔍 Reviewing PR...'; + const initialComment = await githubClient.createPRComment( + owner, + repo, + prNumber, + initialCommentBody, + ); + recordInitialComment(initialComment.id); let builder = injectSyntheticCall( initialBuilder, trackingContext, + 'PostPRComment', + { + comment: 'Post initial review status comment', + owner, + repo, + prNumber, + body: initialCommentBody, + }, + `Comment posted (id: ${initialComment.id}): ${initialComment.htmlUrl}`, + 'gc_initial_comment', + ); + + // Inject PR details, diff, and check status + builder = injectSyntheticCall( + builder, + trackingContext, 'GetPRDetails', { comment: 'Pre-fetching PR details for review context', owner, repo, prNumber }, ctx.prDetailsFormatted, @@ -289,38 +316,63 @@ export async function executeReviewAgent(input: ReviewAgentInput): Promise({ - loggerIdentifier: `review-${prNumber}`, - - onWatchdogTimeout: async () => { - await githubClient.createPRComment( - owner, - repo, - prNumber, - '⚠️ Review agent timed out while reviewing the PR.', - ); - logger.info('Posted timeout notice to PR', { prNumber }); - }, - - setupRepoDir: (log) => setupRepository({ project, log, agentType: 'review', prBranch }), - - buildContext: (repoDir, log) => - buildReviewContext(owner, repo, prNumber, repoDir, project, config, log, input.modelOverride), + const reviewerToken = project.reviewerTokenEnv + ? process.env[project.reviewerTokenEnv] + : undefined; - createBuilder: ({ client, ctx, llmistLogger, trackingContext, fileLogger, repoDir }) => - createReviewAgentBuilder( - client, - ctx, - llmistLogger, - trackingContext, - fileLogger.write.bind(fileLogger), - fileLogger.llmCallLogger, - repoDir, - ), - - injectSyntheticCalls: ({ builder, ctx, trackingContext, repoDir }) => - injectReviewSyntheticCalls(builder, owner, repo, prNumber, ctx, trackingContext, repoDir), + if (project.reviewerTokenEnv && !reviewerToken) { + logger.warn('Reviewer token env configured but not set, falling back to main token', { + reviewerTokenEnv: project.reviewerTokenEnv, + }); + } - interactive, - }); + const runLifecycle = () => + executeAgentLifecycle({ + loggerIdentifier: `review-${prNumber}`, + + onWatchdogTimeout: async () => { + await githubClient.createPRComment( + owner, + repo, + prNumber, + '⚠️ Review agent timed out while reviewing the PR.', + ); + logger.info('Posted timeout notice to PR', { prNumber }); + }, + + setupRepoDir: (log) => setupRepository({ project, log, agentType: 'review', prBranch }), + + buildContext: (repoDir, log) => + buildReviewContext( + owner, + repo, + prNumber, + repoDir, + project, + config, + log, + input.modelOverride, + ), + + createBuilder: ({ client, ctx, llmistLogger, trackingContext, fileLogger, repoDir }) => + createReviewAgentBuilder( + client, + ctx, + llmistLogger, + trackingContext, + fileLogger.write.bind(fileLogger), + fileLogger.llmCallLogger, + repoDir, + ), + + injectSyntheticCalls: ({ builder, ctx, trackingContext, repoDir }) => + injectReviewSyntheticCalls(builder, owner, repo, prNumber, ctx, trackingContext, repoDir), + + interactive, + }); + + if (reviewerToken) { + return withGitHubToken(reviewerToken, runLifecycle); + } + return runLifecycle(); } diff --git a/src/config/schema.ts b/src/config/schema.ts index e970a43b..20eb3284 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -15,6 +15,7 @@ export const ProjectConfigSchema = z.object({ baseBranch: z.string().default('main'), branchPrefix: z.string().default('feature/'), githubTokenEnv: z.string().default('GITHUB_TOKEN'), + reviewerTokenEnv: z.string().optional(), trello: z.object({ boardId: z.string().min(1), diff --git a/src/github/client.ts b/src/github/client.ts index 99a76e35..740a34bd 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -1,9 +1,14 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; import { Octokit } from '@octokit/rest'; import { logger } from '../utils/logging.js'; let client: Octokit | null = null; +const clientStorage = new AsyncLocalStorage(); function getClient(): Octokit { + const scopedClient = clientStorage.getStore(); + if (scopedClient) return scopedClient; + if (!client) { const token = process.env.GITHUB_TOKEN; @@ -16,6 +21,11 @@ function getClient(): Octokit { return client; } +export function withGitHubToken(token: string, fn: () => Promise): Promise { + const scopedClient = new Octokit({ auth: token }); + return clientStorage.run(scopedClient, fn); +} + export interface PRDetails { number: number; title: string; @@ -383,6 +393,31 @@ export async function getAuthenticatedUser(): Promise { return data.login; } +const reviewerUserCache = new Map(); + +export async function getReviewerUser(reviewerTokenEnv?: string): Promise { + if (!reviewerTokenEnv) return null; + const cached = reviewerUserCache.get(reviewerTokenEnv); + if (cached) return cached; + + const token = process.env[reviewerTokenEnv]; + if (!token) { + logger.warn('Reviewer token env var configured but not set', { reviewerTokenEnv }); + return null; + } + + try { + const reviewerClient = new Octokit({ auth: token }); + const { data } = await reviewerClient.users.getAuthenticated(); + reviewerUserCache.set(reviewerTokenEnv, data.login); + return data.login; + } catch (err) { + logger.warn('Failed to resolve reviewer GitHub identity', { error: String(err) }); + return null; + } +} + export function resetGitHubClient(): void { client = null; + reviewerUserCache.clear(); } diff --git a/src/triggers/github/check-suite-success.ts b/src/triggers/github/check-suite-success.ts index 0be8d4ae..4f500359 100644 --- a/src/triggers/github/check-suite-success.ts +++ b/src/triggers/github/check-suite-success.ts @@ -1,4 +1,4 @@ -import { getAuthenticatedUser, githubClient } from '../../github/client.js'; +import { getAuthenticatedUser, getReviewerUser, githubClient } from '../../github/client.js'; import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js'; @@ -57,12 +57,15 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler { const cardId = extractTrelloCardId(prDetails.body); // Skip if we already reviewed this PR (only fire once per PR) - const [reviews, botUser] = await Promise.all([ + const [reviews, botUser, reviewerUser] = await Promise.all([ githubClient.getPRReviews(owner, repo, prNumber), getAuthenticatedUser(), + getReviewerUser(ctx.project.reviewerTokenEnv), ]); - const alreadyReviewed = reviews.some((r) => r.user.login === botUser); + const alreadyReviewed = reviews.some( + (r) => r.user.login === botUser || (reviewerUser && r.user.login === reviewerUser), + ); if (alreadyReviewed) { logger.info('PR already reviewed by us, skipping', { prNumber, diff --git a/src/triggers/github/issue-comment.ts b/src/triggers/github/issue-comment.ts index 8c8ee02b..718ff852 100644 --- a/src/triggers/github/issue-comment.ts +++ b/src/triggers/github/issue-comment.ts @@ -32,7 +32,9 @@ export class IssueCommentTrigger implements TriggerHandler { const [owner, repo] = payload.repository.full_name.split('/'); // Skip comments from ourselves to avoid infinite loops - if (await isSelfAuthored(commentAuthor, { prNumber, authorField: 'commentAuthor' })) { + if ( + await isSelfAuthored(commentAuthor, { prNumber, authorField: 'commentAuthor' }, ctx.project) + ) { return null; } diff --git a/src/triggers/github/pr-review-comment.ts b/src/triggers/github/pr-review-comment.ts index cab6b63f..b386a282 100644 --- a/src/triggers/github/pr-review-comment.ts +++ b/src/triggers/github/pr-review-comment.ts @@ -34,7 +34,9 @@ export class PRReviewCommentTrigger implements TriggerHandler { const commentAuthor = prPayload.comment.user.login; // Skip comments from ourselves to avoid infinite loops - if (await isSelfAuthored(commentAuthor, { prNumber, authorField: 'commentAuthor' })) { + if ( + await isSelfAuthored(commentAuthor, { prNumber, authorField: 'commentAuthor' }, ctx.project) + ) { return null; } diff --git a/src/triggers/github/pr-review-submitted.ts b/src/triggers/github/pr-review-submitted.ts index d5b94f98..5bbadcd8 100644 --- a/src/triggers/github/pr-review-submitted.ts +++ b/src/triggers/github/pr-review-submitted.ts @@ -38,7 +38,9 @@ export class PRReviewSubmittedTrigger implements TriggerHandler { const reviewAuthor = reviewPayload.review.user.login; // Skip reviews from ourselves to avoid infinite loops - if (await isSelfAuthored(reviewAuthor, { prNumber, authorField: 'reviewAuthor' })) { + if ( + await isSelfAuthored(reviewAuthor, { prNumber, authorField: 'reviewAuthor' }, ctx.project) + ) { return null; } diff --git a/src/triggers/github/utils.ts b/src/triggers/github/utils.ts index 2a71f6ed..8e0a7e53 100644 --- a/src/triggers/github/utils.ts +++ b/src/triggers/github/utils.ts @@ -1,4 +1,5 @@ -import { getAuthenticatedUser } from '../../github/client.js'; +import { getAuthenticatedUser, getReviewerUser } from '../../github/client.js'; +import type { ProjectConfig } from '../../types/index.js'; import { logger } from '../../utils/logging.js'; // Trello card URL pattern: https://trello.com/c/SHORT_ID/optional-slug @@ -38,6 +39,7 @@ export function extractTrelloCardUrl(text: string | null): string | null { export async function isSelfAuthored( author: string, context: { prNumber: number; authorField: string }, + project?: ProjectConfig, ): Promise { try { const authenticatedUser = await getAuthenticatedUser(); @@ -54,6 +56,19 @@ export async function isSelfAuthored( error: String(err), }); } + + if (project?.reviewerTokenEnv) { + const reviewerUser = await getReviewerUser(project.reviewerTokenEnv); + if (reviewerUser && (author === reviewerUser || author === `${reviewerUser}[bot]`)) { + logger.info(`Skipping reviewer-authored ${context.authorField}`, { + prNumber: context.prNumber, + [context.authorField]: author, + reviewerUser, + }); + return true; + } + } + return false; } diff --git a/tests/unit/config/schema.test.ts b/tests/unit/config/schema.test.ts index e90cfd48..110c8968 100644 --- a/tests/unit/config/schema.test.ts +++ b/tests/unit/config/schema.test.ts @@ -58,6 +58,39 @@ describe('ProjectConfigSchema', () => { expect(result.branchPrefix).toBe('feature/'); expect(result.githubTokenEnv).toBe('GITHUB_TOKEN'); }); + + it('accepts optional reviewerTokenEnv', () => { + const config = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + reviewerTokenEnv: 'GITHUB_REVIEWER_TOKEN', + trello: { + boardId: 'board123', + lists: {}, + labels: {}, + }, + }; + + const result = ProjectConfigSchema.parse(config); + expect(result.reviewerTokenEnv).toBe('GITHUB_REVIEWER_TOKEN'); + }); + + it('does not require reviewerTokenEnv', () => { + const config = { + id: 'test', + name: 'Test', + repo: 'owner/repo', + trello: { + boardId: 'board123', + lists: {}, + labels: {}, + }, + }; + + const result = ProjectConfigSchema.parse(config); + expect(result.reviewerTokenEnv).toBeUndefined(); + }); }); describe('validateConfig', () => { diff --git a/tests/unit/github/client.test.ts b/tests/unit/github/client.test.ts index 7eee9809..8a730281 100644 --- a/tests/unit/github/client.test.ts +++ b/tests/unit/github/client.test.ts @@ -50,10 +50,14 @@ vi.mock('../../../src/utils/logging.js', () => ({ import { getAuthenticatedUser, + getReviewerUser, githubClient, resetGitHubClient, + withGitHubToken, } from '../../../src/github/client.js'; +import { Octokit } from '@octokit/rest'; + describe('githubClient', () => { const originalEnv = process.env; @@ -591,4 +595,103 @@ describe('githubClient', () => { ); }); }); + + describe('withGitHubToken', () => { + it('scopes a different Octokit instance within the callback', async () => { + // First call uses the default token + mockPulls.get.mockResolvedValue({ + data: { + number: 1, + title: 'PR', + body: null, + state: 'open', + html_url: 'url', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + merged: false, + }, + }); + + await githubClient.getPR('owner', 'repo', 1); + // Default client created with test-token + expect(Octokit).toHaveBeenCalledWith({ auth: 'test-token' }); + + vi.mocked(Octokit).mockClear(); + + // Now call within withGitHubToken scope + await withGitHubToken('reviewer-token', async () => { + await githubClient.getPR('owner', 'repo', 2); + }); + + // A new Octokit was created with reviewer-token + expect(Octokit).toHaveBeenCalledWith({ auth: 'reviewer-token' }); + }); + + it('restores original client after scope exits', async () => { + mockPulls.get.mockResolvedValue({ + data: { + number: 1, + title: 'PR', + body: null, + state: 'open', + html_url: 'url', + head: { ref: 'feat', sha: 'abc' }, + base: { ref: 'main' }, + merged: false, + }, + }); + + // Initialize the default singleton first + await githubClient.getPR('owner', 'repo', 1); + vi.mocked(Octokit).mockClear(); + + await withGitHubToken('reviewer-token', async () => { + await githubClient.getPR('owner', 'repo', 2); + }); + + // The scoped call created a new Octokit with reviewer-token + expect(Octokit).toHaveBeenCalledWith({ auth: 'reviewer-token' }); + vi.mocked(Octokit).mockClear(); + + // After scope, calls should NOT create a new Octokit (uses cached singleton) + await githubClient.getPR('owner', 'repo', 3); + expect(Octokit).not.toHaveBeenCalled(); + }); + }); + + describe('getReviewerUser', () => { + it('returns null when no reviewerTokenEnv provided', async () => { + const result = await getReviewerUser(); + expect(result).toBeNull(); + }); + + it('returns null when env var is not set', async () => { + const result = await getReviewerUser('MISSING_TOKEN'); + expect(result).toBeNull(); + }); + + it('resolves and caches reviewer username', async () => { + process.env.REVIEWER_TOKEN = 'reviewer-pat'; + mockUsers.getAuthenticated.mockResolvedValue({ + data: { login: 'cascade-reviewer' }, + }); + + const result1 = await getReviewerUser('REVIEWER_TOKEN'); + expect(result1).toBe('cascade-reviewer'); + + // Second call should use cache (Octokit only called once for the reviewer) + const result2 = await getReviewerUser('REVIEWER_TOKEN'); + expect(result2).toBe('cascade-reviewer'); + // Two Octokit constructions: one for main client (from beforeEach getPR calls), one for reviewer + // But the reviewer Octokit should only be created once due to caching + }); + + it('returns null on auth failure', async () => { + process.env.REVIEWER_TOKEN = 'bad-token'; + mockUsers.getAuthenticated.mockRejectedValue(new Error('Bad credentials')); + + const result = await getReviewerUser('REVIEWER_TOKEN'); + expect(result).toBeNull(); + }); + }); }); diff --git a/tests/unit/triggers/check-suite-success.test.ts b/tests/unit/triggers/check-suite-success.test.ts index 45b96301..9c5f2c3c 100644 --- a/tests/unit/triggers/check-suite-success.test.ts +++ b/tests/unit/triggers/check-suite-success.test.ts @@ -9,9 +9,10 @@ vi.mock('../../../src/github/client.js', () => ({ getCheckSuiteStatus: vi.fn(), }, getAuthenticatedUser: vi.fn(), + getReviewerUser: vi.fn(), })); -import { getAuthenticatedUser, githubClient } from '../../../src/github/client.js'; +import { getAuthenticatedUser, getReviewerUser, githubClient } from '../../../src/github/client.js'; describe('CheckSuiteSuccessTrigger', () => { const trigger = new CheckSuiteSuccessTrigger(); @@ -50,6 +51,7 @@ describe('CheckSuiteSuccessTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(getReviewerUser).mockResolvedValue(null); }); describe('matches', () => { @@ -261,6 +263,42 @@ describe('CheckSuiteSuccessTrigger', () => { expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); }); + it('returns null when PR was already reviewed by reviewer identity', 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.getPRReviews).mockResolvedValue([ + { + id: 1, + user: { login: 'cascade-reviewer' }, + state: 'approved', + body: 'LGTM', + submittedAt: '', + }, + ]); + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(getReviewerUser).mockResolvedValue('cascade-reviewer'); + + const ctx: TriggerContext = { + project: { ...mockProject, reviewerTokenEnv: 'REVIEWER_TOKEN' }, + source: 'github', + payload: makeCheckSuitePayload(), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(getReviewerUser).toHaveBeenCalledWith('REVIEWER_TOKEN'); + expect(githubClient.getCheckSuiteStatus).not.toHaveBeenCalled(); + }); + it('proceeds when PR has reviews from other users only', async () => { vi.mocked(githubClient.getPR).mockResolvedValue({ number: 42, diff --git a/tests/unit/triggers/issue-comment.test.ts b/tests/unit/triggers/issue-comment.test.ts index 91f9f528..f8275e91 100644 --- a/tests/unit/triggers/issue-comment.test.ts +++ b/tests/unit/triggers/issue-comment.test.ts @@ -4,12 +4,13 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; vi.mock('../../../src/github/client.js', () => ({ getAuthenticatedUser: vi.fn(), + getReviewerUser: vi.fn(), githubClient: { getPR: vi.fn(), }, })); -import { getAuthenticatedUser, githubClient } from '../../../src/github/client.js'; +import { getAuthenticatedUser, getReviewerUser, githubClient } from '../../../src/github/client.js'; describe('IssueCommentTrigger', () => { const trigger = new IssueCommentTrigger(); @@ -53,6 +54,7 @@ describe('IssueCommentTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(getReviewerUser).mockResolvedValue(null); }); describe('matches', () => { @@ -199,6 +201,29 @@ describe('IssueCommentTrigger', () => { expect(result).toBeNull(); }); + it('returns null for reviewer-authored comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(getReviewerUser).mockResolvedValue('cascade-reviewer'); + + const ctx: TriggerContext = { + project: { ...mockProject, reviewerTokenEnv: 'REVIEWER_TOKEN' }, + source: 'github', + payload: makeIssueCommentPayload({ + comment: { + id: 200, + body: 'Reviewer comment', + html_url: 'https://github.com/...', + user: { login: 'cascade-reviewer' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getPR).not.toHaveBeenCalled(); + }); + it('proceeds when getAuthenticatedUser fails', async () => { vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Auth error')); vi.mocked(githubClient.getPR).mockResolvedValue({ diff --git a/tests/unit/triggers/pr-review-comment.test.ts b/tests/unit/triggers/pr-review-comment.test.ts index d928e8e1..f91f4ce1 100644 --- a/tests/unit/triggers/pr-review-comment.test.ts +++ b/tests/unit/triggers/pr-review-comment.test.ts @@ -4,12 +4,13 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; vi.mock('../../../src/github/client.js', () => ({ getAuthenticatedUser: vi.fn(), + getReviewerUser: vi.fn(), githubClient: { getPR: vi.fn(), }, })); -import { getAuthenticatedUser, githubClient } from '../../../src/github/client.js'; +import { getAuthenticatedUser, getReviewerUser, githubClient } from '../../../src/github/client.js'; describe('PRReviewCommentTrigger', () => { const trigger = new PRReviewCommentTrigger(); @@ -56,6 +57,7 @@ describe('PRReviewCommentTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(getReviewerUser).mockResolvedValue(null); }); describe('matches', () => { @@ -187,6 +189,31 @@ describe('PRReviewCommentTrigger', () => { expect(result).toBeNull(); }); + it('returns null for reviewer-authored comment', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(getReviewerUser).mockResolvedValue('cascade-reviewer'); + + const ctx: TriggerContext = { + project: { ...mockProject, reviewerTokenEnv: 'REVIEWER_TOKEN' }, + source: 'github', + payload: makeReviewCommentPayload({ + comment: { + id: 300, + body: 'Reviewer comment', + path: 'src/index.ts', + line: 42, + user: { login: 'cascade-reviewer' }, + html_url: 'https://github.com/...', + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + expect(githubClient.getPR).not.toHaveBeenCalled(); + }); + it('proceeds when getAuthenticatedUser fails', async () => { vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Token error')); vi.mocked(githubClient.getPR).mockResolvedValue({ diff --git a/tests/unit/triggers/pr-review-submitted.test.ts b/tests/unit/triggers/pr-review-submitted.test.ts index c32f5c20..9a77c328 100644 --- a/tests/unit/triggers/pr-review-submitted.test.ts +++ b/tests/unit/triggers/pr-review-submitted.test.ts @@ -4,9 +4,10 @@ import type { TriggerContext } from '../../../src/triggers/types.js'; vi.mock('../../../src/github/client.js', () => ({ getAuthenticatedUser: vi.fn(), + getReviewerUser: vi.fn(), })); -import { getAuthenticatedUser } from '../../../src/github/client.js'; +import { getAuthenticatedUser, getReviewerUser } from '../../../src/github/client.js'; describe('PRReviewSubmittedTrigger', () => { const trigger = new PRReviewSubmittedTrigger(); @@ -53,6 +54,7 @@ describe('PRReviewSubmittedTrigger', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(getReviewerUser).mockResolvedValue(null); }); describe('matches', () => { @@ -197,6 +199,29 @@ describe('PRReviewSubmittedTrigger', () => { expect(result).toBeNull(); }); + it('returns null for reviewer-authored review', async () => { + vi.mocked(getAuthenticatedUser).mockResolvedValue('cascade-bot'); + vi.mocked(getReviewerUser).mockResolvedValue('cascade-reviewer'); + + const ctx: TriggerContext = { + project: { ...mockProject, reviewerTokenEnv: 'REVIEWER_TOKEN' }, + source: 'github', + payload: makeReviewPayload({ + review: { + id: 100, + state: 'changes_requested', + body: 'Fix this', + html_url: 'https://github.com/...', + user: { login: 'cascade-reviewer' }, + }, + }), + }; + + const result = await trigger.handle(ctx); + + expect(result).toBeNull(); + }); + it('proceeds when getAuthenticatedUser fails', async () => { vi.mocked(getAuthenticatedUser).mockRejectedValue(new Error('Token error'));