Skip to content

Commit 2856f28

Browse files
authored
Add review status and summary to PR reviews (#7)
## Summary - add review summary body with severity counts and assessment - set review event to APPROVE/REQUEST_CHANGES based on HIGH findings - post summary-only reviews when there are no inline comments or comments are silenced and update tests ## Testing - ~/.bun/bin/bun test scripts/comment-pr-findings.bun.test.js
1 parent 883c390 commit 2856f28

File tree

3 files changed

+324
-100
lines changed

3 files changed

+324
-100
lines changed

AGENTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ claudecode/
4141
## Testing
4242

4343
```bash
44+
# Python tests
4445
pytest claudecode -v # Run all tests (177 passing)
46+
# JavaScript tests
47+
~/.bun/bin/bun test scripts/comment-pr-findings.bun.test.js
4548
```
4649

4750
## Code Style

scripts/comment-pr-findings.bun.test.js

Lines changed: 167 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ describe('comment-pr-findings.js', () => {
8585
}
8686
});
8787

88+
spawnSyncSpy.mockImplementation((cmd, args) => {
89+
if (cmd === 'gh' && args.includes('api')) {
90+
return { status: 0, stdout: '{}', stderr: '' };
91+
}
92+
return { status: 0, stdout: '{}', stderr: '' };
93+
});
94+
8895
await import('./comment-pr-findings.js');
8996

9097
expect(readFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining('github-event.json'), 'utf8');
@@ -112,15 +119,46 @@ describe('comment-pr-findings.js', () => {
112119
await import('./comment-pr-findings.js');
113120

114121
expect(consoleLogSpy).toHaveBeenCalledWith('Could not read findings file');
115-
expect(spawnSyncSpy).not.toHaveBeenCalled();
116122
});
117123

