fix(tui): esc back-navigation; multi-select Enter stops force-selecting

forge-cli/internal/tui/components/egress_display.go

@@ -32,8 +32,7 @@ func NewEgressDisplay(domains []EgressDomain, primaryStyle, dimStyle, borderStyl
 	kbd := NewKbdHint(kbdKeyStyle, kbdDescStyle)
 	kbd.Bindings = []KeyBinding{
 		{Key: "⏎", Desc: "accept"},
-		{Key: "backspace", Desc: "back"},
-		{Key: "esc", Desc: "quit"},
+		{Key: "esc", Desc: "back"},
 	}
 
 	return EgressDisplay{

forge-cli/internal/tui/components/kbd_hint.go

@@ -42,7 +42,7 @@ func SelectHints() []KeyBinding {
 	return []KeyBinding{
 		{Key: "↑↓", Desc: "navigate"},
 		{Key: "⏎", Desc: "select"},
-		{Key: "esc", Desc: "quit"},
+		{Key: "esc", Desc: "back"},
 	}
 }
 
@@ -52,23 +52,25 @@ func MultiSelectHints() []KeyBinding {
 		{Key: "↑↓", Desc: "navigate"},
 		{Key: "space", Desc: "toggle"},
 		{Key: "⏎", Desc: "confirm"},
-		{Key: "esc", Desc: "quit"},
+		{Key: "esc", Desc: "back"},
 	}
 }
 
 // InputHints returns standard hints for text input components.
 func InputHints() []KeyBinding {
 	return []KeyBinding{
 		{Key: "⏎", Desc: "submit"},
-		{Key: "esc", Desc: "quit"},
+		{Key: "esc", Desc: "back"},
 	}
 }
 
-// ReviewHints returns standard hints for the review step.
+// ReviewHints returns standard hints for the review step. Only `esc` is
+// advertised for back: wizard-level esc supersedes the step-local backspace
+// (StepBackMsg) path, so showing both would imply two behaviors where there's
+// one. See #264 review.
 func ReviewHints() []KeyBinding {
 	return []KeyBinding{
 		{Key: "⏎", Desc: "confirm"},
-		{Key: "backspace", Desc: "back"},
-		{Key: "esc", Desc: "quit"},
+		{Key: "esc", Desc: "back"},
 	}
 }

forge-cli/internal/tui/components/multi_select.go

@@ -118,18 +118,11 @@ func (m MultiSelect) Update(msg tea.Msg) (MultiSelect, tea.Cmd) {
 		case " ":
 			m.Items[m.cursor].Checked = !m.Items[m.cursor].Checked
 		case "enter":
-			// If nothing is checked, auto-check the cursor item so the user
-			// doesn't have to remember Space+Enter for a single selection.
-			anyChecked := false
-			for _, item := range m.Items {
-				if item.Checked {
-					anyChecked = true
-					break
-				}
-			}
-			if !anyChecked && len(m.Items) > 0 {
-				m.Items[m.cursor].Checked = true
-			}
+			// Enter confirms exactly what Space has toggled — including an
+			// empty selection. It must NOT auto-check the cursor item: the
+			// cursor is just the highlight, and steps like Skills / Fallback
+			// legitimately allow selecting nothing. (Previously Enter with
+			// nothing checked silently selected whatever row was highlighted.)
 			m.done = true
 		}
 	}

forge-cli/internal/tui/components/multi_select_test.go

