Skip to content

fix(desktop): surface Tailscale Serve failures and fix AppImage restarts on Linux - #4698

Closed
GenKerensky wants to merge 24 commits into
pingdotgg:mainfrom
GenKerensky:fix/tailscale-serve-enable-error-toast
Closed

fix(desktop): surface Tailscale Serve failures and fix AppImage restarts on Linux#4698
GenKerensky wants to merge 24 commits into
pingdotgg:mainfrom
GenKerensky:fix/tailscale-serve-enable-error-toast

Conversation

@GenKerensky

@GenKerensky GenKerensky commented Jul 28, 2026

Copy link
Copy Markdown

Two Linux desktop problems, both of which end with the user staring at an app that silently did nothing.

Closes #2830 — that report is this exact pair of symptoms: enabling Tailscale HTTPS either hangs or restarts the app "with seemingly no effect", with "no visible errors" while tailscale serve --bg --https=… was failing underneath with "Serve is not enabled on your tailnet."

Supersedes #4552 (closed draft); that branch's head is many commits behind this one.

1. Enabling Tailscale HTTPS failed silently

tailscale serve was configured by the child backend on boot. If the tailnet refused — Serve not enabled, no HTTPS certs, CLI missing — the failure landed in a log line the user never sees. The app restarted and the toggle simply stayed off.

Enabling now preflights tailscale serve in the main process before persisting, so it can fail loudly with the CLI's own guidance and the login.tailscale.com/f/serve admin link. The failure surfaces three ways: a toast, an inline alert inside the setup dialog (which stays open so you can retry), and a status line on the row. The setup URL is shown in full and is selectable, with an Open setup button as a shortcut.

Related fixes in the same area:

  • Disable now tears Serve down from the main process. It previously relied on the child server's acquireRelease finalizer, which only exists if that child booted with Serve enabled. After a failed relaunch it doesn't, so toggling off persisted enabled: false while tailscale serve --https=<port> stayed live on the tailnet. A teardown that fails now refuses to record itself as done, and says the backend may still be reachable.
  • The switch reflects the persisted setting, not endpoint reachability. status comes from an HTTP probe of the MagicDNS URL, so a cert still provisioning or a brief tailnet blip rendered the switch off while Serve was live — and since disabling sits behind the on state, there was no way to turn it off.
  • The toggle renders without LAN network access. Serve only proxies to loopback, so it never depended on network-accessible mode; it was hidden anyway. MagicDNS is still resolved only on explicit opt-in, preserving the fix(desktop): stop looping macOS TCC permission prompts #2745 macOS TCC gate.
  • Failed CLI spawns report "Could not run the tailscale CLI. Is Tailscale installed and on PATH?" instead of a generic port message — the most common failure, previously the least explained.
  • A persistence failure no longer tears down a pre-existing working binding, and the MagicDNS cache is invalidated on user-initiated resolve so "start Tailscale and try again" isn't defeated by a 60s stale miss.

2. AppImage restarts never came back

app.relaunch() re-execs process.execPath, which under AppImage points inside the FUSE mount that is unmounted as the process exits — so the restart died with "Cannot mount AppImage, please check your FUSE setup."

The AppImage path now schedules a detached, delayed re-exec of $APPIMAGE after the current process exits, anchored at cwd: / so it isn't holding the mount. The spawn is awaited, so a failure is logged instead of falling through to exit(0) and vanishing. The single-instance lock is released only once the relaunch is committed, so a failure leaves the app running with the lock rather than letting the next launch open a second instance.

Testing

Unit tests cover the new behavior: eager teardown asserts the exact serve --https=<port> off invocation, a failed teardown keeps enabled: true and carries the exposure warning, the missing-binary case produces the PATH hint, and the relaunch helper resolves only after a real spawn.

Verified end-to-end by driving the built desktop app against a live tailnet:

  • Tailscale absent from PATH → toast, toggle stays off.
  • Tailnet refuses Serve → dialog stays open with the inline alert, full URL, and Open setup; error anchored to the Tailscale HTTPS row.
  • Happy path → enable put https://<magicdns> → proxy http://127.0.0.1:<port> in tailscale serve status, the endpoint returned 200, the app relaunched and came back with the toggle on; disable returned No serve config with the endpoint unreachable and the toggle off.
  • openExternal confirmed to resolve true/false rather than reject, which is why the old .catch() fallback was dead code.

On a real AppImage

Built with dist:desktop:linux and run from the packaged artifact (genuine FUSE mount), across two relaunch cycles:

  • Each relaunch logged kind: appimage-delayed re-execing $APPIMAGE, and the app came back every time with zero "Cannot mount AppImage" errors.
  • Serve re-bound to the relaunched instance's new backend port on enable, and returned No serve config on disable.

This surfaced one more bug, fixed here: the restart worked, but every cycle stranded the previous mount, with its runtime process parked in fuse_dev_do_read forever. Chromium keeps fds open without CLOEXEC and spawn cannot close them, so the detached helper carried nine of them — app.asar, icudtl.dat, the .pak files, the mount directory — into the new process tree, pinning a mount nothing needed. The helper now closes every fd above 2 before sleeping. After the fix: one mount and one runtime process across both cycles.


🤖 Generated with Claude Code

https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R


Note

High Risk
Changes Tailscale Serve enable/disable, persistence rollback, and main-process network exposure in security-sensitive paths, plus process exit/relaunch and single-instance locking on Linux AppImage.

Overview
Tailscale HTTPS no longer fails quietly: enabling preflights tailscale serve in the main process before settings are saved, surfaces DesktopTailscaleServeConfigureError with safe CLI-derived text and an optional admin setup URL (toast, dialog alert, row status), and disables Serve from the main process so a failed child relaunch cannot leave the tailnet exposed while settings say off. Rollback on failed persistence/interrupt, resolveTailscaleHttpsEndpoint for on-demand MagicDNS (new IPC), toggle driven by persisted tailscaleServeEnabled (not HTTP probe), Serve independent of LAN network access, and isShareableOrigin so custom-scheme desktop origins are not offered as pairing links.

Linux AppImage relaunch avoids FUSE mount races via a delayed detached re-exec of $APPIMAGE (bash closes inherited fds when available), awaits helper spawn before exit, and releases the single-instance lock on success or relaunch failure.

@t3tools/tailscale adds stderr diagnostics (serve-not-enabled, no-https-certs), sanitized configure URL extraction, and formatTailscaleServeUserMessage.

Reviewed by Cursor Bugbot for commit 7e0e879. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Surface Tailscale Serve failures with actionable errors and fix AppImage restarts on Linux

  • When enabling/disabling Tailscale Serve fails, a structured DesktopTailscaleServeConfigureError is returned with a sanitized user-facing message, optional Tailscale admin configureUrl, and the original CLI error — replacing silent failures.
  • The Connections settings UI displays inline Tailscale Serve errors with an optional 'Open setup' button that safely opens the Tailscale admin page; the Serve toggle now reflects the persisted tailscaleServeEnabled state and shows a spinner during updates.
  • Enabling Serve runs a CLI preflight before persisting settings; on persistence failure, the tailnet binding is rolled back; on successful enable, requiresRelaunch is set to force child process rebind.
  • AppImage relaunch on Linux now spawns a detached shell helper with a delay to avoid FUSE unmount races, and explicitly releases the single-instance lock before exit.
  • Adds a new resolveTailscaleHttpsEndpoint IPC method and desktop bridge call so the renderer can trigger an on-demand MagicDNS resolve for Tailscale HTTPS setup.
  • Behavioral Change: setTailscaleServeEnabled now actively calls tailscale serve ... off when disabling (instead of relying only on child process finalizers), and re-enabling always sets requiresRelaunch=true.

