Release readiness: debug, retry, dry-run, token expiry, WS warnings by omattsson · Pull Request #55 · omattsson/stackctl

omattsson · 2026-04-30T19:36:06Z

Summary

--debug flag with HTTP request/response logging to stderr and credential masking (X-API-Key, Authorization headers). Also passed as STACKCTL_DEBUG env var to plugins.
Retry with backoff for transient failures (429/502/503/504) on idempotent methods (GET/PUT/DELETE). Respects Retry-After header, capped at 30s. Configurable backoff via Client.RetryBackoff.
--dry-run flag on all destructive operations: stack deploy/stop/clean/delete/rollback, all bulk ops, definition delete, template delete, cluster shared-values delete, orphaned delete, override value/branch/quota delete.
Token near-expiry warning when JWT expires within 5 minutes. Refactored from package-level var to clean return value.
WebSocket warnings on stderr for malformed JSON messages and custom HTTP transport TLS fallback.
Types package tests — JSON round-trip coverage for StackInstance, StackDefinition, ListResponse, WSMessage, QuotaOverride, plus omitempty verification.

Test plan

go vet ./... clean
go test ./... all packages pass (cmd, client, config, output, types, e2e, integration)
Retry-After cap test confirms 3600s server value is capped to 30s
Manual: stackctl --debug stack list shows request/response on stderr
Manual: stackctl stack delete <id> --dry-run prints message without calling API
Manual: expired/near-expiry token triggers warning on stderr

🤖 Generated with Claude Code

…cket warnings, types tests Add --debug flag with request/response logging and credential masking. Add retry with backoff for transient failures (429/502/503/504) on idempotent methods, with Retry-After support capped at 30s. Add --dry-run flag to all destructive operations. Add token near-expiry warning (< 5 min). Add WebSocket malformed message and TLS fallback warnings. Add STACKCTL_DEBUG env passthrough to plugins. Add types package JSON round-trip tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Allows setting the git repository URL on chart configs so the UI branch selector can list branches for OCI-hosted charts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR improves stackctl “release readiness” by adding safer destructive workflows (dry-run), richer debugging hooks, more resilient client behavior (retries), token expiry UX improvements, and additional WebSocket robustness/testing.

Changes:

Added --debug flag and plugin env propagation (STACKCTL_DEBUG), plus client-side debug logging.
Implemented retry-with-backoff for transient HTTP errors on idempotent methods, including Retry-After handling with a 30s cap.
Added --dry-run support across many destructive commands, plus token near-expiry warnings and WebSocket warning output for malformed messages.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
cli/pkg/types/types.go	Adds `source_repo_url` to chart config and update request types.
cli/pkg/types/types_test.go	Introduces JSON round-trip/omitempty tests for several types.
cli/pkg/client/websocket.go	Adds warning output channel and warns/skips malformed WS messages.
cli/pkg/client/websocket_test.go	Updates WS call signature and adds tests for warning output paths.
cli/pkg/client/client.go	Adds debug logging, retry logic with backoff, and `Retry-After` parsing/capping.
cli/pkg/client/client_test.go	Adds tests for debug output and retry behavior (including `Retry-After`).
cli/cmd/token.go	Refactors token loading to return an optional near-expiry warning.
cli/cmd/token_test.go	Updates tests for new loadToken signature and adds expiry-warning coverage.
cli/cmd/template.go	Adds `--dry-run` flag to template delete.
cli/cmd/stack.go	Adds `--dry-run` flags and integrates dry-run checks; routes WS warnings to stderr.
cli/cmd/root.go	Adds global `--debug`, wires it into client creation, and introduces shared `isDryRun`.
cli/cmd/plugins.go	Propagates `--debug` to plugins via `STACKCTL_DEBUG`.
cli/cmd/plugins_test.go	Adds coverage ensuring debug env var propagation behavior.
cli/cmd/override.go	Adds dry-run support for override/branch/quota delete operations.
cli/cmd/orphaned.go	Adds dry-run support for orphaned namespace deletion.
cli/cmd/definition.go	Adds `--source-repo-url` to update-chart and dry-run to definition delete.
cli/cmd/cluster.go	Adds dry-run support for shared-values delete.
cli/cmd/bulk.go	Adds dry-run support to bulk deploy/stop/clean/delete operations.

