fix: improve policy persistence error handling and diagnostics (#19919) by Sikandar1310291 · Pull Request #19921 · google-gemini/gemini-cli

Sikandar1310291 · 2026-02-22T16:20:00Z

Summary

Fixes #19919 — Users on Linux (aarch64) receive "✕ Failed to persist policy for run_shell_command" when selecting "allow for all future sessions", despite the ~/.gemini/policies directory being writable and enablePermanentToolApproval being enabled.

This PR improves the policy persistence logic with better error handling and diagnostics in three ways:

Atomic write diagnostics — Provides richer error messages when the write fails, helping users understand whether the issue is a permissions problem, a filesystem constraint, or something else.
EXDEV + EBUSY cross-device fallback — When fs.rename throws EXDEV (cross-mount) or EBUSY (emulated/Docker volume lock), the code gracefully falls back to copyFile + unlink. This fixes policy persistence on Docker volume mounts and other emulated environments.
Corrupted TOML recovery — If auto-saved.toml contains invalid TOML (e.g., from a partial/interrupted write), the CLI no longer bricks itself. The corrupted file is automatically backed up to auto-saved.toml.bak and a user-visible warning is emitted, while policy persistence resumes from a clean empty state.

Details

The core change is in packages/core/src/policy/config.ts inside the createPolicyUpdater function:

Read path: Added an inner try/catch specifically around toml.parse(). On parse failure, the corrupted file is backed up with .bak suffix, a warning is surfaced via coreEvents.emitFeedback, and persistence continues with existingData = {}.
Write path: The catch on fs.rename now additionally handles EBUSY alongside the existing EXDEV handler, triggering the same copyFile + unlink fallback.

Both fixes are covered by new unit tests in policy-updater.test.ts.

Related Issues

Closes #19919

How to Validate

EBUSY scenario (Docker): Run Gemini CLI inside a Docker container with a bound-mount workspace. Select "Allow for all future sessions" on any shell command. Verify the policy is persisted in auto-saved.toml without error.
Corrupted TOML scenario: Manually corrupt ~/.gemini/policies/auto-saved.toml with invalid content (e.g., invalid;;). Launch Gemini CLI and select "Allow for all future sessions". Verify that:
- A warning message about the corrupted file being backed up appears.
- auto-saved.toml.bak is created with the old corrupt content.
- auto-saved.toml now contains the freshly written valid rule.
Unit tests: Run npm test -w @google/gemini-cli-core -- src/policy/policy-updater.test.ts — all 8 tests should pass including the two new ones.

Pre-Merge Checklist

…e-gemini#19919) - Include actual error cause in user-visible feedback message - Clean up orphaned tmp files when write operations fail - Abort persistence on non-ENOENT read errors (e.g. EACCES) to prevent silently overwriting existing policy files - Add EXDEV cross-device rename fallback (copy + unlink) - Fix existing tests to use proper ENOENT errors - Add 4 new tests for error diagnostics, cleanup, EACCES abort, and EXDEV fallback

gemini-code-assist · 2026-02-22T16:20:21Z

Summary of Changes

Hello @Sikandar1310291, I'm Gemini Code Assist¹! 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 enhances the robustness and user experience of the policy persistence mechanism. It addresses several edge cases and common failure scenarios by providing more detailed error messages, preventing data loss through silent overwrites, ensuring proper cleanup of temporary files, and adding compatibility for cross-device file operations. The changes make the policy persistence more resilient and easier to diagnose when issues arise.

Highlights

Improved Error Diagnostics: The user-visible error message for policy persistence failures now includes the specific reason for the failure (e.g., 'Permission denied'), providing more actionable information than a generic 'Failed to persist policy'.
Orphaned Temporary File Cleanup: Introduced logic to ensure that temporary files created during policy persistence are reliably cleaned up, even if write operations fail, preventing accumulation of unused files.
Prevented Silent Data Overwrites: Modified the policy persistence mechanism to abort the operation on non-ENOENT read errors (e.g., EACCES). Previously, such errors could lead to silently overwriting existing policy files with empty data, which is now prevented by throwing the error.
Cross-Device Rename Fallback: Implemented a fallback mechanism for fs.rename operations. If fs.rename fails with an EXDEV error (indicating a cross-device link issue, common in certain Linux configurations), the system now gracefully falls back to fs.copyFile followed by fs.unlink to ensure persistence across different file systems.
Enhanced Test Coverage: Added four new unit tests to specifically cover the improved error handling, temporary file cleanup, non-ENOENT read error abortion, and the EXDEV rename fallback, ensuring the robustness of these new features.

Changelog

packages/core/src/policy/config.ts
- Removed unused debugLogger import.
- Declared tmpFile with let to allow broader scope for cleanup.
- Modified fs.readFile error handling to re-throw non-ENOENT errors, preventing silent overwrites.
- Implemented a try-catch block around fs.rename to handle EXDEV errors by falling back to fs.copyFile and fs.unlink.
- Added logic to clean up temporary files in the main catch block.
- Updated error feedback messages to include the specific reason for persistence failures.
packages/core/src/policy/persistence.test.ts
- Added a makeNodeError helper function to create Node.js-style errors with a code property for testing.
- Updated existing tests to use makeNodeError for simulating ENOENT errors, aligning with the new error handling logic.
- Added a new test case to verify that error feedback messages include detailed reasons for persistence failures.
- Added a new test case to confirm that temporary files are cleaned up on write failures.
- Added a new test case to ensure persistence is aborted on non-ENOENT read errors.
- Added a new test case to validate the EXDEV rename fallback to copy and unlink.

Activity

No specific activity (comments, reviews, progress updates) has been recorded for this pull request since its creation.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. 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.

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. ↩

gemini-code-assist

Code Review

This pull request significantly improves the error handling and diagnostics for policy persistence. The changes are well-implemented and address several important issues, such as preventing data loss from non-ENOENT read errors and ensuring temporary files are cleaned up on failure. The fallback mechanism for cross-device rename operations is a robust addition. The new unit tests are comprehensive and correctly validate all the fixes. Overall, this is an excellent contribution that increases the reliability of policy management.

Abhijit-2592 · 2026-02-26T19:51:47Z

+              // Fall back to copy + unlink which works across filesystems.
+              if (isNodeError(renameError) && renameError.code === 'EXDEV') {
+                await fs.copyFile(tmpFile, policyFile);
+                await fs.unlink(tmpFile);


Catch unlink errors: await fs.unlink(tmpFile).catch(() => {}); As the policyFile was successfully renamed in the previous line.

Abhijit-2592 · 2026-02-26T20:11:22Z

+      persist: true,
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 0));


