Add patch-only update mode - #28
Conversation
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdded a ChangesPatch-only feature
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (main)
participant Ctx as Context
participant NPM as NPM logic
participant Registry as NPM Registry
participant Log as Logger
CLI->>Ctx: parse flags (--patch-only -> PatchOnly)
Ctx->>NPM: pass context with PatchOnly
NPM->>Registry: get package versions (getOtherNPMPackageVersions)
Registry-->>NPM: return versions
NPM->>NPM: compute latest patch candidate (getLatestPatchNPMPackageVersion)
alt Patch candidate found & should apply
NPM->>Log: record planned update
NPM->>NPM: apply update
else No patch candidate
NPM->>Log: "won't update" / warn
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@npm.go`:
- Around line 206-208: The conditional in the gate uses a redundant disjunct;
update the check in the function that uses Ctx.PatchOnly so it simply reads: if
Ctx.PatchOnly && changeType != SemverChangePatch { return changeType, false }
(remove the always-covered changeType == SemverChangeRevision part). Then add
unit tests in TestClassifyDependencyUpdate that set Ctx.PatchOnly = true and
assert behavior for a major/minor update (blocked), a patch update (allowed),
and a revision/no-op (blocked) to cover the gate and prevent regressions;
reference Ctx.PatchOnly, SemverChangePatch, SemverChangeRevision, and
TestClassifyDependencyUpdate when making changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
npm.go (1)
203-216:⚠️ Potential issue | 🟠 Major
--patch-onlyis bypassed for non-semver version specs.At Line 216, non-semver differences still return
shouldUpdate=true, so--patch-onlycan updateworkspace:*,file:, dist-tags, etc. That breaks patch-only semantics.💡 Proposed fix
func classifyDependencyUpdate(currentVersion DependencyVersion, latestVersion DependencyVersion) (SemverChange, bool) { if currentVersion.HasSemver && latestVersion.HasSemver { changeType := currentVersion.Semver.ChangeType(latestVersion.Semver) if Ctx.PatchOnly && changeType != SemverChangePatch { return changeType, false } return changeType, changeType != SemverChangeNone && changeType != SemverChangeDowngrade } if currentVersion.String() == latestVersion.String() { return SemverChangeNone, false } + if Ctx.PatchOnly { + // Cannot safely classify as patch-only when either side is non-semver. + return SemverChangeInvalid, false + } return SemverChangeInvalid, true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm.go` around lines 203 - 216, The function classifyDependencyUpdate allows non-semver changes (e.g., workspace:*, file:, dist-tags) to be marked shouldUpdate=true even when Ctx.PatchOnly is set; update classifyDependencyUpdate so that if Ctx.PatchOnly is true and either currentVersion.HasSemver or latestVersion.HasSemver is false, it returns a safe change (e.g., SemverChangeInvalid or SemverChangeNone) with shouldUpdate=false. Specifically, in classifyDependencyUpdate (and references to DependencyVersion and Ctx.PatchOnly), add an early guard for Ctx.PatchOnly when versions are non-semver so non-semver diffs do not trigger updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@npm.go`:
- Around line 203-216: The function classifyDependencyUpdate allows non-semver
changes (e.g., workspace:*, file:, dist-tags) to be marked shouldUpdate=true
even when Ctx.PatchOnly is set; update classifyDependencyUpdate so that if
Ctx.PatchOnly is true and either currentVersion.HasSemver or
latestVersion.HasSemver is false, it returns a safe change (e.g.,
SemverChangeInvalid or SemverChangeNone) with shouldUpdate=false. Specifically,
in classifyDependencyUpdate (and references to DependencyVersion and
Ctx.PatchOnly), add an early guard for Ctx.PatchOnly when versions are
non-semver so non-semver diffs do not trigger updates.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@npm.go`:
- Around line 216-219: The current branch that handles Ctx.PatchOnly in the
semver comparison path wrongly returns SemverChangeNone which hides that the
versions are non-semver-comparable; update the handler in the function that
checks Ctx.PatchOnly to return SemverChangeInvalid, false instead of
SemverChangeNone, false and keep the existing log (or adjust it to mention
“invalid/non-semver”) so callers and diagnostics can distinguish a skipped
non-semver comparison from a true “no change” result; refer to Ctx.PatchOnly,
SemverChangeNone and SemverChangeInvalid to locate and modify the return.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if Ctx.PatchOnly { | ||
| log.Warnf("Cannot determine change type for non-semver dependency version '%s' -> '%s', skipping update due to patch-only mode", currentVersion.String(), latestVersion.String()) | ||
| return SemverChangeNone, false | ||
| } |
There was a problem hiding this comment.
Don’t classify non-semver patch-only skips as SemverChangeNone.
Line 218 currently reports “none” even when the real reason is “not semver-comparable under patch-only,” which can mislead logs and diagnostics. Return SemverChangeInvalid with false to preserve reason while still skipping the update.
Suggested fix
if Ctx.PatchOnly {
log.Warnf("Cannot determine change type for non-semver dependency version '%s' -> '%s', skipping update due to patch-only mode", currentVersion.String(), latestVersion.String())
- return SemverChangeNone, false
+ return SemverChangeInvalid, false
}📝 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.
| if Ctx.PatchOnly { | |
| log.Warnf("Cannot determine change type for non-semver dependency version '%s' -> '%s', skipping update due to patch-only mode", currentVersion.String(), latestVersion.String()) | |
| return SemverChangeNone, false | |
| } | |
| if Ctx.PatchOnly { | |
| log.Warnf("Cannot determine change type for non-semver dependency version '%s' -> '%s', skipping update due to patch-only mode", currentVersion.String(), latestVersion.String()) | |
| return SemverChangeInvalid, false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@npm.go` around lines 216 - 219, The current branch that handles Ctx.PatchOnly
in the semver comparison path wrongly returns SemverChangeNone which hides that
the versions are non-semver-comparable; update the handler in the function that
checks Ctx.PatchOnly to return SemverChangeInvalid, false instead of
SemverChangeNone, false and keep the existing log (or adjust it to mention
“invalid/non-semver”) so callers and diagnostics can distinguish a skipped
non-semver comparison from a true “no change” result; refer to Ctx.PatchOnly,
SemverChangeNone and SemverChangeInvalid to locate and modify the return.
Add a dedicated `testjspatch` target that exercises `--patch-only` in the JS fixture, and extend the fixture dependency set to cover patch-only update cases.
When `--patch-only` blocks a normal semver update, fetch registry versions and apply the latest compatible patch release instead.
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@npm.go`:
- Around line 127-130: The new helper currently aborts when Ctx.HTTPClient is
nil (client := Ctx.HTTPClient; if client == nil { return ..., fmt.Errorf(...) })
which breaks the secondary registry lookup even though
getNPMPackageLatestVersion works with a nil client; change the nil branch to
fall back to the default HTTP client (e.g., client = http.DefaultClient) instead
of returning an error so both the primary `/latest` and the subsequent registry
lookup can succeed; update any references in this helper that use client and
keep getNPMPackageLatestVersion behavior unchanged.
- Around line 142-155: The helper currently decodes npm metadata into
result.Time and iterates its keys, which pulls non-version keys like "created"
and "modified"; change the decoded struct to capture the "versions" object
instead (e.g., result.Versions map[string]string `json:"versions"`) and iterate
over result.Versions to build the versions slice so
getLatestPatchNPMPackageVersion and classifyDependencyUpdate only see actual
published version strings.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32c7fea5-3090-474a-ab8e-ba3a0c2bcc71
📒 Files selected for processing (3)
Makefilenpm.gotest_js/package.json
✅ Files skipped from review due to trivial changes (1)
- test_js/package.json
✅ Actions performedReview triggered.
|
…only Read registry versions from the `versions` payload and fall back to the default HTTP client when no client is configured.
Add canary and React dependencies to the JS fixture so patch-only update cases cover more registry metadata.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Closes #4
Summary by CodeRabbit
New Features
--patch-onlycommand-line flag to restrict dependency updates to patch versions only.Bug Fixes / Improvements
Tests / Chores