Skip to content

feat: new app icon & logo from @Tetonne's suggestion (#39)#43

Open
GeiserX wants to merge 2 commits into
mainfrom
feat/icon-tetonne-39
Open

feat: new app icon & logo from @Tetonne's suggestion (#39)#43
GeiserX wants to merge 2 commits into
mainfrom
feat/icon-tetonne-39

Conversation

@GeiserX
Copy link
Copy Markdown
Owner

@GeiserX GeiserX commented Jun 1, 2026

Summary

Implements the icon refresh discussed in #39. @Tetonne suggested a new shield+spear icon and offered it for the app; this PR wires that art in everywhere (app icon, in-app logo, README banner) so it can be tested locally.

Chose the dark-navy variant — it matches the app's existing dark theme and the mood of the old black-background icon, just with a proper macOS squircle instead of a bare black square.

What changed

  • assets/AppIcon.icns — Dock / Finder / Launchpad / notification icon. Full iconset (16→1024, incl. @2x).
  • assets/VPNBypass.png — in-app logo (Settings + menu bar header), 512² transparent.
  • assets/VPNBypass-AppIcon.png — 1024² master source.
  • docs/images/banner.svg — README banner logo slot swapped to the new squircle (now square, 90×90).
  • assets/AppIcon-source-tetonne.png — original art from the issue, kept for provenance.
  • scripts/generate-icon.py — reproducible generator (see note below).

Note on the source art

Tetonne's image is a flattened JPEG — the "transparent" checkerboard is baked into the pixels, not real alpha. scripts/generate-icon.py detects the squircle, crops it, and rebuilds genuine transparency with a macOS continuous-corner (superellipse) mask, then emits every size. Quality is bounded by the ~700px AI-generated source, but it's clean at all sizes.

Test plan

  • Build locally and confirm Dock/Finder icon renders crisp (no gray fringe) at all sizes
  • Check in-app logo in Settings + menu bar header
  • Verify README banner renders on GitHub
  • Confirm menu-bar template icons (state feedback) are untouched — they're intentionally monochrome and out of scope

Closes #39.

Thanks @Tetonne for the icon suggestion!

Summary by CodeRabbit

  • New Features
    • Menu bar header now shows the branded app name with live ON/OFF status; app will display detected VPN/network state in the UI even if the privileged helper isn't ready.
  • Chores
    • Automated generation of app icons and banner assets in multiple sizes from a source image, including a masked RGBA master and additional banner outputs.

Replace the black-background shield icon with the dark-navy shield+spear
squircle suggested by @Tetonne in #39. Applied everywhere: app icon
(AppIcon.icns), in-app logo (VPNBypass.png), and the README banner.

The source art is a flattened JPEG with a baked-in transparency
checkerboard; scripts/generate-icon.py crops the squircle, rebuilds real
alpha via a macOS continuous-corner superellipse mask, and emits the full
iconset plus the 512px in-app logo.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a standalone icon-generation script that detects a content ROI, creates a squircle-style alpha mask, and exports a 1024×1024 RGBA master plus macOS .iconset variants; and adds non-mutating VPN-state detection plus startup/UI plumbing to surface state when the privileged helper is not ready.

Changes

Runtime VPN-state & UI

Layer / File(s) Summary
VPN state detection (non-mutating)
Sources/VPNBypassCore/RouteManager.swift
Adds detectVPNStateOnly() which inspects SSID, VPN interface/type, and local gateway, updates published UI state (isVPNConnected, vpnInterface, vpnType, localGateway, isLoading) and logs a summary without applying routes.
Startup helper readiness handling
Sources/VPNBypassCore/VPNBypassApp.swift
Rewrites the startup path: when HelperManager.shared.ensureHelperReady() is false, call RouteManager.shared.detectVPNStateOnly() to populate UI state and return, skipping route mutations that require the helper; existing flow remains when helper is ready.
Menu bar header simplification
Sources/VPNBypassCore/MenuBarViews.swift
Removes bundled PNG logo rendering from MenuContent.titleHeader, leaving the branded app name and live ON/OFF status indicator.

macOS Icon Generation

