fix(tui): v0.63 review follow-ups — abort lifecycle, slug post-install, wizard lows#147
Merged
Conversation
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
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
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
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
github-actions
Bot
added
installer
5 tasks
This was referenced Jul 16, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Lands the four review-follow-up commits on top of v0.64.1: repairs the ctrl+c abort lifecycle in the install wizard, runs post-install scripts on interactive slug installs (R1), fixes npm package count / resize scroll / elapsed clock / select-screen lows, and logs
install_startedon the pipeline install path.Why?
These close the correctness findings from the v0.63/v0.64 review pass: aborts during the package phase were misreported as install failures, config steps kept mutating the system after a ctrl+c, slug installs silently skipped post-install, and a click right after a resize could toggle the wrong package.
Testing
go vet ./...passesreview_fixes_test.go,applycontext_abort_test.go,post_install_test.go,plan_selection_test.go)make test-unit, 25 packages incl. archtest) green; wizard driven end-to-end on a real pty (tmux), stopping before confirmCross-repo checklist
openboot.dev? — noNotes for reviewer
feat/wizard-deep-polish(stacked PR) builds on this branch — merge this one first.