Skip to content

Conversation

yeabwang
Copy link

@yeabwang yeabwang commented Aug 9, 2025

The Claude Code Security Reviewer project has a Windows-specific issue where the test test_anthropic_api_key_handling failed due to subprocess.run not handling the PowerShell script claude.ps1 correctly, resulting in a "Claude CLI not installed" error. Resolved this by implementing a PlatformAdapter class in platform_utils.py, which uses platform-aware command execution to invoke claude.ps1 via powershell.exe or claude.cmd on Windows, while maintaining compatibility with Linux/macOS. Comprehensive tests in test_utils.py and updated documentation in README.md ensure the fix is robust, preventing other Windows users from encountering this issue.

@Copilot Copilot AI review requested due to automatic review settings August 9, 2025 07:14
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes Windows-specific issues with Claude CLI execution by implementing a platform-aware adapter pattern. The main problem was that subprocess.run couldn't directly execute PowerShell scripts (.ps1) on Windows, causing "Claude CLI not installed" errors.

  • Implements a PlatformAdapter class for cross-platform Claude CLI command execution
  • Adds comprehensive testing utilities with Windows-specific mocking capabilities
  • Updates main audit logic to use platform-aware command execution

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
claudecode/platform_utils.py New platform adapter that handles Windows PowerShell/CMD execution while maintaining Unix compatibility
claudecode/test_utils.py Comprehensive testing utilities with Windows environment simulation and Claude CLI mocking
claudecode/github_action_audit.py Updated to use platform adapter for robust cross-platform Claude CLI validation
README.md Added Windows-specific installation and troubleshooting documentation


1. Install Node.js (v18 or later) and npm:
- Download from https://nodejs.org or use nvm-windows.
2. Install the Claude CLI:npm install -g @anthropic-ai/claude-code
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The installation command is missing proper formatting and line breaks. It should be formatted as a code block for clarity.

Suggested change
2. Install the Claude CLI:npm install -g @anthropic-ai/claude-code
2. Install the Claude CLI:
```bash
npm install -g @anthropic-ai/claude-code

Copilot uses AI. Check for mistakes.


4. Set PowerShell execution policy:Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned
5. Verify installation:claude --version

Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PowerShell commands are not properly formatted as code blocks and appear as continuous text. These should be separate code blocks with proper formatting.

Suggested change
2. Install the Claude CLI:
```powershell
npm install -g @anthropic-ai/claude-code
  1. Add npm global directory to PATH:

    $npmPath = npm config get prefix
    [Environment]::SetEnvironmentVariable("Path", [Environment]::GetEnvironmentVariable("Path", "User") + ";$npmPath", "User")
  2. Set PowerShell execution policy:

    Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned
  3. Verify installation:

    claude --version

Copilot uses AI. Check for mistakes.


4. Set PowerShell execution policy:Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned
5. Verify installation:claude --version

Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing proper formatting for the PowerShell command. Should be in a code block for better readability.

Suggested change
4. Set PowerShell execution policy:
```powershell
Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned
  1. Verify installation:

    claude --version

Copilot uses AI. Check for mistakes.


4. Set PowerShell execution policy:Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned
5. Verify installation:claude --version

Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command should be formatted as a code block for consistency with documentation standards.

Suggested change
5. Verify installation:

claude --version

Copilot uses AI. Check for mistakes.


Expected output: 1.0.71 (Claude Code) or similar.

Troubleshooting
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing proper markdown heading formatting. Should be '### Troubleshooting' to maintain documentation hierarchy.

Suggested change
Troubleshooting
### Troubleshooting

Copilot uses AI. Check for mistakes.

return claude_cmd
except OSError as e:
logger.debug(f"Error checking path {npm_path}: {e}")
continue
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The continue statement is unnecessary here since it's at the end of the loop. The loop will naturally continue to the next iteration.

Suggested change
continue

Copilot uses AI. Check for mistakes.

@yeabwang
Copy link
Author

yeabwang commented Aug 9, 2025

Hello, While running pytest locally on Windows, I encountered two issues:

  1. Claude CLI not found caused by PowerShell’s inability to directly run claude.ps1. This PR addresses that by adding a Windows-specific PlatformAdapter.
  2. shutil.rmtree RecursionError occurs during temporary folder cleanup, leading to an infinite recursion loop. I am still investigating and will address this in a separate fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant