Fix deletion persistence bug (#2847, #1732, #2098, #2076, #2682) (AI) - #2924
Open
ndrewtl wants to merge 1 commit into
Open
Fix deletion persistence bug (#2847, #1732, #2098, #2076, #2682) (AI)#2924ndrewtl wants to merge 1 commit into
ndrewtl wants to merge 1 commit into
Conversation
Authored by Gemini 3.1 Pro
Author
|
Full transcript |
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 ScriptOpen 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
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
await browser.storage.sync.remove("siteContainerMap@@_example.com")
await browser.storage.local.remove("siteContainerMap@@_example.com") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
deletedSiteListvariable insrc/js/background/sync.jsthat causes a key to remain inbrowser.storage.sync, while also being indeletedSites, 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
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
deletedSiteListis reconciled against active sync storage during concurrent or stale device updates.When a user deletes a site assignment on Device A:
deletedSiteListon the sync server.browser.storage.syncarea.However, if Device B (which hasn't pulled this deletion yet) assigns or modifies a different rule before syncing:
sync.storageArea.backup(), which callsupdateSyncSiteAssignments().updateSyncSiteAssignments()iterates through all of Device B's local storage and blindly re uploads its entire local state to browser.storage.sync.At this point, the sync server is corrupted: The rule exists in BOTH the
deletedSiteListand the active sync storage simultaneously.When
reconcileSiteAssignments()runs on the next sync cycle (e.g., on browser restart):deletedSiteListfirst, deleting the rule from local storage.To make matters worse,
deleteSite()had a premature early return: if a rule was already present in thedeletedSiteList, the function aborted before callingsync.storageArea.area.remove(). Because the rule was already trapped in thedeletedSiteList, 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 thedeletedSiteList. If it is, the restoration is skipped. This ensures that even if a stale device accidentally re-uploads a deleted rule, thedeletedSiteListexplicitly overrides it, and the rule dies rather than propagating.2. Fixing the
deleteSiteearly return:The early return in
deleteSite()was modified. Even if a site is already in thedeletedSiteList, the function now continues execution to ensure awaitsync.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
deletedSiteListtakes 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.
Tag issues related to this pull request: