fix(review-pr): parse args from JSON string in workflow script - #53
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 55 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIn ChangesArgs Parsing and Version Bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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. ✨ Finishing Touches🧪 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.
Pull request overview
This PR fixes a runtime bug in the pr-review-toolkit plugin's review-pr workflow script. The Workflow runtime delivers the args payload as a JSON string rather than a parsed object, which caused config.owner, config.repo, and other properties to resolve to undefined, leaking literal "undefined" strings into the agent prompts. The fix parses args when it arrives as a string while preserving the existing object/fallback behavior, and bumps the plugin's patch version.
Changes:
- Parse
argsviaJSON.parsewhen it is a string, otherwise fall back toargs || {}. - Bump
pr-review-toolkitplugin version1.4.0→1.4.1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pr-review-toolkit/skills/review-pr/review-pr.js |
Defensively parses args from a JSON string so config.* properties resolve correctly. |
pr-review-toolkit/.claude-plugin/plugin.json |
Patch version bump reflecting the fix. |
I verified the version bump is self-consistent: marketplace.json does not pin a version for pr-review-toolkit, so no other reference requires updating (the 1.4.0 in git/.claude-plugin/plugin.json belongs to the unrelated git plugin). The parsing change correctly handles both the string and object input cases, and the repository has no unit tests (CI only runs skillsaw, markdownlint, and claude plugin validate), so no test-coverage gap applies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pr-review-toolkit/skills/review-pr/review-pr.js`:
- Line 131: The JSON.parse call in the config constant assignment lacks error
handling, which causes the workflow to abort when args contains malformed JSON.
Wrap the JSON.parse(args) operation in a try-catch block to gracefully handle
parsing errors, falling back to an empty object or default configuration when
the JSON parsing fails instead of propagating the exception.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b423e7e4-daef-4370-ba3d-411d51213a9a
📒 Files selected for processing (2)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/skills/review-pr/review-pr.js
| } | ||
|
|
||
| const config = args || {} | ||
| const config = typeof args === 'string' ? JSON.parse(args) : (args || {}) |
There was a problem hiding this comment.
Guard JSON.parse to prevent workflow abort on malformed string args (Line 131).
JSON.parse(args) is unhandled here; malformed JSON will throw and terminate the workflow instead of falling back, which reintroduces brittle behavior in this path.
Proposed fix
-const config = typeof args === 'string' ? JSON.parse(args) : (args || {})
+const config = (() => {
+ if (typeof args !== "string") return args || {}
+ try {
+ const parsed = JSON.parse(args)
+ return parsed || {}
+ } catch {
+ return {}
+ }
+})()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config = typeof args === 'string' ? JSON.parse(args) : (args || {}) | |
| const config = (() => { | |
| if (typeof args !== "string") return args || {} | |
| try { | |
| const parsed = JSON.parse(args) | |
| return parsed || {} | |
| } catch { | |
| return {} | |
| } | |
| })() |
🤖 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 `@pr-review-toolkit/skills/review-pr/review-pr.js` at line 131, The JSON.parse
call in the config constant assignment lacks error handling, which causes the
workflow to abort when args contains malformed JSON. Wrap the JSON.parse(args)
operation in a try-catch block to gracefully handle parsing errors, falling back
to an empty object or default configuration when the JSON parsing fails instead
of propagating the exception.
The Workflow runtime delivers args as a JSON string, not a parsed object, causing config properties to be undefined. Assisted-by: Claude:claude-opus-4-6
684787c to
12475e8
Compare
Summary
argsas a JSON string, not a parsed object. This causedconfig.owner,config.repo, etc. to all beundefined, producing literal"undefined"strings in downstream agent prompts.argsfrom JSON when it arrives as a string, falling back to the raw value or{}.1.4.0→1.4.1(patch).Test plan
review-prworkflow with a JSONargsobject and confirmconfig.*properties resolve correctlyclaude plugin validate ./pr-review-toolkit— should passSummary by CodeRabbit
Chores
Improvements