From 60c091de7ee9c06123e88c1baf59dca879ede75c Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sat, 16 May 2026 19:28:31 +1000 Subject: [PATCH 1/4] fix: prevent new machine from clobbering fleet state on init A machine_id is derived from hostname, so onboarding a machine that shares a hostname reused an existing machines/.json record. The new machine's first sync overwrote it with empty local state, which detect_removed_packages then flagged as deliberate removals across the fleet. init now detects an existing machine record before the first sync and asks whether it is the same machine or a new one, assigning a unique id when new. Also surface pnpm errors written to stdout: pnpm exits non-zero with the message on stdout, so failures previously showed a blank reason. --- src/cli/commands/init.rs | 84 +++++++++++++++++++++++++++++++++++++++- src/packages/pnpm.rs | 28 +++++++++++--- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/src/cli/commands/init.rs b/src/cli/commands/init.rs index e903fee..0d327fa 100644 --- a/src/cli/commands/init.rs +++ b/src/cli/commands/init.rs @@ -1,7 +1,7 @@ use crate::cli::{Output, Progress, Prompt}; use crate::config::{Config, FeaturesConfig}; use crate::github::GitHubCli; -use crate::sync::{GitBackend, SyncEngine, SyncState}; +use crate::sync::{GitBackend, MachineState, SyncEngine, SyncState}; use anyhow::Result; pub async fn run(repo: Option<&str>, no_daemon: bool, team_only: bool) -> Result<()> { @@ -103,6 +103,8 @@ pub async fn run(repo: Option<&str>, no_daemon: bool, team_only: bool) -> Result crate::sync::check_sync_format_version(&sync_path)?; + resolve_machine_identity(&sync_path)?; + // Setup encryption if enabled if config.security.encrypt_dotfiles { setup_encryption()?; @@ -279,6 +281,86 @@ fn assign_profile_during_init(config: &mut Config) -> Result<()> { Ok(()) } +/// Detect machine_id collisions before the first sync. +/// +/// machine_id is derived from hostname, so two machines with the same hostname +/// share a `machines/.json` record. The new machine's first sync would +/// overwrite the existing record with its (empty) local state and flag every +/// package as removed. On a fresh onboard, if the id is already taken, ask +/// whether this is the same machine or a new one and assign a unique id. +fn resolve_machine_identity(sync_path: &std::path::Path) -> Result<()> { + // Established machines already have an identity — don't re-prompt on reinit. + if SyncState::state_path()?.exists() { + return Ok(()); + } + + let mut state = SyncState::load()?; + let candidate = state.machine_id.clone(); + + let existing = match MachineState::load_from_repo(sync_path, &candidate)? { + Some(m) => m, + None => return Ok(()), + }; + + let pkg_count: usize = existing.packages.values().map(Vec::len).sum(); + Output::warning(&format!( + "A machine named '{}' already exists in your sync repo", + candidate + )); + Output::dim(&format!(" hostname: {}", existing.hostname)); + Output::dim(&format!( + " last sync: {}", + existing + .last_sync + .with_timezone(&chrono::Local) + .format("%Y-%m-%d %H:%M") + )); + Output::dim(&format!(" packages: {}", pkg_count)); + Output::dim(&format!(" dotfiles: {}", existing.dotfiles.len())); + println!(); + + let options = vec![ + "This is a new machine (pick a different name)", + "This is the same machine being re-onboarded", + ]; + // idx 1 = same machine: keep the existing id and its record untouched. + if Prompt::select("Is this the same machine, or a new one?", options, 0)? == 1 { + return Ok(()); + } + + let machines_dir = sync_path.join("machines"); + let suggested = suggest_machine_id(&machines_dir, &candidate); + loop { + let name = Prompt::input("New machine name", Some(&suggested))?; + let name = name.trim(); + // A machine id becomes a `machines/.json` filename. + if !Config::is_safe_profile_name(name) { + Output::error(&format!("Invalid machine name: '{}'", name)); + continue; + } + if machines_dir.join(format!("{}.json", name)).exists() { + Output::error(&format!("'{}' is already taken", name)); + continue; + } + state.machine_id = name.to_string(); + state.save()?; + Output::success(&format!("This machine will sync as '{}'", name)); + return Ok(()); + } +} + +/// Suggest an unused machine id by appending a numeric suffix. +fn suggest_machine_id(machines_dir: &std::path::Path, base: &str) -> String { + let mut n = 2; + loop { + let candidate = format!("{}-{}", base, n); + if !machines_dir.join(format!("{}.json", candidate)).exists() { + return candidate; + } + n += 1; + } +} + /// Try to load config from the synced repo (encrypted). /// Returns None if file doesn't exist or encryption is not set up yet. fn load_synced_config(sync_path: &std::path::Path) -> Option { diff --git a/src/packages/pnpm.rs b/src/packages/pnpm.rs index 3f5354e..548ba66 100644 --- a/src/packages/pnpm.rs +++ b/src/packages/pnpm.rs @@ -15,8 +15,10 @@ impl PnpmManager { let output = Command::new("pnpm").args(args).output().await?; if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(anyhow::anyhow!("pnpm command failed: {}", stderr)); + return Err(anyhow::anyhow!( + "pnpm command failed: {}", + pnpm_error_message(&output.stderr, &output.stdout) + )); } Ok(String::from_utf8(output.stdout)?) @@ -93,8 +95,10 @@ impl PackageManager for PnpmManager { let output = Command::new("pnpm").args(["update", "-g"]).output().await?; if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(anyhow::anyhow!("pnpm update failed: {}", stderr)); + return Err(anyhow::anyhow!( + "pnpm update failed: {}", + pnpm_error_message(&output.stderr, &output.stdout) + )); } Ok(()) @@ -107,10 +111,22 @@ impl PackageManager for PnpmManager { .await?; if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(anyhow::anyhow!("pnpm remove failed: {}", stderr)); + return Err(anyhow::anyhow!( + "pnpm remove failed: {}", + pnpm_error_message(&output.stderr, &output.stdout) + )); } Ok(()) } } + +/// pnpm writes failures to stdout, not stderr — fall back when stderr is empty. +fn pnpm_error_message(stderr: &[u8], stdout: &[u8]) -> String { + let err = String::from_utf8_lossy(stderr); + let trimmed = err.trim(); + if !trimmed.is_empty() { + return trimmed.to_string(); + } + String::from_utf8_lossy(stdout).trim().to_string() +} From 4c7c4390ef11a5b708e0ebade8778bb173ad55be Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sun, 17 May 2026 19:54:37 +1000 Subject: [PATCH 2/4] fix: generate random machine ids instead of using hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deriving machine_id from the hostname meant two machines sharing a hostname also shared a machines/.json record — the second machine's sync overwrote the first's state. The previous interactive collision check only narrowed the window and still mishandled a genuinely re-onboarded machine. machine_id is now a random hex id minted once per machine. Collisions are no longer possible and the hostname is purely display metadata on MachineState. Existing machines keep their hostname-based id since it is already persisted in state.json. --- src/cli/commands/init.rs | 84 +------------------------------------- src/security/encryption.rs | 17 ++++++++ src/security/mod.rs | 2 +- src/sync/state.rs | 8 ++-- 4 files changed, 23 insertions(+), 88 deletions(-) diff --git a/src/cli/commands/init.rs b/src/cli/commands/init.rs index 0d327fa..e903fee 100644 --- a/src/cli/commands/init.rs +++ b/src/cli/commands/init.rs @@ -1,7 +1,7 @@ use crate::cli::{Output, Progress, Prompt}; use crate::config::{Config, FeaturesConfig}; use crate::github::GitHubCli; -use crate::sync::{GitBackend, MachineState, SyncEngine, SyncState}; +use crate::sync::{GitBackend, SyncEngine, SyncState}; use anyhow::Result; pub async fn run(repo: Option<&str>, no_daemon: bool, team_only: bool) -> Result<()> { @@ -103,8 +103,6 @@ pub async fn run(repo: Option<&str>, no_daemon: bool, team_only: bool) -> Result crate::sync::check_sync_format_version(&sync_path)?; - resolve_machine_identity(&sync_path)?; - // Setup encryption if enabled if config.security.encrypt_dotfiles { setup_encryption()?; @@ -281,86 +279,6 @@ fn assign_profile_during_init(config: &mut Config) -> Result<()> { Ok(()) } -/// Detect machine_id collisions before the first sync. -/// -/// machine_id is derived from hostname, so two machines with the same hostname -/// share a `machines/.json` record. The new machine's first sync would -/// overwrite the existing record with its (empty) local state and flag every -/// package as removed. On a fresh onboard, if the id is already taken, ask -/// whether this is the same machine or a new one and assign a unique id. -fn resolve_machine_identity(sync_path: &std::path::Path) -> Result<()> { - // Established machines already have an identity — don't re-prompt on reinit. - if SyncState::state_path()?.exists() { - return Ok(()); - } - - let mut state = SyncState::load()?; - let candidate = state.machine_id.clone(); - - let existing = match MachineState::load_from_repo(sync_path, &candidate)? { - Some(m) => m, - None => return Ok(()), - }; - - let pkg_count: usize = existing.packages.values().map(Vec::len).sum(); - Output::warning(&format!( - "A machine named '{}' already exists in your sync repo", - candidate - )); - Output::dim(&format!(" hostname: {}", existing.hostname)); - Output::dim(&format!( - " last sync: {}", - existing - .last_sync - .with_timezone(&chrono::Local) - .format("%Y-%m-%d %H:%M") - )); - Output::dim(&format!(" packages: {}", pkg_count)); - Output::dim(&format!(" dotfiles: {}", existing.dotfiles.len())); - println!(); - - let options = vec![ - "This is a new machine (pick a different name)", - "This is the same machine being re-onboarded", - ]; - // idx 1 = same machine: keep the existing id and its record untouched. - if Prompt::select("Is this the same machine, or a new one?", options, 0)? == 1 { - return Ok(()); - } - - let machines_dir = sync_path.join("machines"); - let suggested = suggest_machine_id(&machines_dir, &candidate); - loop { - let name = Prompt::input("New machine name", Some(&suggested))?; - let name = name.trim(); - // A machine id becomes a `machines/.json` filename. - if !Config::is_safe_profile_name(name) { - Output::error(&format!("Invalid machine name: '{}'", name)); - continue; - } - if machines_dir.join(format!("{}.json", name)).exists() { - Output::error(&format!("'{}' is already taken", name)); - continue; - } - state.machine_id = name.to_string(); - state.save()?; - Output::success(&format!("This machine will sync as '{}'", name)); - return Ok(()); - } -} - -/// Suggest an unused machine id by appending a numeric suffix. -fn suggest_machine_id(machines_dir: &std::path::Path, base: &str) -> String { - let mut n = 2; - loop { - let candidate = format!("{}-{}", base, n); - if !machines_dir.join(format!("{}.json", candidate)).exists() { - return candidate; - } - n += 1; - } -} - /// Try to load config from the synced repo (encrypted). /// Returns None if file doesn't exist or encryption is not set up yet. fn load_synced_config(sync_path: &std::path::Path) -> Option { diff --git a/src/security/encryption.rs b/src/security/encryption.rs index a032613..87a9204 100644 --- a/src/security/encryption.rs +++ b/src/security/encryption.rs @@ -14,6 +14,13 @@ pub fn generate_key() -> [u8; KEY_SIZE] { key } +/// Generate a random 12-character hex id. +pub fn random_hex_id() -> String { + let mut bytes = [0u8; 6]; + OsRng.fill_bytes(&mut bytes); + hex::encode(bytes) +} + /// Encrypt data using AES-256-GCM /// Format: [nonce (12 bytes)][ciphertext + auth tag] pub fn encrypt(plaintext: &[u8], key: &[u8]) -> Result> { @@ -96,6 +103,16 @@ mod tests { assert_ne!(key1, key2); // Keys should be random } + #[test] + fn test_random_hex_id() { + let id1 = random_hex_id(); + let id2 = random_hex_id(); + + assert_eq!(id1.len(), 12); + assert!(id1.chars().all(|c| c.is_ascii_hexdigit())); + assert_ne!(id1, id2); + } + #[test] fn test_encrypt_decrypt_roundtrip() { let key = generate_key(); diff --git a/src/security/mod.rs b/src/security/mod.rs index acdf0f0..6fa7f03 100644 --- a/src/security/mod.rs +++ b/src/security/mod.rs @@ -58,7 +58,7 @@ pub fn write_owner_only(path: &Path, data: &[u8]) -> Result<()> { Ok(()) } -pub use encryption::{decrypt, encrypt, generate_key}; +pub use encryption::{decrypt, encrypt, generate_key, random_hex_id}; pub use keychain::{ clear_cached_key, get_encryption_key, has_encryption_key, is_unlocked, store_encryption_key_with_passphrase, unlock_with_passphrase, diff --git a/src/sync/state.rs b/src/sync/state.rs index b2204c8..f986ea2 100644 --- a/src/sync/state.rs +++ b/src/sync/state.rs @@ -278,11 +278,11 @@ impl SyncState { } } + /// A random id, not the hostname — hostnames collide across machines, and a + /// collision makes one machine's sync overwrite another's `machines/.json` + /// record. The human-readable hostname lives on `MachineState` as metadata. fn generate_machine_id() -> String { - hostname::get() - .ok() - .and_then(|h| h.into_string().ok()) - .unwrap_or_else(|| "unknown".to_string()) + crate::security::random_hex_id() } pub fn update_file(&mut self, path: &str, hash: String) { From a3aff895d549a6462df1018e799be18ef4735db3 Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sun, 17 May 2026 20:02:47 +1000 Subject: [PATCH 3/4] test: cover pnpm_error_message; tidy random_hex_id doc --- src/packages/pnpm.rs | 20 ++++++++++++++++++++ src/security/encryption.rs | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/packages/pnpm.rs b/src/packages/pnpm.rs index 548ba66..6c913ae 100644 --- a/src/packages/pnpm.rs +++ b/src/packages/pnpm.rs @@ -130,3 +130,23 @@ fn pnpm_error_message(stderr: &[u8], stdout: &[u8]) -> String { } String::from_utf8_lossy(stdout).trim().to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn prefers_trimmed_stderr() { + assert_eq!(pnpm_error_message(b" boom ", b"ignored"), "boom"); + } + + #[test] + fn falls_back_to_stdout_when_stderr_blank() { + assert_eq!(pnpm_error_message(b" \n", b" ENOENT "), "ENOENT"); + } + + #[test] + fn empty_when_both_blank() { + assert_eq!(pnpm_error_message(b"", b" "), ""); + } +} diff --git a/src/security/encryption.rs b/src/security/encryption.rs index 87a9204..a0b58a5 100644 --- a/src/security/encryption.rs +++ b/src/security/encryption.rs @@ -14,7 +14,7 @@ pub fn generate_key() -> [u8; KEY_SIZE] { key } -/// Generate a random 12-character hex id. +/// Generate a random hex id. pub fn random_hex_id() -> String { let mut bytes = [0u8; 6]; OsRng.fill_bytes(&mut bytes); From c11878e85625954d7a088973f16c6c2f264fbafc Mon Sep 17 00:00:00 2001 From: Paddo <653385+paddo@users.noreply.github.com> Date: Sun, 17 May 2026 20:54:42 +1000 Subject: [PATCH 4/4] docs: add changelog entry for machine-id and pnpm fixes --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f8ddce..ea03fc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Onboarding a machine that shares a hostname with an existing machine no longer overwrites that machine's sync state — machine identity is now a random id rather than the hostname +- `pnpm` failures now surface the real error text (pnpm writes errors to stdout, not stderr, so failures previously showed a blank reason) + +### Changed + +- New machines get a random machine id; existing machines keep their hostname-based id, so no migration runs on upgrade. A fleet that already contains two machines sharing a hostname is not auto-repaired — re-id one of them by removing its `~/.tether/state.json` and running `tether init` again + ## [1.11.10] - 2026-04-08 ### Fixed