diff --git a/cmd/gitpoll/main.go b/cmd/gitpoll/main.go index 47c3808..6b37359 100644 --- a/cmd/gitpoll/main.go +++ b/cmd/gitpoll/main.go @@ -78,6 +78,9 @@ func main() { } }() + var execCancel context.CancelFunc + var execDone chan struct{} + // Wire up event listeners eventBus.Subscribe(events.RepoChanged, func(payload interface{}) { err := gitManager.Pull(ctx) @@ -89,12 +92,35 @@ func main() { }) eventBus.Subscribe(events.RepoUpdated, func(payload interface{}) { - err := cmdExecutor.Execute(ctx, logCh) - if err != nil { - eventBus.Publish(events.ErrorOccurred, fmt.Errorf("command execution failed: %w", err)) - return + // Stop previous execution AFTER code is successfully pulled + if execCancel != nil { + execCancel() + // Wait for it to cleanly shut down + if execDone != nil { + <-execDone + } + execCancel = nil + execDone = nil } - eventBus.Publish(events.CommandExecuted, nil) + + // Start new execution + var execCtx context.Context + execCtx, execCancel = context.WithCancel(ctx) + execDone = make(chan struct{}) + + go func(c context.Context, done chan struct{}) { + defer close(done) + err := cmdExecutor.Execute(c, logCh) + if err != nil { + // Check if context was canceled, ignore error if it was a deliberate cancel + if c.Err() == context.Canceled { + return + } + eventBus.Publish(events.ErrorOccurred, fmt.Errorf("command execution failed: %w", err)) + return + } + eventBus.Publish(events.CommandExecuted, nil) + }(execCtx, execDone) }) // Start background worker diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 4835e26..55d0f4a 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -30,6 +30,13 @@ func NewExecutor(cfg *config.Config) Executor { func (e *defaultExecutor) Execute(ctx context.Context, logCh chan<- string) error { // #nosec G204 - shell execution explicitly requested by gitpoll design cmd := exec.CommandContext(ctx, "sh", "-c", e.command) + setProcessGroup(cmd) + + // In Go 1.20+, we can use cmd.Cancel to override how the process is killed + // when the context expires. + cmd.Cancel = func() error { + return killProcessGroup(cmd) + } stdout, err := cmd.StdoutPipe() if err != nil { diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index c1cbb7f..02072c0 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -47,13 +47,54 @@ func TestExecutor_ContextCancellation(t *testing.T) { logCh := make(chan string, 10) ctx, cancel := context.WithCancel(context.Background()) + start := time.Now() go func() { - time.Sleep(50 * time.Millisecond) + time.Sleep(200 * time.Millisecond) cancel() }() err := e.Execute(ctx, logCh) + duration := time.Since(start) + if err == nil { t.Fatal("Expected error due to cancellation, got nil") } + + if duration > 2*time.Second { + t.Fatalf("Cancellation took too long, expected < 2s, got %v", duration) + } +} + +func TestExecutor_ProcessGroupKill(t *testing.T) { + // A script that creates a long running child process + // We use 'sh -c' inherently in Execute. + // So we can just launch a shell loop that ignores simple signals + // or sleeps in a loop. + script := ` +while true; do + sleep 1 +done +` + cfg := &config.Config{Command: script} + e := NewExecutor(cfg) + + logCh := make(chan string, 10) + ctx, cancel := context.WithCancel(context.Background()) + + start := time.Now() + go func() { + time.Sleep(300 * time.Millisecond) + cancel() + }() + + err := e.Execute(ctx, logCh) + duration := time.Since(start) + + if err == nil { + t.Fatal("Expected error due to cancellation, got nil") + } + + if duration > 2*time.Second { + t.Fatalf("Process group kill failed or took too long, got %v", duration) + } } diff --git a/internal/executor/executor_unix.go b/internal/executor/executor_unix.go new file mode 100644 index 0000000..5610f29 --- /dev/null +++ b/internal/executor/executor_unix.go @@ -0,0 +1,26 @@ +//go:build !windows +// +build !windows + +package executor + +import ( + "os/exec" + "syscall" +) + +// setProcessGroup sets the process group ID so that the entire process tree can be killed. +func setProcessGroup(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true +} + +// killProcessGroup sends SIGKILL to the process group. +func killProcessGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return nil + } + // A negative PID sends the signal to all processes in the process group + return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) +} diff --git a/internal/executor/executor_windows.go b/internal/executor/executor_windows.go new file mode 100644 index 0000000..fbf78ea --- /dev/null +++ b/internal/executor/executor_windows.go @@ -0,0 +1,24 @@ +//go:build windows +// +build windows + +package executor + +import ( + "os/exec" +) + +// setProcessGroup on Windows does nothing in this basic implementation. +// For full process tree termination on Windows, one would typically use +// Taskkill or Job Objects. +func setProcessGroup(cmd *exec.Cmd) { + // Not supported natively via Setpgid on Windows +} + +// killProcessGroup on Windows just kills the process itself. +// This might leave orphaned child processes, but is a fallback for Windows. +func killProcessGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return nil + } + return cmd.Process.Kill() +} diff --git a/internal/tui/app.go b/internal/tui/app.go index de1bf36..91bfe26 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -18,6 +18,10 @@ var ( infoStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("241")) errorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("9")) logStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("250")) + boxStyle = lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + BorderForeground(lipgloss.Color("62")). + Padding(0, 1) ) // MonitorModel holds the state for the TUI monitor @@ -105,23 +109,42 @@ func (m *MonitorModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "q", "ctrl+c": m.cancelFunc() // graceful shutdown trigger return m, tea.Quit - case "up", "k": - m.viewport.ScrollUp(1) - case "down", "j": - m.viewport.ScrollDown(1) } + // Forward other key events to viewport (handles up, down, pgup, pgdown, mouse wheel, etc.) + var cmd tea.Cmd + m.viewport, cmd = m.viewport.Update(msg) + cmds = append(cmds, cmd) + + case tea.MouseMsg: + var cmd tea.Cmd + m.viewport, cmd = m.viewport.Update(msg) + cmds = append(cmds, cmd) case tea.WindowSizeMsg: m.width = msg.Width m.height = msg.Height - headerHeight := 8 + headerHeight := lipgloss.Height(m.headerView()) + 2 // include footer and margins + + // The box style uses borders, which consumes space. Let's calculate inner dimensions. + h, v := boxStyle.GetFrameSize() + + vpWidth := msg.Width - h - 2 + vpHeight := msg.Height - headerHeight - v + + if vpWidth < 0 { + vpWidth = 0 + } + if vpHeight < 0 { + vpHeight = 0 + } + if !m.ready { - m.viewport = viewport.New(msg.Width, msg.Height-headerHeight) + m.viewport = viewport.New(vpWidth, vpHeight) m.viewport.SetContent(strings.Join(m.logs, "\n")) m.ready = true } else { - m.viewport.Width = msg.Width - m.viewport.Height = msg.Height - headerHeight + m.viewport.Width = vpWidth + m.viewport.Height = vpHeight } case events.UpdateDetectedMsg: @@ -158,11 +181,7 @@ func (m *MonitorModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Batch(cmds...) } -func (m *MonitorModel) View() string { - if !m.ready { - return "\n Initializing... (Waiting for resize event to set viewport)" - } - +func (m *MonitorModel) headerView() string { title := titleStyle.Render("Gitpoll TUI") statusLine := fmt.Sprintf("Status: %s", m.status) hashLine := fmt.Sprintf("Latest Commit: %s", m.latestHash) @@ -176,11 +195,24 @@ func (m *MonitorModel) View() string { } logHeader := infoStyle.Render("--- Execution Logs ---") + return fmt.Sprintf("%s\n%s", header, logHeader) +} + +func (m *MonitorModel) View() string { + if !m.ready { + return "\n Initializing... (Waiting for resize event to set viewport)" + } + + header := m.headerView() + + // Wrap viewport in a styled box + viewportView := boxStyle. + Width(m.width - 2). // adjust width + Render(logStyle.Render(m.viewport.View())) - body := fmt.Sprintf("%s\n%s\n%s\n", header, logHeader, logStyle.Render(m.viewport.View())) - footer := infoStyle.Render("\nPress 'q' or 'Ctrl+C' to quit. Use Up/Down arrows to scroll logs.") + footer := infoStyle.Render("Press 'q' or 'Ctrl+C' to quit. Use Up/Down arrows to scroll logs.") - return body + footer + return fmt.Sprintf("%s\n%s\n%s", header, viewportView, footer) } type MainState int