feat: ACP CLI provider support with Claude and Codex#22
Conversation
Drop Gemini ACP support, keeping only Claude and Codex providers. Update eino-acp dependency and use its command builders instead of hardcoded command slices.
Summary of ChangesHello, 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 refactors the AI analysis capabilities of the Highlights
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 integrates ACP (Agent Communication Protocol) providers for the workspace analyze command, replacing direct OpenRouter integration with a more flexible system supporting claude and codex backends via the eino-acp library. While the refactoring and new unit tests are positive, it introduces a critical security vulnerability by enabling automatic approval for AI agent actions with shell execution capabilities and exposure to untrusted input, which allows for arbitrary command execution via prompt injection. The most urgent fix is to disable auto-approval for sensitive tools and implement stricter controls over the agent's execution environment. Additionally, minor documentation inconsistencies were found in README.md regarding the gemini provider.
| chatModel, err := einoacp.NewChatModel(ctx, &einoacp.Config{ | ||
| Command: command, | ||
| Cwd: absDir, | ||
| AutoApprove: true, | ||
| }) |
There was a problem hiding this comment.
The AI agent is configured with AutoApprove: true, which allows it to execute tools without any human intervention. Combined with the fact that the agent is provided with an execute tool (which can run shell commands) and processes untrusted input from both the user's question and the log files, this creates a critical security risk. An attacker could use prompt injection (either directly via the question or indirectly via a malicious log line) to trick the agent into executing arbitrary shell commands on the user's machine. This could lead to full system compromise, data exfiltration, or other malicious activities.
To remediate this, you should set AutoApprove: false to ensure that any sensitive action, especially shell command execution, requires explicit user approval. Additionally, consider restricting the execute tool to a safe subset of commands or running it within a restricted sandbox environment.
| chatModel, err := einoacp.NewChatModel(ctx, &einoacp.Config{ | |
| Command: command, | |
| Cwd: absDir, | |
| AutoApprove: true, | |
| }) | |
| chatModel, err := einoacp.NewChatModel(ctx, &einoacp.Config{ | |
| Command: command, | |
| Cwd: absDir, | |
| AutoApprove: false, | |
| }) |
| if question != "" { | ||
| userMessage = "Analyze the log files in the workspace. The user's question: " + question | ||
| } |
There was a problem hiding this comment.
The user-supplied question is directly concatenated into the userMessage sent to the AI agent. This is a classic prompt injection vector. An attacker could provide a malicious question designed to override the agent's instructions and force it to perform unauthorized actions, such as using the execute tool to run arbitrary commands. While natural language input is difficult to sanitize perfectly, you should implement safeguards to detect and prevent common prompt injection patterns, and more importantly, ensure that the agent's capabilities are strictly limited and require human approval for sensitive actions.
| - `OPENROUTER_API_KEY`: Required for `analyze` and `debug run` commands | ||
| - `OPENROUTER_API_KEY`: Required for semantic labeling in `workspace add-log` | ||
| - `MODEL_NAME`: Override default LLM model (default: `google/gemini-3-flash-preview`) | ||
| - Provider-specific auth for ACP agent CLI (for example Claude/Codex/Gemini CLI login credentials) |
There was a problem hiding this comment.
This line mentions Gemini as an example for provider-specific authentication. However, the pull request description states that Gemini support has been removed, and the implementation only supports claude and codex. To avoid confusion, please remove the reference to Gemini.
| - Provider-specific auth for ACP agent CLI (for example Claude/Codex/Gemini CLI login credentials) | |
| - Provider-specific auth for ACP agent CLI (for example Claude/Codex CLI login credentials) |
| | `workspace create <topic>` | Create a workspace under `~/.lapp/workspaces/` | | ||
| | `workspace list` | List all workspace topics | | ||
| | `workspace add-log --topic <topic> <file>` | Add log file and rebuild patterns/notes | | ||
| | `workspace analyze --topic <topic> [question]` | Run AI analysis (`--acp claude|codex|gemini`) | |
There was a problem hiding this comment.
The description for the workspace analyze command lists gemini as a supported provider. According to the pull request description and the implementation, Gemini support has been removed. Please update the documentation to only list the currently supported providers (claude and codex).
| | `workspace analyze --topic <topic> [question]` | Run AI analysis (`--acp claude|codex|gemini`) | | |
| | `workspace analyze --topic <topic> [question]` | Run AI analysis (`--acp claude|codex`) | |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd613eca30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `workspace create <topic>` | Create a workspace under `~/.lapp/workspaces/` | | ||
| | `workspace list` | List all workspace topics | | ||
| | `workspace add-log --topic <topic> <file>` | Add log file and rebuild patterns/notes | | ||
| | `workspace analyze --topic <topic> [question]` | Run AI analysis (`--acp claude|codex|gemini`) | |
There was a problem hiding this comment.
The command table says workspace analyze supports --acp ...|gemini, but BuildACPCommand only accepts claude and codex and returns an error for anything else (pkg/analyzer/provider.go), so users following this README path will hit a hard runtime failure when trying Gemini. Please keep the documented provider list consistent with the actual accepted values.
Useful? React with 👍 / 👎.
2 participants
Summary
workspace analyze, supporting Claude Code and Codex backendseino-acplibrary for provider command resolution instead of hardcoded command slicesTest plan
make buildpassesBuildACPCommandpass (go test ./pkg/analyzer/ -run TestBuildACP)lapp workspace analyze --topic <topic> --acp claudelapp workspace analyze --topic <topic> --acp codex