From 55fe1b613847201781054936a082c1a41427b574 Mon Sep 17 00:00:00 2001 From: Adam McGreggor Date: Sat, 11 Jul 2026 20:18:27 +0100 Subject: [PATCH] fix(pseudohome): bound clone SSH timeout, add run() heartbeat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git_tools.py's clone/fetch SSH command had no BatchMode/ConnectTimeout, unlike the SSH probe in ssh_utils.py. A stalled network path (Tailscale half-up) or unauthorised deploy key hung the clone indefinitely with no output, since Executor.run() captures via pipes rather than streaming. Bound it to 15s so the existing retry/prompt logic actually engages instead of the process just sitting there. Executor.run() now polls via Popen + periodic communicate(timeout=15) instead of a single blocking subprocess.run(), logging "still running" so a slow command is distinguishable from a hung one. Same return/ exception semantics as before (verified: success, check=True failure with populated stdout/stderr, check=False, dry-run, interactive). Also stop assuming the wolfcraig key-copy hint's mDNS .local suffix resolves — it doesn't on a box that isn't on the same LAN (e.g. a cloud VM). Prefer the Tailscale IP, which is reachable regardless. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019nR4GeGS2EDhLypRNm1JhW --- lib/executor.py | 66 ++++++++++++++++-------- lib/installer_utils/git_tools.py | 11 +++- lib/installer_utils/module_pseudohome.py | 31 +++++++++-- 3 files changed, 81 insertions(+), 27 deletions(-) diff --git a/lib/executor.py b/lib/executor.py index 23557a2..ab1878b 100644 --- a/lib/executor.py +++ b/lib/executor.py @@ -84,43 +84,65 @@ def run(self, log.info(f"Executing: {log_cmd}") # --- 5. Actual Execution --- - + full_env = os.environ.copy() if env: full_env.update(env) try: - result = subprocess.run( + process = subprocess.Popen( cmd_list, cwd=cwd, - check=check, stdin=stdin_target, stdout=stdout_target, stderr=stderr_target, env=full_env, - universal_newlines=True + universal_newlines=True, ) - - # Logging success/debug output only if not running interactively - # (since interactive output goes directly to terminal) - if not interactive: - if self.verbose: - log.debug(f"Command Output:\n{result.stdout}\n{result.stderr}") - if not suppress_logging: - log.success(f"Executed: {log_cmd}") - - return result - except subprocess.CalledProcessError as e: - # This block only executes if 'check=True' AND the command failed. - # Output is already logged by the caller if 'interactive' is false. - if not interactive: - log.error(f"Command failed with exit code {e.returncode}: {log_cmd}") - log.error(f"STDOUT:\n{e.stdout}") - log.error(f"STDERR:\n{e.stderr}") - raise except FileNotFoundError: log.critical(f"Command not found: {cmd_list[0]}") sys.exit(1) + + # Poll with a timeout instead of blocking outright, so a slow/stalled + # command (flaky network, unauthorised SSH key, etc.) logs a heartbeat + # instead of looking indistinguishable from a hang. communicate() can + # be safely re-called after a TimeoutExpired without losing output. + heartbeat_seconds = 15 + elapsed = 0 + stdout_data = "" + stderr_data = "" + while True: + try: + stdout_data, stderr_data = process.communicate(timeout=heartbeat_seconds) + break + except subprocess.TimeoutExpired: + elapsed += heartbeat_seconds + if not suppress_logging: + log.info(f"Still running ({elapsed}s elapsed): {log_cmd}") + + result = subprocess.CompletedProcess( + args=cmd_list, returncode=process.returncode, stdout=stdout_data, stderr=stderr_data + ) + + if check and result.returncode != 0: + # This block only executes if 'check=True' AND the command failed. + if not interactive: + log.error(f"Command failed with exit code {result.returncode}: {log_cmd}") + log.error(f"STDOUT:\n{result.stdout}") + log.error(f"STDERR:\n{result.stderr}") + raise subprocess.CalledProcessError( + result.returncode, cmd_list, output=result.stdout, stderr=result.stderr + ) + + # Logging success/debug output only if not running interactively + # (since interactive output goes directly to terminal) + if not interactive: + if self.verbose: + log.debug(f"Command Output:\n{result.stdout}\n{result.stderr}") + if not suppress_logging: + log.success(f"Executed: {log_cmd}") + + return result EXEC = Executor() diff --git a/lib/installer_utils/git_tools.py b/lib/installer_utils/git_tools.py index d022e0c..2b718ea 100644 --- a/lib/installer_utils/git_tools.py +++ b/lib/installer_utils/git_tools.py @@ -39,8 +39,17 @@ def clone_or_update_repo(exec_obj: Executor, env_prefix = "" if ssh_key_path: # Define the SSH command using the deploy key. + # BatchMode+ConnectTimeout are required here: without them a stalled + # network path (e.g. Tailscale half-up) or an unauthorised key hangs + # this clone/fetch indefinitely with zero feedback, since output is + # captured rather than streamed. Bounding it lets the existing + # retry/prompt logic in clone_or_update_private_repo_with_key_check + # actually kick in instead of the process just sitting there. # FIX: Use double quotes for the path to prevent premature string termination in bash -c - ssh_command = f"ssh -i \"{ssh_key_path}\" -o IdentitiesOnly=yes" + ssh_command = ( + f"ssh -i \"{ssh_key_path}\" -o IdentitiesOnly=yes " + "-o BatchMode=yes -o ConnectTimeout=15" + ) # Bundle the environment setting command string directly into the prefix env_prefix = f"GIT_SSH_COMMAND='{ssh_command}' " log.debug(f"Using GIT_SSH_COMMAND prefix: {env_prefix}") diff --git a/lib/installer_utils/module_pseudohome.py b/lib/installer_utils/module_pseudohome.py index 0cc354d..31fe1a2 100644 --- a/lib/installer_utils/module_pseudohome.py +++ b/lib/installer_utils/module_pseudohome.py @@ -21,13 +21,36 @@ PSEUDOHOME_INSTALLER: str = "pseudohome-symlinks" -def _show_wolfcraig_copy_hint(user: str, ssh_dir: str, repo_name: str) -> None: +def _show_wolfcraig_copy_hint(exec_obj: Executor, user: str, ssh_dir: str, repo_name: str) -> None: + # Prefer the Tailscale IP: it's reachable regardless of LAN/mDNS, unlike a + # bare hostname (which needs .local mDNS on the same network to resolve, + # and this box may not be on one — e.g. a cloud VM). hostname = platform.node() + address = hostname + try: + result = exec_obj.run("tailscale ip -4", check=True, run_quiet=True, force_sudo=True) + ts_ip = result.stdout.strip() + if ts_ip: + address = ts_ip + except Exception as e: + log.debug(f"Could not determine Tailscale IP for copy hint: {e}") + pub_key_path = os.path.join(ssh_dir, f"{repo_name}.pub") - cmd = f"ssh {user}@{hostname}.local 'cat {pub_key_path}' | ssh {user}@wolfcraig 'cat >> ~/.ssh/authorized_keys'" + cmd = ( + f"ssh {user}@{address} 'cat {pub_key_path}' " + f"| ssh {user}@wolfcraig 'cat >> ~/.ssh/authorized_keys'" + ) print("\n" + "=" * 70, flush=True) - log.warning("ACTION REQUIRED: run this on your LOCAL machine to authorise the deploy key on wolfcraig:") + log.warning( + "ACTION REQUIRED: run this on your LOCAL machine to authorise the deploy key on wolfcraig:" + ) print("=" * 70, flush=True) + if address == hostname: + log.warning( + f"Could not determine a Tailscale IP; using bare hostname '{address}', " + "which only resolves on the same LAN (no .local mDNS assumed). Swap it " + "for a reachable hostname/IP if the command below doesn't connect:" + ) print(f"\n {cmd}\n", flush=True) print("=" * 70 + "\n", flush=True) for i in range(10, 0, -1): @@ -57,7 +80,7 @@ def setup_pseudohome(exec_obj: Executor) -> None: key_is_new = _create_if_needed_ssh_key(exec_obj, user, ssh_dir, repo_name) if key_is_new: - _show_wolfcraig_copy_hint(user, ssh_dir, repo_name) + _show_wolfcraig_copy_hint(exec_obj, user, ssh_dir, repo_name) # Ensure the .ssh directory itself has strict permissions before use set_ssh_perms(exec_obj, user, ssh_dir)