Macroscope summarized 7e0e879.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ca354f7-9c93-4661-a832-6f5a28948133

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions github-actions Bot added size:XL 500-999 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 28, 2026
Comment thread apps/desktop/src/app/DesktopLifecycle.ts
Comment thread apps/desktop/src/app/DesktopLifecycle.ts
Comment thread apps/desktop/src/backend/DesktopServerExposure.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

1 blocking correctness issue found. This PR introduces significant new runtime behavior including Linux AppImage relaunch logic, Tailscale Serve error handling with preflight/rollback, and new IPC methods. An unresolved high-severity comment identifies a potential network exposure issue when relaunch fails after persistence succeeds. These changes warrant careful human review.

You can customize Macroscope's approvability policy. Learn more.

Comment thread apps/desktop/src/app/resolveDesktopRelaunchOptions.ts
Comment thread apps/desktop/src/app/resolveDesktopRelaunchOptions.ts
Comment thread apps/desktop/src/app/resolveDesktopRelaunchOptions.ts
@GenKerensky

Copy link
Copy Markdown
Author

Pushed 529d2a6e9 (review fixes) and fdaa00855 (merge with main). All four inline findings are addressed and replied to individually; conflicts resolved and the PR is mergeable again.

On the Bugbot dash finding — it was right and my reasoning was wrong, so calling it out rather than burying it in a thread. I had claimed eval made the multi-digit exec N>&- parse-safe. It is not a parse error: dash reads 10 as a command name, and a failed exec terminates a non-interactive shell before || true is ever evaluated. On Debian/Ubuntu the helper would have died before the re-exec, after the parent released the single-instance lock and exited — the app gone for good, which is the exact bug this PR exists to fix. Reproduced in debian:stable-slim with an inherited multi-digit fd (dash cannot even open one itself, so it has to come from the parent):

shape result
with fd cleanup exit=127, relaunch never reached
without REACHED_RELAUNCH, exit=0

The cleanup is now emitted only when the helper runs under bash.

On the merge — worth flagging because it was not mechanical. #4556 replaced detail/configureUrl on TailscaleCommandExitError with a closed stderrDiagnostic label set, since stderr can carry auth keys and the field is logged. This PR needs to name a Serve failure and link the admin page, so instead of picking a side I extended the label set with serve-not-enabled and no-https-certs, added describeTailscaleStderrDiagnostic for prose at the UI edge, and kept the shared message generic — #4556's test asserting that still passes unchanged. configureUrl is back as a separate field, exempt from the label-only rule because it is pattern-matched and validated to https://login.tailscale.com/f/serve... rather than lifted from stderr. Happy to revisit if you would rather the URL travel some other way.

Not mine, but visible on this branch: DesktopObservability and DesktopBackendManager tests fail, and packages/shared/httpReadiness.ts plus scripts/lib/dev-share.test.ts have typecheck errors. All are in main's own files — reverting main's OtlpExporter.layerFlusher line locally makes DesktopBackendManager pass again. I left them alone rather than widen this diff.

Comment thread apps/desktop/src/backend/DesktopServerExposure.ts
Comment thread apps/desktop/src/backend/DesktopServerExposure.ts
// was already enabled on the same port, `tailscale serve --bg` is a
// no-op and disabling it on a persistence failure would tear down a
// working HTTPS endpoint that settings still record as enabled.
preflightedServePort =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High backend/DesktopServerExposure.ts:639

When the user toggles Tailscale Serve enable and specifies a new servePort, ensureTailscaleServe creates a binding on the new port in the preflight, but the preflighted port is only recorded for rollback when a persistence failure occurs. If persistence succeeds but the subsequent lifecycle.relaunch fails (a scenario this code explicitly handles on the disable path), the new-port binding remains live on the tailnet with no child finalizer to remove it — the still-running child only cleans up the old port on shutdown. Consider tracking the preflighted binding through the relaunch step so it is torn down when relaunch fails, or transferring ownership of the new binding without relying solely on a successful relaunch.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/backend/DesktopServerExposure.ts around line 639:

When the user toggles Tailscale Serve enable and specifies a new `servePort`, `ensureTailscaleServe` creates a binding on the new port in the preflight, but the preflighted port is only recorded for rollback when a *persistence failure* occurs. If persistence succeeds but the subsequent `lifecycle.relaunch` fails (a scenario this code explicitly handles on the disable path), the new-port binding remains live on the tailnet with no child finalizer to remove it — the still-running child only cleans up the old port on shutdown. Consider tracking the preflighted binding through the relaunch step so it is torn down when relaunch fails, or transferring ownership of the new binding without relying solely on a successful relaunch.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't think this one is a defect — pushing back rather than changing it, but happy to be shown wrong.

The scenario is: enable with a new servePort, preflight binds it, persistence succeeds, then lifecycle.relaunch fails. Two things make the outcome consistent rather than leaked:

  1. The new-port binding matches persisted settings. After a successful write, settings say enabled: true, port: <new> and stateRef is updated to result.settings.tailscaleServePort (DesktopServerExposure.ts:738-742). A live binding on that port is the state the user asked for. That is also why the rollback is deliberately scoped to persistence failure: once the write lands, tearing the binding down would be undoing the user's request. The invariant this PR is defending is "settings and the tailnet agree", and here they do.

  2. The old-port binding is removed by the child that created it. The finalizer disables configured.servePort — the port that child bound at boot (apps/server/src/server.ts:450) — so the stale binding goes away whenever that child stops, including on the exit path a failed relaunch now takes (689814ae3).

A later disable also targets the right port, since stateRef carries the new one, so the binding never becomes unreachable to the user.

The one edge I'll concede: if the relaunch fails, the app exits and the new-port binding stays live pointing at a backend that is no longer running. That is a refused connection rather than an exposure, and the next launch re-binds it from the same persisted settings — so I don't think it warrants tracking the binding through the relaunch step. If you'd rather it be torn down in that window anyway, say so and I'll add it; it would mean treating a successful persist as provisional until the relaunch lands, which is a bigger design change than I want to make unilaterally.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

@GenKerensky
GenKerensky force-pushed the fix/tailscale-serve-enable-error-toast branch from 689814a to 54bc913 Compare July 28, 2026 17:47
Comment thread packages/tailscale/src/tailscale.ts

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One Effect convention issue found: a newly introduced Effect.catchTag in the Tailscale Serve teardown path. Per the check conventions, statically-known tagged failures should be caught with Effect.catchTags({ ... }) (even for a single tag).

Posted via Macroscope — Effect Service Conventions

Comment thread apps/desktop/src/backend/DesktopServerExposure.ts Outdated
Comment thread apps/web/src/components/settings/ConnectionsSettings.tsx
@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Jul 29, 2026
GenKerensky and others added 13 commits July 29, 2026 12:57
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.
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.
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.
Electron.relaunch spawns the new process while the current one still holds
the single-instance lock, so the child immediately exits after
requestSingleInstanceLock fails. Release the lock first, and preserve
AppImage flag args such as --no-sandbox from Gear Lever launches.
Electron.relaunch starts the next AppImage while the current FUSE mount is
still tearing down, which surfaces "Cannot mount AppImage". Schedule a
detached delayed re-exec of \$APPIMAGE after releasing the single-instance
lock instead.
Tailscale Serve only proxies to loopback, so MagicDNS discovery should not
depend on LAN network-accessible mode. Always resolve Tailscale endpoints
(with the existing status cache) so the settings toggle appears independently.
Keep the pingdotgg#2745 passive gate so Connections mounts do not spawn
`tailscale status` while local-only and Serve is off. Always show the
Tailscale HTTPS toggle and resolve MagicDNS via a dedicated IPC path when
the user enables it, so Serve stays independent of LAN network access.
…unch

Surface the main-process CLI guidance in the enable/disable/resolve toasts
instead of Electron's raw `Error invoking remote method '<channel>': ...`
wrapper, and branch on `openExternal`'s boolean result — it resolves `false`
rather than rejecting, so the "Open setup" `window.open` fallback was dead
code and the button could silently no-op.

Only roll Serve back when the preflight actually created the binding. Over an
already-enabled Serve on the same port `tailscale serve --bg` is a no-op, so a
settings-write failure would tear down a live HTTPS endpoint that settings
still record as enabled.

Force a fresh `tailscale status` on user-initiated endpoint resolution. The
MagicDNS cache stores a failed lookup as a successful `null` for its TTL, so
the toast's "Start Tailscale and try again" advice kept failing for a minute.

Anchor the detached AppImage re-exec helper at `cwd: "/"`; its inherited cwd
can sit inside the `$APPDIR` FUSE mount unmounted during the sleep. Use a
namespace import for `node:child_process` so `vp lint` stops erroring on
`t3code(namespace-node-imports)`, drop the never-called
`resolveDesktopRelaunchOptions` / `DesktopRelaunchOptions`, and fix the
useless-escape warning in both copies of the Serve configure-URL pattern.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
Disabling Tailscale HTTPS relied on the child server's acquireRelease
finalizer, which only exists when that child booted with Serve enabled. After
a relaunch failure — which `lifecycle.relaunch` logs and swallows — the old
child has no finalizer, so toggling off persisted `enabled: false` while
`tailscale serve --https=<port>` stayed live on the tailnet. Disable from the
main process instead, and surface a failure rather than recording a teardown
we could not perform.

Route spawn/timeout/output CLI failures through formatTailscaleServeUserMessage
so the toast says "Could not run the tailscale CLI. Is Tailscale installed and
on PATH?" instead of the generic "on port N" fallback. That is the most common
way enabling fails, and it was the one case the new error path did not explain.

Await the detached AppImage helper's spawn before exiting. `spawn` reports
ENOENT/EAGAIN asynchronously, so a failure previously fell through to exit(0):
the app vanished and never came back, with nothing logged. Release the
single-instance lock only once the relaunch is committed — both paths start the
successor after this process exits, so the ordering is safe, and on failure we
stay running with the lock held instead of letting the next launch open a
second full instance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
…ts dialogs

Enabling resolves MagicDNS through the tailscale CLI before any dialog opens,
and the eager teardown made disabling a CLI round-trip too. The switch was
already gated on `isUpdatingTailscaleServe`, but a dimmed switch was the only
feedback during that wait, so add the house `Spinner` beside it — matching the
spinner the confirm dialogs already show on their action buttons.

Both dialogs already stayed open on failure (they close only after a successful
await), but the reason reached the user solely as a toast, which is easy to miss
behind a modal. Render the failure inline in each dialog instead, carrying the
admin setup link as an action so the fix is one click from the error. Clear the
message when a dialog opens so a previous attempt's failure is not waiting
there, and reuse one openExternal helper for both the toast and the alert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
The Tailscale handlers wrote to desktopServerExposureMutationError, which is
only rendered in the "Network access" row's status slot. A failed Tailscale
enable therefore printed its error under the wrong control, pointing the user
at a setting that had nothing to do with the failure.

Give Tailscale Serve its own error state and render it in the Tailscale HTTPS
row. The two settings are independent — network access is LAN exposure, Serve
is a tailnet proxy — so they need independent status lines.

Found by driving the built desktop app: with `tailscale serve` refused by the
tailnet, the message appeared beneath "Limited to this machine."

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
The alert stripped the admin URL out of the CLI message on the grounds that
the "Open setup" button already carried it. But the CLI phrases this as
"... To enable, visit: <url>", so removing the URL left a dangling colon
pointing at nothing — an empty slot next to a button, which reads as a
rendering bug.

Show the whole message and let `wrap-anywhere` break the URL inside the
dialog's narrow column. The button stays a shortcut rather than the only way
to reach the link: the URL is now selectable and copyable, and the dialog
matches what the toast already displayed. Also wrap the settings-row status
line, which can carry the same long URL.

Verified in the built desktop app against a tailnet that refuses Serve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
GenKerensky and others added 9 commits July 29, 2026 12:58
Drive the Tailscale HTTPS switch from the persisted setting rather than
endpoint reachability. `status` comes from an HTTP probe of the MagicDNS URL,
so a cert still provisioning or a momentary tailnet blip rendered the switch
off while `tailscale serve` was live — and because the disable flow sits behind
the on state, there was then no way to turn it off. This got worse when the row
started rendering unconditionally.

Always append "It may still be reachable on your tailnet" on the disable path.
Routing spawn/timeout failures through formatTailscaleServeUserMessage populated
`detail`, which suppressed that branch of the message, so a teardown blocked by a
missing CLI reported only "Could not run the tailscale CLI" — reading like a
no-op when exposure in fact persists. Regression from the earlier fix; covered
by a test now.

Replace the window.open fallback with an explicit toast. The main window's
setWindowOpenHandler routes window.open back into the same openExternal and
denies the popup, so on refusal the button was still a silent no-op — the
boolean check alone did not fix it. The URL is in the alert text and selectable,
so the toast points there.

Reject out-of-range Serve ports at the IPC boundary; settings persistence
normalizes invalid values to 443, so a bare Schema.Number let the CLI bind one
port while settings recorded another. Round the AppImage helper's sleep to whole
seconds, since POSIX specifies sleep as taking an integer and a shell rejecting
"1.0" would short-circuit the exec with nothing logged. Gate the helper's test
on /bin/sh existing rather than assuming a POSIX dev machine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
…nmount

Found by running a real AppImage. The relaunch worked — new instance, no
"Cannot mount AppImage" — but each restart left the previous AppImage's FUSE
mount in place and its runtime process parked in `fuse_dev_do_read` forever.
Two restarts, two stranded mounts.

Chromium holds fds open without CLOEXEC (`app.asar`, `icudtl.dat`,
`v8_context_snapshot.bin`, the `.pak` files, and the mount directory itself),
and `spawn` cannot close them: `stdio: "ignore"` only covers 0-2. The detached
helper inherited nine of them and carried them through `exec` into the new
runtime, the new Electron, and the backend — all pinning a mount they no longer
have any use for. Setting `cwd: "/"` closed the cwd channel; this is the other
one.

Close every fd above 2 before sleeping, so the unmount happens during the delay
rather than never. The multi-digit `exec N>&-` sits inside an `eval` string
deliberately: shells disagree about fd numbers above 9, and a parse error in the
script body would mean never reaching the exec — an app that quits and never
returns. Deferring the parse makes the worst case "fds stay open" rather than
"the app is gone".

Verified on a packaged AppImage across two relaunch cycles (enable, then
disable): one mount and one runtime process throughout, each relaunch logging
`kind: appimage-delayed`, Serve re-binding to the new backend port and tearing
down cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
Bugbot was right about dash and my reasoning was wrong. `eval "exec 10>&-"`
is not parse-protected: dash reads `10` as a command name, and a failed `exec`
terminates a non-interactive shell immediately, so `2>/dev/null || true` never
runs. On Debian and Ubuntu — where /bin/sh is dash — the helper would die
before the re-exec, after the parent had already released the single-instance
lock and exited. The app would have vanished for good, which is the exact bug
this PR exists to fix.

Reproduced in a Debian container with an inherited multi-digit fd: the old
command exits 127 without relaunching; the new one reaches the relaunch. The
fd cleanup is now emitted only when the helper runs under bash, which every
AppImage-shipping distribution has; /bin/sh gets a plain sleep-and-exec that
relaunches correctly but leaves the old mount behind.

