fix(tui): enhance sub-agent file write permissions and approval handling#1833
fix(tui): enhance sub-agent file write permissions and approval handling#1833knqiufan wants to merge 1 commit into
Conversation
- Approved sub-agents can now write files in delegated roles, allowing `general` and `implementer` children to inherit approved `Suggest` file-write tools after `agent_open` when `approval_mode=auto`. - Implemented a fail-fast mechanism for repeated approval blocks to improve responsiveness. - Added tests to verify the new approval logic and delegated write permissions for sub-agents. This change addresses issues with sub-agent tool execution and enhances the overall approval workflow.
![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 enables approved sub-agents, specifically those with general or implementer roles, to inherit file-write tools and ensures that approval_mode=auto is consistently propagated to child tool contexts. It also introduces a fail-fast mechanism that terminates sub-agents after three consecutive approval blocks to prevent infinite loops. Review feedback suggests implementing an RAII guard for managing resident leases to ensure they are released on all exit paths and recommends using structured error types instead of string matching for detecting approval blocks.
| &agent_id, | ||
| format!("step {steps}/{max_steps}: failed after repeated approval blocks"), | ||
| ); | ||
| release_resident_leases_for(&agent_id); |
There was a problem hiding this comment.
The manual call to release_resident_leases_for here is necessary for this early return, but the same release logic is missing in other early return paths within this function (specifically the Cancelled returns at lines 3370 and 3451). While those lines are not part of this diff, introducing a RAII guard at the start of run_subagent would be a more robust way to ensure leases are always released regardless of how the function exits.
| fn is_subagent_approval_block(result: &str) -> bool { | ||
| result.starts_with("Error:") && result.contains("requires approval") | ||
| } |
There was a problem hiding this comment.
This heuristic for detecting approval blocks relies on string matching which could be fragile if error messages are ever localized or refactored. While acceptable for now given the internal nature of these errors, consider using a structured error type or a specific error code in the future to make this detection more reliable.
2 participants
Summary
generalandimplementersub-agents to run delegatedSuggestfile-write tools afteragent_open.Requiredtools, including shell execution, gated behind parent auto-approval/YOLO mode.approval_mode=autowith childToolContext.auto_approveand fails fast after repeated approval-block errors.Testing
cargo test --all-featurescargo fmt --all -- --checkcargo clippy --all-targets --all-featuresChecklist