Swapped " - " for a semicolon where it joined two independent clauses,
or a comma where the following text was an appositive/dependent phrase
with no subject of its own. Markdown-style list bullets in
tests/live_test.sh's header (leading "- Item") are unaffected - those
are structural, not sentence punctuation.
Also dropped a "Spec §5.4:" citation in edit.rs missed by the earlier
spec-citation cleanup (case-sensitive grep at the time didn't match the
capitalized "Spec").
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Mirrors vmic's tests/live_test.sh convention (colored PASS/FAIL/SKIP,
section headers, an EXIT trap that always cleans up, safety scoped to
test-prefixed names) but adapted for porthole: whole run sandboxed under
a temp PORTHOLE_STATE_DIR_OVERRIDE so it never touches the real
~/.config/porthole or ~/.local/state/porthole.
15 phases covering every mapping kind (-l/-r/-d) against the real
server, a real 2-hop -J ProxyJump (self-jump - only one server is
available), real auth success/failure classification (not just the
synthetic DNS-failure cases cargo test covers), open --all/--once,
close --force vs. graceful vs. idempotent, edit-while-running, the
remove --keep-running orphan path, a transfer export/import round trip
that actually reopens the imported profile, and a guarded wipe phase
that verifies kill_orphaned_supervisors for real.
wipe kills any __supervise process system-wide by design (not scoped to
the sandboxed state dir), so before running it the script scans the
real process table and skips the phase entirely if it finds a live
supervisor that isn't one of its own test profiles - a real tunnel
left open elsewhere is never killed as a side effect of running this
suite.
Verified the CLI-surface and error-path phases (no server needed)
directly against the built binary; phases requiring the real server
are left for a manual run, per the established pattern in this project
of live-server tests being run by hand rather than automated in CI.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The custom --help renderer showed every positional as <name> regardless
of whether it was actually required, which was misleading for open and
transfer (both take an optional profile name). positional_args() now
checks Arg::is_required_set() and renders [name] for an optional one.
transfer's name positional previously only worked with --export and was
rejected outright when combined with --import. It now filters --import
the same way: 'porthole transfer <name> -i file.toml' imports just that
one profile out of the file instead of everything in it, erroring with
a new TransferProfileNotFound if the file doesn't contain it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- --help now shows <PATH.toml> instead of the format-agnostic <PATH>.
- Export warns (naming the affected profiles) that identity files
aren't included, only their local paths; import warns per-profile
when an identity path doesn't resolve on the importing machine,
expanding a leading ~/ the same way ssh.rs does so that check isn't
a false positive for tilde paths.
- transfer now takes an optional profile-name positional (same shape
as OpenArgs.name) so -e/--export can target a single profile instead
of always dumping every saved one; combining it with -i/--import is
rejected with a new TransferNameWithImport error.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
-e/--export writes every saved profile to a single TOML file as a
[[profile]] array; -i/--import reads one back and saves each entry,
failing on the first name collision rather than silently overwriting.
Reuses Profile's existing Serialize/Deserialize impl directly, so the
file format is just the same shape already written to
~/.config/porthole/profiles/<name>.toml, aggregated.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BatchMode=yes already blocked TTY prompts, but a brand-new host with no
known_hosts entry failed outright on first connect since ssh had no way
to ask for acceptance. Add StrictHostKeyChecking=accept-new (TOFU, still
hard-fails on a changed known host) plus LogLevel=ERROR to keep the
resulting "permanently added" notice out of profile logs.
Also force ControlMaster=no/ControlPath=none and ClearAllForwardings=yes
so a user's own ~/.ssh/config can't make porthole's spawned ssh share a
multiplexed connection or apply extra forwards - the supervisor's
process-based tracking assumes one spawned ssh exclusively owns one
tunnel. ServerAliveCountMax=3 makes dead-connection detection time
deterministic against the profile's keepalive, and -T is explicit
no-pty insurance alongside the existing -N. IdentitiesOnly=yes is added
whenever a profile sets an identity file, avoiding auth-failure lockouts
from also offering agent/default keys.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