Exit rather than linger when the relaunch cannot be scheduled. `quitting` is
already set and the backend already torn down by that point, so staying alive
just held the single-instance lock on a dead instance and every later launch
focused *this* window instead of starting a usable one. My earlier comment
claiming the lock protected the user had it backwards.

Re-enable Serve when persisting a disable fails, mirroring the enable path's
rollback. Serve was being torn down before the write with no compensation, so
a failed write left settings and stateRef advertising an HTTPS endpoint that
no longer answered.

Route the helper's stderr to a relaunch log. The delayed `exec` fires after we
have exited, so a missing or non-executable AppImage could not otherwise be
reported at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
…unch

Both findings from the latest Bugbot pass are real, and both are on the log
redirect I added one commit ago to make delayed exec failures diagnosable.

`{ sleep && exec ...; } 2>>log` fails outright when the log's directory is
missing or unwritable — the shell never reaches the re-exec, and the parent has
already exited. Verified in a Debian container: bash exits 1 and dash exits 2,
neither relaunching. That traded a rare silent failure for a guaranteed one,
which is the opposite of the point.

The same redirection also outlived a *successful* exec, so the relaunched app
would spend its entire run appending Chromium stderr to an unrotated file.

Test for the AppImage instead and append the failure line after the exec, with
`2>/dev/null` so a bad log path cannot matter. Verified in both shells: a
missing log directory still relaunches, a missing AppImage leaves the message
in the log, and fd 2 after a successful exec is the inherited stdio rather than
the log file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
`tailscale serve --https=<port> off` exits 1 with "handler does not exist"
when nothing is bound on that port. Confirmed against the real CLI:

    $ tailscale serve --https=8443 off
    error: failed to remove web serve: handler does not exist
    exit=1

The eager teardown treated that as a failure, so settings never recorded
`enabled: false`: the toggle stayed on and the user was warned the backend
might still be reachable, when in fact nothing was serving. It bit hardest in
precisely the case the eager teardown was added for — settings saying enabled
while no child ever bound Serve — leaving the user unable to turn it off at all.

Upstream anticipated this: pingdotgg#4556's label set carries `no-existing-handler` with
a comment naming `serve off` on an unmapped port as the case callers should
recognize. Match on it and continue.

The rollback is now armed only when a binding was actually removed. Re-creating
one that never existed would expose the backend rather than restore it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
`Effect.tapError` only observes typed failures. If the fiber is interrupted
after `ensureTailscaleServe` succeeded but before or during the settings write —
the app quitting mid-toggle is the obvious way — the rollback was skipped and
the new binding stayed live on the tailnet while persisted settings still said
`enabled: false`. A defect in the settings write had the same effect.

That is the silent-exposure case this whole path exists to prevent, so switch to
`Effect.onExit` and compensate on every non-success exit. The finalizer covers
both directions: removing a preflighted binding, or restoring one the eager
teardown removed.

Covered by a test that makes the settings write die and asserts the CLI saw
`serve --bg` followed by `serve off`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
Two things surfaced by rebasing onto current main.

The desktop error mapping reads its reason from the classified
`stderrDiagnostic` label via `describeTailscaleServeExitCause`, rather than the
`detail` field pingdotgg#4556 removed. That adaptation lived only in the merge commit
this branch previously carried, so replaying the commits linearly dropped it.

Main's new `DesktopLifecycle.test.ts` builds a complete `ElectronApp` mock, and
this branch adds `releaseSingleInstanceLock` to that service — so the new test
stopped typechecking once the two met. Added the missing member to the mock.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ajcLHZtGhRQRD6BwscU9R
…he match

The extraction regex runs from a fixed prefix to the end of the URL token, so
validating the parsed origin and path left the query string arriving verbatim
from stderr — and that value reaches TailscaleCommandExitError.message, which
is logged. `?authkey=tskey-...` would have ridden straight through, defeating
the label-only rule stderrDiagnostic exists to enforce.

