Follow-up to the previous comment pass, which covered source comments -
this one is specifically the doc comments that render as --help output:
- Dropped "Deep-dive" (casual/jargon) from status's description in favor
of the imperative-verb pattern every other command description uses
(Save/Start/Stop/Update/List/Delete/Generate).
- Replaced the "SIGTERM + wait" shorthand in close --force's help with a
plain sentence - "+" as an ad-hoc conjunction doesn't belong in
user-facing text.
- Unified all positional NAME descriptions to the same "Name of the
profile to <verb>" shape (add's was "Name for the new profile",
breaking the pattern); open's picked up the same parenthetical-clarifier
convention already used elsewhere ("(ignored with --all)", matching
"(doesn't open it)", "(SOCKS proxy)").
- Trimmed the redundant "SSH" prefix from --via's description (the whole
tool is SSH-specific, so it added nothing); kept it on --port's, where
it disambiguates the SSH connection port from the forward's own port
numbers.
- Fixed the Cli struct's doc comment to match what --command(about)
actually renders (Cargo.toml's description, not the doc comment
verbatim) - it carried a stale "porthole - " prefix that never showed.
- -l/-r/-d now show their actual grammar as the clap value name
(<[BIND:]PORT:HOST:PORT>, <[BIND:]PORT>) instead of a generic <SPEC>,
matching vmic's convention of showing real syntax in --help. The
grammar is no longer also repeated in the flag's help text, since the
value name already carries it.
- --via changes from a single comma-separated flag to a repeatable one
(Vec<String> + value_delimiter = ','), so both `--via a --via b` and
`--via a,b` work and can be mixed. commands/mod.rs's edits_from_mapping
no longer needs to split the string itself - clap does it.
- Trimmed CLI help text that restated grammar/rationale already covered
by the value name or by --via being required.
- Normalized code comments for tone/format consistency: dropped
cross-references to vmic's own internals as justification (this
codebase should read as self-contained), removed markdown-style
*emphasis* asterisks that don't render in plain comments, tightened
several run-on sentences into plain declarative ones, and fixed one
comment in wipe.rs that inaccurately described close_instance's force
path (it sends SIGKILL, not SIGTERM).
- spec/porthole-spec.md's --via row updated to document the repeatable
form alongside comma-separated.
Cargo.toml/Cargo.lock/atomic.rs also carry an external toml crate bump
and formatting pass picked up from the working tree.
Ports vmic's print_full_help (main.rs) verbatim in spirit: one line per
subcommand with its positional args inline, an indented per-flag block
underneath with globally-aligned columns, a separate Aliases section, and
NO_COLOR-aware coloring - so `porthole`/`porthole -h`/`porthole help` show
every subcommand's flags without drilling into each one's own --help,
matching vmic's actual rendered output exactly in structure.
Also shortened --via's clap value_name from the full grammar
([user@]host[:port][,...]) to HOPS - the long form blew out the column
alignment for every other flag's help text; the full grammar is still in
the flag's help string itself.
Dropped vmic's MULTI_CHAR_ALIASES workaround (for short flags like -sv
that clap can't express natively) since porthole has no multi-char short
flags today - add it back if one shows up.
Full CLI per spec v0.2 - add/open/close/edit/status/list/remove/wipe/
completions, plus a hidden `__supervise` subcommand that IS the
supervisor process.
- profile.rs: TOML-backed profiles at ~/.config/porthole/profiles/,
validated -l/-r/-d mapping + --via grammar, atomic writes.
- instance.rs: JSON runtime state at ~/.local/state/porthole/, an
flock-based lock file that's the source of truth for "is this open"
(survives a crash/kill -9 without stale-lock cleanup), pid liveness
checked against /proc rather than trusted from disk.
- supervisor.rs: the __supervise loop - spawns ssh, traps SIGTERM/SIGINT
into a flag (rather than inferring intent from ssh's exit status),
classifies failures as fatal/known-transient/unrecognized, backs off
with a stability-reset, rotates its log.
- ssh.rs: builds the ssh invocation, including splitting --via into a
-J jump chain plus the mandatory positional target.
- open.rs: the detach/re-exec dance (setsid via pre_exec) and a bounded
wait for the supervisor to reach Up/Error before open returns, so an
immediate failure surfaces as a non-zero exit instead of a false
"opened" - this took a real bug fix during smoke testing, since the
instance file's initial state (Reconnecting, meaning "attempt in
flight") was indistinguishable from "already failed once" by state
alone.
- close.rs: SIGTERM+wait, or SIGKILL the whole process group with
--force so ssh can't be left orphaned.
Smoke-tested against invalid/unreachable hosts (no real infrastructure
touched): CLI surface, validation errors, add/edit/list/status/remove,
the reconnect/backoff loop with live state transitions, close mid-retry,
edit-while-running's warning, open --all, and wipe. cargo test: 14/14.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found both while translating the spec into code:
- `ssh -J a,b,c` alone isn't valid - ssh -J only carries jump hosts
before the final hop; it still needs a positional destination. --via's
last entry has to double as that target, which also makes --via
mandatory (there's no other field naming "the host ssh connects to"),
reversing the earlier "no" in the required column.
- A plain single-pid SIGKILL on `close --force` would leave the `ssh`
child orphaned, since a killed process can't forward anything to it.
Documents sending SIGKILL to the whole process group instead, relying
on the supervisor's own setsid() call making it the group leader.
Bumps to v0.2. Resolves the open questions and design gaps flagged during
review before implementation starts:
- Spells out the supervisor detach/re-exec mechanism (§3), mirroring how
vmic solves the same one-shot-CLI-can't-host-a-daemon problem.
- Forces BatchMode/ExitOnForwardFailure/ConnectTimeout on every ssh
invocation so headless failures surface instead of hanging on a TTY
prompt (§3.1).
- Adds backoff + stderr-based fatal/transient failure classification so
reconnect doesn't retry forever against a permanently broken profile
(§4).
- Collapses the Instance `state` enum to up/reconnecting/error, with
"no instance file" as the sole meaning of closed, removing the prior
ambiguity around close vs. crash vs. down (§2.2, §3).
- Fixes --via's conflicting comma-list-vs-repeated-flag examples by
committing to a straight ssh -J passthrough grammar (§3.1/§5.1).
- Resolves the edit --restart open question: no --restart, stays explicit.
- Simplifies exit codes to 0/1/2 (was a 6-code table with its own TODO).
- Adds a lock file for concurrent-open safety and a log rotation policy.
- Calls out reboot/logout survival as an explicit v0.1 non-goal, with
`open --all` as the only hook left for external autostart wiring (§8).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
