From 2c97b6140ccc3a1010bd17a1c0d0b0f5a110f66b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torjus=20H=C3=A5kestad?= Date: Sat, 7 Feb 2026 06:19:08 +0100 Subject: [PATCH] fix: check only final responses in AllSucceeded to determine deployment success The CLI was incorrectly reporting "some deployments failed" even when deployments succeeded. This was because AllSucceeded() checked if every response had StatusCompleted, but the Responses slice contains all messages including intermediate ones like "started". Since started != completed, it returned false. Now AllSucceeded() only examines final responses (using IsFinal()) and checks that each host's final status is completed. Co-Authored-By: Claude Opus 4.5 --- cmd/homelab-deploy/main.go | 2 +- internal/cli/deploy.go | 24 +++++++++++++++++++++--- internal/cli/deploy_test.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/cmd/homelab-deploy/main.go b/cmd/homelab-deploy/main.go index 83fc321..22a3617 100644 --- a/cmd/homelab-deploy/main.go +++ b/cmd/homelab-deploy/main.go @@ -16,7 +16,7 @@ import ( "github.com/urfave/cli/v3" ) -const version = "0.1.5" +const version = "0.1.6" func main() { app := &cli.Command{ diff --git a/internal/cli/deploy.go b/internal/cli/deploy.go index bf53922..058300f 100644 --- a/internal/cli/deploy.go +++ b/internal/cli/deploy.go @@ -28,14 +28,32 @@ type DeployResult struct { Errors []error } -// AllSucceeded returns true if all responses indicate success. +// AllSucceeded returns true if all hosts' final responses indicate success. func (r *DeployResult) AllSucceeded() bool { + if len(r.Errors) > 0 { + return false + } + + // Track the final status for each host + finalStatus := make(map[string]messages.Status) for _, resp := range r.Responses { - if resp.Status != messages.StatusCompleted { + if resp.Status.IsFinal() { + finalStatus[resp.Hostname] = resp.Status + } + } + + // Need at least one host with a final status + if len(finalStatus) == 0 { + return false + } + + // All final statuses must be completed + for _, status := range finalStatus { + if status != messages.StatusCompleted { return false } } - return len(r.Responses) > 0 && len(r.Errors) == 0 + return true } // HostCount returns the number of unique hosts that responded. diff --git a/internal/cli/deploy_test.go b/internal/cli/deploy_test.go index bc27166..22a8fe0 100644 --- a/internal/cli/deploy_test.go +++ b/internal/cli/deploy_test.go @@ -49,6 +49,40 @@ func TestDeployResult_AllSucceeded(t *testing.T) { errors: []error{nil}, // placeholder error want: false, }, + { + name: "with intermediate responses - success", + responses: []*messages.DeployResponse{ + {Hostname: "host1", Status: messages.StatusStarted}, + {Hostname: "host1", Status: messages.StatusCompleted}, + }, + want: true, + }, + { + name: "with intermediate responses - failure", + responses: []*messages.DeployResponse{ + {Hostname: "host1", Status: messages.StatusStarted}, + {Hostname: "host1", Status: messages.StatusFailed}, + }, + want: false, + }, + { + name: "multiple hosts with intermediate responses", + responses: []*messages.DeployResponse{ + {Hostname: "host1", Status: messages.StatusStarted}, + {Hostname: "host2", Status: messages.StatusStarted}, + {Hostname: "host1", Status: messages.StatusCompleted}, + {Hostname: "host2", Status: messages.StatusCompleted}, + }, + want: true, + }, + { + name: "only intermediate responses - no final", + responses: []*messages.DeployResponse{ + {Hostname: "host1", Status: messages.StatusStarted}, + {Hostname: "host1", Status: messages.StatusAccepted}, + }, + want: false, + }, } for _, tc := range tests {