fix(ego-lint): make R-LIFE-25 bare connectSignal always FAIL regardless of disconnectSignal elsewhere (#91)

## Summary

- **Bare `connectSignal()` calls now always FAIL** — a global
`has_dbus_disconnect` flag was masking bare (un-stored) connects
whenever *any* `disconnectSignal()` existed elsewhere. Bare connects are
structurally un-cleanable (can't call `disconnectSignal(id)` without
storing `id`), so they should never be suppressed.
- **Stored connects without `disconnectSignal()` in the same file →
WARN** (per-file scoping replaces global flag)
- **`.disconnectSignal.bind()` recognized** as valid cleanup pattern

## Test plan

- [x] 615 test assertions pass (`bash tests/run-tests.sh`)
- [x] `dbus-signal-auto-cleanup@test` — exits 1 with FAIL (was exit 0
with PASS)
- [x] `dbus-signal-stored-no-disconnect@test` — exits 0 with WARN (new
fixture)
- [x] `dbus-signal-bind-cleanup@test` — exits 0, clean (new fixture)
- [ ] Field test: `bash scripts/field-test-runner.sh --no-fetch` —
media-controls shows FAIL for bare connects

Closes #90

🤖 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

@@ -2074,14 +2074,16 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 - **Tested by**: `tests/fixtures/messagetray-source-leak@test/`
 
 ### R-LIFE-25: D-Bus proxy signal leak (connectSignal without disconnectSignal)
-- **Severity**: blocking
+- **Severity**: blocking (bare connects), advisory (stored IDs without disconnect)
 - **Checked by**: check-lifecycle.py
-- **Rule**: `proxy.connectSignal('...')` calls where the return value is not stored (no `=` or `.push()` before `.connectSignal` on the line) and no `disconnectSignal()` call is present in the extension. GObject cleanup mechanisms (`connectObject`, `disconnectObject`, `SignalTracker`, `connectSmart`) do NOT clean up D-Bus proxy signals and are not considered valid cleanup.
+- **Rule**: Two tiers of detection:
+  - **Bare connects** (FAIL): `proxy.connectSignal('...')` where the return value is not stored (no `=` or `.push()` before `.connectSignal` on the line). These are always leaks — you cannot call `disconnectSignal(id)` without having stored `id`. Not masked by `disconnectSignal()` elsewhere.
+  - **Stored without disconnect** (WARN): `connectSignal()` return values are stored but no `disconnectSignal()` call exists in the same file (per-file scoping). GObject cleanup mechanisms (`connectObject`, `disconnectObject`, `SignalTracker`, `connectSmart`) do NOT clean up D-Bus proxy signals and are not considered valid cleanup. `.disconnectSignal.bind()` is recognized as valid cleanup.
 - **Rationale**: Without a stored signal ID, there is no way to call `proxy.disconnectSignal(id)` later. These D-Bus signal handlers accumulate across enable/disable cycles, causing duplicate callbacks and memory leaks. This is distinct from GObject `.connect()` — `connectSignal()` is specific to `Gio.DBusProxy` for subscribing to D-Bus signals (e.g., `PropertiesChanged`, `NameOwnerChanged`).
 - **Fix**: Store the return value and call `proxy.disconnectSignal(id)` in `disable()`.
 - **Scope exclusions**: prefs.js (manages own lifecycle), `service/` directory (daemon lifecycle).
 - **Known limitations**: Multi-line `.connectSignal(\n'...'` calls are not detected (per-line scanning).
-- **Tested by**: `tests/fixtures/dbus-signal-leak@test/`, `tests/fixtures/dbus-signal-auto-cleanup@test/`, `tests/fixtures/dbus-signal-array-storage@test/`, `tests/fixtures/dbus-signal-mixed-cleanup@test/`
+- **Tested by**: `tests/fixtures/dbus-signal-leak@test/`, `tests/fixtures/dbus-signal-auto-cleanup@test/`, `tests/fixtures/dbus-signal-array-storage@test/`, `tests/fixtures/dbus-signal-mixed-cleanup@test/`, `tests/fixtures/dbus-signal-stored-no-disconnect@test/`, `tests/fixtures/dbus-signal-bind-cleanup@test/`
 
 ### R-LIFE-26: Module-scope mutable state not cleared in disable()
 - **Severity**: advisory

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

@@ -27,7 +27,7 @@
   - R-LIFE-22: Stage actor leak (add_child/addChrome without remove)
   - R-LIFE-23: .destroy without parentheses (property access, not method call)
   - R-LIFE-24: MessageTray source leak (add without destroy)
-  - R-LIFE-25: D-Bus proxy signal leak (connectSignal without disconnectSignal)
+  - R-LIFE-25: D-Bus proxy signal leak (bare connectSignal always fails; stored without disconnect warns)
   - R-LIFE-26: Module-scope mutable state (Map/Set) not cleared in disable()
   - R-LIFE-27: Module-scope prototype mutation
   - R-SEC-16: Clipboard + keybinding cross-reference
@@ -1291,7 +1291,13 @@ def check_gsettings_signal_leak(ext_dir):
 
 
 def check_dbus_signal_leak(ext_dir):
-    """R-LIFE-25: Bare proxy.connectSignal() without stored ID is a guaranteed leak."""
+    """R-LIFE-25: D-Bus proxy signal leak detection.
+
+    Bare proxy.connectSignal() without stored ID is a guaranteed leak (FAIL).
+    Stored connectSignal() IDs without disconnectSignal() in same file are
+    advisory (WARN) — per-file scoping avoids cross-file false positives.
+    Recognizes .disconnectSignal.bind() as valid cleanup pattern.
+    """
     js_files = find_js_files(ext_dir, exclude_prefs=True)
     if not js_files:
         return
@@ -1303,17 +1309,18 @@ def check_dbus_signal_leak(ext_dir):
         return
 
     bare_connects = []
-    has_dbus_disconnect = False
+    stored_no_disconnect_files = []
 
     for filepath in js_files:
         content = strip_comments(read_file(filepath))
         rel = os.path.relpath(filepath, ext_dir)
 
-        # D-Bus-specific cleanup detection: only disconnectSignal counts
+        # Per-file D-Bus cleanup detection: disconnectSignal() or .disconnectSignal.bind(
         # (connectObject/disconnectObject manage GObject signals, not D-Bus signals)
-        if re.search(r'\.disconnectSignal\s*\(', content):
-            has_dbus_disconnect = True
+        file_has_disconnect = bool(
+            re.search(r'\.disconnectSignal\s*[\(.]', content))
 
+        file_stored_connects = []
         prev_stripped = ''
         for lineno, line in enumerate(content.splitlines(), 1):
             stripped = line.strip()
@@ -1322,36 +1329,52 @@ def check_dbus_signal_leak(ext_dir):
                 if stripped:
                     prev_stripped = stripped
                 continue
-            # Skip if return value is stored (has = before .connectSignal on this line)
+            # Skip connectObject/connectSmart variants
+            if re.search(r'\.(connectObject|connectSmart)\s*\(', stripped):
+                prev_stripped = stripped
+                continue
+            # Check if return value is stored (has = before .connectSignal on this line)
             if re.search(r'=\s*\S+\.connectSignal\s*\(', stripped):
+                file_stored_connects.append(f"{rel}:{lineno}")
                 prev_stripped = stripped
                 continue
-            # Skip if pushed to array (.push(proxy.connectSignal(...)))
+            # Check if pushed to array (.push(proxy.connectSignal(...)))
             if re.search(r'\.push\s*\(\s*\S+\.connectSignal\s*\(', stripped):
+                file_stored_connects.append(f"{rel}:{lineno}")
                 prev_stripped = stripped
                 continue
-            # Skip if this line is inside a .push() call (multi-line pattern)
+            # Check if this line is inside a .push() call (multi-line pattern)
             if re.search(r'\.push\s*\(\s*$', prev_stripped):
-                prev_stripped = stripped
-                continue
-            # Skip connectObject/connectSmart variants
-            if re.search(r'\.(connectObject|connectSmart)\s*\(', stripped):
+                file_stored_connects.append(f"{rel}:{lineno}")
                 prev_stripped = stripped
                 continue
             bare_connects.append(f"{rel}:{lineno}")
             prev_stripped = stripped
 
-    if bare_connects and not has_dbus_disconnect:
+        # Track files with stored connects but no disconnect
+        if file_stored_connects and not file_has_disconnect:
+            stored_no_disconnect_files.append((rel, file_stored_connects))
+
+    # Bare connects always FAIL — can't disconnect what you never stored
+    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/dbus-signal-leak",
-               f"{count} bare proxy.connectSignal() without stored ID "
-               f"and no disconnectSignal() cleanup: {locs}{suffix}")
-    elif bare_connects:
-        result("PASS", "lifecycle/dbus-signal-leak",
-               "Bare connectSignal() found but disconnectSignal() cleanup present")
-    # If no bare connectSignal calls, skip silently
+               f"{count} bare proxy.connectSignal() without stored ID: "
+               f"{locs}{suffix}")
+
+    # Stored connects in files without any disconnectSignal → advisory
+    if stored_no_disconnect_files:
+        all_locs = []
+        for _rel, locs in stored_no_disconnect_files:
+            all_locs.extend(locs)
+        count = len(all_locs)
+        loc_str = ', '.join(all_locs[:5])
+        suffix = f' (and {count - 5} more)' if count > 5 else ''
+        result("WARN", "lifecycle/dbus-signal-leak",
+               f"{count} stored connectSignal() ID(s) without "
+               f"disconnectSignal() in same file: {loc_str}{suffix}")
 
 
 def check_settings_cleanup(ext_dir):

