Proposal: Persistent and Resettable Tool Approvals #3652
varshneydevansh
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Issue
Linked PR
1. The Core Problem: A "Write-Only" Permission
A user (
@imadraude
) on the Gemini CLI GitHub filed Issue #3009 because they found a major usability problem.shell
). The user chose the "Yes, allow always..." option.gemini config --reset-approvals
or an entry they could remove from the~/.gemini/config.yaml
file.configstore
), not the user-facingconfig.yaml
. There was no command and no documented method to find and edit this file to undo the permission. The user was permanently stuck with the auto-approval.This created a poor user experience where a permission, once granted, could not be easily revoked.
2. The Proposed Solution: A Multi-Phase Fix
I thought took on this issue and created Pull Request #3201. My plan was comprehensive and addressed the user's problem directly, while also improving the feature's behavior.
Phase 1: The Emergency Reset (
--reset-tool-approvals
)The first and most crucial part was to give users an "escape hatch." This was a new command-line flag to solve the immediate problem.
How it works: When a user runs
gemini --reset-tool-approvals
, the CLI finds thesettings.json
file and clears the list of auto-approved tools.Code Example (from
integration-tests/reset_tool_approval.test.ts
):This new test file perfectly demonstrates the intended functionality.
Phase 2: Making "Allow Always" Persistent and Transparent
I also realized the original "always" was misleading—it only applied to the current session. The plan was to make it truly persistent.
How it works: When a user selects "Yes, allow always," the tool's name is saved to the
autoApprovedTools
array in the user-visible~/.gemini/settings.json
file. On all future runs, the CLI checks this list and will bypass the confirmation prompt if the tool is present.Code Example (from
packages/cli/src/gemini.tsx
):This code captures the user's choice in the UI and saves it.
3. The Hidden Dragon: The Circular Dependency
Implementing this feature revealed a major architectural flaw in the codebase: a circular dependency. This is where the real complexity of the pull request came from.
The Conflict:
packages/cli
) naturally depends on the application's core logic (packages/core
).CoreToolScheduler
(inpackages/core
) needed to know which tools were already approved. This meant it needed access to the settings.settings.ts
, which loads and savessettings.json
) was located inpackages/cli
.cli
depends oncore
, and nowcore
needs to depend oncli
. The build system cannot resolve this:cli -> core -> cli
.The Refactor:
The only way to fix this was to move the shared dependency to the more fundamental layer. The contributor correctly identified that all settings logic should live in
core
.This is what the file changes in your screenshot show.
packages/cli/src/config/settings.ts
andsettings.test.ts
.packages/core/src/config/settings.ts
andsettings.test.ts
are now the new home for this logic.This refactoring required changing import paths in over 50 files, which is why the pull request became so large (
+1,016 −5,820
lines).4. The Outcome: A Rejected (but valuable) Pull Request
A project collaborator (
NTaylorMullen
) reviewed the pull request and closed it. This was not because the code was bad, but for two key process-related reasons:5. "What Shall We Do?" - The Path Forward
The maintainer's feedback provides a clear roadmap for how to proceed successfully.
Start a Discussion First: Before writing any more code, open a new GitHub "Discussion" or a feature-request "Issue".
--reset-tool-approvals
command is the right solution for the immediate problem?"Break the PR into Smaller, Focused Changes: Do not submit one giant PR. Create a series of small, logical PRs that can be reviewed and merged independently.
PR #ONE: The Simple Fix. Create a PR that only adds the
--reset-tool-approvals
flag. It should not include the logic for persistent approvals. This directly fixes the original bug report, is a small change, and is highly likely to be accepted.PR #TWO: The Architectural Refactor. If the circular dependency must be fixed even for the simple reset command, submit that as a separate PR.
PR #THREE: The New Feature. Once the previous PRs are merged and the feature has been approved in the discussion, submit a final, small PR that introduces the persistent "Allow Always" logic.
This step-by-step approach is much safer, easier to review, and far more likely to be successfully merged into the project.
Beta Was this translation helpful? Give feedback.
All reactions