fix: close 9 defects from adversarial bug-hunt sweep (#125)

- client: close the socket when readLoop exits, so reconnect swaps stop
  leaking fds and server slots; TUI closes the replaced old client
- server: grace-poll the spawned stderr token before the legacy probe
  (HTTP can answer before the token line flushes and 403-fail a server
  bodek itself spawned) and fail fast if the child exits mid-wait
- tokens: serialize persistence under the store mutex (interleaved
  Set/Delete could resurrect deleted tokens on disk) and quarantine a
  corrupt store as sessions.json.corrupt instead of silently wiping it
- settings: staged tmp+rename save so a crash cannot truncate config.json
- workspace: report failed persists on stderr instead of swallowing them
- client: 30s client for bulk reads (session detail, export); export
  errors instead of silently truncating past the 64 MiB bound
- cmd: SIGINT/SIGTERM quits the Bubble Tea program instead of a side-
  goroutine os.Exit that skipped terminal restore and deferred teardown
- tui: regression tests for untrusted-wrapper folding

Author: jkyberneees

cmd/bodek/main.go

@@ -228,9 +228,6 @@ func run() error {
 		cwd = "."
 	}
 
-	// Gracefully shutdown on SIGINT/SIGTERM so the server gets a clean exit.
-	setupSignalHandler(srv, cl)
-
 	model := tui.New(cl, tui.Options{
 		Sandbox:      cfg.sandbox,
 		CWD:          cwd,
@@ -268,20 +265,25 @@ func run() error {
 	tui.RestoreEnhancedKeys()
 	p := tea.NewProgram(model, buildProgramOptions(cfg.plain)...)
 	defer tui.RestoreEnhancedKeys()
+
+	// SIGINT/SIGTERM must not hard-exit from a side goroutine: racing Bubble
+	// Tea's own signal handling leaves the alt screen up and raw mode on
+	// (RestoreEnhancedKeys and the deferred restores never run), corrupting
+	// the terminal. Quit the program instead — Run returns cleanly, so the
+	// deferred srv.Stop() and cl.Close() still execute with terminal state
+	// intact.
+	setupSignalHandler(p)
 	if _, err := p.Run(); err != nil {
 		return fmt.Errorf("TUI exited: %w", err)
 	}
 	return nil
 }
 
-func setupSignalHandler(srv *server.Conn, cl *client.Client) {
+func setupSignalHandler(p *tea.Program) {
 	ch := make(chan os.Signal, 1)
 	signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
 	go func() {
 		<-ch
-		_ = cl.Close()
-		srv.Stop()
-		tui.RestoreEnhancedKeys()
-		os.Exit(0)
+		p.Quit()
 	}()
 }

internal/client/client.go

@@ -245,6 +245,7 @@ type Client struct {
 
 	stopOnce   sync.Once    // lazy: only jobs-stop pays the longer timeout budget
 	stopClient *http.Client // 12s — odek's stop endpoint blocks stopGrace+4s
+	slowHTTP   *http.Client // 30s — bulk payload reads (session detail, export)
 }
 
 // Dial connects to an odek serve WebSocket. wsURL is the ws:// endpoint,
@@ -273,6 +274,11 @@ func Dial(wsURL, origin, baseURL, token string) (*Client, error) {
 		// batch-drains this channel, but a deep buffer keeps readLoop from
 		// blocking (which trips odek's 30s write watchdog and drops the socket).
 		Events: make(chan Event, eventBuffer),
+		// Large transcript payloads (SessionDetail of a long session, full
+		// exports) legitimately take longer than interactive REST calls; a
+		// dedicated 30s client serves them so the 3s interactive budget does
+		// not cut their body reads.
+		slowHTTP: &http.Client{Timeout: 30 * time.Second},
 	}
 	go c.readLoop()
 	return c, nil
@@ -309,6 +315,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)
 	var pending *Event
 	n := 0
 	flush := func() {

internal/client/export_red_test.go

@@ -0,0 +1,23 @@
+package client
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// RED: ExportSession must not silently truncate an export exceeding the
+// in-memory bound — it must error, not return a clipped file.
+func TestExportSessionRejectsOversized(t *testing.T) {
+	big := strings.Repeat("x", maxExportBytes+1)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(big))
+	}))
+	defer srv.Close()
+
+	c := &Client{baseURL: srv.URL, http: srv.Client()}
+	if _, err := c.ExportSession("s1", "", "md"); err == nil {
+		t.Fatal("oversized export silently truncated; want error")
+	}
+}

internal/client/rest.go

