From d3083fce1ccf73932ef47fe22cb6785002a276e4 Mon Sep 17 00:00:00 2001
From: Ilya Kiselev <kis-ilya-a@yandex.ru>
Date: Sun, 5 Apr 2026 21:52:37 +0300
Subject: [PATCH] cmd/push: align WriteTable format and add output flag test

Address Copilot review feedback:
- WriteTable now uses "Pushed:"/"Digest:" labels consistent with the
  registry client's built-in output (pkg/registry/client.go:746-747),
  so the default --output table experience is familiar to existing users
- Add TestPushOutputFlagCompletion to verify the --output flag is
  properly registered and offers json/yaml/table completions
- Document that Pusher.Push and action.Push.Run signature changes are
  intentional breaking changes in the Helm v4 major release

Signed-off-by: Ilya Kiselev <kis-ilya-a@yandex.ru>
---
 pkg/action/push.go   |  7 ++++++-
 pkg/cmd/push.go      | 14 ++++++--------
 pkg/cmd/push_test.go | 18 ++++++++++++++++--
 pkg/pusher/pusher.go |  7 ++++++-
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/pkg/action/push.go b/pkg/action/push.go
index 285005f86..5eb6110b8 100644
--- a/pkg/action/push.go
+++ b/pkg/action/push.go
@@ -80,7 +80,12 @@ func NewPushWithOpts(opts ...PushOpt) *Push {
 	return p
 }
 
-// Run executes 'helm push' against the given chart archive.
+// Run executes 'helm push' against the given chart archive and returns the
+// structured push result containing the ref and manifest digest.
+//
+// Note: the return type changed from (string, error) to (*registry.PushResult, error)
+// in Helm v4 as an intentional breaking change, enabling structured access to
+// push metadata without text parsing.
 func (p *Push) Run(chartRef string, remote string) (*registry.PushResult, error) {
 	c := uploader.ChartUploader{
 		Out:     io.Discard,
diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go
index 1d135fa64..56d882595 100644
--- a/pkg/cmd/push.go
+++ b/pkg/cmd/push.go
@@ -19,7 +19,6 @@ package cmd
 import (
 	"fmt"
 	"io"
-	"text/tabwriter"
 
 	"github.com/spf13/cobra"
 
@@ -58,14 +57,13 @@ type pushWriter struct {
 	result pushResult
 }
 
-// WriteTable writes a minimal human-readable push result to the provided output.
-// The registry client prints progress to stderr; this writes the structured
-// result (ref + digest) to the command's stdout stream.
+// WriteTable writes the push result in human-readable form, using the same
+// "Pushed:"/"Digest:" labels as the registry client's built-in output so that
+// the default (--output table) experience is consistent and familiar.
 func (w *pushWriter) WriteTable(out io.Writer) error {
-	tw := tabwriter.NewWriter(out, 0, 0, 2, ' ', 0)
-	fmt.Fprintf(tw, "REF\t%s\n", w.result.Ref)
-	fmt.Fprintf(tw, "DIGEST\t%s\n", w.result.Digest)
-	return tw.Flush()
+	fmt.Fprintf(out, "Pushed: %s\n", w.result.Ref)
+	fmt.Fprintf(out, "Digest: %s\n", w.result.Digest)
+	return nil
 }
 
 // WriteJSON writes the push result in JSON format
diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go
index 177cb88c6..114a4e38d 100644
--- a/pkg/cmd/push_test.go
+++ b/pkg/cmd/push_test.go
@@ -28,6 +28,20 @@ func TestPushFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "push package.tgz oci://localhost:5000", false)
 }
 
+// TestPushOutputFlagCompletion verifies that the --output flag is registered
+// on the push command and that its shell completion offers table/json/yaml.
+func TestPushOutputFlagCompletion(t *testing.T) {
+	_, out, err := executeActionCommandC(storageFixture(), "__complete push --output ''")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, want := range []string{"json", "yaml", "table"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("output flag completion missing %q, got: %q", want, out)
+		}
+	}
+}
+
 func TestPushWriterTable(t *testing.T) {
 	w := &pushWriter{result: pushResult{
 		Ref:    "oci://example.com/charts/mychart:1.0.0",
@@ -38,8 +52,8 @@ func TestPushWriterTable(t *testing.T) {
 		t.Fatal(err)
 	}
 	got := buf.String()
-	if !strings.Contains(got, "REF") || !strings.Contains(got, "DIGEST") {
-		t.Errorf("table output missing headers, got: %q", got)
+	if !strings.Contains(got, "Pushed:") || !strings.Contains(got, "Digest:") {
+		t.Errorf("table output missing Pushed:/Digest: labels, got: %q", got)
 	}
 	if !strings.Contains(got, "oci://example.com/charts/mychart:1.0.0") {
 		t.Errorf("table output missing Ref value, got: %q", got)
diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go
index adaa26d06..a5df97c26 100644
--- a/pkg/pusher/pusher.go
+++ b/pkg/pusher/pusher.go
@@ -70,8 +70,13 @@ func WithPlainHTTP(plainHTTP bool) Option {
 }
 
 // Pusher is an interface to support upload to the specified URL.
+//
+// Note: the Push method signature was updated in Helm v4 to return
+// *registry.PushResult alongside the error, enabling callers to obtain
+// structured push metadata (ref, digest) without parsing text output.
+// This is an intentional breaking change in the v4 major release.
 type Pusher interface {
-	// Push file content by url string, returning the push result and any error
+	// Push uploads the chart at chartRef to url and returns the push result.
 	Push(chartRef, url string, options ...Option) (*registry.PushResult, error)
 }