feat(ego-lint): add lifecycle leak detection (R-LIFE-21, R-LIFE-22, R-LIFE-24) (#48)

## Summary

- **R-LIFE-21** (blocking): Detect bare
`settings.connect('changed::...')` without stored signal ID and no
auto-cleanup mechanism (connectObject/SignalTracker). 4 of 10
field-tested extensions had this issue.
- **R-LIFE-22** (blocking): Detect `global.stage.add_child()`,
`addTopChrome()`, `addChrome()` without matching removal or `.destroy()`
in disable/destroy. Supersedes R-LIFE-19 (advisory pattern rule) with
smarter Tier 2 check that verifies cleanup actually exists.
- **R-LIFE-24** (advisory): Detect `Main.messageTray.add()` without
matching `.destroy()` in disable/destroy.

### Key design decisions
- Only tracks `this._xxx` patterns (local variables are rare and
impractical to track cross-scope)
- `= null` is NOT valid cleanup for R-LIFE-22 (doesn't remove actor from
stage)
- `.destroy()` IS valid cleanup for R-LIFE-22 (destroying a Clutter
actor removes it from parent)
- FAIL for R-LIFE-22 (visible artifacts every cycle), WARN for R-LIFE-24
(lower impact)
- Removes R-LIFE-19 pattern rule from patterns.yaml (superseded by
R-LIFE-22)

### Field test results
Clipboard Indicator now correctly triggers both R-LIFE-22 (FAIL for
`_historyLabel` on global.stage) and R-LIFE-24 (WARN for `_notifSource`
in messageTray).

Closes #35, closes #36

## Test plan
- [x] `bash tests/run-tests.sh` — 553 assertions pass (was 549)
- [x] `gsettings-bare-connect@test` triggers FAIL for R-LIFE-21
- [x] `stage-add-child@test` exits 1 with `[FAIL]
lifecycle/stage-actor-leak`
- [x] `stage-add-child-clean@test` exits 0 with `[PASS]
lifecycle/stage-actor-leak`
- [x] `messagetray-source-leak@test` exits 0 with `[WARN]
lifecycle/messagetray-source-leak`
- [x] Direct ego-lint on clipboard-indicator: R-LIFE-22 FAIL + R-LIFE-24
WARN, R-LIFE-19 absent

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

---------

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

Author: ZviBaratz

field-tests/annotations/clipboard-indicator.yaml

@@ -76,10 +76,10 @@ findings:
   - id: "resource-tracking/ownership::orphan detected"
     classification: tp
     notes: "1 orphan detected — correct"
-  # ego-review blocking issues
-  - id: "lifecycle::_historyLabel on global.stage never removed"
+  # stage/messageTray actor leaks (now detected by ego-lint)
+  - id: "lifecycle/stage-actor-leak::this._historyLabel"
     classification: tp
-    notes: "B1: Actor leak on every enable/disable cycle — ego-review finding"
-  - id: "lifecycle::_notifSource never destroyed in disable()"
+    notes: "B1: Actor leak on every enable/disable cycle — now detected by R-LIFE-22"
+  - id: "lifecycle/messagetray-source-leak::this._notifSource"
     classification: tp
-    notes: "B2: Notification source persists — ego-review finding"
+    notes: "B2: Notification source persists — now detected by R-LIFE-24"

rules/patterns.yaml

@@ -790,14 +790,6 @@
 # --- Lifecycle (advisory) ---
 # Pattern-based lifecycle checks (complement script-based checks in check-lifecycle.py).
 
-- id: R-LIFE-19
-  pattern: "global\\.stage\\.add_child\\s*\\("
-  scope: ["*.js"]
-  severity: advisory
-  message: "global.stage.add_child() — ensure matching remove_child() in disable()/destroy()"
-  category: lifecycle
-  fix: "In disable(), call global.stage.remove_child(this._actor); this._actor = null;"
-
 # --- GNOME 44-46 migration (version-gated) ---
 # APIs removed or renamed in GNOME 44-46 transition to ESM.
 

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

@@ -2049,13 +2049,22 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 - **Fix**: Add `this._session.abort()` before nullifying the session reference in `disable()`.
 - **Tested by**: `tests/fixtures/soup-session-no-abort@test/`
 
-### R-LIFE-19: global.stage.add_child() without matching remove_child()
+### R-LIFE-22: Stage/chrome actor leak (add without remove)
+- **Severity**: blocking
+- **Checked by**: check-lifecycle.py
+- **Rule**: `global.stage.add_child()`, `Main.layoutManager.addTopChrome()`, and `Main.layoutManager.addChrome()` must have a matching removal (`remove_child`/`removeTopChrome`/`removeChrome`) or `.destroy()` in `disable()`/`destroy()`. Only tracks `this._xxx` patterns.
+- **Rationale**: Actors added to `global.stage` or the layout manager chrome persist on the compositor stage across enable/disable cycles. Unlike container widgets that are destroyed with their parent, stage children must be explicitly removed or they leak — causing visible artifacts and memory growth. Nulling the reference (`this._actor = null`) does NOT remove the actor from the stage.
+- **Fix**: In `disable()`, call `global.stage.remove_child(this._actor); this._actor.destroy(); this._actor = null;` or just `this._actor.destroy()` (destroying a Clutter actor removes it from its parent).
+- **Tested by**: `tests/fixtures/stage-add-child@test/`, `tests/fixtures/stage-add-child-clean@test/` (negative)
+- **Supersedes**: R-LIFE-19 (advisory pattern rule)
+
+### R-LIFE-24: MessageTray source leak (add without destroy)
 - **Severity**: advisory
-- **Checked by**: apply-patterns.py
-- **Rule**: `global.stage.add_child()` must have a matching `remove_child()` in `disable()` or `destroy()`.
-- **Rationale**: Actors added to `global.stage` persist on the compositor stage across enable/disable cycles. Unlike container widgets that are destroyed with their parent, stage children must be explicitly removed or they leak — causing visual artifacts and memory growth.
-- **Fix**: In `disable()`, call `global.stage.remove_child(this._actor); this._actor = null;`.
-- **Tested by**: `tests/fixtures/stage-add-child@test/`
+- **Checked by**: check-lifecycle.py
+- **Rule**: `Main.messageTray.add(this._source)` must have a matching `this._source.destroy()` in `disable()`/`destroy()`. Only tracks `this._xxx` patterns.
+- **Rationale**: Notification sources added to the message tray persist if not explicitly destroyed. While lower impact than stage actor leaks (notifications expire and sources may be GC'd), they can accumulate across enable/disable cycles.
+- **Fix**: In `disable()`, call `this._source.destroy(); this._source = null;`.
+- **Tested by**: `tests/fixtures/messagetray-source-leak@test/`
 
 ### R-LIFE-20: Bus name ownership without release
 - **Severity**: advisory
@@ -2065,6 +2074,16 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 - **Fix**: In `disable()`, call `Gio.bus_unown_name(this._ownerId)`.
 - **Tested by**: `tests/fixtures/bus-name-leak@test/`
 
+### 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 `=` before `.connect` on the line) and no auto-cleanup mechanism (`disconnectObject`, `connectObject`, `SignalTracker`, `connectSmart`) is present in the extension.
+- **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).
+- **Known limitations**: Multi-line `.connect(\n'changed::...'` calls are not detected (per-line scanning). Signal IDs stored via `.push()` rather than direct assignment may false-positive.
+- **Tested by**: `tests/fixtures/gsettings-bare-connect@test/`, `tests/fixtures/gsettings-auto-cleanup@test/`
+
 ---
 
 ## Security (R-SEC) — continued

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

@@ -23,6 +23,9 @@
   - R-LIFE-17: Timeout ID reassignment without prior Source.remove()
   - R-LIFE-18: Subprocess without cancellation in disable/destroy
   - R-LIFE-20: Bus name ownership without release
+  - R-LIFE-21: GSettings signal leak (bare connect without disconnect)
+  - R-LIFE-22: Stage actor leak (add_child/addChrome without remove)
+  - R-LIFE-24: MessageTray source leak (add without destroy)
   - R-SEC-16: Clipboard + keybinding cross-reference
   - R-FILE-07: Missing export default class
 
@@ -1009,6 +1012,172 @@ def check_widget_lifecycle(ext_dir):
                f"All {len(created_widgets)} widget(s) properly cleaned up")
 
 
