Merge branch 'main' into fix/media-controls-false-positives

Author: ZviBaratz

CLAUDE.md

@@ -138,6 +138,22 @@ test(ego-lint): add fixture for deprecated ByteArray usage
 - **Obfuscation regex pitfalls**: `[a-z]\d+` catches unicode escapes (`\u2013`); `re.IGNORECASE` on guard patterns catches `console.debug` when matching `DEBUG`
 - **Git history rewritten 2026-02-27**: `git filter-repo` removed internal files. All SHAs before that date are invalid
 
+## Repository Workflow
+
+- **Squash merge only** — merge commits and rebase merges are disabled. Every PR becomes a single commit on main with the PR title (conventional commit) and PR body as the commit message
+- **Required status check**: `test` must pass before merging (enforced by `main-protection` ruleset). Admin can bypass for hotfixes
+- **Auto-merge**: Enabled — can be activated per-PR to merge automatically when CI passes
+- **Branch protection**: `main` is protected against deletion and force push via repository ruleset (not classic branch protection)
+- **Labels**: `false-positive`, `new-rule`, `severity-change`, `ego-lint`, `ego-review` (plus GitHub defaults)
+
+## Development Workflow
+
+- **Issue first**: Create a GitHub issue describing the problem or improvement before starting work. Label with `false-positive`, `new-rule`, or `severity-change` as appropriate
+- **Branch per issue**: Create a branch from `main` named `fix/<short-description>`, `feat/<short-description>`, or `docs/<short-description>`
+- **One concern per PR**: Each PR should address a single logical concern (one fix, one feature, one doc update). Split multi-concern work into separate PRs
+- **PR closes issue**: Include `Closes #N` in the PR description to auto-close the issue on merge
+- **Tests before PR**: Run `bash tests/run-tests.sh` and verify all assertions pass before pushing
+
 ## Releasing
 
 release-please automates versioning, CHANGELOG updates, git tags, and GitHub Releases:

docs/internal/README.md

@@ -14,7 +14,11 @@ Run the full ego-submit pipeline against real extensions to surface false positi
 | [Blur my Shell](field-test-blur-my-shell.md) | 2026-03-01 | 49 | 7,743 | 21→7 | 7 | 14 | GObject.registerClass, src/ layout |
 | [GSConnect](field-test-gsconnect.md) | 2026-03-02 | 65 | 24,680 | 19→13 | 10 | 6 | Service daemon context, JSDoc noise |
 | [V-Shell](field-test-v-shell.md) | 2026-03-02 | 28 | 19,201 | 23→3 | 3 | 0 | Multi-version compat guards; all initial FPs resolved |
-| [Dash to Panel](field-test-dash-to-panel.md) | 2026-03-03 | 17 | 16,583 | 19→16 | 10 | 9 | Comment FPs, version-compat guards |
+| [Dash to Panel](field-test-dash-to-panel.md) | 2026-03-03 | 17 | 16,583 | 19→16† | 10 | 9 | Comment FPs, version-compat guards |
+| [Tiling Shell](field-test-tiling-shell.md) | 2026-03-03 | 64 | 11,371 | 21→4 | 4 | 0 | Compiled TS, keybindings, constructors-from-enable |
+| [Media Controls](field-test-media-controls.md) | 2026-03-04 | 17 | ~5,000 | 246W/3F→31W/6F | 6 | 0 | MPRIS D-Bus, onDestroy, src/ layout, provenance |
+
+† Tiling Shell session #29 fixes (constructor-only-in-extension.js) resolve 4 of Dash to Panel's init/shell-modification FP FAILs; re-run pending.
 
 ### Cumulative Calibration Lessons
 
@@ -43,7 +47,15 @@ Patterns discovered across field tests — encode here so future rules avoid rep
 *WARN noise:*
 
 12. **License header URLs are the largest single source of false R-SEC-03 WARNs**: GPL boilerplate with `gnu.org/licenses` URLs triggered the external-URL rule. Now suppressed by guard pattern.