@@ -116,15 +116,24 @@ func (c *Client) UpdateSession(id, token string, name *string, pinned *bool) err
 func (c *Client) ExportSession(id, token, format string) ([]byte, error) {
 	u := fmt.Sprintf("%s/api/sessions/%s/export?format=%s",
 		c.baseURL, url.PathEscape(id), url.QueryEscape(format))
-	resp, err := c.do(http.MethodGet, u, token)
+	resp, err := c.doWith(c.slowHTTP, http.MethodGet, u, token)
 	if err != nil {
 		return nil, err
 	}
 	defer func() { _ = resp.Body.Close() }()
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("export: status %s", resp.Status)
 	}
-	return io.ReadAll(io.LimitReader(resp.Body, maxExportBytes))
+	data, err := io.ReadAll(io.LimitReader(resp.Body, maxExportBytes+1))
+	if err != nil {
+		return nil, err
+	}
+	if int64(len(data)) > maxExportBytes {
+		// Read one byte past the bound so an oversized export fails loudly
+		// instead of returning a silently clipped transcript.
+		return nil, fmt.Errorf("export too large (exceeds %d bytes)", maxExportBytes)
+	}
+	return data, nil
 }
 
 // maxExportBytes bounds an exported transcript read into memory.
@@ -157,7 +166,7 @@ func (c *Client) Health() (Health, error) {
 // back to the token passed in.
 func (c *Client) SessionDetail(id, token string) (Session, string, error) {
 	var s Session
-	resp, err := c.do(http.MethodGet, c.baseURL+"/api/sessions/"+url.PathEscape(id), token)
+	resp, err := c.doWith(c.slowHTTP, http.MethodGet, c.baseURL+"/api/sessions/"+url.PathEscape(id), token)
 	if err != nil {
 		return s, "", err
 	}
@@ -327,6 +336,13 @@ func (c *Client) Subagents(runKey string) ([]SubagentEntry, error) {
 }
 
 func (c *Client) do(method, u, sessionToken string) (*http.Response, error) {
+	return c.doWith(c.http, method, u, sessionToken)
+}
+
+func (c *Client) doWith(h *http.Client, method, u, sessionToken string) (*http.Response, error) {
+	if h == nil {
+		h = c.http // zero-value Clients fall back to the interactive budget
+	}
 	req, err := http.NewRequest(method, u, nil)
 	if err != nil {
 		return nil, err
@@ -337,7 +353,7 @@ func (c *Client) do(method, u, sessionToken string) (*http.Response, error) {
 	if sessionToken != "" {
 		req.Header.Set("X-Session-Token", sessionToken)
 	}
-	return c.http.Do(req)
+	return h.Do(req)
 }
 
 // postJSON issues a JSON POST with the standard token headers attached.

internal/server/server.go

@@ -139,7 +139,7 @@ func Connect(opts Options) (*Conn, error) {
 		}
 		// A current odek prints its token to stderr at startup; an older one
 		// prints nothing, so stop waiting as soon as the server answers.
-		if err := waitSpawned(c.BaseURL, c.scan, readyTimeout); err != nil {
+		if err := waitSpawned(c.BaseURL, c.scan, c.procAlive, readyTimeout); err != nil {
 			c.Stop()
 			return nil, fmt.Errorf("odek serve did not become ready: %w", err)
 		}
@@ -383,20 +383,50 @@ func waitReady(baseURL string, timeout time.Duration) error {
 	return fmt.Errorf("timed out after %s", timeout)
 }
 
-// waitSpawned waits until a spawned server prints its token line or answers
-// HTTP, whichever comes first. Old odek versions print no token line, so
-// readiness alone also ends the wait (the legacy token path handles those).
-func waitSpawned(baseURL string, scan *tokenScanWriter, 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.
+func (c *Conn) procAlive() bool {
+	return c.proc != nil && c.proc.Process != nil &&
+		c.proc.Process.Signal(syscall.Signal(0)) == nil
+}
+
+// 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
+// grace poll for the token: the HTTP listener can answer before the stderr
+// token line flushes, and falling to the legacy probe in that window makes
+// /api/models 403 hard-fail a server bodek itself just spawned. A child that
+// dies mid-wait fails immediately instead of burning the full timeout.
+func waitSpawned(baseURL string, scan *tokenScanWriter, alive func() bool, timeout time.Duration) error {
 	deadline := time.Now().Add(timeout)
+	readyAt := time.Time{}
 	for time.Now().Before(deadline) {
-		if scan.Token() != "" || probeReady(baseURL) {
+		if scan.Token() != "" {
 			return nil
 		}
+		if probeReady(baseURL) {
+			if readyAt.IsZero() {
+				readyAt = time.Now()
+			} else if time.Since(readyAt) >= spawnedTokenGrace {
+				return nil // genuinely a tokenless (old) server
+			}
+		} else if !readyAt.IsZero() {
+			readyAt = time.Time{} // flapping: restart the grace clock
+		}
+		if alive != nil && !alive() {
+			return fmt.Errorf("odek serve exited before becoming ready")
+		}
 		time.Sleep(150 * time.Millisecond)
 	}
 	return fmt.Errorf("timed out after %s", timeout)
 }
 
+// spawnedTokenGrace is how long a ready-but-tokenless spawned server is
+// polled for the stderr token line before the legacy path takes over.
+const spawnedTokenGrace = 2 * time.Second
+
 // probeReady reports whether the server root answers without a server error.
 func probeReady(baseURL string) bool {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second)

internal/settings/settings.go

@@ -77,7 +77,31 @@ func Save(s Settings) error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(path, append(data, '\n'), 0o600)
+	// Staged tmp + rename: a crash mid plain WriteFile left a truncated
+	// config.json, which Load reports as malformed — every setting reset.
+	tmp, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".*.tmp")
+	if err != nil {
+		return err
+	}
+	tmpName := tmp.Name()
+	if _, err := tmp.Write(append(data, '\n')); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Close(); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := os.Chmod(tmpName, 0o600); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := os.Rename(tmpName, path); err != nil {
+		_ = os.Remove(tmpName) // don't leave the staged copy behind
+		return err
+	}
+	return nil
 }
 
 // Bool resolves an optional persisted boolean against its built-in default.

internal/tokens/tokens.go

@@ -24,23 +24,38 @@ type Store struct {
 	path string
 }
 
-// Open loads the token store from ~/.bodek/sessions.json. It never fails: on
-// any error it returns an in-memory-only store.
+// Open loads the token store from ~/.bodek/sessions.json. It never fails:
+// any problem yields an in-memory-only store.
 func Open() *Store {
-	s := &Store{m: map[string]string{}}
 	home, err := os.UserHomeDir()
 	if err != nil {
-		return s
+		return &Store{m: map[string]string{}}
 	}
-	s.path = filepath.Join(home, ".bodek", "sessions.json")
-	if data, err := os.ReadFile(s.path); err == nil {
-		_ = json.Unmarshal(data, &s.m)
-		// JSON null unmarshals into a nil map; keep an empty map so Set
-		// cannot panic on assignment. Corrupt JSON still leaves the
-		// in-memory-empty store Open already constructed.
-		if s.m == nil {
-			s.m = map[string]string{}
+	return openAt(filepath.Join(home, ".bodek", "sessions.json"))
+}
+
+// openAt loads a store from an explicit path (test seam). A corrupt store is
+// quarantined as <path>.corrupt instead of being silently overwritten —
+// silently dropping every saved token on one bad write made resume loss
+// undiagnosable.
+func openAt(path string) *Store {
+	s := &Store{m: map[string]string{}, path: path}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return s // missing store: fresh start
+	}
+	if err := json.Unmarshal(data, &s.m); err != nil {
+		if qErr := os.Rename(path, path+".corrupt"); qErr == nil {
+			warnPersist(fmt.Errorf("corrupt store quarantined as %s.corrupt: %w", path, err))
+		} else {
+			warnPersist(fmt.Errorf("corrupt store kept in place: %w", err))
+			s.path = "" // never overwrite bytes we could not parse
 		}
+		s.m = map[string]string{}
+		return s
+	}
+	if s.m == nil { // JSON null unmarshals into a nil map
+		s.m = map[string]string{}
 	}
 	return s
 }
@@ -61,20 +76,12 @@ func (s *Store) Set(id, token string) {
 		return
 	}
 	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.m[id] == token {
-		s.mu.Unlock()
 		return // no change; skip the disk write
 	}
 	s.m[id] = token
-	snapshot := make(map[string]string, len(s.m))
-	for k, v := range s.m {
-		snapshot[k] = v
-	}
-	path := s.path
-	s.mu.Unlock()
-	if err := persist(path, snapshot); err != nil {
-		warnPersist(err)
-	}
+	s.persistLocked()
 }
 
 // Delete removes a session's token and persists the store (best-effort).
@@ -83,18 +90,24 @@ func (s *Store) Delete(id string) {
 		return
 	}
 	s.mu.Lock()
+	defer s.mu.Unlock()
 	if _, ok := s.m[id]; !ok {
-		s.mu.Unlock()
 		return
 	}
 	delete(s.m, id)
+	s.persistLocked()
+}
+
+// persistLocked writes the store while the mutex is held. Snapshot-then-
+// persist-outside-the-lock let interleaved Set/Delete writes reorder on
+// disk: an older snapshot landing last resurrected deleted tokens and
+// dropped minted ones, silently breaking session resume.
+func (s *Store) persistLocked() {
 	snapshot := make(map[string]string, len(s.m))
 	for k, v := range s.m {
 		snapshot[k] = v
 	}
-	path := s.path
-	s.mu.Unlock()
-	if err := persist(path, snapshot); err != nil {
+	if err := persist(s.path, snapshot); err != nil {
 		warnPersist(err)
 	}
 }