Skip to content

Show help for a bare ts dev proxy invocation #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dhruv8sh wants to merge 3 commits into
base: main
Choose a base branch
from
+43 −13
Draft

Show help for a bare ts dev proxy invocation #1167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1476137
Show help for a bare ts dev proxy invocation
dhruv8sh Sep 15, 2026
ae1d7c6
Address PR review: preserve CA error attachments, fix test process-ex…
dhruv8sh Sep 15, 2026
85ac1ff
Move test-only clap import to module scope, harden second parse trap
dhruv8sh Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions crates/trusted-server-cli/src/commands/dev/proxy/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ fn resolve_basic_auth(args: &ProxyArgs) -> Result<Option<BasicAuth>, ConfigError

#[cfg(test)]
mod tests {
use clap::Parser as _;
use hyper::header::HeaderValue;
use rustls::pki_types::ServerName;

Expand All @@ -359,30 +360,26 @@ mod tests {
};

fn base_args() -> crate::commands::dev::proxy::ProxyArgs {
// Construct via clap so defaults match the real surface.
use clap::Parser;
#[derive(clap::Parser)]
struct W {
#[command(flatten)]
a: crate::commands::dev::proxy::ProxyArgs,
}
W::parse_from(["ts"]).a
parse_args(&[
"ts",
"--listen",
crate::commands::dev::proxy::DEFAULT_LISTEN,
])
}

fn parse_args(argv: &[&str]) -> crate::commands::dev::proxy::ProxyArgs {

@prk-Jr prk-Jr Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, but worth fixing while you are here. parse_args uses Parser::parse_from, and arg_required_else_help now applies to every Command that ProxyArgs is flattened into — including this test wrapper. On the help short-circuit, parse_from calls Error::exit(), i.e. std::process::exit(2). That does not fail one test; it aborts the whole test binary with no test name and no attribution:

error: test failed ... process didn't exit successfully: ... (exit status: 2)
note: test exited abnormally

Reproduced by adding a defaults-only parse_args(&["ts"]) call. This PR works around the two existing call sites by passing --listen, but the trap stays armed for whoever writes the next defaults-only test. Switching to the fallible parser turns it back into an ordinary test failure:

        W::try_parse_from(argv).expect("should parse proxy args").a

Verified: compiles, cargo fmt --check clean, all 17 config tests pass.

crates/trusted-server-cli/tests/support/mod.rs:207 has the identical Wrapper::parse_from shape. All its current call sites pass arguments, so it is not live today, but it is the same latent trap.

@dhruv8sh dhruv8sh Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ae1d7c6 — switched parse_args to try_parse_from(...).expect("should parse proxy args"). Verified the exit trap is real: a defaults-only parse_args(&["ts"]) call aborts the whole test binary with parse_from, and becomes an ordinary panic/failure with try_parse_from. Left tests/support/mod.rs:207 alone since it's genuinely out of scope here (no live call site), but noted it for whoever touches that next.

use clap::Parser;
#[derive(clap::Parser)]
struct W {
#[command(flatten)]
a: crate::commands::dev::proxy::ProxyArgs,
}
W::parse_from(argv).a
W::try_parse_from(argv).expect("should parse proxy args").a
}

#[test]
fn clap_parses_rewrite_host_as_a_bool() {
assert!(
!parse_args(&["ts"]).rewrite_host,
!parse_args(&["ts", "--insecure"]).rewrite_host,
"absent --rewrite-host is false"
);
assert!(
Expand Down
7 changes: 6 additions & 1 deletion crates/trusted-server-cli/src/commands/dev/proxy/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,13 @@ async fn finish_interrupted_run<Restore, Stop, Drain>(
let _ = tokio::time::timeout(std::time::Duration::from_secs(2), drain_manager).await;
}

/// Default `--listen` address, shared with the `config` tests so they cannot
/// silently drift from the real default.
pub const DEFAULT_LISTEN: &str = "127.0.0.1:18080";

/// `ts dev proxy [OPTIONS]` — see the design spec §4.
#[derive(Debug, clap::Args)]
#[command(arg_required_else_help = true)]
Comment thread
dhruv8sh marked this conversation as resolved.
pub struct ProxyArgs {
/// Rewrite rule `FROM=TO` (repeatable).
#[arg(long = "map", value_name = "FROM=TO")]
Expand All @@ -94,7 +99,7 @@ pub struct ProxyArgs {
pub to: Option<String>,

/// Proxy listen address. Non-loopback requires `--allow-non-loopback`.
#[arg(long, value_name = "ADDR", default_value = "127.0.0.1:18080")]
#[arg(long, value_name = "ADDR", default_value = DEFAULT_LISTEN)]
pub listen: String,

/// Permit binding a non-loopback `--listen` (disables blind tunnel/forward).
Expand Down
28 changes: 28 additions & 0 deletions crates/trusted-server-cli/src/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,4 +678,32 @@ mod tests {
"error should explain unsupported option"
);
}

#[test]
#[cfg(target_os = "macos")]
fn dev_proxy_bare_invocation_shows_help_before_running() {
let error = Args::try_parse_from(["ts", "dev", "proxy"])
.expect_err("a bare `ts dev proxy` should short-circuit to help, not run");
assert_eq!(
error.kind(),
clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand,
"should print help instead of touching system proxy state or attempting sudo"
);
}

#[test]
#[cfg(target_os = "macos")]
fn dev_proxy_ca_subcommands_still_parse_under_arg_required_else_help() {
for action in ["path", "install", "uninstall", "regenerate"] {
parse(&["ts", "dev", "proxy", "ca", action]);
}
}

#[test]
#[cfg(target_os = "macos")]
fn dev_proxy_partial_rule_parses_instead_of_showing_help() {
// An explicit but incomplete rule (`--from` with no `--to`) must reach
// `run` and surface the concise no-rule error there, not clap help.
parse(&["ts", "dev", "proxy", "--from", "a.example.com"]);
}
}
2 changes: 1 addition & 1 deletion crates/trusted-server-cli/tests/support/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ fn resolve(argv: &[&str]) -> config::ResolvedConfig {
#[command(flatten)]
args: trusted_server_cli::commands::dev::proxy::ProxyArgs,
}
let parsed = Wrapper::parse_from(argv);
let parsed = Wrapper::try_parse_from(argv).expect("should parse proxy args");
config::resolve(&parsed.args).expect("should resolve test config")
}

Expand Down
Loading