fix: bug-hunt round — lifecycle, client framing, TUI state, input and layout fixes

AGENTS.md

@@ -245,7 +245,8 @@ modifier routes to the composer), and `Alt+A`/`Alt+D`/`Alt+T`
   `KeyRunes`), so every printable — spaces, punctuation, paste — must
   append. Wrap the answer by cell width and paint only a capped tail so
   a long paste cannot push `View` past the terminal; the buffer keeps
-  the full text. Expiry autocloses the card and
+  the full text. A question taller than the card is windowed with a
+  pager row (`Alt+PgUp`/`Alt+PgDn`), never painted in full. Expiry autocloses the card and
   parks `focusIdx` plus the viewport on the latest transcript message
   (a surviving queued successor must not yank scrollback). The unfocused
   queue is a shelf chip; `^Q` unfolds the strip (`qfocus`).

README.md

@@ -614,7 +614,7 @@ can encode them:
 | `d` / `Alt+D` | Deny (plain key: empty composer only) |
 | `t` / `Alt+T` | Trust this risk class until the current connection ends, only when the server offers it and friction is off (plain key: empty composer only) |
 | `Tab` | Expand/collapse command and description details |
-| `Alt+PgUp` / `Alt+PgDn` | Page expanded approval details |
+| `Alt+PgUp` / `Alt+PgDn` | Page expanded approval details, or a clarify question taller than its card |
 | `Esc` | Return from confirmation editing, or fold details; otherwise arm turn cancellation when busy |
 | `^X` | Arm turn cancellation from any state; `y` confirms |
 | `PgUp` / `PgDn` / `^U` / `^D` | Scroll the transcript (approval/clarify cards still own `^U` for paging) |

cmd/bodek/bughunt_regress_test.go

@@ -1,11 +1,16 @@
 package main
 
 import (
+	"errors"
+	"flag"
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
+	tea "github.com/charmbracelet/bubbletea"
+
 	"github.com/BackendStack21/bodek/internal/settings"
 )
 
@@ -60,3 +65,26 @@ func TestRegressServerLogNotReusedWhenPermissive(t *testing.T) {
 		t.Fatalf("token-bearing log is group/world accessible: %v", fi.Mode().Perm())
 	}
 }
+
+// Bubble Tea winning the SIGINT race is still the orderly exit.
+func TestRegressTUIInterruptIsCleanExit(t *testing.T) {
+	if err := tuiExitErr(tea.ErrInterrupted); err != nil {
+		t.Fatalf("interrupt reported as failure: %v", err)
+	}
+	if err := tuiExitErr(errors.New("boom")); err == nil {
+		t.Fatal("real failure swallowed")
+	}
+}
+
+// -h prints usage once and is not an error exit.
+func TestRegressHelpPrintsUsageOnce(t *testing.T) {
+	t.Setenv("BODEK_CONFIG", filepath.Join(t.TempDir(), "config.json"))
+	var out strings.Builder
+	_, err := parseConfig([]string{"-h"}, &out)
+	if !errors.Is(err, flag.ErrHelp) {
+		t.Fatalf("err = %v, want flag.ErrHelp", err)
+	}
+	if n := strings.Count(out.String(), "Usage:"); n != 1 {
+		t.Fatalf("usage printed %d times", n)
+	}
+}

