diff --git a/.opencode/agents/review.md b/.opencode/agents/review.md new file mode 100644 index 0000000..02a64af --- /dev/null +++ b/.opencode/agents/review.md @@ -0,0 +1,57 @@ +--- +description: You are GoSys-Reviewer, a Staff-Level Systems Software Engineer and expert Go (Golang) code reviewer. You specialize in high-concurrency applications, OS-level process management, and high-throughput backend services. +mode: primary +model: openai/gpt-5.4 +temperature: 0.1 +permission: + edit: deny + bash: + "*": ask + "git diff": allow + "git log*": allow + "git *": allow + "grep *": allow + webfetch: deny +color: "#e01da6" +--- + +# Role Definition +You are `GoSys-Reviewer`, a Staff-Level Systems Software Engineer and expert Go (Golang) code reviewer. You specialize in high-concurrency applications, OS-level process management, and high-throughput backend services. + +# Primary Objective +Review the provided Go source code, pull request, or diff. Your goal is to identify bugs, concurrency flaws, memory inefficiencies, and deviations from idiomatic Go. You must provide constructive, actionable, and strictly technically accurate feedback structured for a professional engineering team. + +# Core Review Focus Areas + +## 1. Concurrency & Synchronization +- **Goroutine Leaks:** Scrutinize all `go func()` calls. Ensure every goroutine has a clear, deterministic exit path (e.g., via `context.Context` cancellation, channel closure, or `sync.WaitGroup`). +- **Race Conditions:** Look for shared mutable state accessed without proper synchronization (`sync.Mutex`, `sync.RWMutex`, or atomic operations). +- **Channel Operations:** Flag potential deadlocks, unbuffered channels blocking indefinitely, or writing to closed channels. + +## 2. Process & Lifecycle Management +- **Context Propagation:** Verify that `context.Context` is passed as the first argument in call chains and is correctly respected for timeouts and cancellations. +- **Signal Handling:** For systems-level code, ensure `os/signal` is properly used to intercept `SIGTERM` and `SIGINT` to allow for graceful shutdown, particularly for container-friendly execution. +- **Sub-process Execution:** When `os/exec` is used, check for proper handling of `Stdout/Stderr`, zombie process prevention, and input sanitization. + +## 3. Memory & Resource Efficiency +- **I/O & File Management:** Ensure file descriptors, sockets, and HTTP response bodies are explicitly closed using `defer` immediately after successful allocation. +- **Allocations:** Look for unnecessary heap allocations. Suggest `sync.Pool` for highly repetitive allocations or pre-allocating slice capacities (`make([]T, 0, capacity)`). +- **Pointer Semantics:** Verify appropriate use of pointers versus value receivers. Flag large structs passed by value. + +## 4. Idiomatic Go (Effective Go) +- **Error Handling:** Ensure errors are handled explicitly, wrapped intelligently using `fmt.Errorf("... %w", err)`, and not silently swallowed. +- **Interface Segregation:** Prefer small, consumer-defined interfaces over large, monolithic provider interfaces. +- **Naming Conventions:** Enforce standard Go naming (e.g., `MixedCaps` for variables, descriptive names for exported identifiers, concise names for local variables). + +# Required Output Structure +You must structure your review using the exact markdown format below. If a section has no findings, explicitly state "No issues found." + +### Review Summary +[Provide a 2-3 sentence high-level assessment of the code's architectural approach, quality, and primary risks.] + +### Critical Issues (Blocking) +[List severe bugs, race conditions, memory leaks, or architectural flaws that must be fixed before merging. Provide code snippets showing the fix.] +- **Issue:** - **Impact:** - **Suggested Fix:** ### ⚠️ Minor & Non-Blocking Suggestions +[List optimizations, refactoring opportunities, or stylistic improvements.] +- **Suggestion:** - **Rationale:** ### 💡 Idiomatic Go / Systems Best Practices +[Provide one specific, educational tip related to the code provided to help the author deepen their systems programming knowledge.] diff --git a/internal/app/process.go b/internal/app/process.go index 44b7389..3040293 100644 --- a/internal/app/process.go +++ b/internal/app/process.go @@ -15,7 +15,7 @@ func StartProcess(cmd model.Command, addr string, ch chan<- model.Event) (*model ch <- model.Event{ Time: time.Now().Local(), Type: model.EventTypeProcessStarting, - Body: fmt.Sprintf("spawning process '%s'", process.CommandString(cmd)), + Body: fmt.Sprintf("starting process '%s'", process.CommandString(cmd)), } proc := process.NewProcess(cmd, addr, ch) @@ -38,7 +38,7 @@ func StopProcess(proc *model.Process, ch chan<- model.Event, sig syscall.Signal) ch <- model.Event{ Time: time.Now().Local(), Type: model.EventTypeProcessSignaled, - Body: fmt.Sprintf("process with pid '%d' is being killed", proc.Exec.Process.Pid), + Body: fmt.Sprintf("process received signal '%s'", sig.String()), PID: proc.Exec.Process.Pid, } @@ -56,13 +56,18 @@ func waitForProcessExit(proc *model.Process, ch chan<- model.Event) { if proc == nil || proc.Exec == nil { return } + defer func() { + if proc.Done != nil { + close(proc.Done) + } + }() if err := proc.Exec.Wait(); err != nil { if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { ch <- model.Event{ Time: time.Now().Local(), Type: model.EventTypeProcessExited, - Body: fmt.Sprintf("process pid '%d' exited", proc.Exec.Process.Pid), + Body: "process exited", PID: proc.Exec.Process.Pid, ExitCode: exitErr.ExitCode(), } @@ -79,12 +84,13 @@ func waitForProcessExit(proc *model.Process, ch chan<- model.Event) { return } + code := proc.Exec.ProcessState.ExitCode() ch <- model.Event{ Time: time.Now().Local(), Type: model.EventTypeProcessExited, - Body: fmt.Sprintf("process pid '%d' exited", proc.Exec.Process.Pid), + Body: "process exited", PID: proc.Exec.Process.Pid, - ExitCode: 0, + ExitCode: code, } process.UpdateStatus(proc, false, ch) } diff --git a/internal/app/session.go b/internal/app/session.go index 6713506..2888376 100644 --- a/internal/app/session.go +++ b/internal/app/session.go @@ -1,10 +1,14 @@ package app import ( + "errors" + "fmt" "sync" "syscall" + "time" "termtap.dev/internal/model" + "termtap.dev/internal/process" "termtap.dev/internal/proxy" ) @@ -14,9 +18,18 @@ type Session struct { ch chan model.Event proxy *model.ProxyServer proc *model.Process + cmd model.Command + addr string once sync.Once + + restartMu sync.Mutex + restarting bool + stopped bool } +var ErrRestartInProgress = errors.New("restart already in progress") +var ErrSessionStopped = errors.New("session is stopped") + func StartSession(cmd model.Command, addr string) (*Session, error) { msgs := make(chan model.Event, 256) @@ -38,6 +51,8 @@ func StartSession(cmd model.Command, addr string) (*Session, error) { ch: msgs, proxy: ps, proc: proc, + cmd: cmd, + addr: addr, }, nil } @@ -47,7 +62,83 @@ func (s *Session) Stop() { } s.once.Do(func() { - StopProcess(s.proc, s.ch, syscall.SIGTERM) - proxy.Destroy(s.proxy, s.ch) + s.restartMu.Lock() + s.stopped = true + proc := s.proc + proxyServer := s.proxy + ch := s.ch + s.restartMu.Unlock() + + StopProcess(proc, ch, syscall.SIGTERM) + proxy.Destroy(proxyServer, ch) }) } + +func (s *Session) RestartProcess() error { + if s == nil { + return fmt.Errorf("session is nil") + } + + s.restartMu.Lock() + if s.stopped { + s.restartMu.Unlock() + return ErrSessionStopped + } + if s.restarting { + s.restartMu.Unlock() + return ErrRestartInProgress + } + s.restarting = true + current := s.proc + cmd := s.cmd + addr := s.addr + ch := s.ch + s.restartMu.Unlock() + + defer func() { + s.restartMu.Lock() + s.restarting = false + s.restartMu.Unlock() + }() + + ch <- model.Event{ + Time: time.Now().Local(), + Type: model.EventTypeProcessRestarting, + Body: fmt.Sprintf("restarting process '%s'", process.CommandString(cmd)), + } + + if current != nil { + StopProcess(current, ch, syscall.SIGTERM) + if !waitForProcessStop(current, 3*time.Second) { + return fmt.Errorf("timeout while waiting for process to stop") + } + } + + proc, err := StartProcess(cmd, addr, ch) + if err != nil { + return err + } + + s.restartMu.Lock() + defer s.restartMu.Unlock() + if s.stopped { + StopProcess(proc, ch, syscall.SIGTERM) + return fmt.Errorf("session stopped during restart: %w", ErrSessionStopped) + } + s.proc = proc + + return nil +} + +func waitForProcessStop(proc *model.Process, timeout time.Duration) bool { + if proc == nil || proc.Done == nil { + return true + } + + select { + case <-proc.Done: + return true + case <-time.After(timeout): + return false + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index cf8919d..8fef507 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -26,7 +26,11 @@ func Run(args []string) { } defer session.Stop() - if err := tui.Run(session.Events); err != nil { + controls := tui.Controls{ + Restart: session.RestartProcess, + } + + if err := tui.Run(session.Events, controls); err != nil { log.Fatalln(err) } } diff --git a/internal/model/event.go b/internal/model/event.go index 5d532d1..6d990d2 100644 --- a/internal/model/event.go +++ b/internal/model/event.go @@ -16,12 +16,13 @@ const ( EventTypeRequestFinished EventType = "RequestFinished" EventTypeRequestFailed EventType = "RequestFailed" - EventTypeProcessStarting EventType = "ProcessStarting" - EventTypeProcessStarted EventType = "ProcessStarted" - EventTypeProcessExited EventType = "ProcessExited" - EventTypeProcessSignaled EventType = "ProcessSignaled" - EventTypeProcessStdout EventType = "ProcessStdout" - EventTypeProcessStderr EventType = "ProcessStderr" + EventTypeProcessStarting EventType = "ProcessStarting" + EventTypeProcessRestarting EventType = "ProcessRestarting" + EventTypeProcessStarted EventType = "ProcessStarted" + EventTypeProcessExited EventType = "ProcessExited" + EventTypeProcessSignaled EventType = "ProcessSignaled" + EventTypeProcessStdout EventType = "ProcessStdout" + EventTypeProcessStderr EventType = "ProcessStderr" EventTypeFatal EventType = "Fatal" EventTypeWarn EventType = "Warn" diff --git a/internal/model/process.go b/internal/model/process.go index 59d600a..7850076 100644 --- a/internal/model/process.go +++ b/internal/model/process.go @@ -11,4 +11,5 @@ type Process struct { Command Command Exec *exec.Cmd Running bool + Done chan struct{} } diff --git a/internal/process/runner.go b/internal/process/runner.go index 521b1b9..18b7ad7 100644 --- a/internal/process/runner.go +++ b/internal/process/runner.go @@ -50,6 +50,7 @@ func NewProcess(cmd model.Command, addr string, ch chan<- model.Event) *model.Pr Command: cmd, Exec: proc, Running: false, + Done: make(chan struct{}), } } @@ -106,7 +107,7 @@ func UpdateStatus(proc *model.Process, running bool, ch chan<- model.Event) { ch <- model.Event{ Time: time.Now().Local(), Type: t, - Body: fmt.Sprintf("Set process pid '%d' status to %s", proc.Exec.Process.Pid, status), + Body: fmt.Sprintf("set process status to %s", status), PID: proc.Exec.Process.Pid, } } diff --git a/internal/tui/messages.go b/internal/tui/messages.go index 91bac3a..1702878 100644 --- a/internal/tui/messages.go +++ b/internal/tui/messages.go @@ -19,6 +19,10 @@ type TickMsg struct { Now time.Time } +type RestartResultMsg struct { + err error +} + const tick = 20 * time.Millisecond func tickCmd() tea.Cmd { @@ -26,3 +30,13 @@ func tickCmd() tea.Cmd { return TickMsg{Now: t} }) } + +func restartCmd(restart func() error) tea.Cmd { + if restart == nil { + return nil + } + + return func() tea.Msg { + return RestartResultMsg{err: restart()} + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 9330872..59fd773 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -15,7 +15,8 @@ const ( ) type Model struct { - channel <-chan model.Event + channel <-chan model.Event + controls Controls events []model.Event requests []model.Request @@ -27,13 +28,19 @@ type Model struct { showEvents bool showStd bool showSearch bool + restarting bool now time.Time } -func NewModel(ch <-chan model.Event) Model { +type Controls struct { + Restart func() error +} + +func NewModel(ch <-chan model.Event, controls Controls) Model { return Model{ channel: ch, + controls: controls, events: make([]model.Event, 0, maxEvents), requests: make([]model.Request, 0, maxRequests), width: 0, @@ -41,12 +48,13 @@ func NewModel(ch <-chan model.Event) Model { showEvents: false, showStd: false, showSearch: false, + restarting: false, theme: newTheme(), } } -func Run(ch <-chan model.Event) error { - p := tea.NewProgram(NewModel(ch), tea.WithAltScreen()) +func Run(ch <-chan model.Event, controls Controls) error { + p := tea.NewProgram(NewModel(ch, controls), tea.WithAltScreen()) _, err := p.Run() return err } diff --git a/internal/tui/panes.go b/internal/tui/panes.go index 5c9d88c..3851419 100644 --- a/internal/tui/panes.go +++ b/internal/tui/panes.go @@ -23,7 +23,7 @@ func (m Model) renderStatusBar(w int) string { avg := int(msSum) / max(1, len(m.requests)) left := fmt.Sprintf(" tap %3d reqs | %d err | avg %dms", len(m.requests), errCount, avg) - right := "j/k nav / search tab panel e events o output r replay ctrl+r restart q quit " + right := "j/k nav / search tab panel e events o output r/^r restart q quit " spaceSize := max(w-(len(left)+len(right)), 0) space := strings.Repeat(" ", spaceSize) @@ -127,7 +127,7 @@ func (m Model) renderEventsPane(w, h int) []string { for _, event := range events { var ( eTime string = m.theme.TextMuted.Render(event.Time.Format("15:04:05") + " ") - eType string = getEventColor(m.theme, event.Type).Render(fmt.Sprintf("%-15s ", event.Type)) + eType string = getEventColor(m.theme, event.Type).Render(fmt.Sprintf("%-17s ", event.Type)) avail int = max(0, w-lipgloss.Width(eTime+eType)) body string = clampRendered(m.theme.Text.Render(event.Body), avail) diff --git a/internal/tui/style.go b/internal/tui/style.go index c31534b..2fe3d85 100644 --- a/internal/tui/style.go +++ b/internal/tui/style.go @@ -22,6 +22,7 @@ type Theme struct { EventSession lipgloss.Style EventProcess lipgloss.Style EventProxy lipgloss.Style + EventRestart lipgloss.Style EventRequestInFlight lipgloss.Style EventSuccess lipgloss.Style EventWarn lipgloss.Style @@ -89,6 +90,10 @@ func newTheme() Theme { Foreground(violetBlue). Background(background). Bold(true), + EventRestart: lipgloss.NewStyle(). + Foreground(cyan). + Background(background). + Bold(true), EventRequestInFlight: lipgloss.NewStyle(). Foreground(cyan). Background(background). @@ -136,6 +141,9 @@ func getEventColor(theme Theme, event model.EventType) lipgloss.Style { case model.EventTypeRequestStarted: return theme.EventRequestInFlight + case model.EventTypeProcessRestarting: + return theme.EventRestart + case model.EventTypeRequestFinished: return theme.EventSuccess diff --git a/internal/tui/update.go b/internal/tui/update.go index 074f655..ef57001 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -27,6 +27,15 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg.String() { case "ctrl+c", "q": return m, tea.Quit + case "r", tea.KeyCtrlR.String(): + if m.restarting { + return m, nil + } + if m.controls.Restart == nil { + return m, nil + } + m.restarting = true + return m, restartCmd(m.controls.Restart) case "e": m.showEvents = !m.showEvents case "o": @@ -39,13 +48,24 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case ErrMsg: - m.events = append(m.events, model.Event{ + m.pushEvent(model.Event{ Time: time.Now().Local(), Type: model.EventTypeWarn, Body: fmt.Sprintf("tui event stream closed: %v", msg.err), }) return m, nil + case RestartResultMsg: + m.restarting = false + if msg.err != nil { + m.pushEvent(model.Event{ + Time: time.Now().Local(), + Type: model.EventTypeWarn, + Body: fmt.Sprintf("failed to restart process: %v", msg.err), + }) + } + return m, nil + case EventMsg: m.pushEvent(msg.value) m.applyMessage(msg.value)