chore(review): inline review/fix prompts and add ws-reset workflow helper#1695
Conversation
…lper - Extract `pnpm review review` and `pnpm review fix` agent prompts into self-contained templates under `scripts/review/prompts/` so they no longer depend on Claude Code's named subagent registry (works with any LLM CLI via `--agent`). - Detect merge conflicts at sync time and inject a tailored "resolve-first" or "flag-as-blocker" block into the prompt depending on whether the run is `fix` (apply) or `review` (review-only). - Tighten push guidance in the fix prompt to point at the contributor's fork (already wired via `pushRemote` in `sync_pr`) so commits land on the actual PR head branch, not the maintainer's `origin` copy. - Add `pnpm reset` -> `scripts/ws-reset.sh` to hard-reset local `main` to `upstream/main` and refresh submodules, with a working-tree-dirty safety check (`--force` to override). - Add `.claude/commands/ws-reset.md` as the slash-command surface for the same flow inside Claude Code.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR refactors the codebase to support agent-agnostic, template-driven automation for PR review, PR fix, and issue work workflows. It introduces workspace reset utilities, adds merge conflict detection to PR sync, externalizes hardcoded prompts into Markdown templates with placeholder substitution, and provides new shortcut CLI scripts that orchestrate LLM-based review and fix tasks while preserving the ability to choose different AI agents. ChangesWorkspace reset, conflict-aware sync, and template-driven workflows
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/ws-reset.md:
- Around line 8-15: The docs currently inline destructive git commands (git
fetch..., git checkout main, git reset --hard upstream/main, git submodule
update...) instead of using the guarded helper; replace that block with a single
call to the centralized script (bash scripts/ws-reset.sh) followed by git status
so the documentation uses scripts/ws-reset.sh (the guarded reset helper) and
avoids duplicating unsafe reset logic.
In `@scripts/review/fix.sh`:
- Line 52: The subshell call uses an unquoted expansion $(printf -- '- %s\n'
$REVIEW_CONFLICT_FILES) which breaks on filenames with spaces/globs; change it
to a quoted expansion or explicit iteration: either use $(printf -- '- %s\n'
"$REVIEW_CONFLICT_FILES") if REVIEW_CONFLICT_FILES is a single whitespace-safe
value, or (safer) replace with a loop like for f in
"${REVIEW_CONFLICT_FILES[@]}"; do printf -- '- %s\n' "$f"; done so printf
receives each conflict path as a single quoted argument (update the occurrence
of $(printf -- '- %s\n' $REVIEW_CONFLICT_FILES) accordingly).
In `@scripts/review/lib.sh`:
- Around line 120-124: The current merge failure path treats any git merge error
as a conflict; update the failed-merge branch to inspect actual unmerged files
(use git diff --name-only --diff-filter=U) and only set REVIEW_HAS_CONFLICTS and
REVIEW_CONFLICT_FILES when that list is non-empty (i.e., real conflicts); if
there are no unmerged files, exit/fail fast instead of continuing. Locate the if
! git merge --no-edit main; then block and change its body to compute the
unmerged files list, conditionally set REVIEW_HAS_CONFLICTS and
REVIEW_CONFLICT_FILES when non-empty, and otherwise abort the script with a
non-zero exit.
In `@scripts/review/prompts/review.md`:
- Line 149: Replace the unlabeled fenced code block that begins with "## PR
`#__PR__` — Review posted" by adding a language identifier (e.g., change the
opening "```" to "```text") so the block is labeled and markdownlint MD040 is
satisfied; ensure the closing fence remains "```" and no other content is
altered.
In `@scripts/review/review.sh`:
- Line 52: The printf invocation is expanding REVIEW_CONFLICT_FILES unquoted
which splits and glob-expands filenames; update the call that uses printf -- '-
%s\n' to safely expand REVIEW_CONFLICT_FILES by quoting the variable (or, if
REVIEW_CONFLICT_FILES is an array, use the array expansion form) so filenames
with spaces or glob chars are preserved—alternatively iterate over lines from
REVIEW_CONFLICT_FILES with a safe read (read -r) and call printf for each entry;
locate the printf line that references REVIEW_CONFLICT_FILES and replace the
unquoted expansion with a quoted/iterated-safe expansion.
In `@scripts/ws-reset.sh`:
- Around line 36-39: The current dirty-tree check using git diff --quiet HEAD --
misses untracked files; update the conditional that references the FORCE
variable to run git status --porcelain --untracked-files=all and treat any
non-empty output as a dirty tree, i.e., capture the status output and if it is
non-empty (and FORCE != 1) print the same error message and exit; locate the
check that uses the FORCE variable and the git diff command in the script and
replace it with a git status --porcelain --untracked-files=all based detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e8267e4-2124-4f6a-baf9-2ad0af77a43a
📒 Files selected for processing (8)
.claude/commands/ws-reset.mdpackage.jsonscripts/review/fix.shscripts/review/lib.shscripts/review/prompts/fix.mdscripts/review/prompts/review.mdscripts/review/review.shscripts/ws-reset.sh
- Move `scripts/review/`, `scripts/work/`, and `scripts/ws-reset.sh` under `scripts/shortcuts/` (preserved git history via `git mv`). - Add `scripts/shortcuts/README.md` index documenting each shortcut and the shared design (prompts as Markdown templates with placeholders; `awk` substitution; agent-agnostic via `--agent`). - Update `package.json` script paths and rewire `work/start.sh` to source `lib.sh` via the new relative path. - Extract `pnpm work` agent prompt into `scripts/shortcuts/work/prompts/start.md` so it works with any LLM CLI (matches the `review` layout). - Pre-push hook: keep the original "run all checks, accumulate, fail at end" structure, but emit a friendly closing message telling the user to re-commit and push after auto-fix paths. Split the message for format/lint (auto-fixed) vs. compile/Rust/cmd-tokens (must fix by hand).
There was a problem hiding this comment.
♻️ Duplicate comments (4)
scripts/shortcuts/review/prompts/review.md (1)
149-149:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced block.
Line 149 opens a code fence without a language, which can fail markdownlint MD040.
Proposed fix
-``` +```text ## PR `#__PR__` — Review posted ...</details> ```shell #!/bin/bash set -euo pipefail nl -ba scripts/shortcuts/review/prompts/review.md | sed -n '146,153p'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/shortcuts/review/prompts/review.md` at line 149, The markdown code fence opened on the snippet in review.md lacks a language tag (```), which triggers markdownlint MD040; replace the bare fence with a tagged fence (e.g., change the opening "```" to "```text") so the fenced block is explicitly labeled — update the fenced block around the PR review template (the block that starts with "## PR `#__PR__` — Review posted") to use a language tag like "text".scripts/shortcuts/review/lib.sh (1)
120-124:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when
git mergefails without unmerged paths.Line 120 currently classifies any merge failure as a conflict. If merge fails for another reason, the workflow continues with invalid state. Check unmerged files first; if none exist, abort.
Proposed fix
if ! git merge --no-edit main; then - echo "[review] ! conflicts detected in PR #$pr, continuing." - REVIEW_HAS_CONFLICTS=1 REVIEW_CONFLICT_FILES=$(git diff --name-only --diff-filter=U | sort -u) + if [ -z "$REVIEW_CONFLICT_FILES" ]; then + fail "git merge main failed for a non-conflict reason" + return 1 + fi + echo "[review] ! conflicts detected in PR #$pr, continuing." + REVIEW_HAS_CONFLICTS=1 fi#!/bin/bash set -euo pipefail nl -ba scripts/shortcuts/review/lib.sh | sed -n '116,130p' rg -n 'git merge --no-edit main|REVIEW_CONFLICT_FILES=.*diff-filter=U|if \[ -z "\$REVIEW_CONFLICT_FILES" \]' scripts/shortcuts/review/lib.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/shortcuts/review/lib.sh` around lines 120 - 124, The current handling after the failing git merge treats any merge error as a conflict; change the logic around the git merge --no-edit main failure so you first collect unmerged paths via git diff --name-only --diff-filter=U into REVIEW_CONFLICT_FILES, and only if that list is non-empty set REVIEW_HAS_CONFLICTS=1 and continue; if REVIEW_CONFLICT_FILES is empty, fail fast (exit non-zero) and do not proceed. Update the block referencing REVIEW_HAS_CONFLICTS and REVIEW_CONFLICT_FILES to implement this conditional check and early abort behavior.scripts/shortcuts/review/fix.sh (1)
52-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRender conflict file list safely (quoted/line-wise).
Line 52 uses unquoted expansion for
REVIEW_CONFLICT_FILES, which can corrupt filenames containing whitespace or glob characters.Proposed fix
-$(printf -- '- %s\n' $REVIEW_CONFLICT_FILES) +$(printf '%s\n' "$REVIEW_CONFLICT_FILES" | sed 's/^/- /')#!/bin/bash set -euo pipefail nl -ba scripts/shortcuts/review/fix.sh | sed -n '47,54p' rg -n '\$\(printf -- '\''- %s\\n'\'' \$REVIEW_CONFLICT_FILES\)' scripts/shortcuts/review/fix.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/shortcuts/review/fix.sh` at line 52, The current command "$(printf -- '- %s\n' $REVIEW_CONFLICT_FILES)" performs unquoted word-splitting and globbing on REVIEW_CONFLICT_FILES; change it to render filenames safely by iterating line-wise and quoting each name, e.g. use: printf -- '- %s\n' "${REVIEW_CONFLICT_FILES[@]}" if REVIEW_CONFLICT_FILES is an array, or otherwise pipe the variable into a while-read loop (printf -- '%s\n' "$REVIEW_CONFLICT_FILES" | while IFS= read -r f; do printf -- '- %s\n' "$f"; done) so each filename is handled quoted and without glob expansion.scripts/shortcuts/review/review.sh (1)
52-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winQuote conflict-file rendering to preserve filenames.
Line 52 expands
REVIEW_CONFLICT_FILESunquoted, so spaces/globs in paths can be split incorrectly.Proposed fix
-$(printf -- '- %s\n' $REVIEW_CONFLICT_FILES) +$(printf '%s\n' "$REVIEW_CONFLICT_FILES" | sed 's/^/- /')#!/bin/bash set -euo pipefail nl -ba scripts/shortcuts/review/review.sh | sed -n '47,54p' rg -n '\$\(printf -- '\''- %s\\n'\'' \$REVIEW_CONFLICT_FILES\)' scripts/shortcuts/review/review.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/shortcuts/review/review.sh` at line 52, The printf invocation expands REVIEW_CONFLICT_FILES unquoted which allows word-splitting/globbing and breaks filenames with spaces; replace the single unquoted expansion of REVIEW_CONFLICT_FILES used with $(printf -- '- %s\n' $REVIEW_CONFLICT_FILES) by iterating over the lines safely (e.g., feed REVIEW_CONFLICT_FILES into a while IFS= read -r file; do printf -- '- %s\n' "$file"; done or convert REVIEW_CONFLICT_FILES into an array and printf each element) and ensure you always quote the per-file variable ("$file" or "${array[i]}") so filenames with spaces or glob characters are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/shortcuts/review/fix.sh`:
- Line 52: The current command "$(printf -- '- %s\n' $REVIEW_CONFLICT_FILES)"
performs unquoted word-splitting and globbing on REVIEW_CONFLICT_FILES; change
it to render filenames safely by iterating line-wise and quoting each name, e.g.
use: printf -- '- %s\n' "${REVIEW_CONFLICT_FILES[@]}" if REVIEW_CONFLICT_FILES
is an array, or otherwise pipe the variable into a while-read loop (printf --
'%s\n' "$REVIEW_CONFLICT_FILES" | while IFS= read -r f; do printf -- '- %s\n'
"$f"; done) so each filename is handled quoted and without glob expansion.
In `@scripts/shortcuts/review/lib.sh`:
- Around line 120-124: The current handling after the failing git merge treats
any merge error as a conflict; change the logic around the git merge --no-edit
main failure so you first collect unmerged paths via git diff --name-only
--diff-filter=U into REVIEW_CONFLICT_FILES, and only if that list is non-empty
set REVIEW_HAS_CONFLICTS=1 and continue; if REVIEW_CONFLICT_FILES is empty, fail
fast (exit non-zero) and do not proceed. Update the block referencing
REVIEW_HAS_CONFLICTS and REVIEW_CONFLICT_FILES to implement this conditional
check and early abort behavior.
In `@scripts/shortcuts/review/prompts/review.md`:
- Line 149: The markdown code fence opened on the snippet in review.md lacks a
language tag (```), which triggers markdownlint MD040; replace the bare fence
with a tagged fence (e.g., change the opening "```" to "```text") so the fenced
block is explicitly labeled — update the fenced block around the PR review
template (the block that starts with "## PR `#__PR__` — Review posted") to use a
language tag like "text".
In `@scripts/shortcuts/review/review.sh`:
- Line 52: The printf invocation expands REVIEW_CONFLICT_FILES unquoted which
allows word-splitting/globbing and breaks filenames with spaces; replace the
single unquoted expansion of REVIEW_CONFLICT_FILES used with $(printf -- '-
%s\n' $REVIEW_CONFLICT_FILES) by iterating over the lines safely (e.g., feed
REVIEW_CONFLICT_FILES into a while IFS= read -r file; do printf -- '- %s\n'
"$file"; done or convert REVIEW_CONFLICT_FILES into an array and printf each
element) and ensure you always quote the per-file variable ("$file" or
"${array[i]}") so filenames with spaces or glob characters are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04a291bb-2d40-4866-bdfa-1cea53524d44
📒 Files selected for processing (18)
.husky/pre-pushpackage.jsonscripts/shortcuts/README.mdscripts/shortcuts/review/README.mdscripts/shortcuts/review/cli.shscripts/shortcuts/review/coverage.shscripts/shortcuts/review/fix.shscripts/shortcuts/review/lib.shscripts/shortcuts/review/merge.shscripts/shortcuts/review/prompts/fix.mdscripts/shortcuts/review/prompts/review.mdscripts/shortcuts/review/review.shscripts/shortcuts/review/sync.shscripts/shortcuts/work/README.mdscripts/shortcuts/work/cli.shscripts/shortcuts/work/prompts/start.mdscripts/shortcuts/work/start.shscripts/shortcuts/ws-reset.sh
✅ Files skipped from review due to trivial changes (6)
- package.json
- scripts/shortcuts/review/README.md
- scripts/shortcuts/work/README.md
- scripts/shortcuts/README.md
- scripts/shortcuts/work/cli.sh
- .husky/pre-push
Apply six CodeRabbit suggestions on PR tinyhumansai#1695: - `.claude/commands/ws-reset.md`: delegate to the guarded `scripts/shortcuts/ws-reset.sh` helper instead of inlining destructive `git reset --hard` steps, avoiding doc drift and data-loss risk. - `scripts/shortcuts/ws-reset.sh`: replace `git diff --quiet HEAD --` with `git status --porcelain --untracked-files=all` so the dirty-tree guard catches untracked files too (matches the documented behavior). - `scripts/shortcuts/review/lib.sh`: distinguish real merge conflicts from other merge failures — only set `REVIEW_HAS_CONFLICTS=1` when there are actually unmerged files; otherwise fail fast via the `fail` helper. - `scripts/shortcuts/review/{fix,review}.sh`: quote `$REVIEW_CONFLICT_FILES` in the printf expansion (SC2086) so conflict paths with spaces or glob chars render correctly in the agent prompt. - `scripts/shortcuts/review/prompts/review.md`: add `text` language tag to the final-report fenced code block (markdownlint MD040).
graycyrus
left a comment
There was a problem hiding this comment.
Walkthrough
This PR extracts the pnpm review review/fix agent prompts into self-contained Markdown templates under scripts/shortcuts/review/prompts/, making the workflow agent-agnostic. It also adds conflict detection in sync_pr, a pnpm reset workspace-reset helper, and improves the pre-push hook error messaging. Good direction overall — the templated approach is cleaner and the conflict surfacing is a real improvement.
Change Summary
| File | Change |
|---|---|
scripts/review/ → scripts/shortcuts/review/ |
Directory rename |
scripts/work/ → scripts/shortcuts/work/ |
Directory rename |
scripts/shortcuts/review/prompts/{review,fix}.md |
New prompt templates |
scripts/shortcuts/work/prompts/start.md |
New prompt template |
scripts/shortcuts/review/{review,fix}.sh |
Rewritten to load + substitute templates |
scripts/shortcuts/review/lib.sh |
Conflict detection + export vars |
scripts/shortcuts/ws-reset.sh |
New workspace reset helper |
.claude/commands/ws-reset.md |
Slash command for ws-reset |
.husky/pre-push |
Better error messaging |
package.json |
Updated script paths + reset |
Findings
CodeRabbit already covered quoting issues in conflict-file expansion, the merge-failure handling in lib.sh, and the ws-reset command doc. I'm focusing on what it missed.
(2 issues below, posted as inline comments)
|
|
||
| prompt=$(awk -v pr="$REVIEW_PR" -v repo="$REVIEW_REPO_RESOLVED" \ | ||
| -v head_repo="$REVIEW_HEAD_REPO" -v head_branch="$REVIEW_HEAD_BRANCH" \ | ||
| -v conflict="$conflict_block" ' |
There was a problem hiding this comment.
[major] start.sh has an esc() function that escapes \ → \\ and & → \\& in awk gsub replacement strings (lines 128-130 in the new start.sh). Neither fix.sh nor review.sh do this.
This matters for conflict_block — it's multi-line markdown that could contain & in filenames or prose. Awk's gsub interprets & in the replacement as "the matched text", so an & in a conflict filename would silently corrupt the rendered prompt.
Suggested fix — add the same esc() wrapper from start.sh:
prompt=$(awk -v pr="$REVIEW_PR" -v repo="$REVIEW_REPO_RESOLVED" \
-v head_repo="$REVIEW_HEAD_REPO" -v head_branch="$REVIEW_HEAD_BRANCH" \
-v conflict="$conflict_block" '
function esc(s) {
gsub(/\\/, "\\\\", s); gsub(/&/, "\\\\&", s); return s
}
BEGIN {
pr=esc(pr); repo=esc(repo); head_repo=esc(head_repo);
head_branch=esc(head_branch); conflict=esc(conflict);
}
{
gsub(/__PR__/, pr);
...
}
' "$template")Same applies to review.sh (line 60).
| fi | ||
| echo "============================================================" | ||
| exit 1 | ||
| fi No newline at end of file |
There was a problem hiding this comment.
[minor] File still ends without a trailing newline (\ No newline at end of file). POSIX requires text files to end with a newline, and some tools (notably wc -l, cat piping) behave unexpectedly without it. Easy fix — just add a blank line after fi.
Summary
pnpm review reviewandpnpm review fixagent prompts into self-contained templates underscripts/review/prompts/so they no longer rely on Claude Code's named subagent registry — works with any LLM CLI via--agent.fix(apply) orreview(review-only).fixprompt to point at the contributor's fork (already wired viapushRemoteinsync_pr) so commits land on the actual PR head branch, not the maintainer'sorigincopy.pnpm reset→scripts/ws-reset.shto hard-reset localmaintoupstream/mainand refresh submodules, with a working-tree-dirty safety check (--forceto override)..claude/commands/ws-reset.mdas the slash-command surface for the same flow inside Claude Code.Problem
The previous
pnpm review review/pnpm review fixscripts produced a one-liner prompt that told the agent to "use the pr-reviewer / pr-manager-lite agent" — only meaningful inside Claude Code where those named subagents exist. Other CLIs (codex,gemini, etc.) couldn't drive the same workflow.Additionally, when the merged-in
mainintroduced conflicts duringsync_pr, the script only logged a warning. The downstream agent had no idea conflicts existed and would silently produce broken reviews / push half-merged trees.Lastly, no quick way to reset the workspace back to a clean
mainafter PR-review sessions.Solution
scripts/review/prompts/{review,fix}.md) with__PR__/__REPO__/__HEAD_REPO__/__HEAD_BRANCH__/__CONFLICT_BLOCK__placeholders.sync_prnow usesgit merge --no-edit mainand detects unresolved conflicts viagit diff --name-only --diff-filter=U, exportingREVIEW_HAS_CONFLICTSandREVIEW_CONFLICT_FILESfor callers.review.sh/fix.shsubstitute placeholders withawk(safer thansedfor multi-line blocks) and inject context-appropriate conflict instructions.scripts/ws-reset.shbails out on a dirty tree unless--forceis passed, thengit fetch upstream,git checkout main,git reset --hard upstream/main,git submodule update --init --recursive.Submission Checklist
pnpm review review/fixandpnpm reset.gh/ local git.Impact
pnpm resetis destructive onmainbut guarded by working-tree-dirty check;--forceopt-in is required to override.pnpm review review/pnpm review fixinvocations work identically; the agent now receives a richer, agent-agnostic prompt.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
Validation Blocked
Behavior Changes
pnpm review review/fixnow produces self-contained prompts (agent-agnostic) and surfaces merge conflicts to the agent.pnpm resetis now available; review-fix runs push to the contributor's fork (updating the actual PR) when run on apr/<N>branch synced viasync_pr.Parity Contract
--agent claudecontinues to work.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Release Notes
New Features
pnpm resetcommand to hard-reset workspace to upstream main and refresh submodules.Improvements
Documentation