From 2c24c55f0ed54bf6618f8698ebf5e92bea33ff3b Mon Sep 17 00:00:00 2001
From: dvelton <48307985+dvelton@users.noreply.github.com>
Date: Sat, 4 Apr 2026 18:06:59 -0700
Subject: [PATCH] Address Copilot code review feedback

- Fix browser leak in render_url_to_pdf (try/finally around Playwright)
- Remove setup.sh references from SKILL.md (not bundled in plugin)
- Use consistent <path-to>/eyeball.py paths in SKILL.md
- Update plugin README install instructions for awesome-copilot
- Add Windows pywin32 install step to SKILL.md

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 plugins/eyeball/README.md       | 33 ++++++---------------------
 skills/eyeball/SKILL.md         | 16 ++++++-------
 skills/eyeball/tools/eyeball.py | 40 ++++++++++++++++++---------------
 3 files changed, 37 insertions(+), 52 deletions(-)

diff --git a/plugins/eyeball/README.md b/plugins/eyeball/README.md
index 2d970ccb..6d1ad3d6 100644
--- a/plugins/eyeball/README.md
+++ b/plugins/eyeball/README.md
@@ -22,44 +22,25 @@ If the analysis says "Section 9.3 allows termination for cause with a 30-day cur
 
 ### Install the plugin
 
-Point your CLI at this repo and ask it to install the plugin for you, with this prompt:
+Install via the Copilot CLI plugin system. In a Copilot CLI conversation:
 
 ```
-Install the plugin at github.com/dvelton/eyeball for me.
-```
-
-Or:
-
-Install via the Copilot CLI plugin system, or clone the repo:
-
-```bash
-git clone https://github.com/dvelton/eyeball.git
+install the eyeball plugin from github/awesome-copilot
 ```
 
 ### Install dependencies
 
-**macOS / Linux:**
-
-```bash
-cd eyeball
-bash setup.sh
-```
-
-**Windows (PowerShell):**
-
-```powershell
-cd eyeball
-.\setup.ps1
-```
-
-**Manual install (any platform):**
+After installing the plugin, install the Python dependencies:
 
 ```bash
 pip install pymupdf pillow python-docx playwright
 python -m playwright install chromium
 ```
 
-On Windows, `pywin32` is also needed for Microsoft Word automation and is installed automatically by the setup script.
+On Windows, also install pywin32 for Word automation:
+```bash
+pip install pywin32
+```
 
 ### Verify setup
 
diff --git a/skills/eyeball/SKILL.md b/skills/eyeball/SKILL.md
index 6edff3f2..5ecd73fe 100644
--- a/skills/eyeball/SKILL.md
+++ b/skills/eyeball/SKILL.md
@@ -42,17 +42,17 @@ Before first use, check that dependencies are installed:
 python3 <path-to>/eyeball.py setup-check
 ```
 
-If anything is missing, run the setup script from the eyeball plugin directory:
-```bash
-bash <path-to>/setup.sh
-```
-
-Or install manually:
+If anything is missing, install the required dependencies:
 ```bash
 pip3 install pymupdf pillow python-docx playwright
 python3 -m playwright install chromium
 ```
 
+On Windows, also install pywin32 for Word automation:
+```bash
+pip install pywin32
+```
+
 ## Workflow
 
 Follow these steps exactly. The order matters.
@@ -62,7 +62,7 @@ Follow these steps exactly. The order matters.
 Before writing any analysis, extract and read the full text of the source document:
 
 ```bash
-python3 eyeball.py extract-text --source "<path-or-url>"
+python3 <path-to>/eyeball.py extract-text --source "<path-or-url>"
 ```
 
 Read the output carefully. Identify actual section numbers, headings, page numbers, and key language.
@@ -118,7 +118,7 @@ RIGHT -- includes the section number for precision, targets the correct page:
 Construct a JSON array of sections and call the build command:
 
 ```bash
-python3 eyeball.py build \
+python3 <path-to>/eyeball.py build \
   --source "<path-or-url>" \
   --output ~/Desktop/<title>.docx \
   --title "Analysis Title" \
diff --git a/skills/eyeball/tools/eyeball.py b/skills/eyeball/tools/eyeball.py
index 3c9ab860..d14cdb5d 100755
--- a/skills/eyeball/tools/eyeball.py
+++ b/skills/eyeball/tools/eyeball.py
@@ -241,26 +241,30 @@ def render_url_to_pdf(url, output_pdf_path):
         )
 
     with sync_playwright() as p:
-        browser = p.chromium.launch(headless=True)
-        page = browser.new_page()
-        page.goto(url, wait_until="networkidle", timeout=30000)
+        browser = None
+        try:
+            browser = p.chromium.launch(headless=True)
+            page = browser.new_page()
+            page.goto(url, wait_until="networkidle", timeout=30000)
 
-        # Clean up navigation/footer elements for cleaner output
-        page.evaluate("""
-            document.querySelectorAll(
-                'header, footer, nav, [data-testid="header"], [data-testid="footer"], '
-                + '.site-header, .site-footer, #cookie-banner, .cookie-consent'
-            ).forEach(el => el.remove());
-        """)
+            # Clean up navigation/footer elements for cleaner output
+            page.evaluate("""
+                document.querySelectorAll(
+                    'header, footer, nav, [data-testid="header"], [data-testid="footer"], '
+                    + '.site-header, .site-footer, #cookie-banner, .cookie-consent'
+                ).forEach(el => el.remove());
+            """)
 
-        page.pdf(
-            path=output_pdf_path,
-            format="Letter",
-            print_background=True,
-            margin={"top": "0.5in", "bottom": "0.5in",
-                    "left": "0.75in", "right": "0.75in"}
-        )
-        browser.close()
+            page.pdf(
+                path=output_pdf_path,
+                format="Letter",
+                print_background=True,
+                margin={"top": "0.5in", "bottom": "0.5in",
+                        "left": "0.75in", "right": "0.75in"}
+            )
+        finally:
+            if browser is not None:
+                browser.close()
 
 
 # ---------------------------------------------------------------------------