Skip to content

Fix deletion persistence bug (#2847, #1732, #2098, #2076, #2682) (AI) - #2924

Open
ndrewtl wants to merge 1 commit into
mozilla:mainfrom
ndrewtl:fix-deletion-persistence-bug
Open

Fix deletion persistence bug (#2847, #1732, #2098, #2076, #2682) (AI)#2924
ndrewtl wants to merge 1 commit into
mozilla:mainfrom
ndrewtl:fix-deletion-persistence-bug

Conversation

@ndrewtl

@ndrewtl ndrewtl commented Jul 31, 2026

Copy link
Copy Markdown

Description

I am a longtime user and fan of this project who has been haunted by issue #2847 for a long time. It also appears that issues #1732, #2098, #2076, and #2682 have the same root cause. I am a dev, but do not have experience with Firefox browser extensions. In a fit of frustration, I asked an LLM (Gemini 3.1 Pro) to attempt to diagnose and solve the issue. Gemini 3.1 diagnosed an issue– improper iteration over the deletedSiteList variable in src/js/background/sync.js that causes a key to remain in browser.storage.sync, while also being in deletedSites, which causes the previous configuration to 'resurrect' after syncing. I ran another LLM (GPT 5.6-Luna) over the codebase independently and it diagnosed the same issue.

The LLM proposed a five-line fix, which should be reviewable by any of the maintainers. I freely admit that I don't understand how this extension works, but I believe there is a good chance that this fix is correct, because

  1. Two separate AI's provided very similar patches without collaborating. If they were confabulating, I would expect them to suggest different changes, not similar changes in the same part of the codebase.
  2. The first AI provided a workaround that allowed me to successfully fix the bug via the Dev console, which prevented the rules from resurrecting. This strongly signals that the AI understands the root cause of the issue.

I have not run this code myself, as I'm not sure I understand even how to build this project. Therefore, it is possible that this patch is incorrect, which is why I would ask for any existing project maintainers to read it over and understand. I have also attached a longform transcript of my conversation with Gemini, for transparency's sake.

If this patch is genuinely incorrect, a reviewer should be able to reject it quite easily. On the other hand, if it is correct, it will ease the burden of many people who have struggled with the above-mentioned bugs, not least myself. I understand that project maintainers might not want to deal with AI slop. However I think the reward / tradeoff here is such that a very small amount of dev time might save many users from frustration.

I asked Gemini to describe its diagnosis and fix. What follows below is its summary of the content and change it applied.

AI Summary

This PR addresses a long-standing and highly reported synchronization bug (the "Zombie Rules" bug) that causes deleted containers and site assignments to permanently resurrect themselves across devices, despite repeated deletions by the user.

1. The Root Cause of the Bug

The underlying cause is a state inconsistency triggered by the way the deletedSiteList is reconciled against active sync storage during concurrent or stale device updates.

When a user deletes a site assignment on Device A:

  1. The site is added to the deletedSiteList on the sync server.
  2. The site is removed from the active browser.storage.sync area.

However, if Device B (which hasn't pulled this deletion yet) assigns or modifies a different rule before syncing:

  1. Device B triggers sync.storageArea.backup(), which calls updateSyncSiteAssignments().
  2. updateSyncSiteAssignments() iterates through all of Device B's local storage and blindly re uploads its entire local state to browser.storage.sync.
  3. Because Device B has not yet removed the original deleted rule locally, it accidentally re-uploads the deleted rule back to the sync server's active storage.

At this point, the sync server is corrupted: The rule exists in BOTH the deletedSiteList and the active sync storage simultaneously.

When reconcileSiteAssignments() runs on the next sync cycle (e.g., on browser restart):

  1. It processes deletedSiteList first, deleting the rule from local storage.
  2. In the very next block, it iterates over the active sync storage, finds the re-uploaded rule, and immediately restores it to local storage.
  3. This triggers another backup, keeping the rule alive forever.

To make matters worse, deleteSite() had a premature early return: if a rule was already present in the deletedSiteList, the function aborted before calling sync.storageArea.area.remove(). Because the rule was already trapped in the deletedSiteList, any subsequent attempts by the user to delete the resurrected rule would abort early and fail to remove the rule from active sync storage, cementing its zombie status.

2. How this Fix works

This PR introduces two logic changes to break the resurrection loop:
1. Explicit skipping in the restoration loop:
Inside reconcileSiteAssignments(), the loop that iterates over active sync storage now explicitly checks if the site is present in the deletedSiteList. If it is, the restoration is skipped. This ensures that even if a stale device accidentally re-uploads a deleted rule, the deletedSiteList explicitly overrides it, and the rule dies rather than propagating.
2. Fixing the deleteSite early return:
The early return in deleteSite() was modified. Even if a site is already in the deletedSiteList, the function now continues execution to ensure await sync.storageArea.area.remove(siteStoreKey) is called. This guarantees that deleting a zombie rule actually wipes it from the active sync storage, allowing the user to permanently kill rules that were previously trapped in this state.

3. Why this fixes the bug:

By enforcing that the deletedSiteList takes strict precedence over conflicting active assignments, we prevent stale devices from resurrecting deleted rules. By fixing the deletion function, we allow users to permanently clear corrupted rules from sync storage regardless of their historical presence in the deleted list.

Type of change

Select all that apply.

  • Bug fix
  • New feature
  • Major change (fix or feature that would cause existing functionality to work differently than in the current version)

Tag issues related to this pull request:

Authored by Gemini 3.1 Pro
@ndrewtl

ndrewtl commented Jul 31, 2026

Copy link
Copy Markdown
Author

transcript_full.jsonl.zip

Full transcript

@ndrewtl

ndrewtl commented Jul 31, 2026

Copy link
Copy Markdown
Author

I am also attaching the workaround / fix that I applied without updating the code. This allowed me to identify and "hard delete" URL's that were locked to a particular site, and it did succeed. This might be useful to @devurandom as the originator of the bug I am experiencing. The following is all AI output.

Diagnostic Script

Open the Extension Console (about:debugging#/runtime/this-firefox -> Inspect on Multi-Account Containers -> Console tab) and paste this snippet:

    (async () => {                                                                                                                                                                      
      const syncData = await browser.storage.sync.get();                                                                                                                                
      const deletedSites = syncData.deletedSiteList || [];                                                                                                                              
      const zombies = [];                                                                                                                                                               
                                                                                                                                                                                        
      for (const key of Object.keys(syncData)) {                                                                                                                                        
        if (key.startsWith("siteContainerMap@@_") && deletedSites.includes(key)) {                                                                                                      
          zombies.push(key);                                                                                                                                                            
        }                                                                                                                                                                               
      }                                                                                                                                                                                 
                                                                                                                                                                                        
      if (zombies.length === 0) {                                                                                                                                                       
        console.log("✅ State is clean. No state inconsistencies detected.");                                                                                                           
      } else {                                                                                                                                                                          
        console.warn(`⚠️ Found ${zombies.length} inconsistent (zombie) rules!`);                                                                                                        
        zombies.forEach(zKey => {                                                                                                                                                       
            console.log("Zombie URL:", zKey.replace("siteContainerMap@@_", ""));                                                                                                        
        });                                                                                                                                                                             
                                                                                                                                                                                        
        console.log("\nTo fix all of them automatically, copy and run the following command:");                                                                                         
        console.log(`await browser.storage.sync.remove(${JSON.stringify(zombies)})`);                                                                                                   
      }                                                                                                                                                                                 
    })();

How it works

  1. It downloads your entire browser.storage.sync database.
  2. It cross-references every active site rule against the deletedSiteList.
  3. If it finds an overlap, it identifies it as a "zombie" rule that will continually resurrect itself due to the bug.
  4. If it finds any, it dynamically generates the exact command you need to run to fix all of them at once in a single batch operation.

You can run this snippet anytime you suspect an assignment didn't delete properly to ensure your sync state remains clean until the upstream bug fix is released.

Fix: Delete the rule via Extension Console

  1. In Firefox, open a new tab and navigate to about:debugging#/runtime/this-firefox
  2. Scroll down to find Firefox Multi-Account Containers in your list of extensions.
  3. Click the Inspect button next to it. This will open a Developer Tools window for the extension.
  4. Go to the Console tab in the Developer Tools.
  5. You will need to know the exact domain name of the site (e.g., www.example.com or github.com). Run the following two commands in the console, pressing Enter after each (replace example.com with your actual domain):
    await browser.storage.sync.remove("siteContainerMap@@_example.com")                                                                                                                 
                                                                                                                                                                                        
    await browser.storage.local.remove("siteContainerMap@@_example.com") 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant