From 47039fcb45d0751655b5d8e13c8ae27ed7381a7d Mon Sep 17 00:00:00 2001 From: Lance Wang Date: Tue, 22 Sep 2026 12:59:01 +0800 Subject: [PATCH 1/2] Support concurrent Tailscale shares --- README.md | 6 +- skills/remote-installer/SKILL.md | 59 ++-- src/background.rs | 11 +- src/exposure.rs | 449 ++++++++++++++++++++++++------- src/main.rs | 218 ++++++++------- 5 files changed, 525 insertions(+), 218 deletions(-) diff --git a/README.md b/README.md index df4287e..6f3d6e4 100644 --- a/README.md +++ b/README.md @@ -161,13 +161,13 @@ same lifecycle ownership. | `--provider auto` (default) | Detect and start every installed provider | | `--provider tailscale-serve` | Keep the link private to your tailnet | | `--provider tailscale-funnel` | Create a public Tailscale link | -| `--https-port PORT` | Tailscale Serve port; auto mode picks another supported Funnel port; `--funnel-port` is a compatibility alias | +| `--https-port PORT` | Require an exact Tailscale HTTPS port; when omitted, select an available port; `--funnel-port` is a compatibility alias | Use either `--expire-after` or `--timeout`, not both. Run `remote-installer share --help` for every option. -With the default `--provider auto`, Remote Installer checks for the Tailscale and cloudflared CLIs, starts every provider it can use in parallel, and prints a warning for each unavailable or unready provider. Tailscale Serve and Funnel use different HTTPS ports automatically so both can run in the same share. If you select one provider explicitly, only that provider is started. The terminal labels every result as `Public internet` or `Tailnet only` so the access boundary is visible next to the URL. +With the default `--provider auto`, Remote Installer checks for the Tailscale and cloudflared CLIs, starts every provider it can use, and prints a warning for each unavailable or unready provider. Tailscale Serve and Funnel use different HTTPS ports automatically so both can run in the same share. Tailscale configuration writes are serialized while Cloudflare starts independently. If you select one provider explicitly, only that provider is started. The terminal labels every result as `Public internet` or `Tailnet only` so the access boundary is visible next to the URL. -Remote Installer does not overwrite an existing Tailscale Serve or Funnel configuration. Auto mode warns and skips Tailscale while another available provider can continue; explicitly selecting the conflicting Tailscale mode returns an error. +Remote Installer does not overwrite existing Tailscale Serve or Funnel routes. When `--https-port` is omitted, concurrent shares reserve different available ports on the node. An explicitly requested occupied port returns an error instead of replacing its route. Stopping a share closes only the foreground sessions owned by that process; it never runs a global `serve reset`. Tailscale Serve requires the phone to be on the same tailnet (or otherwise allowed by its access policy). Tailscale Funnel creates a public link and does not require Tailscale on the phone. The older `--provider tailscale` spelling is kept as an alias for `tailscale-funnel`; use the explicit provider name in new commands. diff --git a/skills/remote-installer/SKILL.md b/skills/remote-installer/SKILL.md index 57a64d1..05e912e 100644 --- a/skills/remote-installer/SKILL.md +++ b/skills/remote-installer/SKILL.md @@ -3,7 +3,8 @@ name: remote-installer description: >- Put an iOS or Android build on a real phone or tablet over the air with the `remote-installer` CLI — it validates the build, opens a temporary HTTPS - tunnel, and prints an install URL plus a QR code to scan. Use this whenever + tunnel, and prints one or more install URLs plus QR codes to scan. Use this + whenever someone wants a build onto a physical device without TestFlight or a cable: "get this on my phone", "send this build to a tester", "share the IPA or APK", "install this on my iPad", "let QA try this build", "make a link for this @@ -16,10 +17,11 @@ description: >- # Sharing a mobile build over the air -`remote-installer share ` validates an iOS or Android build, stands up a -temporary HTTPS tunnel in front of a loopback server, and prints an install -page URL plus a QR code. iOS uses `itms-services://`; Android downloads a signed -standalone APK for the system installer. Stopping the process kills the link. +`remote-installer share ` validates an iOS or Android build, stands up +temporary HTTPS tunnels in front of a loopback server, and prints an install +page URL plus a QR code for every provider that becomes ready. iOS uses +`itms-services://`; Android downloads a signed standalone APK for the system +installer. Stopping the process kills the links. The published CLI is currently macOS only. iOS `.app` handling shells out to Apple system tools. APK handling uses Android SDK `apkanalyzer` and `apksigner` @@ -102,14 +104,16 @@ a source checkout or binary path. `cloudflared` (`brew install cloudflared`) and Tailscale (`brew install --cask tailscale`), starts every provider that is available, and warns about the rest. No Cloudflare account is needed for the Quick Tunnel. Select one -provider explicitly when you need only that route. Auto mode may print several -working links; they are alternate origins for one staged artifact, one download -quota, and one lifecycle rather than separate copies. +provider explicitly only when the user requests it or an access requirement +calls for one route. Otherwise keep the default auto mode. Auto mode may print +several working links; they are alternate origins for one staged artifact, one +download quota, and one lifecycle rather than separate copies. -Remote Installer refuses to replace an existing Tailscale Serve or Funnel -configuration. Auto mode warns and skips Tailscale while another available -provider can continue; an explicitly selected Tailscale provider reports the -conflict and stops. Do not reset the user's existing configuration to force it. +Remote Installer preserves existing Tailscale Serve and Funnel routes. When +`--https-port` is omitted, concurrent shares reserve different available ports +on the node. An explicitly requested occupied port reports the conflict instead +of replacing its route. Do not reset the user's existing configuration to +force it. For APKs, ensure Android SDK Command-Line Tools and Build Tools are installed. If automatic discovery fails, pass `--apkanalyzer-bin` and `--apksigner-bin`. @@ -132,8 +136,9 @@ remote-installer share /path/to/MyApp.ipa \ ``` This also works through `npx --yes @icodesign/remote-installer`. The returned -JSON contains the share ID and ready install URLs. Do not send a URL before the -command reports a ready session. +JSON contains the share ID and a `links` array of ready provider results. Do not +send any URL before the command reports a ready session, and do not collapse +that array to its first entry. **Set `--timeout` on essentially every run.** It takes plain seconds and shuts the whole thing down when it elapses — tunnel closed, temporary copy deleted. @@ -171,8 +176,8 @@ writing a command a human will read. Passing both is an error, so pick one. Other flags worth knowing: `--provider tailscale-serve` (private to the tailnet), `--provider tailscale-funnel` (public through Tailscale), and `--provider tailscale` (the compatibility alias for Funnel), `--https-port` -(the Serve port in auto mode; `--funnel-port` is its visible compatibility -alias), `--no-qr`, +(require an exact Tailscale port instead of automatic allocation; +`--funnel-port` is its visible compatibility alias), `--no-qr`, `--cloudflared-bin`, and `--tailscale-bin`. ## Reading the output @@ -201,8 +206,22 @@ provider is reported as a warning while the other links remain usable. Read the For Android, `Requires` contains an API level and `Install link` is the granted HTTPS `download.apk` URL. Give the user the install page in either case. -Give the user the **Install page** URL. That's the one to open on the phone and -the one to paste into a message. +Return **every ready Install page URL** to the user, not just the first or a +preferred provider. Label each URL with its provider and access scope (`Public +internet` or `Tailnet only`) so the user can choose which route to open. A +provider warning is not a reason to omit the other successful links. If only +one provider becomes ready, return that one and briefly mention that it was the +only available route. + +The **Install page** URLs are the ones to open on the phone and paste into a +message. Do not substitute the native `Install link` values. A concise reply +with multiple results can look like: + +```text +- Cloudflare Quick Tunnel (Public internet): https://.../install/... +- Tailscale Serve (Tailnet only): https://.../install/... +- Tailscale Funnel (Public internet): https://.../install/... +``` The QR code is terminal art printed below that banner. Don't try to reproduce it in your reply — say it's in their terminal and to scan it with the phone @@ -223,8 +242,8 @@ Download complete: MyApp.ipa (214.6 MB in 38s) Download interrupted: MyApp.ipa at 62% (133.1 MB / 214.6 MB) ``` -If the user asks whether it worked, inspect the managed session rather than -guessing: +If the user asks whether it worked or asks for the links again, inspect the +managed session rather than guessing, and return every ready provider link: ```bash remote-installer status diff --git a/src/background.rs b/src/background.rs index 462b33a..5f64e92 100644 --- a/src/background.rs +++ b/src/background.rs @@ -308,8 +308,6 @@ fn worker_arguments(args: &ShareArgs, executable: String) -> Vec { .to_string(), "--provider".to_owned(), args.provider.cli_name().to_owned(), - "--https-port".to_owned(), - args.https_port.to_string(), "--listen".to_owned(), args.listen.to_string(), "--no-qr".to_owned(), @@ -320,6 +318,9 @@ fn worker_arguments(args: &ShareArgs, executable: String) -> Vec { .display() .to_string(), ]; + if let Some(port) = args.https_port { + result.extend(["--https-port".to_owned(), port.to_string()]); + } if let Some(maximum) = args.max_downloads { result.extend(["--max-downloads".to_owned(), maximum.to_string()]); } @@ -716,6 +717,12 @@ mod tests { assert!(joined.contains("--max-downloads 1"), "{joined}"); assert!(joined.contains("--provider tailscale-serve"), "{joined}"); assert!(joined.contains("--no-qr"), "{joined}"); + assert!(!joined.contains("--https-port"), "{joined}"); + + let mut explicit = background_args(temporary.path()); + explicit.https_port = Some(10001); + let joined = worker_arguments(&explicit, "/native/remote-installer".to_owned()).join(" "); + assert!(joined.contains("--https-port 10001"), "{joined}"); } #[test] diff --git a/src/exposure.rs b/src/exposure.rs index 77fd431..d35816b 100644 --- a/src/exposure.rs +++ b/src/exposure.rs @@ -1,4 +1,5 @@ -use std::collections::VecDeque; +use std::collections::{BTreeSet, VecDeque}; +use std::fs::{File, OpenOptions, TryLockError}; use std::net::{IpAddr, Ipv4Addr, SocketAddr, TcpListener}; use std::path::{Path, PathBuf}; use std::process::{Output, Stdio}; @@ -18,6 +19,9 @@ use url::Url; const TAILSCALE_STARTUP_TIMEOUT: Duration = Duration::from_secs(120); const TAILSCALE_POLL_INTERVAL: Duration = Duration::from_millis(500); const TAILSCALE_DNS_STATUS_TIMEOUT: Duration = Duration::from_secs(5); +const TAILSCALE_COORDINATION_TIMEOUT: Duration = Duration::from_secs(300); +const TAILSCALE_COORDINATION_POLL_INTERVAL: Duration = Duration::from_millis(250); +const TAILSCALE_COORDINATION_PROGRESS_INTERVAL: Duration = Duration::from_secs(5); const TAILSCALE_APP_CLI: &str = "/Applications/Tailscale.app/Contents/MacOS/Tailscale"; const CLOUDFLARED_HOMEBREW_CLI: &str = "/opt/homebrew/bin/cloudflared"; @@ -107,12 +111,26 @@ pub enum ExposureError { }, #[error("tunnel process I/O failed: {0}")] Io(#[from] std::io::Error), - #[error("Tailscale returned invalid JSON: {0}")] - Json(#[from] serde_json::Error), + #[error( + "Tailscale command `{command}` returned invalid JSON: {source}; stdout: {stdout}; stderr: {stderr}" + )] + Json { + command: String, + stdout: String, + stderr: String, + #[source] + source: serde_json::Error, + }, #[error("Tailscale is not ready: {0}")] TailscaleNotReady(String), - #[error("an existing Tailscale {mode} configuration is active; refusing to replace it")] - ExistingTailscaleConfiguration { mode: &'static str }, + #[error( + "Tailscale HTTPS port {port} is already in use; omit --https-port to select another port automatically" + )] + TailscalePortInUse { port: u16 }, + #[error("no available Tailscale {mode} HTTPS port was found")] + NoTailscalePortAvailable { mode: &'static str }, + #[error("could not coordinate Tailscale startup: {0}")] + TailscaleCoordination(String), #[error("Tailscale {mode} requires a valid HTTPS port, got {port}")] InvalidTailscalePort { mode: &'static str, port: u16 }, #[error("{program} failed: {message}")] @@ -143,6 +161,18 @@ pub struct ExposureSession { inner: ExposureSessionInner, } +/// Serializes Tailscale configuration changes made by remote-installer +/// processes on this Mac and reserves non-overlapping node HTTPS ports. +/// +/// Tailscale owns one per-node ServeConfig that can contain many foreground +/// sessions. Holding this coordinator until the planned Tailscale children are +/// ready prevents two remote-installer processes from selecting the same port +/// from the same configuration snapshot. +pub struct TailscaleStartupCoordinator { + _lock: File, + occupied_ports: BTreeSet, +} + enum ExposureSessionInner { Tailscale(TailscaleSession), Cloudflare(CloudflareSession), @@ -215,10 +245,8 @@ impl ExposureSession { .await } - /// Start a provider after the caller has performed one shared Tailscale - /// preflight. Auto mode uses this for its parallel Serve/Funnel starts so - /// the second child does not mistake the first child that this same command - /// just created for a pre-existing user configuration. + /// Start a provider after the caller has acquired the shared Tailscale + /// startup coordinator and reserved a non-overlapping HTTPS port. pub async fn start_without_configuration_check( provider: ExposureProvider, target: &Url, @@ -265,39 +293,6 @@ impl ExposureSession { } } - /// Check that the Tailscale CLI can serve this process and that no - /// existing Serve/Funnel configuration would be overwritten by auto mode. - pub async fn check_tailscale_for_auto( - binary_override: Option<&Path>, - ) -> Result<(), ExposureError> { - let binary = discover_binary( - "Tailscale", - "tailscale", - binary_override, - &[TAILSCALE_APP_CLI], - TAILSCALE_INSTALL_HINT, - )?; - let status: TailscaleStatus = command_json(&binary, &["status", "--json"]).await?; - if status.backend_state != "Running" { - return Err(ExposureError::TailscaleNotReady(format!( - "backend state is {}", - status.backend_state - ))); - } - status - .self_node - .and_then(|node| node.dns_name) - .filter(|name| !name.trim_matches('.').is_empty()) - .ok_or_else(|| { - ExposureError::TailscaleNotReady("the current node has no MagicDNS name".into()) - })?; - // Check both modes before either child is spawned. Serve and Funnel - // keep separate foreground entries, so checking only Serve could let - // auto mode overwrite a user's existing Funnel configuration. - ensure_empty_tailscale_configuration(&binary, TailscaleMode::Serve).await?; - ensure_empty_tailscale_configuration(&binary, TailscaleMode::Funnel).await - } - pub fn provider(&self) -> ExposureProvider { self.provider } @@ -325,6 +320,86 @@ impl ExposureSession { } } +impl TailscaleStartupCoordinator { + /// Acquire the per-user startup lock and snapshot the node ports already + /// owned by foreground or background Serve/Funnel configurations. + pub async fn acquire(binary_override: Option<&Path>) -> Result { + let lock = acquire_tailscale_startup_lock().await?; + let binary = discover_binary( + "Tailscale", + "tailscale", + binary_override, + &[TAILSCALE_APP_CLI], + TAILSCALE_INSTALL_HINT, + )?; + tailscale_dns_name(&binary).await?; + let occupied_ports = tailscale_ports_in_use(&binary).await?; + Ok(Self { + _lock: lock, + occupied_ports, + }) + } + + /// Reserve a Serve port. An explicit request is strict; an omitted port + /// prefers the conventional ports, then moves into a stable high range. + pub fn reserve_serve_port(&mut self, requested: Option) -> Result { + self.reserve_port(TailscaleMode::Serve, requested, &[443, 8443, 10000]) + } + + /// Reserve a Funnel port. Funnel only supports its three public ports. + pub fn reserve_funnel_port(&mut self, requested: Option) -> Result { + self.reserve_port(TailscaleMode::Funnel, requested, &[443, 8443, 10000]) + } + + /// Auto mode prefers 8443 for Funnel so the ordinary Serve URL can retain + /// 443. The explicitly requested Serve port is excluded from consideration. + pub fn reserve_auto_funnel_port( + &mut self, + requested_serve_port: Option, + ) -> Result { + let candidates = [8443, 10000, 443]; + let port = candidates + .into_iter() + .find(|port| Some(*port) != requested_serve_port && !self.occupied_ports.contains(port)) + .ok_or(ExposureError::NoTailscalePortAvailable { + mode: TailscaleMode::Funnel.display_name(), + })?; + self.occupied_ports.insert(port); + Ok(port) + } + + fn reserve_port( + &mut self, + mode: TailscaleMode, + requested: Option, + preferred: &[u16], + ) -> Result { + if let Some(port) = requested { + validate_tailscale_port(mode, port)?; + if !self.occupied_ports.insert(port) { + return Err(ExposureError::TailscalePortInUse { port }); + } + return Ok(port); + } + + let candidate = preferred + .iter() + .copied() + .find(|port| !self.occupied_ports.contains(port)) + .or_else(|| { + (mode == TailscaleMode::Serve) + .then(|| (10001..=u16::MAX).find(|port| !self.occupied_ports.contains(port))) + .flatten() + }) + .ok_or(ExposureError::NoTailscalePortAvailable { + mode: mode.display_name(), + })?; + validate_tailscale_port(mode, candidate)?; + self.occupied_ports.insert(candidate); + Ok(candidate) + } +} + impl TailscaleSession { async fn wait_for_exit(&mut self) -> Result<(), ExposureError> { let status = self.child.wait().await?; @@ -438,22 +513,9 @@ async fn start_tailscale( &[TAILSCALE_APP_CLI], TAILSCALE_INSTALL_HINT, )?; - let status: TailscaleStatus = command_json(&binary, &["status", "--json"]).await?; - if status.backend_state != "Running" { - return Err(ExposureError::TailscaleNotReady(format!( - "backend state is {}", - status.backend_state - ))); - } - let dns_name = status - .self_node - .and_then(|node| node.dns_name) - .filter(|name| !name.trim_matches('.').is_empty()) - .ok_or_else(|| { - ExposureError::TailscaleNotReady("the current node has no MagicDNS name".into()) - })?; + let dns_name = tailscale_dns_name(&binary).await?; if check_existing_tailscale_configuration { - ensure_empty_tailscale_configuration(&binary, mode).await?; + ensure_tailscale_port_available(&binary, https_port).await?; } let warnings = tailscale_dns_diagnostics(&binary, mode).await?; @@ -671,18 +733,106 @@ async fn start_cloudflare_once( ))) } -async fn ensure_empty_tailscale_configuration( - binary: &Path, - mode: TailscaleMode, -) -> Result<(), ExposureError> { - let current: Value = command_json(binary, &[mode.command(), "status", "--json"]).await?; - if empty_configuration(¤t) { - Ok(()) - } else { - Err(ExposureError::ExistingTailscaleConfiguration { - mode: mode.display_name(), +async fn acquire_tailscale_startup_lock() -> Result { + let home = std::env::var_os("HOME") + .ok_or_else(|| ExposureError::TailscaleCoordination("HOME is not set".to_owned()))?; + let directory = PathBuf::from(home) + .join("Library") + .join("Caches") + .join("remote-installer"); + let file = tokio::task::spawn_blocking(move || { + std::fs::create_dir_all(&directory)?; + OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(directory.join("tailscale-startup.lock")) + }) + .await + .map_err(|error| ExposureError::TailscaleCoordination(error.to_string()))? + .map_err(ExposureError::Io)?; + + let started_at = Instant::now(); + let deadline = started_at + TAILSCALE_COORDINATION_TIMEOUT; + let mut next_progress = started_at + TAILSCALE_COORDINATION_PROGRESS_INTERVAL; + loop { + match file.try_lock() { + Ok(()) => return Ok(file), + Err(TryLockError::WouldBlock) => { + let now = Instant::now(); + if now >= deadline { + return Err(ExposureError::TailscaleCoordination( + "timed out waiting for another remote-installer process to finish configuring Tailscale" + .to_owned(), + )); + } + if now >= next_progress { + eprintln!( + "Still waiting for another remote-installer process to finish configuring Tailscale... ({}s elapsed)", + started_at.elapsed().as_secs() + ); + next_progress += TAILSCALE_COORDINATION_PROGRESS_INTERVAL; + } + sleep(TAILSCALE_COORDINATION_POLL_INTERVAL).await; + } + Err(TryLockError::Error(error)) => return Err(ExposureError::Io(error)), + } + } +} + +async fn tailscale_dns_name(binary: &Path) -> Result { + let status: TailscaleStatus = command_json(binary, &["status", "--json"]).await?; + if status.backend_state != "Running" { + return Err(ExposureError::TailscaleNotReady(format!( + "backend state is {}", + status.backend_state + ))); + } + status + .self_node + .and_then(|node| node.dns_name) + .filter(|name| !name.trim_matches('.').is_empty()) + .ok_or_else(|| { + ExposureError::TailscaleNotReady("the current node has no MagicDNS name".into()) }) +} + +async fn ensure_tailscale_port_available(binary: &Path, port: u16) -> Result<(), ExposureError> { + if tailscale_ports_in_use(binary).await?.contains(&port) { + Err(ExposureError::TailscalePortInUse { port }) + } else { + Ok(()) + } +} + +async fn tailscale_ports_in_use(binary: &Path) -> Result, ExposureError> { + let serve: Value = command_json(binary, &["serve", "status", "--json"]).await?; + let funnel: Value = command_json(binary, &["funnel", "status", "--json"]).await?; + let mut ports = node_tailscale_ports(&serve); + ports.extend(node_tailscale_ports(&funnel)); + Ok(ports) +} + +/// Collect only node-level and foreground ports. Named Tailscale Services use +/// distinct virtual IPs, so their endpoint numbers do not collide with this +/// node's Serve/Funnel listeners. +fn node_tailscale_ports(configuration: &Value) -> BTreeSet { + fn collect_tcp_ports(configuration: &Value, ports: &mut BTreeSet) { + let Some(tcp) = configuration.get("TCP").and_then(Value::as_object) else { + return; + }; + ports.extend(tcp.keys().filter_map(|port| port.parse::().ok())); } + + let mut ports = BTreeSet::new(); + collect_tcp_ports(configuration, &mut ports); + if let Some(sessions) = configuration.get("Foreground").and_then(Value::as_object) { + for session in sessions.values() { + collect_tcp_ports(session, &mut ports); + } + } + ports } async fn tailscale_dns_diagnostics( @@ -801,7 +951,29 @@ async fn command_json( arguments: &[&str], ) -> Result { let output = run_checked("Tailscale", binary, arguments).await?; - Ok(serde_json::from_slice(&output.stdout)?) + serde_json::from_slice(&output.stdout).map_err(|source| ExposureError::Json { + command: arguments.join(" "), + stdout: command_output_summary(&output.stdout), + stderr: command_output_summary(&output.stderr), + source, + }) +} + +fn command_output_summary(output: &[u8]) -> String { + const MAX_CHARS: usize = 512; + + if output.is_empty() { + return "".into(); + } + + let text = String::from_utf8_lossy(output); + let mut chars = text.chars(); + let summary = chars.by_ref().take(MAX_CHARS).collect::(); + if chars.next().is_some() { + format!("{summary:?} (truncated, {} bytes total)", output.len()) + } else { + format!("{summary:?}") + } } async fn run_checked( @@ -830,10 +1002,6 @@ async fn run_checked( }) } -fn empty_configuration(value: &Value) -> bool { - matches!(value, Value::Null) || value.as_object().is_some_and(serde_json::Map::is_empty) -} - fn tailscale_public_url(dns_name: &str, https_port: u16) -> Result { let host = dns_name.trim_end_matches('.'); let value = if https_port == 443 { @@ -913,10 +1081,41 @@ mod tests { use super::*; #[test] - fn accepts_only_an_empty_tailscale_configuration() { - assert!(empty_configuration(&serde_json::json!({}))); - assert!(empty_configuration(&Value::Null)); - assert!(!empty_configuration(&serde_json::json!({"TCP":{"443":{}}}))); + fn finds_node_and_foreground_ports_without_claiming_service_vips() { + let ports = node_tailscale_ports(&serde_json::json!({ + "TCP": {"443": {"HTTPS": true}}, + "Foreground": { + "first": {"TCP": {"8443": {"HTTPS": true}}}, + "second": {"TCP": {"10000": {"HTTPS": true}}} + }, + "Services": { + "svc:other": {"TCP": {"10001": {"HTTPS": true}}} + } + })); + assert_eq!(ports, BTreeSet::from([443, 8443, 10000])); + } + + #[test] + fn allocates_non_overlapping_ports_for_a_second_auto_share() { + let mut coordinator = TailscaleStartupCoordinator { + _lock: tempfile::tempfile().unwrap(), + occupied_ports: BTreeSet::from([443, 8443]), + }; + assert_eq!(coordinator.reserve_auto_funnel_port(None).unwrap(), 10000); + assert_eq!(coordinator.reserve_serve_port(None).unwrap(), 10001); + } + + #[test] + fn an_explicit_occupied_port_is_not_silently_replaced() { + let mut coordinator = TailscaleStartupCoordinator { + _lock: tempfile::tempfile().unwrap(), + occupied_ports: BTreeSet::from([443]), + }; + let error = coordinator.reserve_serve_port(Some(443)).unwrap_err(); + assert!(matches!( + error, + ExposureError::TailscalePortInUse { port: 443 } + )); } #[test] @@ -984,6 +1183,39 @@ mod tests { ); } + #[cfg(unix)] + #[tokio::test] + async fn invalid_tailscale_json_reports_the_command_and_bounded_output() { + use std::os::unix::fs::PermissionsExt; + + let temporary = tempfile::tempdir().unwrap(); + let binary = temporary.path().join("tailscale"); + let long_output = "x".repeat(600); + std::fs::write( + &binary, + format!("#!/bin/sh\nprintf '%s' '{long_output}'\nprintf '%s' 'diagnostic' >&2\n"), + ) + .unwrap(); + let mut permissions = std::fs::metadata(&binary).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&binary, permissions).unwrap(); + + let error = command_json::(&binary, &["serve", "status", "--json"]) + .await + .unwrap_err() + .to_string(); + assert!(error.contains("`serve status --json`")); + assert!(error.contains("expected value at line 1 column 1")); + assert!(error.contains("diagnostic")); + assert!(error.contains("truncated, 600 bytes total")); + assert!(!error.contains(&long_output)); + } + + #[test] + fn empty_command_output_has_an_explicit_summary() { + assert_eq!(command_output_summary(b""), ""); + } + #[test] fn validates_tailscale_port_rules_by_mode() { assert!(validate_tailscale_port(TailscaleMode::Serve, 1).is_ok()); @@ -1028,7 +1260,7 @@ mod tests { #[cfg(unix)] #[tokio::test] - async fn auto_preflight_checks_both_tailscale_modes_before_starting() { + async fn port_snapshot_checks_both_tailscale_modes() { let temporary = tempfile::tempdir().unwrap(); let binary = temporary.path().join("tailscale"); let log = temporary.path().join("commands.log"); @@ -1036,14 +1268,14 @@ mod tests { &binary, &log, r#"{"BackendState":"Running","Self":{"DNSName":"mac.example.ts.net."}}"#, - "{}", + r#"{"Foreground":{"existing":{"TCP":{"443":{"HTTPS":true}}}}}"#, ); - ExposureSession::check_tailscale_for_auto(Some(&binary)) - .await - .unwrap(); + assert_eq!( + tailscale_ports_in_use(&binary).await.unwrap(), + BTreeSet::from([443]) + ); let commands = std::fs::read_to_string(log).unwrap(); - assert!(commands.contains("status --json")); assert!(commands.contains("serve status --json")); assert!(commands.contains("funnel status --json")); } @@ -1119,7 +1351,8 @@ mod tests { assert!(!commands.contains(" off")); assert!(!commands.contains("--bg")); assert_fake_child_exited(&binary); - assert!(!commands.contains("funnel")); + assert!(commands.contains("funnel status --json")); + assert!(!commands.contains("funnel --yes")); assert!(!commands.contains("reset")); } @@ -1328,10 +1561,10 @@ mod tests { #[cfg(unix)] #[tokio::test] - async fn refuses_existing_configuration_without_starting_or_resetting_it() { - for (provider, mode) in [ - (ExposureProvider::TailscaleServe, "Serve"), - (ExposureProvider::TailscaleFunnel, "Funnel"), + async fn refuses_an_occupied_port_without_starting_or_resetting_it() { + for provider in [ + ExposureProvider::TailscaleServe, + ExposureProvider::TailscaleFunnel, ] { let temporary = tempfile::tempdir().unwrap(); let binary = temporary.path().join("tailscale"); @@ -1346,20 +1579,52 @@ mod tests { let error = ExposureSession::start(provider, &target, Some(&binary), None, 443) .await .err() - .expect("existing configuration should be refused"); + .expect("occupied port should be refused"); assert_eq!( error.to_string(), - format!( - "an existing Tailscale {mode} configuration is active; refusing to replace it" - ) + "Tailscale HTTPS port 443 is already in use; omit --https-port to select another port automatically" ); let commands = std::fs::read_to_string(log).unwrap(); - assert!(commands.contains(&format!("{} status --json", mode.to_lowercase()))); + assert!(commands.contains("serve status --json")); + assert!(commands.contains("funnel status --json")); assert!(!commands.contains(" --yes ")); assert!(!commands.contains("reset")); } } + #[cfg(unix)] + #[tokio::test] + async fn allows_a_second_serve_session_on_an_unused_port() { + let temporary = tempfile::tempdir().unwrap(); + let binary = temporary.path().join("tailscale"); + let log = temporary.path().join("commands.log"); + write_fake_tailscale( + &binary, + &log, + r#"{"BackendState":"Running","Self":{"DNSName":"mac.example.ts.net."}}"#, + r#"{"Foreground":{"existing":{"TCP":{"443":{"HTTPS":true}}}}}"#, + ); + let target = Url::parse("http://127.0.0.1:49152").unwrap(); + let mut session = ExposureSession::start( + ExposureProvider::TailscaleServe, + &target, + Some(&binary), + None, + 10001, + ) + .await + .unwrap(); + assert_eq!( + session.public_base_url().as_str(), + "https://mac.example.ts.net:10001/" + ); + session.stop().await.unwrap(); + + let commands = std::fs::read_to_string(log).unwrap(); + assert!(commands.contains("serve --yes --https=10001")); + assert!(!commands.contains("reset")); + } + #[cfg(unix)] #[tokio::test] async fn reports_a_stopped_tailscale_backend() { diff --git a/src/main.rs b/src/main.rs index 5b21323..dadc57a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,9 @@ use clap::{Args, Parser, Subcommand, ValueEnum}; use futures_util::stream::{FuturesUnordered, StreamExt}; use remote_installer::apk::ApkToolchain; use remote_installer::artifact_input::{self, PreparationStage, SigningPolicy}; -use remote_installer::exposure::{ExposureProvider, ExposureSession, provider_binary_available}; +use remote_installer::exposure::{ + ExposureProvider, ExposureSession, TailscaleStartupCoordinator, provider_binary_available, +}; use remote_installer::http::{self, HttpState}; use remote_installer::model::{Artifact, Availability, PlatformMetadata}; use remote_installer::service::{ShareConfig, ShareService}; @@ -97,17 +99,16 @@ struct ShareArgs { /// not control APK checks. #[arg(long)] allow_unsigned: bool, - /// HTTPS port used by Tailscale Serve or an explicitly selected Funnel. - /// Auto mode picks another supported Funnel port; `--funnel-port` remains - /// a visible compatibility alias. + /// Exact HTTPS port used by Tailscale Serve or an explicitly selected + /// Funnel. When omitted, an available port is selected automatically; + /// `--funnel-port` remains a visible compatibility alias. #[arg( long = "https-port", visible_alias = "funnel-port", value_name = "PORT", - default_value_t = 443, value_parser = parse_https_port )] - https_port: u16, + https_port: Option, /// Explicit path to the Tailscale CLI. #[arg(long, value_name = "PATH")] tailscale_bin: Option, @@ -274,8 +275,10 @@ async fn share(args: ShareArgs) -> Result<(), Box Vec { - match args.provider { - ShareProvider::Auto => vec![ - ProviderPlan { - provider: ExposureProvider::TailscaleServe, - https_port: args.https_port, - }, - ProviderPlan { - provider: ExposureProvider::TailscaleFunnel, - // Serve and Funnel cannot share a Tailscale HTTPS port. Keep - // both available in auto mode by selecting another supported - // Funnel port; explicit provider selection retains the exact - // --https-port value the caller requested. - https_port: auto_funnel_port(args.https_port), - }, - ProviderPlan { - provider: ExposureProvider::Cloudflare, - https_port: args.https_port, - }, - ], - ShareProvider::TailscaleServe => vec![ProviderPlan { - provider: ExposureProvider::TailscaleServe, - https_port: args.https_port, - }], - ShareProvider::TailscaleFunnel => vec![ProviderPlan { - provider: ExposureProvider::TailscaleFunnel, - https_port: args.https_port, - }], - ShareProvider::Cloudflare => vec![ProviderPlan { - provider: ExposureProvider::Cloudflare, - https_port: args.https_port, - }], - } -} - -fn auto_funnel_port(serve_port: u16) -> u16 { - [443, 8443, 10000] - .into_iter() - .find(|port| *port != serve_port) - .unwrap_or(8443) -} - async fn start_exposures( args: &ShareArgs, target: &Url, ) -> Result, Box> { let auto = matches!(args.provider, ShareProvider::Auto); - let mut plans = provider_plans(args); + let wants_tailscale = auto + || matches!( + args.provider, + ShareProvider::TailscaleServe | ShareProvider::TailscaleFunnel + ); let tailscale_available = provider_binary_available( ExposureProvider::TailscaleServe, args.tailscale_bin.as_deref(), @@ -507,36 +472,85 @@ async fn start_exposures( args.tailscale_bin.as_deref(), args.cloudflared_bin.as_deref(), ); - let tailscale_ready = if auto && tailscale_available { - match ExposureSession::check_tailscale_for_auto(args.tailscale_bin.as_deref()).await { - Ok(()) => true, + let mut tailscale_coordinator = if wants_tailscale && (tailscale_available || !auto) { + match TailscaleStartupCoordinator::acquire(args.tailscale_bin.as_deref()).await { + Ok(coordinator) => Some(coordinator), Err(error) => { - eprintln!( - "Warning: Tailscale is unavailable; skipping Tailscale providers: {error}" - ); - false + if auto { + eprintln!( + "Warning: Tailscale is unavailable; skipping Tailscale providers: {error}" + ); + None + } else { + return Err(format!("Tailscale could not start: {error}").into()); + } } } } else { - false + None }; - if auto { - plans.retain(|plan| { - let available = match plan.provider { - ExposureProvider::TailscaleServe - | ExposureProvider::TailscaleFunnel - | ExposureProvider::Tailscale => tailscale_available && tailscale_ready, - ExposureProvider::Cloudflare => cloudflare_available, - }; - if !available { + let mut plans = Vec::new(); + match args.provider { + ShareProvider::Auto => { + if let Some(coordinator) = tailscale_coordinator.as_mut() { + // Funnel has only three supported ports. Reserve it first so + // Serve, which accepts any valid port, cannot consume the last + // scarce Funnel route during a concurrent share. + match coordinator.reserve_auto_funnel_port(args.https_port) { + Ok(https_port) => plans.push(ProviderPlan { + provider: ExposureProvider::TailscaleFunnel, + https_port, + }), + Err(error) => eprintln!( + "Warning: Tailscale Funnel could not reserve a port; skipping provider: {error}" + ), + } + match coordinator.reserve_serve_port(args.https_port) { + Ok(https_port) => plans.push(ProviderPlan { + provider: ExposureProvider::TailscaleServe, + https_port, + }), + Err(error) => eprintln!( + "Warning: Tailscale Serve could not reserve a port; skipping provider: {error}" + ), + } + } else if !tailscale_available { eprintln!( - "Warning: {} is unavailable or not ready; skipping provider.", - plan.provider.name() + "Warning: Tailscale is unavailable; skipping Tailscale Serve and Funnel." ); } - available - }); + if cloudflare_available { + plans.push(ProviderPlan { + provider: ExposureProvider::Cloudflare, + https_port: args.https_port.unwrap_or(443), + }); + } else { + eprintln!("Warning: Cloudflare Quick Tunnel is unavailable; skipping provider."); + } + } + ShareProvider::TailscaleServe => { + let coordinator = tailscale_coordinator + .as_mut() + .ok_or("Tailscale CLI was not found")?; + plans.push(ProviderPlan { + provider: ExposureProvider::TailscaleServe, + https_port: coordinator.reserve_serve_port(args.https_port)?, + }); + } + ShareProvider::TailscaleFunnel => { + let coordinator = tailscale_coordinator + .as_mut() + .ok_or("Tailscale CLI was not found")?; + plans.push(ProviderPlan { + provider: ExposureProvider::TailscaleFunnel, + https_port: coordinator.reserve_funnel_port(args.https_port)?, + }); + } + ShareProvider::Cloudflare => plans.push(ProviderPlan { + provider: ExposureProvider::Cloudflare, + https_port: args.https_port.unwrap_or(443), + }), } if plans.is_empty() { return Err("no supported tunnel provider is available".into()); @@ -546,21 +560,25 @@ async fn start_exposures( .iter() .map(|plan| plan.provider.name()) .collect::>(); + let tailscale_starts = Arc::new(tokio::sync::Mutex::new(())); + let mut tailscale_pending = plans + .iter() + .filter(|plan| is_tailscale_provider(plan.provider)) + .count(); let mut starts = FuturesUnordered::new(); for plan in plans { let provider = plan.provider; let tailscale_binary = args.tailscale_bin.as_deref(); let cloudflared_binary = args.cloudflared_bin.as_deref(); - let use_shared_tailscale_preflight = auto - && tailscale_ready - && matches!( - provider, - ExposureProvider::TailscaleServe - | ExposureProvider::TailscaleFunnel - | ExposureProvider::Tailscale - ); + let coordinated_tailscale_start = + tailscale_coordinator.is_some() && is_tailscale_provider(provider); + let tailscale_starts = Arc::clone(&tailscale_starts); starts.push(async move { - let result = if use_shared_tailscale_preflight { + let result = if coordinated_tailscale_start { + // Tailscale updates one shared ServeConfig with optimistic + // concurrency. Serialize this process's Serve/Funnel writes; + // the file-backed coordinator serializes other instances. + let _serial = tailscale_starts.lock().await; ExposureSession::start_without_configuration_check( provider, target, @@ -594,6 +612,12 @@ async fn start_exposures( biased; Some((plan, result)) = starts.next() => { pending.retain(|name| *name != plan.provider.name()); + if is_tailscale_provider(plan.provider) { + tailscale_pending -= 1; + if tailscale_pending == 0 { + drop(tailscale_coordinator.take()); + } + } results.push((plan, result)); } _ = tokio::signal::ctrl_c() => { @@ -628,6 +652,15 @@ async fn start_exposures( Ok(exposures) } +fn is_tailscale_provider(provider: ExposureProvider) -> bool { + matches!( + provider, + ExposureProvider::TailscaleServe + | ExposureProvider::TailscaleFunnel + | ExposureProvider::Tailscale + ) +} + fn provider_order(provider: ExposureProvider) -> u8 { match provider { ExposureProvider::TailscaleServe => 0, @@ -920,24 +953,7 @@ mod tests { panic!("share command") }; assert!(matches!(args.provider, ShareProvider::Auto)); - let plans = provider_plans(&args); - assert_eq!( - plans.iter().map(|plan| plan.provider).collect::>(), - vec![ - ExposureProvider::TailscaleServe, - ExposureProvider::TailscaleFunnel, - ExposureProvider::Cloudflare, - ] - ); - assert_eq!(plans[0].https_port, 443); - assert_eq!(plans[1].https_port, 8443); - } - - #[test] - fn auto_funnel_port_avoids_the_serve_port() { - assert_eq!(auto_funnel_port(443), 8443); - assert_eq!(auto_funnel_port(8443), 443); - assert_eq!(auto_funnel_port(10000), 443); + assert_eq!(args.https_port, None); } #[test] @@ -983,11 +999,11 @@ mod tests { fn https_port_and_funnel_port_alias_share_one_argument() { assert_eq!( share_args(&["--provider", "tailscale-serve", "--https-port", "8080"]).https_port, - 8080 + Some(8080) ); assert_eq!( share_args(&["--provider", "tailscale-funnel", "--funnel-port", "8443"]).https_port, - 8443 + Some(8443) ); } From 1bf67dfe7e045c51eecc8b36dbb3a7e0a0528ebb Mon Sep 17 00:00:00 2001 From: Lance Wang Date: Tue, 22 Sep 2026 17:16:33 +0800 Subject: [PATCH 2/2] Fix Rust command enum lint --- src/background.rs | 4 ++-- src/main.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/background.rs b/src/background.rs index 5f64e92..ea1acdb 100644 --- a/src/background.rs +++ b/src/background.rs @@ -631,7 +631,7 @@ mod tests { args.background = false; args.no_qr = true; args.managed_session = Some(directory.to_owned()); - args + *args } fn state() -> SessionState { @@ -743,7 +743,7 @@ mod tests { else { panic!("share command") }; - let error = start(args).await.unwrap_err().to_string(); + let error = start(*args).await.unwrap_err().to_string(); assert!( error.contains("requires --expire-after or --timeout"), "{error}" diff --git a/src/main.rs b/src/main.rs index dadc57a..8d5169e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,7 +37,7 @@ struct Cli { enum Command { /// Share one IPA, signed iOS .app, or signed standalone APK. #[command(after_help = SHARE_EXAMPLES)] - Share(ShareArgs), + Share(Box), /// Show whether a background share is still running. Status(SessionArgs), /// Print the saved stdout and stderr for a background share. @@ -206,7 +206,7 @@ async fn main() -> ExitCode { .with_target(false) .init(); let result = match Cli::parse().command { - Command::Share(args) => run_share(args).await, + Command::Share(args) => run_share(*args).await, Command::Status(args) => background::status(&args.id, args.json), Command::Logs(args) => background::logs(&args.id), Command::Stop(args) => background::stop(&args.id, args.json).await, @@ -1055,7 +1055,7 @@ mod tests { let Command::Share(args) = Cli::try_parse_from(full).unwrap().command else { panic!("share command") }; - args + *args } #[test]