refactor(managed): remove legacy runtime #52

2026-08-09T14:01:20Z

Lochnair commented

2026-08-09 14:01:20 +00:00

(Migrated from github.com)

No description provided.

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

2026-08-09 14:08:18 +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: c599c84715

ℹ️ 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:** `c599c84715` <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/run.rs

						
				@@ -673,2 +440,4 @@

				        ManagedTargetFailure, ManagedTargetFailureKind, ManagedTargetFailurePhase, TargetId,

				    };

				    use std::sync::Arc;

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

2026-08-09 14:08:18 +00:00

Bound event draining before checking for shutdown

When probes are produced faster than stdout and statistics processing can consume them, this loop need not ever observe Empty: a continuously advancing broadcast channel can repeatedly return events or Lagged. Because the outer loop checks Ctrl-C, peer closure, and final status only after this drain returns, a high-rate run or slow output sink can become indefinitely unresponsive to shutdown; the analogous TUI drain also starves input and rendering. Process a bounded batch per outer iteration so control checks remain reachable.

Useful? React with 👍 / 👎.

**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Bound event draining before checking for shutdown** When probes are produced faster than stdout and statistics processing can consume them, this loop need not ever observe `Empty`: a continuously advancing broadcast channel can repeatedly return events or `Lagged`. Because the outer loop checks Ctrl-C, peer closure, and final status only after this drain returns, a high-rate run or slow output sink can become indefinitely unresponsive to shutdown; the analogous TUI drain also starves input and rendering. Process a bounded batch per outer iteration so control checks remain reachable. Useful? React with 👍 / 👎.

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

						
				@@ -691,0 +465,4 @@

				            end_reason: ManagedTargetEndReason::Failed(ManagedTargetFailure {

				                phase: ManagedTargetFailurePhase::Receiving,

				                kind: ManagedTargetFailureKind::Protocol,

				                message: "primary failure".into(),

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

2026-08-09 14:08:18 +00:00

Report cleanup failures on otherwise successful outcomes

When a finite test completes but the subsequent drain or close fails, the unified runtime preserves TestComplete as the primary end reason and records the error in cleanup_failure; this function only examines end_reason, so the CLI exits successfully without any diagnostic about the failed close. The TUI outcome handling similarly ignores this field, so both frontends should surface cleanup_failure even when the primary outcome remains successful.

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Report cleanup failures on otherwise successful outcomes** When a finite test completes but the subsequent drain or close fails, the unified runtime preserves `TestComplete` as the primary end reason and records the error in `cleanup_failure`; this function only examines `end_reason`, so the CLI exits successfully without any diagnostic about the failed close. The TUI outcome handling similarly ignores this field, so both frontends should surface `cleanup_failure` even when the primary outcome remains successful. Useful? React with 👍 / 👎.

crates/irtt-cli/src/cmd/tui/ui.rs

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

2026-08-09 14:08:18 +00:00

Re-enable the TUI unit-test module

cfg(any()) is always false, so all 45 TUI tests below this attribute are silently excluded, including coverage for the target outcome and status mappings changed in this commit. An all-features test run consequently executes no cmd::tui::ui tests; update the fixtures for the unified managed API rather than disabling the entire module.

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Re-enable the TUI unit-test module** `cfg(any())` is always false, so all 45 TUI tests below this attribute are silently excluded, including coverage for the target outcome and status mappings changed in this commit. An all-features test run consequently executes no `cmd::tui::ui` tests; update the fixtures for the unified managed API rather than disabling the entire module. Useful? React with 👍 / 👎.

crates/irtt-client/src/lib.rs

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

2026-08-09 14:08:18 +00:00

Update documentation for the removed legacy exports

After these root-level legacy exports are removed, the library documentation still directs users to crate::ManagedClient, which makes RUSTDOCFLAGS='-D warnings' cargo doc -p irtt-client --all-features --no-deps fail, while README.md:179-242 still contains a managed-client example and guidance using ManagedClient, SubscriberConfig, ManagedClientGroup, and EventSubscription. Users following the published library instructions therefore receive uncompilable code; migrate those references to the new managed API and enable the required tokio feature.

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Update documentation for the removed legacy exports** After these root-level legacy exports are removed, the library documentation still directs users to `crate::ManagedClient`, which makes `RUSTDOCFLAGS='-D warnings' cargo doc -p irtt-client --all-features --no-deps` fail, while README.md:179-242 still contains a managed-client example and guidance using `ManagedClient`, `SubscriberConfig`, `ManagedClientGroup`, and `EventSubscription`. Users following the published library instructions therefore receive uncompilable code; migrate those references to the new `managed` API and enable the required `tokio` feature. 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#52