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

[initializ-mk]

Two forge init wizard UX fixes from TUI feedback.

1. Back-navigation everywhere (esc)

Previously only a few steps (review / fallback / egress) emitted a backspace-back; most screens had no way to go back. Now esc goes back one step from any step, handled at the wizard level so it works regardless of a step's sub-phase or text inputs (esc is never an editing key, so intercepting it there is safe).

• esc → previous step (re-initialized).
• esc on the first step → cancel (nowhere to go back).
• ctrl+c → cancel (unchanged).
• Key hints updated from esc quit → esc back.

2. Multi-select Enter stops force-selecting the highlighted row

The multi-select confirmed on Enter by auto-checking the cursor row when nothing was toggled — so "just press Enter" silently selected whatever was highlighted, even though Space is the toggle key. Now Enter confirms exactly what Space toggled, including an empty selection. Steps like Skills / Fallback legitimately allow selecting nothing.

Tests

• wizard_nav_test.go — esc steps back from step 2→1→0, and cancels on the first step.
• multi_select_test.go — Enter confirms empty; Space+Enter selects the cursor row; cursor-move then Space toggles the correct row.

Build + golangci-lint clean; forge-cli/internal/tui/... tests green.

Companion to #263 (drop builtin-tool selection screen; web search gets its own step) — same wizard, independent changes.

[initializ-mk]

Review: esc back-navigation; multi-select Enter fix

Both changes are well-motivated. The multi-select half is merge-ready: the Enter fix is exactly right, all three tests are meaningful, and I verified no step on this branch requires a non-empty multi-select result (ToolsStep handles empty — Summary = "none" — and Skills/Fallback are documented empty-legal). Handling esc at the wizard level is the correct architecture, and splitting ctrl+c (cancel) from esc (back) matches TUI conventions.

The back-navigation half has a blocking gap — see the inline comment on wizard.go: esc-back re-enters a step whose complete flag was never reset, and most steps short-circuit Update on it, soft-locking the wizard. Details inline.

2. Medium: Apply is cumulative into the context — redoing a step can leak stale values

advanceStep calls Apply on every forward pass and there is no undo on back. Set-style Applies self-heal on re-advance, but conditional writes don't: complete the tools step having selected web_search + entered a key (EnvVars["TAVILY_API_KEY"] written on advance), esc back, deselect web_search, confirm → BuiltinTools is corrected but the stale TAVILY_API_KEY / WEB_SEARCH_PROVIDER env vars persist in the context and land in the generated .env for a tool the user deselected.

Fix: steps should clear the keys they own at the top of Apply — making it "write my current state," not "add my current state." Worth fixing in this PR since back-nav is exactly what makes redo reachable.

Nits

• Whole-step vs sub-phase back: esc from a step's later sub-phase (e.g. a key input) jumps to the previous step, not the previous sub-phase. Defensible, but users may expect esc to unwind one screen — worth a deliberate one-liner in the PR description.
• Abandoned async validation: esc during a validating phase drops the in-flight ValidationResultMsg (routed to the now-current previous step, which ignores it). Harmless once the reset contract from the blocking finding lands — re-entry restarts the phase — just noting the interaction.

Verdict

Hold for the soft-lock fix (inline on wizard.go) — as shipped, pressing esc after completing Provider/Channel/Review (most of the wizard) leaves the session stuck, a worse outcome than the missing back-navigation it replaces. The Apply leak should ride along. The multi-select change needs nothing.

[initializ-mk]

Blocking: esc-back lands on an inert step and soft-locks the wizard.

This is w.current-- + Init() with no state reset, and the steps aren't uniform about resetting themselves. On this branch:

• SkillsStep.Init() resets (s.complete = false, phase, prompt) — designed for re-entry via the pre-existing StepBackMsg path.
• ProviderStep.Init(), ChannelStep.Init(), ReviewStep.Init() do not reset — and every step's Update begins with if s.complete { return s, nil }.

