ego-review

ego-review

Simulated EGO reviewer code review for GNOME Shell extensions.

This skill guides a thorough manual code review that covers everything an extensions.gnome.org reviewer checks, plus common rejection patterns learned from real submissions.

Reviewer Context

Understanding the real EGO review process helps calibrate review severity:

• Primary reviewer processes 15,000+ lines of code per day across many extensions
• Review priority order: Security → Stability (resource leaks) → Code readability → Lifecycle correctness → API correctness → Metadata → AI pattern detection
• Lifecycle cleanup is the #1 rejection cause — not security, not metadata
• AI slop triggers deeper scrutiny: When a reviewer spots one AI-generated pattern, they examine the entire extension more carefully. A clean extension with one pkexec usage gets more leeway than one with typeof super.destroy === 'function' guards everywhere
• The domino effect: Bad patterns in published extensions get copied. Reviewers are stricter on patterns they've seen spread (excessive try-catch, empty catches, TypeScript JSDoc)
• Developer understanding matters: When asked to explain code, developers who respond with more AI-generated text are immediately flagged. Extensions where the developer clearly understands the code get more benefit of the doubt

Phase 0: Automated Baseline

1. Run ego-lint on the extension directory (invoke the ego-lint skill)
2. Capture all FAIL/WARN/PASS results
3. For each FAIL/WARN, note the check name — Phases 2-5 should NOT re-report issues already caught by ego-lint (avoid duplication)
4. Focus manual review on issues ego-lint CANNOT detect (semantic, cross-file, design-level)

Phase 1: Discovery

1. Read metadata.json -- note UUID, shell-version, settings-schema, any session-modes
2. Glob all .js files -- identify extension.js, prefs.js, lib/ modules
3. Check for helper scripts, polkit rules, or other resources
4. Note the extension's purpose and complexity level

Phase 1b: Licensing & Legal

Using licensing-checklist.md:

1. Verify LICENSE/COPYING file exists and is GPL-compatible
2. Check for code that appears borrowed from other extensions (attribution needed)
3. Verify no copyrighted/trademarked content without permission
4. Check extension name, description, and UI text for CoC compliance

Phase 2: Lifecycle Audit

Using lifecycle-checklist.md:

1. Read extension.js — verify enable/disable symmetry

2. Check constructor constraints (no resource allocation in constructor)