+def extract_cleanup_bodies(js_files):
+    """Extract concatenated text of disable/destroy/onDestroy method bodies."""
+    cleanup_re = re.compile(
+        r'\b(?:disable|destroy|_destroy\w*|onDestroy)\s*\(')
+    cleanup_text = ''
+    for filepath in js_files:
+        content = strip_comments(read_file(filepath))
+        lines = content.splitlines()
+        i = 0
+        while i < len(lines):
+            if cleanup_re.search(lines[i]):
+                depth = 0
+                started = False
+                for j in range(i, len(lines)):
+                    depth += lines[j].count('{') - lines[j].count('}')
+                    if '{' in lines[j]:
+                        started = True
+                    cleanup_text += lines[j] + '\n'
+                    if started and depth <= 0:
+                        break
+                i = j + 1 if started else i + 1
+            else:
+                i += 1
+    return cleanup_text
+
+
+def check_stage_actor_lifecycle(ext_dir):
+    """R-LIFE-22: Stage actor add without matching remove in disable()/destroy()."""
+    js_files = find_js_files(ext_dir, exclude_prefs=True)
+    if not js_files:
+        return
+
+    # Patterns that add actors to stage/chrome
+    add_patterns = [
+        (r'global\.stage\.add_child\s*\(\s*this\.(_\w+)', 'global.stage.remove_child'),
+        (r'Main\.layoutManager\.addTopChrome\s*\(\s*this\.(_\w+)', 'removeTopChrome'),
+        (r'Main\.layoutManager\.addChrome\s*\(\s*this\.(_\w+)', 'removeChrome'),
+    ]
+
+    added_actors = []  # (var_name, file_rel, lineno, remove_method)
+
+    for filepath in js_files:
+        content = strip_comments(read_file(filepath))
+        rel = os.path.relpath(filepath, ext_dir)
+        for lineno, line in enumerate(content.splitlines(), 1):
+            for add_re, remove_method in add_patterns:
+                m = re.search(add_re, line)
+                if m:
+                    added_actors.append((m.group(1), rel, lineno, remove_method))
+
+    if not added_actors:
+        return
+
+    cleanup_text = extract_cleanup_bodies(js_files)
+
+    leaked = []
+    for var_name, rel, lineno, remove_method in added_actors:
+        escaped = re.escape(var_name)
+        has_remove = bool(re.search(
+            rf'{re.escape(remove_method)}\s*\(\s*this\.{escaped}', cleanup_text))
+        has_destroy = bool(re.search(
+            rf'this\.{escaped}\.destroy\s*\(', cleanup_text))
+        if not has_remove and not has_destroy:
+            leaked.append(f"this.{var_name} ({rel}:{lineno})")
+
+    if leaked:
+        result("FAIL", "lifecycle/stage-actor-leak",
+               f"Actor(s) added to stage/chrome but not removed in "
+               f"disable()/destroy(): {', '.join(leaked[:5])}")
+    else:
+        result("PASS", "lifecycle/stage-actor-leak",
+               f"All {len(added_actors)} stage/chrome actor(s) properly cleaned up")
+
+
+def check_message_tray_lifecycle(ext_dir):
+    """R-LIFE-24: MessageTray source added without destroy in disable()/destroy()."""
+    js_files = find_js_files(ext_dir, exclude_prefs=True)
+    if not js_files:
+        return
+
+    add_re = re.compile(r'Main\.messageTray\.add\s*\(\s*this\.(_\w+)')
+    added_sources = []  # (var_name, file_rel, lineno)
+
+    for filepath in js_files:
+        content = strip_comments(read_file(filepath))
+        rel = os.path.relpath(filepath, ext_dir)
+        for lineno, line in enumerate(content.splitlines(), 1):
+            m = add_re.search(line)
+            if m:
+                added_sources.append((m.group(1), rel, lineno))
+
+    if not added_sources:
+        return
+
+    cleanup_text = extract_cleanup_bodies(js_files)
+
+    leaked = []
+    for var_name, rel, lineno in added_sources:
+        escaped = re.escape(var_name)
+        has_destroy = bool(re.search(
+            rf'this\.{escaped}\.destroy\s*\(', cleanup_text))
+        if not has_destroy:
+            leaked.append(f"this.{var_name} ({rel}:{lineno})")
+
+    if leaked:
+        result("WARN", "lifecycle/messagetray-source-leak",
+               f"MessageTray source(s) added but not destroyed in "
+               f"disable()/destroy(): {', '.join(leaked[:5])}")
+    else:
+        result("PASS", "lifecycle/messagetray-source-leak",
+               f"All {len(added_sources)} messageTray source(s) properly cleaned up")
+
+
+def check_gsettings_signal_leak(ext_dir):
+    """R-LIFE-21: Bare settings.connect() without stored ID is a guaranteed leak."""
+    js_files = find_js_files(ext_dir, exclude_prefs=True)
+    if not js_files:
+        return
+
+    # Skip service/ directory (different lifecycle)
+    js_files = [f for f in js_files
+                if '/service/' not in f.replace(os.sep, '/')]
+    if not js_files:
+        return
+
+    bare_connects = []
+    has_auto_cleanup = 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
+
+        for lineno, line in enumerate(content.splitlines(), 1):
+            stripped = line.strip()
+            # Match settings.connect('changed...') without assignment
+            if not re.search(r"\.connect\s*\(\s*['\"]changed", stripped):
+                continue
+            # Skip if return value is stored (has = before .connect on this line)
+            if re.search(r'=\s*\S+\.connect\s*\(', stripped):
+                continue
+            # Skip connectObject/connectSmart variants
+            if re.search(r'\.(connectObject|connectSmart)\s*\(', stripped):
+                continue
+            bare_connects.append(f"{rel}:{lineno}")
+
+    if bare_connects and not has_auto_cleanup:
+        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:
+        result("PASS", "lifecycle/gsettings-signal-leak",
+               "Bare settings.connect() found but auto-cleanup mechanism present")
+    # If no bare connects, skip silently
+
+
 def check_settings_cleanup(ext_dir):
     """Detect getSettings() without cleanup in disable()."""
     ext_file = os.path.join(ext_dir, 'extension.js')
@@ -1071,6 +1240,9 @@ def main():
     check_soup_session_abort(ext_dir)
     check_destroy_then_null(ext_dir)
     check_widget_lifecycle(ext_dir)
+    check_stage_actor_lifecycle(ext_dir)
+    check_message_tray_lifecycle(ext_dir)
+    check_gsettings_signal_leak(ext_dir)
     check_settings_cleanup(ext_dir)
 
 

tests/assertions/field-test-improvements.sh

@@ -43,11 +43,25 @@ assert_exit_code "exits with 0 (no FAIL)" 0
 assert_output_not_contains "no constructor-resources WARN for prefs.js" "\[WARN\].*quality/constructor-resources"
 echo ""
 
-# --- stage-add-child (R-LIFE-19) ---
+# --- stage-add-child (R-LIFE-22) ---
 echo "=== stage-add-child ==="
 run_lint "stage-add-child@test"
-assert_exit_code "exits with 0 (advisory only)" 0
-assert_output_contains "warns on global.stage.add_child" "\[WARN\].*R-LIFE-19"
+assert_exit_code "exits with 1 (FAIL — stage actor leak)" 1
+assert_output_contains "fails on stage actor leak" "\[FAIL\].*lifecycle/stage-actor-leak"
+echo ""
+
+# --- messagetray-source-leak (R-LIFE-24) ---
+echo "=== messagetray-source-leak ==="
+run_lint "messagetray-source-leak@test"
+assert_exit_code "exits with 0 (WARN only)" 0
+assert_output_contains "warns on messageTray source leak" "\[WARN\].*lifecycle/messagetray-source-leak"
+echo ""
+
+# --- stage-add-child-clean (R-LIFE-22 negative) ---
+echo "=== stage-add-child-clean ==="
+run_lint "stage-add-child-clean@test"
+assert_exit_code "exits with 0 (clean)" 0
+assert_output_not_contains "no stage actor leak" "lifecycle/stage-actor-leak.*FAIL"
 echo ""
 
 # --- shell-keybinding-mode (R-DEPR-11) ---

