From 40437bf78cbedb648969acdcfba79d486c7af217 Mon Sep 17 00:00:00 2001 From: Overlord Date: Fri, 14 Aug 2026 12:32:56 +0200 Subject: [PATCH] =?UTF-8?q?Remove=20spec=20=C2=A7N.N=20citations=20from=20?= =?UTF-8?q?comments,=20keep=20them=20self-contained?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments citing the spec document instead of just stating the fact directly made their usefulness depend on cross-referencing a separate file. Reworded each one to stand alone - same content, citation dropped, folded into a normal sentence where it was mid-clause rather than trailing. spec/porthole-spec.md itself is untouched. Co-Authored-By: Claude Sonnet 5 --- src/cli.rs | 2 +- src/commands/close.rs | 10 +++++----- src/commands/mod.rs | 2 +- src/commands/open.rs | 24 ++++++++++++------------ src/commands/status.rs | 2 +- src/commands/wipe.rs | 6 +++--- src/instance.rs | 5 ++--- src/profile.rs | 8 ++++---- src/ssh.rs | 8 ++++---- src/supervisor.rs | 10 +++++----- tests/live_test.sh | 2 +- 11 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index a7970b1..68fb4ae 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -48,7 +48,7 @@ pub enum Commands { Completions { shell: Shell }, /// Internal: runs the supervisor loop for one profile. Not for direct - /// use - `open` spawns this itself (spec §3). + /// use - `open` spawns this itself. #[command(hide = true, name = "__supervise")] Supervise { name: String }, } diff --git a/src/commands/close.rs b/src/commands/close.rs index fa63b07..4ca44d3 100644 --- a/src/commands/close.rs +++ b/src/commands/close.rs @@ -18,8 +18,8 @@ pub fn run(args: CloseArgs) -> Result<()> { } /// Stops `name`'s supervisor if one is actually running, and clears any -/// stale instance file either way (spec §5.3) - shared with `remove` and -/// `wipe`. Returns whether anything was actually running. +/// stale instance file either way - shared with `remove` and `wipe`. +/// Returns whether anything was actually running. pub fn close_instance(name: &str, force: bool) -> Result { let Some(pid) = instance::running_pid(name)? else { instance::delete(name)?; // clears a stale file left by a crash @@ -27,9 +27,9 @@ pub fn close_instance(name: &str, force: bool) -> Result { }; if force { - // spec §3 step 4: SIGKILL the whole process group (the supervisor - // is its own group leader via setsid), not just the supervisor pid - // - a plain single-pid SIGKILL would leave `ssh` orphaned. + // SIGKILL the whole process group (the supervisor is its own + // group leader via setsid), not just the supervisor pid - a plain + // single-pid SIGKILL would leave `ssh` orphaned. unsafe { libc::kill(-pid, libc::SIGKILL) }; } else { unsafe { libc::kill(pid, libc::SIGTERM) }; diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 93340b9..76ee52d 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -12,7 +12,7 @@ pub mod wipe; use crate::cli::MappingArgs; use crate::profile::ProfileEdits; -/// Turns clap's `MappingArgs` into a `ProfileEdits` (spec §5.1). `--via` is +/// Turns clap's `MappingArgs` into a `ProfileEdits`. `--via` is /// collected by clap itself: `value_delimiter = ','` splits each /// occurrence on commas, and the field being a `Vec` allows repeated /// `--via` flags, so both `--via a,b` and `--via a --via b` reach here as diff --git a/src/commands/open.rs b/src/commands/open.rs index ff426c4..7af4032 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -1,5 +1,5 @@ -//! `open` - spec §3/§5.2. Validates, then either runs the supervisor loop -//! inline (`--foreground`) or spawns a detached copy of this binary +//! `open` validates, then either runs the supervisor loop inline +//! (`--foreground`) or spawns a detached copy of this binary //! (`porthole __supervise `) and waits briefly for it to confirm. use crate::cli::OpenArgs; @@ -25,8 +25,8 @@ pub fn run(args: OpenArgs) -> Result<()> { } /// Opens every `reconnect: true` profile that isn't already running - the -/// hook external autostart mechanisms are meant to call (spec §5.2/§8). -/// Per-profile failures are warnings, not a whole-batch failure. +/// hook external autostart mechanisms are meant to call. Per-profile +/// failures are warnings, not a whole-batch failure. fn open_all(once: bool) -> Result<()> { let profiles = profile::list_all()?; let mut opened = 0; @@ -57,8 +57,8 @@ fn open_one(name: &str, foreground: bool, once: bool) -> Result<()> { return Ok(()); } // Clear a stale instance file left by a crash before spawning. The - // lock, not this file, is the authority on "already open" (spec - // §5.2) - this just keeps `status` from reading stale state mid-spawn. + // lock, not this file, is the authority on "already open" - this just + // keeps `status` from reading stale state mid-spawn. instance::delete(name)?; if foreground { @@ -73,10 +73,10 @@ fn open_one(name: &str, foreground: bool, once: bool) -> Result<()> { wait_for_confirmation(name) } -/// Spawns `porthole __supervise ` fully detached (spec §3 steps 1-2): -/// stdin from `/dev/null`, stdout/stderr appended to the profile's log, and -/// `setsid()` in the child so it leaves this process's session and survives -/// the terminal closing. +/// Spawns `porthole __supervise ` fully detached: stdin from +/// `/dev/null`, stdout/stderr appended to the profile's log, and +/// `setsid()` in the child so it leaves this process's session and +/// survives the terminal closing. fn spawn_detached(name: &str, once: bool) -> Result<()> { let exe = std::env::current_exe()?; let log_path = instance::log_path(name); @@ -99,8 +99,8 @@ fn spawn_detached(name: &str, once: bool) -> Result<()> { /// Blocks briefly for the detached supervisor to reach a conclusive state, /// so an immediate failure (bad auth, bind conflict, unresolvable host) is -/// reported with a non-zero exit instead of `open` appearing to succeed -/// (spec §5.2). The instance file's initial write is always +/// reported with a non-zero exit instead of `open` appearing to succeed. +/// The instance file's initial write is always /// `State::Reconnecting`, since the first attempt has not concluded yet; /// that value is indistinguishable from "already failed once, backing /// off". This function waits specifically for `Up` or `Error`, not merely diff --git a/src/commands/status.rs b/src/commands/status.rs index afa0f50..de92b7a 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -11,7 +11,7 @@ pub fn run(args: StatusArgs) -> Result<()> { let inst = instance::load(&name)?; // An instance file whose pid isn't actually alive means the supervisor // crashed without cleaning up - report that, rather than trusting a - // state the process table disagrees with (spec §2.2). + // state the process table disagrees with. let live = inst.as_ref().is_some_and(|i| instance::supervisor_alive(i.pid, &name)); if args.json { diff --git a/src/commands/wipe.rs b/src/commands/wipe.rs index 79ee09e..2ec0154 100644 --- a/src/commands/wipe.rs +++ b/src/commands/wipe.rs @@ -42,9 +42,9 @@ pub fn run(args: WipeArgs) -> Result<()> { } /// Kills any supervisor process not backed by a tracked profile (e.g. one -/// orphaned after a crash), matched by cmdline rather than tracked state -/// (spec §5.8). Sends SIGTERM to each supervisor's process group so its -/// `ssh` child is included. +/// orphaned after a crash), matched by cmdline rather than tracked state. +/// Sends SIGTERM to each supervisor's process group so its `ssh` child is +/// included. fn kill_orphaned_supervisors() -> u32 { let mut killed = 0; let Ok(entries) = std::fs::read_dir("/proc") else { return 0 }; diff --git a/src/instance.rs b/src/instance.rs index 10b834c..851f0b4 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -111,9 +111,8 @@ pub fn instance_path(name: &str) -> PathBuf { state_dir().join(format!("{name}.j pub fn lock_path(name: &str) -> PathBuf { state_dir().join(format!("{name}.lock")) } pub fn log_path(name: &str) -> PathBuf { state_dir().join(format!("{name}.log")) } -/// Loads the instance file for `name`, if any. `None` means closed - the -/// absence of this file is the closed state (spec §2.2); there is no -/// separate enum value for it. +/// Loads the instance file for `name`, if any. `None` means closed - its +/// absence *is* the closed state; there is no separate enum value for it. pub fn load(name: &str) -> Result> { let path = instance_path(name); diff --git a/src/profile.rs b/src/profile.rs index d103b81..4406242 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -42,9 +42,9 @@ impl Kind pub struct Profile { pub name: String, pub kind: Kind, - /// Raw `-L/-R/-D` payload, without the flag itself - see spec §5.1. + /// Raw `-L/-R/-D` payload, without the flag itself. pub mapping: String, - /// Ordered hop list, each `[user@]host[:port]` - see spec §3.1. + /// Ordered hop list, each `[user@]host[:port]`. #[serde(default)] pub via: Vec, pub user: Option, @@ -132,8 +132,8 @@ impl Profile /// Splits `via` into the `-J` jump-chain value (comma-joined, all but /// the last hop; `None` for a single-hop `via`) and the final `ssh` - /// connection target - see spec §3.1. Every path that constructs a - /// `Profile` validates `via` as non-empty. + /// connection target. Every path that constructs a `Profile` validates + /// `via` as non-empty. pub fn ssh_target(&self) -> (Option, &str) { match self.via.split_last() { diff --git a/src/ssh.rs b/src/ssh.rs index 0667f34..e3c56d2 100644 --- a/src/ssh.rs +++ b/src/ssh.rs @@ -1,17 +1,17 @@ -//! Builds the `ssh` invocation for a profile - spec §3.1. +//! Builds the `ssh` invocation for a profile. use std::process::{ Stdio, Command }; use crate::profile::Profile; /// Builds the `ssh` command for `profile`, stdio wired for the supervisor -/// to capture (stdout/stderr piped so failure text can be classified per -/// spec §4.2; stdin from `/dev/null` since porthole never wants a shell). +/// to capture (stdout/stderr piped so failure text can be classified; +/// stdin from `/dev/null` since porthole never wants a shell). pub fn build(profile: &Profile) -> Command { let mut cmd = Command::new("ssh"); cmd.stdin(Stdio::null()).stdout(Stdio::piped()).stderr(Stdio::piped()); - // Forced flags, see spec §3.1. + // Flags forced on every invocation, not user-configurable. cmd.args([ "-o", "BatchMode=yes", diff --git a/src/supervisor.rs b/src/supervisor.rs index 1ac5f7d..54a54b2 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -1,4 +1,4 @@ -//! The `__supervise` loop - spec §3/§4. Runs as a detached, re-exec'd copy +//! The `__supervise` loop runs as a detached, re-exec'd copy //! of this same binary (`porthole __supervise `, see `main.rs`); owns //! the `ssh` child process for one profile's entire supervised lifetime. @@ -15,10 +15,10 @@ use crate::profile::{ self, Profile }; use crate::instance::{ self, Lock, State, Instance }; /// How long a connection must survive before its uptime resets the backoff -/// counter back to the base delay - spec §4.1. +/// counter back to the base delay. const STABLE_THRESHOLD_SECS: i64 = 60; /// Consecutive unrecognized (not pattern-matched) failures before porthole -/// gives up on an apparently-permanently-broken profile - spec §4.2. +/// gives up on an apparently-permanently-broken profile. const MAX_UNRECOGNIZED_STREAK: u32 = 10; /// How long `ssh` must stay alive before porthole treats it as connected; /// see `run_ssh_once` for the heuristic this backs. @@ -32,7 +32,7 @@ extern "C" fn handle_sigterm(_sig: libc::c_int) { SHUTDOWN.store(true, Ordering: /// Traps SIGTERM and SIGINT into a flag instead of the default /// terminate-immediately behavior. This is how `close` (SIGTERM) and -/// `-f/--foreground`'s Ctrl-C (SIGINT, spec §5.2) are distinguished from a +/// `-f/--foreground`'s Ctrl-C (SIGINT) are distinguished from a /// dropped `ssh` connection: by which signal arrived, not by inferring /// intent from `ssh`'s exit status. In foreground mode this function runs /// in the process the terminal sends Ctrl-C to directly, since @@ -215,7 +215,7 @@ fn join_all(handles: [Option>; N]) { } } -/// Classifies `ssh`'s captured stderr per spec §4.2. Fatal patterns stop +/// Classifies `ssh`'s captured stderr. Fatal patterns stop /// the reconnect loop outright; known-transient patterns retry without /// counting toward the unrecognized-failure escalation; anything else /// still retries, but does count toward it. diff --git a/tests/live_test.sh b/tests/live_test.sh index bb9ea51..06dcd2a 100755 --- a/tests/live_test.sh +++ b/tests/live_test.sh @@ -34,7 +34,7 @@ # ~/.config/porthole or ~/.local/state/porthole. # - Every profile created is named "$NAME_..." (default prefix # porttestsuite); no operation targets anything outside that prefix. -# - `wipe` (spec §5.8 / src/commands/wipe.rs) kills ANY process on the +# - `wipe` (src/commands/wipe.rs) kills ANY process on the # whole system whose cmdline contains "__supervise", regardless of # which state dir it belongs to - it is NOT scoped by the sandboxing # above. Before running it, this script scans the real process table