Add local directory support to npx CLI (--path option)
#5
Conversation
Agent-Id: agent-2dd07d62-259b-4803-8feb-e4628a75e02e Linked-Note-Id: 72504be3-8319-4597-ab00-2c454d04e146
- Add --path/-p option to Commander CLI in npx/src/index.ts - Make --url and --path mutually exclusive with validation - Update ReviewOptions interface to support both modes - Skip GitHub token requirement for local path mode - Pass --path to Python CLI when provided instead of --url - All three files (index.ts, cli.ts, python-runner.ts) updated consistently Agent-Id: agent-878b18ab-2ad7-486a-9f3a-56e26a705499
Agent-Id: agent-4f5819dd-c4c4-4d91-944c-fc754aab9ec5 Linked-Note-Id: 12de8f2f-c265-4f1e-afba-11c93caa7e4b
Agent-Id: agent-699e5407-af2c-4926-9c29-778f775d9a9a Linked-Note-Id: c509caf1-e1b9-4c01-9551-5c55ecd1e62a
- Convert grep command from shell=True string to list-based subprocess call - Add '--' separator to prevent query from being interpreted as grep flags - Prevents shell injection attacks via user-provided search queries - Maintains backward compatibility with regex patterns and special characters - Verified with test queries including malicious payloads Agent-Id: agent-fce47f03-d32a-4dda-b056-e23ed5a831f7
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 enhances the AsyncReview npx CLI by introducing the capability to review local codebases directly, rather than being limited to GitHub URLs. This change broadens the tool's applicability, allowing developers to perform code reviews on projects that are not hosted on GitHub or to review local changes before pushing them to a remote repository. The implementation includes robust handling for local file access, git integration for diffs, and a clear separation of concerns between local and remote review modes. 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://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant enhancement, adding support for reviewing local directories via the --path option. The implementation is well-structured, with a clear separation of concerns between the new local data fetcher, the local repository tools, and the main review runner. Security aspects, particularly preventing path traversal and command injection, have been handled robustly. The corresponding changes in the TypeScript CLI wrapper are also clean and correct. I've provided a few suggestions to improve Python idioms, fix a bug in file filtering logic, and clean up some dead code. Overall, this is a high-quality contribution.
| # Skip ignore patterns | ||
| if entry in IGNORE_PATTERNS: | ||
| continue |
There was a problem hiding this comment.
The check if entry in IGNORE_PATTERNS: only performs an exact string match, which means it will fail to handle wildcard patterns like *.egg-info from the IGNORE_PATTERNS set. This can lead to listing files that should be ignored.
To fix this, you can use the fnmatch module to correctly handle glob-style patterns. You will also need to add import fnmatch at the top of the file.
| # Skip ignore patterns | |
| if entry in IGNORE_PATTERNS: | |
| continue | |
| # Skip ignore patterns | |
| if any(fnmatch.fnmatch(entry, pattern) for pattern in IGNORE_PATTERNS): | |
| continue |
| file_count = [0] # Use list to allow modification in nested function | ||
|
|
||
| def walk_tree(dir_path: str, prefix: str = "", depth: int = 0) -> None: | ||
| """Recursively walk directory tree.""" | ||
| if depth > max_depth or file_count[0] >= max_files: | ||
| return | ||
|
|
||
| try: | ||
| entries = sorted(os.listdir(dir_path)) | ||
| except (PermissionError, OSError): | ||
| return | ||
|
|
||
| dirs = [] | ||
| files = [] | ||
|
|
||
| for entry in entries: | ||
| if should_ignore(entry): | ||
| continue | ||
| full_path = os.path.join(dir_path, entry) | ||
| if os.path.isdir(full_path): | ||
| dirs.append(entry) | ||
| else: | ||
| files.append(entry) | ||
|
|
||
| # Process directories first | ||
| for i, dir_name in enumerate(dirs): | ||
| is_last_dir = (i == len(dirs) - 1) and len(files) == 0 | ||
| connector = "└── " if is_last_dir else "├── " | ||
| lines.append(f"{prefix}{connector}{dir_name}/") | ||
|
|
||
| if file_count[0] < max_files: | ||
| next_prefix = prefix + (" " if is_last_dir else "│ ") | ||
| walk_tree(os.path.join(dir_path, dir_name), next_prefix, depth + 1) | ||
|
|
||
| # Process files | ||
| for i, file_name in enumerate(files): | ||
| if file_count[0] >= max_files: | ||
| break | ||
| is_last = i == len(files) - 1 | ||
| connector = "└── " if is_last else "├── " | ||
| lines.append(f"{prefix}{connector}{file_name}") | ||
| file_count[0] += 1 | ||
|
|
There was a problem hiding this comment.
Using a list [0] to simulate a mutable integer is a common workaround, but a more idiomatic Python 3 approach is to use the nonlocal keyword. This makes the code's intent clearer and is the modern way to modify variables in an enclosing scope.
file_count = 0
def walk_tree(dir_path: str, prefix: str = "", depth: int = 0) -> None:
"""Recursively walk directory tree."""
nonlocal file_count
if depth > max_depth or file_count >= max_files:
return
try:
entries = sorted(os.listdir(dir_path))
except (PermissionError, OSError):
return
dirs = []
files = []
for entry in entries:
if should_ignore(entry):
continue
full_path = os.path.join(dir_path, entry)
if os.path.isdir(full_path):
dirs.append(entry)
else:
files.append(entry)
# Process directories first
for i, dir_name in enumerate(dirs):
is_last_dir = (i == len(dirs) - 1) and len(files) == 0
connector = "└── " if is_last_dir else "├── "
lines.append(f"{prefix}{connector}{dir_name}/")
if file_count < max_files:
next_prefix = prefix + (" " if is_last_dir else "│ ")
walk_tree(os.path.join(dir_path, dir_name), next_prefix, depth + 1)
# Process files
for i, file_name in enumerate(files):
if file_count >= max_files:
break
is_last = i == len(files) - 1
connector = "└── " if is_last else "├── "
lines.append(f"{prefix}{connector}{file_name}")
file_count += 1| except Exception: | ||
| return [] # Soft fail |
There was a problem hiding this comment.
Catching a bare Exception is generally too broad. It can mask unexpected errors and even catch system-exiting exceptions like KeyboardInterrupt or SystemExit. It's better to catch more specific exceptions related to the subprocess call, such as subprocess.SubprocessError or OSError.
| except Exception: | |
| return [] # Soft fail | |
| except (subprocess.SubprocessError, OSError): | |
| return [] # Soft fail |
| if expert: | ||
| actual_question = get_expert_prompt(question) | ||
| review_mode = "Expert Review" | ||
| elif question: | ||
| actual_question = question | ||
| review_mode = "Review" | ||
| else: |
There was a problem hiding this comment.
The review_mode variable is assigned a value in both branches of the if/elif statement but is never used. This constitutes dead code and should be removed to improve code clarity.
if expert:
actual_question = get_expert_prompt(question)
elif question:
actual_question = question
else:Agent-Id: agent-83b013f9-a1e5-47aa-a26c-8741f631560f Linked-Note-Id: 8acd5eaf-3498-4009-8095-8355f7338af9
- Updated _ensure_configured() to pass tools=self._create_tool_functions() to dspy.RLM() - Simplified review() and review_local() to use direct rlm.aforward() calls - Removed AGENTIC_TOOLS_PROMPT constant (~60 lines) - Removed _process_tool_requests() and _run_rlm_with_tools() methods (~140 lines) - Removed unused state dicts (_repo_files, _repo_dirs, _search_results) - Removed unused imports (re, Prediction, REPLHistory) - Updated class docstring to reference native DSPy RLM tools - Kept _load_local_checklist(), review_pr(), review_issue() aliases - Implemented on_step callback using result.trajectory if available Agent-Id: agent-367d7d70-1d10-4e5d-9d84-ec8956d914ae
DSPy RLM's tools parameter expects dict[str, Callable] not list. This fixes the 'list object has no attribute items' error. Agent-Id: agent-01069cef-6b08-41f1-a15d-29feb2ccaea1
…ends
LLMs naturally call list_dir(".") to list root directory. Previously
sanitize_path rejected "." as invalid. Now ".", "./" and "/" are
treated as root (same as empty string) in list_directory.
Agent-Id: agent-01069cef-6b08-41f1-a15d-29feb2ccaea1
DSPy's verbose=True already shows each RLM iteration's reasoning and code in real-time. The post-hoc on_step trajectory callback was replaying all steps again after completion, causing every step to appear twice in output. Agent-Id: agent-01069cef-6b08-41f1-a15d-29feb2ccaea1
1 participant
What
Adds a
--path/-poption to the AsyncReview npx CLI, allowing users to review local codebases (not just GitHub URLs). When given a local directory path, the tool reads local files and (if available) the git diff, then runs the same RLM-powered review.Why
Previously, AsyncReview could only review GitHub PRs and Issues via URL. This limits usability for developers who want to review local code before pushing, or review projects that aren't on GitHub.
How
New files
npx/python/cli/local_fetcher.py— Builds review context from local directories: detects git repos, capturesgit diff(staged + unstaged), generates project structure treenpx/python/cli/local_repo_tools.py— Drop-in replacement forRepoToolsthat reads from local filesystem instead of GitHub API. SupportsFETCH_FILE,LIST_DIR,SEARCH_CODEcommands identicallyModified files
npx/src/index.ts— Added--pathoption, validates mutual exclusion with--urlnpx/src/cli.ts— GitHub token is NOT requested in local modenpx/src/python-runner.ts— Passes--pathto Python CLI when setnpx/python/cli/main.py— Added--patharg via mutually exclusive group,run_local_review()function,--submitblocked with--pathnpx/python/cli/virtual_runner.py— Addedreview_local()method usingLocalRepoTools+build_local_context()Usage
Key Design Decisions
--urland--pathare mutually exclusive — enforced at both TypeScript and Python layers--submitis blocked with--path(can't post comments to GitHub from local reviews)LocalRepoToolsvalidates all resolved paths stay within the root directoryFETCH_FILE,LIST_DIR,SEARCH_CODE) work identically for both modesVerification
--urland--pathoptions--submit+--path→ clear error messageLocalRepoToolsinterface matchesRepoToolsexactly (5 methods, identical signatures)../etc/passwdetc.)npm installfirst)Checklist
--urlflow is unchanged--pathadditions)Pull Request opened by Augment Code with guidance from the PR author