Skip to content

fix: agent storage issue on update#643

Merged
shadowfax92 merged 2 commits intomainfrom
fix/storage-clear-fix
Apr 3, 2026
Merged

fix: agent storage issue on update#643
shadowfax92 merged 2 commits intomainfrom
fix/storage-clear-fix

Conversation

@shadowfax92
Copy link
Copy Markdown
Contributor

  • fix: agent storage erase issue fix
  • fix: remove the guard against remote

@github-actions github-actions bot added the fix label Apr 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR fixes two related problems: BrowserOS extension chrome.storage.local data being erased during transient update-cycle uninstalls, and a guard that was incorrectly blocking remote extension loading. The storage fix skips DataDeleter::StartDeleting for BrowserOS extensions in PostUninstallExtension, and a new ReconstructPrefsFromInstalledExtensions fallback in the loader prevents orphan-detection from removing already-installed extensions when the installer returns empty prefs. A user-disable guard for BrowserOS extensions is also added to CanDisableExtension.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/cleanup suggestions with no blocking correctness issues

The core logic — skipping DataDeleter for BrowserOS extensions and reconstructing prefs from the registry — is sound and well-guarded. Remaining comments are about verbose logging (noisy but not harmful) and the fallback potentially masking future failures (best-practice concern, not a current breakage).

chrome_extension_registrar_delegate.cc and browseros_extension_maintainer.cc have LOG(INFO) calls worth trimming before this becomes production-stable logging

Important Files Changed

Filename Overview
packages/browseros/chromium_patches/chrome/browser/extensions/chrome_extension_registrar_delegate.cc Adds storage preservation for BrowserOS extensions during uninstall and a user-disable guard; both LOG(INFO) calls are noisy for routine operations
packages/browseros/chromium_patches/chrome/browser/browseros/extensions/browseros_extension_loader.cc Adds ReconstructPrefsFromInstalledExtensions fallback when prefs are empty; useful safety net but may silently mask persistent failures now that the root-cause storage fix is in place
packages/browseros/chromium_patches/chrome/browser/browseros/extensions/browseros_extension_loader.h Header declaration for ReconstructPrefsFromInstalledExtensions added correctly
packages/browseros/chromium_patches/chrome/browser/browseros/extensions/browseros_extension_maintainer.cc Full maintainer implementation; ForceUpdateCheck contains a per-extension logging loop that runs every 15 minutes and appears to be leftover debug instrumentation

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[StartLoading] --> B[BrowserOSExtensionInstaller]
    B --> C{OnInstallComplete}
    C -->|prefs non-empty| D[LoadFinished with prefs]
    C -->|prefs empty| E[ReconstructPrefsFromInstalledExtensions]
    E --> F[Walk ExtensionRegistry\nfor known BrowserOS IDs]
    F --> D
    D --> G[OnStartupComplete]
    G -->|from_bundled=true| H[InstallBundledExtensionsNow\n+2s delay]
    G -->|from_bundled=false| I[InstallRemoteExtensionsNow\n+2s delay]
    G --> J[BrowserOSExtensionMaintainer.Start\n+60s initial delay]
    J --> K[RunMaintenanceCycle\nevery 15 min]
    K --> L[FetchConfig]
    L --> M[ExecuteMaintenanceTasks]
    M --> M1[UninstallDeprecatedExtensions]
    M --> M2[ReinstallMissingExtensions]
    M --> M3[ReenableDisabledExtensions]
    M --> M4[ForceUpdateCheck]
    M --> M5[LogExtensionHealth]
    M --> K

    style E fill:#ffe0b2
    style F fill:#ffe0b2
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/browseros/chromium_patches/chrome/browser/extensions/chrome_extension_registrar_delegate.cc
Line: 36-41

Comment:
**Noisy log on every disable-check**

`CanDisableExtension` is called by the extension system on every UI interaction that touches extension state (extensions page, right-click menus, policy checks, etc.). Emitting `LOG(INFO)` here will flood the log with repetitive entries at normal browser usage rates.

```suggestion
  // - BrowserOS extensions cannot be disabled by users
  if (browseros::IsBrowserOSExtension(extension->id())) {
    return false;
  }
```

**Rule Used:** Remove excessive Logging.log statements after debu... ([source](https://app.greptile.com/review/custom-context?memory=e9511efb-1267-4789-ab21-7aa76d8ec478))

**Learnt From**
[browseros-ai/BrowserOS-agent#126](https://github.com/browseros-ai/BrowserOS-agent/pull/126)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/browseros/chromium_patches/chrome/browser/browseros/extensions/browseros_extension_maintainer.cc
Line: 327-336

Comment:
**Verbose per-extension logging on every maintenance cycle**

The loop that logs each extension's installed version/state runs inside `ForceUpdateCheck`, which fires every 15 minutes. This generates per-extension `LOG(INFO)` lines on every cycle and looks like leftover debug instrumentation. The actual update-check logic below the loop does not depend on the loop's output.

**Rule Used:** Remove excessive Logging.log statements after debu... ([source](https://app.greptile.com/review/custom-context?memory=e9511efb-1267-4789-ab21-7aa76d8ec478))

**Learnt From**
[browseros-ai/BrowserOS-agent#126](https://github.com/browseros-ai/BrowserOS-agent/pull/126)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/browseros/chromium_patches/chrome/browser/extensions/chrome_extension_registrar_delegate.cc
Line: 21-24

Comment:
**Noisy log on each update-cycle uninstall**

`PostUninstallExtension` is invoked transiently on every update cycle for BrowserOS extensions (exactly the scenario this PR is fixing). The `LOG(INFO)` here will print on every such cycle; consider removing or downgrading it once the fix is confirmed stable.

**Rule Used:** Remove excessive Logging.log statements after debu... ([source](https://app.greptile.com/review/custom-context?memory=e9511efb-1267-4789-ab21-7aa76d8ec478))

**Learnt From**
[browseros-ai/BrowserOS-agent#126](https://github.com/browseros-ai/BrowserOS-agent/pull/126)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/browseros/chromium_patches/chrome/browser/browseros/extensions/browseros_extension_loader.cc
Line: 94-100

Comment:
**Fallback reconstruction may mask chronic failures**

`ReconstructPrefsFromInstalledExtensions` is a useful safety net for the one-time startup race, but with the storage-preservation fix in `chrome_extension_registrar_delegate.cc` now landing in the same PR, extensions should no longer be erased during update cycles. If prefs are still empty after both the bundled CRX and remote paths succeed, that indicates a deeper problem that would be silently papered over here on every restart. Consider logging a metric or at least a `LOG(ERROR)` (rather than `LOG(WARNING)`) so the condition is visible in production monitoring.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: remove the guard against remote" | Re-trigger Greptile

@shadowfax92 shadowfax92 merged commit ff5386a into main Apr 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant