Skip to content

Commit c3c2f1e

Browse files
authored
docs: add /review skill for automated PR review sweeps (#474)
* fix: prevent findDbPath from escaping git worktree boundary findDbPath() walks up from cwd looking for .codegraph/graph.db. In a git worktree (e.g. .claude/worktrees/agent-xxx/), this crosses the worktree boundary and finds the main repo's database instead. Add findRepoRoot() using `git rev-parse --show-toplevel` (which returns the correct root for both repos and worktrees) and use it as a ceiling in findDbPath(). The walk-up now stops at the git boundary, so each worktree resolves to its own database. * fix: address review — real git ceiling test, debug log, robust non-git test - Ceiling test now uses `git init` to create a real git repo boundary, and verifies the outer DB is NOT found (without the ceiling fix it would be). Added separate test for finding DB within the boundary. - Non-git test uses a fresh mkdtemp dir instead of os.tmpdir(). - Added debug log when ceiling stops the walk-up. - Removed unused `origFindRepoRoot` variable. * test: strengthen findRepoRoot and ceiling tests per review feedback - Mock execFileSync via vi.mock to verify caching calls exactly once and cache bypass calls twice (not just comparing return values) - Mock execFileSync to throw in "not in git repo" test so the null path is always exercised regardless of host environment - Rename "does not use cache" test to "bypasses cache" with spy assertion * Revert "test: strengthen findRepoRoot and ceiling tests per review feedback" This reverts commit 83efde5. * Reapply "test: strengthen findRepoRoot and ceiling tests per review feedback" This reverts commit ccb8bd5. * fix: address Greptile review — flaky non-git test and misleading import name - Mock execFileSync to throw in "falls back gracefully when not in a git repo" test, preventing flakiness when os.tmpdir() is inside a git repo - Rename realExecFileSync → execFileSyncForSetup with comment explaining it resolves to the spy due to vi.mock hoisting * fix: resolve symlinks in findDbPath to fix ceiling check on macOS On macOS, os.tmpdir() returns /var/folders/... but git rev-parse returns /private/var/folders/... (resolved symlink). The ceiling comparison failed because the paths didn't match. Use fs.realpathSync on cwd to normalize symlinks before comparing against the ceiling. Impact: 1 functions changed, 1 affected * style: format connection.js try/catch block Impact: 1 functions changed, 1 affected * test: harden findDbPath fallback test to mock execFileSync The 'returns default path when no DB found' test didn't control the git ceiling — if tmpDir was inside a git repo, findRepoRoot() would return a non-null ceiling and the default path would differ from emptyDir. Mock execFileSync to throw so the cwd fallback is always exercised. * fix: normalize findRepoRoot paths with realpathSync for cross-platform ceiling On macOS, os.tmpdir() returns /var/... but git resolves symlinks to /private/var/..., and on Windows, 8.3 short names (RUNNER~1) differ from long names (runneradmin). findDbPath normalizes dir via fs.realpathSync but findRepoRoot only used path.resolve, causing the ceiling comparison to fail — the walk crossed the worktree boundary. Fix findRepoRoot to use fs.realpathSync on git output, and resolve test paths after directory creation so assertions match. Impact: 1 functions changed, 77 affected * fix: use stat-based path comparison for ceiling check on Windows git rev-parse resolves 8.3 short names (RUNNER~1 → runneradmin) but fs.realpathSync may not, causing the string comparison to fail and the walk to escape the git ceiling boundary. Replace string equality with isSameDirectory() that falls back to dev+ino comparison when paths differ textually but refer to the same directory. Also fix the test assertion to use findRepoRoot() for the expected ceiling path instead of the test's worktreeRoot, since git may resolve paths differently than realpathSync. Impact: 2 functions changed, 137 affected * fix: remove test-only _resetRepoRootCache from public barrel export * fix: tighten test assertions and key repo root cache on cwd Impact: 2 functions changed, 2 affected * fix: normalize ceiling path in test to handle Windows 8.3 short names * fix: normalize ceiling path with realpathSync to handle Windows 8.3 short names Impact: 1 functions changed, 1 affected * fix: normalize Windows 8.3 short paths in ceiling boundary test On Windows CI, fs.realpathSync(process.cwd()) and git rev-parse --show-toplevel can resolve 8.3 short names (RUNNER~1) differently than long names (runneradmin). Apply realpathSync to both sides of the assertion so the comparison is path-equivalent. * docs: add Titan Paradigm Claude Code skills for autonomous codebase cleanup Five skills implementing the full Titan Paradigm pipeline: - /titan-recon: graph build, embeddings, health baseline, domain mapping - /titan-gauntlet: 4-pillar audit (17 rules) with batch processing and resume - /titan-sync: dependency cluster analysis and ordered execution planning - /titan-gate: staged change validation with auto-rollback on failure - /titan-reset: escape hatch to clean all artifacts and snapshots Skills use artifact-passing between phases to manage context window limits. Published as docs/examples for users to copy, with live copies in .claude/skills/. Updated titan-paradigm.md with skill references and new backlog items. * fix: remove duplicate declarations in connection.js from merge * docs: add /review skill for automated PR review sweeps Skill checks all open PRs, resolves conflicts (via merge, never rebase), fixes CI failures, addresses all reviewer comments (Claude + Greptile), replies to each comment, and re-triggers reviewers in a loop until clean. * Revert "docs: add /review skill for automated PR review sweeps" This reverts commit cc6dafd. * docs: add /review skill for automated PR review sweeps Skill checks all open PRs, resolves conflicts (via merge, never rebase), fixes CI failures, addresses all reviewer comments (Claude + Greptile), replies to each comment, and re-triggers reviewers in a loop until clean. * fix: resolve Windows CI test failure and doc inaccuracies Replace realpathSync comparison in db.test.js ceiling test with existence-based assertions to avoid Windows 8.3 short name mismatch (RUNNER~1 vs runneradmin). Fix artifact count (5 -> 6 files) in skills README and rule count (31 -> 17 rules) in titan-paradigm.md. * fix: address Greptile review feedback on review skill Add /worktree isolation step at the start of the workflow, consistent with the CLAUDE.md parallel-session rules. Update step 2f to group commits by concern rather than batching all fixes into a single commit. * fix: use gh pr checkout for fork-compatible PR switching * fix: add iteration cap to review retry loop * fix: add clean-tree guard before PR checkout in review skill * fix: allow force-push for commitlint failures in review skill rules * fix: add scope creep detection rule to review skill
1 parent ee2bb45 commit c3c2f1e

14 files changed

Lines changed: 3003 additions & 4 deletions

File tree

.claude/skills/review/SKILL.md

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
---
2+
name: review
3+
description: Check all open PRs, resolve conflicts, update branches, address Claude and Greptile review concerns, fix CI failures, and retrigger reviewers until clean
4+
allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Agent
5+
---
6+
7+
# PR Review Sweep
8+
9+
You are performing a full review sweep across all open PRs in this repository. Your goal is to bring every PR to a clean, mergeable state: no conflicts, CI passing, all reviewer comments addressed, and reviewers re-triggered until satisfied.
10+
11+
---
12+
13+
## Step 0: Worktree Isolation
14+
15+
Before doing anything else, run `/worktree` to get an isolated copy of the repo. CLAUDE.md mandates that every session starts with `/worktree` to prevent cross-session interference. All subsequent steps run inside the worktree.
16+
17+
---
18+
19+
## Step 1: Discover Open PRs
20+
21+
```bash
22+
gh pr list --repo optave/codegraph --state open --json number,title,headRefName,baseRefName,mergeable,statusCheckRollup,reviewDecision --limit 50
23+
```
24+
25+
Record each PR's number, branch, base, merge status, and CI state.
26+
27+
---
28+
29+
## Step 2: Process Each PR
30+
31+
For **each** open PR, perform the following steps in order. Process PRs one at a time to avoid cross-contamination.
32+
33+
### 2a. Switch to the PR branch
34+
35+
Ensure the working tree is clean before switching to avoid cross-PR contamination:
36+
37+
```bash
38+
if [ -n "$(git status --porcelain)" ]; then
39+
git stash push -m "pre-checkout stash"
40+
fi
41+
```
42+
43+
Then check out the PR branch:
44+
45+
```bash
46+
gh pr checkout <number>
47+
```
48+
49+
### 2b. Resolve merge conflicts
50+
51+
Check if the PR has conflicts with its base branch:
52+
53+
```bash
54+
gh pr view <number> --json mergeable --jq '.mergeable'
55+
```
56+
57+
If `CONFLICTING`:
58+
59+
1. Merge the base branch into the head branch (never rebase):
60+
```bash
61+
git merge origin/<base-branch>
62+
```
63+
2. Resolve all conflicts by reading the conflicting files, understanding both sides, and making the correct resolution.
64+
3. After resolving, stage the resolved files by name (not `git add .`), commit with: `fix: resolve merge conflicts with <base-branch>`
65+
4. Push the updated branch.
66+
67+
### 2c. Check CI status
68+
69+
```bash
70+
gh pr checks <number>
71+
```
72+
73+
If any checks are failing:
74+
75+
1. Read the failing check logs:
76+
```bash
77+
gh run view <run-id> --log-failed
78+
```
79+
2. Diagnose the failure — read the relevant source files, understand the error.
80+
3. Fix the issue in code.
81+
4. Run tests locally to verify: `npm test`
82+
5. Run lint locally: `npm run lint`
83+
6. Commit the fix with a descriptive message: `fix: <what was broken and why>`
84+
7. Push and wait for CI to re-run. Check again:
85+
```bash
86+
gh pr checks <number>
87+
```
88+
8. Repeat until CI is green.
89+
90+
### 2d. Gather all review comments
91+
92+
Fetch **all** review comments from both Claude and Greptile:
93+
94+
```bash
95+
# PR review comments (inline code comments)
96+
gh api repos/optave/codegraph/pulls/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, path: .path, line: .line, created_at: .created_at}'
97+
98+
# PR reviews (top-level review bodies)
99+
gh api repos/optave/codegraph/pulls/<number>/reviews --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, state: .state}'
100+
101+
# Issue-style comments (includes @greptileai trigger responses)
102+
gh api repos/optave/codegraph/issues/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, created_at: .created_at}'
103+
```
104+
105+
### 2e. Address every comment
106+
107+
For **each** review comment — including minor suggestions, nits, style feedback, and optional improvements:
108+
109+
1. **Read the comment carefully.** Understand what the reviewer is asking for.
110+
2. **Read the relevant code** at the file and line referenced.
111+
3. **Make the change.** Even if the comment is marked as "nit" or "suggestion" or "minor" — address it. The goal is zero outstanding comments.
112+
4. **If you disagree** with a suggestion (e.g., it would introduce a bug or contradicts project conventions), do NOT silently ignore it. Reply to the comment explaining why you chose a different approach.
113+
5. **Reply to each comment** explaining what you did:
114+
```bash
115+
gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \
116+
-f body="Fixed — <brief description of what was changed>"
117+
```
118+
For issue-style comments, reply on the issue:
119+
```bash
120+
gh api repos/optave/codegraph/issues/<number>/comments \
121+
-f body="Addressed: <summary of changes made>"
122+
```
123+
124+
### 2f. Commit and push fixes
125+
126+
After addressing all comments for a PR:
127+
128+
1. Stage only the files you changed.
129+
2. Group changes by concern — each logically distinct fix gets its own commit (e.g., one commit for a missing validation, another for a naming change). Do not lump all feedback into a single commit.
130+
3. Use descriptive messages per commit: `fix: <what this specific change does> (#<number>)`
131+
4. Push to the PR branch.
132+
133+
### 2g. Re-trigger reviewers
134+
135+
**Greptile:** Always re-trigger after pushing fixes. Post a comment:
136+
137+
```bash
138+
gh api repos/optave/codegraph/issues/<number>/comments \
139+
-f body="@greptileai"
140+
```
141+
142+
**Claude (claude-code-review / claude bot):** Only re-trigger if you addressed something Claude specifically suggested. If you did:
143+
144+
```bash
145+
gh api repos/optave/codegraph/issues/<number>/comments \
146+
-f body="@claude"
147+
```
148+
149+
If all changes were only in response to Greptile feedback, do NOT re-trigger Claude.
150+
151+
### 2h. Wait and re-check
152+
153+
After re-triggering:
154+
155+
1. Wait for the new reviews to come in (check after a reasonable interval).
156+
2. Fetch new comments again (repeat Step 2d).
157+
3. If there are **new** comments from Greptile or Claude, go back to Step 2e and address them.
158+
4. **Repeat this loop for a maximum of 3 rounds.** If after 3 rounds there are still actionable comments, mark the PR as "needs human review" in the summary table and move to the next PR.
159+
5. Verify CI is still green after all changes.
160+
161+
---
162+
163+
## Step 3: Summary
164+
165+
After processing all PRs, output a summary table:
166+
167+
```
168+
| PR | Branch | Conflicts | CI | Comments Addressed | Reviewers Re-triggered | Status |
169+
|----|--------|-----------|----|--------------------|----------------------|--------|
170+
| #N | branch | resolved/none | green/red | N comments | greptile, claude | ready/needs-work |
171+
```
172+
173+
---
174+
175+
## Rules
176+
177+
- **Never rebase.** Always `git merge <base>` to resolve conflicts.
178+
- **Never force-push** unless fixing a commit message that fails commitlint. Amend + force-push is the only way to fix a pushed commit title (messages are part of the SHA). This is safe on feature branches. For all other problems, fix with a new commit.
179+
- **Address ALL comments**, even minor/nit/optional ones. Leave zero unaddressed.
180+
- **Always reply to comments** explaining what was done. Don't just fix silently.
181+
- **Always re-trigger Greptile** after pushing fixes — it must confirm satisfaction.
182+
- **Only re-trigger Claude** if you addressed Claude's feedback specifically.
183+
- **No co-author lines** in commit messages.
184+
- **No Claude Code references** in commit messages or comments.
185+
- **Run tests and lint locally** before pushing any fix.
186+
- **One concern per commit** — don't lump conflict resolution with code fixes.
187+
- **Flag scope creep.** If a PR's diff contains files unrelated to its stated purpose (e.g., a docs PR carrying `src/` or test changes from a merged feature branch), flag it immediately. Split the unrelated changes into a separate branch and PR. Do not proceed with review until the PR is scoped correctly — scope creep is not acceptable.
188+
- If a PR is fundamentally broken beyond what review feedback can fix, note it in the summary and skip to the next PR.

0 commit comments

Comments
 (0)