Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 2 additions & 0 deletions config/projects.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"repo": "zbigniewsobiecki/niu",
"baseBranch": "dev",
"branchPrefix": "feature/",
"reviewerTokenEnv": "GITHUB_REVIEWER_TOKEN",
"trello": {
"boardId": "694ec393370da080b52eb64c",
"lists": {
Expand Down Expand Up @@ -51,6 +52,7 @@
"repo": "zbigniewsobiecki/car-dealership",
"baseBranch": "dev",
"branchPrefix": "feature/",
"reviewerTokenEnv": "GITHUB_REVIEWER_TOKEN",
"trello": {
"boardId": "698db5df2b873930c7c38bc0",
"lists": {
Expand Down
3 changes: 2 additions & 1 deletion src/agents/prompts/templates/review.eta
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path> --json` via Tmux to drill into modules touched by the PR. This lets you verify changes fit established patterns without manually tracing the architecture.

Expand Down Expand Up @@ -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)
120 changes: 86 additions & 34 deletions src/agents/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -190,6 +192,7 @@ function getReviewGadgets() {
new GetPRDiff(),
new GetPRChecks(),
new CreatePRReview(),
new UpdatePRComment(),
new Finish(),
];
}
Expand Down Expand Up @@ -227,10 +230,34 @@ async function injectReviewSyntheticCalls(
trackingContext: TrackingContext,
repoDir: string,
): Promise<BuilderType> {
// 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,
Expand Down Expand Up @@ -289,38 +316,63 @@ export async function executeReviewAgent(input: ReviewAgentInput): Promise<Agent
return { success: false, output: '', error: `Invalid repo format: ${repoFullName}` };
}

return executeAgentLifecycle<ReviewContextData>({
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<ReviewContextData>({
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();
}
1 change: 1 addition & 0 deletions src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
35 changes: 35 additions & 0 deletions src/github/client.ts
Original file line number Diff line number Diff line change
@@ -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<Octokit>();

function getClient(): Octokit {
const scopedClient = clientStorage.getStore();
if (scopedClient) return scopedClient;

if (!client) {
const token = process.env.GITHUB_TOKEN;

Expand All @@ -16,6 +21,11 @@ function getClient(): Octokit {
return client;
}

export function withGitHubToken<T>(token: string, fn: () => Promise<T>): Promise<T> {
const scopedClient = new Octokit({ auth: token });
return clientStorage.run(scopedClient, fn);
}

export interface PRDetails {
number: number;
title: string;
Expand Down Expand Up @@ -383,6 +393,31 @@ export async function getAuthenticatedUser(): Promise<string> {
return data.login;
}

const reviewerUserCache = new Map<string, string>();

export async function getReviewerUser(reviewerTokenEnv?: string): Promise<string | null> {
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();
}
9 changes: 6 additions & 3 deletions src/triggers/github/check-suite-success.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/triggers/github/issue-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion src/triggers/github/pr-review-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion src/triggers/github/pr-review-submitted.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
17 changes: 16 additions & 1 deletion src/triggers/github/utils.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<boolean> {
try {
const authenticatedUser = await getAuthenticatedUser();
Expand All @@ -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;
}

Expand Down
33 changes: 33 additions & 0 deletions tests/unit/config/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading