Conversation
shadowfax92
commented
Apr 3, 2026
- fix: agent storage erase issue fix
- fix: remove the guard against remote
Greptile SummaryThis PR fixes two related problems: BrowserOS extension Confidence Score: 5/5Safe 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
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
Prompt To Fix All With AIThis 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 |