+		if time.Now().After(t.ExpiresAt) {
+			return "", "", fmt.Errorf("token expired. Run 'stackctl login' to re-authenticate")
+		}
+		remaining := time.Until(t.ExpiresAt)


+	if c.Debug && c.DebugWriter != nil {
+		fmt.Fprintf(c.DebugWriter, "→ %s %s\n", method, u)
+	}
+
+	start := time.Now()
 	resp, err := c.HTTPClient.Do(req)
 	if err != nil {
+		if c.Debug && c.DebugWriter != nil {
+			fmt.Fprintf(c.DebugWriter, "✗ %s (%s)\n", err, time.Since(start).Truncate(time.Millisecond))
+		}
 		return nil, fmt.Errorf("making request: %w", err)
 	}

+	if c.Debug && c.DebugWriter != nil {
+		fmt.Fprintf(c.DebugWriter, "← %d %s (%s)\n", resp.StatusCode, http.StatusText(resp.StatusCode), time.Since(start).Truncate(time.Millisecond))
+	}


+// doWithRetry wraps do with retry logic for transient failures on idempotent methods.
+func (c *Client) doWithRetry(method, path string, body interface{}) (*http.Response, error) {
+	if !idempotentMethods[method] {
+		return c.do(method, path, body)
+	}
+
+	backoff := c.RetryBackoff
+	if backoff == nil {
+		backoff = defaultRetryBackoff
+	}
+	var lastErr error
+
+	for attempt := 0; attempt <= len(backoff); attempt++ {
+		resp, err := c.do(method, path, body)
+		if err == nil {
+			return resp, nil
+		}
+
+		apiErr, ok := err.(*APIError)
+		if !ok || !retryableStatuses[apiErr.StatusCode] {
+			return nil, err
+		}
+		lastErr = err
+
+		if attempt < len(backoff) {
+			wait := backoff[attempt]
+			if apiErr.StatusCode == http.StatusTooManyRequests {
+				if ra := apiErr.retryAfter; ra > 0 {
+					if ra > maxRetryAfter {
+						ra = maxRetryAfter
+					}
+					wait = ra
+				}
+			}
+			if c.Debug && c.DebugWriter != nil {
+				fmt.Fprintf(c.DebugWriter, "↻ retrying in %s (attempt %d/%d)\n", wait, attempt+1, len(backoff))
+			}
+			time.Sleep(wait)
+		}
+	}


+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempts++
+		if attempts == 1 {
+			w.Header().Set("Retry-After", "3600")
+			w.WriteHeader(http.StatusTooManyRequests)
+			json.NewEncoder(w).Encode(types.ErrorResponse{Error: "rate limited"})
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+		json.NewEncoder(w).Encode(map[string]string{"ok": "true"})
+	}))
+	defer server.Close()
+
+	c := New(server.URL)
+	c.RetryBackoff = []time.Duration{time.Millisecond, time.Millisecond}
+	start := time.Now()
+	var result map[string]string
+	err := c.Get("/test", &result)
+	require.NoError(t, err)
+	assert.Equal(t, 2, attempts)
+	// Capped at 30s, not 3600s — but should complete within ~31s (we test < 35s)
+	assert.Less(t, time.Since(start), 35*time.Second)


 func deleteByID(cmd *cobra.Command, args []string, promptFmt string, resolveFn func(*client.Client, string) (string, error), deleteFn func(*client.Client, string) error, successFmt string) error {
 	c, err := newClient()
 	if err != nil {
 		return err
 	}

 	id, err := resolveFn(c, args[0])
 	if err != nil {
 		return err
 	}

+	if isDryRun(cmd, "Would delete %s", id) {
+		return nil
+	}
+
 	confirmed, err := confirmAction(cmd, fmt.Sprintf(promptFmt, id))
 	if err != nil {
 		return err
 	}
 	if !confirmed {
 		printer.PrintMessage("Aborted.")
 		return nil
 	}

 	if err := deleteFn(c, id); err != nil {
 		return err
 	}

 	if printer.Quiet {
 		fmt.Fprintln(printer.Writer, id)
 		return nil
 	}

 	printer.PrintMessage(successFmt, id)
 	return nil
 }