3. Reference the resource-tracking findings from Phase 0 lint. ego-lint already ran build-resource-graph.py and check-resources.py. Use the resource-tracking/* findings from the lint results as the starting point. Do NOT re-run build-resource-graph.py.

4. For each resource-tracking FAIL/WARN from lint: read the cited file:line to verify it's a true leak. Classify as: TRUE LEAK (blocking) | JUSTIFIED (note why) | FALSE POSITIVE (skip). For true leaks, include the fix in the report.

5. For ownership chains: if lint reports orphans, verify parent calls child's destroy() in its own disable()/destroy() and that destroy order is reverse of creation. If lint reports 0 orphans, do a brief spot-check of 1-2 ownership chains to verify graph accuracy, but do not perform a full ownership walk.

6. Resource tracking table: if the report needs a resource tracking table, build it from the lint JSON's resource-tracking/* findings rather than re-running build-resource-graph.py --format=table.

7. If the graph reports 0 orphans and complete ownership chains: abbreviate this phase — focus on async guards and cleanup ordering below

8. Async guard verification: For every await in enable-path code, verify a _destroyed check follows the resume point

9. Verify cleanup ordering (reverse order of creation)

10. Check for _destroyed flag pattern in async operations

11. Verify session mode handling if applicable

Phase 3: Signal & Resource Audit

1. Abbreviate this phase if Phase 2 found 0 orphans AND Phase 0 lint has no resource-tracking/* or lifecycle/* FAILs/WARNs. In this case, do a single spot-check: pick 1 resource entry from the graph and verify by reading the cited file:line that create/destroy are correctly paired.
2. Otherwise, for each resource-tracking or lifecycle FAIL/WARN from lint, verify by reading the cited code — focus on issues ego-lint cannot judge semantically (e.g., whether a cleanup pattern is architecturally correct).
3. Check for resource types lint still misses: custom cleanup methods (_cleanup(), _teardown(), _clear()) not recognized by the resource graph. GSettings signal leaks and D-Bus connectSignal leaks are now automated — do not re-check manually.
4. Only do a full manual grep if lint reported orphans AND you suspect the graph missed resources after the spot-checks above.

Phase 4: Security Review

Using security-checklist.md:

1. Check subprocess/command execution patterns
2. Verify pkexec usage and helper script input validation
3. Check clipboard operations and disclosure
4. Check for network access
5. Verify file path handling (no traversal)
6. Check for telemetry/analytics patterns (banned by EGO)
7. Check clipboard access and disclosure requirements
8. Check for extension system interference (ExtensionManager usage)

Reviewer perspective notes:

• When a reviewer sees pkexec, they immediately check the helper script for input validation
• When a reviewer sees network access, they check if it's disclosed in the description
• When a reviewer sees clipboard access, they check if the user initiated it

Phase 4b: Accessibility (if extension adds UI)

Apply accessibility-checklist.md:

1. Identify all custom UI elements added by the extension
2. Check A1-A7 for each element
3. Note: standard St.Button, PopupMenu, QuickToggle have built-in accessibility — only flag custom widgets

Phase 5: Code Quality

Using code-quality-checklist.md:

1. Check for deprecated modules and APIs
2. Check for web API usage (setTimeout, fetch, etc.)
3. Look for AI code artifacts (imaginary APIs, hallucinated imports)
4. Check for excessive logging
5. Verify private API usage is documented
6. Check error handling patterns
7. Hallucinated API cross-reference: Verify that every API method called actually exists in the declared shell-version range. Common hallucinations: Meta.Screen, St.Button.set_label(), GLib.source_remove(), Clutter.Actor.show_all()
8. GObject pattern verification: Check that registerClass calls have GTypeName, that destroy() chains to super.destroy(), that GObject properties emit notify
9. Prefs.js specific checks: Verify fillPreferencesWindow() exists, GTK4/Adwaita patterns used correctly, no deprecated GTK3 patterns, no Shell imports
10. Check for var declarations (should use const/let)
11. Check import ordering (GI → resource → extension)
12. Check for console.log() (banned — only debug/warn/error allowed)

Reviewer perspective notes:

• When a reviewer sees console.log(), they think "developer forgot to clean up debug logging"
• When a reviewer sees module-level let, they think "will this persist across enable/disable cycles?"
• When a reviewer sees try { super.destroy() } catch, they think "AI-generated code"

Phase 5a: AI Pattern Analysis

Using ai-slop-checklist.md (46-item checklist):

1. Split the 46 items by automation status:
   • 1a. Automated items (24 of 46): Items 1-2, 4-5, 8, 11-12, 14-15, 18-26, 28, 34, 36, 41-42, 44. Pull the trigger count directly from Phase 0 lint — do NOT re-read descriptions or re-search the code.
   • 1b. Manual-only items (15 of 46): Items 3, 7, 9, 13, 27, 31-32, 35, 37-40, 43, 45-46. Search extension source for each pattern — these require semantic judgment ego-lint cannot provide.
   • 1c. Partial items (7 of 46): Items 6, 10, 16-17, 29-30, 33. Use ego-lint's result as a starting point, then apply manual judgment for aspects ego-lint cannot cover.
   • 1d. Combine: automated triggered count + manual triggered count + partial triggered count = total score.
2. Record whether it triggers, with file:line references
3. Note whether the pattern is justified by context (check "NOT a signal" exceptions)
4. Count JS files to determine threshold tier:
   • <10 files: standard thresholds (4-6 BLOCKING)
   • 10+ files: size-adjusted thresholds (5-8 BLOCKING)
5. Check ego-lint's quality/code-provenance score — if provenance-score >= 3, apply +2 credit to BLOCKING threshold
6. Include count, category breakdown, threshold used, and assessment in the report
7. When score reaches BLOCKING, provide specific file:line citations for each triggered item so the developer has an actionable fix list

AI Defense Report (for extensions with triggered AI patterns)

For each triggered AI pattern, include a Defense column in the analysis:

#	Pattern	Triggered?	File:Line	Defense
1	Excessive try-catch	Yes	ext.js:45	All try-catch wraps D-Bus calls (justified)
8	TypeScript JSDoc	Yes	lib/api.js:12	Only on 2 exported functions (below threshold)

Defense indicators to check for each triggered item:

• Is the pattern contextually justified? (try-catch around D-Bus, JSDoc on shared API)
• Does the code show domain knowledge that contradicts AI generation?
• Is there a consistent personal style across the file?
• Does the provenance score suggest genuine authorship?

If more triggered items have valid defenses than not, downgrade the verdict by one tier (BLOCKING → ADVISORY, ADVISORY → note only).

Output Format

## EGO Review Report — [Extension Name] v[version]

### Verdict: [LIKELY APPROVED | NEEDS REVISION | LIKELY REJECTED]

**Rejection Risk**: [LOW | MEDIUM | HIGH | VERY HIGH]

---

### Section 1: Blocking Issues (Must Fix)

#### [B1] Issue title (category)
**File**: path/to/file.js:line
**What**: Description of the issue
**Why reviewers reject this**: Explanation with reviewer perspective
**Fix**:
```js
// BEFORE
old code

// AFTER
fixed code

---

Section 2: Justification Required

Items that are acceptable IF properly documented:

[J1] pkexec usage

File: path/to/file.js:line Status: Requires reviewer justification Template: [Include pkexec justification template from security checklist]

---

Section 3: Advisory Issues (May Cause Questions)

[A1] Issue title

File: path/to/file.js:line What: Description Reviewer perspective: What the reviewer thinks when they see this Suggestion: How to fix

---

Section 4: Automated Check Summary (from ego-lint)

Category	Pass	Fail	Warn
Metadata	N	N	N
Security	N	N	N
Lifecycle	N	N	N
Quality	N	N	N

---

Section 5: AI Pattern Analysis

Score: N/46 triggered — [ADVISORY | BLOCKING] Triggered items: list with file:line Assessment: interpretation

---

Section 6: Submission Readiness

Ready to submit? [YES | NO] — N blocking issues remain

Action items (priority order):

1. First thing to fix
2. Second thing to fix

## Rejection-Risk Scoring Model

Calculate based on findings:

| Finding | Risk Points |
|---------|-------------|
| Each BLOCKING lifecycle issue | +3 |
| Each BLOCKING security issue | +4 |
| Each BLOCKING API hallucination | +5 (indicates AI) |
| AI slop score >= 3 | +5 |
| AI slop score >= 6 | +10 |
| Each ADVISORY issue | +1 |
| Justified advisory (with docs) | +0 |

**Verdict thresholds:**
- 0-2 points: **LIKELY APPROVED** — minor or no issues
- 3-6 points: **NEEDS REVISION** — fixable, resubmit after changes
- 7-12 points: **LIKELY REJECTED** — significant issues
- 13+ points: **LIKELY REJECTED** — fundamental problems or AI-generated

**Rough correspondence with ego-simulate scores:**

| ego-review risk | ego-simulate score | Interpretation |
|-----------------|-------------------|----------------|
| 0-2 | 0-4 | Likely to pass |
| 3-6 | 5-9 | May need revision |
| 7+ | 10+ | Likely rejected |

The scales use different inputs (ego-review: finding category points;
ego-simulate: taxonomy weights), so this mapping is approximate. When both
tools are run, prefer ego-review's assessment as the authoritative verdict.

## When to Use

- Before submitting to extensions.gnome.org
- After making significant changes to an extension
- When reviewing someone else's GNOME extension code
- As a learning tool for new extension developers