fix(ego-lint): enhance GSettings signal leak detection for helper classes (#87)

## Summary

- Change R-LIFE-21 auto-cleanup detection from extension-wide to
**per-file**
- Previously, `connectObject`/`disconnectObject` in any file (e.g., for
widget signals) gave a free pass to bare `.connect('changed::...')`
calls in all other files
- Now auto-cleanup in one file does not suppress findings in other
files, because `connectObject`/`disconnectObject` manage signals
per-object-per-owner

**False negatives fixed:**
- media-controls: 28 bare GSettings handlers now detected (were missed
because `MenuSlider.js` used `connectObject` for widget signals)
- just-perfection: 1 bare handler in `SupportNotifier.js` now detected
(was missed because `Manager.js` used `settings.connectObject()`)

Closes #78

## Test plan

- [x] New fixture `gsettings-cross-file-leak@test` — bare connects in
`extension.js` with `connectObject` only in `helper.js` → FAIL
- [x] Existing fixtures unchanged: `gsettings-bare-connect@test` (FAIL),
`gsettings-auto-cleanup@test` (PASS), `gsettings-array-storage@test` (no
FP)
- [x] All 610 test assertions pass
- [x] Verified against field test cache: media-controls (28 hits),
just-perfection (1 hit), dash-to-panel (no regression)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: ZviBaratz

skills/ego-lint/references/rules-reference.md

@@ -2113,12 +2113,13 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 ### R-LIFE-21: GSettings signal leak (bare connect without disconnect)
 - **Severity**: blocking
 - **Checked by**: check-lifecycle.py
-- **Rule**: `settings.connect('changed::...')` calls where the return value is not stored (no `=` or `.push()` before `.connect` on the line) and no auto-cleanup mechanism (`disconnectObject`, `connectObject`, `SignalTracker`, `connectSmart`) is present in the extension.
+- **Rule**: `settings.connect('changed::...')` calls where the return value is not stored (no `=` or `.push()` before `.connect` on the line) and no auto-cleanup mechanism (`disconnectObject`, `connectObject`, `SignalTracker`, `connectSmart`) is present in the **same file**.
 - **Rationale**: Without a stored signal ID, there is no way to call `.disconnect(id)` later. These handlers accumulate across enable/disable cycles, causing duplicate callbacks, memory leaks, and potential crashes. This is the highest-impact gap found in field testing — 4 of 10 extensions had this issue.
 - **Fix**: Use `connectObject()` (recommended — auto-disconnects via `disconnectObject(this)` in disable) or store the return value and call `disconnect(id)` in `disable()`.
 - **Scope exclusions**: prefs.js (manages own lifecycle), `service/` directory (daemon lifecycle).
+- **Auto-cleanup detection**: Per-file (not extension-wide). `connectObject`/`disconnectObject` in a helper file does not suppress bare `.connect()` findings in other files, because signal ownership is per-object-per-owner.
 - **Known limitations**: Multi-line `.connect(\n'changed::...'` calls are not detected (per-line scanning).
-- **Tested by**: `tests/fixtures/gsettings-bare-connect@test/`, `tests/fixtures/gsettings-auto-cleanup@test/`, `tests/fixtures/gsettings-array-storage@test/`
+- **Tested by**: `tests/fixtures/gsettings-bare-connect@test/`, `tests/fixtures/gsettings-auto-cleanup@test/`, `tests/fixtures/gsettings-array-storage@test/`, `tests/fixtures/gsettings-cross-file-leak@test/`
 
 ### R-LIFE-23: .destroy without parentheses
 - **Severity**: advisory

skills/ego-lint/scripts/check-lifecycle.py

@@ -1229,20 +1229,21 @@ def check_gsettings_signal_leak(ext_dir):
         return
 
     bare_connects = []
-    has_auto_cleanup = False
+    has_auto_cleanup_bare = False
 
     for filepath in js_files:
         content = strip_comments(read_file(filepath))
         rel = os.path.relpath(filepath, ext_dir)
 
-        # Extension-wide (not per-file) auto-cleanup detection: connectObject/
-        # disconnectObject typically operate on `this`, so a central disable()
-        # calling disconnectObject(this) cleans signals registered from any file.
-        if (re.search(r'\.disconnectObject\s*\(', content) or
-                re.search(r'\.connectObject\s*\(', content) or
-                re.search(r'\b(SignalTracker|SignalManager|connectSmart|disconnectSmart)\b', content)):
-            has_auto_cleanup = True
+        # Per-file auto-cleanup detection: connectObject/disconnectObject
+        # manage signals per-object-per-owner, so auto-cleanup in one file
+        # does NOT protect bare .connect() calls in a different file.
+        file_has_auto_cleanup = bool(
+            re.search(r'\.disconnectObject\s*\(', content) or
+            re.search(r'\.connectObject\s*\(', content) or
+            re.search(r'\b(SignalTracker|SignalManager|connectSmart|disconnectSmart)\b', content))
 
+        file_bare_connects = []
         prev_stripped = ''
         for lineno, line in enumerate(content.splitlines(), 1):
             stripped = line.strip()
@@ -1267,17 +1268,23 @@ def check_gsettings_signal_leak(ext_dir):
             if re.search(r'\.(connectObject|connectSmart)\s*\(', stripped):
                 prev_stripped = stripped
                 continue
-            bare_connects.append(f"{rel}:{lineno}")
+            file_bare_connects.append(f"{rel}:{lineno}")
             prev_stripped = stripped
 
-    if bare_connects and not has_auto_cleanup:
+        if file_bare_connects:
+            if file_has_auto_cleanup:
+                has_auto_cleanup_bare = True
+            else:
+                bare_connects.extend(file_bare_connects)
+
+    if bare_connects:
         count = len(bare_connects)
         locs = ', '.join(bare_connects[:5])
         suffix = f' (and {count - 5} more)' if count > 5 else ''
         result("FAIL", "lifecycle/gsettings-signal-leak",
                f"{count} bare settings.connect('changed::...') without stored ID "
                f"and no disconnectObject/connectObject cleanup: {locs}{suffix}")
-    elif bare_connects:
+    elif has_auto_cleanup_bare:
         result("PASS", "lifecycle/gsettings-signal-leak",
                "Bare settings.connect() found but auto-cleanup mechanism present")
     # If no bare connects, skip silently

tests/assertions/gsettings-signal-leak.sh

@@ -19,3 +19,9 @@ run_lint "gsettings-array-storage@test"
 assert_exit_code "exits with 0 (array storage is not bare connect)" 0
 assert_output_not_contains "no gsettings-signal-leak FP on array storage" "\[FAIL\].*lifecycle/gsettings-signal-leak"
 echo ""
+
+echo "=== gsettings-cross-file-leak ==="
+run_lint "gsettings-cross-file-leak@test"
+assert_exit_code "exits with 1 (cross-file leak detected)" 1
+assert_output_contains "fails on cross-file bare GSettings connect" "\[FAIL\].*lifecycle/gsettings-signal-leak"
+echo ""

tests/fixtures/gsettings-cross-file-leak@test/LICENSE

@@ -0,0 +1 @@
+SPDX-License-Identifier: GPL-2.0-or-later

tests/fixtures/gsettings-cross-file-leak@test/extension.js

@@ -0,0 +1,16 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+
+export default class CrossFileLeakExtension extends Extension {
+    enable() {
+        this._settings = this.getSettings();
+        this._settings.connect('changed::interval', () => this._onChanged());
+        this._settings.connect('changed::mode', () => this._onMode());
+    }
+
+    _onChanged() {}
+    _onMode() {}
+
+    disable() {
+        this._settings = null;
+    }
+}

tests/fixtures/gsettings-cross-file-leak@test/helper.js

@@ -0,0 +1,18 @@
+import St from 'gi://St';
+
+export class SliderHelper {
+    constructor() {
+        this._slider = new St.Slider();
+        this._slider.connectObject(
+            'notify::value', () => this._onValueChanged(),
+            this
+        );
+    }
+
+    _onValueChanged() {}
+
+    destroy() {
+        this._slider.disconnectObject(this);
+        this._slider = null;
+    }
+}

tests/fixtures/gsettings-cross-file-leak@test/metadata.json

@@ -0,0 +1,7 @@
+{
+    "uuid": "gsettings-cross-file-leak@test",
+    "name": "GSettings Cross-File Leak Test",
+    "description": "Tests that auto-cleanup in one file does not suppress bare connect in another",
+    "shell-version": ["47"],
+    "url": ""
+}