fix(env): prune orphaned lock entries + idempotent UpsertEnv

pkg/cli/env_test.go

@@ -603,6 +603,104 @@ func TestUpdateEnvs_DryRun_SkipsLockWrite(t *testing.T) {
 	}
 }
 
+// TestUpdateEnvs_PrunesOrphanedLockEntries verifies that a lock env entry no
+// longer present in b.yaml (e.g. a renamed label) is pruned on update — even
+// when the live env is up-to-date — so `b verify` stops reporting its stale
+// hashes. Regression for the kubehz-cluster case: lock held #main (configured)
+// plus an orphaned #kubeone twin sharing a dest with a stale hash.
+func TestUpdateEnvs_PrunesOrphanedLockEntries(t *testing.T) {
+	saveHooks(t)
+
+	tmpDir := t.TempDir()
+	lk := &lock.Lock{
+		Envs: []lock.EnvEntry{
+			{Ref: "git@github.com:org/lok8s", Label: "main", Commit: "abc",
+				Files: []lock.LockFile{{Path: "render.sh", Dest: ".lok8s/render.sh", SHA256: "good"}}},
+			{Ref: "git@github.com:org/lok8s", Label: "kubeone", Commit: "old",
+				Files: []lock.LockFile{{Path: "render.sh", Dest: ".lok8s/render.sh", SHA256: "stale"}}},
+		},
+	}
+	lock.WriteLock(tmpDir, lk, "v1.0")
+
+	// #main is up to date → SyncEnv skips; the prune must still run + persist.
+	syncEnvFunc = func(cfg env.EnvConfig, projectRoot, cacheRoot string, lockEntry *lock.EnvEntry) (*env.SyncResult, error) {
+		return &env.SyncResult{Ref: cfg.Ref, Label: cfg.Label, Commit: "abc", Skipped: true, Message: "(up to date)"}, nil
+	}
+
+	errOut := &bytes.Buffer{}
+	io := &streams.IO{Out: &bytes.Buffer{}, ErrOut: errOut}
+	shared := NewSharedOptions(io, nil)
+	shared.Config = &state.State{
+		Envs: state.EnvList{
+			{Key: "git@github.com:org/lok8s#main"}, // only #main configured
+		},
+	}
+	shared.loadedConfigPath = filepath.Join(tmpDir, "b.yaml")
+	shared.bVersion = "v1.0"
+
+	o := &UpdateOptions{SharedOptions: shared}
+	if err := o.updateEnvs(nil); err != nil {
+		t.Fatalf("updateEnvs error: %v", err)
+	}
+
+	lk2, _ := lock.ReadLock(tmpDir)
+	if len(lk2.Envs) != 1 {
+		t.Fatalf("expected 1 env after prune, got %d", len(lk2.Envs))
+	}
+	if lk2.Envs[0].Label != "main" {
+		t.Errorf("kept env label = %q, want main", lk2.Envs[0].Label)
+	}
+	if lk2.FindEnv("git@github.com:org/lok8s", "kubeone") != nil {
+		t.Error("orphaned #kubeone entry should be pruned")
+	}
+	if !strings.Contains(errOut.String(), "pruned") {
+		t.Errorf("expected prune notice on stderr, got: %q", errOut.String())
+	}
+}
+
+// TestUpdateEnvs_DedupsConfiguredLockEntries covers Copilot's edge case: two
+// lock entries for the SAME configured (ref,label). When the env is up-to-date
+// SyncEnv skips it, so UpsertEnv never collapses the twin — the normalize pass
+// must dedup configured keys (keep first) so the rewritten lock is clean.
+func TestUpdateEnvs_DedupsConfiguredLockEntries(t *testing.T) {
+	saveHooks(t)
+
+	tmpDir := t.TempDir()
+	lk := &lock.Lock{
+		Envs: []lock.EnvEntry{
+			{Ref: "github.com/org/infra", Label: "", Commit: "abc",
+				Files: []lock.LockFile{{Path: "a.yaml", Dest: "a.yaml", SHA256: "good"}}},
+			{Ref: "github.com/org/infra", Label: "", Commit: "abc",
+				Files: []lock.LockFile{{Path: "a.yaml", Dest: "a.yaml", SHA256: "stale-twin"}}},
+		},
+	}
+	lock.WriteLock(tmpDir, lk, "v1.0")
+
+	syncEnvFunc = func(cfg env.EnvConfig, projectRoot, cacheRoot string, lockEntry *lock.EnvEntry) (*env.SyncResult, error) {
+		return &env.SyncResult{Ref: cfg.Ref, Label: cfg.Label, Commit: "abc", Skipped: true, Message: "(up to date)"}, nil
+	}
+
+	io := &streams.IO{Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}}
+	shared := NewSharedOptions(io, nil)
+	shared.Config = &state.State{Envs: state.EnvList{{Key: "github.com/org/infra"}}}
+	shared.loadedConfigPath = filepath.Join(tmpDir, "b.yaml")
+	shared.bVersion = "v1.0"
+
+	o := &UpdateOptions{SharedOptions: shared}
+	if err := o.updateEnvs(nil); err != nil {
+		t.Fatalf("updateEnvs error: %v", err)
+	}
+
+	lk2, _ := lock.ReadLock(tmpDir)
+	if len(lk2.Envs) != 1 {
+		t.Fatalf("expected 1 env after dedup, got %d", len(lk2.Envs))
+	}
+	// First occurrence kept (the up-to-date / FindEnv-visible one).
+	if len(lk2.Envs[0].Files) != 1 || lk2.Envs[0].Files[0].SHA256 != "good" {
+		t.Errorf("expected the first entry kept (good hash), got %+v", lk2.Envs[0].Files)
+	}
+}
+
 // --- Feature 10: group filtering ---
 
 func TestUpdateEnvs_GroupFilter(t *testing.T) {

pkg/cli/update.go

@@ -827,12 +827,67 @@ func (o *UpdateOptions) updateEnvs(refs []string) error {
 		return aggregateEnvErrors(refusedEnvs, failedEnvs)
 	}
 
+	// Reconcile the lock to the config before writing: drop env entries no
+	// longer in b.yaml (e.g. an env whose label was renamed). Such orphans are
+	// never re-synced (FindEnv/UpsertEnv key on the configured ref+label) yet
+	// `b verify` still checks them, so their stale hashes report mismatches
+	// forever for dests a live env now owns.
+	o.pruneOrphanedEnvs(lk)
+
 	if err := lock.WriteLock(lockDir, lk, o.bVersion); err != nil {
 		return err
 	}
 	return aggregateEnvErrors(refusedEnvs, failedEnvs)
 }
 
+// pruneOrphanedEnvs normalizes the lock's env entries against the config,
+// returning the number removed. It drops entries whose (ref,label) is not in
+// b.yaml (orphans, e.g. a renamed label) AND collapses duplicates of the same
+// configured (ref,label), keeping the first occurrence. The dedup matters when
+// the env is up-to-date this run: SyncEnv skips it so UpsertEnv never runs to
+// collapse a same-key twin, yet the lock is still rewritten — without this, the
+// stale duplicate would survive and `b verify` would keep flagging it.
+//
+// Keys are derived exactly as updateEnvs writes them (gitcache.RefBase/RefLabel
+// of the config key), so a live config env — even one not synced this run
+// (group/arg filtered, or up-to-date) — is never pruned. No-op when no env is
+// configured.
+func (o *UpdateOptions) pruneOrphanedEnvs(lk *lock.Lock) int {
+	if lk == nil || o.Config == nil || len(o.Config.Envs) == 0 {
+		return 0
+	}
+	configured := make(map[string]bool, len(o.Config.Envs))
+	for _, e := range o.Config.Envs {
+		configured[gitcache.RefBase(e.Key)+"\x00"+gitcache.RefLabel(e.Key)] = true
+	}
+	seen := make(map[string]bool, len(lk.Envs))
+	kept := lk.Envs[:0]
+	var pruned []string
+	for _, e := range lk.Envs {
+		key := e.Ref + "\x00" + e.Label
+		display := e.Ref
+		if e.Label != "" {
+			display += "#" + e.Label
+		}
+		switch {
+		case !configured[key]:
+			pruned = append(pruned, display+" (not in b.yaml)")
+		case seen[key]:
+			pruned = append(pruned, display+" (duplicate)")
+		default:
+			seen[key] = true
+			kept = append(kept, e)
+		}
+	}
+	if len(pruned) == 0 {
+		return 0
+	}
+	lk.Envs = kept
+	fmt.Fprintf(o.IO.ErrOut, "  pruned %d stale env entry(ies) from b.lock: %s\n",
+		len(pruned), strings.Join(pruned, ", "))
+	return len(pruned)
+}
+
 // aggregateEnvErrors returns a single error summarizing safety refusals
 // and hard sync failures, or nil when neither happened. Both lists are
 // reported when both are non-empty so the user sees the full story in

pkg/lock/lock.go

@@ -139,15 +139,27 @@ func (l *Lock) RemoveEnv(ref, label string) bool {
 	return false
 }
 
-// UpsertEnv adds or updates an env entry in the lock.
+// UpsertEnv adds or updates an env entry in the lock. It replaces the first
+// matching (ref,label) entry in place and drops any further duplicates, so the
+// write is idempotent: a lock that somehow accumulated two entries for the same
+// env collapses to one here instead of leaving a stale twin behind.
 func (l *Lock) UpsertEnv(entry EnvEntry) {
-	for i := range l.Envs {
-		if l.Envs[i].Ref == entry.Ref && l.Envs[i].Label == entry.Label {
-			l.Envs[i] = entry
-			return
+	replaced := false
+	kept := l.Envs[:0]
+	for _, e := range l.Envs {
+		if e.Ref == entry.Ref && e.Label == entry.Label {
+			if !replaced {
+				kept = append(kept, entry)
+				replaced = true
+			}
+			continue // drop duplicate
 		}
+		kept = append(kept, e)
+	}
+	if !replaced {
+		kept = append(kept, entry)
 	}
-	l.Envs = append(l.Envs, entry)
+	l.Envs = kept
 }
 
 // SHA256File computes the SHA256 checksum of a file.

pkg/lock/lock_test.go

@@ -190,6 +190,41 @@ func TestUpsertEnv(t *testing.T) {
 	}
 }
 
+// TestUpsertEnv_CollapsesDuplicates verifies UpsertEnv is idempotent: a lock
+// that already holds two entries for the same (ref,label) collapses to a single
+// updated entry, rather than leaving a stale duplicate behind.
+func TestUpsertEnv_CollapsesDuplicates(t *testing.T) {
+	lk := &Lock{
+		Envs: []EnvEntry{
+			{Ref: "github.com/org/infra", Label: "main", Commit: "stale"},
+			{Ref: "github.com/org/other", Label: "", Commit: "keep"},
+			{Ref: "github.com/org/infra", Label: "main", Commit: "alsostale"},
+		},
+	}
+
+	lk.UpsertEnv(EnvEntry{Ref: "github.com/org/infra", Label: "main", Commit: "fresh"})
+
+	// Exactly one infra#main entry, with the fresh commit; the other env intact.
+	var infra int
+	for _, e := range lk.Envs {
+		if e.Ref == "github.com/org/infra" && e.Label == "main" {
+			infra++
+			if e.Commit != "fresh" {
+				t.Errorf("infra#main commit = %q, want fresh", e.Commit)
+			}
+		}
+	}
+	if infra != 1 {
+		t.Errorf("expected exactly 1 infra#main entry after collapse, got %d", infra)
+	}
+	if len(lk.Envs) != 2 {
+		t.Errorf("expected 2 envs total (infra#main + other), got %d", len(lk.Envs))
+	}
+	if lk.FindEnv("github.com/org/other", "") == nil {
+		t.Error("unrelated env was dropped")
+	}
+}
+
 func TestReadWriteLockWithEnvs(t *testing.T) {
 	dir := t.TempDir()