Add 8 deep code understanding and GitHub context tools for RLM agent#6
Add 8 deep code understanding and GitHub context tools for RLM agent#6sng-asyncfunc wants to merge 13 commits intoadd-local-folder-supportfrom
Conversation
Agent-Id: agent-c0b5d14b-9960-4052-a205-b89fc16f7194 Linked-Note-Id: eee4905d-b15d-494a-9b50-bdcd1a97b737
Agent-Id: agent-94ef2215-ac3c-4366-93d1-3c0f66ea589d Linked-Note-Id: 26c3d4bf-dbd2-4452-8cd9-ee89596fd587
Agent-Id: agent-3ef1c9c1-e571-4f01-bb14-c018d8f700a3 Linked-Note-Id: 826b702f-3958-4e1e-9923-f0d6d8ff55e6
Agent-Id: agent-cc31288f-e5e9-4421-ba73-8d1ff26e2d94 Linked-Note-Id: a8a01828-257a-42df-be9a-a164c786caa3
Summary of ChangesHello @sng-asyncfunc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the RLM agent's analytical power by introducing a suite of new tools. These additions allow the agent to perform more sophisticated code analysis, such as understanding symbol definitions, usage patterns, and class hierarchies, while also leveraging GitHub-specific context like PR comments, blame information, and commit history. The changes aim to make the agent more intelligent and context-aware during code reviews, providing richer insights without adding new external dependencies. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces 8 new tools for deep code understanding and GitHub context analysis. However, a major concern is that most of these tools are vulnerable to injection attacks. User-supplied inputs are directly concatenated into GitHub search queries, which could lead to information leakage by expanding the search scope to other repositories. Additionally, the get_pr_comments tool is susceptible to an SSRF-like attack, potentially leaking the GITHUB_TOKEN through a malicious pr_number. Beyond these security concerns, critical and high-severity code issues include a bug in local grep usage preventing symbol definition, a significant mis-implementation of the get_blame tool, and several misleading docstrings. Implementing strict input validation and sanitization for all tool arguments, alongside resolving these code issues, is crucial for the reliability, usability, and security of these new agentic capabilities.
npx/python/cli/local_repo_tools.py
Outdated
| symbol = symbol.strip() | ||
|
|
||
| # Build grep command to find definitions | ||
| args = ["grep", "-rn"] |
There was a problem hiding this comment.
The grep command is missing the -E flag for extended regular expressions. The pattern (def|class) {symbol} on line 217 uses ( and | which are special characters in extended regex. Without -E, grep will perform a literal string search for (def|class) ..., which will cause the symbol definition search to fail.
| args = ["grep", "-rn"] | |
| args = ["grep", "-rn", "-E"] |
npx/python/cli/repo_tools.py
Outdated
| client = await self._get_client() | ||
|
|
||
| # Search for calls to the function | ||
| search_query = f"{func_name}( repo:{self.owner}/{self.repo}" |
There was a problem hiding this comment.
The func_name argument is directly concatenated into the GitHub search query, allowing for potential search scope expansion via qualifiers like repo:.
Recommendation: Sanitize the input to prevent search qualifier injection.
| client = await self._get_client() | ||
|
|
||
| # Search for class definition | ||
| search_query = f"class {class_name} repo:{self.owner}/{self.repo}" |
There was a problem hiding this comment.
The class_name argument is directly concatenated into the GitHub search query, allowing for potential search scope expansion via qualifiers like repo:.
Recommendation: Sanitize the input to prevent search qualifier injection.
npx/python/cli/repo_tools.py
Outdated
| client = await self._get_client() | ||
|
|
||
| # Search for function or class definition | ||
| search_query = f"(def {symbol}|class {symbol}) repo:{self.owner}/{self.repo}" |
There was a problem hiding this comment.
The symbol argument is directly concatenated into the GitHub search query. An attacker could provide a malicious string containing search qualifiers like repo: to expand the search scope to other repositories, potentially leaking sensitive information from private repositories accessible by the GITHUB_TOKEN used by the agent.
Recommendation: Sanitize the symbol input to remove or escape GitHub search qualifiers, or ensure the search is strictly limited to the intended repository.
| reviews_url = f"{GITHUB_API_BASE}/repos/{self.owner}/{self.repo}/pulls/{pr_num}/reviews" | ||
| comments_url = f"{GITHUB_API_BASE}/repos/{self.owner}/{self.repo}/pulls/{pr_num}/comments" |
There was a problem hiding this comment.
The pr_number argument is not validated and is used to construct API URLs. An attacker can provide a string with path traversal characters (e.g., 1/../../other/repo/pulls/1) to redirect the API request to a different repository. This leaks the GITHUB_TOKEN to the target repository's API and allows unauthorized access to its data.
Recommendation: Validate that pr_number is a positive integer before using it to construct URLs.
| async def get_blame(self, path: str, line_range: str = "") -> str: | ||
| """Get blame information for a file or line range. | ||
|
|
||
| Parses line_range like "10-20" and returns commit info for those lines. | ||
| """ |
There was a problem hiding this comment.
The implementation of get_blame for RepoTools is incorrect and misleading for several reasons:
- Incorrect Functionality: It does not perform a 'blame' operation. Instead, it fetches the commit history for the file, which is nearly identical to what
get_commit_historyalready does. This is inconsistent with the tool's name and its local counterpart. - Unused Parameter: The
line_rangeparameter is ignored by the logic but is included in the result string, which is confusing. - Better Alternative: GitHub's REST API provides a dedicated blame endpoint (
GET /repos/{owner}/{repo}/blame/{file_path}) that should be used for a correct implementation.
I recommend refactoring this to use the correct GitHub Blame API or removing it to avoid redundancy and confusion.
| rel_path = os.path.relpath(file_path, self.root_path) | ||
|
|
||
| # Extract parent classes from "class X(Parent1, Parent2):" pattern | ||
| import re |
There was a problem hiding this comment.
The import re statement is located inside the get_type_hierarchy method. According to PEP 8, imports should be placed at the top of the file. This improves readability, avoids the overhead of re-importing on every function call, and makes dependencies clear at a glance.
| Searches for the class definition, parses parent classes using regex, | ||
| and recursively resolves parent classes. |
There was a problem hiding this comment.
The docstring claims that this method "recursively resolves parent classes." However, the current implementation only finds and lists the direct parents of the given class. The recursive resolution is not implemented, which could be misleading to users of the tool.
| Searches for the class definition, parses parent classes using regex, | |
| and recursively resolves parent classes. | |
| Searches for the class definition, parses parent classes using regex, | |
| and lists its direct parent classes. |
| Searches for calls to func_name, and if depth > 0, searches for what | ||
| func_name calls by fetching its definition. |
There was a problem hiding this comment.
The docstring states that the method "searches for what func_name calls by fetching its definition." The implementation, however, only finds callers of func_name (incoming calls) and does not analyze the function's body to find the functions it calls (outgoing calls). This discrepancy should be corrected to avoid confusion.
| Searches for calls to func_name, and if depth > 0, searches for what | |
| func_name calls by fetching its definition. | |
| Searches for calls to func_name to find its callers. The `depth` parameter | |
| is not currently used to find outgoing calls. |
| try: | ||
| async with _semaphore: | ||
| reviews_resp = await client.get(reviews_url, headers=_get_headers(), timeout=30.0) | ||
| comments_resp = await client.get(comments_url, headers=_get_headers(), timeout=30.0) | ||
| except Exception: |
There was a problem hiding this comment.
The API calls to fetch PR reviews and comments are made sequentially. Since these are independent network requests, they can be executed concurrently to improve performance and reduce latency.
| try: | |
| async with _semaphore: | |
| reviews_resp = await client.get(reviews_url, headers=_get_headers(), timeout=30.0) | |
| comments_resp = await client.get(comments_url, headers=_get_headers(), timeout=30.0) | |
| except Exception: | |
| try: | |
| reviews_task = client.get(reviews_url, headers=_get_headers(), timeout=30.0) | |
| comments_task = client.get(comments_url, headers=_get_headers(), timeout=30.0) | |
| async with _semaphore: | |
| reviews_resp, comments_resp = await asyncio.gather(reviews_task, comments_task) | |
| except Exception: |
Agent-Id: agent-2846491c-78df-4055-b940-b6a40d8d6b57
Agent-Id: agent-70e37131-163d-49cb-9174-6232453bba80 Linked-Note-Id: d5abf445-7117-4b8f-86cb-33948be43b56
Agent-Id: agent-b72d39d4-8a97-4b7c-aa15-0bfe144c3578 Linked-Note-Id: 0a7fd7b0-70f2-4231-9046-0aba7e253b79
Agent-Id: agent-9e1e97de-0a5b-406c-80cc-8dde043f34b2 Linked-Note-Id: dbc451b8-d063-4f70-8a39-53450f0eae5f
Agent-Id: agent-d5721890-ade1-4abf-b5e0-f60219dfefa5 Linked-Note-Id: 9b9f5a65-5004-4d36-9803-22c2d53277ec
Agent-Id: agent-0fc17279-fb9d-4975-a183-14054aa72912
Agent-Id: agent-0fc17279-fb9d-4975-a183-14054aa72912
…vior Use GitHub-compatible code search queries for by splitting and lookups, scoping by first, and falling back repo-wide to avoid false negatives from regex-like query syntax. Tighten local symbol matching with escaped standalone identifiers and context-directory scoping, switch debug prints to structured logging, and quote call-site searches for literal matching. Add targeted helper tests for symbol definition, usage search, call graph queries, blame filtering, commit history, related issues, and local definition resolution to prevent regressions.
1 participant
Summary
Implements 8 new agentic tools for the RLM agent to perform deep code analysis and leverage GitHub context during reviews. These tools enable symbol definition lookup, usage analysis, type hierarchy inspection, call graph traversal, PR comment review, blame tracking, commit history analysis, and issue linking.
New Tools
Category A: Deep Code Understanding
get_symbol_definition(symbol, context_file)find_usages(symbol, scope_path)get_type_hierarchy(class_name)get_call_graph(func_name, depth)Category B: GitHub Interaction & Context
get_pr_comments(pr_number)get_blame(path, line_range)get_commit_history(path, limit)get_related_issues(query_text)Files Modified
npx/python/cli/repo_tools.py— 8 new async methods inRepoTools(GitHub API mode),pr_numberconstructor param, updatedTOOL_DESCRIPTIONSnpx/python/cli/local_repo_tools.py— 8 new async methods inLocalRepoToolsusing git/grep for local filesystem modenpx/python/cli/virtual_runner.py— 8 sync wrappers in_create_tool_functions(), returnslist[Callable]for DSPy RLM compatibility,pr_numberwiringImplementation Details
grep -rn,git blame,git log,git log --grepsubprocessesget_pr_comments,get_related_issues) gracefully degrade in local mode[ERROR: ...]stubs)Testing
RepoToolsandLocalRepoToolsget_symbol_definition,get_blame,list_dir,fetch_fileduring a live PR reviewPull Request opened by Augment Code with guidance from the PR author