fix(pr-review-toolkit): launch review workflow by plugin-workflow name - #97
Conversation
Claude Code 2.1.251 validates Workflow scriptPath against the session's readable set (working directory, added directories) before opening the file, so the skill's plugin-cache scriptPath is now rejected in normal sessions. Register review-pr.js as a plugin workflow via the manifest `workflows` key and launch it as `pr-review-toolkit:review-pr`, which the CLI loads as installed-plugin content without involving the model's read scope. Older CLIs without plugin workflows predate the scriptPath restriction, so the skill falls back to the original scriptPath launch there. The workflow script itself is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014c2jqqgQhooKnviBPN8tjY
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe plugin registers ChangesReview workflow registration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR changes review workflow launch and compatibility handling; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ReviewSkill
participant ClaudeCode
participant ReviewScript
ReviewSkill->>ClaudeCode: Invoke pr-review-toolkit:review-pr-analysis with args
ClaudeCode->>ReviewScript: Load registered plugin workflow
ClaudeCode-->>ReviewSkill: Return workflow result
ClaudeCode-->>ReviewSkill: Report workflow name not found
ReviewSkill->>ClaudeCode: Retry with scriptPath and args
ClaudeCode->>ReviewScript: Execute review-pr.js
ClaudeCode-->>ReviewSkill: Return fallback result
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🟢 Approval recommended
The manifest, skill instructions, documentation, and version bump are consistent and complete.
Pull request overview
Registers the review script as a named plugin workflow to remain compatible with Claude Code’s hardened path validation.
Changes:
- Adds named workflow registration and bumps the plugin version.
- Uses name-based invocation with a legacy fallback.
- Documents the workflow and compatibility behavior.
File summaries
| File | Description |
|---|---|
pr-review-toolkit/.claude-plugin/plugin.json |
Registers the workflow and bumps version. |
pr-review-toolkit/skills/review-pr/SKILL.md |
Updates launch and fallback instructions. |
pr-review-toolkit/README.md |
Documents named invocation and compatibility. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…kill Plugin workflows register as <plugin>:<meta.name> and surface as slash commands, so a workflow named review-pr claimed the same qualified name as the review-pr skill and won slash dispatch: bare invocations skipped the skill's setup (PR resolution, head verification, merge-base pinning) and hit the workflow's arg guards. Rename the workflow to review-pr-analysis, update the skill's name-mode launch to match, and mark the workflow description as internal so its command wrapper steers users back to /pr-review-toolkit:review-pr. Verified in a fresh install: the workflow registry lists only the new name, and slash dispatch of /pr-review-toolkit:review-pr reaches the skill's flow again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014c2jqqgQhooKnviBPN8tjY
There was a problem hiding this comment.
🟡 Changes recommended
The PR description advertises and claims verification of a workflow name different from the implemented name.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
Problem
Claude Code 2.1.251 hardened the Workflow tool:
scriptPathis now validated against the session's readable set (working directory,/add-dired directories, session-internal paths, explicitRead(...)allow rules) before the file is opened. Thereview-prskill launched its analysis workflow viascriptPath: ${CLAUDE_SKILL_DIR}/review-pr.js, which expands to an absolute plugin-cache path — outside that set in any normal session, worktree or not. The launch now fails with:Symlink/hardlink indirection is defeated by a realpath re-check in the same hardening, and passing inline
scriptalongside an out-of-scopescriptPathis also rejected, so the launch mechanism itself had to change.Fix
Register
review-pr.jsas a plugin workflow and launch it by name:plugin.json: add"workflows": ["./skills/review-pr/review-pr.js"](a manifest key recognized by Claude Code's plugin loader; validated withclaude plugin validate). The script registers aspr-review-toolkit:review-pr-analysis, and name-mode resolution loads its content through the CLI itself as installed-plugin content — the model's read scope is never involved, so it works under worktree isolation and every permission mode. Version bumped 2.0.1 → 2.1.0 (minor: new plugin component, backward-compatible behavior change).review-pr.js: the workflow'smeta.nameisreview-pr-analysis, deliberately distinct from the skill's name. Registered plugin workflows surface as slash commands under<plugin>:<workflow-name>, so a workflow namedreview-prwould claim the same qualified name as the skill and shadow its/pr-review-toolkit:review-prentry — dispatching bare workflow invocations (which fail on the arg guards) instead of the skill that computes those args. The description marks the workflow as internal and points to the skill. Nothing else in the script changed.SKILL.md: the launch step invokes Workflow withname: pr-review-toolkit:review-pr-analysisand the sameargs. If the name is not found (older Claude Code without plugin workflows — versions that also predate the scriptPath restriction), it falls back to the original${CLAUDE_SKILL_DIR}/review-pr.jsscriptPath launch; if both fail, it stops with an honest error.README.md: workflow section and control-flow diagram updated to name-mode, plus a compatibility note covering the 2.1.251 hardening and the name-collision rationale.The file stays in the skill directory (declared via the manifest key rather than moved to a
workflows/dir) so the fallback path remains valid;${CLAUDE_PLUGIN_ROOT}is not substituted in SKILL.md content, only${CLAUDE_SKILL_DIR}is.Verification
claude plugin validate .andclaude plugin validate ./pr-review-toolkitpass with the new manifest keynpx markdownlint-cli2— 0 issues;uvx skillsaw --strict— 0 errors/warnings, grade Apr-review-toolkit:review-pr-analysisas an available named workflow (and no longer lists a collidingpr-review-toolkit:review-prworkflow) — confirming the script loads and itsmetaparses through the plugin-workflow loader/pr-review-toolkit:review-prreaches the skill (runs its preflight and constraints), not the workflow's auto-generated command wrapper/pr-review-toolkit:review-pron a PR after installing 2.1.0🤖 Generated with Claude Code
https://claude.ai/code/session_014c2jqqgQhooKnviBPN8tjY
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores