feat: [#336] add DNS resolution check to test command

docs/issues/336-add-dns-resolution-check-to-test-command.md

@@ -25,20 +25,24 @@ Add an optional DNS resolution check to the `test` command that verifies configu
 ### Module Structure Requirements
 
 - [ ] Follow DDD layer separation (see [docs/codebase-architecture.md](../docs/codebase-architecture.md))
-- [ ] DNS resolution logic in infrastructure layer
-- [ ] DNS check orchestration in application layer (as part of test steps)
+- [ ] DNS resolution logic in infrastructure layer (`src/infrastructure/dns/`)
+- [ ] DNS check orchestration and result types in application layer (`src/application/command_handlers/test/`)
+- [ ] DNS warning rendering in presentation layer (`src/presentation/controllers/test/`)
 - [ ] Use appropriate module organization (see [docs/contributing/module-organization.md](../docs/contributing/module-organization.md))
 
 ### Architectural Constraints
 
 - [ ] DNS resolution belongs in infrastructure layer (external system interaction)
-- [ ] Test command steps orchestrate DNS checks (application layer)
+- [ ] Test command handler returns structured `TestResult` with DNS warnings (application layer)
+- [ ] `TestCommandHandler` must NOT use `UserOutput` — return data, let presentation display it
+- [ ] Presentation controller renders DNS warnings from `TestResult` (follows `ListCommandHandler` → `EnvironmentList` pattern)
 - [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../docs/contributing/error-handling.md))
-- [ ] Output uses `UserOutput` methods (see [docs/contributing/output-handling.md](../docs/contributing/output-handling.md))
+- [ ] Output uses `UserOutput` methods only in presentation layer (see [docs/contributing/output-handling.md](../docs/contributing/output-handling.md))
 
 ### Anti-Patterns to Avoid
 
-- ❌ Using `println!` or direct stdout/stderr access (use `UserOutput`)
+- ❌ Using `println!` or direct stdout/stderr access (use `UserOutput` in presentation layer)
+- ❌ Passing `UserOutput` to command handlers in the application layer
 - ❌ Making DNS checks required/strict (should be advisory warnings)
 - ❌ Mixing DNS resolution logic in application layer
 - ❌ Failing infrastructure tests due to DNS issues
@@ -96,6 +100,53 @@ When DNS is correctly configured:
 
 3. **Only when domains configured** - Skip the check entirely if no domains are defined
 
+4. **Return structured result, don't use `UserOutput` in the application layer** - The
+   `TestCommandHandler` must return a structured `TestResult` type containing DNS warnings,
+   following the same pattern as `ListCommandHandler` which returns `EnvironmentList`. The
+   presentation layer is responsible for rendering the warnings to the user.
+
+   **Rationale**: In this project, "command handlers" handle user commands (not strictly CQRS
+   commands). Some are pure commands that modify state (`create`, `provision`), while others
+   are read-only queries (`list`, `show`, `test`). The `ListCommandHandler` already returns
+   a structured DTO (`EnvironmentList`) that the presentation layer renders — the test command
+   should follow this established pattern.
+
+   **Why not pass `UserOutput` to the handler?**
+   - `UserOutput` is a presentation concern — no other command handler uses it
+   - Passing it to the application layer breaks DDD layer separation
+   - It makes the handler harder to test (needs to mock output instead of asserting on return values)
+   - The handler should produce data; the controller should display it
+
+   **Implementation pattern** (consistent with `ListCommandHandler` → `EnvironmentList`):
+
+   ```rust
+   // Application layer: returns structured data
+   pub async fn execute(&self, env_name: &EnvironmentName)
+       -> Result<TestResult, TestCommandHandlerError>
+
+   // TestResult contains advisory DNS warnings
+   pub struct TestResult {
+       pub dns_warnings: Vec<DnsWarning>,
+   }
+
+   pub struct DnsWarning {
+       pub domain: DomainName,
+       pub expected_ip: IpAddr,
+       pub issue: DnsIssue,
+   }
+
+   pub enum DnsIssue {
+       ResolutionFailed(String),
+       IpMismatch { resolved_ips: Vec<IpAddr> },
+   }
+
+   // Presentation layer: renders the warnings
+   let result = handler.execute(&env_name).await?;
+   for warning in &result.dns_warnings {
+       self.progress.output().lock().borrow_mut().warn(&format!("DNS check: {warning}"));
+   }
+   ```
+
 ### Implementation Details
 
 - Use system DNS resolution (not internal resolution)
@@ -119,13 +170,14 @@ When DNS is correctly configured:
 - [ ] Add error types for DNS resolution failures
 - [ ] Add unit tests for DNS resolver
 
-### Phase 2: Add DNS Check Step (2-3 hours)
+### Phase 2: Add DNS Check to Test Command Handler (2-3 hours)
 
-- [ ] Add DNS check logic to test command steps in `src/application/steps/test/`
-- [ ] Extract domain names from environment configuration (trackers, API, Grafana, etc.)
-- [ ] Compare resolved IPs with expected instance IP
-- [ ] Format output using `UserOutput` (warnings, not errors)
-- [ ] Handle cases where no domains are configured (skip check)
+- [ ] Add `TestResult`, `DnsWarning`, `DnsIssue` types to `src/application/command_handlers/test/`
+- [ ] Change `TestCommandHandler.execute()` return type from `Result<(), ...>` to `Result<TestResult, ...>`
+- [ ] Add DNS check logic in handler: extract domains from tracker config, use `DnsResolver`, collect warnings
+- [ ] Handle cases where no domains are configured (return empty warnings)
+- [ ] Update presentation controller to render DNS warnings from `TestResult`
+- [ ] Remove any `UserOutput` parameter from `TestCommandHandler.execute()`
 
 ### Phase 3: Integration and Testing (1-2 hours)
 

