fix(review): address GNOME extension review findings

src/core/lifecycleScope.ts

@@ -50,6 +50,7 @@ class ManagedSourceImpl implements ManagedSource {
 
 export class LifecycleScope {
   private _teardowns: Teardown[] | null = [];
+  private _sources: ManagedSourceImpl[] | null = [];
 
   onDispose(teardown: Teardown): void {
     if (!this._teardowns) {
@@ -71,15 +72,30 @@ export class LifecycleScope {
 
   manageSource(remove: SourceRemover): ManagedSource {
     const source = new ManagedSourceImpl(remove);
-    this.onDispose(() => source.dispose());
+    if (!this._sources) {
+      source.dispose();
+      return source;
+    }
+
+    this._sources.push(source);
     return source;
   }
 
   dispose(): void {
-    if (!this._teardowns) return;
-    const teardowns = this._teardowns;
+    if (!this._teardowns || !this._sources) return;
+
+    // Main-loop sources are owned explicitly by the scope. Every module/widget disposes
+    // its scope from disable()/destroy(), so no timeout or idle callback survives teardown.
+    const ownedSources = this._sources;
+    const ownedTeardowns = this._teardowns;
+    this._sources = null;
     this._teardowns = null;
-    for (const teardown of teardowns.reverse()) {
+
+    for (const source of ownedSources.reverse()) {
+      source.dispose();
+    }
+
+    for (const teardown of ownedTeardowns.reverse()) {
       teardown();
     }
   }

src/core/mainLoop.ts

@@ -7,6 +7,12 @@ export function removeSource(sourceId: number): 0 {
   return 0;
 }
 
+/**
+ * Registers one replaceable GLib source with an owner's LifecycleScope.
+ *
+ * ManagedSource.replace() removes the previous source before creating another one,
+ * and LifecycleScope.dispose() removes every source still active on disable/destroy.
+ */
 export function createManagedSource(scope: LifecycleScope): ManagedSource {
   return scope.manageSource(removeSource);
 }

src/desktop/trayIcons/dbusMenu.ts

@@ -31,8 +31,6 @@ const DBUS_MENU_XML = `
   </interface>
 </node>`;
 
-const DBusMenuInterfaceInfo = Gio.DBusInterfaceInfo.new_for_xml(DBUS_MENU_XML);
-
 // @ts-ignore — _promisify is a GJS extension not reflected in .d.ts
 Gio._promisify(Gio.DBusProxy.prototype, 'init_async');
 // @ts-ignore
@@ -48,13 +46,15 @@ type MenuNode = {
 };
 
 export class DBusMenuClient {
+  private readonly _interfaceInfo: Gio.DBusInterfaceInfo;
   private _proxy: Gio.DBusProxy | null = null;
   private _busName: string;
   private _objectPath: string;
   private _cancellable: Gio.Cancellable;
   private _initialized = false;
 
   constructor(busName: string, objectPath: string) {
+    this._interfaceInfo = Gio.DBusInterfaceInfo.new_for_xml(DBUS_MENU_XML);
     this._busName = busName;
     this._objectPath = objectPath;
     this._cancellable = new Gio.Cancellable();
@@ -69,7 +69,7 @@ export class DBusMenuClient {
       g_name: this._busName,
       g_object_path: this._objectPath,
       g_interface_name: DBUS_MENU_IFACE,
-      g_interface_info: DBusMenuInterfaceInfo,
+      g_interface_info: this._interfaceInfo,
       g_flags: Gio.DBusProxyFlags.DO_NOT_LOAD_PROPERTIES,
     });
 

src/desktop/trayIcons/sniHost.ts

@@ -37,7 +37,6 @@ const SNI_ITEM_XML = `
   </interface>
 </node>`;
 
-const SniItemInterfaceInfo = Gio.DBusInterfaceInfo.new_for_xml(SNI_ITEM_XML);
 const MIN_PIXMAP_SIZE = 8;
 const SYMBOLIC_CHANNEL_TOLERANCE = 18;
 const SYMBOLIC_REQUIRED_RATIO = 0.92;
@@ -63,6 +62,7 @@ type SniHostOptions = {
 };
 
 export class SniHost {
+  private readonly _itemInterfaceInfo: Gio.DBusInterfaceInfo;
   private _entries = new Map<string, SniEntry>();
   private _pendingRegistrations = new Map<string, Gio.Cancellable>();
   private _callbacks: HostCallbacks;
@@ -71,6 +71,7 @@ export class SniHost {
   private _shouldRecolorSymbolicPixmaps: () => boolean;
 
   constructor(watcher: SniWatcher, callbacks: HostCallbacks, options: SniHostOptions = {}) {
+    this._itemInterfaceInfo = Gio.DBusInterfaceInfo.new_for_xml(SNI_ITEM_XML);
     this._watcher = watcher;
     this._callbacks = callbacks;
     this._getColorScheme = options.getColorScheme || (() => 'prefer-dark');
@@ -86,7 +87,7 @@ export class SniHost {
     const proxy = new Gio.DBusProxy({
       g_connection: Gio.DBus.session,
       g_interface_name: 'org.kde.StatusNotifierItem',
-      g_interface_info: SniItemInterfaceInfo,
+      g_interface_info: this._itemInterfaceInfo,
       g_name: busName,
       g_object_path: objectPath,
       g_flags: Gio.DBusProxyFlags.GET_INVALIDATED_PROPERTIES,

src/panel/auroraMenu.ts

@@ -52,11 +52,10 @@ const MENU_ICONS = {
 
 type MenuIconKey = keyof typeof MENU_ICONS;
 
-type MenuCommand = {
+type MenuAction = {
   title: string;
   iconName: string;
-  argv?: string[];
-  activate?: () => void;
+  activate: () => void;
 };
 
 type RecentSubmenuItem = PopupMenu.PopupSubMenuMenuItem & {
@@ -183,27 +182,28 @@ export class AuroraMenu extends Module {
     menu.removeAll();
     this._lockMenuWidth(menu);
 
-    let hasItems = this._addCommandIfVisible(menu, SHOW_ABOUT_KEY, {
+    let hasItems = this._addActionIfVisible(menu, SHOW_ABOUT_KEY, {
       title: _('About This PC'),
-      argv: ['gnome-control-center', 'about'],
       iconName: 'help-about-symbolic',
+      activate: () => this._spawn(['gnome-control-center', 'about']),
     });
 
     const filesAdded = await this._addSection(
       menu,
       hasItems,
       [
         () =>
-          this._addCommandIfVisible(menu, SHOW_HOME_KEY, {
+          this._addActionIfVisible(menu, SHOW_HOME_KEY, {
             title: _('Home Folder'),
-            argv: ['xdg-open', GLib.get_home_dir()],
             iconName: 'user-home-symbolic',
+            activate: () => this._spawn(['xdg-open', GLib.get_home_dir()]),
           }),
         () =>
-          this._addCommandIfVisible(menu, SHOW_DOWNLOADS_KEY, {
+          this._addActionIfVisible(menu, SHOW_DOWNLOADS_KEY, {
             title: _('Downloads'),
-            argv: ['xdg-open', this._getDownloadsDirectory() || GLib.get_home_dir()],
             iconName: 'folder-download-symbolic',
+            activate: () =>
+              this._spawn(['xdg-open', this._getDownloadsDirectory() || GLib.get_home_dir()]),
           }),
         () => this._addRecentItems(menu, cancellable),
       ],
@@ -218,19 +218,20 @@ export class AuroraMenu extends Module {
       hasItems,
       [
         () =>
-          this._addCommandIfVisible(menu, SHOW_SETTINGS_KEY, {
+          this._addActionIfVisible(menu, SHOW_SETTINGS_KEY, {
             title: _('System Settings'),
-            argv: ['gnome-control-center'],
             iconName: 'emblem-system-symbolic',
+            activate: () => this._spawn(['gnome-control-center']),
           }),
         () =>
-          this._addCommandIfVisible(menu, SHOW_SOFTWARE_KEY, {
+          this._addActionIfVisible(menu, SHOW_SOFTWARE_KEY, {
             title: _('Software'),
-            argv: this._parseCommand(APP_STORE_COMMAND_KEY, ['gnome-software']),
             iconName: 'system-software-install-symbolic',
+            activate: () =>
+              this._spawn(this._parseCommand(APP_STORE_COMMAND_KEY, ['gnome-software'])),
           }),
         () =>
-          this._addCommandIfVisible(menu, SHOW_EXTENSIONS_KEY, {
+          this._addActionIfVisible(menu, SHOW_EXTENSIONS_KEY, {
             title: _('Extensions'),
             iconName: 'application-x-addon-symbolic',
             activate: () => this._openExtensionsManager(),
@@ -247,24 +248,21 @@ export class AuroraMenu extends Module {
     );
   }
 
-  private _addCommand(menu: PopupMenu.PopupMenu, command: MenuCommand): void {
-    const item = new PopupMenu.PopupMenuItem(command.title);
-    this._decorateItem(item, command.iconName);
-    item.connect('activate', () => {
-      if (command.activate) command.activate();
-      else if (command.argv) this._spawn(command.argv);
-    });
+  private _addAction(menu: PopupMenu.PopupMenu, action: MenuAction): void {
+    const item = new PopupMenu.PopupMenuItem(action.title);
+    this._decorateItem(item, action.iconName);
+    item.connect('activate', action.activate);
     menu.addMenuItem(item);
   }
 
-  private _addCommandIfVisible(
+  private _addActionIfVisible(
     menu: PopupMenu.PopupMenu,
     visibleKey: string,
-    command: MenuCommand,
+    action: MenuAction,
   ): boolean {
     if (!this.context.settings.getBoolean(visibleKey)) return false;
 
-    this._addCommand(menu, command);
+    this._addAction(menu, action);
     return true;
   }
 
@@ -302,10 +300,11 @@ export class AuroraMenu extends Module {
       const argv = this._parseCommandLine(command.command, CUSTOM_ITEMS_KEY);
       if (argv.length === 0) continue;
 
-      this._addCommand(menu, {
+      this._addAction(menu, {
         title: command.label,
-        argv,
         iconName: 'application-x-executable-symbolic',
+        // The final argv is exactly the result of GLib.shell_parse_argv(); no shell is involved.
+        activate: () => this._spawn(argv),
       });
       added = true;
     }

tests/unit/core/lifecycleScope.test.ts

@@ -85,6 +85,35 @@ test('LifecycleScope — disposes an active managed source', () => {
   assert.deepEqual(removed, [9]);
 });
 
+test('LifecycleScope — disposes every active source in reverse registration order', () => {
+  const removed: number[] = [];
+  const scope = new LifecycleScope();
+  const first = scope.manageSource((id) => removed.push(id));
+  const second = scope.manageSource((id) => removed.push(id));
+
+  first.replace(() => 1);
+  second.replace(() => 2);
+  scope.dispose();
+
+  assert.deepEqual(removed, [2, 1]);
+});
+
+test('LifecycleScope — remains idempotent when a source remover reenters dispose', () => {
+  const events: string[] = [];
+  const scope = new LifecycleScope();
+  const source = scope.manageSource((id) => {
+    events.push(`remove:${id}`);
+    scope.dispose();
+  });
+  scope.onDispose(() => events.push('teardown'));
+  source.replace(() => 5);
+
+  assert.doesNotThrow(() => scope.dispose());
+  scope.dispose();
+
+  assert.deepEqual(events, ['remove:5', 'teardown']);
+});
+
 test('LifecycleScope — does not create a managed source after disposal', () => {
   const removed: number[] = [];
   let created = false;