9bfe1d5 fix: address CodeRabbit review — extract deploy helper, typed auth error, scope comment · gordon

M internal/adapters/in/cli/push.go +18 -14


@@ -202,20 +202,7 @@ func runPush(ctx context.Context, req pushRequest) error {
202	202		}
203	203		fmt.Printf("Domain: %s\n", styles.Theme.Bold.Render(pushDomain))
204	204
205		-	// Signal the server to suppress event-based deploys for this image.
206		-	// The CLI will trigger an explicit deploy after push completes.
207		-	skipExplicitDeploy := req.NoDeploy
208		-	if !req.NoDeploy {
209		-	if err := handle.plane.DeployIntent(ctx, imageName); err != nil {
210		-	if isInsufficientScope(err) {
211		-	fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")
212		-	skipExplicitDeploy = true
213		-	} else {
214		-	// Non-fatal: worst case we get a redundant deploy via event
215		-	fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
216		-	}
217		-	}
218		-	}
	205	+	skipExplicitDeploy := shouldSkipDeploy(ctx, handle.plane, imageName, req.NoDeploy)
219	206
220	207		build := buildConfig{
221	208		Enabled: req.Build.Enabled,

@@ -243,6 +230,23 @@ func runPush(ctx context.Context, req pushRequest) error {
243	230		return nil
244	231		}
245	232
	233	+	// shouldSkipDeploy signals deploy intent and returns whether the explicit deploy
	234	+	// should be skipped (either because --no-deploy was set, or the token lacks scope).
	235	+	func shouldSkipDeploy(ctx context.Context, plane ControlPlane, imageName string, noDeploy bool) bool {
	236	+	if noDeploy {
	237	+	return true
	238	+	}
	239	+	if err := plane.DeployIntent(ctx, imageName); err != nil {
	240	+	if isInsufficientScope(err) {
	241	+	fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")
	242	+	return true
	243	+	}
	244	+	// Non-fatal: worst case we get a redundant deploy via event
	245	+	fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
	246	+	}
	247	+	return false
	248	+	}
	249	+
246	250		// isInsufficientScope returns true if the error is an HTTP 403 Forbidden response,
247	251		// meaning the token lacks the required admin scope.
248	252		func isInsufficientScope(err error) bool {

 	}
 	fmt.Printf("Domain: %s\n", styles.Theme.Bold.Render(pushDomain))
-	// Signal the server to suppress event-based deploys for this image.
+	skipExplicitDeploy := shouldSkipDeploy(ctx, handle.plane, imageName, req.NoDeploy)
-	// The CLI will trigger an explicit deploy after push completes.
-	skipExplicitDeploy := req.NoDeploy
-	if !req.NoDeploy {
-		if err := handle.plane.DeployIntent(ctx, imageName); err != nil {
-			if isInsufficientScope(err) {
-				fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")
-				skipExplicitDeploy = true
-			} else {
-				// Non-fatal: worst case we get a redundant deploy via event
-				fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
-			}
-		}
-	}
 	build := buildConfig{
 		Enabled:    req.Build.Enabled,
 	return nil
 }
+// shouldSkipDeploy signals deploy intent and returns whether the explicit deploy
+// should be skipped (either because --no-deploy was set, or the token lacks scope).
+func shouldSkipDeploy(ctx context.Context, plane ControlPlane, imageName string, noDeploy bool) bool {
+	if noDeploy {
+		return true
+	}
+	if err := plane.DeployIntent(ctx, imageName); err != nil {
+		if isInsufficientScope(err) {
+			fmt.Fprintln(os.Stderr, "info: deploy intent skipped (insufficient scope), server will auto-deploy on image receive")
+			return true
+		}
+		// Non-fatal: worst case we get a redundant deploy via event
+		fmt.Fprintf(os.Stderr, "warning: failed to register deploy intent: %v\n", err)
+	}
+	return false
+}
 // isInsufficientScope returns true if the error is an HTTP 403 Forbidden response,
 // meaning the token lacks the required admin scope.
 func isInsufficientScope(err error) bool {

M internal/adapters/in/cli/push_image_ops.go +9 -1


@@ -2,8 +2,10 @@ package cli
2	2
3	3		import (
4	4		"context"
	5	+	"errors"
5	6		"fmt"
6	7		"io"
	8	+	"net/http"
7	9		"os"
8	10		"os/exec"
9	11		"strings"

@@ -111,11 +113,17 @@ func (d *dockerImageOps) Push(ctx context.Context, ref string) error {
111	113		return pusher.Push(ctx, ref)
112	114		}
113	115
114		-	// isAuthError returns true if the error looks like an HTTP 401 authentication failure.
	116	+	// isAuthError returns true if the error indicates an HTTP 401 authentication failure.
	117	+	// Prefers typed HTTPError from the remote client; falls back to string matching for
	118	+	// errors from registrypush which uses its own error format.
115	119		func isAuthError(err error) bool {
116	120		if err == nil {
117	121		return false
118	122		}
	123	+	var httpErr *remote.HTTPError
	124	+	if errors.As(err, &httpErr) {
	125	+	return httpErr.StatusCode == http.StatusUnauthorized
	126	+	}
119	127		msg := err.Error()
120	128		return strings.Contains(msg, "status 401") \|\| strings.Contains(msg, "401 Unauthorized")
121	129		}

 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"os/exec"
 	"strings"
 	return pusher.Push(ctx, ref)
 }
-// isAuthError returns true if the error looks like an HTTP 401 authentication failure.
+// isAuthError returns true if the error indicates an HTTP 401 authentication failure.
+// Prefers typed HTTPError from the remote client; falls back to string matching for
+// errors from registrypush which uses its own error format.
 func isAuthError(err error) bool {
 	if err == nil {
 		return false
 	}
+	var httpErr *remote.HTTPError
+	if errors.As(err, &httpErr) {
+		return httpErr.StatusCode == http.StatusUnauthorized
+	}
 	msg := err.Error()
 	return strings.Contains(msg, "status 401") || strings.Contains(msg, "401 Unauthorized")
 }

M internal/adapters/in/cli/remote/auth.go +2 -0


@@ -78,6 +78,8 @@ func (c Client) Authenticate(ctx context.Context, username, password string) (
78	78		// The subject must match the token's sub claim (use VerifyAuth to obtain it).
79	79		// Returns the raw short-lived token string (without a "Bearer " prefix).
80	80		func (c *Client) ExchangeRegistryToken(ctx context.Context, subject string) (string, error) {
	81	+	// Wildcard scope (repository:*:push,pull) grants push/pull to all repositories.
	82	+	// The server downsopes this to the parent token's actual scopes during exchange.
81	83		url := c.baseURL + "/auth/token?scope=repository:*:push,pull&service=gordon-registry"
82	84
83	85		req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)


@@ -78,6 +78,8 @@ func (c Client) Authenticate(ctx context.Context, username, password string) (
78	// The subject must match the token's sub claim (use VerifyAuth to obtain it).	78		// The subject must match the token's sub claim (use VerifyAuth to obtain it).
79	// Returns the raw short-lived token string (without a "Bearer " prefix).	79		// Returns the raw short-lived token string (without a "Bearer " prefix).
80	func (c *Client) ExchangeRegistryToken(ctx context.Context, subject string) (string, error) {	80		func (c *Client) ExchangeRegistryToken(ctx context.Context, subject string) (string, error) {
		81	+	// Wildcard scope (repository:*:push,pull) grants push/pull to all repositories.
		82	+	// The server downsopes this to the parent token's actual scopes during exchange.
81	url := c.baseURL + "/auth/token?scope=repository:*:push,pull&service=gordon-registry"	83		url := c.baseURL + "/auth/token?scope=repository:*:push,pull&service=gordon-registry"
82		84
83	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)	85		req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)