From 7c44f786938ea4490aa7eb807c8a3b8c814563f6 Mon Sep 17 00:00:00 2001 From: Benny Yen Date: Sun, 26 Apr 2026 17:18:53 +0800 Subject: [PATCH] fix(cli): propagate signal cancellation, expose post-push warnings in JSON - Wire SIGINT/SIGTERM cancellation through fang.WithNotifySignal so Ctrl+C during a non-interactive bump unwinds the in-flight git push and hooks (executor.Execute already plumbs ctx into PushTag and RunHooks). - Add PostPushWarnings to JSONOutput; outputJSON was silently dropping result.PostPushWarnings, leaving --json users blind to fail-open hook failures that the text and TUI paths already surface. - Consolidate the per-hook CancelFunc cleanup in the TUI into clearHookCancel(), and call it in HookCompleteMsg so the final hook's context is released promptly instead of lingering until the next phase starts (or never, for the last phase). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/output_test.go | 58 +++++++++++++++++++++++++++++++++++++ internal/cli/root.go | 30 ++++++++++++------- internal/tui/model.go | 19 +++++++----- 3 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 internal/cli/output_test.go diff --git a/internal/cli/output_test.go b/internal/cli/output_test.go new file mode 100644 index 0000000..b6b669f --- /dev/null +++ b/internal/cli/output_test.go @@ -0,0 +1,58 @@ +package cli + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/benny123tw/bumpkin/internal/executor" +) + +func TestOutputJSON_IncludesPostPushWarnings(t *testing.T) { + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + result := &executor.Result{ + PreviousVersion: "1.0.0", + NewVersion: "1.1.0", + TagName: "v1.1.0", + CommitHash: "abc1234", + TagCreated: true, + Pushed: true, + PostPushWarnings: []string{"hook 'notify': exit 1", "hook 'changelog': timeout"}, + } + + require.NoError(t, outputJSON(cmd, result, nil)) + + var out JSONOutput + require.NoError(t, json.Unmarshal(buf.Bytes(), &out)) + assert.True(t, out.Success) + assert.Equal(t, "v1.1.0", out.TagName) + assert.Equal(t, + []string{"hook 'notify': exit 1", "hook 'changelog': timeout"}, + out.PostPushWarnings, + ) +} + +func TestOutputJSON_OmitsEmptyPostPushWarnings(t *testing.T) { + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + result := &executor.Result{ + PreviousVersion: "1.0.0", + NewVersion: "1.1.0", + TagName: "v1.1.0", + CommitHash: "abc1234", + TagCreated: true, + Pushed: true, + } + + require.NoError(t, outputJSON(cmd, result, nil)) + assert.NotContains(t, buf.String(), "post_push_warnings") +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 1eb700c..c64984e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "syscall" "github.com/charmbracelet/fang" "github.com/spf13/cobra" @@ -44,15 +45,16 @@ var ( // JSONOutput represents the JSON output format for non-interactive mode type JSONOutput struct { - Success bool `json:"success"` - PreviousVersion string `json:"previous_version"` - NewVersion string `json:"new_version"` - TagName string `json:"tag_name"` - CommitHash string `json:"commit_hash"` - TagCreated bool `json:"tag_created"` - Pushed bool `json:"pushed"` - DryRun bool `json:"dry_run"` - Error string `json:"error,omitempty"` + Success bool `json:"success"` + PreviousVersion string `json:"previous_version"` + NewVersion string `json:"new_version"` + TagName string `json:"tag_name"` + CommitHash string `json:"commit_hash"` + TagCreated bool `json:"tag_created"` + Pushed bool `json:"pushed"` + DryRun bool `json:"dry_run"` + PostPushWarnings []string `json:"post_push_warnings,omitempty"` + Error string `json:"error,omitempty"` } type rootCommand struct { @@ -310,7 +312,7 @@ func runNonInteractive(cmd *cobra.Command, repo *git.Repository, cfg *config.Con PostPushHooks: cfg.Hooks.PostPush, } - result, err := executor.Execute(context.Background(), req) + result, err := executor.Execute(cmd.Context(), req) if err != nil { return handleError(cmd, err, "bump failed") } @@ -369,6 +371,7 @@ func outputJSON(cmd *cobra.Command, result *executor.Result, err error) error { output.CommitHash = result.CommitHash output.TagCreated = result.TagCreated output.Pushed = result.Pushed + output.PostPushWarnings = result.PostPushWarnings } encoder := json.NewEncoder(cmd.OutOrStdout()) @@ -424,7 +427,12 @@ func outputText(cmd *cobra.Command, result *executor.Result) error { // On failure, it writes the error to stderr and exits the process with the error's exit code. func Execute(info BuildInfo) { c := newRootCommand(info) - if err := fang.Execute(context.Background(), c.cmd); err != nil { + err := fang.Execute( + context.Background(), + c.cmd, + fang.WithNotifySignal(os.Interrupt, syscall.SIGTERM), + ) + if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(GetExitCode(err)) } diff --git a/internal/tui/model.go b/internal/tui/model.go index 71d05a8..41a0ef5 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -269,6 +269,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case HookCompleteMsg: + m.clearHookCancel() if !msg.Success { // For post-push hooks, we use fail-open (warnings, not errors) if m.hookPhase == hooks.PostPush { @@ -335,9 +336,7 @@ func (m Model) handleCtrlC() (tea.Model, tea.Cmd) { // Check if this is second ctrl+c within 3 seconds if m.cancelPending && time.Since(m.cancelPendingTime) < 3*time.Second { // Cancel the hook - if m.hookCancelFunc != nil { - m.hookCancelFunc() - } + m.clearHookCancel() m.cancelPending = false m.err = fmt.Errorf("hook cancelled by user") m.state = StateError @@ -880,11 +879,6 @@ func (m *Model) startNextHook() tea.Cmd { DryRun: m.config.DryRun, } - // Call previous cancel before overwriting — hooks completing normally don't cancel their own context. - if m.hookCancelFunc != nil { - m.hookCancelFunc() - } - ctx, cancel := context.WithCancel(context.Background()) m.hookCancelFunc = cancel @@ -906,6 +900,15 @@ func (m *Model) startNextHook() tea.Cmd { ) } +// clearHookCancel cancels and clears the current hook's cancel func, if any. +// Safe to call multiple times. +func (m *Model) clearHookCancel() { + if m.hookCancelFunc != nil { + m.hookCancelFunc() + m.hookCancelFunc = nil + } +} + // continueExecution proceeds with the execution after hooks complete func (m *Model) continueExecution() tea.Cmd { // Determine what to do next based on hook phase