refactor(cli): centralize managed target preparation #94

2026-08-14T18:31:23Z

Lochnair commented

2026-08-14 18:31:23 +00:00

(Migrated from github.com)

Summary

Both client applets duplicated the whole step from parsed arguments to a runnable managed setup: target_specs → prepare_managed_targets, a ClientConfig built from a "primary" target, pacing.into(), and an identical ManagedClientConfig skeleton — each with its own copy of the MANAGED_EVENT_CAPACITY constant.
The primary-target step was the awkward part. ClientArgs::to_client_config() and TuiArgs::to_client_config() each hunted for the first positional-or-labelled target, expect-ed it to exist, and used its address — which the managed driver then replaced per target from ManagedTargetConfig::server_addr. The address was dead data, and the expect was a latent panic on a path the parser did not guarantee.
New shared::client::prepare does the step once. prepare_managed_run(common, duration, TargetSelection) validates the target set first, then returns a ManagedRunSetup with the validated targets, one target-free ClientConfig template, and the pacing, plus managed_config() / managed_targets() for handing to BlockingManagedClient.
CommonClientArgs::to_client_config no longer takes a server address, and both applets' to_client_config/target_specs/managed_targets/primary_target_addr collapse into one prepare().

Behavior

No user-visible argument spelling, default, help text, or error message changed.
Applet-specific semantics are preserved deliberately: duration stays applet-specific (client 10s, TUI 0/continuous) rather than being pulled into the shared group; the client still accepts no target so --list-columns works; the TUI still rejects an empty target list at parse time.
Two expect calls that would panic if a target were missing are gone; the empty-target case is a normal error from preparation.
The shared template's server_addr is now ClientConfig::default()'s value instead of an arbitrary target's. The managed driver overwrites it per target before validating, so no run observes the template value.

Scope

Explicitly not changed:

Protocol defaults and ClientConfig field semantics.
Argument spelling, per-applet help text, and per-applet requirement semantics — the clap declarations stay in each applet because their required_unless_present and long-help genuinely differ.
No CLI domain model was introduced: ManagedRunSetup is three fields and two accessors.
Output, TUI rendering, and managed driver behavior.

Verification

Run from the task worktree, all successful:

cargo fmt --all -- --check
cargo check --workspace --all-targets --all-features --locked
cargo test --workspace --all-targets --all-features --locked
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
cargo check -p irtt-cli --no-default-features --features client --all-targets --locked
cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked
cargo check -p irtt-cli --no-default-features --features server --all-targets --locked
git diff --check

New preparation tests cover single target, multiple targets in argument order, mixed positional and labelled targets, pacing mapping, the target-free shared template with a shared HMAC key and no per-target auth override, the empty-target error with its --list-columns hint, and duplicate-label rejection before any configuration is built. Existing applet tests now assert through prepare(), and the --list-columns and TUI target-requirement paths are asserted explicitly.

## Summary - Both client applets duplicated the whole step from parsed arguments to a runnable managed setup: `target_specs` → `prepare_managed_targets`, a `ClientConfig` built from a "primary" target, `pacing.into()`, and an identical `ManagedClientConfig` skeleton — each with its own copy of the `MANAGED_EVENT_CAPACITY` constant. - The primary-target step was the awkward part. `ClientArgs::to_client_config()` and `TuiArgs::to_client_config()` each hunted for the first positional-or-labelled target, `expect`-ed it to exist, and used its address — which the managed driver then replaced per target from `ManagedTargetConfig::server_addr`. The address was dead data, and the `expect` was a latent panic on a path the parser did not guarantee. - New `shared::client::prepare` does the step once. `prepare_managed_run(common, duration, TargetSelection)` validates the target set *first*, then returns a `ManagedRunSetup` with the validated targets, one target-free `ClientConfig` template, and the pacing, plus `managed_config()` / `managed_targets()` for handing to `BlockingManagedClient`. - `CommonClientArgs::to_client_config` no longer takes a server address, and both applets' `to_client_config`/`target_specs`/`managed_targets`/`primary_target_addr` collapse into one `prepare()`. ## Behavior - No user-visible argument spelling, default, help text, or error message changed. - Applet-specific semantics are preserved deliberately: duration stays applet-specific (client `10s`, TUI `0`/continuous) rather than being pulled into the shared group; the client still accepts no target so `--list-columns` works; the TUI still rejects an empty target list at parse time. - Two `expect` calls that would panic if a target were missing are gone; the empty-target case is a normal error from preparation. - The shared template's `server_addr` is now `ClientConfig::default()`'s value instead of an arbitrary target's. The managed driver overwrites it per target before validating, so no run observes the template value. ## Scope Explicitly not changed: - Protocol defaults and `ClientConfig` field semantics. - Argument spelling, per-applet help text, and per-applet requirement semantics — the clap declarations stay in each applet because their `required_unless_present` and long-help genuinely differ. - No CLI domain model was introduced: `ManagedRunSetup` is three fields and two accessors. - Output, TUI rendering, and managed driver behavior. ## Verification Run from the task worktree, all successful: - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked` - `cargo check -p irtt-cli --no-default-features --features client --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features server --all-targets --locked` - `git diff --check` New preparation tests cover single target, multiple targets in argument order, mixed positional and labelled targets, pacing mapping, the target-free shared template with a shared HMAC key and no per-target auth override, the empty-target error with its `--list-columns` hint, and duplicate-label rejection before any configuration is built. Existing applet tests now assert through `prepare()`, and the `--list-columns` and TUI target-requirement paths are asserted explicitly.

👍 1

chatgpt-codex-connector[bot] (Migrated from github.com) reviewed

2026-08-15 03:02:27 +00:00

chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2fcc97eb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `e2fcc97eb4` <details> <summary>ℹ️ About Codex in GitHub</summary> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>

crates/irtt-cli/src/cmd/client/args.rs

						
				@@ -123,4 +115,2 @@

				        })

				    }

				}

chatgpt-codex-connector[bot] (Migrated from github.com) commented

2026-08-15 03:02:27 +00:00

Retain compatibility for the replaced public argument APIs

For downstream crates using irtt-cli as a library, replacing these methods with prepare() removes the public ClientArgs::{to_client_config,target_specs,managed_targets} methods and target aliases; the same break occurs for TuiArgs, while CommonClientArgs::to_client_config changes arity. Those consumers will fail to compile on what is otherwise a behavior-preserving refactor, so retain compatibility wrappers and aliases—possibly deprecated—while adding the centralized preparation path.

AGENTS.md reference: AGENTS.md:L77-L77

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Retain compatibility for the replaced public argument APIs** For downstream crates using `irtt-cli` as a library, replacing these methods with `prepare()` removes the public `ClientArgs::{to_client_config,target_specs,managed_targets}` methods and target aliases; the same break occurs for `TuiArgs`, while `CommonClientArgs::to_client_config` changes arity. Those consumers will fail to compile on what is otherwise a behavior-preserving refactor, so retain compatibility wrappers and aliases—possibly deprecated—while adding the centralized preparation path. AGENTS.md reference: [AGENTS.md:L77-L77](https://github.com/Lochnair/irtt-rs/blob/e2fcc97eb4bf0feb1294297c94271ba0d113325d/AGENTS.md#L77-L77) Useful? React with 👍 / 👎.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lochnair-Labs/irtt-rs#94