Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions internal/cli/output_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
30 changes: 19 additions & 11 deletions internal/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"os"
"syscall"

"github.com/charmbracelet/fang"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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))
}
Expand Down
19 changes: 11 additions & 8 deletions internal/tui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
}
}
Comment on lines +905 to +910

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The addition of clearHookCancel is a good improvement for managing hook lifecycles. However, the hooks are still being started with context.Background() in startNextHook (line 882). To fully achieve the PR's goal of propagating signal cancellation, the TUI should ideally receive and use the command context (from cobra.Command) so that signals handled by fang in the root command can also trigger cancellation of in-flight TUI hooks.


// continueExecution proceeds with the execution after hooks complete
func (m *Model) continueExecution() tea.Cmd {
// Determine what to do next based on hook phase
Expand Down
Loading