Skip to content

Conversation

jamubc
Copy link
Owner

@jamubc jamubc commented Aug 11, 2025

Description

Brief description of changes

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Testing

  • I have tested these changes locally
  • I have updated the documentation if needed

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @jamubc, 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!

I've implemented a patch primarily focused on enhancing the Windows compatibility and robustness of command execution within the gemini-mcp-tool. This involves significant improvements to how external commands are spawned and how prompts are passed, particularly addressing issues related to file paths and special characters on Windows. Concurrently, I've streamlined the project by removing several unused dependencies, contributing to a leaner and more efficient codebase. These changes are reflected in a version bump to 1.1.5 and updated troubleshooting documentation.

Highlights

  • Version Bump: I've updated the project version from 1.1.4 to 1.1.5 across package.json, package-lock.json, and src/index.ts, signifying a new release that includes important bug fixes and improvements.
  • Improved Windows Compatibility and Command Execution: I've made significant changes to how commands are executed and prompts are handled, especially for Windows. The commandExecutor.ts now explicitly uses shell: true when spawning processes, which helps resolve ENOENT errors by allowing the system shell to correctly locate executables. Additionally, geminiExecutor.ts now intelligently uses stdin for complex prompts (e.g., those with @ symbols or in change mode) to bypass shell escaping issues, while also improving quoting for simpler prompts on Windows.
  • Dependency Cleanup: I've performed a substantial cleanup of the project's dependencies by removing several unused libraries like ai, chalk, d3-shape, and inquirer, along with their transitive dependencies. This reduces the overall package size and potential attack surface.
  • Cache Management Improvements: I've enhanced the cache management in src/utils/chunkCache.ts by increasing the cache key length to reduce collision risk and implementing more robust cleanup logic that identifies and removes invalid cache files.
  • Documentation Update: I've updated the troubleshooting.md documentation to reflect the common 'Spawn ENOENT errors' on Windows and provided guidance on how to resolve them, directly linking to the improvements made in this patch.
  • New Unit Tests: I've added new unit tests in tests/geminiExecutor.test.ts to specifically cover the new prompt handling and command execution logic, ensuring the changes work as expected across different scenarios.
Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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 or fill out our survey to provide feedback.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a patch primarily aimed at improving Windows compatibility. The key change is a refactoring of the command execution logic to use stdin for complex prompts, which is a robust way to handle shell escaping issues. The PR also removes several dependencies, simplifying the project, and adds unit tests for the new execution logic, which is a great improvement. However, the introduction of shell: true in spawn calls creates a critical command injection vulnerability that must be addressed. I've left specific comments on the vulnerable lines with suggestions for a fix.

@jamubc jamubc marked this pull request as ready for review August 11, 2025 10:12
@Copilot Copilot AI review requested due to automatic review settings August 11, 2025 10:12
Copilot

This comment was marked as outdated.

jamubc added 2 commits August 11, 2025 03:26
Removed tests/geminiExecutor.test.ts and added tests/geminiExecutor.test.js to switch test implementation from TypeScript to JavaScript. Updated package.json dependencies.
Copy link

openhands-ai bot commented Aug 11, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • CI

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #27

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

@jamubc jamubc requested a review from Copilot August 11, 2025 11:03
Copilot

This comment was marked as outdated.

@jamubc
Copy link
Owner Author

jamubc commented Aug 11, 2025

## **Main Windows Patch**  
**Version:** `1.1.4``1.1.5`  

### **Summary**
This update addresses command execution failures on Windows (`spawn ENOENT`) and improves cross-platform prompt handling. It also adds CI/CD improvements, caching fixes, updated documentation, and new tests.

---

## **Changes**

### 💻 Windows Compatibility & Command Execution
- **`src/utils/commandExecutor.ts`**
  - Enabled `spawn` with `shell: true` to allow Windows to locate executables via the system shell.
  - **⚠ Security Risk:** `shell: true` enables command injection.  
    - **Recommendation:** Restrict to Windows (`shell: process.platform === 'win32'`) or avoid entirely.

- **`src/utils/geminiExecutor.ts`**
  - Complex prompts (e.g., containing `@` or quotes) are now passed via **`stdin`** instead of CLI args to avoid shell escaping issues.
  - Simple prompts continue to use `-p` flag with platform-specific quote escaping.

---

