fix: trigger native captive portal popup for Starbucks and Komeda WiFi - #705
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
shunkakinoki
added
the
bug
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability of captive portal detection and triggering for macOS users on Starbucks and Komeda WiFi networks. By switching from a basic Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe neverssl-keepalive service is enhanced by replacing the ping-based connectivity check with a curl-based approach. The macOS launcher PATH environment is expanded for broader command availability, and a new "openclaw" dependency is added to package.json. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Mesa DescriptionTL;DRFixed an issue where the native captive portal popup was not triggering for Starbucks and Komeda WiFi by updating the What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new dependency, openclaw (version 2026.1.29), by updating package.json and bun.lock to include the package and its numerous sub-dependencies. Concurrently, the neverssl-keepalive service is modified: its default.nix configuration now expands the PATH environment variable to include standard system binary directories (/usr/bin:/bin:/usr/sbin:/sbin), and the keepalive.sh script has been updated to use a CURL_OK flag, derived from the neverssl.com check, to determine when to restart WiFi in specific captive portal scenarios, replacing a previous ping check. No review comments were provided for this pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 35: The trustedDependencies list is out of alphabetical order: move the
"openclaw" entry so it appears after "open-composer" within the
trustedDependencies object; edit the package.json trustedDependencies block
(identify the "trustedDependencies" key and the "open-composer" and "openclaw"
entries) to reorder the entries alphabetically, leaving all other keys and
values unchanged.
🧹 Nitpick comments (2)
package.json (1)
64-65: Alphabetical sorting violation in trustedDependencies.
"openclaw"should come after"open-composer"alphabetically to follow the coding guidelines for configuration files.🔧 Proposed fix for alphabetical ordering
"mcporter", - "openclaw", "open-composer", + "openclaw", "oxfmt",home-manager/services/neverssl-keepalive/keepalive.sh (1)
7-14: Stale comment - references ping but code uses curl.Line 7 comment says "Silently ping neverssl.com" but the implementation uses
curl. Update the comment to reflect the actual behavior.📝 Proposed comment fix
-# Silently ping neverssl.com - ignore failures (network may be unavailable) +# Check connectivity via neverssl.com - failures indicate captive portal or no network CURL_OK=false
| "cline": "^1.0.10", | ||
| "mcporter": "^0.7.3", | ||
| "open-composer": "^0.8.23", | ||
| "openclaw": "^2026.1.29", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if openclaw exists on npm
curl -s https://registry.npmjs.org/openclaw | jq '.name, .description, ."dist-tags".latest' 2>/dev/null || echo "Package not found or error fetching"Repository: shunkakinoki/dotfiles
Length of output: 142
🏁 Script executed:
# Read package.json around line 35 to verify dependencies section and sorting
head -n 50 package.json | tail -n 20Repository: shunkakinoki/dotfiles
Length of output: 537
🏁 Script executed:
# Read package.json around line 64 to verify trustedDependencies section
sed -n '55,75p' package.jsonRepository: shunkakinoki/dotfiles
Length of output: 361
🏁 Script executed:
# Get full dependencies and trustedDependencies sections to verify alphabetical ordering
python3 << 'EOF'
import json
with open('package.json', 'r') as f:
data = json.load(f)
print("=== dependencies (keys only) ===")
if 'dependencies' in data:
for key in sorted(data['dependencies'].keys()):
print(f" {key}")
print("\n=== Actual order in file ===")
# Since we can't preserve exact order from json.load, let's check the raw file
print("\n=== trustedDependencies (if present) ===")
if 'trustedDependencies' in data:
print("Found trustedDependencies:")
for item in data['trustedDependencies']:
print(f" {item}")
EOFRepository: shunkakinoki/dotfiles
Length of output: 1058
Fix alphabetical ordering in trustedDependencies.
The openclaw entry in trustedDependencies violates the alphabetical sorting requirement—it should appear after open-composer, not before. Move it to maintain alphabetical order per the configuration file guidelines.
Note: The openclaw package is a legitimate npm package (WhatsApp gateway CLI), and the dependencies section at line 35 is already correctly sorted.
🤖 Prompt for AI Agents
In `@package.json` at line 35, The trustedDependencies list is out of alphabetical
order: move the "openclaw" entry so it appears after "open-composer" within the
trustedDependencies object; edit the package.json trustedDependencies block
(identify the "trustedDependencies" key and the "open-composer" and "openclaw"
entries) to reorder the entries alphabetically, leaving all other keys and
values unchanged.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix the native captive portal popup detection for Starbucks and Komeda WiFi networks on macOS. The main change updates the detection logic to use curl's exit status instead of ping for more reliable captive portal detection.
Changes:
- Modified captive portal detection in
keepalive.shto use curl exit status via aCURL_OKflag instead of ping - Added system paths to the macOS launchd service PATH for command availability
- Added the "openclaw" package as a dependency (unrelated to the PR's stated purpose)
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| package.json | Added "openclaw" dependency with version ^2026.1.29 and included it in trustedDependencies |
| bun.lock | Updated lock file to include openclaw package and its transitive dependencies |
| home-manager/services/neverssl-keepalive/keepalive.sh | Changed captive portal detection from ping-based to curl-based using CURL_OK flag |
| home-manager/services/neverssl-keepalive/default.nix | Added system paths to macOS launchd service PATH |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "cline": "^1.0.10", | ||
| "mcporter": "^0.7.3", | ||
| "open-composer": "^0.8.23", | ||
| "openclaw": "^2026.1.29", |
There was a problem hiding this comment.
The addition of the "openclaw" package and its inclusion in trustedDependencies appears unrelated to the PR's stated purpose of fixing captive portal detection for Starbucks and Komeda WiFi. The PR description makes no mention of this dependency. This should either be removed or explained in the PR description.
| @@ -5,8 +5,10 @@ | |||
| set -euo pipefail | |||
|
|
|||
| # Silently ping neverssl.com - ignore failures (network may be unavailable) | |||
There was a problem hiding this comment.
The comment still refers to "ping neverssl.com" but the code has been changed to use curl. The comment should be updated to accurately reflect the current implementation, such as "Check if neverssl.com is reachable via curl".
| # Silently ping neverssl.com - ignore failures (network may be unavailable) | |
| # Silently check if neverssl.com is reachable via curl - ignore failures (network may be unavailable) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/services/neverssl-keepalive/keepalive.sh">
<violation number="1" location="home-manager/services/neverssl-keepalive/keepalive.sh:22">
P2: Captive portal intercepts (3xx/portal HTML) return curl exit 0, so `CURL_OK` stays true and WiFi restart/popup never triggers, regressing captive portal detection.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Check for Starbucks or Komeda networks | ||
| if [[ $SSID == *"STARBUCKS"* ]] || [[ $SSID == *"Komeda_Wi-Fi"* ]]; then | ||
| if ! ping -c 1 -W 2 1.1.1.1 >/dev/null 2>&1; then | ||
| if [[ $CURL_OK == false ]]; then |
There was a problem hiding this comment.
P2: Captive portal intercepts (3xx/portal HTML) return curl exit 0, so CURL_OK stays true and WiFi restart/popup never triggers, regressing captive portal detection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/neverssl-keepalive/keepalive.sh, line 22:
<comment>Captive portal intercepts (3xx/portal HTML) return curl exit 0, so `CURL_OK` stays true and WiFi restart/popup never triggers, regressing captive portal detection.</comment>
<file context>
@@ -17,7 +19,7 @@ if [[ $OSTYPE == "darwin"* ]]; then
# Check for Starbucks or Komeda networks
if [[ $SSID == *"STARBUCKS"* ]] || [[ $SSID == *"Komeda_Wi-Fi"* ]]; then
- if ! ping -c 1 -W 2 1.1.1.1 >/dev/null 2>&1; then
+ if [[ $CURL_OK == false ]]; then
# Restart WiFi to trigger macOS captive portal popup
networksetup -setairportpower en0 off
</file context>
Changes
Technical Details
keepalive.shto use curl exit status for captive portal detectionCURL_OKflag to track curl status and prevent unnecessary WiFi restartsTesting
Generated with Claude Code by glm-4.7
Summary by cubic
Triggers the macOS native captive portal popup on Starbucks and Komeda Wi‑Fi by switching neverssl-keepalive to curl-based detection and fixing the service PATH. Improves reliability and avoids unnecessary Wi‑Fi restarts.
Bug Fixes
Dependencies
Written for commit 8a5e8be. Summary will update on new commits.