Layer / File(s) Summary
Icon generation pipeline and exports
scripts/generate-icon.py
Implements ROI detection from /tmp/tetonne1.png using saturation/brightness heuristics, crops and resizes to 1024×1024, recolors light low-saturation fringe pixels to navy, generates an antialiased superellipse-like alpha mask, composes an RGBA master master_1024.png, writes a .iconset with size/@2x variants, and emits VPNBypass.png, banner_logo.png, and banner_logo.b64.txt.

Sequence Diagram(s)

sequenceDiagram
  participant AppDelegate
  participant HelperManager
  participant RouteManager
  participant NetworkInterfaces
  AppDelegate->>HelperManager: ensureHelperReady()
  alt helper not ready
    AppDelegate->>RouteManager: detectVPNStateOnly() (async)
    RouteManager->>NetworkInterfaces: query SSID / gateway / interfaces
    RouteManager-->>AppDelegate: publish isVPNConnected, vpnInterface, vpnType, localGateway, isLoading=false
  else helper ready
    AppDelegate->>RouteManager: detectAndApplyRoutesAsync()
  end
Loading

🎯 3 (Moderate) | ⏱️ ~20 minutes

"A rabbit nibble, pixels align,
I square the corners, navy shine.
Routes I scent but do not bind,
App shows state while helpers wind.
Icons packed, small art refined 🐰"

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive Code changes address issue #39 requirements: new modern squircle app icon with rounded corners is implemented, logo updates are wired throughout, and VPN state detection is enhanced. However, GUI contrast improvements and xattr documentation are not addressed. Clarify whether contrast optimization and xattr documentation are deferred to separate issues or if this PR is scope-limited to icon/logo only.
Out of Scope Changes check ❓ Inconclusive Changes to RouteManager.detectVPNStateOnly() and AppDelegate startup behavior appear to address the 'Setting Up...' spinner issue mentioned in commit notes but are not explicitly discussed in the PR description or linked issue. Document in the PR description how RouteManager and AppDelegate changes relate to the VPN state display when the helper is not ready.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately reflects the main change: integrating a new app icon and logo from @Tetonne's suggestion, addressing issue #39.
Description check ✅ Passed Description covers Summary, Type of Change (implied as New Feature), and provides detailed context. However, Testing and Checklist sections are incomplete with unchecked boxes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/icon-tetonne-39

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
scripts/generate-icon.py (6)

3-3: ⚡ Quick win

Remove unused math import.

The math module is imported but never used in the script.

♻️ Proposed fix
-import os, math
+import os
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` at line 3, Remove the unused math import from the
top of scripts/generate-icon.py by deleting the ", math" portion of the import
statement (the line "import os, math") so only the used modules (e.g., os) are
imported; ensure no other references to math remain in the file and run a quick
linter/flake8 check to confirm.

1-3: ⚡ Quick win

Document script dependencies.

The script requires numpy and Pillow, but there's no requirements.txt or inline documentation. Add a docstring or comment at the top listing dependencies and usage.

📝 Example docstring
+#!/usr/bin/env python3
+"""
+Generate macOS app icon assets from a source image.
+
+Dependencies:
+  - numpy
+  - Pillow (PIL)
+
+Usage:
+  python generate-icon.py <source_image> [output_dir]
+"""
+
 import numpy as np
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` around lines 1 - 3, Add a module-level docstring at
the top of scripts/generate-icon.py that lists the runtime dependencies (numpy,
Pillow), shows the pip install command (e.g., pip install numpy pillow), and
provides a one-line usage example or invocation pattern for the script; update
or create the top-of-file docstring so future readers and automated tooling can
see the dependencies and how to run the script.

27-27: ⚡ Quick win

Document squircle parameters.

The superellipse exponent n=5.0 and edge width edge=0.06 are magic numbers. Add comments or extract as constants to clarify their effect on the mask shape.

♻️ Example
 # macOS continuous-corner squircle (superellipse) mask, full-bleed
+SQUIRCLE_EXPONENT = 5.0  # higher = more square, lower = more circular
+ANTIALIAS_EDGE = 0.06    # edge softness (0.06 = ~3% of radius)
+
 N=1024
 yy,xx=np.mgrid[0:N,0:N].astype(float)
 xn=(xx-(N-1)/2)/((N-1)/2); yn=(yy-(N-1)/2)/((N-1)/2)
-n=5.0
+n=SQUIRCLE_EXPONENT
 d=np.abs(xn)**n+np.abs(yn)**n
 # antialiased edge around d==1
-edge=0.06
+edge=ANTIALIAS_EDGE
 alpha=np.clip((1.0-d)/edge + 0.5, 0, 1)

Also applies to: 30-30

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` at line 27, The squircle parameters are magic
numbers; extract n and edge into clearly named constants (e.g.,
SQUIRCLE_EXPONENT = 5.0 and EDGE_WIDTH = 0.06) and add brief comments explaining
their effect on the mask shape (higher exponent -> sharper corners, edge
controls border thickness/feather). Update occurrences of n and edge in
scripts/generate-icon.py (and the matching spots around line 30) to use these
constants so the intent is clear and easier to tune.

12-13: 💤 Low value

Split semicolon-separated statements for readability.

Multiple statements on one line reduce readability and are flagged by linters (E702).

♻️ Example for lines 12-13
-mx=np.maximum(np.maximum(r,g),b); mn=np.minimum(np.minimum(r,g),b)
-sat=mx-mn; bright=mx
+mx = np.maximum(np.maximum(r,g),b)
+mn = np.minimum(np.minimum(r,g),b)
+sat = mx-mn
+bright = mx

Also applies to: 20-20, 26-26, 40-40

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` around lines 12 - 13, Split the semicolon-separated
statements into separate lines to fix lint E702 and improve readability: replace
combined assignments like "mx=np.maximum(np.maximum(r,g),b);
mn=np.minimum(np.minimum(r,g),b)" with two distinct statements (one for mx, one
for mn), and do the same for other occurrences such as the "sat=mx-mn;
bright=mx" pair and the similar semicolon-joined lines at the other spots noted;
locate these by the variables used (mx, mn, sat, bright, r, g, b) in
generate-icon.py and refactor each semicolon-delimited expression into its own
line.

19-19: ⚡ Quick win

Extract magic margin constant.

The 6 pixel margin is unexplained. Extract it as a named constant for clarity.

♻️ Example
+CROP_MARGIN = 6  # pixels
-half = side/2.0 + 6   # tiny margin
+half = side/2.0 + CROP_MARGIN
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` at line 19, The hard-coded 6px "tiny margin" in the
calculation half = side/2.0 + 6 should be extracted into a clearly named
constant (e.g., ICON_MARGIN_PX or DEFAULT_MARGIN_PX) declared near the top of
scripts/generate-icon.py; replace the literal in the expression with that
constant and add a short comment explaining its purpose so the margin is
self-documented and easy to adjust.

14-14: ⚡ Quick win

Document or extract magic numbers for maintainability.

The heuristic (sat>28) | ((bright<140)&(b>=r)) uses unexplained constants (28, 140). These make the code hard to tune or understand. Consider adding comments explaining the thresholds or extracting them as named constants.

♻️ Example refactor
+# Heuristic thresholds: detect saturated colors or dark-blue regions
+SAT_THRESHOLD = 28      # min saturation delta (max-min RGB)
+BRIGHT_THRESHOLD = 140  # max brightness for dark region detection
+
-core = (sat>28) | ((bright<140)&(b>=r))
+core = (sat > SAT_THRESHOLD) | ((bright < BRIGHT_THRESHOLD) & (b >= r))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` at line 14, Extract the hard-coded numeric
thresholds in the core pixel heuristic into named constants (e.g., SAT_THRESHOLD
= 28, BRIGHT_THRESHOLD = 140) and replace the inline literals in the expression
used to compute core (the line assigning core = (sat>28) |
((bright<140)&(b>=r))). Add a short comment above the constants explaining their
meaning (why saturation and brightness thresholds were chosen) so future
maintainers can tune them easily, and update any related documentation or tests
that rely on these thresholds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/generate-icon.py`:
- Line 9: Add explicit error handling around opening the source image: check
that the SRC path exists (os.path.exists(SRC)) before calling Image.open or wrap
Image.open(SRC) in a try/except FileNotFoundError to print a clear user-facing
message and exit (use sys.exit(1)). Ensure any new code references the same SRC
variable and the Image.open(...) call so the script fails gracefully when the
source image is missing and informs the user how to fix it.
- Around line 5-7: The script currently hardcodes SRC and OUT (SRC =
"/tmp/tetonne1.png", OUT = "/tmp/iconout") making it non-portable; update
generate-icon.py to parse command-line arguments (e.g., using argparse) to
accept --src and --out (provide sensible defaults such as
"assets/AppIcon-source-tetonne.png" for src and a local output folder for out),
replace the hardcoded SRC and OUT variables with the parsed values, and keep the
existing os.makedirs(OUT, exist_ok=True) behavior so the output directory is
created as needed; ensure any references to SRC and OUT inside functions/classes
are updated to use the new variables (search for SRC, OUT, os.makedirs to locate
usages).
- Around line 14-16: The core mask computation can produce empty xs/ys arrays
causing xs.min()/ys.min() to raise ValueError; after computing ys,xs =
np.where(core) (referencing variables core, xs, ys and the bounding calc
x0,x1,y0,y1) add a guard that checks if xs.size==0 or ys.size==0 and fail
gracefully (raise a clear exception, log an error, or return early) with a
helpful message like "no core pixels detected" before attempting xs.min()/max()
or ys.min()/max().

---

Nitpick comments:
In `@scripts/generate-icon.py`:
- Line 3: Remove the unused math import from the top of scripts/generate-icon.py
by deleting the ", math" portion of the import statement (the line "import os,
math") so only the used modules (e.g., os) are imported; ensure no other
references to math remain in the file and run a quick linter/flake8 check to
confirm.
- Around line 1-3: Add a module-level docstring at the top of
scripts/generate-icon.py that lists the runtime dependencies (numpy, Pillow),
shows the pip install command (e.g., pip install numpy pillow), and provides a
one-line usage example or invocation pattern for the script; update or create
the top-of-file docstring so future readers and automated tooling can see the
dependencies and how to run the script.
- Line 27: The squircle parameters are magic numbers; extract n and edge into
clearly named constants (e.g., SQUIRCLE_EXPONENT = 5.0 and EDGE_WIDTH = 0.06)
and add brief comments explaining their effect on the mask shape (higher
exponent -> sharper corners, edge controls border thickness/feather). Update
occurrences of n and edge in scripts/generate-icon.py (and the matching spots
around line 30) to use these constants so the intent is clear and easier to
tune.
- Around line 12-13: Split the semicolon-separated statements into separate
lines to fix lint E702 and improve readability: replace combined assignments
like "mx=np.maximum(np.maximum(r,g),b); mn=np.minimum(np.minimum(r,g),b)" with
two distinct statements (one for mx, one for mn), and do the same for other
occurrences such as the "sat=mx-mn; bright=mx" pair and the similar
semicolon-joined lines at the other spots noted; locate these by the variables
used (mx, mn, sat, bright, r, g, b) in generate-icon.py and refactor each
semicolon-delimited expression into its own line.
- Line 19: The hard-coded 6px "tiny margin" in the calculation half = side/2.0 +
6 should be extracted into a clearly named constant (e.g., ICON_MARGIN_PX or
DEFAULT_MARGIN_PX) declared near the top of scripts/generate-icon.py; replace
the literal in the expression with that constant and add a short comment
explaining its purpose so the margin is self-documented and easy to adjust.
- Line 14: Extract the hard-coded numeric thresholds in the core pixel heuristic
into named constants (e.g., SAT_THRESHOLD = 28, BRIGHT_THRESHOLD = 140) and
replace the inline literals in the expression used to compute core (the line
assigning core = (sat>28) | ((bright<140)&(b>=r))). Add a short comment above
the constants explaining their meaning (why saturation and brightness thresholds
were chosen) so future maintainers can tune them easily, and update any related
documentation or tests that rely on these thresholds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 130ff0ea-c234-4919-9a77-d7c2ec6ef464

📥 Commits

Reviewing files that changed from the base of the PR and between c003ff8 and e3b48ba.

⛔ Files ignored due to path filters (4)
  • assets/AppIcon-source-tetonne.png is excluded by !**/*.png
  • assets/VPNBypass-AppIcon.png is excluded by !**/*.png
  • assets/VPNBypass.png is excluded by !**/*.png
  • docs/images/banner.svg is excluded by !**/*.svg
📒 Files selected for processing (2)
  • assets/AppIcon.icns
  • scripts/generate-icon.py

Comment thread scripts/generate-icon.py
Comment on lines +5 to +7
SRC = "/tmp/tetonne1.png"
OUT = "/tmp/iconout"
os.makedirs(OUT, exist_ok=True)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Replace hardcoded /tmp paths with configurable arguments.

The script uses hardcoded /tmp paths, making it non-reproducible and non-portable. For a versioned "reproducible generator," the source and output paths must be configurable. The PR summary mentions this script generates assets from assets/AppIcon-source-tetonne.png, yet the script points to /tmp/tetonne1.png.

🔧 Suggested approach

Add command-line arguments:

+import sys
+
+if len(sys.argv) < 2:
+    print("Usage: python generate-icon.py <source_image> [output_dir]")
+    sys.exit(1)
+
-SRC = "/tmp/tetonne1.png"
-OUT = "/tmp/iconout"
+SRC = sys.argv[1]
+OUT = sys.argv[2] if len(sys.argv) > 2 else "assets"
 os.makedirs(OUT, exist_ok=True)
🧰 Tools
🪛 Ruff (0.15.14)

[error] 5-5: Probable insecure usage of temporary file or directory: "/tmp/tetonne1.png"

(S108)


[error] 6-6: Probable insecure usage of temporary file or directory: "/tmp/iconout"

(S108)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` around lines 5 - 7, The script currently hardcodes
SRC and OUT (SRC = "/tmp/tetonne1.png", OUT = "/tmp/iconout") making it
non-portable; update generate-icon.py to parse command-line arguments (e.g.,
using argparse) to accept --src and --out (provide sensible defaults such as
"assets/AppIcon-source-tetonne.png" for src and a local output folder for out),
replace the hardcoded SRC and OUT variables with the parsed values, and keep the
existing os.makedirs(OUT, exist_ok=True) behavior so the output directory is
created as needed; ensure any references to SRC and OUT inside functions/classes
are updated to use the new variables (search for SRC, OUT, os.makedirs to locate
usages).

Comment thread scripts/generate-icon.py
OUT = "/tmp/iconout"
os.makedirs(OUT, exist_ok=True)

im = Image.open(SRC).convert("RGB")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add error handling for missing source file.

The script will crash with FileNotFoundError if the source image does not exist. Add a check or let the user know the requirement upfront.

🛡️ Proposed fix
+if not os.path.isfile(SRC):
+    print(f"Error: Source image not found: {SRC}")
+    sys.exit(1)
+
 im = Image.open(SRC).convert("RGB")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` at line 9, Add explicit error handling around
opening the source image: check that the SRC path exists (os.path.exists(SRC))
before calling Image.open or wrap Image.open(SRC) in a try/except
FileNotFoundError to print a clear user-facing message and exit (use
sys.exit(1)). Ensure any new code references the same SRC variable and the
Image.open(...) call so the script fails gracefully when the source image is
missing and informs the user how to fix it.

Comment thread scripts/generate-icon.py Outdated
Comment on lines +14 to +16
core = (sat>28) | ((bright<140)&(b>=r))
ys,xs=np.where(core)
x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add validation for empty core detection.

If no pixels match the saturation/brightness heuristic (line 14), xs and ys will be empty arrays, causing xs.min() on line 16 to raise ValueError. Add a check to fail gracefully if core detection finds nothing.

🛡️ Proposed fix
 core = (sat>28) | ((bright<140)&(b>=r))
 ys,xs=np.where(core)
+if len(xs) == 0:
+    print("Error: Could not detect content region in source image.")
+    sys.exit(1)
 x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
core = (sat>28) | ((bright<140)&(b>=r))
ys,xs=np.where(core)
x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max()
core = (sat>28) | ((bright<140)&(b>=r))
ys,xs=np.where(core)
if len(xs) == 0:
print("Error: Could not detect content region in source image.")
sys.exit(1)
x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` around lines 14 - 16, The core mask computation can
produce empty xs/ys arrays causing xs.min()/ys.min() to raise ValueError; after
computing ys,xs = np.where(core) (referencing variables core, xs, ys and the
bounding calc x0,x1,y0,y1) add a guard that checks if xs.size==0 or ys.size==0
and fail gracefully (raise a clear exception, log an error, or return early)
with a helpful message like "no core pixels detected" before attempting
xs.min()/max() or ys.min()/max().

