Share SSH command construction in remotes

Four builders repeated the same batch-mode SSH option list and the same
POSIX "~/.local/bin/herdr or PATH" fallback script, and the delete/stop
helpers were near-identical copies. Build every remote command through
ssh_command()/posix_herdr_script() and run Session mutations through one
helper. Generated argv is byte-for-byte unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

herdr_hub/remotes.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import base64
 import codecs
 from collections import deque
 from dataclasses import dataclass
@@ -7,26 +8,47 @@ import fcntl
 import json
 import os
 from pathlib import Path
+import re
+import shlex
 import subprocess
 import tempfile
 import time
-from typing import Iterable, Sequence
+from typing import Callable, Iterable, Sequence
 from urllib.parse import SplitResult, unquote, urlsplit
 
 from .domain import is_valid_session_name
 
 
 REGISTRY_VERSION = 1
-REMOTE_DIRECT_COMMAND = "herdr session list --json"
-REMOTE_LIST_SCRIPT = (
-    'if [ -x "$HOME/.local/bin/herdr" ]; then '
-    'exec "$HOME/.local/bin/herdr" session list --json; '
-    "elif command -v herdr >/dev/null 2>&1; then "
-    "exec herdr session list --json; "
-    "else echo 'herdr is not installed on the remote host' >&2; exit 127; fi"
+# Non-interactive, non-replayed SSH: fail fast instead of prompting.
+SSH_BATCH_OPTIONS = (
+    "-T",
+    "-o", "BatchMode=yes",
+    "-o", "StrictHostKeyChecking=yes",
+    "-o", "ConnectionAttempts=1",
 )
 
 
+def posix_herdr_script(arguments: str, *, prefix: str = "") -> str:
+    """POSIX sh snippet running `herdr ARGUMENTS` with the per-user fallback.
+
+    A non-interactive SSH PATH often lacks ~/.local/bin; ``arguments`` must
+    already be shell-quoted. Exit 127 lets callers tell "no herdr" apart from
+    a failed command.
+    """
+    return (
+        prefix + 'if [ -x "$HOME/.local/bin/herdr" ]; then '
+        f'exec "$HOME/.local/bin/herdr" {arguments}; '
+        "elif command -v herdr >/dev/null 2>&1; then "
+        f"exec herdr {arguments}; "
+        "else echo 'herdr is not installed on the remote host' >&2; exit 127; fi"
+    )
+
+
+REMOTE_DIRECT_COMMAND = "herdr session list --json"
+REMOTE_LIST_SCRIPT = posix_herdr_script("session list --json")
+
+
 class RegistryError(RuntimeError):
     """A remote registry error which is safe to show to the user."""
 
@@ -309,6 +331,39 @@ def ssh_destination_arguments(target: str) -> list[str]:
     return arguments
 
 
+def ssh_command(
+    ssh_bin: str,
+    entry: RemoteEntry,
+    remote_command: str,
+    *,
+    connect_timeout: int = 4,
+) -> list[str]:
+    """One batch-mode SSH invocation running ``remote_command`` on the target."""
+    return [
+        ssh_bin,
+        *SSH_BATCH_OPTIONS,
+        "-o",
+        f"ConnectTimeout={connect_timeout}",
+        *ssh_destination_arguments(entry.target),
+        remote_command,
+    ]
+
+
+def remote_herdr_command(
+    ssh_bin: str,
+    entry: RemoteEntry,
+    arguments: str,
+    *,
+    connect_timeout: int = 4,
+    posix_fallback: bool = False,
+) -> list[str]:
+    """Run `herdr ARGUMENTS` remotely, via PATH or the POSIX ~/.local/bin fallback."""
+    remote_command = (
+        posix_herdr_script(arguments) if posix_fallback else f"herdr {arguments}"
+    )
+    return ssh_command(ssh_bin, entry, remote_command, connect_timeout=connect_timeout)
+
+
 def remote_session_command(
     ssh_bin: str,
     entry: RemoteEntry,
@@ -316,20 +371,19 @@ def remote_session_command(
     connect_timeout: int = 4,
     posix_fallback: bool = False,
 ) -> list[str]:
-    return [
+    return remote_herdr_command(
         ssh_bin,
-        "-T",
-        "-o",
-        "BatchMode=yes",
-        "-o",
-        "StrictHostKeyChecking=yes",
-        "-o",
-        "ConnectionAttempts=1",
-        "-o",
-        f"ConnectTimeout={connect_timeout}",
-        *ssh_destination_arguments(entry.target),
-        REMOTE_LIST_SCRIPT if posix_fallback else REMOTE_DIRECT_COMMAND,
-    ]
+        entry,
+        "session list --json",
+        connect_timeout=connect_timeout,
+        posix_fallback=posix_fallback,
+    )
+
+
+def _session_arguments(action: str, session: str) -> str:
+    if not is_valid_session_name(session):
+        raise ValueError("invalid remote Session name")
+    return f"session {action} -- {session}"
 
 
 def remote_delete_session_command(
@@ -340,30 +394,13 @@ def remote_delete_session_command(
     connect_timeout: int = 4,
     posix_fallback: bool = False,
 ) -> list[str]:
-    if not is_valid_session_name(session):
-        raise ValueError("invalid remote Session name")
-    direct_command = f"herdr session delete --json -- {session}"
-    fallback_command = (
-        'if [ -x "$HOME/.local/bin/herdr" ]; then '
-        f'exec "$HOME/.local/bin/herdr" session delete --json -- {session}; '
-        "elif command -v herdr >/dev/null 2>&1; then "
-        f"exec herdr session delete --json -- {session}; "
-        "else echo 'herdr is not installed on the remote host' >&2; exit 127; fi"
-    )
-    return [
+    return remote_herdr_command(
         ssh_bin,
-        "-T",
-        "-o",
-        "BatchMode=yes",
-        "-o",
-        "StrictHostKeyChecking=yes",
-        "-o",
-        "ConnectionAttempts=1",
-        "-o",
-        f"ConnectTimeout={connect_timeout}",
-        *ssh_destination_arguments(entry.target),
-        fallback_command if posix_fallback else direct_command,
-    ]
+        entry,
+        _session_arguments("delete --json", session),
+        connect_timeout=connect_timeout,
+        posix_fallback=posix_fallback,
+    )
 
 
 def remote_stop_session_command(
@@ -374,30 +411,54 @@ def remote_stop_session_command(
     connect_timeout: int = 4,
     posix_fallback: bool = False,
 ) -> list[str]:
-    if not is_valid_session_name(session):
-        raise ValueError("invalid remote Session name")
-    direct_command = f"herdr session stop -- {session}"
-    fallback_command = (
-        'if [ -x "$HOME/.local/bin/herdr" ]; then '
-        f'exec "$HOME/.local/bin/herdr" session stop -- {session}; '
-        "elif command -v herdr >/dev/null 2>&1; then "
-        f"exec herdr session stop -- {session}; "
-        "else echo 'herdr is not installed on the remote host' >&2; exit 127; fi"
-    )
-    return [
+    return remote_herdr_command(
         ssh_bin,
-        "-T",
-        "-o",
-        "BatchMode=yes",
-        "-o",
-        "StrictHostKeyChecking=yes",
-        "-o",
-        "ConnectionAttempts=1",
-        "-o",
-        f"ConnectTimeout={connect_timeout}",
-        *ssh_destination_arguments(entry.target),
-        fallback_command if posix_fallback else direct_command,
-    ]
+        entry,
+        _session_arguments("stop", session),
+        connect_timeout=connect_timeout,
+        posix_fallback=posix_fallback,
+    )
+
+
+def _run_remote_session_command(
+    build: Callable[..., list[str]],
+    error_type: type[RuntimeError],
+    action: str,
+    ssh_bin: str,
+    entry: RemoteEntry,
+    session: str,
+    timeout: float,
+) -> tuple[str, str]:
+    """Run a Session mutation, retrying once through the POSIX PATH fallback.
+
+    Returns decoded (stdout, stderr); any non-zero exit becomes ``error_type``.
+    """
+    result: subprocess.CompletedProcess[bytes] | None = None
+    for posix_fallback in (False, True):
+        command = build(ssh_bin, entry, session, posix_fallback=posix_fallback)
+        try:
+            result = subprocess.run(
+                command,
+                capture_output=True,
+                check=False,
+                timeout=timeout,
+            )
+        except subprocess.TimeoutExpired as error:
+            raise error_type(
+                f"remote Session {action} timed out after {timeout:g}s"
+            ) from error
+        except OSError as error:
+            raise error_type(f"could not start remote Session {action}: {error}") from error
+        if result.returncode != 127 or posix_fallback:
+            break
+
+    assert result is not None
+    stdout = decode_remote_output(result.stdout)
+    stderr = decode_remote_output(result.stderr)
+    if result.returncode != 0:
+        detail = (stderr or stdout or f"ssh exited with status {result.returncode}").strip()
+        raise error_type(" ".join(detail.splitlines()))
+    return stdout, stderr
 
 
 def delete_remote_session(
@@ -408,38 +469,15 @@ def delete_remote_session(
     timeout: float = 8.0,
 ) -> None:
     """Delete one stopped Herdr Session through a non-interactive SSH call."""
-    result: subprocess.CompletedProcess[bytes] | None = None
-    for posix_fallback in (False, True):
-        command = remote_delete_session_command(
-            ssh_bin,
-            entry,
-            session,
-            posix_fallback=posix_fallback,
-        )
-        try:
-            result = subprocess.run(
-                command,
-                capture_output=True,
-                check=False,
-                timeout=timeout,
-            )
-        except subprocess.TimeoutExpired as error:
-            raise RemoteSessionDeleteError(
-                f"remote Session delete timed out after {timeout:g}s"
-            ) from error
-        except OSError as error:
-            raise RemoteSessionDeleteError(
-                f"could not start remote Session delete: {error}"
-            ) from error
-        if result.returncode != 127 or posix_fallback:
-            break
-
-    assert result is not None
-    stdout = decode_remote_output(result.stdout)
-    stderr = decode_remote_output(result.stderr)
-    if result.returncode != 0:
-        detail = (stderr or stdout or f"ssh exited with status {result.returncode}").strip()
-        raise RemoteSessionDeleteError(" ".join(detail.splitlines()))
+    stdout, _ = _run_remote_session_command(
+        remote_delete_session_command,
+        RemoteSessionDeleteError,
+        "delete",
+        ssh_bin,
+        entry,
+        session,
+        timeout,
+    )
     try:
         payload = json.loads(stdout)
     except json.JSONDecodeError as error:
@@ -458,38 +496,15 @@ def stop_remote_session(
     timeout: float = 8.0,
 ) -> None:
     """Stop one running Herdr Session through a non-interactive SSH call."""
-    result: subprocess.CompletedProcess[bytes] | None = None
-    for posix_fallback in (False, True):
-        command = remote_stop_session_command(
-            ssh_bin,
-            entry,
-            session,
-            posix_fallback=posix_fallback,
-        )
-        try:
-            result = subprocess.run(
-                command,
-                capture_output=True,
-                check=False,
-                timeout=timeout,
-            )
-        except subprocess.TimeoutExpired as error:
-            raise RemoteSessionStopError(
-                f"remote Session stop timed out after {timeout:g}s"
-            ) from error
-        except OSError as error:
-            raise RemoteSessionStopError(
-                f"could not start remote Session stop: {error}"
-            ) from error
-        if result.returncode != 127 or posix_fallback:
-            break
-
-    assert result is not None
-    stdout = decode_remote_output(result.stdout)
-    stderr = decode_remote_output(result.stderr)
-    if result.returncode != 0:
-        detail = (stderr or stdout or f"ssh exited with status {result.returncode}").strip()
-        raise RemoteSessionStopError(" ".join(detail.splitlines()))
+    _run_remote_session_command(
+        remote_stop_session_command,
+        RemoteSessionStopError,
+        "stop",
+        ssh_bin,
+        entry,
+        session,
+        timeout,
+    )
 
 
 def clip_utf8_tail(raw: bytes, limit: int) -> bytes:
@@ -781,24 +796,13 @@ def remote_control_command(
     Windows SSH commonly uses cmd.exe; only a base64 PowerShell script crosses
     that shell. ProcessStartInfo then receives Windows-quoted native arguments.
     """
-    import base64
-    import re
-    import shlex
-
     if any(not re.fullmatch(r"[A-Z_][A-Z_0-9]*", key) for key in unset_environment):
         raise ValueError("invalid environment variable name")
     if any("\0" in argument for argument in arguments):
         raise ValueError("NUL in remote argument")
     if platform == "posix":
         unset = "".join(f"unset {key}; " for key in unset_environment)
-        argv = shlex.join(arguments)
-        script = (
-            unset + 'if [ -x "$HOME/.local/bin/herdr" ]; then '
-            f'exec "$HOME/.local/bin/herdr" {argv}; '
-            'elif command -v herdr >/dev/null 2>&1; then '
-            f"exec herdr {argv}; "
-            "else echo 'herdr is not installed on the remote host' >&2; exit 127; fi"
-        )
+        script = posix_herdr_script(shlex.join(arguments), prefix=unset)
     elif platform == "windows":
         def literal(value: str) -> str:
             return "'" + value.replace("'", "''") + "'"
@@ -831,6 +835,4 @@ def remote_control_command(
         script = "powershell.exe -NoLogo -NoProfile -NonInteractive -EncodedCommand " + encoded
     else:
         raise ValueError("unknown remote platform")
-    return [ssh_bin, "-T", "-o", "BatchMode=yes", "-o", "StrictHostKeyChecking=yes",
-            "-o", "ConnectionAttempts=1", "-o", "ConnectTimeout=4",
-            *ssh_destination_arguments(entry.target), script]
+    return ssh_command(ssh_bin, entry, script)