Conversation
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughAdds an MCP (Model Context Protocol) server and toolset (search, cat, deps, fetch, resolve, where), CLI wiring ( Changes
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant Server as ksrc MCP Server
participant Handler as Tool Handler
participant Resolver as Resolution Service
participant Runner as Executor/Runner
Client->>Server: Call tool (e.g., "search")
Server->>Handler: Route request to handler
Handler->>Resolver: Resolve sources (filters/coords)
Resolver->>Runner: Execute resolution commands
Runner-->>Resolver: Return resolved sources
Resolver-->>Handler: Provide sources
Handler->>Runner: Run tool ops (ripgrep, read jar, etc.)
Runner-->>Handler: Return output
Handler-->>Server: Format text/result or error
Server-->>Client: Return tool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f82d99fbdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/auto-assign-issues.yml:
- Around line 14-18: The workflow currently references the action with a mutable
tag ("uses: pozil/auto-assign-issue@v2"), which should be pinned to a specific
commit SHA to avoid supply-chain risk; replace the tag with the full commit SHA
for pozil/auto-assign-issue (e.g., "uses: pozil/auto-assign-issue@<commit-sha>")
and keep the existing inputs (repo-token and assignees: Nek-12) unchanged—obtain
the SHA from the action repo's commit history and update the uses line
accordingly.
In `@go.mod`:
- Around line 5-15: Update the two outdated module requirements in go.mod:
replace golang.org/x/oauth2 v0.30.0 and github.com/yosida95/uritemplate/v3
v3.0.2 with their current stable releases (e.g., oauth2 v0.34.0 and the latest
uritemplate v3.x), then run module resolution to verify compatibility and update
the lockfile (ensure this change is reflected in the require block referencing
those module paths).
In `@internal/cat/cat.go`:
- Around line 40-43: The splitLineRange function currently uses
strings.NewReplacer which blindly removes "-" and can strip negative signs;
replace this with regex-based parsing in splitLineRange that extracts signed
integers (e.g., using a pattern like -?\d+) or matches an explicit two-number
range pattern (e.g., optional sign + digits, separator, optional sign + digits).
Validate that exactly two numeric tokens are found and return them; do not
globally replace separators so negative signs remain intact. Use the function
name splitLineRange to locate and update the implementation accordingly.
In `@internal/mcpserver/handlers.go`:
- Line 535: The error string in the return statement mistakenly includes
coord.String() twice and contains a backtick typo; update the fmt.Errorf call in
internal/mcpserver/handlers.go so the coordinate is included only once (remove
the duplicate %s/coord.String()) and change `` `fetch() `` to `` `fetch` `` in
the message, ensuring the final format string and single coord.String() argument
match.
- Around line 389-423: The file-id branch that handles resolve.ParseFileID
currently calls service.ResolveSources but never emits diagnostics; after
calling service.ResolveSources (in the block handling arg containing "!/")
invoke emitDiagnostics(result.Meta, s.verbose) immediately after the
ResolveSources return (and before checking len(result.Sources) or calling
findJarByCoord) so verbose diagnostics are produced consistently with the
coordinate and path-lookup branches.
🧹 Nitpick comments (2)
internal/cli/mcp.go (1)
13-27: Prefer cmd.Context() and disallow extra args.Using
cmd.Context()preserves cancellation signals, andcobra.NoArgsprevents silent acceptance of unexpected args.♻️ Proposed change
cmd := &cobra.Command{ Use: "mcp", Short: "Run MCP server over stdio", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + if ctx == nil { + ctx = context.Background() + } set, err := mcpserver.ParseTools(tools) if err != nil { return err } - return mcpserver.Run(context.Background(), mcpserver.Options{ + return mcpserver.Run(ctx, mcpserver.Options{ Runner: app.Runner, Verbose: app.Verbose, Tools: set, Version: versionString(), }) }, }internal/mcpserver/handlers.go (1)
69-71: Consider removing the identity function or documenting its purpose.
toolNamecurrently returns its input unchanged. If this is a placeholder for future namespacing/prefixing, consider adding a comment. Otherwise, it can be inlined at call sites.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/mcpserver/handlers.go`:
- Around line 250-257: The handleCat flow omits emitting diagnostics after
resolving sources; after calling Service.ResolveSources (variable result) add a
call to emitDiagnostics(result.Meta, s.verbose) (as done in other handlers)
immediately after the error check and before checking result.Sources (i.e.,
before the existing len(result.Sources) == 0 branch) so verbose diagnostics are
emitted for cat operations.
🧹 Nitpick comments (1)
internal/mcpserver/handlers.go (1)
69-71: Consider removing the identity function or adding a comment.
toolNamecurrently returns its input unchanged. If this is a placeholder for future namespacing (e.g., prefixing withksrc_), consider adding a comment explaining the intent. Otherwise, it can be removed and tool names used directly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/mcpserver/handlers.go`:
- Around line 515-523: In decodeInput[T any], guard against a nil Params pointer
before dereferencing it: update the early-return condition in the decodeInput
function to check req.Params is not nil (i.e., ensure you check req == nil ||
req.Params == nil || req.Params.Arguments == nil) so you don't access
req.Params.Arguments when Params is nil; this prevents a potential panic during
json.Unmarshal handling.
Release 0.6.0
Closes #18
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.