fix: address code review feedback on image pruning

- Fix estimateRegistryTagsToPrune: remove incorrect early return for keepReleases=0
  When keepReleases is 0, user wants to keep only 'latest' tag, but early
  return caused preview to report 0 tags to prune incorrectly.

- Fix pruneWithSpinner: properly drive spinner animation
  Use model.Update(spinner.TickMsg{}) and View() methods instead of
  calling Init() repeatedly.

- Fix test idempotency: reset runtime pruneReport between prunes
  Ensures second prune truly tests idempotency by asserting zero
  runtime results after clearing fake runtime's state.

commit: 0ad73b17203389b26d7dcf248d3ceb0c1e649cca
author: brice · 8mo ago · 2026-02-13 18:30 UTC
parent: 7c11f18
signature: unsigned

2 files · +15 -10

M internal/adapters/in/cli/images.go +10 -10


@@ -8,6 +8,7 @@ import (
8	8		"strings"
9	9		"time"
10	10
	11	+	"github.com/charmbracelet/bubbles/spinner"
11	12		"github.com/charmbracelet/lipgloss"
12	13		"github.com/spf13/cobra"
13	14

@@ -173,15 +174,18 @@ func pruneWithSpinner(ctx context.Context, client imagesClient, req dto.ImagePru
173	174		if result.resp == nil {
174	175		return nil, fmt.Errorf("prune operation completed but no response received")
175	176		}
176		-	// Success - wait for final frame to render
	177	+	// Success - set final message and render
177	178		model.SetMessage(cliRenderSuccess("Pruning complete"))
178		-	time.Sleep(100 * time.Millisecond)
179		-	cmd := model.Init()
180		-	fmt.Print(cmd)
	179	+	updatedModel, _ := model.Update(spinner.TickMsg{})
	180	+	if m, ok := updatedModel.(components.SpinnerModel); ok {
	181	+	fmt.Print(m.View())
	182	+	}
181	183		return result.resp, nil
182	184		case <-time.After(100 * time.Millisecond):
183		-	cmd := model.Init()
184		-	fmt.Print(cmd)
	185	+	updatedModel, _ := model.Update(spinner.TickMsg{})
	186	+	if m, ok := updatedModel.(components.SpinnerModel); ok {
	187	+	fmt.Print(m.View())
	188	+	}
185	189		}
186	190		}
187	191		}

@@ -378,10 +382,6 @@ type previewRegistryTag struct {
378	382		}
379	383
380	384		func estimateRegistryTagsToPrune(images []dto.Image, keepReleases int) int {
381		-	if keepReleases <= 0 {
382		-	return 0
383		-	}
384		-
385	385		tagsByRepo := collectTagsByRepository(images)
386	386		removed := 0
387	387

 	"strings"
 	"time"
+	"github.com/charmbracelet/bubbles/spinner"
 	"github.com/charmbracelet/lipgloss"
 	"github.com/spf13/cobra"
 			if result.resp == nil {
 				return nil, fmt.Errorf("prune operation completed but no response received")
 			}
-			// Success - wait for final frame to render
+			// Success - set final message and render
 			model.SetMessage(cliRenderSuccess("Pruning complete"))
-			time.Sleep(100 * time.Millisecond)
+			updatedModel, _ := model.Update(spinner.TickMsg{})
-			cmd := model.Init()
+			if m, ok := updatedModel.(components.SpinnerModel); ok {
-			fmt.Print(cmd)
+				fmt.Print(m.View())
+			}
 			return result.resp, nil
 		case <-time.After(100 * time.Millisecond):
-			cmd := model.Init()
+			updatedModel, _ := model.Update(spinner.TickMsg{})
-			fmt.Print(cmd)
+			if m, ok := updatedModel.(components.SpinnerModel); ok {
+				fmt.Print(m.View())
+			}
 		}
 	}
 }
 }
 func estimateRegistryTagsToPrune(images []dto.Image, keepReleases int) int {
-	if keepReleases <= 0 {
-		return 0
-	}
 	tagsByRepo := collectTagsByRepository(images)
 	removed := 0

M internal/usecase/images/service_test.go +5 -0


@@ -480,6 +480,9 @@ func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) {
480	480		assert.Equal(t, 1, report1.Registry.TagsRemoved, "first prune should remove v1")
481	481		assert.Equal(t, 1, report1.Registry.BlobsRemoved, "first prune should remove orphan")
482	482
	483	+	// Reset runtime prune report to test idempotency properly (runtime would have nothing to prune second time)
	484	+	rt.pruneReport = pkgruntime.PruneReport{}
	485	+
483	486		// Second prune - should be idempotent (nothing else to remove)
484	487		report2, err := svc.Prune(context.Background(), domain.ImagePruneOptions{
485	488		KeepLast: 1, PruneDangling: true, PruneRegistry: true,

@@ -487,6 +490,8 @@ func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) {
487	490		require.NoError(t, err)
488	491		assert.Equal(t, 0, report2.Registry.TagsRemoved, "second prune should remove nothing (idempotent)")
489	492		assert.Equal(t, 0, report2.Registry.BlobsRemoved, "second prune should remove nothing (idempotent)")
	493	+	assert.Equal(t, 0, report2.Runtime.DeletedCount, "second prune should remove no runtime images")
	494	+	assert.Equal(t, int64(0), report2.Runtime.SpaceReclaimed, "second prune should reclaim no runtime space")
490	495		}
491	496
492	497		func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) {


@@ -480,6 +480,9 @@ func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) {
480	assert.Equal(t, 1, report1.Registry.TagsRemoved, "first prune should remove v1")	480		assert.Equal(t, 1, report1.Registry.TagsRemoved, "first prune should remove v1")
481	assert.Equal(t, 1, report1.Registry.BlobsRemoved, "first prune should remove orphan")	481		assert.Equal(t, 1, report1.Registry.BlobsRemoved, "first prune should remove orphan")
482		482
		483	+	// Reset runtime prune report to test idempotency properly (runtime would have nothing to prune second time)
		484	+	rt.pruneReport = pkgruntime.PruneReport{}
		485	+
483	// Second prune - should be idempotent (nothing else to remove)	486		// Second prune - should be idempotent (nothing else to remove)
484	report2, err := svc.Prune(context.Background(), domain.ImagePruneOptions{	487		report2, err := svc.Prune(context.Background(), domain.ImagePruneOptions{
485	KeepLast: 1, PruneDangling: true, PruneRegistry: true,	488		KeepLast: 1, PruneDangling: true, PruneRegistry: true,

@@ -487,6 +490,8 @@ func TestService_Prune_IdempotentMultiplePrunesDoNotAccumulate(t *testing.T) {
487	require.NoError(t, err)	490		require.NoError(t, err)
488	assert.Equal(t, 0, report2.Registry.TagsRemoved, "second prune should remove nothing (idempotent)")	491		assert.Equal(t, 0, report2.Registry.TagsRemoved, "second prune should remove nothing (idempotent)")
489	assert.Equal(t, 0, report2.Registry.BlobsRemoved, "second prune should remove nothing (idempotent)")	492		assert.Equal(t, 0, report2.Registry.BlobsRemoved, "second prune should remove nothing (idempotent)")
		493	+	assert.Equal(t, 0, report2.Runtime.DeletedCount, "second prune should remove no runtime images")
		494	+	assert.Equal(t, int64(0), report2.Runtime.SpaceReclaimed, "second prune should reclaim no runtime space")
490	}	495		}
491		496
492	func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) {	497		func TestService_Prune_RunsBothRuntimeAndRegistry(t *testing.T) {