tests/assertions/gsettings-signal-leak.sh

@@ -0,0 +1,15 @@
+# R-LIFE-21: GSettings signal leak detection
+# Sourced by run-tests.sh
+
+echo "=== gsettings-bare-connect ==="
+run_lint "gsettings-bare-connect@test"
+assert_exit_code "exits with 1 (has failures)" 1
+assert_output_contains "fails on bare GSettings connect" "\[FAIL\].*lifecycle/gsettings-signal-leak"
+echo ""
+
+echo "=== gsettings-auto-cleanup ==="
+run_lint "gsettings-auto-cleanup@test"
+assert_exit_code "exits with 0 (no failures)" 0
+assert_output_contains "passes with auto-cleanup" "\[PASS\].*lifecycle/gsettings-signal-leak"
+assert_output_not_contains "no gsettings-signal-leak FP" "\[FAIL\].*lifecycle/gsettings-signal-leak"
+echo ""

tests/assertions/signal-balance-fp.sh

@@ -4,6 +4,7 @@ run_lint "signal-non-gobject@test"
 assert_exit_code "exits with 0 (no failures)" 0
 assert_output_contains "signal balance OK" "\[PASS\].*lifecycle/signal-balance"
 assert_output_not_contains "no signal imbalance warning" "\[WARN\].*lifecycle/signal-balance"