cmd/bodek/diagnose.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -19,6 +20,9 @@ func connectWithDiagnosis(opts server.Options, stderr io.Writer, stdin io.Reader
 		if err == nil {
 			return srv, nil
 		}
+		if errors.Is(err, server.ErrInvalidURL) {
+			return nil, err // a typo, not a startup failure: no card, no retry loop
+		}
 		card := server.Diagnose(err, tui.MissingProvider())
 		if stderr != nil && card != "" {
 			_, _ = fmt.Fprintln(stderr, card)

cmd/bodek/diagnose_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"errors"
 	"io"
 	"strings"
 	"testing"
@@ -36,3 +37,15 @@ func TestConnectWithDiagnosisNonTTY(t *testing.T) {
 		t.Errorf("card missing retry: %q", stderr.String())
 	}
 }
+
+// An invalid --url is a typo no retry fixes: no startup card, no loop.
+func TestConnectWithDiagnosisInvalidURLSkipsRetry(t *testing.T) {
+	var stderr bytes.Buffer
+	_, err := connectWithDiagnosis(server.Options{URL: "ftp://127.0.0.1:1"}, &stderr, bytes.NewReader(nil))
+	if !errors.Is(err, server.ErrInvalidURL) {
+		t.Fatalf("err = %v, want ErrInvalidURL", err)
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("printed a startup card for a bad --url: %q", stderr.String())
+	}
+}

cmd/bodek/main.go

@@ -4,6 +4,7 @@
 package main
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -12,6 +13,7 @@ import (
 	"reflect"
 	"runtime"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 
@@ -158,8 +160,7 @@ func parseConfig(args []string, output io.Writer) (config, error) {
 		_, _ = fmt.Fprintf(fs.Output(), "  bodek -- --prompt-caching                         # pass extra flags to odek serve\n")
 	}
 	if err := fs.Parse(args); err != nil {
-		fs.Usage()
-		return config{}, err
+		return config{}, err // Parse already printed usage
 	}
 	cfg.extraArgs = fs.Args()
 	return cfg, nil
@@ -209,6 +210,9 @@ func run() error {
 	}
 
 	cfg, err := parseConfig(os.Args[1:], os.Stderr)
+	if errors.Is(err, flag.ErrHelp) {
+		return nil
+	}
 	if err != nil {
 		return err
 	}
@@ -350,8 +354,12 @@ func run() error {
 	// 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)
+	stopSignals := setupSignalHandler(p.Quit)
 	final, runErr := p.Run()
+	// From here the terminal is cooked again and Bubble Tea's own handler is
+	// gone: release ours so the first Ctrl+C during the shutdown countdown
+	// aborts it instead of landing in a channel nobody reads.
+	stopSignals()
 	// The alt screen is down: suggest resuming the session that was just
 	// closed, with its exact id (Ctrl+C included — Run returns the same
 	// way). The live model id wins over the store; a crashed TUI still
@@ -363,20 +371,41 @@ func run() error {
 		}
 		printResumeHint(ws, cwd, liveID, os.Stderr)
 	}
-	if runErr != nil {
-		return fmt.Errorf("TUI exited: %w", runErr)
+	return tuiExitErr(runErr)
+}
+
+// tuiExitErr maps Run's result to run's: Bubble Tea's own SIGINT handler
+// races setupSignalHandler's Quit, and either winner is the same orderly
+// exit the user asked for.
+func tuiExitErr(err error) error {
+	if err == nil || errors.Is(err, tea.ErrInterrupted) {
+		return nil
 	}
-	return nil
+	return fmt.Errorf("TUI exited: %w", err)
 }
 
-func setupSignalHandler(p *tea.Program) {
+// setupSignalHandler turns the first SIGINT/SIGTERM into quit and returns a
+// stop func that releases the signals (idempotent).
+func setupSignalHandler(quit func()) (stop func()) {
 	ch := make(chan os.Signal, 1)
+	done := make(chan struct{})
 	signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
 	go func() {
-		<-ch
+		select {
+		case <-ch:
+		case <-done:
+			return
+		}
 		// Restore default handling so a second Ctrl+C / SIGTERM can abort a
 		// slow shutdown instead of being swallowed by this channel.
 		signal.Stop(ch)
-		p.Quit()
+		quit()
 	}()
+	var once sync.Once
+	return func() {
+		once.Do(func() {
+			signal.Stop(ch)
+			close(done)
+		})
+	}
 }

cmd/bodek/signal_unix_test.go

@@ -0,0 +1,43 @@
+//go:build unix
+
+package main
+
+import (
+	"os"
+	"os/signal"
+	"sync/atomic"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// A quit from inside the TUI (no signal yet) must release the handler, or
+// the first Ctrl+C of the shutdown countdown is swallowed.
+func TestRegressSignalHandlerReleasedAfterRun(t *testing.T) {
+	guard := make(chan os.Signal, 4) // keeps SIGTERM from killing the test
+	signal.Notify(guard, syscall.SIGTERM)
+	defer signal.Stop(guard)
+
+	var quits atomic.Int32
+	stop := setupSignalHandler(func() { quits.Add(1) })
+	stop()
+	stop() // idempotent
+	_ = syscall.Kill(os.Getpid(), syscall.SIGTERM)
+	<-guard
+	time.Sleep(50 * time.Millisecond)
+	if n := quits.Load(); n != 0 {
+		t.Fatalf("released handler still quit %d time(s)", n)
+	}
+
+	stop = setupSignalHandler(func() { quits.Add(1) })
+	defer stop()
+	_ = syscall.Kill(os.Getpid(), syscall.SIGTERM)
+	<-guard
+	deadline := time.Now().Add(2 * time.Second)
+	for quits.Load() == 0 && time.Now().Before(deadline) {
+		time.Sleep(10 * time.Millisecond)
+	}
+	if quits.Load() != 1 {
+		t.Fatal("live handler did not quit on SIGTERM")
+	}
+}

internal/client/bughunt_round3_test.go

@@ -0,0 +1,92 @@
+package client
+
+import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	ws "golang.org/x/net/websocket"
+)
+
+// odek's 404 for an unknown session must not read as a missing route:
+// that is permanent and switched the jobs watcher off for the whole run.
+func TestRegressSessionNotFoundIsNotRouteMissing(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "session not found", http.StatusNotFound)
+	}))
+	defer srv.Close()
+	c := testClient(srv)
+	if _, err := c.Jobs("gone", "tok"); !errors.Is(err, ErrSessionNotFound) || errors.Is(err, ErrJobsUnavailable) {
+		t.Errorf("Jobs err = %v, want ErrSessionNotFound", err)
+	}
+	// The plan route stays permanent: odek < v1.27 answers its missing
+	// route with this very body (the id parser swallows "{id}/plan").
+	if _, err := c.SessionPlan("gone", "tok"); !errors.Is(err, ErrPlanUnavailable) {
+		t.Errorf("SessionPlan err = %v, want ErrPlanUnavailable", err)
+	}
+}
+
+// Frames go out unescaped and never past odek's 8 MiB cap, which would
+// make odek drop the connection and lose the prompt silently.
+func TestRegressPromptFrameBounds(t *testing.T) {
+	got := make(chan int, 4)
+	mux := http.NewServeMux()
+	mux.Handle("/ws", ws.Handler(func(c *ws.Conn) {
+		c.MaxPayloadBytes = 32 << 20
+		for {
+			var s string
+			if err := ws.Message.Receive(c, &s); err != nil {
+				return
+			}
+			got <- len(s)
+		}
+	}))
+	cl, _ := newTestServer(t, mux)
+
+	// 3 MiB of markup fits only without HTML escaping (6 bytes per <).
+	html := strings.Repeat("<a>&", 3<<20/4)
+	if err := cl.SendPrompt("review", PromptOpts{Attachments: []Attachment{{Name: "x.html", Content: html}}}); err != nil {
+		t.Fatalf("markup prompt refused: %v", err)
+	}
+	select {
+	case n := <-got:
+		if n > maxFrameBytes {
+			t.Fatalf("frame %d bytes exceeds the cap", n)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("frame never arrived")
+	}
+
+	big := strings.Repeat("x", 9<<20)
+	err := cl.SendPrompt("too big", PromptOpts{Attachments: []Attachment{{Name: "big.txt", Content: big}}})
+	if !errors.Is(err, ErrFrameTooLarge) {
+		t.Fatalf("oversized prompt err = %v, want ErrFrameTooLarge", err)
+	}
+}
+
+// Close must not wait out writeTimeout behind a write stuck on a peer that
+// stopped reading — the TUI calls it on the update goroutine.
+func TestRegressCloseDoesNotWaitOutStuckWrite(t *testing.T) {
+	oldGrace := closeGrace
+	closeGrace = 100 * time.Millisecond
+	t.Cleanup(func() { closeGrace = oldGrace })
+	mux := http.NewServeMux()
+	mux.Handle("/ws", ws.Handler(func(c *ws.Conn) { time.Sleep(20 * time.Second) }))
+	cl, _ := newTestServer(t, mux)
+
+	sent := make(chan struct{})
+	go func() {
+		defer close(sent)
+		_ = cl.SendPrompt("stuck", PromptOpts{Attachments: []Attachment{{Name: "a", Content: strings.Repeat("x", 7<<20)}}})
+	}()
+	time.Sleep(300 * time.Millisecond) // let the write fill the socket buffers
+	start := time.Now()
+	_ = cl.Close()
+	if d := time.Since(start); d > 10*time.Second {
+		t.Fatalf("Close blocked %v behind the stuck write", d)
+	}
+	<-sent
+}

internal/client/client.go

@@ -8,8 +8,10 @@
 package client
 
 import (
+	"bytes"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -625,14 +627,39 @@ func (c *Client) SendSkillPromptResponse(action, skillName string) error {
 // prompt on a slow link.
 var writeTimeout = 60 * time.Second
 
+// maxFrameBytes is odek serve's inbound WebSocket message cap (8 MiB). A
+// larger frame makes odek drop the whole connection — the prompt is lost
+// behind a bare "connection lost" — so send refuses it with a reason.
+const maxFrameBytes = 8 << 20
+
+// ErrFrameTooLarge reports a frame over odek's inbound message cap.
+var ErrFrameTooLarge = errors.New("message exceeds odek serve's 8 MiB frame limit")
+
 // send marshals v and writes it as one frame, serialized against other writers.
 func (c *Client) send(v any) error {
+	// No HTML escaping: < > & would each grow to six bytes, so attached
+	// source files could blow past the frame cap that their size passed.
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false)
+	if err := enc.Encode(v); err != nil {
+		return err
+	}
+	frame := bytes.TrimSuffix(buf.Bytes(), []byte("\n"))
+	if len(frame) > maxFrameBytes {
+		return fmt.Errorf("%w (%.1f MiB encoded) — attach fewer or smaller files", ErrFrameTooLarge, float64(len(frame))/(1<<20))
+	}
 	c.wmu.Lock()
 	defer c.wmu.Unlock()
 	_ = c.conn.SetWriteDeadline(time.Now().Add(writeTimeout))
-	return ws.JSON.Send(c.conn, v)
+	return ws.Message.Send(c.conn, string(frame)) // string → text frame, as JSON.Send
 }
 
+// closeGrace bounds Close's close-frame write. A send stuck on a half-open
+// link holds the write lock until writeTimeout; Close moves that deadline
+// in so it (and the update loop calling it) cannot wedge for up to 60s.
+var closeGrace = time.Second
+
 // noRedirect keeps REST calls on the origin they were addressed to. Go
 // strips Authorization on cross-host redirects but copies custom headers,
 // so following one would hand X-Odek-Ws-Token / X-Session-Token (and a
@@ -652,6 +679,7 @@ func (c *Client) Close() error {
 	if c.conn == nil {
 		return nil
 	}
+	_ = c.conn.SetWriteDeadline(time.Now().Add(closeGrace)) // applies to a pending write too
 	return c.conn.Close()
 }