Prefer vi.waitFor as fixed wait can introduce flakiness. Also please change in other places

Abhijit-2592 · 2026-02-26T20:21:12Z

+    (fs.mkdir as unknown as Mock).mockRejectedValue(
+      new Error('Permission denied'),
+    );


Prefer vi.mocked(fs.mkdir).mockRejectedValue(new Error('Permission denied'));. Also please change in other places accordingly

Abhijit-2592

Thanks for your contribution. Overall LGTM. I have some minor comments that needs to be addressed before merging

mrpmohiburrahman · 2026-03-03T20:24:22Z

+            } catch (renameError) {
+              // Cross-device rename fails with EXDEV on some Linux mount configurations.
+              // Fall back to copy + unlink which works across filesystems.
+              if (isNodeError(renameError) && renameError.code === 'EXDEV') {


Here, it explicitly checks for code === 'EXDEV'.

Earlier on line 518, tmpFile is created inside the exact same directory
as policyFile (${policyFile}.${tmpSuffix}.tmp). Because these two files
are intrinsically on the same filesystem, a POSIX rename operation will
never logically throw an EXDEV (Cross-device link) error in this specific scenario.

When users run into this bug structurally (often via single-file bind mounts
in Docker where the target inode is locked), Linux actually throws
EBUSY, not EXDEV. Because the logic explicitly checks for
EXDEV and the else block on line 537 re-throws everything else, an
EBUSY error will bypass your great fallback code entirely and still fail to
persist the file.

Suggestion: We should update the check on line 533 to handle both codes:
if (isNodeError(renameError) && (renameError.code === 'EXDEV' || renameError.code === 'EBUSY'))

Let me know what you think.

mrpmohiburrahman · 2026-03-03T21:17:01Z

            } catch (error) {
-              if (!isNodeError(error) || error.code !== 'ENOENT') {
-                debugLogger.warn(
-                  `Failed to parse ${policyFile}, overwriting with new policy.`,
-                  error,
-                );
+              if (isNodeError(error) && error.code === 'ENOENT') {
+                // File doesn't exist yet, start fresh
+              } else {
+                // Non-ENOENT read errors (e.g. EACCES) should abort persistence
+                // to avoid silently overwriting the existing file with empty data
+                throw error;


If a user manually edits their policy file and accidentally introduces a typo
that breaks the TOML format (like forgetting a quote mark), toml.parse() on
line 458 throws a generic Error.

Because a generic parsing error doesn't have a .code property, the new
if (isNodeError(error)) check on line 467 returns false. The new logic
then falls through to the else block on line 472 and throws, completely
aborting the save process.

This traps the CLI in a permanently "bricked" state. Every time the user clicks
"Always Allow" in the future, the app crashes behind the scenes and says "Failed to persist policy." It will never auto-save again until they manually find and delete the corrupted file.

Suggestion (Safe Backup & Recover): To preserve data and keep the app working, we should catch TOML parsing errors specifically. Instead of aborting (the new way) or deleting their old rules (the old way), the gold standard is to rename the corrupted file to auto-saved.toml.bak so they don't lose data, start a fresh existingData object so the save can succeed, and use emitFeedback('warning', ...) to tell the user we safely backed up their broken file!

Here is one way to do It like this, inside the catch on line 466:

} catch (error) { if (isNodeError(error) && error.code === 'ENOENT') { // File doesn't exist yet, start fresh } else if (!isNodeError(error)) { // This is a parsing error (no Error.code) coreEvents.emitFeedback( 'warning', `Syntax error found in policy file. Backing up corrupted file to ${policyFile}.bak and starting fresh.` ); await fs.copyFile(policyFile, `${policyFile}.bak`).catch(() => {}); existingData = {}; // Clear existingData so the new command overrides it } else { // This is a real filesystem error (e.g. EACCES). Throw to prevent silent failure. throw error; } }

Let me know what you think.

Sikandar1310291 · 2026-03-04T17:03:26Z

Hi @Abhijit-2592, thanks for the review!

I've addressed all the feedback and pushed the updates:

Used vi.waitFor instead of fixed timeout waits to avoid flakiness in tests.
Caught unlink errors (await fs.unlink(tmpFile).catch(() => {});) after successfully renaming the policy file.
Used vi.mocked(...).mockRejectedValue(...) as suggested.
Fixed the CI pipeline failure: The failing tests on GitHub were due to a TypeScript compilation error (error TS2345) related to the FileHandle mock. I've explicitly cast mockFileHandle as unknown as fs.FileHandle to satisfy the TypeScript compiler.

All local tests and type checks are passing now. Let me know if there's anything else needed!

krishdef7 · 2026-03-06T03:49:12Z

Thanks for addressing the earlier review feedback, the changes look good.

I also noticed the branch currently has merge conflicts with main.

If it helps, I’d be happy to help resolve the conflicts and push an updated commit on top of this PR, along with the two remaining review suggestions from @mrpmohiburrahman (the EBUSY fallback and the TOML parse recovery).

Let me know if you'd like me to take care of that.

cocosheng-g · 2026-04-09T23:24:42Z

Hi @Sikandar1310291, this PR is being closed due to merge conflicts and 34 days of inactivity. To keep the review queue manageable, we are closing stale blocked PRs. Please feel free to re-open this PR or open a new one once the issues are resolved! Thank you for your contribution.

Sikandar1310291 requested a review from a team as a code owner February 22, 2026 16:20

gemini-code-assist Bot reviewed Feb 22, 2026

View reviewed changes

gemini-cli Bot added the area/core Issues related to User Interface, OS Support, Core Functionality label Feb 22, 2026

SandyTao520 force-pushed the main branch from 4a78a96 to 0bf3a01 Compare February 26, 2026 03:03

gemini-cli Bot added priority/p2 Important but can be addressed in a future release. help wanted We will accept PRs from all issues marked as "help wanted". Thanks for your support! labels Feb 26, 2026

Abhijit-2592 reviewed Feb 26, 2026

View reviewed changes

krishdef7 mentioned this pull request Feb 27, 2026

"Failed to persist policy" error despite correct permissions #19919

Closed

fix(policy): address PR 19921 feedback on tests and config

4fdb081

mrpmohiburrahman reviewed Mar 3, 2026

View reviewed changes

test: fix TypeScript compile error in FileHandle mock

9ef7b6f

krishdef7 mentioned this pull request Mar 7, 2026

fix(policy): add EBUSY fallback and TOML parse recovery (#19919) #21541

Merged

cocosheng-g closed this Apr 9, 2026

sripasg added the size/l A large sized PR label Jun 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: improve policy persistence error handling and diagnostics (#19919)#19921

fix: improve policy persistence error handling and diagnostics (#19919)#19921
Sikandar1310291 wants to merge 3 commits into
google-gemini:mainfrom
Sikandar1310291:fix/persist-policy-error-19919

Sikandar1310291 commented Feb 22, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot commented Feb 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Abhijit-2592 Feb 26, 2026 •

edited

Loading

Uh oh!

Abhijit-2592 Feb 26, 2026 •

edited

Loading

Uh oh!

Abhijit-2592 Feb 26, 2026 •

edited

Loading

Uh oh!

Abhijit-2592 left a comment

Uh oh!

mrpmohiburrahman Mar 3, 2026

Uh oh!

mrpmohiburrahman Mar 3, 2026

Uh oh!

Sikandar1310291 commented Mar 4, 2026

Uh oh!

krishdef7 commented Mar 6, 2026 •

edited

Loading

Uh oh!

cocosheng-g commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

Sikandar1310291 commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Related Issues

How to Validate

Pre-Merge Checklist

Uh oh!

gemini-code-assist Bot commented Feb 22, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Abhijit-2592 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abhijit-2592 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abhijit-2592 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abhijit-2592 left a comment

Choose a reason for hiding this comment

Uh oh!

mrpmohiburrahman Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

mrpmohiburrahman Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Sikandar1310291 commented Mar 4, 2026

Uh oh!

krishdef7 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cocosheng-g commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Sikandar1310291 commented Feb 22, 2026 •

edited

Loading

Abhijit-2592 Feb 26, 2026 •

edited

Loading

Abhijit-2592 Feb 26, 2026 •

edited

Loading

Abhijit-2592 Feb 26, 2026 •

edited

Loading

krishdef7 commented Mar 6, 2026 •

edited

Loading