fix(desktop): surface Tailscale Serve setup failures with admin link - #4552
fix(desktop): surface Tailscale Serve setup failures with admin link#4552GenKerensky wants to merge 4 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Preflight `tailscale serve` when enabling HTTPS from the desktop app so failures (e.g. Serve disabled on the tailnet) show an error toast with the same configure URL the CLI prints, instead of silently restarting with an unchecked toggle.
b892ae3 to
e470b80
Compare
There was a problem hiding this comment.
The Effect Service Conventions check found one issue in the new DesktopTailscaleServeConfigureError. Details inline.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
The Effect Service Conventions check found one issue in the new DesktopTailscaleServeConfigureError. Details inline.
Posted via Macroscope — Effect Service Conventions
After a successful Serve preflight, disable Serve when writing desktop settings fails so HTTPS exposure is not left active while UI/settings still report disabled.
Align DesktopTailscaleServeConfigureError with Effect service conventions: preserve the underlying TailscaleCommandError as optional cause, store only safe structural detail/configureUrl fields, and derive message from those attributes instead of a free-form reason string.
| preflightedServePort = servePort; | ||
| } | ||
|
|
||
| const result = yield* desktopSettings |
There was a problem hiding this comment.
🟠 High backend/DesktopServerExposure.ts:594
When enabling Tailscale Serve, the rollback uses Effect.tapError, which only fires on typed errors. If desktopSettings.setTailscaleServe is interrupted or throws a defect after ensureTailscaleServe has already opened the HTTPS mapping, the rollback to disableTailscaleServe never runs — leaving Serve active while the UI and settings still show it as disabled. Consider using Effect.ensuring or an acquire-release pattern so the cleanup executes on all non-success exits, not just typed persistence errors.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/backend/DesktopServerExposure.ts around line 594:
When enabling Tailscale Serve, the rollback uses `Effect.tapError`, which only fires on typed errors. If `desktopSettings.setTailscaleServe` is interrupted or throws a defect after `ensureTailscaleServe` has already opened the HTTPS mapping, the rollback to `disableTailscaleServe` never runs — leaving Serve active while the UI and settings still show it as disabled. Consider using `Effect.ensuring` or an acquire-release pattern so the cleanup executes on all non-success exits, not just typed persistence errors.
Electron's process.execPath inside an AppImage points at the temporary mount, which is unmounted on exit. Use the APPIMAGE env path so restart actually brings the app back up.
| if (appImagePath) { | ||
| return { | ||
| execPath: appImagePath, | ||
| args: [], |
There was a problem hiding this comment.
🟡 Medium app/resolveDesktopRelaunchOptions.ts:23
The AppImage branch returns args: [], dropping all command-line arguments. When the app is launched with a meaningful argument (e.g. a file path or runtime switch) and then relaunches, the relaunched instance starts with no arguments and skips the requested startup behavior. The fix is to preserve argv.slice(1) in both branches, since switching execPath to $APPIMAGE doesn't require discarding args.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/app/resolveDesktopRelaunchOptions.ts around line 23:
The AppImage branch returns `args: []`, dropping all command-line arguments. When the app is launched with a meaningful argument (e.g. a file path or runtime switch) and then relaunches, the relaunched instance starts with no arguments and skips the requested startup behavior. The fix is to preserve `argv.slice(1)` in both branches, since switching `execPath` to `$APPIMAGE` doesn't require discarding args.
1 participant
Summary
tailscale servewhen enabling Tailscale HTTPS in the desktop app, so setup failures fail the Enable action instead of silently restarting with an unchecked toggle.login.tailscale.com/f/serveconfigure URL) without leaking raw stderr secrets.Problem
Enable persisted
tailscaleServeEnabled: trueand relaunched the app, but the child server swallowedtailscale servefailures as warnings. The UI toggle is driven by HTTPS probe success, so it stayed off with no user-visible error. Locally this was:Test plan
vp run --filter @t3tools/tailscale testDesktopServerExposureunit tests (including Serve-not-enabled preflight failure)Note
Surface Tailscale Serve setup failures with admin link and structured errors
setTailscaleServeEnabledfails, a newDesktopTailscaleServeConfigureErroris thrown with a user-readable reason and optionalconfigureUrlextracted from sanitized CLI output (never raw stderr).extractTailscaleServeDiagnostics,formatTailscaleServeUserMessage,extractTailscaleServeConfigureUrl) parse and sanitize CLI errors into structured diagnostics and human-readable messages.setTailscaleServeEnablednow performs a preflightensureTailscaleServecheck before persisting settings, and always returnsrequiresRelaunch=trueon successful enable even when settings are unchanged.📊 Macroscope summarized 4a0919b. 4 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.