fix: close 8 defects from bug-hunt sweep

internal/client/client.go

@@ -10,8 +10,10 @@ package client
 import (
 	"encoding/json"
 	"fmt"
+	"net"
 	"net/http"
 	"net/url"
+	"strings"
 	"sync"
 	"time"
 
@@ -260,7 +262,7 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) {
 	}
 	cfg.Header.Set("X-Odek-Ws-Token", token)
 
-	conn, err := ws.DialConfig(cfg)
+	conn, err := dialWS(cfg)
 	if err != nil {
 		return nil, fmt.Errorf("ws dial: %w", err)
 	}
@@ -284,6 +286,45 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) {
 	return c, nil
 }
 
+// wsDialTimeout bounds the TCP dial and the WS handshake: ws.DialConfig has
+// no timeout of its own, so a black-holed remote blocked Dial until the OS
+// TCP timeout.
+var wsDialTimeout = 10 * time.Second
+
+// dialWS dials the server under an explicit deadline and runs the WS
+// handshake over the same (deadline-carrying) connection, then clears the
+// deadline so the live stream is unbounded.
+func dialWS(cfg *ws.Config) (*ws.Conn, error) {
+	raw, err := net.DialTimeout("tcp", hostPortAddr(cfg.Location), wsDialTimeout)
+	if err != nil {
+		return nil, err
+	}
+	if err := raw.SetDeadline(time.Now().Add(wsDialTimeout)); err != nil {
+		_ = raw.Close()
+		return nil, err
+	}
+	conn, err := ws.NewClient(cfg, raw)
+	if err != nil {
+		_ = raw.Close()
+		return nil, err
+	}
+	_ = raw.SetDeadline(time.Time{}) // live stream: no deadline
+	return conn, nil
+}
+
+// hostPortAddr extracts host:port from a ws/wss URL, defaulting to the
+// scheme's standard port when absent (odek serve always prints one, but a
+// hand-typed ws://host URL should still dial).
+func hostPortAddr(u *url.URL) string {
+	if u.Host != "" && !strings.Contains(u.Host, ":") {
+		if u.Scheme == "wss" || u.Scheme == "https" {
+			return u.Host + ":443"
+		}
+		return u.Host + ":80"
+	}
+	return u.Host
+}
+
 // Resources queries the server's @-reference completion endpoint.
 func (c *Client) Resources(query string, limit int) ([]Resource, error) {
 	u := fmt.Sprintf("%s/api/resources?q=%s&limit=%d",
@@ -315,7 +356,7 @@ var readIdleTimeout = 45 * time.Second
 
 func (c *Client) readLoop() {
 	defer close(c.Events)
-	defer c.conn.Close() // release the fd even when the sender never closes (reconnect swap)
+	defer func() { _ = c.conn.Close() }() // release the fd even when the sender never closes (reconnect swap)
 	var pending *Event
 	n := 0
 	flush := func() {

internal/client/dial_timeout_test.go

@@ -0,0 +1,47 @@
+package client
+
+import (
+	"net"
+	"testing"
+	"time"
+)
+
+// Regression: Dial had no dial timeout — a black-holed (or silently
+// non-responsive) remote blocked until the OS TCP timeout. The dial (and
+// the WS handshake over the same connection) must be bounded.
+func TestDialIsBoundedAgainstSilentPeer(t *testing.T) {
+	old := wsDialTimeout
+	wsDialTimeout = 200 * time.Millisecond
+	defer func() { wsDialTimeout = old }()
+
+	// A peer that accepts TCP but never reads or writes: the WS handshake
+	// can never complete, so only an explicit deadline can unblock Dial.
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer func() { _ = ln.Close() }()
+	go func() {
+		for {
+			c, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			// Hold the connection open, silently — never respond.
+			time.Sleep(10 * time.Second)
+			_ = c.Close()
+		}
+	}()
+
+	addr := ln.Addr().String()
+	done := make(chan error, 1)
+	go func() {
+		_, err := Dial("ws://"+addr+"/ws", "http://127.0.0.1", "http://"+addr, "tok")
+		done <- err
+	}()
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Dial blocked past the dial timeout against a silent peer")
+	}
+}

internal/server/proc_liveness_test.go

@@ -0,0 +1,57 @@
+//go:build !windows
+
+package server
+
+import (
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+// Regression: procAlive probed with Signal(0), which succeeds on an
+// exited-but-unreaped child — the zombie still counts as alive, so a crashed
+// `odek serve` burned the full ready timeout instead of failing fast.
+func TestProcAliveDetectsExitedUnreapedChild(t *testing.T) {
+	bin, err := exec.LookPath("true")
+	if err != nil {
+		t.Skip("no 'true' binary")
+	}
+	c := &Conn{proc: exec.Command(bin)}
+	if err := c.proc.Start(); err != nil {
+		t.Fatalf("start: %v", err)
+	}
+	c.startReaper()
+
+	deadline := time.Now().Add(5 * time.Second)
+	for time.Now().Before(deadline) {
+		if !c.procAlive() {
+			return
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatal("exited child still reported alive (Signal(0) zombie probe)")
+}
+
+// Regression: waitSpawned's timeout error carried no server output, so a
+// bind failure (port taken, config error) was undiagnosable — the stderr
+// tail must be included in the timeout error.
+func TestWaitSpawnedTimeoutIncludesStderrTail(t *testing.T) {
+	// A port nothing listens on: every probe refuses fast.
+	baseURL := "http://127.0.0.1:1"
+	scan := &tokenScanWriter{w: nilDiscard{}}
+	scan.Write([]byte("listen tcp: bind: address already in use\n"))
+
+	err := waitSpawned(baseURL, scan, func() bool { return true }, 50*time.Millisecond)
+	if err == nil {
+		t.Fatal("waitSpawned unexpectedly succeeded against a dead port")
+	}
+	if !strings.Contains(err.Error(), "bind: address already in use") {
+		t.Fatalf("timeout error missing stderr tail:\n%v", err)
+	}
+}
+
+// nilDiscard is a zero io.Writer (io.Discard import stays out of this file).
+type nilDiscard struct{}
+
+func (nilDiscard) Write(p []byte) (int, error) { return len(p), nil }

internal/server/server.go

@@ -16,6 +16,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"syscall"
 	"time"
 
@@ -61,10 +62,12 @@ type Conn struct {
 	Token   string // per-instance CSRF token
 	Version string // engine version as printed by `<bin> version` (e.g. "v0.2.0"); spawn mode only
 
-	proc    *exec.Cmd        // non-nil when bodek spawned the server
-	scan    *tokenScanWriter // non-nil when bodek spawned the server
-	watch   func()           // cancels the orphan watchdog (nil when none)
-	watchMu sync.Mutex
+	proc     *exec.Cmd        // non-nil when bodek spawned the server
+	scan     *tokenScanWriter // non-nil when bodek spawned the server
+	reaped   atomic.Bool      // the reaper observed the child exit
+	reapDone chan struct{}    // closed when the reaper's Wait returns
+	watch    func()           // cancels the orphan watchdog (nil when none)
+	watchMu  sync.Mutex
 
 	// OnStopEvent, when set, receives shutdown progress from Stop.
 	OnStopEvent func(StopEvent)
@@ -195,6 +198,7 @@ func (c *Conn) spawn(opts Options, addr string) error {
 		return fmt.Errorf("start odek serve: %w", err)
 	}
 	c.proc = cmd
+	c.startReaper()
 	c.startWatchdog()
 	return nil
 }
@@ -273,8 +277,13 @@ func (c *Conn) Stop() {
 		c.OnStopEvent(StopStopping)
 	}
 	c.signalServer(syscall.SIGINT)
-	done := make(chan struct{})
-	go func() { _ = c.proc.Wait(); close(done) }()
+	// The reaper owns Wait (started at spawn); select on its completion
+	// instead of a second Wait, which exec.Cmd forbids.
+	done := c.reapDone
+	if done == nil {
+		done = make(chan struct{})
+		go func() { _ = c.proc.Wait(); close(done) }()
+	}
 	select {
 	case <-done:
 	case <-time.After(stopTimeout):
@@ -306,12 +315,16 @@ func splitTokenURL(raw string) (base, token string) {
 //	  WebSocket: ws://127.0.0.1:8080/ws
 //	  WS token:  <hex>
 type tokenScanWriter struct {
-	w   io.Writer
-	mu  sync.Mutex
-	buf []byte // partial line not yet terminated by '\n'
-	tok string
+	w    io.Writer
+	mu   sync.Mutex
+	buf  []byte // partial line not yet terminated by '\n'
+	tok  string
+	tail []string // last complete lines, bounded, for failure diagnostics
 }
 
+// maxTailLines bounds the stderr tail kept for error reporting.
+const maxTailLines = 4
+
 func (s *tokenScanWriter) Write(p []byte) (int, error) {
 	s.scan(p)
 	return s.w.Write(p)
@@ -324,6 +337,16 @@ func (s *tokenScanWriter) Token() string {
 	return s.tok
 }
 
+// Tail returns the last n complete stderr lines, joined for error text.
+func (s *tokenScanWriter) Tail(n int) string {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if len(s.tail) > n {
+		s.tail = s.tail[len(s.tail)-n:]
+	}
+	return strings.Join(s.tail, "; ")
+}
+
 func (s *tokenScanWriter) scan(p []byte) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -338,6 +361,10 @@ func (s *tokenScanWriter) scan(p []byte) {
 		}
 		line := string(s.buf[:i])
 		s.buf = s.buf[i+1:]
+		s.tail = append(s.tail, line)
+		if len(s.tail) > maxTailLines {
+			s.tail = s.tail[len(s.tail)-maxTailLines:]
+		}
 		if tok := parseTokenLine(line); tok != "" {
 			s.tok = tok
 			return
@@ -384,14 +411,34 @@ func waitReady(baseURL string, timeout time.Duration) error {
 }
 
 // procAlive reports whether the spawned child is still running. Signal(0)
-// probes liveness without waiting: ProcessState stays nil until Wait is
-// called (Stop's job, after Connect returns), so it can never report death
-// here.
+// alone cannot tell an exited-but-unreaped child (zombie) from a live one —
+// the ProcessState stays nil until Wait is reaped — so startReaper reaps in
+// the background and procAlive consults that result first.
 func (c *Conn) procAlive() bool {
+	if c.reaped.Load() {
+		return false
+	}
 	return c.proc != nil && c.proc.Process != nil &&
 		c.proc.Process.Signal(syscall.Signal(0)) == nil
 }
 
+// startReaper waits for the spawned child in the background so its exit is
+// observed immediately (no zombie) and procAlive can report death. The
+// result feeds Stop's Wait — Stop must never Wait the same Cmd twice.
+func (c *Conn) startReaper() {
+	if c.proc == nil {
+		return
+	}
+	proc := c.proc
+	done := make(chan struct{})
+	c.reapDone = done
+	go func() {
+		_ = proc.Wait()
+		c.reaped.Store(true)
+		close(done)
+	}()
+}
+
 // waitSpawned waits until a spawned server answers HTTP or prints its token
 // line. Old odek versions print no token, so readiness alone eventually ends
 // the wait (the legacy token path handles those) — but only after a short
@@ -415,12 +462,30 @@ func waitSpawned(baseURL string, scan *tokenScanWriter, alive func() bool, timeo
 		} else if !readyAt.IsZero() {
 			readyAt = time.Time{} // flapping: restart the grace clock
 		}
+		// Re-check the token before declaring death: a fast-exiting
+		// (or token-print-and-exit) server can flush its banner after
+		// this iteration's top-of-loop check — the token wins.
 		if alive != nil && !alive() {
-			return fmt.Errorf("odek serve exited before becoming ready")
+			if scan == nil || scan.Token() == "" {
+				return fmt.Errorf("odek serve exited before becoming ready%w", stderrTail(scan))
+			}
+			return nil
 		}
 		time.Sleep(150 * time.Millisecond)
 	}
-	return fmt.Errorf("timed out after %s", timeout)
+	return fmt.Errorf("timed out after %s%w", timeout, stderrTail(scan))
+}
+
+// stderrTail returns the captured server stderr tail for inclusion in a
+// waitSpawned error (bind failures, config errors), or nil when empty.
+func stderrTail(scan *tokenScanWriter) error {
+	if scan == nil {
+		return nil
+	}
+	if tail := scan.Tail(maxTailLines); tail != "" {
+		return fmt.Errorf(": %s", tail)
+	}
+	return nil
 }
 
 // spawnedTokenGrace is how long a ready-but-tokenless spawned server is

internal/tui/approval.go

@@ -105,7 +105,7 @@ func (m *Model) handleApprovalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 		return m, m.armConfirm(confirmQuit, "bodek")
 	case "enter":
 		return m, m.submit()
-	case "shift+enter", "alt+enter", "ctrl+j":
+	case "shift+enter", "ctrl+enter", "alt+enter", "ctrl+j":
 		return m, m.insertNewline()
 	}
 	return m, m.updateApprovalComposer(msg)
@@ -329,6 +329,9 @@ func (m *Model) answer(action string) tea.Cmd {
 		dl = m.apprDeadlines[0]
 		m.apprDeadlines = m.apprDeadlines[1:] // keep the parallel expiry queue in lockstep
 	}
+	if len(m.apprBells) > 0 {
+		m.apprBells = m.apprBells[1:]
+	}
 	m.approvals = m.approvals[1:]
 	m.resetApprovalInput()
 	if len(m.approvals) > 0 {