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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- `prenup run` now cancels its context on SIGINT/SIGTERM instead of letting
the OS terminate the process outright. Previously, an external kill (a
CI/editor timeout, a git GUI's "cancel" button, etc.) skipped all deferred
cleanup, so the `clean_worktree` stash was never popped, leaving an
orphaned stash that surfaced as a confusing conflict later. A second
SIGINT/SIGTERM still forces immediate termination.
- Once a run is canceled this way, any task that hadn't started yet is now
reported as a single, clear "run interrupted" skip instead of a wall of
misleading per-task "context canceled" failures.

## [[v0.1.0](https://github.com/C2FO/prenup/releases/tag/v0.1.0)] - 2026-07-04
### Fixed
Expand Down
23 changes: 21 additions & 2 deletions internal/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import (
"errors"
"fmt"
"os"
"os/signal"
"path/filepath"
"strings"
"syscall"
"time"

tea "github.com/charmbracelet/bubbletea"
Expand Down Expand Up @@ -92,8 +94,8 @@ func runRun(cmd *cobra.Command, _ []string, opts runOptions) error {
opts = readRunOptions(cmd)
}

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctx, stop := newRunContext()
defer stop()

repoRoot, err := git.RepoRoot("")
if err != nil {
Expand Down Expand Up @@ -167,6 +169,23 @@ func runRun(cmd *cobra.Command, _ []string, opts runOptions) error {
return nil
}

// newRunContext wires SIGINT/SIGTERM to ctx cancellation rather than letting
// the OS terminate the process outright. Without this, an external kill
// (e.g. a CI/editor timeout, or a git GUI's "cancel" sending SIGTERM to the
// hook process) skips Go's deferred cleanup entirely, so the clean-worktree
// stash created by beginStash is never popped (endStash's defer never runs)
// — leaving an orphaned stash that surprises the user on their next
// `git stash pop` or rebase. Cancellation gives BashExecutor.Run (which
// already reacts to ctx via cmd.Cancel) a chance to gracefully stop the
// in-flight task so Run can return and the deferred endStash still executes.
//
// A second SIGINT/SIGTERM reverts to the OS default (immediate termination)
// per signal.NotifyContext's documented behavior, so an unresponsive task
// can still be force-killed.
func newRunContext() (context.Context, context.CancelFunc) {
return signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
}

// printlnSafe writes msg to stdout when non-empty. Returns the underlying
// write error so callers can propagate it.
func printlnSafe(msg string) error {
Expand Down
42 changes: 42 additions & 0 deletions internal/cli/run_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (
"os/exec"
"path/filepath"
"strings"
"syscall"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -187,3 +189,43 @@ func TestLoadRunConfigExplicitPath(t *testing.T) {
is.Equal(1, cfg.Version)
is.Len(cfg.Tasks, 1)
}

// TestNewRunContext_CanceledOnSIGTERM pins the fix for orphaned stashes: an
// external SIGTERM (e.g. a CI/editor timeout killing the hook process)
// must cancel the run context rather than letting the OS terminate the
// process before the deferred stash-pop cleanup in runRun/runner.Run runs.
// Not run with t.Parallel(): both tests deliver real OS signals to this
// process, and interleaving them with other signal-registering tests could
// make failures nondeterministic.
func TestNewRunContext_CanceledOnSIGTERM(t *testing.T) {
must := require.New(t)

ctx, stop := newRunContext()
defer stop()

must.NoError(syscall.Kill(os.Getpid(), syscall.SIGTERM))

select {
case <-ctx.Done():
// expected: signal delivery canceled the context.
case <-time.After(5 * time.Second):
t.Fatal("context was not canceled after SIGTERM")
}
}

// TestNewRunContext_CanceledOnSIGINT mirrors the SIGTERM case for Ctrl-C.
func TestNewRunContext_CanceledOnSIGINT(t *testing.T) {
must := require.New(t)

ctx, stop := newRunContext()
defer stop()

must.NoError(syscall.Kill(os.Getpid(), syscall.SIGINT))

select {
case <-ctx.Done():
// expected: signal delivery canceled the context.
case <-time.After(5 * time.Second):
t.Fatal("context was not canceled after SIGINT")
}
}
28 changes: 27 additions & 1 deletion internal/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ type Result struct {
// FailedTasks is the names of tasks that ended in TaskStatusFailed,
// in selection order. Empty when Failed == 0.
FailedTasks []string
// Interrupted is true when ctx was canceled (e.g. by an external
// SIGINT/SIGTERM) before every selected task had a chance to start.
// Callers can use this to distinguish "the run was cut short" from
// "every attempted task genuinely failed".
Interrupted bool
}

// Run executes the plan's selected tasks and emits events to opts.Sink.
Expand Down Expand Up @@ -167,10 +172,31 @@ func Run(ctx context.Context, plan Plan, opts Options) (Result, error) {
})
continue
}
// Once ctx is canceled (e.g. an external SIGINT/SIGTERM), stop
// starting new tasks: exec.CommandContext would just fail each one
// immediately with "context canceled", which previously surfaced as
// a wall of misleading per-task failures instead of one clear
// "run was interrupted" outcome. A task already in flight when
// cancellation happens is unaffected by this check — it observes
// ctx via its own exec.CommandContext call and runs to whatever
// outcome that produces (typically a cancellation-driven failure,
// but a task that finishes just as the signal arrives can still
// succeed); only tasks that haven't started yet are skipped here.
if ctx.Err() != nil {
result.Interrupted = true
sink.Emit(Event{
Kind: EventTaskCompleted,
Time: time.Now(),
Task: pt.Task.Name,
Status: TaskStatusSkipped,
Message: "run interrupted: " + ctx.Err().Error(),
})
continue
}
runOneTask(ctx, pt, plan.RepoRoot, opts, exec, sink, maxPar, &result)
}

if result.Failed > 0 {
if result.Failed > 0 || result.Interrupted {
result.ExitCode = 1
}
sink.Emit(Event{
Expand Down
107 changes: 107 additions & 0 deletions internal/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@ package runner_test
import (
"context"
"errors"
"os"
osexec "os/exec"
"path/filepath"
"sync"
"sync/atomic"
"testing"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"

"github.com/c2fo/prenup/internal/config"
"github.com/c2fo/prenup/internal/git"
"github.com/c2fo/prenup/internal/runner"
"github.com/c2fo/prenup/internal/runner/mocks"
)
Expand Down Expand Up @@ -183,6 +188,108 @@ func (s *RunnerTestSuite) TestFailFastWithinTaskAcrossModules() {
s.True(foundFailureLine, "expected a synthetic failure-attribution line on EventLine/stderr")
}

// TestContextCancellationSkipsRemainingTasksAndPopsStash pins the fix for
// C2FO/prenup#2: once ctx is canceled (e.g. by an external SIGINT/SIGTERM),
// Run must not attempt any task that hasn't started yet (previously it did,
// and each one failed near-instantly with a misleading "context canceled"
// error), and the clean_worktree stash created by beginStash must still be
// popped once Run returns despite the run ending early.
func (s *RunnerTestSuite) TestContextCancellationSkipsRemainingTasksAndPopsStash() {
repo := s.T().TempDir()
runGit := func(args ...string) {
cmd := osexec.Command("git", args...) //nolint:gosec // G204: fixed binary, test-controlled args.
cmd.Dir = repo
out, err := cmd.CombinedOutput()
s.Require().NoErrorf(err, "git %v: %s", args, out)
}
runGit("init", "-b", "main")
runGit("config", "user.email", "test@example.com")
runGit("config", "user.name", "test")
runGit("config", "commit.gpgsign", "false")

trackedFile := filepath.Join(repo, "tracked.txt")
s.Require().NoError(os.WriteFile(trackedFile, []byte("committed\n"), 0o600))
runGit("add", "tracked.txt")
runGit("commit", "-m", "seed")

// Dirty the worktree so beginStash actually creates a stash (a clean
// worktree is a no-op Stash, which wouldn't exercise the pop path).
const unstagedContent = "unstaged-in-progress\n"
s.Require().NoError(os.WriteFile(trackedFile, []byte(unstagedContent), 0o600))

cfg := config.Config{
Version: 1,
Tasks: []config.Task{
{Name: "A", Command: "cmd-a", DefaultSelected: true},
{Name: "B", Command: "cmd-b", DefaultSelected: true},
},
}
plan := runner.BuildPlan(cfg, repo, []string{"tracked.txt"}, []string{"."}, nil)
s.Require().Len(plan.Tasks, 2)
s.Require().True(plan.Tasks[0].Selected)
s.Require().True(plan.Tasks[1].Selected)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var calls int32
fakeExec := mocks.NewExecutor(s.T())
fakeExec.EXPECT().
Run(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
RunAndReturn(func(_ context.Context, _, _ string, _ map[string]string, _ func(runner.Stream, string)) error {
atomic.AddInt32(&calls, 1)
// Simulate a signal arriving while task A is executing: cancel
// the shared context (as newRunContext's signal handler would)
// right as the task finishes, mirroring the window between a
// task completing and prenup noticing the cancellation.
cancel()
return nil
}).
Once()

sink := s.newRecordingSink()
opts := runner.Options{
Executor: fakeExec,
Git: git.New(repo),
Sink: sink.mock,
CleanWorktree: true,
}

result, err := runner.Run(ctx, plan, opts)
s.Require().NoError(err)

s.Equal(int32(1), atomic.LoadInt32(&calls), "task B's executor must never run once ctx is canceled")
s.True(result.Interrupted, "result must flag that the run was cut short")
s.Equal(1, result.Succeeded, "task A itself completed before the cancellation was noticed")
s.Equal(0, result.Failed, "task B must be reported as skipped, not a fabricated failure")
s.Equal(1, result.ExitCode, "an interrupted run must still exit non-zero even with zero real failures")

var sawSkippedB bool
events := sink.snapshot()
for i := range events {
ev := &events[i]
if ev.Kind == runner.EventTaskCompleted && ev.Task == "B" {
s.Equal(runner.TaskStatusSkipped, ev.Status)
s.Contains(ev.Message, "run interrupted")
sawSkippedB = true
}
}
s.True(sawSkippedB, "expected a task_completed/skipped event for the un-started task B")

// The core regression: beginStash's stash must still be popped even
// though the run ended early, and the pre-run unstaged edit must be
// restored intact.
listCmd := osexec.Command("git", "stash", "list")
listCmd.Dir = repo
out, err := listCmd.Output()
s.Require().NoError(err)
s.Empty(string(out), "prenup's autostash must be popped, not left orphaned")

restored, err := os.ReadFile(trackedFile) //nolint:gosec // G304: test-controlled path inside t.TempDir().
s.Require().NoError(err)
s.Equal(unstagedContent, string(restored), "the unstaged edit must be restored after the stash pop")
}

func (s *RunnerTestSuite) TestBuildPlanAppliesPathFilter() {
cfg := config.Config{
Version: 1,
Expand Down
Loading