- Recolor JPEG checkerboard fringe to navy before superellipse mask so the
  app icon no longer shows white edges on the left/right.
- Remove the logo image from the menu-bar dropdown header, keeping only the
  branded app name text.
- When the privileged helper is not ready, detect VPN state for display only
  instead of leaving isLoading=true forever, which clears the permanent
  "Setting Up..." spinner.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/generate-icon.py`:
- Around line 77-83: The base64 encoding step opens the image file without
closing it; change the one-liner that calls open(f"{OUT}/banner_logo.png",
"rb").read() to explicitly open the image in a with statement (e.g., with
open(f"{OUT}/banner_logo.png", "rb") as img:) and use img.read() for
base64.b64encode, then write the decoded string to the existing
banner_logo.b64.txt file inside its with block so both file handles (the image
and the output) are properly closed; update references to OUT and
banner_logo.png accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b6cc7012-23dd-48aa-bbe5-4e44a059b54d

📥 Commits

Reviewing files that changed from the base of the PR and between e3b48ba and bc1e277.

⛔ Files ignored due to path filters (3)
  • assets/VPNBypass-AppIcon.png is excluded by !**/*.png
  • assets/VPNBypass.png is excluded by !**/*.png
  • docs/images/banner.svg is excluded by !**/*.svg
📒 Files selected for processing (5)
  • Sources/VPNBypassCore/MenuBarViews.swift
  • Sources/VPNBypassCore/RouteManager.swift
  • Sources/VPNBypassCore/VPNBypassApp.swift
  • assets/AppIcon.icns
  • scripts/generate-icon.py
💤 Files with no reviewable changes (1)
  • Sources/VPNBypassCore/MenuBarViews.swift

Comment thread scripts/generate-icon.py
Comment on lines +77 to +83
# banner logo + base64
import base64
small = master.resize((160, 160), Image.LANCZOS)
small.save(f"{OUT}/banner_logo.png")
with open(f"{OUT}/banner_logo.b64.txt", "w") as f:
f.write(base64.b64encode(open(f"{OUT}/banner_logo.png", "rb").read()).decode())
print("logo + banner assets written")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Close file handle properly in base64 encoding.

Line 82 opens a file handle without closing it. While Python's garbage collector will eventually clean it up, this is poor resource management.

🛡️ Proposed fix
-with open(f"{OUT}/banner_logo.b64.txt", "w") as f:
-    f.write(base64.b64encode(open(f"{OUT}/banner_logo.png", "rb").read()).decode())
+with open(f"{OUT}/banner_logo.b64.txt", "w") as f:
+    with open(f"{OUT}/banner_logo.png", "rb") as img:
+        f.write(base64.b64encode(img.read()).decode())
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/generate-icon.py` around lines 77 - 83, The base64 encoding step
opens the image file without closing it; change the one-liner that calls
open(f"{OUT}/banner_logo.png", "rb").read() to explicitly open the image in a
with statement (e.g., with open(f"{OUT}/banner_logo.png", "rb") as img:) and use
img.read() for base64.b64encode, then write the decoded string to the existing
banner_logo.b64.txt file inside its with block so both file handles (the image
and the output) are properly closed; update references to OUT and
banner_logo.png accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: GUI night/day

1 participant