Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors the resolution pipeline by extracting orchestration logic from the Gradle layer into a new resolution service. The top-level Gradle Changes
Sequence DiagramsequenceDiagram
actor CLI as CLI Command
participant Flags as CLI Flags
participant Service as resolution.Service
participant Traversal as gradle.Traversal
participant Gradle as Gradle Invocation
participant Cache as Cache Store
CLI->>Flags: Parse flags → ResolveFlags
CLI->>Service: flags.ToRequest() → Request
Service->>Service: buildResolveAttempts(req)
loop For each attempt
Service->>Traversal: gradle.Resolve(ctx, opts)
Traversal->>Gradle: resolveOnce(root)
Gradle-->>Traversal: Result
alt buildSrc enabled
Traversal->>Gradle: resolveOnce(buildSrc)
Gradle-->>Traversal: Result
end
alt included builds enabled
loop For each included build (BFS)
Traversal->>Gradle: resolveOnce(includedBuild)
Gradle-->>Traversal: Result
end
end
Traversal-->>Service: ResolveResult
alt Gradle success
Service->>Service: mergeSources/mergeDeps
else Gradle failure
Service->>Cache: cacheFallbackSources(req)
Cache-->>Service: cached sources/coords
end
end
Service->>Service: applyFilters (if enabled)
Service-->>CLI: Result {Sources, Deps, Meta}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/cli-api.md`:
- Around line 78-84: The Usage block under the "ksrc open <file-id|path>"
section should be converted to a proper subheading and the fenced code block
should include a language; replace the bold "Usage" line with a markdown heading
(e.g., "#### Usage") and change the triple-backtick fence to specify the
language as "text" so the example becomes a fenced block with ```text followed
by the command "ksrc open <file-id|path> [flags]" then closing ```; update the
section around the "ksrc open <file-id|path>" heading accordingly.
In `@internal/resolution/service.go`:
- Around line 250-299: The bug is that isExactToken treats empty strings as
exact which makes hasExactSelector and cacheFallbackSources take the
exact-selector branch when coord.Version is absent; change isExactToken (used by
hasExactSelector) so that after trimming it returns false for empty values
(i.e., treat empty as non-exact), keep the existing wildcard/charset/comma
checks otherwise; this will cause hasExactSelector to return false for missing
versions and let cacheFallbackSources follow the fallback path instead of
calling resolve.FindCachedSources with an empty version.
In `@internal/search/rg.go`:
- Around line 115-131: The code in parseRgOutput currently uses
strings.TrimSpace(line), which removes all leading/trailing whitespace and may
hide real leading spaces in ripgrep output; change this to only strip line
terminators (e.g. use strings.TrimRight(line, "\r\n")) so leading/trailing
spaces inside the line are preserved, keeping parseRgLine/mapping behavior
intact for function parseRgOutput and preserving exact match text.
Comment on lines
+78
to
84
| ### `ksrc open <file-id|path>` | ||
| Open a file in `$PAGER` (defaults to `less -R`). | ||
|
|
||
| **Usage** | ||
| ``` | ||
| ksrc open <path> [flags] | ||
| ksrc open <file-id|path> [flags] | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix markdownlint MD036/MD040 in the ksrc open usage block.
Use a heading instead of bold text and specify a language for the fence.
📝 Suggested fix
-**Usage**
-```
-ksrc open <file-id|path> [flags]
-```
+#### Usage
+```text
+ksrc open <file-id|path> [flags]
+```📝 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.
Suggested change
| ### `ksrc open <file-id|path>` | |
| Open a file in `$PAGER` (defaults to `less -R`). | |
| **Usage** | |
| ``` | |
| ksrc open <path> [flags] | |
| ksrc open <file-id|path> [flags] | |
| ``` | |
| ### `ksrc open <file-id|path>` | |
| Open a file in `$PAGER` (defaults to `less -R`). | |
| #### Usage |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
81-81: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/cli-api.md` around lines 78 - 84, The Usage block under the "ksrc open
<file-id|path>" section should be converted to a proper subheading and the
fenced code block should include a language; replace the bold "Usage" line with
a markdown heading (e.g., "#### Usage") and change the triple-backtick fence to
specify the language as "text" so the example becomes a fenced block with
```text followed by the command "ksrc open <file-id|path> [flags]" then closing
```; update the section around the "ksrc open <file-id|path>" heading
accordingly.
Comment on lines
+250
to
+299
| func cacheFallbackSources(req Request, dep string, applyFilters bool) ([]resolve.SourceJar, []resolve.Coord, error) { | ||
| var sources []resolve.SourceJar | ||
| var err error | ||
|
|
||
| if dep != "" { | ||
| coord, err := resolve.ParseCoord(dep) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| sources, err = resolve.FindCachedSources(coord.Group, coord.Artifact, coord.Version) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| } else if req.All || !hasExactSelector(req) { | ||
| sources, err = resolve.FindAllCachedSources() | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| } else { | ||
| coord, _ := resolve.SelectorToCoord(req.Module, req.Group, req.Artifact, req.Version) | ||
| sources, err = resolve.FindCachedSources(coord.Group, coord.Artifact, coord.Version) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| } | ||
|
|
||
| if applyFilters { | ||
| sources = resolve.FilterSources(sources, req.Module, req.Group, req.Artifact, req.Version) | ||
| } | ||
| return sources, collectCoords(sources), nil | ||
| } | ||
|
|
||
| func hasExactSelector(req Request) bool { | ||
| coord, ok := resolve.SelectorToCoord(req.Module, req.Group, req.Artifact, req.Version) | ||
| if !ok { | ||
| return false | ||
| } | ||
| return isExactToken(coord.Group) && isExactToken(coord.Artifact) && isExactToken(coord.Version) | ||
| } | ||
|
|
||
| func isExactToken(value string) bool { | ||
| value = strings.TrimSpace(value) | ||
| if value == "" { | ||
| return true | ||
| } | ||
| if strings.ContainsAny(value, "*?[]") { | ||
| return false | ||
| } | ||
| return !strings.Contains(value, ",") | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the function definitions for selector and cache operations
rg -n "func SelectorToCoord" --type go
echo "---"
rg -n "func FindCachedSources" --type go
echo "---"
rg -n "func FindAllCachedSources" --type go
echo "---"
# Also search for how empty versions are handled elsewhere
rg -n "Version.*=.*\"\"" --type go -A 2 -B 2Repository: respawn-app/ksrc
Length of output: 2295
🏁 Script executed:
#!/bin/bash
# Read the selector and cache implementations
cat -n internal/resolve/selector.go
echo "=== SEPARATOR ==="
cat -n internal/resolve/cache.goRepository: respawn-app/ksrc
Length of output: 4716
🏁 Script executed:
#!/bin/bash
# Verify the exact behavior of isExactToken with empty string
cat -n internal/resolution/service.go | sed -n '260,270p'Repository: respawn-app/ksrc
Length of output: 508
Incorrect handling of empty version: should not be treated as "exact."
isExactToken returns true for empty strings, causing hasExactSelector to incorrectly return true when coord.Version is empty. This triggers the exact selector branch (case 3), which calls FindCachedSources with an empty version instead of falling back to FindAllCachedSources. While FindCachedSources internally recovers by calling HighestCachedVersion, the control flow is semantically incorrect—empty version represents an absent version and should be treated as non-exact.
Suggested guard
func hasExactSelector(req Request) bool {
coord, ok := resolve.SelectorToCoord(req.Module, req.Group, req.Artifact, req.Version)
if !ok {
return false
}
+ if strings.TrimSpace(coord.Version) == "" {
+ return false
+ }
return isExactToken(coord.Group) && isExactToken(coord.Artifact) && isExactToken(coord.Version)
}This ensures that absent (empty) versions explicitly trigger the fallback path, aligning with the principle of preferring project-resolved dependency versions and falling back to cache-latest only when the version is absent.
🤖 Prompt for AI Agents
In `@internal/resolution/service.go` around lines 250 - 299, The bug is that
isExactToken treats empty strings as exact which makes hasExactSelector and
cacheFallbackSources take the exact-selector branch when coord.Version is
absent; change isExactToken (used by hasExactSelector) so that after trimming it
returns false for empty values (i.e., treat empty as non-exact), keep the
existing wildcard/charset/comma checks otherwise; this will cause
hasExactSelector to return false for missing versions and let
cacheFallbackSources follow the fallback path instead of calling
resolve.FindCachedSources with an empty version.
Merged
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 by CodeRabbit
Release Notes
New Features
--scope,--config,--targets,--subproject,--offline,--refresh,--buildscript, and--include-buildsfor enhanced filtering and build configuration.ksrc opencommand syntax to accept file IDs in addition to file paths.Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.