Skip to content

Commit 2986a4c

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 c076de5 commit 2986a4c

File tree

13 files changed

+2187
-1495
lines changed

13 files changed

+2187
-1495
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: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
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+
isResolved: false,
42+
isOutdated: false,
43+
path: 'src/foo.ts',
44+
line: 10,
45+
startLine: null,
46+
comments: {
47+
nodes: [{ author: { login: 'coderabbitai' }, body: '🟠 Major: Fix this' }],
48+
},
49+
}],
50+
},
51+
});
52+
const comments = extractBotComments(pr);
53+
expect(comments).toHaveLength(1);
54+
expect(comments[0].path).toBe('src/foo.ts');
55+
expect(comments[0].author).toBe('coderabbitai');
56+
});
57+
58+
it('skips resolved threads', () => {
59+
const pr = makePrNode({
60+
reviewThreads: {
61+
nodes: [{
62+
isResolved: true,
63+
isOutdated: false,
64+
path: 'src/foo.ts',
65+
line: 10,
66+
startLine: null,
67+
comments: {
68+
nodes: [{ author: { login: 'coderabbitai' }, body: 'Fix this' }],
69+
},
70+
}],
71+
},
72+
});
73+
expect(extractBotComments(pr)).toHaveLength(0);
74+
});
75+
76+
it('skips outdated threads', () => {
77+
const pr = makePrNode({
78+
reviewThreads: {
79+
nodes: [{
80+
isResolved: false,
81+
isOutdated: true,
82+
path: 'src/foo.ts',
83+
line: 10,
84+
startLine: null,
85+
comments: {
86+
nodes: [{ author: { login: 'coderabbitai' }, body: 'Fix this' }],
87+
},
88+
}],
89+
},
90+
});
91+
expect(extractBotComments(pr)).toHaveLength(0);
92+
});
93+
94+
it('skips non-bot authors', () => {
95+
const pr = makePrNode({
96+
reviewThreads: {
97+
nodes: [{
98+
isResolved: false,
99+
isOutdated: false,
100+
path: 'src/foo.ts',
101+
line: 10,
102+
startLine: null,
103+
comments: {
104+
nodes: [{ author: { login: 'humanuser' }, body: 'Fix this' }],
105+
},
106+
}],
107+
},
108+
});
109+
expect(extractBotComments(pr)).toHaveLength(0);
110+
});
111+
112+
it('recognizes all known bot logins', () => {
113+
const bots = ['coderabbitai', 'github-actions', 'dependabot', 'renovate'];
114+
for (const bot of bots) {
115+
const pr = makePrNode({
116+
reviewThreads: {
117+
nodes: [{
118+
isResolved: false,
119+
isOutdated: false,
120+
path: 'src/test.ts',
121+
line: 1,
122+
startLine: null,
123+
comments: {
124+
nodes: [{ author: { login: bot }, body: 'Fix' }],
125+
},
126+
}],
127+
},
128+
});
129+
expect(extractBotComments(pr)).toHaveLength(1);
130+
}
131+
});
132+
133+
it('returns empty for PR with no review threads', () => {
134+
const pr = makePrNode({ reviewThreads: undefined });
135+
expect(extractBotComments(pr)).toEqual([]);
136+
});
137+
});
138+
139+
// ── detectIssues ─────────────────────────────────────────────────────────────
140+
141+
describe('detectIssues', () => {
142+
it('detects bot-review-major for actionable severity', () => {
143+
const pr = makePrNode({
144+
reviewThreads: {
145+
nodes: [{
146+
isResolved: false,
147+
isOutdated: false,
148+
path: 'src/foo.ts',
149+
line: 10,
150+
startLine: null,
151+
comments: {
152+
nodes: [{ author: { login: 'coderabbitai' }, body: '🟠 Major: Important issue' }],
153+
},
154+
}],
155+
},
156+
});
157+
const result = detectIssues(pr, 0);
158+
expect(result.issues).toContain('bot-review-major');
159+
expect(result.issues).not.toContain('bot-review-nitpick');
160+
});
161+
162+
it('detects bot-review-nitpick for non-actionable severity', () => {
163+
const pr = makePrNode({
164+
reviewThreads: {
165+
nodes: [{
166+
isResolved: false,
167+
isOutdated: false,
168+
path: 'src/foo.ts',
169+
line: 10,
170+
startLine: null,
171+
comments: {
172+
nodes: [{ author: { login: 'coderabbitai' }, body: '🧹 Nitpick: Minor style' }],
173+
},
174+
}],
175+
},
176+
});
177+
const result = detectIssues(pr, 0);
178+
expect(result.issues).toContain('bot-review-nitpick');
179+
expect(result.issues).not.toContain('bot-review-major');
180+
});
181+
182+
it('detects stale PRs', () => {
183+
const pr = makePrNode({
184+
updatedAt: '2020-01-01T00:00:00Z',
185+
});
186+
const result = detectIssues(pr, Date.now());
187+
expect(result.issues).toContain('stale');
188+
});
189+
190+
it('does not flag recent PRs as stale', () => {
191+
const pr = makePrNode({
192+
updatedAt: new Date().toISOString(),
193+
});
194+
const result = detectIssues(pr, Date.now() - 1000);
195+
expect(result.issues).not.toContain('stale');
196+
});
197+
198+
it('handles null PR body gracefully', () => {
199+
const pr = makePrNode({ body: null });
200+
const result = detectIssues(pr, 0);
201+
expect(result.issues).toContain('missing-testplan');
202+
expect(result.issues).toContain('missing-issue-ref');
203+
});
204+
205+
it('handles null statusCheckRollup', () => {
206+
const pr = makePrNode({
207+
commits: {
208+
nodes: [{
209+
commit: { statusCheckRollup: null },
210+
}],
211+
},
212+
});
213+
const result = detectIssues(pr, 0);
214+
expect(result.issues).not.toContain('ci-failure');
215+
});
216+
217+
it('detects ERROR state in StatusContext nodes', () => {
218+
const pr = makePrNode({
219+
commits: {
220+
nodes: [{
221+
commit: {
222+
statusCheckRollup: {
223+
contexts: { nodes: [{ state: 'ERROR' }] },
224+
},
225+
},
226+
}],
227+
},
228+
});
229+
const result = detectIssues(pr, 0);
230+
expect(result.issues).toContain('ci-failure');
231+
});
232+
233+
it('accepts Fixes and Resolves as issue refs', () => {
234+
for (const keyword of ['Closes #1', 'Fixes #2', 'Resolves #3']) {
235+
const pr = makePrNode({ body: `## Test plan\n\n${keyword}` });
236+
const result = detectIssues(pr, 0);
237+
expect(result.issues).not.toContain('missing-issue-ref');
238+
}
239+
});
240+
});

0 commit comments

Comments
 (0)