Fix Headscale installer enrollment flow on macOS#4
Conversation
📝 WalkthroughWalkthroughAdds explicit server flag handling to install scripts and README, maps CLI ChangesVPN Setup Parameter Support and macOS Enrollment
Sequence Diagram(s)sequenceDiagram
participant InstallerScript
participant TailscaleApp
participant MacOSCLI
participant HeadscaleServer
InstallerScript->>TailscaleApp: open app (open -a Tailscale)
TailscaleApp->>MacOSCLI: expose embedded CLI binary
InstallerScript->>MacOSCLI: poll readiness (wait_for_macos_cli_ready)
MacOSCLI->>InstallerScript: ready status
InstallerScript->>MacOSCLI: run `tailscale up` (macos_tailscale up --authkey / browser)
MacOSCLI->>HeadscaleServer: enroll & fetch config
HeadscaleServer->>MacOSCLI: return auth/connection state
MacOSCLI->>InstallerScript: status --json (verify_macos_connection)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@vpn/install.sh`:
- Around line 33-36: The --server|-s) case currently assigns SERVER_URL="$2"
without validating availability; update the case for "--server|-s)" to verify
that $2 exists and does not begin with '-' (indicating another flag) before
assigning SERVER_URL and shifting 2, and if the check fails print a clear
error/usage message and exit non‑zero so callers cannot pass a flag as the
server URL.
- Around line 247-249: The one-shot call to verify_macos_connection may race
with macOS state transitions and should be retried briefly before calling error;
modify the block around verify_macos_connection to implement a short retry loop
(e.g., retry every 0.5–1s for up to 5–10s) so that verify_macos_connection is
invoked repeatedly until success or timeout, and only call error "Tailscale app
is installed..." if the retries all fail; keep the existing error message and
ensure the retry loop is cancellable/limited to avoid hanging the installer.
🪄 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: 57f003e6-d59d-4909-bee8-845f7172c5a3
📒 Files selected for processing (3)
README.mdvpn/install.ps1vpn/install.sh
| echo " - Click Tailscale menu bar icon > Settings" | ||
| echo " - Go to 'CLI' tab" | ||
| echo " - Click 'Enable CLI'" | ||
| echo "" |
There was a problem hiding this comment.
Reveiw why it's taking out the CLI installation instructions
There was a problem hiding this comment.
The old instructions told users to manually open Tailscale → Settings → CLI → "Enable CLI" (and on some setups, create a /usr/local/bin/tailscale symlink by hand). That step is gone
because the script now drives the macOS app's embedded CLI programmatically — it no longer needs the user to click anything in the menu bar.
Concretely, three new functions in vpn/install.sh replace the manual step:
- find_macos_cli() locates the binary inside the app bundle (/Applications/Tailscale.app/Contents/MacOS/Tailscale)
- macos_tailscale() wraps every CLI call with TAILSCALE_BE_CLI=1 — that env var is Tailscale's documented way to invoke the embedded CLI without the GUI "Enable CLI" toggle
- wait_for_macos_cli_ready() polls tailscale status until the backend is responsive
So the removed instructions weren't dropped — they were automated. Users no longer have to enable the CLI or set up symlinks manually.
| } | ||
|
|
||
| wait_for_macos_cli_ready() { | ||
| local i |
There was a problem hiding this comment.
manually test this this if you can to be sure it works.
There was a problem hiding this comment.
Both candidate paths the function checks exist and are executable:
$ ls -l /Applications/Tailscale.app/Contents/MacOS/Tailscale
-rwxr-xr-x ... Tailscale
$ ls -l /Applications/Tailscale.app/Contents/MacOS/tailscale
-rwxr-xr-x ... tailscale
(They're the same inode — one is a case-different reference to the other.)
The function returns 1 if neither is present. The hard-error path that surfaces a clear message is verify_macos_installation() (around line 229), which calls find_macos_cli with if
! find_macos_cli; then error "..."; fi.
End-to-end: ran the installer through the macOS path; find_macos_cli correctly resolved TAILSCALE_CLI to the bundled binary, and subsequent macos_tailscale up calls worked without
any manual CLI-enable step. Happy to record a session if you want to see it.
…on handling for macOS
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vpn/install.sh (1)
184-184: 💤 Low valueRedundant stderr redirection.
&>/dev/nullalready redirects both stdout and stderr, making the trailing2>&1redundant.Suggested fix
- if tailscale version &>/dev/null 2>&1; then + if tailscale version &>/dev/null; then🤖 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 `@vpn/install.sh` at line 184, The conditional checking whether the tailscale binary exists uses redundant redirection in the command "if tailscale version &>/dev/null 2>&1; then"; update the if-statement so stderr/stdout are redirected only once (e.g., keep "&>/dev/null" or use ">/dev/null 2>&1") for the "tailscale version" invocation inside vpn/install.sh to remove the redundant "2>&1" while preserving the same behavior.
🤖 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.
Nitpick comments:
In `@vpn/install.sh`:
- Line 184: The conditional checking whether the tailscale binary exists uses
redundant redirection in the command "if tailscale version &>/dev/null 2>&1;
then"; update the if-statement so stderr/stdout are redirected only once (e.g.,
keep "&>/dev/null" or use ">/dev/null 2>&1") for the "tailscale version"
invocation inside vpn/install.sh to remove the redundant "2>&1" while preserving
the same behavior.
1 participant
Summary by CodeRabbit
Documentation
Enhancements