@@ -0,0 +1,53 @@
+package components
+
+import (
+	"testing"
+
+	tea "github.com/charmbracelet/bubbletea"
+)
+
+func twoItemSelect() MultiSelect {
+	return MultiSelect{Items: []MultiSelectItem{
+		{Label: "alpha", Value: "alpha"},
+		{Label: "beta", Value: "beta"},
+	}}
+}
+
+// TestMultiSelect_EnterConfirmsEmpty pins the fix: pressing Enter with
+// nothing toggled confirms an EMPTY selection instead of silently
+// auto-selecting the highlighted row.
+func TestMultiSelect_EnterConfirmsEmpty(t *testing.T) {
+	m := twoItemSelect()
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeyEnter})
+	if !m.Done() {
+		t.Fatal("Enter should confirm the selection")
+	}
+	if vals := m.SelectedValues(); len(vals) != 0 {
+		t.Errorf("Enter with nothing toggled must select nothing, got %v", vals)
+	}
+}
+
+// TestMultiSelect_SpaceThenEnterSelects confirms Space still toggles the
+// cursor row and Enter confirms it.
+func TestMultiSelect_SpaceThenEnterSelects(t *testing.T) {
+	m := twoItemSelect()
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeySpace}) // toggle cursor row (alpha)
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeyEnter})
+	vals := m.SelectedValues()
+	if len(vals) != 1 || vals[0] != "alpha" {
+		t.Errorf("Space+Enter should select 'alpha', got %v", vals)
+	}
+}
+
+// TestMultiSelect_MoveThenSpaceSelectsCorrectRow guards that toggling
+// follows the cursor, not row 0.
+func TestMultiSelect_MoveThenSpaceSelectsCorrectRow(t *testing.T) {
+	m := twoItemSelect()
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeyDown})  // cursor -> beta
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeySpace}) // toggle beta
+	m, _ = m.Update(tea.KeyMsg{Type: tea.KeyEnter})
+	vals := m.SelectedValues()
+	if len(vals) != 1 || vals[0] != "beta" {
+		t.Errorf("expected only 'beta', got %v", vals)
+	}
+}

forge-cli/internal/tui/steps/channel_step.go

@@ -104,6 +104,11 @@ func (s *ChannelStep) Title() string { return "Channel Connector" }
 func (s *ChannelStep) Icon() string  { return "📡" }
 
 func (s *ChannelStep) Init() tea.Cmd {
+	// Reset on (re-)entry so BACK navigation doesn't land on a completed step
+	// whose Update short-circuits on `s.complete` and strands the wizard.
+	// Re-entry contract — SkillsStep/CompressionStep precedent (#264 review).
+	s.complete = false
+	s.phase = channelSelectPhase
 	return s.selector.Init()
 }
 
@@ -553,6 +558,11 @@ func (s *ChannelStep) Summary() string {
 
 func (s *ChannelStep) Apply(ctx *tui.WizardContext) {
 	ctx.Channel = s.channel
+	// Write current state, not accumulate: back-navigation + choosing a
+	// different channel must not leave the prior channel's tokens behind.
+	// ChannelStep exclusively owns ChannelTokens, so a full reset is safe.
+	// See #264 review.
+	clear(ctx.ChannelTokens)
 	for k, v := range s.tokens {
 		ctx.ChannelTokens[k] = v
 	}

forge-cli/internal/tui/steps/fallback_step.go

@@ -57,6 +57,17 @@ func (s *FallbackStep) Title() string { return "Fallback Providers" }
 func (s *FallbackStep) Icon() string  { return "🔄" }
 
 func (s *FallbackStep) Init() tea.Cmd {
+	// Reset on (re-)entry. Prepare() resets these too, but the wizard only
+	// calls Prepare on FORWARD advance — BACK navigation calls Init() alone,
+	// so without this a completed step short-circuits Update and strands the
+	// wizard. Re-entry contract (#264 review).
+	s.complete = false
+	s.validating = false
+	s.phase = fallbackAskPhase
+	s.selected = nil
+	s.collected = nil
+	s.keyIndex = 0
+
 	// Build the "Add fallback providers?" selector
 	items := []components.SingleSelectItem{
 		{Label: "No", Value: "no", Description: "Use only the primary provider", Icon: "⏭️"},

forge-cli/internal/tui/steps/name_step.go

@@ -60,6 +60,9 @@ func (s *NameStep) Init() tea.Cmd {
 		s.name = s.prefill
 		return func() tea.Msg { return tui.StepCompleteMsg{} }
 	}
+	// Reset on BACK navigation so the completed step doesn't short-circuit
+	// Update and strand the wizard. Re-entry contract (#264 review).
+	s.complete = false
 	return s.input.Init()
 }
 

forge-cli/internal/tui/steps/provider_step.go

@@ -143,6 +143,12 @@ func (s *ProviderStep) Title() string { return "Model Provider" }
 func (s *ProviderStep) Icon() string  { return "🤖" }
 
 func (s *ProviderStep) Init() tea.Cmd {
+	// Init also runs on BACK navigation into this step. Reset to the initial
+	// phase so a completed step doesn't short-circuit Update (which begins
+	// with `if s.complete`) and strand the wizard. Re-entry contract —
+	// SkillsStep/CompressionStep precedent (#264 review).
+	s.complete = false
+	s.phase = providerSelectPhase
 	return s.selector.Init()
 }
 

forge-cli/internal/tui/steps/reentry_test.go

@@ -0,0 +1,124 @@
+package steps
+
+import (
+	"testing"
+
+	"github.com/initializ/forge/forge-cli/internal/tui"
+)
+
+// TestStepInitResetsComplete pins the re-entry contract from the #264 review:
+// every step's Init() must reset `complete` to false, because the wizard calls
+// Init() (not Prepare) on BACK navigation and every step's Update short-
+// circuits on `if s.complete`. Without the reset, esc-back lands on an inert
+// step that ignores all input and soft-locks the wizard.
+//
+// The test drives REAL steps (a stateless mock can't catch the soft-lock —
+// see the wizard_nav_test note): construct the step, force it complete, call
+// Init(), and assert it's usable again.
+func TestStepInitResetsComplete(t *testing.T) {
+	styles := tui.NewStyleSet(tui.DarkTheme)
+	noKey := func(string, string) error { return nil }
+
+	t.Run("name", func(t *testing.T) {
+		s := NewNameStep(styles, "") // no prefill
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("NameStep.Init() must reset complete")
+		}
+	})
+	t.Run("provider", func(t *testing.T) {
+		s := NewProviderStep(styles, noKey)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("ProviderStep.Init() must reset complete")
+		}
+	})
+	t.Run("channel", func(t *testing.T) {
+		s := NewChannelStep(styles)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("ChannelStep.Init() must reset complete")
+		}
+	})
+	t.Run("web_search", func(t *testing.T) {
+		s := NewWebSearchStep(styles, noKey)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("WebSearchStep.Init() must reset complete")
+		}
+	})
+	t.Run("fallback", func(t *testing.T) {
+		s := NewFallbackStep(styles, noKey)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("FallbackStep.Init() must reset complete")
+		}
+	})
+	t.Run("auth", func(t *testing.T) {
+		s := NewAuthStep(styles)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("AuthStep.Init() must reset complete")
+		}
+	})
+	t.Run("compression", func(t *testing.T) {
+		s := NewCompressionStep(styles)
+		s.complete = true
+		s.Init()
+		if s.complete {
+			t.Error("CompressionStep.Init() must reset complete")
+		}
+	})
+}
+
+// TestWebSearchStepApply_ClearsStaleKeys pins finding #2: redoing the
+// web-search step (after back-nav) with "No web search" chosen must not leak
+// the previously-entered provider/key into the context.
+func TestWebSearchStepApply_ClearsStaleKeys(t *testing.T) {
+	styles := tui.NewStyleSet(tui.DarkTheme)
+	ctx := tui.NewWizardContext()
+
+	// First pass: Tavily chosen with a key.
+	s := NewWebSearchStep(styles, func(string, string) error { return nil })
+	s.provider = "tavily"
+	s.keyName = "TAVILY_API_KEY"
+	s.key = "tvly-x"
+	s.Apply(ctx)
+	if ctx.EnvVars["TAVILY_API_KEY"] == "" || ctx.EnvVars["WEB_SEARCH_PROVIDER"] == "" {
+		t.Fatal("first Apply should have written the web-search env vars")
+	}
+	if !containsWebSearch(ctx.BuiltinTools) {
+		t.Fatal("first Apply should have recorded web_search")
+	}
+
+	// Redo: user went back and chose "No web search".
+	s.provider = ""
+	s.keyName = ""
+	s.key = ""
+	s.Apply(ctx)
+
+	if _, ok := ctx.EnvVars["TAVILY_API_KEY"]; ok {
+		t.Error("stale TAVILY_API_KEY leaked after choosing No web search")
+	}
+	if _, ok := ctx.EnvVars["WEB_SEARCH_PROVIDER"]; ok {
+		t.Error("stale WEB_SEARCH_PROVIDER leaked after choosing No web search")
+	}
+	if containsWebSearch(ctx.BuiltinTools) {
+		t.Error("stale web_search left in BuiltinTools after deselect")
+	}
+}
+
+func containsWebSearch(s []string) bool {
+	for _, v := range s {
+		if v == "web_search" {
+			return true
+		}
+	}
+	return false
+}

