Fix: Python path handling for paths with spaces #439
Conversation
- parsePythonCommand now detects file paths (containing / or \) - Prevents splitting paths like '/Users/name/Library/Application Support/...' - Fixes 'spawn ENOENT' error when Python is in venv with spaces in path - Still handles space-separated commands like 'py -3' correctly
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Comment |
|
|
|
Thank you for your contribution! Before we can accept your PR, you need to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with exactly: You can read the full CLA here: CLA.md Why do we need a CLA? Auto Claude is licensed under AGPL-3.0. The CLA ensures the project has proper licensing flexibility should we introduce additional licensing options in the future. You retain full copyright ownership of your contributions. I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Summary of ChangesHello @arttus, 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 addresses a critical issue where Python paths containing spaces were being incorrectly parsed, causing 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. 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
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![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
The parsePythonCommand function in auto-claude-ui/src/main/python-detector.ts has been updated to correctly parse Python command strings that are direct file paths. Previously, the function would always split the pythonPath by spaces, which could lead to incorrect parsing for full file paths. The new logic first checks if the pythonPath contains path separators ('/' or ''); if so, it treats the entire string as the command and returns an empty array for base arguments. Otherwise, it proceeds with the original behavior of splitting by spaces to handle commands like "py -3". The JSDoc and inline comments were also updated to reflect this change.
AlexMadera
left a comment
AlexMadera
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
Blocked: Cannot verify 1 claim(s) in PR. Evidence required before merge.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Verification Failed: Referenced file path does not exist in repository (auto-claude-ui/src/main/python-detector.ts:0)
- Redundancy: PR may be outdated - current implementation more comprehensive (apps/frontend/src/main/python-detector.ts:215)
Findings Summary
- Critical: 1 issue(s)
- High: 2 issue(s)
- Medium: 1 issue(s)
Generated by Auto Claude PR Review
Findings (4 selected of 4 total)
🔴 [CRITICAL] Referenced file path does not exist in repository
📁 auto-claude-ui/src/main/python-detector.ts:0
The PR claims to modify auto-claude-ui/src/main/python-detector.ts but this path does not exist. The actual file is located at apps/frontend/src/main/python-detector.ts. The package name in package.json is 'auto-claude-ui' but the directory structure uses 'apps/frontend/'. This indicates the PR either targets a non-existent branch structure or has incorrect metadata.
Suggested fix:
Update PR to reference the correct file path: `apps/frontend/src/main/python-detector.ts`. Verify the PR was created against the correct branch with the current repository structure.
🟠 [HIGH] PR may be outdated - current implementation more comprehensive
📁 apps/frontend/src/main/python-detector.ts:215
The current implementation of parsePythonCommand (lines 215-258) includes quote handling, existsSync checks, isLikelyPath detection, filtered splitting, and error handling. The PR diff shows a simpler implementation with only path separator detection. This suggests either: (1) the PR was already merged and improved upon, or (2) the PR is outdated and targets an old version of the code.
Suggested fix:
Verify the PR branch is up-to-date with main. If the fix was already applied, close the PR. If targeting an older version, rebase onto main and update the changes.
🟠 [HIGH] Edge case bug: paths with spaces AND arguments handled incorrectly
📁 apps/frontend/src/main/python-detector.ts:246
Both the current code and the PR's proposed fix incorrectly handle paths like /path/with spaces/python -u. The code returns the entire string as the command: ["/path/with spaces/python -u", []] instead of correctly parsing: ["/path/with spaces/python", ["-u"]]. This would cause spawn() to fail with ENOENT when users configure Python with default arguments.
Suggested fix:
Implement proper quote-aware parsing or require users to quote paths with spaces. Consider: (1) parsing quoted paths like `"/path/to/python" -u`, (2) validating at config time that paths with separators must exist on disk, or (3) changing the API to separate command from default arguments.
🟡 [MEDIUM] Missing unit tests for parsePythonCommand function
📁 apps/frontend/src/main/python-detector.ts:215
No dedicated unit tests exist for the parsePythonCommand function despite it being used in 14+ files across the codebase and handling complex edge cases (paths with spaces, quotes, arguments). The only testing is through integration tests which mock the function.
Suggested fix:
Add comprehensive unit tests covering: empty input, simple commands ('python3'), commands with args ('py -3'), absolute paths, paths with spaces, paths with spaces AND arguments, quoted paths, Windows paths, UNC paths, relative paths, and edge cases with multiple spaces.
This review was generated by Auto Claude.
|
@arttus wrong branch. Develop is the default branch to PR against. |
5 participants
Problem
The
parsePythonCommandfunction was splitting Python paths on spaces, causing failures when the Python venv is located in a directory with spaces (e.g.,/Users/name/Library/Application Support/auto-claude-ui/python-venv/bin/python).This resulted in spawn errors like:
Solution
Updated
parsePythonCommandinsrc/main/python-detector.tsto detect file paths (containing/or\) and avoid splitting them on spaces.Changes
/or\are returned as-is with no splittingpy -3still work correctlyTesting
~/Library/Application Support/auto-claude-ui/python-venv/python3orpy -3Impact
Fixes merge/stage operations for users on macOS (and potentially Windows) who have Python environments in paths with spaces.