Feat/hooks tui - #6
Merged
Merged
Conversation
Introduce a two-step interactive flow for `spin new`: a param form followed by a hooks review screen. The hooks screen lists every [[pre]]/[[post]] command and _pre/_post script the template will run, shows the selected hook's content while navigating, and lets the user preview a single hook (Enter) or run the full scaffold via a centered Run/Skip modal (R). Hook execution streams into the right pane, the success summary (created + cd hint) is mirrored there and reprinted after quit, and the screen restores the terminal on exit via the alternate screen buffer. This also adds HookView/CollectHooks and verbose-gated single-hook streaming in the template package, and restores the non-TUI RenderToWithPost call in runNew (it had been left commented out, so non-interactive `spin new` printed success without rendering files). 💘 Generated with Crush Assisted-by: Crush:hy3-free
Replace the duplicated hookOutput/tuiHookOutput and chanWriter with a single hookTreeWriter in cmd/hook_output.go. The writer emits prefixed lines through an injectable emit function — os.Stdout for CLI, a channel for the TUI. Add a StepStart callback to HookOptions so callers can style per-step headers without leaking rendering concerns into the template engine. Also remove the orphaned RunSingleHook and hookAssetDir. 💘 Generated with Crush Assisted-by: Crush:deepseek-v4-pro
Remove vestigial single-hook preview (startSingleRun, runningAll, chanWriter, "enter" keybind) now that the TUI always runs the full scaffold. Add a 3-choice Run/Skip/Cancel modal, ANSI-aware viewport wrapping via ansi.Hardwrap, and named layout constants replacing the magic numbers (42, -3, 20). Size the modal canvas to the rendered base instead of m.width so the view pane's right border is never clipped. Also add regression tests for Cancel dismissal, modal border clipping, and update assertions for the new modal content. 💘 Generated with Crush Assisted-by: Crush:deepseek-v4-pro
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds hook discovery and output routing, a full-screen hook review TUI with run/skip/cancel controls, streamed scaffold execution, and integration with the interactive ChangesHook review flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant runNew
participant newTUIModel
participant hooksModel
participant tpl.RenderToWithPost
participant channelHookOutput
runNew->>newTUIModel: start interactive template flow
newTUIModel->>hooksModel: transition after parameter resolution
hooksModel->>tpl.RenderToWithPost: submit run or skip mode
tpl.RenderToWithPost->>channelHookOutput: stream hook output lines
channelHookOutput->>hooksModel: deliver runLineMsg
tpl.RenderToWithPost->>hooksModel: deliver runDoneMsg
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/new_tui.go (1)
118-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute quit keys through the active step first.
cmd/new_tui.gointerceptsesc/q/ctrl+cbeforem.hooks.update, so the hooks-step modal’sescclose path and hooks-stepctrl+chandling never run in the real TUI flow. Handle these keys in the active step first, or limit the global shortcut tostepForm.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/new_tui.go` around lines 118 - 124, Update the KeyPressMsg handling around the active step dispatch so esc, q, and ctrl+c are offered to m.hooks.update before global quit handling. Preserve the global tea.Quit and tea.Interrupt shortcuts for stepForm, while allowing the hooks-step modal close and ctrl+c behavior to execute first.
🧹 Nitpick comments (1)
cmd/hooks_tui.go (1)
201-204: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winQuadratic output growth on streamed runs. Each
runLineMsgappends tom.outputand then re-wraps the entire accumulated buffer viawrapForView, so a verbose hook that emits N lines does O(N²) wrapping/allocation work. Consider wrapping only the incoming chunk and appending to the viewport, or capping/truncating retained output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/hooks_tui.go` around lines 201 - 204, Optimize the runLineMsg handling so streamed output does not re-wrap the entire accumulated m.output on every message. Update the flow around wrapForView to wrap only the incoming msg.text and append it incrementally to the viewport, or otherwise cap retained output while preserving bottom-scrolling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/new_tui.go`:
- Around line 336-343: Update the TUI execution flow around tea.NewProgram and
p.Run to capture the final Bubble Tea model returned by p.Run, then read
hooks.didRun and resolved from that final model rather than the original m.
Preserve the existing collectResolved fallback using the final model’s params
and name before returning.
In `@internal/template/hooks_view_test.go`:
- Around line 19-26: Handle the errors returned by f.Close and each
f.WriteString call in the test helper, including writes in the pres and posts
loops. Update the helper’s error-handling flow so errcheck passes while
preserving the generated template content and cleanup behavior.
---
Outside diff comments:
In `@cmd/new_tui.go`:
- Around line 118-124: Update the KeyPressMsg handling around the active step
dispatch so esc, q, and ctrl+c are offered to m.hooks.update before global quit
handling. Preserve the global tea.Quit and tea.Interrupt shortcuts for stepForm,
while allowing the hooks-step modal close and ctrl+c behavior to execute first.
---
Nitpick comments:
In `@cmd/hooks_tui.go`:
- Around line 201-204: Optimize the runLineMsg handling so streamed output does
not re-wrap the entire accumulated m.output on every message. Update the flow
around wrapForView to wrap only the incoming msg.text and append it
incrementally to the viewport, or otherwise cap retained output while preserving
bottom-scrolling behavior.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8def50bb-5ee7-4aa6-9a56-238c75474d14
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
cmd/hook_output.gocmd/hooks_tui.gocmd/hooks_tui_test.gocmd/new.gocmd/new_tui.gogo.modinternal/template/hooks_view.gointernal/template/hooks_view_test.gointernal/template/pre_hook.go
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/template/pre_hook.go (2)
185-208: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQuote hook script paths before passing them to
sh -c. Filenames with spaces or shell metacharacters will be split/interpreted whenrenderedis executed, soscriptCommandshould shell-escape the path and a fixture should cover such a filename.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/template/pre_hook.go` around lines 185 - 208, The scriptCommand function currently returns unescaped paths, allowing spaces and shell metacharacters in hook filenames to be split or interpreted when rendered through sh -c. Update scriptCommand to shell-escape the generated scriptPath for both executable and sh command forms, and add a fixture covering a filename with spaces or shell metacharacters.
139-143: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn
Flusherrors from hook output.flushWriterdropshookTreeWriter.Flush()failures, so a sink error on the final buffered line can be lost while the hook still returns success. Propagate that error from both call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/template/pre_hook.go` around lines 139 - 143, Update both hook output flush call sites around c.Run() to return errors from flushWriter instead of discarding them. Change flushWriter to propagate hookTreeWriter.Flush() failures, and ensure the pre- and post-hook paths return that error while preserving the existing command-error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/template/pre_hook.go`:
- Around line 185-208: The scriptCommand function currently returns unescaped
paths, allowing spaces and shell metacharacters in hook filenames to be split or
interpreted when rendered through sh -c. Update scriptCommand to shell-escape
the generated scriptPath for both executable and sh command forms, and add a
fixture covering a filename with spaces or shell metacharacters.
- Around line 139-143: Update both hook output flush call sites around c.Run()
to return errors from flushWriter instead of discarding them. Change flushWriter
to propagate hookTreeWriter.Flush() failures, and ensure the pre- and post-hook
paths return that error while preserving the existing command-error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c2978d1-4b4c-4eec-b016-3fe8c5ecc471
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
cmd/hooks_tui_test.gocmd/new_prompts.gogo.modinternal/template/pre_hook.go
💤 Files with no reviewable changes (1)
- cmd/new_prompts.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- cmd/hooks_tui_test.go
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced Jul 22, 2026
Merged
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 this does
Adds an interactive hook review screen to
spin new. Before any shellcommands run, the user sees every
[[pre]]and[[post]]hook thetemplate will execute and chooses Run / Skip / Cancel.
Why
spin new --yesskipped the trust prompt entirely.spin newwithout--yesprinted a console prompt but showed zero detail about what hookswould run. Neither path let users inspect the commands before execution.
This is important for a scaffolder that shells out to arbitrary templates.
How it works
spin newnow has two TUI steps instead of one:The hooks TUI shows a list of hooks on the left (inline commands and
script files) with a detail pane on the right. Pressing
Ropens aRun / Skip / Cancel modal centered over the view. The user reviews
every command before anything executes.
Key changes
cmd/hook_output.go-- sharedhookTreeWriterthat emits tree-styledhook output for both the CLI (
os.Stdout) and the TUI (channel).Replaces the old
hookOutput/tuiHookOutput/chanWriterduplication.cmd/hooks_tui.go-- the Bubble Tea model with list, viewport, modal,ANSI-aware text wrapping, and canvas compositing for the centered
Run/Skip/Cancel dialog.
internal/template/pre_hook.go-- newStepStartcallback inHookOptionsso callers can style per-step headers without pullinglipgloss into the template engine. Added
flushWriterto drainbuffered output after each hook command finishes.
startSingleRun,runningAll,chanWriter,orphaned
RunSingleHookandhookAssetDir.rendered base, not
m.width, so the view pane's right border is neverclipped.
Testing
pump()Before / after
Before:
spin newprompts "Run hooks? [y/N]" with no detail.After: a scrollable hook list with Run / Skip / Cancel, live output
streaming, and the ability to inspect every command before execution.
Summary by CodeRabbit