forge-cli/internal/tui/steps/review_step.go

@@ -86,6 +86,11 @@ func (s *ReviewStep) Title() string { return "Review & Generate" }
 func (s *ReviewStep) Icon() string  { return "🚀" }
 
 func (s *ReviewStep) Init() tea.Cmd {
+	// Review is the terminal step (confirming it quits the wizard), so it is
+	// not normally re-entered via BACK — but reset `complete` anyway to honor
+	// the uniform re-entry contract (#264 review). `prepared` is left intact
+	// so the rendered summary survives.
+	s.complete = false
 	return nil
 }
 

forge-cli/internal/tui/steps/step_auth.go

@@ -137,7 +137,14 @@ func NewAuthStep(styles *tui.StyleSet) *AuthStep {
 func (s *AuthStep) Title() string { return "Authentication" }
 func (s *AuthStep) Icon() string  { return "🔐" }
 
-func (s *AuthStep) Init() tea.Cmd { return s.selector.Init() }
+func (s *AuthStep) Init() tea.Cmd {
+	// Reset on (re-)entry so BACK navigation restarts at the provider choice
+	// instead of short-circuiting on a stale `s.complete`. Re-entry contract
+	// (#264 review).
+	s.complete = false
+	s.phase = authSelectPhase
+	return s.selector.Init()
+}
 
 func (s *AuthStep) Update(msg tea.Msg) (tui.Step, tea.Cmd) {
 	if s.complete {

forge-cli/internal/tui/steps/web_search_step.go

@@ -84,6 +84,12 @@ func (s *WebSearchStep) Title() string { return "Web Search" }
 func (s *WebSearchStep) Icon() string  { return "🔍" }
 
 func (s *WebSearchStep) Init() tea.Cmd {
+	// Reset on (re-)entry so BACK navigation restarts at the provider choice
+	// instead of short-circuiting on a stale `s.complete`. Re-entry contract
+	// — SkillsStep/CompressionStep precedent (#264 review).
+	s.complete = false
+	s.validating = false
+	s.phase = webSearchChoosePhase
 	return s.choose.Init()
 }
 
@@ -246,6 +252,17 @@ func (s *WebSearchStep) Summary() string {
 }
 
 func (s *WebSearchStep) Apply(ctx *tui.WizardContext) {
+	// Apply must write CURRENT state, not accumulate. Back-navigation makes
+	// redo reachable: choosing a provider + key, then going back and picking
+	// "No web search", must not leave the stale key / provider in the context
+	// (they'd land in the generated .env for a tool the user dropped). Clear
+	// the keys this step exclusively owns first, then re-write the current
+	// selection. See #264 review.
+	delete(ctx.EnvVars, "WEB_SEARCH_PROVIDER")
+	delete(ctx.EnvVars, "TAVILY_API_KEY")
+	delete(ctx.EnvVars, "PERPLEXITY_API_KEY")
+	ctx.BuiltinTools = removeStr(ctx.BuiltinTools, "web_search")
+
 	if s.provider == "" {
 		return
 	}
@@ -257,3 +274,15 @@ func (s *WebSearchStep) Apply(ctx *tui.WizardContext) {
 		ctx.EnvVars[s.keyName] = s.key
 	}
 }
+
+// removeStr returns slice with all occurrences of val removed, preserving
+// order. Used so Apply can rewrite BuiltinTools idempotently on redo.
+func removeStr(slice []string, val string) []string {
+	out := slice[:0:0]
+	for _, s := range slice {
+		if s != val {
+			out = append(out, s)
+		}
+	}
+	return out
+}