src/application/command_handlers/common/endpoint_builder.rs

@@ -0,0 +1,128 @@
+//! Service endpoint builder utilities
+//!
+//! This module provides utilities for constructing `ServiceEndpoint` objects
+//! from domain configuration types and runtime deployment data (like instance IPs).
+//!
+//! These builders are used by command handlers that need to create endpoints for
+//! external validation, testing, or other operations that require service URLs.
+//!
+//! ## Design Rationale
+//!
+//! These functions exist in the application layer (not domain) because:
+//! - They combine domain configuration with runtime deployment state
+//! - Domain configuration types shouldn't know about deployment IPs
+//! - Multiple command handlers may need to build endpoints
+//! - They translate domain types → infrastructure types (`ServiceEndpoint`)
+
+use std::net::{IpAddr, SocketAddr};
+
+use crate::domain::tracker::config::{HttpApiConfig, HttpTrackerConfig, TrackerConfig};
+use crate::shared::ServiceEndpoint;
+
+/// Build a `ServiceEndpoint` for the HTTP API from configuration and instance IP
+///
+/// Creates either an HTTP or HTTPS endpoint depending on whether TLS is enabled
+/// in the configuration. For TLS endpoints, the domain is used with the instance
+/// IP for local resolution (no DNS dependency).
+///
+/// # Arguments
+///
+/// * `instance_ip` - The IP address of the deployed instance
+/// * `config` - The HTTP API configuration containing port and TLS settings
+///
+/// # Returns
+///
+/// A `ServiceEndpoint` configured for the HTTP API health check endpoint.
+///
+/// # Panics
+///
+/// Panics if the configuration produces an invalid URL (this should never happen
+/// with valid configuration types from the domain layer).
+#[must_use]
+pub fn build_api_endpoint(instance_ip: IpAddr, config: &HttpApiConfig) -> ServiceEndpoint {
+    let port = config.bind_address().port();
+    let path = "/api/health_check";
+    let socket_addr = SocketAddr::new(instance_ip, port);
+
+    if let Some(domain) = config.tls_domain() {
+        ServiceEndpoint::https(domain, path, instance_ip)
+            .expect("Valid TLS domain should produce valid HTTPS URL")
+    } else {
+        ServiceEndpoint::http(socket_addr, path)
+            .expect("Valid socket address should produce valid HTTP URL")
+    }
+}
+
+/// Build a `ServiceEndpoint` for an HTTP Tracker from configuration and instance IP
+///
+/// Creates either an HTTP or HTTPS endpoint depending on whether TLS is enabled
+/// in the configuration. For TLS endpoints, the domain is used with the instance
+/// IP for local resolution (no DNS dependency).
+///
+/// # Arguments
+///
+/// * `instance_ip` - The IP address of the deployed instance
+/// * `config` - The HTTP Tracker configuration containing port and TLS settings
+///
+/// # Returns
+///
+/// A `ServiceEndpoint` configured for the HTTP Tracker health check endpoint.
+///
+/// # Panics
+///
+/// Panics if the configuration produces an invalid URL (this should never happen
+/// with valid configuration types from the domain layer).
+#[must_use]
+pub fn build_http_tracker_endpoint(
+    instance_ip: IpAddr,
+    config: &HttpTrackerConfig,
+) -> ServiceEndpoint {
+    let port = config.bind_address().port();
+    let path = "/health_check";
+    let socket_addr = SocketAddr::new(instance_ip, port);
+
+    if let Some(domain) = config.tls_domain() {
+        ServiceEndpoint::https(domain, path, instance_ip)
+            .expect("Valid TLS domain should produce valid HTTPS URL")
+    } else {
+        ServiceEndpoint::http(socket_addr, path)
+            .expect("Valid socket address should produce valid HTTP URL")
+    }
+}
+
+/// Build all tracker service endpoints from configuration and instance IP
+///
+/// This is a convenience function that builds both the HTTP API endpoint and
+/// all HTTP Tracker endpoints in a single call. It's the recommended way to
+/// construct endpoints when you need all tracker services.
+///
+/// # Arguments
+///
+/// * `instance_ip` - The IP address of the deployed instance
+/// * `tracker_config` - The complete tracker configuration
+///
+/// # Returns
+///
+/// A tuple containing:
+/// - The HTTP API `ServiceEndpoint`
+/// - A vector of HTTP Tracker `ServiceEndpoint`s (one per configured tracker)
+///
+/// # Panics
+///
+/// Panics if any configuration produces an invalid URL (this should never happen
+/// with valid configuration types from the domain layer).
+#[must_use]
+pub fn build_all_tracker_endpoints(
+    instance_ip: IpAddr,
+    tracker_config: &TrackerConfig,
+) -> (ServiceEndpoint, Vec<ServiceEndpoint>) {
+    let api_endpoint = build_api_endpoint(instance_ip, tracker_config.http_api());
+
+    let http_tracker_endpoints = tracker_config
+        .http_trackers()
+        .iter()
+        .map(|config| build_http_tracker_endpoint(instance_ip, config))
+        .collect();
+
+    (api_endpoint, http_tracker_endpoints)
+}

src/application/command_handlers/common/mod.rs

@@ -3,6 +3,7 @@
 //! This module provides shared functionality used across multiple command handlers
 //! to reduce code duplication and improve maintainability.
 
+pub mod endpoint_builder;
 pub mod failure_context;
 
 /// Result type for step execution in command handlers