Skip to content

Commit d76ca3e

Browse files
authored
fix: session-lifecycle, heartbeat, update-check, and settings bugs (#142)
* fix(tui): block /copy-session-id placeholder window and /new re-persist - /copy-session-id: ordering-based awaitSessionConfirm gate so the connect-time placeholder id cannot be copied before the post-prompt session frame confirms the real id; resumed sessions keep copying - /new: one-shot suppressRemember stops the reconnect session frame from re-persisting the placeholder id into workspaces.json - heartbeat: skip ping and pingSentAt stamp while disconnected; reset the stamp on reconnect swap so RTT pairs with the live socket - update: Newer parses the latest tag via pseudoBase so prerelease latest releases no longer read as always up-to-date - settings: refuse persistence for the run when the config file fails to parse instead of overwriting it with a near-empty file * fix(server): derace TestConnectSpawnTokenFromStderr banner token The fake banner carried ?token=cafef00d, so waitSpawned returned as soon as line 1 was scanned — Stop() then closed the pipe while lines 2-3 were still in flight, failing the passthrough assertions on fast runners. The banner now omits the token so the wait ends on the WS token line, which also makes the token assertion prove the WS-token path instead of the banner fallback.
1 parent 04f48c9 commit d76ca3e

12 files changed

Lines changed: 283 additions & 21 deletions

‎cmd/bodek/main.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,24 @@ type config struct {
5050
extraArgs []string
5151

5252
persist settings.Settings // the loaded file, re-saved when /theme switches
53+
// persistDisabled is set when the settings file could not be read;
54+
// a full-replace Save would then overwrite the (unparsed) file with
55+
// a near-empty struct, so all persistence is refused for the run.
56+
persistDisabled bool
57+
}
58+
59+
// save persists the current preference set. When the settings file was
60+
// unreadable at startup it refuses to write — the user's hand-written
61+
// file must never be clobbered with a partially-populated struct — and
62+
// reports the refusal as an error (a silent no-op success would hide
63+
// that /theme etc. are not being persisted). Startup already printed a
64+
// one-time warning via parseConfig, so callers surface this at most once
65+
// per action.
66+
func (c *config) save() error {
67+
if c.persistDisabled {
68+
return fmt.Errorf("settings file could not be parsed; changes are not saved this run")
69+
}
70+
return settings.Save(c.persist)
5371
}
5472

5573
func parseConfig(args []string, output io.Writer) (config, error) {
@@ -63,6 +81,12 @@ func parseConfig(args []string, output io.Writer) (config, error) {
6381
_, _ = fmt.Fprintf(output, "bodek: ignoring settings file: %v\n", err)
6482
}
6583
st = settings.Settings{}
84+
// The file stays untouched on disk; refusing to save protects
85+
// every unparsed key from the full-replace write.
86+
cfg.persistDisabled = true
87+
}
88+
if output != nil && cfg.persistDisabled {
89+
_, _ = fmt.Fprintln(output, "bodek: settings changes will not be saved this run (file could not be parsed)")
6690
}
6791
cfg.persist = st
6892
fs := flag.NewFlagSet("bodek", flag.ContinueOnError)
@@ -275,15 +299,15 @@ func run() error {
275299
ResumeSession: cfg.sessionID,
276300
OnThemeChange: func(name string) error {
277301
cfg.persist.Theme = name
278-
return settings.Save(cfg.persist)
302+
return cfg.save()
279303
},
280304
OnVerbosityChange: func(name string) error {
281305
cfg.persist.Verbosity = name
282-
return settings.Save(cfg.persist)
306+
return cfg.save()
283307
},
284308
OnThinkingChange: func(level string) error {
285309
cfg.persist.Thinking = level
286-
return settings.Save(cfg.persist)
310+
return cfg.save()
287311
},
288312
Reconnect: func() (*client.Client, error) {
289313
return client.Dial(srv.WSURL, srv.Origin, srv.BaseURL, srv.Token)

‎cmd/bodek/settings_persist_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
"testing"
9+
)
10+
11+
// A malformed settings file must never be overwritten by a later
12+
// preference change: persistence is refused for the run instead of
13+
// saving the zeroed struct as a full-replace write.
14+
func TestParseConfigBrokenFileRefusesPersistence(t *testing.T) {
15+
dir := t.TempDir()
16+
path := filepath.Join(dir, "config.json")
17+
broken := "{not json"
18+
if err := os.WriteFile(path, []byte(broken), 0o600); err != nil {
19+
t.Fatal(err)
20+
}
21+
t.Setenv("BODEK_CONFIG", path)
22+
23+
var out bytes.Buffer
24+
cfg, err := parseConfig(nil, &out)
25+
if err != nil {
26+
t.Fatalf("parseConfig returned error: %v", err)
27+
}
28+
if !cfg.persistDisabled {
29+
t.Fatal("parseConfig did not disable persistence for a broken settings file")
30+
}
31+
if warning := out.String(); !strings.Contains(warning, "not be saved") {
32+
t.Fatalf("startup warning %q does not mention persistence being disabled", warning)
33+
}
34+
35+
// Simulate a theme change: the save path must leave the broken file
36+
// untouched rather than replacing it with a minimal settings file,
37+
// and must report the refusal instead of a silent success.
38+
cfg.persist.Theme = "dracula"
39+
if err := cfg.save(); err == nil {
40+
t.Fatal("save on a disabled persistence run must report an error, not succeed silently")
41+
}
42+
data, err := os.ReadFile(path)
43+
if err != nil {
44+
t.Fatal(err)
45+
}
46+
if string(data) != broken {
47+
t.Fatalf("broken settings file was overwritten: %q", data)
48+
}
49+
50+
// A healthy settings file keeps persistence enabled.
51+
ok := filepath.Join(dir, "ok.json")
52+
t.Setenv("BODEK_CONFIG", ok)
53+
cfg2, err := parseConfig(nil, &bytes.Buffer{})
54+
if err != nil {
55+
t.Fatal(err)
56+
}
57+
if cfg2.persistDisabled {
58+
t.Fatal("persistence disabled for a valid (missing) settings file")
59+
}
60+
cfg2.persist.Theme = "ember-light"
61+
if err := cfg2.save(); err != nil {
62+
t.Fatalf("save returned error: %v", err)
63+
}
64+
if _, err := os.Stat(ok); err != nil {
65+
t.Fatalf("expected saved settings file: %v", err)
66+
}
67+
}

‎internal/server/server_internal_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,12 @@ func fakeOdekScript(t *testing.T, stderrLines ...string) string {
179179
func TestConnectSpawnTokenFromStderr(t *testing.T) {
180180
// A current odek serve prints its token to stderr; Connect must pick it up
181181
// from the "WS token:" line while passing stderr through verbatim.
182+
// No ?token= in the banner: the wait must end on the "WS token:" line,
183+
// which also guarantees every earlier stderr line has been copied
184+
// through before Stop closes the pipe (a token-bearing banner let
185+
// Connect return after line 1 and race the copier on lines 2-3).
182186
bin := fakeOdekScript(t,
183-
"odek serve ⚡ http://127.0.0.1:9999/?token=cafef00d",
187+
"odek serve ⚡ http://127.0.0.1:9999",
184188
" WebSocket: ws://127.0.0.1:9999/ws",
185189
" WS token: cafef00d",
186190
)

‎internal/tui/clipboard.go‎

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

‎internal/tui/events.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,16 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) {
8888
case "session":
8989
prevSession := m.sessionID
9090
m.sessionID = ev.SessionID
91+
m.awaitSessionConfirm = false // any session frame confirms the current id
92+
if m.suppressRemember {
93+
m.suppressRemember = false // /new: the placeholder frame must not re-persist
94+
} else if !m.freshStart {
95+
m.rememberSession(m.homePrompt)
96+
}
9197
if ev.AuthToken != "" {
9298
m.authToken = ev.AuthToken
9399
m.tokens.Set(ev.SessionID, ev.AuthToken)
94100
}
95-
if !m.freshStart {
96-
m.rememberSession(m.homePrompt)
97-
}
98101
if prevSession != "" && ev.SessionID != prevSession {
99102
m.planResetPending = true // switch/attach: drop + refetch at the tail
100103
m.resetJobsState() // jobs are session-scoped; re-baseline on the next snapshot

‎internal/tui/heartbeat_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package tui
2+
3+
import (
4+
"testing"
5+
"time"
6+
)
7+
8+
// A heartbeat fired while the connection is down must not send a ping
9+
// (the dead client would swallow it) and must not stamp the RTT clock —
10+
// a stamp with no possible pong leaves a bogus latency measurement armed.
11+
func TestHeartbeatSkipsWhileDisconnected(t *testing.T) {
12+
m := newTestModel()
13+
m.disconn = true
14+
m.handleHeartbeat()
15+
if !m.pingSentAt.IsZero() {
16+
t.Fatalf("pingSentAt stamped while disconnected: %v", m.pingSentAt)
17+
}
18+
}
19+
20+
// The reconnect swap must drop any outstanding ping stamp from the dead
21+
// socket, so the first pong on the fresh connection cannot pair with a
22+
// send that left before the swap.
23+
func TestReconnectResetsPingSentAt(t *testing.T) {
24+
m := wired(t) // live stand-in: m.cl is a real connected client
25+
m.disconn = true
26+
m.pingSentAt = time.Now()
27+
m.handleReconnect(reconnectMsg{attempt: 0, cl: m.cl})
28+
if !m.pingSentAt.IsZero() {
29+
t.Fatalf("pingSentAt survived the reconnect swap: %v", m.pingSentAt)
30+
}
31+
}

‎internal/tui/input.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,15 @@ func (m *Model) sendPrompt(text string) tea.Cmd {
515515
m.ta.Reset()
516516
m.closeAC()
517517
m.busy = true
518-
m.sessionLive = true // the first prompt creates the session
518+
// The id stays a placeholder until a session frame confirms it — but
519+
// odek does not re-emit a session frame per turn, so only a
520+
// still-unconfirmed id (re)arms the gate; a confirmed one must keep
521+
// copying across later prompts. Evaluated before sessionLive flips:
522+
// the first prompt finds it false and arms; later prompts of an
523+
// already-confirmed session do not.
524+
armSessionConfirm := !m.sessionLive || m.awaitSessionConfirm
525+
m.sessionLive = true // the first prompt creates the session
526+
m.awaitSessionConfirm = armSessionConfirm
519527
m.cancelAck = false // a fresh run's errors are real errors again
520528
m.failBellFired = false // a fresh local turn re-arms the failure BEL
521529
m.skillSuggest = nil // the suggestion's window closed with the turn

‎internal/tui/model.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,16 @@ type Model struct {
261261
histIdx int // index into history while navigating
262262
histDraft string // input stashed while navigating history
263263

264-
model string
265-
sandbox bool
266-
sessionID string
267-
sessionLive bool // a prompt created/adopted this session — copy-session-id's gate
268-
authToken string // session-scoped token (for cancel / resume)
269-
pendModel string // model to apply on the next prompt
270-
thinking string // canonical: "" inherit, or disabled|low|medium|high
271-
expandAll bool // Ctrl+E: render every step's full output/logs
264+
model string
265+
sandbox bool
266+
sessionID string
267+
sessionLive bool // a prompt created/adopted this session — copy-session-id's gate
268+
awaitSessionConfirm bool // armed per prompt; cleared by the confirming session frame
269+
suppressRemember bool // one-shot: /new's first post-reconnect session frame must not re-persist
270+
authToken string // session-scoped token (for cancel / resume)
271+
pendModel string // model to apply on the next prompt
272+
thinking string // canonical: "" inherit, or disabled|low|medium|high
273+
expandAll bool // Ctrl+E: render every step's full output/logs
272274

273275
odekVersion string // engine version, shown in the cockpit stats sheet ("" hides it)
274276
bodekVersion string // bodek's own version, for the startup update check
@@ -508,6 +510,11 @@ func (m *Model) armHeartbeat() tea.Cmd {
508510
}
509511

510512
func (m *Model) handleHeartbeat() tea.Cmd {
513+
if m.disconn {
514+
// The socket is down and reconnect is backing off: pinging the dead
515+
// client only stamps an RTT clock no pong can ever answer. Re-arm.
516+
return m.armHeartbeat()
517+
}
511518
m.pingSentAt = time.Now()
512519
cl := m.cl
513520
if cl == nil {
@@ -1244,12 +1251,14 @@ func (m *Model) startFreshSession() tea.Cmd {
12441251
homeFetch := m.clearConversation()
12451252
m.sessionID = ""
12461253
m.sessionLive = false // /new: no session until the next prompt creates one
1254+
m.awaitSessionConfirm = false
12471255
m.authToken = ""
12481256
m.pendModel = m.model // the new session re-asserts the active model
12491257
m.resetPlanState()
12501258
m.resetJobsState()
12511259
clearHome(m)
12521260
m.freshStart = true
1261+
m.suppressRemember = true // the fresh connection's first session frame carries only a placeholder
12531262
m.pendingResume = ""
12541263
if m.ws != nil && m.opts.CWD != "" {
12551264
m.ws.ClearSession(m.opts.CWD)

‎internal/tui/reconnect.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func (m *Model) handleReconnect(msg reconnectMsg) (tea.Model, tea.Cmd) {
7070
m.cl = msg.cl
7171
m.events = msg.cl.Events
7272
m.disconn = false
73+
m.pingSentAt = time.Time{} // a pre-swap send must not pair with a post-swap pong
7374
m.status = "ready"
7475
// Session continuity survives the drop: session_switch adopts the
7576
// session on the fresh connection (restoring the server-side memory
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package tui
2+
3+
import (
4+
"path/filepath"
5+
"strings"
6+
"testing"
7+
8+
"github.com/BackendStack21/bodek/internal/client"
9+
"github.com/BackendStack21/bodek/internal/workspace"
10+
)
11+
12+
// Regression tests for the connect-time placeholder session id: between the
13+
// first prompt and the session frame that confirms the minted id, m.sessionID
14+
// may still hold the placeholder the connect-time session event stamped —
15+
// /copy-session-id must never hand it to the clipboard, and /new must not let
16+
// the fresh connection's first session frame re-persist it.
17+
18+
// BUG 1: a prompt arms sessionLive while the id is still the connect-time
19+
// placeholder; /copy-session-id must wait for the post-prompt session frame.
20+
func TestCopySessionIDPlaceholderNotCopiedBetweenPromptAndConfirm(t *testing.T) {
21+
m := newTestModel()
22+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
23+
_ = m.sendPrompt("hi") // arms sessionLive; the confirm frame has not arrived
24+
25+
runCopySessionID(m)
26+
if m.copyFlashing() {
27+
t.Error("placeholder id copied before the post-prompt session frame confirmed it")
28+
}
29+
if n := len(m.notices); n == 0 || !strings.Contains(m.notices[n-1], "no session") {
30+
t.Errorf("placeholder window must show the no-session note, got %v", m.notices)
31+
}
32+
33+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"})
34+
runCopySessionID(m)
35+
if !m.copyFlashing() {
36+
t.Error("confirmed session id must copy after the post-prompt session frame")
37+
}
38+
}
39+
40+
// BUG 3: the old wall-clock gate compared sessionIDAt against
41+
// lastPromptStart, which sendPrompt re-stamps on every dispatch. odek does
42+
// not re-emit a session frame per turn, so after the SECOND prompt the
43+
// confirmed stamp was older than the prompt stamp and /copy-session-id
44+
// permanently reported "no session yet". The ordering-based gate
45+
// (awaitSessionConfirm, armed per prompt, cleared by any session frame)
46+
// must keep the confirmed id copyable across later prompts.
47+
func TestCopySessionIDStillCopiesAfterSecondPrompt(t *testing.T) {
48+
m := newTestModel()
49+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
50+
_ = m.sendPrompt("first")
51+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"}) // confirms the minted id
52+
_ = m.sendPrompt("second") // no further session frame arrives
53+
54+
runCopySessionID(m)
55+
if !m.copyFlashing() {
56+
t.Error("the confirmed session id must still copy after a second prompt")
57+
}
58+
}
59+
60+
// Resume path is unaffected: an adopted session is live and copyable without
61+
// any prompt.
62+
func TestCopySessionIDResumedSessionCopiesWithoutPrompt(t *testing.T) {
63+
m := wired(t)
64+
m.handleSessionDetail(sessionDetailMsg{sess: client.Session{ID: "sess-resumed"}})
65+
runCopySessionID(m)
66+
if !m.copyFlashing() {
67+
t.Error("an adopted (resumed) session id must copy without a prompt")
68+
}
69+
}
70+
71+
// BUG 2: the reconnect consumes freshStart before the fresh connection's
72+
// session frame is ingested, so rememberSession re-persists the placeholder.
73+
func TestNewReconnectDoesNotRepersistPlaceholder(t *testing.T) {
74+
t.Setenv("BODEK_WORKSPACE", filepath.Join(t.TempDir(), "workspaces.json"))
75+
m := wired(t)
76+
m.ws = workspace.Open()
77+
m.opts.CWD = "/tmp/bodek-test-fresh-remember"
78+
_ = m.ws.Save(m.opts.CWD, workspace.State{SessionID: "s1"})
79+
seedSessionState(m)
80+
81+
runNew(m)
82+
if got := m.ws.Load(m.opts.CWD).SessionID; got != "" {
83+
t.Fatalf("/new must clear the mapping, got %q", got)
84+
}
85+
86+
m.disconn = true
87+
m.handleReconnect(reconnectMsg{cl: m.cl})
88+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-placeholder"})
89+
90+
if got := m.ws.Load(m.opts.CWD).SessionID; got != "" {
91+
t.Errorf("fresh connection's placeholder frame re-persisted session %q", got)
92+
}
93+
94+
// The suppression is one-shot: the real id from a later frame persists.
95+
m.handleEvent(client.Event{Type: "session", SessionID: "sess-real"})
96+
if got := m.ws.Load(m.opts.CWD).SessionID; got != "sess-real" {
97+
t.Errorf("real session id must persist after the fresh session starts, got %q", got)
98+
}
99+
}

0 commit comments

Comments
 (0)