fix: session-lifecycle, heartbeat, update-check, and settings bugs

cmd/bodek/main.go

@@ -50,6 +50,24 @@ type config struct {
 	extraArgs []string
 
 	persist settings.Settings // the loaded file, re-saved when /theme switches
+	// persistDisabled is set when the settings file could not be read;
+	// a full-replace Save would then overwrite the (unparsed) file with
+	// a near-empty struct, so all persistence is refused for the run.
+	persistDisabled bool
+}
+
+// save persists the current preference set. When the settings file was
+// unreadable at startup it refuses to write — the user's hand-written
+// file must never be clobbered with a partially-populated struct — and
+// reports the refusal as an error (a silent no-op success would hide
+// that /theme etc. are not being persisted). Startup already printed a
+// one-time warning via parseConfig, so callers surface this at most once
+// per action.
+func (c *config) save() error {
+	if c.persistDisabled {
+		return fmt.Errorf("settings file could not be parsed; changes are not saved this run")
+	}
+	return settings.Save(c.persist)
 }
 
 func parseConfig(args []string, output io.Writer) (config, error) {
@@ -63,6 +81,12 @@ func parseConfig(args []string, output io.Writer) (config, error) {
 			_, _ = fmt.Fprintf(output, "bodek: ignoring settings file: %v\n", err)
 		}
 		st = settings.Settings{}
+		// The file stays untouched on disk; refusing to save protects
+		// every unparsed key from the full-replace write.
+		cfg.persistDisabled = true
+	}
+	if output != nil && cfg.persistDisabled {
+		_, _ = fmt.Fprintln(output, "bodek: settings changes will not be saved this run (file could not be parsed)")
 	}
 	cfg.persist = st
 	fs := flag.NewFlagSet("bodek", flag.ContinueOnError)
@@ -275,15 +299,15 @@ func run() error {
 		ResumeSession: cfg.sessionID,
 		OnThemeChange: func(name string) error {
 			cfg.persist.Theme = name
-			return settings.Save(cfg.persist)
+			return cfg.save()
 		},
 		OnVerbosityChange: func(name string) error {
 			cfg.persist.Verbosity = name
-			return settings.Save(cfg.persist)
+			return cfg.save()
 		},
 		OnThinkingChange: func(level string) error {
 			cfg.persist.Thinking = level
-			return settings.Save(cfg.persist)
+			return cfg.save()
 		},
 		Reconnect: func() (*client.Client, error) {
 			return client.Dial(srv.WSURL, srv.Origin, srv.BaseURL, srv.Token)

cmd/bodek/settings_persist_test.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// A malformed settings file must never be overwritten by a later
+// preference change: persistence is refused for the run instead of
+// saving the zeroed struct as a full-replace write.
+func TestParseConfigBrokenFileRefusesPersistence(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+	broken := "{not json"
+	if err := os.WriteFile(path, []byte(broken), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("BODEK_CONFIG", path)
+
+	var out bytes.Buffer
+	cfg, err := parseConfig(nil, &out)
+	if err != nil {
+		t.Fatalf("parseConfig returned error: %v", err)
+	}
+	if !cfg.persistDisabled {
+		t.Fatal("parseConfig did not disable persistence for a broken settings file")
+	}
+	if warning := out.String(); !strings.Contains(warning, "not be saved") {
+		t.Fatalf("startup warning %q does not mention persistence being disabled", warning)
+	}
+
+	// Simulate a theme change: the save path must leave the broken file
+	// untouched rather than replacing it with a minimal settings file,
+	// and must report the refusal instead of a silent success.
+	cfg.persist.Theme = "dracula"
+	if err := cfg.save(); err == nil {
+		t.Fatal("save on a disabled persistence run must report an error, not succeed silently")
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(data) != broken {
+		t.Fatalf("broken settings file was overwritten: %q", data)
+	}
+
+	// A healthy settings file keeps persistence enabled.
+	ok := filepath.Join(dir, "ok.json")
+	t.Setenv("BODEK_CONFIG", ok)
+	cfg2, err := parseConfig(nil, &bytes.Buffer{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if cfg2.persistDisabled {
+		t.Fatal("persistence disabled for a valid (missing) settings file")
+	}
+	cfg2.persist.Theme = "ember-light"
+	if err := cfg2.save(); err != nil {
+		t.Fatalf("save returned error: %v", err)
+	}
+	if _, err := os.Stat(ok); err != nil {
+		t.Fatalf("expected saved settings file: %v", err)
+	}
+}

internal/server/server_internal_test.go

@@ -179,8 +179,12 @@ func fakeOdekScript(t *testing.T, stderrLines ...string) string {
 func TestConnectSpawnTokenFromStderr(t *testing.T) {
 	// A current odek serve prints its token to stderr; Connect must pick it up
 	// from the "WS token:" line while passing stderr through verbatim.
+	// No ?token= in the banner: the wait must end on the "WS token:" line,
+	// which also guarantees every earlier stderr line has been copied
+	// through before Stop closes the pipe (a token-bearing banner let
+	// Connect return after line 1 and race the copier on lines 2-3).
 	bin := fakeOdekScript(t,
-		"odek serve ⚡  http://127.0.0.1:9999/?token=cafef00d",
+		"odek serve ⚡  http://127.0.0.1:9999",
 		"  WebSocket: ws://127.0.0.1:9999/ws",
 		"  WS token:  cafef00d",
 	)

internal/tui/clipboard.go

@@ -244,8 +244,13 @@ func (m *Model) copySessionID() tea.Cmd {
 	// sessionLive, not just a non-empty id: the connect-time session event
 	// stamps an id before any prompt — that placeholder is never the
 	// operator's session, so the command stays dormant until a prompt (or
-	// a resume) makes the session real.
-	if !m.sessionLive || !validSessionID(m.sessionID) {
+	// a resume) makes the session real. Even then the id is still the
+	// placeholder until the confirming session frame lands: sendPrompt arms
+	// awaitSessionConfirm and any session frame clears it, so the gate holds
+	// exactly for the window between the prompt and its confirmation — and
+	// never blocks a confirmed id again on later prompts (odek does not
+	// re-emit a session frame per turn).
+	if !m.sessionLive || !validSessionID(m.sessionID) || m.awaitSessionConfirm {
 		return m.transientNoteCmd("no session yet — the id exists once a session is created; send a prompt first")
 	}
 	return m.copyText(m.sessionID)

internal/tui/events.go

@@ -88,13 +88,16 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) {
 	case "session":
 		prevSession := m.sessionID
 		m.sessionID = ev.SessionID
+		m.awaitSessionConfirm = false // any session frame confirms the current id
+		if m.suppressRemember {
+			m.suppressRemember = false // /new: the placeholder frame must not re-persist
+		} else if !m.freshStart {
+			m.rememberSession(m.homePrompt)
+		}
 		if ev.AuthToken != "" {
 			m.authToken = ev.AuthToken
 			m.tokens.Set(ev.SessionID, ev.AuthToken)
 		}
-		if !m.freshStart {
-			m.rememberSession(m.homePrompt)
-		}
 		if prevSession != "" && ev.SessionID != prevSession {
 			m.planResetPending = true // switch/attach: drop + refetch at the tail
 			m.resetJobsState()        // jobs are session-scoped; re-baseline on the next snapshot

internal/tui/heartbeat_test.go

@@ -0,0 +1,31 @@
+package tui
+
+import (
+	"testing"
+	"time"
+)
+
+// A heartbeat fired while the connection is down must not send a ping
+// (the dead client would swallow it) and must not stamp the RTT clock —
+// a stamp with no possible pong leaves a bogus latency measurement armed.
+func TestHeartbeatSkipsWhileDisconnected(t *testing.T) {
+	m := newTestModel()
+	m.disconn = true
+	m.handleHeartbeat()
+	if !m.pingSentAt.IsZero() {
+		t.Fatalf("pingSentAt stamped while disconnected: %v", m.pingSentAt)
+	}
+}
+
+// The reconnect swap must drop any outstanding ping stamp from the dead
+// socket, so the first pong on the fresh connection cannot pair with a
+// send that left before the swap.
+func TestReconnectResetsPingSentAt(t *testing.T) {
+	m := wired(t) // live stand-in: m.cl is a real connected client
+	m.disconn = true
+	m.pingSentAt = time.Now()
+	m.handleReconnect(reconnectMsg{attempt: 0, cl: m.cl})
+	if !m.pingSentAt.IsZero() {
+		t.Fatalf("pingSentAt survived the reconnect swap: %v", m.pingSentAt)
+	}
+}

internal/tui/input.go

@@ -515,7 +515,15 @@ func (m *Model) sendPrompt(text string) tea.Cmd {
 	m.ta.Reset()
 	m.closeAC()
 	m.busy = true
-	m.sessionLive = true    // the first prompt creates the session
+	// The id stays a placeholder until a session frame confirms it — but
+	// odek does not re-emit a session frame per turn, so only a
+	// still-unconfirmed id (re)arms the gate; a confirmed one must keep
+	// copying across later prompts. Evaluated before sessionLive flips:
+	// the first prompt finds it false and arms; later prompts of an
+	// already-confirmed session do not.
+	armSessionConfirm := !m.sessionLive || m.awaitSessionConfirm
+	m.sessionLive = true // the first prompt creates the session
+	m.awaitSessionConfirm = armSessionConfirm
 	m.cancelAck = false     // a fresh run's errors are real errors again
 	m.failBellFired = false // a fresh local turn re-arms the failure BEL
 	m.skillSuggest = nil    // the suggestion's window closed with the turn

internal/tui/model.go

@@ -261,14 +261,16 @@ type Model struct {
 	histIdx   int      // index into history while navigating
 	histDraft string   // input stashed while navigating history
 
-	model       string
-	sandbox     bool
-	sessionID   string
-	sessionLive bool   // a prompt created/adopted this session — copy-session-id's gate
-	authToken   string // session-scoped token (for cancel / resume)
-	pendModel   string // model to apply on the next prompt
-	thinking    string // canonical: "" inherit, or disabled|low|medium|high
-	expandAll   bool   // Ctrl+E: render every step's full output/logs
+	model               string
+	sandbox             bool
+	sessionID           string
+	sessionLive         bool   // a prompt created/adopted this session — copy-session-id's gate
+	awaitSessionConfirm bool   // armed per prompt; cleared by the confirming session frame
+	suppressRemember    bool   // one-shot: /new's first post-reconnect session frame must not re-persist
+	authToken           string // session-scoped token (for cancel / resume)
+	pendModel           string // model to apply on the next prompt
+	thinking            string // canonical: "" inherit, or disabled|low|medium|high
+	expandAll           bool   // Ctrl+E: render every step's full output/logs
 
 	odekVersion  string // engine version, shown in the cockpit stats sheet ("" hides it)
 	bodekVersion string // bodek's own version, for the startup update check
@@ -508,6 +510,11 @@ func (m *Model) armHeartbeat() tea.Cmd {
 }
 
 func (m *Model) handleHeartbeat() tea.Cmd {
+	if m.disconn {
+		// The socket is down and reconnect is backing off: pinging the dead
+		// client only stamps an RTT clock no pong can ever answer. Re-arm.
+		return m.armHeartbeat()
+	}
 	m.pingSentAt = time.Now()
 	cl := m.cl
 	if cl == nil {
@@ -1244,12 +1251,14 @@ func (m *Model) startFreshSession() tea.Cmd {
 	homeFetch := m.clearConversation()
 	m.sessionID = ""
 	m.sessionLive = false // /new: no session until the next prompt creates one
+	m.awaitSessionConfirm = false
 	m.authToken = ""
 	m.pendModel = m.model // the new session re-asserts the active model
 	m.resetPlanState()
 	m.resetJobsState()
 	clearHome(m)
 	m.freshStart = true
+	m.suppressRemember = true // the fresh connection's first session frame carries only a placeholder
 	m.pendingResume = ""
 	if m.ws != nil && m.opts.CWD != "" {
 		m.ws.ClearSession(m.opts.CWD)

internal/tui/reconnect.go

@@ -70,6 +70,7 @@ func (m *Model) handleReconnect(msg reconnectMsg) (tea.Model, tea.Cmd) {
 		m.cl = msg.cl
 		m.events = msg.cl.Events
 		m.disconn = false
+		m.pingSentAt = time.Time{} // a pre-swap send must not pair with a post-swap pong
 		m.status = "ready"
 		// Session continuity survives the drop: session_switch adopts the
 		// session on the fresh connection (restoring the server-side memory

internal/tui/session_placeholder_test.go

@@ -0,0 +1,99 @@
+package tui
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/BackendStack21/bodek/internal/client"
+	"github.com/BackendStack21/bodek/internal/workspace"
+)
+
+// Regression tests for the connect-time placeholder session id: between the
+// first prompt and the session frame that confirms the minted id, m.sessionID
+// may still hold the placeholder the connect-time session event stamped —
+// /copy-session-id must never hand it to the clipboard, and /new must not let
+// the fresh connection's first session frame re-persist it.
+
+// BUG 1: a prompt arms sessionLive while the id is still the connect-time
+// placeholder; /copy-session-id must wait for the post-prompt session frame.
+func TestCopySessionIDPlaceholderNotCopiedBetweenPromptAndConfirm(t *testing.T) {
+	m := newTestModel()
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
+	_ = m.sendPrompt("hi") // arms sessionLive; the confirm frame has not arrived
+
+	runCopySessionID(m)
+	if m.copyFlashing() {
+		t.Error("placeholder id copied before the post-prompt session frame confirmed it")
+	}
+	if n := len(m.notices); n == 0 || !strings.Contains(m.notices[n-1], "no session") {
+		t.Errorf("placeholder window must show the no-session note, got %v", m.notices)
+	}
+
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"})
+	runCopySessionID(m)
+	if !m.copyFlashing() {
+		t.Error("confirmed session id must copy after the post-prompt session frame")
+	}
+}
+
+// BUG 3: the old wall-clock gate compared sessionIDAt against
+// lastPromptStart, which sendPrompt re-stamps on every dispatch. odek does
+// not re-emit a session frame per turn, so after the SECOND prompt the
+// confirmed stamp was older than the prompt stamp and /copy-session-id
+// permanently reported "no session yet". The ordering-based gate
+// (awaitSessionConfirm, armed per prompt, cleared by any session frame)
+// must keep the confirmed id copyable across later prompts.
+func TestCopySessionIDStillCopiesAfterSecondPrompt(t *testing.T) {
+	m := newTestModel()
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
+	_ = m.sendPrompt("first")
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"}) // confirms the minted id
+	_ = m.sendPrompt("second")                                           // no further session frame arrives
+
+	runCopySessionID(m)
+	if !m.copyFlashing() {
+		t.Error("the confirmed session id must still copy after a second prompt")
+	}
+}
+
+// Resume path is unaffected: an adopted session is live and copyable without
+// any prompt.
+func TestCopySessionIDResumedSessionCopiesWithoutPrompt(t *testing.T) {
+	m := wired(t)
+	m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "sess-resumed"}})
+	runCopySessionID(m)
+	if !m.copyFlashing() {
+		t.Error("an adopted (resumed) session id must copy without a prompt")
+	}
+}
+
+// BUG 2: the reconnect consumes freshStart before the fresh connection's
+// session frame is ingested, so rememberSession re-persists the placeholder.
+func TestNewReconnectDoesNotRepersistPlaceholder(t *testing.T) {
+	t.Setenv("BODEK_WORKSPACE", filepath.Join(t.TempDir(), "workspaces.json"))
+	m := wired(t)
+	m.ws = workspace.Open()
+	m.opts.CWD = "/tmp/bodek-test-fresh-remember"
+	_ = m.ws.Save(m.opts.CWD, workspace.State{SessionID: "s1"})
+	seedSessionState(m)
+
+	runNew(m)
+	if got := m.ws.Load(m.opts.CWD).SessionID; got != "" {
+		t.Fatalf("/new must clear the mapping, got %q", got)
+	}
+
+	m.disconn = true
+	m.handleReconnect(reconnectMsg{cl: m.cl})
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
+
+	if got := m.ws.Load(m.opts.CWD).SessionID; got != "" {
+		t.Errorf("fresh connection's placeholder frame re-persisted session %q", got)
+	}
+
+	// The suppression is one-shot: the real id from a later frame persists.
+	m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"})
+	if got := m.ws.Load(m.opts.CWD).SessionID; got != "sess-real" {
+		t.Errorf("real session id must persist after the fresh session starts, got %q", got)
+	}
+}