From 9e0e2a74512ef0fa532a8d70101c7f44703caf3e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:25 +0000 Subject: [PATCH 01/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/chart/providers/argocd/wait.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/internal/chart/providers/argocd/wait.go b/internal/chart/providers/argocd/wait.go index 6542fafa..2ad74b93 100644 --- a/internal/chart/providers/argocd/wait.go +++ b/internal/chart/providers/argocd/wait.go @@ -131,7 +131,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn // Function to stop spinner safely stopSpinner := func() { - dash.Stop() + if dash != nil { + dash.Stop() + } spinnerMutex.Lock() defer spinnerMutex.Unlock() if !spinnerStopped && spinner != nil { @@ -290,7 +292,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn case <-ticker.C: // Check timeout if time.Since(startTime) > timeout { - dash.Fail(fmt.Sprintf("Timeout after %v", timeout)) + if dash != nil { + dash.Fail(fmt.Sprintf("Timeout after %v", timeout)) + } spinnerMutex.Lock() if !spinnerStopped && spinner != nil { spinner.Fail(fmt.Sprintf("Timeout after %v", timeout)) @@ -467,7 +471,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn // staleness checks use the same tick. now := time.Now() if fatal := fatalManifest.observe(apps, now); len(fatal) > 0 { - dash.Fail("Applications cannot render manifests from the deployed revision") + if dash != nil { + dash.Fail("Applications cannot render manifests from the deployed revision") + } spinnerMutex.Lock() if !spinnerStopped && spinner != nil { spinner.Fail("Applications cannot render manifests from the deployed revision") @@ -489,7 +495,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn // events, so it says WHY, not just that it hung. if cand := degraded.observe(apps, now); len(cand) > 0 { if diag, stuck := m.diagnoseFailingApps(localCtx, cand); len(stuck) > 0 { - dash.Fail("An application is Degraded with a workload that will not recover") + if dash != nil { + dash.Fail("An application is Degraded with a workload that will not recover") + } spinnerMutex.Lock() if !spinnerStopped && spinner != nil { spinner.Fail("An application is Degraded with a workload that will not recover") @@ -541,7 +549,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn // this the default experience was a static "Installing ArgoCD // applications..." for up to the full 60m timeout, with no way to tell // a working install from a wedged one. - dash.Update(currentlyReady, totalApps, apps) + if dash != nil { + dash.Update(currentlyReady, totalApps, apps) + } if totalApps > 0 { spinnerMutex.Lock() if !spinnerStopped && spinner != nil { @@ -724,7 +734,9 @@ func (m *Manager) WaitForApplications(ctx context.Context, config config.ChartIn spinnerMutex.Unlock() if len(mm) > 0 { - dash.Fail("Deployed ref does not match the requested ref") + if dash != nil { + dash.Fail("Deployed ref does not match the requested ref") + } return refMismatchError(config.AppOfApps.GitHubBranch, mm) } From 8b802bba50e9b6c6fb7940b50c145b2a3a6c71a0 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:26 +0000 Subject: [PATCH 02/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/cluster/utils/cmd_helpers.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/internal/cluster/utils/cmd_helpers.go b/internal/cluster/utils/cmd_helpers.go index c9ae8168..83779628 100644 --- a/internal/cluster/utils/cmd_helpers.go +++ b/internal/cluster/utils/cmd_helpers.go @@ -31,16 +31,7 @@ func InitGlobalFlags() { // GetCommandService creates a command service for business logic operations func GetCommandService() *cluster.ClusterService { - // Use injected executor if available (for testing) - if globalFlags != nil && globalFlags.Executor != nil { - return cluster.NewClusterService(globalFlags.Executor) - } - - // Create real executor with current flags - dryRun := globalFlags != nil && globalFlags.Global != nil && globalFlags.Global.DryRun - verbose := globalFlags != nil && globalFlags.Global != nil && globalFlags.Global.Verbose - exec := executor.NewRealCommandExecutor(dryRun, verbose) - return cluster.NewClusterService(exec) + return cluster.NewClusterService(CommandExecutor()) } // CommandExecutor returns the executor commands should shell through: the From 02525e0a1f9adfa8e60e0e50d96c83215394e822 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:27 +0000 Subject: [PATCH 03/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/cluster/providers/gke/provider.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/cluster/providers/gke/provider.go b/internal/cluster/providers/gke/provider.go index cef5fd07..29668ef2 100644 --- a/internal/cluster/providers/gke/provider.go +++ b/internal/cluster/providers/gke/provider.go @@ -26,6 +26,13 @@ import ( // out of a terraform 409 error so an orphan can be named concretely. var gcpResourcePathRE = regexp.MustCompile(`projects/[^'"\s]+`) +// safeClusterNameRE matches GKE's own cluster-name constraints (lowercase +// alphanumeric and hyphens, starting with a letter). Any name that fails this +// check cannot be a valid GKE cluster name anyway, so rejecting it here also +// guarantees it can never be used to inject syntax into a gcloud --filter +// expression (spaces, "OR", parentheses, quotes, etc.). +var safeClusterNameRE = regexp.MustCompile(`^[a-z]([-a-z0-9]*[a-z0-9])?$`) + // orphanFromInterruptedCreate detects the specific failure where terraform // tries to create a resource that already exists in GCP (HTTP 409 / // alreadyExists). This is the signature of a create interrupted (SIGINT) after @@ -153,7 +160,17 @@ func (p *Provider) ensureProjectServices(ctx context.Context, project string) er // line equal to the cluster name. Anything else — non-zero exit, empty or // unrelated output — is treated as "does not exist"; a genuinely broken API // call fails later with a clearer terraform error anyway. +// +// config.Name is rejected up front unless it matches GKE's own cluster-name +// character set (lowercase alphanumeric and hyphens). This is not just +// defense in depth: it also guarantees the name cannot contain characters +// meaningful to gcloud's --filter expression syntax (spaces, "OR", +// parentheses, quotes), so it cannot alter the filter's semantics and bypass +// this collision guard. func (p *Provider) preflightNameCollision(ctx context.Context, config models.ClusterConfig) error { + if !safeClusterNameRE.MatchString(config.Name) { + return fmt.Errorf("cluster name %q is not a valid GKE cluster name (must match %s)", config.Name, safeClusterNameRE.String()) + } result, err := p.executor.Execute(ctx, "gcloud", "container", "clusters", "list", "--project", config.Cloud.Project, "--filter=name="+config.Name, "--format=value(name)") if err != nil || result == nil { From 334db0a4f2f7b2ae57337fca0520b4b568bc3cfd Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:28 +0000 Subject: [PATCH 04/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- cmd/app/upgrade.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/cmd/app/upgrade.go b/cmd/app/upgrade.go index 294fdbe5..4076bbf5 100644 --- a/cmd/app/upgrade.go +++ b/cmd/app/upgrade.go @@ -106,7 +106,9 @@ func runUpgradeChangeRef(cmd *cobra.Command, args []string, flags *InstallFlags, // let the wait sync them once progress stalls instead of timing out (N3). req.SyncStragglersOnStall = true - pterm.Info.Printf("Upgrading OpenFrame to ref %q\n", flags.resolvedRef()) + if !flags.Silent { + pterm.Info.Printf("Upgrading OpenFrame to ref %q\n", flags.resolvedRef()) + } if err := services.InstallChartsWithConfigContext(cmd.Context(), req); err != nil { return sharedErrors.HandleGlobalError(err, verbose) } @@ -135,10 +137,12 @@ func runUpgradeForceSync(cmd *cobra.Command, args []string, flags *InstallFlags, return sharedErrors.HandleGlobalError(previewOutOfSync(cmd.Context(), manager, verbose, prune), verbose) } - if prune { + if prune && !flags.Silent { pterm.Warning.Println("Refreshing and syncing with --prune: resources removed from git will be DELETED.") } - pterm.Info.Println("Refreshing and syncing the OpenFrame platform...") + if !flags.Silent { + pterm.Info.Println("Refreshing and syncing the OpenFrame platform...") + } if err := manager.RefreshAndSync(cmd.Context(), prune); err != nil { return sharedErrors.HandleGlobalError(err, verbose) } @@ -157,7 +161,9 @@ func runUpgradeForceSync(cmd *cobra.Command, args []string, flags *InstallFlags, if err := manager.WaitForApplications(cmd.Context(), waitCfg); err != nil { return sharedErrors.HandleGlobalError(err, verbose) } - pterm.Success.Println("OpenFrame platform re-synced.") + if !flags.Silent { + pterm.Success.Println("OpenFrame platform re-synced.") + } return nil } @@ -203,7 +209,16 @@ func resolveUpgradeTarget(cmd *cobra.Command, args []string, flags *InstallFlags if err != nil { return nil, "", fmt.Errorf("could not use context %q: %w", contextName, err) } - return cfg, clusterNameArg(args), nil + // A positional cluster name, if also given, is only a label for this + // --context-resolved config (it does not re-resolve the context), so + // only use it when the two cannot conflict: no --context-derived name + // of its own exists. Otherwise prefer contextName itself so the + // ClusterName reported in wait/status output matches the config that + // was actually resolved. + if name := clusterNameArg(args); name != "" { + return cfg, name, nil + } + return cfg, contextName, nil } if name := clusterNameArg(args); name != "" { From c20b9fda6077957a1db51e024954ef5f0b706724 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:29 +0000 Subject: [PATCH 05/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/cluster/discovery/gke.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/cluster/discovery/gke.go b/internal/cluster/discovery/gke.go index a996bfa0..09f61c71 100644 --- a/internal/cluster/discovery/gke.go +++ b/internal/cluster/discovery/gke.go @@ -74,6 +74,9 @@ func (d *GKEDiscoverer) configurations(ctx context.Context) ([]gcloudConfigurati if err != nil { return nil, fmt.Errorf("listing gcloud configurations: %w", err) } + if result == nil { + return nil, nil + } var configs []gcloudConfiguration if err := json.Unmarshal([]byte(result.Stdout), &configs); err != nil { return nil, fmt.Errorf("parsing gcloud configurations: %w", err) From f79bad1a4d953a7befa205a7549045ef23fcb476 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:30 +0000 Subject: [PATCH 06/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/cluster/discovery/eks.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/cluster/discovery/eks.go b/internal/cluster/discovery/eks.go index 9bcf4fa3..f5346fe8 100644 --- a/internal/cluster/discovery/eks.go +++ b/internal/cluster/discovery/eks.go @@ -200,6 +200,9 @@ func (d *EKSDiscoverer) describe(ctx context.Context, profile, region, name stri if err != nil { return models.ClusterInfo{}, "", fmt.Errorf("describing cluster %s: %w", name, err) } + if result == nil { + return models.ClusterInfo{}, "", fmt.Errorf("unparseable describe-cluster for %s", name) + } var c eksCluster if err := json.Unmarshal([]byte(result.Stdout), &c); err != nil { return models.ClusterInfo{}, "", fmt.Errorf("unparseable describe-cluster for %s", name) From 2acd7a77e3d5fcf52c1876b6c6df6dc898154db1 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:31 +0000 Subject: [PATCH 07/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/chart/prerequisites/certificates/certificates.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/chart/prerequisites/certificates/certificates.go b/internal/chart/prerequisites/certificates/certificates.go index a1e27e57..77d4e69c 100644 --- a/internal/chart/prerequisites/certificates/certificates.go +++ b/internal/chart/prerequisites/certificates/certificates.go @@ -177,11 +177,11 @@ func (c *CertificateInstaller) generateCertificates() error { if commandExists("apt-get") { updateCmd := exec.Command("sudo", "apt-get", "update", "-y") if err := updateCmd.Run(); err != nil { - pterm.Debug.Printf("apt-get update failed (certutil install is optional): %v\n", err) + pterm.Warning.Printf("apt-get update failed (certutil install is optional, NSS trust for Firefox/Chrome will not be configured): %v\n", err) } installCmd := exec.Command("sudo", "apt-get", "install", "-y", "libnss3-tools", "ca-certificates") if err := installCmd.Run(); err != nil { - pterm.Debug.Printf("apt-get install of certutil/ca-certificates failed (optional): %v\n", err) + pterm.Warning.Printf("apt-get install of certutil/ca-certificates failed (optional, NSS trust for Firefox/Chrome will not be configured): %v\n", err) } } } From 85921034923e40a7cdc029e70a9bf4061cbb669e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:32 +0000 Subject: [PATCH 08/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/chart/providers/argocd/refassert.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/chart/providers/argocd/refassert.go b/internal/chart/providers/argocd/refassert.go index 67947bac..8d6b62ac 100644 --- a/internal/chart/providers/argocd/refassert.go +++ b/internal/chart/providers/argocd/refassert.go @@ -101,5 +101,6 @@ func refMismatchError(requestedRef string, m []refMismatch) error { fmt.Fprintf(&b, " - %s is on %q, not %q\n", x.App, x.Got, x.Want) } b.WriteString("Use a branch whose chart reads repository.branch, or pin these applications' targetRevision by hand.") - return fmt.Errorf("%s", b.String()) + return selfDiagnosedError{msg: b.String()} } + From c7a8bd18f508cb4fa513ea2bf8ce279914857e92 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:33 +0000 Subject: [PATCH 09/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/chart/providers/argocd/sync.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/chart/providers/argocd/sync.go b/internal/chart/providers/argocd/sync.go index d9969eb0..2f68f94b 100644 --- a/internal/chart/providers/argocd/sync.go +++ b/internal/chart/providers/argocd/sync.go @@ -246,6 +246,8 @@ type syncGroup struct { // (legacy manifests → caller keeps the ungated single-pass behaviour). A child // missing the label — or carrying a non-numeric value — on an otherwise // labeled install falls into defaultSyncGroup, mirroring the template default. +// A non-numeric value (e.g. a typo like "1a") is surfaced with a warning +// rather than silently absorbed, since it is likely a misconfiguration. func groupChildren(children []unstructured.Unstructured) (groups []syncGroup, labeled bool) { byNumber := map[int][]string{} var all []string @@ -257,6 +259,9 @@ func groupChildren(children []unstructured.Unstructured) (groups []syncGroup, la labeled = true if n, err := strconv.Atoi(strings.TrimSpace(v)); err == nil { group = n + } else { + pterm.Warning.Printf("Application %q has a non-numeric %s label (%q); defaulting to sync group %d\n", + name, SyncGroupLabel, v, defaultSyncGroup) } } byNumber[group] = append(byNumber[group], name) From 1cf1493534842345cb8d31c3b21a256f81064b21 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:34 +0000 Subject: [PATCH 10/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- internal/cluster/ui/wizard.go | 139 ++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 67 deletions(-) diff --git a/internal/cluster/ui/wizard.go b/internal/cluster/ui/wizard.go index 361da5c0..bc993e08 100644 --- a/internal/cluster/ui/wizard.go +++ b/internal/cluster/ui/wizard.go @@ -74,93 +74,98 @@ func (w *ConfigWizard) Run() (ClusterConfig, error) { steps := NewWizardSteps() - // Step 1: Cluster name - name, err := steps.PromptClusterName(w.config.Name) - if err != nil { - return ClusterConfig{}, err - } - w.config.Name = name - - // Step 2: Cluster type - clusterType, err := steps.PromptClusterType() - if err != nil { - return ClusterConfig{}, err - } - w.config.Type = clusterType + // Loop instead of recursing so repeated "no" answers reuse the same + // stack frame and state rather than growing the call stack indefinitely. + for { + // Step 1: Cluster name + name, err := steps.PromptClusterName(w.config.Name) + if err != nil { + return ClusterConfig{}, err + } + w.config.Name = name - // Step 3 (cloud only): project/region + instance type. The k3s version - // list below is meaningless for cloud clusters, whose version comes from - // the module default. - if clusterType == models.ClusterTypeEKS || clusterType == models.ClusterTypeGKE { - if clusterType == models.ClusterTypeGKE { - project, err := steps.PromptProject() - if err != nil { - return ClusterConfig{}, err + // Step 2: Cluster type + clusterType, err := steps.PromptClusterType() + if err != nil { + return ClusterConfig{}, err + } + w.config.Type = clusterType + + // Step 3 (cloud only): project/region + instance type. The k3s version + // list below is meaningless for cloud clusters, whose version comes from + // the module default. + if clusterType == models.ClusterTypeEKS || clusterType == models.ClusterTypeGKE { + if clusterType == models.ClusterTypeGKE { + project, err := steps.PromptProject() + if err != nil { + return ClusterConfig{}, err + } + w.config.Project = project + + region, err := steps.PromptRegion("GCP Region", "us-central1", w.config.Project) + if err != nil { + return ClusterConfig{}, err + } + w.config.Region = region + } else { + profile, err := steps.PromptProfile() + if err != nil { + return ClusterConfig{}, err + } + w.config.Profile = profile + + region, err := steps.PromptAWSRegion("AWS Region", "us-east-1", w.config.Profile) + if err != nil { + return ClusterConfig{}, err + } + w.config.Region = region } - w.config.Project = project - region, err := steps.PromptRegion("GCP Region", "us-central1", w.config.Project) - if err != nil { - return ClusterConfig{}, err + // Mirrors the template default: the Free-Tier-eligible drop-in for + // m6i.large, so the wizard's suggestion also works on a new AWS account. + defaultMachine := "m7i-flex.large" + if clusterType == models.ClusterTypeGKE { + defaultMachine = "e2-standard-4" } - w.config.Region = region - } else { - profile, err := steps.PromptProfile() + machineType, err := steps.PromptMachineType(defaultMachine) if err != nil { return ClusterConfig{}, err } - w.config.Profile = profile - - region, err := steps.PromptAWSRegion("AWS Region", "us-east-1", w.config.Profile) - if err != nil { - return ClusterConfig{}, err - } - w.config.Region = region + w.config.MachineType = machineType + w.config.K8sVersion = "" } - // Mirrors the template default: the Free-Tier-eligible drop-in for - // m6i.large, so the wizard's suggestion also works on a new AWS account. - defaultMachine := "m7i-flex.large" - if clusterType == models.ClusterTypeGKE { - defaultMachine = "e2-standard-4" - } - machineType, err := steps.PromptMachineType(defaultMachine) + // Step 4: Node count + nodeCount, err := steps.PromptNodeCount(w.config.NodeCount) if err != nil { return ClusterConfig{}, err } - w.config.MachineType = machineType - w.config.K8sVersion = "" - } + w.config.NodeCount = nodeCount - // Step 4: Node count - nodeCount, err := steps.PromptNodeCount(w.config.NodeCount) - if err != nil { - return ClusterConfig{}, err - } - w.config.NodeCount = nodeCount + // Step 5 (k3d only): Kubernetes version + if clusterType == models.ClusterTypeK3d { + k8sVersion, err := steps.PromptK8sVersion() + if err != nil { + return ClusterConfig{}, err + } + w.config.K8sVersion = k8sVersion + } - // Step 5 (k3d only): Kubernetes version - if clusterType == models.ClusterTypeK3d { - k8sVersion, err := steps.PromptK8sVersion() + // Step 6: Confirmation + domainConfig := w.config.ToDomain() + confirmed, err := steps.ConfirmConfiguration(domainConfig) if err != nil { return ClusterConfig{}, err } - w.config.K8sVersion = k8sVersion - } - // Step 6: Confirmation - domainConfig := w.config.ToDomain() - confirmed, err := steps.ConfirmConfiguration(domainConfig) - if err != nil { - return ClusterConfig{}, err - } + if !confirmed { + // User wants to modify - restart wizard from step 1, reusing + // the current state instead of recursing. + continue + } - if !confirmed { - // User wants to modify - restart wizard - return w.Run() + return w.config, nil } - - return w.config, nil } // ConfigurationHandler handles cluster configuration flows From 54a2298e4bd6009f3243ba510bed70a9b1315a76 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:35 +0000 Subject: [PATCH 11/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- .../cluster/providers/terraform/engine.go | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/internal/cluster/providers/terraform/engine.go b/internal/cluster/providers/terraform/engine.go index 73735189..eb4cc2ea 100644 --- a/internal/cluster/providers/terraform/engine.go +++ b/internal/cluster/providers/terraform/engine.go @@ -13,6 +13,7 @@ import ( "github.com/flamingo-stack/openframe-cli/internal/shared/download" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" + "github.com/pterm/pterm" ) // Runner is the subset of *tfexec.Terraform the engine uses; an interface so @@ -145,16 +146,25 @@ const OpLogName = "terraform.log" // propagates a sink error: exec.Cmd returns a stdout-writer error from Wait, // so a disk filling up mid-apply would report the terraform run as FAILED // while terraform actually completed and changed resources. The log is a -// record of the operation — it must never decide its outcome. +// record of the operation — it must never decide its outcome. The first sink +// write failure is still surfaced once, at WARN level, so a user relying on +// OpLogName for post-mortem debugging isn't silently left with a truncated +// log and no indication it happened. type bestEffortTee struct { progress io.Writer sink io.Writer // nil once a write failed; never re-enabled + logPath string // used only for the one-time warning below + warned bool } func (t *bestEffortTee) Write(p []byte) (int, error) { if t.sink != nil { if _, err := t.sink.Write(p); err != nil { t.sink = nil + if !t.warned { + t.warned = true + pterm.Warning.Printfln("operation log %s stopped recording (write error: %v); the on-disk log will be incomplete", t.logPath, err) + } } } return t.progress.Write(p) @@ -164,7 +174,8 @@ func (t *bestEffortTee) Write(p []byte) (int, error) { // writer, teed into dir's terraform.log when it can be opened. Logging is // best-effort end to end — a directory that cannot take the log (read-only, // gone) skips it, and a write failure after opening only stops the mirroring -// (bestEffortTee above); neither may ever fail the operation itself. +// (bestEffortTee above, which also warns once); neither may ever fail the +// operation itself. // The returned close is always safe to call; logPath is empty when no log is // being written. func (e *Engine) opSinks(dir, op string) (w io.Writer, close func(), logPath string) { @@ -173,8 +184,12 @@ func (e *Engine) opSinks(dir, op string) (w io.Writer, close func(), logPath str if err != nil { return progress, func() {}, "" } - fmt.Fprintf(f, "=== terraform %s — %s ===\n", op, time.Now().UTC().Format(time.RFC3339)) - return &bestEffortTee{progress: progress, sink: f}, func() { _ = f.Close() }, f.Name() + if _, err := fmt.Fprintf(f, "=== terraform %s — %s ===\n", op, time.Now().UTC().Format(time.RFC3339)); err != nil { + pterm.Warning.Printfln("operation log %s could not be written to (%v); the on-disk log will be incomplete", f.Name(), err) + _ = f.Close() + return progress, func() {}, "" + } + return &bestEffortTee{progress: progress, sink: f, logPath: f.Name()}, func() { _ = f.Close() }, f.Name() } // opFailure wraps a failed apply/destroy, pointing at the full log when one From c377c877687fb273c17b6aacd10a4ec891731e80 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 05:07:36 +0000 Subject: [PATCH 12/12] fix(adhoc-sweep-fixes): 14 review findings across 12 files --- tests/integration/common/cluster_management_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/integration/common/cluster_management_test.go b/tests/integration/common/cluster_management_test.go index 04140d45..05386fb4 100644 --- a/tests/integration/common/cluster_management_test.go +++ b/tests/integration/common/cluster_management_test.go @@ -28,9 +28,7 @@ func TestCreateTestCluster(t *testing.T) { defer CleanupTestCluster(name) err := CreateTestCluster(name) - if err != nil { - t.Logf("Failed to create cluster (may be expected): %v", err) - } + assert.NoError(t, err, "Failed to create cluster") } func TestClusterExists(t *testing.T) {