feat(desktop): add Tailscale endpoint add-on#2363
feat(desktop): add Tailscale endpoint add-on#2363juliusmarminge merged 19 commits intot3code/advertised-endpointsfrom
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 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)
Comment |
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a substantial new feature adding Tailscale endpoint discovery and desktop-managed SSH launch capabilities. It includes ~1400 lines of new SSH tunnel management code, new IPC channels, UI components for password prompts, and changes to the environment connection service. Unresolved review comments have identified a medium-severity race condition in tunnel management and potential SSH process leaks that warrant attention before merging. You can customize Macroscope's approvability policy. Learn more. |
a9381f0 to
660f950
Compare
c48c1fa to
946d3b6
Compare
660f950 to
7bccf71
Compare
946d3b6 to
b02652e
Compare
7bccf71 to
2045416
Compare
97b548b to
0f13603
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Tailscale endpoints advertised as available in local-only mode
- Added an early return in getDesktopAdvertisedEndpoints to skip Tailscale endpoint resolution when exposure mode is 'local-only', preventing unreachable CGNAT addresses from being advertised.
Or push these changes by commenting:
@cursor push 14fa24e035
Preview (14fa24e035)
diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts
--- a/apps/desktop/src/main.ts
+++ b/apps/desktop/src/main.ts
@@ -326,6 +326,9 @@
exposure,
customHttpsEndpointUrls: resolveCustomHttpsEndpointUrls(),
});
+ if (exposure.mode === "local-only") {
+ return coreEndpoints;
+ }
const tailscaleEndpoints = await resolveTailscaleAdvertisedEndpoints({
port: backendPort,
networkInterfaces: OS.networkInterfaces(),You can send follow-ups to the cloud agent here.
0f13603 to
b9c798a
Compare
1001ee1 to
ebaa38f
Compare
b9c798a to
b642027
Compare
ebaa38f to
3335b4e
Compare
b642027 to
1ea2ac7
Compare
3335b4e to
811089e
Compare
1ea2ac7 to
bdf8358
Compare
811089e to
cdce392
Compare
bdf8358 to
ce784b1
Compare
cdce392 to
ee85eb4
Compare
ce784b1 to
ddb27a7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Tailscale IP may duplicate core LAN endpoint
- Added isTailscaleIpv4Address check to isUsableLanIpv4Address so Tailscale CGNAT addresses (100.64.0.0/10) are excluded from LAN endpoint selection, preventing duplicate entries and ensuring the real LAN IP is used.
Or push these changes by commenting:
@cursor push 9d54008f10
Preview (9d54008f10)
diff --git a/apps/desktop/src/serverExposure.ts b/apps/desktop/src/serverExposure.ts
--- a/apps/desktop/src/serverExposure.ts
+++ b/apps/desktop/src/serverExposure.ts
@@ -8,6 +8,7 @@
AdvertisedEndpointProvider,
DesktopServerExposureMode,
} from "@t3tools/contracts";
+import { isTailscaleIpv4Address } from "./tailscaleEndpointProvider.ts";
const DESKTOP_LOOPBACK_HOST = "127.0.0.1";
const DESKTOP_LAN_BIND_HOST = "0.0.0.0";
@@ -47,7 +48,9 @@
};
const isUsableLanIpv4Address = (address: string): boolean =>
- !address.startsWith("127.") && !address.startsWith("169.254.");
+ !address.startsWith("127.") &&
+ !address.startsWith("169.254.") &&
+ !isTailscaleIpv4Address(address);
export function resolveLanAdvertisedHost(
networkInterfaces: NodeJS.Dict<NetworkInterfaceInfo[]>,You can send follow-ups to the cloud agent here.
ee85eb4 to
f04975d
Compare
ddb27a7 to
b585c3d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Network interfaces snapshot called twice, risking inconsistency
- Captured OS.networkInterfaces() once into a local variable and passed it to both resolveDesktopServerExposure and resolveTailscaleAdvertisedEndpoints, eliminating the redundant syscall and ensuring both consumers use the same network snapshot.
Or push these changes by commenting:
@cursor push 795c4f4466
Preview (795c4f4466)
diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts
--- a/apps/desktop/src/main.ts
+++ b/apps/desktop/src/main.ts
@@ -315,10 +315,11 @@
}
async function getDesktopAdvertisedEndpoints() {
+ const networkInterfaces = OS.networkInterfaces();
const exposure = resolveDesktopServerExposure({
mode: desktopServerExposureMode,
port: backendPort,
- networkInterfaces: OS.networkInterfaces(),
+ networkInterfaces,
...(backendAdvertisedHost ? { advertisedHostOverride: backendAdvertisedHost } : {}),
});
const coreEndpoints = resolveDesktopCoreAdvertisedEndpoints({
@@ -328,7 +329,7 @@
});
const tailscaleEndpoints = await resolveTailscaleAdvertisedEndpoints({
port: backendPort,
- networkInterfaces: OS.networkInterfaces(),
+ networkInterfaces,
});
return [...coreEndpoints, ...tailscaleEndpoints];
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit b585c3d. Configure here.
f04975d to
3009dd0
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
b585c3d to
f74ec04
Compare
- Discover SSH hosts and persist SSH targets - Bootstrap tunneled SSH sessions with desktop password prompts - Extend IPC and storage tests for SSH metadata
- Validate SSH targets and known-host parsing more strictly - Retry desktop SSH session refresh on auth failures - Preserve saved registry state when bearer persistence fails
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Resolve dev remote package specs to `t3@nightly` - Cover the dev fallback in sshEnvironment tests
- surface stdout when remote launch or pairing fails - report parse errors and invalid remote port or credential values
- Add a capped scroll area for discovered SSH hosts - Keep the manual SSH form always visible and simplify the dialog layout - Ensure the scroll area viewport respects inherited max height
- No functional change - Keep staged code style consistent
- Move SSH IPC handlers and password prompt state out of main.ts - Keep SSH environment launch and auth flow owned by sshEnvironment.ts
- Externalize askpass, remote launch, and runner helpers into script assets - Copy SSH scripts into `dist-electron` for packaging - Co-authored-by: codex <codex@users.noreply.github.com>
- Remove native password prompts from posix and Windows scripts - Fail loudly when T3_SSH_AUTH_SECRET is missing
- use type-only imports required by verbatim module syntax - fix SSH desktop build/typecheck regressions and auth test isolation - tighten browser test selectors for the add-environment dialog Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
e46a25f
into
t3code/advertised-endpoints
| async dispose(): Promise<void> { | ||
| const entries = [...this.tunnels.values()]; | ||
| this.tunnels.clear(); | ||
| this.pendingTunnelEntries.clear(); | ||
| await Promise.all(entries.map((entry) => stopTunnel(entry).catch(() => undefined))); | ||
| } |
There was a problem hiding this comment.
🟢 Low src/sshEnvironment.ts:1041
dispose() clears pendingTunnelEntries without awaiting or cancelling the in-flight promises, so a tunnel creation already in progress continues executing. When it completes, it calls this.tunnels.set(key, tunnelEntry) (line 1022), adding an SSH child process to the already-cleared tunnels map. This orphaned process is never stopped because dispose() already finished its stopTunnel calls, resulting in a leaked SSH process.
async dispose(): Promise<void> {
+ const pending = [...this.pendingTunnelEntries.values()];
+ await Promise.allSettled(pending);
const entries = [...this.tunnels.values()];
this.tunnels.clear();
this.pendingTunnelEntries.clear();
await Promise.all(entries.map((entry) => stopTunnel(entry).catch(() => undefined)));
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/src/sshEnvironment.ts around lines 1041-1046:
`dispose()` clears `pendingTunnelEntries` without awaiting or cancelling the in-flight promises, so a tunnel creation already in progress continues executing. When it completes, it calls `this.tunnels.set(key, tunnelEntry)` (line 1022), adding an SSH child process to the already-cleared `tunnels` map. This orphaned process is never stopped because `dispose()` already finished its `stopTunnel` calls, resulting in a leaked SSH process.
Evidence trail:
apps/desktop/src/sshEnvironment.ts lines 1039-1043 (dispose method), line 1022 (tunnels.set), lines 958-973 (ChildProcess.spawn), line 1032 (pendingTunnelEntries.set), line 948 (IIFE async function starts immediately)
| this.tunnels.set(key, tunnelEntry); | ||
| try { | ||
| await tunnelReady; | ||
| return tunnelEntry; | ||
| } catch (error) { | ||
| await stopTunnel(tunnelEntry).catch(() => undefined); | ||
| this.deleteTunnelIfCurrent(tunnelEntry); | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
🟡 Medium src/sshEnvironment.ts:1022
In ensureTunnelEntry, the tunnel entry is added to this.tunnels at line 1022 before tunnelReady is awaited. A concurrent call with the same key can find this entry at line 931, fail its 2-second health check at line 935 (since the tunnel is still starting with a 20-second timeout), then kill the tunnel at line 938—destroying the tunnel the first call is still establishing. The pending-check at line 944 happens too late, after the tunnel is already killed. Consider moving the this.tunnels.set(key, tunnelEntry) call after the await tunnelReady succeeds, so the entry is only visible to other callers once it's actually ready.
- this.tunnels.set(key, tunnelEntry);
try {
await tunnelReady;
+ this.tunnels.set(key, tunnelEntry);
return tunnelEntry;
} catch (error) {
await stopTunnel(tunnelEntry).catch(() => undefined);
- this.deleteTunnelIfCurrent(tunnelEntry);
throw error;
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/src/sshEnvironment.ts around lines 1022-1030:
In `ensureTunnelEntry`, the tunnel entry is added to `this.tunnels` at line 1022 before `tunnelReady` is awaited. A concurrent call with the same key can find this entry at line 931, fail its 2-second health check at line 935 (since the tunnel is still starting with a 20-second timeout), then kill the tunnel at line 938—destroying the tunnel the first call is still establishing. The pending-check at line 944 happens too late, after the tunnel is already killed. Consider moving the `this.tunnels.set(key, tunnelEntry)` call after the `await tunnelReady` succeeds, so the entry is only visible to other callers once it's actually ready.
Evidence trail:
apps/desktop/src/sshEnvironment.ts lines 931-939 (entry lookup, 2s health check, stopTunnel); line 944-946 (pendingTunnelEntries check after tunnel already killed); lines 1018-1024 (20s timeout waitForHttpReady, this.tunnels.set before await tunnelReady); line 26 (SSH_READY_TIMEOUT_MS = 20_000)
| const removed = await removeConnection(activeRecord.environmentId).catch(() => false); | ||
| if (!removed) { | ||
| await connection.dispose().catch(() => undefined); | ||
| } |
There was a problem hiding this comment.
🟢 Low runtime/service.ts:1261
In the auth error recovery path, when the recursive call to ensureSavedEnvironmentConnection at line 1251 fails, the outer catch block at lines 1259-1265 disposes connection again even though it was already disposed at line 1249. The local connection variable still holds the original disposed connection, so line 1264 executes connection.dispose() a second time. While the .catch(() => undefined) suppresses the error, this double-dispose is incorrect cleanup logic that could cause issues if dispose() has non-idempotent side effects.
const removed = await removeConnection(activeRecord.environmentId).catch(() => false);
if (!removed) {
- await connection.dispose().catch(() => undefined);
+ // Already disposed at line 1249, no need to dispose again
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/environments/runtime/service.ts around lines 1261-1264:
In the auth error recovery path, when the recursive call to `ensureSavedEnvironmentConnection` at line 1251 fails, the outer catch block at lines 1259-1265 disposes `connection` again even though it was already disposed at line 1249. The local `connection` variable still holds the original disposed connection, so line 1264 executes `connection.dispose()` a second time. While the `.catch(() => undefined)` suppresses the error, this double-dispose is incorrect cleanup logic that could cause issues if `dispose()` has non-idempotent side effects.
Evidence trail:
apps/web/src/environments/runtime/service.ts lines 1249-1264 (line 1249: `await connection.dispose().catch(() => undefined)` in auth recovery, line 1251: recursive call that can throw, lines 1259-1265: catch block, line 1264: `await connection.dispose().catch(() => undefined)` called again). apps/web/src/environments/runtime/connection.ts lines 84, 141-149, 167-170 (line 84: `let disposed = false;` local variable, lines 141-149: `cleanup` function that sets disposed=true, lines 167-170: `dispose` method that calls `cleanup()` and `client.dispose()` without checking if already disposed). apps/web/src/rpc/wsTransport.ts lines 203-208 (WsTransport.dispose() is idempotent with early return, but this doesn't prevent the logic error at the connection level).


Summary
Validation
Stacked on #2362.
Co-authored-by: codex codex@users.noreply.github.com
Note
High Risk
High risk: introduces new SSH-based remote launch/tunneling with password prompting and token bootstrapping, plus expands persistence/IPC and connection lifecycle logic where mistakes could break remote connectivity or mishandle credentials.
Overview
Adds two new remote-connection capabilities: a Tailscale advertised-endpoint add-on (Tailnet IP + MagicDNS-derived HTTPS candidate) and a desktop-managed SSH launch/tunnel flow that can start/reuse a remote
t3server, forward it to loopback, and pair/save it like other environments.Extends the saved-environment record format with optional
desktopSshmetadata and updates desktop + web persistence to round-trip it. The desktop main process now exposes new SSH IPC APIs (host discovery, ensure tunnel, session/bootstrap helpers, password prompt/resolve) and merges Tailscale endpoints intogetAdvertisedEndpoints.Updates the web Connections settings UI to add an SSH add-environment mode (discovered hosts + manual entry) and adds an in-app SSH password prompt dialog. Refactors environment connection service to support SSH-forwarded session/token flows, including retry/recovery on
ssh_http:401, deduping pending connections, and safer rollback when credential persistence fails.Reviewed by Cursor Bugbot for commit c5c99a7. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Tailscale endpoint discovery and SSH environment add-on to the desktop app
resolveTailscaleAdvertisedEndpointsin tailscaleEndpointProvider.ts to discover Tailnet IPv4 and MagicDNS HTTPS endpoints from local network interfaces andtailscale status --jsonoutput, merging them into the advertised endpoints list.DesktopSshEnvironmentBridge) for renderer-to-main SSH operations.connectDesktopSshEnvironmentin service.ts to bootstrap, persist, and reconnect SSH-backed saved environments, including 401 recovery via bearer re-issuance and registry rollback on failure.~/.ssh/configandknown_hosts, manual host entry with IPv6 support, and Connect/Reconnect actions.SshPasswordPromptDialogin SshPasswordPromptDialog.tsx to collect SSH passwords in-app and resolve them back to the main process.requires-configurationwithunknownreachability by default.Macroscope summarized c5c99a7.