Repository navigation
Conversation
Assisted-by: LLM
Assisted-by: LLM
Add the hooks module scaffold for the review-pr mod: hooks.json and a register entry, the shared run-state types (declared once in the self-contained types/index.d.ts contract and re-exported from hooks/lib/types.ts), the run state atom, and the lens roster pinned to the workflow REVIEWERS. Bump the plugin to 3.0.0 and point its manifest at the types contract. Exclude the mod's hooks.json from skillsaw's claude-hooks-valid rule, which expects a command-hooks file. Assisted-by: LLM
Assisted-by: LLM
The host scan follows `$` only into functions declared in the same file as the hook, so a shared lib/state.ts cannot be consumed. Drop it and declare the run atom with local getRun/setRun in register.ts. Add hooks/lib/io.ts with the injected-I/O types that pure lib code will take instead of `$`, and ignore the .claude-plugin/types folder the engine writes beside a mod. Assisted-by: LLM
Register submit_findings and submit_followup so review lens agents deposit results into the mod's run state. Deposits are validated against the run nonce and selected lenses by pure applyDeposit, which also answers a missing run without throwing. Assisted-by: LLM
During an active review run, a subagent's Bash call is auto-allowed when Claude Code would otherwise ask and the command is read-only git (optionally piped through head or tail). The guard parses the command into the words the shell would produce and refuses anything it cannot fully account for, including git --output, a lone background &, quote or escape tricks, expansions and globs. Assisted-by: LLM
Judge the whole Bash input, not only the command: allow only when the input keys are command, description, timeout or run_in_background, and leave a rule-backed ask, a deny and an allow to Claude Code. The pure decision now lives in shouldAutoAllow. Refuse reads outside the checkout (absolute, ~ and .. paths, --no-index, --contents, -O/--orderfile, --ignore-revs-file, blame -S), unquoted ^ (a zsh glob), --show-signature, %G formats, --help and -h. head and tail need a dashed count. Non-subagent Bash calls skip the run lookup. Assisted-by: LLM
git parse-options accepts bundled short options, so `git blame -wSl f` slipped past the `-S` check and, with a committed symlink as the revs file, printed the target's lines in an error. Match -S anywhere in a blame short-option cluster, as -O already is. Assisted-by: LLM
Ask a model which review lenses the diff needs, through the injected Io, and fall back to every lens on any failure. parseSelection shape-checks the reply so non-object JSON is rejected. Assisted-by: LLM
Add pure git and GitHub collection helpers over an injected Io: checkout environment, base range pinning, PR resolution, PR metadata, review threads and the reviewer's reviews. Thread parsing follows the observed get_review_comments shape. Assisted-by: LLM
Take the head filter's PRs as candidates whatever their SHA, so a stale checkout reaches the head-SHA check instead of a misleading "no PR" error. Scan every page of open PRs so duplicates yield "Several", and say so when the 2000-PR cap hides the match. Strip userinfo through the last @ before the first /. Report threads cut short of their total_count as an incomplete read. Assisted-by: LLM
Port the workflow's "have I reviewed this PR before?" detection into a pure detectFollowUp, and compute what changed since the reviewed commit with git (computeDelta over an injected Io). parseDeltaHunks turns any-context git diff output into head-side line ranges and is reusable by posting. Assisted-by: LLM
The host cuts process stdout at 4 MiB, so a cut diff came back as available with a partial file list, and files missing from it read as unchanged. ProcResult now carries truncated, and computeDelta reports the delta unavailable when the diff was cut. Also share one SHA_RE, drop the stale mergeBase comment, reuse the Delta file type, and state what the delta still reads as unchanged. Assisted-by: LLM
prepare_review runs the skill's preflight as code: resolve the PR, verify the checkout is its open head in its base repository, pin the review range, collect threads and reviews, detect a follow-up and its delta, and select lenses. It refuses while a launched run is in progress and stores the prepared run. The Workflow launch hook gates review-pr-analysis on a fresh preparation for the same PR and HEAD, claims it atomically with a run nonce before the workflow starts, records the taskId, and gives the nonce back when the launch does not start. It rewrites args to the full launch payload. PrMeta gains baseRepo from base.repo.full_name for the origin check. Assisted-by: LLM
The mod now collects the PR, selects lenses, detects follow-up reviews,
synthesizes and owns the board. The workflow keeps only the Analyze
phase: one read-only agent per lens the mod passed, plus the follow-up
verifier when there is follow-up context. Each agent reports through the
mod's submit_findings / submit_followup deposit tools and returns only
{ submitted }.
Assisted-by: LLM
The review-pr skill is now glue: call the mod's prepare_review tool, then launch the Workflow. The read-only lens agent gains the two deposit tools and a Reporting section. Drafting rules move to references/drafting.md for the review pane's draft request; the collector, selector and synthesis agents and the board and posting references are deleted, along with the skill's preflight allowlist. Assisted-by: LLM
Recognise the review workflow's completion notice for the recorded taskId on both delivery paths (queued_command attachment, idle task-notification submit), rewrite it to one line so no workflow result enters the transcript, and advance the run: a completed notice marks the run synthesizing and finishes it detached; any other status fails it. Only the first notice for a run in progress advances it. Assisted-by: LLM
Read the status inside the notice block that holds our task id and replace only that block, keeping other notices and surrounding text. A text with no wrapper is still taken whole. Guard the detached finish and failure writes on the claimed taskId, a progress phase and the synthesizing flag so they never land on a newer or cancelled run. The completed line now says the board is opening, and finish failures are logged to the debug sink. Assisted-by: LLM
Port the workflow's routing and finalizeBoard to pure functions over the run: severity/confidence routing, synthesis groups and merges, existing-thread overlap, follow-up delta demotion, F and P ids, and degradation warnings that add to the run's prepare-time warnings without repeating them or calling a partial read a failed one. Add promote, demote and too-picky board edits, and type the board's summary, follow-up and meta. Assisted-by: LLM
Group the deposited findings with one Sonnet completion using the workflow's synthesis prompt, now with the output shape spelled out since a completion has no schema. Accept the first of two answers that parses and puts every finding in exactly one group with a board section; otherwise list findings unmerged. finishRun now builds the board from the claimed run and writes it only while that run is still synthesizing. Assisted-by: LLM
A routing note says why a finding is not recommended; finalizeBoard already removes it on Recommended. Promote now does the same, so a finding demoted by too-picky or unchanged code and promoted back does not reach drafting with a stale reason. Assisted-by: LLM
The board copied a group's title, claim, note and overlap rationale from model output into string fields unchecked; it now uses each only when it is a string, so a wrong type costs no retry. synthesize returns why it failed and finishRun logs it, and logs a skipped or dropped board. The collection warnings prepare and the board share now live in one module. Assisted-by: LLM
Add the pure draft prompt builder and the set_drafts mod tool. The prompt names drafting.md by the path it is given and carries the selected items as JSON, marked untrusted. Drafts must cover every selected id exactly once, either as a draft's id or in its alsoCovers, so a finding and the follow-up item on its thread share one reply. set_drafts is accepted only while drafting or previewing, and accepting is a pure function so posting can extend it. drafting.md describes the merge and drafts an item with no reply target as a body comment. Assisted-by: LLM
set_drafts now fixes the posting plan when drafts are accepted: line anchors are checked against the PR diff (default context, pinned), and line drafts outside it or with an unreadable diff move into the review body. postReview posts exactly that plan through the GitHub MCP server: head re-check, replies, then a pending review with line comments and a submit, or one create with the event. Failures stop and name what posted; a failed line comment or submit deletes the pending review. Also: prepare_review refuses while a review is posting, and the Task 14 drafting fixes (non-retry rejections, per-item reply targets, cleaner stored drafts, drafting.md merge wording). Assisted-by: LLM
After a partial post, a reword had to cover the posted follow-up item again; merged with an unposted finding, the whole draft was left out of the plan as already posted. finishPosting now removes posted ids from run.selected, so a reword drafts only what is still to post. Assisted-by: LLM
GitHub's MCP server returns timeouts, resets and 5xx answers as error results, so only a 4xx refusal proves a write did not land. Any other failed reply or review submission now counts as possibly posted and is never resent; a submit whose pending review cannot be deleted counts as possibly submitted. Posting starts only for the plan and event the preview showed, and finishPosting changes only the run it claimed. Blocker and refusal texts say when everything has posted, and set_drafts refusals Claude cannot fix say not to call again. Assisted-by: LLM
finishPosting identified the run by its plan, so a plan replaced or dropped while posting lost the posted ids and a later post could send them again. It now matches the launch nonce, task, PR and head, and always records what posted on that run. An approval whose replies posted but which did not is no longer blocked as "everything posted", and the redraft hint shows on every failed line comment. Assisted-by: LLM
The pane (id pr-review) draws the run in every phase: progress, the board with selection, promote/demote, too picky, approve without comments and ask-Claude fields, drafting, the frozen preview of the posting plan with its tally and event choice, posting, done and failed. Draft moves the run to drafting before asking Claude; Post sends exactly the plan and event the preview showed. prepare_review and a finished board open the pane; /review-board opens it on demand. Assisted-by: LLM
R32 makes Comment the default event; Request changes is only marked suggested when a selected item is critical, so the user picks it. Assisted-by: LLM
Only /review-board gives the pane the keys; prepare and the board reopen it unfocused, and an open that waits undrawn says to run /review-board. Post, Approve without comments and the Approve event have no hotkey, so no stray letter can post (R46). Text longer than 10000 characters is drawn as several whole children; one-liners are bounded; bidi controls and line separators are cleaned; links in review text are not opened. Leaving an approval-only preview drops its Approve; replies-only previews say no review event is submitted. The completed notice and SKILL.md name /review-board. Pane tests share one engine fixture. Assisted-by: LLM
The review pane gets a Cancel review button (no hotkey) in the progress, board, drafting and preview views. It marks the run failed with "Cancelled", keeps its taskId so a late completion notice is still rewritten, clears synthesizing so a board being built is dropped, and stops the workflow with TaskStop without waiting for it. Cancel is refused while posting. session.end stops the workflow of a run in progress, resets the run and passes the end on. session.start ends a run found posting as done with "Posting was interrupted; check the PR." A post whose outcome cannot be written now ends the run the same way instead of staying in posting. Deposits are accepted only while the run is in progress. A workflow launch that returns after a cancel no longer revives the run and is stopped, and a preparation cancelled before it launched cannot launch. Assisted-by: LLM
A TaskStop that is still unanswered when session.end resolves shows the hook leaves the stop fire-and-forget within the 1.5 s session.end budget. Assisted-by: LLM
A launch denied after a cancel now tells Claude to stop instead of preparing the review again. A workflow launch whose run is gone (a session that ended, a newer run) is stopped. A completion notice for a cancelled run reads as cancelled. session.end drops the run and reads its workflow in one state operation, and a failed stop is logged. A reload that finds a run mid-synthesis starts the synthesis again. Assisted-by: LLM
Rewrite the plugin README around the review pane (prepare, workflow, board, draft, preview, post), the /review-board command, and the requirements: Claude Code 2.1.287+, personal machines only. Describe the lens agents' auto-allowed read-only git and the GitHub tools the mod calls. Add a "Mod hybrid (3.0.0)" section to DESIGN_NOTES with the platform facts and the reasoning, including the same-file `$` rule and the injected Io. Mark the 2.x sections that 3.0 moved into the mod, and drop the references to the deleted collector, selector and synthesis agents, board.md, posting.md and the scriptPath fallback. Update the repo CLAUDE.md architecture line and the top-level README plugin entry, and demote the second H1 in the implementation plan so markdownlint passes (MD025). Assisted-by: LLM
The README said the minimum Claude Code version is recorded only in the README, but the skill states it too. DESIGN_NOTES said every platform fact was probed, but some come from the mods API types. Also note that 3.0 adds the deposit tools to the specialist allowlist. Assisted-by: LLM
Scope the Ask Claude input to the items that have one, and say the drafting, reword and ask turns are ordinary Claude turns that may prompt. Narrow the possibly-posted rule to the writes it covers, name the subagent Bash scope, use the real overlap tag and cancelled notice, and reword the intro. Add 3.0 notes to the 2.x passages on dropped synthesis indexes, the follow-up delta and the anchor check, and fix the deposit validation description. Assisted-by: LLM
The pane draws review text through clean() (bidi controls dropped, other control characters turned to spaces, \r\n to \n), but posting sent the raw draft bodies, so a body with U+202E or ESC posted text the person never saw in the preview. Move clean() to hooks/lib/text.ts (pane.ts re-exports it) and apply it in cleanDrafts to each draft's body and path (a moved line draft writes its path into the review body). The stored drafts and the posting plan both come from cleanDrafts, so the plan holds exactly the text the preview draws. clean() is idempotent, so the by-value preview check (R43) is unaffected. validateDrafts now checks the body and a line draft's path after sanitising, so text that sanitises to nothing is rejected as empty. Assisted-by: LLM
- Record the launch's taskId only when the Workflow call answered
without isError and its result carries no error (a script that fails
its syntax check answers a taskId and an error); otherwise give the
nonce back, as for a missing taskId (launchedTaskId).
- A next() that rejects is answered as a deny, "The review workflow did
not start: <message>", instead of rethrowing (a hook that throws shows
a failed-hook warning, and a failing release hid the original error).
release() never rejects; a failure is logged to the debug log.
- Deny a review workflow call that also carries script, scriptPath or
resumeFromRunId ("Launch the review workflow by name only."), checked
before the preparation is claimed, so the injected payload cannot
reach another script or a replayed run (launchFieldsError).
Assisted-by: LLM
- collectThreads keeps the threads of the pages already read when a later page fails (tool error, throw, bad shape), as R19 intends; prepare then warns that review threads may be incomplete. - The posting view says what to do if posting seems stuck (cancel and prepare are refused while posting): check the PR, /clear resets. - The failed view lists what had already posted, e.g. a review cancelled after a partial post. - prepare's errors name the plugin-qualified /pr-review-toolkit:review-pr. Assisted-by: LLM
- Args that are not valid JSON (or not an object) throw the same
"needs args injected by the pr-review-toolkit mod" message as missing
args, instead of a bare JSON SyntaxError or TypeError.
- The verifier's git range is named only when the delta is available
and carries its files array, which the prompt tells it to read.
- The reporting section says to return {"submitted": false} when the
deposit cannot be accepted.
- Pin every git command template the workflow prompts name (manifest,
per-file and full patches, history, the follow-up diff and log) as
read-only in the bash guard tests, so a guard change cannot silently
send lens agents back to permission prompts.
Assisted-by: LLM
The launch hook now denies a review workflow call that also names a script, scriptPath or resumeFromRunId; list it with the gate's other denials and in the note on launching by path. Assisted-by: LLM
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hooks/register.ts,hooks/pane.tsx, pure logic inhooks/lib/). It handles preflight, PR collection (with pagination), follow-up detection and delta, lens selection, synthesis, the review board, drafting (set_drafts) and posting.review-pr.jsnow launches the lens agents (and the follow-up verifier). They deposit results through mod tools (submit_findings,submit_followup), so no results come back through the transcript. The mod injects the workflow's args at launch and rewrites its completion notice to one line./review-board):ask, and never overrides a deny or ask rule.references/board.mdandposting.md, and the skill's!preflight andscriptPathfallback. Drafting rules move toreferences/drafting.md.allowedMcpServersorallowManagedModsOnlyblock the mod.The design brief and the implementation plan, with an execution-time amendment for the same-file
$host rule, are indocs/superpowers/plans/2026-10-02-review-pr-mod.md.Test plan
cd pr-review-toolkit && claude plugin test: 367 pass, 0 failclaude plugin validate ./pr-review-toolkitandclaude plugin validate .uvx skillsaw --strict, markdownlint, TypeScript type-check (tsc against the 2.1.287 mods API types)pr-review-toolkitdisabled so--plugin-dirisn't shadowed:claude --plugin-dir ./pr-review-toolkit --worktree '<PR url>'then/pr-review-toolkit:review-prscript/scriptPath/resumeFromRunId; if any is set, the fix is inlaunchFieldsError.git logruns without a prompt, and itsrmstill prompts.drafting.mdandset_draftsprompt under the default permission mode.isRefusalagainst real server text), and COMMENT and REQUEST_CHANGES each with line comments and an empty body./clearmid-run stops the workflow./review-prin plan mode.This PR was written in part with the assistance of generative AI.