Both copies now rebuild the URL from a literal origin and path, keeping only a
`node` parameter that looks like a stable ID. Unrecognized parameters cost at
most the admin console's node preselection; /f/serve is still the page that
fixes the failure. The web copy is a duplicate of the package helper (that
package is Node-only and cannot be imported into the renderer) and had the same
hole, so it gets the same treatment.

Also:

- Hold the Tailscale HTTPS switch disabled until exposure state loads, matching
  the network access toggle above it. Unresolved state renders as off, so an
  already-enabled Serve looked disabled, and clicking would enable against
  DEFAULT_TAILSCALE_SERVE_PORT rather than open the disable dialog.
- Use Effect.catchTags for the single-tag teardown catch, per house convention.
- Add the two diagnostic labels this branch introduced to dev-share's
  explanation map. That Record is exhaustive over the label union, so extending
  the union broke its compile.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UZ1f7q6ZVbn6uqWgWRzh9
…ronApp mock

Same integration break as DesktopLifecycle.test.ts: main added a new
ElectronApp mock while this branch added a member to the service, so their
test stops typechecking once the two meet. Neither side is wrong alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UZ1f7q6ZVbn6uqWgWRzh9
@GenKerensky
GenKerensky force-pushed the fix/tailscale-serve-enable-error-toast branch from f61efdd to fe544c7 Compare July 29, 2026 16:59
Comment thread apps/web/src/components/settings/ConnectionsSettings.tsx
… clients

`isLocalBackendRemotelyReachable` gates the Authorized clients section, and its
Tailscale operand was an HTTP probe of the MagicDNS URL. That was harmless while
Serve implied network-accessible mode, because the LAN operand covered the same
cases. This branch lets Serve stand alone without LAN, which leaves the probe
carrying the condition by itself — so a cert still provisioning, or a brief
tailnet blip, hid Authorized clients while the toggle directly above it
correctly showed Serve on.

Persisted `tailscaleServeEnabled` now counts on its own, for the same reason the
switch reflects the setting rather than the probe. Strictly additive: the
section appears in more cases, never fewer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UZ1f7q6ZVbn6uqWgWRzh9

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4a9c9e4. Configure here.

Comment thread apps/web/src/components/settings/ConnectionsSettings.tsx
Follow-on from 4a9c9e4. Counting persisted Serve as remotely reachable made
Authorized clients render in a state it never used to: Serve on, MagicDNS not
resolved yet. There `endpointPairingUrl` and `endpointUrl` are both empty, so
`shareablePairingUrl` fell through to the current origin — and the packaged app
loads from `t3code-dev://app/`, whose hostname `app` is not a loopback name.
The loopback-only test therefore called it shareable, and the UI offered copy
and QR for an Electron-only URL instead of the token-only fallback.

The loopback check was always a proxy for "can another device open this"; a
custom scheme fails that for the same reason localhost does, and only became
reachable here once the section could render without a resolved endpoint.
`isShareableOrigin` now requires a web scheme too, and lives in pairingUrls.ts
next to the other pairing URL helpers, where it is unit tested.

Hosted web (https, non-loopback) and dev (localhost) are both unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UZ1f7q6ZVbn6uqWgWRzh9
@GenKerensky

Copy link
Copy Markdown
Author

Closing in favour of two focused PRs, per CONTRIBUTING's "do not mix unrelated fixes together" and the size guidance:

They share no code, so either can be reviewed and merged independently. Every line of this PR is accounted for in one of the two (445 + 1453 = 1898), and both were verified to typecheck, lint, and pass tests standalone against current main.

Thanks to @cursor and @macroscopeapp — the findings raised here are all carried into the split branches.

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

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tailscale HTTPS - Show the "Serve is not enabled on your tailnet." error in GUI.

1 participant