So the exact flow this feature exists for — complete step N, realize at N+1 you made a mistake, press esc — re-enters a step whose complete is still true: it renders its final view, ignores all input, and can never emit StepCompleteMsg again (which this file's own comment says is the sole advancement path). The user is soft-locked; the only working keys are esc (deeper into more inert steps) or ctrl+c (lose everything). Sub-phase state has the same problem: esc away from a validating phase and return, and the step waits forever for a ValidationResultMsg that was already dropped.

The defect shape predates this PR (StepBackMsg below has it too), but this PR promotes back-nav from three steps to all of them, so it should carry the fix.

Fix: establish a re-entry contract — every step's Init() resets to its initial phase (the SkillsStep precedent), or add a Reset() the wizard calls on back-entry, or recreate the step. Then pin it with a round-trip test on a REAL step: complete → esc → change answer → advance.

[initializ-mk]

This mock is why the soft-lock (see wizard.go comment) is invisible to the suite: navMockStep is a stateless value type that never completes, so these tests verify index arithmetic, not usability after back. The scenario that matters — complete a real step (whose complete flag then short-circuits Update), esc back into it, change the answer, advance again — needs a stateful step, either a real one or a mock with the complete guard the real steps share.

[initializ-mk]

Nit: the review screen now advertises two back keys — backspace back (step-local StepBackMsg) and esc back (wizard-level). Either drop the backspace hint since wizard-level esc supersedes it, or add a line on why both stay.

[initializ-mk]

Thanks — thorough catch on the soft-lock. All addressed.

Blocking: esc-back soft-lock (wizard.go). Fixed by establishing the re-entry contract you identified — every step's Init() now resets to its initial phase + complete = false (the SkillsStep/CompressionStep precedent). Fixed name / provider / channel / tools / fallback / auth / review (compression/egress/skills already did). The wizard already calls Init() on both esc and StepBackMsg, so this closes the lock for every step.

Test mock (wizard_nav_test.go:10). Replaced the stateless value mock with a stateful one that mirrors the real contract — Update short-circuits on complete, space completes it, Init() resets. New TestWizard_BackReEntersUsableStep drives the exact flow (complete → esc-back → re-complete → advance) and fails on the soft-lock (if stepB.complete { t.Fatal("SOFT-LOCK…") }). Plus steps/reentry_test.go asserts the contract on the real steps directly (construct → mark complete → Init() → assert reset).

Finding #2 (Apply leak). Fixed for the two cases where the step exclusively owns its keys: ToolsStep.Apply clears WEB_SEARCH_PROVIDER / TAVILY_API_KEY / PERPLEXITY_API_KEY before re-writing (your exact example — deselecting web_search on redo no longer leaks the key), and ChannelStep.Apply clears ChannelTokens. Both covered by tests.

The provider/fallback/skills Applies share the *_API_KEY namespace, so a naive clear there could wipe a key another step legitimately set (there's even a latent primary-vs-fallback collision). Rather than risk a wrong clear, I filed that broader cumulative-Apply problem as #296 with options (per-step key tracking / rebuild-context-on-advance / namespace fallback keys).

Nits:

• kbd_hint: review screen now advertises only esc back (dropped the redundant backspace hint, with a comment on why).
• Whole-step vs sub-phase back + abandoned async validation: noted in the PR description as deliberate — esc unwinds a whole step, and the reset contract means re-entry restarts the phase cleanly (the dropped ValidationResultMsg is harmless because the phase restarts).

Tests + lint green (the multi-select half was already merge-ready per your review).

[initializ-mk]

Re-review of c3ff280 (+ main merge) — both findings fixed, CI green

All 9 checks pass. Verification:

Blocking finding (soft-lock) — fixed comprehensively. The re-entry contract is now uniform: Init() resets complete + phase in name / provider / channel / fallback / auth / review (skills / compression / egress already did). Pinned at three layers:

• TestStepInitResetsComplete drives seven real steps through force-complete → Init() → assert usable;
• the test mock was rebuilt as a stateful pointer type mirroring the real steps' if s.complete guard;
• TestWizard_BackReEntersUsableStep is the exact round-trip from the review — complete → esc back → assert not inert (explicit "SOFT-LOCK" failure message) → re-complete → re-advance.

Medium finding (cumulative Apply) — fixed where safely possible, rest tracked. The tools/web-search step deletes its three env keys before rewriting; ChannelStep clears ChannelTokens. Deliberately NOT clearing the shared *_API_KEY namespace in provider/fallback/skills was the right call (a naive clear could wipe a key another step wrote) — filed as #296. One caveat on "exclusively owns TAVILY_API_KEY": skills like tavily-research also require that key via the skills step, but that overlap is precisely #296's scope.

Cross-PR reconciliation — handled properly. #263 merged into main mid-review (deleting ToolsStep, adding WebSearchStep, which had neither the Init reset nor a write-not-add Apply). The merge commit reconciled it fully: WebSearchStep.Init() resets complete/validating/phase, its Apply clears the env keys and removes web_search from BuiltinTools before re-adding (also fixing the append-duplication landmine flagged in the #263 review context), and reentry_test.go covers WebSearchStep for both the reset and the leak.

Nit — taken: the review screen's redundant backspace back hint was dropped with a rationale comment.

Verdict

Ready to merge. Both findings fixed with regression tests at the right level; the mid-flight merge with #263 was reconciled carefully.

(Housekeeping note, unrelated to this PR: the isolated-world evaluation promised by the snapshot.js trust-boundary comment in #260 is still not tracked in #293 or any open issue — please file it so that comment is true.)