fix(core): make subagents aware of active approval modes#23608
fix(core): make subagents aware of active approval modes#23608
Conversation
🧠 Model Steering GuidanceThis PR modifies files that affect the model's behavior (prompts, tools, or instructions).
This is an automated guidance message triggered by steering logic signatures. |
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 enhances the core agent system by making subagents context-aware of global approval modes, specifically Plan Mode. This prevents subagents from attempting unauthorized operations, such as direct file edits, when operating under restrictions. By injecting explicit constraints into the system prompt and dynamically adjusting agent descriptions, the change aims to improve agent reliability and prevent them from getting stuck in failure loops, leading to more predictable and controlled agent behavior. Highlights
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. Footnotes
|
|
Size Change: +679 B (0%) Total Size: 26.5 MB
ℹ️ View Unchanged
|
![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 effectively makes subagents aware of Plan Mode by injecting execution constraints into the system prompt and dynamically adjusting the Generalist Agent's description. This should resolve the issue of subagents getting stuck in failure loops. The accompanying tests are well-written and validate the new behavior. However, as noted in the specific comment, there's a potential omission regarding Auto-Edit Mode, which was mentioned in the PR description but doesn't appear to be handled in the implementation.
| if (approvalMode === ApprovalMode.PLAN) { | ||
| const plansDir = this.context.config.storage.getPlansDir(); | ||
| finalPrompt += `\n\n# Execution Constraints\nYou are currently operating in Plan Mode. Your write tools are globally restricted to only modifying plan (.md) files in the plans directory: ${plansDir}/. Do not attempt to modify source code directly.`; | ||
| } |
There was a problem hiding this comment.
The pull request description states that subagents should be made aware of both Plan Mode and Auto-Edit Mode. This implementation correctly injects constraints for Plan Mode, but Auto-Edit Mode is not handled. If Auto-Edit Mode imposes global restrictions that could cause subagents to fail (similar to the issue this PR fixes for Plan Mode), its constraints should also be injected into the system prompt here. Consider adding an else if (approvalMode === ApprovalMode.AUTO_EDIT) block to provide the necessary context for that mode.
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of subagents being unaware of global approval modes, particularly Plan Mode. The changes to inject context into the LocalAgentExecutor system prompt and dynamically adjust the Generalist Agent description are well-implemented and correctly tested. I have one suggestion to improve the maintainability of the generalist-agent.ts file by reducing code duplication.
| get description() { | ||
| if (context.config.getApprovalMode() === ApprovalMode.PLAN) { | ||
| return 'A general-purpose AI agent with access to all tools. Highly recommended for tasks that are turn-intensive or involve processing large amounts of data. Use this to keep the main session history lean and efficient. Excellent for: large-scale investigation and batch planning across multiple files.'; | ||
| } | ||
| return 'A general-purpose AI agent with access to all tools. Highly recommended for tasks that are turn-intensive or involve processing large amounts of data. Use this to keep the main session history lean and efficient. Excellent for: batch refactoring/error fixing across multiple files, running commands with high-volume output, and speculative investigations.'; | ||
| }, |
There was a problem hiding this comment.
To improve maintainability and reduce code duplication, you can refactor the description getter. The two long description strings are mostly identical except for the 'Excellent for:' part. By extracting the common base string, you can make the code more concise and easier to maintain, preventing the shared text from diverging in the future.
get description() {
const baseDescription =
'A general-purpose AI agent with access to all tools. Highly recommended for tasks that are turn-intensive or involve processing large amounts of data. Use this to keep the main session history lean and efficient. Excellent for: ';
if (context.config.getApprovalMode() === ApprovalMode.PLAN) {
return `${baseDescription}large-scale investigation and batch planning across multiple files.`;
}
return `${baseDescription}batch refactoring/error fixing across multiple files, running commands with high-volume output, and speculative investigations.`;
},
There was a problem hiding this comment.
+1
This helps make it more readable
There was a problem hiding this comment.
Refactored this to reduce duplication as suggested.
gemini-cli
bot
added
area/agent
| 'A general-purpose AI agent with access to all tools. Highly recommended for tasks that are turn-intensive or involve processing large amounts of data. Use this to keep the main session history lean and efficient. Excellent for: batch refactoring/error fixing across multiple files, running commands with high-volume output, and speculative investigations.', | ||
| get description() { | ||
| if (context.config.getApprovalMode() === ApprovalMode.PLAN) { | ||
| return 'A general-purpose AI agent with access to all tools. Highly recommended for tasks that are turn-intensive or involve processing large amounts of data. Use this to keep the main session history lean and efficient. Excellent for: large-scale investigation and batch planning across multiple files.'; |
There was a problem hiding this comment.
For this piece, what is the delta? the "excellent for ..." piece is the only difference I see.
Did you notice any change with this specific ablation for plan mode?
There was a problem hiding this comment.
The delta is indeed just the 'excellent for...' part. In testing, without this change, the main agent would often delegate 'fix these 5 files' tasks to the Generalist even in Plan Mode because the description still advertised it as being excellent for refactoring. By updating the description, we guide the main agent to only use the Generalist for investigative or planning tasks that are permissible in Plan Mode, rather than having it spawn a subagent that immediately hits execution constraints.
| const approvalMode = this.context.config.getApprovalMode(); | ||
| if (approvalMode === ApprovalMode.PLAN) { | ||
| const plansDir = this.context.config.storage.getPlansDir(); | ||
| finalPrompt += `\n\n# Execution Constraints\nYou are currently operating in Plan Mode. Your write tools are globally restricted to only modifying plan (.md) files in the plans directory: ${plansDir}/. Do not attempt to modify source code directly.`; |
There was a problem hiding this comment.
If we have this, why is the change in the generalist needed?
I can see that the two pieces of instructions are conflicting, but that is essentially what the generalist is made for. It is ideally an instance of gemini cli itself.
If a user doesn't want that in their plan mode, it seems better to discourage using / giving plan mode access to the generalist
There was a problem hiding this comment.
The change in the generalist is primarily for better routing at the main agent level. While local-executor provides the 'safety net' by enforcing constraints, the dynamic description helps the main agent (the 'orchestrator') make better decisions about when to invoke the Generalist. If the description remains 'Excellent for: batch refactoring', the main agent will continue to try and use it for that purpose, leading to wasted turns and confusing failure loops for the user.
2 participants
Summary
Make subagents explicitly aware of global Approval Modes (Plan Mode, Auto-Edit Mode) to prevent them from attempting globally blocked actions. This fixes an issue where subagents would get stuck in failure loops in Plan Mode due to lack of context.
Details
LocalAgentExecutorsystem prompt when the globalApprovalModeisPLAN.Generalist Agentdescription inPLANmode to avoid misleading routing for batch refactoring tasks.Related Issues
Fixes #23582
How to Validate
npm test -w @google/gemini-cli-core -- src/agents/generalist-agent.test.ts src/agents/local-executor.test.ts/plan), invoke a subagent, and verify it does not attempt unauthorized file edits and explicitly knows it is in plan mode.Pre-Merge Checklist