tests/assertions/dbus-signal-leak.sh

@@ -10,20 +10,34 @@ echo ""
 
 echo "=== dbus-signal-auto-cleanup ==="
 run_lint "dbus-signal-auto-cleanup@test"
-assert_exit_code "exits with 0 (no failures)" 0
-assert_output_contains "passes with auto-cleanup" "\[PASS\].*lifecycle/dbus-signal-leak"
-assert_output_not_contains "no dbus-signal-leak FP" "\[FAIL\].*lifecycle/dbus-signal-leak"
+assert_exit_code "exits with 1 (bare connect always fails)" 1
+assert_output_contains "fails on bare connectSignal despite other cleanup" "\[FAIL\].*lifecycle/dbus-signal-leak"
+assert_output_not_contains "no WARN when file has disconnectSignal" "\[WARN\].*lifecycle/dbus-signal-leak"
 echo ""
 
 echo "=== dbus-signal-array-storage ==="
 run_lint "dbus-signal-array-storage@test"
 assert_exit_code "exits with 0 (array storage is not bare connect)" 0
 assert_output_not_contains "no dbus-signal-leak FP on array storage" "\[FAIL\].*lifecycle/dbus-signal-leak"
+assert_output_not_contains "no WARN when file has disconnectSignal" "\[WARN\].*lifecycle/dbus-signal-leak"
 echo ""
 
 echo "=== dbus-signal-mixed-cleanup ==="
 run_lint "dbus-signal-mixed-cleanup@test"
 assert_exit_code "exits with 1 (has failures)" 1
 assert_output_contains "fails despite GObject cleanup" "\[FAIL\].*lifecycle/dbus-signal-leak"
