umputun / ralphex / 21343491481

79%

push

github

Fix: kill entire process group on context cancellation (#21)

* fix: kill entire process group on context cancellation

When ralphex exits (max iterations, Ctrl+C, error), child processes spawned
by claude (like test commands) were being orphaned because exec.CommandContext
only kills the direct child process.

Changes:
- Use process groups (Setpgid: true) for spawned commands
- On context cancellation, kill entire process group with syscall.Kill(-pid)
- Add test that verifies child processes are killed with parent

This prevents orphaned processes that could block subsequent ralphex runs
due to file lock contention.

* refactor: extract shared process group cleanup helper

Address code review feedback:
- Extract duplicated process group logic into shared procgroup.go helper
- Make Wait() idempotent using sync.Once (prevents panic on double-call)
- Use SIGTERM before SIGKILL for graceful shutdown (100ms delay)
- Log kill errors instead of silently ignoring them
- Move Unix-specific tests to procgroup_test.go with build tag
- Replace time.Sleep with require.Eventually for less flaky tests
- Add test for Wait() idempotency

* fix: address code review feedback for process group cleanup

- Add PID validation before attempting process group kill
- Replace fmt.Printf with log.Printf for proper logging
- Extract gracefulShutdownDelay constant (100ms)
- Always attempt SIGKILL even if SIGTERM fails
- Improve comments: clarify Wait() idempotency and setupProcessGroup ordering
- Capitalize doc comments per Go conventions

* fix: address codex review findings

Add pre-start context check to both Claude and Codex runners to avoid
spawning processes when the context is already canceled. This addresses
the valid finding that switching from exec.CommandContext to exec.Command
lost the pre-start cancellation behavior.

Note: The "Windows build break risk" finding was rejected as Windows
support is explicitly excluded in CLAUDE.md - the project only targets
Linux/macOS... (continued)

58 of 71 new or added lines in 3 files covered. (81.69%)

3310 of 4206 relevant lines covered (78.7%)

66.69 hits per line