Skip to content

Commit 0c77562

Browse files
OAGrclaude
andcommitted
refactor(pr-patrol): decompose monolith, fix bugs, expand test coverage
- Split 1,560-line index.ts into 8 focused modules: types.ts, state.ts, detection.ts, scoring.ts, merge.ts, prompts.ts, execution.ts, reflection.ts - Add pr-patrol/**/*.test.ts to vitest config (tests were silently ignored) - Fix empty catch blocks violating error-handling rules - Add missing stale PR section in buildPrompt() - Fix summary comment mid-line truncation (take last ~500 chars by line) - Fix overlap comment replace() to use replaceAll() - Add staleHours CLI option (was env-var only) - Move state persistence from /tmp to ~/.cache/pr-patrol/state/ with backward-compatible migration from legacy location - Parallelize overlap detection file fetches (Promise.allSettled) - Include PR #1826 budget/timeout improvements in the decomposed modules - Add 46 new tests (scoring, prompts, detection) → 74 total Closes #1830 Closes #1831 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b0037c5 commit 0c77562

File tree

13 files changed

+2199
-1505
lines changed

13 files changed

+2199
-1505
lines changed

crux/commands/pr-patrol.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ Daemon Options:
197197
--max-turns=N Max Claude turns per fix (default: 40)
198198
--timeout=N Hard timeout in minutes per fix (default: 30)
199199
--cooldown=N Skip recently-processed PRs for N seconds (default: 1800)
200+
--stale-hours=N Hours before a PR is considered stale (default: 48)
200201
--model=MODEL Claude model (default: sonnet)
201202
--skip-perms Add --dangerously-skip-permissions to Claude CLI
202203
--verbose Detailed output

