From 2afcbc4be485fe654ef0af17a5404690b8964bf0 Mon Sep 17 00:00:00 2001
From: fullstackjam <fullstackjam@outlook.com>
Date: Sat, 11 Jul 2026 22:42:38 +0800
Subject: [PATCH 1/4] fix(tui): repair ctrl+c abort lifecycle

Three defects in the install-wizard abort/force-quit machinery (added in
#144), all surfaced by an adversarial re-review of the v0.63.0..HEAD range:

- C1: a single ctrl+c cancels the context, which SIGKILLs the in-flight
  brew/npm subprocess, so ApplyContext returns a non-nil error ("signal:
  killed"). The `m.aborting && installErr == nil` guard therefore never
  assigned ErrAborted for the common case (abort during the package phase),
  and the CLI misreported the abort as an install failure and nagged the
  screen-recording reminder. Now join ErrAborted with the cause whenever
  aborting, so errors.Is(err, ErrAborted) holds.

- M3: the config steps (shell/dotfiles/macOS/post-install) take no ctx and
  ApplyContext had no ctx.Err() gate, so an aborted install kept symlinking
  dotfiles and rewriting macOS defaults after the user asked to stop. Gate
  between phases so an abort skips not-yet-started work.

- C2: force-quit (2nd ctrl+c) returned tea.Quit immediately while the
  install goroutine was still running, racing the deferred os.Stdout
  restore and letting the engine mutate the system after control returned.
  Run/RunPipeline now join the goroutine (via a done channel closed once
  ApplyContext returns) before restoring stdout.

The prior abort test only injected a nil-error installDoneMsg -- the one
case that already worked -- masking C1. Updated to inject a killed-
subprocess error, plus tests for the ctx gate and the goroutine join.

Claude-Session: https://claude.ai/code/session_01LyhLX9U3aNuUUAKX1H9JbQ
---
 internal/installer/applycontext_abort_test.go | 54 +++++++++++++++++++
 internal/installer/installer.go               | 46 +++++++++++-----
 internal/ui/tui/wizard/install.go             | 22 ++++++--
 internal/ui/tui/wizard/wizard.go              | 30 +++++++++--
 internal/ui/tui/wizard/wizard_test.go         | 32 ++++++++++-
 5 files changed, 160 insertions(+), 24 deletions(-)
 create mode 100644 internal/installer/applycontext_abort_test.go

diff --git a/internal/installer/applycontext_abort_test.go b/internal/installer/applycontext_abort_test.go
new file mode 100644
index 0000000..b780a63
--- /dev/null
+++ b/internal/installer/applycontext_abort_test.go
@@ -0,0 +1,54 @@
+package installer
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/openbootdotdev/openboot/internal/macos"
+)
+
+// recordReporter captures Header calls so a test can assert which install
+// phases ApplyContext actually entered.
+type recordReporter struct{ headers []string }
+
+func (r *recordReporter) Header(msg string) { r.headers = append(r.headers, msg) }
+func (r *recordReporter) Info(string)       {}
+func (r *recordReporter) Success(string)    {}
+func (r *recordReporter) Warn(string)       {}
+func (r *recordReporter) Error(string)      {}
+func (r *recordReporter) Muted(string)      {}
+
+// A cancelled context must stop ApplyContext before the config steps run, so a
+// ctrl+c abort doesn't keep symlinking dotfiles / rewriting macOS defaults after
+// the user asked to stop. (M3: the config steps take no ctx, so without the
+// ctx.Err() gates they would run to completion regardless of cancellation.)
+func TestApplyContextAbortsBeforeConfigStepsOnCancel(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // pre-cancelled — simulates ctrl+c landing during the package phase
+
+	plan := InstallPlan{
+		SkipGit:        true, // no git, no packages → reach the first ctx gate immediately
+		InstallOhMyZsh: true, // would emit "Shell Configuration" if the step ran
+		DotfilesURL:    "https://github.com/example/dotfiles",
+		MacOSPrefs: []macos.Preference{
+			{Domain: "com.apple.dock", Key: "tilesize", Type: "int", Value: "48"},
+		},
+	}
+
+	rr := &recordReporter{}
+	err := ApplyContext(ctx, plan, rr)
+
+	require.ErrorIs(t, err, context.Canceled, "an aborted apply reports the cancellation")
+	for _, h := range rr.headers {
+		assert.NotContains(t, h, "Shell Config", "aborted install must not run the shell step")
+		assert.NotContains(t, h, "Dotfiles", "aborted install must not run dotfiles")
+		assert.NotContains(t, h, "macOS Prefer", "aborted install must not run macOS prefs")
+	}
+	// It's fine to have entered the package header before the gate fired; the
+	// point is that no *config* step started.
+	assert.NotContains(t, strings.Join(rr.headers, "|"), "Post-Install")
+}
diff --git a/internal/installer/installer.go b/internal/installer/installer.go
index 1ddaaa3..d8f88b5 100644
--- a/internal/installer/installer.go
+++ b/internal/installer/installer.go
@@ -120,27 +120,37 @@ func ApplyContext(ctx context.Context, plan InstallPlan, r Reporter) error {
 		softErrs = append(softErrs, fmt.Errorf("brew: %w", err))
 	}
 
+	// ctrl+c cancels ctx. brew/npm honour it (exec.CommandContext), but the
+	// config steps below take no ctx, so without these gates an aborted install
+	// would keep symlinking dotfiles and rewriting macOS defaults after the user
+	// asked to stop. Bail between phases so an abort skips not-yet-started work.
+	if err := ctx.Err(); err != nil {
+		return abortWith(softErrs, err)
+	}
+
 	if err := applyNpm(ctx, plan, r); err != nil {
 		r.Error(fmt.Sprintf("npm package installation failed: %v", err))
 		softErrs = append(softErrs, fmt.Errorf("npm: %w", err))
 	}
 
 	if !plan.PackagesOnly {
-		if err := applyShell(plan, r); err != nil {
-			r.Error(fmt.Sprintf("Shell setup failed: %v", err))
-			softErrs = append(softErrs, fmt.Errorf("shell: %w", err))
-		}
-		if err := applyDotfiles(plan, r); err != nil {
-			r.Error(fmt.Sprintf("Dotfiles setup failed: %v", err))
-			softErrs = append(softErrs, fmt.Errorf("dotfiles: %w", err))
+		configSteps := []struct {
+			name, failMsg string
+			apply         func(InstallPlan, Reporter) error
+		}{
+			{"shell", "Shell setup failed", applyShell},
+			{"dotfiles", "Dotfiles setup failed", applyDotfiles},
+			{"macos", "macOS configuration failed", applyMacOSPrefs},
+			{"post-install", "Post-install script failed", applyPostInstall},
 		}
-		if err := applyMacOSPrefs(plan, r); err != nil {
-			r.Error(fmt.Sprintf("macOS configuration failed: %v", err))
-			softErrs = append(softErrs, fmt.Errorf("macos: %w", err))
-		}
-		if err := applyPostInstall(plan, r); err != nil {
-			r.Error(fmt.Sprintf("Post-install script failed: %v", err))
-			softErrs = append(softErrs, fmt.Errorf("post-install: %w", err))
+		for _, s := range configSteps {
+			if err := ctx.Err(); err != nil {
+				return abortWith(softErrs, err)
+			}
+			if err := s.apply(plan, r); err != nil {
+				r.Error(fmt.Sprintf("%s: %v", s.failMsg, err))
+				softErrs = append(softErrs, fmt.Errorf("%s: %w", s.name, err))
+			}
 		}
 	}
 
@@ -154,6 +164,14 @@ func ApplyContext(ctx context.Context, plan InstallPlan, r Reporter) error {
 	return nil
 }
 
+// abortWith joins the soft errors accumulated so far with the context
+// cancellation cause, for when ApplyContext bails out early on a cancelled
+// context (ctrl+c). Returning here stops the remaining steps so an aborted
+// install doesn't keep mutating the system.
+func abortWith(softErrs []error, cause error) error {
+	return errors.Join(append(softErrs, cause)...)
+}
+
 func showCompletionFromPlan(plan InstallPlan, r Reporter, errCount int) {
 	ui.Println()
 	if errCount > 0 {
diff --git a/internal/ui/tui/wizard/install.go b/internal/ui/tui/wizard/install.go
index c11cd7f..7c1e24d 100644
--- a/internal/ui/tui/wizard/install.go
+++ b/internal/ui/tui/wizard/install.go
@@ -2,6 +2,7 @@ package wizard
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -111,7 +112,7 @@ func (m Model) startInstall() (tea.Model, tea.Cmd) {
 // spawnInstall runs the install engine on a background goroutine, streaming
 // progress onto the event channel. It returns immediately.
 func (m Model) spawnInstall(ctx context.Context, plan installer.InstallPlan) tea.Cmd {
-	ch := m.events
+	ch, done := m.events, m.installDone
 	return func() tea.Msg {
 		go func() {
 			sink := func(ev progress.Event) { ch <- evMsg{ev: ev} }
@@ -120,6 +121,11 @@ func (m Model) spawnInstall(ctx context.Context, plan installer.InstallPlan) tea
 			err := installer.ApplyContext(ctx, plan, chanReporter{ch: ch})
 			restoreBrew()
 			restoreNpm()
+			// Signal that the engine has stopped touching os.Stdout (ApplyContext
+			// returned) BEFORE the channel send, so Run can restore stdout without
+			// racing us — even on a force-quit where nothing drains the channel and
+			// the send below could block on a full buffer.
+			close(done)
 			ch <- installDoneMsg{err: err}
 		}()
 		return nil
@@ -146,7 +152,7 @@ func toPhaseStates(ps []PipelinePhase) []phaseState {
 // path) on a goroutine, wiring the same brew/npm sinks as spawnInstall so
 // package progress streams into the shared install screen.
 func (m Model) startPipeline() tea.Cmd {
-	ch, run, ctx := m.events, m.pipelineRun, m.pipelineCtx
+	ch, done, run, ctx := m.events, m.installDone, m.pipelineRun, m.pipelineCtx
 	return func() tea.Msg {
 		go func() {
 			sink := func(ev progress.Event) { ch <- evMsg{ev: ev} }
@@ -158,6 +164,8 @@ func (m Model) startPipeline() tea.Cmd {
 			err := run(ctx, chanReporter{ch: ch})
 			restoreBrew()
 			restoreNpm()
+			// See spawnInstall: close before the send so Run can join safely.
+			close(done)
 			ch <- installDoneMsg{err: err}
 		}()
 		return nil
@@ -231,8 +239,14 @@ func (m Model) onInstallEvent(msg tea.Msg) (tea.Model, tea.Cmd) {
 		m.installing = false
 		m.done = true
 		m.installErr = t.err
-		if m.aborting && m.installErr == nil {
-			m.installErr = ErrAborted
+		if m.aborting {
+			// A ctrl+c cancel SIGKILLs the in-flight brew/npm subprocess, so t.err
+			// is usually non-nil ("signal: killed"). The old `&& t.err == nil` guard
+			// therefore left ErrAborted unset for the common case (abort during the
+			// package phase), and the CLI misreported the abort as an install
+			// failure. Join so errors.Is(err, ErrAborted) holds while the underlying
+			// cause stays in the chain for the log.
+			m.installErr = errors.Join(ErrAborted, t.err)
 		}
 		if m.cancel != nil {
 			m.cancel()
diff --git a/internal/ui/tui/wizard/wizard.go b/internal/ui/tui/wizard/wizard.go
index 203e221..a5f9713 100644
--- a/internal/ui/tui/wizard/wizard.go
+++ b/internal/ui/tui/wizard/wizard.go
@@ -93,6 +93,7 @@ type Model struct {
 
 	// ── install ──
 	events      chan tea.Msg
+	installDone chan struct{} // closed by the install goroutine once ApplyContext returns (Run joins on it)
 	plan        installer.InstallPlan
 	phases      []phaseState
 	logs        []logLine
@@ -120,9 +121,10 @@ func New(version string, opts *config.InstallOptions) Model {
 		loadouts:  newLoadouts(),
 		installed: map[string]bool{},
 		cats:      config.GetCategories(),
-		hoverRow:  -1,
-		selected:  map[string]bool{},
-		events:    make(chan tea.Msg, 1024),
+		hoverRow:    -1,
+		selected:    map[string]bool{},
+		events:      make(chan tea.Msg, 1024),
+		installDone: make(chan struct{}),
 	}
 }
 
@@ -375,9 +377,27 @@ func Run(version string, opts *config.InstallOptions) (plan installer.InstallPla
 		return installer.InstallPlan{}, false, fmt.Errorf("run wizard: %w", runErr)
 	}
 	fm := final.(Model)
+	// Join the install goroutine before the deferred restore() flips os.Stdout
+	// back, so a force-quit (2nd ctrl+c) can neither race the engine's stdout
+	// writes nor return control to the shell while it is still mutating the system.
+	fm.joinInstall()
 	return fm.plan, fm.confirmed, fm.installErr
 }
 
+// joinInstall blocks until the install goroutine has finished (it closes
+// installDone once ApplyContext returns and the brew/npm sinks are restored).
+// No-op when no install was started. The timeout is a safety valve so a wedged
+// subprocess can't hang the process on the terminal indefinitely.
+func (m Model) joinInstall() {
+	if !m.confirmed && m.pipelineRun == nil {
+		return // no install goroutine was ever spawned
+	}
+	select {
+	case <-m.installDone:
+	case <-time.After(30 * time.Second):
+	}
+}
+
 // redirectOutput sends stdout+stderr to /dev/null for the alt-screen's lifetime
 // (subprocess progress must not paint over Bubble Tea) and returns the real
 // stdout to render onto plus a restore func.
@@ -417,5 +437,7 @@ func RunPipeline(version string, phases []PipelinePhase, run func(context.Contex
 	if runErr != nil {
 		return fmt.Errorf("run install: %w", runErr)
 	}
-	return final.(Model).installErr
+	fm := final.(Model)
+	fm.joinInstall() // see Run: join before the deferred restore()
+	return fm.installErr
 }
diff --git a/internal/ui/tui/wizard/wizard_test.go b/internal/ui/tui/wizard/wizard_test.go
index a627cf2..3b24721 100644
--- a/internal/ui/tui/wizard/wizard_test.go
+++ b/internal/ui/tui/wizard/wizard_test.go
@@ -2,6 +2,7 @@ package wizard
 
 import (
 	"context"
+	"errors"
 	"io"
 	"strings"
 	"testing"
@@ -422,9 +423,16 @@ func TestCtrlCDuringInstallAbortsHonestly(t *testing.T) {
 	assert.Nil(t, cmd, "first ctrl+c must not quit — it waits for the engine")
 	require.True(t, m.aborting)
 
-	next, _ = m.Update(installDoneMsg{})
+	// Cancelling the context SIGKILLs the in-flight brew/npm subprocess, so the
+	// engine reports a NON-nil error, not a clean nil. The abort must still be
+	// recognised as ErrAborted — the regression was that the old guard only set
+	// ErrAborted when the error happened to be nil, so an abort during the
+	// package phase (the common case) was misreported as an install failure.
+	killErr := errors.New("signal: killed")
+	next, _ = m.Update(installDoneMsg{err: killErr})
 	m = next.(Model)
-	assert.ErrorIs(t, m.installErr, ErrAborted)
+	assert.ErrorIs(t, m.installErr, ErrAborted, "abort with a killed subprocess is still ErrAborted")
+	assert.ErrorIs(t, m.installErr, killErr, "underlying cause stays in the chain for the log")
 	assert.True(t, m.quit)
 	for _, p := range m.phases {
 		assert.False(t, p.finished, "aborted phases must not show as finished")
@@ -655,6 +663,26 @@ func TestInstallGoroutineStreamsToDone(t *testing.T) {
 	for _, p := range m.phases {
 		assert.Truef(t, p.finished, "phase %q finished", p.name)
 	}
+	// C2: the goroutine closes installDone once ApplyContext returns, so Run's
+	// joinInstall can wait for it before restoring os.Stdout (no force-quit race).
+	select {
+	case <-m.installDone:
+	default:
+		t.Fatal("installDone must be closed once the install goroutine finishes")
+	}
+}
+
+// joinInstall must not block when no install goroutine was ever started (e.g.
+// the user quit on the boot/select screen) — otherwise Run would hang 30s.
+func TestJoinInstallNoopWhenNoInstall(t *testing.T) {
+	m := New("1", &config.InstallOptions{})
+	returned := make(chan struct{})
+	go func() { m.joinInstall(); close(returned) }()
+	select {
+	case <-returned:
+	case <-time.After(2 * time.Second):
+		t.Fatal("joinInstall must be a no-op when no install ran")
+	}
 }
 
 func logTexts(ls []logLine) []string {

From 724bd4372b9d06f35852f8753a28c44a16f422da Mon Sep 17 00:00:00 2001
From: fullstackjam <fullstackjam@outlook.com>
Date: Sat, 11 Jul 2026 22:46:15 +0800
Subject: [PATCH 2/4] fix(cli): run post-install scripts on interactive slug
 installs

R1 regression from #146: routing `install <slug>` / `-u` / `--from` on a
TTY through the wizard pipeline forces plan.Silent=true to keep prompts out
of the alt-screen. But Silent is overloaded -- applyPostInstall also *skips
execution* when Silent, so a RemoteConfig carrying a post_install block had
its script silently dropped, where v0.63.0 (linear, Silent=false) previewed,
confirmed, and ran it.

The alt-screen can't host the script preview + confirm, so defer post-install
to after teardown on a normal terminal (mirroring the screen-recording
reminder): strip it from the streamed plan via splitPostInstall and run it via
installer.RunPostInstallAfterTUI on a clean run, before the reminder (Step 8
order). A failing script stays a soft error, not fatal.

Scope: RemoteConfig sources only. Presets, --packages-only, the bare wizard
(no PostInstall in the plan), and sync are unaffected.

Claude-Session: https://claude.ai/code/session_01LyhLX9U3aNuUUAKX1H9JbQ
---
 internal/cli/install.go                    | 31 +++++++++++++++++---
 internal/cli/post_install_test.go          | 34 ++++++++++++++++++++++
 internal/installer/installer.go            | 19 ++++++++++++
 internal/installer/installer_extra_test.go |  7 +++++
 4 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 internal/cli/post_install_test.go

diff --git a/internal/cli/install.go b/internal/cli/install.go
index 4d71db5..eb8f4a2 100644
--- a/internal/cli/install.go
+++ b/internal/cli/install.go
@@ -194,15 +194,28 @@ func runPipelineInstall(ctx context.Context) error {
 	// re-run afterwards via ShowScreenRecordingReminderAfterTUI.
 	plan.Silent = true
 
-	runErr := wizard.RunPipeline(installCfg.Version, wizard.PhasesForPlan(plan),
+	// Post-install needs a script preview + confirm, which the alt-screen can't
+	// host. Strip it from the streamed plan and run it after teardown on a normal
+	// terminal (Silent=true would otherwise gate it out entirely — the R1 bug).
+	streamed, deferredPostInstall := splitPostInstall(plan)
+
+	runErr := wizard.RunPipeline(installCfg.Version, wizard.PhasesForPlan(streamed),
 		func(ctx context.Context, r installer.Reporter) error {
-			return installer.ApplyContext(ctx, plan, r)
+			return installer.ApplyContext(ctx, streamed, r)
 		})
 	if errors.Is(runErr, wizard.ErrAborted) || errors.Is(runErr, context.Canceled) {
 		return fmt.Errorf("installation aborted — partially applied changes are logged in ~/.openboot/logs")
 	}
-	// The install ran (cleanly or with soft errors): show the reminder on the
-	// restored terminal, then persist the sync source on a clean run.
+	// The install ran (cleanly or with soft errors). On a clean run, run the
+	// deferred post-install script (Step 8, matching the linear order) before the
+	// reminder; a failing script is a soft error, not fatal. Then show the
+	// reminder and persist the sync source.
+	if runErr == nil && len(deferredPostInstall) > 0 {
+		plan.PostInstall = deferredPostInstall
+		if piErr := installer.RunPostInstallAfterTUI(plan); piErr != nil {
+			ui.Error(fmt.Sprintf("Post-install script failed: %v", piErr))
+		}
+	}
 	installer.ShowScreenRecordingReminderAfterTUI(plan)
 	if runErr == nil {
 		saveSyncSourceIfRemote(installCfg)
@@ -210,6 +223,16 @@ func runPipelineInstall(ctx context.Context) error {
 	return runErr
 }
 
+// splitPostInstall separates a plan into the version streamed through the wizard
+// pipeline (post-install removed — the alt-screen can't host its confirm) and
+// the post-install script to run after teardown. Keeping it a pure function
+// makes the split testable without a TTY.
+func splitPostInstall(plan installer.InstallPlan) (streamed installer.InstallPlan, postInstall []string) {
+	postInstall = plan.PostInstall
+	plan.PostInstall = nil
+	return plan, postInstall
+}
+
 // runInstallWizard launches the full-screen install TUI and runs the resulting
 // install. The wizard owns the whole interactive flow (planning + apply); back
 // on the normal terminal, follow-ups that can't run inside the alt-screen
diff --git a/internal/cli/post_install_test.go b/internal/cli/post_install_test.go
new file mode 100644
index 0000000..bd527f2
--- /dev/null
+++ b/internal/cli/post_install_test.go
@@ -0,0 +1,34 @@
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/openbootdotdev/openboot/internal/installer"
+)
+
+// splitPostInstall must remove the post-install script from the plan streamed
+// through the wizard pipeline (the alt-screen can't host its confirm) while
+// preserving it for the after-teardown run. This is the R1 regression: the
+// pipeline path forced plan.Silent=true, which silently gated out post-install
+// execution that the linear path used to preview, confirm, and run.
+func TestSplitPostInstall(t *testing.T) {
+	plan := installer.InstallPlan{
+		Formulae:    []string{"jq"},
+		PostInstall: []string{"echo hi", "echo bye"},
+	}
+
+	streamed, deferred := splitPostInstall(plan)
+
+	assert.Nil(t, streamed.PostInstall, "streamed plan must not carry post-install")
+	assert.Equal(t, []string{"echo hi", "echo bye"}, deferred, "post-install deferred to after teardown")
+	assert.Equal(t, []string{"jq"}, streamed.Formulae, "the rest of the plan is untouched")
+	assert.Equal(t, []string{"echo hi", "echo bye"}, plan.PostInstall, "the caller's plan is not mutated")
+}
+
+func TestSplitPostInstall_Empty(t *testing.T) {
+	streamed, deferred := splitPostInstall(installer.InstallPlan{Formulae: []string{"jq"}})
+	assert.Empty(t, deferred)
+	assert.Equal(t, []string{"jq"}, streamed.Formulae)
+}
diff --git a/internal/installer/installer.go b/internal/installer/installer.go
index d8f88b5..bfa93f8 100644
--- a/internal/installer/installer.go
+++ b/internal/installer/installer.go
@@ -216,6 +216,25 @@ func ShowScreenRecordingReminderAfterTUI(plan InstallPlan) {
 	showScreenRecordingReminderFromPlan(plan)
 }
 
+// RunPostInstallAfterTUI runs the plan's post-install script on a normal
+// terminal after the full-screen wizard has torn down. The wizard forces
+// plan.Silent=true to keep prompts out of the alt-screen — but that also gates
+// out post-install *execution* (applyPostInstall skips when Silent), so a
+// slug/RemoteConfig install streamed through the pipeline would silently drop a
+// script the linear path would have previewed, confirmed, and run. The CLI
+// defers it here with Silent cleared, so the script gets its preview + confirm
+// and runs, matching the linear installer. No-op when there is no script.
+func RunPostInstallAfterTUI(plan InstallPlan) error {
+	if len(plan.PostInstall) == 0 {
+		return nil
+	}
+	plan.Silent = false
+	if err := applyPostInstall(plan, ConsoleReporter{}); err != nil {
+		return fmt.Errorf("post-install: %w", err)
+	}
+	return nil
+}
+
 func showScreenRecordingReminderFromPlan(plan InstallPlan) {
 	if plan.DryRun || plan.Silent {
 		return
diff --git a/internal/installer/installer_extra_test.go b/internal/installer/installer_extra_test.go
index 14213c5..78fb097 100644
--- a/internal/installer/installer_extra_test.go
+++ b/internal/installer/installer_extra_test.go
@@ -20,6 +20,13 @@ func TestShowScreenRecordingReminderFromPlan_DryRun_NoOp(t *testing.T) {
 	})
 }
 
+// RunPostInstallAfterTUI must be a no-op (no prompt, no error) when the plan has
+// no post-install script — it's called on every pipeline install. The
+// script-present path needs a TTY + real exec, so it's covered by e2e, not here.
+func TestRunPostInstallAfterTUI_NoOpWhenEmpty(t *testing.T) {
+	assert.NoError(t, RunPostInstallAfterTUI(InstallPlan{}))
+}
+
 func TestShowScreenRecordingReminderFromPlan_Silent_NoOp(t *testing.T) {
 	plan := InstallPlan{DryRun: false, Silent: true}
 	assert.NotPanics(t, func() {

From a60700cdc1b96405e3dd6c64dfac9a0d2a91674a Mon Sep 17 00:00:00 2001
From: fullstackjam <fullstackjam@outlook.com>
Date: Sat, 11 Jul 2026 22:56:45 +0800
Subject: [PATCH 3/4] fix(tui): npm package count, resize scroll, elapsed
 clock, select lows

Grab-bag of install-wizard defects from the v0.63.0..HEAD review:

- M1: the npm outer retry (applyNpm) re-runs the install and re-emits an
  "already installed" skip for every package a prior attempt installed,
  breaking the one-terminal-event-per-package invariant. The renderer's
  skippedPkgs counter (unclamped, unlike incPhase) inflated past the total,
  so the completion footer could show "-1 packages". Dedup: ignore a skip
  for a package that already produced a terminal event (terminalSeen).

- M2: WindowSizeMsg updated height but didn't re-clamp scroll. selectList
  renders from a re-clamped copy while selectHitTest reads the stored
  scroll, so a click right after a resize (before any mouse motion
  re-clamps) could toggle the wrong package. Re-clamp on resize.

- C3: the tick loop never stopped, so the completion footer's elapsed clock
  kept counting up while the user read it. Freeze it at done.

- selVisible counted height-6 but selectList renders height-4 rows, leaving
  two blank rows at the bottom and two the keyboard cursor can't reach.
  Align the counts.

- A click on the vertical divider column toggled the adjacent package;
  treat the divider as neither pane.

- Fix the stale hover comment (hover no longer skips the keyboard-cursor row).

Claude-Session: https://claude.ai/code/session_01LyhLX9U3aNuUUAKX1H9JbQ
---
 internal/ui/tui/wizard/install.go           |  28 +++++
 internal/ui/tui/wizard/review_fixes_test.go | 109 ++++++++++++++++++++
 internal/ui/tui/wizard/select.go            |  17 ++-
 internal/ui/tui/wizard/wizard.go            |  64 +++++++-----
 4 files changed, 187 insertions(+), 31 deletions(-)
 create mode 100644 internal/ui/tui/wizard/review_fixes_test.go

diff --git a/internal/ui/tui/wizard/install.go b/internal/ui/tui/wizard/install.go
index 7c1e24d..5b6bb42 100644
--- a/internal/ui/tui/wizard/install.go
+++ b/internal/ui/tui/wizard/install.go
@@ -281,6 +281,16 @@ func (m *Model) applyProgressEvent(ev progress.Event) {
 			m.curStep = ev.Name
 		}
 	case progress.StepOK:
+		// The npm outer retry (applyNpm) re-runs the install and re-emits an
+		// "already installed" skip for every package a prior attempt installed —
+		// breaking the one-terminal-event-per-package invariant. Ignore a skip for
+		// a package that already had a terminal event, so skippedPkgs (which the
+		// completion footer subtracts from the package total) can't be inflated
+		// past the total and render a negative "N packages".
+		if ev.Detail == progress.SkipDetail && m.terminalSeen[termKey(ev)] {
+			return
+		}
+		m.markTerminal(ev)
 		m.incPhase(ev.Phase)
 		text := ev.Name
 		if ev.Detail != "" {
@@ -293,11 +303,29 @@ func (m *Model) applyProgressEvent(ev progress.Event) {
 			m.appendLog(logLine{mark: "✓", markColor: cAccent, text: text, color: cMuted})
 		}
 	case progress.StepFail:
+		m.markTerminal(ev)
 		m.incPhase(ev.Phase)
 		m.appendLog(logLine{mark: "✗", markColor: cDanger, text: ev.Name + " (" + ev.Detail + ")", color: cDanger})
 	}
 }
 
+// termKey identifies a package's terminal event by phase + name. Empty for
+// unnamed events (e.g. the batch StepStart), which are never deduped.
+func termKey(ev progress.Event) string {
+	if ev.Name == "" {
+		return ""
+	}
+	return ev.Phase + "/" + ev.Name
+}
+
+// markTerminal records that a package produced a terminal event, so a later
+// re-emitted skip for it (npm retry) can be recognised as a duplicate.
+func (m *Model) markTerminal(ev progress.Event) {
+	if k := termKey(ev); k != "" {
+		m.terminalSeen[k] = true
+	}
+}
+
 // activatePhase marks phase name active and everything before it finished.
 func (m *Model) activatePhase(name string) {
 	if name == "" {
diff --git a/internal/ui/tui/wizard/review_fixes_test.go b/internal/ui/tui/wizard/review_fixes_test.go
new file mode 100644
index 0000000..dcf900f
--- /dev/null
+++ b/internal/ui/tui/wizard/review_fixes_test.go
@@ -0,0 +1,109 @@
+package wizard
+
+import (
+	"fmt"
+	"testing"
+
+	tea "github.com/charmbracelet/bubbletea"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/openbootdotdev/openboot/internal/config"
+	"github.com/openbootdotdev/openboot/internal/progress"
+)
+
+func npmEvent(name string, status progress.Status, detail string) evMsg {
+	return evMsg{ev: progress.Event{Phase: progress.PhaseNpm, Name: name, Status: status, Detail: detail}}
+}
+
+// M1: the npm outer retry re-runs the install and re-emits an "already
+// installed" skip for every package a prior attempt installed. The renderer
+// must not double-count those, or the completion footer subtracts an inflated
+// skip count from the package total and renders a negative "N packages".
+func TestNpmRetrySkipsDoNotInflatePackageCount(t *testing.T) {
+	m := New("1", &config.InstallOptions{})
+	m.screen, m.installing = scrInstall, true
+	m.phases = toPhaseStates([]PipelinePhase{{Name: progress.PhaseNpm, Total: 2, Pkg: true}})
+
+	// Attempt 1: pkgA installs, pkgB fails.
+	m = send(m, npmEvent("pkgA", progress.StepOK, ""))
+	m = send(m, npmEvent("pkgB", progress.StepFail, "boom"))
+	// Attempt 2: the retry re-scans and re-emits pkgA as already-installed while
+	// pkgB now installs.
+	m = send(m, npmEvent("pkgA", progress.StepOK, progress.SkipDetail))
+	m = send(m, npmEvent("pkgB", progress.StepOK, ""))
+
+	assert.Equal(t, 0, m.skippedPkgs, "a re-emitted skip for an already-counted package must not count")
+	assert.GreaterOrEqual(t, m.pkgCount()-m.skippedPkgs, 0, "footer package tally must never go negative")
+}
+
+// The dedup must not swallow a genuine already-installed skip (the common,
+// non-retry case where a package really was present before this run).
+func TestGenuinePreInstalledSkipStillCounts(t *testing.T) {
+	m := New("1", &config.InstallOptions{})
+	m.screen, m.installing = scrInstall, true
+	m.phases = toPhaseStates([]PipelinePhase{{Name: progress.PhaseNpm, Total: 1, Pkg: true}})
+
+	m = send(m, npmEvent("pkgA", progress.StepOK, progress.SkipDetail))
+	assert.Equal(t, 1, m.skippedPkgs, "a genuine already-installed skip counts once")
+}
+
+// C3: the elapsed clock on the completion footer must freeze once the install
+// is done, not keep counting up while the user reads it.
+func TestElapsedFreezesWhenDone(t *testing.T) {
+	m := New("1", &config.InstallOptions{})
+	m.screen, m.installing = scrInstall, true
+
+	for i := 0; i < 10; i++ {
+		m = send(m, tickMsg{})
+	}
+	require.Equal(t, 10, m.ticks)
+
+	m = send(m, installDoneMsg{})
+	require.True(t, m.done)
+	frozen := m.elapsed()
+
+	next, cmd := m.Update(tickMsg{})
+	m = next.(Model)
+	assert.Nil(t, cmd, "the tick loop stops re-arming once done")
+	assert.Equal(t, 10, m.ticks, "ticks frozen at completion")
+	assert.Equal(t, frozen, m.elapsed(), "elapsed clock frozen at completion")
+}
+
+// M2: a resize must re-clamp the stored scroll, so selectHitTest (which reads
+// the stored scroll) agrees with selectList (which renders from a re-clamped
+// copy). Without it, a click right after a resize toggles the wrong package.
+func TestWindowResizeReclampsSelectScroll(t *testing.T) {
+	m := New("1", &config.InstallOptions{})
+	m = send(m, tea.WindowSizeMsg{Width: 96, Height: 40})
+	m.screen = scrSelect
+	m.cats = []config.Category{{Name: "big", Packages: makePackages(50)}}
+	m.catCur, m.selFocus = 0, focusList
+	m.rowCur = 49
+	m = m.clampSelScroll()
+	require.Greater(t, m.scroll, 0, "cursor at the end of a long list should be scrolled at height 40")
+
+	m = send(m, tea.WindowSizeMsg{Width: 96, Height: 14})
+	assert.Equal(t, m.clampSelScroll().scroll, m.scroll,
+		"resize must re-clamp the stored scroll so hit-test matches the render")
+}
+
+// The vertical divider column between the two panes is neither pane — a click
+// there must hit nothing, not toggle the adjacent package.
+func TestSelectHitTestDividerColumnIsNoHit(t *testing.T) {
+	m := finishProbes(sized(96, 30))
+	m = send(m, key("2"))
+	require.Equal(t, scrSelect, m.screen)
+
+	kind, idx := m.selectHitTest(sidebarW, 5)
+	assert.Equal(t, hitNone, kind)
+	assert.Equal(t, -1, idx)
+}
+
+func makePackages(n int) []config.Package {
+	ps := make([]config.Package, n)
+	for i := range ps {
+		ps[i] = config.Package{Name: fmt.Sprintf("pkg%02d", i)}
+	}
+	return ps
+}
diff --git a/internal/ui/tui/wizard/select.go b/internal/ui/tui/wizard/select.go
index 193acda..37c4b2d 100644
--- a/internal/ui/tui/wizard/select.go
+++ b/internal/ui/tui/wizard/select.go
@@ -63,9 +63,13 @@ func (m Model) pool() []config.Package {
 	return nil
 }
 
-// selVisible is the number of package rows the list area can show.
+// selVisible is the number of package rows the list area can show. It must match
+// selectList's own count (body height − search row − blank) exactly, or the
+// scroll clamp and the render disagree: leaving blank rows at the bottom and
+// rows the keyboard cursor can never reach. body height = m.height − title −
+// status = m.height − 2; the list then spends 2 rows on the search + blank.
 func (m Model) selVisible() int {
-	v := m.height - 6 // chrome (2) + search row + blank + slack
+	v := m.height - 4
 	if v < 3 {
 		v = 3
 	}
@@ -286,6 +290,9 @@ func (m Model) selectHitTest(x, y int) (selHit, int) {
 		}
 		return hitNone, -1
 	}
+	if x == sidebarW {
+		return hitNone, -1 // the vertical divider column — neither pane
+	}
 	if bodyRow < 2 {
 		return hitNone, -1
 	}
@@ -427,9 +434,9 @@ func (m Model) selectList(w, h int) []string {
 	}
 	for i := start; i < end; i++ {
 		line := m.renderRow(pool[i], i == m2.rowCur, w)
-		// Subtle background highlight on the row the mouse is hovering over, so
-		// the user sees where a click would land. Skip when it's also the keyboard
-		// cursor — that row already has a prominent marker (› + bold).
+		// Subtle background highlight on the row the mouse is hovering over, so the
+		// user sees where a click would land — including when it's also the
+		// keyboard cursor, so the "clickable" affordance is never dropped.
 		if i == m.hoverRow {
 			line = hoverBg(line)
 		}
diff --git a/internal/ui/tui/wizard/wizard.go b/internal/ui/tui/wizard/wizard.go
index a5f9713..3f8d3ee 100644
--- a/internal/ui/tui/wizard/wizard.go
+++ b/internal/ui/tui/wizard/wizard.go
@@ -92,19 +92,20 @@ type Model struct {
 	confCur      int
 
 	// ── install ──
-	events      chan tea.Msg
-	installDone chan struct{} // closed by the install goroutine once ApplyContext returns (Run joins on it)
-	plan        installer.InstallPlan
-	phases      []phaseState
-	logs        []logLine
-	curStep     string
-	installing  bool
-	aborting    bool // ctrl+c received mid-install; waiting for the engine to stop
-	done        bool
-	installErr  error
-	skippedPkgs int // terminal events with SkipDetail (already installed)
-	installTick int // ticks value when install started, for elapsed
-	cancel      context.CancelFunc
+	events       chan tea.Msg
+	installDone  chan struct{} // closed by the install goroutine once ApplyContext returns (Run joins on it)
+	plan         installer.InstallPlan
+	phases       []phaseState
+	logs         []logLine
+	curStep      string
+	installing   bool
+	aborting     bool // ctrl+c received mid-install; waiting for the engine to stop
+	done         bool
+	installErr   error
+	skippedPkgs  int             // terminal events with SkipDetail (already installed)
+	terminalSeen map[string]bool // packages that produced a terminal event, for skip dedup (npm retry)
+	installTick  int             // ticks value when install started, for elapsed
+	cancel       context.CancelFunc
 
 	// ── pipeline mode (RunPipeline: sync-source & slug installs reuse this screen) ──
 	pipelineRun func(context.Context, installer.Reporter) error // non-nil ⇒ start on install screen
@@ -114,17 +115,18 @@ type Model struct {
 // New builds a wizard model for the given version and resolved install options.
 func New(version string, opts *config.InstallOptions) Model {
 	return Model{
-		version:   version,
-		opts:      opts,
-		screen:    scrBoot,
-		probes:    newProbes(),
-		loadouts:  newLoadouts(),
-		installed: map[string]bool{},
-		cats:      config.GetCategories(),
-		hoverRow:    -1,
-		selected:    map[string]bool{},
-		events:      make(chan tea.Msg, 1024),
-		installDone: make(chan struct{}),
+		version:      version,
+		opts:         opts,
+		screen:       scrBoot,
+		probes:       newProbes(),
+		loadouts:     newLoadouts(),
+		installed:    map[string]bool{},
+		cats:         config.GetCategories(),
+		hoverRow:     -1,
+		selected:     map[string]bool{},
+		events:       make(chan tea.Msg, 1024),
+		installDone:  make(chan struct{}),
+		terminalSeen: map[string]bool{},
 	}
 }
 
@@ -150,11 +152,21 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	switch msg := msg.(type) {
 	case tea.WindowSizeMsg:
 		m.width, m.height = msg.Width, msg.Height
-		return m, nil
+		// Re-clamp the select scroll to the new height: selectList renders from a
+		// re-clamped copy, but selectHitTest reads the stored scroll, so without
+		// this a click right after a resize (before any mouse motion re-clamps)
+		// could toggle the wrong package. Harmless on the other screens.
+		return m.clampSelScroll(), nil
 
 	case tickMsg:
+		// Stop animating once the install screen is done: nothing on it spins, and
+		// leaving the clock running makes the completion footer's elapsed time keep
+		// counting up while the user reads it. Checking before the increment
+		// freezes elapsed() exactly at completion.
+		if m.done {
+			return m, nil
+		}
 		m.ticks++
-		// Keep ticking while anything is animating (probing or installing).
 		return m, tickCmd()
 
 	case tea.KeyMsg:

From 21418e8aabdf31ee904f5ba932a4c8aa72d763ae Mon Sep 17 00:00:00 2001
From: fullstackjam <fullstackjam@outlook.com>
Date: Sat, 11 Jul 2026 22:57:20 +0800
Subject: [PATCH 4/4] chore(installer): log install_started on the pipeline
 install path

The wizard pipeline path (PlanForConfig -> RunPipeline -> ApplyContext)
skips runInstallContext, so a streamed slug/preset install logged
install_completed with no matching install_started. Emit it in
PlanForConfig, mirroring runInstallContext's event.

Claude-Session: https://claude.ai/code/session_01LyhLX9U3aNuUUAKX1H9JbQ
---
 internal/installer/installer.go | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/internal/installer/installer.go b/internal/installer/installer.go
index bfa93f8..b7e2519 100644
--- a/internal/installer/installer.go
+++ b/internal/installer/installer.go
@@ -91,6 +91,16 @@ func runInstallContext(ctx context.Context, opts *config.InstallOptions, st *con
 func PlanForConfig(cfg *config.Config) (InstallPlan, error) {
 	opts := cfg.ToInstallOptions()
 	st := cfg.ToInstallState()
+	// Mirror runInstallContext's install_started event: the pipeline path skips
+	// runInstallContext, so without this a streamed install logs install_completed
+	// (from ApplyContext) with no matching install_started.
+	slog.Info("install_started",
+		"version", opts.Version,
+		"preset", opts.Preset,
+		"user", opts.User,
+		"dry_run", opts.DryRun,
+		"silent", opts.Silent,
+	)
 	if err := checkDependencies(opts, st); err != nil {
 		return InstallPlan{}, fmt.Errorf("check dependencies: %w", err)
 	}