Merge branch 'main' into feat/field-test-pipeline

Author: ZviBaratz

docs/internal/field-test-just-perfection.md

@@ -0,0 +1,110 @@
+# Field Test #10: Just Perfection
+
+## Pre-flight
+
+- **Extension**: Just Perfection (`just-perfection-desktop@just-perfection`)
+- **Source**: https://github.com/jrahmatzadeh/just-perfection
+- **GNOME versions**: 45, 46, 47, 48, 49, 50
+- **File count**: 7 JS files, 2 CSS files; ~5,300 JS lines (Manager.js alone is 1,636 non-blank)
+- **License**: GPL-3.0-only (in repo root, not in `src/`)
+- **Why this extension**: Most popular GNOME extension; exercises connectObject/disconnectObject signal management (67 signals), prototype backup/restore via `#originals` map, `src/` directory layout, GNOME 50 support (widest version range), CSS-heavy (23.7%), private field usage (`#timeoutIds`, `#originals`, `#shellVersion`), 20 Shell.ui imports
+
+## Step 1: ego-lint
+
+```bash
+./ego-lint --verbose /tmp/just-perfection/src
+```
+
+### Results (before fixes)
+
+| Status | Count |
+|--------|-------|
+| PASS   | 202   |
+| FAIL   | 3     |
+| WARN   | 414   |
+| SKIP   | 12    |
+| Exit   | 1     |
+
+### Classification of Each FAIL
+
+| Rule | File:Line | TP/FP | Root Cause (if FP) |
+|------|-----------|-------|-------------------|
+| `license` | — | **FP** | LICENSE in repo root, not in `src/`; no parent-dir fallback for src/ layout |
+| `R-VER48-07` | stylesheet.css:167 | TP | `.quick-menu-toggle` renamed to `.quick-toggle-has-menu` in GNOME 48 |
+| `metadata/future-shell-version` | — | TP | GNOME 50 > CURRENT_STABLE (49) |
+
+### Classification of Key WARNs
+
+| Rule | Count | Classification | Notes |
+|------|-------|---------------|-------|
+| R-SLOP-01 (JSDoc @param) | 129 | **FP** | Provenance score=3 but threshold was >=4; PR #23 fixes threshold to >=3 |
+| R-SLOP-02 (JSDoc @returns) | 271 | **FP** | Same root cause as R-SLOP-01 |
+| `lifecycle/prototype-override` | 5 (3 unique) | TP + **bug** | Advisory is valid (prototypes restored via close() chain). Regex catches both override AND restore → duplicates |
+| `metadata/uuid-matches-dir` | 1 | **FP** | Dir is `src`, not UUID; meaningless for src/ layout |
+| `metadata/session-modes-consistency` | 1 | **FP** | `sessionMode.isLocked` used as guard — not lock screen functionality |
+| `quality/impossible-state` | 1 | **FP** | Same isLocked guard pattern |
+| `quality/comment-density` | 1 | **FP** | SupportNotifier.js has 40% comments from thorough hand-written JSDoc |
+| `metadata/deprecated-version` | 1 | TP | `version` field ignored by EGO for GNOME 45+ |
+| `quality/file-complexity` | 1 | TP | Manager.js 1,636 non-blank lines |
+| `quality/private-api` | 1 | TP | statusArea access (legitimate for a UI tweaker) |
+| `disclosure/private-api` | 1 | TP | Private API not disclosed in metadata description |
+
+## Fixes Implemented
+
+Split across 3 PRs per one-concern-per-PR convention:
+
+### PR: fix/src-layout-license (closes #26)
+
+| Rule/Check | Root Cause | Fix |
+|------------|-----------|-----|
+| `license` (FAIL) | No parent-dir fallback for src/ layout | `ego-lint.sh`: check parent dir when `basename == "src"` |
+| `metadata/uuid-matches-dir` | Dir is `src` for src/ layout | `check-metadata.py`: skip when `dir_name == "src"` |
+
+### PR: fix/islocked-guard (closes #27)
+
+| Rule/Check | Root Cause | Fix |
+|------------|-----------|-----|
+| `session-modes-consistency` | `isLocked` is always a guard | `check-metadata.py`: narrow regex to `sessionMode.currentMode` only |
+| `quality/impossible-state` | `isLocked` is always a guard | `check-quality.py`: remove `isLocked` from check, keep `currentMode` comparisons |
+| `quality/comment-density` | 40% threshold too aggressive | `check-quality.py`: raise threshold from 40% to 50% |
+
+### PR: fix/proto-override-dedup (closes #28)
+
+| Rule/Check | Root Cause | Fix |
+|------------|-----------|-----|
+| `lifecycle/prototype-override` (dupes) | Regex matches override + restore | `check-lifecycle.py`: deduplicate by `(file, prototype)` key |
+
+### Test Fixtures Added
+
+| Fixture | What It Tests |
+|---------|--------------|
+| `src-license-fallback@test` | License found in parent dir for src/ layout; UUID-dir skip |
+| `islocked-guard@test` | `sessionMode.isLocked` guard not flagged as impossible-state or session-modes inconsistency |
+| `proto-override-dedup@test` | Each prototype warned only once despite override + restore pattern |
+
+### Existing Fixture Updated
+
+| Fixture | Change | Reason |
+|---------|--------|--------|
+| `ai-slop@test` | `sessionMode.isLocked` → `sessionMode.currentMode !== 'unlock-dialog'` | `isLocked` no longer triggers impossible-state (by design) |
+
+## Results (after all fixes)
+
+| Status | Before | After | Change |
+|--------|--------|-------|--------|
+| FAIL   | 3      | 2     | -1 (license FP fixed) |
+| WARN   | 414    | 408   | -6 (5 FPs + 2 dupes fixed) |
+
+**With PR #23 merged** (provenance threshold >=3): WARNs would drop to ~8, all true positives.
+
+## Calibration Lessons Learned
+
+1. **`sessionMode.isLocked` is always a guard**: Reading lock state to decide behavior is defensive coding, not lock screen functionality. Only `currentMode === 'unlock-dialog'` comparisons suggest active lock screen mode usage.
+
+2. **`src/` layout needs holistic support**: License check and UUID-dir match didn't handle src/ layout. Multiple checks already had src/ fallbacks (CSS, prefs, metadata), but these two were missed.
+
+3. **Prototype override + restore creates duplicates**: When an extension stores originals and restores them (Just Perfection's `#originals` map pattern), the same regex catches both the override and the restore. Deduplication by `(file, prototype_name)` is essential.
+
+4. **Comment-density threshold interacts with provenance**: Well-documented hand-written code (provenance=3) naturally has higher comment density (40%+) from thorough JSDoc. The 40% threshold was too aggressive; 50% avoids FPs while still catching AI slop (typically 55-70%).
+
+5. **Just Perfection validates PR #23**: Provenance score=3 with 400 JSDoc warnings confirms the threshold change from >=4 to >=3 is needed.

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

@@ -831,7 +831,7 @@ Rules that detect patterns commonly found in AI-generated extensions. These are
 - **Rule**: JSDoc comments should not use TypeScript-style `@param {Type} name` annotations.
 - **Rationale**: GNOME Shell extensions are plain JavaScript (not TypeScript). JSDoc type annotations in the `{Type}` format are a strong signal that code was generated by an AI trained primarily on TypeScript codebases. EGO reviewers recognize this pattern.
 - **Fix**: Remove type annotations from JSDoc comments, or remove JSDoc entirely if the code is self-documenting. GJS does not process JSDoc types.
-- **Provenance suppression**: When `quality/code-provenance` reports a score >= 4 (strong hand-written indicators), R-SLOP-01 warnings are automatically suppressed. High-provenance code uses JSDoc as intentional documentation, not AI slop. A `provenance/jsdoc-suppressed` PASS line is emitted instead.
+- **Provenance suppression**: When `quality/code-provenance` reports a score >= 3 (moderate-to-high hand-written indicators), R-SLOP-01 warnings are automatically suppressed. Hand-written code uses JSDoc as intentional documentation, not AI slop. A `provenance/jsdoc-suppressed` PASS line is emitted instead.
 
 #### Example
 
@@ -862,7 +862,7 @@ _updateIndicator(level, charging, icon) {
 - **Rule**: JSDoc comments should not use TypeScript-style `@returns {Type}` annotations.
 - **Rationale**: Same as R-SLOP-01. `@returns {Type}` is a TypeScript convention that has no effect in GJS and signals AI-generated code.
 - **Fix**: Remove `@returns {Type}` annotations from JSDoc comments.
-- **Provenance suppression**: Same as R-SLOP-01 — suppressed when provenance score >= 4.
+- **Provenance suppression**: Same as R-SLOP-01 — suppressed when provenance score >= 3.
 
 #### Example
 
@@ -921,9 +921,9 @@ Heuristic rules that detect code patterns commonly seen in AI-generated or over-
 ### R-QUAL-02: Impossible state checks
 - **Severity**: advisory
 - **Checked by**: check-quality.py
-- **Rule**: Extension code should not check for states that are impossible given its configuration. For example, checking `Main.sessionMode.isLocked` without declaring `unlock-dialog` in `session-modes`.
-- **Rationale**: If the extension does not declare `unlock-dialog` in `session-modes`, it is never active on the lock screen, so checking lock state is dead code. This pattern is common in AI-generated extensions that copy-paste lock-screen handling without understanding when it applies.
-- **Fix**: Either add `"unlock-dialog"` to `session-modes` in `metadata.json` (if lock-screen support is intended) or remove the lock-state checking code.
+- **Rule**: Extension code should not check for states that are impossible given its configuration. For example, comparing `Main.sessionMode.currentMode` with `'unlock-dialog'` without declaring `unlock-dialog` in `session-modes`. Note: `sessionMode.isLocked` is exempt — reading lock state is always a defensive guard pattern, not evidence of lock screen functionality.
+- **Rationale**: If the extension does not declare `unlock-dialog` in `session-modes`, it is never active on the lock screen, so checking `currentMode` against lock-screen values is dead code. This pattern is common in AI-generated extensions that copy-paste lock-screen handling without understanding when it applies.
+- **Fix**: Either add `"unlock-dialog"` to `session-modes` in `metadata.json` (if lock-screen support is intended) or remove the `currentMode` comparison code.
 
 ### R-QUAL-03: Over-engineered async coordination
 - **Severity**: advisory
@@ -1561,7 +1561,7 @@ destroy() {
 ### R-QUAL-11: Comment Density
 - **Severity**: advisory
 - **Checked by**: check-quality.py
-- **Rule**: Flags files where >40% of lines (after first 10) are comments.
+- **Rule**: Flags files where >50% of lines (after first 10) are comments.
 - **Rationale**: Excessive comments explaining obvious code is a strong AI slop signal.
 - **Fix**: Remove redundant comments. Only keep comments that explain non-obvious logic or API quirks.
 - **Tested by**: `tests/fixtures/quality-signals@test/`
@@ -1952,7 +1952,7 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 ### metadata/session-modes-consistency: SessionMode usage without declaration
 - **Severity**: advisory
 - **Checked by**: check-metadata.py
-- **Rule**: Warns if the extension code references `Main.sessionMode.currentMode` or `sessionMode.isLocked` but `metadata.json` does not declare `session-modes` with `unlock-dialog`.
+- **Rule**: Warns if the extension code references `Main.sessionMode.currentMode` but `metadata.json` does not declare `session-modes` with `unlock-dialog`. Note: `sessionMode.isLocked` is exempt — reading lock state is a defensive guard, not evidence of lock screen usage.
 - **Rationale**: Code that checks session mode without the proper declaration is either dead code (the extension won't run in lock screen mode) or a misunderstanding of the lifecycle.
 - **Fix**: Either add `"session-modes": ["user", "unlock-dialog"]` to metadata.json, or remove the session mode checks from the code.
 
@@ -2011,8 +2011,8 @@ Rules for APIs removed or changed in specific GNOME Shell versions. These rules
 ### resource-tracking/no-destroy-method
 - **Severity**: advisory
 - **Checked by**: check-resources.py → build-resource-graph.py
-- **Rule**: Module creates resources but has no `destroy()` or `disable()` method for cleanup.
-- **Fix**: Add a `destroy()` method that cleans up all resources.
+- **Rule**: Module creates resources but has no cleanup method (`destroy()`, `disable()`, or `onDestroy()`) for cleanup.
+- **Fix**: Add a `destroy()` method that cleans up all resources, or use `onDestroy()` connected via `this.connect('destroy', this.onDestroy.bind(this))`.
 
 ### resource-tracking/destroy-not-called
 - **Severity**: advisory

skills/ego-lint/scripts/apply-patterns.py

@@ -91,6 +91,11 @@ def _unescape_yaml_double(s):
 def _get_shell_versions(ext_dir):
     """Read shell-version from metadata.json and return as list of ints."""
     metadata_path = os.path.join(ext_dir, 'metadata.json')
+    # src/ layout fallback
+    if not os.path.isfile(metadata_path):
+        src_path = os.path.join(ext_dir, 'src', 'metadata.json')
+        if os.path.isfile(src_path):
+            metadata_path = src_path
     if not os.path.isfile(metadata_path):
         return []
     try:

skills/ego-lint/scripts/build-resource-graph.py

@@ -329,6 +329,10 @@ def scan_file(file_path, ext_dir):
     has_private_destroy = bool(re.search(
         r'(?:^|\s)_destroy\w*\s*\([^)]*\)\s*\{', content, re.MULTILINE
     ))
+    # Detect onDestroy() — common pattern via this.connect("destroy", this.onDestroy.bind(this))
+    has_on_destroy = bool(re.search(
+        r'(?:^|\s)onDestroy\s*\([^)]*\)\s*\{', content, re.MULTILINE
+    ))
 
     return {
         'rel': rel,
@@ -339,6 +343,7 @@ def scan_file(file_path, ext_dir):
         'has_destroy': has_destroy,
         'has_disable': has_disable,
         'has_private_destroy': has_private_destroy,
+        'has_on_destroy': has_on_destroy,
         'child_refs': child_refs,
         'content': content,
     }
@@ -436,9 +441,9 @@ def detect_orphans(file_scans, ownership):
     """Detect orphaned resources in files that are owned by other files.
 
     An orphan is a resource created in a child module where:
-    1. No matching cleanup exists in that module's destroy() method, OR
-    2. The module has no destroy() method but creates resources, OR
-    3. The module's destroy() is never called by its parent.
+    1. The module has no cleanup method (destroy/disable/onDestroy) but creates resources, OR
+    2. The module has a cleanup method but its parent never calls it, OR
+    3. The cleanup method exists and is called, but specific resources aren't cleaned up.
 
     Only flags resources in files that ARE owned by another file.
     """
@@ -463,7 +468,8 @@ def detect_orphans(file_scans, ownership):
         has_destroy = scan['has_destroy']
         has_disable = scan['has_disable']
         has_private_destroy = scan['has_private_destroy']
-        has_cleanup_method = has_destroy or has_disable or has_private_destroy
+        has_on_destroy = scan['has_on_destroy']
+        has_cleanup_method = has_destroy or has_disable or has_private_destroy or has_on_destroy
 
         if not creates:
             continue
@@ -486,27 +492,27 @@ def detect_orphans(file_scans, ownership):
                         parent_calls_destroy = True
                         break
 
-        # Case 1: Module creates resources but has no destroy/disable method
+        # Case 1: Module creates resources but has no cleanup method
         if not has_cleanup_method:
             for c in creates:
                 orphans.append({
                     'file': rel,
                     'line': c['line'],
                     'type': c['type'],
                     'pattern': c['pattern'],
-                    'reason': f"no destroy()/disable() method in {rel}",
+                    'reason': f"no cleanup method (destroy/disable/onDestroy) in {rel}",
                 })
             continue
 
-        # Case 2: Module has destroy() but parent never calls it
+        # Case 2: Module has cleanup method but parent never calls it
         if not parent_calls_destroy:
             for c in creates:
                 orphans.append({
                     'file': rel,
                     'line': c['line'],
                     'type': c['type'],
                     'pattern': c['pattern'],
-                    'reason': f"parent does not call destroy() on {rel}",
+                    'reason': f"parent does not call cleanup method on {rel}",
                 })
             continue
 
@@ -517,7 +523,7 @@ def detect_orphans(file_scans, ownership):
         # releasing references even when the resource auto-cleans itself
         nulled_refs = set()
         content = scan['content']
-        for method_name in ('destroy', 'disable', '_destroy'):
+        for method_name in ('destroy', 'disable', '_destroy', 'onDestroy'):
             mb = find_method_body(content, method_name)
             if mb:
                 _, _, body = mb

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

@@ -152,6 +152,11 @@ def main():
 
     # Read metadata description once
     meta_path = os.path.join(ext_dir, 'metadata.json')
+    # src/ layout fallback
+    if not os.path.isfile(meta_path):
+        src_path = os.path.join(ext_dir, 'src', 'metadata.json')
+        if os.path.isfile(src_path):
+            meta_path = src_path
     description = ''
     if os.path.isfile(meta_path):
         try:

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

@@ -374,15 +374,23 @@ def check_injection_manager(ext_dir):
 
     # WS1-D: Detect direct prototype overrides
     prototype_overrides = []
+    seen_overrides = set()
     for filepath in js_files:
         content = strip_comments(read_file(filepath))
         rel = os.path.relpath(filepath, ext_dir)
         # SomeClass.prototype.methodName = ...
         for m in re.finditer(r'(\w+\.prototype\.\w+)\s*=', content):
-            prototype_overrides.append((rel, m.group(1)))
+            key = (rel, m.group(1))
+            if key not in seen_overrides:
+                seen_overrides.add(key)
+                prototype_overrides.append(key)
         # Object.assign(SomeClass.prototype, ...)
         for m in re.finditer(r'Object\.assign\s*\(\s*(\w+\.prototype)', content):
-            prototype_overrides.append((rel, f"Object.assign({m.group(1)}, ...)"))
+            label = f"Object.assign({m.group(1)}, ...)"
+            key = (rel, label)
+            if key not in seen_overrides:
+                seen_overrides.add(key)
+                prototype_overrides.append(key)
 
     if prototype_overrides:
         # Check if disable() restores prototypes
@@ -464,6 +472,10 @@ def check_selective_disable(ext_dir):
 def check_unlock_dialog_comment(ext_dir):
     """R-LIFE-14: unlock-dialog session mode should have explanatory comment in disable()."""
     metadata_path = os.path.join(ext_dir, 'metadata.json')
+    if not os.path.isfile(metadata_path):
+        src_path = os.path.join(ext_dir, 'src', 'metadata.json')
+        if os.path.isfile(src_path):
+            metadata_path = src_path
     if not os.path.isfile(metadata_path):
         return
 
@@ -541,6 +553,10 @@ def check_clipboard_keybinding(ext_dir):
 def check_lockscreen_signals(ext_dir):
     """R-LIFE-11: Lock screen signal safety — keyboard signals with unlock-dialog mode."""
     metadata_path = os.path.join(ext_dir, 'metadata.json')
+    if not os.path.isfile(metadata_path):
+        src_path = os.path.join(ext_dir, 'src', 'metadata.json')
+        if os.path.isfile(src_path):
+            metadata_path = src_path
     if not os.path.isfile(metadata_path):
         return