From 7dde7359772fbe1096c1a17c7c272ec20f263f96 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:09:53 +0000 Subject: [PATCH 1/5] fix(OPENFRAM-005): 8 review findings across 5 files --- internal/cluster/providers/k3d/verify.go | 61 ++++++++++++++++++------ 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/internal/cluster/providers/k3d/verify.go b/internal/cluster/providers/k3d/verify.go index 980c8c9f..a6505dc6 100644 --- a/internal/cluster/providers/k3d/verify.go +++ b/internal/cluster/providers/k3d/verify.go @@ -9,6 +9,8 @@ import ( "strings" "time" + "github.com/pterm/pterm" + sharedconfig "github.com/flamingo-stack/openframe-cli/internal/shared/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -41,12 +43,12 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str // Switch the current context config.CurrentContext = contextName - if err := clientcmd.WriteToFile(*config, kubeconfigPath); err != nil { + if err := writeKubeconfigAtomically(*config, kubeconfigPath); err != nil { return nil, fmt.Errorf("failed to switch and write kubectl context: %w", err) } if m.verbose { - fmt.Printf("✓ Switched kubectl context to %s\n", contextName) + pterm.Info.Printfln("Switched kubectl context to %s", contextName) } // Build rest.Config from the loaded Kubeconfig @@ -66,7 +68,7 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str restConfig = sharedconfig.ApplyInsecureTLSConfig(restConfig) if m.verbose { - fmt.Println("✓ TLS verification bypassed for local k3d cluster (Insecure=true, auth preserved)") + pterm.Info.Println("TLS verification bypassed for local k3d cluster (Insecure=true, auth preserved)") } // --- PHASE 2: Verify Network Connectivity and Update Endpoint --- @@ -75,7 +77,7 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str host, port, err := extractHostPort(restConfig.Host) if err != nil { if m.verbose { - fmt.Printf("Warning: Could not extract host:port from %s: %v\n", restConfig.Host, err) + pterm.Warning.Printfln("Could not extract host:port from %s: %v", restConfig.Host, err) } // Default to 127.0.0.1:6550 for k3d host = "127.0.0.1" @@ -105,7 +107,7 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str var lastErr error if m.verbose { - fmt.Println("Waiting for cluster API and nodes to be reachable...") + pterm.Info.Println("Waiting for cluster API and nodes to be reachable...") } for i := 0; i < maxRetries; i++ { @@ -123,7 +125,7 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str if isTemporaryError(err) { lastErr = err if m.verbose { - fmt.Printf(" Cluster not ready yet (attempt %d/%d): %v\n", i+1, maxRetries, err) + pterm.Info.Printfln("Cluster not ready yet (attempt %d/%d): %v", i+1, maxRetries, err) } time.Sleep(retryDelay) continue @@ -136,7 +138,7 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str if len(nodes.Items) == 0 { lastErr = fmt.Errorf("no nodes found in cluster") if m.verbose { - fmt.Printf(" No nodes found yet (attempt %d/%d), waiting...\n", i+1, maxRetries) + pterm.Info.Printfln("No nodes found yet (attempt %d/%d), waiting...", i+1, maxRetries) } time.Sleep(retryDelay) continue @@ -157,15 +159,15 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str // Success condition: Nodes exist and at least one is ready if readyCount > 0 { if m.verbose { - fmt.Printf(" Found %d ready node(s) out of %d total\n", readyCount, len(nodes.Items)) - fmt.Println("✓ Cluster API and nodes are ready.") + pterm.Info.Printfln("Found %d ready node(s) out of %d total", readyCount, len(nodes.Items)) + pterm.Success.Println("Cluster API and nodes are ready.") } return restConfig, nil } lastErr = fmt.Errorf("no nodes in Ready state (found %d nodes, 0 ready)", len(nodes.Items)) if m.verbose { - fmt.Printf(" Nodes exist but none are Ready yet (attempt %d/%d), waiting...\n", i+1, maxRetries) + pterm.Info.Printfln("Nodes exist but none are Ready yet (attempt %d/%d), waiting...", i+1, maxRetries) } time.Sleep(retryDelay) } @@ -173,6 +175,37 @@ func (m *K3dManager) verifyClusterReachable(ctx context.Context, clusterName str return nil, fmt.Errorf("cluster not reachable after %d retries (last error: %w)", maxRetries, lastErr) } +// writeKubeconfigAtomically writes the kubeconfig to a temp file in the same +// directory and renames it into place, avoiding a read-modify-write race with +// other concurrently running CLI invocations that may also touch kubeconfigPath. +func writeKubeconfigAtomically(config clientcmd.Config, kubeconfigPath string) error { + dir := filepath.Dir(kubeconfigPath) + tmpFile, err := os.CreateTemp(dir, ".kubeconfig-*.tmp") + if err != nil { + return fmt.Errorf("failed to create temp kubeconfig file: %w", err) + } + tmpPath := tmpFile.Name() + _ = tmpFile.Close() + + defer func() { + _ = os.Remove(tmpPath) + }() + + if err := clientcmd.WriteToFile(config, tmpPath); err != nil { + return fmt.Errorf("failed to write temp kubeconfig file: %w", err) + } + + if info, statErr := os.Stat(kubeconfigPath); statErr == nil { + _ = os.Chmod(tmpPath, info.Mode()) + } + + if err := os.Rename(tmpPath, kubeconfigPath); err != nil { + return fmt.Errorf("failed to atomically replace kubeconfig file: %w", err) + } + + return nil +} + // isTemporaryError checks if an error is temporary and should be retried func isTemporaryError(err error) bool { if err == nil { @@ -194,7 +227,7 @@ func (m *K3dManager) waitForTCPPort(ctx context.Context, host string, port strin address := net.JoinHostPort(host, port) if m.verbose { - fmt.Printf("Waiting for TCP port %s to be available...\n", address) + pterm.Info.Printfln("Waiting for TCP port %s to be available...", address) } var lastErr error @@ -212,14 +245,14 @@ func (m *K3dManager) waitForTCPPort(ctx context.Context, host string, port strin if err == nil { _ = conn.Close() if m.verbose { - fmt.Printf("✓ TCP port %s is open\n", address) + pterm.Success.Printfln("TCP port %s is open", address) } return nil } lastErr = err if m.verbose { - fmt.Printf(" TCP port not ready yet (attempt %d/%d): %v\n", i+1, maxRetries, err) + pterm.Info.Printfln("TCP port not ready yet (attempt %d/%d): %v", i+1, maxRetries, err) } time.Sleep(retryDelay) } @@ -301,7 +334,7 @@ func (m *K3dManager) cleanupStaleLockFiles(ctx context.Context) error { } if m.verbose { - fmt.Println("✓ Cleaned up stale kubeconfig lock files") + pterm.Success.Println("Cleaned up stale kubeconfig lock files") } return nil From 77d2c7c7e55b13e341f3e0c0fac6b6f93dbb6b9b Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:09:54 +0000 Subject: [PATCH 2/5] fix(OPENFRAM-005): 8 review findings across 5 files --- internal/chart/prerequisites/helm/helm.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/chart/prerequisites/helm/helm.go b/internal/chart/prerequisites/helm/helm.go index 4d9ae1eb..0d7caea6 100644 --- a/internal/chart/prerequisites/helm/helm.go +++ b/internal/chart/prerequisites/helm/helm.go @@ -10,6 +10,7 @@ import ( "github.com/flamingo-stack/openframe-cli/internal/platform" "github.com/flamingo-stack/openframe-cli/internal/shared/download" "github.com/flamingo-stack/openframe-cli/internal/shared/wsllauncher" + "github.com/flamingo-stack/openframe-cli/internal/ui" ) type HelmInstaller struct{} @@ -95,12 +96,12 @@ func (h *HelmInstaller) installVerified() error { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - fmt.Printf("Downloading verified helm %s...\n", download.Helm.Version) + ui.Info(fmt.Sprintf("Downloading verified helm %s...", download.Helm.Version)) path, err := (download.Downloader{}).InstallPinnedTool(ctx, download.Helm, binDir) if err != nil { return fmt.Errorf("verified helm install failed: %w", err) } download.PrependToPath(binDir) - fmt.Printf("Installed verified helm %s to %s\n", download.Helm.Version, path) + ui.Info(fmt.Sprintf("Installed verified helm %s to %s", download.Helm.Version, path)) return nil } From 94fa69d98020a78c5f934e02453a47465b0c9e76 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:09:55 +0000 Subject: [PATCH 3/5] fix(OPENFRAM-005): 8 review findings across 5 files --- internal/cluster/prerequisites/gcloud/gcloud.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/cluster/prerequisites/gcloud/gcloud.go b/internal/cluster/prerequisites/gcloud/gcloud.go index b528a5e8..0e2c95f0 100644 --- a/internal/cluster/prerequisites/gcloud/gcloud.go +++ b/internal/cluster/prerequisites/gcloud/gcloud.go @@ -4,6 +4,7 @@ package gcloud import ( "context" + "errors" "fmt" "os/exec" "runtime" @@ -18,7 +19,14 @@ var ( lookPath = exec.LookPath runQuiet = func(name string, args ...string) error { cmd := exec.Command(name, args...) // #nosec G204 -- explicit argv, no shell; command and args are internal, not untrusted input - return cmd.Run() + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return fmt.Errorf("command %q exited with code %d: %w", name, exitErr.ExitCode(), err) + } + return err + } + return nil } ) From a2a1ccb16bb82ebc4d5b6cec3ca0cd7ac9590b3a Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:09:56 +0000 Subject: [PATCH 4/5] fix(OPENFRAM-005): 8 review findings across 5 files --- internal/cluster/discovery/auth.go | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/internal/cluster/discovery/auth.go b/internal/cluster/discovery/auth.go index 2eb8a8bb..6d2aba7c 100644 --- a/internal/cluster/discovery/auth.go +++ b/internal/cluster/discovery/auth.go @@ -44,7 +44,20 @@ func NewAuthFlow(exec executor.CommandExecutor) *AuthFlow { cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - return cmd.Run() + if err := cmd.Run(); err != nil { + exitCode := -1 + var exitErr *osexec.ExitError + if errorsAsExitError(err, &exitErr) { + exitCode = exitErr.ExitCode() + } + return &executor.CommandError{ + Command: "gcloud", + Args: args, + ExitCode: exitCode, + Err: err, + } + } + return nil }, } } @@ -125,6 +138,10 @@ func (f *AuthFlow) login(ctx context.Context, prompt string, args []string, veri return fmt.Errorf("%s", manualHint) } if err := f.runLogin(ctx, args...); err != nil { + var cmdErr *executor.CommandError + if errorsAsExitError(err, &cmdErr) { + return err + } return fmt.Errorf("gcloud %s failed: %w", strings.Join(args, " "), err) } if !verified() { @@ -133,3 +150,10 @@ func (f *AuthFlow) login(ctx context.Context, prompt string, args []string, veri pterm.Success.Println("Google Cloud authentication complete") return nil } + +// errorsAsExitError is a small wrapper around errors.As kept local so this +// file only needs the standard "errors" package's As semantics without an +// extra top-level import line churn beyond what's already here. +func errorsAsExitError(err error, target interface{}) bool { + return errorsAs(err, target) +} From 088607d743aa443cb3319821227830b43f5c6da1 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:09:57 +0000 Subject: [PATCH 5/5] fix(OPENFRAM-005): 8 review findings across 5 files --- internal/shared/wsllauncher/install.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/shared/wsllauncher/install.go b/internal/shared/wsllauncher/install.go index bf71f054..c8a62065 100644 --- a/internal/shared/wsllauncher/install.go +++ b/internal/shared/wsllauncher/install.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/flamingo-stack/openframe-cli/internal/shared/executor" "github.com/flamingo-stack/openframe-cli/internal/shared/selfupdate" "github.com/flamingo-stack/openframe-cli/internal/shared/ui/spinner" ) @@ -114,7 +115,7 @@ func localInstallScript(windowsPath string) string { func installLocalBinaryInWSL(windowsPath string) error { cmd := exec.Command("wsl", wslArgv("bash", "-lc", localInstallScript(windowsPath))...) // #nosec G204 -- path is single-quoted into a self-contained script if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("installing local openframe binary into WSL failed: %w\n%s", err, string(out)) + return executor.NewCommandError(cmd, out, err, fmt.Sprintf("installing local openframe binary into WSL failed: %v", err)) } return nil } @@ -146,7 +147,7 @@ func installOpenframeInWSL(version, goarch string) error { cmd.Stdin = bytes.NewReader(binary) if out, err := cmd.CombinedOutput(); err != nil { sp.Fail("Installing openframe inside WSL failed") - return fmt.Errorf("installing openframe inside WSL failed: %w\n%s", err, string(out)) + return executor.NewCommandError(cmd, out, err, fmt.Sprintf("installing openframe inside WSL failed: %v", err)) } sp.Success("OpenFrame is installed inside WSL") return nil