+func isDryRun(cmd *cobra.Command, format string, args ...interface{}) bool {
+	dryRun, _ := cmd.Flags().GetBool("dry-run")
+	if dryRun {
+		printer.PrintMessage(format, args...)
+		return true
+	}
+	return false
+}


- Token expiry: treat remaining <= 0 as expired (edge case at exact expiry) - Debug logging: add request header output with credential masking (Authorization and X-API-Key show first 8 chars + ***) - Retry: inject Sleeper func for testability, use it in doWithRetry - Test: CapsRetryAfterAt30s no longer sleeps 30s (uses mock sleeper) - Tests: add dry-run coverage for stack delete/deploy and bulk delete/clean Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

+	case "authorization", "x-api-key":
+		if len(value) <= 8 {
+			return "***"
+		}
+		return value[:8] + "***"


+	start := time.Now()
+	var result map[string]string
+	err := c.Get("/test", &result)
+	require.NoError(t, err)
+	assert.Equal(t, 2, attempts)
+	assert.GreaterOrEqual(t, time.Since(start), 900*time.Millisecond)


+func TestPluginEnv_PassesDebugFlag(t *testing.T) {
+	t.Parallel()
+	root := &cobra.Command{Use: "stackctl"}
+	root.PersistentFlags().Bool("debug", false, "")
+	require.NoError(t, root.PersistentFlags().Set("debug", "true"))
+
+	env := pluginEnv(root)
+	found := false
+	for _, kv := range env {
+		if strings.HasPrefix(kv, "STACKCTL_DEBUG=") {
+			assert.Equal(t, "STACKCTL_DEBUG=1", kv)
+			found = true
+		}
+	}
+	assert.True(t, found, "STACKCTL_DEBUG should be set when --debug flag is changed")
+}
+
+func TestPluginEnv_OmitsDebugWhenNotChanged(t *testing.T) {
+	t.Parallel()
+	root := &cobra.Command{Use: "stackctl"}
+	root.PersistentFlags().Bool("debug", false, "")
+
+	env := pluginEnv(root)
+	for _, kv := range env {
+		if strings.HasPrefix(kv, "STACKCTL_DEBUG=") {
+			t.Fatal("STACKCTL_DEBUG should not be set when --debug flag is not changed")
+		}
+	}
+}
+
+func TestPluginEnv_NilCommand(t *testing.T) {
+	t.Parallel()
+	env := pluginEnv(nil)
+	assert.NotEmpty(t, env)



+	if isDryRun(cmd, "Would delete %s", id) {
+		return nil
+	}


- maskCredential: keep only scheme for Authorization ("Bearer ***"), show only last 4 chars for API keys ("***abcd") - Retry test: use mock Sleeper for Retry-After:1 test (was sleeping 1s) - plugins_test: remove t.Parallel() — cmd tests share process globals - deleteByID: check --dry-run before newClient/resolveFn to avoid side effects (credential loading, API calls) in dry-run mode Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fixes GO-2026-4971 (net.Dial panic with NUL on Windows) and GO-2026-4918 (HTTP/2 infinite loop on bad SETTINGS_MAX_FRAME_SIZE). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

 	c := client.New(apiURL)

+	// Debug: flag > env
+	if flagDebug || os.Getenv("STACKCTL_DEBUG") == "1" {


 func deleteByID(cmd *cobra.Command, args []string, promptFmt string, resolveFn func(*client.Client, string) (string, error), deleteFn func(*client.Client, string) error, successFmt string) error {
+	if isDryRun(cmd, "Would delete %s", args[0]) {
+		return nil
+	}


@@ -41,6 +41,8 @@ func (c *Client) StreamDeploymentLogs(ctx context.Context, instanceID string, w
 			dialer = &websocket.Dialer{
 				TLSClientConfig: t.TLSClientConfig,


@@ -252,6 +252,10 @@ Examples:
 			return err
 		}

+		if isDryRun(cmd, "Would deploy stack %s", id) {


 			return err
 		}

+		if isDryRun(cmd, "Would deploy %d stacks: %s", len(ids), strings.Join(ids, ", ")) {
+			return nil


 			return err
 		}

+		if isDryRun(cmd, "Would delete quota override for instance %s", instanceID) {
+			return nil


- Debug flag: --debug=false now overrides STACKCTL_DEBUG=1 env var (uses PersistentFlags().Changed to detect explicit flag) - Dry-run: check before client creation and name resolution in all commands (stack deploy/stop/clean/rollback, bulk deploy/stop/clean/delete, override quota delete, override chart delete). Added rawBulkArgs helper. - WebSocket dialer: clone DefaultDialer and override TLSClientConfig instead of creating empty dialer (preserves proxy, handshake timeout) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

@@ -429,6 +431,7 @@ Examples:
 			ChartName:     current.ChartName,
 			ChartPath:     current.RepoURL,


+		apiErr := &APIError{StatusCode: resp.StatusCode}
+		if ra := resp.Header.Get("Retry-After"); ra != "" {
+			if secs, err := strconv.Atoi(ra); err == nil && secs > 0 {
+				apiErr.retryAfter = time.Duration(secs) * time.Second
+			}
+		}


- definition update-chart: do not set ChartPath from RepoURL (different API fields); leave ChartPath empty unless --chart-path is provided - Retry-After: support both delta-seconds and HTTP-date formats per RFC 9110; extracted parseRetryAfter helper using http.ParseTime Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

+func parseRetryAfter(value string) time.Duration {
+	if secs, err := strconv.Atoi(value); err == nil && secs > 0 {
+		return time.Duration(secs) * time.Second
+	}
+	if t, err := http.ParseTime(value); err == nil {
+		if d := time.Until(t); d > 0 {


+		if remaining < 5*time.Minute {
+			mins := int(remaining.Minutes()) + 1
+			unit := "minutes"
+			if mins == 1 {
+				unit = "minute"
+			}
+			warning = fmt.Sprintf("Warning: token expires in %d %s. Run 'stackctl login' to refresh.", mins, unit)
+		}


Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Olof Mattsson and others added 2 commits April 30, 2026 21:35

feat: add --source-repo-url flag to definition update-chart

38fdfbc

Allows setting the git repository URL on chart configs so the UI branch selector can list branches for OCI-hosted charts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 6, 2026 09:29

Copilot started reviewing on behalf of omattsson May 6, 2026 09:29 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

omattsson requested a review from Copilot May 8, 2026 17:22

Copilot started reviewing on behalf of omattsson May 8, 2026 17:23 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

omattsson requested a review from Copilot May 8, 2026 18:32

Copilot started reviewing on behalf of omattsson May 8, 2026 18:33 View session

deps: bump Go 1.26.2 → 1.26.3 (fix govulncheck CVEs)

8837f78

Fixes GO-2026-4971 (net.Dial panic with NUL on Windows) and GO-2026-4918 (HTTP/2 infinite loop on bad SETTINGS_MAX_FRAME_SIZE). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI reviewed May 8, 2026

View reviewed changes

omattsson requested a review from Copilot May 8, 2026 18:50

Copilot started reviewing on behalf of omattsson May 8, 2026 18:51 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

omattsson requested a review from Copilot May 8, 2026 18:58

Copilot started reviewing on behalf of omattsson May 8, 2026 18:59 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

fix: trim Retry-After whitespace, use math.Ceil for token expiry warning

6481b31

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

omattsson merged commit 403fdee into main May 8, 2026
6 checks passed

omattsson deleted the fix/release-readiness branch May 8, 2026 19:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release readiness: debug, retry, dry-run, token expiry, WS warnings#55

Release readiness: debug, retry, dry-run, token expiry, WS warnings#55
omattsson merged 8 commits into
mainfrom
fix/release-readiness

omattsson commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -41,6 +41,8 @@ func (c *Client) StreamDeploymentLogs(ctx context.Context, instanceID string, w
		dialer = &websocket.Dialer{
		TLSClientConfig: t.TLSClientConfig,

		@@ -429,6 +431,7 @@ Examples:
		ChartName: current.ChartName,
		ChartPath: current.RepoURL,

Conversation

omattsson commented Apr 30, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants