diff --git a/src/atomic.rs b/src/atomic.rs index f00c406..708f9bd 100644 --- a/src/atomic.rs +++ b/src/atomic.rs @@ -1,7 +1,7 @@ //! Atomic file writes: write to a sibling temp file, then `rename` over the //! target. Matters here specifically for the instance JSON file, which the //! supervisor rewrites on every state change while it may be alive for -//! months - a reader (`status`/`list`) must never observe a half-written +//! months; a reader (`status`/`list`) must never observe a half-written //! file, and a crash mid-write must never corrupt the last-known-good state. use std::io::Write; diff --git a/src/cli.rs b/src/cli.rs index 68fb4ae..9ee444b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -48,12 +48,12 @@ pub enum Commands { Completions { shell: Shell }, /// Internal: runs the supervisor loop for one profile. Not for direct - /// use - `open` spawns this itself. + /// use; `open` spawns this itself. #[command(hide = true, name = "__supervise")] Supervise { name: String }, } -/// Shared mapping/connection flags for `add` and `edit` - kept as one +/// Shared mapping/connection flags for `add` and `edit`, kept as one /// struct (`#[command(flatten)]`ed into both) so the two can never drift. #[derive(Args, Default)] pub struct MappingArgs { diff --git a/src/commands/close.rs b/src/commands/close.rs index 4ca44d3..fd5cd8d 100644 --- a/src/commands/close.rs +++ b/src/commands/close.rs @@ -18,7 +18,7 @@ pub fn run(args: CloseArgs) -> Result<()> { } /// Stops `name`'s supervisor if one is actually running, and clears any -/// stale instance file either way - shared with `remove` and `wipe`. +/// 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 { @@ -28,7 +28,7 @@ pub fn close_instance(name: &str, force: bool) -> Result { if force { // SIGKILL the whole process group (the supervisor is its own - // group leader via setsid), not just the supervisor pid - a plain + // 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 { diff --git a/src/commands/edit.rs b/src/commands/edit.rs index d1046e9..20841c6 100644 --- a/src/commands/edit.rs +++ b/src/commands/edit.rs @@ -19,7 +19,7 @@ pub fn run(args: EditArgs) -> Result<()> { p.apply_edits(&edits)?; profile::save(&p)?; - // Spec ยง5.4: edit never restarts a running instance - just warn. + // edit never restarts a running instance, just warn. if instance::running_pid(&name)?.is_some() { ui::warn(&format!( "'{name}' is currently open; this change won't take effect until the next open/close cycle." diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 76ee52d..92491bd 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -34,7 +34,7 @@ pub fn edits_from_mapping(m: &MappingArgs) -> ProfileEdits { } } -/// `true` if `MappingArgs` carries no edits at all - used by `edit` to +/// `true` if `MappingArgs` carries no edits at all, used by `edit` to /// reject a no-op invocation. pub fn mapping_is_empty(m: &MappingArgs) -> bool { m.local.is_none() diff --git a/src/commands/open.rs b/src/commands/open.rs index 7af4032..1cd4ff0 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -24,7 +24,7 @@ pub fn run(args: OpenArgs) -> Result<()> { open_one(&name, args.foreground, args.once) } -/// Opens every `reconnect: true` profile that isn't already running - the +/// Opens every `reconnect: true` profile that isn't already running, the /// hook external autostart mechanisms are meant to call. Per-profile /// failures are warnings, not a whole-batch failure. fn open_all(once: bool) -> Result<()> { @@ -57,7 +57,7 @@ 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" - this just + // lock, not this file, is the authority on "already open"; this just // keeps `status` from reading stale state mid-spawn. instance::delete(name)?; diff --git a/src/commands/status.rs b/src/commands/status.rs index de92b7a..53ac5f4 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -10,7 +10,7 @@ pub fn run(args: StatusArgs) -> Result<()> { let p = profile::load(&name)?; 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 + // crashed without cleaning up; report that, rather than trusting a // state the process table disagrees with. let live = inst.as_ref().is_some_and(|i| instance::supervisor_alive(i.pid, &name)); diff --git a/src/commands/transfer.rs b/src/commands/transfer.rs index 271fa46..62cd8ff 100644 --- a/src/commands/transfer.rs +++ b/src/commands/transfer.rs @@ -1,4 +1,4 @@ -//! Bulk profile backup/restore - a flat TOML array of the same `Profile` +//! Bulk profile backup/restore, a flat TOML array of the same `Profile` //! records `profile::save`/`load` already read and write, so it round-trips //! through the exact same serialization with nothing profile-specific here. @@ -78,7 +78,7 @@ fn import(path: &str, name: Option<&str>) -> Result<()> { Ok(()) } -/// Identity files are never included in the export, only the path - warn so +/// Identity files are never included in the export, only the path; warn so /// that doesn't come as a surprise on the importing end. fn warn_about_identities(profiles: &[Profile]) { let names: Vec<&str> = profiles.iter().filter(|p| p.identity.is_some()).map(|p| p.name.as_str()).collect(); @@ -91,7 +91,7 @@ fn warn_about_identities(profiles: &[Profile]) { } /// After import, flag any profile whose identity path doesn't resolve on -/// this machine - the most likely sign of a not-yet-copied key file. +/// this machine, the most likely sign of a not-yet-copied key file. fn warn_about_missing_identities(profiles: &[Profile]) { for p in profiles { if let Some(identity) = &p.identity { @@ -107,7 +107,7 @@ fn warn_about_missing_identities(profiles: &[Profile]) { } /// Expands a leading `~/` the same way `ssh` itself does at spawn time -/// (`ssh.rs`) - without this, a valid `~/...` identity path would be +/// (`ssh.rs`); without this, a valid `~/...` identity path would be /// misreported as missing since `Path::is_file` never expands `~` on its own. fn expand_home(path: &str) -> PathBuf { match path.strip_prefix("~/").zip(dirs::home_dir()) { diff --git a/src/instance.rs b/src/instance.rs index 851f0b4..695189f 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -111,7 +111,7 @@ 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 - its +/// 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> { @@ -146,7 +146,7 @@ pub fn delete(name: &str) -> Result<()> pub fn process_alive(pid: i32) -> bool { unsafe { libc::kill(pid, 0) == 0 } } /// True only if `pid` is a live process whose cmdline identifies it as the -/// supervisor for `name` - guards against a stale or reused pid. +/// supervisor for `name`, guards against a stale or reused pid. pub fn supervisor_alive(pid: i32, name: &str) -> bool { if !process_alive(pid) { return false }; @@ -159,7 +159,7 @@ pub fn supervisor_alive(pid: i32, name: &str) -> bool /// The live supervisor pid for `name`, if one is currently running /// (checked against the process table, not just the instance file's -/// last-known value - so a crash or reboot is detected as "not running" +/// last-known value, so a crash or reboot is detected as "not running" /// rather than trusting stale on-disk state). pub fn running_pid(name: &str) -> Result> { diff --git a/src/main.rs b/src/main.rs index 5ce62f9..0ad60ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,8 +53,8 @@ fn main() } } -/// True for a bare `porthole` invocation, or top-level `-h`/`--help`/`help` -/// - i.e. anything that should show the expanded help rather than being +/// True for a bare `porthole` invocation, or top-level `-h`/`--help`/`help`, +/// i.e. anything that should show the expanded help rather than being /// handled (or rejected) by a specific subcommand. fn wants_top_level_help(args: &[String]) -> bool { match args.get(1..) { diff --git a/tests/live_test.sh b/tests/live_test.sh index 06dcd2a..6c5f384 100755 --- a/tests/live_test.sh +++ b/tests/live_test.sh @@ -5,7 +5,7 @@ # auth (success and failure), a real two-hop ProxyJump, a real SOCKS proxy # carrying real traffic. Deliberately NOT part of `cargo test` (same reason # as vmic's tests/live_test.sh: this needs a real remote server, not a CI -# sandbox) - live-only, opt-in, run by hand. +# sandbox), live-only, opt-in, run by hand. # # Usage: # tests/live_test.sh [options] @@ -22,21 +22,21 @@ # -h, --help # # Known limitation: only one real server is available, so multi-hop (`-J`) -# is tested by chaining the server through itself (--via user@host,user@host) -# - a real two-hop ProxyJump handshake, just with both hops the same box. +# is tested by chaining the server through itself (--via user@host,user@host), +# a real two-hop ProxyJump handshake, just with both hops the same box. # There is no way to test a genuine distinct-host chain without a second # server. # # Safety: # - Profiles/instances/locks/logs are sandboxed for the whole run under # one `PORTHOLE_STATE_DIR_OVERRIDE` temp dir (profile.rs/instance.rs -# both honor it) - this suite NEVER touches the real +# both honor it); this suite NEVER touches the real # ~/.config/porthole or ~/.local/state/porthole. # - Every profile created is named "$NAME_..." (default prefix # porttestsuite); no operation targets anything outside that prefix. # - `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 +# which state dir it belongs to; it is NOT scoped by the sandboxing # above. Before running it, this script scans the real process table # and skips the wipe phase entirely (not "wipe only the safe parts") # if it finds a live __supervise process that isn't one of this run's @@ -48,10 +48,10 @@ # never writes to your real ~/.ssh/known_hosts (only porthole's own # spawned ssh does, against the real file, using the accept-new policy # already forced in src/ssh.rs). -# - An EXIT trap always attempts full cleanup - closes/removes every +# - An EXIT trap always attempts full cleanup, closes/removes every # test-prefixed profile in every sandbox dir used, force-kills any # stray matching __supervise process, deletes the throwaway bad-auth -# key - even on failure or Ctrl-C. +# key, even on failure or Ctrl-C. set -uo pipefail cd "$(dirname "${BASH_SOURCE[0]}")/.." @@ -117,7 +117,7 @@ p_run() { LAST_OUT="$(PORTHOLE_STATE_DIR_OVERRIDE="$1" "$BIN" "${@:2}" 2>&1)"; L # status --json's "state" field is always one of closed/up/reconnecting/error # (print_json normalizes a dead-supervisor instance file to "error" too, see -# status.rs) - polling that key is far more robust than scraping the padded +# status.rs); polling that key is far more robust than scraping the padded # human-readable field. json_field() { # json_field -> value, empty if absent/no instance p "$1" status "$2" --json 2>/dev/null | sed -n "s/.*\"$3\": \"\\{0,1\\}\\([^\",]*\\)\"\\{0,1\\},\\{0,1\\}\$/\\1/p" | head -1 @@ -140,7 +140,7 @@ wait_port_closed() { # wait_port_closed [tries, x0.5s] } SSH_PROBE_OPTS=(-o BatchMode=yes -o ConnectTimeout=5 -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=/dev/null) -# mktemp's output is captured via $(...), which forks a subshell - any +# mktemp's output is captured via $(...), which forks a subshell; any # array append done *inside* a function called that way would be lost when # the subshell exits, so state dirs are appended here at the call site # instead of through a helper function. @@ -408,7 +408,7 @@ expect_contains "warns it's left running untracked" "left running untracked" p_run "$STATE_DIR" status "${NAME}_keep"; assert_eq "profile is gone from tracking" "$LAST_CODE" "1" assert_true "the untracked process is still actually alive" pgrep -f "__supervise ${NAME}_keep\$" assert_true "port $KEEP_PORT is still live, untracked" port_open "$KEEP_PORT" -# left running on purpose - phase 15's wipe is what's being tested against it +# left running on purpose; phase 15's wipe is what's being tested against it # --------------------------------------------------------------------------- section "Phase 14: transfer round trip against a live profile"