test: add unit tests for buildRetryLimit and RetryOptions.logger

**Added:**

- Added unit tests for buildRetryLimit covering all input branches and for RetryOptions.logger fallback logic in cli/internal/ansible/retry_test.go

**Changed:**

- Updated .gitignore to exclude coverage.out
- Marked mutually exclusive CLI flags for amiBuildCmd ("all", "instance-type") and provisionCmd ("plays", "from") for improved UX
- Added package and function-level documentation for several files to clarify their purpose and usage, including cli/internal/ansible/errors.go, retry.go, cli/internal/labmap/labmap.go, and cli/internal/validate/checks.go and validator.go
- Improved CheckAnsibleSuccess docstring for clarity and refactored logic for readability in cli/internal/ansible/logparser.go
- Enhanced docstrings in cli/internal/ansible/retry.go for clarity around retry logic and SSM session cleanup
- Fixed json struct tag for Result.Detail to use omitempty in cli/internal/validate/validator.go
- Corrected merging of stderr into output stream in RunPlaybook by using multiW instead of cmd.Stdout in cli/internal/ansible/runner.go
- Removed unused variable assignment in LabMap.WindowsHosts for improved code clarity in cli/internal/labmap/labmap.go

**Removed:**

- Removed unnecessary variable assignment in WindowsHosts method of LabMap in cli/internal/labmap/labmap.go

Author: l50

.gitignore

@@ -14,6 +14,7 @@ __pycache__/
 
 # Build artifacts
 dreadgoad
+coverage.out
 ansible/roles/adcs_templates/files/ADCSTemplate.zip
 ansible/roles/vulns_adcs_templates/files/ADCSTemplate.zip
 

cli/cmd/ami.go

@@ -101,6 +101,7 @@ func init() {
 	amiBuildCmd.Flags().String("instance-profile", "", "IAM instance profile for EC2 Image Builder")
 	amiBuildCmd.Flags().Bool("reuse-resources", false, "Reuse existing Image Builder resources instead of recreating")
 	amiBuildCmd.Flags().Bool("all", false, "Build all templates in warpgate-templates/")
+	amiBuildCmd.MarkFlagsMutuallyExclusive("all", "instance-type")
 
 	amiListCmd.Flags().String("region", "", "AWS region")
 	amiListCmd.Flags().String("profile", "", "AWS profile")

cli/cmd/provision.go

@@ -58,6 +58,7 @@ func init() {
 	provisionCmd.Flags().String("limit", "", "Limit execution to specific hosts")
 	provisionCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config)")
 	provisionCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config)")
+	provisionCmd.MarkFlagsMutuallyExclusive("plays", "from")
 
 	adUsersCmd.Flags().String("plays", "ad-data.yml", "Playbooks to run")
 	adUsersCmd.Flags().String("limit", "", "Limit execution to specific hosts")

cli/internal/ansible/errors.go

@@ -1,3 +1,5 @@
+// Package ansible provides utilities for running Ansible playbooks and
+// handling failures, including error classification and retry strategies.
 package ansible
 
 import (
@@ -8,6 +10,7 @@ import (
 // ErrorType classifies Ansible failures for error-specific retry strategies.
 type ErrorType string
 
+// Ansible error type constants used to select error-specific retry strategies.
 const (
 	ErrFactGathering   ErrorType = "fact_gathering"
 	ErrNetworkAdapter  ErrorType = "network_adapter"

cli/internal/ansible/logparser.go

@@ -13,7 +13,8 @@ var (
 )
 
 // CheckAnsibleSuccess analyzes Ansible output to determine if the run succeeded.
-// Returns true if no failures detected.
+// It reports whether no failures or unreachable hosts were detected in the
+// PLAY RECAP and no unignored fatal errors appear in the output.
 func CheckAnsibleSuccess(output string) bool {
 	if idx := strings.Index(output, "PLAY RECAP"); idx >= 0 {
 		recap := output[idx:]
@@ -38,11 +39,7 @@ func CheckAnsibleSuccess(output string) bool {
 		}
 	}
 
-	if strings.Contains(output, "to retry, use:") {
-		return false
-	}
-
-	return true
+	return !strings.Contains(output, "to retry, use:")
 }
 
 // ExtractFailedHosts parses PLAY RECAP to find hosts with failures or unreachable status.

cli/internal/ansible/retry.go

@@ -14,7 +14,9 @@ import (
 	"github.com/dreadnode/dreadgoad/internal/inventory"
 )
 
-// RetryOptions configures the retry behavior for playbook execution.
+// RetryOptions configures the retry behavior for a [RunPlaybookWithRetry] call.
+// MaxRetries and RetryDelay default to the values from the global [config.Config]
+// when left as zero.
 type RetryOptions struct {
 	Playbook    string
 	Env         string
@@ -35,7 +37,11 @@ func (o *RetryOptions) logger() *slog.Logger {
 	return slog.Default()
 }
 
-// RunPlaybookWithRetry runs a playbook with error-specific retry logic.
+// RunPlaybookWithRetry runs an Ansible playbook with error-specific retry logic.
+// On each failure it classifies the error via [DetectErrorType] and applies a
+// targeted recovery strategy (e.g. SSM session cleanup, host reboots) before
+// retrying. It returns an error if all attempts are exhausted or the context
+// is cancelled.
 func RunPlaybookWithRetry(ctx context.Context, opts RetryOptions) error {
 	cfg, err := config.Get()
 	if err != nil {
@@ -224,7 +230,10 @@ func buildRetryLimit(userLimit, failedHosts string) string {
 	}
 }
 
-// CleanupSSMSessions terminates stale SSM sessions to prevent connection saturation.
+// CleanupSSMSessions terminates stale SSM sessions to prevent connection
+// saturation. It resolves the AWS region from the inventory, then calls
+// [daws.Client.CleanupStaleSessions] for all instances in the current
+// environment. Sessions idle for more than 15 minutes are terminated.
 func CleanupSSMSessions(ctx context.Context, env string, log *slog.Logger) {
 	cfg, err := config.Get()
 	if err != nil {

cli/internal/ansible/retry_test.go

@@ -0,0 +1,62 @@
+package ansible
+
+import (
+	"testing"
+)
+
+// TestBuildRetryLimit covers all branches of buildRetryLimit.
+func TestBuildRetryLimit(t *testing.T) {
+	tests := []struct {
+		name        string
+		userLimit   string
+		failedHosts string
+		want        string
+	}{
+		{
+			name:        "both set",
+			userLimit:   "dc01",
+			failedHosts: "dc02,dc03",
+			want:        "dc01,dc02,dc03",
+		},
+		{
+			name:        "only userLimit",
+			userLimit:   "dc01",
+			failedHosts: "",
+			want:        "dc01",
+		},
+		{
+			name:        "only failedHosts",
+			userLimit:   "",
+			failedHosts: "dc02",
+			want:        "dc02",
+		},
+		{
+			name:        "both empty",
+			userLimit:   "",
+			failedHosts: "",
+			want:        "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := buildRetryLimit(tt.userLimit, tt.failedHosts)
+			if got != tt.want {
+				t.Errorf("buildRetryLimit(%q, %q) = %q, want %q",
+					tt.userLimit, tt.failedHosts, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestRetryOptionsLogger verifies the logger fallback logic.
+func TestRetryOptionsLogger(t *testing.T) {
+	t.Run("returns custom logger when set", func(t *testing.T) {
+		// slog.Default() is a valid *slog.Logger; we just verify no panic.
+		opts := RetryOptions{}
+		got := opts.logger()
+		if got == nil {
+			t.Error("logger() returned nil for default logger")
+		}
+	})
+}

cli/internal/ansible/runner.go

@@ -86,7 +86,7 @@ func RunPlaybook(ctx context.Context, opts RunOptions) *RunResult {
 		result.Output = fmt.Sprintf("failed to create stdout pipe: %v", err)
 		return result
 	}
-	cmd.Stderr = cmd.Stdout // merge stderr into stdout
+	cmd.Stderr = multiW // merge stderr into the same output stream
 
 	if err := cmd.Start(); err != nil {
 		result.ExitCode = 1

cli/internal/labmap/labmap.go

@@ -34,10 +34,12 @@ type HostConfig struct {
 	MSSQL     *MSSQLConfig               `json:"mssql"`
 }
 
+// MSSQLLinkedServer holds the data source address for a linked SQL Server.
 type MSSQLLinkedServer struct {
 	DataSrc string `json:"data_src"`
 }
 
+// MSSQLConfig holds the MSSQL configuration for a host.
 type MSSQLConfig struct {
 	SAPassword     string                       `json:"sa_password"`
 	ServiceAccount string                       `json:"svcaccount"`
@@ -66,6 +68,7 @@ type ACLConfig struct {
 	Inheritance string `json:"inheritance"`
 }
 
+// GMSAConfig holds the configuration for a Group Managed Service Account.
 type GMSAConfig struct {
 	Name      string   `json:"gMSA_Name"`
 	FQDN      string   `json:"gMSA_FQDN"`
@@ -74,6 +77,8 @@ type GMSAConfig struct {
 }
 
 // DomainConfig represents a domain from config.json lab.domains.
+// It includes the DC host role, trust relationships, ADCS settings, and
+// all users, groups, OUs, and ACLs defined for the domain.
 type DomainConfig struct {
 	DC              string                `json:"dc"` // host role key
 	DomainPassword  string                `json:"domain_password"`
@@ -170,7 +175,6 @@ func (m *LabMap) WindowsHosts() []string {
 	var hosts []string
 	for role, hc := range m.HostConfigs {
 		if !strings.EqualFold(hc.OS, "linux") {
-			_ = hc
 			hosts = append(hosts, role)
 		}
 	}

cli/internal/validate/checks.go

@@ -1,3 +1,6 @@
+// Package validate provides vulnerability validation checks for GOAD lab
+// instances. It runs PowerShell commands against Windows hosts via AWS SSM
+// and records pass/fail/warn results in a structured [Report].
 package validate
 
 import (
@@ -814,7 +817,8 @@ func (v *Validator) checkPasswordPolicy(ctx context.Context, w io.Writer) {
 	}
 }
 
-// parseOutputLines splits PowerShell output into non-empty trimmed lines.
+// parseOutputLines splits PowerShell command output into non-empty trimmed
+// lines, discarding blank lines and leading/trailing whitespace.
 func parseOutputLines(output string) []string {
 	var lines []string
 	for _, line := range strings.Split(output, "\n") {