118-
test('should exit early when findings array is empty', async () => {
119-
readFileSyncSpy.mockReturnValue('[]');
124+
test('should post summary review when findings array is empty', async () => {
125+
readFileSyncSpy.mockImplementation((path) => {
126+
if (path.includes('github-event.json')) {
127+
return JSON.stringify({
128+
pull_request: {
129+
number: 123,
130+
head: { sha: 'abc123' }
131+
}
132+
});
133+
}
134+
if (path === 'findings.json') {
135+
return '[]';
136+
}
137+
});
138+
139+
let reviewDataCaptured = null;
140+
spawnSyncSpy.mockImplementation((cmd, args, options) => {
141+
if (cmd === 'gh' && args.includes('api')) {
142+
const endpoint = args[1];
143+
const method = args[args.indexOf('--method') + 1] || 'GET';
144+
145+
if (endpoint.includes('/pulls/123/reviews') && method === 'POST') {
146+
if (options && options.input) {
147+
reviewDataCaptured = JSON.parse(options.input);
148+
}
149+
return { status: 0, stdout: '{}', stderr: '' };
150+
}
151+
return { status: 0, stdout: '{}', stderr: '' };
152+
}
153+
return { status: 0, stdout: '{}', stderr: '' };
154+
});
120155

121156
await import('./comment-pr-findings.js');
122-
123-
expect(spawnSyncSpy).not.toHaveBeenCalled();
157+
158+
expect(reviewDataCaptured).toBeTruthy();
159+
expect(reviewDataCaptured.event).toBe('APPROVE');
160+
expect(reviewDataCaptured.body).toContain('Summary: No findings were reported.');
161+
expect(reviewDataCaptured.body).toContain('Assessment:');
124162
});
125163

126164
test('should process findings correctly', async () => {
@@ -192,6 +230,128 @@ describe('comment-pr-findings.js', () => {
192230
// Verify review data was captured
193231
expect(reviewDataCaptured).toBeTruthy();
194232
expect(reviewDataCaptured.comments).toHaveLength(1);
233+
expect(reviewDataCaptured.event).toBe('REQUEST_CHANGES');
234+
expect(reviewDataCaptured.body).toContain('Summary: 1 finding');
235+
expect(reviewDataCaptured.body).toContain('Assessment:');
236+
});
237+
238+
test('should approve when findings are medium or low only', async () => {
239+
const mockFindings = [
240+
{
241+
file: 'alpha.py',
242+
line: 5,
243+
description: 'Potential edge case',
244+
severity: 'MEDIUM',
245+
category: 'correctness'
246+
},
247+
{
248+
file: 'beta.py',
249+
line: 12,
250+
description: 'Minor perf issue',
251+
severity: 'LOW',
252+
category: 'performance'
253+
}
254+
];
255+
256+
const mockPrFiles = [
257+
{ filename: 'alpha.py', patch: '@@ -1,1 +1,1 @@' },
258+
{ filename: 'beta.py', patch: '@@ -1,1 +1,1 @@' }
259+
];
260+
261+
readFileSyncSpy.mockImplementation((path) => {
262+
if (path.includes('github-event.json')) {
263+
return JSON.stringify({
264+
pull_request: {
265+
number: 123,
266+
head: { sha: 'abc123' }
267+
}
268+
});
269+
}
270+
if (path === 'findings.json') {
271+
return JSON.stringify(mockFindings);
272+
}
273+
});
274+
275+
let reviewDataCaptured = null;
276+
spawnSyncSpy.mockImplementation((cmd, args, options) => {
277+
if (cmd === 'gh' && args.includes('api')) {
278+
const endpoint = args[1];
279+
const method = args[args.indexOf('--method') + 1] || 'GET';
280+
281+
if (endpoint.includes('/pulls/123/files')) {
282+
return { status: 0, stdout: JSON.stringify(mockPrFiles), stderr: '' };
283+
}
284+
if (endpoint.includes('/pulls/123/comments') && method === 'GET') {
285+
return { status: 0, stdout: '[]', stderr: '' };
286+
}
287+
if (endpoint.includes('/pulls/123/reviews') && method === 'POST') {
288+
if (options && options.input) {
289+
reviewDataCaptured = JSON.parse(options.input);
290+
}
291+
return { status: 0, stdout: '{}', stderr: '' };
292+
}
293+
return { status: 0, stdout: '{}', stderr: '' };
294+
}
295+
return { status: 0, stdout: '{}', stderr: '' };
296+
});
297+
298+
await import('./comment-pr-findings.js');
299+
300+
expect(reviewDataCaptured).toBeTruthy();
301+
expect(reviewDataCaptured.event).toBe('APPROVE');
302+
expect(reviewDataCaptured.body).toContain('Summary: 2 findings');
303+
expect(reviewDataCaptured.body).toContain('HIGH: 0');
304+
expect(reviewDataCaptured.comments).toHaveLength(2);
305+
});
306+
307+
test('should post summary review when comments are silenced', async () => {
308+
process.env.SILENCE_CLAUDECODE_COMMENTS = 'true';
309+
310+
const mockFindings = [{
311+
file: 'app.py',
312+
line: 3,
313+
description: 'Risky behavior',
314+
severity: 'HIGH',
315+
category: 'security'
316+
}];
317+
318+
readFileSyncSpy.mockImplementation((path) => {
319+
if (path.includes('github-event.json')) {
320+
return JSON.stringify({
321+
pull_request: {
322+
number: 123,
323+
head: { sha: 'abc123' }
324+
}
325+
});
326+
}
327+
if (path === 'findings.json') {
328+
return JSON.stringify(mockFindings);
329+
}
330+
});
331+
332+
let reviewDataCaptured = null;
333+
spawnSyncSpy.mockImplementation((cmd, args, options) => {
334+
if (cmd === 'gh' && args.includes('api')) {
335+
const endpoint = args[1];
336+
const method = args[args.indexOf('--method') + 1] || 'GET';
337+
338+
if (endpoint.includes('/pulls/123/reviews') && method === 'POST') {
339+
if (options && options.input) {
340+
reviewDataCaptured = JSON.parse(options.input);
341+
}
342+
return { status: 0, stdout: '{}', stderr: '' };
343+
}
344+
return { status: 0, stdout: '{}', stderr: '' };
345+
}
346+
return { status: 0, stdout: '{}', stderr: '' };
347+
});
348+
349+
await import('./comment-pr-findings.js');
350+
351+
expect(reviewDataCaptured).toBeTruthy();
352+
expect(reviewDataCaptured.event).toBe('REQUEST_CHANGES');
353+
expect(reviewDataCaptured.body).toContain('Summary: 1 finding');
354+
expect(reviewDataCaptured.comments).toBeUndefined();
195355
});
196356
});
197357

@@ -685,7 +845,7 @@ describe('comment-pr-findings.js', () => {
685845

686846
await import('./comment-pr-findings.js');
687847
expect(consoleLogSpy).toHaveBeenCalledWith('File not-in-diff.py not in PR diff, skipping');
688-
expect(consoleLogSpy).toHaveBeenCalledWith('No findings to comment on PR diff');
848+
expect(consoleLogSpy).toHaveBeenCalledWith('No inline comments to add; posting summary review only');
689849
});
690850
});
691-
});
851+
});

0 commit comments

Comments
 (0)