crux/pr-patrol/detection.test.ts

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { detectIssues, extractBotComments } from './detection.ts';
3+
import type { GqlPrNode } from './types.ts';
4+
5+
function makePrNode(overrides: Partial<GqlPrNode> = {}): GqlPrNode {
6+
return {
7+
number: 1,
8+
title: 'Test PR',
9+
headRefName: 'claude/test',
10+
mergeable: 'MERGEABLE',
11+
isDraft: false,
12+
createdAt: '2026-01-01T00:00:00Z',
13+
updatedAt: '2026-03-05T00:00:00Z',
14+
body: '## Summary\n\n- [x] Task done\n\n## Test plan\n\n- [x] Tests pass\n\nCloses #1',
15+
labels: { nodes: [{ name: 'ready-to-merge' }] },
16+
commits: {
17+
nodes: [
18+
{
19+
commit: {
20+
statusCheckRollup: {
21+
contexts: {
22+
nodes: [{ conclusion: 'SUCCESS' }],
23+
},
24+
},
25+
},
26+
},
27+
],
28+
},
29+
reviewThreads: { nodes: [] },
30+
...overrides,
31+
};
32+
}
33+
34+
// ── extractBotComments ───────────────────────────────────────────────────────
35+
36+
describe('extractBotComments', () => {
37+
it('extracts unresolved bot comments', () => {
38+
const pr = makePrNode({
39+
reviewThreads: {
40+
nodes: [{
41+
id: 'thread-1',
42+
isResolved: false,
43+
isOutdated: false,
44+
path: 'src/foo.ts',
45+
line: 10,
46+
startLine: null,
47+
comments: {
48+
nodes: [{ author: { login: 'coderabbitai' }, body: '🟠 Major: Fix this' }],
49+
},
50+
}],
51+
},
52+
});
53+
const comments = extractBotComments(pr);
54+
expect(comments).toHaveLength(1);
55+
expect(comments[0].path).toBe('src/foo.ts');
56+
expect(comments[0].author).toBe('coderabbitai');
57+
});
58+
59+
it('skips resolved threads', () => {
60+
const pr = makePrNode({
61+
reviewThreads: {
62+
nodes: [{
63+
id: 'thread-1',
64+
isResolved: true,
65+
isOutdated: false,
66+
path: 'src/foo.ts',
67+
line: 10,
68+
startLine: null,
69+
comments: {
70+
nodes: [{ author: { login: 'coderabbitai' }, body: 'Fix this' }],
71+
},
72+
}],
73+
},
74+
});
75+
expect(extractBotComments(pr)).toHaveLength(0);
76+
});
77+
78+
it('skips outdated threads', () => {
79+
const pr = makePrNode({
80+
reviewThreads: {
81+
nodes: [{
82+
id: 'thread-1',
83+
isResolved: false,
84+
isOutdated: true,
85+
path: 'src/foo.ts',
86+
line: 10,
87+
startLine: null,
88+
comments: {
89+
nodes: [{ author: { login: 'coderabbitai' }, body: 'Fix this' }],
90+
},
91+
}],
92+
},
93+
});
94+
expect(extractBotComments(pr)).toHaveLength(0);
95+
});
96+
97+
it('skips non-bot authors', () => {
98+
const pr = makePrNode({
99+
reviewThreads: {
100+
nodes: [{
101+
id: 'thread-1',
102+
isResolved: false,
103+
isOutdated: false,
104+
path: 'src/foo.ts',
105+
line: 10,
106+
startLine: null,
107+
comments: {
108+
nodes: [{ author: { login: 'humanuser' }, body: 'Fix this' }],
109+
},
110+
}],
111+
},
112+
});
113+
expect(extractBotComments(pr)).toHaveLength(0);
114+
});
115+
116+
it('recognizes all known bot logins', () => {
117+
const bots = ['coderabbitai', 'github-actions', 'dependabot', 'renovate'];
118+
for (const bot of bots) {
119+
const pr = makePrNode({
120+
reviewThreads: {
121+
nodes: [{
122+
id: 'thread-1',
123+
isResolved: false,
124+
isOutdated: false,
125+
path: 'src/test.ts',
126+
line: 1,
127+
startLine: null,
128+
comments: {
129+
nodes: [{ author: { login: bot }, body: 'Fix' }],
130+
},
131+
}],
132+
},
133+
});
134+
expect(extractBotComments(pr)).toHaveLength(1);
135+
}
136+
});
137+
138+
it('returns empty for PR with no review threads', () => {
139+
const pr = makePrNode({ reviewThreads: undefined });
140+
expect(extractBotComments(pr)).toEqual([]);
141+
});
142+
});
143+
144+
// ── detectIssues ─────────────────────────────────────────────────────────────
145+
146+
describe('detectIssues', () => {
147+
it('detects bot-review-major for actionable severity', () => {
148+
const pr = makePrNode({
149+
reviewThreads: {
150+
nodes: [{
151+
id: 'thread-1',
152+
isResolved: false,
153+
isOutdated: false,
154+
path: 'src/foo.ts',
155+
line: 10,
156+
startLine: null,
157+
comments: {
158+
nodes: [{ author: { login: 'coderabbitai' }, body: '🟠 Major: Important issue' }],
159+
},
160+
}],
161+
},
162+
});
163+
const result = detectIssues(pr, 0);
164+
expect(result.issues).toContain('bot-review-major');
165+
expect(result.issues).not.toContain('bot-review-nitpick');
166+
});
167+
168+
it('detects bot-review-nitpick for non-actionable severity', () => {
169+
const pr = makePrNode({
170+
reviewThreads: {
171+
nodes: [{
172+
id: 'thread-1',
173+
isResolved: false,
174+
isOutdated: false,
175+
path: 'src/foo.ts',
176+
line: 10,
177+
startLine: null,
178+
comments: {
179+
nodes: [{ author: { login: 'coderabbitai' }, body: '🧹 Nitpick: Minor style' }],
180+
},
181+
}],
182+
},
183+
});
184+
const result = detectIssues(pr, 0);
185+
expect(result.issues).toContain('bot-review-nitpick');
186+
expect(result.issues).not.toContain('bot-review-major');
187+
});
188+
189+
it('detects stale PRs', () => {
190+
const pr = makePrNode({
191+
updatedAt: '2020-01-01T00:00:00Z',
192+
});
193+
const result = detectIssues(pr, Date.now());
194+
expect(result.issues).toContain('stale');
195+
});
196+
197+
it('does not flag recent PRs as stale', () => {
198+
const pr = makePrNode({
199+
updatedAt: new Date().toISOString(),
200+
});
201+
const result = detectIssues(pr, Date.now() - 1000);
202+
expect(result.issues).not.toContain('stale');
203+
});
204+
205+
it('handles null PR body gracefully', () => {
206+
const pr = makePrNode({ body: null });
207+
const result = detectIssues(pr, 0);
208+
expect(result.issues).toContain('missing-testplan');
209+
expect(result.issues).toContain('missing-issue-ref');
210+
});
211+
212+
it('handles null statusCheckRollup', () => {
213+
const pr = makePrNode({
214+
commits: {
215+
nodes: [{
216+
commit: { statusCheckRollup: null },
217+
}],
218+
},
219+
});
220+
const result = detectIssues(pr, 0);
221+
expect(result.issues).not.toContain('ci-failure');
222+
});
223+
224+
it('detects ERROR state in StatusContext nodes', () => {
225+
const pr = makePrNode({
226+
commits: {
227+
nodes: [{
228+
commit: {
229+
statusCheckRollup: {
230+
contexts: { nodes: [{ state: 'ERROR' }] },
231+
},
232+
},
233+
}],
234+
},
235+
});
236+
const result = detectIssues(pr, 0);
237+
expect(result.issues).toContain('ci-failure');
238+
});
239+
240+
it('accepts Fixes and Resolves as issue refs', () => {
241+
for (const keyword of ['Closes #1', 'Fixes #2', 'Resolves #3']) {
242+
const pr = makePrNode({ body: `## Test plan\n\n${keyword}` });
243+
const result = detectIssues(pr, 0);
244+
expect(result.issues).not.toContain('missing-issue-ref');
245+
}
246+
});
247+
});

0 commit comments

Comments
 (0)