-13. **JSDoc documentation creates massive R-SLOP-01/02 WARN noise on mature projects**: GSConnect (222 WARNs) and Dash to Panel both showed that real JSDoc on mature code triggers AI slop heuristics. Provenance scoring (`quality/code-provenance >= 4`) now gates R-SLOP-01/02 post-filter.
+13. **JSDoc documentation creates massive R-SLOP-01/02 WARN noise on mature projects**: GSConnect (222 WARNs) and Dash to Panel both showed that real JSDoc on mature code triggers AI slop heuristics. Provenance scoring (`quality/code-provenance >= 3`) now gates R-SLOP-01/02 post-filter.
+
+*Compiled TypeScript patterns:*
+
+14. **Helper file constructors called from enable() are not init-time violations**: Tiling Shell had 13 FP FAILs from Shell globals in class constructors that are only ever instantiated from `enable()`. Constructor checks are now restricted to `extension.js` only — matching the existing `GOBJECT_CONSTRUCTORS` guard.
+15. **Arrow function definitions at module scope are lazy**: `const fn = () => Main.layoutManager.monitors` doesn't execute at module load time. The function body is deferred until the first call.
+16. **Same-line ternary guards need `guard-window: 1`**: Version compat patterns like `api.exists ? old(args) : new()` have the guard and deprecated API on the same line. `guard-window: 1` (minimum valid value; the current line is always included in the lookback window) handles this.
+17. **Single long lines ≠ minification**: Compiled TypeScript may have one or two very long lines (keyboard constant chains, export lists) in otherwise readable files. The minified-js check now requires 3+ lines > 500 chars.
+18. **Compiled TypeScript (esbuild) generates systematic WARN noise**: `var` declarations from `__defProp`/`__publicField` helpers (R-DEPR-09), verbose parameter names (R-SLOP-38), and many small classes without `destroy()` methods (resource-tracking/no-destroy-method). These are build artifacts, not author issues.
 
 ## Other Artifacts
 

docs/internal/field-test-media-controls.md