### ⚙ CI/CD Improvements
- **`.github/workflows/ci.yml`**
  - Added `lint` step.
  - Enabled npm caching for faster installs.
  - Updated job name to `Build & Test`.
- **`.github/workflows/deploy-docs.yml`**
  - Updated to latest GitHub Actions versions (`checkout@v4`, `setup-node@v4`, etc.).

---

### 🗄 Caching Updates
- **`src/utils/chunkCache.ts`**
  - Expiration now determined via internal `timestamp` in cache files (instead of file mtime).
  - Automatically removes corrupted or invalid cache files.

---

### 📚 Documentation
- Updated `docs/resources/troubleshooting.md` with guidance for Windows `spawn ENOENT` errors and the fix in `v1.1.5`.

---

### ✅ Testing
- **`tests/geminiExecutor.test.ts`**
  - Added unit tests for new prompt handling logic (`stdin` vs `-p`), with OS-specific coverage.

---

## **Risk Notes**
- **Security:** Enabling `shell: true` in `spawn` introduces significant command injection risk.  
- **Mitigation Suggestion:** Limit shell usage to Windows only or implement safer command execution alternatives.

---

## **Version Bump**
- `package.json` and `src/index.ts` updated to `1.1.5`.

---

## **PR Checklist**
- [ ] Confirm `shell: true` is restricted to Windows only or secured.
- [ ] Verify new prompt handling works across OSes.
- [ ] Run full test suite on Windows, macOS, and Linux.
- [ ] Review caching logic changes for performance impact.
- [ ] Validate updated documentation is accurate.

Copy link

Thank you for the comprehensive summary of your changes! It's very helpful to see the detailed breakdown of the bug fixes, CI/CD improvements, caching updates, documentation, and new tests.

I noticed the openhands-ai[bot] previously flagged failing GitHub Actions. Your summary indicates you've made improvements to .github/workflows/ci.yml by adding a lint step, enabling npm caching, and updating the job name. This is great progress towards resolving those issues.

If the CI is still failing after these changes, please let me know, and I can take another look to help debug. Ensuring the CI passes is crucial for merging this PR.

I also want to echo your Risk Notes regarding the shell: true setting in spawn. It's a critical security concern, and your suggestion to limit its usage to Windows only or implement safer alternatives is very important. It's good to see this highlighted in your PR checklist as well.

@jamubc jamubc requested a review from Copilot August 11, 2025 11:16
Copilot

This comment was marked as outdated.

@jamubc jamubc requested a review from Copilot August 11, 2025 11:34
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

This PR improves Windows compatibility for the gemini-cli MCP server by fixing shell command execution issues and implementing proper cross-platform handling.

  • Enhanced command execution with stdin support and proper Windows shell handling
  • Improved quote escaping for cross-platform compatibility
  • Added Windows-specific compatibility notes in troubleshooting documentation

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/utils/commandExecutor.ts Added Windows shell support and stdin handling for command execution
src/utils/geminiExecutor.ts Implemented platform-specific quote escaping and stdin-based prompt handling
src/utils/chunkCache.ts Enhanced cache file handling with timestamp-based expiration
tests/geminiExecutor.test.js Added integration tests for Gemini CLI functionality
package.json Updated version to 1.1.5 and added test script
docs/resources/troubleshooting.md Added Windows-specific troubleshooting guidance
CHANGELOG.md Added entry for Windows compatibility improvements
.github/workflows/* Enhanced CI/CD workflow structure and naming
Multiple version files Updated version numbers to 1.1.5

@@ -33,7 +33,7 @@ export function cacheChunks(prompt: string, chunks: EditChunk[]): string {

// Generate deterministic cache key from prompt
const promptHash = createHash('sha256').update(prompt).digest('hex');
const cacheKey = promptHash.slice(0, 8);
const cacheKey = promptHash.slice(0, 12); // Longer slice to reduce collision risk
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The change from 8 to 12 characters increases cache key length but lacks justification. Consider documenting the collision rate analysis that prompted this change or making the key length configurable.

Suggested change
const cacheKey = promptHash.slice(0, 12); // Longer slice to reduce collision risk
const cacheKey = promptHash.slice(0, CACHE_KEY_LENGTH); // Use configurable length

Copilot uses AI. Check for mistakes.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jamubc
Copy link
Owner Author

jamubc commented Aug 11, 2025

@alexx-ftw Any concerns?

@alexx-ftw
Copy link

Added some more minor improvements you might want to include

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.

2 participants