From f938c401d3d1ba8f6298a79a58043edc656ce64d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Thu, 27 Aug 2026 19:55:05 +0200 Subject: [PATCH] fix(tui): autoclose every notice in the strip with severity-based TTLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sticky notices (errors, disconnects, shutdown hints) stored a zero expiry and never armed the expiry timer, so an error like "llm: stream idle for over 1m0s" stayed on screen forever. Nothing in the strip is sticky anymore: info traces keep the 3s TTL, and a new alertTTL (10s) covers errors, warnings, and disconnect notes — long enough to read, bounded like everything else. Durable state lives in the header badge (disconnected / server shut down) and footer retry hints. The noticeSeq/noticeTimer dance is replaced by noticeSweep(), which schedules at the earliest pending expiry and re-arms on every tick until the strip is clean — no per-caller bookkeeping to get wrong. --- README.md | 4 +- internal/tui/coverage_test.go | 25 +++---- internal/tui/events.go | 73 +++++++++++-------- internal/tui/input.go | 9 ++- internal/tui/model.go | 13 ++-- internal/tui/notices_test.go | 123 +++++++++++++++++++++++++++++--- internal/tui/plan.go | 2 +- internal/tui/promptflow_test.go | 17 +++-- internal/tui/reconnect_test.go | 10 ++- internal/tui/shutdown_test.go | 4 +- 10 files changed, 203 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 657d7da..f583f51 100644 --- a/README.md +++ b/README.md @@ -314,7 +314,9 @@ one `Esc`. spinner, smart autoscroll that never yanks you while you read history, and a scroll-position indicator. - **Engine notices** — skill loads, memory merges, and agent signals appear as - quiet status lines. + quiet status lines. Nothing lingers: info traces fade after 3s, and + errors, warnings, and disconnect notes autoclose after 10s (connection + state stays visible in the header badge). - **Version display** — the header shows bodek's own version next to the logo and the spawned odek's version next to the model name. - **Update hint** — at startup, a quiet note appears when a newer bodek release diff --git a/internal/tui/coverage_test.go b/internal/tui/coverage_test.go index b73cd9b..23de53c 100644 --- a/internal/tui/coverage_test.go +++ b/internal/tui/coverage_test.go @@ -145,25 +145,20 @@ func TestAddNoteRingBuffer(t *testing.T) { func TestTransientNoticeExpires(t *testing.T) { m := wired(t) - m.addNote("sticky") + m.addNote("alert tier") m.addTransientNote("skill · loaded") - if cmd := m.noticeTimer(0); cmd == nil { - t.Fatal("transient notice should schedule an expiry timer") + if cmd := m.noticeSweep(); cmd == nil { + t.Fatal("pending notices should arm the expiry sweep") } - // Expiry sweep drops the transient trace but keeps the sticky note. + // A sweep landing at the info TTL drops the transient trace but keeps + // the alert — and re-arms the sweep for the alert's own expiry. m.noticeExp[1] = time.Now().Add(-time.Second) - if _, cmd := m.Update(noticeExpireMsg{seq: m.noticeSeq}); cmd != nil { - t.Error("expiry sweep should not reschedule") + if _, cmd := m.Update(noticeExpireMsg{}); cmd == nil { + t.Error("expiry sweep should reschedule while an alert is pending") } - if got := strings.Join(m.notices, "\n"); got != "sticky" { + if got := strings.Join(m.notices, "\n"); got != "alert tier" { t.Errorf("notices after expiry = %q", got) } - // A stale timer must not clear notices that have not expired yet. - m.addTransientNote("skill · saved") - m.Update(noticeExpireMsg{seq: m.noticeSeq - 1}) - if len(m.notices) != 2 { - t.Errorf("stale timer cleared notices: %v", m.notices) - } } func TestRenderNoticesHidesExpired(t *testing.T) { @@ -244,8 +239,8 @@ func TestSubmitGuards(t *testing.T) { } m.disconn = true m.ta.SetValue("hi") - if cmd := m.submit(); cmd != nil { - t.Error("submit while disconnected should be nil (the warning is sticky, no expiry to arm)") + if cmd := m.submit(); cmd == nil { + t.Error("submit while disconnected should arm the warning's expiry sweep") } if m.ta.Value() != "hi" { t.Error("submit while disconnected must keep the draft") diff --git a/internal/tui/events.go b/internal/tui/events.go index 60802c3..5495d3b 100644 --- a/internal/tui/events.go +++ b/internal/tui/events.go @@ -19,13 +19,18 @@ import ( // subagent) stay on screen before fading out. const noticeTTL = 3 * time.Second -// noticeExpireMsg fires noticeTTL after a transient notice was added. -type noticeExpireMsg struct { - seq int -} +// alertTTL is how long alert-tier notices (errors, warnings, disconnects, +// shutdown / upgrade hints) dwell before fading — longer than the info +// traces so a glance away doesn't miss them, but bounded like everything +// else in the strip. Durable state (disconnected, server shut down) lives +// in the header badge, not here. +const alertTTL = 10 * time.Second + +// noticeExpireMsg fires when the earliest pending notice expiry passes; the +// handler prunes expired notices and re-arms the sweep while any remain. +type noticeExpireMsg struct{} func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { - prevSeq := m.noticeSeq stream := false // high-frequency event: coalesce the render (see queueRender) switch ev.Type { case "session": @@ -296,7 +301,7 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { m.status = "server shut down" m.addNote("server shut down · ⏎ starts a fresh instance") m.refresh() - return m, nil + return m, m.noticeSweep() } // A turn in flight when the socket drops will never finish: close it // out with an interrupted marker instead of leaving it streaming @@ -311,10 +316,9 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { m.status = "reconnecting…" m.addTransientNote("connection lost — reconnecting…") m.refresh() - // The interim note fades via the sweep armed by noticeTimer; the - // reconnect outcome (success or the sticky ⏎-retry hint) replaces - // it within seconds either way. - return m, tea.Batch(cmd, m.noticeTimer(prevSeq)) + // The interim note fades via the sweep; the reconnect outcome + // (success or the ⏎-retry hint) replaces it within seconds. + return m, tea.Batch(cmd, m.noticeSweep()) } m.status = "disconnected" m.addNote("disconnected from odek serve") @@ -322,15 +326,15 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { m.addNote("server log · " + m.opts.LogPath) } m.refresh() - return m, nil + return m, m.noticeSweep() } if stream { - return m, tea.Batch(listen(m.events), m.noticeTimer(prevSeq), m.queueRender()) + return m, tea.Batch(listen(m.events), m.noticeSweep(), m.queueRender()) } m.refresh() // A turn that just ended (done / error) drains the next queued prompt. - return m, tea.Batch(listen(m.events), m.noticeTimer(prevSeq), m.sendQueued(), m.planFollowup()) + return m, tea.Batch(listen(m.events), m.noticeSweep(), m.sendQueued(), m.planFollowup()) } // stepGlyphs returns up to 4 deduped tool glyphs for a turn's steps, in @@ -545,26 +549,24 @@ func (m *Model) closeTurn(msg *message) { msg.rendered = m.render(msg.content) } -// addNote appends a sticky notice (errors, disconnects) that stays until -// pushed out by newer ones. +// addNote appends an alert-tier notice (errors, warnings, disconnects) that +// dwells for alertTTL before fading — long enough to read, bounded like +// every notice in the strip. func (m *Model) addNote(s string) { - m.pushNote(s, time.Time{}) + m.pushNote(s, time.Now().Add(alertTTL)) } // addTransientNote appends an info trace that fades after noticeTTL. func (m *Model) addTransientNote(s string) { m.pushNote(s, time.Now().Add(noticeTTL)) - m.noticeSeq++ } -// transientNoteCmd adds a transient note and returns the cmd that sweeps it -// after noticeTTL. handleEvent arms the sweep itself; every other caller -// (key handlers, async results) must batch this cmd or the note only fades -// on the next unrelated render. +// transientNoteCmd adds a transient note and returns the sweep cmd. Every +// caller outside handleEvent must batch this cmd or the note only fades on +// the next unrelated render. func (m *Model) transientNoteCmd(s string) tea.Cmd { - prev := m.noticeSeq m.addTransientNote(s) - return m.noticeTimer(prev) + return m.noticeSweep() } func (m *Model) pushNote(s string, exp time.Time) { @@ -590,15 +592,26 @@ func (m *Model) pruneNotices(now time.Time) { m.noticeExp = keptExp } -// noticeTimer schedules the expiry sweep when a transient notice was added -// since prevSeq; otherwise it returns nil. -func (m *Model) noticeTimer(prevSeq int) tea.Cmd { - if m.noticeSeq == prevSeq { +// noticeSweep schedules the next expiry sweep at the earliest pending +// notice expiry; nil when the strip has nothing pending. The tick handler +// prunes and re-arms, so expired notes disappear even on an idle TUI and +// the timer chain stops itself once the strip is clean. +func (m *Model) noticeSweep() tea.Cmd { + var earliest time.Time + for _, exp := range m.noticeExp { + if earliest.IsZero() || exp.Before(earliest) { + earliest = exp + } + } + if earliest.IsZero() { return nil } - seq := m.noticeSeq - return tea.Tick(noticeTTL, func(time.Time) tea.Msg { - return noticeExpireMsg{seq: seq} + d := time.Until(earliest) + if d < 0 { + d = 0 + } + return tea.Tick(d, func(time.Time) tea.Msg { + return noticeExpireMsg{} }) } diff --git a/internal/tui/input.go b/internal/tui/input.go index a8d29af..0ad5962 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -109,16 +109,15 @@ func (m *Model) submit() tea.Cmd { } if m.disconn { // Keep the draft — swallowing it silently reads as a lost message. - // Sticky (not transient): the warning must outlive a glance away, so - // it stays until newer notices push it out. Deduped, since every - // enter re-posts it. Retry is ⏎ on an empty input — the hint spells - // that out. + // Alert tier: it dwells past a glance away but still autocloses, + // and is re-posted (deduped) on every enter while disconnected. + // Retry is ⏎ on an empty input — the hint spells that out. const warn = "disconnected — your draft is kept · clear the input, then ⏎ to retry" if n := len(m.notices); n == 0 || m.notices[n-1] != warn { m.addNote(warn) } m.refresh() - return nil + return m.noticeSweep() } if m.busy { // Queue mid-turn prompts instead of dropping them; the queue drains diff --git a/internal/tui/model.go b/internal/tui/model.go index decc2e9..339e47c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -234,8 +234,7 @@ type Model struct { status string notices []string - noticeExp []time.Time // parallel to notices; zero = sticky, else expires at - noticeSeq int // bumped on each transient notice, to invalidate stale timers + noticeExp []time.Time // parallel to notices; when each one fades disconn bool quitting bool @@ -375,7 +374,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.finalize() m.relayout() // the busy status line releases its row m.refresh() - return m, m.sendQueued() + return m, tea.Batch(m.sendQueued(), m.noticeSweep()) case acResultMsg: if msg.seq != m.ac.seq || m.ac.mode != acRef { @@ -478,7 +477,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.addNote("shutdown failed: " + msg.err.Error()) m.refresh() } - return m, nil + return m, m.noticeSweep() case mgmtMsg: m.handleMgmtMsg(msg) @@ -512,7 +511,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.refresh() return m, cmd } - return m, nil + return m, m.noticeSweep() case updateCheckMsg: // Silent on error or when already current: the hint only ever nags @@ -521,7 +520,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.addNote(fmt.Sprintf("⬆ bodek %s available — run `bodek upgrade`", msg.latest)) m.refresh() } - return m, nil + return m, m.noticeSweep() case eventMsg: return m.handleEvent(client.Event(msg)) @@ -532,7 +531,7 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case noticeExpireMsg: m.pruneNotices(time.Now()) m.refresh() - return m, nil + return m, m.noticeSweep() // re-arm while pending notices remain case tea.MouseMsg: if msg.Action == tea.MouseActionPress && msg.Button == tea.MouseButtonLeft && m.panel == panelNone && !m.ac.open { diff --git a/internal/tui/notices_test.go b/internal/tui/notices_test.go index 317a059..aa3fc17 100644 --- a/internal/tui/notices_test.go +++ b/internal/tui/notices_test.go @@ -32,6 +32,108 @@ func assertFadingNotice(t *testing.T, m *Model, cmdReturned bool, substr string) } } +// assertAlertDwell pins the alert-tier contract for one posting path: the +// note exists, it carries an expiry inside (noticeTTL, alertTTL] — errors +// must outlive the 3s info traces, yet nothing in the strip is sticky — the +// path armed the sweep (an un-armed note lingers on an idle TUI), and after +// alertTTL it is gone from the strip. +func assertAlertDwell(t *testing.T, m *Model, cmdArmed bool, substr string) { + t.Helper() + note, exp := lastNoteMatching(m, substr) + if note == "" { + t.Fatalf("notice %q not posted: %v", substr, m.notices) + } + if exp.IsZero() { + t.Fatalf("notice %q is sticky — nothing in the strip may be sticky", note) + } + if dwell := time.Until(exp); dwell <= noticeTTL || dwell > alertTTL { + t.Errorf("notice %q dwell = %v, want (noticeTTL, alertTTL]", note, dwell) + } + if !cmdArmed { + t.Errorf("notice %q posted without arming the sweep", note) + } + m.pruneNotices(time.Now().Add(alertTTL + time.Second)) + if again, _ := lastNoteMatching(m, substr); again != "" { + t.Errorf("notice %q survived alertTTL: %q", substr, again) + } +} + +// TestNoticesAutoclose is the regression for the never-disappearing +// "error: iteration 22: llm: stream idle…" notice: every addNote path — +// errors with and without an open turn, disconnects — posts into the strip +// as an alert that fades after alertTTL. Durable state stays in the header +// badge; the strip holds only bounded messages. +func TestNoticesAutoclose(t *testing.T) { + // Error on a turn that already produced prose → strip note. + m := newTestModel() + m.msgs = append(m.msgs, + message{role: roleUser, content: "go"}, + message{role: roleAsst, content: "partial reply", streaming: true}) + m.curIdx = 1 + _, cmd := m.handleEvent(client.Event{Type: "error", + Message: "iteration 22: llm: stream idle for over 1m0s without an event"}) + assertAlertDwell(t, m, cmd != nil, "error: iteration 22") + + // Error with no open turn → the other addNote path. + m2 := newTestModel() + _, cmd = m2.handleEvent(client.Event{Type: "error", Message: "boom"}) + assertAlertDwell(t, m2, cmd != nil, "error: boom") + + // Disconnect without a reconnect hook → the sticky-family notes. + m3 := newTestModel() + m3.opts.LogPath = "/tmp/bodek.log" + _, cmd = m3.handleEvent(client.Event{Type: client.EventDisconnected}) + if cmd == nil { + t.Error("disconnect notes posted without arming the sweep") + } + for _, substr := range []string{"disconnected from odek serve", "server log · /tmp/bodek.log"} { + note, exp := lastNoteMatching(m3, substr) + if note == "" { + t.Fatalf("notice %q not posted: %v", substr, m3.notices) + } + if dwell := time.Until(exp); dwell <= noticeTTL || dwell > alertTTL { + t.Errorf("notice %q dwell = %v, want (noticeTTL, alertTTL]", note, dwell) + } + } + m3.pruneNotices(time.Now().Add(alertTTL + time.Second)) + if len(m3.notices) != 0 { + t.Errorf("disconnect notes survived alertTTL: %v", m3.notices) + } +} + +// TestNoticeSweepRearms pins the sweep lifecycle: schedule at the earliest +// pending expiry, prune-and-rearm on every tick, stop once the strip is +// clean — so the timer chain cannot pile up or die early. +func TestNoticeSweepRearms(t *testing.T) { + m := newTestModel() + if cmd := m.noticeSweep(); cmd != nil { + t.Error("empty strip must not arm the sweep") + } + m.addTransientNote("info trace") + m.addNote("error: boom") + if cmd := m.noticeSweep(); cmd == nil { + t.Fatal("pending notices must arm the sweep") + } + // A tick landing between the two expiries drops the trace, keeps the + // alert, and re-arms for the remaining expiry. + m.noticeExp[0] = time.Now().Add(-time.Second) + _, cmd := m.Update(noticeExpireMsg{}) + if len(m.notices) != 1 || m.notices[0] != "error: boom" { + t.Fatalf("first sweep dropped the wrong notes: %v", m.notices) + } + if cmd == nil { + t.Fatal("sweep must re-arm while an alert is still pending") + } + // Everything expired: the strip empties and the sweep stops. + m.noticeExp[0] = time.Now().Add(-time.Second) + if _, cmd := m.Update(noticeExpireMsg{}); cmd != nil { + t.Error("clean strip must not re-arm the sweep") + } + if len(m.notices) != 0 { + t.Errorf("expired alert survived the sweep: %v", m.notices) + } +} + func lastNoteMatching(m *Model, substr string) (string, time.Time) { for i := len(m.notices) - 1; i >= 0; i-- { if strings.Contains(m.notices[i], substr) { @@ -94,9 +196,11 @@ func TestStatusConfirmationsFade(t *testing.T) { assertFadingNotice(t, m6, cmd != nil, "reconnected to odek serve") } -// TestActionableNotesStaySticky verifies the flip side: notes the user must -// act on (the ⏎-retry hint after reconnects give up) never fade. -func TestActionableNotesStaySticky(t *testing.T) { +// TestActionableNotesFade verifies the ⏎-retry hint after reconnects give +// up is alert-tier: it dwells past a glance away but autocloses like every +// strip note — the durable affordances are the header's disconnected badge +// and the footer's r-retry hint, not the strip. +func TestActionableNotesFade(t *testing.T) { m := wired(t) m.disconn = true m.Update(reconnectMsg{attempt: maxReconnectAttempts, err: errTest{}}) @@ -104,11 +208,14 @@ func TestActionableNotesStaySticky(t *testing.T) { if note == "" { t.Fatalf("retry hint not posted: %v", m.notices) } - if !exp.IsZero() { - t.Error("the retry hint must stay sticky until acted on") + if exp.IsZero() { + t.Error("the retry hint should carry an expiry — nothing in the strip is sticky") + } + if !m.disconn { + t.Error("the disconnected badge must outlive the fading hint") } - m.pruneNotices(time.Now().Add(time.Hour)) - if note2, _ := lastNoteMatching(m, "⏎ to retry"); note2 == "" { - t.Error("sticky retry hint was pruned") + m.pruneNotices(time.Now().Add(alertTTL + time.Second)) + if note2, _ := lastNoteMatching(m, "⏎ to retry"); note2 != "" { + t.Errorf("retry hint survived alertTTL: %q", note2) } } diff --git a/internal/tui/plan.go b/internal/tui/plan.go index 458965d..63e9266 100644 --- a/internal/tui/plan.go +++ b/internal/tui/plan.go @@ -35,7 +35,7 @@ const ( ) // planDebounceMsg arms the trailing edge of the WS-trigger window; a newer -// trigger supersedes it via sequence compare (noticeTimer pattern). +// trigger supersedes it via sequence compare (the expiry-sweep pattern). type planDebounceMsg struct{ seq int } // planTickMsg re-arms the tab-visible poll (runsTickMsg pattern). diff --git a/internal/tui/promptflow_test.go b/internal/tui/promptflow_test.go index 14ea064..e91ab57 100644 --- a/internal/tui/promptflow_test.go +++ b/internal/tui/promptflow_test.go @@ -4,6 +4,7 @@ import ( "errors" "strings" "testing" + "time" tea "github.com/charmbracelet/bubbletea" @@ -407,10 +408,11 @@ func TestDisconnectedFooterHidesRetryWithDraft(t *testing.T) { } } -// TestDisconnectedSubmitWarningSticky verifies the submit-while-disconnected -// warning is sticky (not a 3s transient), keeps the draft, and does not +// TestDisconnectedSubmitWarningFades verifies the submit-while-disconnected +// warning is alert-tier (it dwells well past a glance away, but still +// autocloses like everything in the strip), keeps the draft, and does not // stack a duplicate on every enter. -func TestDisconnectedSubmitWarningSticky(t *testing.T) { +func TestDisconnectedSubmitWarningFades(t *testing.T) { m := newTestModel() m.disconn = true m.ta.SetValue("hello") @@ -423,8 +425,8 @@ func TestDisconnectedSubmitWarningSticky(t *testing.T) { if !strings.Contains(m.notices[last], "draft is kept") { t.Errorf("warning text = %q", m.notices[last]) } - if !m.noticeExp[last].IsZero() { - t.Error("disconnect warning should be sticky (no expiry)") + if m.noticeExp[last].IsZero() { + t.Error("disconnect warning should carry an expiry (no sticky notes)") } if m.ta.Value() != "hello" { t.Errorf("draft should be kept, got %q", m.ta.Value()) @@ -434,6 +436,11 @@ func TestDisconnectedSubmitWarningSticky(t *testing.T) { if len(m.notices) != last+1 { t.Errorf("duplicate warning posted: %v", m.notices) } + + m.pruneNotices(time.Now().Add(alertTTL + time.Second)) + if len(m.notices) != 0 { + t.Errorf("warning survived alertTTL: %v", m.notices) + } } // TestCancelFeedback verifies the cancel path acknowledges itself: a note diff --git a/internal/tui/reconnect_test.go b/internal/tui/reconnect_test.go index 574ae18..f199647 100644 --- a/internal/tui/reconnect_test.go +++ b/internal/tui/reconnect_test.go @@ -28,14 +28,18 @@ func TestDisconnectSchedulesReconnect(t *testing.T) { } } -// Without a Reconnect hook the disconnect keeps the terminal behavior. +// Without a Reconnect hook the disconnect stays dead — no reconnect is +// scheduled, only the (expiring) notes and their sweep ride along. func TestDisconnectWithoutHookStaysDead(t *testing.T) { m := newTestModel() _, cmd := m.handleEvent(client.Event{Type: client.EventDisconnected}) - if cmd != nil { - t.Error("no reconnect hook: expected no scheduled command") + if cmd == nil { + t.Error("the disconnect notes should arm their expiry sweep") + } + if !m.disconn { + t.Error("input must stay blocked — no reconnect hook means the drop is final") } if m.status != "disconnected" { t.Errorf("status = %q, want disconnected", m.status) diff --git a/internal/tui/shutdown_test.go b/internal/tui/shutdown_test.go index f627ea5..dd70375 100644 --- a/internal/tui/shutdown_test.go +++ b/internal/tui/shutdown_test.go @@ -44,8 +44,8 @@ func TestShutdownTypedConfirmation(t *testing.T) { if m.status != "server shut down" { t.Errorf("status = %q, want the expected-drop state", m.status) } - if note, exp := lastNoteMatching(m, "⏎ starts a fresh instance"); note == "" || !exp.IsZero() { - t.Errorf("fresh-start hint missing or non-sticky: %q", note) + if note, exp := lastNoteMatching(m, "⏎ starts a fresh instance"); note == "" || exp.IsZero() { + t.Errorf("fresh-start hint missing or not expiring: %q", note) } // No reconnect was scheduled: a retry tick would have flipped the status. if strings.HasPrefix(m.status, "reconnect") {