@@ -0,0 +1,134 @@
+# Field Test: Media Controls
+
+**Extension**: Media Controls (`mediacontrols@cliffniff.github.com`)
+**Source**: https://github.com/cliffniff/media-controls
+**Category**: Audio/media control (MPRIS)
+**Downloads**: ~437,000
+**GNOME versions**: 46, 47, 48, 49
+**Codebase**: 17 JS files, 5,064 lines (hand-written JavaScript, no TypeScript compilation)
+**Layout**: `src/` directory (modern build-system layout)
+**License**: GPL-2.0-or-later
+
+## Why This Extension
+
+First audio/media extension tested. Exercises under-represented patterns:
+- D-Bus proxy lifecycle (MPRIS bus interaction, 3 proxies per player)
+- Signal-heavy architecture (41 manual .connect() calls)
+- `src/` directory layout (build-system pattern, metadata.json in src/)
+- Extensive TypeScript-compatible JSDoc typing strategy
+- Custom St.Widget subclasses with GObject.registerClass
+
+## Architecture Overview
+
+- **extension.js** (787 LOC): Main entry point, player discovery via D-Bus NameOwnerChanged, keybinding lifecycle
+- **PlayerProxy.js** (609 LOC): MPRIS D-Bus wrapper — 3 proxies per player (Mpris, MprisPlayer, Properties), change-listener Map, fallback polling
+- **PanelButton.js** (1,236 LOC): Panel button + popup menu UI, extends PanelMenu.Button, `onDestroy()` cleanup via destroy signal
+- **ScrollingLabel.js** (391 LOC): Animated scrolling text widget, extends St.ScrollView, `destroy()` override with timer cleanup
+- **MenuSlider.js** (278 LOC): Track position slider, extends St.BoxLayout, `onDestroy()` via destroy signal connection
+- **prefs.js** (393 LOC): Preferences with AppChooser, BlacklistedPlayers, ElementList, LabelList widgets
+- **types/dbus.js** (239 LOC): Full MPRIS D-Bus type definitions in JSDoc
+
+Import segregation is clean: shell helpers in `helpers/shell/`, prefs helpers in `helpers/prefs/`, shared utils in `utils/`.
+
+## Raw Results
+
+```
+432 checks — 135 passed, 3 failed, 246 warnings, 48 skipped
+Exit code: 1
+```
+
+### Results by Check Name
+
+| Check | Count | Severity | Assessment |
+|-------|-------|----------|------------|
+| R-SLOP-02 (JSDoc @returns) | 103 | WARN | **FP** — intentional TypeScript-compatible JSDoc |
+| R-SLOP-01 (JSDoc @param) | 70 | WARN | **FP** — same |
+| resource-tracking/no-destroy-method | 45 | WARN | **FP** — `onDestroy()` not recognized as cleanup |
+| R-SLOP-11 (GLib.source_remove) | 10 | WARN | **TP** — should use `GLib.Source.remove()` |
+| R-SLOP-17 (typeof method guard) | 3 | WARN | **Debatable** — defensive check for optional API methods |
+| lifecycle/prototype-override | 2 | WARN | **TP** — MprisSource.prototype._addPlayer not restored |
+| metadata/exists | 1 | FAIL | **FP** — metadata.json in `src/`, no fallback |
+| file-structure/metadata.json | 1 | FAIL | **FP** — same |
+| css/shell-class-override | 1 | FAIL | **TP** — `.panel-button` overrides Shell theme class |
+| lifecycle/signal-balance | 1 | WARN | **Needs analysis** — 41 connects vs 10 disconnects |
+| lifecycle/async-destroyed-guard | 1 | WARN | **TP** |
+| lifecycle/soup-session-abort | 1 | WARN | **TP** — no abort() on disable |
+| async/no-cancellable | 1 | WARN | **TP** — async calls without Gio.Cancellable |
+| async/missing-cancellable | 1 | WARN | **TP** — _async() without cancellable |
+| quality/constructor-resources | 1 | WARN | **TP** — .connect() in AppChooser constructor |
+| quality/comment-density | 1 | WARN | **TP advisory** — utils/common.js 45% comments |
+| quality/private-api | 1 | WARN | **TP** — Main.panel private API access |
+| quality/excessive-null-checks | 1 | WARN | **TP advisory** — 117 null checks, ratio 0.025 |
+| R-QUAL-33 (Gio._promisify) | 1 | WARN | **TP advisory** — 4 files with undocumented promisify |
+
+**Total**: 218 FP (89%), 28 TP/debatable (11%)
+
+## False Positive Analysis
+
+### 1. R-SLOP-01/02 JSDoc Flood (173 WARNs)
+
+**Root cause**: Provenance score 3, threshold >= 4 for suppression.
+
+Media Controls uses intentional TypeScript-compatible JSDoc throughout all 17 files. The project includes `jsconfig.json`, `@girs/*` type packages, and a dedicated `types/dbus.js` with full MPRIS type definitions. This is clearly a deliberate typing strategy, not AI slop.
+
+The provenance score of 3 means "strong hand-written indicators" (domain vocabulary: 12 hits, nontrivial algorithms: 40 hits, consistent naming). The threshold of >= 4 is too conservative for JSDoc suppression.
+
+**Fix**: Lower provenance threshold to >= 3.
+
+### 2. resource-tracking/no-destroy-method (45 WARNs)
+
+**Root cause**: `build-resource-graph.py` recognizes `destroy()`, `disable()`, and `_destroy*()` as cleanup methods, but NOT `onDestroy()`.
+
+Both MenuSlider.js and PanelButton.js use `this.connect("destroy", this.onDestroy.bind(this))` to register cleanup handlers. The `onDestroy()` method contains proper signal disconnection, timer removal, and null assignments. The resource graph doesn't recognize this pattern.
+
+- MenuSlider.js: 8 resource warnings, has `onDestroy()` at line 250
+- PanelButton.js: 37 resource warnings, has `onDestroy()` at line 1186
+
+**Fix**: Add `onDestroy()` recognition in build-resource-graph.py cleanup method detection.
+
+### 3. metadata.json Missing (2 FAILs)
+
+**Root cause**: ego-lint.sh has `src/` fallback for extension.js but NOT for metadata.json. check-metadata.py also lacks the fallback.
+
+Media Controls uses a build-system layout with all source in `src/` — metadata.json lives at `src/metadata.json`, which is standard for extensions using gnome-extensions pack with a build step.
+
+**Fix**: Add consistent `src/` fallback for metadata.json in ego-lint.sh and check-metadata.py.
+
+## True Positives
+
+### Lifecycle Issues
+- **prototype-override** (2): `MprisSource.prototype._addPlayer` is monkey-patched but the original is not saved/restored in `disable()`. Genuine concern for multi-extension compatibility.
+- **soup-session-abort** (1): Soup.Session created for album art fetching but no `.abort()` in cleanup path. Pending HTTP requests will continue after disable.
+- **async-destroyed-guard** (1): Multiple async/await chains without `_destroyed` guard. Extension could act on stale state after disable if async operations complete late.
+- **signal-balance** (1): 41 manual `.connect()` vs 10 `.disconnect()`. Needs deeper analysis — some may use `.connectObject()` auto-management, some may be on child widgets (auto-cleaned on destroy).
+
+### Async Safety
+- **no-cancellable + missing-cancellable** (2): D-Bus proxy creation and file operations use async without `Gio.Cancellable`. These should be cancellable via disable() to prevent stale state.
+
+### CSS
+- **shell-class-override** (1 FAIL): `.panel-button` in stylesheet.css overrides GNOME Shell's panel button styling for ALL panel buttons, not just this extension's. Also uses `.popup-menu-container` and `.popup-menu-box` (not flagged but same concern).
+
+### Code Quality
+- **constructor-resources** (1): AppChooser.js:51 has `.connect()` in constructor. Since this is a prefs widget (not extension lifecycle), the impact is limited but still worth noting.
+- **GLib.source_remove** (10): Uses non-idiomatic `GLib.source_remove()` instead of `GLib.Source.remove()`. Functional but not correct GJS API.
+
+## Calibration Lessons
+
+1. **`onDestroy()` is a common cleanup pattern** — build-resource-graph.py should recognize it alongside `destroy()`, `disable()`, and `_destroy*()`
+2. **Provenance threshold of 4 is too conservative for JSDoc suppression** — score 3 is sufficient to indicate hand-written code, especially when JSDoc is extensive and consistent
+3. **`src/` layout support remains inconsistent** — extension.js has fallback, metadata.json does not. Need systematic audit of all src/ fallbacks
+4. **TypeScript-compatible JSDoc is becoming more common** — popular extensions like Media Controls use it intentionally. The R-SLOP-01/02 rules need better calibration for this pattern
+5. **D-Bus proxy lifecycle is well-handled** — no false positives from D-Bus-specific checks. The MPRIS pattern (3 proxies per player, NameOwnerChanged tracking) exercises these checks well
+6. **CSS shell-class-override works correctly** — caught legitimate `.panel-button` override. However, it missed `.popup-menu-container` and `.popup-menu-box` (potential gap in the shell class list)
+
+## Comparison with Prior Tests
+
+| Metric | Media Controls | Avg of Prior 8 |
+|--------|---------------|----------------|
+| FP rate (pre-fix) | 89% | ~40-76% |
+| Dominant FP cause | JSDoc + onDestroy | Varied |
+| True positive rate | 11% | ~24-60% |
+| Security issues | 0 | 0-6 |
+| Code provenance | 3/5 | 2-4/5 |
+
+The high FP rate is driven by two specific gaps. After fixes, the extension's true positive findings are meaningful and actionable.