+assert_output_not_contains "no gsettings-signal-leak FP" "\[FAIL\].*lifecycle/gsettings-signal-leak"
 echo ""
 
 # --- signal-destroy-connect (Sub-issue B: destroy signal not counted) ---
@@ -12,6 +13,7 @@ run_lint "signal-destroy-connect@test"
 assert_exit_code "exits with 0 (no failures)" 0
 assert_output_contains "signal balance OK" "\[PASS\].*lifecycle/signal-balance"
 assert_output_not_contains "no signal imbalance warning" "\[WARN\].*lifecycle/signal-balance"
+assert_output_not_contains "no gsettings-signal-leak FP" "\[FAIL\].*lifecycle/gsettings-signal-leak"
 echo ""
 
 # --- signal-local-variable (Sub-issue C: local-variable signal not counted) ---
@@ -20,4 +22,5 @@ run_lint "signal-local-variable@test"
 assert_exit_code "exits with 0 (no failures)" 0
 assert_output_contains "signal balance OK" "\[PASS\].*lifecycle/signal-balance"
 assert_output_not_contains "no signal imbalance warning" "\[WARN\].*lifecycle/signal-balance"
+assert_output_not_contains "no gsettings-signal-leak FP" "\[FAIL\].*lifecycle/gsettings-signal-leak"
 echo ""

tests/assertions/widget-lifecycle.sh

@@ -8,3 +8,4 @@ assert_output_contains "widget not destroyed in disable()" "\[WARN\].*lifecycle/
 echo "=== settings-no-null ==="
 run_lint "settings-no-null@test"
 assert_output_contains "settings not nulled in disable()" "\[WARN\].*lifecycle/settings-cleanup"
+assert_output_not_contains "no gsettings-signal-leak FP (stored ID)" "\[FAIL\].*lifecycle/gsettings-signal-leak"

tests/fixtures/gsettings-auto-cleanup@test/LICENSE

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

tests/fixtures/gsettings-auto-cleanup@test/extension.js

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