High Priority: Improve process cleanup and server readiness checks in E2eTestRunner

[justin808]

Context

From PR #13 code review - improvements needed to E2eTestRunner for robustness.

High Priority Issues

1. Process Group Cleanup Risk (e2e_test_runner.rb:164-186)

Problem: Process termination logic has potential for orphaned processes when Process.kill raises EPERM.

Current behavior: Silent fallback without logging makes debugging difficult.

Recommendation:

• Add warning logs when fallback to single-process kill occurs
• Log process group ID being terminated for debugging

def send_term_signal
  if @server_pgid
    Process.kill('TERM', -@server_pgid)
  else
    safe_kill_process('TERM', @server_pid)
  end
rescue Errno::ESRCH, Errno::EPERM => e
  puts "Warning: Failed to kill process group #{@server_pgid}: #{e.message}, trying single process"
  safe_kill_process('TERM', @server_pid)
end

2. Hard-coded Port Assumption (e2e_test_runner.rb:147)

Problem: Accepts any 2xx-4xx response as ready, including 404s.

Current code:

def server_responding?
  url = "http://localhost:#{@port}"
  response = Net::HTTP.get_response(URI(url))
  response.code.to_i < 500
rescue Errno::ECONNREFUSED, Errno::EADDRNOTAVAIL, SocketError
  false
end

Recommendation:

• Check for 200 status specifically
• Or verify specific endpoint (e.g., /health or root path)
• Document expected status codes

def server_responding?
  url = "http://localhost:#{@port}"
  response = Net::HTTP.get_response(URI(url))
  # Accept 200-399 (success and redirects), reject 404 and 5xx
  (200..399).cover?(response.code.to_i)
rescue Errno::ECONNREFUSED, Errno::EADDRNOTAVAIL, SocketError
  false
end

Testing

• Update tests to verify warning logging
• Add test cases for 404 response handling
• Verify proper cleanup when EPERM occurs

Acceptance Criteria

• Warning logged when process group kill fails
• Server readiness check rejects 404 responses
• Tests added for both scenarios
• All existing tests still pass