-assert_output_not_contains "no false auto-cleanup pass" "\[PASS\].*lifecycle/dbus-signal-leak"
+echo ""
+
+echo "=== dbus-signal-stored-no-disconnect ==="
+run_lint "dbus-signal-stored-no-disconnect@test"
+assert_exit_code "exits with 0 (advisory only)" 0
+assert_output_contains "warns on stored connect without disconnect" "\[WARN\].*lifecycle/dbus-signal-leak"
+assert_output_not_contains "no FAIL for stored connect" "\[FAIL\].*lifecycle/dbus-signal-leak"
+echo ""
+
+echo "=== dbus-signal-bind-cleanup ==="
+run_lint "dbus-signal-bind-cleanup@test"
+assert_exit_code "exits with 0 (bind cleanup recognized)" 0
+assert_output_not_contains "no FAIL with bind cleanup" "\[FAIL\].*lifecycle/dbus-signal-leak"
+assert_output_not_contains "no WARN with bind cleanup" "\[WARN\].*lifecycle/dbus-signal-leak"
 echo ""

tests/fixtures/dbus-signal-bind-cleanup@test/LICENSE

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

tests/fixtures/dbus-signal-bind-cleanup@test/extension.js

@@ -0,0 +1,21 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+import Gio from 'gi://Gio';
+
+export default class extends Extension {
+    enable() {
+        this._proxy = Gio.DBusProxy.new_for_bus_sync(
+            Gio.BusType.SESSION, 0, null,
+            'org.mpris.MediaPlayer2', '/org/mpris/MediaPlayer2',
+            'org.mpris.MediaPlayer2.Player', null);
+        this._seekedId = this._proxy.connectSignal('Seeked', () => {});
+        this._cleanups = [
+            this._proxy.disconnectSignal.bind(this._proxy, this._seekedId),
+        ];
+    }
+
+    disable() {
+        this._cleanups.forEach(cb => cb());
+        this._cleanups = null;
+        this._proxy = null;
+    }
+}

tests/fixtures/dbus-signal-bind-cleanup@test/metadata.json

@@ -0,0 +1,7 @@
+{
+  "uuid": "dbus-signal-bind-cleanup@test",
+  "name": "D-Bus Signal Bind Cleanup Test",
+  "shell-version": ["45"],
+  "description": "Fixture for R-LIFE-25: .disconnectSignal.bind() recognized as valid cleanup",
+  "url": "https://example.com"
+}

tests/fixtures/dbus-signal-stored-no-disconnect@test/LICENSE

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

tests/fixtures/dbus-signal-stored-no-disconnect@test/extension.js

@@ -0,0 +1,16 @@
+import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
+import Gio from 'gi://Gio';
+
+export default class extends Extension {
+    enable() {
+        this._proxy = Gio.DBusProxy.new_for_bus_sync(
+            Gio.BusType.SESSION, 0, null,
+            'org.freedesktop.DBus', '/org/freedesktop/DBus',
+            'org.freedesktop.DBus', null);
+        this._signalId = this._proxy.connectSignal('NameOwnerChanged', () => {});
+    }
+
+    disable() {
+        this._proxy = null;
+    }
+}

tests/fixtures/dbus-signal-stored-no-disconnect@test/metadata.json

@@ -0,0 +1,7 @@
+{
+  "uuid": "dbus-signal-stored-no-disconnect@test",
+  "name": "D-Bus Signal Stored No Disconnect Test",
+  "shell-version": ["45"],
+  "description": "Fixture for R-LIFE-25: stored connectSignal ID without disconnectSignal",
+  "url": "https://example.com"
+}