feat(cli): plan-json polish — destructive row in error + effectiveDryRun helper

pkg/cli/update.go

@@ -183,6 +183,22 @@ func (o *UpdateOptions) Run() error {
 	return o.runAll()
 }
 
+// effectiveDryRun reports whether this update invocation should be
+// treated as dry-run by callers that route through this helper.
+// `--dry-run` is the obvious case. `--plan-json` is also dry-run-like
+// because the user only wants plan output and side effects such as
+// file writes, hooks, or lock writes would be surprising.
+//
+// Today the helper centralizes:
+//   - the per-env `cfg.DryRun` decision in `updateEnvs`
+//   - the lock-write suppression at the end of `updateEnvs`
+//
+// New dry-run-like flags should be added here so future code paths
+// that rely on this helper interpret them consistently.
+func (o *UpdateOptions) effectiveDryRun() bool {
+	return o.DryRun || o.PlanJSON
+}
+
 // runAll updates all binaries and envs from config.
 func (o *UpdateOptions) runAll() error {
 	// Update binaries — but NOT in plan-json mode, where binary
@@ -317,8 +333,10 @@ func (o *UpdateOptions) updateEnvs(refs []string) error {
 		if o.Safety != "" {
 			safety = state.NormalizeSafety(o.Safety)
 		}
-		// --plan-json implies --dry-run; the user only wants the plan.
-		isDryRun := o.DryRun || o.PlanJSON
+		// `--plan-json` implies `--dry-run` — the helper hides this
+		// dependency from the rest of the loop so future dry-run-like
+		// flags only need to be added in one place.
+		isDryRun := o.effectiveDryRun()
 
 		// We only need a dry-run "plan" pass when the safety mode might
 		// reject the apply (strict, or prompt where the user has to be
@@ -492,10 +510,12 @@ func (o *UpdateOptions) updateEnvs(refs []string) error {
 		// partial plan generation as success.
 		return aggregateEnvErrors(refusedEnvs, failedEnvs)
 	}
-	if o.DryRun {
-		// Don't write the lock in dry-run mode, but still surface
-		// any per-env refusals or failures so CI and users can
-		// detect that planning was only partially successful.
+	if o.effectiveDryRun() {
+		// Don't write the lock in any dry-run-like mode, but still
+		// surface any per-env refusals or failures so CI and users
+		// can detect that planning was only partially successful.
+		// Routed through effectiveDryRun() so future dry-run-like
+		// flags get the same lock-write suppression for free.
 		return aggregateEnvErrors(refusedEnvs, failedEnvs)
 	}
 
@@ -550,7 +570,8 @@ func (o *UpdateOptions) gateApply(safety string, plan *env.Plan, isDryRun bool)
 		return false, nil
 	}
 
-	destructive := plan.HasDestructive()
+	destructiveRows := plan.DestructiveRows()
+	destructive := len(destructiveRows) > 0
 
 	switch safety {
 	case state.SafetyAuto:
@@ -560,7 +581,8 @@ func (o *UpdateOptions) gateApply(safety string, plan *env.Plan, isDryRun bool)
 	case state.SafetyStrict:
 		// Refuse if any destructive row is present.
 		if destructive {
-			return false, fmt.Errorf("strict safety: plan contains destructive changes — use --safety=prompt or --safety=auto to apply")
+			return false, destructiveRefusalError("strict safety", destructiveRows,
+				"use --safety=prompt or --safety=auto to apply, or --dry-run to preview")
 		}
 		return true, nil
 
@@ -572,7 +594,8 @@ func (o *UpdateOptions) gateApply(safety string, plan *env.Plan, isDryRun bool)
 		// On non-TTY, prompt collapses to strict — refuse on destructive.
 		if !isTTYFunc() {
 			if destructive {
-				return false, fmt.Errorf("prompt safety on non-TTY: plan contains destructive changes — re-run with --yes, --safety=auto, or --dry-run")
+				return false, destructiveRefusalError("prompt safety on non-TTY", destructiveRows,
+					"re-run with --yes or --safety=auto, set safety: auto in b.yaml, or --dry-run to preview")
 			}
 			return true, nil
 		}
@@ -585,6 +608,49 @@ func (o *UpdateOptions) gateApply(safety string, plan *env.Plan, isDryRun bool)
 	return false, fmt.Errorf("unknown safety value %q", safety)
 }
 
+// destructiveRefusalError builds the user-facing error string for a
+// safety gate refusal. The message has three parts to make CI failure
+// triage fast:
+//
+//  1. Which gate refused (so users know what to flip).
+//  2. A breakdown of WHAT is destructive: count by action type, plus
+//     the first row's path so the user has a concrete file to look at
+//     without scrolling back through the printed plan.
+//  3. The recovery hint (which flag/setting to use).
+//
+// Example:
+//
+//	strict safety: plan contains 2 overwrite, 1 conflict
+//	  (first: hetzner/legacy.yaml) — use --safety=prompt or
+//	  --safety=auto to apply, or --dry-run to preview
+//
+// Per N1 from PR #128 reviewer feedback.
+func destructiveRefusalError(gate string, rows []env.PlanRow, recovery string) error {
+	if len(rows) == 0 {
+		return fmt.Errorf("%s: plan contains destructive changes — %s", gate, recovery)
+	}
+	// Count by action.
+	var overwrite, conflict int
+	for _, r := range rows {
+		switch r.Action {
+		case env.PlanOverwrite:
+			overwrite++
+		case env.PlanConflict:
+			conflict++
+		}
+	}
+	var parts []string
+	if overwrite > 0 {
+		parts = append(parts, fmt.Sprintf("%d overwrite", overwrite))
+	}
+	if conflict > 0 {
+		parts = append(parts, fmt.Sprintf("%d conflict", conflict))
+	}
+	breakdown := strings.Join(parts, ", ")
+	first := rows[0].Dest
+	return fmt.Errorf("%s: plan contains %s (first: %s) — %s", gate, breakdown, first, recovery)
+}
+
 // confirmApply prompts the user with a y/N question. Default is N (safer).
 func (o *UpdateOptions) confirmApply(destructive bool) bool {
 	r := o.stdinReader

pkg/cli/update_test.go

@@ -1832,6 +1832,17 @@ func TestUpdateEnvs_SafetyStrict_RefusesDestructive(t *testing.T) {
 	if !strings.Contains(errBuf.String(), "strict safety") {
 		t.Errorf("expected strict refusal message, got: %q", errBuf.String())
 	}
+	// The improved error message includes a count of destructive
+	// rows AND the first destructive row's path so users can find
+	// the offending file without scrolling. The fixture above uses
+	// "a.yaml" with "replaced (local changes overwritten)" which
+	// maps to PlanOverwrite. Per N1 from PR #128 reviewer feedback.
+	if !strings.Contains(errBuf.String(), "1 overwrite") {
+		t.Errorf("expected destructive count in error message, got: %q", errBuf.String())
+	}
+	if !strings.Contains(errBuf.String(), "first: a.yaml") {
+		t.Errorf("expected first destructive path in error message, got: %q", errBuf.String())
+	}
 }
 
 // TestUpdateEnvs_SafetyStrict_AppliesNonDestructive: strict ALLOWS plans

pkg/env/plan.go

@@ -78,6 +78,21 @@ func (p *Plan) HasDestructive() bool {
 	return false
 }
 
+// DestructiveRows returns every plan row whose action is destructive
+// (currently `overwrite` or `conflict`). Rows are returned in the
+// same order they appear in Plan.Rows, which PlanFromResult sorts by
+// destination path — so callers that pin "the first destructive row"
+// for diagnostic messages get a stable, dest-sorted choice.
+func (p *Plan) DestructiveRows() []PlanRow {
+	var out []PlanRow
+	for _, r := range p.Rows {
+		if r.Action.IsDestructive() {
+			out = append(out, r)
+		}
+	}
+	return out
+}
+
 // CountByAction returns a map[action]count over the plan's rows. Useful
 // for summary lines like "12 add, 3 update, 1 conflict".
 func (p *Plan) CountByAction() map[PlanAction]int {